From 7ca7e38717cab85cfd01d38c9345f841891aae81 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 18 Oct 2026 08:04:33 +0800 Subject: [PATCH] fix(native): retain bounded plutil refusal diagnostics Read failed conversion diagnostics from both output streams, retaining raw bounded excerpts, observed byte totals, exit status and truncation. Remove only repeated path prefixes from display text and report a useful reason when the command prints no diagnostic. Reuse native failure capture bounds rather than expanding accepted nonzero output into errors. Adapted from PR #4851. Fixes #4849. Validation: six controlled boundary failures on baseline, including the stdout-only empty-reason defect; final46 passed across6files, real macOS CLI/MCP controls, static checks, docs5 and discovery1565. On macOS26.3 plutil uses stderr; stdout-only coverage is controlled output. Separate selected-target classification defect is reproduced and tracked as#4923. Co-authored-by: apitoagents <339136651+apitoagents@users.noreply.github.com> --- docs/native-investigation.md | 4 + src/native/NativeMacOSProvider.ts | 35 ++++- .../nativePlistSelectionFailureCli.test.ts | 80 ++++++++++- .../mcp/nativePlistDiagnosticsMcp.test.ts | 54 +++++++ .../native/nativePlistDiagnostics.test.ts | 133 ++++++++++++++++++ 5 files changed, 299 insertions(+), 7 deletions(-) create mode 100644 tests/boundary/mcp/nativePlistDiagnosticsMcp.test.ts create mode 100644 tests/boundary/providers/native/nativePlistDiagnostics.test.ts 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); +});