diff --git a/src/application/javascript/JavaScriptArtifactReconstruction.ts b/src/application/javascript/JavaScriptArtifactReconstruction.ts index e16e67b568..ae9ce8f8f7 100644 --- a/src/application/javascript/JavaScriptArtifactReconstruction.ts +++ b/src/application/javascript/JavaScriptArtifactReconstruction.ts @@ -146,14 +146,10 @@ export const reconstructJavaScriptArtifact = async ( const earlyCleanup = await scope.release(readerOwner); readerOpen = false; if (earlyCleanup.kind === "failed") - throw ArtifactReaderFailure.withCleanup( - earlyCleanup.cause, - ArtifactReaderFailure.cleanupObservation( - earlyCleanup.cause, - readerOwner.resource, - ), - { kind: "artifact-inventory", inventory: snapshot }, - ); + throw scope.cleanupFailure({ kind: "completed" }, [earlyCleanup], { + kind: "artifact-inventory", + inventory: snapshot, + }); await reportPhase( "parse_javascript_sources", `Processing ${String(files.files.length - (files.source_selection?.unselected_files ?? 0))} selected application files`, @@ -410,19 +406,13 @@ export const reconstructJavaScriptArtifact = async ( if (readerOpen) { const cleanupAttempt = await scope.release(readerOwner); if (cleanupAttempt.kind === "failed") { - const cleanup = ArtifactReaderFailure.cleanupObservation( - cleanupAttempt.cause, - readerOwner.resource, - ); const partialObservation: ArtifactInventoryPartialObservation = { kind: "artifact-inventory", inventory: snapshot, }; - throw ArtifactReaderFailure.withCleanup( - reconstructionOutcome.kind === "failed" - ? reconstructionOutcome.cause - : cleanupAttempt.cause, - cleanup, + throw scope.cleanupFailure( + reconstructionOutcome, + [cleanupAttempt], partialObservation, ); } diff --git a/src/artifacts/ArtifactHash.ts b/src/artifacts/ArtifactHash.ts index 7c462eb3c8..35d3ac61b3 100644 --- a/src/artifacts/ArtifactHash.ts +++ b/src/artifacts/ArtifactHash.ts @@ -32,6 +32,25 @@ export const hashReadable = async ( signal?: AbortSignal, integrityAlgorithm?: "sha256" | "md5", asarHeaderAdmission?: AsarHeaderAdmission, +): Promise => { + const completed = await hashReadableToCompletion( + stream, + signal, + integrityAlgorithm, + asarHeaderAdmission, + ); + abortIfNeeded(signal); + return completed; +}; + +/** Keep a fully delivered digest so its owner can validate before late cancellation. + * Source stability remains the stream or caller's responsibility. + */ +export const hashReadableToCompletion = async ( + stream: Readable, + signal?: AbortSignal, + integrityAlgorithm?: "sha256" | "md5", + asarHeaderAdmission?: AsarHeaderAdmission, ): Promise => { const hash = createHash("sha256"); const md5 = integrityAlgorithm === "md5" ? createHash("md5") : undefined; @@ -45,7 +64,6 @@ export const hashReadable = async ( md5?.update(chunk); probe.write(chunk); } - abortIfNeeded(signal); return { sha256: hash.digest("hex"), ...(md5 === undefined ? {} : { md5: md5.digest("hex") }), diff --git a/src/artifacts/javascript/JavaScriptArtifactFiles.ts b/src/artifacts/javascript/JavaScriptArtifactFiles.ts index e5fd6dfba9..38519d7ca6 100644 --- a/src/artifacts/javascript/JavaScriptArtifactFiles.ts +++ b/src/artifacts/javascript/JavaScriptArtifactFiles.ts @@ -16,7 +16,7 @@ import { type ArtifactReader, } from "../ArtifactReader.js"; import { streamChunkToBuffer } from "../StreamBytes.js"; -import { hashReadable } from "../ArtifactHash.js"; +import { hashReadableToCompletion } from "../ArtifactHash.js"; import type { ArtifactOccurrence } from "../../domain/artifactGraph.js"; import type { ArtifactInventorySnapshot } from "../../domain/artifactInventorySnapshot.js"; import { AnalysisInputError } from "../../domain/analysisErrorCore.js"; @@ -400,17 +400,13 @@ const verifyEntryBytes = async ( expected: Pick, signal?: AbortSignal, ): Promise => { - try { - const digest = await hashReadable(stream, signal); - if (digest.sha256 !== expected.sha256 || digest.bytes !== expected.bytes) - throw new ArtifactReaderFailure( - "integrity", - `Artifact entry changed after inventory: ${expected.path}`, - ); - } catch (cause: unknown) { - abortIfNeeded(signal); - throw cause; - } + const digest = await hashReadableToCompletion(stream, signal); + if (digest.sha256 !== expected.sha256 || digest.bytes !== expected.bytes) + throw new ArtifactReaderFailure( + "integrity", + `Artifact entry changed after inventory: ${expected.path}`, + ); + abortIfNeeded(signal); }; const expectedInventory = ( diff --git a/tests/process-global/javascriptArtifactVerification.test.ts b/tests/process-global/javascriptArtifactVerification.test.ts new file mode 100644 index 0000000000..94656d8a71 --- /dev/null +++ b/tests/process-global/javascriptArtifactVerification.test.ts @@ -0,0 +1,177 @@ +import type { FileHandle } from "node:fs/promises"; +import { open, realpath, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it, onTestFinished, vi } from "vitest"; +import { analyzeJavaScriptApplication } from "../../src/application/javascript/JavaScriptApplicationService.js"; +import { ArtifactResourceScope } from "../../src/artifacts/ArtifactResourceScope.js"; +import { OwnedFileHandle } from "../../src/filesystem/OwnedFileHandle.js"; +import { DirectoryArtifactReader } from "../../src/artifacts/DirectoryArtifactReader.js"; +import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; + +const injected = vi.hoisted(() => ({ + opened: undefined as ((path: unknown, file: FileHandle) => void) | undefined, +})); +vi.mock("node:fs/promises", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + open: async (...args: Parameters) => { + const file = await actual.open(...args); + injected.opened?.(args[0], file); + return file; + }, + }; +}); + +const causesOf = (error: unknown): unknown[] => { + if (!(error instanceof Error)) return [error]; + return [ + error, + ...causesOf(error.cause), + ...(error instanceof AggregateError ? error.errors.flatMap(causesOf) : []), + ]; +}; + +it.each( + ["addon.node", "unselected.js"].flatMap((filename) => + [ + "physical failure", + "failed cleanup", + "completed cleanup", + "abort only", + "changed bytes", + "completed mismatch cancellation", + "completed matching cancellation", + ].map((outcome) => ({ filename, outcome })), + ), +)( + "preserves $filename verifier provenance through $outcome", + async ({ filename, outcome }) => { + const root = await realpath( + await createTestTempDirectory("rea-js-verifier-failure-"), + ); + const path = join(root, filename); + await writeFile(path, "original payload"); + await writeFile(join(root, "main.js"), "module.exports = 42;\n"); + const scope = new ArtifactResourceScope(); + const controller = new AbortController(); + const reason = { source: "artifact source acquisition" }; + const physical = Object.assign( + new Error("physical acquisition read denied"), + { code: "EIO", errno: -5, syscall: "read" }, + ); + let acquisition = false; + let blocked = + outcome === "failed cleanup" || outcome === "completed cleanup"; + let independentBlocked = false; + let descriptor: FileHandle | undefined; + const openEntry = DirectoryArtifactReader.prototype.open; + vi.spyOn(DirectoryArtifactReader.prototype, "open").mockImplementation( + async function (this: DirectoryArtifactReader, entry, signal) { + const stream = await openEntry.call(this, entry, signal); + if ( + acquisition && + entry.adapterKey === path && + outcome.startsWith("completed ") && + outcome !== "completed cleanup" + ) + stream.once("end", () => controller.abort(reason)); + return stream; + }, + ); + injected.opened = (candidate, file) => { + if (!acquisition || String(candidate) !== path) return; + descriptor = file; + const close = file.close.bind(file); + vi.spyOn(file, "close").mockImplementation(async () => { + if (blocked) throw new Error("acquisition descriptor close denied"); + await close(); + }); + if (outcome === "abort only") controller.abort(reason); + if (outcome === "physical failure" || outcome === "failed cleanup") + vi.spyOn(file, "read").mockImplementationOnce(async () => { + controller.abort(reason); + throw physical; + }); + }; + onTestFinished(async () => { + injected.opened = undefined; + blocked = false; + independentBlocked = false; + await scope.close(); + vi.restoreAllMocks(); + }); + const result = await analyzeJavaScriptApplication( + { input_path: root, source_paths: ["main.js"] }, + scope, + { + signal: controller.signal, + progress: { + report: async (update) => { + if (update.phase !== "read_javascript_artifacts") return; + acquisition = true; + if ( + outcome === "changed bytes" || + outcome === "completed mismatch cancellation" + ) + await writeFile(path, "modified payload"); + }, + }, + }, + ); + if (result.ok) throw new Error("Expected acquisition failure"); + if ( + outcome === "abort only" || + outcome === "completed matching cancellation" + ) + expect(result.error).toMatchObject({ + reason: "cancelled", + cause: reason, + }); + else if ( + outcome === "changed bytes" || + outcome === "completed mismatch cancellation" + ) + expect(result.error).toMatchObject({ reason: "integrity" }); + else { + expect(result.error).toMatchObject({ reason: "io" }); + if (outcome !== "completed cleanup") + expect(causesOf(result.error)).toContain(physical); + } + expect(descriptor).toBeDefined(); + if (blocked) { + const independentPath = join(root, "independent-owner"); + await writeFile(independentPath, "separately retained bytes"); + const independent = await open(independentPath); + const closeIndependent = independent.close.bind(independent); + independentBlocked = true; + vi.spyOn(independent, "close").mockImplementation(async () => { + if (independentBlocked) + throw new Error("independent owner close denied"); + await closeIndependent(); + }); + scope.retain({ + kind: "file-handle", + resource: independentPath, + handle: new OwnedFileHandle(independent), + }); + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.partialObservation).toMatchObject({ + kind: "artifact-inventory", + }); + await expect(scope.close()).rejects.toBeDefined(); + expect(descriptor!.fd).toBeGreaterThanOrEqual(0); + blocked = false; + await expect(scope.close()).rejects.toBeDefined(); + expect(descriptor!.fd).toBe(-1); + expect(result.error.cleanupIncomplete).toBe(false); + expect(independent.fd).toBeGreaterThanOrEqual(0); + independentBlocked = false; + await scope.close(); + expect(independent.fd).toBe(-1); + } + await scope.close(); + expect(descriptor!.fd).toBe(-1); + expect(result.error.cleanupIncomplete).toBe(false); + }, +);