Skip to content

fix(core): evaluate server-supplied schema patterns in bounded time (v2) - #2877

Merged
cliffhall merged 5 commits into
v2/mainfrom
v2/fix/2874-output-schema-regex-dos
Nov 4, 2026
Merged

cliffhall merged 5 commits into
v2/mainfrom
v2/fix/2874-output-schema-regex-dos

Conversation

@cliffhall

Copy link
Copy Markdown
Member

Closes #2874

Fix for GHSA-2fw8-7jmw-r937, developed and reviewed in the advisory's private fork (inspector-GHSA-2fw8-7jmw-r937#2) and landed here as an ordinary PR. The commits are the fork branch's, unchanged.

Fix for the v2 line: server-supplied patterns in tool output schemas and app-elicitation schemas are now evaluated in bounded time.

Problem

InspectorClient.callTool runs validateToolOutput on every result. App-rendered elicitations go through validateAppElicitResult. Both used the SDK's AjvJsonSchemaValidator, and Ajv compiles a schema's pattern / patternProperties with the native backtracking RegExp. The schema comes from the server. A pattern such as ^(a+)+$ checked against a near-match structuredContent string takes exponential time on the thread doing the validation. That is the UI thread in the web client and the event loop in the CLI, TUI and mcpdo, since all of them go through core/.

SDK validation. The SDK Client.callTool also validates structuredContent, using this._jsonSchemaValidator: the default Ajv in Node, the CfWorker validator in the browser. InspectorClient never reaches it:

  • the non-task path sends tools/call through client.request
  • the task path goes through ext-tasks, which does not call Client.callTool
  • no jsonSchemaValidator is passed to new Client

So the provider swapped here is the only place a server pattern is evaluated on these paths. Other SDK consumers that call Client.callTool are exposed in the SDK itself. That is an upstream matter, not changed here.

Approach: a linear-time engine behind Ajv's code.regExp hook

core/json/linearRegExp.ts is a Thompson-NFA ("Pike VM") matcher for the regular subset of ECMAScript regex syntax.

  • Bounded work. A search costs (input length + 1) × program size, checked before it runs.
  • Same answers as the native engine. Each single-character atom (literal, \d, \p{L}, [^a-z], .) is decided by the native engine against one code point, wrapped as ^(?:atom)$. That wrapper cannot backtrack, and it keeps exact ECMAScript character semantics, u-flag code-point handling included, without reimplementing them.
  • Pure JS. No dependency is added.

