[cliffhall]

Closes #2878

The v1 fix for GHSA-2fw8-7jmw-r937, developed and reviewed in the advisory's private fork (inspector-GHSA-2fw8-7jmw-r937#1) and landed here as an ordinary PR. The commit is the fork branch's, unchanged. The v2 fix is #2877.

This is the v1-line fix. Server-supplied pattern and patternProperties regexes are no longer evaluated, and the UI and CLI say so visibly.

Problem

A tool's outputSchema and an elicitation's requestedSchema both come from the server. Ajv compiled their regexes with the native backtracking RegExp, so a pattern like ^(a+)+$, checked against a near-match string, takes exponential time. Three v1 paths reach it:

• the web client's tool-result validation (schemaUtils.validateToolOutput)
• the elicitation form submit (ElicitationRequest)
• the CLI: client.callTool makes the SDK validate structuredContent itself

Approach: don't evaluate them

utils/stripSchemaPatterns.ts returns a copy of the schema with every pattern removed. It walks the nested subschemas: properties, items (both forms), additionalProperties, allOf/anyOf/oneOf/not, $defs/definitions, dependencies, propertyNames, if/then/else, and so on. It never mutates the original, because the UI displays that object.

• patternProperties is dropped together with that object's additionalProperties. Which keys count as "additional" depends on the regexes. Dropping only patternProperties would make additionalProperties: false falsely reject keys the server declared, so neither keyword is evaluated. Declared properties still are.
• Every other keyword still validates and still reports mismatches: type, required, enum, minLength and the rest.
• When anything was stripped, a note says so. The text is "N regex pattern(s) in this schema were not evaluated". It appears with the tool-result validation status and on the elicitation form. In the CLI it is appended to a validation error, or written to stderr when validation passes, which keeps stdout's JSON clean.
• CLI wiring: RegexFreeJsonSchemaValidator is passed as jsonSchemaValidator to new Client(...). It strips the schema, then delegates to the SDK's own AjvJsonSchemaValidator (@modelcontextprotocol/sdk/validation/ajv).
• No dependency, manifest or lockfile changes.

Trade-off: on v1, a value that violates a server's pattern is no longer reported as a mismatch. The note makes the gap visible instead of implying the check passed. v1 is security-fix-only, so a small removal beats shipping a regex engine.

Tests

• stripSchemaPatterns.test.ts:
  • every nested location is stripped, and a property named pattern survives
  • patternProperties and additionalProperties are dropped together
  • the original object is unchanged
• schemaUtils.test.ts:
  • a 10k-char near-match against ^(a+)+$ returns isValid: true with the note
  • minLength, enum, required and type still fail
  • the tool's schema is not mutated
• ToolsTab.test.tsx: the note renders with the result.
• ElicitationRequest.test.tsx: the note renders, a near-match submits, and minLength is still enforced.
• cli/__tests__/regexFreeSchemaValidator.test.ts:
  • the same assertions against the wrapper
  • parity with the SDK validator when the schema has no regexes
  • an end-to-end Client.callTool over InMemoryTransport that returns the near-match result and emits the note

Results

• npm run lint: passed
• npm test: 36 suites, 615 tests, all passing
• npm run build: passed
• npm run test-cli: 5 files, 90 tests, all passing

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Server-provided regexes remain executable through unsanitized reference targets and the form renderer, leaving the denial-of-service risk unresolved.

5 open issues

• Pass the regex-free schema to DynamicJsonForm · New
• Sanitize or reject references to regex-bearing schema locations · New
• Preserve additionalProperties when patternProperties is empty · New
• Preserve or rewrite references before deleting patternProperties · New
• Avoid changing validation semantics in regex-dependent compositions · New

What changed in this PR

This PR implements the v1 mitigation for GHSA-2fw8-7jmw-r937 by stripping server-supplied schema regexes before validation and reporting skipped checks. Unlike the v2 fix, it avoids adding a bounded-time regex engine.

Changes:

• Adds schema-copying helpers and integrates them into web and CLI validation.
• Displays skipped-pattern notices in tool results, elicitation forms, and CLI diagnostics.
• Adds regression tests for malicious patterns, remaining constraints, schema preservation, and SDK integration.

