diff --git a/docs/native-investigation.md b/docs/native-investigation.md index 4dfa9609c4..24403aa012 100644 --- a/docs/native-investigation.md +++ b/docs/native-investigation.md @@ -474,15 +474,16 @@ estimated 64 MiB decode budget before parsing; this is not a hard process-memory bound. JSON decode memory has not been measured and has no equivalent decode estimate. A refused input reports non-retryable `resource_constraint` with `reported_limits.maximum_input_bytes` or `maximum_decoded_bytes` as applicable. -Ordinary direct-source inspection admits 998 source nesting levels, derived from -the existing 1000-level JSON result capacity by reserving one level for typed -scalar fields and one for the result's `value` field. Deeper input reports +Ordinary direct-source decoding has a ceiling of 998 source nesting levels, +derived from the existing 1000-level JSON result capacity by reserving one level +for typed scalar fields and one for the result's `value` field. Deeper input reports `resource_constraint` with `reported_limits.maximum_nesting_depth: 998` and names -the fixed nesting allowance. Compatibility conversions can encounter an earlier -native utility refusal; their diagnostics remain explicit. Archive and Interface -Builder decoding retain their separate 128-level allowance. Worker limits and -host memory do not raise these fixed allowances, and the unchanged request fails -the same way. +the fixed nesting allowance. XML admission also requires native validation, which +can refuse input below this decoding ceiling. Compatibility conversions can +likewise encounter an earlier native utility refusal; both retain native +diagnostics. Archive and Interface Builder decoding retain their separate +128-level allowance. Worker limits and host memory do not raise these fixed +allowances, and the unchanged request fails the same way. A plist explicitly selected through the path input or as the active plist target reports an unreadable or malformed selection as `invalid_request`. A missing or diff --git a/src/contracts/native/nativeToolContracts.ts b/src/contracts/native/nativeToolContracts.ts index ff5d16aa0a..dc64d451dd 100644 --- a/src/contracts/native/nativeToolContracts.ts +++ b/src/contracts/native/nativeToolContracts.ts @@ -115,7 +115,7 @@ export const NATIVE_TOOL_CONTRACTS = [ ), native( "inspect_plist", - "Requires an open binary or bundle-resource session; use open_binary or open_bundle_resources first. Parse its Info.plist by default, or select a local plist with path within that session. Supplying path does not open a session. Returns normalized JSON rather than plutil text. native_command_timeout_ms is each file or plutil command's wall clock (omitted means 60000 ms). The 67108864-byte plist input and decode allowances and the 128-level nesting allowance have no runtime override. The 4096-byte markup probe is a probe window, not a document cap, and has no runtime override.", + "Requires an open binary or bundle-resource session; use open_binary or open_bundle_resources first. Parse its Info.plist by default, or select a local plist with path within that session. Supplying path does not open a session. Returns normalized JSON rather than plutil text. native_command_timeout_ms is each file or plutil command's wall clock (omitted means 60000 ms). The 67108864-byte plist input and decode allowances and the 998-level nesting allowance have no runtime override. XML sources also require native plutil validation, which can refuse shallower inputs with its own diagnostics. The 4096-byte markup probe is a probe window, not a document cap, and has no runtime override.", z.object({ path: localPathStringSchema .optional() diff --git a/src/native/NativeMacOSProvider.ts b/src/native/NativeMacOSProvider.ts index 9e08cbbd79..dabbab2fee 100644 --- a/src/native/NativeMacOSProvider.ts +++ b/src/native/NativeMacOSProvider.ts @@ -1003,7 +1003,7 @@ class NativeMacOSClient implements AnalysisClient { }; const selected = await requireRegularPlist(fileInput); if (!selected.ok) return selected; - const classified = await this.#run( + const classified = await this.#runAdmitted( "inspect_plist", "file", ["-b", plist.value.path], @@ -1096,7 +1096,7 @@ class NativeMacOSClient implements AnalysisClient { if (format === "xml") { parsed = parsePlistXmlSource(source.value); if (parsed.ok) { - const validated = await this.#run( + const validated = await this.#runAdmitted( "inspect_plist", "plutil", ["-lint", "--", fileInput.selection.path], @@ -1160,7 +1160,7 @@ class NativeMacOSClient implements AnalysisClient { ): Promise> { const { path } = fileInput.selection; const { signal, timeoutMs } = fileInput; - const json = await this.#run( + const json = await this.#runAdmitted( "inspect_plist", "plutil", ["-convert", "json", "-o", "-", "--", path], @@ -1180,7 +1180,7 @@ class NativeMacOSClient implements AnalysisClient { AnalysisOutputError | AnalysisResourceConstraintError >; if (jsonAttempt === undefined || jsonAttempt.needsNumberTypes) { - const converted = await this.#run( + const converted = await this.#runAdmitted( "inspect_plist", "plutil", ["-convert", "xml1", "-o", "-", "--", path], @@ -1244,14 +1244,7 @@ class NativeMacOSClient implements AnalysisClient { operation: NativeToolName, tool: string, arguments_: readonly string[], - options: { - readonly signal?: AbortSignal | undefined; - readonly timeoutMs: number; - readonly acceptNonZero?: boolean; - readonly acceptNonZeroIf?: - | ((capture: NativeCommandCapture) => boolean) - | undefined; - }, + options: NativeClientCommandOptions, ): Promise> { if (this.#closed) return err( @@ -1261,6 +1254,16 @@ class NativeMacOSClient implements AnalysisClient { "Native provider client is closed", ), ); + return this.#runAdmitted(operation, tool, arguments_, options); + } + + /** Child commands borrow the admitted file workflow's lifetime through close. */ + async #runAdmitted( + operation: NativeToolName, + tool: string, + arguments_: readonly string[], + options: NativeClientCommandOptions, + ): Promise> { const command = this.runner.run(tool, arguments_, { ...(options.signal === undefined ? {} : { signal: options.signal }), timeoutMs: options.timeoutMs, @@ -1375,6 +1378,15 @@ const nativeOutputFailure = ( ): AnalysisError => new AnalysisOutputError(operation, "Native output parsing failed", { cause }); +interface NativeClientCommandOptions { + readonly signal?: AbortSignal | undefined; + readonly timeoutMs: number; + readonly acceptNonZero?: boolean; + readonly acceptNonZeroIf?: + | ((capture: NativeCommandCapture) => boolean) + | undefined; +} + interface NativeObservation { readonly result: JsonValue; readonly limitations: readonly string[]; diff --git a/tests/acceptance/applications/nativePlistNesting.test.ts b/tests/acceptance/applications/nativePlistNesting.test.ts index 6515819ff6..a7f4488fcc 100644 --- a/tests/acceptance/applications/nativePlistNesting.test.ts +++ b/tests/acceptance/applications/nativePlistNesting.test.ts @@ -1,3 +1,4 @@ +import { execFile } from "node:child_process"; import { readFile, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { buildBinary, type PlistValue } from "plist"; @@ -11,6 +12,25 @@ import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { cliTest } from "../../support/cli/cliFixture.js"; +const lintXmlPlist = (path: string) => + new Promise<{ exitCode: number; stdout: string; stderr: string }>( + (resolve, reject) => { + execFile( + "/usr/bin/plutil", + ["-lint", "--", path], + { timeout: 10_000 }, + (error, stdout, stderr) => { + const exitCode = error === null ? 0 : error.code; + if (typeof exitCode !== "number") { + reject(error); + return; + } + resolve({ exitCode, stdout, stderr }); + }, + ); + }, + ); + describe.skipIf(process.platform !== "darwin")( "ordinary plist source nesting", () => { @@ -34,6 +54,7 @@ describe.skipIf(process.platform !== "darwin")( { format: "binary", depth: 129, data: true }, { format: "xml", depth: 129, data: true }, { format: "binary", depth: 200, data: false }, + { format: "xml", depth: 200, data: false }, { format: "json", depth: 200, data: false }, { format: "binary", depth: 998, data: true }, { format: "binary", depth: 999, data: true }, @@ -53,9 +74,16 @@ describe.skipIf(process.platform !== "darwin")( : Buffer.from( format === "json" ? JSON.stringify(value) - : `${"level".repeat(depth)}AAE=${"".repeat(depth)}`, + : `${"level".repeat(depth)}${data ? "AAE=" : "1"}${"".repeat(depth)}`, ); await writeFile(path, bytes); + // The decoder ceiling does not override native XML admission. + // Compare its upper-edge admission with the actual utility, while + // requiring the historical depth-129/200 controls to succeed below. + const native = + format === "xml" && depth === 998 + ? await lintXmlPlist(path) + : undefined; const output = await cli.run({ arguments: ["inspect-plist", path, "--json"], environment, @@ -83,6 +111,26 @@ describe.skipIf(process.platform !== "darwin")( ); continue; } + if (native !== undefined && native.exitCode !== 0) { + expect(output.exitCode, output.stderr).toBe(1); + const expectedFailure = { + code: "invalid_request", + retryable: false, + details: { + operation: "inspect_plist", + captured_output: { + exit_code: native.exitCode, + stdout: native.stdout, + stderr: native.stderr, + }, + }, + }; + expect(output.json).toMatchObject(expectedFailure); + expect(parseMcpToolError(response).error).toMatchObject( + expectedFailure, + ); + continue; + } expect( output.exitCode, JSON.stringify({ @@ -105,6 +153,7 @@ describe.skipIf(process.platform !== "darwin")( expect(cliResult.format).toBe(format === "json" ? "unknown" : format); expect(cliResult.provenance.map((item) => item.tool)).toEqual([ "file", + ...(format === "xml" ? ["plutil"] : []), ]); let leaf: unknown = cliResult.value; for (let level = 0; level < depth; level += 1) { diff --git a/tests/boundary/providers/native/nativePlistFileCleanup.test.ts b/tests/boundary/providers/native/nativePlistFileCleanup.test.ts index c3037b25cd..57021b5d04 100644 --- a/tests/boundary/providers/native/nativePlistFileCleanup.test.ts +++ b/tests/boundary/providers/native/nativePlistFileCleanup.test.ts @@ -332,41 +332,70 @@ it("preserves an opaque cancellation observed during successful prefix close", a expect((await client.close()).ok).toBe(true); }); -it("joins an admitted prefix read when client close starts concurrently", async () => { - const { client, path, prototype } = await fixture(); - let enter!: () => void; - let resume!: () => void; - const entered = new Promise((resolve) => { - enter = resolve; - }); - const gate = new Promise((resolve) => { - resume = resolve; - }); - const original = prototype.read; - const read = vi - .spyOn(prototype, "read") - .mockImplementationOnce(async function ( - this: FileHandle, - ...args: Parameters - ) { - enter(); - await gate; - return Reflect.apply(original, this, args); +it.for(["source", "json", "xml"] as const)( + "joins an admitted prefix read and its %s commands when client close starts concurrently", + async (route) => { + const { client, path, prototype, runner } = await fixture(); + if (route !== "source") await writeFile(path, "{ name = value; }"); + const run = runner.run.bind(runner); + const commands = vi + .spyOn(runner, "run") + .mockImplementation(async (tool, args, options) => { + const result = await run(tool, args, options); + if (route !== "xml" || tool !== "plutil" || !result.ok) return result; + const stdout = + args[1] === "xml1" ? "value" : ""; + return ok({ + ...result.value, + stdout, + stdoutBytes: Buffer.byteLength(stdout), + exitCode: args[1] === "json" ? 1 : 0, + }); + }); + onTestFinished(() => commands.mockRestore()); + let enter!: () => void; + let resume!: () => void; + const entered = new Promise((resolve) => { + enter = resolve; }); - onTestFinished(() => { + const gate = new Promise((resolve) => { + resume = resolve; + }); + const original = prototype.read; + const read = vi + .spyOn(prototype, "read") + .mockImplementationOnce(async function ( + this: FileHandle, + ...args: Parameters + ) { + enter(); + await gate; + return Reflect.apply(original, this, args); + }); + onTestFinished(() => { + resume(); + read.mockRestore(); + }); + const inspecting = client.execute("inspect_plist", { path }); + await entered; + let finished = false; + const closing = client.close().then((result) => { + finished = true; + return result; + }); + await new Promise((resolve) => setImmediate(resolve)); + expect(finished).toBe(false); resume(); - read.mockRestore(); - }); - const inspecting = client.execute("inspect_plist", { path }); - await entered; - let finished = false; - const closing = client.close().then((result) => { - finished = true; - return result; - }); - await new Promise((resolve) => setImmediate(resolve)); - expect(finished).toBe(false); - resume(); - expect((await inspecting).ok).toBe(true); - expect((await closing).ok).toBe(true); -}); + const inspected = await inspecting; + expect( + inspected.ok, + inspected.ok ? undefined : inspected.error.message, + ).toBe(true); + expect((await closing).ok).toBe(true); + expect(commands.mock.calls.map(([tool, args]) => [tool, args[0]])).toEqual([ + ["file", "-b"], + ["plutil", route === "source" ? "-lint" : "-convert"], + ...(route === "xml" ? [["plutil", "-convert"]] : []), + ]); + }, +); diff --git a/tests/boundary/providers/native/nativePlistTextSource.test.ts b/tests/boundary/providers/native/nativePlistTextSource.test.ts index ae1845d627..bab6753811 100644 --- a/tests/boundary/providers/native/nativePlistTextSource.test.ts +++ b/tests/boundary/providers/native/nativePlistTextSource.test.ts @@ -148,6 +148,7 @@ it.for([ const replay = new NativeFixtureRunner({ file: fixture.description }); const tools: string[] = []; let conversionIndex = 0; + let validations = 0; const client = new NativeMacOSProvider( {}, { @@ -155,6 +156,11 @@ it.for([ tools.push(tool); const result = await replay.run(tool, args); if (tool !== "plutil" || !result.ok) return result; + if (args[0] === "-lint") { + expect(args).toEqual(["-lint", "--", path]); + validations += 1; + return result; + } const conversion = fixture.conversions[conversionIndex++]; if (conversion === undefined) throw new Error("Unexpected conversion of directly supported source"); @@ -181,7 +187,14 @@ it.for([ source_path: path, }); expect(result.provenance.map((item) => item.tool)).toEqual(tools); - expect(tools).toEqual(["file", ...fixture.conversions.map(() => "plutil")]); + const directXml = + fixture.format === "xml" && fixture.conversions.length === 0; + expect(validations).toBe(directXml ? 1 : 0); + expect(tools).toEqual([ + "file", + ...(directXml ? ["plutil"] : []), + ...fixture.conversions.map(() => "plutil"), + ]); expect(conversionIndex).toBe(fixture.conversions.length); if (fixture.limitations !== undefined) expect(result.limitations).toEqual(fixture.limitations);