[PR #6194] [CLOSED] fix(electron): complete short fuse image reads #7197

Closed
opened 2026-11-01 21:46:04 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6194
Author: @mertkayacs
Created: 10/22/2026
Status: ❌ Closed

Base: main ← Head: fix/electron-fuse-short-reads


📝 Commits (2)

  • 55ae5b1 test(electron): cover short fuse image reads
  • 2509303 fix(electron): complete short fuse image reads

📊 Changes

2 files changed (+97 additions, -10 deletions)

View changed files

📝 src/electron/ElectronFuseProvider.test.ts (+70 -2)
📝 src/electron/ElectronFuseReader.ts (+27 -8)

📄 Description

Summary

Complete legal positive short reads while inspecting Electron fuse images. This prevents unchanged images from being reported as changed when one positional file read returns fewer bytes than requested.

No linked issue: I found this while reading the Electron fuse reader, and the change is small enough that the evidence fits here.

Problem and expected behavior

FileHandle.read reports the number of bytes actually read, and a positive result can be smaller than the requested length. The Electron fuse reader currently requires every individual read to fill the requested range. A valid unchanged image therefore raises RegularFileChangedError after a positive short read.

The reader should retry from the returned position until the requested range is complete. A zero-byte read before the admitted file extent should continue to signal truncation or mutation.

Change and scope

Add one exact-read helper at the file-reading boundary and use it for container identification, chunk scanning, and fuse-wire reads. The helper preserves positional reads, checks cancellation between attempts, and keeps the existing zero-byte failure.

Add a regression that uses a real file descriptor while limiting each native read request to three bytes. The test verifies that the complete Mach-O fuse report is recovered through multiple reads.

This is limited to Electron fuse image reads. It does not change target admission, format parsing, result schemas, or file mutation authority.

Contract and boundary impact

  • Semantic owner and earliest changed stage: ElectronFuseReader owns exact file-range reads. The earliest changed stage is file I/O before container identification, hashing, and sentinel parsing.
  • Contribution scope fit and linked maintainer decision: Routine correctness fix within the existing static artifact inspection boundary. No advance maintainer scope decision is required.
  • CLI and MCP/tool-catalog contract: No schema or command change. Existing Electron fuse inspection succeeds for legal positive short reads instead of returning a file-changed error.
  • Provider, bridge, target-format, or platform compatibility: Built-in Electron fuse inspection only. Mach-O, ELF, and PE parsing contracts are unchanged.
  • Evidence, artifact, provenance, or reconstruction contract: Artifact bytes, SHA-256 calculation, selected path, resolved path, and fuse evidence are unchanged. The reader now consumes the entire admitted range before interpreting it.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): The existing caller-selected, read-only regular file and FileHandle lifecycle are unchanged. Node's documented FileHandle.read byte count controls progress. Cancellation is checked before each retry.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): None.

Expected effects:

  • 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: Added completes short positional reads before interpreting the image in src/electron/ElectronFuseProvider.test.ts. It limits native reads to three bytes and asserts the complete Mach-O fuse result.
  • Base reproduction or other evidence: On base revision 22c4b528d2e06bee8c5f5034f5b184fec0954724, the new test failed with RegularFileChangedError; 1 test failed and 27 passed. Node documents that end of file on an unchanged file is indicated by a zero-byte read, so a positive short read must not be treated as end of file or mutation.
  • User-visible CLI/MCP output (if applicable): No separate CLI or MCP workflow ran. The production reader returned a normal report through the source test after the fix.
  • Remaining proof gaps: The short-read condition was injected around a real local descriptor rather than observed from a specific filesystem. No non-Linux platform or CLI/MCP end-to-end workflow ran.

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 run test:local -- src/electron/ElectronFuseProvider.test.ts on the test-only commit: failed as expected with 1 failed and 27 passed.
  • npm run test:focused -- src/electron/ElectronFuseProvider.test.ts: 1 file passed, 28 tests passed.
  • npm run test:local -- src/electron/ElectronFuseArchitecture.test.ts src/electron/ElectronFuseCleanup.test.ts src/electron/ElectronFuseCompletion.test.ts src/electron/ElectronFuseProvider.test.ts src/electron/ElectronFuseReaderCleanup.test.ts: 5 files passed, 80 tests passed.
  • npm run check:fast: typecheck and lint both passed.
  • git diff upstream/main...HEAD --check: passed with no output.

Compatibility, safety, and release

  • Breaking changes or migration steps: None.
  • Real Hopper/Ghidra, browser, or OS coverage: Not applicable to providers and browser workflows. Verified on Node.js 24.18.0 (the pinned version) and 22.22.3, Linux x86_64 only.
  • Package or release metadata impact: None.
  • Security, privacy, process, or containment review: Read-only local file authority and existing size bounds are unchanged. The loop advances only on positive reads, stops on zero, and checks cancellation between reads. No network, process, mutation, or credential boundary is added.

Author readiness

Human owner: @mertkayacs

  • 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.

