diff --git a/README.md b/README.md index 4a93656..48d3d65 100644 --- a/README.md +++ b/README.md @@ -168,7 +168,7 @@ export default function transform({ operation, response }) { ## Observability -`/metrics` serves Prometheus text format: `mcp_openapi_tool_calls_total`, `_failed_total`, `_cancelled_total`, `_in_flight`, `_by_status_total{status}`, `mcp_openapi_retries_total`, `mcp_openapi_tool_call_latency_avg_ms`, and a latency histogram `mcp_openapi_tool_call_latency_ms_bucket`. Tool call start/completion and retry events are also emitted as MCP logging notifications. +`/metrics` serves Prometheus text format: `mcp_openapi_build_info{version}`, `mcp_openapi_uptime_seconds`, `mcp_openapi_tool_calls_total`, `_failed_total`, `_cancelled_total`, `_in_flight`, `_by_status_total{status}`, `mcp_openapi_retries_total`, `mcp_openapi_tool_call_latency_avg_ms`, and a latency histogram `mcp_openapi_tool_call_latency_ms_bucket`. Tool call start/completion and retry events are also emitted as MCP logging notifications. ## Library usage diff --git a/src/compile-cache.ts b/src/compile-cache.ts index 7ddbdd5..793e021 100644 --- a/src/compile-cache.ts +++ b/src/compile-cache.ts @@ -61,9 +61,14 @@ export async function compileDocumentWithCache( operations: [...operations.values()] }; - const abs = resolve(cachePath); - await mkdir(dirname(abs), { recursive: true }); - await writeFile(abs, JSON.stringify(entry), "utf8"); + // The cache is an optimization; a spec that cannot be cached must still serve. + try { + const abs = resolve(cachePath); + await mkdir(dirname(abs), { recursive: true }); + await writeFile(abs, JSON.stringify(entry), "utf8"); + } catch (error) { + process.stderr.write(`Compile cache write skipped: ${error instanceof Error ? error.message : String(error)}\n`); + } return operations; } diff --git a/src/compiler.ts b/src/compiler.ts index 91fa4af..eca0a84 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -343,7 +343,7 @@ function buildInputSchema(parameters: ParameterSpec[], bodySchema?: JsonSchema): continue; } - const groupProperties: Record = {}; + const groupProperties: Record = Object.create(null); const groupRequired: string[] = []; for (const param of items) { groupProperties[param.name] = param.schema ?? { type: "string" }; @@ -537,43 +537,58 @@ function pickContentWithSchema(content: Record): { contentType: } function normalizeJsonSchema(schema: JsonSchema): JsonSchema { - return normalizeNode(schema, new WeakMap()) as JsonSchema; + return normalizeNode(schema, new WeakMap(), new WeakSet()) as JsonSchema; } -function normalizeNode(node: T, seen: WeakMap): T { +function normalizeNode(node: T, seen: WeakMap, ancestors: WeakSet): T { if (!isObject(node)) { return node; } + // A node that contains itself (recursive $ref after dereferencing) cannot be + // JSON-serialized or compiled by AJV; cut the back-reference to the + // permissive empty schema. Non-cyclic reuse of shared subschemas is kept. + if (ancestors.has(node)) { + return {} as T; + } + if (seen.has(node)) { return seen.get(node) as T; } - if (Array.isArray(node)) { - const arr: unknown[] = []; - seen.set(node, arr); - for (const item of node) { - arr.push(normalizeNode(item, seen)); + ancestors.add(node); + try { + if (Array.isArray(node)) { + const arr: unknown[] = []; + for (const item of node) { + arr.push(normalizeNode(item, seen, ancestors)); + } + seen.set(node, arr); + return arr as T; } - return arr as T; - } - const source = node as Record; - const out: Record = {}; - seen.set(node, out); + const source = node as Record; + // Null prototype: schema keys are attacker-controlled and a "__proto__" + // key assigned to a plain object mutates its prototype instead of + // defining a property. + const out: Record = Object.create(null); - for (const [key, value] of Object.entries(source)) { - if (key === "nullable") { - continue; + for (const [key, value] of Object.entries(source)) { + if (key === "nullable") { + continue; + } + out[key] = normalizeNode(value, seen, ancestors); } - out[key] = normalizeNode(value, seen); - } - if (source.nullable === true) { - applyNullable(out); - } + if (source.nullable === true) { + applyNullable(out); + } - return out as T; + seen.set(node, out); + return out as T; + } finally { + ancestors.delete(node); + } } function applyNullable(schema: Record): void { diff --git a/src/lint.ts b/src/lint.ts index caf47be..8557c92 100644 --- a/src/lint.ts +++ b/src/lint.ts @@ -145,11 +145,16 @@ function collectBrokenInternalRefDiagnostics( value: unknown, root: Record, diagnostics: LintDiagnostic[], - location = "$" + location = "$", + visited: WeakSet = new WeakSet() ): void { if (Array.isArray(value)) { + if (visited.has(value)) { + return; + } + visited.add(value); for (let index = 0; index < value.length; index += 1) { - collectBrokenInternalRefDiagnostics(value[index], root, diagnostics, `${location}[${index}]`); + collectBrokenInternalRefDiagnostics(value[index], root, diagnostics, `${location}[${index}]`, visited); } return; } @@ -158,6 +163,13 @@ function collectBrokenInternalRefDiagnostics( return; } + // Dereferenced documents with recursive $refs are cyclic; each node is + // visited once. + if (visited.has(value)) { + return; + } + visited.add(value); + const record = value as Record; const ref = typeof record["$ref"] === "string" ? record["$ref"] : undefined; if (ref?.startsWith("#/") && resolveJsonPointer(root, ref) === undefined) { @@ -170,7 +182,7 @@ function collectBrokenInternalRefDiagnostics( } for (const [key, child] of Object.entries(record)) { - collectBrokenInternalRefDiagnostics(child, root, diagnostics, `${location}.${key}`); + collectBrokenInternalRefDiagnostics(child, root, diagnostics, `${location}.${key}`, visited); } } diff --git a/src/metrics.ts b/src/metrics.ts index dfce021..52df2b8 100644 --- a/src/metrics.ts +++ b/src/metrics.ts @@ -43,9 +43,20 @@ export function observeLatency(ms: number): void { metrics.latencyBuckets.le_inf += 1; } +let buildVersion = "0.0.0"; +const startedAtMs = Date.now(); + +export function setBuildInfo(version: string): void { + buildVersion = version; +} + export function renderPrometheus(): string { const avgLatency = metrics.toolCallsTotal > 0 ? metrics.toolCallLatencyMsTotal / metrics.toolCallsTotal : 0; return [ + "# TYPE mcp_openapi_build_info gauge", + `mcp_openapi_build_info{version="${buildVersion}"} 1`, + "# TYPE mcp_openapi_uptime_seconds gauge", + `mcp_openapi_uptime_seconds ${Math.floor((Date.now() - startedAtMs) / 1000)}`, "# TYPE mcp_openapi_tool_calls_total counter", `mcp_openapi_tool_calls_total ${metrics.toolCallsTotal}`, "# TYPE mcp_openapi_tool_calls_failed_total counter", diff --git a/src/server.ts b/src/server.ts index fe96d7a..6469e0b 100644 --- a/src/server.ts +++ b/src/server.ts @@ -34,7 +34,7 @@ import { import { executeOperation } from "./http.js"; import { STREAMABLE_TEST_HTML, SSE_TEST_HTML } from "./html.js"; import { paginateWithCursor } from "./pagination.js"; -import { observeLatency, observeStatus, renderPrometheus, metrics } from "./metrics.js"; +import { observeLatency, observeStatus, renderPrometheus, setBuildInfo, metrics } from "./metrics.js"; import type { CompileOptions, OperationModel, RuntimeOptions } from "./types.js"; import { zodFromJsonSchema } from "./zod-schema.js"; import { compileDocumentWithCache } from "./compile-cache.js"; @@ -128,6 +128,7 @@ let inFlightCalls = 0; let responseTransform: ((ctx: { operation: OperationModel; response: { body: unknown; status: number } }) => unknown | Promise) | undefined; async function main(): Promise { + setBuildInfo(PKG_VERSION); const cli = parseArgs(process.argv.slice(2)); if (cli.command === "init") { @@ -214,7 +215,7 @@ function createMcpServer(state: RuntimeState, cli: CliOptions): Server { for (const spec of state.specDocs) { if (uri === `openapi://${spec.name}/spec`) { return { - contents: [{ uri, mimeType: "application/json", text: JSON.stringify(spec.doc, null, 2) }] + contents: [{ uri, mimeType: "application/json", text: safeJsonStringify(spec.doc, 2) }] }; } if (uri === `openapi://${spec.name}/tools`) { @@ -1510,6 +1511,29 @@ function isObject(value: unknown): value is object { return value !== null && typeof value === "object"; } +// Dereferenced documents with recursive $refs contain object cycles; +// back-references are rendered as the string "[Circular]". +function safeJsonStringify(value: unknown, indent?: number): string { + const ancestors: object[] = []; + return JSON.stringify( + value, + function (this: unknown, _key: string, val: unknown) { + if (typeof val !== "object" || val === null) { + return val; + } + while (ancestors.length > 0 && ancestors[ancestors.length - 1] !== this) { + ancestors.pop(); + } + if (ancestors.includes(val)) { + return "[Circular]"; + } + ancestors.push(val); + return val; + }, + indent + ); +} + main().catch((error) => { process.stderr.write(`Fatal: ${error instanceof Error ? error.message : String(error)}\n`); process.exit(1); diff --git a/src/zod-schema.ts b/src/zod-schema.ts index cc3d166..d75d8df 100644 --- a/src/zod-schema.ts +++ b/src/zod-schema.ts @@ -102,7 +102,9 @@ function convertSchema(schema: unknown, seen: WeakMap): ZodT const props = isObject(s.properties) ? (s.properties as Record) : {}; const required = Array.isArray(s.required) ? new Set(s.required.filter((x): x is string => typeof x === "string")) : new Set(); - const shape: Record = {}; + // Null prototype: a "__proto__" property name assigned to a plain object + // would replace the shape's prototype and silently skip validation. + const shape: Record = Object.create(null); for (const [key, value] of Object.entries(props)) { const inner = convertSchema(value, seen); shape[key] = required.has(key) ? inner : inner.optional(); diff --git a/test/adversarial.test.ts b/test/adversarial.test.ts new file mode 100644 index 0000000..3a02daa --- /dev/null +++ b/test/adversarial.test.ts @@ -0,0 +1,156 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { mkdtemp, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; +import { ReadResourceResultSchema } from "@modelcontextprotocol/sdk/types.js"; +import { compileOperations } from "../src/compiler.js"; +import { loadOpenApiDocument } from "../src/openapi.js"; +import { zodFromJsonSchema } from "../src/zod-schema.js"; +import { renderPrometheus, setBuildInfo } from "../src/metrics.js"; + +const tsxCli = resolve("node_modules/tsx/dist/cli.mjs"); + +function cyclicSpec(): Record { + return { + openapi: "3.0.3", + info: { title: "Cyclic API", version: "1.0.0" }, + servers: [{ url: "https://cyclic.invalid" }], + paths: { + "/nodes": { + get: { + operationId: "listNodes", + responses: { + "200": { + description: "ok", + content: { + "application/json": { schema: { $ref: "#/components/schemas/Node" } } + } + } + } + } + } + }, + components: { + schemas: { + Node: { + type: "object", + properties: { + id: { type: "string" }, + child: { $ref: "#/components/schemas/Node" } + } + } + } + } + }; +} + +test("recursive $ref specs compile to serializable, validatable operations", async () => { + const dir = await mkdtemp(join(tmpdir(), "mcp-openapi-cyclic-")); + const specPath = join(dir, "cyclic.json"); + await writeFile(specPath, JSON.stringify(cyclicSpec()), "utf8"); + + const doc = await loadOpenApiDocument(specPath); + const operations = compileOperations(doc); + const op = operations.get("listNodes"); + assert.ok(op); + + // The compiled model must be cycle-free: cacheable and schema-compilable. + const serialized = JSON.stringify(op); + assert.ok(serialized.length > 0); + assert.ok(op.outputSchema); + const validator = zodFromJsonSchema(op.outputSchema); + const parsed = validator.safeParse({ id: "a", child: { id: "b", child: {} } }); + assert.equal(parsed.success, true); +}); + +test("server startup succeeds on a recursive spec (--validate-spec)", async () => { + const dir = await mkdtemp(join(tmpdir(), "mcp-openapi-cyclic-cli-")); + const specPath = join(dir, "cyclic.json"); + await writeFile(specPath, JSON.stringify(cyclicSpec()), "utf8"); + + const result = spawnSync( + process.execPath, + [tsxCli, "src/server.ts", "--spec", specPath, "--cache-path", join(dir, "cache.json"), "--validate-spec"], + { cwd: process.cwd(), encoding: "utf8" } + ); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /Compiled 1 tools/); +}); + +test("spec resource for a recursive spec renders with [Circular] markers", async () => { + const dir = await mkdtemp(join(tmpdir(), "mcp-openapi-cyclic-res-")); + const specPath = join(dir, "cyclic.json"); + await writeFile(specPath, JSON.stringify(cyclicSpec()), "utf8"); + + const client = new Client({ name: "adversarial-test", version: "0.1.0" }, { capabilities: {} }); + const transport = new StdioClientTransport({ + command: process.execPath, + args: ["dist/server.js", "--spec", specPath, "--cache-path", join(dir, "cache.json")], + cwd: process.cwd(), + stderr: "pipe" + }); + + try { + await client.connect(transport); + const result = await client.request( + { method: "resources/read", params: { uri: "openapi://cyclic/spec" } }, + ReadResourceResultSchema + ); + const text = String(result.contents[0]?.text); + const doc = JSON.parse(text) as Record; + assert.equal(typeof doc.openapi, "string"); + assert.ok(text.includes("[Circular]")); + } finally { + await transport.close(); + } +}); + +test("__proto__ schema properties are validated, not silently dropped", () => { + const schema = { + type: "object", + additionalProperties: false, + properties: { + ["__proto__"]: { type: "string" }, + ok: { type: "boolean" } + } + }; + + const validator = zodFromJsonSchema(schema); + + const good = validator.safeParse(JSON.parse('{"__proto__": "s", "ok": true}')); + assert.equal(good.success, true); + + const bad = validator.safeParse(JSON.parse('{"__proto__": 5, "ok": true}')); + assert.equal(bad.success, false, "a __proto__ property violating its schema must fail validation"); + + assert.equal(({} as Record).polluted, undefined); + assert.equal(Object.prototype.hasOwnProperty.call(Object.prototype, "polluted"), false); +}); + +test("duplicate operationIds within a spec get numeric suffixes", () => { + const doc = { + openapi: "3.0.3", + info: { title: "Dup", version: "1.0.0" }, + servers: [{ url: "https://dup.invalid" }], + paths: { + "/a": { get: { operationId: "getThing", responses: { "200": { description: "ok" } } } }, + "/b": { get: { operationId: "getThing", responses: { "200": { description: "ok" } } } } + } + }; + + const operations = compileOperations(doc as Record); + assert.ok(operations.has("getThing")); + assert.ok(operations.has("getThing_2")); +}); + +test("metrics expose build_info and uptime", () => { + setBuildInfo("9.9.9-test"); + const rendered = renderPrometheus(); + assert.match(rendered, /mcp_openapi_build_info\{version="9\.9\.9-test"\} 1/); + assert.match(rendered, /mcp_openapi_uptime_seconds \d+/); +});