diff --git a/src/artifacts/ArtifactProvider.ts b/src/artifacts/ArtifactProvider.ts index 4a3d02fc2..941b75d0f 100644 --- a/src/artifacts/ArtifactProvider.ts +++ b/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"); }; diff --git a/src/artifacts/apple/KeyedArchiveReader.test.ts b/src/artifacts/apple/KeyedArchiveReader.test.ts index 870dc55e7..3910eef5c 100644 --- a/src/artifacts/apple/KeyedArchiveReader.test.ts +++ b/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"], + }), + ], + }), + ); }); }); diff --git a/src/artifacts/apple/KeyedArchiveReader.ts b/src/artifacts/apple/KeyedArchiveReader.ts index e94861997..fafa70fbe 100644 --- a/src/artifacts/apple/KeyedArchiveReader.ts +++ b/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 }, + ]); diff --git a/src/domain/apple/keyedArchive.ts b/src/domain/apple/keyedArchive.ts index 2fc6a792f..0f945b58b 100644 --- a/src/domain/apple/keyedArchive.ts +++ b/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 diff --git a/tests/boundary/filesystem/keyedArchiveSelection.test.ts b/tests/boundary/filesystem/keyedArchiveSelection.test.ts new file mode 100644 index 000000000..28ebed847 --- /dev/null +++ b/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) => + 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), + }); + }, +); diff --git a/tests/boundary/mcp/artifactMcp.inlineInventory.test.ts b/tests/boundary/mcp/artifactMcp.inlineInventory.test.ts index 5afbb74ad..6d14f9df4 100644 --- a/tests/boundary/mcp/artifactMcp.inlineInventory.test.ts +++ b/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();