From c06dc2d356f6bfb2f00aec33b295c41fe53f9e16 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:12:01 +0800 Subject: [PATCH] fix(native): retain completed debugger evidence through reply cleanup --- src/native/pwndbg/PwndbgCoreContext.ts | 159 +++++++++--- .../pwntools/PwntoolsRecordedCrashProvider.ts | 32 ++- .../native/pwndbgReplyFinalization.test.ts | 244 ++++++++++++++++++ 3 files changed, 387 insertions(+), 48 deletions(-) create mode 100644 tests/boundary/native/pwndbgReplyFinalization.test.ts diff --git a/src/native/pwndbg/PwndbgCoreContext.ts b/src/native/pwndbg/PwndbgCoreContext.ts index 3dcc40dac0..1d3c5eecb6 100644 --- a/src/native/pwndbg/PwndbgCoreContext.ts +++ b/src/native/pwndbg/PwndbgCoreContext.ts @@ -2,6 +2,7 @@ import { bindArtifactFileCleanup, artifactFileCancellationCause, } from "../../artifacts/ArtifactFileOperation.js"; +import { safeParseUtf8Json } from "../../domain/safeJson.js"; import { access, mkdir, stat, writeFile } from "node:fs/promises"; import { constants } from "node:fs"; import { randomUUID } from "node:crypto"; @@ -20,10 +21,15 @@ import { recordedCrashDebuggerSchema, type RecordedCrash, } from "../../domain/native/recordedCrash.js"; -import { readStableArtifact } from "../../artifacts/readStableArtifact.js"; +import { + completedStableArtifact, + readStableArtifact, +} from "../../artifacts/readStableArtifact.js"; import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; import type { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; import { AnalysisError } from "../../domain/analysisErrorBase.js"; +import { analysisErrorWithCleanupFailure } from "../../domain/analysisErrorCleanup.js"; +import { ProviderCleanupError } from "../../domain/providerCleanupError.js"; import { capturedPwntoolsOutput, pwntoolsDecoderFailure, @@ -48,6 +54,12 @@ const replySchema = z.discriminatedUnion("ok", [ }), ]); +/** Completed debugger observations stay separate from their reply descriptor's close. */ +export interface PwndbgCoreInspection { + readonly context: Extract; + readonly finalization: AnalysisError | undefined; +} + /** Add mapping candidates from an owned core-only debugger, using the existing snapshot. */ export const inspectPwndbgCore = async ({ environment, @@ -74,7 +86,7 @@ export const inspectPwndbgCore = async ({ kind: "file" | "directory", mode: number, ) => Promise; -}): Promise => { +}): Promise => { const gdb = environment.REA_PWNDBG_GDB ?? "/usr/bin/gdb"; const entry = environment.REA_PWNDBG_GDBINIT ?? ""; const venv = environment.REA_PWNDBG_VENV_PATH ?? ""; @@ -189,15 +201,33 @@ export const inspectPwndbgCore = async ({ ); output = capturedPwntoolsOutput(execution); let reply: z.output; + let finalization: AnalysisError | undefined; try { - const raw = await readStableArtifact( - replyPath, - PWNTOOLS_LIMITS.outputBytes, - options?.signal, - undefined, - resources, - ); - reply = replySchema.parse(JSON.parse(raw.bytes.toString("utf8"))); + let raw; + try { + raw = await readStableArtifact( + replyPath, + PWNTOOLS_LIMITS.outputBytes, + options?.signal, + undefined, + resources, + ); + } catch (cause: unknown) { + const completed = completedStableArtifact(cause); + if ( + !(cause instanceof ArtifactReaderFailure) || + completed === undefined + ) + throw cause; + finalization = completedReplyFailure( + cause, + recordedCrashStageOutput(diagnostics, output), + ); + raw = completed; + } + const decoded = safeParseUtf8Json(raw.bytes); + if (!decoded.ok) throw decoded.cause; + reply = replySchema.parse(decoded.value); } catch (cause: unknown) { if ( cause instanceof ArtifactReaderFailure && @@ -212,7 +242,7 @@ export const inspectPwndbgCore = async ({ : {}), }); bindArtifactFileCleanup(cause, error); - throw error; + throw pwndbgContextFinalizationFailure(error, finalization); } const error = new AnalysisOutputError( OPERATION, @@ -226,38 +256,52 @@ export const inspectPwndbgCore = async ({ }, ); bindArtifactFileCleanup(cause, error); - throw error; + throw pwndbgContextFinalizationFailure(error, finalization); } if (!reply.ok) { if (reply.reason === "unavailable" || reply.reason === "unsupported") - throw new AnalysisCapabilityUnavailableError( - "pwndbg-core", - OPERATION, - reply.message, - { - userMessage: reply.message, - capturedOutput: recordedCrashStageOutput(diagnostics, output), - }, + throw pwndbgContextFinalizationFailure( + new AnalysisCapabilityUnavailableError( + "pwndbg-core", + OPERATION, + reply.message, + { + userMessage: reply.message, + capturedOutput: recordedCrashStageOutput(diagnostics, output), + }, + ), + finalization, ); if (reply.reason === "format" || reply.reason === "output-limit") - throw new AnalysisOutputError(OPERATION, reply.message, { - capturedOutput: recordedCrashStageOutput(diagnostics, output), - }); - throw new ProviderAdapterError("pwndbg-core", OPERATION, { - diagnostics: { - stage: "debugger", - reason: reply.message, - captured_output: { ...recordedCrashStageOutput(diagnostics, output) }, - }, - }); + throw pwndbgContextFinalizationFailure( + new AnalysisOutputError(OPERATION, reply.message, { + capturedOutput: recordedCrashStageOutput(diagnostics, output), + }), + finalization, + ); + throw pwndbgContextFinalizationFailure( + new ProviderAdapterError("pwndbg-core", OPERATION, { + diagnostics: { + stage: "debugger", + reason: reply.message, + captured_output: { + ...recordedCrashStageOutput(diagnostics, output), + }, + }, + }), + finalization, + ); } if (reply.value.status !== "available") - throw new AnalysisOutputError( - OPERATION, - "Requested debugger context returned no core evidence.", - { capturedOutput: recordedCrashStageOutput(diagnostics, output) }, + throw pwndbgContextFinalizationFailure( + new AnalysisOutputError( + OPERATION, + "Requested debugger context returned no core evidence.", + { capturedOutput: recordedCrashStageOutput(diagnostics, output) }, + ), + finalization, ); - return { ...reply.value, diagnostics: output }; + return { context: { ...reply.value, diagnostics: output }, finalization }; } catch (cause: unknown) { if (cause instanceof AnalysisError && cause.cleanupIncomplete) throw cause; const primary = pwntoolsDecoderFailure( @@ -272,6 +316,51 @@ export const inspectPwndbgCore = async ({ } }; +const completedReplyFailure = ( + cause: ArtifactReaderFailure, + capturedOutput: AnalysisCapturedOutput, +): AnalysisError => { + const cancellation = + cause.reason === "cancelled" + ? new AnalysisCancelledError(OPERATION, { + cause: artifactFileCancellationCause(cause), + capturedOutput, + }) + : undefined; + if (cause.cleanup === undefined) { + if (cancellation !== undefined) return cancellation; + return new AnalysisOutputError(OPERATION, cause.message, { + cause, + capturedOutput, + }); + } + const cleanup = new ProviderCleanupError( + "pwndbg-core", + cause.cleanup.resources, + { stage: "debugger_reply_descriptor_close", reason: cause.cleanup.reason }, + { operation: OPERATION, cause, capturedOutput }, + ); + bindArtifactFileCleanup(cause, cleanup); + return cancellation === undefined + ? cleanup + : pwndbgContextFinalizationFailure(cancellation, cleanup); +}; + +/** A rejected/malformed context stays primary over its descriptor finalization. */ +export const pwndbgContextFinalizationFailure = ( + primary: AnalysisError, + finalization: AnalysisError | undefined, +): AnalysisError => + finalization === undefined + ? primary + : finalization.cleanupIncomplete + ? analysisErrorWithCleanupFailure( + primary, + finalization, + OPERATION, + ).bindCleanupDependencies([primary, finalization]) + : primary.retainSecondaryFailure(finalization); + /** Preserve both stages' exact text and their independently observed truncation state. */ export const recordedCrashStageOutput = ( basic: AnalysisCapturedOutput, diff --git a/src/native/pwntools/PwntoolsRecordedCrashProvider.ts b/src/native/pwntools/PwntoolsRecordedCrashProvider.ts index 607b161adb..bbf3047665 100644 --- a/src/native/pwntools/PwntoolsRecordedCrashProvider.ts +++ b/src/native/pwntools/PwntoolsRecordedCrashProvider.ts @@ -18,6 +18,7 @@ import { validateRecordedCrashSources } from "./RecordedCrashSourceBindings.js"; import { validateRecordedCrashStructure } from "./RecordedCrashStructureBindings.js"; import { inspectPwndbgCore, + pwndbgContextFinalizationFailure, recordedCrashStageOutput, } from "../pwndbg/PwndbgCoreContext.js"; import type { z } from "zod"; @@ -133,7 +134,7 @@ export class PwntoolsRecordedCrashProvider snapshot.bytes, ); if (!authority.ok) throw authority.error; - const context = await inspectPwndbgCore({ + const completed = await inspectPwndbgCore({ environment, architecture: value.architecture, rootPath, @@ -143,20 +144,25 @@ export class PwntoolsRecordedCrashProvider runCommand, timeoutMs, }); + const context = completed.context; const enriched = validate(context); if (!enriched.success) - throw new AnalysisOutputError( - "inspect_recorded_crash", - `Recorded debugger context is invalid: ${enriched.error.issues[0]?.message ?? "schema mismatch"}`, - { - capturedOutput: - context.status === "available" - ? recordedCrashStageOutput( - diagnostics, - context.diagnostics, - ) - : diagnostics, - }, + throw pwndbgContextFinalizationFailure( + new AnalysisOutputError( + "inspect_recorded_crash", + `Recorded debugger context is invalid: ${enriched.error.issues[0]?.message ?? "schema mismatch"}`, + { + capturedOutput: recordedCrashStageOutput( + diagnostics, + context.diagnostics, + ), + }, + ), + completed.finalization, + ); + if (completed.finalization !== undefined) + throw completed.finalization.retainPartialObservation( + enriched.data, ); return enriched.data; } catch (cause: unknown) { diff --git a/tests/boundary/native/pwndbgReplyFinalization.test.ts b/tests/boundary/native/pwndbgReplyFinalization.test.ts new file mode 100644 index 0000000000..2a63bb6c72 --- /dev/null +++ b/tests/boundary/native/pwndbgReplyFinalization.test.ts @@ -0,0 +1,244 @@ +import { createHash } from "node:crypto"; +import { mkdir, writeFile } from "node:fs/promises"; +import type { FileHandle } from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it, onTestFinished, vi } from "vitest"; + +const replyClose = vi.hoisted(() => ({ + path: "", + denied: false, + onClose: undefined as (() => void) | undefined, + handles: [] as FileHandle[], +})); + +vi.mock("node:fs/promises", async (original) => { + const actual = await original(); + return { + ...actual, + open: async (...args: Parameters) => { + const handle = await actual.open(...args); + if (args[0] === replyClose.path) { + replyClose.handles.push(handle); + const close = handle.close.bind(handle); + handle.close = async () => { + if (replyClose.denied) { + replyClose.onClose?.(); + throw new Error("controlled debugger reply close denial"); + } + await close(); + replyClose.onClose?.(); + }; + } + return handle; + }, + }; +}); + +import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; +import { + AnalysisError, + primaryAnalysisError, +} from "../../../src/domain/analysisErrorBase.js"; +import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; +import { recordedCrashSchema } from "../../../src/domain/native/recordedCrash.js"; +import { PwntoolsRecordedCrashProvider } from "../../../src/native/pwntools/PwntoolsRecordedCrashProvider.js"; +import { runOwnedCommand } from "../../../src/process/OwnedCommand.js"; +import { + recordedCrashDebuggerFixture, + recordedCrashFixture, + recordedCrashFixtureBytes, +} from "../../fixtures/binaryDiagnostics/recordedCrash.js"; +import { + createTestWorkspace, + removeTestWorkspace, +} from "../../support/workspace/workspaceFixture.js"; + +const fixture = async ( + reply = Buffer.from( + JSON.stringify({ ok: true, value: recordedCrashDebuggerFixture() }), + ), +) => { + const workspace = await createTestWorkspace("rea-pwndbg-reply-finalization-"); + const path = workspace.path("recorded.core"); + const value = recordedCrashFixture(path); + const bytes = recordedCrashFixtureBytes(value); + const sha256 = createHash("sha256").update(bytes).digest("hex"); + await writeFile(path, bytes); + const init = workspace.path("gdbinit.py"); + const venv = workspace.path("venv"); + await writeFile(init, "# authored debugger fixture\n"); + await mkdir(venv); + const resources = new ArtifactResourceScope(); + replyClose.path = workspace.path("debugger/reply.json"); + replyClose.denied = false; + replyClose.onClose = undefined; + replyClose.handles = []; + const provider = new PwntoolsRecordedCrashProvider({ + REA_PWNDBG_GDB: process.execPath, + REA_PWNDBG_GDBINIT: init, + REA_PWNDBG_VENV_PATH: venv, + }); + onTestFinished(async () => { + replyClose.denied = false; + replyClose.onClose = undefined; + await resources.close(); + await provider.close(); + await removeTestWorkspace(workspace.root); + }); + const { + artifact: _artifact, + diagnostics: _diagnostics, + decoder_diagnostics: _decoderDiagnostics, + debugger: _debugger, + ...payload + } = value; + const exits: (number | null | undefined)[] = []; + // Exercise the real projection and owned-process boundary on every host. + // The child authors debugger output; this does not establish GDB/pwndbg support. + const project = async (signal: AbortSignal) => + provider.profile.project({ + input: { path, include_debugger_context: true }, + payload, + snapshot: { bytes, sha256 }, + rootPath: workspace.root, + resources, + diagnostics: value.decoder_diagnostics, + options: { signal }, + timeoutMs: 10_000, + runCommand: async (spawn) => { + const output = await runOwnedCommand( + { + ...spawn, + command: process.execPath, + arguments: [ + "-e", + `const fs=require('node:fs');const request=JSON.parse(fs.readFileSync(process.argv[1],'utf8'));fs.writeFileSync(request.reply_path,Buffer.from(${JSON.stringify(reply.toString("base64"))},'base64'));process.stdout.write('authored debugger output');`, + spawn.arguments.at(-1) ?? "", + ], + }, + { timeoutMs: 10_000, diagnosticBytes: 1024 * 1024 }, + ); + exits.push(output.exitCode); + return output; + }, + }); + return { project, resources, value, sha256, exits }; +}; + +it.each([ + { cancel: false, denied: false }, + { cancel: true, denied: false }, + { cancel: false, denied: true }, + { cancel: true, denied: true }, +])( + "retains authored completed debugger output through reply close (cancel=$cancel, denied=$denied)", + async ({ cancel, denied }) => { + const selected = await fixture(); + replyClose.denied = denied; + const controller = new AbortController(); + const reason = { stage: "completed debugger output close" }; + if (cancel) replyClose.onClose = () => controller.abort(reason); + const result = await selected.project(controller.signal).then( + (value) => ({ ok: true as const, value }), + (cause: unknown) => { + if (!(cause instanceof AnalysisError)) throw cause; + return { ok: false as const, error: cause }; + }, + ); + expect(result.ok).toBe(!cancel && !denied); + const completed = recordedCrashSchema.parse( + result.ok ? result.value : result.error.partialObservation, + ); + expect(completed).toMatchObject({ + artifact: { path: selected.value.artifact.path, sha256: selected.sha256 }, + threads: selected.value.threads, + debugger: { + ...recordedCrashDebuggerFixture(), + diagnostics: { + stdout: "authored debugger output", + stderr: "", + truncated: false, + }, + }, + }); + expect(selected.exits).toEqual([0]); + if (!result.ok) { + if (cancel) { + expect(primaryAnalysisError(result.error)._tag).toBe( + "AnalysisCancelledError", + ); + expect(primaryAnalysisError(result.error).cause).toBe(reason); + } + expect(result.error.cleanupIncomplete).toBe(denied); + expect(projectAnalysisError(result.error)).toMatchObject({ + details: { partial_observation: completed }, + }); + if (denied) { + expect(result.error.cleanupResources).toEqual([replyClose.path]); + expect(replyClose.handles[0]?.fd).toBeGreaterThanOrEqual(0); + await expect(selected.resources.close()).rejects.toBeDefined(); + replyClose.denied = false; + await selected.resources.close(); + expect(result.error.cleanupIncomplete).toBe(false); + expect(result.error.cleanupResources).toEqual([]); + expect(result.error.partialObservation).toEqual(completed); + } + } + expect(replyClose.handles.every((handle) => handle.fd === -1)).toBe(true); + }, +); + +it.each(["malformed JSON", "malformed UTF-8", "producer refusal"])( + "keeps %s primary alongside denied close and retains only the earlier core facts", + async (scenario) => { + const reply = + scenario === "malformed JSON" + ? Buffer.from("{") + : scenario === "producer refusal" + ? Buffer.from( + JSON.stringify({ + ok: false, + reason: "unsupported", + message: "authored refusal", + }), + ) + : Buffer.from( + JSON.stringify({ + ok: true, + value: recordedCrashDebuggerFixture(), + }), + ); + if (scenario === "malformed UTF-8") { + const marker = reply.indexOf("source seam"); + expect(marker).toBeGreaterThanOrEqual(0); + reply[marker] = 0xff; + } + const selected = await fixture(reply); + replyClose.denied = true; + const controller = new AbortController(); + const reason = { stage: "rejected debugger reply close" }; + replyClose.onClose = () => controller.abort(reason); + const error: unknown = await selected + .project(controller.signal) + .catch((cause: unknown) => cause); + if (!(error instanceof AnalysisError)) + throw new Error("Expected debugger reply failure"); + expect(controller.signal.reason).toBe(reason); + expect(primaryAnalysisError(error)._tag).toBe( + scenario === "producer refusal" + ? "AnalysisCapabilityUnavailableError" + : "AnalysisOutputError", + ); + expect(error.partialObservation).toMatchObject({ + artifact: { path: selected.value.artifact.path, sha256: selected.sha256 }, + threads: selected.value.threads, + debugger: { status: "unavailable", requested: true }, + }); + expect(error.cleanupIncomplete).toBe(true); + replyClose.denied = false; + await selected.resources.close(); + expect(error.cleanupIncomplete).toBe(false); + expect(error.cleanupResources).toEqual([]); + expect(replyClose.handles.every((handle) => handle.fd === -1)).toBe(true); + }, +);