From 0e83d92c583ae36afe00b2c23dd2a0f1d025cbca Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 20 Oct 2026 19:54:14 +0800 Subject: [PATCH] fix(mcp): advertise existing absolute path constraints Reuse the schema-visible host path owner across session, firmware, managed, browser, Electron and Evidence-file inputs. Keep existing runtime and CLI path semantics. Refs #5493. --- src/contracts/managed/managedToolContracts.ts | 9 +- src/contracts/sessionLifecycleInputs.ts | 25 +- src/contracts/sessionStatusContract.ts | 10 +- src/contracts/sessionToolContracts.ts | 14 +- src/domain/analysisView/analysisView.ts | 14 +- src/domain/browserScenarioValues.ts | 10 +- src/domain/firmware/firmwareAnalysis.ts | 22 +- .../javascript/electronActiveObservation.ts | 14 +- .../mcp/absoluteLocalPathSchema.test.ts | 224 ++++++++++++++++++ 9 files changed, 256 insertions(+), 86 deletions(-) create mode 100644 tests/boundary/mcp/absoluteLocalPathSchema.test.ts diff --git a/src/contracts/managed/managedToolContracts.ts b/src/contracts/managed/managedToolContracts.ts index ead714fb7a..5fb4f70c70 100644 --- a/src/contracts/managed/managedToolContracts.ts +++ b/src/contracts/managed/managedToolContracts.ts @@ -1,20 +1,15 @@ import { z } from "zod"; -import { isAbsolute } from "node:path"; import type { ToolContract } from "../toolContractTypes.js"; import { managedOutputSchemas } from "../toolOutputSchemaGroups.js"; import { toolContractMetadata } from "../toolEffects.js"; import { requireOutputSchema } from "../toolOutputSchemaPrimitives.js"; -import { localPathStringSchema } from "../../domain/localPath.js"; +import { absoluteLocalPathStringSchema } from "../../domain/localPath.js"; import { analysisDetailSchema } from "../analysisDetail.js"; /** Exact caller boundary for execution-free PE/CLI triage and identity. */ export const managedTargetInputSchema = z.strictObject({ - path: localPathStringSchema - .refine(isAbsolute, { - message: - "path must be an absolute local filesystem path (for example /tmp/Example.dll or C:\\Example.dll)", - }) + path: absoluteLocalPathStringSchema .describe( "Explicit absolute managed PE/CLI path; omit to reuse the previously selected managed target", ) diff --git a/src/contracts/sessionLifecycleInputs.ts b/src/contracts/sessionLifecycleInputs.ts index 775da39cd7..4b03c1f66d 100644 --- a/src/contracts/sessionLifecycleInputs.ts +++ b/src/contracts/sessionLifecycleInputs.ts @@ -1,6 +1,5 @@ -import { isAbsolute } from "node:path"; import { executableFormatHintSchema } from "../domain/dosCom.js"; -import { localPathStringSchema } from "../domain/localPath.js"; +import { absoluteLocalPathStringSchema } from "../domain/localPath.js"; import { binaryArchitectureSchema, rawBinaryEntryPointSchema, @@ -10,24 +9,14 @@ import { z } from "zod"; import { analysisProviderSelectorSchema } from "./providerSelection.js"; /** Caller-supplied analysis snapshot path; must name its destination absolutely. */ -const snapshotPathSchema = localPathStringSchema - .refine(isAbsolute, { - message: - "snapshot_path must be an absolute local filesystem path (for example /tmp/rea/analysis.json or C:\\rea\\analysis.json)", - }) - .describe( - "Absolute local filesystem path for the analysis snapshot; relative paths are rejected.", - ); +const snapshotPathSchema = absoluteLocalPathStringSchema.describe( + "Absolute local filesystem path for the analysis snapshot; relative paths are rejected.", +); /** Caller-supplied analysis target path; must name the target absolutely. */ -const binaryTargetPathSchema = localPathStringSchema - .refine(isAbsolute, { - message: - "path must be an absolute local filesystem path (for example /tmp/fixture.bin or C:\\analysis\\fixture.bin)", - }) - .describe( - "Absolute local filesystem path for the analysis target; relative paths are rejected.", - ); +const binaryTargetPathSchema = absoluteLocalPathStringSchema.describe( + "Absolute local filesystem path for the analysis target; relative paths are rejected.", +); /** Input contract for opening a target with an optional staged snapshot. */ export const openBinaryInputSchema = z diff --git a/src/contracts/sessionStatusContract.ts b/src/contracts/sessionStatusContract.ts index 68385a728d..1c4d321338 100644 --- a/src/contracts/sessionStatusContract.ts +++ b/src/contracts/sessionStatusContract.ts @@ -1,7 +1,5 @@ import { z } from "zod"; -import { isAbsolute } from "node:path"; - -import { localPathStringSchema } from "../domain/localPath.js"; +import { absoluteLocalPathStringSchema } from "../domain/localPath.js"; /** Optional expectations used to compare the live session with its caller. */ export const binarySessionInputSchema = z.strictObject({ @@ -10,11 +8,7 @@ export const binarySessionInputSchema = z.strictObject({ .string() .regex(/^[a-f0-9]{64}$/u) .optional(), - expected_server_path: localPathStringSchema - .refine(isAbsolute, { - message: - "expected_server_path must be an absolute local filesystem path (for example /opt/rea/dist/main.js)", - }) + expected_server_path: absoluteLocalPathStringSchema .describe( "Absolute local filesystem path expected for the running server; relative paths are rejected.", ) diff --git a/src/contracts/sessionToolContracts.ts b/src/contracts/sessionToolContracts.ts index dfcd63accd..f43f4802de 100644 --- a/src/contracts/sessionToolContracts.ts +++ b/src/contracts/sessionToolContracts.ts @@ -1,8 +1,7 @@ import { matchNativeFunctionsInputSchema } from "./native/functionMatchingContracts.js"; import { z } from "zod"; -import { isAbsolute } from "node:path"; -import { localPathStringSchema } from "../domain/localPath.js"; +import { absoluteLocalPathStringSchema } from "../domain/localPath.js"; import { artifactComparisonInputSchema } from "../domain/artifactComparison.js"; import { callPathInputSchema } from "../domain/callPathSchemas.js"; import { changedBehaviorInputSchema } from "../domain/changedBehavior.js"; @@ -55,14 +54,9 @@ const session = < /** Session-owned Evidence bundle export options. */ export const exportEvidenceBundleInputSchema = z.strictObject({ - path: localPathStringSchema - .refine(isAbsolute, { - message: - "path must be an absolute local filesystem path (for example /tmp/rea/evidence.json or C:\\rea\\evidence.json)", - }) - .describe( - "Absolute local filesystem path for the exported evidence bundle; relative paths are rejected.", - ), + path: absoluteLocalPathStringSchema.describe( + "Absolute local filesystem path for the exported evidence bundle; relative paths are rejected.", + ), overwrite: z.boolean().default(false), }); diff --git a/src/domain/analysisView/analysisView.ts b/src/domain/analysisView/analysisView.ts index ba9b70da48..46828d2102 100644 --- a/src/domain/analysisView/analysisView.ts +++ b/src/domain/analysisView/analysisView.ts @@ -20,9 +20,8 @@ import { NATIVE_INVENTORY_OPERATIONS, nativeInventoryOperationSchema, } from "../native/nativeInventory.js"; -import { isAbsolute } from "node:path"; -import { localPathStringSchema } from "../localPath.js"; +import { absoluteLocalPathStringSchema } from "../localPath.js"; import { z } from "zod"; import { desktopViewRequestSchema, @@ -297,14 +296,9 @@ export const analysisViewSourceSchema = z.discriminatedUnion("kind", [ }), z.strictObject({ kind: z.literal("evidence-file"), - path: localPathStringSchema - .refine(isAbsolute, { - message: - "path must be an absolute local filesystem path (for example /tmp/capture.json or C:\\rea\\capture.json)", - }) - .describe( - "Absolute local path of one completed Evidence JSON file. REA reads and authenticates it, then projects it like retained Evidence, so large captures are not charged as inline input.", - ), + path: absoluteLocalPathStringSchema.describe( + "Absolute local path of one completed Evidence JSON file. REA reads and authenticates it, then projects it like retained Evidence, so large captures are not charged as inline input.", + ), }), ]); diff --git a/src/domain/browserScenarioValues.ts b/src/domain/browserScenarioValues.ts index e31f2e4055..6491a664e4 100644 --- a/src/domain/browserScenarioValues.ts +++ b/src/domain/browserScenarioValues.ts @@ -1,9 +1,8 @@ import { z } from "zod"; -import { isAbsolute } from "node:path"; import { IDENTIFIER_PATTERN } from "./stringPatterns.js"; -import { localPathStringSchema } from "./localPath.js"; +import { absoluteLocalPathStringSchema } from "./localPath.js"; import { browserNetworkContentSelectionSchema } from "./browserNetworkEvidence.js"; import { browserScenarioEventFamilySchema } from "./browserScenarioCaptureValues.js"; import { @@ -18,11 +17,6 @@ import { export const scenarioIdentifierSchema = z.string().regex(IDENTIFIER_PATTERN); -const browserExecutablePathSchema = localPathStringSchema.refine(isAbsolute, { - message: - "executable_path must be an absolute local filesystem path (for example /opt/chromium/chrome or C:\\chromium\\chrome.exe)", -}); - const browserScenarioBaseUrlSchema = z .string() .min(1) @@ -77,7 +71,7 @@ export type BrowserScenarioUrl = z.infer; export const browserScenarioBrowserSchema = z.discriminatedUnion("mode", [ z.strictObject({ mode: z.literal("launch"), - executable_path: browserExecutablePathSchema.describe( + executable_path: absoluteLocalPathStringSchema.describe( "Absolute local filesystem path for the browser executable; relative paths are rejected.", ), headless: z diff --git a/src/domain/firmware/firmwareAnalysis.ts b/src/domain/firmware/firmwareAnalysis.ts index 852f380cd4..e25a378fbf 100644 --- a/src/domain/firmware/firmwareAnalysis.ts +++ b/src/domain/firmware/firmwareAnalysis.ts @@ -1,20 +1,14 @@ import { z } from "zod"; -import { isAbsolute } from "node:path"; import { digestSchema } from "../digests.js"; import { jsonValueSchema, type JsonValue } from "../jsonValue.js"; -import { localPathStringSchema } from "../localPath.js"; +import { absoluteLocalPathStringSchema } from "../localPath.js"; const bytes = z.number().int().min(0).max(Number.MAX_SAFE_INTEGER); const range = z.strictObject({ offset: bytes, length: bytes.positive() }); const target = { - path: localPathStringSchema - .refine(isAbsolute, { - message: - "path must be an absolute local filesystem path (for example /tmp/firmware.bin or C:\\firmware\\firmware.bin)", - }) - .describe( - "Absolute local firmware file; never executed. Relative paths are rejected.", - ), + path: absoluteLocalPathStringSchema.describe( + "Absolute local firmware file; never executed. Relative paths are rejected.", + ), }; /** Explicit firmware inspection and extraction intent, independent of engines. */ @@ -22,11 +16,9 @@ export const firmwareInputSchemas = { inspect_firmware_regions: z.strictObject(target), extract_firmware: z.strictObject({ ...target, - output_directory: localPathStringSchema - .refine(isAbsolute, { - message: "output_directory must be an absolute local filesystem path", - }) - .describe("Absolute, absent output directory; parent must exist"), + output_directory: absoluteLocalPathStringSchema.describe( + "Absolute, absent output directory; parent must exist", + ), range: range .optional() .describe("Explicit file-byte interval; omit to extract the whole file"), diff --git a/src/domain/javascript/electronActiveObservation.ts b/src/domain/javascript/electronActiveObservation.ts index 0740f80f24..9579e3d671 100644 --- a/src/domain/javascript/electronActiveObservation.ts +++ b/src/domain/javascript/electronActiveObservation.ts @@ -1,12 +1,6 @@ import { z } from "zod"; -import { isAbsolute } from "node:path"; +import { absoluteLocalPathStringSchema } from "../localPath.js"; -import { localPathStringSchema } from "../localPath.js"; - -const pathInputSchema = localPathStringSchema.refine(isAbsolute, { - message: - "Electron executable, application, and root paths must be absolute local filesystem paths (for example /Applications/Electron.app/Contents/MacOS/Electron)", -}); const absolutePathSchema = z .string() .min(1) @@ -66,13 +60,13 @@ const actionSchema = z.discriminatedUnion("kind", [ /** Input for one explicit, provider-owned Electron runtime experiment. */ export const electronActiveObservationInputSchema = z.strictObject({ - executable_path: pathInputSchema.describe( + executable_path: absoluteLocalPathStringSchema.describe( "Absolute local filesystem path for the Electron executable; relative paths are rejected.", ), - application_path: pathInputSchema.describe( + application_path: absoluteLocalPathStringSchema.describe( "Absolute local filesystem path for the Electron application; relative paths are rejected.", ), - application_root: pathInputSchema + application_root: absoluteLocalPathStringSchema .optional() .describe( "Absolute local filesystem root for application-relative paths; omit to derive it from the application path. Relative paths are rejected.", diff --git a/tests/boundary/mcp/absoluteLocalPathSchema.test.ts b/tests/boundary/mcp/absoluteLocalPathSchema.test.ts new file mode 100644 index 0000000000..2340dc1e17 --- /dev/null +++ b/tests/boundary/mcp/absoluteLocalPathSchema.test.ts @@ -0,0 +1,224 @@ +import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { Ajv2020 } from "ajv/dist/2020.js"; +import { resolve } from "node:path"; +import { expect, it } from "vitest"; + +import { createTestBinarySession } from "../../fixtures/binarySession.js"; +import { createServer } from "../../../src/server/createServer.js"; +import { silentLogger } from "../../../src/logger.js"; + +interface PathInputCase { + readonly name: string; + readonly field: string; + readonly valid: Readonly>; + readonly invalid: Readonly>; +} + +const createMcp = async (firmwareCalls: string[]) => { + const session = createTestBinarySession(() => { + throw new Error("Invalid path input must not acquire a binary provider"); + }); + const server = createServer( + { kind: "session", session }, + { + logger: silentLogger, + firmwareAnalysis: { + execute: async (request) => { + firmwareCalls.push(request.operation); + throw new Error("Invalid path input reached firmware analysis"); + }, + close: async () => {}, + }, + }, + ); + const client = new Client({ name: "absolute-path-schema", version: "1" }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + + const close = async (): Promise => { + const failures: unknown[] = []; + for (const release of [ + () => client.close(), + () => server.close(), + () => session.close(), + ]) { + try { + await release(); + } catch (cause) { + failures.push(cause); + } + } + if (failures.length > 0) + throw new AggregateError(failures, "MCP path fixture cleanup failed"); + }; + + try { + await server.connect(serverTransport); + await client.connect(clientTransport); + return { client, close }; + } catch (cause: unknown) { + await close(); + throw cause; + } +}; + +const inputCases = ( + absolute: string, + relative: string, +): readonly PathInputCase[] => { + const pathCase = ( + name: string, + field: string, + valid: Readonly>, + invalid: Readonly>, + ): PathInputCase => ({ name, field, valid, invalid }); + + return [ + pathCase("open_binary", "path", { path: absolute }, { path: relative }), + pathCase( + "open_binary", + "snapshot_path", + { path: absolute, snapshot_path: absolute }, + { path: absolute, snapshot_path: relative }, + ), + pathCase( + "close_binary", + "snapshot_path", + { snapshot_path: absolute }, + { snapshot_path: relative }, + ), + pathCase( + "export_evidence_bundle", + "path", + { path: absolute }, + { path: relative }, + ), + pathCase( + "binary_session", + "expected_server_path", + { expected_server_path: absolute }, + { expected_server_path: relative }, + ), + ...[ + "inspect_managed_artifact", + "inspect_managed_members", + "inspect_managed_native_boundaries", + ].map((name) => + pathCase(name, "path", { path: absolute }, { path: relative }), + ), + pathCase( + "inspect_firmware_regions", + "path", + { path: absolute }, + { path: relative }, + ), + pathCase( + "extract_firmware", + "path", + { path: absolute, output_directory: absolute }, + { path: relative, output_directory: absolute }, + ), + pathCase( + "extract_firmware", + "output_directory", + { path: absolute, output_directory: absolute }, + { path: absolute, output_directory: relative }, + ), + pathCase( + "inspect_analysis_view", + "source.path", + { + source: { kind: "evidence-file", path: absolute }, + view: { kind: "summary" }, + }, + { + source: { kind: "evidence-file", path: relative }, + view: { kind: "summary" }, + }, + ), + pathCase( + "capture_browser_scenario", + "browser.executable_path", + { + browser: { mode: "launch", executable_path: absolute }, + start_url: { url: "https://example.test/" }, + actions: [ + { step_id: "wait", action: "wait_for_timeout", duration_ms: 1 }, + ], + }, + { + browser: { mode: "launch", executable_path: relative }, + start_url: { url: "https://example.test/" }, + actions: [ + { step_id: "wait", action: "wait_for_timeout", duration_ms: 1 }, + ], + }, + ), + ...["executable_path", "application_path", "application_root"].map( + (field) => + pathCase( + "capture_electron_scenario", + field, + { + executable_path: absolute, + application_path: absolute, + [field]: absolute, + }, + { + executable_path: absolute, + application_path: absolute, + [field]: relative, + }, + ), + ), + ]; +}; + +it("advertises host-absolute path constraints after SDK schema conversion", async () => { + const firmwareCalls: string[] = []; + const { client, close } = await createMcp(firmwareCalls); + try { + const listed = await client.listTools(); + const ajv = new Ajv2020({ strict: false, validateFormats: false }); + const absolute = resolve("rea-absolute-input"); + const relative = "relative-input"; + + for (const { name, field, valid, invalid } of inputCases( + absolute, + relative, + )) { + const schema = listed.tools.find( + (tool) => tool.name === name, + )?.inputSchema; + if (schema === undefined) + throw new Error(`Missing advertised tool ${name}`); + const validate = ajv.compile(schema); + expect(validate(invalid), `${name}.${field}`).toBe(false); + expect( + validate(valid), + `${name}.${field}: ${JSON.stringify(validate.errors)}`, + ).toBe(true); + } + expect(firmwareCalls).toEqual([]); + } finally { + await close(); + } +}); + +it("rejects relative paths through in-memory MCP handlers", async () => { + const firmwareCalls: string[] = []; + const { client, close } = await createMcp(firmwareCalls); + try { + for (const { name, field, invalid } of inputCases( + resolve("rea-absolute-input"), + "relative-input", + )) { + const result = await client.callTool({ name, arguments: invalid }); + expect(result.isError, `${name}.${field}`).toBe(true); + expect(JSON.stringify(result), `${name}.${field}`).toContain("absolute"); + } + expect(firmwareCalls).toEqual([]); + } finally { + await close(); + } +});