morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #3935] [Bug] Provider CLI cleanup failures discard completed results and primary diagnostics #1544

New issue

Closed

opened 2026-10-16 21:06:55 +00:00 by bsakaguchi · 5 comments

bsakaguchi commented

2026-10-16 21:06:55 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 15, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/3935

Before submitting

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

Area

CLI

Observed behavior

decode-android-resources and inspect-wasm-artifact discard completed analysis Evidence or original failure diagnostics when workspace cleanup fails and the provider close retry also fails. The executable CLI returns a generic UNKNOWN cleanup response. The WABT primary service error already contains completed Evidence, which the final CLI response loses. These observations used fixture launchers through unmodified production adapters and real filesystem permission failures, not real Apktool or WABT engines.

Reproduction

Use a non-root POSIX account and the built checkout. These are the executed fixture protocols, not real format-validation claims. Give every subprocess an isolated home and temporary directory inside a disposable fixture directory. Set case_root to that directory's absolute path. Restore mode 0700 on retained workspace directories after recording each result, then remove the disposable fixtures.

WABT executable CLI reproduction

Create home and tools directories under the fixture directory. Create selected.wasm containing exactly 76 bytes with value 0x01. Read the complete repository fixture tests/fixtures/wasm/objdump.txt as the objdump response.
Create executable shell launchers named wasm-validate, wasm-objdump, and wasm2wat under tools. Every launcher must answer --version with 1.0.42 and a newline, then exit 0. For other calls, the validator exits 0, objdump prints the exact fixture bytes, and wasm2wat prints (module (@custom "test" "payload")) followed by a newline and exits 0.
Add two environment-controlled modes. If FAIL_VALIDATE=1, the validator prints module.wasm: primary-validation-failure-marker and a newline to stderr and exits 1. If BLOCK_CLEANUP=1, wasm2wat changes the mode of its current working directory to 0500 before returning its normal response. In validation-failure mode, the validator makes that same mode change before exiting instead. These tool children run in REA's disposable private runtime root. Do not change permissions on any other directory.
Run the following command for all four FAIL_VALIDATE and BLOCK_CLEANUP combinations, 0/0, 0/1, 1/0, and 1/1, with a fresh runtime directory for each run:

HOME="$case_root/home" TMPDIR="$case_root/runtime" \
REA_WABT_BIN_DIRECTORY="$case_root/tools" FAIL_VALIDATE=0 BLOCK_CLEANUP=1 \
node scripts/rea.mjs inspect-wasm-artifact "$case_root/selected.wasm" --format json

Record stdout, stderr, exit status, and retained workspace paths before restoring permissions. The fake producer permits the provider to complete its documented parsing and identity checks. Mode 0500 permits reading the retained snapshot but prevents deletion of its children, so both real cleanup attempts fail.

Apktool executable CLI reproduction

This protocol was executed once for every mode combination.

Create home and runtime directories under a disposable fixture directory. Create fixture.apk containing exactly apk-bytes-for-apktool without a newline. Create an executable shell launcher named apktool there.
The launcher answers --version with 2.7.0-dirty and a newline, then exits 0. For decode, the first argument is d and the fifth argument is the existing output directory. Normally write apktool.yml there with a versionInfo mapping containing versionCode: '7' and versionName: 1.2.3. Write AndroidManifest.xml containing <manifest package="org.example.completed"><application/></manifest> and res/values/strings.xml containing <resources><string name="marker">completed-observation-marker</string></resources>. End each document with a newline. Print I: completed decode and a newline, then exit 0.
When FAIL_DECODE=1, skip document creation, print Exception in thread "main" brut.androlib.AndrolibException: primary-decode-failure-marker and a newline to stderr, and exit 1. When BLOCK_CLEANUP=1, change the mode of the parent of the fifth argument to 0500 before either decode exit. This is the provider-owned workspace root inside the isolated temporary directory.
Run all four FAIL_DECODE and BLOCK_CLEANUP combinations, 0/0, 0/1, 1/0, and 1/1, with fresh home and runtime directories. Record stdout, stderr and exit status, then restore permissions:

