morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

[GH-ISSUE #1318] [Bug] extract-artifact can report complete success after an inventoried file disappears #462

New issue

Closed

opened 2026-10-15 18:25:15 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-15 18:25:15 +00:00

Owner

Originally created by @dakjdakd on GitHub (Oct 9, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/1318

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Environment and scope

Area: Evidence, artifacts, or reconstruction.
REA revision: 411498d45f860eba2fbc9d7051af20237b7a4cb1.
Runtime and platform: Windows x64; regression and fix verification used Node 24.18.0 and npm 11.16.0.
Target: synthetic directory-backed .app bundle, with a minimal Mach-O fixture and ordinary resource files.
Interface: CLI extract-artifact; the shared provider also serves MCP extract_artifact.
Earliest failure stage: Artifact or evidence registration, when incomplete materialization is committed as a successful result.

Observed behavior

extract-artifact can return successful Evidence and publish an incomplete output directory when a regular file disappears between inventory and materialization. The file was included in the source inventory, but if the extraction reader never encounters it during its second traversal, there is no error. The successful result then describes the smaller set of files that happened to be copied as the selected set.

This is reproducible with a real directory-backed application bundle through the public CLI. It does not require a corrupt archive, a mocked reader, or a failure while opening a selected entry. The important distinction is that the missing file is absent before the second traversal reaches its directory. Checks applied only to entries that the reader actually yields cannot observe that omission.

The affected code is ArtifactExtraction.ts at revision 411498d4. Both deleting a file and deleting its containing directory can produce the incomplete-success outcome.

Why this matters

Extraction is used to turn an artifact inventory into files that subsequent analysis can consume. A successful extraction therefore needs to establish that every regular occurrence selected from the active artifact was materialized. Individual hashes proving that the copied files are correct do not establish that the output contains every expected file.

In a changing source tree, a packaging job, application updater, or another local process can remove a resource while extraction is running. Those are examples of environments that can produce the verified trigger, rather than additional workflows claimed to have been tested here. The observed consequence is narrower and concrete: incomplete output is published as successful, so callers have no reason to reject it before using it for further analysis or reconstruction.

The successful result also carries this limitation text:

All regular files in the active artifact were materialized; nested archive contents remain represented by their containing file.

That statement is false for the reproduced output. containment_verified: true remains valid as a containment observation, but it cannot substitute for completeness. The original source manifest is retained while selected_occurrence_ids is reduced to the successful subset, making the result harder for a consumer to identify as incomplete.

Reproduction and observed result

The fixture is a synthetic Demo.app containing three regular files:

Demo.app/
  Contents/Info.plist
  Contents/MacOS/demo
  Contents/Resources/z/removed.txt

demo has a minimal Mach-O header and a larger payload so that copying it provides time to remove the later resource. The bundle is only a local parser/extraction fixture; the executable is not launched.

After building the checkout with npm ci and npm run build:cached, the actual public invocation is:

node scripts/rea.mjs extract-artifact /absolute/path/to/Demo.app --json

For the verified run, the CLI child process had TEMP, TMP, and TMPDIR set to the owned fixture directory. A recursive filesystem watcher observed creation of rea-extracted-*/Contents/MacOS/demo and immediately removed Contents/Resources/z/removed.txt from the source. The watcher confirmed that the deletion executed; this was not an unqualified assumption about timing.

On the affected implementation, the command exited 0. The source inventory contained the missing resource, but the published result contained only:

{
  "extracted_paths": [
    "Contents/Info.plist",
    "Contents/MacOS/demo"
  ],
  "selected_occurrence_count": 2,
  "containment_verified": true,
  "cleanup": {
    "attempted": false,
    "verified": true,
    "residual_paths": []
  }
}

This is a projection of the observed successful Evidence, rather than the complete response shape. The missing resource was the third expected regular file.

For reliable regression coverage, the prepared test synchronizes deletion at the existing SafeOutputTree.create boundary: it calls the real implementation, performs the filesystem change after inventory, and lets the real extraction reader continue. That avoids a sleep-based race. It is a production-boundary integration test, with real inventory, file bytes, output writes, error translation, and rollback; the small synchronization hook is explicit.

Expected behavior

If any regular occurrence expected from the active artifact is not materialized, extraction should fail before committing its owned output tree. The failure should retain an integrity reason and name the missing logical paths. Already-written output should follow the normal rollback path rather than remaining available as a successful extraction.

An unchanged input should continue to extract successfully. Distinct paths with identical bytes must each be materialized, even when their inventory nodes share an artifact_id. Nested archive contents should retain their current semantics: for example, an expanded inventory of a directory containing app.asar does not mean the extraction output must also contain app.asar/main.js; copying the containing app.asar is the intended operation.

Cause

materializeSelection creates a lookup from selected logical paths, then iterates the extraction reader. It already rejects a newly encountered regular file that is missing from the inventory, and SafeOutputTree.write verifies the digest of each copied entry.

The reverse check is absent. There is no final comparison between the expected selection and the entries successfully written. Instead, the loop accumulates materialized, and createExtractionResult receives that smaller array. Once iteration finishes normally, output.commit() runs even if the original lookup still includes paths that were never encountered.

The bug therefore comes from treating a completed iterator as proof that the original selection was exhausted. An iterator can finish normally after a directory or file has disappeared.

Proposed fix and scope

A focused fix is prepared: use the existing logical-path lookup as the set of remaining expected entries, remove a path only after its write and digest verification succeed, and fail with a typed integrity error if any paths remain after iteration. Successful results should use the expected selection rather than reconstructing intent from the copied subset.

Before that check, the expected selection needs to exclude expanded members already represented by a containing regular archive file. The prepared change handles that explicitly and preserves the existing validation of selected occurrences. No new CLI option, tool schema, archive expansion policy, or partial-success contract is needed.

This is separate from errors opening an entry that was already yielded and from the nested-integrity reporting work in #1083. The gap here is the missing expected occurrence that never reaches the extraction loop at all.

Proposed implementation

The focused fix and its regression coverage are available in https://github.com/morluto/rea/pull/1319. That PR addresses this issue on its own branch.

Originally created by @dakjdakd on GitHub (Oct 9, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/1318 ## Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ## Environment and scope - **Area:** Evidence, artifacts, or reconstruction. - **REA revision:** `411498d45f860eba2fbc9d7051af20237b7a4cb1`. - **Runtime and platform:** Windows x64; regression and fix verification used Node `24.18.0` and npm `11.16.0`. - **Target:** synthetic directory-backed `.app` bundle, with a minimal Mach-O fixture and ordinary resource files. - **Interface:** CLI `extract-artifact`; the shared provider also serves MCP `extract_artifact`. - **Earliest failure stage:** Artifact or evidence registration, when incomplete materialization is committed as a successful result. ## Observed behavior `extract-artifact` can return successful Evidence and publish an incomplete output directory when a regular file disappears between inventory and materialization. The file was included in the source inventory, but if the extraction reader never encounters it during its second traversal, there is no error. The successful result then describes the smaller set of files that happened to be copied as the selected set. This is reproducible with a real directory-backed application bundle through the public CLI. It does not require a corrupt archive, a mocked reader, or a failure while opening a selected entry. The important distinction is that the missing file is absent **before the second traversal reaches its directory**. Checks applied only to entries that the reader actually yields cannot observe that omission. The affected code is [ArtifactExtraction.ts at revision 411498d4](https://github.com/morluto/rea/blob/411498d45f860eba2fbc9d7051af20237b7a4cb1/src/artifacts/extraction/ArtifactExtraction.ts). Both deleting a file and deleting its containing directory can produce the incomplete-success outcome. ## Why this matters Extraction is used to turn an artifact inventory into files that subsequent analysis can consume. A successful extraction therefore needs to establish that every regular occurrence selected from the active artifact was materialized. Individual hashes proving that the copied files are correct do not establish that the output contains every expected file. In a changing source tree, a packaging job, application updater, or another local process can remove a resource while extraction is running. Those are examples of environments that can produce the verified trigger, rather than additional workflows claimed to have been tested here. The observed consequence is narrower and concrete: incomplete output is published as successful, so callers have no reason to reject it before using it for further analysis or reconstruction. The successful result also carries this limitation text: > All regular files in the active artifact were materialized; nested archive contents remain represented by their containing file. That statement is false for the reproduced output. `containment_verified: true` remains valid as a containment observation, but it cannot substitute for completeness. The original source manifest is retained while `selected_occurrence_ids` is reduced to the successful subset, making the result harder for a consumer to identify as incomplete. ## Reproduction and observed result The fixture is a synthetic `Demo.app` containing three regular files: ```text Demo.app/ Contents/Info.plist Contents/MacOS/demo Contents/Resources/z/removed.txt ``` `demo` has a minimal Mach-O header and a larger payload so that copying it provides time to remove the later resource. The bundle is only a local parser/extraction fixture; the executable is not launched. After building the checkout with `npm ci` and `npm run build:cached`, the actual public invocation is: ```sh node scripts/rea.mjs extract-artifact /absolute/path/to/Demo.app --json ``` For the verified run, the CLI child process had `TEMP`, `TMP`, and `TMPDIR` set to the owned fixture directory. A recursive filesystem watcher observed creation of `rea-extracted-*/Contents/MacOS/demo` and immediately removed `Contents/Resources/z/removed.txt` from the source. The watcher confirmed that the deletion executed; this was not an unqualified assumption about timing. On the affected implementation, the command exited `0`. The source inventory contained the missing resource, but the published result contained only: ```json { "extracted_paths": [ "Contents/Info.plist", "Contents/MacOS/demo" ], "selected_occurrence_count": 2, "containment_verified": true, "cleanup": { "attempted": false, "verified": true, "residual_paths": [] } } ``` This is a projection of the observed successful Evidence, rather than the complete response shape. The missing resource was the third expected regular file. For reliable regression coverage, the prepared test synchronizes deletion at the existing `SafeOutputTree.create` boundary: it calls the real implementation, performs the filesystem change after inventory, and lets the real extraction reader continue. That avoids a sleep-based race. It is a production-boundary integration test, with real inventory, file bytes, output writes, error translation, and rollback; the small synchronization hook is explicit. ## Expected behavior If any regular occurrence expected from the active artifact is not materialized, extraction should fail before committing its owned output tree. The failure should retain an integrity reason and name the missing logical paths. Already-written output should follow the normal rollback path rather than remaining available as a successful extraction. An unchanged input should continue to extract successfully. Distinct paths with identical bytes must each be materialized, even when their inventory nodes share an `artifact_id`. Nested archive contents should retain their current semantics: for example, an expanded inventory of a directory containing `app.asar` does not mean the extraction output must also contain `app.asar/main.js`; copying the containing `app.asar` is the intended operation. ## Cause `materializeSelection` creates a lookup from selected logical paths, then iterates the extraction reader. It already rejects a newly encountered regular file that is missing from the inventory, and `SafeOutputTree.write` verifies the digest of each copied entry. The reverse check is absent. There is no final comparison between the expected selection and the entries successfully written. Instead, the loop accumulates `materialized`, and `createExtractionResult` receives that smaller array. Once iteration finishes normally, `output.commit()` runs even if the original lookup still includes paths that were never encountered. The bug therefore comes from treating a completed iterator as proof that the original selection was exhausted. An iterator can finish normally after a directory or file has disappeared. ## Proposed fix and scope A focused fix is prepared: use the existing logical-path lookup as the set of remaining expected entries, remove a path only after its write and digest verification succeed, and fail with a typed integrity error if any paths remain after iteration. Successful results should use the expected selection rather than reconstructing intent from the copied subset. Before that check, the expected selection needs to exclude expanded members already represented by a containing regular archive file. The prepared change handles that explicitly and preserves the existing validation of selected occurrences. No new CLI option, tool schema, archive expansion policy, or partial-success contract is needed. This is separate from errors opening an entry that was already yielded and from the nested-integrity reporting work in [#1083](https://github.com/morluto/rea/pull/1083). The gap here is the missing expected occurrence that never reaches the extraction loop at all. ## Proposed implementation The focused fix and its regression coverage are available in https://github.com/morluto/rea/pull/1319. That PR addresses this issue on its own branch.

bsakaguchi

2026-10-15 18:25:15 +00:00

closed this issue
added the
area: cli

P1: high

bug
labels

bsakaguchi referenced this issue

2026-10-16 21:13:19 +00:00

[PR #462] [MERGED] fix(ci): guard Release Please PR branch parsing #2009

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#462

No description provided.

Rows
Columns