Merge pull request #921 from morluto/fix/dmg-apfs-detach

fix(artifacts): accept APFS image disks ejected with their container

src/artifacts/NativeDmgArtifactReader.ts

@@ -23,6 +23,13 @@ const attachOutputSchema = z.object({
     }),
   ),
 });
+const infoOutputSchema = z.object({
+  images: z.array(
+    z.object({
+      "system-entities": z.array(z.object({ "dev-entry": z.string() })),
+    }),
+  ),
+});
 
 /** Narrow host seam for tested, shell-free hdiutil lifecycle operations. */
 export interface NativeDmgHost {
@@ -113,7 +120,10 @@ export class NativeDmgArtifactReader implements ArtifactReader {
         });
         this.#provenance.push(command(["detach", device], ["mount"]));
       } catch (cause: unknown) {
-        detachFailure ??= cause;
+        // Detaching a synthesized APFS container also ejects the image that
+        // backs it, so a later whole disk of the same image may already be
+        // gone. Only a device that is still attached is a cleanup failure.
+        if (await this.#isAttached(device)) detachFailure ??= cause;
       }
     }
     this.#devices = [];
@@ -131,6 +141,27 @@ export class NativeDmgArtifactReader implements ArtifactReader {
       );
   }
 
+  /**
+   * Report whether hdiutil still lists a device; unknown state counts as
+   * attached. Like detach, this cleanup query runs after the inventory
+   * snapshot has captured provenance, so it is not recorded there.
+   */
+  async #isAttached(device: string): Promise<boolean> {
+    try {
+      const info = await runChecked(this.host, ["info", "-plist"], undefined, {
+        timeoutMs: DETACH_TIMEOUT_MS,
+      });
+      const parsed = infoOutputSchema.parse(parse(info.stdout));
+      return parsed.images.some((image) =>
+        image["system-entities"].some(
+          (entity) => entity["dev-entry"] === device,
+        ),
+      );
+    } catch {
+      return true;
+    }
+  }
+
   async attach(signal?: AbortSignal): Promise<void> {
     await runChecked(this.host, ["verify", this.path], signal);
     this.#provenance.push(command(["verify", this.path], ["read"]));

tests/boundary/providers/native/nativeDmgArtifactReader.test.ts

@@ -61,6 +61,15 @@ describe("native DMG artifact reader", () => {
       run(arguments_) {
         if (arguments_[0] === "detach")
           return Promise.reject(new Error("detach failed"));
+        if (arguments_[0] === "info")
+          return Promise.resolve({
+            stdout: build({
+              images: [
+                { "system-entities": [{ "dev-entry": "/dev/disk-fixture" }] },
+              ],
+            }),
+            exitCode: 0,
+          });
         if (arguments_[0] !== "attach")
           return Promise.resolve({ stdout: "", exitCode: 0 });
         return Promise.resolve({
@@ -125,3 +134,85 @@ it("owns the canonical mount root and detaches each observed whole image once",
     ["detach", "/dev/disk41"],
   ]);
 });
+
+describe("APFS disk image detach", () => {
+  // hdiutil reports the image disk and its synthesized APFS container as two
+  // whole disks; detaching the container ejects the image disk as well.
+  const apfsHost = (stillListed: readonly string[]) => {
+    const calls: string[][] = [];
+    const host: NativeDmgHost = {
+      async run(arguments_) {
+        calls.push([...arguments_]);
+        if (arguments_[0] === "detach" && arguments_[1] === "/dev/disk4")
+          throw new Error("hdiutil: detach failed - No such file or directory");
+        if (arguments_[0] === "info")
+          return {
+            stdout: build({
+              images:
+                stillListed.length === 0
+                  ? []
+                  : [
+                      {
+                        "system-entities": stillListed.map((device) => ({
+                          "dev-entry": device,
+                        })),
+                      },
+                    ],
+            }),
+            exitCode: 0,
+          };
+        if (arguments_[0] !== "attach") return { stdout: "", exitCode: 0 };
+        const mountRoot = arguments_[arguments_.indexOf("-mountroot") + 1];
+        if (mountRoot === undefined) throw new Error("missing mount root");
+        const mountPoint = join(mountRoot, "Fixture");
+        await mkdir(mountPoint);
+        return {
+          stdout: build({
+            "system-entities": [
+              { "dev-entry": "/dev/disk4" },
+              { "dev-entry": "/dev/disk4s1" },
+              { "dev-entry": "/dev/disk5s1", "mount-point": mountPoint },
+              { "dev-entry": "/dev/disk5" },
+            ],
+          }),
+          exitCode: 0,
+        };
+      },
+    };
+    return { calls, host };
+  };
+
+  it("accepts an image disk that its container detach already ejected", async () => {
+    const { calls, host } = apfsHost([]);
+    const reader = await NativeDmgArtifactReader.create(
+      "/tmp/image.dmg",
+      undefined,
+      host,
+    );
+    await reader.close();
+    expect(
+      calls.filter((call) => call[0] !== "verify" && call[0] !== "attach"),
+    ).toEqual([
+      ["detach", "/dev/disk5"],
+      ["detach", "/dev/disk4"],
+      ["info", "-plist"],
+    ]);
+    expect(reader.provenance().map(({ arguments: args }) => args[0])).toEqual([
+      "verify",
+      "attach",
+      "detach",
+    ]);
+  });
+
+  it("still fails when the device that rejected detach remains attached", async () => {
+    const { host } = apfsHost(["/dev/disk4", "/dev/disk4s1"]);
+    const reader = await NativeDmgArtifactReader.create(
+      "/tmp/image.dmg",
+      undefined,
+      host,
+    );
+    await expect(reader.close()).rejects.toThrow(
+      "DMG detach or mount-root cleanup failed",
+    );
+  });
+});