HOME="$case_root/home" TMPDIR="$case_root/runtime" \
REA_APKTOOL_COMMAND="$case_root/apktool" FAIL_DECODE=0 BLOCK_CLEANUP=1 \
node scripts/rea.mjs decode-android-resources "$case_root/fixture.apk" --format json

Other handler groups listed below are source-derived, rather than runtime reproductions.

Expected behavior

Cleanup failure must preserve completed observations, available partial facts, and meaningful primary failure reasons. The CLI should report structured cleanup failure with retained resources while keeping the completed Evidence or original diagnostics. An initial cleanup error that already carries an observation must retain that observation when close fails again. Existing one-shot command finalization implements this preservation contract.

docs/tool-design.md:81 requires observations to survive cleanup failure. src/cli/finishAnalysisCommand.ts:13,57-65 already preserves successful output as partial observation or combines primary and cleanup failures.

Impact and scope

Repeated secondary cleanup failure removes useful analysis results and recovery diagnostics. The trigger is bounded to failed workspace cleanup; normal controls succeed. No caller input data loss was observed. Restoring workspace permissions and rerunning may produce a new result, but recovery of the discarded original result is unverified.

Observed handler groups: Apktool and WASM. Source-proved related groups: firmware, web module trace, web source location, EVM and JavaScript recovery. ADB, Android, JEB and Flutter handlers have the same result-replacing close branch by inspection, but their production cleanup triggers were not independently established.

REA version

rea 6.5.0 at source commit 4692bb3330; freshness checked by source at origin/main 22ec8bdb2f

Runtime and platform

Node 24.18.0; macOS arm64; fake Apktool launcher version 2.7.0-dirty and WABT fixture launcher versions 1.0.42.

Target and artifact context

Disposable synthetic APK and WASM inputs. Only provider-owned disposable workspace permissions were changed, then restored.

Interface

CLI

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

The WABT four-case matrix ran twice with fresh workspaces per invocation:

Primary operation	Cleanup	Result
succeeds	permitted	Exit 0; complete Evidence, artifact identity and WAT; no retained root
succeeds	blocked	Exit 1; only `{"code":"UNKNOWN","message":"Provider wabt-artifact adapter failed during inspect_wasm_artifact"}`; root retained
validation fails	permitted	Exit 1; `invalid_request`, primary marker, captured stderr and partial Evidence; no retained root
validation fails	blocked	Exit 1; same UNKNOWN response; primary diagnostics and partial Evidence absent; root retained

All eight WABT CLI stderr streams were empty. The four Apktool cases similarly preserved results with permitted cleanup and returned only {"code":"UNKNOWN","message":"Apktool workspace cleanup failed"} with blocked cleanup. The failed-decode control otherwise returned unsupported_target and the original stderr marker.

A full source CLI test observed WABT's primary cleanup_incomplete result containing details.partial_observation, two failed cleanup attempts, and the final UNKNOWN response without that observation. Passing the same primary result and rejecting close to finishAnalysisCommand retained the observation, selected artifact identity, primary projection and cleanup projection.

Baseline locators: src/cli/apktoolCommands.ts:30-38, src/apktool/ApktoolProvider.ts:497-525, src/cli/wasmCommands.ts:58-81, and src/wasm/WabtArtifactProvider.ts:56,445. Registrations are at src/cli.ts:104,114.

Current main still has the identical Apktool CLI result-replacing branch and workspace-cleanup AggregateError. Commit 089ef732e changes version-probe/process ownership cleanup, not workspace result preservation. WABT and the other cited CLI handlers remain unchanged.

Typed outcome or recovery expected

Keep cleanup as a failure with retained-resource paths, while preserving completed Evidence, original errors and captured stderr. Cover success plus close failure, primary failure plus close failure, and an initial cleanup error carrying partial Evidence plus a second close failure. Preserve permitted-cleanup and signal controls.

Analysis or suspected cause

Source-derived: Apktool returns the saved operation result only after close, so a close rejection replaces it. WASM returns its primary projection inside a try whose finally awaits close, which likewise replaces the return. The same finalization contract applies across the related handler groups. finishAnalysisCommand demonstrates an existing preservation policy.

Additional context

Related closed #1501 addressed firmware provider-level retention; the remaining CLI close retry can still replace that retained outcome. Open PR #3492 modifies web module and source-location cleanup/finalization only. It does not address the reproduced Apktool/WABT commands or the other lifecycle groups. PR #2406 adds component decoding and does not restore this repeated WABT workspace-cleanup case.

Originally created by @coygeek on GitHub (Oct 15, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/3935 ### 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. ### Area CLI ### Observed behavior `decode-android-resources` and `inspect-wasm-artifact` discard completed analysis Evidence or original failure diagnostics when workspace cleanup fails and the provider close retry also fails. The executable CLI returns a generic `UNKNOWN` cleanup response. The WABT primary service error already contains completed Evidence, which the final CLI response loses. These observations used fixture launchers through unmodified production adapters and real filesystem permission failures, not real Apktool or WABT engines. ### Reproduction Use a non-root POSIX account and the built checkout. These are the executed fixture protocols, not real format-validation claims. Give every subprocess an isolated home and temporary directory inside a disposable fixture directory. Set `case_root` to that directory's absolute path. Restore mode `0700` on retained workspace directories after recording each result, then remove the disposable fixtures. **WABT executable CLI reproduction** 1. Create `home` and `tools` directories under the fixture directory. Create `selected.wasm` containing exactly 76 bytes with value `0x01`. Read the complete repository fixture `tests/fixtures/wasm/objdump.txt` as the objdump response. 2. Create executable shell launchers named `wasm-validate`, `wasm-objdump`, and `wasm2wat` under `tools`. Every launcher must answer `--version` with `1.0.42` and a newline, then exit 0. For other calls, the validator exits 0, objdump prints the exact fixture bytes, and wasm2wat prints `(module (@custom "test" "payload"))` followed by a newline and exits 0. 3. Add two environment-controlled modes. If `FAIL_VALIDATE=1`, the validator prints `module.wasm: primary-validation-failure-marker` and a newline to stderr and exits 1. If `BLOCK_CLEANUP=1`, wasm2wat changes the mode of its current working directory to `0500` before returning its normal response. In validation-failure mode, the validator makes that same mode change before exiting instead. These tool children run in REA's disposable private runtime root. Do not change permissions on any other directory. 4. Run the following command for all four `FAIL_VALIDATE` and `BLOCK_CLEANUP` combinations, `0/0`, `0/1`, `1/0`, and `1/1`, with a fresh `runtime` directory for each run: ```sh HOME="$case_root/home" TMPDIR="$case_root/runtime" \ REA_WABT_BIN_DIRECTORY="$case_root/tools" FAIL_VALIDATE=0 BLOCK_CLEANUP=1 \ node scripts/rea.mjs inspect-wasm-artifact "$case_root/selected.wasm" --format json ``` 5. Record stdout, stderr, exit status, and retained workspace paths before restoring permissions. The fake producer permits the provider to complete its documented parsing and identity checks. Mode `0500` permits reading the retained snapshot but prevents deletion of its children, so both real cleanup attempts fail. **Apktool executable CLI reproduction** This protocol was executed once for every mode combination. 1. Create `home` and `runtime` directories under a disposable fixture directory. Create `fixture.apk` containing exactly `apk-bytes-for-apktool` without a newline. Create an executable shell launcher named `apktool` there. 2. The launcher answers `--version` with `2.7.0-dirty` and a newline, then exits 0. For decode, the first argument is `d` and the fifth argument is the existing output directory. Normally write `apktool.yml` there with a `versionInfo` mapping containing `versionCode: '7'` and `versionName: 1.2.3`. Write `AndroidManifest.xml` containing `<manifest package="org.example.completed"><application/></manifest>` and `res/values/strings.xml` containing `<resources><string name="marker">completed-observation-marker</string></resources>`. End each document with a newline. Print `I: completed decode` and a newline, then exit 0. 3. When `FAIL_DECODE=1`, skip document creation, print `Exception in thread "main" brut.androlib.AndrolibException: primary-decode-failure-marker` and a newline to stderr, and exit 1. When `BLOCK_CLEANUP=1`, change the mode of the parent of the fifth argument to `0500` before either decode exit. This is the provider-owned workspace root inside the isolated temporary directory. 4. Run all four `FAIL_DECODE` and `BLOCK_CLEANUP` combinations, `0/0`, `0/1`, `1/0`, and `1/1`, with fresh home and runtime directories. Record stdout, stderr and exit status, then restore permissions: ```sh HOME="$case_root/home" TMPDIR="$case_root/runtime" \ REA_APKTOOL_COMMAND="$case_root/apktool" FAIL_DECODE=0 BLOCK_CLEANUP=1 \ node scripts/rea.mjs decode-android-resources "$case_root/fixture.apk" --format json ``` Other handler groups listed below are source-derived, rather than runtime reproductions. ### Expected behavior Cleanup failure must preserve completed observations, available partial facts, and meaningful primary failure reasons. The CLI should report structured cleanup failure with retained resources while keeping the completed Evidence or original diagnostics. An initial cleanup error that already carries an observation must retain that observation when close fails again. Existing one-shot command finalization implements this preservation contract. `docs/tool-design.md:81` requires observations to survive cleanup failure. `src/cli/finishAnalysisCommand.ts:13,57-65` already preserves successful output as partial observation or combines primary and cleanup failures. ### Impact and scope Repeated secondary cleanup failure removes useful analysis results and recovery diagnostics. The trigger is bounded to failed workspace cleanup; normal controls succeed. No caller input data loss was observed. Restoring workspace permissions and rerunning may produce a new result, but recovery of the discarded original result is unverified. Observed handler groups: Apktool and WASM. Source-proved related groups: firmware, web module trace, web source location, EVM and JavaScript recovery. ADB, Android, JEB and Flutter handlers have the same result-replacing close branch by inspection, but their production cleanup triggers were not independently established. ### REA version rea 6.5.0 at source commit 4692bb33300c7cb77b095b6cc0c33da861c1b860; freshness checked by source at origin/main 22ec8bdb2f7d4626587f23b1eed4a207df600386 ### Runtime and platform Node 24.18.0; macOS arm64; fake Apktool launcher version 2.7.0-dirty and WABT fixture launcher versions 1.0.42. ### Target and artifact context Disposable synthetic APK and WASM inputs. Only provider-owned disposable workspace permissions were changed, then restored. ### Interface CLI ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence The WABT four-case matrix ran twice with fresh workspaces per invocation: | Primary operation | Cleanup | Result | | --- | --- | --- | | succeeds | permitted | Exit 0; complete Evidence, artifact identity and WAT; no retained root | | succeeds | blocked | Exit 1; only `{"code":"UNKNOWN","message":"Provider wabt-artifact adapter failed during inspect_wasm_artifact"}`; root retained | | validation fails | permitted | Exit 1; `invalid_request`, primary marker, captured stderr and partial Evidence; no retained root | | validation fails | blocked | Exit 1; same UNKNOWN response; primary diagnostics and partial Evidence absent; root retained | All eight WABT CLI stderr streams were empty. The four Apktool cases similarly preserved results with permitted cleanup and returned only `{"code":"UNKNOWN","message":"Apktool workspace cleanup failed"}` with blocked cleanup. The failed-decode control otherwise returned `unsupported_target` and the original stderr marker. A full source CLI test observed WABT's primary `cleanup_incomplete` result containing `details.partial_observation`, two failed cleanup attempts, and the final UNKNOWN response without that observation. Passing the same primary result and rejecting close to `finishAnalysisCommand` retained the observation, selected artifact identity, primary projection and cleanup projection. Baseline locators: `src/cli/apktoolCommands.ts:30-38`, `src/apktool/ApktoolProvider.ts:497-525`, `src/cli/wasmCommands.ts:58-81`, and `src/wasm/WabtArtifactProvider.ts:56,445`. Registrations are at `src/cli.ts:104,114`. Current main still has the identical Apktool CLI result-replacing branch and workspace-cleanup AggregateError. Commit `089ef732e` changes version-probe/process ownership cleanup, not workspace result preservation. WABT and the other cited CLI handlers remain unchanged. ### Typed outcome or recovery expected Keep cleanup as a failure with retained-resource paths, while preserving completed Evidence, original errors and captured stderr. Cover success plus close failure, primary failure plus close failure, and an initial cleanup error carrying partial Evidence plus a second close failure. Preserve permitted-cleanup and signal controls. ### Analysis or suspected cause Source-derived: Apktool returns the saved operation result only after close, so a close rejection replaces it. WASM returns its primary projection inside a try whose finally awaits close, which likewise replaces the return. The same finalization contract applies across the related handler groups. `finishAnalysisCommand` demonstrates an existing preservation policy. ### Additional context Related closed #1501 addressed firmware provider-level retention; the remaining CLI close retry can still replace that retained outcome. Open PR #3492 modifies web module and source-location cleanup/finalization only. It does not address the reproduced Apktool/WABT commands or the other lifecycle groups. PR #2406 adds component decoding and does not restore this repeated WABT workspace-cleanup case.

bsakaguchi closed this issue

2026-10-16 21:06:56 +00:00

bsakaguchi commented

2026-10-16 21:06:57 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 15, 2026):

