Merge pull request #1226 from morluto/fix/keyed-archive-kind-refusal

fix(artifacts): report non-archive keyed-archive selections as unsupported targets

src/artifacts/apple/KeyedArchiveReader.ts

@@ -1,7 +1,11 @@
 import { createHash } from "node:crypto";
+import { join } from "node:path";
 import { parseBinary } from "plist";
 import { z } from "zod";
-import { AnalysisInputError } from "../../domain/analysisErrorCore.js";
+import {
+  AnalysisInputError,
+  AnalysisUnsupportedTargetError,
+} from "../../domain/analysisErrorCore.js";
 import type { JsonValue } from "../../domain/jsonValue.js";
 import { projectPlistValue } from "../../domain/apple/plistValue.js";
 import { createPlistNumberProjection } from "../../domain/apple/plistNumbers.js";
@@ -14,6 +18,7 @@ import {
   parseXmlPropertyList,
 } from "../../domain/propertyListKeys.js";
 import {
+  KeyedArchiveKindError,
   keyedArchiveInputSchema,
   keyedArchiveResultSchema,
   projectKeyedArchive,
@@ -33,8 +38,9 @@ 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(
+    throw new KeyedArchiveKindError(
       "Selected file is a compiled NIBArchive, not a Foundation plist archive; decode it with decode_interface_builder",
+      "Decode compiled NIBArchive files with decode_interface_builder on the app bundle.",
     );
   const binary = bytes.subarray(0, 8).toString("ascii") === "bplist00";
   const xmlText = binary ? undefined : decodeXmlPlistText(bytes);
@@ -147,6 +153,14 @@ export const inspectBundleKeyedArchive = async (input: {
         });
       } catch (cause) {
         if (cause instanceof AnalysisInputError) throw cause;
+        // An intact file of another kind is not a damaged archive.
+        if (cause instanceof KeyedArchiveKindError)
+          throw new AnalysisUnsupportedTargetError(
+            "inspect_keyed_archive",
+            join(input.bundlePath, entry.path),
+            cause.message,
+            { cause, remediationAction: cause.remediationAction },
+          );
         throw new ArtifactReaderFailure(
           cause instanceof RangeError ? "limit" : "format",
           `Cannot decode selected Foundation keyed archive: ${cause instanceof Error ? cause.message : String(cause)}`,

src/domain/apple/keyedArchive.ts

@@ -2,6 +2,22 @@ import { z } from "zod";
 import { AnalysisInputError } from "../analysisErrorCore.js";
 import { jsonValueSchema, type JsonValue } from "../jsonValue.js";
 
+/**
+ * The selected file is intact but is another kind of file, so another tool
+ * applies. A malformed NSKeyedArchiver archive is not this error.
+ */
+export class KeyedArchiveKindError extends TypeError {
+  override readonly name = "KeyedArchiveKindError";
+
+  constructor(
+    message: string,
+    /** The workflow that reads this kind of file. */
+    readonly remediationAction: string,
+  ) {
+    super(message);
+  }
+}
+
 /** Select one archive within the active bundle and optionally one named root. */
 export const keyedArchiveInputSchema = z.strictObject({
   path: z.string().min(1).default("."),
@@ -68,16 +84,28 @@ const uid = (value: JsonValue | undefined): number | undefined => {
   return typeof marker === "number" ? marker : undefined;
 };
 
+const archiverMismatch = (
+  archive: Record<string, JsonValue> | undefined,
+): string => {
+  if (archive === undefined) return "its root is not a dictionary";
+  if (!Object.hasOwn(archive, "$archiver")) return "it has no $archiver key";
+  return typeof archive.$archiver === "string"
+    ? `its $archiver is ${JSON.stringify(archive.$archiver)}`
+    : "its $archiver is not a string";
+};
+
 /** Interpret serialized Foundation references without instantiating archive classes. */
 export const projectKeyedArchive = (
   value: unknown,
   selection: { root?: string | undefined; offset: number; limit: number },
 ) => {
   const archive = record(jsonValueSchema.parse(value));
-  if (
-    archive?.$archiver !== "NSKeyedArchiver" ||
-    !Array.isArray(archive.$objects)
-  )
+  if (archive?.$archiver !== "NSKeyedArchiver")
+    throw new KeyedArchiveKindError(
+      `Selected property list is not an NSKeyedArchiver archive: ${archiverMismatch(archive)}`,
+      "Inspect an ordinary property list with inspect_plist; inspect_keyed_archive decodes NSKeyedArchiver archives only.",
+    );
+  if (!Array.isArray(archive.$objects))
     throw new TypeError("Expected NSKeyedArchiver with an $objects array");
   const table = archive.$objects;
   if (table.length > 200_000)

tests/boundary/filesystem/keyedArchiveSelection.test.ts

@@ -6,7 +6,12 @@ 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 { AnalysisError } from "../../../src/domain/analysisErrorBase.js";
+import {
+  AnalysisInputError,
+  AnalysisUnsupportedTargetError,
+} from "../../../src/domain/analysisErrorCore.js";
+import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js";
 import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
 
 const archive = build({
@@ -76,24 +81,62 @@ it("reports an absent named root with the archive's available roots", async () =
 });
 
 it.each([
-  ["plain plist", Buffer.from(build({ plain: true })), "NSKeyedArchiver"],
+  [
+    "plain plist",
+    Buffer.from(build({ plain: true })),
+    "it has no $archiver key",
+    "inspect_plist",
+  ],
+  [
+    "plist with another archiver",
+    Buffer.from(build({ $archiver: "NSArchiver", $objects: [] })),
+    'its $archiver is "NSArchiver"',
+    "inspect_plist",
+  ],
   [
     "compiled NIBArchive",
     Buffer.concat([Buffer.from("NIBArchive"), Buffer.alloc(40)]),
+    "compiled NIBArchive",
     "decode_interface_builder",
   ],
 ])(
-  "keeps the decoder reason for a selected %s",
-  async (_label, bytes, detail) => {
+  "reports a selected %s as an unsupported target with the workflow that reads it",
+  async (_label, bytes, reason, workflow) => {
     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),
+    expect(rejected).toBeInstanceOf(AnalysisUnsupportedTargetError);
+    expect(projectAnalysisError(asAnalysisError(rejected))).toMatchObject({
+      code: "unsupported_target",
+      message: expect.stringContaining(reason),
+      remediation: { action: expect.stringContaining(workflow) },
+      details: {
+        operation: "inspect_keyed_archive",
+        path: join(root, "Contents", "Resources", "Other.nib"),
+      },
     });
   },
 );
+
+it("keeps a damaged NSKeyedArchiver archive a format failure", async () => {
+  const root = await bundle();
+  await writeFile(
+    join(root, "Contents", "Resources", "Damaged.plist"),
+    Buffer.from(build({ $archiver: "NSKeyedArchiver", $top: {} })),
+  );
+  const rejected = await inspect(root, {
+    path: "Contents/Resources/Damaged.plist",
+  }).catch((cause: unknown) => cause);
+  expect(rejected).toBeInstanceOf(ArtifactReaderFailure);
+  expect(rejected).toMatchObject({
+    reason: "format",
+    message: expect.stringContaining("$objects array"),
+  });
+});
+
+const asAnalysisError = (value: unknown): AnalysisError => {
+  if (!(value instanceof AnalysisError)) throw value;
+  return value;
+};