feat(core,server,client): implement SEP-2106 (tool schemas conform to JSON Schema 2020-12)#2249
feat(core,server,client): implement SEP-2106 (tool schemas conform to JSON Schema 2020-12)#2249mattzcarey wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: a277433 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
felixweinberger
left a comment
felixweinberger
left a comment
There was a problem hiding this comment.
Awesome thanks for working on this!
Quick note before a deeper read - there should be a currently failing conformance scenario in expected-failures.yaml here:
Can we remove that expected failure and confirm conformance passes with this change?
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // SEP-2106: reject non-local $refs (SSRF) and over-budget schemas (composition DoS) before compiling. | ||
| assertSchemaSafeToCompile(schema); | ||
|
|
There was a problem hiding this comment.
🔴 The new assertSchemaSafeToCompile() guard throws inside getValidator(), and Client.listTools() → cacheToolMetadata() eagerly compiles every advertised tool's outputSchema with no per-tool error handling — so a single tool advertising a non-same-document $ref/$dynamicRef or an over-budget schema makes the entire listTools() reject, leaving the client unable to list or call any tool from that server (and listChanged auto-refresh surfaces the error on every notification). Consider catching per-tool in cacheToolMetadata() or deferring the guard to validation/callTool time so the failure is scoped to the offending tool; the same applies to CfWorkerJsonSchemaValidator.getValidator.
Extended reasoning...
What the bug is. This PR adds assertSchemaSafeToCompile(schema) to the top of both AjvJsonSchemaValidator.getValidator() (packages/core/src/validators/ajvProvider.ts:75-77) and CfWorkerJsonSchemaValidator.getValidator(). The guard throws synchronously for (a) any $ref/$dynamicRef that does not begin with # and (b) schemas exceeding the depth-64 / 10k-subschema bounds. The guard itself is a sensible security measure — the problem is where the throw surfaces. On the client, cacheToolMetadata() (packages/client/src/client/client.ts:944-954) iterates every advertised tool and calls this._jsonSchemaValidator.getValidator(tool.outputSchema) eagerly, with no try/catch, and it is invoked synchronously from listTools() (client.ts:~1004), also with no try/catch. Any throw from the new guard therefore rejects the whole listTools() promise.\n\nConcrete walk-through. 1) A server advertises two tools: good_tool (plain object outputSchema) and weird_tool whose outputSchema contains { "$ref": "https://example.com/schemas/forecast.json" } — a spec-valid JSON Schema 2020-12 schema, which SEP-2106 (this PR) explicitly allows servers to advertise. 2) The client calls client.listTools(). 3) The tools/list request succeeds and cacheToolMetadata(result.tools) runs. 4) For weird_tool, getValidator() calls assertSchemaSafeToCompile(), which throws JSON Schema contains a non-local "$ref" .... 5) The throw propagates out of cacheToolMetadata() and out of listTools(), so the caller gets a rejection and never sees good_tool either. 6) Because callTool() relies on the validator cache populated by listTools(), the application typically cannot use any tool from that server. 7) If listChanged.tools.onChanged auto-refresh is configured, every notifications/tools/list_changed re-triggers the same failure (the fetcher's catch just forwards the error to onChanged).\n\nWhy this is a regression introduced/amplified by this PR, not pre-existing. Pre-PR, the CfWorker provider's Validator constructor does not eagerly dereference external refs, so listTools() succeeded and only that one tool's callTool validation could fail — post-PR the whole tools/list path is poisoned on the browser/workerd default. The depth/subschema-count throws are entirely new for both providers: a deeply nested but legitimate schema that previously compiled fine now breaks listing every tool. (On the AJV/Node path an unresolvable external $ref did already throw at compile() time pre-PR, so that one sub-case is partially pre-existing — but the PR adds the new throw sites, extends the behavior to CfWorker, and SEP-2106 makes $ref-bearing output schemas far more likely to appear in the wild, so the blast radius grows materially.) Nothing in the PR's tests covers the client-side listTools() behavior when a server advertises such a schema, and the PR description frames the guard as rejecting that schema, not as disabling the whole server.\n\nImpact. A single misbehaving (or merely externally-referencing) tool definition acts as a denial-of-service against the client's view of the entire server: listTools() rejects, no validators are cached, and callTool() for unrelated, perfectly valid tools is effectively unusable. For hosts that aggregate many third-party MCP servers, one bad tool schema knocks out a whole server's toolset rather than just the offending tool.\n\nHow to fix. Keep the guard, but scope its failure to the offending tool. Either (1) wrap the per-tool getValidator() call in cacheToolMetadata() in a try/catch — skip caching a validator for that tool (or cache a validator that always fails) and optionally log/annotate, so listTools() still returns and other tools keep working, and the offending tool fails at callTool/validation time with a clear error; or (2) defer assertSchemaSafeToCompile to validation time so the throw happens inside the per-tool validator path that callTool() already wraps in error handling. The same per-tool isolation should apply to both built-in providers. A test asserting that listTools() succeeds when one tool advertises a non-local $ref outputSchema would lock the behavior in.
… schemas Tools' inputSchema/outputSchema now conform to full JSON Schema 2020-12 and structuredContent may be any JSON value (#2192). - outputSchema accepts any 2020-12 schema (arrays/primitives/objects/compositions); inputSchema keeps `type: "object"` but allows all 2020-12 keywords. Updated the generated spec types, the zod schemas, and standardSchemaToJsonSchema (output path no longer forces `type: "object"`). - structuredContent widens from `{ [key: string]: unknown }` to `unknown` (source-breaking for typed consumers). - Stronger typing: new CallToolResultWithStructuredContent<T>; client.callTool<T>() generic (cast-free via overload); registerTool type-checks a handler's structuredContent against its outputSchema's inferred output. - Fix falsy-structuredContent bug (=== undefined) in server + client. - Server auto-emits a serialized TextContent fallback for non-object structuredContent (pre-SEP client interop). - Security: validators reject non-same-document $ref/$dynamicRef (SSRF) and schemas exceeding depth/subschema bounds (composition DoS) via schemaBounds. Adds unit + integration tests, migration docs (migration.md + migration-SKILL.md), and a changeset.
Register the SEP-2106 conformance scenario in the everythingClient and remove it from expected-failures.yaml. The client already blocks non-local $ref dereferencing via assertSchemaSafeToCompile.
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
mattzcarey
force-pushed
the
feat/sep-2106-json-schema-2020-12
branch
from
6b451c5 to
a277433
Compare
2 participants
Implements SEP-2106 — tool
inputSchema/outputSchemaconform to JSON Schema 2020-12, andstructuredContentmay be any JSON value.Closes #2192.
What changed
Schema surface
inputSchemakeepstype: "object"at the root but now accepts any JSON Schema 2020-12 keyword (composition, conditional,$ref/$defs/$anchor, …).outputSchemamay now be any valid JSON Schema 2020-12 — objects, arrays, primitives, or compositions (was restricted totype: "object").structuredContentwidens from{ [key: string]: unknown }tounknown(source-breaking for typed consumers).spec.types.ts(surgically, matching the upstream regen for these fields), the hand-maintained zod inschemas.ts, andstandardSchemaToJsonSchema(theoutputpath no longer forcestype: "object";inputand prompt args stay object-only).Stronger typing (cast-free)
CallToolResultWithStructuredContent<T>type.client.callTool<T = JSONValue>()is generic so callers get a precisely typedstructuredContent. Implemented via a method overload so the body stays cast-free.McpServer.registerToolnow type-checks a handler's returnedstructuredContentagainst the tool'soutputSchemainferred output (ToolResultFor<Output>threaded throughToolCallback).Behavior
0/false/""as "no structured content"; now check=== undefined.structuredContentautomatically also emit a serializedTextContentblock so pre-SEP clients can fall back to the text content (object output, or results that already carry text, are untouched).Security
$ref/$dynamicRef(SSRF guard) and reject schemas exceeding depth / subschema-count bounds (composition-DoS guard), via the newschemaBounds.ts. CustomjsonSchemaValidatorimplementations are unaffected.Tests & docs
test/integration/test/sep2106.test.ts(array/primitive round-trips, falsy values, TextContent auto-inject, typedcallTool<T>) andpackages/core/test/validators/schemaBounds.test.ts; provider-level SSRF tests added tovalidators.test.ts.docs/migration.mdanddocs/migration-SKILL.md; minor changeset.Verification
typecheck:all✅ ·lint:all✅ ·build:all✅ ·sync:snippetsno-diff ✅ · core 566, client 367, server 68, integration 427 ✅.Note on generated types
spec.types.tscarries only the SEP-2106 field changes (so the spec↔SDK assignability test passes) rather than a fullfetch:spec-typesregen, which would pull unrelated drift owned by the dedicatedupdate-spec-typesbranch. The edits match that branch verbatim for these fields, so a later full regen merges cleanly.