[Capability] Bound what a schema can cost to validate (SEP-2106) - #436
Open
chr-hertel wants to merge 5 commits into
Open
[Capability] Bound what a schema can cost to validate (SEP-2106)#436chr-hertel wants to merge 5 commits into
chr-hertel wants to merge 5 commits into
Conversation
chr-hertel
requested review from
CodeWithKyrian,
Nyholm and
soyuka
as code owners
August 18, 2026 06:59
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a schema “complexity guard” to preflight JSON Schemas used in capability discovery/tool argument validation, preventing unsafe $ref usage and bounding validation cost before opis/json-schema performs an expensive walk.
Changes:
- Add
Mcp\Capability\Discovery\SchemaComplexityGuardto reject non-local$refand structurally “ruinous” schemas (depth/subschema/property-map ceilings). - Wire the guard into
SchemaValidatorby default (configurable via constructor), cap reported errors at 100, and improve messaging for unsupported$schemadialects. - Add focused unit coverage for the guard and document the feature in
CHANGELOG.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/Unit/Capability/Discovery/SchemaComplexityGuardTest.php | Adds unit tests covering external $ref rejection and complexity bounds behavior. |
| src/Capability/Discovery/SchemaValidator.php | Integrates the guard, caps/maxes errors, and improves unsupported dialect error reporting. |
| src/Capability/Discovery/SchemaComplexityGuard.php | New pre-validation guard implementing external $ref refusal and complexity estimation. |
| CHANGELOG.md | Documents the new guard, the error cap, and the improved dialect error message. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+68
to
+83
| public function check(array|object $schema): ?string | ||
| { | ||
| $root = self::toArray($schema); | ||
|
|
||
| if (null !== $reason = $this->findExternalRef($root, 0)) { | ||
| return $reason; | ||
| } | ||
|
|
||
| try { | ||
| $this->cost($root, $root, [], 0, new \stdClass()); | ||
| } catch (\OverflowException $e) { | ||
| return $e->getMessage(); | ||
| } | ||
|
|
||
| return null; | ||
| } |
Member
Author
There was a problem hiding this comment.
Fixed in 0672eb9 — check() now catches \JsonException from toArray() and returns it as a refusal reason instead of leaking it.
Comment on lines
+210
to
+214
| private static function resolve(string $pointer, array $root): ?array | ||
| { | ||
| if ('#' === $pointer || '' === $pointer) { | ||
| return $root; | ||
| } |
Member
Author
There was a problem hiding this comment.
Fixed in 0672eb9 — dropped the unreachable '' === $pointer branch in resolve().
Refuses two shapes before opis/json-schema walks them (SEP-2106): a $ref naming anything outside the document, and a composition that expands past a subschema budget, a nesting depth, or a property-map size. The external $ref was already safe, but only by omission - the SDK registers no resolver, so it failed as an opaque "unresolved reference". The guard now states the rule up front. The composition bound was a real hole: sixteen nested two-branch anyOfs took 9.0s and 65536 error objects. Validator:: setMaxErrors() bounds the report, not the walk, so the guard is structural and runs first. The budget resolves same-document $refs, so the $defs- compressed form of the same bomb - a few hundred bytes on the wire - is caught along with the expanded one. Recursive schemas and long reference chains still pass. SchemaValidator also caps reported errors at 100, and reports an unsupported $schema dialect as such, naming it, instead of as an internal fault.
A chain of {"$ref": ...} nodes is meant to cost nothing regardless of
length, but cost()/refCost() resolved it by mutual recursion - one
native call frame per link. A chain long enough (~17-20k links, well
within default maxProperties/maxDepth combined across sibling maps)
exhausted the stack or its backing memory before the subschema budget
or depth ceiling ever got a chance to refuse it: the guard was
bypassable by the exact class of input it exists to stop.
refCost() now walks the chain in a loop at constant stack depth,
handing only the schema found at the end of it to cost() for its own
already depth-bounded recursion.
chr-hertel
force-pushed
the
pr/schema-complexity-guard
branch
from
August 29, 2026 13:26
bc3f595 to
616f142
Compare
A server killed by a signal writes nothing, so the conditional dump stayed silent on exactly the failure being chased. Drop with the commit below it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
New
Mcp\Capability\Discovery\SchemaComplexityGuard, wired intoSchemaValidatorby default and configurable through its constructor, refuses two shapes beforeopis/json-schemawalks them:$refnaming anything outside the document — an SSRF primitive if it were ever dereferenced. The SDK registers no resolver, so this was already safe by omission: an external$reffailed as an opaque "unresolved reference". The guard states the rule up front and tests it.anyOfs took 9.0 s and produced 65,536 error objects.Validator::setMaxErrors()bounds the report, not the walk, so the guard has to be structural and run first. Measured after: refused in 0.1 s.The budget resolves same-document
$refs, so the$defs-compressed form of a composition bomb — a few hundred bytes on the wire, a million subschema evaluations to walk — is caught along with the expanded one. Recursive schemas and long reference chains still pass.SchemaValidatoralso caps reported errors at 100, and reports an unsupported$schemadialect as such, naming the dialect, rather than as an opaque internal fault.Part of SEP-2106.