diff --git a/src/artifacts/NativeDmgArtifactReader.ts b/src/artifacts/NativeDmgArtifactReader.ts index 93c12ece3..d8636f4ae 100644 --- a/src/artifacts/NativeDmgArtifactReader.ts +++ b/src/artifacts/NativeDmgArtifactReader.ts @@ -1,4 +1,4 @@ -import { mkdtemp, realpath, rm } from "node:fs/promises"; +import { mkdtemp, realpath, rm, type FileHandle } from "node:fs/promises"; import { tmpdir } from "node:os"; import { basename, join } from "node:path"; import type { Readable } from "node:stream"; @@ -11,6 +11,12 @@ import { type ArtifactTraversalOptions, } from "./ArtifactReader.js"; import { DirectoryArtifactReader } from "./DirectoryArtifactReader.js"; +import { + NonRegularFileReadError, + RegularFileAdmissionFailure, + openRegularFile, +} from "../filesystem/RegularFile.js"; +import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; import { execFileOutput, execFileOutputFailure, @@ -44,16 +50,24 @@ export interface NativeDmgHost { delay?(milliseconds: number): Promise; } +interface NativeDmgFailureClassification { + readonly reason: ArtifactReaderFailure["reason"]; + readonly verifyDiagnostic?: string; + /** + * hdiutil could not recognize the target, for a reason it reported only in + * localized text; the cause stays unknown unless the target cannot be opened. + */ + readonly unrecognizedImage?: boolean; +} + class NativeDmgCommandFailure extends ArtifactReaderFailure { readonly processNotStarted: boolean; readonly verifyDiagnostic: string | undefined; + readonly unrecognizedImage: boolean; readonly exitCode: number | null; constructor( - failure: { - readonly reason: ArtifactReaderFailure["reason"]; - readonly verifyDiagnostic?: string; - }, + failure: NativeDmgFailureClassification, message: string, readonly stdout: string, options?: { @@ -69,6 +83,7 @@ class NativeDmgCommandFailure extends ArtifactReaderFailure { ); this.processNotStarted = options?.processNotStarted ?? false; this.verifyDiagnostic = failure.verifyDiagnostic; + this.unrecognizedImage = failure.unrecognizedImage ?? false; this.exitCode = options?.exitCode ?? null; } } @@ -136,6 +151,7 @@ export class NativeDmgArtifactReader implements ArtifactReader { #closed = false; #closePromise: Promise | undefined; readonly #activeDirectoryOperations = new Set>(); + #verificationProbeHandle: OwnedFileHandle | undefined; constructor( private readonly path: string, @@ -207,6 +223,7 @@ export class NativeDmgArtifactReader implements ArtifactReader { async #closeOwnedResources(): Promise { await this.#initialization?.catch(() => undefined); await Promise.allSettled(this.#activeDirectoryOperations); + await this.#closeVerificationProbeHandle(); await this.#closeMountedDirectory(); let detachFailure = await this.#detachOwnedDevices(); if ( @@ -253,6 +270,23 @@ export class NativeDmgArtifactReader implements ArtifactReader { ); } + async #closeVerificationProbeHandle(): Promise { + const owner = this.#verificationProbeHandle; + if (owner === undefined) return; + try { + await owner.close(); + this.#verificationProbeHandle = undefined; + } catch (cause: unknown) { + throw ArtifactReaderFailure.withCleanup( + cause, + ArtifactReaderFailure.cleanupObservation( + cause, + `DMG verification target descriptor ${this.path}`, + ), + ); + } + } + async #detachOwnedDevices(): Promise { let detachFailure = await this.#reconcileAttachState(); let remainingDevices: string[] = []; @@ -455,13 +489,7 @@ export class NativeDmgArtifactReader implements ArtifactReader { const arguments_ = ["verify", this.path]; const deadline = performance.now() + HDIUTIL_COMMAND_TIMEOUT_MS; for (let attempt = 0; ; attempt += 1) { - if (signal?.aborted === true) - throw new NativeDmgCommandFailure( - { reason: "cancelled" }, - "DMG operation cancelled", - "", - { cause: signal.reason }, - ); + this.#throwIfVerificationCancelled(signal); try { await runChecked(this.host, arguments_, signal, { timeoutMs: Math.max(1, Math.ceil(deadline - performance.now())), @@ -471,10 +499,28 @@ export class NativeDmgArtifactReader implements ArtifactReader { } catch (cause: unknown) { if (!(cause instanceof NativeDmgCommandFailure)) throw cause; this.#provenance.push(command(arguments_, ["read"], cause.exitCode)); + if (cause.unrecognizedImage) { + const unreadable = await this.#isUnreadableTarget(signal, cause); + this.#throwIfVerificationCancelled(signal); + if (unreadable) + throw new NativeDmgCommandFailure( + { reason: "io" }, + cause.message, + cause.stdout, + { + cause: cause.cause, + ...(cause.exitCode === null + ? {} + : { exitCode: cause.exitCode }), + }, + ); + } const delayMs = VERIFY_SETTLE_DELAYS_MS[attempt]; - // Retry only the producer's explicit transient diagnostic, before any mount. + // Retry only the producer's explicit transient diagnostic, or a localized + // refusal of a readable target that may be the same, before any mount. if ( - cause.verifyDiagnostic !== "Resource temporarily unavailable" || + (cause.verifyDiagnostic !== "Resource temporarily unavailable" && + !cause.unrecognizedImage) || delayMs === undefined || deadline - performance.now() <= delayMs ) @@ -523,6 +569,72 @@ export class NativeDmgArtifactReader implements ArtifactReader { this.#devices = []; return "none"; } + + #cancelledVerification(cause: unknown): NativeDmgCommandFailure { + return new NativeDmgCommandFailure( + { reason: "cancelled" }, + "DMG operation cancelled", + "", + { cause }, + ); + } + + #throwIfVerificationCancelled(signal: AbortSignal | undefined): void { + if (signal?.aborted === true) + throw this.#cancelledVerification(signal.reason); + } + + async #isUnreadableTarget( + signal: AbortSignal | undefined, + primaryFailure: NativeDmgCommandFailure, + ): Promise { + let handle: FileHandle; + try { + handle = await openRegularFile(this.path, { + symlinks: "follow", + ...(signal === undefined ? {} : { signal }), + }); + } catch (cause: unknown) { + if (cause instanceof RegularFileAdmissionFailure) { + this.#verificationProbeHandle = cause.owner; + const failure = + signal?.aborted === true + ? this.#cancelledVerification(signal.reason) + : primaryFailure; + throw ArtifactReaderFailure.withCleanup( + failure, + ArtifactReaderFailure.cleanupObservation( + cause.cleanupCause, + `DMG verification target descriptor ${this.path}`, + ), + ); + } + if (signal?.aborted === true) + throw this.#cancelledVerification(signal.reason); + if (cause instanceof NonRegularFileReadError) return false; + return classifyCommandFailure(cause, undefined).reason === "io"; + } + + const owner = new OwnedFileHandle(handle); + this.#verificationProbeHandle = owner; + try { + await owner.close(); + this.#verificationProbeHandle = undefined; + } catch (cause: unknown) { + throw ArtifactReaderFailure.withCleanup( + signal?.aborted === true + ? this.#cancelledVerification(signal.reason) + : primaryFailure, + ArtifactReaderFailure.cleanupObservation( + cause, + `DMG verification target descriptor ${this.path}`, + ), + ); + } + if (signal?.aborted === true) + throw this.#cancelledVerification(signal.reason); + return false; + } } const failureMessage = (cause: unknown): string => @@ -644,24 +756,17 @@ const processOutput = (cause: unknown, field: "stdout" | "stderr"): string => { const classifyCommandFailure = ( cause: unknown, operation: string | undefined, -): { - readonly reason: ArtifactReaderFailure["reason"]; - readonly verifyDiagnostic?: string; -} => { +): NativeDmgFailureClassification => { if (typeof cause !== "object" || cause === null) return { reason: "unavailable" }; const code = Reflect.get(cause, "code"); const exitCode = Reflect.get(cause, "exitCode"); if (typeof code === "number" || typeof exitCode === "number") { if (operation !== "verify") return { reason: "unavailable" }; - const diagnostic = verifyFailureDiagnostic( + return classifyVerifyFailure( processOutput(cause, "stdout"), processOutput(cause, "stderr"), ); - return { - reason: verifyFailureReason(diagnostic), - ...(diagnostic === undefined ? {} : { verifyDiagnostic: diagnostic }), - }; } if (code === "ENOENT") { const syscall = Reflect.get(cause, "syscall"); @@ -687,9 +792,26 @@ const classifyCommandFailure = ( return { reason: "unavailable" }; }; +const classifyVerifyFailure = ( + stdout: string, + stderr: string, +): NativeDmgFailureClassification => { + const diagnostic = verifyFailureDiagnostic(stdout, stderr); + const reason = verifyFailureReason(diagnostic); + const untranslated = + reason === undefined + ? untranslatedVerifyFailure(stdout, stderr) + : undefined; + return { + ...untranslated, + reason: reason ?? untranslated?.reason ?? "unavailable", + ...(diagnostic === undefined ? {} : { verifyDiagnostic: diagnostic }), + }; +}; + const verifyFailureReason = ( diagnostic: string | undefined, -): ArtifactReaderFailure["reason"] => { +): ArtifactReaderFailure["reason"] | undefined => { switch (diagnostic) { // hdiutil follows AppleLanguages, so en-GB and en-AU spell it "recognised". case "image not recognized": @@ -702,8 +824,10 @@ const verifyFailureReason = ( case "Permission denied": case "Input/output error": return "io"; - default: + case "Resource temporarily unavailable": return "unavailable"; + default: + return undefined; } }; @@ -712,13 +836,33 @@ const verifyFailureDiagnostic = ( stderr: string, ): string | undefined => { const prefix = "hdiutil: verify failed - "; - return `${stdout}\n${stderr}` - .split(/\r?\n/u) - .map((line) => line.trim()) + return outputLines(stdout, stderr) .find((line) => line.startsWith(prefix)) ?.slice(prefix.length); }; +// hdiutil localizes the trailing failure reason but not its own status lines. +const INVALID_CHECKSUM_LINE = /^hdiutil: verify: checksum of ".*" is INVALID$/u; +const UNRECOGNIZED_IMAGE_LINE = + /^hdiutil: verify: unable to recognize ".*" as a disk image\./u; + +const untranslatedVerifyFailure = ( + stdout: string, + stderr: string, +): NativeDmgFailureClassification | undefined => { + const lines = outputLines(stdout, stderr); + if (lines.some((line) => INVALID_CHECKSUM_LINE.test(line))) + return { reason: "integrity" }; + // Malformed, unreadable and transiently busy targets all produce this line; + // only the localized reason distinguishes them, so the cause stays unknown. + if (lines.some((line) => UNRECOGNIZED_IMAGE_LINE.test(line))) + return { reason: "unavailable", unrecognizedImage: true }; + return undefined; +}; + +const outputLines = (stdout: string, stderr: string): string[] => + `${stdout}\n${stderr}`.split(/\r?\n/u).map((line) => line.trim()); + const command = ( arguments_: readonly string[], effects: ArtifactCommand["effects"], diff --git a/tests/boundary/providers/native/nativeDmgArtifactReader.test.ts b/tests/boundary/providers/native/nativeDmgArtifactReader.test.ts index 72d11abec..8ac69daae 100644 --- a/tests/boundary/providers/native/nativeDmgArtifactReader.test.ts +++ b/tests/boundary/providers/native/nativeDmgArtifactReader.test.ts @@ -21,6 +21,7 @@ import { } from "../../../../src/artifacts/ArtifactReader.js"; import { ArtifactResourceScope } from "../../../../src/artifacts/ArtifactResourceScope.js"; import { DirectoryArtifactReader } from "../../../../src/artifacts/DirectoryArtifactReader.js"; +import { OwnedFileHandle } from "../../../../src/filesystem/OwnedFileHandle.js"; import { NativeDmgArtifactReader, type NativeDmgHost, @@ -1344,6 +1345,194 @@ describe("native DMG verification resource availability", () => { }); }); +// Captured from hdiutil with AppleLanguages set to es-419. +const localizedImageRefusal = (path: string, reason: string) => ({ + stdout: "", + stderr: `hdiutil: verify: unable to recognize ${JSON.stringify(path)} as a disk image. (${reason})\nhdiutil: verify failed - ${reason}\n`, + exitCode: 1, +}); + +describe("native DMG localized verification diagnostics", () => { + it("classifies a localized checksum mismatch from the untranslated status line", async () => { + const run = vi.fn(async () => ({ + stdout: "", + stderr: + 'Calculando suma de disco entero (partición desconocida: 0)…\ndisco entero (partición desconocida:: calculado CRC32 $3816B077, esperado CRC32 $A738EA1C\nhdiutil: verify: checksum of "/tmp/image.dmg" is INVALID\nhdiutil: verify failed - suma no válida\n', + exitCode: 1, + })); + await expect( + createAttachedReader("/tmp/image.dmg", process.env, undefined, { run }), + ).rejects.toMatchObject({ + reason: "integrity", + message: expect.stringContaining("suma no válida"), + }); + expect(run).toHaveBeenCalledTimes(1); + }); + + it.each([ + ["a malformed image", "imagen no reconocida"], + ["a transiently busy image", "Recurso no disponible temporalmente"], + ])( + "keeps a localized refusal of a readable target unknown and retries it like %s could be", + async (_target, reason) => { + const root = await createTestTempDirectory("rea-localized-dmg-readable-"); + const path = join(root, "image.dmg"); + await writeFile(path, "This may or may not be a disk image.\n"); + const run = vi.fn(async () => localizedImageRefusal(path, reason)); + const delay = vi.fn(async () => {}); + const reader = new NativeDmgArtifactReader(path, process.env, { + run, + delay, + }); + await expect(entryPaths(reader)).rejects.toMatchObject({ + reason: "unavailable", + message: expect.stringContaining(reason), + }); + await reader.close(); + expect(run.mock.calls).toHaveLength(3); + expect(delay.mock.calls).toEqual([[250], [500]]); + expect(reader.provenance().map(({ exit_code }) => exit_code)).toEqual([ + 1, 1, 1, + ]); + }, + ); + + it("attaches after a localized transient refusal clears", async () => { + const { host, state } = createOmittedInventoryHost(); + state.inventoryMode = "listed"; + state.detachAttempts = 4; + const root = await createTestTempDirectory("rea-localized-dmg-transient-"); + const path = join(root, "image.dmg"); + await writeFile(path, "A disk image that is briefly busy.\n"); + let attempts = 0; + const reader = await createAttachedReader(path, process.env, undefined, { + ...host, + delay: async () => {}, + async run(arguments_, signal, options) { + if (arguments_[0] === "verify") { + attempts += 1; + if (attempts === 1) + return localizedImageRefusal( + path, + "Recurso no disponible temporalmente", + ); + } + return host.run(arguments_, signal, options); + }, + }); + onTestFinished(() => reader.close()); + expect(attempts).toBe(2); + await reader.close(); + expect(state.attached).toBe(false); + }); + + it("classifies a localized refusal of a missing target as an I/O failure", async () => { + const root = await createTestTempDirectory("rea-localized-dmg-io-"); + const path = join(root, "missing.dmg"); + const run = vi.fn(async () => + localizedImageRefusal(path, "No existe este archivo o directorio"), + ); + const delay = vi.fn(async () => {}); + await expect( + createAttachedReader(path, process.env, undefined, { run, delay }), + ).rejects.toMatchObject({ + reason: "io", + message: expect.stringContaining("No existe este archivo o directorio"), + }); + expect(run).toHaveBeenCalledTimes(1); + expect(delay).not.toHaveBeenCalled(); + }); + + it.skipIf(process.platform === "win32")( + "does not block on a target replaced by a named pipe", + async () => { + const root = await createTestTempDirectory("rea-localized-dmg-fifo-"); + const path = join(root, "image.dmg"); + await execFileOutput("mkfifo", [path]); + const run = vi.fn(async () => + localizedImageRefusal(path, "imagen no reconocida"), + ); + await expect( + createAttachedReader(path, process.env, undefined, { + run, + delay: async () => {}, + }), + ).rejects.toMatchObject({ reason: "unavailable" }); + expect(run).toHaveBeenCalledTimes(3); + }, + 10_000, + ); + + it("keeps a localized failure without an untranslated status line unavailable", async () => { + const run = vi.fn(async () => ({ + stdout: "", + stderr: "hdiutil: verify failed - error desconocido\n", + exitCode: 1, + })); + await expect( + createAttachedReader("/tmp/image.dmg", process.env, undefined, { run }), + ).rejects.toMatchObject({ reason: "unavailable" }); + }); +}); + +describe("native DMG localized verification cleanup", () => { + it("preserves cancellation that arrives before the localized target probe", async () => { + const root = await createTestTempDirectory("rea-localized-dmg-cancel-"); + const path = join(root, "image.dmg"); + await writeFile( + path, + "Readable target; hdiutil refusal remains ambiguous.\n", + ); + const controller = new AbortController(); + const run = vi.fn(async () => { + controller.abort(new Error("cancel localized verification")); + return localizedImageRefusal(path, "Recurso no disponible temporalmente"); + }); + const delay = vi.fn(async () => {}); + await expect( + createAttachedReader(path, process.env, controller.signal, { + run, + delay, + }), + ).rejects.toMatchObject({ reason: "cancelled" }); + expect(run).toHaveBeenCalledTimes(1); + expect(delay).not.toHaveBeenCalled(); + }); + + it("retains a failed verification-probe close for reader cleanup", async () => { + const root = await createTestTempDirectory("rea-localized-dmg-close-"); + const path = join(root, "image.dmg"); + await writeFile( + path, + "Readable target; hdiutil refusal remains ambiguous.\n", + ); + const close = vi + .spyOn(OwnedFileHandle.prototype, "close") + .mockImplementationOnce(() => + Promise.reject(new Error("injected probe descriptor close failure")), + ); + try { + const reader = new NativeDmgArtifactReader(path, process.env, { + run: async () => localizedImageRefusal(path, "imagen no reconocida"), + delay: async () => {}, + }); + await expect(entryPaths(reader)).rejects.toMatchObject({ + reason: "unavailable", + message: expect.stringContaining("imagen no reconocida"), + cleanup: { + resources: expect.arrayContaining([ + `DMG verification target descriptor ${path}`, + ]), + }, + }); + await reader.close(); + expect(close).toHaveBeenCalledTimes(2); + } finally { + close.mockRestore(); + } + }); +}); + describe("native DMG real verification diagnostics", () => { it.skipIf(process.platform !== "darwin")( "classifies a real hdiutil verify refusal as a format failure with captured output", @@ -1356,7 +1545,13 @@ describe("native DMG real verification diagnostics", () => { ); expect(failure).toBeInstanceOf(ArtifactReaderFailure); if (!(failure instanceof ArtifactReaderFailure)) return; - expect(failure.reason).toBe("format"); + // Only English output establishes the cause; a localized reason for a + // readable target stays unknown (see the localized diagnostics suite). + expect(failure.reason).toBe( + /verify failed - image not recogni[sz]ed/u.test(failure.message) + ? "format" + : "unavailable", + ); expect(failure.message).toContain('"command":"/usr/bin/hdiutil"'); expect(failure.message).toContain('"arguments":["verify"'); expect(failure.message).toContain('"exitCode":1'); @@ -1374,10 +1569,9 @@ describe("native DMG real verification diagnostics", () => { await writeFile(unreadablePath, "not a disk image\n"); await chmod(unreadablePath, 0); try { - for (const [path, expectedDiagnostic] of [ - [missingPath, "No such file or directory"], - [unreadablePath, "Permission denied"], - ] as const) { + for (const path of [missingPath, unreadablePath]) { + // hdiutil localizes the errno text, but not its own status line. + const expectedDiagnostic = `hdiutil: verify: unable to recognize ${JSON.stringify(path)} as a disk image.`; const failure = await createAttachedReader(path, process.env).catch( (cause: unknown) => cause, ); @@ -1388,7 +1582,7 @@ describe("native DMG real verification diagnostics", () => { expect(failure.message).toContain('"arguments":["verify"'); expect(failure.message).toContain('"exitCode":1'); expect(failure.message).toContain('"stdout":""'); - expect(failure.message).toContain(expectedDiagnostic); + expect(failure.message).toContain("unable to recognize"); expect(failure.cause).toBeInstanceOf(Error); if (!(failure.cause instanceof Error)) continue; expect(Reflect.get(failure.cause, "code")).toBe(1); @@ -1459,9 +1653,9 @@ describe("native DMG real verification diagnostics", () => { expect(failure.message).toContain('"command":"/usr/bin/hdiutil"'); expect(failure.message).toContain('"arguments":["verify"'); expect(failure.message).toContain('"exitCode":1'); - expect(failure.message).toContain("calculated CRC32"); - expect(failure.message).toContain("expected CRC32"); - expect(failure.message).toContain("verify failed - invalid checksum"); + // hdiutil localizes the CRC labels and failure reason, but not this status line. + expect(failure.message).toContain("CRC32"); + expect(failure.message).toContain("is INVALID"); expect(failure.cause).toBeInstanceOf(Error); if (!(failure.cause instanceof Error)) return; expect(Reflect.get(failure.cause, "code")).toBe(1);