core/mcp/linearPatternValidator.ts is a jsonSchemaValidator. It reproduces the SDK provider exactly: per-$schema engines (Ajv2020 / Ajv2019 / draft-07), the same options, ajv-formats (the SDK's re-exported addFormats), the same $id reuse and the same errorsText. Its one difference is code.regExp → the linear engine.

  • The SDK provider can't be used with a custom engine: it accepts one Ajv instance and then drops its dialect dispatch.
  • A per-validation ledger caps the total pattern work at PATTERN_EVALUATION_BUDGET (2e7 instruction-visits). Many patterned strings in one payload therefore cannot add up to a stall either.
  • ajv is already a root dependency and already in all four bundler external lists, and the subpath imports ajv/dist/2019.js / ajv/dist/2020.js are covered by each list's subpath regex. No manifest changes.

Honest reporting. Lookaround, backreferences, a program past the size cap, and input that would exceed the budget cannot be evaluated. In those cases the result is valid: false, with a message naming the pattern and the reason, e.g. pattern "(x)\1" was not evaluated (backreference), so the value could not be verified against the schema. Real Ajv errors are still listed alongside. An unchecked pattern never reads as a pass. On the strict path it rejects like any other mismatch; on the skipOutputValidation (MCP Apps) path it surfaces as the non-fatal advisory.

Alternatives considered:

  • Static "unsafe shape" detection (reject nested quantifiers) has false positives on common patterns such as ^([a-z]+\.)+[a-z]+$. It also misses polynomial blowups: measured on V8, even unanchored a*b took ~3 s on a 100k-character input.
  • Capping input length alone does nothing for exponential patterns.
  • Moving validation to a worker with a timeout means three different isolation mechanisms across browser, CLI and TUI, and an async boundary inside synchronous validation.
  • re2 is a native dependency, which is a poor fit for an npx-installed package.

Tests

  • clients/web/src/test/core/json/linearRegExp.test.ts:
    • a differential corpus against RegExp.prototype.test, with and without u, covering every parser branch
    • the unsupported constructs
    • catastrophic shapes (^(a+)+$, ^(a|a)*$, ^(a|aa)+$, ^(a*)*$, ^(\w+\s?)+$, a*a*a*a*a*b) answered on 5,000-character near-matches; the bound is asserted, not timed
  • clients/web/src/test/core/mcp/linearPatternValidator.test.ts:
    • parity with AjvJsonSchemaValidator, results and error text
    • dialect dispatch, $id reuse, invalid pattern
    • ReDoS mismatch reported and a matching value accepted
    • patternProperties
    • unevaluated reported as invalid, alone and alongside real errors
    • budget cap and per-validation reset
    • validateToolOutput with the new provider
  • inspectorClient-raw-wire.test.ts: InspectorClient.callTool with a ^(a+)+$ output schema and a 10k near-match rejects with must match pattern instead of hanging.

Coverage: linearPatternValidator.ts 100/100/100/100. linearRegExp.ts ≥ 95 branches, and its unreachable defensive branches (input the native constructor has already rejected) carry justified v8 ignores.

npm run local:gate: green (exit 0).

Not in scope

  • ajv-formats' built-in format regexes are fixed, vetted patterns rather than server input, so they are untouched.
  • clients/web/src/utils/schemaUtils.ts still has a bare new Ajv() validateToolOutput. It is dead code: nothing imports it outside its own test, so it is untouched here.

🤖 Generated with Claude Code

A tool's outputSchema (and an app elicitation's requestedSchema) comes
from the server, and its `pattern` / `patternProperties` regexes were
run by Ajv on the native backtracking RegExp engine. A pattern with
nested or ambiguous repetition evaluated against a near-match value
takes exponential time on the thread doing the validation.

InspectorClient now validates those schemas with
LinearPatternJsonSchemaValidator: the SDK's Ajv configuration (same
per-dialect engines, options, formats and error text) with Ajv's
`code.regExp` hook routed to a linear-time matcher (core/json/
linearRegExp.ts, a Pike VM whose single-character atoms are decided by
the native engine against one code point). Matching costs input length
x program size, and the total per validation is capped. A pattern that
cannot be evaluated (lookaround, backreferences) or would exceed the
cap is reported as unverified, never as valid.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall cliffhall added the v2 Issues and PRs for v2 label Nov 4, 2026
@cliffhall
cliffhall requested a balanced review from Copilot November 4, 2026 20:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Counted empty regex repeats can trigger trillions of synchronous compilation iterations without reaching the program-size cap, freezing the validation thread before evaluation limits apply.

1 open issue
What changed in this PR

This PR addresses #2874 by replacing native regex evaluation for server-supplied tool-output and app-elicitation schemas with a bounded-time validator shared across Inspector clients.

Changes:

  • Adds a linear-time regex matcher with program-size and evaluation limits.
  • Integrates an Ajv provider that reports unsupported or over-budget patterns as unverified.
  • Adds parity, budget, and catastrophic-pattern regression tests.
File Description
core/​mcp/​toolOutputValidation.ts Updates provider documentation to describe bounded pattern checks.
core/​mcp/​linearPatternValidator.ts Adds dialect-aware validation and a shared pattern-work budget.
core/​mcp/​inspectorClient.ts Uses the new provider for tool outputs and app elicitations.
core/​json/​linearRegExp.ts Implements the regex parser, compiler, and bounded matcher.
clients/​web/​src/​test/​core/​mcp/​linearPatternValidator.test.ts Tests provider parity, reporting, and budget enforcement.
clients/​web/​src/​test/​core/​mcp/​inspectorClient-raw-wire.test.ts Adds a tool-call regression for a catastrophic pattern.
clients/​web/​src/​test/​core/​json/​linearRegExp.test.ts Tests regex compatibility, unsupported syntax, and matching limits.

Review effort: Balanced

Comment thread core/json/linearRegExp.ts Resolved
A counted repeat of a body that emits no instructions, such as
`(){1000000000000}` or `(a{0}){1000000000000}`, looped `min` times without
ever reaching the program-size cap, so compilation could stall before the
evaluation budget applied. Stop expanding once a copy emits nothing (further
copies are equivalent), and cap total AST node visits at MAX_COMPILE_STEPS so
no repeat can cost unbounded work regardless of what its body emits.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall

Copy link
Copy Markdown
Member Author

Copilot round 1 (review 5775546285): one finding, fixed in a3a4d0d (reply).

  • Counted repeat of an empty body: expansion now stops once one copy emits nothing, so (){1000000000000}, (a{0}){1000000000000}, ((){1000}){1000000000} and the {n,} / {n,m} forms compile at once to a program of at most 4 instructions that matches only the empty string.
  • Compile-work cap: MAX_COMPILE_STEPS (10 × MAX_PROGRAM_SIZE) now charges every AST visit, so no pattern can loop during compilation, whatever its body emits. The {0,N} path was already bounded by the program-size cap, and a test now pins it.
  • Tests: three new tests in linearRegExp.test.ts (109/109 pass). They assert program sizes and match results, not wall-clock time.
  • Gate: local:gate passed (exit 0). linearRegExp.ts is at 99.6 / 96.2 / 100 / 100 coverage.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Per-pattern compilation limits allow schemas with many patterns to consume excessive CPU and memory before evaluation, while quantified assertion groups are incorrectly rejected despite being valid.

1 open issue
1 previously missed issue

In code that hasn't changed since last review

Medium severity Allow quantifiers on groups containing assertions

core/​json/​linearRegExp.ts:147

Native RegExp permits quantifiers on groups containing assertions, such as (\b)?a, (^)*a, and ($){0}a; all three match "a". Because parseGroup() returns its body directly, this guard mistakes those groups for bare assertions and reports them as unsupported, causing the provider to reject valid values. Remove this guard—the initial native constructor already rejects genuinely invalid bare-assertion quantifiers—and add these cases to the differential corpus.

1 resolved since last review

Review effort: Balanced

Comment thread core/mcp/linearPatternValidator.ts Resolved
The per-pattern compile limits restarted for every pattern, so a schema of
many distinct near-limit patterns could build millions of instructions
before any value was validated. Charge every pattern a schema compiles to
one SCHEMA_COMPILE_BUDGET (node visits and program size); once it runs out
the remaining patterns are declined, and every validation against that
schema reports it could not be verified.

Also accept quantified groups around an assertion such as `(\b)?a`,
`(^)*a` and `($){0}a`, which the native engine accepts: a bare quantified
assertion is already a native syntax error, so the parser's guard only ever
rejected valid patterns. They are added to the differential corpus.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall

Copy link
Copy Markdown
Member Author

