[PR #2215] [CLOSED] fix(frida): validate caller-selected script file before starting a session #5089

New issue

Closed

opened 2026-10-31 01:03:46 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:03:46 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2215
Author: @chqcjq
Created: 10/12/2026
Status: ❌ Closed

Base: main ← Head: fix/frida-preflight-script-file

📝 Commits (2)

3e392ab fix(frida): validate caller-selected script file before starting a session
b76d248 fix(frida): preserve file provenance, abort before preflight, isolate exit-code assertion

📊 Changes

4 files changed (+380 additions, -14 deletions)

View changed files

📝 src/application/frida/FridaInstrumentationService.test.ts (+162 -1)
📝 src/application/frida/FridaInstrumentationService.ts (+71 -12)
📝 tests/boundary/cli/fridaCli.test.ts (+83 -1)
📝 tests/boundary/mcp/fridaMcp.test.ts (+64 -0)

📄 Description

Fixes #2209.

Problem

instrument-with-frida spawn <program> --script-file <path> (and MCP instrument_with_frida with source_kind: "file") started the Frida session before reading the caller-selected script file. If the file did not exist, REA spawned the target paused, loadScript failed to read the file, and the cleanup path resumed the paused spawn by design — so the target program ran without any instrumentation, and the caller received a bare invalid_request with no issue naming the script path.

Fix

FridaInstrumentationService.instrument now reads a source_kind: "file" script before any device, attach, or spawn effect. The file content is then passed through the existing inline path unchanged.

Missing/unreadable file -> AnalysisInputError with issues: [{ path: ["path"], reason: "invalid_value", message: <path> }], and the session is never started.
EACCES/EPERM -> AnalysisAccessDeniedError (access_denied) naming the path, and the session is never started.
The reader is injectable so the access-denied mapping is tested deterministically on every platform.

Verification

Service (FridaInstrumentationService.test.ts, 14 passed): missing file rejected with zero session/script/resume/close calls; existing file is read once and loaded as inline; access denied maps to access_denied before any session effect.
CLI boundary (fridaCli.test.ts, 4 passed): --script-file <missing> returns invalid_request naming the path, sets process.exitCode = 1, and the provider only observes process-level cleanup — no session, script, or spawn.
MCP boundary (fridaMcp.test.ts, 4 passed): source_kind: "file" with a missing path returns isError: true / invalid_request naming the path with zero startSession calls.
Full Frida suite: 43/43 passed. npm run check, docs:check, and verify:test-discovery (923 files) all pass.
verify:frida requires the optional frida native binding, which is not installed on this host; that failure is environmental and unrelated to this change (the boundary tests stub the provider).

Local codex review was rate-limited (HTTP 429) twice and could not complete; the staged diff was reviewed by hand instead — no actionable findings.

_{🔄 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/2215 **Author:** [@chqcjq](https://github.com/chqcjq) **Created:** 10/12/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/frida-preflight-script-file` --- ### 📝 Commits (2) - [`3e392ab`](https://github.com/morluto/rea/commit/3e392ab29ccbb2d351c3c9c15857935ad7448353) fix(frida): validate caller-selected script file before starting a session - [`b76d248`](https://github.com/morluto/rea/commit/b76d248f46b5c7d5cfb9daa09cdebb90f7be2adb) fix(frida): preserve file provenance, abort before preflight, isolate exit-code assertion ### 📊 Changes **4 files changed** (+380 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `src/application/frida/FridaInstrumentationService.test.ts` (+162 -1) 📝 `src/application/frida/FridaInstrumentationService.ts` (+71 -12) 📝 `tests/boundary/cli/fridaCli.test.ts` (+83 -1) 📝 `tests/boundary/mcp/fridaMcp.test.ts` (+64 -0) </details> ### 📄 Description Fixes #2209. ## Problem `instrument-with-frida spawn <program> --script-file <path>` (and MCP `instrument_with_frida` with `source_kind: "file"`) started the Frida session before reading the caller-selected script file. If the file did not exist, REA spawned the target paused, `loadScript` failed to read the file, and the cleanup path resumed the paused spawn by design — so **the target program ran without any instrumentation**, and the caller received a bare `invalid_request` with no issue naming the script path. ## Fix `FridaInstrumentationService.instrument` now reads a `source_kind: "file"` script **before** any device, attach, or spawn effect. The file content is then passed through the existing inline path unchanged. - Missing/unreadable file -> `AnalysisInputError` with `issues: [{ path: ["path"], reason: "invalid_value", message: <path> }]`, and the session is never started. - `EACCES`/`EPERM` -> `AnalysisAccessDeniedError` (`access_denied`) naming the path, and the session is never started. - The reader is injectable so the access-denied mapping is tested deterministically on every platform. ## Verification - **Service** (`FridaInstrumentationService.test.ts`, 14 passed): missing file rejected with zero session/script/resume/close calls; existing file is read once and loaded as inline; access denied maps to `access_denied` before any session effect. - **CLI boundary** (`fridaCli.test.ts`, 4 passed): `--script-file <missing>` returns `invalid_request` naming the path, sets `process.exitCode = 1`, and the provider only observes process-level cleanup — no session, script, or spawn. - **MCP boundary** (`fridaMcp.test.ts`, 4 passed): `source_kind: "file"` with a missing path returns `isError: true` / `invalid_request` naming the path with zero `startSession` calls. - Full Frida suite: 43/43 passed. `npm run check`, `docs:check`, and `verify:test-discovery` (923 files) all pass. - `verify:frida` requires the optional `frida` native binding, which is not installed on this host; that failure is environmental and unrelated to this change (the boundary tests stub the provider). Local `codex review` was rate-limited (HTTP 429) twice and could not complete; the staged diff was reviewed by hand instead — no actionable findings. --- <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:46 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns