fix(dmg): classify localized verification failures safely

Use untranslated checksum status and nonblocking file admission for localized verification failures. Keep ambiguous readable targets unknown and bounded retries intact; retain failed descriptor ownership and cancellation.

Refs: #4146

Co-authored-by: Tomas Galmarini <tomigalmarini@gmail.com>

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<void>;
 }
 
+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<void> | undefined;
   readonly #activeDirectoryOperations = new Set<Promise<unknown>>();
+  #verificationProbeHandle: OwnedFileHandle | undefined;
 
   constructor(
     private readonly path: string,
@@ -207,6 +223,7 @@ export class NativeDmgArtifactReader implements ArtifactReader {
   async #closeOwnedResources(): Promise<void> {
     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<void> {
+    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<unknown> {
     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<boolean> {
+    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"],

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);