morluto/rea

Fork 0

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

[GH-ISSUE #5865] [Bug] Frida CLI cleanup failures put completed observations in diagnostics instead of the typed field #3554

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/5865

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

When Frida instrumentation has captured a completed observation but final provider cleanup rejects, the CLI reports cleanup_incomplete and retains the result only at details.diagnostics.partial_observation. The typed details.partial_observation field is absent. The nested result contains valid Evidence and a camelCase cleanupError member. Device and process discovery use the same misplaced diagnostic location.

This was executed through the actual CLI with simulated provider responses and cleanup failure; no real Frida target or leaked resource was used.

Reproduction

Use the same source checkout, locked dependencies and existing runtime artifacts. Save the following focused test as tests/process-global/repro.test.ts, run the command below, then delete the test. The failing-cleanup case returns a typed failure from closeSession and rejects closeAll; the other case is a successful cleanup control. These assertions characterize the current response.

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

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

Retain the completed value through the typed partial-observation channel so it is projected as details.partial_observation, while continuing to report cleanup_incomplete. The lifecycle contract requires completed results to survive one-shot cleanup. The existing shared finalizer retains successful values on the error before projection. Diagnostic metadata and completed observations should remain distinguishable.

Impact and scope

P2: consumers using the shared error projection cannot find the completed observation in its typed field. The bytes remain available under details.diagnostics.partial_observation; explicit extraction is a demonstrated workaround. Instrumentation and both discovery commands were exercised. The instrumentation cleanup case ran twice; each discovery case ran once. Naturally occurring cleanup failure frequency and real session ownership were not tested. No observation corruption or security boundary crossing was observed.

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 and captured send message. Additional discovery cases used empty device/process inventories. Cleanup rejection was injected at the supported provider port.

Interface

CLI

Earliest failure stage

Transport or result projection

Relevant output or evidence

Instrumentation returned:

{"code":"cleanup_incomplete","detailsKeys":["provider_id","operation","cleanup","resources","diagnostics","execution_failure"],"retainedKeys":["evidence","cleanupError"],"nestedEvidenceId":"ev_c8a8f71655f62dd9dff8b3ff4cfc87941ee7627cf4abd8ad4d04956c797872d3"}

The completed Evidence at details.diagnostics.partial_observation.evidence passed the named schema. details.partial_observation was absent. Separate executed CLI cases of list-frida-devices and list-frida-processes also returned cleanup_incomplete with their completed inventory objects only at details.diagnostics.partial_observation. The final focused run passed seven cases in one file. This establishes adapter behavior under controlled failures, not an actual native cleanup failure.

Typed outcome or recovery expected

Expose the completed observation at details.partial_observation while preserving the cleanup failure, reported resources and primary error rules. Set or retain the error partialObservation property rather than placing a similarly named key in diagnostics. Unwrapping instrumentation Evidence consistently with the normal-success contract is a separate representation concern.

Analysis or suspected cause

Source-derived: withProviderCleanup supplies partial_observation in the third constructor argument, which ProviderCleanupError defines as diagnostics. The projection copies diagnostics as metadata and emits details.partial_observation only from the typed property. The generic projection behaves as implemented; the local Frida helper chooses the wrong channel.

Additional context

Existing process-discovery CLI coverage already asserts the diagnostic nesting, so the branch is tested but the invariant is not enforced. Related #3935 and merged #4238 addressed result loss in other CLI families; that repair does not change this local Frida helper. #5839 concerns native inventory fallback classification, #5842 concerns resolved export cleanup state, and #5756 concerns a lost CDP termination fact. None repairs this placement error. 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/5865 ### 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 When Frida instrumentation has captured a completed observation but final provider cleanup rejects, the CLI reports cleanup_incomplete and retains the result only at details.diagnostics.partial_observation. The typed details.partial_observation field is absent. The nested result contains valid Evidence and a camelCase cleanupError member. Device and process discovery use the same misplaced diagnostic location. This was executed through the actual CLI with simulated provider responses and cleanup failure; no real Frida target or leaked resource was used. ### Reproduction Use the same source checkout, locked dependencies and existing runtime artifacts. Save the following focused test as `tests/process-global/repro.test.ts`, run the command below, then delete the test. The failing-cleanup case returns a typed failure from closeSession and rejects closeAll; the other case is a successful cleanup control. These assertions characterize the current response. ```sh npx --no-install vitest run tests/process-global/repro.test.ts --maxWorkers 1 ``` ```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 Retain the completed value through the typed partial-observation channel so it is projected as details.partial_observation, while continuing to report cleanup_incomplete. The [lifecycle contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L132-L136) requires completed results to survive one-shot cleanup. The [existing shared finalizer](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/cli/finishAnalysisCommand.ts#L54-L70) retains successful values on the error before projection. Diagnostic metadata and completed observations should remain distinguishable. ### Impact and scope P2: consumers using the shared error projection cannot find the completed observation in its typed field. The bytes remain available under details.diagnostics.partial_observation; explicit extraction is a demonstrated workaround. Instrumentation and both discovery commands were exercised. The instrumentation cleanup case ran twice; each discovery case ran once. Naturally occurring cleanup failure frequency and real session ownership were not tested. No observation corruption or security boundary crossing was observed. ### 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 and captured send message. Additional discovery cases used empty device/process inventories. Cleanup rejection was injected at the supported provider port. ### Interface CLI ### Earliest failure stage Transport or result projection ### Relevant output or evidence Instrumentation returned: ```json {"code":"cleanup_incomplete","detailsKeys":["provider_id","operation","cleanup","resources","diagnostics","execution_failure"],"retainedKeys":["evidence","cleanupError"],"nestedEvidenceId":"ev_c8a8f71655f62dd9dff8b3ff4cfc87941ee7627cf4abd8ad4d04956c797872d3"} ``` The completed Evidence at details.diagnostics.partial_observation.evidence passed the named schema. details.partial_observation was absent. Separate executed CLI cases of list-frida-devices and list-frida-processes also returned cleanup_incomplete with their completed inventory objects only at details.diagnostics.partial_observation. The final focused run passed seven cases in one file. This establishes adapter behavior under controlled failures, not an actual native cleanup failure. ### Typed outcome or recovery expected Expose the completed observation at details.partial_observation while preserving the cleanup failure, reported resources and primary error rules. Set or retain the error partialObservation property rather than placing a similarly named key in diagnostics. Unwrapping instrumentation Evidence consistently with the normal-success contract is a separate representation concern. ### Analysis or suspected cause Source-derived: [withProviderCleanup](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/cli/fridaCommands.ts#L36-L62) supplies partial_observation in the third constructor argument, which [ProviderCleanupError](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/providerCleanupError.ts#L11-L27) defines as diagnostics. The [projection](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/analysisErrorProjection.ts#L63-L104) copies diagnostics as metadata and emits details.partial_observation only from the typed property. The generic projection behaves as implemented; the local Frida helper chooses the wrong channel. ### Additional context Existing process-discovery CLI coverage already asserts the diagnostic nesting, so the branch is tested but the invariant is not enforced. Related #3935 and merged #4238 addressed result loss in other CLI families; that repair does not change this local Frida helper. #5839 concerns native inventory fallback classification, #5842 concerns resolved export cleanup state, and #5756 concerns a lost CDP termination fact. None repairs this placement error. 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):

Checked the reported fix on main at d87e73f2db.

Frida one-shot discovery and instrumentation now use the shared finishAnalysisCommand lifecycle finalizer. It retains successful results using the typed partialObservation API; instrumentation first unwraps the Evidence to match normal success. CLI tests assert details.partial_observation for completed device/process discovery and the process-global finalization test validates all three output contracts, including completed instrumentation Evidence.

Implementation: fridaCommands.ts, finishAnalysisCommand.ts.

The relevant regression checks in fridaCli.test.ts, 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 one-shot discovery and instrumentation now use the shared finishAnalysisCommand lifecycle finalizer. It retains successful results using the typed partialObservation API; instrumentation first unwraps the Evidence to match normal success. CLI tests assert details.partial_observation for completed device/process discovery and the process-global finalization test validates all three output contracts, including completed instrumentation Evidence. Implementation: [fridaCommands.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/cli/fridaCommands.ts#L34), [finishAnalysisCommand.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/cli/finishAnalysisCommand.ts#L73). The relevant regression checks in [fridaCli.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/boundary/cli/fridaCli.test.ts), [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 01:09:40 +00:00

[PR #3554] [CLOSED] docs(readme): sync Turkish README with English edition #5819

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

No description provided.

Rows
Columns