diff --git a/src/android/JadxSession.ts b/src/android/JadxSession.ts index 08c1e94ac1..dbaf9c5cea 100644 --- a/src/android/JadxSession.ts +++ b/src/android/JadxSession.ts @@ -1,5 +1,5 @@ import { throwIfJadxInterrupted } from "./JadxInterruption.js"; -import { Client } from "@modelcontextprotocol/client"; +import { OwnedMcpClient } from "../process/OwnedMcpClient.js"; import { createAnalysisExecution } from "../application/AnalysisProvider.js"; import type { AndroidAnalysisExecution } from "../application/android/AndroidAnalysisPort.js"; import type { AndroidRequest } from "../domain/android/androidAnalysis.js"; @@ -64,7 +64,10 @@ type JadxCallObservation = { /** One owned engine connection; its wire protocol stays inside the Android adapter. */ export class JadxSession { readonly transport: JadxMcpTransport; - readonly #client = new Client({ name: "rea-android-adapter", version: "1" }); + readonly #client = new OwnedMcpClient({ + name: "rea-android-adapter", + version: "1", + }); #loaded: ReturnType | undefined; #connected = false; #partialContext: diff --git a/src/cli/artifactCommands.ts b/src/cli/artifactCommands.ts index ec98f5004d..e84ef66c96 100644 --- a/src/cli/artifactCommands.ts +++ b/src/cli/artifactCommands.ts @@ -13,6 +13,7 @@ import { withCommandCancellation } from "./commandCancellation.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { ARTIFACT_GRAPH_PROVIDER } from "../domain/investigationProviders.js"; import { keyedArchiveInputSchema } from "../domain/apple/keyedArchive.js"; +import { dylibResolutionInputSchema } from "../domain/apple/dylibResolution.js"; import { localPathStringSchema } from "../domain/localPath.js"; import { z } from "incur"; import { cliDecimalField } from "./options.js"; @@ -109,12 +110,9 @@ const registerDylibResolutionCommand = ( ), }), options: z.object({ - root: z - .array(z.string()) - .optional() - .describe( - "Process root relative to the bundle or the Mach-O's directory (repeatable)", - ), + root: dylibResolutionInputSchema.shape.roots.describe( + "Process root relative to the bundle or the Mach-O's directory (repeatable)", + ), architecture: z .enum(["arm64", "arm64e", "x86_64"]) .optional() diff --git a/src/cli/utilityCommands.ts b/src/cli/utilityCommands.ts index 244ce3c768..0b9e3ec257 100644 --- a/src/cli/utilityCommands.ts +++ b/src/cli/utilityCommands.ts @@ -229,8 +229,7 @@ const registerNativeCommands = ( path: localPathStringSchema.describe("App or Mach-O path"), }), options: z.object({ - relativePath: z - .string() + relativePath: localPathStringSchema .optional() .describe( "Plist path relative to the app root, or an absolute path (default: the bundle's Info.plist)", diff --git a/src/mcpDoctor.ts b/src/mcpDoctor.ts index 4113e4577d..5a7397723b 100644 --- a/src/mcpDoctor.ts +++ b/src/mcpDoctor.ts @@ -1,4 +1,5 @@ import { Client, SdkError, SdkErrorCode } from "@modelcontextprotocol/client"; +import { OwnedMcpClient } from "./process/OwnedMcpClient.js"; import { Help } from "incur"; import { performance } from "node:perf_hooks"; import { resolve } from "node:path"; @@ -135,7 +136,7 @@ export const runProductionMcpDoctor = async (options: McpDoctorOptions) => { }, stderr.append, ); - const client = new Client({ name: "rea-mcp-doctor", version: "1" }); + const client = new OwnedMcpClient({ name: "rea-mcp-doctor", version: "1" }); let result: ProductionMcpDoctorResult; let cleanupFailure: { readonly cause: unknown } | undefined; try { diff --git a/src/process/OwnedMcpClient.ts b/src/process/OwnedMcpClient.ts new file mode 100644 index 0000000000..7df1ea23d8 --- /dev/null +++ b/src/process/OwnedMcpClient.ts @@ -0,0 +1,12 @@ +import { Client } from "@modelcontextprotocol/client"; + +/** Observe SDK-initiated close failures while keeping them available to callers. */ +export class OwnedMcpClient extends Client { + override close(): Promise { + const closing = super.close(); + // The SDK starts close without awaiting it after a legacy handshake fails. + // The transport owner still receives the original failure when it joins cleanup. + void closing.catch(() => undefined); + return closing; + } +} diff --git a/tests/boundary/cli/dylibResolutionCli.test.ts b/tests/boundary/cli/dylibResolutionCli.test.ts index a06e5231e3..47ad439874 100644 --- a/tests/boundary/cli/dylibResolutionCli.test.ts +++ b/tests/boundary/cli/dylibResolutionCli.test.ts @@ -66,6 +66,33 @@ const expectRootInputError = async ( }); }; +/** The CLI option shares the contract's root format, so it refuses an escaping root itself and names the option. */ +const expectEscapingRootRefusedByCli = async ( + cli: TestCli, + app: string, +): Promise => { + const escaping = await cli.run({ + arguments: [ + "trace-dylib-resolution", + app, + "--root", + "../outside", + "--json", + ], + environment: ENVIRONMENT, + }); + expect(escaping.exitCode).toBe(1); + expect(escaping.json).toMatchObject({ + code: "VALIDATION_ERROR", + fieldErrors: [ + expect.objectContaining({ + path: expect.stringMatching(/^root/u), + message: expect.stringContaining("Use a normalized path"), + }), + ], + }); +}; + const fixtureApp = async (): Promise => { const app = join( await createTestTempDirectory("rea-dylib-cli-"), @@ -534,8 +561,8 @@ describe("trace-dylib-resolution CLI", () => { const outside = await createTestTempDirectory("rea-dylib-outside-"); await writeFiles(outside, { Tool: machoImage({}) }); await symlink(outside, join(app, "Contents/Linked"), "dir"); + await expectEscapingRootRefusedByCli(cli, app); for (const [root, reason, message] of [ - ["../outside", "invalid_format", "Use a normalized path"], ["Contents/Info.plist", "invalid_value", "is not a Mach-O image"], [ "Contents/Linked/Tool", diff --git a/tests/boundary/cli/emptyLocalTargetAdmission.test.ts b/tests/boundary/cli/emptyLocalTargetAdmission.test.ts index f2ff4e58d1..84dfc79c17 100644 --- a/tests/boundary/cli/emptyLocalTargetAdmission.test.ts +++ b/tests/boundary/cli/emptyLocalTargetAdmission.test.ts @@ -1,4 +1,5 @@ import { Cli } from "incur"; +import { Ajv2020 } from "ajv/dist/2020.js"; import { expect, it, vi } from "vitest"; import { registerArtifactCommands } from "../../../src/cli/artifactCommands.js"; import { registerCoreBinaryCommands } from "../../../src/cli/coreBinaryCommands.js"; @@ -132,6 +133,32 @@ it.each([ }, ); +it.each([ + { + command: "inspect-plist", + field: "relativePath", + valid: "Contents/Info.plist", + invalid: ["", "Contents/a\x00b"], + }, + { + command: "trace-dylib-resolution", + field: "root", + valid: ["Contents/MacOS/app"], + invalid: [[""], ["Contents/a\x00b"], ["../outside"]], + }, +])( + "advertises the $field option constraints", + async ({ command, field, valid, invalid }) => { + const { serve } = registry(); + const schema = (await serve([command, "--schema"])) as { options: object }; + const validate = new Ajv2020({ strict: false }).compile(schema.options); + expect(validate({})).toBe(true); + expect(validate({ [field]: valid })).toBe(true); + for (const value of invalid) + expect(validate({ [field]: value })).toBe(false); + }, +); + it("rejects an empty or NUL --snapshot option before analysis", async () => { const { serve, runs } = registry(); expect(await serve(["inspect", ".", "--snapshot", ""])).toMatchObject({ @@ -150,3 +177,24 @@ it("rejects an empty or NUL --snapshot option before analysis", async () => { }); for (const run of runs) expect(run).not.toHaveBeenCalled(); }); + +it.each([ + { argv: ["inspect-plist", ".", "--relative-path"], field: "relativePath" }, + { argv: ["trace-dylib-resolution", ".", "--root"], field: "root" }, +])( + "rejects an empty or NUL $field option on the CLI and names that option", + async ({ argv, field }) => { + const { serve, runs } = registry(); + for (const value of ["", "Contents/a\x00b"]) { + const result = (await serve([...argv, value])) as { + code: string; + fieldErrors: readonly { path: string }[]; + }; + expect(result.code).toBe("VALIDATION_ERROR"); + expect( + new Set(result.fieldErrors.map(({ path }) => path.split(".")[0])), + ).toEqual(new Set([field])); + } + for (const run of runs) expect(run).not.toHaveBeenCalled(); + }, +);