From 229e1f6a47c273c52f918c00ba9abc0fa31d5fef Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 18 Aug 2026 02:47:16 +0200 Subject: [PATCH 1/8] [Capability] Bound what a schema can cost to validate 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. --- CHANGELOG.md | 1 + .../Discovery/SchemaComplexityGuard.php | 258 ++++++++++++++++++ src/Capability/Discovery/SchemaValidator.php | 41 ++- .../Discovery/SchemaComplexityGuardTest.php | 220 +++++++++++++++ 4 files changed, 519 insertions(+), 1 deletion(-) create mode 100644 src/Capability/Discovery/SchemaComplexityGuard.php create mode 100644 tests/Unit/Capability/Discovery/SchemaComplexityGuardTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index f085872a..ba217f86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ All notable changes to `mcp/sdk` will be documented in this file. * [BC Break] `Schema\JsonRpc\Error` accepts `null` as `$id`; an unreadable id now omits the member instead of sending `"id": ""`. `MessageFactory` decodes a missing or null id as an id-less error. * Preserve the request `id` on an invalid-but-parseable message (`-32600`) via `InvalidInputMessageException::getRequestId()`. * [BC Break] Drop the SDK-only name pattern on `ResourceDefinition`/`ResourceTemplate` `$name`; the spec allows any string. +* Refuse a JSON Schema that is unsafe or ruinous to validate before `opis/json-schema` walks it (SEP-2106): a `$ref` naming anything outside the document, and a composition expanding past a subschema budget, nesting depth or property-map size. New `Capability\Discovery\SchemaComplexityGuard`, wired into `SchemaValidator` by default and configurable through its constructor — sixteen nested two-branch `anyOf`s went from 9.0s to refused in 0.1s. `SchemaValidator` also caps reported errors at 100 and names an unsupported `$schema` dialect instead of reporting an internal fault. * Log expected tool failures (`ToolCallException`) at debug level instead of error. * Add `annotations` to `ImageContent`. * Fix empty tool/resource schemas serializing as `[]` instead of `{}`. diff --git a/src/Capability/Discovery/SchemaComplexityGuard.php b/src/Capability/Discovery/SchemaComplexityGuard.php new file mode 100644 index 00000000..3a0b8a86 --- /dev/null +++ b/src/Capability/Discovery/SchemaComplexityGuard.php @@ -0,0 +1,258 @@ + + */ +final class SchemaComplexityGuard +{ + /** + * Keywords whose value is a map of name to subschema, rather than a + * subschema itself. Their keys are user-chosen and must not be read as + * keywords. + */ + private const SCHEMA_MAPS = ['properties', 'patternProperties', '$defs', 'definitions', 'dependentSchemas']; + + /** + * @param int $maxDepth how deeply subschemas may nest + * @param int $maxSubschemas ceiling on estimated subschema evaluations + * @param int $maxProperties ceiling on named subschemas in any one map + */ + public function __construct( + private readonly int $maxDepth = 32, + private readonly int $maxSubschemas = 10_000, + private readonly int $maxProperties = 1_000, + ) { + } + + /** + * @param array|object $schema + * + * @return string|null the reason to refuse, or null when the schema is within bounds + */ + 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; + } + + /** + * @param array $node + */ + private function findExternalRef(array $node, int $depth): ?string + { + if ($depth > $this->maxDepth) { + return \sprintf('Schema nests deeper than the %d levels this validator accepts.', $this->maxDepth); + } + + foreach ($node as $key => $value) { + if ('$ref' === $key && \is_string($value) && !str_starts_with($value, '#')) { + return \sprintf('Schema contains the non-local reference "%s"; only same-document "#" references are resolved.', $value); + } + + if (\is_array($value) && null !== $reason = $this->findExternalRef($value, $depth + 1)) { + return $reason; + } + } + + return null; + } + + /** + * Estimated subschema evaluations $node can trigger. + * + * @param array $node + * @param array $root + * @param list $stack pointers currently being resolved, so a cycle is not followed twice + * @param \stdClass $memo cost per already-resolved pointer + * + * @throws \OverflowException as soon as the running estimate passes the ceiling + */ + private function cost(array $node, array $root, array $stack, int $depth, object $memo): int + { + if ($depth > $this->maxDepth) { + throw new \OverflowException(\sprintf('Schema nests deeper than the %d levels this validator accepts.', $this->maxDepth)); + } + + if (isset($node['$ref']) && \is_string($node['$ref'])) { + return $this->refCost($node['$ref'], $root, $stack, $depth, $memo); + } + + $total = 1; + + foreach ($node as $key => $value) { + if (!\is_array($value)) { + continue; + } + + if (\in_array($key, self::SCHEMA_MAPS, true)) { + if (\count($value) > $this->maxProperties) { + throw new \OverflowException(\sprintf('Schema declares more than %d entries under "%s".', $this->maxProperties, $key)); + } + + foreach ($value as $subschema) { + if (\is_array($subschema)) { + $total += $this->cost($subschema, $root, $stack, $depth + 1, $memo); + } + } + + $this->assertWithinBudget($total); + + continue; + } + + // Everything else holding an array is either a subschema or a list + // of them; a keyword holding plain data contributes nothing but is + // harmless to walk, since only its own nesting is counted. + if (array_is_list($value)) { + foreach ($value as $subschema) { + if (\is_array($subschema)) { + $total += $this->cost($subschema, $root, $stack, $depth + 1, $memo); + } + } + } else { + $total += $this->cost($value, $root, $stack, $depth + 1, $memo); + } + + $this->assertWithinBudget($total); + } + + return $total; + } + + /** + * @param array $root + * @param list $stack + */ + private function refCost(string $pointer, array $root, array $stack, int $depth, object $memo): int + { + // A back-edge: recursive schemas are legitimate, and how far one + // unrolls is decided by the data, not the schema. + if (\in_array($pointer, $stack, true)) { + return 1; + } + + if (isset($memo->{$pointer})) { + return $memo->{$pointer}; + } + + $target = self::resolve($pointer, $root); + + if (null === $target) { + // Unresolvable same-document pointers are the validator's business + // to report; nothing here can be expensive. + return 1; + } + + // Depth is lexical nesting, which following a reference is not: a long + // chain of `$defs` referring to one another is flat and cheap. What + // bounds this is the subschema budget and the cycle check above, and + // the pointer set is finite, so the recursion is too. + $cost = $this->cost($target, $root, [...$stack, $pointer], $depth, $memo); + $memo->{$pointer} = $cost; + + return $cost; + } + + /** + * Resolves a same-document JSON pointer (`#`, `#/$defs/name`). + * + * @param array $root + * + * @return array|null + */ + private static function resolve(string $pointer, array $root): ?array + { + if ('#' === $pointer || '' === $pointer) { + return $root; + } + + if (!str_starts_with($pointer, '#/')) { + return null; + } + + $node = $root; + + foreach (explode('/', substr($pointer, 2)) as $segment) { + $segment = str_replace(['~1', '~0'], ['/', '~'], rawurldecode($segment)); + + if (!\is_array($node) || !\array_key_exists($segment, $node)) { + return null; + } + + $node = $node[$segment]; + } + + return \is_array($node) ? $node : null; + } + + private function assertWithinBudget(int $total): void + { + if ($total > $this->maxSubschemas) { + throw new \OverflowException(\sprintf('Schema composes more than %d subschemas, which this validator refuses to walk.', $this->maxSubschemas)); + } + } + + /** + * @param array|object $schema + * + * @return array + */ + private static function toArray(array|object $schema): array + { + if (\is_array($schema)) { + return $schema; + } + + /** @var array $decoded */ + $decoded = json_decode(json_encode($schema, \JSON_THROW_ON_ERROR), true, flags: \JSON_THROW_ON_ERROR); + + return $decoded; + } +} diff --git a/src/Capability/Discovery/SchemaValidator.php b/src/Capability/Discovery/SchemaValidator.php index 56174bdc..ae4d3969 100644 --- a/src/Capability/Discovery/SchemaValidator.php +++ b/src/Capability/Discovery/SchemaValidator.php @@ -30,11 +30,23 @@ */ class SchemaValidator { + /** + * Ceiling on reported errors. Opis walks the whole schema regardless — this + * only bounds the array built out of it, which a composition blow-up can + * make the larger cost of the two. {@see SchemaComplexityGuard} is what + * bounds the walk. + */ + private const MAX_REPORTED_ERRORS = 100; + private ?Validator $jsonSchemaValidator = null; + private SchemaComplexityGuard $complexityGuard; + public function __construct( private LoggerInterface $logger = new NullLogger(), + ?SchemaComplexityGuard $complexityGuard = null, ) { + $this->complexityGuard = $complexityGuard ?? new SchemaComplexityGuard(); } /** @@ -81,6 +93,14 @@ public function validateAgainstJsonSchema(mixed $data, array|object $schema): ar return [['pointer' => '', 'keyword' => 'internal', 'message' => 'Internal validation preparation error.']]; } + // Before the validator sees it: a schema can be cheap to send and + // ruinous to walk, and refusing it is only possible up front. + if (null !== $reason = $this->complexityGuard->check($schemaObject)) { + $this->logger->warning('MCP SDK: Refused a schema the complexity guard rejected.', ['reason' => $reason]); + + return [['pointer' => '', 'keyword' => 'schema', 'message' => $reason]]; + } + $validator = $this->getJsonSchemaValidator(); try { @@ -92,6 +112,13 @@ public function validateAgainstJsonSchema(mixed $data, array|object $schema): ar 'schema' => json_encode($schemaObject), ]); + // "Unsupported draft-XXXX" is the one failure here that is the + // schema's doing rather than ours, and the spec asks for an error + // that names the dialect. + if (str_contains($e->getMessage(), 'Unsupported draft')) { + return [['pointer' => '', 'keyword' => '$schema', 'message' => \sprintf('Unsupported JSON Schema dialect: %s. This validator supports 2020-12 (the default when no "$schema" is given) and the drafts opis/json-schema implements.', $e->getMessage())]]; + } + return [['pointer' => '', 'keyword' => 'internal', 'message' => 'Schema validation process failed: '.$e->getMessage()]]; } @@ -124,7 +151,12 @@ private function getJsonSchemaValidator(): Validator { if (null === $this->jsonSchemaValidator) { $this->jsonSchemaValidator = new Validator(); - // Potentially configure resolver here if needed later + $this->jsonSchemaValidator->setMaxErrors(self::MAX_REPORTED_ERRORS); + // No resolver is registered, and none should be: a `$ref` naming an + // absolute URI must never be fetched, which is a MUST in the + // specification's JSON Schema rules. SchemaComplexityGuard refuses + // such a schema before it reaches here, so this is the second of + // two locks rather than the only one. } return $this->jsonSchemaValidator; @@ -169,6 +201,13 @@ private function convertDataForValidator(mixed $data): mixed */ private function collectSubErrors(ValidationError $error, array &$collectedErrors): void { + // The error tree fans out with the schema, so a composition-heavy + // schema produces far more leaves than Opis's own cap admits. Past the + // ceiling there is nothing left to learn from another one. + if (\count($collectedErrors) >= self::MAX_REPORTED_ERRORS) { + return; + } + $subErrors = $error->subErrors(); if (empty($subErrors)) { $collectedErrors[] = [ diff --git a/tests/Unit/Capability/Discovery/SchemaComplexityGuardTest.php b/tests/Unit/Capability/Discovery/SchemaComplexityGuardTest.php new file mode 100644 index 00000000..f26d0c69 --- /dev/null +++ b/tests/Unit/Capability/Discovery/SchemaComplexityGuardTest.php @@ -0,0 +1,220 @@ +guard = new SchemaComplexityGuard(); + } + + /** + * @return iterable}> + */ + public static function ordinarySchemas(): iterable + { + yield 'empty' => [[]]; + yield 'flat object' => [[ + 'type' => 'object', + 'properties' => ['a' => ['type' => 'string'], 'b' => ['type' => 'integer']], + 'required' => ['a'], + ]]; + yield 'nested objects' => [[ + 'type' => 'object', + 'properties' => ['outer' => ['type' => 'object', 'properties' => ['inner' => ['type' => 'string']]]], + ]]; + yield 'array with items' => [['type' => 'array', 'items' => ['type' => 'string']]]; + yield 'modest composition' => [[ + 'type' => 'object', + 'properties' => ['v' => ['anyOf' => [['type' => 'string'], ['type' => 'integer'], ['type' => 'null']]]], + ]]; + yield 'local $ref through $defs' => [[ + '$defs' => ['name' => ['type' => 'string', 'minLength' => 1]], + 'type' => 'object', + 'properties' => ['first' => ['$ref' => '#/$defs/name'], 'last' => ['$ref' => '#/$defs/name']], + ]]; + yield 'if/then/else' => [[ + 'type' => 'object', + 'if' => ['properties' => ['kind' => ['const' => 'a']]], + 'then' => ['required' => ['x']], + 'else' => ['required' => ['y']], + ]]; + yield 'a property literally named $ref' => [[ + 'type' => 'object', + 'properties' => ['$ref' => ['type' => 'string']], + ]]; + } + + /** + * @param array $schema + */ + #[DataProvider('ordinarySchemas')] + #[TestDox('an ordinary schema passes untouched')] + public function testOrdinarySchemasPass(array $schema): void + { + $this->assertNull($this->guard->check($schema)); + } + + /** + * @return iterable + */ + public static function externalRefs(): iterable + { + yield 'https' => ['https://evil.example/schema.json']; + yield 'http' => ['http://169.254.169.254/latest/meta-data/']; + yield 'file' => ['file:///etc/passwd']; + yield 'relative document' => ['common.json#/$defs/name']; + yield 'protocol-relative' => ['//evil.example/schema.json']; + } + + #[DataProvider('externalRefs')] + #[TestDox('a reference outside the document is refused, and nothing is fetched')] + public function testExternalRefIsRefused(string $ref): void + { + $reason = $this->guard->check([ + 'type' => 'object', + 'properties' => ['a' => ['$ref' => $ref]], + ]); + + $this->assertNotNull($reason); + $this->assertStringContainsString('non-local reference', $reason); + $this->assertStringContainsString($ref, $reason); + } + + #[TestDox('a same-document reference is not mistaken for an external one')] + public function testLocalRefIsAllowed(): void + { + $this->assertNull($this->guard->check([ + '$defs' => ['n' => ['type' => 'integer']], + '$ref' => '#/$defs/n', + ])); + } + + #[TestDox('nesting past the depth ceiling is refused')] + public function testExcessiveDepthIsRefused(): void + { + $schema = ['type' => 'string']; + for ($i = 0; $i < 60; ++$i) { + $schema = ['type' => 'object', 'properties' => ['n' => $schema]]; + } + + $this->assertStringContainsString('nests deeper', (string) $this->guard->check($schema)); + } + + #[TestDox('an expanded composition bomb is refused')] + public function testExpandedCompositionBombIsRefused(): void + { + // Fourteen levels: 2^14 branches, but only 28 levels of nesting, so it + // is the subschema budget and not the depth ceiling that refuses it. + $branch = ['type' => 'string']; + for ($i = 0; $i < 14; ++$i) { + $branch = ['anyOf' => [$branch, $branch]]; + } + + $this->assertStringContainsString('subschemas', (string) $this->guard->check($branch)); + } + + #[TestDox('the same bomb written with $defs — a few hundred bytes — is refused too')] + public function testRefCompressedCompositionBombIsRefused(): void + { + // Each level doubles by referencing the level below twice. Linear on the + // wire, exponential to walk: this is the shape a size cap cannot catch. + $defs = ['a0' => ['type' => 'string']]; + for ($i = 1; $i <= 20; ++$i) { + $defs['a'.$i] = ['anyOf' => [['$ref' => '#/$defs/a'.($i - 1)], ['$ref' => '#/$defs/a'.($i - 1)]]]; + } + + $schema = ['$defs' => $defs, '$ref' => '#/$defs/a20']; + + $this->assertLessThan(2048, \strlen((string) json_encode($schema))); + $this->assertStringContainsString('subschemas', (string) $this->guard->check($schema)); + } + + #[TestDox('a long chain of local references is flat, not deep')] + public function testLongLocalRefChainIsAllowed(): void + { + // Following a reference is not nesting: this is 60 links and costs 60 + // steps, which a depth ceiling applied to resolution would refuse. + $defs = ['a0' => ['type' => 'string']]; + for ($i = 1; $i <= 60; ++$i) { + $defs['a'.$i] = ['$ref' => '#/$defs/a'.($i - 1)]; + } + + $this->assertNull($this->guard->check(['$defs' => $defs, '$ref' => '#/$defs/a60'])); + } + + #[TestDox('a recursive schema is allowed: how far it unrolls is the data\'s doing')] + public function testRecursiveSchemaIsAllowed(): void + { + $this->assertNull($this->guard->check([ + '$defs' => [ + 'node' => [ + 'type' => 'object', + 'properties' => [ + 'value' => ['type' => 'string'], + 'children' => ['type' => 'array', 'items' => ['$ref' => '#/$defs/node']], + ], + ], + ], + '$ref' => '#/$defs/node', + ])); + } + + #[TestDox('an oversized property map is refused')] + public function testOversizedPropertyMapIsRefused(): void + { + $properties = []; + for ($i = 0; $i < 1_500; ++$i) { + $properties['p'.$i] = ['type' => 'string']; + } + + $this->assertStringContainsString('entries under "properties"', (string) $this->guard->check([ + 'type' => 'object', + 'properties' => $properties, + ])); + } + + #[TestDox('an unresolvable local pointer is left for the validator to report')] + public function testUnresolvableLocalPointerPasses(): void + { + $this->assertNull($this->guard->check(['$ref' => '#/$defs/missing'])); + } + + #[TestDox('the bounds are configurable')] + public function testBoundsAreConfigurable(): void + { + $schema = [ + 'type' => 'object', + 'properties' => ['a' => ['type' => 'object', 'properties' => ['b' => ['type' => 'string']]]], + ]; + + $this->assertNull((new SchemaComplexityGuard())->check($schema)); + $this->assertStringContainsString('nests deeper', (string) (new SchemaComplexityGuard(maxDepth: 1))->check($schema)); + $this->assertStringContainsString('subschemas', (string) (new SchemaComplexityGuard(maxSubschemas: 2))->check($schema)); + } + + #[TestDox('an object schema is accepted as well as an array one')] + public function testObjectSchemaIsAccepted(): void + { + $schema = json_decode('{"type":"object","properties":{"a":{"$ref":"https://evil.example/s.json"}}}'); + + $this->assertStringContainsString('non-local reference', (string) $this->guard->check($schema)); + } +} From 4f161e0d71baf77b3841d7dbf9f6ba0bb3d5aeb6 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 18 Aug 2026 09:22:09 +0200 Subject: [PATCH 2/8] Turn schema decode failure into a reason string; drop dead pointer branch --- src/Capability/Discovery/SchemaComplexityGuard.php | 8 ++++++-- .../Capability/Discovery/SchemaComplexityGuardTest.php | 9 +++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/Capability/Discovery/SchemaComplexityGuard.php b/src/Capability/Discovery/SchemaComplexityGuard.php index 3a0b8a86..cec7aac7 100644 --- a/src/Capability/Discovery/SchemaComplexityGuard.php +++ b/src/Capability/Discovery/SchemaComplexityGuard.php @@ -67,7 +67,11 @@ public function __construct( */ public function check(array|object $schema): ?string { - $root = self::toArray($schema); + try { + $root = self::toArray($schema); + } catch (\JsonException $e) { + return \sprintf('Schema could not be decoded as JSON: %s', $e->getMessage()); + } if (null !== $reason = $this->findExternalRef($root, 0)) { return $reason; @@ -209,7 +213,7 @@ private function refCost(string $pointer, array $root, array $stack, int $depth, */ private static function resolve(string $pointer, array $root): ?array { - if ('#' === $pointer || '' === $pointer) { + if ('#' === $pointer) { return $root; } diff --git a/tests/Unit/Capability/Discovery/SchemaComplexityGuardTest.php b/tests/Unit/Capability/Discovery/SchemaComplexityGuardTest.php index f26d0c69..d6d4984e 100644 --- a/tests/Unit/Capability/Discovery/SchemaComplexityGuardTest.php +++ b/tests/Unit/Capability/Discovery/SchemaComplexityGuardTest.php @@ -217,4 +217,13 @@ public function testObjectSchemaIsAccepted(): void $this->assertStringContainsString('non-local reference', (string) $this->guard->check($schema)); } + + #[TestDox('an object schema that cannot be encoded as JSON is refused, not thrown')] + public function testUnencodableObjectSchemaIsRefused(): void + { + $schema = new \stdClass(); + $schema->bad = \NAN; + + $this->assertStringContainsString('could not be decoded as JSON', (string) $this->guard->check($schema)); + } } From 616f142702c666711ae22492df4d0cd2de6a4308 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 18 Aug 2026 21:32:26 +0200 Subject: [PATCH 3/8] Walk bare $ref chains iteratively, not by mutual recursion 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. --- .../Discovery/SchemaComplexityGuard.php | 74 ++++++++++++++----- .../Discovery/SchemaComplexityGuardTest.php | 21 ++++++ 2 files changed, 75 insertions(+), 20 deletions(-) diff --git a/src/Capability/Discovery/SchemaComplexityGuard.php b/src/Capability/Discovery/SchemaComplexityGuard.php index cec7aac7..209fcbc6 100644 --- a/src/Capability/Discovery/SchemaComplexityGuard.php +++ b/src/Capability/Discovery/SchemaComplexityGuard.php @@ -171,35 +171,69 @@ private function cost(array $node, array $root, array $stack, int $depth, object } /** + * Chases a same-document `$ref`, and every bare `$ref` it in turn points + * to, without recursing: a node that is only `{"$ref": ...}` contributes + * nothing of its own, so a schema chaining many of them (a "flat" `$defs` + * indirection) is meant to be free regardless of length. Resolving that + * chain by mutual recursion with {@see cost()} spent one native call + * frame per link, so a chain long enough — a size none of the other + * bounds catch, since a chain's cost is deliberately independent of its + * length — exhausted the stack or the memory backing it before this + * class ever got to refuse anything. Walking the chain in a loop keeps + * this at constant stack depth; only the schema found at the end of it, + * if any, is handed to cost() for its own depth-bounded recursion. + * * @param array $root - * @param list $stack + * @param list $stack pointers being resolved by an enclosing call */ private function refCost(string $pointer, array $root, array $stack, int $depth, object $memo): int { - // A back-edge: recursive schemas are legitimate, and how far one - // unrolls is decided by the data, not the schema. - if (\in_array($pointer, $stack, true)) { - return 1; - } + $visited = []; - if (isset($memo->{$pointer})) { - return $memo->{$pointer}; - } + while (true) { + // A back-edge: recursive schemas are legitimate, and how far one + // unrolls is decided by the data, not the schema. + if (\in_array($pointer, $stack, true) || isset($visited[$pointer])) { + return $this->memoizeAll($visited, 1, $memo); + } + + if (isset($memo->{$pointer})) { + return $this->memoizeAll($visited, $memo->{$pointer}, $memo); + } + + $target = self::resolve($pointer, $root); + + if (null === $target) { + // Unresolvable same-document pointers are the validator's + // business to report; nothing here can be expensive. + return $this->memoizeAll($visited, 1, $memo); + } - $target = self::resolve($pointer, $root); + $visited[$pointer] = true; - if (null === $target) { - // Unresolvable same-document pointers are the validator's business - // to report; nothing here can be expensive. - return 1; + if (!isset($target['$ref']) || !\is_string($target['$ref'])) { + // Depth is lexical nesting, which following a reference is + // not: a long chain of `$defs` referring to one another is + // flat and cheap. What bounds this is the subschema budget + // and the cycle check above, and the pointer set is finite, + // so the walk is too. + $cost = $this->cost($target, $root, [...$stack, ...array_keys($visited)], $depth, $memo); + + return $this->memoizeAll($visited, $cost, $memo); + } + + $pointer = $target['$ref']; } + } - // Depth is lexical nesting, which following a reference is not: a long - // chain of `$defs` referring to one another is flat and cheap. What - // bounds this is the subschema budget and the cycle check above, and - // the pointer set is finite, so the recursion is too. - $cost = $this->cost($target, $root, [...$stack, $pointer], $depth, $memo); - $memo->{$pointer} = $cost; + /** + * @param array $pointers + */ + private function memoizeAll(array $pointers, int $cost, object $memo): int + { + foreach ($pointers as $pointer => $_) { + $memo->{$pointer} = $cost; + } return $cost; } diff --git a/tests/Unit/Capability/Discovery/SchemaComplexityGuardTest.php b/tests/Unit/Capability/Discovery/SchemaComplexityGuardTest.php index d6d4984e..ce613d7d 100644 --- a/tests/Unit/Capability/Discovery/SchemaComplexityGuardTest.php +++ b/tests/Unit/Capability/Discovery/SchemaComplexityGuardTest.php @@ -160,6 +160,27 @@ public function testLongLocalRefChainIsAllowed(): void $this->assertNull($this->guard->check(['$defs' => $defs, '$ref' => '#/$defs/a60'])); } + #[TestDox('a long chain of bare $refs is walked without recursing per link')] + public function testLongLocalRefChainDoesNotRecursePerLink(): void + { + // Bare {"$ref": ...} nodes chained together are meant to be free + // regardless of length, and used to be resolved by mutual recursion + // between cost() and refCost(): one native call frame per link. A + // chain long enough exhausted the stack, or the memory backing it, + // long before the subschema budget below ever got a chance to fire — + // 20,000 links reliably faulted with the old implementation. This + // uses a guard with a raised budget so the chain is not refused for + // an unrelated reason, and asserts it resolves at all. + $defs = ['a0' => ['type' => 'string']]; + for ($i = 1; $i < 20_000; ++$i) { + $defs['a'.$i] = ['$ref' => '#/$defs/a'.($i - 1)]; + } + + $guard = new SchemaComplexityGuard(maxSubschemas: 1_000_000, maxProperties: 1_000_000); + + $this->assertNull($guard->check(['$defs' => $defs, '$ref' => '#/$defs/a19999'])); + } + #[TestDox('a recursive schema is allowed: how far it unrolls is the data\'s doing')] public function testRecursiveSchemaIsAllowed(): void { From 3d87cc8a4833cd06691c69ce9303e32e5e0978bd Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 18 Aug 2026 12:09:38 +0200 Subject: [PATCH 4/8] Dump inspector server output for CI debugging --- .../Http/HttpInspectorSnapshotTestCase.php | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/Inspector/Http/HttpInspectorSnapshotTestCase.php b/tests/Inspector/Http/HttpInspectorSnapshotTestCase.php index 5db629e4..72f6153d 100644 --- a/tests/Inspector/Http/HttpInspectorSnapshotTestCase.php +++ b/tests/Inspector/Http/HttpInspectorSnapshotTestCase.php @@ -29,6 +29,27 @@ protected function tearDown(): void $this->stopServer(); } + private function dumpServerOutputForDiagnosis(): void + { + if (!isset($this->serverProcess)) { + return; + } + + $out = $this->serverProcess->getOutput(); + $err = $this->serverProcess->getErrorOutput(); + + if ('' !== $out || '' !== $err) { + fwrite(\STDERR, \sprintf( + "\n[DIAG] server on port %d (pid target %s), exit code %s\n--- stdout ---\n%s\n--- stderr ---\n%s\n[/DIAG]\n", + $this->serverPort, + (string) getmypid(), + var_export($this->serverProcess->getExitCode(), true), + $out, + $err, + )); + } + } + abstract protected function getServerScript(): string; protected function getServerConnectionArgs(): array @@ -71,6 +92,7 @@ private function stopServer(): void { if (isset($this->serverProcess)) { $this->serverProcess->stop(1, \SIGTERM); + $this->dumpServerOutputForDiagnosis(); } } From 6be09b7b873e0be097e8660237976c05b61abf7d Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Sat, 29 Aug 2026 16:08:28 +0200 Subject: [PATCH 5/8] TEMP: always dump the inspector server's exit code and signal 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. --- .../Http/HttpInspectorSnapshotTestCase.php | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/tests/Inspector/Http/HttpInspectorSnapshotTestCase.php b/tests/Inspector/Http/HttpInspectorSnapshotTestCase.php index 72f6153d..70ebc1a4 100644 --- a/tests/Inspector/Http/HttpInspectorSnapshotTestCase.php +++ b/tests/Inspector/Http/HttpInspectorSnapshotTestCase.php @@ -38,16 +38,29 @@ private function dumpServerOutputForDiagnosis(): void $out = $this->serverProcess->getOutput(); $err = $this->serverProcess->getErrorOutput(); - if ('' !== $out || '' !== $err) { - fwrite(\STDERR, \sprintf( - "\n[DIAG] server on port %d (pid target %s), exit code %s\n--- stdout ---\n%s\n--- stderr ---\n%s\n[/DIAG]\n", - $this->serverPort, - (string) getmypid(), - var_export($this->serverProcess->getExitCode(), true), - $out, - $err, - )); + // Both throw unless the process has actually terminated, and losing + // the dump to an exception in tearDown is the one outcome that would + // make this pointless. + try { + $exit = var_export($this->serverProcess->getExitCode(), true); + $signal = var_export($this->serverProcess->getTermSignal(), true); + } catch (\Throwable $e) { + $exit = $signal = 'unavailable ('.$e->getMessage().')'; } + + // Unconditional: a server killed by a signal (a stack overflow, say) + // exits without writing anything, and the exit code is then the only + // thing that says so. + fwrite(\STDERR, \sprintf( + "\n[DIAG] server on port %d (pid target %s), running %s, exit code %s, signal %s\n--- stdout ---\n%s\n--- stderr ---\n%s\n[/DIAG]\n", + $this->serverPort, + (string) getmypid(), + var_export($this->serverProcess->isRunning(), true), + $exit, + $signal, + $out, + $err, + )); } abstract protected function getServerScript(): string; From 386ae835fd9b3587dfa72c53b6ab1ca4ca915af6 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Sat, 29 Aug 2026 21:35:47 +0200 Subject: [PATCH 6/8] TEMP EXPERIMENT: cap reported errors at 1 Isolates whether the inspector SIGSEGV comes from the enlarged Opis error tree (setMaxErrors 100 vs its default 1) and collectSubErrors' unbounded recursion over it. Revert with the two diagnostic commits. --- src/Capability/Discovery/SchemaValidator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Capability/Discovery/SchemaValidator.php b/src/Capability/Discovery/SchemaValidator.php index ae4d3969..b9552dde 100644 --- a/src/Capability/Discovery/SchemaValidator.php +++ b/src/Capability/Discovery/SchemaValidator.php @@ -36,7 +36,7 @@ class SchemaValidator * make the larger cost of the two. {@see SchemaComplexityGuard} is what * bounds the walk. */ - private const MAX_REPORTED_ERRORS = 100; + private const MAX_REPORTED_ERRORS = 1; private ?Validator $jsonSchemaValidator = null; From 69976d84b661aa0e7bcd93c7669e5745ffc9e8ef Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Sat, 29 Aug 2026 21:41:56 +0200 Subject: [PATCH 7/8] TEMP: trace SchemaValidator phases to stderr Restores MAX_REPORTED_ERRORS to 100 (the maxErrors=1 experiment came back negative) and marks each phase, so the last line before the SIGSEGV names which one dies. Drop with the other TEMP commits. --- src/Capability/Discovery/SchemaValidator.php | 24 ++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/Capability/Discovery/SchemaValidator.php b/src/Capability/Discovery/SchemaValidator.php index b9552dde..d037589c 100644 --- a/src/Capability/Discovery/SchemaValidator.php +++ b/src/Capability/Discovery/SchemaValidator.php @@ -36,7 +36,7 @@ class SchemaValidator * make the larger cost of the two. {@see SchemaComplexityGuard} is what * bounds the walk. */ - private const MAX_REPORTED_ERRORS = 1; + private const MAX_REPORTED_ERRORS = 100; private ?Validator $jsonSchemaValidator = null; @@ -57,8 +57,15 @@ public function __construct( * * @return list array of validation errors, empty if valid */ + private static function trace(string $phase): void + { + // STDERR is not defined under the cli-server SAPI; php://stderr is. + file_put_contents('php://stderr', '[TRACE] '.$phase."\n", \FILE_APPEND); + } + public function validateAgainstJsonSchema(mixed $data, array|object $schema): array { + self::trace('enter'); if (\is_array($data) && empty($data)) { $data = new \stdClass(); } @@ -78,7 +85,9 @@ public function validateAgainstJsonSchema(mixed $data, array|object $schema): ar // --- Data Preparation --- // Opis Validator generally prefers objects for object validation + self::trace('convertData:in'); $dataToValidate = $this->convertDataForValidator($data); + self::trace('convertData:out'); } catch (\JsonException $e) { $this->logger->error('MCP SDK: Invalid schema structure provided for validation (JSON conversion failed).', ['exception' => $e]); @@ -95,7 +104,10 @@ public function validateAgainstJsonSchema(mixed $data, array|object $schema): ar // Before the validator sees it: a schema can be cheap to send and // ruinous to walk, and refusing it is only possible up front. - if (null !== $reason = $this->complexityGuard->check($schemaObject)) { + self::trace('guard:in'); + $reason = $this->complexityGuard->check($schemaObject); + self::trace('guard:out'); + if (null !== $reason) { $this->logger->warning('MCP SDK: Refused a schema the complexity guard rejected.', ['reason' => $reason]); return [['pointer' => '', 'keyword' => 'schema', 'message' => $reason]]; @@ -104,7 +116,9 @@ public function validateAgainstJsonSchema(mixed $data, array|object $schema): ar $validator = $this->getJsonSchemaValidator(); try { + self::trace('opis:in'); $result = $validator->validate($dataToValidate, $schemaObject); + self::trace('opis:out'); } catch (\Throwable $e) { $this->logger->error('MCP SDK: JSON Schema validation failed internally.', [ 'exception' => $e, @@ -123,14 +137,20 @@ public function validateAgainstJsonSchema(mixed $data, array|object $schema): ar } if ($result->isValid()) { + self::trace('valid:done'); + return []; } + self::trace('invalid:collecting'); + $formattedErrors = []; $topError = $result->error(); if ($topError) { + self::trace('collect:in'); $this->collectSubErrors($topError, $formattedErrors); + self::trace('collect:out'); } if (empty($formattedErrors) && $topError) { // Fallback From 5443de831f7ed47ebcf3e9e01389fcec9ea90bb0 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Sat, 29 Aug 2026 21:47:03 +0200 Subject: [PATCH 8/8] TEMP: trace inside SchemaComplexityGuard::check() All eight CI segfaults die between guard:in and guard:out. This splits check() into toArray / findExternalRef / cost and dumps the schema it was handed, since the schema published by tools/list is accepted in 0.03ms locally. Drop with the other TEMP commits. --- .../Discovery/SchemaComplexityGuard.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/Capability/Discovery/SchemaComplexityGuard.php b/src/Capability/Discovery/SchemaComplexityGuard.php index 209fcbc6..b0125a80 100644 --- a/src/Capability/Discovery/SchemaComplexityGuard.php +++ b/src/Capability/Discovery/SchemaComplexityGuard.php @@ -65,21 +65,40 @@ public function __construct( * * @return string|null the reason to refuse, or null when the schema is within bounds */ + private static function gtrace(string $m): void + { + file_put_contents('php://stderr', '[GTRACE] '.$m."\n", \FILE_APPEND); + } + public function check(array|object $schema): ?string { + self::gtrace('toArray:in type='.get_debug_type($schema)); + try { $root = self::toArray($schema); } catch (\JsonException $e) { return \sprintf('Schema could not be decoded as JSON: %s', $e->getMessage()); } + $json = (string) json_encode($root); + self::gtrace('toArray:out bytes='.\strlen($json).' keys='.implode(',', array_slice(array_keys($root), 0, 12))); + self::gtrace('schema='.substr($json, 0, 1500)); + + self::gtrace('extref:in'); if (null !== $reason = $this->findExternalRef($root, 0)) { + self::gtrace('extref:refused'); + return $reason; } + self::gtrace('extref:out'); try { + self::gtrace('cost:in'); $this->cost($root, $root, [], 0, new \stdClass()); + self::gtrace('cost:out'); } catch (\OverflowException $e) { + self::gtrace('cost:overflow'); + return $e->getMessage(); }