morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #3356] test(cli): cover Frida argument admission and spawn ordering #5720

New issue

Open

opened 2026-10-31 01:08:54 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:08:54 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3356
Author: @1052326311
Created: 10/14/2026
Status: 🔄 Open

Base: main ← Head: fix/frida-attach-arguments

📝 Commits (4)

d30dfa4 fix(cli): reject spawn arguments in Frida attach mode
5857962 Merge remote-tracking branch 'origin/main' into sweep/3356
a1c5adf test(cli): remove redundant Frida recorder assignments
786d725 Merge remote-tracking branch 'origin/main' into HEAD

📊 Changes

1 file changed (+66 additions, -8 deletions)

View changed files

📝 tests/boundary/cli/fridaCli.test.ts (+66 -8)

📄 Description

Summary

Add distinct Frida CLI regression coverage requested in the maintainer scope review. The attach-mode --argument rejection already landed independently on main in 45d5e801; this PR's net diff is test-only. Related issue #3184 is already closed and is not being claimed as a new fix.

Problem and expected behavior

Current main covers a nonempty attach argument. Preserve the additional boundaries: an explicitly empty --argument '' must also be rejected before any provider method is called, while repeated spawn arguments must reach the shared service in their original order. These assertions protect existing CLI admission and argument forwarding.

Change and scope

Extract the existing in-memory Frida recorder for reuse; add ordered spawn forwarding and empty-string attach controls.
For all invalid-input cases, assert that no provider operation is invoked, rather than checking only startSession.
Retain main's nonempty attach case and canonical remediation; remove the redundant stdout/exitCode assignments noted in review.

The semantic owner remains src/cli/fridaCommands.ts; no production, CLI/MCP catalog, provider, Evidence, package or generated metadata contract changes. This is existing-boundary regression coverage; no new capability or scope decision is required. Tests execute the real command/shared-service path with an in-memory provider. They do not launch, attach, inject or contact a real target.

Evidence and validation

Current head a1c5adfe2c9395573d9675f501437095a359ade2, retaining the maintainer's merge 5857962b and its source resolution:

npm run test:focused -- tests/boundary/cli/fridaCli.test.ts — 8 passed on macOS arm64, pinned Node 24.18.0/npm 11.16.0.
Executed mutation controls: allowing only empty attach arguments through admission makes precisely that case fail (1 failed / 7 passed); reversing forwarded spawn arguments makes precisely the spawn case fail (1 failed / 7 passed). Both source mutations were restored; the final source again passes all 8 tests.
npm run check, npm run docs:check (including 5 README checks), and git diff --check — pass. Staged content contains only the CLI test file; no secrets/generated artifacts.
Current main 55cef256 was compared: the ordered-spawn/empty-attach/all-provider controls remain absent; the production rejection is already present. The net feature patch remains confined to this test file. Hosted verification for this exact head is now complete: CI required passed, including static checks, source tests, package build, all five test shards and portable tests. Unselected documentation/provider/native lanes were skipped, not executed. Greptile’s updated review reports 5/5 on a1c5adfe, confirms the distinct tests are absent from the supplied base, and identifies no new security issue. The reviewer explicitly did not run tests; executed test evidence comes from the local/hosted runs above. There are no inline review threads. Automated results do not establish human readiness or grant merge permission.

No real Frida provider or Windows execution was performed; these tests establish CLI/service dispatch and invalid-input admission only. No performance or production-fix authorship/adoption claim is made.

Outcome correction

I previously closed this as a superseded production fix. That correctly identified the independently landed guard but missed the distinct test contribution explicitly requested above. This continuation narrows the title/body to that remaining test scope and preserves the maintainer's branch changes; it does not reintroduce the production fix or count the main commit as this PR's authored merge.

Author readiness

Human owner: @1052326311

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.