Confirmed: cleanup failure must not erase the outcome — the WABT case already holds completed Evidence in the primary service error, and the final CLI response downgrading it to a generic UNKNOWN cleanup error hides real diagnostic value from the caller. Cleanup errors should be reported alongside (or as a distinct field) while preserving the completed result and original diagnostics.

@XinwangTao commented on GitHub (Oct 15, 2026): Confirmed: cleanup failure must not erase the outcome — the WABT case already holds completed Evidence in the primary service error, and the final CLI response downgrading it to a generic UNKNOWN cleanup error hides real diagnostic value from the caller. Cleanup errors should be reported alongside (or as a distinct field) while preserving the completed result and original diagnostics.

bsakaguchi commented

2026-10-16 21:06:58 +00:00

Author

Owner

@yankikucuk commented on GitHub (Oct 15, 2026):

Working on this. Reproduced with your WABT launcher matrix: with blocked cleanup the CLI returned only the generic UNKNOWN response. decode-android-resources and inspect-wasm-artifact now finish through the shared finishAnalysisCommand finalizer the Go, capture and signal commands already use, so completed Evidence is retained as the cleanup error's partial observation, a primary failure keeps its diagnostics beside the cleanup failure, and the retained workspace stays listed. The other handler groups you listed by inspection are left for follow-ups. Draft PR follows shortly.

