diff --git a/docs/native-investigation.md b/docs/native-investigation.md index aa138587e8..97fe97c0d3 100644 --- a/docs/native-investigation.md +++ b/docs/native-investigation.md @@ -268,6 +268,10 @@ markup, including UTF-8 and BOM-marked UTF-16 sources. Unidentified text and OpenStep plists, including a data-only value such as ``, report `unknown` even when plutil decodes their values through an XML conversion. +A plist decode refusal includes plutil's diagnostic from either output stream. +`details.captured_output` retains bounded stdout/stderr excerpts, observed byte +totals and exit status. `truncated: true` means those excerpts are incomplete. + `inspect_macho.selected_architecture` identifies the exact selected lipo slice and its `container_file_offset` in the input file. A thin Mach-O has a known offset of zero; an unknown universal-slice offset stays null. The Evidence diff --git a/src/native/NativeMacOSProvider.ts b/src/native/NativeMacOSProvider.ts index 1d89473118..deb595feff 100644 --- a/src/native/NativeMacOSProvider.ts +++ b/src/native/NativeMacOSProvider.ts @@ -829,14 +829,27 @@ class NativeMacOSClient implements AnalysisClient { { signal, acceptNonZero: !jsonDecoded }, ); if (xml !== undefined && !xml.ok) return xml; - if (xml !== undefined && xml.value.exitCode !== 0) + if (xml !== undefined && xml.value.exitCode !== 0) { + const capture = failureCapture(xml.value, false); return err( unreadablePlist( plist.value, requested, - `plutil could not decode it as a property list (${plutilDiagnostic(xml.value.stderr, plist.value)})`, + `plutil could not decode it as a property list (${plutilDiagnostic(capture, plist.value)})`, + { + capturedOutput: { + stdout: capture.stdout, + stderr: capture.stderr, + truncated: capture.truncated, + stdout_bytes: capture.stdoutBytes, + stderr_bytes: capture.stderrBytes, + exit_code: capture.exitCode, + signal: capture.signal, + }, + }, ), ); + } const parsed = xml === undefined ? parsePlistJson(json.value.stdout) @@ -1244,10 +1257,20 @@ const parseEntitlements = ( }; }; -/** plutil prefixes its diagnostic with the path the message already names. */ -const plutilDiagnostic = (stderr: string, path: string): string => { - const text = stderr.trim(); - return text.startsWith(`${path}: `) ? text.slice(path.length + 2) : text; +/** Failed plutil conversions can report on either stream; preserve both. */ +const plutilDiagnostic = ( + capture: Pick, + path: string, +): string => { + const diagnostics = [capture.stderr, capture.stdout] + .map((stream) => { + const text = stream.trim(); + return text.startsWith(`${path}: `) ? text.slice(path.length + 2) : text; + }) + .filter((text) => text.length > 0); + return diagnostics.length === 0 + ? "plutil printed no diagnostic" + : diagnostics.join("\n"); }; /** The bundle whose Contents/MacOS directory directly holds `path`. */ diff --git a/tests/boundary/cli/nativePlistSelectionFailureCli.test.ts b/tests/boundary/cli/nativePlistSelectionFailureCli.test.ts index 640e12b635..ad32e71d17 100644 --- a/tests/boundary/cli/nativePlistSelectionFailureCli.test.ts +++ b/tests/boundary/cli/nativePlistSelectionFailureCli.test.ts @@ -1,7 +1,8 @@ -import { chmod, mkdir, writeFile } from "node:fs/promises"; +import { chmod, mkdir, readFile, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { describe, expect } from "vitest"; +import { z } from "zod"; import { thinMach } from "../../../src/domain/binaryTarget.fixture.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; @@ -12,6 +13,17 @@ const environment = { REA_ANALYSIS_PROVIDER: "auto", } as const; +const capturedDiagnostic = (value: unknown): string => { + const { stdout, stderr } = z + .object({ + details: z.object({ + captured_output: z.object({ stdout: z.string(), stderr: z.string() }), + }), + }) + .parse(value).details.captured_output; + return `${stdout}\n${stderr}`; +}; + /** An app bundle whose Contents holds a directory and a non-plist file. */ const fixtureApp = async () => { const app = join(await createTestTempDirectory("rea-plist-fail-"), "A.app"); @@ -130,3 +142,69 @@ describe.skipIf(process.platform !== "darwin")( ); }, ); + +describe.skipIf(process.platform !== "darwin")( + "native plist CLI decode diagnostics", + () => { + cliTest.for([ + "not a property list", + "value 42 ", + "valuex", + ])( + "preserves native decode diagnostics for %s", + async (source, { cli }) => { + const app = await fixtureApp(); + const malformed = join(app, "Contents/Broken.plist"); + await writeFile(malformed, source); + const selected = await cli.run({ + arguments: [ + "inspect-plist", + app, + "--relative-path", + "Contents/Broken.plist", + "--json", + ], + environment, + }); + expect(selected.exitCode).toBe(1); + expect(selected.json).toMatchObject({ + code: "invalid_request", + details: { + issues: [ + { message: expect.stringContaining("Property List error:") }, + ], + captured_output: { exit_code: 1 }, + }, + }); + expect(capturedDiagnostic(selected.json)).toContain( + "Property List error:", + ); + const directory = await createTestTempDirectory( + "rea-plist-default-diagnostic-", + ); + const program = join(directory, "program"); + await writeFile(program, thinMach(0xfeedfacf, 0x100000c)); + await mkdir(join(directory, "Contents")); + const defaultPlist = join(directory, "Contents/Info.plist"); + await writeFile(defaultPlist, source); + const fallback = await cli.run({ + arguments: ["inspect-plist", program, "--json"], + environment, + }); + expect(fallback.exitCode).toBe(1); + expect(fallback.json).toMatchObject({ + code: "capability_unavailable", + details: { + reason: expect.stringContaining("Property List error:"), + captured_output: { exit_code: 1 }, + }, + }); + expect(capturedDiagnostic(fallback.json)).toContain( + "Property List error:", + ); + expect(await readFile(malformed, "utf8")).toBe(source); + expect(await readFile(defaultPlist, "utf8")).toBe(source); + }, + ); + }, +); diff --git a/tests/boundary/mcp/nativePlistDiagnosticsMcp.test.ts b/tests/boundary/mcp/nativePlistDiagnosticsMcp.test.ts new file mode 100644 index 0000000000..a8c0bab4b9 --- /dev/null +++ b/tests/boundary/mcp/nativePlistDiagnosticsMcp.test.ts @@ -0,0 +1,54 @@ +import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it } from "vitest"; +import { thinMach } from "../../../src/domain/binaryTarget.fixture.js"; +import { connectLocalToolsMcp } from "../../fixtures/localToolsMcp.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +it.skipIf(process.platform !== "darwin")( + "preserves real plutil diagnostics and typed selection failures through MCP", + async () => { + const root = await createTestTempDirectory("rea-plist-diagnostic-mcp-"); + const program = join(root, "program"); + await writeFile(program, thinMach(0xfeedfacf, 0x100000c)); + await mkdir(join(root, "Contents")); + const path = join(root, "Contents/Info.plist"); + const source = + "number 42 "; + await writeFile(path, source); + const { call } = await connectLocalToolsMcp({ + HOME: root, + USERPROFILE: root, + }); + const opened = await call("open_binary", { path: program }); + expect(opened.isError).not.toBe(true); + for (const selected of [undefined, path]) { + const result = await call( + "inspect_plist", + selected === undefined ? {} : { path: selected }, + ); + const { error } = parseMcpToolError(result); + if ( + error.code !== "invalid_request" && + error.code !== "capability_unavailable" + ) + throw new Error(`Unexpected failure: ${error.code}`); + expect(error.code).toBe( + selected === undefined ? "capability_unavailable" : "invalid_request", + ); + const text = + selected === undefined + ? error.message + : JSON.stringify(error.details?.issues); + expect(text).toContain("Property List error:"); + expect(error.details).toMatchObject({ + captured_output: { exit_code: 1, truncated: false }, + }); + expect(JSON.stringify(error.details?.captured_output)).toContain( + "Property List error:", + ); + } + expect(await readFile(path, "utf8")).toBe(source); + }, +); diff --git a/tests/boundary/providers/native/nativePlistDiagnostics.test.ts b/tests/boundary/providers/native/nativePlistDiagnostics.test.ts new file mode 100644 index 0000000000..9d4d327d1f --- /dev/null +++ b/tests/boundary/providers/native/nativePlistDiagnostics.test.ts @@ -0,0 +1,133 @@ +import { mkdir, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it, onTestFinished } from "vitest"; +import { NativeMacOSProvider } from "../../../../src/native/NativeMacOSProvider.js"; +import { + NATIVE_FAILURE_DIAGNOSTIC_BYTES, + type NativeCommandRunner, +} from "../../../../src/native/CommandRunner.js"; +import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js"; +import { ok } from "../../../../src/domain/result.js"; +import { thinMach } from "../../../../src/domain/binaryTarget.fixture.js"; +import { + NativeFixtureRunner, + nativeMachoTarget, +} from "../../../fixtures/nativeCommands.js"; +import { createTestTempDirectory } from "../../../fixtures/temporaryDirectory.js"; + +type Streams = { readonly stdout: string; readonly stderr: string }; + +const fixture = async (streams: (path: string) => Streams) => { + const root = await createTestTempDirectory("rea-plist-diagnostic-"); + const target = join(root, "program"); + await writeFile(target, thinMach(0xfeedfacf, 0x100000c)); + await mkdir(join(root, "Contents")); + const path = join(root, "Contents/Info.plist"); + await writeFile(path, "not a plist"); + const output = streams(path); + const replay = new NativeFixtureRunner(); + const runner: NativeCommandRunner = { + async run(tool, args) { + const capture = await replay.run(tool, args); + if (tool !== "plutil" || !capture.ok) return capture; + return ok({ + ...capture.value, + ...output, + exitCode: 1, + stdoutBytes: Buffer.byteLength(output.stdout), + stderrBytes: Buffer.byteLength(output.stderr), + }); + }, + }; + const provider = new NativeMacOSProvider({}, runner, "darwin"); + const client = provider.createClient(nativeMachoTarget(target)); + onTestFinished(async () => { + await client.close(); + }); + return { client, path, output }; +}; + +it.each(["stdout", "stderr", "both", "whitespace", "empty"] as const)( + "preserves selected/default plist failure diagnostics from %s", + async (source) => { + const { client, path, output } = await fixture((path) => ({ + stdout: + source === "stdout" || source === "both" + ? `${path}: invalid integer on line 3\n` + : "", + stderr: + source === "stderr" || source === "both" + ? `${path}: additional parse diagnostic\n` + : source === "whitespace" + ? " \n\t" + : "", + })); + for (const requested of [undefined, path]) { + const result = await client.execute( + "inspect_plist", + requested === undefined ? {} : { path: requested }, + ); + if (result.ok) throw new Error("Expected decode refusal"); + const projected = projectAnalysisError(result.error); + const text = JSON.stringify( + requested === undefined + ? projected.details?.reason + : projected.details?.issues, + ); + if (source === "stdout" || source === "both") + expect(text).toContain("invalid integer on line 3"); + if (source === "stderr" || source === "both") + expect(text).toContain("additional parse diagnostic"); + if (source === "whitespace" || source === "empty") + expect(text).toContain("plutil printed no diagnostic"); + expect(projected).toMatchObject({ + code: + requested === undefined + ? "capability_unavailable" + : "invalid_request", + details: { + captured_output: { + ...output, + truncated: false, + stdout_bytes: Buffer.byteLength(output.stdout), + stderr_bytes: Buffer.byteLength(output.stderr), + exit_code: 1, + signal: null, + }, + }, + }); + } + expect((await client.close()).ok).toBe(true); + }, +); + +it("bounds failure excerpts and preserves observed byte counts and truncation", async () => { + const { client, path, output } = await fixture(() => ({ + stdout: "é".repeat(NATIVE_FAILURE_DIAGNOSTIC_BYTES), + stderr: "z".repeat(NATIVE_FAILURE_DIAGNOSTIC_BYTES + 1), + })); + const result = await client.execute("inspect_plist", { path }); + if (result.ok) throw new Error("Expected decode refusal"); + expect(projectAnalysisError(result.error)).toMatchObject({ + details: { + captured_output: { + truncated: true, + stdout_bytes: Buffer.byteLength(output.stdout), + stderr_bytes: Buffer.byteLength(output.stderr), + exit_code: 1, + }, + }, + }); + const capture = result.error.capturedOutput; + expect(capture).toBeDefined(); + expect(Buffer.byteLength(capture!.stdout)).toBeLessThanOrEqual( + NATIVE_FAILURE_DIAGNOSTIC_BYTES, + ); + expect(Buffer.byteLength(capture!.stderr)).toBeLessThanOrEqual( + NATIVE_FAILURE_DIAGNOSTIC_BYTES, + ); + expect(capture!.stdout).not.toContain("\ufffd"); + expect( + Buffer.byteLength(JSON.stringify(projectAnalysisError(result.error))), + ).toBeLessThan(8 * NATIVE_FAILURE_DIAGNOSTIC_BYTES); +});