_{🔄 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/3356 **Author:** [@1052326311](https://github.com/1052326311) **Created:** 10/14/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/frida-attach-arguments` --- ### 📝 Commits (4) - [`d30dfa4`](https://github.com/morluto/rea/commit/d30dfa4207a96d11655abee2755b8a98c515c5da) fix(cli): reject spawn arguments in Frida attach mode - [`5857962`](https://github.com/morluto/rea/commit/5857962be4e0b6352e03222ab9e629ed5e002bbe) Merge remote-tracking branch 'origin/main' into sweep/3356 - [`a1c5adf`](https://github.com/morluto/rea/commit/a1c5adfe2c9395573d9675f501437095a359ade2) test(cli): remove redundant Frida recorder assignments - [`786d725`](https://github.com/morluto/rea/commit/786d72553478a49573dab5a44ad95502e2f5117a) Merge remote-tracking branch 'origin/main' into HEAD ### 📊 Changes **1 file changed** (+66 additions, -8 deletions) <details> <summary>View changed files</summary> 📝 `tests/boundary/cli/fridaCli.test.ts` (+66 -8) </details> ### 📄 Description ## Summary Add distinct Frida CLI regression coverage requested in [the maintainer scope review](https://github.com/morluto/rea/pull/3356#issuecomment-6186294979). The attach-mode `--argument` rejection already landed independently on main in `45d5e801`; this PR's net diff is **test-only**. Related issue #3184 is already closed and is not being claimed as a new fix. ## Problem and expected behavior Current main covers a nonempty attach argument. Preserve the additional boundaries: an explicitly empty `--argument ''` must also be rejected before **any** provider method is called, while repeated spawn arguments must reach the shared service in their original order. These assertions protect existing CLI admission and argument forwarding. ## Change and scope - Extract the existing in-memory Frida recorder for reuse; add ordered spawn forwarding and empty-string attach controls. - For all invalid-input cases, assert that no provider operation is invoked, rather than checking only `startSession`. - Retain main's nonempty attach case and canonical remediation; remove the redundant stdout/exitCode assignments noted in review. The semantic owner remains `src/cli/fridaCommands.ts`; no production, CLI/MCP catalog, provider, Evidence, package or generated metadata contract changes. This is existing-boundary regression coverage; no new capability or scope decision is required. Tests execute the real command/shared-service path with an in-memory provider. They do not launch, attach, inject or contact a real target. ## Evidence and validation Current head `a1c5adfe2c9395573d9675f501437095a359ade2`, retaining the maintainer's merge `5857962b` and its source resolution: - `npm run test:focused -- tests/boundary/cli/fridaCli.test.ts` — **8 passed** on macOS arm64, pinned Node 24.18.0/npm 11.16.0. - Executed mutation controls: allowing only empty attach arguments through admission makes precisely that case fail (1 failed / 7 passed); reversing forwarded spawn arguments makes precisely the spawn case fail (1 failed / 7 passed). Both source mutations were restored; the final source again passes all 8 tests. - `npm run check`, `npm run docs:check` (including 5 README checks), and `git diff --check` — pass. Staged content contains only the CLI test file; no secrets/generated artifacts. - Current main `55cef256` was compared: the ordered-spawn/empty-attach/all-provider controls remain absent; the production rejection is already present. The net feature patch remains confined to this test file. Hosted verification for this exact head is now complete: [CI required](https://github.com/morluto/rea/actions/runs/38726259351) passed, including static checks, source tests, package build, all five test shards and portable tests. Unselected documentation/provider/native lanes were skipped, not executed. [Greptile’s updated review](https://github.com/morluto/rea/pull/3356#issuecomment-6185848372) reports 5/5 on `a1c5adfe`, confirms the distinct tests are absent from the supplied base, and identifies no new security issue. The reviewer explicitly did not run tests; executed test evidence comes from the local/hosted runs above. There are no inline review threads. Automated results do not establish human readiness or grant merge permission. No real Frida provider or Windows execution was performed; these tests establish CLI/service dispatch and invalid-input admission only. No performance or production-fix authorship/adoption claim is made. ## Outcome correction I previously closed this as a superseded production fix. That correctly identified the independently landed guard but missed the distinct test contribution explicitly requested above. This continuation narrows the title/body to that remaining test scope and preserves the maintainer's branch changes; it does not reintroduce the production fix or count the main commit as this PR's authored merge. ## Author readiness  Human owner: @1052326311 - [ ] 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-31 01:08:54 +00:00

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

No description provided.

Rows
Columns