From 08fbeb8bdab27118b6c57039aef7c0fa8b6463bc Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 18 Oct 2026 10:48:31 +0800 Subject: [PATCH] fix(pwntools): preserve producer cancellation outcomes Keep opaque reasons at admission, queued work, command/file projection and successful final cleanup. Normalize Node's ABORT_ERR envelope only at the snapshot write API that produced it, using the verified original cause. Remove downstream signal/name/identity guesses in decoder and debugger catches, including reply-close and path-probe handling. The initial regressions failed 15 cases with seven controls passing. The complete final series passes 68 focused tests, static checks, docs and discovery. Real owned Node/file fixtures use the actual macOS inspector; only the Linux x64 availability gate is simulated. No native pwntools or GDB qualification or performance improvement is claimed. Refs #4945 --- src/native/pwndbg/PwndbgCoreContext.ts | 38 ++--- src/native/pwntools/PwntoolsDecoder.ts | 83 ++++++----- src/native/pwntools/PwntoolsFailures.test.ts | 43 ++++++ src/native/pwntools/PwntoolsFailures.ts | 24 +++- .../boundary/native/PwndbgCoreContext.test.ts | 81 ++++++++++- tests/process-global/pwntoolsCleanup.test.ts | 6 +- .../pwntoolsCliFinalization.test.ts | 133 +++++++++++++++++- 7 files changed, 342 insertions(+), 66 deletions(-) create mode 100644 src/native/pwntools/PwntoolsFailures.test.ts diff --git a/src/native/pwndbg/PwndbgCoreContext.ts b/src/native/pwndbg/PwndbgCoreContext.ts index 962c60f3f..3911c72eb 100644 --- a/src/native/pwndbg/PwndbgCoreContext.ts +++ b/src/native/pwndbg/PwndbgCoreContext.ts @@ -1,6 +1,5 @@ import { bindArtifactFileCleanup, - ArtifactFileCleanupFailure, artifactFileCancellationCause, } from "../../artifacts/ArtifactFileOperation.js"; import { access, mkdir, stat, writeFile } from "node:fs/promises"; @@ -114,7 +113,7 @@ export const inspectPwndbgCore = async ({ } checkCancelled(); } catch (cause: unknown) { - if (options?.signal?.aborted) checkCancelled(); + if (cause instanceof AnalysisCancelledError) throw cause; throw unavailable( `${key} is unavailable at ${path}: ${cause instanceof Error ? cause.message : String(cause)}. Configure the requested core-only context, or omit include_debugger_context for basic recorded evidence.`, cause, @@ -154,7 +153,7 @@ export const inspectPwndbgCore = async ({ failureContext, ); try { - options?.signal?.throwIfAborted(); + checkCancelled(); const execution = await runCommand( { command: environment.REA_PWNTOOLS_PYTHON ?? "", @@ -198,16 +197,10 @@ export const inspectPwndbgCore = async ({ } catch (cause: unknown) { if ( cause instanceof ArtifactReaderFailure && - (cause.reason === "cancelled" || - (cause instanceof ArtifactFileCleanupFailure && - cause.fileCleanupFailure.outcome.kind === "completed" && - options?.signal?.aborted)) + cause.reason === "cancelled" ) { const error = new AnalysisCancelledError(OPERATION, { - cause: - cause.reason === "cancelled" - ? artifactFileCancellationCause(cause) - : options?.signal?.reason, + cause: artifactFileCancellationCause(cause), capturedOutput: recordedCrashStageOutput(diagnostics, output), ...(cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined @@ -263,21 +256,14 @@ export const inspectPwndbgCore = async ({ return { ...reply.value, diagnostics: output }; } catch (cause: unknown) { if (cause instanceof AnalysisError && cause.cleanupIncomplete) throw cause; - const primary = - options?.signal?.aborted && - (cause === options.signal.reason || - (cause instanceof Error && cause.name === "AbortError")) - ? new AnalysisCancelledError(OPERATION, { - capturedOutput: recordedCrashStageOutput(diagnostics, output), - }) - : pwntoolsDecoderFailure( - cause, - "debugger", - gdb, - environment.REA_PWNTOOLS_PYTHON ?? "", - await readPwntoolsFailureEvidence(cause, stage, resources), - failureContext, - ); + const primary = pwntoolsDecoderFailure( + cause, + "debugger", + gdb, + environment.REA_PWNTOOLS_PYTHON ?? "", + await readPwntoolsFailureEvidence(cause, stage, resources), + failureContext, + ); throw primary; } }; diff --git a/src/native/pwntools/PwntoolsDecoder.ts b/src/native/pwntools/PwntoolsDecoder.ts index 5fbd6a1c3..a0b968248 100644 --- a/src/native/pwntools/PwntoolsDecoder.ts +++ b/src/native/pwntools/PwntoolsDecoder.ts @@ -1,6 +1,5 @@ import { bindArtifactFileCleanup, - ArtifactFileCleanupFailure, artifactFileCancellationCause, } from "../../artifacts/ArtifactFileOperation.js"; import { @@ -156,7 +155,11 @@ export class PwntoolsDecoder< ): Promise> { if (options?.signal?.aborted) return Promise.resolve( - err(new AnalysisCancelledError(this.profile.operation)), + err( + new AnalysisCancelledError(this.profile.operation, { + cause: options?.signal?.reason, + }), + ), ); if (this.#closing) return Promise.resolve(err(this.#closedError())); const key = Symbol(); @@ -167,7 +170,11 @@ export class PwntoolsDecoder< const operation = this.#tail.then(() => { this.#admissions.settle(key, "admitted"); if (options?.signal?.aborted) - return err(new AnalysisCancelledError(this.profile.operation)); + return err( + new AnalysisCancelledError(this.profile.operation, { + cause: options?.signal?.reason, + }), + ); return this.#closing ? err(this.#closedError()) : inspect(); }); this.#tail = operation.then( @@ -176,7 +183,11 @@ export class PwntoolsDecoder< ); return admission.then((state) => state === "cancelled" - ? err(new AnalysisCancelledError(this.profile.operation)) + ? err( + new AnalysisCancelledError(this.profile.operation, { + cause: options?.signal?.reason, + }), + ) : operation, ); } @@ -225,7 +236,10 @@ export class PwntoolsDecoder< context, ); try { - if (options?.signal?.aborted) throw new AnalysisCancelledError(OPERATION); + if (options?.signal?.aborted) + throw new AnalysisCancelledError(OPERATION, { + cause: options.signal.reason, + }); if (process.platform !== "linux" || process.arch !== "x64") throw new AnalysisCapabilityUnavailableError( this.identity.id, @@ -284,11 +298,26 @@ export class PwntoolsDecoder< const requestPath = join(root.path, "request.json"); const snapshotPath = join(root.path, "object.snapshot"); const replyPath = join(root.path, "reply.json"); - await writeFile(snapshotPath, snapshot.bytes, { - flag: "wx", - mode: 0o600, - ...(options?.signal === undefined ? {} : { signal: options.signal }), - }); + try { + await writeFile(snapshotPath, snapshot.bytes, { + flag: "wx", + mode: 0o600, + ...(options?.signal === undefined ? {} : { signal: options.signal }), + }); + } catch (cause: unknown) { + // Normalize Node's abort envelope only at the write that produced it. + if ( + options?.signal?.aborted && + cause instanceof Error && + "code" in cause && + cause.code === "ABORT_ERR" && + cause.cause === options.signal.reason + ) + throw new AnalysisCancelledError(OPERATION, { + cause: options.signal.reason, + }); + throw cause; + } await writeFile( requestPath, JSON.stringify({ snapshot_path: snapshotPath, reply_path: replyPath }), @@ -329,16 +358,10 @@ export class PwntoolsDecoder< } catch (cause: unknown) { if ( cause instanceof ArtifactReaderFailure && - (cause.reason === "cancelled" || - (cause instanceof ArtifactFileCleanupFailure && - cause.fileCleanupFailure.outcome.kind === "completed" && - options?.signal?.aborted)) + cause.reason === "cancelled" ) { const error = new AnalysisCancelledError(OPERATION, { - cause: - cause.reason === "cancelled" - ? artifactFileCancellationCause(cause) - : options?.signal?.reason, + cause: artifactFileCancellationCause(cause), capturedOutput, ...(cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined @@ -444,21 +467,14 @@ export class PwntoolsDecoder< root?.path, this.#resources, ); - const primary = - options?.signal?.aborted && - (cause === options.signal.reason || - (cause instanceof Error && cause.name === "AbortError") || - (cause instanceof ArtifactReaderFailure && - cause.cleanup !== undefined)) - ? new AnalysisCancelledError(OPERATION, { cause }) - : pwntoolsDecoderFailure( - cause, - phase, - selectedPath, - executablePath, - evidence, - context, - ); + const primary = pwntoolsDecoderFailure( + cause, + phase, + selectedPath, + executablePath, + evidence, + context, + ); result = err(primary); } if (this.#pendingCleanup !== undefined) { @@ -480,6 +496,7 @@ export class PwntoolsDecoder< return result.ok && options?.signal?.aborted ? err( new AnalysisCancelledError(OPERATION, { + cause: options.signal.reason, partialObservation: result.value, capturedOutput: { stdout: result.value.diagnostics.stdout, diff --git a/src/native/pwntools/PwntoolsFailures.test.ts b/src/native/pwntools/PwntoolsFailures.test.ts new file mode 100644 index 000000000..0e01feb83 --- /dev/null +++ b/src/native/pwntools/PwntoolsFailures.test.ts @@ -0,0 +1,43 @@ +import { expect, it } from "vitest"; +import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; +import { OwnedCommandFailure } from "../../process/OwnedCommand.js"; +import { pwntoolsDecoderFailure } from "./PwntoolsFailures.js"; + +it.each([undefined, null, false, 0, "stop", { stop: true }, new Error("stop")])( + "preserves opaque cancellation from command and artifact owners: %s", + (reason) => { + for (const failure of [ + new OwnedCommandFailure("cancelled", "cancelled", null, null, { + cause: new Error("transport wrapper"), + cancellation: { reason }, + }), + new ArtifactReaderFailure("cancelled", "cancelled", { + cause: new Error("file wrapper"), + cancellation: { reason }, + }), + ]) { + const result = pwntoolsDecoderFailure(failure, "decoder", "/selected"); + expect(result._tag).toBe("AnalysisCancelledError"); + expect(result.cause).toBe(reason); + expect(Object.hasOwn(result, "cause")).toBe(true); + } + }, +); + +it("retains legacy owner causes without guessing cancellation from error text", () => { + const reason = { stop: "legacy" }; + const result = pwntoolsDecoderFailure( + new OwnedCommandFailure("cancelled", "cancelled", null, null, { + cause: reason, + }), + "decoder", + "/selected", + ); + expect(result.cause).toBe(reason); + const physical = Object.assign(new Error("physical failure"), { + name: "AbortError", + }); + expect(pwntoolsDecoderFailure(physical, "decoder", "/selected")._tag).toBe( + "ProviderAdapterError", + ); +}); diff --git a/src/native/pwntools/PwntoolsFailures.ts b/src/native/pwntools/PwntoolsFailures.ts index 17e3a8296..084a5fc38 100644 --- a/src/native/pwntools/PwntoolsFailures.ts +++ b/src/native/pwntools/PwntoolsFailures.ts @@ -1,3 +1,7 @@ +import { + artifactFileCancellationCause, + bindArtifactFileCleanup, +} from "../../artifacts/ArtifactFileOperation.js"; import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; import { AnalysisError, @@ -91,7 +95,13 @@ export const pwntoolsDecoderFailure = ( { operation: context.operation, cause }, ); if (cause.reason === "cancelled") - return new AnalysisCancelledError(context.operation, outputOptions); + return new AnalysisCancelledError(context.operation, { + ...outputOptions, + cause: + cause.cancellation === undefined + ? cause.cause + : cause.cancellation.reason, + }); if (cause.reason === "timeout") return new AnalysisTimeoutError( context.operation, @@ -175,8 +185,16 @@ export const pwntoolsDecoderFailure = ( cause, }, ); - if (cause.reason === "cancelled") - return new AnalysisCancelledError(context.operation); + if (cause.reason === "cancelled") { + const error = new AnalysisCancelledError(context.operation, { + cause: artifactFileCancellationCause(cause), + cleanup: cause.cleanup, + partialObservation: cause.partialObservation, + capturedOutput: context.precedingOutput, + }); + bindArtifactFileCleanup(cause, error); + return error; + } if ( cause.reason === "limit" || cause.reason === "path" || diff --git a/tests/boundary/native/PwndbgCoreContext.test.ts b/tests/boundary/native/PwndbgCoreContext.test.ts index f4b72876f..cd0397010 100644 --- a/tests/boundary/native/PwndbgCoreContext.test.ts +++ b/tests/boundary/native/PwndbgCoreContext.test.ts @@ -3,6 +3,7 @@ import { execFile } from "node:child_process"; import { promisify } from "node:util"; import { join } from "node:path"; import { expect, it, onTestFinished } from "vitest"; +import { OwnedCommandFailure } from "../../../src/process/OwnedCommand.js"; import { AnalysisCancelledError } from "../../../src/domain/analysisErrorCore.js"; import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; import { @@ -14,7 +15,7 @@ import { inspectPwndbgCore } from "../../../src/native/pwndbg/PwndbgCoreContext. const diagnostics = { stdout: "", stderr: "", truncated: false }; it.each(["pre-aborted", "aborted-during-probe"] as const)( - "cancellation wins over debugger path failures: %s", + "observes cancellation at the debugger admission checkpoint: %s", async (scenario) => { const workspace = await createTestWorkspace("rea-pwndbg-cancel-"); onTestFinished(() => removeTestWorkspace(workspace.root)); @@ -31,7 +32,7 @@ it.each(["pre-aborted", "aborted-during-probe"] as const)( resolve(); }); }); - throw new Error("path became unavailable"); + return; } throw new Error("path is unavailable"); }; @@ -118,3 +119,79 @@ it.each(["x86_64-little-endian", "aarch64-little-endian"] as const)( expect(requested).toBe(true); }, ); + +it.each(["cancelled", "physical-name", "physical-identity"])( + "uses the debugger command owner's outcome despite a late signal: %s", + async (outcome) => { + const workspace = await createTestWorkspace("rea-pwndbg-command-outcome-"); + onTestFinished(() => removeTestWorkspace(workspace.root)); + const controller = new AbortController(); + const reason = { stage: "command handoff" }; + const failure = new OwnedCommandFailure( + outcome === "cancelled" ? "cancelled" : "process", + "owned command outcome", + null, + null, + { cancellation: outcome === "cancelled" ? { reason } : undefined }, + ); + if (outcome === "physical-name") failure.name = "AbortError"; + const inspection = inspectPwndbgCore({ + environment: { + REA_PWNDBG_GDB: workspace.path("gdb"), + REA_PWNDBG_GDBINIT: workspace.path("gdbinit.py"), + REA_PWNDBG_VENV_PATH: workspace.path("venv"), + }, + architecture: "x86_64-little-endian", + rootPath: workspace.root, + resources: new ArtifactResourceScope(), + diagnostics: { stdout: "basic evidence", stderr: "", truncated: false }, + options: { signal: controller.signal }, + probePath: async () => {}, + runCommand: async () => { + controller.abort(outcome === "physical-identity" ? failure : reason); + throw failure; + }, + }); + const result = await inspection.catch((cause: unknown) => cause); + if (!(result instanceof Error)) + throw new Error("Expected a typed command failure"); + expect(result).toMatchObject({ + _tag: + outcome === "cancelled" + ? "AnalysisCancelledError" + : "ProviderAdapterError", + }); + expect(result.cause).toBe(outcome === "cancelled" ? reason : failure); + }, +); + +it("retains a physical debugger path failure when cancellation arrives in the probe", async () => { + const workspace = await createTestWorkspace("rea-pwndbg-probe-failure-"); + onTestFinished(() => removeTestWorkspace(workspace.root)); + const controller = new AbortController(); + const physical = Object.assign(new Error("path denied"), { code: "EACCES" }); + const inspection = inspectPwndbgCore({ + environment: { + REA_PWNDBG_GDB: workspace.path("gdb"), + REA_PWNDBG_GDBINIT: workspace.path("gdbinit.py"), + REA_PWNDBG_VENV_PATH: workspace.path("venv"), + }, + architecture: "x86_64-little-endian", + rootPath: workspace.root, + resources: new ArtifactResourceScope(), + diagnostics, + options: { signal: controller.signal }, + probePath: async () => { + controller.abort({ late: true }); + throw physical; + }, + runCommand: () => { + throw new Error("No command may start after probe failure"); + }, + }); + const failure = await inspection.catch((cause: unknown) => cause); + expect(failure).toMatchObject({ + _tag: "AnalysisCapabilityUnavailableError", + cause: physical, + }); +}); diff --git a/tests/process-global/pwntoolsCleanup.test.ts b/tests/process-global/pwntoolsCleanup.test.ts index 97e35ae66..f976c2083 100644 --- a/tests/process-global/pwntoolsCleanup.test.ts +++ b/tests/process-global/pwntoolsCleanup.test.ts @@ -228,11 +228,15 @@ it.runIf(supportedHost)( await started; const controller = new AbortController(); const cancelled = provider.inspect({ path }, { signal: controller.signal }); - controller.abort(); + const reason = { stage: "queued" }; + controller.abort(reason); await expect(cancelled).resolves.toMatchObject({ ok: false, error: { _tag: "AnalysisCancelledError" }, }); + const cancelledResult = await cancelled; + if (cancelledResult.ok) throw new Error("Expected queued cancellation"); + expect(cancelledResult.error.cause).toBe(reason); const queued = provider.inspect({ path }); const closing = provider.close(); const late = provider.inspect({ path }); diff --git a/tests/process-global/pwntoolsCliFinalization.test.ts b/tests/process-global/pwntoolsCliFinalization.test.ts index 4ba9f2c12..8728e68bc 100644 --- a/tests/process-global/pwntoolsCliFinalization.test.ts +++ b/tests/process-global/pwntoolsCliFinalization.test.ts @@ -22,6 +22,10 @@ import { PrivateRuntimeRoot } from "../../src/process/PrivateRuntimeRoot.js"; import { cleanupOwnedProcessGroup } from "../../src/process/ProcessOwnership.js"; import { prepareProcessOwnershipInspection } from "../../src/process/ProcessOwnershipObservation.js"; import { spawnOwnedProviderProcess } from "../../src/process/ProviderProcess.js"; +import { + waitForProviderProcessReady, + stopProviderProcessFixture, +} from "../fixtures/providerProcess.js"; import { binaryLayoutFixture } from "../fixtures/binaryDiagnostics/layout.js"; import { createTestWorkspace, @@ -275,6 +279,7 @@ it.runIf(supportedHost)( async () => { const { path, root, launch } = await fixture(); const controller = new AbortController(); + const reason = { stage: "final cleanup" }; let removals = 0; const provider = new PwntoolsLayoutProvider( { REA_PWNTOOLS_PYTHON: process.execPath }, @@ -283,7 +288,7 @@ it.runIf(supportedHost)( path: root.path, close: async () => { if (++removals === 1) { - controller.abort(); + controller.abort(reason); throw new Error("transient removal failure"); } await root.close(); @@ -297,6 +302,8 @@ it.runIf(supportedHost)( ); if (result.ok) throw new Error("Cancelled inspection must not publish success"); + expect(result.error.cause).toBe(reason); + expect(result.error.partialObservation).toBeDefined(); expect(projectAnalysisError(result.error)).toMatchObject({ code: "cancelled", details: { @@ -306,3 +313,127 @@ it.runIf(supportedHost)( await expect(access(root.path)).rejects.toMatchObject({ code: "ENOENT" }); }, ); + +it.runIf(supportedHost).each(["inspect", "inspectAndClose"] as const)( + "preserves pre-admission reasons without acquiring resources: %s", + async (method) => { + const reason = { stage: "pre-admission" }; + const controller = new AbortController(); + controller.abort(reason); + const provider = new PwntoolsLayoutProvider({}); + onTestFinished(() => provider.close()); + const result = await provider[method]( + { path: "/not-acquired" }, + { signal: controller.signal }, + ); + if (result.ok) throw new Error("Expected pre-admission cancellation"); + expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(result.error.cause).toBe(reason); + const retry = await provider.inspect({ path: "/not-acquired" }); + if (retry.ok) throw new Error("Unconfigured provider must fail"); + expect(retry.error.message).not.toContain("Provider is closing"); + }, +); + +it.runIf(supportedHost)( + "normalizes cancellation from the actual snapshot write boundary", + async () => { + const { path, root, launch } = await fixture(); + const reason = { stage: "snapshot write" }; + const controller = new AbortController(); + const provider = new PwntoolsLayoutProvider( + { REA_PWNTOOLS_PYTHON: process.execPath }, + launch, + async () => { + controller.abort(reason); + return root; + }, + ); + onTestFinished(() => provider.close()); + const result = await provider.inspect( + { path }, + { signal: controller.signal }, + ); + if (result.ok) throw new Error("Expected snapshot write cancellation"); + expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(result.error.cause).toBe(reason); + await expect(access(root.path)).rejects.toMatchObject({ code: "ENOENT" }); + }, +); + +it.runIf(supportedHost).each(["AbortError-name", "same-as-reason"])( + "preserves a physical runtime acquisition failure despite late cancellation: %s", + async (scenario) => { + const { path, root, launch } = await fixture(); + onTestFinished(() => root.close()); + const physical = Object.assign( + new Error("owned runtime acquisition failed"), + { name: scenario === "AbortError-name" ? "AbortError" : "Error" }, + ); + const controller = new AbortController(); + const provider = new PwntoolsLayoutProvider( + { REA_PWNTOOLS_PYTHON: process.execPath }, + launch, + async () => { + controller.abort( + scenario === "same-as-reason" ? physical : { late: true }, + ); + throw physical; + }, + ); + onTestFinished(() => provider.close()); + const result = await provider.inspect( + { path }, + { signal: controller.signal }, + ); + if (result.ok) throw new Error("Expected runtime acquisition failure"); + expect(result.error._tag).toBe("ProviderAdapterError"); + expect(result.error.cause).toBe(physical); + }, +); + +it.runIf(supportedHost)( + "preserves cancellation across a late owned process handshake", + async () => { + const { path, root } = await fixture(); + const controller = new AbortController(); + const reason = { stage: "late owned handshake" }; + let child: + | Awaited>["process"] + | undefined; + const provider = new PwntoolsLayoutProvider( + { REA_PWNTOOLS_PYTHON: process.execPath }, + async (input) => { + const launched = await spawnOwnedProviderProcess({ + ...input, + arguments: [ + "-e", + "process.stdout.write('ready\\n'); setInterval(() => {}, 1000)", + ], + }); + child = launched.process; + await waitForProviderProcessReady(launched.process); + controller.abort(reason); + return launched; + }, + async () => root, + ); + onTestFinished(async () => { + try { + await provider.close(); + } finally { + if (child !== undefined) await stopProviderProcessFixture(child); + } + }); + const result = await provider.inspect( + { path }, + { signal: controller.signal }, + ); + if (result.ok) throw new Error("Expected handshake cancellation"); + expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(result.error.cause).toBe(reason); + expect(child).toBeDefined(); + expect(child?.exitCode !== null || child?.signalCode !== null).toBe(true); + await expect(access(root.path)).rejects.toMatchObject({ code: "ENOENT" }); + }, +);