morluto/rea

Fork 0

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

[GH-ISSUE #6072] [Bug] Frida CLI omits typed cancellation during final provider cleanup #3714

New issue

Closed

opened 2026-10-31 00:43:18 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:43:18 +00:00

Owner

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

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

instrument-with-frida does not emit typed cancellation when SIGINT or SIGTERM arrives during the final CLI-owned provider.closeAll() await. In a controlled provider integration, it returns the same {evidence, cleanup_error: null} payload as an uncancelled run while process.exitCode becomes 130 or 143.

An additional test uses the production Frida manager with simulated native objects: an initial detach fails, final provider cleanup retries detach, SIGINT arrives during that retry, and retry then succeeds. The command still returns Evidence plus the earlier cleanup diagnostic, without code: "cancelled", with exit status 130. No actual Frida binding or target process ran.

Reproduction

Save the following as tests/process-global/fridaFinalCleanupRepro.test.ts in the prepared source checkout. Run node node_modules/vitest/vitest.mjs run tests/process-global/fridaFinalCleanupRepro.test.ts --maxWorkers 1, then remove it. These are the executed Frida cases; assertions characterize current behavior. The first three cases substitute provider-port results to isolate clean finalization. The last two use the unmodified production manager and control only native handles. Signals are dispatched synchronously to the process signal listeners with process.emit, after explicit cleanup-entry barriers.

import { createHash } from 'node:crypto';
import { expect, it } from 'vitest';
import { createCli } from '../../src/cli.js';
import { FridaInstrumentationManager } from '../../src/frida/FridaInstrumentationManager.js';

it.each([undefined, 'SIGINT', 'SIGTERM'] as const)('Frida final close signal %s', async (signal) => {
  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;
  let sessionClosed = false;
  provider.closeSession = async () => { sessionClosed = true; return {ok:true,value:{targetResumeObserved:false}}; };
  let begin!: () => void, release!: () => void;
  const started = new Promise<void>(r => {begin=r;});
  const barrier = new Promise<void>(r => {release=r;});
  provider.closeAll = async () => { expect(sessionClosed).toBe(true); begin(); await barrier; };
  let stdout='';
  const savedExitCode=process.exitCode;
  try {
    process.exitCode=0;
    const pending=createCli({},undefined,provider).serve(['instrument-with-frida','attach','71','--script','send(1)','--duration-ms','0','--json'],{env:{},exit:()=>{},stdout:s=>{stdout+=s;}});
    await started;
    if (signal !== undefined) process.emit(signal);
    release();
    await pending;
    const output=JSON.parse(stdout);
    console.log('FRIDA',JSON.stringify({signal:signal??'none',keys:Object.keys(output),code:output.code??null,cleanup:output.cleanup_error,processExitCode:process.exitCode,evidenceId:output.evidence?.evidence_id,messages:output.evidence?.normalized_result?.messages}));
    expect(output.code).toBeUndefined();
    expect(output.evidence.evidence_id).toBeTruthy();
    expect(output.cleanup_error).toBeNull();
    expect(process.exitCode).toBe(signal==='SIGINT'?130:signal==='SIGTERM'?143:0);
  } finally { release(); process.exitCode=savedExitCode; }
});

it.each([undefined, 'SIGINT'] as const)('production Frida manager retries final detach; signal %s', async signal => {
  let begin!: () => void, release!: () => void;
  const entered = new Promise<void>(r => { begin = r; });
  const barrier = new Promise<void>(r => { release = r; });
  let detachCalls=0, detached=false, unloads=0;
  const native = {
    pid:71, detached:{connect(){},disconnect(){}}, isDetached:()=>detached,
    async createScript(){let destroyed=false;let callback:any;return {
      get isDestroyed(){return destroyed;},message:{connect(fn:any){callback=fn;},disconnect(){}},
      async load(){callback?.({type:'send',payload:1},null);},async unload(){unloads++;destroyed=true;}
    };},
    async detach(){detachCalls++;if(detachCalls===1)throw new Error('transient detach failure');begin();await barrier;detached=true;}
  };
  const device={id:'local',name:'fixture',type:'local',async getProcessByPid(){return {pid:71,name:'fixture'};},async attach(){return native;}};
  const provider=new FridaInstrumentationManager({deviceManager:{async getDeviceById(){return device;}} as any});
  let stdout='';const saved=process.exitCode;
  try {
    process.exitCode=0;
    const pending=createCli({},undefined,provider).serve(['instrument-with-frida','attach','71','--script','send(1)','--duration-ms','0','--json'],{env:{},exit:()=>{},stdout:s=>{stdout+=s;}});
    await entered;
    if(signal)process.emit(signal);
    release();await pending;
    const output=JSON.parse(stdout);
    console.log('FRIDA_REAL_MANAGER',JSON.stringify({signal:signal??'none',code:output.code??null,cleanup:output.cleanup_error,processExitCode:process.exitCode,detachCalls,detached,unloads,messages:output.evidence?.normalized_result?.messages}));
    expect(output.code).toBeUndefined();expect(output.evidence).toBeDefined();
    expect(output.cleanup_error).toContain('transient detach failure');
    expect(detachCalls).toBe(2);expect(detached).toBe(true);expect(unloads).toBe(1);
    expect(process.exitCode).toBe(signal?130:1);
  } finally {release();await provider.closeAll();process.exitCode=saved;}
});

Expected behavior

The cancellable-observation contract requires a final signal check after awaited finalization and retention of collected observations under typed cancellation. The one-shot Frida command is an observation. Completed messages should survive; cancellation does not imply incomplete capture or undo script effects.

The guide’s committed-load cutoff concerns persistent script loading, where late cancellation must not unload a committed script. This command owns a one-shot observation and awaits disposable provider shutdown before returning. No atomic publication or persistent-load receipt supersedes that final check. Preserve any earlier primary error or unresolved cleanup classification. In the native-retry case, final cleanup has succeeded; keep the earlier cleanup diagnostic in the retained observation.

Impact and scope

P2: bounded completion-classification mismatch. A JSON consumer misses the documented cancelled outcome and its partial-observation channel even though the process status records a handled interrupt. Captured messages survive in the returned Evidence, and checking exit status distinguishes the interrupted cases in the test; no message loss or target damage is claimed. The prolonged clean-provider wait is simulated. The production-manager asynchronous case requires a transient initial cleanup failure that succeeds on retry. Real-device frequency and a prolonged completely clean native shutdown were not established.

REA version

rea-agents 6.7.0; source revision 52415975942a4b9c53a817915b313ccc404f280e.

Runtime and platform

macOS 27.0.1 arm64; Node.js v24.22.0; locked Vitest 5.0.3. The repository-pinned Node 24.18.0 was not exercised. Source CLI execution with simulated native/provider boundaries; no packaged subprocess or real-device qualification.

Target and artifact context

Synthetic local attach session, PID 71, zero-duration observation, inline send(1), one retained send message. The production manager creates/releases its own script/session identities over simulated native handles.

Interface

CLI

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Reduced observations from the completed run:

{"provider":"controlled","signal":"none","code":null,"cleanup_error":null,"process_exit_code":0,"message_retained":true}
{"provider":"controlled","signal":"SIGINT","code":null,"cleanup_error":null,"process_exit_code":130,"message_retained":true}
{"provider":"controlled","signal":"SIGTERM","code":null,"cleanup_error":null,"process_exit_code":143,"message_retained":true}
{"provider":"production-manager","signal":"none","code":null,"earlier_detach_diagnostic_retained":true,"process_exit_code":1,"detach_attempts":2,"detached":true,"unloads":1}
{"provider":"production-manager","signal":"SIGINT","code":null,"earlier_detach_diagnostic_retained":true,"process_exit_code":130,"detach_attempts":2,"detached":true,"unloads":1}

In each case code: null above denotes an absent top-level code, not a literal payload field. The interrupted outputs use the ordinary Evidence wrapper. The final focused run passed all seven tests, including these five Frida cases and two environment tests.

Typed outcome or recovery expected

After successful final cleanup, project AnalysisCancelledError for instrument_with_frida with the observed abort reason and collected Evidence as partial observation. Retain any earlier cleanup diagnostic and actual completed cleanup facts. If cleanup remains incomplete, preserve its classification and recovery information. Do not roll back a committed persistent script load or claim the target was terminated.

Analysis or suspected cause

withProviderCleanup returns its prior Result after awaiting closeAll without receiving/checking the signal. The command projects that Result as success. The shared service checks after closeSession, but cannot observe cancellation arriving during this later CLI await. The manager can retry retained native cleanup at that later boundary. The command cancellation wrapper only assigns the signal-derived exit status.

Additional context

Upstream main 8e3bf56e11bcb56d0e8754fdfa32a7a9f004ca13 has unchanged relevant implementation. Closed #5082 repaired the inner service’s final closeSession/native-detach check; this is the subsequent CLI closeAll boundary. #5864 concerns the successful wrapper shape and #5865 concerns partial-observation placement when final cleanup rejects. Neither adds this missing check. #5232 concerns preserving a native resume failure. The persistent-load fixes #2535/#2544/#3979 govern a different lifetime and cutoff. No repair or real-native scheduling frequency was measured.

Related but separate: #6048 concerns final provider-result consumption in native value tracing, and #6049 concerns Hopper regex matching-worker termination. This report concerns the Frida CLI-owned closeAll await; these producing boundaries require separate completion checks.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/6072 ### 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 `instrument-with-frida` does not emit typed cancellation when SIGINT or SIGTERM arrives during the final CLI-owned `provider.closeAll()` await. In a controlled provider integration, it returns the same `{evidence, cleanup_error: null}` payload as an uncancelled run while `process.exitCode` becomes 130 or 143. An additional test uses the production Frida manager with simulated native objects: an initial detach fails, final provider cleanup retries detach, SIGINT arrives during that retry, and retry then succeeds. The command still returns Evidence plus the earlier cleanup diagnostic, without `code: "cancelled"`, with exit status 130. No actual Frida binding or target process ran. ### Reproduction Save the following as `tests/process-global/fridaFinalCleanupRepro.test.ts` in the prepared source checkout. Run `node node_modules/vitest/vitest.mjs run tests/process-global/fridaFinalCleanupRepro.test.ts --maxWorkers 1`, then remove it. These are the executed Frida cases; assertions characterize current behavior. The first three cases substitute provider-port results to isolate clean finalization. The last two use the unmodified production manager and control only native handles. Signals are dispatched synchronously to the process signal listeners with `process.emit`, after explicit cleanup-entry barriers. ```ts import { createHash } from 'node:crypto'; import { expect, it } from 'vitest'; import { createCli } from '../../src/cli.js'; import { FridaInstrumentationManager } from '../../src/frida/FridaInstrumentationManager.js'; it.each([undefined, 'SIGINT', 'SIGTERM'] as const)('Frida final close signal %s', async (signal) => { 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; let sessionClosed = false; provider.closeSession = async () => { sessionClosed = true; return {ok:true,value:{targetResumeObserved:false}}; }; let begin!: () => void, release!: () => void; const started = new Promise<void>(r => {begin=r;}); const barrier = new Promise<void>(r => {release=r;}); provider.closeAll = async () => { expect(sessionClosed).toBe(true); begin(); await barrier; }; let stdout=''; const savedExitCode=process.exitCode; try { process.exitCode=0; const pending=createCli({},undefined,provider).serve(['instrument-with-frida','attach','71','--script','send(1)','--duration-ms','0','--json'],{env:{},exit:()=>{},stdout:s=>{stdout+=s;}}); await started; if (signal !== undefined) process.emit(signal); release(); await pending; const output=JSON.parse(stdout); console.log('FRIDA',JSON.stringify({signal:signal??'none',keys:Object.keys(output),code:output.code??null,cleanup:output.cleanup_error,processExitCode:process.exitCode,evidenceId:output.evidence?.evidence_id,messages:output.evidence?.normalized_result?.messages})); expect(output.code).toBeUndefined(); expect(output.evidence.evidence_id).toBeTruthy(); expect(output.cleanup_error).toBeNull(); expect(process.exitCode).toBe(signal==='SIGINT'?130:signal==='SIGTERM'?143:0); } finally { release(); process.exitCode=savedExitCode; } }); it.each([undefined, 'SIGINT'] as const)('production Frida manager retries final detach; signal %s', async signal => { let begin!: () => void, release!: () => void; const entered = new Promise<void>(r => { begin = r; }); const barrier = new Promise<void>(r => { release = r; }); let detachCalls=0, detached=false, unloads=0; const native = { pid:71, detached:{connect(){},disconnect(){}}, isDetached:()=>detached, async createScript(){let destroyed=false;let callback:any;return { get isDestroyed(){return destroyed;},message:{connect(fn:any){callback=fn;},disconnect(){}}, async load(){callback?.({type:'send',payload:1},null);},async unload(){unloads++;destroyed=true;} };}, async detach(){detachCalls++;if(detachCalls===1)throw new Error('transient detach failure');begin();await barrier;detached=true;} }; const device={id:'local',name:'fixture',type:'local',async getProcessByPid(){return {pid:71,name:'fixture'};},async attach(){return native;}}; const provider=new FridaInstrumentationManager({deviceManager:{async getDeviceById(){return device;}} as any}); let stdout='';const saved=process.exitCode; try { process.exitCode=0; const pending=createCli({},undefined,provider).serve(['instrument-with-frida','attach','71','--script','send(1)','--duration-ms','0','--json'],{env:{},exit:()=>{},stdout:s=>{stdout+=s;}}); await entered; if(signal)process.emit(signal); release();await pending; const output=JSON.parse(stdout); console.log('FRIDA_REAL_MANAGER',JSON.stringify({signal:signal??'none',code:output.code??null,cleanup:output.cleanup_error,processExitCode:process.exitCode,detachCalls,detached,unloads,messages:output.evidence?.normalized_result?.messages})); expect(output.code).toBeUndefined();expect(output.evidence).toBeDefined(); expect(output.cleanup_error).toContain('transient detach failure'); expect(detachCalls).toBe(2);expect(detached).toBe(true);expect(unloads).toBe(1); expect(process.exitCode).toBe(signal?130:1); } finally {release();await provider.closeAll();process.exitCode=saved;} }); ``` ### Expected behavior The [cancellable-observation contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L101-L112) requires a final signal check after awaited finalization and retention of collected observations under typed cancellation. [The one-shot Frida command](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/frida-instrumentation.md#L7) is an observation. Completed messages should survive; cancellation does not imply incomplete capture or undo script effects. The guide’s committed-load cutoff concerns persistent script loading, where late cancellation must not unload a committed script. This command owns a one-shot observation and awaits disposable provider shutdown before returning. No atomic publication or persistent-load receipt supersedes that final check. Preserve any earlier primary error or unresolved cleanup classification. In the native-retry case, final cleanup has succeeded; keep the earlier cleanup diagnostic in the retained observation. ### Impact and scope P2: bounded completion-classification mismatch. A JSON consumer misses the documented cancelled outcome and its partial-observation channel even though the process status records a handled interrupt. Captured messages survive in the returned Evidence, and checking exit status distinguishes the interrupted cases in the test; no message loss or target damage is claimed. The prolonged clean-provider wait is simulated. The production-manager asynchronous case requires a transient initial cleanup failure that succeeds on retry. Real-device frequency and a prolonged completely clean native shutdown were not established. ### REA version rea-agents 6.7.0; source revision `52415975942a4b9c53a817915b313ccc404f280e`. ### Runtime and platform macOS 27.0.1 arm64; Node.js v24.22.0; locked Vitest 5.0.3. The repository-pinned Node 24.18.0 was not exercised. Source CLI execution with simulated native/provider boundaries; no packaged subprocess or real-device qualification. ### Target and artifact context Synthetic local attach session, PID 71, zero-duration observation, inline send(1), one retained send message. The production manager creates/releases its own script/session identities over simulated native handles. ### Interface CLI ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Reduced observations from the completed run: ```jsonl {"provider":"controlled","signal":"none","code":null,"cleanup_error":null,"process_exit_code":0,"message_retained":true} {"provider":"controlled","signal":"SIGINT","code":null,"cleanup_error":null,"process_exit_code":130,"message_retained":true} {"provider":"controlled","signal":"SIGTERM","code":null,"cleanup_error":null,"process_exit_code":143,"message_retained":true} {"provider":"production-manager","signal":"none","code":null,"earlier_detach_diagnostic_retained":true,"process_exit_code":1,"detach_attempts":2,"detached":true,"unloads":1} {"provider":"production-manager","signal":"SIGINT","code":null,"earlier_detach_diagnostic_retained":true,"process_exit_code":130,"detach_attempts":2,"detached":true,"unloads":1} ``` In each case `code: null` above denotes an absent top-level code, not a literal payload field. The interrupted outputs use the ordinary Evidence wrapper. The final focused run passed all seven tests, including these five Frida cases and two environment tests. ### Typed outcome or recovery expected After successful final cleanup, project AnalysisCancelledError for instrument_with_frida with the observed abort reason and collected Evidence as partial observation. Retain any earlier cleanup diagnostic and actual completed cleanup facts. If cleanup remains incomplete, preserve its classification and recovery information. Do not roll back a committed persistent script load or claim the target was terminated. ### Analysis or suspected cause [withProviderCleanup](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/cli/fridaCommands.ts#L36-L62) returns its prior Result after awaiting closeAll without receiving/checking the signal. [The command](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/cli/fridaCommands.ts#L278-L298) projects that Result as success. The [shared service](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/frida/FridaInstrumentationService.ts#L409-L435) checks after closeSession, but cannot observe cancellation arriving during this later CLI await. [The manager](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/frida/FridaInstrumentationManager.ts#L774-L818) can retry retained native cleanup at that later boundary. [The command cancellation wrapper](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/cli/commandCancellation.ts#L14-L31) only assigns the signal-derived exit status. ### Additional context Upstream main `8e3bf56e11bcb56d0e8754fdfa32a7a9f004ca13` has unchanged relevant implementation. Closed [#5082](https://github.com/morluto/rea/issues/5082) repaired the inner service’s final closeSession/native-detach check; this is the subsequent CLI closeAll boundary. #5864 concerns the successful wrapper shape and #5865 concerns partial-observation placement when final cleanup rejects. Neither adds this missing check. #5232 concerns preserving a native resume failure. The persistent-load fixes #2535/#2544/#3979 govern a different lifetime and cutoff. No repair or real-native scheduling frequency was measured. Related but separate: [#6048](https://github.com/morluto/rea/issues/6048) concerns final provider-result consumption in native value tracing, and [#6049](https://github.com/morluto/rea/issues/6049) concerns Hopper regex matching-worker termination. This report concerns the Frida CLI-owned closeAll await; these producing boundaries require separate completion checks.

bsakaguchi closed this issue

2026-10-31 00:43:18 +00:00

bsakaguchi commented

2026-10-31 00:43:19 +00:00

Author

Owner

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

Checked the reported fix on main at d87e73f2db.

Frida CLI now uses the shared finishAnalysisCommand with the same cancellation signal and provider.closeAll cleanup wrapper. That finalizer awaits close, dispatches queued interrupts, then converts successful collected Evidence into AnalysisCancelledError with its partial observation when the signal is aborted. Incomplete cleanup is composed and retains precedence; primary errors stay primary.

Implementation: fridaCommands.ts, fridaCommands.ts.

The relevant regression checks in fridaFinalizationCli.test.ts passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.

@coygeek commented on GitHub (Oct 24, 2026): Checked the reported fix on `main` at d87e73f2db50be72d33a4b3c7199af35518b441c. Frida CLI now uses the shared finishAnalysisCommand with the same cancellation signal and provider.closeAll cleanup wrapper. That finalizer awaits close, dispatches queued interrupts, then converts successful collected Evidence into AnalysisCancelledError with its partial observation when the signal is aborted. Incomplete cleanup is composed and retains precedence; primary errors stay primary. Implementation: [fridaCommands.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/cli/fridaCommands.ts#L34), [fridaCommands.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/cli/fridaCommands.ts#L266). The relevant regression checks in [fridaFinalizationCli.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/process-global/fridaFinalizationCli.test.ts) passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.

bsakaguchi referenced this issue

2026-10-31 00:52:15 +00:00

[GH-ISSUE #8097] [Feature] Inspect ONNX Runtime .ort models (ORTM FlatBuffers) in inspect_model_artifact #4670

bsakaguchi referenced this issue

2026-11-01 21:10:53 +00:00

[GH-ISSUE #3582] [Tracker] Ready to close or merge: issues and pull requests (October 2026) #1330

bsakaguchi referenced this issue

2026-11-01 21:10:54 +00:00

[GH-ISSUE #3795] [Tracker] Open documentation issues and pull requests (October 2026) #1468

bsakaguchi referenced this issue

2026-11-01 21:10:57 +00:00

[GH-ISSUE #3796] [Tracker] Open feature requests by area (October 2026) #1471