File	Description
client/​src/​utils/​stripSchemaPatterns.ts	Adds schema sanitization and notice formatting.
client/​src/​utils/​schemaUtils.ts	Sanitizes tool schemas and tracks skipped patterns.
client/​src/​utils/​__tests__/​stripSchemaPatterns.test.ts	Tests traversal, keyword removal, and immutability.
client/​src/​utils/​__tests__/​schemaUtils.test.ts	Tests skipped patterns and remaining validation.
client/​src/​components/​ToolResults.tsx	Displays skipped-pattern notices with results.
client/​src/​components/​ElicitationRequest.tsx	Sanitizes submit validation and adds a notice.
client/​src/​components/​__tests__/​ToolsTab.test.tsx	Tests the tool-result notice.
client/​src/​components/​__tests__/​ElicitationRequest.test.tsx	Tests elicitation notices and validation behavior.
cli/​src/​utils/​stripSchemaPatterns.ts	Adds the CLI schema sanitizer.
cli/​src/​utils/​regexFreeSchemaValidator.ts	Wraps SDK validation with sanitization and notices.
cli/​src/​index.ts	Registers the validator and sends notices to stderr.
cli/​__tests__/​regexFreeSchemaValidator.test.ts	Tests the wrapper and end-to-end tool calls.

_{Review effort: Balanced}

[cliffhall]

Copilot round 1 (review 5775588134): five findings, all fixed in 89300ee and answered in each thread. The client and cli copies of stripSchemaPatterns.ts are still byte-identical.

1. Form renderer: DynamicJsonForm now gets the regex-free schema. The original is kept only for the schema display.

