From f86206535f8ec71014781f9d432a84b01e183e5d Mon Sep 17 00:00:00 2001 From: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com> Date: Wed, 21 Oct 2026 23:28:12 +0800 Subject: [PATCH] fix(ci): validate artifact MCP replies against named schemas (#6021) Bind artifact lifecycle and analysis calls to their advertised named SDK output schemas, sharing discovery per connection and rebinding on reconnect. Reject schema overrides and ambiguous catalogs, preserve request options, and retain primary failures while attempting all owned cleanup. Add real-stdio boundary regressions for malformed output, discovery reuse, reconnection, schema ambiguity, override rejection and timeout preservation. Assisted-by: Reimu (gensokyo-skills:reimu-incident-triage) Assisted-by: Kogasa (gensokyo-skills:kogasa-surprise-testing) --- docs/testing.md | 16 ++ scripts/lib/artifact-e2e.mjs | 79 ++++++- scripts/verify-native-calls.mjs | 4 +- scripts/verify/apple/asset-catalog.mjs | 3 +- scripts/verify/apple/keyed-archive.mjs | 3 +- scripts/verify/apple/macho-occurrences.mjs | 3 +- scripts/verify/apple/macos-dylib-e2e.mjs | 3 +- scripts/verify/apple/native-provenance.mjs | 25 +-- .../process/artifactMcpVerifierOutput.test.ts | 126 +++++++++++ tests/fixtures/artifactMcpVerifierProbe.mjs | 209 ++++++++++++++++++ 10 files changed, 447 insertions(+), 24 deletions(-) create mode 100644 tests/boundary/process/artifactMcpVerifierOutput.test.ts create mode 100644 tests/fixtures/artifactMcpVerifierProbe.mjs 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", +);