Merge pull request #1070 from morluto/fix/keyed-archive-selection-errors

fix(artifacts): report keyed-archive selection mistakes as invalid input

src/artifacts/ArtifactProvider.ts

@@ -21,7 +21,10 @@ import {
   type ArtifactAnalysisOperation,
 } from "../contracts/artifactToolContracts.js";
 import type { BinaryTarget } from "../domain/binaryTarget.js";
-import { AnalysisCapabilityUnavailableError } from "../domain/analysisErrorCore.js";
+import {
+  AnalysisCapabilityUnavailableError,
+  AnalysisInputError,
+} from "../domain/analysisErrorCore.js";
 import { ArtifactOperationError } from "../domain/artifactOperationError.js";
 import { type AnalysisError } from "../domain/analysisErrorBase.js";
 import type { JsonValue } from "../domain/jsonValue.js";
@@ -116,10 +119,15 @@ class ArtifactClient implements AnalysisClient {
           parameters.path !== "." &&
           parameters.path !== basename(this.target.path)
         )
-          throw new ArtifactReaderFailure(
-            "path",
-            "For an active plist, path must select that archive (omit path or use its basename)",
-          );
+          throw new AnalysisInputError(operation, undefined, [
+            {
+              path: ["path"],
+              reason: "invalid_value",
+              message:
+                "For an active plist, path must select that archive (omit path or use its basename).",
+              expected: basename(this.target.path),
+            },
+          ]);
         const bundlePath = standalone
           ? dirname(this.target.path)
           : this.target.sourcePath;
@@ -352,6 +360,8 @@ const translateFailure = (
       cause.details,
       cause.message,
     );
+  // Caller-selection failures are already typed; keep their correction details.
+  if (cause instanceof AnalysisInputError) return cause;
   return new ArtifactOperationError(operation, "io");
 };
 

src/artifacts/apple/KeyedArchiveReader.test.ts

@@ -125,6 +125,16 @@ describe("inert keyed archive decoding", () => {
         offset: 0,
         limit: 1,
       }),
-    ).toThrow("does not exist");
+    ).toThrow(
+      expect.objectContaining({
+        issues: [
+          expect.objectContaining({
+            path: ["root"],
+            message: expect.stringContaining("does not exist"),
+            expected: ["root", "other"],
+          }),
+        ],
+      }),
+    );
   });
 });

src/artifacts/apple/KeyedArchiveReader.ts

@@ -2,6 +2,7 @@ import { createHash } from "node:crypto";
 import { TextDecoder } from "node:util";
 import { parseBinary } from "plist";
 import { z } from "zod";