@yankikucuk commented on GitHub (Oct 15, 2026): Working on this. Reproduced with your WABT launcher matrix: with blocked cleanup the CLI returned only the generic UNKNOWN response. `decode-android-resources` and `inspect-wasm-artifact` now finish through the shared `finishAnalysisCommand` finalizer the Go, capture and signal commands already use, so completed Evidence is retained as the cleanup error's partial observation, a primary failure keeps its diagnostics beside the cleanup failure, and the retained workspace stays listed. The other handler groups you listed by inspection are left for follow-ups. Draft PR follows shortly.

bsakaguchi commented

2026-10-16 21:06:58 +00:00

Author

Owner

@morluto commented on GitHub (Oct 15, 2026):

Reproduced through public inspect-wasm-artifact with controlled WABT executables and a real filesystem permission denial during owned scratch cleanup. The base CLI discarded the completed result and returned only generic UNKNOWN. The successful control returned Evidence and removed its scratch root. These launchers are controlled seams, not real WABT.

Thirteen one-shot command families now reuse the existing finishAnalysisCommand: ADB, Android/JADX, Apktool, EVM, firmware, Flutter, JavaScript recovery, JEB, PCAP, SQLite, WASM, web-module trace and web-source location. Duplicated close/finally handling is removed. The shared finalizer retains a completed observation or combines primary and cleanup diagnostics while awaiting close.

