morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

[GH-ISSUE #5864] [Bug] instrument-with-frida CLI wraps Evidence instead of returning the documented record #3553

New issue

Closed

opened 2026-10-31 00:41:48 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:41:48 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 21, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/5864

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

CLI

Observed behavior

A successful instrument-with-frida ... --json call returns an outer {evidence, cleanup_error} object instead of the documented Evidence record. The top-level response fails the shared instrument_with_frida output schema; its nested evidence passes. This was executed through the actual CLI and shared service with a simulated Frida provider. No native binding or target process ran.

Reproduction

From the source checkout with locked dependencies and existing runtime artifacts, save the following as tests/process-global/repro.test.ts and run:

npx --no-install vitest run tests/process-global/repro.test.ts --maxWorkers 1

This is the Frida portion of the focused test that ran. It includes a successful cleanup control and a separate cleanup-failure case. Assertions describe the currently observed behavior. Delete the temporary file after running it.

import { createHash } from "node:crypto";
import { expect, it } from "vitest";
import { createCli } from "../../src/cli.js";
import { toolContract } from "../../src/contracts/toolContracts.js";
import { FridaInstrumentationManager } from "../../src/frida/FridaInstrumentationManager.js";
import { ProviderCleanupError } from "../../src/domain/providerCleanupError.js";

async function run(args: string[], provider?: FridaInstrumentationManager) {
  let stdout = '';
  try {
    await createCli({}, undefined, provider).serve(args, {
      env: {}, exit: () => {}, stdout: (s) => { stdout += s; },
    });
    return JSON.parse(stdout);
  } finally { process.exitCode = 0; }
}

it.each([false, true])('Frida result shape with final cleanup failure=%s', async (fails) => {
  const provider = new FridaInstrumentationManager();
  provider.startSession = async () => ({ ok: true, value: {
    sessionId: '00000000-0000-4000-8000-000000000001', deviceId: 'local',
    target: 'fixture', pid: 71, mode: 'attach', state: 'running',
  }});
  provider.loadScript = async () => ({ ok: true, value: {
    scriptId: '00000000-0000-4000-8000-000000000002', sourceKind: 'inline', sourcePath: null,
    sourceSha256: createHash('sha256').update('send(1)').digest('hex'),
    messages: [{ type: 'send', payload: 1 }], messagesTruncated: false,
  }});
  provider.status = () => undefined;
  provider.closeSession = async () => fails
    ? { ok: false, error: new ProviderCleanupError('frida', ['fixture session'], {}, { operation: 'close_frida_session' }) }
    : { ok: true, value: { targetResumeObserved: false } };
  provider.closeAll = async () => { if (fails) throw new Error('controlled closeAll failure'); };
  const output = await run(['instrument-with-frida', 'attach', '71', '--script', 'send(1)', '--duration-ms', '0', '--json'], provider);
  if (!fails) {
    expect(output.evidence_id).toBeUndefined();
    expect(toolContract('instrument_with_frida').outputSchema.safeParse(output).success).toBe(false);
    expect(toolContract('instrument_with_frida').outputSchema.safeParse(output.evidence).success).toBe(true);
    console.log('FRIDA_SUCCESS', JSON.stringify({ keys: Object.keys(output), nestedEvidenceId: output.evidence.evidence_id, cleanup: output.cleanup_error }));
  } else {
    expect(output.code).toBe('cleanup_incomplete');
    expect(output.details.partial_observation).toBeUndefined();
    const retained = output.details.diagnostics.partial_observation;
    expect(toolContract('instrument_with_frida').outputSchema.safeParse(retained.evidence).success).toBe(true);
    console.log('FRIDA_CLEANUP', JSON.stringify({ code: output.code, detailsKeys: Object.keys(output.details), retainedKeys: Object.keys(retained), nestedEvidenceId: retained.evidence.evidence_id }));
  }
});

Expected behavior

Return the Evidence record at the top level, including evidence_id and normalized_result. Preserve cleanup information already present at normalized_result.cleanup_error. The CLI guide establishes shared Evidence contracts, the Frida guide explicitly says this one-shot CLI returns Evidence, and the named output schema describes the Evidence result.

Impact and scope

P2: a JSON consumer following the documented Evidence shape cannot find evidence_id or normalized_result at the top level. The completed observation survives intact under .evidence; extracting that field is a demonstrated workaround. The successful cleanup case reproduced twice in the focused runs. Real Frida compatibility and frequency were not measured. No Evidence consumer command or import was exercised.

REA version

rea-agents 6.7.0; source revision 5241597594. Fetched main f801b2fbfd has identical relevant code.

Runtime and platform

Node.js v26.12.0; macOS 27.0.1 arm64; Vitest 5.0.3. The repository-pinned Node 24.18.0 was not exercised. Simulated Frida provider only.

Target and artifact context

Synthetic local attach session, PID 71, inline send(1) script, zero-duration observation, one captured send message. No real process was attached.

Interface

CLI

Earliest failure stage

Transport or result projection

Relevant output or evidence

The successful case printed:

{"keys":["evidence","cleanup_error"],"nestedEvidenceId":"ev_9a9151f9df5b0249758677e968e0579a1bbdbf629d0dda50ab60ac972fa21d50","cleanup":null}

The schema assertions rejected the outer response and accepted output.evidence. The complete focused run passed seven cases in one test file; the other cases checked final cleanup diagnostics and JavaScript owner cleanup. These are source CLI executions, not a spawned release executable or MCP parity run.

Typed outcome or recovery expected

The normal successful response should pass the named Evidence output schema without caller unwrapping. Preserve the completed messages, source digest, cleanup status and any existing nonzero exit behavior for incomplete cleanup.

Analysis or suspected cause

Source-derived: the CLI handler manually wraps result.value.evidence. The MCP handler passes that Evidence directly to delivery. The shared service already embeds cleanup_error inside the Evidence normalized result.

Additional context

The related closed device-list report #4637 addressed deviceId versus device_id in another command; its fix does not change this wrapper. The separate final-cleanup diagnostic placement case in the test concerns failure output and needs a different restoration check. NOT_ENOUGH_INFO: no regression range or real-provider failure frequency was established.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5864 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area CLI ### Observed behavior A successful `instrument-with-frida ... --json` call returns an outer `{evidence, cleanup_error}` object instead of the documented Evidence record. The top-level response fails the shared `instrument_with_frida` output schema; its nested `evidence` passes. This was executed through the actual CLI and shared service with a simulated Frida provider. No native binding or target process ran. ### Reproduction From the source checkout with locked dependencies and existing runtime artifacts, save the following as `tests/process-global/repro.test.ts` and run: ```sh npx --no-install vitest run tests/process-global/repro.test.ts --maxWorkers 1 ``` This is the Frida portion of the focused test that ran. It includes a successful cleanup control and a separate cleanup-failure case. Assertions describe the currently observed behavior. Delete the temporary file after running it. ```ts import { createHash } from "node:crypto"; import { expect, it } from "vitest"; import { createCli } from "../../src/cli.js"; import { toolContract } from "../../src/contracts/toolContracts.js"; import { FridaInstrumentationManager } from "../../src/frida/FridaInstrumentationManager.js"; import { ProviderCleanupError } from "../../src/domain/providerCleanupError.js"; async function run(args: string[], provider?: FridaInstrumentationManager) { let stdout = ''; try { await createCli({}, undefined, provider).serve(args, { env: {}, exit: () => {}, stdout: (s) => { stdout += s; }, }); return JSON.parse(stdout); } finally { process.exitCode = 0; } } it.each([false, true])('Frida result shape with final cleanup failure=%s', async (fails) => { const provider = new FridaInstrumentationManager(); provider.startSession = async () => ({ ok: true, value: { sessionId: '00000000-0000-4000-8000-000000000001', deviceId: 'local', target: 'fixture', pid: 71, mode: 'attach', state: 'running', }}); provider.loadScript = async () => ({ ok: true, value: { scriptId: '00000000-0000-4000-8000-000000000002', sourceKind: 'inline', sourcePath: null, sourceSha256: createHash('sha256').update('send(1)').digest('hex'), messages: [{ type: 'send', payload: 1 }], messagesTruncated: false, }}); provider.status = () => undefined; provider.closeSession = async () => fails ? { ok: false, error: new ProviderCleanupError('frida', ['fixture session'], {}, { operation: 'close_frida_session' }) } : { ok: true, value: { targetResumeObserved: false } }; provider.closeAll = async () => { if (fails) throw new Error('controlled closeAll failure'); }; const output = await run(['instrument-with-frida', 'attach', '71', '--script', 'send(1)', '--duration-ms', '0', '--json'], provider); if (!fails) { expect(output.evidence_id).toBeUndefined(); expect(toolContract('instrument_with_frida').outputSchema.safeParse(output).success).toBe(false); expect(toolContract('instrument_with_frida').outputSchema.safeParse(output.evidence).success).toBe(true); console.log('FRIDA_SUCCESS', JSON.stringify({ keys: Object.keys(output), nestedEvidenceId: output.evidence.evidence_id, cleanup: output.cleanup_error })); } else { expect(output.code).toBe('cleanup_incomplete'); expect(output.details.partial_observation).toBeUndefined(); const retained = output.details.diagnostics.partial_observation; expect(toolContract('instrument_with_frida').outputSchema.safeParse(retained.evidence).success).toBe(true); console.log('FRIDA_CLEANUP', JSON.stringify({ code: output.code, detailsKeys: Object.keys(output.details), retainedKeys: Object.keys(retained), nestedEvidenceId: retained.evidence.evidence_id })); } }); ``` ### Expected behavior Return the Evidence record at the top level, including evidence_id and normalized_result. Preserve cleanup information already present at normalized_result.cleanup_error. The [CLI guide](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/cli.md#L3-L5) establishes shared Evidence contracts, the [Frida guide](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/frida-instrumentation.md#L7) explicitly says this one-shot CLI returns Evidence, and the [named output schema](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/contracts/fridaToolContracts.ts#L291-L298) describes the Evidence result. ### Impact and scope P2: a JSON consumer following the documented Evidence shape cannot find evidence_id or normalized_result at the top level. The completed observation survives intact under .evidence; extracting that field is a demonstrated workaround. The successful cleanup case reproduced twice in the focused runs. Real Frida compatibility and frequency were not measured. No Evidence consumer command or import was exercised. ### REA version rea-agents 6.7.0; source revision 52415975942a4b9c53a817915b313ccc404f280e. Fetched main f801b2fbfd28f4e3713adefddc7bd73db44c284c has identical relevant code. ### Runtime and platform Node.js v26.12.0; macOS 27.0.1 arm64; Vitest 5.0.3. The repository-pinned Node 24.18.0 was not exercised. Simulated Frida provider only. ### Target and artifact context Synthetic local attach session, PID 71, inline send(1) script, zero-duration observation, one captured send message. No real process was attached. ### Interface CLI ### Earliest failure stage Transport or result projection ### Relevant output or evidence The successful case printed: ```json {"keys":["evidence","cleanup_error"],"nestedEvidenceId":"ev_9a9151f9df5b0249758677e968e0579a1bbdbf629d0dda50ab60ac972fa21d50","cleanup":null} ``` The schema assertions rejected the outer response and accepted output.evidence. The complete focused run passed seven cases in one test file; the other cases checked final cleanup diagnostics and JavaScript owner cleanup. These are source CLI executions, not a spawned release executable or MCP parity run. ### Typed outcome or recovery expected The normal successful response should pass the named Evidence output schema without caller unwrapping. Preserve the completed messages, source digest, cleanup status and any existing nonzero exit behavior for incomplete cleanup. ### Analysis or suspected cause Source-derived: the [CLI handler](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/cli/fridaCommands.ts#L291-L296) manually wraps result.value.evidence. The [MCP handler](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/server/registerFridaTools.ts#L285-L289) passes that Evidence directly to delivery. The shared service already embeds cleanup_error inside the Evidence normalized result. ### Additional context The related closed device-list report #4637 addressed deviceId versus device_id in another command; its fix does not change this wrapper. The separate final-cleanup diagnostic placement case in the test concerns failure output and needs a different restoration check. NOT_ENOUGH_INFO: no regression range or real-provider failure frequency was established.

bsakaguchi closed this issue

2026-10-31 00:41:48 +00:00

bsakaguchi commented

2026-10-31 00:41:49 +00:00

Author

Owner

@coygeek commented on GitHub (Oct 24, 2026):

At d87e73f2db50be72d33a4b3c7199af35518b441c, the CLI returns result.value.evidence directly. The one-shot CLI test parses the entire successful JSON response against the named instrument_with_frida Evidence schema, covering the reported wrapper mismatch. Finalization assertions also validate retained Evidence during interrupted cleanup.

I ran the cited regression files on this revision; all passed as part of the focused 28-file, 603-test run. Provider-facing checks here use controlled fixtures; this does not claim a run with the real analysis engine. Closing as completed on main.

@coygeek commented on GitHub (Oct 24, 2026): At `d87e73f2db50be72d33a4b3c7199af35518b441c`, the CLI [returns `result.value.evidence` directly](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/cli/fridaCommands.ts#L266-L287). The [one-shot CLI test](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/boundary/cli/fridaCli.test.ts#L184-L220) parses the entire successful JSON response against the named `instrument_with_frida` Evidence schema, covering the reported wrapper mismatch. [Finalization assertions](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/process-global/fridaFinalizationCli.test.ts#L90-L107) also validate retained Evidence during interrupted cleanup. I ran the cited regression files on this revision; all passed as part of the focused 28-file, 603-test run. Provider-facing checks here use controlled fixtures; this does not claim a run with the real analysis engine. Closing as completed on main.

bsakaguchi referenced this issue

2026-10-31 01:09:40 +00:00

[PR #3555] [MERGED] fix(browser): bound CDP handshakes and command responses #5820

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

No description provided.

Rows
Columns