diff --git a/docs/testing.md b/docs/testing.md index e19db750ba..9376578c4a 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -256,6 +256,22 @@ pass that definition through the SDK's `toolDefinition` call option. This keeps the selected output-schema validation without eagerly compiling unrelated catalog schemas. Re-read definitions if the server advertises a changed list. +Artifact verification uses `artifactMcpCall` for both lifecycle and analysis +calls. A fresh SDK client has no held tool listing, so a bare `callTool` can +skip output-schema validation. The helper discovers actual advertised +definitions, rejects missing or ambiguous schemas, and validates the requested +tool without compiling unrelated catalog schemas. REA registers process-static +tool contracts; bindings share one discovery per negotiated connection and are +discarded on reconnect or failed discovery. Keep raw artifact calls on this +shared path and retain explicit request options such as timeouts. +Caller-supplied tool definitions are rejected: the helper owns selection from +the advertised catalog so a stale or permissive schema cannot bypass it. + +The regression fixture changes only `canonical_path` in a real stdio response +to a number. The old helper accepts that response; the named schema must reject +it. This client-boundary fault injection does not establish native Apple +behavior, which remains covered by the macOS verification lane. + Report unavailable authority as a named skip. Await owned cleanup before a verifier reports success. Native process inspectors are released at Vitest worker teardown; fork termination does not run Node exit hooks, and per-file diff --git a/scripts/lib/artifact-e2e.mjs b/scripts/lib/artifact-e2e.mjs index 444639319e..a2ab021fef 100644 --- a/scripts/lib/artifact-e2e.mjs +++ b/scripts/lib/artifact-e2e.mjs @@ -4,6 +4,67 @@ import { promisify } from "node:util"; import { fileURLToPath } from "node:url"; import { Client } from "@modelcontextprotocol/client"; import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; +import { closeVerifierResources } from "./verifier-cleanup.mjs"; + +// REA registers process-static tool contracts. Bind their parsed definitions to +// the SDK's negotiated connection identity, including when a Client reconnects. +const toolBindings = new WeakMap(); + +const advertisedTool = async (client, name) => { + const connection = client.getServerVersion(); + assert.ok(connection, "MCP must be initialized before binding tool schemas"); + let binding = toolBindings.get(client); + if (binding?.connection !== connection) { + binding = { + connection, + definitions: client.listTools().then(({ tools }) => { + const definitions = new Map(); + for (const tool of tools) { + assert.ok( + !definitions.has(tool.name), + `Duplicate MCP tool: ${tool.name}`, + ); + definitions.set(tool.name, tool); + } + return definitions; + }), + }; + toolBindings.set(client, binding); + } + let definitions; + try { + definitions = await binding.definitions; + assert.equal( + client.getServerVersion(), + connection, + "MCP connection changed while binding tool schemas", + ); + } catch (cause) { + if (toolBindings.get(client) === binding) toolBindings.delete(client); + throw cause; + } + const tool = definitions.get(name); + assert.ok(tool, `MCP catalog did not advertise ${name}`); + return tool; +}; + +/** Call an advertised tool with SDK output validation of its actual schema. */ +export async function artifactMcpCall(client, parameters, options = {}) { + options.signal?.throwIfAborted(); + assert.equal( + options.toolDefinition, + undefined, + "Artifact verification owns the advertised schema; pass request options only", + ); + const toolDefinition = await advertisedTool(client, parameters.name); + assert.notEqual( + toolDefinition.outputSchema, + undefined, + `MCP tool omitted its output schema: ${parameters.name}`, + ); + options.signal?.throwIfAborted(); + return client.callTool(parameters, { ...options, toolDefinition }); +} const environment = () => { const compileCache = @@ -66,9 +127,10 @@ const withOpenedArtifactMcp = async (target, openTool, verify) => { stderr: "pipe", }); const client = new Client({ name: "artifact-real-e2e", version: "1" }); + let failure; try { await client.connect(transport); - const opened = await client.callTool({ + const opened = await artifactMcpCall(client, { name: openTool, arguments: { path: target }, }); @@ -76,9 +138,15 @@ const withOpenedArtifactMcp = async (target, openTool, verify) => { if (openTool === "open_bundle_resources") assert.equal(opened.structuredContent?.result?.caller_path, target); await verify(client); + } catch (cause) { + failure = cause; + throw cause; } finally { - await client.close(); - await transport.close(); + toolBindings.delete(client); + await closeVerifierResources( + [() => client.close(), () => transport.close()], + failure, + ); } }; @@ -94,7 +162,10 @@ export async function withBundleResourcesMcp(target, verify) { /** Require a successful structured MCP tool result rather than its text rendering. */ export async function artifactMcpResult(client, name, arguments_ = {}) { - const result = await client.callTool({ name, arguments: arguments_ }); + const result = await artifactMcpCall(client, { + name, + arguments: arguments_, + }); assert.notEqual(result.isError, true, JSON.stringify(result)); assert.ok( result.structuredContent?.normalized_result, diff --git a/scripts/verify-native-calls.mjs b/scripts/verify-native-calls.mjs index 7a22620756..2d43a04573 100644 --- a/scripts/verify-native-calls.mjs +++ b/scripts/verify-native-calls.mjs @@ -10,6 +10,7 @@ import { promisify } from "node:util"; import { artifactCli, artifactMcpResult, + artifactMcpCall, withArtifactMcp, } from "./lib/artifact-e2e.mjs"; @@ -225,7 +226,8 @@ const checkHardenedRuntime = async (directory, binary) => { assert.equal(denied.code, "capability_unavailable"); assert.match(denied.details.reason, /^debugger-attach-denied: /u); await withArtifactMcp(blocked, async (client) => { - const deniedMcp = await client.callTool( + const deniedMcp = await artifactMcpCall( + client, { name: "observe_native_calls", arguments: INPUT, diff --git a/scripts/verify/apple/asset-catalog.mjs b/scripts/verify/apple/asset-catalog.mjs index 4de295f06f..a194d92111 100644 --- a/scripts/verify/apple/asset-catalog.mjs +++ b/scripts/verify/apple/asset-catalog.mjs @@ -9,6 +9,7 @@ import { promisify } from "node:util"; import { artifactCli, artifactMcpResult, + artifactMcpCall, withArtifactMcp, withBundleResourcesMcp, } from "../../lib/artifact-e2e.mjs"; @@ -97,7 +98,7 @@ try { offset: first.next_offset, }); assert.deepEqual([...first.records, ...next.records], cli.records); - const invalid = await client.callTool({ + const invalid = await artifactMcpCall(client, { name: "inspect_asset_catalog", arguments: { offset: -1 }, }); diff --git a/scripts/verify/apple/keyed-archive.mjs b/scripts/verify/apple/keyed-archive.mjs index 471b76ef64..e44929d7e1 100644 --- a/scripts/verify/apple/keyed-archive.mjs +++ b/scripts/verify/apple/keyed-archive.mjs @@ -9,6 +9,7 @@ import { createHash } from "node:crypto"; import { artifactCli, artifactMcpResult, + artifactMcpCall, withArtifactMcp, } from "../../lib/artifact-e2e.mjs"; import { @@ -53,7 +54,7 @@ try { }); assert.deepEqual(page.objects, [graph.objects[1]]); assert.equal(page.next_offset, 2); - const invalid = await client.callTool({ + const invalid = await artifactMcpCall(client, { name: "inspect_keyed_archive", arguments: { path: "../other.plist" }, }); diff --git a/scripts/verify/apple/macho-occurrences.mjs b/scripts/verify/apple/macho-occurrences.mjs index 9a7da4f4e0..d1719af43b 100644 --- a/scripts/verify/apple/macho-occurrences.mjs +++ b/scripts/verify/apple/macho-occurrences.mjs @@ -5,6 +5,7 @@ import { relative, sep } from "node:path"; import { parseArtifactInventoryEvidence } from "../../../dist/domain/artifactInventoryEvidence.js"; import { artifactCliEvidence, + artifactMcpCall, withArtifactMcp, } from "../../lib/artifact-e2e.mjs"; @@ -53,7 +54,7 @@ export async function verifyMachOOccurrences(app, executable) { assert.equal(node.sha256, direct.manifest.root_sha256); assert.equal(node.format, root.artifact_format); await withArtifactMcp(app, async (client) => { - const reply = await client.callTool({ + const reply = await artifactMcpCall(client, { name: "inspect_artifact", arguments: {}, }); diff --git a/scripts/verify/apple/macos-dylib-e2e.mjs b/scripts/verify/apple/macos-dylib-e2e.mjs index d9538c4cce..59a55bb7c1 100644 --- a/scripts/verify/apple/macos-dylib-e2e.mjs +++ b/scripts/verify/apple/macos-dylib-e2e.mjs @@ -7,6 +7,7 @@ import { promisify } from "node:util"; import { artifactCliEvidence, artifactMcpResult, + artifactMcpCall, withArtifactMcp, } from "../../lib/artifact-e2e.mjs"; @@ -180,7 +181,7 @@ export async function verifyDylibResolution(app) { await artifactMcpResult(client, "trace_dylib_resolution"), trace, ); - const invalid = await client.callTool({ + const invalid = await artifactMcpCall(client, { name: "trace_dylib_resolution", arguments: { roots: ["../outside"] }, }); diff --git a/scripts/verify/apple/native-provenance.mjs b/scripts/verify/apple/native-provenance.mjs index 63ae2183b0..a225fafb95 100644 --- a/scripts/verify/apple/native-provenance.mjs +++ b/scripts/verify/apple/native-provenance.mjs @@ -5,6 +5,7 @@ import { readFile } from "node:fs/promises"; import { toolContract } from "../../../dist/contracts/toolContracts.js"; import { artifactCliEvidence, + artifactMcpCall, withArtifactMcp, withBundleResourcesMcp, } from "../../lib/artifact-e2e.mjs"; @@ -27,7 +28,7 @@ export async function verifyNativeProvenance(app, executable) { assert.equal(resourcePlistCli.subject, null); assert.equal(resourcePlistCli.raw_result, null); await withBundleResourcesMcp(app, async (client) => { - const response = await client.callTool({ + const response = await artifactMcpCall(client, { name: "inspect_plist", arguments: {}, }); @@ -58,13 +59,10 @@ export async function verifyNativeProvenance(app, executable) { assert.equal(binaryPlistCli.raw_result, null); assert.equal(binaryPlistCli.normalized_result.resource_root, undefined); await withArtifactMcp(app, async (client) => { - const definitions = new Map( - (await client.listTools()).tools.map((tool) => [tool.name, tool]), - ); - const plistResponse = await client.callTool( - { name: "inspect_plist", arguments: {} }, - { toolDefinition: definitions.get("inspect_plist") }, - ); + const plistResponse = await artifactMcpCall(client, { + name: "inspect_plist", + arguments: {}, + }); assert.notEqual(plistResponse.isError, true, JSON.stringify(plistResponse)); const binaryPlistMcp = toolContract("inspect_plist").outputSchema.parse( plistResponse.structuredContent, @@ -98,13 +96,10 @@ export async function verifyNativeProvenance(app, executable) { arguments_, ), ); - const response = await client.callTool( - { - name: operation, - arguments: operation === "demangle_swift" ? { symbols } : {}, - }, - { toolDefinition: definitions.get(operation) }, - ); + const response = await artifactMcpCall(client, { + name: operation, + arguments: operation === "demangle_swift" ? { symbols } : {}, + }); assert.notEqual(response.isError, true, JSON.stringify(response)); const mcp = toolContract(operation).outputSchema.parse( response.structuredContent, diff --git a/tests/boundary/process/artifactMcpVerifierOutput.test.ts b/tests/boundary/process/artifactMcpVerifierOutput.test.ts new file mode 100644 index 0000000000..91da42a924 --- /dev/null +++ b/tests/boundary/process/artifactMcpVerifierOutput.test.ts @@ -0,0 +1,126 @@ +import { execFile } from "node:child_process"; +import { mkdir, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { promisify } from "node:util"; +import { expect, it } from "vitest"; +import { z } from "zod"; + +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +const exec = promisify(execFile); +const observationSchema = z.strictObject({ + accepted: z.boolean(), + catalogRequests: z.number().int().nonnegative(), + corruptedReplies: z.number().int().nonnegative(), + toolCalls: z.number().int().nonnegative(), + toolTimeouts: z.array(z.number().nullable()), + error: z + .strictObject({ + name: z.string(), + message: z.string(), + code: z.union([z.number(), z.string()]).nullable(), + }) + .nullable(), +}); + +async function probe(mode: string, name = "Fixture.app") { + const root = await createTestTempDirectory("rea-artifact-mcp-schema-"); + const target = join(root, name); + await mkdir(join(target, "Contents", "Resources"), { recursive: true }); + await writeFile( + join(target, "Contents", "Info.plist"), + 'CFBundleIdentifierdev.rea.schema-probe', + ); + const { stdout } = await exec( + process.execPath, + [ + fileURLToPath( + new URL("../../fixtures/artifactMcpVerifierProbe.mjs", import.meta.url), + ), + mode, + target, + ], + // Retain cleanup time within the existing 30s test budget. + { timeout: 25_000 }, + ); + return observationSchema.parse(JSON.parse(stdout)); +} + +it("validates the real resource admission reply against its advertised schema", async () => { + expect(await probe("healthy", "Fixture.app")).toEqual({ + accepted: true, + catalogRequests: 1, + corruptedReplies: 0, + toolCalls: 1, + toolTimeouts: [null], + error: null, + }); +}); + +it.each(["Fixture.app", "Fixture with spaces 雨.app"])( + "rejects a corrupted canonical path in a real admission reply: %s", + async (name) => { + const result = await probe("corrupt", name); + expect(result.accepted).toBe(false); + expect(result.catalogRequests).toBe(1); + expect(result.corruptedReplies).toBe(1); + expect(result.error).toMatchObject({ + name: "ProtocolError", + code: -32602, + message: expect.stringMatching(/output schema.*canonical_path/u), + }); + }, +); + +it.each(["reuse", "concurrent"])( + "shares one schema discovery within one MCP connection: %s", + async (mode) => { + const result = await probe(mode); + expect(result.accepted).toBe(true); + expect(result.catalogRequests).toBe(1); + expect(result.toolCalls).toBe(mode === "reuse" ? 3 : 2); + }, +); + +it("binds the new advertised schema after reconnecting the same client", async () => { + const result = await probe("reconnect"); + expect(result.accepted).toBe(false); + expect(result.catalogRequests).toBe(2); + expect(result.toolCalls).toBe(2); + expect(result.error).toMatchObject({ + code: -32602, + message: expect.stringContaining("__rea_second_connection"), + }); +}); + +it("rejects an omitted output schema before calling the advertised tool", async () => { + const result = await probe("missing-schema"); + expect(result.accepted).toBe(false); + expect(result.toolCalls).toBe(0); + expect(result.error?.message).toMatch(/omitted its output schema/u); +}); + +it("rejects a supplied schema override before dispatching that call", async () => { + const result = await probe("override"); + expect(result.accepted).toBe(false); + expect(result.toolCalls).toBe(1); + expect(result.error?.message).toMatch(/owns the advertised schema/u); +}); + +it("preserves explicit request timeouts on schema-bound calls", async () => { + const result = await probe("request-options"); + expect(result.accepted).toBe(true); + expect(result.catalogRequests).toBe(1); + expect(result.toolTimeouts).toEqual([null, 12_345]); +}); + +it.each(["duplicate-first", "duplicate-last"])( + "rejects ambiguous named schemas independently of catalog order: %s", + async (mode) => { + const result = await probe(mode); + expect(result.accepted).toBe(false); + expect(result.toolCalls).toBe(0); + expect(result.error?.message).toMatch(/Duplicate MCP tool/u); + }, +); diff --git a/tests/fixtures/artifactMcpVerifierProbe.mjs b/tests/fixtures/artifactMcpVerifierProbe.mjs new file mode 100644 index 0000000000..731d39881f --- /dev/null +++ b/tests/fixtures/artifactMcpVerifierProbe.mjs @@ -0,0 +1,209 @@ +import assert from "node:assert/strict"; +import { fileURLToPath } from "node:url"; +import { Client } from "@modelcontextprotocol/client"; +import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; + +import * as artifact from "../../scripts/lib/artifact-e2e.mjs"; +import { closeVerifierResources } from "../../scripts/lib/verifier-cleanup.mjs"; + +const [mode, target] = process.argv.slice(2); +assert.ok( + [ + "healthy", + "corrupt", + "reuse", + "concurrent", + "reconnect", + "missing-schema", + "duplicate-first", + "duplicate-last", + "override", + "request-options", + ].includes(mode), +); +assert.equal(typeof target, "string"); + +// Isolate the fault to this child client. The server, resource admission and +// advertised schema are real; alter only the reply before tool validation. +const request = Client.prototype.request; +let catalogRequests = 0; +let corruptedReplies = 0; +let toolCalls = 0; +let connection = 0; +const toolTimeouts = []; +Client.prototype.request = async function (input, options) { + if (input.method === "tools/list") catalogRequests++; + if (input.method === "tools/call") { + toolCalls++; + toolTimeouts.push(options?.timeout ?? null); + } + const result = await request.call(this, input, options); + if ( + mode === "override" && + input.method === "tools/call" && + input.params.name === "binary_session" + ) { + corruptedReplies++; + return { ...result, structuredContent: { result: 777 } }; + } + if (input.method === "tools/list") { + if (mode === "missing-schema") + return { + ...result, + tools: result.tools.map((tool) => { + if (tool.name !== "open_bundle_resources") return tool; + const { outputSchema: _schema, ...withoutSchema } = tool; + return withoutSchema; + }), + }; + if (mode === "duplicate-first" || mode === "duplicate-last") { + const listed = result.tools.find( + (tool) => tool.name === "open_bundle_resources", + ); + assert.ok(listed); + const duplicate = { + ...listed, + outputSchema: { type: "object", required: ["__rea_wrong_schema"] }, + }; + return { + ...result, + tools: + mode === "duplicate-first" + ? [duplicate, ...result.tools] + : [...result.tools, duplicate], + }; + } + if (mode === "reconnect" && connection === 2) + return { + ...result, + tools: result.tools.map((tool) => + tool.name === "binary_session" + ? { + ...tool, + outputSchema: { + type: "object", + required: ["__rea_second_connection"], + }, + } + : tool, + ), + }; + } + if ( + mode !== "corrupt" || + input.method !== "tools/call" || + input.params.name !== "open_bundle_resources" + ) + return result; + assert.equal( + typeof result.structuredContent?.result?.canonical_path, + "string", + "The real reply omitted the field selected for fault injection", + ); + corruptedReplies++; + return { + ...result, + structuredContent: { + ...result.structuredContent, + result: { ...result.structuredContent.result, canonical_path: 777 }, + }, + }; +}; + +const status = async (client) => { + const result = await artifact.artifactMcpCall(client, { + name: "binary_session", + arguments: {}, + }); + assert.notEqual(result.isError, true, JSON.stringify(result)); +}; + +async function standaloneConnections() { + const client = new Client({ name: "artifact-binding-probe", version: "1" }); + let firstVersion; + for ( + connection = 1; + connection <= (mode === "reconnect" ? 2 : 1); + connection++ + ) { + const transport = new StdioClientTransport({ + command: process.execPath, + args: [ + fileURLToPath(new URL("../../scripts/rea.mjs", import.meta.url)), + "mcp", + ], + env: { + PATH: process.env.PATH ?? "/usr/bin:/bin", + REA_LOG_LEVEL: "silent", + }, + stderr: "pipe", + }); + let failure; + try { + await client.connect(transport); + const version = client.getServerVersion(); + assert.ok(version); + if (firstVersion !== undefined) { + assert.notEqual(version, firstVersion); + assert.equal(version.name, firstVersion.name); + assert.equal(version.version, firstVersion.version); + } + firstVersion = version; + if (mode === "concurrent") + await Promise.all([status(client), status(client)]); + else await status(client); + } catch (cause) { + failure = cause; + throw cause; + } finally { + await closeVerifierResources( + [() => client.close(), () => transport.close()], + failure, + ); + } + } +} + +let error; +try { + if (mode === "concurrent" || mode === "reconnect") + await standaloneConnections(); + else + await artifact.withBundleResourcesMcp(target, async (client) => { + if (mode === "reuse") await Promise.all([status(client), status(client)]); + if (mode === "override") + await artifact.artifactMcpCall( + client, + { name: "binary_session", arguments: {} }, + { + toolDefinition: { + name: "binary_session", + inputSchema: { type: "object" }, + outputSchema: { type: "object" }, + }, + }, + ); + if (mode === "request-options") + await artifact.artifactMcpCall( + client, + { name: "binary_session", arguments: {} }, + { timeout: 12_345 }, + ); + }); +} catch (cause) { + error = { + name: cause instanceof Error ? cause.name : "unknown", + message: cause instanceof Error ? cause.message : String(cause), + code: cause?.code ?? null, + }; +} +process.stdout.write( + JSON.stringify({ + accepted: error === undefined, + catalogRequests, + corruptedReplies, + toolCalls, + toolTimeouts, + error: error ?? null, + }) + "\n", +);