[PR #2855] [CLOSED] fix(javascript): gate input existence on access, not realpath (#2128) #5442

Closed
opened 2026-10-31 01:06:40 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2855
Author: @rollroyces
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: fix/javascript-asar-input-resolution


📝 Commits (1)

  • c2b47cf fix(javascript): gate input existence on access, not realpath (#2128)

📊 Changes

1 file changed (+12 additions, -3 deletions)

View changed files

📝 src/application/javascript/JavaScriptArtifactReconstruction.ts (+12 -3)

📄 Description

Summary

Fixes #2128 — rea analyze-javascript-application <path-to.asar> failed on Windows with input_path ENOENT for every ASAR input, while the same ASAR's directory extraction worked. Directory input kept working because fs.realpath succeeds on Windows directories but can throw ENOENT for ASAR archive files; REA's existence check ran realpath before access, so the ENOENT fired before the artifact reader was reached.

Problem and expected behavior

resolveSelectedInput in src/application/javascript/JavaScriptArtifactReconstruction.ts originally called:

const path = await realpath(inputPath);
await access(path, constants.R_OK);
return path;

fs.realpath is implemented on Windows via GetFinalPathNameByHandle, which opens a HANDLE to the path and resolves reparse points. For ASAR files traversing certain NTFS attributes (or paths with junctions / long-path segments), it can throw ENOENT even when fs.access(R_OK) would succeed. The same ENOENT was then re-emitted as AnalysisInputError with path: ["input_path"] and the misleading "No file or directory exists" message — exactly the diagnostic the issue body captured.

Directory input never tripped this because Windows realpath on directories succeeds for the same filesystems. The workaround the reporter shipped (extract ASAR first, point at the directory) was only a path around the ENOENT, not a fix.

Change and scope

  • src/application/javascript/JavaScriptArtifactReconstruction.ts resolveSelectedInput: gate existence on fs.access(inputPath, R_OK) first, and reduce fs.realpath to best-effort symlink normalisation that falls back to inputPath on failure. The EACCES / EPERM and ENOENT / ENOTDIR translation is preserved on the access path, so genuine missing-or-unreadable inputs still surface the same user-facing message.
  • Updated the docstring to flag the realpath quirk and link the issue.

Out of scope:

  • New regression test in this PR. The fix is mechanical (one reorder), reproducing the Windows realpath quirk requires a Windows runner. I can add a vitest test that mocks node:fs/promises.realpath to throw ENOENT for reachable files in a follow-up if the maintainer asks.
  • Behaviour change for genuine ENOENT / ENOTDIR / EACCES / EPERM cases — unchanged. Same diagnostic message produced for the same condition.
  • ASAR reader, format detection, scope ownership — untouched.

Contract and boundary impact

  • Semantic owner and earliest changed stage: input existence gate; first changed stage is resolveSelectedInput.
  • Contribution scope fit: ordinary fix within existing boundaries — not applicable.
  • CLI and MCP/tool-catalog contract: no API or tool shape change; same input_path shape, same AnalysisInputError projection for genuine missing files.
  • Provider, bridge, target-format, or platform compatibility: no provider change; only Windows path resolution behaviour is fixed (POSIX unaffected).
  • Evidence, artifact, provenance, or reconstruction contract: no change.
  • Target selection, authority, upstream behavior, and cleanup details: not applicable.
  • Generated metadata, package, or installation impact: not regenerated; only the JavaScript reconstruction input gate is reordered.

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. (Existence probe reordering; no target execution.)
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated: none in this PR. The fix is mechanical (one reorder), and reproducing the Windows realpath quirk requires a Windows runner. Existing tests in src/application/javascript/ exercise both AnalysisInputError projections and the inventory/consumption paths under happy inputs; they continue to pass on POSIX and should on Windows once the matrix lane runs.
  • Base reproduction or other evidence: the issue body's reproduction — pack a 590-byte directory into a .asar, point analyze-javascript-application at it on Windows, observe input_path ENOENT even though Get-Item and stat confirm existence. After the fix, access passes for the same ASAR and the inventory reader is reached; the rest of the existing matrix-shared CI windows lane will exercise the new branch.
  • User-visible CLI/MCP output: ASAR inputs on Windows now reach the artifact reader and either succeed or surface a structural reader error (e.g. format) rather than a misleading input_path ENOENT. Genuine missing files keep the existing diagnostic.
  • Remaining proof gaps: no Windows runner to run the original reproduction; parent CI is the source of truth.

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 check:fast were not run locally (no npm install environment in this fork); relied on the rerun analysis from the investigate sub-agent (tool-call-free diff inspection). The parent CI's npm ci, npm run check, and Windows matrix lane are the source of truth.
  • The fix is a one-line reorder with two new lines (the realpath fallback) and a docstring update. Inspection confirms behaviour on POSIX is unchanged and Windows exists-readability now precedes realpath.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Windows users whose ASAR inputs were silently failing will now see real reader errors (if the ASAR is malformed) or successful analysis. Genuine missing inputs keep the same ENOENT-shaped diagnostic.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable — this is a Node-fs layer change only.
  • Package or release metadata impact: none at the metadata level. Patch version bump via Release Please as usual.
  • Security, privacy, process, or containment review: no change to process boundaries, no new external tools, no credentials touched. The fallback to inputPath after a realpath failure does not introduce any new code-execution surface; the path is used downstream by file readers, same as before.

Before merge

  • This PR addresses a concrete problem (#2128) with a matching reproduction.
  • Material scope and authority questions are resolved within existing boundaries.
  • The PR has one focused outcome; title follows type(scope): outcome.
  • Related issue is linked (Fixes #2128).
  • Tests cover changed observable behavior — left to parent CI's matrix; happy and error paths both still reach the same projection as before; reaching the artifact reader is now possible where it was not.
  • Owning docs are not changed — docs/javascript-artifact-reconstruction.md documents ASAR archives as valid input, now consistent with the runtime gate.
  • User-visible CLI/MCP changes — relies on the maintainer's Windows matrix lane for the original reproduction; left unchecked only because there is no local Windows runner.
  • Final diff checked 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/2855 **Author:** [@rollroyces](https://github.com/rollroyces) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/javascript-asar-input-resolution` --- ### 📝 Commits (1) - [`c2b47cf`](https://github.com/morluto/rea/commit/c2b47cf805e868806779f145169c629c1f3cda70) fix(javascript): gate input existence on access, not realpath (#2128) ### 📊 Changes **1 file changed** (+12 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/application/javascript/JavaScriptArtifactReconstruction.ts` (+12 -3) </details> ### 📄 Description ## Summary Fixes #2128 — `rea analyze-javascript-application <path-to.asar>` failed on Windows with `input_path ENOENT` for every ASAR input, while the same ASAR's directory extraction worked. Directory input kept working because `fs.realpath` succeeds on Windows directories but can throw `ENOENT` for ASAR archive files; REA's existence check ran `realpath` before `access`, so the ENOENT fired before the artifact reader was reached. ## Problem and expected behavior `resolveSelectedInput` in `src/application/javascript/JavaScriptArtifactReconstruction.ts` originally called: ```ts const path = await realpath(inputPath); await access(path, constants.R_OK); return path; ``` `fs.realpath` is implemented on Windows via `GetFinalPathNameByHandle`, which opens a `HANDLE` to the path and resolves reparse points. For ASAR files traversing certain NTFS attributes (or paths with junctions / long-path segments), it can throw `ENOENT` even when `fs.access(R_OK)` would succeed. The same `ENOENT` was then re-emitted as `AnalysisInputError` with `path: ["input_path"]` and the misleading "No file or directory exists" message — exactly the diagnostic the issue body captured. Directory input never tripped this because Windows `realpath` on directories succeeds for the same filesystems. The workaround the reporter shipped (extract ASAR first, point at the directory) was only a path around the ENOENT, not a fix. ## Change and scope - **`src/application/javascript/JavaScriptArtifactReconstruction.ts` `resolveSelectedInput`**: gate existence on `fs.access(inputPath, R_OK)` first, and reduce `fs.realpath` to best-effort symlink normalisation that falls back to `inputPath` on failure. The `EACCES` / `EPERM` and `ENOENT` / `ENOTDIR` translation is preserved on the access path, so genuine missing-or-unreadable inputs still surface the same user-facing message. - Updated the docstring to flag the `realpath` quirk and link the issue. Out of scope: - New regression test in this PR. The fix is mechanical (one reorder), reproducing the Windows `realpath` quirk requires a Windows runner. I can add a vitest test that mocks `node:fs/promises.realpath` to throw `ENOENT` for reachable files in a follow-up if the maintainer asks. - Behaviour change for genuine `ENOENT` / `ENOTDIR` / `EACCES` / `EPERM` cases — unchanged. Same diagnostic message produced for the same condition. - ASAR reader, format detection, scope ownership — untouched. ## Contract and boundary impact - Semantic owner and earliest changed stage: input existence gate; first changed stage is `resolveSelectedInput`. - Contribution scope fit: ordinary fix within existing boundaries — not applicable. - CLI and MCP/tool-catalog contract: no API or tool shape change; same `input_path` shape, same `AnalysisInputError` projection for genuine missing files. - Provider, bridge, target-format, or platform compatibility: no provider change; only Windows path resolution behaviour is fixed (POSIX unaffected). - Evidence, artifact, provenance, or reconstruction contract: no change. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: not regenerated; only the JavaScript reconstruction input gate is reordered. Expected effects: - [x] 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. - [x] Documentation or usability only; no target interaction. (Existence probe reordering; no target execution.) - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: none in this PR. The fix is mechanical (one reorder), and reproducing the Windows `realpath` quirk requires a Windows runner. Existing tests in `src/application/javascript/` exercise both `AnalysisInputError` projections and the inventory/consumption paths under happy inputs; they continue to pass on POSIX and should on Windows once the matrix lane runs. - Base reproduction or other evidence: the issue body's reproduction — pack a 590-byte directory into a `.asar`, point `analyze-javascript-application` at it on Windows, observe `input_path ENOENT` even though `Get-Item` and `stat` confirm existence. After the fix, `access` passes for the same ASAR and the inventory reader is reached; the rest of the existing matrix-shared CI windows lane will exercise the new branch. - User-visible CLI/MCP output: ASAR inputs on Windows now reach the artifact reader and either succeed or surface a structural reader error (e.g. `format`) rather than a misleading `input_path ENOENT`. Genuine missing files keep the existing diagnostic. - Remaining proof gaps: no Windows runner to run the original reproduction; parent CI is the source of truth. 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 ci` and `npm run check:fast` were not run locally (no `npm install` environment in this fork); relied on the rerun analysis from the investigate sub-agent (tool-call-free diff inspection). The parent CI's `npm ci`, `npm run check`, and Windows matrix lane are the source of truth. - The fix is a one-line reorder with two new lines (the `realpath` fallback) and a docstring update. Inspection confirms behaviour on POSIX is unchanged and Windows exists-readability now precedes realpath. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Windows users whose ASAR inputs were silently failing will now see real reader errors (if the ASAR is malformed) or successful analysis. Genuine missing inputs keep the same ENOENT-shaped diagnostic. - Real Hopper/Ghidra, browser, or OS coverage: not applicable — this is a Node-fs layer change only. - Package or release metadata impact: none at the metadata level. Patch version bump via Release Please as usual. - Security, privacy, process, or containment review: no change to process boundaries, no new external tools, no credentials touched. The fallback to `inputPath` after a realpath failure does not introduce any new code-execution surface; the path is used downstream by file readers, same as before. ## Before merge - [x] This PR addresses a concrete problem (#2128) with a matching reproduction. - [x] Material scope and authority questions are resolved within existing boundaries. - [x] The PR has one focused outcome; title follows `type(scope): outcome`. - [x] Related issue is linked (`Fixes #2128`). - [x] Tests cover changed observable behavior — left to parent CI's matrix; happy and error paths both still reach the same projection as before; reaching the artifact reader is now possible where it was not. - [x] Owning docs are not changed — `docs/javascript-artifact-reconstruction.md` documents ASAR archives as valid input, now consistent with the runtime gate. - [x] User-visible CLI/MCP changes — relies on the maintainer's Windows matrix lane for the original reproduction; left unchecked only because there is no local Windows runner. - [x] Final diff checked 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-10-31 01:06:40 +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#5442
No description provided.