The public CLI regression uses a valid 76-byte WASM module, independently compiled by Node before the probe. A cleanup denial now returns canonical cleanup_incomplete with the valid completed observation retained; validation failure plus cleanup denial preserves both diagnostics. Permissions are restored on the disposable fixture during teardown. The mode-denial test is explicitly unavailable on Windows/root; it passed on this non-root macOS host.

Related CLI/acceptance suites passed (55 tests, one existing skip in a 12-file run). The local patch is uncommitted/unreleased. No real Android device or unavailable external engine was exercised.

@morluto commented on GitHub (Oct 15, 2026): Reproduced through public `inspect-wasm-artifact` with controlled WABT executables and a real filesystem permission denial during owned scratch cleanup. The base CLI discarded the completed result and returned only generic `UNKNOWN`. The successful control returned Evidence and removed its scratch root. These launchers are controlled seams, not real WABT. Thirteen one-shot command families now reuse the existing `finishAnalysisCommand`: ADB, Android/JADX, Apktool, EVM, firmware, Flutter, JavaScript recovery, JEB, PCAP, SQLite, WASM, web-module trace and web-source location. Duplicated close/finally handling is removed. The shared finalizer retains a completed observation or combines primary and cleanup diagnostics while awaiting close. The public CLI regression uses a valid 76-byte WASM module, independently compiled by Node before the probe. A cleanup denial now returns canonical `cleanup_incomplete` with the valid completed observation retained; validation failure plus cleanup denial preserves both diagnostics. Permissions are restored on the disposable fixture during teardown. The mode-denial test is explicitly unavailable on Windows/root; it passed on this non-root macOS host. Related CLI/acceptance suites passed (55 tests, one existing skip in a 12-file run). The local patch is uncommitted/unreleased. No real Android device or unavailable external engine was exercised.

bsakaguchi commented

2026-10-16 21:06:58 +00:00

Author

Owner

@morluto commented on GitHub (Oct 16, 2026):

Fixed on main in 0e1f0bf2d. The affected one-shot CLI workflows use the shared finalizer to retain completed Evidence and primary diagnostics when provider cleanup also fails.

WABT/Apktool CLI controls use fixture executables and real filesystem cleanup denial; native engine behavior is not claimed.

Hosted main CI is still pending; this closure is based on the implemented fix and scoped regression evidence.

@morluto commented on GitHub (Oct 16, 2026): Fixed on `main` in [0e1f0bf2d](https://github.com/morluto/rea/commit/0e1f0bf2d06943e59253b8a66923a1ae6005e81e). The affected one-shot CLI workflows use the shared finalizer to retain completed Evidence and primary diagnostics when provider cleanup also fails. WABT/Apktool CLI controls use fixture executables and real filesystem cleanup denial; native engine behavior is not claimed. Hosted main CI is still pending; this closure is based on the implemented fix and scoped regression evidence.

bsakaguchi commented

2026-10-16 21:06:58 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 16, 2026):

Fix confirmed on main — routing the one-shot CLI families through the shared finishAnalysisCommand finalizer is the right call: it keeps completed Evidence as a partial observation when cleanup fails and pairs primary diagnostics with the cleanup error instead of replacing them. Closing as completed is reasonable; the follow-up #4240 for the remaining cleanup-resource diagnostics gap is worth keeping an eye on.

@XinwangTao commented on GitHub (Oct 16, 2026): Fix confirmed on main — routing the one-shot CLI families through the shared finishAnalysisCommand finalizer is the right call: it keeps completed Evidence as a partial observation when cleanup fails and pairs primary diagnostics with the cleanup error instead of replacing them. Closing as completed is reasonable; the follow-up #4240 for the remaining cleanup-resource diagnostics gap is worth keeping an eye on.

bsakaguchi referenced this issue

2026-10-17 21:12:17 +00:00

[PR #1544] [MERGED] feat(ida): support optional native IDA Free MCP in attached mode #2960

bsakaguchi referenced this issue

2026-10-31 01:03:57 +00:00

[PR #2236] [CLOSED] fix(ida): update native bridge for main and verify with IDA Pro #5104

bsakaguchi referenced this issue

2026-10-31 01:04:27 +00:00

[PR #2324] [MERGED] fix(ida): preserve native coverage and verify returned values #5164