morluto/rea

Fork 0

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

[PR #4764] [MERGED] fix(evidence): verify staged output identity after its final write #6397

New issue

Closed

opened 2026-10-31 01:14:09 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:14:09 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4764
Author: @kaoru0822-kitauji
Created: 10/17/2026
Status: ✅ Merged
Merged: 10/17/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/evidence-export-appledouble

📝 Commits (3)

94afed1 fix(evidence): verify staged output identity after its final write
865cda7 Merge remote-tracking branch 'origin/main' into fix/evidence-export-appledouble
478a83c test(evidence): cover failed final staged identity reads

📊 Changes

3 files changed (+160 additions, -14 deletions)

View changed files

📝 src/application/JsonFiles.ts (+1 -1)
📝 src/filesystem/OwnedStagedOutput.ts (+18 -10)
📝 tests/boundary/filesystem/jsonOutputOwnership.test.ts (+141 -3)

📄 Description

Summary

Fixes #4613

On macOS, evidence-export to a mounted FAT32 or exFAT volume failed every time with cleanup_incomplete. Each attempt also left a .rea-write-* staging directory behind. The writer recorded the staged file's identity while the file was still empty. On these filesystems the file gets its real identity only on its first write. The writer then rejected its own staged file as replaced, at both publication and cleanup. With this change, --overwrite exports succeed on both filesystems. Default exports still fail, because the volumes have no hard links, but they now fail with cleanup complete and no staging left.

Problem and expected behavior

Trigger: rea evidence-export bundle.json <FAT32-or-exFAT mount>/out.json --json on macOS.
Invariant that was broken: the staged-output owner must be able to recognize its own exclusively created file. It may delete only that file and then its staging directory. A file that was really swapped in must still be refused.
Observed on these volumes (Node probe, macOS 26 / Darwin 25.6): fstat on a new empty file returns ino 2^64-1, a placeholder, on both FAT32 and exFAT. After the first write, the descriptor reports the cluster-based ID (20 on FAT32, 9 on exFAT). That ID stays the same through fsync, close and a later pathname lstat.
OwnedStagedOutput.setFile sampled fstat right after open(..., "wx"). assertReadyForPublication and #cleanup then compared the pathname lstat against that placeholder and threw Staged file identity changed. Both publication modes failed this way. Cleanup refused to remove the file, so the staging directory stayed.
The ._content file in the issue is not a separate cause. It is the macOS AppleDouble companion. In the probe, the kernel removed it when content was unlinked, and moved it along on rename. It also removed the directory's ._.rea-write-* companion on rmdir. REA does not need to delete any ._* file itself.

Change and scope

OwnedStagedOutput.closeFile() now fstats the owned descriptor right before closing it, after the last write. That identity is used for the pathname check before publication and for cleanup. The identity sampled when the file was created is removed (setFile is now synchronous and only takes ownership of the handle). There is now one point that records identity.
If the observation fails, the identity becomes unknown, not stale. Cleanup then closes the handle but refuses to remove the pathname, as before.
The check is still against REA's own exclusively created descriptor. A staged pathname that was replaced while the descriptor was open is still refused (the existing refuses to publish a staged path replaced after its descriptor opened test is unchanged and passes).
Intentionally not included: the default (no --overwrite) mode still publishes with link(2). FAT32 and exFAT return ENOTSUP for that, so a default export to these volumes still fails. The failure is generic (execution_failure, reason: io, "Check file permissions"), but cleanup is complete and the destination stays absent. Two ways to fix this would change the publication contract, and that needs a maintainer decision: (a) a typed unsupported-destination refusal, which would add a new EvidenceFileError reason, or (b) an exclusive streamed fallback like the ADB screen capture fallback, whose visibility differs from atomic publication. Neither is in this PR.
Contribution scope: ordinary fix within existing ownership boundaries; maintainer scope decision not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: src/filesystem/OwnedStagedOutput.ts, which owns staged-file identity. src/application/JsonFiles.ts only drops an await on the now-synchronous setFile.
Contribution scope fit and linked maintainer decision: fix within existing ownership rules; not applicable.
CLI and MCP/tool-catalog contract: no schema or error-shape change. --overwrite exports to FAT32/exFAT now succeed. Default exports there now return execution_failure with complete cleanup, instead of cleanup_incomplete with a retained staging directory.
Provider, bridge, target-format, or platform compatibility: none on APFS, ext4 or other filesystems whose IDs don't change on first write. There, the identity at close equals the identity at creation.
Evidence, artifact, provenance, or reconstruction contract: none. The bytes written and the publication receipt are unchanged.
Target selection, authority, upstream behavior, and cleanup details: cleanup still deletes only the exact content entry, after an identity match, and then the verified staging directory. It still refuses any other entry. No new deletion authority.
Generated metadata, package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated: tests/boundary/filesystem/jsonOutputOwnership.test.ts. The new seam wraps node:fs/promises.open for files inside .rea-write-*, so the descriptor stat reports a placeholder ino while the file is empty. This models the FAT/exFAT behavior seen on the real volumes. New cases:
- publishes when the staged file ID is assigned by its first write (overwrite: false|true): publication succeeds, the receipt is correct, the destination bytes are correct and no staging remains.
- removes cancelled staging after the first write assigned its file ID: cancellation after a partial write completes cleanup.
- closes the descriptor and retains staged content when its final identity is unknown: the final descriptor stat() rejects. The descriptor still closes, publication fails with Cannot verify staged file identity and no receipt, cleanup is reported incomplete, and content is not deleted, including after a retried resources.close(). This was added for review feedback; it found no defect.
- The three direct OwnedStagedOutput tests now call setFile without await.
Base reproduction:
- Executed: with src/ reset to origin/main and only the new tests applied, the 3 new tests fail with Staged file identity changed: …/.rea-write-*/content (Tests 3 failed | 8 passed (11)). With the fix: 11 passed (11). Re-confirmed after rebasing onto 9c56d09c0.
- Real volumes (executed, macOS, 64 MiB hdiutil FAT32 and ExFAT images, isolated HOME, the issue's empty bundle). Before the fix: every default and --overwrite export returned cleanup_incomplete and left a .rea-write-* directory with content and ._content. After the fix (built from ae029fe2f + this change):
  - --overwrite returned exit 0 with {"bytes":105,"records":0}, and the bytes matched the APFS control.
  - Default export returned exit 1 with execution_failure / reason: io, and no cleanup details.
  - In both modes, no .rea-write-* entry and no ._.rea-write-* companion remained.
  - The existing-destination control still returned invalid_request / reason: exists and kept KEEP EXISTING.

User-visible CLI output after the fix (FAT32, paths normalized):

{"path":"$W/fat32/overwrite.json","bytes":105,"records":0}
{"error":"Analysis failed","code":"execution_failure","category":"execution_failure","message":"Evidence file could not be accessed. Check file permissions and try again.","retryable":false,"remediation":{"action":"Evidence file could not be accessed. Check file permissions and try again."},"details":{"operation":"write","reason":"io","path":"$W/fat32/output-1.json"}}

Remaining proof gaps:
- Default-mode export to link-less filesystems still fails with a generic message. That needs the maintainer decision described above.
- Only macOS-hosted FAT32/exFAT disk images were tested. Not tested: physical removable media, Linux vfat/exfat, Windows, network filesystems.
- On FAT, every empty file shares the placeholder ID. A staged file that is still empty at close, for example after cancellation before the first write, can't be told apart from another empty file by dev/ino. That behavior was there before this change and isn't addressed here.
- If the final descriptor stat() fails, the staging stays retained for the session, because a closed descriptor can't be observed again. This is covered by a test and is the conservative choice.
- Tests ran on Node v26.5.0, not the .nvmrc pin of 24.18.0, which isn't installed on this machine.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

npm ci and npm run build:cached: passed
npm run test:focused -- tests/boundary/filesystem/jsonOutputOwnership.test.ts with src/ at origin/main: 3 failed, 8 passed (expected base failure)
npm run test:focused -- tests/boundary/filesystem/jsonOutputOwnership.test.ts tests/boundary/mcp/evidenceExportCancellation.test.ts tests/boundary/mcp/evidenceImportCancellation.test.ts tests/boundary/mcp/retainedApplicationEvidenceMcp.test.ts tests/boundary/mcp/lateAbortApplicationEvidenceMcp.test.ts src/server/EvidenceMcpServer.test.ts src/application/investigation/InvestigationRecords.test.ts src/domain/analysisErrorProjection.test.ts src/contracts/sessionPathInputs.test.ts: 9 files passed; 101 passed, 3 skipped
After merging origin/main (3fc007534) and adding the identity-failure test: npm run test:focused -- tests/boundary/filesystem/jsonOutputOwnership.test.ts, 12 passed (12)
npx tsc --noEmit -p .: passed (also re-run after the merge and the new test)
npx oxfmt --check on the changed files: passed. After the new test: npx oxfmt and npx oxlint on the test file were clean.
npx oxlint on the changed files: 0 errors. There are 4 max-params warnings on lines this PR doesn't touch in src/application/JsonFiles.ts.
Real hdiutil FAT32 and ExFAT reproduction, before and after the fix: results as above. Every image was detached and deleted.

Compatibility, safety, and release

Breaking changes or migration steps: none. OwnedStagedOutput.setFile is now synchronous (internal API; all callers updated).
Real Hopper/Ghidra, browser, or OS coverage: macOS (Darwin 25.6, arm64) with real FAT32 and exFAT volumes mounted from disk images; no provider involved.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the identity check is still against REA's own O_EXCL descriptor. Replaced staged paths are still refused. Cleanup authority is unchanged.

Author readiness

Human owner:

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/4764 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/17/2026 **Status:** ✅ Merged **Merged:** 10/17/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/evidence-export-appledouble` --- ### 📝 Commits (3) - [`94afed1`](https://github.com/morluto/rea/commit/94afed1ca8b3197a261c36f2d3d286eb27ce2769) fix(evidence): verify staged output identity after its final write - [`865cda7`](https://github.com/morluto/rea/commit/865cda76cbbfee4f4574558b42a0e42eab3c104a) Merge remote-tracking branch 'origin/main' into fix/evidence-export-appledouble - [`478a83c`](https://github.com/morluto/rea/commit/478a83cf6640340b8a66ddb49d72f3cc7fe3c36d) test(evidence): cover failed final staged identity reads ### 📊 Changes **3 files changed** (+160 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `src/application/JsonFiles.ts` (+1 -1) 📝 `src/filesystem/OwnedStagedOutput.ts` (+18 -10) 📝 `tests/boundary/filesystem/jsonOutputOwnership.test.ts` (+141 -3) </details> ### 📄 Description ## Summary Fixes #4613 On macOS, `evidence-export` to a mounted FAT32 or exFAT volume failed every time with `cleanup_incomplete`. Each attempt also left a `.rea-write-*` staging directory behind. The writer recorded the staged file's identity while the file was still empty. On these filesystems the file gets its real identity only on its first write. The writer then rejected its own staged file as replaced, at both publication and cleanup. With this change, `--overwrite` exports succeed on both filesystems. Default exports still fail, because the volumes have no hard links, but they now fail with cleanup complete and no staging left. ## Problem and expected behavior - Trigger: `rea evidence-export bundle.json <FAT32-or-exFAT mount>/out.json --json` on macOS. - Invariant that was broken: the staged-output owner must be able to recognize its own exclusively created file. It may delete only that file and then its staging directory. A file that was really swapped in must still be refused. - Observed on these volumes (Node probe, macOS 26 / Darwin 25.6): `fstat` on a new empty file returns `ino` 2^64-1, a placeholder, on both FAT32 and exFAT. After the first `write`, the descriptor reports the cluster-based ID (`20` on FAT32, `9` on exFAT). That ID stays the same through `fsync`, `close` and a later pathname `lstat`. - `OwnedStagedOutput.setFile` sampled `fstat` right after `open(..., "wx")`. `assertReadyForPublication` and `#cleanup` then compared the pathname `lstat` against that placeholder and threw `Staged file identity changed`. Both publication modes failed this way. Cleanup refused to remove the file, so the staging directory stayed. - The `._content` file in the issue is not a separate cause. It is the macOS AppleDouble companion. In the probe, the kernel removed it when `content` was unlinked, and moved it along on rename. It also removed the directory's `._.rea-write-*` companion on `rmdir`. REA does not need to delete any `._*` file itself. ## Change and scope - `OwnedStagedOutput.closeFile()` now `fstat`s the owned descriptor right before closing it, after the last write. That identity is used for the pathname check before publication and for cleanup. The identity sampled when the file was created is removed (`setFile` is now synchronous and only takes ownership of the handle). There is now one point that records identity. - If the observation fails, the identity becomes unknown, not stale. Cleanup then closes the handle but refuses to remove the pathname, as before. - The check is still against REA's own exclusively created descriptor. A staged pathname that was replaced while the descriptor was open is still refused (the existing `refuses to publish a staged path replaced after its descriptor opened` test is unchanged and passes). - Intentionally not included: the default (no `--overwrite`) mode still publishes with `link(2)`. FAT32 and exFAT return `ENOTSUP` for that, so a default export to these volumes still fails. The failure is generic (`execution_failure`, `reason: io`, "Check file permissions"), but cleanup is complete and the destination stays absent. Two ways to fix this would change the publication contract, and that needs a maintainer decision: (a) a typed unsupported-destination refusal, which would add a new `EvidenceFileError` reason, or (b) an exclusive streamed fallback like the ADB screen capture fallback, whose visibility differs from atomic publication. Neither is in this PR. - Contribution scope: ordinary fix within existing ownership boundaries; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/filesystem/OwnedStagedOutput.ts`, which owns staged-file identity. `src/application/JsonFiles.ts` only drops an `await` on the now-synchronous `setFile`. - Contribution scope fit and linked maintainer decision: fix within existing ownership rules; not applicable. - CLI and MCP/tool-catalog contract: no schema or error-shape change. `--overwrite` exports to FAT32/exFAT now succeed. Default exports there now return `execution_failure` with complete cleanup, instead of `cleanup_incomplete` with a retained staging directory. - Provider, bridge, target-format, or platform compatibility: none on APFS, ext4 or other filesystems whose IDs don't change on first write. There, the identity at close equals the identity at creation. - Evidence, artifact, provenance, or reconstruction contract: none. The bytes written and the publication receipt are unchanged. - Target selection, authority, upstream behavior, and cleanup details: cleanup still deletes only the exact `content` entry, after an identity match, and then the verified staging directory. It still refuses any other entry. No new deletion authority. - Generated metadata, package, or installation impact: none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [x] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/filesystem/jsonOutputOwnership.test.ts`. The new seam wraps `node:fs/promises.open` for files inside `.rea-write-*`, so the descriptor `stat` reports a placeholder `ino` while the file is empty. This models the FAT/exFAT behavior seen on the real volumes. New cases: - `publishes when the staged file ID is assigned by its first write (overwrite: false|true)`: publication succeeds, the receipt is correct, the destination bytes are correct and no staging remains. - `removes cancelled staging after the first write assigned its file ID`: cancellation after a partial write completes cleanup. - `closes the descriptor and retains staged content when its final identity is unknown`: the final descriptor `stat()` rejects. The descriptor still closes, publication fails with `Cannot verify staged file identity` and no receipt, cleanup is reported incomplete, and `content` is not deleted, including after a retried `resources.close()`. This was added for review feedback; it found no defect. - The three direct `OwnedStagedOutput` tests now call `setFile` without `await`. - Base reproduction: - Executed: with `src/` reset to `origin/main` and only the new tests applied, the 3 new tests fail with `Staged file identity changed: …/.rea-write-*/content` (`Tests 3 failed | 8 passed (11)`). With the fix: `11 passed (11)`. Re-confirmed after rebasing onto `9c56d09c0`. - Real volumes (executed, macOS, 64 MiB `hdiutil` FAT32 and ExFAT images, isolated HOME, the issue's empty bundle). Before the fix: every default and `--overwrite` export returned `cleanup_incomplete` and left a `.rea-write-*` directory with `content` and `._content`. After the fix (built from `ae029fe2f` + this change): - `--overwrite` returned exit 0 with `{"bytes":105,"records":0}`, and the bytes matched the APFS control. - Default export returned exit 1 with `execution_failure` / `reason: io`, and no `cleanup` details. - In both modes, no `.rea-write-*` entry and no `._.rea-write-*` companion remained. - The existing-destination control still returned `invalid_request` / `reason: exists` and kept `KEEP EXISTING`. - User-visible CLI output after the fix (FAT32, paths normalized): ```json {"path":"$W/fat32/overwrite.json","bytes":105,"records":0} {"error":"Analysis failed","code":"execution_failure","category":"execution_failure","message":"Evidence file could not be accessed. Check file permissions and try again.","retryable":false,"remediation":{"action":"Evidence file could not be accessed. Check file permissions and try again."},"details":{"operation":"write","reason":"io","path":"$W/fat32/output-1.json"}} ``` - Remaining proof gaps: - Default-mode export to link-less filesystems still fails with a generic message. That needs the maintainer decision described above. - Only macOS-hosted FAT32/exFAT disk images were tested. Not tested: physical removable media, Linux `vfat`/`exfat`, Windows, network filesystems. - On FAT, every empty file shares the placeholder ID. A staged file that is still empty at close, for example after cancellation before the first write, can't be told apart from another empty file by `dev`/`ino`. That behavior was there before this change and isn't addressed here. - If the final descriptor `stat()` fails, the staging stays retained for the session, because a closed descriptor can't be observed again. This is covered by a test and is the conservative choice. - Tests ran on Node v26.5.0, not the `.nvmrc` pin of 24.18.0, which isn't installed on this machine. For evidence-bearing changes: - [ ] Observed, derived, and inferred claims remain distinguishable. - [ ] Artifact identity, source provenance, and failed attempts remain preserved. - [ ] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm ci` and `npm run build:cached`: passed - `npm run test:focused -- tests/boundary/filesystem/jsonOutputOwnership.test.ts` with `src/` at `origin/main`: 3 failed, 8 passed (expected base failure) - `npm run test:focused -- tests/boundary/filesystem/jsonOutputOwnership.test.ts tests/boundary/mcp/evidenceExportCancellation.test.ts tests/boundary/mcp/evidenceImportCancellation.test.ts tests/boundary/mcp/retainedApplicationEvidenceMcp.test.ts tests/boundary/mcp/lateAbortApplicationEvidenceMcp.test.ts src/server/EvidenceMcpServer.test.ts src/application/investigation/InvestigationRecords.test.ts src/domain/analysisErrorProjection.test.ts src/contracts/sessionPathInputs.test.ts`: 9 files passed; 101 passed, 3 skipped - After merging `origin/main` (`3fc007534`) and adding the identity-failure test: `npm run test:focused -- tests/boundary/filesystem/jsonOutputOwnership.test.ts`, 12 passed (12) - `npx tsc --noEmit -p .`: passed (also re-run after the merge and the new test) - `npx oxfmt --check` on the changed files: passed. After the new test: `npx oxfmt` and `npx oxlint` on the test file were clean. - `npx oxlint` on the changed files: 0 errors. There are 4 `max-params` warnings on lines this PR doesn't touch in `src/application/JsonFiles.ts`. - Real `hdiutil` FAT32 and ExFAT reproduction, before and after the fix: results as above. Every image was detached and deleted. ## Compatibility, safety, and release - Breaking changes or migration steps: none. `OwnedStagedOutput.setFile` is now synchronous (internal API; all callers updated). - Real Hopper/Ghidra, browser, or OS coverage: macOS (Darwin 25.6, arm64) with real FAT32 and exFAT volumes mounted from disk images; no provider involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the identity check is still against REA's own `O_EXCL` descriptor. Replaced staged paths are still refused. Cleanup authority is unchanged. ## Author readiness Human owner: - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:14:09 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-03 13:38:36 +00:00

[PR #6981] [MERGED] fix(android): report DEX static values of static final fields #7545

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#6397

No description provided.

Rows
Columns