[PR #2211] [MERGED] fix(frida): reject an unreadable script file before starting a session #5086

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

📋 Pull Request Information

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

Base: main ← Head: fix/issue-2209


📝 Commits (3)

  • e0ed313 fix(frida): reject an unreadable script file before starting a session
  • 4046536 fix(frida): let cancellation win over the script file check
  • 6116a9d refactor(frida): keep the pre-start checks out of instrument

📊 Changes

2 files changed (+127 additions, -0 deletions)

View changed files

📝 src/application/frida/FridaInstrumentationService.test.ts (+63 -0)
📝 src/application/frida/FridaInstrumentationService.ts (+64 -0)

📄 Description

Summary

FridaInstrumentationService.instrument, the shared CLI (instrument-with-frida) and MCP (instrument_with_frida) one-shot workflow, starts the Frida session before the provider reads a file script source. If the file is missing:

  • the session spawns the target paused (or attaches);
  • loadScript fails;
  • cleanup closes the session, and by design a paused spawn is resumed (REA never kills the target).

So the target program ran without instrumentation, and the caller got a bare invalid_request with no issue.

The workflow now checks that a file source is a readable regular file before #startSession:

  • a missing path or a non-file returns invalid_request with an invalid_value issue at ["path"] (Cannot read Frida script file <path>: no file exists there. / it is not a regular file.);
  • EACCES/EPERM returns access_denied.

Inline sources are unchanged. The provider still reads the file when loading, so a file that changes between the check and the load is handled as before.

Fixes #2209

Before / after (macOS, Frida via rea-agents; target is a compiled program that appends to a marker file when it runs)

$ rea instrument-with-frida spawn …/fr/target --script-file /nonexistent.js --duration-ms 300 --format json
before: {"code":"invalid_request", …}  — no issues;  marker file: "ran 96778" (target executed)
after:  {"code":"invalid_request","details":{"issues":[{"path":["path"],"reason":"invalid_value","message":"Cannot read Frida script file /nonexistent.js: no file exists there."}]}}  — marker file absent

Validation

  • New service tests (src/application/frida/FridaInstrumentationService.test.ts) cover a missing file and a directory. They assert the issue and that the provider's startSession is never called. They fail on main and pass with this change; all Frida tests pass (64 across the CLI, MCP, manager, service and contract suites).
  • npm run lint passes.
  • npm test: 9246 passed, 14 failed in 7 files. These are the same environmental files that fail on clean origin/main on this host: Hopper not installed, Command Line Tools only (ibtool), hdiutil, and inspector target-location cases.
  • Real-provider scope: Frida on this macOS host can spawn a self-built binary but generally cannot attach (Unable to access process … from the current user account). Instrumentation itself was not exercised here, only the pre-start rejection and the absence of the target run.

Remaining

The separate MCP load_frida_script operation still reports a file read failure as AnalysisInputError("load_frida_script") without an issue naming the path, from FridaInstrumentationManager.#loadScript. There the caller has already started the session explicitly, so no side effect is introduced by REA. I left it out to keep this change to the pre-start ordering.

🤖 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/2211 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/12/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/issue-2209` --- ### 📝 Commits (3) - [`e0ed313`](https://github.com/morluto/rea/commit/e0ed313d4068de15604fcc532358ec46e981785f) fix(frida): reject an unreadable script file before starting a session - [`4046536`](https://github.com/morluto/rea/commit/4046536a837f2877afb119dc49ff1f18070d6a01) fix(frida): let cancellation win over the script file check - [`6116a9d`](https://github.com/morluto/rea/commit/6116a9def574ac88152f22dbc0523727b8f38710) refactor(frida): keep the pre-start checks out of instrument ### 📊 Changes **2 files changed** (+127 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/application/frida/FridaInstrumentationService.test.ts` (+63 -0) 📝 `src/application/frida/FridaInstrumentationService.ts` (+64 -0) </details> ### 📄 Description ## Summary `FridaInstrumentationService.instrument`, the shared CLI (`instrument-with-frida`) and MCP (`instrument_with_frida`) one-shot workflow, starts the Frida session before the provider reads a `file` script source. If the file is missing: - the session spawns the target paused (or attaches); - `loadScript` fails; - cleanup closes the session, and by design a paused spawn is resumed (REA never kills the target). So **the target program ran without instrumentation**, and the caller got a bare `invalid_request` with no issue. The workflow now checks that a `file` source is a readable regular file before `#startSession`: - a missing path or a non-file returns `invalid_request` with an `invalid_value` issue at `["path"]` (`Cannot read Frida script file <path>: no file exists there.` / `it is not a regular file.`); - EACCES/EPERM returns `access_denied`. Inline sources are unchanged. The provider still reads the file when loading, so a file that changes between the check and the load is handled as before. Fixes #2209 ## Before / after (macOS, Frida via rea-agents; `target` is a compiled program that appends to a marker file when it runs) ```text $ rea instrument-with-frida spawn …/fr/target --script-file /nonexistent.js --duration-ms 300 --format json before: {"code":"invalid_request", …} — no issues; marker file: "ran 96778" (target executed) after: {"code":"invalid_request","details":{"issues":[{"path":["path"],"reason":"invalid_value","message":"Cannot read Frida script file /nonexistent.js: no file exists there."}]}} — marker file absent ``` ## Validation - New service tests (`src/application/frida/FridaInstrumentationService.test.ts`) cover a missing file and a directory. They assert the issue and that the provider's `startSession` is never called. They fail on main and pass with this change; all Frida tests pass (64 across the CLI, MCP, manager, service and contract suites). - `npm run lint` passes. - `npm test`: 9246 passed, 14 failed in 7 files. These are the same environmental files that fail on clean `origin/main` on this host: Hopper not installed, Command Line Tools only (`ibtool`), `hdiutil`, and inspector target-location cases. - Real-provider scope: Frida on this macOS host can spawn a self-built binary but generally cannot attach (`Unable to access process … from the current user account`). Instrumentation itself was not exercised here, only the pre-start rejection and the absence of the target run. ## Remaining The separate MCP `load_frida_script` operation still reports a file read failure as `AnalysisInputError("load_frida_script")` without an issue naming the path, from `FridaInstrumentationManager.#loadScript`. There the caller has already started the session explicitly, so no side effect is introduced by REA. I left it out to keep this change to the pre-start ordering. 🤖 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:03:44 +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#5086
No description provided.