[PR #2882] [MERGED] fix(cli): keep file URLs out of operator-relative path resolution #5466

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2882
Author: @shubhamahuja9999
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/cli-electron-file-url-paths


📝 Commits (1)

  • c270092 fix(cli): keep file URLs out of operator-relative path resolution

📊 Changes

3 files changed (+84 additions, -4 deletions)

View changed files

📝 src/cliJsonInput.ts (+13 -4)
📝 src/cliJsonPaths.test.ts (+34 -0)
📝 tests/acceptance/applications/electronInvalidPaths.test.ts (+37 -0)

📄 Description

Summary

Fixes #2577

CLI path normalization (resolveCliJsonPaths) resolved every nonblank selected string against the operator's working directory. A file URL in an Electron path field, such as executable_path: "file:///opt/electron", became an invented local path (<cwd>/file:/opt/electron, or <cwd>\file:\opt\electron on Windows). The CLI then reported ENOENT for that invented location instead of the field's local-filesystem-path requirement.

Problem and expected behavior

Smallest trigger: rea capture-electron-scenario '{"executable_path":"file:///opt/electron","application_path":"<abs>/main.js"}' --format json.

  • Before: invalid_request with Cannot resolve executable_path <cwd>\file:\opt\electron: ENOENT …. The caller's input was rewritten and they are pointed at the wrong location.
  • After: invalid_request on executable_path with the shared schema message "Electron executable, application, and root paths must be absolute local filesystem paths …". No filesystem lookup of an invented path happens.

Violated invariant (AGENTS.md): absolute paths, file URLs and HTTP paths have different semantics, and caller-selected inputs must be preserved.

Change and scope

  • src/cliJsonInput.ts: resolveCliJsonPaths leaves values matching file: or scheme:// unchanged, so the shared schema's existing absolute-local-path refinement rejects them. One-letter prefixes stay paths, so Windows drive paths (C:\…) and relative names containing a colon (release:v1/electron) still resolve exactly as before.
  • This is the single CLI owner of operator-relative path normalization, used by capture-electron-scenario and the browser scenario browser.executable_path, so both get the fix.
  • Not included: file URLs are not converted to paths. They are rejected, as the issue's restoration check asks, because the MCP contract defines these fields as local filesystem paths.

Contribution scope: ordinary CLI input fix; maintainer decision not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: CLI JSON path normalization before shared schema validation.
  • Contribution scope fit and linked maintainer decision: not applicable.
  • CLI and MCP/tool-catalog contract: no schema change. URL inputs to CLI path fields now get the documented field error instead of an ENOENT for an invented path. MCP is unchanged; it never resolved relative values.
  • Provider, bridge, target-format, or platform compatibility: none. Windows drive paths are covered by a test.
  • Evidence, artifact, provenance, or reconstruction contract: none.
  • Target selection, authority, upstream behavior, and cleanup details: URL inputs are rejected before any filesystem lookup or Electron launch.
  • Generated metadata, package, or installation impact: 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.

None of these apply: input validation only.

Evidence and regression coverage

  • tests/acceptance/applications/electronInvalidPaths.test.ts, "rejects a file URL path field without resolving an invented local path": runs the compiled CLI. It asserts exit code 1, invalid_request on executable_path with the local-path message, and no ENOENT.
  • src/cliJsonPaths.test.ts: file:///opt/electron, FILE:electron and https://example.test/app stay unchanged and are rejected by electronActiveObservationInputSchema on executable_path. Controls bin/electron, release:v1/electron and C:\Electron\electron.exe still resolve.
  • Base reproduction: with only src/cliJsonInput.ts reverted, the 3 URL unit cases and the CLI case fail. The CLI received Cannot resolve executable_path C:\…\rea\file:\opt\electron: ENOENT. The controls pass both before and after.
  • Representative CLI output after the fix: {"code":"invalid_request", … "issues":[{"path":["executable_path"], … "message":"Electron executable, application, and root paths must be absolute local filesystem paths (for example /Applications/Electron.app/Contents/MacOS/Electron)"}]}
  • Remaining proof gaps: verified on Windows only; POSIX behaviour follows the same regex, and the issue reproduced on macOS.

Validation performed

Windows 11, Node.js 24.18.0, npm 11.16.0, on main at 688defca0:

  • npm run test:local -- src/cliJsonPaths.test.ts: 3 failed without the fix; 13/13 with it.
  • npm run test:focused -- tests/acceptance/applications/electronInvalidPaths.test.ts: the new case failed without the fix; 2/2 with it.
  • npm run test:focused -- the 38 test files referencing Electron/browser scenario paths or executable_path: the same 23 tests fail with and without this change, which makes them pre-existing on Windows. For example, scenarioCliInputDiagnostics looks for raw C:\… paths inside JSON-escaped output. The only differences between the runs are the 4 new tests, which fail without the fix and pass with it.
  • npm run check:fast (typecheck and lint): passed, including as the pre-push hook.
  • npm run verify:module-boundaries and oxfmt --check on changed files: passed.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Previously a URL in these fields always failed, with a misleading ENOENT; it now fails with the correct field error.
  • Real Hopper/Ghidra, browser, or OS coverage: Windows; no Electron or browser launch is involved.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: prevents a rewritten filesystem lookup; no new execution.

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/2882 **Author:** [@shubhamahuja9999](https://github.com/shubhamahuja9999) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/cli-electron-file-url-paths` --- ### 📝 Commits (1) - [`c270092`](https://github.com/morluto/rea/commit/c2700928af6106cb70ccb348a8a0984fbfeb34a1) fix(cli): keep file URLs out of operator-relative path resolution ### 📊 Changes **3 files changed** (+84 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `src/cliJsonInput.ts` (+13 -4) 📝 `src/cliJsonPaths.test.ts` (+34 -0) 📝 `tests/acceptance/applications/electronInvalidPaths.test.ts` (+37 -0) </details> ### 📄 Description ## Summary Fixes #2577 CLI path normalization (`resolveCliJsonPaths`) resolved every nonblank selected string against the operator's working directory. A file URL in an Electron path field, such as `executable_path: "file:///opt/electron"`, became an invented local path (`<cwd>/file:/opt/electron`, or `<cwd>\file:\opt\electron` on Windows). The CLI then reported ENOENT for that invented location instead of the field's local-filesystem-path requirement. ## Problem and expected behavior Smallest trigger: `rea capture-electron-scenario '{"executable_path":"file:///opt/electron","application_path":"<abs>/main.js"}' --format json`. - Before: `invalid_request` with `Cannot resolve executable_path <cwd>\file:\opt\electron: ENOENT …`. The caller's input was rewritten and they are pointed at the wrong location. - After: `invalid_request` on `executable_path` with the shared schema message "Electron executable, application, and root paths must be absolute local filesystem paths …". No filesystem lookup of an invented path happens. Violated invariant (AGENTS.md): absolute paths, file URLs and HTTP paths have different semantics, and caller-selected inputs must be preserved. ## Change and scope - `src/cliJsonInput.ts`: `resolveCliJsonPaths` leaves values matching `file:` or `scheme://` unchanged, so the shared schema's existing absolute-local-path refinement rejects them. One-letter prefixes stay paths, so Windows drive paths (`C:\…`) and relative names containing a colon (`release:v1/electron`) still resolve exactly as before. - This is the single CLI owner of operator-relative path normalization, used by `capture-electron-scenario` and the browser scenario `browser.executable_path`, so both get the fix. - Not included: file URLs are not converted to paths. They are rejected, as the issue's restoration check asks, because the MCP contract defines these fields as local filesystem paths. Contribution scope: ordinary CLI input fix; maintainer decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: CLI JSON path normalization before shared schema validation. - Contribution scope fit and linked maintainer decision: not applicable. - CLI and MCP/tool-catalog contract: no schema change. URL inputs to CLI path fields now get the documented field error instead of an ENOENT for an invented path. MCP is unchanged; it never resolved relative values. - Provider, bridge, target-format, or platform compatibility: none. Windows drive paths are covered by a test. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: URL inputs are rejected before any filesystem lookup or Electron launch. - Generated metadata, package, or installation impact: 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. None of these apply: input validation only. ## Evidence and regression coverage - `tests/acceptance/applications/electronInvalidPaths.test.ts`, "rejects a file URL path field without resolving an invented local path": runs the compiled CLI. It asserts exit code 1, `invalid_request` on `executable_path` with the local-path message, and no ENOENT. - `src/cliJsonPaths.test.ts`: `file:///opt/electron`, `FILE:electron` and `https://example.test/app` stay unchanged and are rejected by `electronActiveObservationInputSchema` on `executable_path`. Controls `bin/electron`, `release:v1/electron` and `C:\Electron\electron.exe` still resolve. - Base reproduction: with only `src/cliJsonInput.ts` reverted, the 3 URL unit cases and the CLI case fail. The CLI received `Cannot resolve executable_path C:\…\rea\file:\opt\electron: ENOENT`. The controls pass both before and after. - Representative CLI output after the fix: `{"code":"invalid_request", … "issues":[{"path":["executable_path"], … "message":"Electron executable, application, and root paths must be absolute local filesystem paths (for example /Applications/Electron.app/Contents/MacOS/Electron)"}]}` - Remaining proof gaps: verified on Windows only; POSIX behaviour follows the same regex, and the issue reproduced on macOS. ## Validation performed Windows 11, Node.js 24.18.0, npm 11.16.0, on `main` at `688defca0`: - `npm run test:local -- src/cliJsonPaths.test.ts`: 3 failed without the fix; 13/13 with it. - `npm run test:focused -- tests/acceptance/applications/electronInvalidPaths.test.ts`: the new case failed without the fix; 2/2 with it. - `npm run test:focused --` the 38 test files referencing Electron/browser scenario paths or `executable_path`: the same 23 tests fail with and without this change, which makes them pre-existing on Windows. For example, `scenarioCliInputDiagnostics` looks for raw `C:\…` paths inside JSON-escaped output. The only differences between the runs are the 4 new tests, which fail without the fix and pass with it. - `npm run check:fast` (typecheck and lint): passed, including as the pre-push hook. - `npm run verify:module-boundaries` and `oxfmt --check` on changed files: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Previously a URL in these fields always failed, with a misleading ENOENT; it now fails with the correct field error. - Real Hopper/Ghidra, browser, or OS coverage: Windows; no Electron or browser launch is involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: prevents a rewritten filesystem lookup; no new execution. ## 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. 🤖 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:06:53 +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#5466
No description provided.