From cab652712538f15e60a801b5bec52c3e779bcd7b Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 04:05:23 +0800 Subject: [PATCH] fix(rizin): preserve primary outcomes across final cleanup --- docs/reverse-engineering-tools.md | 2 +- src/rizin/RizinDebugSessionManager.ts | 315 +++++++++++------- .../RizinDebugSessionCoverage.part2.test.ts | 4 +- .../RizinDebugSessionCoverage.part3.test.ts | 183 +++++++++- 4 files changed, 376 insertions(+), 128 deletions(-) diff --git a/docs/reverse-engineering-tools.md b/docs/reverse-engineering-tools.md index 1c50b0d890..9600192550 100644 --- a/docs/reverse-engineering-tools.md +++ b/docs/reverse-engineering-tools.md @@ -60,7 +60,7 @@ close_gdb_session {"session_id":"…"} ## Rizin -`start_rizin_debug_session` and `rea debug-with-rizin` require an explicit `inferior_tty` on Linux. Rizin launches the target with stdout and stderr routed to that caller-selected TTY and `dbg.exitkills=false`; the target can therefore continue running and writing to the TTY after REA closes the debugger. This is a visible target output effect, and the caller is responsible for selecting a suitable terminal that remains available. Before closing, REA serializes after the active command, parses the structured `olj` descriptor list, detaches only a unique `ptrace://` descriptor, confirms its absence, reads back `dbg.exitkills=false`, and verifies `/proc//fd/1` and `/proc//fd/2` still identify the selected TTY by device and inode. If a previously identified inferior exits after Rizin confirms detach, REA verifies that the original process has ended instead of requiring output descriptors from an exited process. Unresolved identity or ambiguous readback keeps the Rizin session owned for retry. After output limits or a failed command interval, cleanup first waits up to 1000 ms for a fresh private NUL-delimited marker, discarding earlier frames before obtaining that new inferior readback. Synchronization never reopens user command admission. A missing marker, unavailable transport, or unknown inferior identity retains the cleanup owner. Caller cancellation keeps its exact reason and typed cancellation outcome even when cleanup remains incomplete; a successful retry resolves that retained cleanup state. Persistent Rizin debugger sessions are unavailable outside Linux because this verification depends on `/proc`. +`start_rizin_debug_session` and `rea debug-with-rizin` require an explicit `inferior_tty` on Linux. Rizin launches the target with stdout and stderr routed to that caller-selected TTY and `dbg.exitkills=false`; the target can therefore continue running and writing to the TTY after REA closes the debugger. This is a visible target output effect, and the caller is responsible for selecting a suitable terminal that remains available. Before closing, REA serializes after the active command, parses the structured `olj` descriptor list, detaches only a unique `ptrace://` descriptor, confirms its absence, reads back `dbg.exitkills=false`, and verifies `/proc//fd/1` and `/proc//fd/2` still identify the selected TTY by device and inode. If a previously identified inferior exits after Rizin confirms detach, REA verifies that the original process has ended instead of requiring output descriptors from an exited process. Unresolved identity or ambiguous readback keeps the Rizin session owned for retry. After output limits or a failed command interval, cleanup first waits up to 1000 ms for a fresh private NUL-delimited marker, discarding earlier frames before obtaining that new inferior readback. Synchronization never reopens user command admission. A missing marker, unavailable transport, or unknown inferior identity retains the cleanup owner. Caller cancellation keeps its exact reason and typed cancellation outcome even when cleanup remains incomplete; a successful retry resolves that retained cleanup state. Cancellation during final cleanup retains the captured observation and cleanup owner. An already observed startup or protocol failure remains primary and retains a later cancellation as a secondary failure; recovering the transport does not erase that original diagnosis. Persistent Rizin debugger sessions are unavailable outside Linux because this verification depends on `/proc`. Persistent command Evidence retains the caller-selected and resolved startup paths in `parameters.launch_provenance`. These paths describe launch provenance, not the current target after unrestricted commands. `rizin_debug_session_status.output_invalid_utf8` reports whether lossy frame decoding has been observed during the session. Command Evidence sets `output_invalid_utf8` only when loss was observed in frames collected for that command, including retained partial output; earlier startup or command frames do not mark later valid output as lossy. diff --git a/src/rizin/RizinDebugSessionManager.ts b/src/rizin/RizinDebugSessionManager.ts index 7a3551e8a1..5bb4592cdd 100644 --- a/src/rizin/RizinDebugSessionManager.ts +++ b/src/rizin/RizinDebugSessionManager.ts @@ -15,6 +15,7 @@ import type { AnalysisError, AnalysisErrorOptions, } from "../domain/analysisErrorBase.js"; +import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { createEvidence, type Evidence } from "../domain/evidence.js"; import { err, ok, type Result } from "../domain/result.js"; import { @@ -541,62 +542,56 @@ export class RizinDebugSessionManager { session.ready = true; return ok({ session_id: id, backend: input.backend ?? null }); } catch (caught) { - const cause = + const protocolFailure = session.protocolError; + const cancellation = signal?.aborted || caught instanceof AnalysisCancelledError ? rizinCancellation("start_rizin_debug_session", signal, caught) - : caught; + : undefined; const stopped = await this.#stopDebuggerOnly(session); const capturedOutput = this.#startupCapturedOutput(session); if (stopped.status !== "incomplete") this.#sessions.delete(id); - if (stopped.status === "incomplete") - return err( - this.#retainCleanupFailure( - session, - "start_rizin_debug_session", - stopped.reason, - { cause, capturedOutput }, - ), + let primary: AnalysisError; + if (cancellation !== undefined) + primary = rizinCancellation( + "start_rizin_debug_session", + undefined, + cancellation, + { capturedOutput }, ); - if (signal?.aborted || cause instanceof AnalysisCancelledError) - return err( - rizinCancellation("start_rizin_debug_session", signal, cause, { - capturedOutput, - }), + else if (protocolFailure !== undefined) + primary = new AnalysisCapabilityUnavailableError( + "rizin", + "start_rizin_debug_session", + protocolFailure, + { cause: caught, capturedOutput }, ); - if (session.protocolError !== undefined) - return err( - new AnalysisCapabilityUnavailableError( - "rizin", - "start_rizin_debug_session", - session.protocolError, - { cause, capturedOutput }, - ), + else if (caught instanceof RizinProcessExitError) + primary = new AnalysisCapabilityUnavailableError( + "rizin", + "start_rizin_debug_session", + caught.message, + { cause: caught, capturedOutput }, ); - if (cause instanceof RizinProcessExitError) { - return err( - new AnalysisCapabilityUnavailableError( - "rizin", - "start_rizin_debug_session", - cause.message, - { - cause, - capturedOutput, - }, - ), - ); - } - return err( - new AnalysisTimeoutError( + else + primary = new AnalysisTimeoutError( "start_rizin_debug_session", this.#startupTimeoutMs, { - cause, + cause: caught, capturedOutput, ...fixedDeadlineGuidance( "Rizin debug session startup", this.#startupTimeoutMs, ), }, + ); + return err( + this.#finalizeFailure( + session, + "start_rizin_debug_session", + primary, + stopped, + signal, ), ); } @@ -681,7 +676,8 @@ export class RizinDebugSessionManager { "Unsolicited Rizin frames arrived before a command; command output can no longer be safely correlated."; this.#failProtocol(session, message); return err( - await this.#stopAfterProtocolFailure(session, undefined, { + await this.#stopAfterProtocolFailure(session, message, undefined, { + signal, commandTurnHeld: true, }), ); @@ -737,9 +733,12 @@ export class RizinDebugSessionManager { const captured = received.value; if (signal?.aborted) throw rizinCancellation("rizin_debug_command", signal); + let stopped: Awaited> = { + status: "stopped", + }; let cleanupFailure: string | undefined; if (captured.outputTruncated) { - const stopped = await this.#stopDebuggerOnly(session, true); + stopped = await this.#stopDebuggerOnly(session, true); if (stopped.status === "incomplete") cleanupFailure = stopped.reason; } const result = commandResult( @@ -767,18 +766,12 @@ export class RizinDebugSessionManager { "The command is unrestricted and may access local files, the shell, network, or mutate the target.", ], }); - if (cleanupFailure !== undefined) - return err( - this.#retainCleanupFailure( - session, - "rizin_debug_command", - cleanupFailure, - { - partialObservation: evidence, - }, - ), - ); - return ok({ result, evidence }); + return this.#finalizeCommandObservation( + session, + { result, evidence }, + stopped, + signal, + ); } catch (cause) { // Capture is owned by this dispatched command, independently of which // cancellation, protocol or cleanup outcome ultimately takes precedence. @@ -817,28 +810,24 @@ export class RizinDebugSessionManager { ]); session.protocolError = "A Rizin command was cancelled; the owned session was stopped because command output can no longer be correlated safely."; - const stopped = await this.#stopDebuggerOnly(session, true); - if (stopped.status === "incomplete") - return err( - this.#retainCleanupFailure( - session, - "rizin_debug_command", - stopped.reason, - { - cause: rizinCancellation("rizin_debug_command", signal, cause, { - ...(capturedOutput === undefined ? {} : { capturedOutput }), - }), - ...(evidence === undefined - ? {} - : { partialObservation: evidence }), - }, - ), - ); - return err( - rizinCancellation("rizin_debug_command", signal, cause, { + const primary = rizinCancellation( + "rizin_debug_command", + signal, + cause, + { ...(capturedOutput === undefined ? {} : { capturedOutput }), ...(evidence === undefined ? {} : { partialObservation: evidence }), - }), + }, + ); + const stopped = await this.#stopDebuggerOnly(session, true); + return err( + this.#finalizeFailure( + session, + "rizin_debug_command", + primary, + stopped, + signal, + ), ); } if (session.protocolError !== undefined) { @@ -848,10 +837,18 @@ export class RizinDebugSessionManager { "The command is unrestricted and may access local files, the shell, network, or mutate the target.", ]); return err( - await this.#stopAfterProtocolFailure(session, cause, { - commandTurnHeld: true, - ...(evidence === undefined ? {} : { partialObservation: evidence }), - }), + await this.#stopAfterProtocolFailure( + session, + session.protocolError, + cause, + { + commandTurnHeld: true, + signal, + ...(evidence === undefined + ? {} + : { partialObservation: evidence }), + }, + ), ); } if (interruption?.kind === "timeout") { @@ -871,19 +868,12 @@ export class RizinDebugSessionManager { launchProvenance: session.launchProvenance, timeoutMs: this.#frameTimeoutMs, }); - if (stopped.status === "incomplete") - return err( - this.#retainCleanupFailure( - session, - "rizin_debug_command", - stopped.reason, - { - cause, - partialObservation: evidence, - }, - ), - ); - return ok({ result, evidence }); + return this.#finalizeCommandObservation( + session, + { result, evidence }, + stopped, + signal, + ); } if (interruption?.kind === "exited") { const result = commandResult(interruption.capture, "unknown"); @@ -899,16 +889,12 @@ export class RizinDebugSessionManager { ], }); const stopped = await this.#stopDebuggerOnly(session, true); - if (stopped.status === "incomplete") - return err( - this.#retainCleanupFailure( - session, - "rizin_debug_command", - stopped.reason, - { cause, partialObservation: evidence }, - ), - ); - return ok({ result, evidence }); + return this.#finalizeCommandObservation( + session, + { result, evidence }, + stopped, + signal, + ); } session.protocolError = "Rizin did not return the current command-completion marker; the session was stopped to prevent late output from being attributed to a later command."; @@ -921,23 +907,25 @@ export class RizinDebugSessionManager { ...(evidence === undefined ? {} : { partialObservation: evidence }), }; const stopped = await this.#stopDebuggerOnly(session, true); - if (stopped.status === "incomplete") - return err( - this.#retainCleanupFailure( - session, - "rizin_debug_command", - stopped.reason, - errorOptions, - ), - ); - return err( - new AnalysisTimeoutError("rizin_debug_command", this.#frameTimeoutMs, { + const primary = new AnalysisTimeoutError( + "rizin_debug_command", + this.#frameTimeoutMs, + { ...errorOptions, ...fixedDeadlineGuidance( "The Rizin debug command", this.#frameTimeoutMs, ), - }), + }, + ); + return err( + this.#finalizeFailure( + session, + "rizin_debug_command", + primary, + stopped, + signal, + ), ); } finally { if (acquired) { @@ -1416,6 +1404,81 @@ export class RizinDebugSessionManager { return captured.value.output; } + #finalizeCommandObservation( + session: RizinDebugSession, + observation: { + readonly result: RizinDebugCommandResult; + readonly evidence: Evidence; + }, + stopped: Awaited>, + signal?: AbortSignal, + ): Result { + if (signal?.aborted) { + const primary = rizinCancellation( + "rizin_debug_command", + signal, + undefined, + { + capturedOutput: { + stdout: observation.result.output, + stderr: "", + truncated: observation.result.output_truncated, + }, + partialObservation: observation.evidence, + }, + ); + return err( + this.#finalizeFailure( + session, + "rizin_debug_command", + primary, + stopped, + signal, + ), + ); + } + return stopped.status === "incomplete" + ? err( + this.#retainCleanupFailure( + session, + "rizin_debug_command", + stopped.reason, + { partialObservation: observation.evidence }, + ), + ) + : ok(observation); + } + + #finalizeFailure( + session: RizinDebugSession, + operation: string, + primary: AnalysisError, + stopped: Awaited>, + signal?: AbortSignal, + ): AnalysisError { + if (signal?.aborted && !(primary instanceof AnalysisCancelledError)) + primary.retainSecondaryFailure(rizinCancellation(operation, signal), { + finalization_cancelled: true, + }); + if (stopped.status !== "incomplete") return primary; + if (primary instanceof AnalysisCancelledError) + return this.#retainCleanupFailure(session, operation, stopped.reason, { + cause: primary, + }); + const cleanup = this.#retainCleanupFailure( + session, + operation, + stopped.reason, + ); + const composed = analysisErrorWithCleanupFailure( + primary, + cleanup, + operation, + ).bindCleanupDependencies([cleanup]); + session.cleanupFailures.add(composed); + return composed; + } + #retainCleanupFailure( session: RizinDebugSession, operation: string, @@ -1446,10 +1509,12 @@ export class RizinDebugSessionManager { async #stopAfterProtocolFailure( session: RizinDebugSession, + protocolFailure: string, cause: unknown, options: { readonly commandTurnHeld?: boolean; readonly partialObservation?: Evidence; + readonly signal?: AbortSignal | undefined; } = {}, ): Promise { const stopped = await this.#stopDebuggerOnly( @@ -1462,19 +1527,19 @@ export class RizinDebugSessionManager { ? {} : { partialObservation: options.partialObservation }), }; - return stopped.status === "incomplete" - ? this.#retainCleanupFailure( - session, - "rizin_debug_command", - stopped.reason, - errorOptions, - ) - : new AnalysisCapabilityUnavailableError( - "rizin", - "rizin_debug_command", - session.protocolError!, - errorOptions, - ); + const primary = new AnalysisCapabilityUnavailableError( + "rizin", + "rizin_debug_command", + protocolFailure, + errorOptions, + ); + return this.#finalizeFailure( + session, + "rizin_debug_command", + primary, + stopped, + options.signal, + ); } } diff --git a/tests/boundary/providers/RizinDebugSessionCoverage.part2.test.ts b/tests/boundary/providers/RizinDebugSessionCoverage.part2.test.ts index 6a899798f8..92bf472dcc 100644 --- a/tests/boundary/providers/RizinDebugSessionCoverage.part2.test.ts +++ b/tests/boundary/providers/RizinDebugSessionCoverage.part2.test.ts @@ -75,7 +75,9 @@ it.skipIf(process.platform === "win32").each([false, true])( const result = await manager.execute(started.value.session_id, "dr"); expect(result.ok).toBe(false); if (!result.ok && incomplete) - expect(result.error.message).toContain("injected stop uncertainty"); + expect(result.error.cleanup?.reason).toContain( + "injected stop uncertainty", + ); }, ); diff --git a/tests/boundary/providers/RizinDebugSessionCoverage.part3.test.ts b/tests/boundary/providers/RizinDebugSessionCoverage.part3.test.ts index e8c04d66a0..63b1a3262a 100644 --- a/tests/boundary/providers/RizinDebugSessionCoverage.part3.test.ts +++ b/tests/boundary/providers/RizinDebugSessionCoverage.part3.test.ts @@ -216,7 +216,9 @@ it.skipIf(process.platform === "win32").each([false, true])( ); expect(result.ok).toBe(false); if (!result.ok && incomplete) - expect(result.error.message).toContain("injected stop uncertainty"); + expect(result.error.cleanup?.reason).toContain( + "injected stop uncertainty", + ); }, ); @@ -375,3 +377,182 @@ it.skipIf(process.platform === "win32").each([false, true])( expect(manager.status(started.value.session_id)).toBeUndefined(); }, ); + +it.skipIf(process.platform === "win32")( + "retains the producing protocol diagnosis after cleanup synchronization recovers transport", + async () => { + const injected = createInjectedProcess({ + onWrite: (text, control) => { + const marker = /!echo (.+)\n/u.exec(text)?.[1]; + if (marker === undefined) return; + if (text.startsWith("!echo __REA_CLEANUP_SYNC_")) + control.output(`${marker}\0`); + else if (text.startsWith("olj\n")) control.output(`[]\0${marker}\0`); + else throw new Error(`Unexpected command: ${text}`); + }, + }); + const { manager, started } = await startInjected(injected.spawnProcess, { + stopProcess: stopOwnedDebuggerProcess, + }); + if (!started.ok) throw started.error; + injected.control.output("unsolicited\0"); + const result = await manager.execute( + started.value.session_id, + "never dispatched", + ); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toMatchObject({ + _tag: "AnalysisCapabilityUnavailableError", + reason: + "Unsolicited Rizin frames arrived before a command; command output can no longer be safely correlated.", + }); + expect(result.error.cleanupIncomplete).toBe(false); + } + expect(manager.status(started.value.session_id)?.state).toBe("closed"); + }, +); + +it.skipIf(process.platform === "win32").each([ + { stage: "output", incomplete: false }, + { stage: "output", incomplete: true }, + { stage: "timeout", incomplete: false }, + { stage: "timeout", incomplete: true }, + { stage: "exited", incomplete: false }, + { stage: "exited", incomplete: true }, +] as const)( + "preserves cancellation during $stage final cleanup (incomplete=$incomplete)", + async ({ stage, incomplete }) => { + const controller = new AbortController(); + const reason = { boundary: "final cleanup" }; + let enterStop!: () => void; + const entered = new Promise((resolve) => { + enterStop = resolve; + }); + let releaseStop!: () => void; + const released = new Promise((resolve) => { + releaseStop = resolve; + }); + let stopping = true; + const injected = createInjectedProcess({ + onWrite: (_text, control) => { + if (stage === "output") + control.output("beyond selected output budget\0"); + if (stage === "exited") { + control.output("partial\0"); + control.exit(); + } + }, + }); + const { manager, started } = await startInjected(injected.spawnProcess, { + maxCommandOutputBytes: stage === "output" ? 8 : 128, + frameTimeoutMs: 10, + stopProcess: async () => { + if (stopping) { + enterStop(); + await released; + } + return incomplete && stopping + ? { status: "incomplete", reason: "stop settlement pending" } + : { status: "stopped" }; + }, + }); + if (!started.ok) throw started.error; + const pending = manager.execute( + started.value.session_id, + "noisy", + controller.signal, + ); + await entered; + controller.abort(reason); + releaseStop(); + const result = await pending; + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toMatchObject({ + _tag: "AnalysisCancelledError", + cleanupIncomplete: incomplete, + }); + expect(result.error.cause).toBe(reason); + expect(result.error.capturedOutput?.stdout).toBe( + stage === "output" ? "beyond s" : stage === "exited" ? "partial" : "", + ); + expect(result.error.partialObservation).toBeDefined(); + if (incomplete) { + stopping = false; + expect((await manager.close(started.value.session_id)).ok).toBe(true); + expect(result.error.cleanupIncomplete).toBe(false); + } + }, +); + +it.skipIf(process.platform === "win32")( + "retains a startup protocol diagnosis across successful cleanup synchronization", + async () => { + const injected = createInjectedProcess({ + startup: (control) => control.stdinError(), + onWrite: (text, control) => { + const marker = /!echo (.+)\n/u.exec(text)?.[1]; + if (marker === undefined) return; + control.output( + text.startsWith("olj\n") ? `[]\0${marker}\0` : `${marker}\0`, + ); + }, + }); + const { started } = await startInjected(injected.spawnProcess, { + stopProcess: stopOwnedDebuggerProcess, + }); + expect(started.ok).toBe(false); + if (!started.ok) + expect(started.error).toMatchObject({ + _tag: "AnalysisCapabilityUnavailableError", + reason: "Rizin stdin stream failed: injected stdin error", + cleanupIncomplete: false, + }); + }, +); + +it.skipIf(process.platform === "win32").each([false, true])( + "preserves cancellation observed during failed startup cleanup (incomplete=%s)", + async (incomplete) => { + const controller = new AbortController(); + const reason = { boundary: "startup cleanup" }; + const injected = createInjectedProcess({ + startup: (control) => control.stdinError(), + }); + let retained = true; + const manager = createManager( + { REA_RIZIN_COMMAND: "injected-rizin" }, + { + spawnProcess: injected.spawnProcess, + stopProcess: async () => { + controller.abort(reason); + return incomplete && retained + ? { status: "incomplete", reason: "stop settlement pending" } + : { status: "stopped" }; + }, + }, + ); + const result = await manager.start( + { path: "/tmp/injected", inferior_tty: await rizinTestInferiorTty() }, + controller.signal, + ); + expect(result.ok).toBe(false); + if (result.ok) return; + const primary = result.error.cleanupPrimary ?? result.error; + expect(primary).toMatchObject({ + _tag: "AnalysisCapabilityUnavailableError", + reason: "Rizin stdin stream failed: injected stdin error", + }); + expect(primary.secondaryFailures[0]).toMatchObject({ + _tag: "AnalysisCancelledError", + }); + expect(primary.secondaryFailures[0]?.cause).toBe(reason); + expect(result.error.cleanupIncomplete).toBe(incomplete); + if (incomplete) { + retained = false; + await manager.closeAll(); + expect(result.error.cleanupIncomplete).toBe(false); + } + }, +);