🔄 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/6194 **Author:** [@mertkayacs](https://github.com/mertkayacs) **Created:** 10/22/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/electron-fuse-short-reads` --- ### 📝 Commits (2) - [`55ae5b1`](https://github.com/morluto/rea/commit/55ae5b1c4ab7607ab18694469f8533ccf0aa0417) test(electron): cover short fuse image reads - [`2509303`](https://github.com/morluto/rea/commit/25093036b4d5e7de978216a2656cab9a9a4a6cf1) fix(electron): complete short fuse image reads ### 📊 Changes **2 files changed** (+97 additions, -10 deletions) <details> <summary>View changed files</summary> 📝 `src/electron/ElectronFuseProvider.test.ts` (+70 -2) 📝 `src/electron/ElectronFuseReader.ts` (+27 -8) </details> ### 📄 Description ## Summary Complete legal positive short reads while inspecting Electron fuse images. This prevents unchanged images from being reported as changed when one positional file read returns fewer bytes than requested. No linked issue: I found this while reading the Electron fuse reader, and the change is small enough that the evidence fits here. ## Problem and expected behavior `FileHandle.read` reports the number of bytes actually read, and a positive result can be smaller than the requested length. The Electron fuse reader currently requires every individual read to fill the requested range. A valid unchanged image therefore raises `RegularFileChangedError` after a positive short read. The reader should retry from the returned position until the requested range is complete. A zero-byte read before the admitted file extent should continue to signal truncation or mutation. ## Change and scope Add one exact-read helper at the file-reading boundary and use it for container identification, chunk scanning, and fuse-wire reads. The helper preserves positional reads, checks cancellation between attempts, and keeps the existing zero-byte failure. Add a regression that uses a real file descriptor while limiting each native read request to three bytes. The test verifies that the complete Mach-O fuse report is recovered through multiple reads. This is limited to Electron fuse image reads. It does not change target admission, format parsing, result schemas, or file mutation authority. ## Contract and boundary impact - Semantic owner and earliest changed stage: `ElectronFuseReader` owns exact file-range reads. The earliest changed stage is file I/O before container identification, hashing, and sentinel parsing. - Contribution scope fit and linked maintainer decision: Routine correctness fix within the existing static artifact inspection boundary. No advance maintainer scope decision is required. - CLI and MCP/tool-catalog contract: No schema or command change. Existing Electron fuse inspection succeeds for legal positive short reads instead of returning a file-changed error. - Provider, bridge, target-format, or platform compatibility: Built-in Electron fuse inspection only. Mach-O, ELF, and PE parsing contracts are unchanged. - Evidence, artifact, provenance, or reconstruction contract: Artifact bytes, SHA-256 calculation, selected path, resolved path, and fuse evidence are unchanged. The reader now consumes the entire admitted range before interpreting it. - Target selection, authority, upstream behavior, and cleanup details (if applicable): The existing caller-selected, read-only regular file and `FileHandle` lifecycle are unchanged. Node's documented `FileHandle.read` byte count controls progress. Cancellation is checked before each retry. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): None. Expected effects: - [ ] 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: Added `completes short positional reads before interpreting the image` in `src/electron/ElectronFuseProvider.test.ts`. It limits native reads to three bytes and asserts the complete Mach-O fuse result. - Base reproduction or other evidence: On base revision `22c4b528d2e06bee8c5f5034f5b184fec0954724`, the new test failed with `RegularFileChangedError`; 1 test failed and 27 passed. Node documents that end of file on an unchanged file is indicated by a zero-byte read, so a positive short read must not be treated as end of file or mutation. - User-visible CLI/MCP output (if applicable): No separate CLI or MCP workflow ran. The production reader returned a normal report through the source test after the fix. - Remaining proof gaps: The short-read condition was injected around a real local descriptor rather than observed from a specific filesystem. No non-Linux platform or CLI/MCP end-to-end workflow ran. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:local -- src/electron/ElectronFuseProvider.test.ts` on the test-only commit: failed as expected with 1 failed and 27 passed. - `npm run test:focused -- src/electron/ElectronFuseProvider.test.ts`: 1 file passed, 28 tests passed. - `npm run test:local -- src/electron/ElectronFuseArchitecture.test.ts src/electron/ElectronFuseCleanup.test.ts src/electron/ElectronFuseCompletion.test.ts src/electron/ElectronFuseProvider.test.ts src/electron/ElectronFuseReaderCleanup.test.ts`: 5 files passed, 80 tests passed. - `npm run check:fast`: typecheck and lint both passed. - `git diff upstream/main...HEAD --check`: passed with no output. ## Compatibility, safety, and release - Breaking changes or migration steps: None. - Real Hopper/Ghidra, browser, or OS coverage: Not applicable to providers and browser workflows. Verified on Node.js 24.18.0 (the pinned version) and 22.22.3, Linux x86_64 only. - Package or release metadata impact: None. - Security, privacy, process, or containment review: Read-only local file authority and existing size bounds are unchanged. The loop advances only on positive reads, stops on zero, and checks cancellation between reads. No network, process, mutation, or credential boundary is added. ## Author readiness Human owner: @mertkayacs - [x] 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. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:46:04 +00:00
Sign in to join this conversation.
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#7197
No description provided.