diff --git a/src/artifacts/inventory/scanReader.ts b/src/artifacts/inventory/scanReader.ts index aa2a0b391c..6873b14d74 100644 --- a/src/artifacts/inventory/scanReader.ts +++ b/src/artifacts/inventory/scanReader.ts @@ -120,6 +120,7 @@ const visitArtifactEntries = async ( iterator: currentReader.entries(context.signal)[Symbol.asyncIterator](), }, ]; + let failure: { readonly cause: unknown } | undefined; try { while (stack.length > 0) { const frame = stack.at(-1); @@ -194,10 +195,27 @@ const visitArtifactEntries = async ( }); } } + } catch (cause: unknown) { + failure = { cause }; } finally { // Readers remain owned by the inventory until its complete observation is built. - for (const { iterator } of stack) await iterator.return?.(); + for (const { iterator, prefix, reader } of stack.reverse()) { + try { + await iterator.return?.(); + } catch (cause: unknown) { + failure = { + cause: ArtifactReaderFailure.withCleanup( + failure === undefined ? cause : failure.cause, + ArtifactReaderFailure.cleanupObservation( + cause, + `artifact traversal for ${prefix || reader.format}`, + ), + ), + }; + } + } } + if (failure !== undefined) throw failure.cause; }; const digestArtifactEntry = async ( diff --git a/tests/boundary/filesystem/artifactInventorySnapshot.test.ts b/tests/boundary/filesystem/artifactInventorySnapshot.test.ts index a72c5de187..daf49215dd 100644 --- a/tests/boundary/filesystem/artifactInventorySnapshot.test.ts +++ b/tests/boundary/filesystem/artifactInventorySnapshot.test.ts @@ -1,16 +1,19 @@ -import { rm, writeFile } from "node:fs/promises"; +import { mkdir, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; -import { describe, expect, it, onTestFinished } from "vitest"; +import { createPackage } from "@electron/asar"; +import { describe, expect, it, onTestFinished, vi } from "vitest"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { translateArtifactFailure } from "../../../src/artifacts/ArtifactProvider.js"; import { ArtifactReaderFailure, + type ArtifactEntry, type ArtifactReader, } from "../../../src/artifacts/ArtifactReader.js"; import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; +import { AsarArtifactReader } from "../../../src/artifacts/AsarArtifactReader.js"; import { DirectoryArtifactReader } from "../../../src/artifacts/DirectoryArtifactReader.js"; import { scanCanonicalArtifactInventory as scanCanonicalArtifactInventoryOwned } from "../../../src/artifacts/inventory/scanCanonical.js"; import { scanArtifactInventory } from "../../fixtures/artifactInventory.js"; @@ -89,3 +92,82 @@ describe("artifact inventory snapshot", () => { expect(closeAttempts).toBe(2); }); }); + +it("unwinds every nested traversal and preserves its primary failure", async () => { + const root = await createTestTempDirectory( + "rea-inventory-traversal-cleanup-", + ); + const contents = join(root, "contents"); + const bundle = join(root, "bundle"); + await Promise.all([mkdir(contents), mkdir(bundle)]); + await writeFile(join(contents, "member.js"), "module.exports = 1;"); + await createPackage(contents, join(bundle, "app.asar")); + const directory = new DirectoryArtifactReader(bundle); + const resourceScope = new ArtifactResourceScope(); + const returned: string[] = []; + onTestFinished(async () => { + vi.restoreAllMocks(); + await resourceScope.close(); + }); + const nestedEntries = AsarArtifactReader.prototype.entries; + vi.spyOn(AsarArtifactReader.prototype, "entries").mockImplementation( + function ( + this: AsarArtifactReader, + signal, + ): AsyncIterableIterator { + const iterator = nestedEntries.call(this, signal)[Symbol.asyncIterator](); + return { + [Symbol.asyncIterator]() { + return this; + }, + async next() { + await iterator.next(); + throw new ArtifactReaderFailure( + "integrity", + "primary nested member failure", + ); + }, + async return() { + returned.push("nested"); + await iterator.return?.(); + return { done: true as const, value: undefined }; + }, + }; + }, + ); + const reader: ArtifactReader = { + format: "directory", + entries(signal): AsyncIterableIterator { + const iterator = directory.entries(signal)[Symbol.asyncIterator](); + return { + [Symbol.asyncIterator]() { + return this; + }, + next: () => iterator.next(), + async return() { + returned.push("root"); + await iterator.return?.(); + throw new Error("root iterator cleanup failed"); + }, + }; + }, + open: (entry, signal) => directory.open(entry, signal), + provenance: () => [], + close: () => directory.close(), + }; + + const failure = await scanCanonicalArtifactInventoryOwned( + bundle, + { resourceScope }, + () => reader, + ).catch((cause: unknown) => cause); + expect(failure).toMatchObject({ + reason: "integrity", + message: "primary nested member failure", + cleanup: { + reason: "root iterator cleanup failed", + resources: ["artifact traversal for directory"], + }, + }); + expect(returned).toEqual(["nested", "root"]); +});