Copilot round 2 (review 5775802397): two findings, both fixed in 34c90ea.

  1. Per-pattern compile limits (reply). LinearPatternJsonSchemaValidator now creates one CompileBudget per schema compile, counting compile steps and total program size, and charges every pattern in that schema to it. The limit is SCHEMA_COMPILE_BUDGET, ten patterns' worth of the per-pattern limits.
    • Once the budget runs out, the remaining patterns are declined at no cost. Every validation against that schema then reports that it could not be verified, including an empty object and cached revalidations.
    • Tests: 1,000 distinct ^a{9000}b<i>$ patterns, a value reaching a declined pattern, a fresh budget for the next schema, and unit tests for the budget.
  2. Quantified assertion groups (the review body's "previously missed" item, which has no thread). The claim is correct: native RegExp rejects bare \b?a, ^*a and $*a, and accepts (\b)?a, (^)*a and ($){0}a, under both flag settings. The guard is removed, so these compile as ordinary zero-width repeats. The three cases and ^(\b|$)+$ are now in the differential corpus.

local:gate: the first run exited 1 on a timing test, scripts/lib/render-smoke.test.mjs ("a paint landing just under the deadline…"). This diff touches nothing under scripts/. The re-run exited 0.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Ajv’s nested-ID schema lookup can compile without budget or exhaustion tracking, bypassing schema-wide limits and allowing millions of instructions to be allocated synchronously.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread core/mcp/linearPatternValidator.ts Resolved
…dget

Ajv compiles a schema registered by $id, including a $defs entry of an
earlier schema, on its first getSchema lookup. That lookup ran with no
compile budget active, so it bypassed the schema-wide limit and the
over-budget tracking. Run the lookup and the compile under one tracked
budget, and make a budget mandatory at the regex hook: outside a tracked
entry point a provider-lifetime budget applies, so no path compiles a
pattern unbudgeted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall

Copy link
Copy Markdown
Member Author

Copilot round 3 (review 5776127719): one finding, fixed in 7d8b9ac (reply).

  • Lookup and compile share one budget. getValidator now runs the getSchema(id) lookup and the compile fallback inside one tracked() call, with one fresh SCHEMA_COMPILE_BUDGET and over-budget tracking. A schema compiled lazily on a nested-$id lookup is now charged like a direct compile.
  • A budget is always set where patterns are compiled. Outside a tracked call, a provider-lifetime budget applies, so no path into the regex hook runs unbudgeted. The provider uses no other Ajv entry point.
  • Test: "charges a schema compiled on lookup by a nested $id to the budget too". It fails on the previous code and passes now.
  • Gate: local:gate passed (exit 0).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed because large server-controlled patterns can cause synchronous allocations and stalls before compilation budgets are checked, creating a critical resource-exhaustion risk.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

Decline a pattern longer than MAX_PATTERN_LENGTH (8192) before the native
syntax check or the parser sees it, and charge every pattern's length to
the shared compile budget up front, so many long but cheap patterns add up
against the schema-wide limit and an exhausted budget skips the native
constructor too. The parser builds a constant number of AST nodes per
character, so the length charge also covers parsing.

Read quantifier braces with a sticky regex at the parser's position
instead of slicing the rest of the pattern at every `{`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall

Copy link
Copy Markdown
Member Author

Copilot round 4 (review 5776331150): no inline comments. Its headline (large patterns allocate before any budget is checked) is addressed in cbd467e.

  • One real superlinear spot, fixed. readQuantifier and the non-unicode literal-{ branch ran QUANTIFIER_BRACES on this.src.slice(this.pos) at every {, which could make parsing quadratic. It now uses a sticky regex at the parser's position, with no copy. The rest of the parser is a single pass with depth capped at 64.
  • MAX_PATTERN_LENGTH (8192) is checked before any work. A longer pattern is declined as unverified.
  • Each pattern's length is charged to the shared schema budget up front. That also covers parsing, and an exhausted budget now skips the native constructor.
  • Tests: a spy confirms over-length and over-budget patterns never reach the native constructor, and 200 long patterns exhaust the schema budget.

local:gate passed (exit 0).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The custom regex engine and Ajv-wide budget enforcement require specialist review and full Ajv/browser validation beyond the passing differential checks.

No open issues 🎉

Review effort: Balanced

@cliffhall
cliffhall merged commit 0f98f3d into v2/main Nov 4, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Issues and PRs for v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[GHSA-2fw8-7jmw-r937] - Tool outputSchema pattern can stall validation (ReDoS)

3 participants