+import { AnalysisInputError } from "../../domain/analysisErrorCore.js";
 import type { JsonValue } from "../../domain/jsonValue.js";
 import { projectPlistValue } from "../../domain/apple/plistValue.js";
 import {
@@ -25,6 +26,10 @@ export const decodeKeyedArchiveBytes = (
 ) => {
   if (bytes.length > MAX_BYTES)
     throw new RangeError("Keyed archive exceeds 64 MiB");
+  if (bytes.subarray(0, 10).toString("ascii") === "NIBArchive")
+    throw new TypeError(
+      "Selected file is a compiled NIBArchive, not a Foundation plist archive; decode it with decode_interface_builder",
+    );
   const binary = bytes.subarray(0, 8).toString("ascii") === "bplist00";
   const parsed = binary
     ? { value: parseBinary(bytes), omittedPrototypeKeys: 0 }
@@ -83,18 +88,20 @@ export const inspectBundleKeyedArchive = async (input: {
       .split("/")
       .some((part) => part === ".." || part === "." || part.length === 0)
   )
-    throw new ArtifactReaderFailure(
-      "path",
-      "Archive path must be a canonical relative bundle path",
+    throw archivePathError(
+      "invalid_format",
+      selected.path === "."
+        ? "Select a keyed archive path relative to the active app bundle."
+        : `Archive path must be a canonical relative bundle path without empty, '.', or '..' segments: ${selected.path}`,
     );
   const reader = new DirectoryArtifactReader(input.bundlePath);
   try {
     for await (const entry of reader.entries(input.signal)) {
       if (entry.path !== selected.path) continue;
       if (entry.kind !== "file")
-        throw new ArtifactReaderFailure(
-          "path",
-          "Archive must be a regular file, not a symlink or directory",
+        throw archivePathError(
+          "invalid_value",
+          `Archive path selects a ${entry.kind}, not a regular file: ${selected.path}`,
         );
       if ((entry.declaredSize ?? 0) > MAX_BYTES)
         throw new ArtifactReaderFailure(
@@ -132,18 +139,28 @@ export const inspectBundleKeyedArchive = async (input: {
           ...decodeKeyedArchiveBytes(bytes, selected),
         });
       } catch (cause) {
+        if (cause instanceof AnalysisInputError) throw cause;
         throw new ArtifactReaderFailure(
           cause instanceof RangeError ? "limit" : "format",
-          "Cannot decode selected Foundation keyed archive",
+          `Cannot decode selected Foundation keyed archive: ${cause instanceof Error ? cause.message : String(cause)}`,
           { cause },
         );
       }
     }
-    throw new ArtifactReaderFailure(
-      "path",
-      `Archive not found in bundle: ${selected.path}`,
+    throw archivePathError(
+      "invalid_value",
+      `No regular file exists at ${selected.path} in the active app bundle.`,
     );
   } finally {
     await reader.close();
   }
 };
+
+/** A caller-selected archive path, not the artifact, failed its constraint. */
+const archivePathError = (
+  reason: "invalid_format" | "invalid_value",
+  message: string,
+) =>
+  new AnalysisInputError("inspect_keyed_archive", undefined, [
+    { path: ["path"], reason, message },
+  ]);

src/domain/apple/keyedArchive.ts

@@ -1,4 +1,5 @@
 import { z } from "zod";
+import { AnalysisInputError } from "../analysisErrorCore.js";
 import { jsonValueSchema, type JsonValue } from "../jsonValue.js";
 
 /** Select one archive within the active bundle and optionally one named root. */
@@ -85,7 +86,14 @@ export const projectKeyedArchive = (
   if (top === undefined || Object.keys(top).length === 0)
     throw new TypeError("Keyed archive has no $top roots");
   if (selection.root !== undefined && !Object.hasOwn(top, selection.root))
-    throw new TypeError(`Archive root does not exist: ${selection.root}`);
+    throw new AnalysisInputError("inspect_keyed_archive", undefined, [
+      {
+        path: ["root"],
+        reason: "invalid_value",
+        message: `Archive root does not exist: ${selection.root}. Select one of the archive's $top roots or omit root.`,
+        expected: Object.keys(top),
+      },
+    ]);
   const roots =
     selection.root === undefined
       ? top

tests/boundary/filesystem/keyedArchiveSelection.test.ts

@@ -0,0 +1,99 @@
+import { mkdir, writeFile } from "node:fs/promises";
+import { join } from "node:path";
+
+import { build } from "plist";
+import { expect, it } from "vitest";
+
+import { ArtifactReaderFailure } from "../../../src/artifacts/ArtifactReader.js";
+import { inspectBundleKeyedArchive } from "../../../src/artifacts/apple/KeyedArchiveReader.js";
+import { AnalysisInputError } from "../../../src/domain/analysisErrorCore.js";
+import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
+
+const archive = build({
+  $archiver: "NSKeyedArchiver",
+  $version: 100000,
+  $objects: ["$null", "value"],
+  $top: { root: { CF$UID: 1 }, other: { CF$UID: 1 } },
+});
+
+const bundle = async () => {
+  const root = await createTestTempDirectory("rea-keyed-selection-");
+  await mkdir(join(root, "Contents", "Resources"), { recursive: true });
+  await writeFile(
+    join(root, "Contents", "Resources", "Model.plist"),
+    Buffer.from(archive),
+  );
+  return root;
+};
+
+const inspect = (bundlePath: string, parameters: Record<string, unknown>) =>
+  inspectBundleKeyedArchive({
+    bundlePath,
+    targetSha256: "a".repeat(64),
+    parameters,
+  });
+
+it.each([
+  [".", "invalid_format", "relative to the active app bundle"],
+  ["Contents/../Model.plist", "invalid_format", "canonical relative"],
+  ["Contents//Model.plist", "invalid_format", "canonical relative"],
+  ["Contents/Resources", "invalid_value", "selects a directory"],
+  ["Contents/Resources/Missing.plist", "invalid_value", "No regular file"],
+] as const)(
+  "reports caller-selected archive path %j as invalid input",
+  async (path, reason, message) => {
+    const root = await bundle();
+    const rejected = await inspect(root, { path }).catch(
+      (cause: unknown) => cause,
+    );
+    expect(rejected).toBeInstanceOf(AnalysisInputError);
+    expect(rejected).toMatchObject({
+      issues: [
+        {
+          path: ["path"],
+          reason,
+          message: expect.stringContaining(message),
+        },
+      ],
+    });
+  },
+);
+
+it("reports an absent named root with the archive's available roots", async () => {
+  const root = await bundle();
+  await expect(
+    inspect(root, { path: "Contents/Resources/Model.plist", root: "missing" }),
+  ).rejects.toMatchObject({
+    issues: [
+      {
+        path: ["root"],
+        reason: "invalid_value",
+        message: expect.stringContaining("missing"),
+        expected: ["root", "other"],
+      },
+    ],
+  });
+});
+
+it.each([
+  ["plain plist", Buffer.from(build({ plain: true })), "NSKeyedArchiver"],
+  [
+    "compiled NIBArchive",
+    Buffer.concat([Buffer.from("NIBArchive"), Buffer.alloc(40)]),
+    "decode_interface_builder",
+  ],
+])(
+  "keeps the decoder reason for a selected %s",
+  async (_label, bytes, detail) => {
+    const root = await bundle();
+    await writeFile(join(root, "Contents", "Resources", "Other.nib"), bytes);
+    const rejected = await inspect(root, {
+      path: "Contents/Resources/Other.nib",
+    }).catch((cause: unknown) => cause);
+    expect(rejected).toBeInstanceOf(ArtifactReaderFailure);
+    expect(rejected).toMatchObject({
+      reason: "format",
+      message: expect.stringContaining(detail),
+    });
+  },
+);

tests/boundary/mcp/artifactMcp.inlineInventory.test.ts

@@ -62,6 +62,40 @@ it("inspects a standalone keyed archive through MCP with original object identit
       arguments: { path: "other.plist" },
     });
     expect(rejected.isError).toBe(true);
+    expect(rejected.structuredContent).toMatchObject({
+      error: {
+        code: "invalid_request",
+        details: {
+          issues: [
+            {
+              path: ["path"],
+              reason: "invalid_value",
+              expected: "archive.plist",
+            },
+          ],
+        },
+      },
+    });
+    const missingRoot = await client.callTool({
+      name: "inspect_keyed_archive",
+      arguments: { root: "missing" },
+    });
+    expect(missingRoot.isError).toBe(true);
+    expect(missingRoot.structuredContent).toMatchObject({
+      error: {
+        code: "invalid_request",
+        details: {
+          issues: [
+            {
+              path: ["root"],
+              reason: "invalid_value",
+              message: expect.stringContaining("missing"),
+              expected: Object.keys(graph.roots),
+            },
+          ],
+        },
+      },
+    });
   } finally {
     await client.close();
     await server.close();