From 3cf6206614e65b5ebb6284a5709090dfb8f9aaee Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 20 Oct 2026 19:26:17 +0800 Subject: [PATCH] fix(mcp): require absolute evidence bundle paths --- src/contracts/sessionToolContracts.ts | 6 +- src/contracts/sessionToolSchemas.ts | 26 ++-- src/domain/localPath.test.ts | 28 ++++ src/domain/localPath.ts | 6 + .../boundary/cli/bundleComparisonCli.test.ts | 88 ++++++++++++- .../boundary/mcp/bundleComparisonMcp.test.ts | 120 +++++++++++++++++- 6 files changed, 258 insertions(+), 16 deletions(-) create mode 100644 src/domain/localPath.test.ts diff --git a/src/contracts/sessionToolContracts.ts b/src/contracts/sessionToolContracts.ts index 215345c792..e98cc45745 100644 --- a/src/contracts/sessionToolContracts.ts +++ b/src/contracts/sessionToolContracts.ts @@ -3,7 +3,6 @@ import { z } from "zod"; import { isAbsolute } from "node:path"; import { localPathStringSchema } from "../domain/localPath.js"; import { artifactComparisonInputSchema } from "../domain/artifactComparison.js"; -import { bundleComparisonInputSchema } from "../domain/bundleComparison.js"; import { callPathInputSchema } from "../domain/callPathSchemas.js"; import { changedBehaviorInputSchema } from "../domain/changedBehavior.js"; import { functionComparisonInputSchema } from "../domain/functionComparisonSchemas.js"; @@ -23,6 +22,7 @@ import { import { binarySessionInputSchema } from "./sessionStatusContract.js"; import { addressContextInputSchema, + bundleComparisonMcpInputSchema, importEvidenceBundleInputSchema, listUnknownsInputSchema, navigationContextInputSchema, @@ -114,8 +114,8 @@ export const SESSION_TOOL_CONTRACTS = [ ), session( "compare_bundles", - "Compare two canonical Evidence bundles by exact record membership, explicit one-to-one observation pairs, and complete residual-unknown revision histories. Missing bundle members describe omission only, never behavioral equivalence; output is digest-anchored and returns every change inline.", - bundleComparisonInputSchema, + "Compare two canonical Evidence bundles by exact record membership, explicit one-to-one observation pairs, and complete residual-unknown revision histories. Supply absolute local filesystem paths; relative paths are rejected. Missing bundle members describe omission only, never behavioral equivalence; output is digest-anchored and returns every change inline.", + bundleComparisonMcpInputSchema, ), session( "find_changed_behavior", diff --git a/src/contracts/sessionToolSchemas.ts b/src/contracts/sessionToolSchemas.ts index f48a5bcfb4..dbe5346825 100644 --- a/src/contracts/sessionToolSchemas.ts +++ b/src/contracts/sessionToolSchemas.ts @@ -1,7 +1,7 @@ import { z } from "zod"; -import { isAbsolute } from "node:path"; -import { localPathStringSchema } from "../domain/localPath.js"; +import { absoluteLocalPathStringSchema } from "../domain/localPath.js"; +import { bundleComparisonInputSchema } from "../domain/bundleComparison.js"; import { processTraceSpecificationSchema } from "../domain/process/processTraceSpecification.js"; import { evidenceSchema } from "../domain/evidence.js"; @@ -27,16 +27,22 @@ export const addressContextInputSchema = z.strictObject({ /** Session-owned Evidence bundle import options. */ export const importEvidenceBundleInputSchema = z.strictObject({ - path: localPathStringSchema - .refine(isAbsolute, { - message: - "path must be an absolute local filesystem path (for example /tmp/evidence.json or C:\\rea\\evidence.json)", - }) - .describe( - "Absolute local filesystem path for the evidence bundle to import; relative paths are rejected.", - ), + path: absoluteLocalPathStringSchema.describe( + "Absolute local filesystem path for the evidence bundle to import; relative paths are rejected.", + ), }); +/** MCP compares files selected on the server host, so require explicit paths. */ +export const bundleComparisonMcpInputSchema = + bundleComparisonInputSchema.extend({ + left_bundle_path: absoluteLocalPathStringSchema.describe( + "Absolute local filesystem path to the left Evidence bundle.", + ), + right_bundle_path: absoluteLocalPathStringSchema.describe( + "Absolute local filesystem path to the right Evidence bundle.", + ), + }); + /** Evidence references for deterministic process comparison. */ export const processComparisonInputSchema = z.strictObject({ left: evidenceSchema, diff --git a/src/domain/localPath.test.ts b/src/domain/localPath.test.ts new file mode 100644 index 0000000000..1bf3290d91 --- /dev/null +++ b/src/domain/localPath.test.ts @@ -0,0 +1,28 @@ +import { isAbsolute } from "node:path"; + +import { expect, it } from "vitest"; + +import { absoluteLocalPathStringSchema } from "./localPath.js"; + +it("matches the host path module for absolute local paths", () => { + for (const path of [ + "/absolute/path", + "/", + "relative/path", + "C:\\absolute\\path", + "c:/absolute/path", + "C:relative/path", + "\\\\server\\share\\path", + "\\rooted\\path", + "//server/share/path", + "file:///absolute/path", + ]) { + expect(absoluteLocalPathStringSchema.safeParse(path).success).toBe( + isAbsolute(path), + ); + } + expect(absoluteLocalPathStringSchema.safeParse("").success).toBe(false); + expect( + absoluteLocalPathStringSchema.safeParse("/absolute/with\0nul").success, + ).toBe(false); +}); diff --git a/src/domain/localPath.ts b/src/domain/localPath.ts index 789803a210..f6035536e7 100644 --- a/src/domain/localPath.ts +++ b/src/domain/localPath.ts @@ -19,6 +19,12 @@ export const nulFreeStringSchema = z /** A selected local filesystem path: nonempty and free of NUL bytes. */ export const localPathStringSchema = nulFreeStringSchema.min(1); +/** A caller-selected path that is absolute on the host running REA. */ +export const absoluteLocalPathStringSchema = localPathStringSchema.regex( + process.platform === "win32" ? /^(?:[\\/]|[A-Za-z]:[\\/])/u : /^\//u, + "must be an absolute local filesystem path", +); + /** Test lexical containment on the host platform; callers must resolve symlinks first. */ export const isPathWithinRoot = (root: string, path: string): boolean => { const value = relative(root, path); diff --git a/tests/boundary/cli/bundleComparisonCli.test.ts b/tests/boundary/cli/bundleComparisonCli.test.ts index 620cd0eaf2..17faf22b67 100644 --- a/tests/boundary/cli/bundleComparisonCli.test.ts +++ b/tests/boundary/cli/bundleComparisonCli.test.ts @@ -1,7 +1,10 @@ import { writeFile } from "node:fs/promises"; import { join } from "node:path"; +import { fileURLToPath } from "node:url"; -import { expect } from "vitest"; +import { Client } from "@modelcontextprotocol/client"; +import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; +import { expect, onTestFinished } from "vitest"; import { createEvidence } from "../../../src/domain/evidence.js"; import { bundleComparisonResultSchema } from "../../../src/domain/bundleComparison.js"; @@ -103,3 +106,86 @@ cliTest( }); }, ); + +cliTest( + "rejects relative bundle paths over stdio MCP while CLI resolves them from its cwd", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-bundle-path-boundary-"); + const leftPath = join(root, "left.json"); + const rightPath = join(root, "right.json"); + const encoded = serializeEvidenceBundle(createEvidenceBundle([])); + await Promise.all([ + writeFile(leftPath, encoded), + writeFile(rightPath, encoded), + ]); + + const client = new Client({ + name: "bundle-path-contract-test", + version: "1", + }); + const transport = new StdioClientTransport({ + command: process.execPath, + args: [ + fileURLToPath(new URL("../../../scripts/rea.mjs", import.meta.url)), + "mcp", + ], + cwd: root, + env: { + PATH: process.env.PATH ?? "", + HOME: root, + REA_LOG_LEVEL: "silent", + }, + stderr: "pipe", + }); + transport.stderr?.on("data", () => undefined); + onTestFinished(async () => { + try { + await client.close(); + } finally { + await transport.close(); + } + }); + + await client.connect(transport); + for (const [left, right, rejectedField] of [ + ["left.json", "right.json", "left_bundle_path"], + ["left.json", rightPath, "left_bundle_path"], + [leftPath, "right.json", "right_bundle_path"], + ] as const) { + const result = await client.callTool({ + name: "compare_bundles", + arguments: { left_bundle_path: left, right_bundle_path: right }, + }); + expect(result.isError).toBe(true); + expect(result.content[0]).toMatchObject({ + text: expect.stringContaining( + `${rejectedField}: must be an absolute local filesystem path`, + ), + }); + } + + const absolute = await client.callTool({ + name: "compare_bundles", + arguments: { + left_bundle_path: leftPath, + right_bundle_path: rightPath, + }, + }); + expect(absolute.isError).not.toBe(true); + expect(absolute.structuredContent).toMatchObject({ + normalized_result: { status: "unchanged" }, + }); + + const relativeCli = await cli.run({ + arguments: ["compare", "left.json", "right.json", "--json"], + cwd: root, + environment: { HOME: root, REA_LOG_LEVEL: "silent" }, + }); + expect(relativeCli.exitCode).toBe(0); + expect(bundleComparisonResultSchema.parse(relativeCli.json)).toMatchObject({ + status: "unchanged", + changes: [], + }); + }, + 60_000, +); diff --git a/tests/boundary/mcp/bundleComparisonMcp.test.ts b/tests/boundary/mcp/bundleComparisonMcp.test.ts index 24dd42dbfe..a5ae67f41d 100644 --- a/tests/boundary/mcp/bundleComparisonMcp.test.ts +++ b/tests/boundary/mcp/bundleComparisonMcp.test.ts @@ -4,13 +4,18 @@ import { rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; -import { afterEach, describe, expect, it } from "vitest"; +import { Ajv2020 } from "ajv/dist/2020.js"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { createTestBinarySession } from "../../fixtures/binarySession.js"; +import { compareEvidenceBundlesCommand } from "../../../src/application/EvidenceBundleCommands.js"; import { toolContract } from "../../../src/contracts/toolContracts.js"; -import { bundleComparisonResultSchema } from "../../../src/domain/bundleComparison.js"; +import { + bundleComparisonInputSchema, + bundleComparisonResultSchema, +} from "../../../src/domain/bundleComparison.js"; import { createEvidence } from "../../../src/domain/evidence.js"; import { createEvidenceBundle, @@ -140,6 +145,117 @@ describe("bundle comparison MCP semantic status", () => { }); }); +it("requires absolute bundle paths without changing server-cwd semantics", async () => { + const root = await createTestTempDirectory("rea-bundle-relative-mcp-"); + roots.push(root); + const encoded = serializeEvidenceBundle(createEvidenceBundle([])); + await Promise.all([ + writeFile(join(root, "left.json"), encoded), + writeFile(join(root, "right.json"), encoded), + ]); + const connected = await connect(); + const cwd = vi.spyOn(process, "cwd").mockReturnValue(root); + try { + const compare = (leftPath: string, rightPath: string) => + connected.client.callTool({ + name: "compare_bundles", + arguments: { + left_bundle_path: leftPath, + right_bundle_path: rightPath, + }, + }); + const relativeCases = [ + ["left.json", "right.json", "left_bundle_path"], + ["left.json", join(root, "right.json"), "left_bundle_path"], + [join(root, "left.json"), "right.json", "right_bundle_path"], + ] as const; + for (const [leftPath, rightPath, rejectedField] of relativeCases) { + const result = await compare(leftPath, rightPath); + expect(result.isError).toBe(true); + expect(result.content[0]).toMatchObject({ + text: expect.stringContaining( + `${rejectedField}: must be an absolute local filesystem path`, + ), + }); + } + expect( + bundleComparisonInputSchema.parse({ + left_bundle_path: "left.json", + right_bundle_path: "right.json", + }), + ).toMatchObject({ + left_bundle_path: "left.json", + right_bundle_path: "right.json", + }); + const cliCommand = await compareEvidenceBundlesCommand({ + leftPath: "left.json", + rightPath: "right.json", + }); + expect(cliCommand).toMatchObject({ + ok: true, + value: { status: "unchanged" }, + }); + const listed = await connected.client.listTools(); + const comparisonTool = listed.tools.find( + ({ name }) => name === "compare_bundles", + ); + if (comparisonTool === undefined) + throw new Error("Expected the compare_bundles MCP contract"); + const validate = new Ajv2020({ + strict: false, + validateFormats: false, + }).compile(comparisonTool.inputSchema); + const absolutePaths = { + left_bundle_path: join(root, "left.json"), + right_bundle_path: join(root, "right.json"), + }; + expect(validate(absolutePaths)).toBe(true); + expect(validate({ ...absolutePaths, left_bundle_path: "left.json" })).toBe( + false, + ); + expect(validate({ ...absolutePaths, left_bundle_path: "" })).toBe(false); + expect( + validate({ + ...absolutePaths, + left_bundle_path: `${absolutePaths.left_bundle_path}\0suffix`, + }), + ).toBe(false); + const relativeImport = await connected.client.callTool({ + name: "import_evidence_bundle", + arguments: { path: "left.json" }, + }); + expect(relativeImport.isError).toBe(true); + expect(relativeImport.content[0]).toMatchObject({ + text: expect.stringContaining("absolute local filesystem path"), + }); + const importTool = listed.tools.find( + ({ name }) => name === "import_evidence_bundle", + ); + if (importTool === undefined) + throw new Error("Expected the import_evidence_bundle MCP contract"); + const validateImport = new Ajv2020({ + strict: false, + validateFormats: false, + }).compile(importTool.inputSchema); + expect(validateImport({ path: absolutePaths.left_bundle_path })).toBe(true); + expect(validateImport({ path: "left.json" })).toBe(false); + expect( + validateImport({ path: `${absolutePaths.left_bundle_path}\0` }), + ).toBe(false); + const absolute = await compare( + join(root, "left.json"), + join(root, "right.json"), + ); + expect(absolute.isError).not.toBe(true); + expect(absolute.structuredContent).toMatchObject({ + normalized_result: { status: "unchanged" }, + }); + } finally { + await connected.close(); + cwd.mockRestore(); + } +}); + describe("bundle comparison MCP integration", () => { it("reads caller-selected bundle files and returns complete Evidence", async () => { const root = await createTestTempDirectory("rea-bundle-mcp-");