From 8dc2d7bec4539128a53b97136c7df5ff7f04e0e8 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 9 Nov 2026 19:42:54 +0800 Subject: [PATCH] fix(firmware): retain command output after report read failure (#9622) * fix(javascript): preserve recovery report cancellation * fix(javascript): retain recovery output on report failures * fix(firmware): retain output on report read failure --- src/firmware/FirmwareCommand.ts | 17 ++- src/firmware/FirmwareProvider.ts | 2 + .../firmware/firmwareReportFiles.test.ts | 126 ++++++++++++++++++ 3 files changed, 144 insertions(+), 1 deletion(-) diff --git a/src/firmware/FirmwareCommand.ts b/src/firmware/FirmwareCommand.ts index 1ec49a9d42..cce526ca7f 100644 --- a/src/firmware/FirmwareCommand.ts +++ b/src/firmware/FirmwareCommand.ts @@ -417,6 +417,7 @@ export const readFirmwareCommandReport = async (context: { execution: Awaited>; files: FirmwareFileAccess; signal?: AbortSignal; + deadline?: FirmwareDeadline; }): Promise => { try { return await context.files.report( @@ -425,7 +426,21 @@ export const readFirmwareCommandReport = async (context: { context.signal, ); } catch (cause: unknown) { - if (cause instanceof ArtifactReaderFailure) throw cause; + if (cause instanceof ArtifactReaderFailure) { + const { engine, operation, execution, deadline } = context; + const failure = firmwareFailure(cause, { engine, operation, deadline }); + if (cause.partialObservation !== undefined) + failure.retainPartialObservation(cause.partialObservation); + throw failure.retainCapturedOutput({ + stdout: execution.stdout.text, + stderr: execution.stderr.text, + stdout_bytes: execution.stdout.bytes, + stderr_bytes: execution.stderr.bytes, + exit_code: execution.exit_code ?? null, + signal: execution.signal, + truncated: false, + }); + } const { engine, operation, execution } = context; const reportFailure = cause instanceof AnalysisOutputError diff --git a/src/firmware/FirmwareProvider.ts b/src/firmware/FirmwareProvider.ts index 4941cb2049..a13591414c 100644 --- a/src/firmware/FirmwareProvider.ts +++ b/src/firmware/FirmwareProvider.ts @@ -463,6 +463,7 @@ export class FirmwareProvider implements FirmwareAnalysisPort { execution: processRun, files: this.#files, signal, + deadline, }); result = firmwareResultSchemas.inspect_firmware_regions.parse({ analysis_timeout_ms: analysisTimeoutMs, @@ -507,6 +508,7 @@ export class FirmwareProvider implements FirmwareAnalysisPort { execution: processRun, files: this.#files, signal, + deadline, }); const normalized = normalizeUnblobReport(report, { inputPath: target.inputPath, diff --git a/tests/boundary/firmware/firmwareReportFiles.test.ts b/tests/boundary/firmware/firmwareReportFiles.test.ts index c5847c23c4..41868f917b 100644 --- a/tests/boundary/firmware/firmwareReportFiles.test.ts +++ b/tests/boundary/firmware/firmwareReportFiles.test.ts @@ -7,6 +7,9 @@ import { FirmwareFileAccess, } from "../../../src/firmware/FirmwareFiles.js"; import { readFirmwareCommandReport } from "../../../src/firmware/FirmwareCommand.js"; +import { ArtifactReaderFailure } from "../../../src/artifacts/ArtifactReader.js"; +import { AnalysisCleanupLease } from "../../../src/artifacts/AnalysisCleanupLease.js"; +import { firmwareFailure } from "../../../src/firmware/FirmwareFailures.js"; import { FIRMWARE_LIMITS } from "../../../src/firmware/FirmwareRelease.js"; import { AnalysisError } from "../../../src/domain/analysisErrorBase.js"; import { AnalysisResourceConstraintError } from "../../../src/domain/analysisErrorCore.js"; @@ -457,3 +460,126 @@ it("rejects Binwalk extraction activity under every own JSON member", async () = await files.close(); } }); + +it.each([false, true])( + "retains completed command output after report read failure (cleanup pending: %s)", + async (pending) => { + const lease = new AnalysisCleanupLease(async () => {}); + const reader = pending + ? ArtifactReaderFailure.withCleanup( + new Error("report read failed"), + { reason: "close failed", resources: ["/work/report"] }, + undefined, + [lease], + ) + : new ArtifactReaderFailure("io", "report read failed"); + const files = new FirmwareFileAccess(); + vi.spyOn(files, "report").mockRejectedValue(reader); + const cause: unknown = await readFirmwareCommandReport({ + path: "/work/report", + engine: "binwalk", + operation: "inspect_firmware_regions", + files, + execution: { + stdout: { text: "found output", bytes: 12, observedBytes: 12 }, + stderr: { text: "warning", bytes: 7, observedBytes: 7 }, + exit_code: 0, + signal: null, + command: "/opt/binwalk", + args: [], + resource_limiter: "/usr/bin/prlimit", + }, + }).catch((failure: unknown) => failure); + const failure = firmwareFailure(cause, { + engine: "binwalk", + operation: "inspect_firmware_regions", + }); + expect( + projectAnalysisError(failure).details?.captured_output, + ).toMatchObject({ + stdout: "found output", + stderr: "warning", + stdout_bytes: 12, + stderr_bytes: 7, + exit_code: 0, + }); + if (pending) { + expect(failure.cleanupIncomplete).toBe(true); + await lease.release(); + expect(failure.cleanupIncomplete).toBe(false); + } + await files.close(); + }, +); + +it("retains the analysis timeout and command output while report cleanup is pending and after retry", async () => { + const deadlineSignal = new AbortController(); + const reason = new Error("analysis deadline"); + deadlineSignal.abort(reason); + const lease = new AnalysisCleanupLease(async () => {}); + const reader = ArtifactReaderFailure.withCleanup( + new ArtifactReaderFailure("cancelled", "deadline interrupted report read", { + cause: reason, + }), + { reason: "close failed", resources: ["/work/report"] }, + { + kind: "file-snapshot-hash", + path: "/work/report", + sha256: "a".repeat(64), + bytes: 12, + }, + [lease], + ); + const files = new FirmwareFileAccess(); + vi.spyOn(files, "report").mockRejectedValue(reader); + const failure: unknown = await readFirmwareCommandReport({ + path: "/work/report", + engine: "binwalk", + operation: "inspect_firmware_regions", + files, + deadline: { + phase: "analysis", + timeoutMs: 42, + signal: deadlineSignal.signal, + expiresAtMs: 0, + }, + execution: { + stdout: { text: "found output", bytes: 12, observedBytes: 12 }, + stderr: { text: "warning", bytes: 7, observedBytes: 7 }, + exit_code: 0, + signal: null, + command: "/opt/binwalk", + args: [], + resource_limiter: "/usr/bin/prlimit", + }, + }).catch((cause: unknown) => cause); + if (!(failure instanceof AnalysisError)) + throw new Error("Expected typed report failure"); + expect(failure.cleanupIncomplete).toBe(true); + expect(failure.partialObservation).toMatchObject({ + kind: "file-snapshot-hash", + path: "/work/report", + }); + expect(projectAnalysisError(failure).details?.captured_output).toMatchObject({ + stdout: "found output", + stderr: "warning", + exit_code: 0, + }); + await lease.release(); + expect(failure.cleanupIncomplete).toBe(false); + expect( + projectAnalysisError(failure).details?.partial_observation, + ).toMatchObject({ kind: "file-snapshot-hash", path: "/work/report" }); + expect(projectAnalysisError(failure)).toMatchObject({ + code: "provider_timeout", + details: { + timeout_ms: 42, + captured_output: { + stdout: "found output", + stderr: "warning", + exit_code: 0, + }, + }, + }); + await files.close(); +});