2. $ref targets: a $ref is kept only when it is a local #/… pointer to a schema position the walk sanitized, and it is rewritten to that position. These are removed and counted as skipped checks:

   • refs into non-schema locations (#/default)
   • remote or plain-name refs
   • any ref in a document with an $id
   • $dynamicRef / $recursiveRef

   A property named pattern, and a pattern inside const / enum / default values, are left alone.

3. Empty patternProperties: kept as is, along with its additionalProperties. additionalProperties is dropped, and counted, only when the map had keys.

4. Removed subschemas: they are sanitized and moved under generated definitions names, and refs into them are rewritten, so the result always compiles.

5. No false failures: a not, oneOf, if (with its then / else), contains (with minContains / maxContains) or unevaluated* whose subschema lost a constraint is removed and counted as a skipped check. Independent siblings are kept. The note now reports skipped checks too.

Checks: prettier-check passed. Client lint passed and its tests pass 634/634, cli tests pass 95/95, and the root build passed. The client tests compile against its Ajv 6, and the cli tests against the SDK's Ajv 8.

[Copilot]

🟡 Changes recommended

Repeated per-child container cloning makes schema sanitization quadratic, blocking the UI thread and CLI event loop for large schemas.

1 open issue

• Avoid quadratic schema copies during sanitization · New

5 resolved since last review

• Sanitize or reject references to regex-bearing schema locations
• Pass the regex-free schema to DynamicJsonForm
• Avoid changing validation semantics in regex-dependent compositions
• Preserve or rewrite references before deleting patternProperties
• Preserve additionalProperties when patternProperties is empty

_{Review effort: Balanced}

[cliffhall]

Copilot round 2 (review 5775855544): one finding, fixed in 248ba5f (reply). The client and cli copies are still byte-identical.

• Container copying: each node now copies its map or array once, then assigns its sanitized children into that copy.
• Two other quadratic spots, also fixed:
  • Taint propagation now spreads from a work list along reversed parent and referrer edges, and each position enters it once.
  • Positions are now numbered instead of keyed by path strings.
• Rewritten $ref pointers are built once per position and reused.
• The elicitation form re-sanitized on every render. It now uses useMemo keyed on the request.
• Tests: a 10,000-property schema, and width/depth scaling tests. Each requires 4× the input to cost under 10× the time, which a quadratic helper fails.

Checks: prettier-check passed. Client lint passed and its tests pass 636/636, cli tests pass 96/96, and the root build passed.

[Copilot]

🟡 Changes recommended

The schema transform can cause relocated relative IDs to collide and activate constraints previously ignored alongside $ref, breaking validation and potentially blocking web submissions or CLI operations.

2 open issues

• Relocated relative $id values collide across parent scopes · New
• Removing $ref activates sibling constraints ignored by Ajv 6 · New

1 resolved since last review

• Avoid quadratic schema copies during sanitization

_{Review effort: Balanced}

[cliffhall]

Copilot round 3 (review 5776046271): two findings, fixed in fe7ffd9 and answered in each thread. Instead of more rewriting, the helper now holds one invariant, stated in its header: the output schema accepts everything the input accepts, or validation is skipped and reported.

• $id collisions: a schema that declares $id, id, $anchor, $dynamicAnchor or $recursiveAnchor anywhere in a schema position is never compiled. It is reported as not validated: the web client returns valid with the note, and the CLI writes the note to stderr. Nothing is relocated when identifiers are present.
• $ref siblings: a $ref that cannot be kept now turns its whole node, siblings included, into {}, counted as one skipped check. That matches Ajv 6's ignore-siblings behavior, and under Ajv 8 it only loosens. On Ajv 6.15 the comment's exact example already rejects "s" before the transform, so the client test asserts that the transformed schema accepts every probed value.
• Corpus tests: a corpus test in each copy (Ajv 6 in client, Ajv 8 in cli) covers the examples from all three rounds. Every value the original accepts is still accepted.

Checks: prettier-check passed. Client lint passed and its tests pass 642/642, cli tests pass 99/99, and the root build passed.

[Copilot]

🟡 Changes recommended

The validator-only hasOutputSchema() check suppresses validation-skipped notes for identifier-bearing schemas, leaving missing structured content unreported.

1 open issue

• Recognize cached schema notes when checking for output schemas · New

2 resolved since last review

• Removing $ref activates sibling constraints ignored by Ajv 6
• Relocated relative $id values collide across parent scopes

_{Review effort: Balanced}

[cliffhall]

Copilot round 4 (review 5776208596): one finding, fixed in 2bc33dc (reply). hasOutputSchema() now also checks the note map, so a tool whose schema was skipped shows the "not validated" note and reports missing structured content. Its label now reads "Not validated against output schema" instead of "✓ Valid". New ToolsTab rendering tests cover both cases. The CLI has no equivalent gate, because the SDK Client checks for missing structured content itself.

Checks: prettier-check passed. Client lint passed and its tests pass 644/644, cli tests pass 99/99, and the root build passed.

[Copilot]

🔵 Needs a closer look

Human assessment is needed because a critical reference-resolution ambiguity in both schema utilities can silently select the wrong schema and reject valid values.

No open issues 🎉

1 resolved since last review

• Recognize cached schema notes when checking for output schemas

_{Review effort: Balanced}

[cliffhall]

Copilot round 5 (review 5776329825): no inline comments. Its headline ("a reference-resolution ambiguity … can silently select the wrong schema") was real, and it is fixed in 002c413.

• Cause: an encoded / in a pointer. With #/definitions/x%2Fy in a schema holding both definitions["x/y"] and definitions.x.y, Ajv 6 splits before decoding (key x/y), while Ajv 8 decodes first (x → y). Our resolver followed Ajv 6, so under the CLI's Ajv 8 the rewritten ref picked a different schema.
• Fix: a ref containing %2F, %00 or a raw NUL is now treated as unresolvable, so its node becomes {}. That only loosens, and it is counted as a skipped check.
• Differential check: about 40 pointer cases (~0/~1, percent-encoding, #, array indices, keys with / or ~, definitions and $defs together, generated-name collisions, two paths to one pointer), comparing the original and transformed schemas under client Ajv 6 and SDK Ajv 8. This was the only case where the transform was stricter.

Checks: prettier-check passed. Client lint passed and its tests pass 645/645, cli tests pass 101/101, and the root build passed.

[Copilot]

🔵 Needs a closer look

Security-sensitive reference and composition rewrites across browser Ajv 6 and SDK validators require maintainer review and independent runtime validation to confirm regex exclusion and preserved schema acceptance.

No open issues 🎉

_{Review effort: Balanced}