[Server] Enforce x-mcp-header where a request uses it (SEP-2243) (#453)
* [Server] Enforce x-mcp-header where a request uses it Only top-level properties were inspected, so an annotation on a nested property was silently unenforced - the header could say anything. The walk now follows a chain of properties keys to any depth and reads the argument at that exact path. Integer values compare numerically, so a client writing 42.0 for a body value of 42 is no longer rejected. * [Server] Stop comparing numeric-looking strings as numbers is_numeric() and loose == both treat a string like "042" as equal to "42", so a string argument could pass header validation against a header that doesn't match it exactly. Gate the numeric comparison on the argument's actual type and a decimal-only header instead.
This commit is contained in:
Christopher Hertel
GitHub
parent
be9fad4115
commit
7e38c94954
1 changed files with 225 additions and 12 deletions
|
|
@ -11,6 +11,9 @@
|
|||
|
||||
namespace Mcp\Tests\Unit\Server\Stateless;
|
||||
|
||||
use Mcp\Capability\Registry;
|
||||
use Mcp\Capability\RegistryInterface;
|
||||
use Mcp\Schema\Tool;
|
||||
use Mcp\Server\Stateless\StandardHeaderValidator;
|
||||
use PHPUnit\Framework\Attributes\DataProvider;
|
||||
use PHPUnit\Framework\Attributes\TestDox;
|
||||
|
|
@ -173,6 +176,148 @@ class StandardHeaderValidatorTest extends TestCase
|
|||
yield 'non-string name is ignored' => ['tools/call', ['name' => 42], null];
|
||||
}
|
||||
|
||||
#[TestDox('an annotation on a nested property is found through the properties chain')]
|
||||
public function testNestedMirroredPropertyIsFound(): void
|
||||
{
|
||||
$schema = [
|
||||
'type' => 'object',
|
||||
'properties' => [
|
||||
'target' => [
|
||||
'type' => 'object',
|
||||
'properties' => [
|
||||
'region' => ['type' => 'string', 'x-mcp-header' => 'Region'],
|
||||
],
|
||||
],
|
||||
'top' => ['type' => 'string', 'x-mcp-header' => 'Top'],
|
||||
],
|
||||
];
|
||||
|
||||
$this->assertSame(
|
||||
['Region' => ['target', 'region'], 'Top' => ['top']],
|
||||
StandardHeaderValidator::mirroredProperties($schema),
|
||||
);
|
||||
}
|
||||
|
||||
#[TestDox('an annotation the chain cannot reach statically is not mirrored')]
|
||||
#[DataProvider('unreachableAnnotations')]
|
||||
public function testUnreachableAnnotationsAreNotMirrored(array $properties): void
|
||||
{
|
||||
$this->assertSame([], StandardHeaderValidator::mirroredProperties([
|
||||
'type' => 'object',
|
||||
'properties' => $properties,
|
||||
]));
|
||||
}
|
||||
|
||||
/**
|
||||
* @return iterable<string, array{array<string, mixed>}>
|
||||
*/
|
||||
public static function unreachableAnnotations(): iterable
|
||||
{
|
||||
yield 'under items' => [['a' => ['type' => 'array', 'items' => ['type' => 'string', 'x-mcp-header' => 'X']]]];
|
||||
yield 'under anyOf' => [['a' => ['anyOf' => [['type' => 'string', 'x-mcp-header' => 'X']]]]];
|
||||
yield 'under if' => [['a' => ['if' => ['type' => 'string', 'x-mcp-header' => 'X']]]];
|
||||
}
|
||||
|
||||
#[TestDox('an integer is compared numerically, so 42 and 42.0 agree')]
|
||||
public function testIntegerParamsCompareNumerically(): void
|
||||
{
|
||||
$validator = new StandardHeaderValidator(self::registryWithMirroredTool());
|
||||
|
||||
$this->assertNull($validator->validate(
|
||||
'tools/call',
|
||||
['name' => 'mirrored', 'arguments' => ['retries' => 42]],
|
||||
['Mcp-Method' => 'tools/call', 'Mcp-Name' => 'mirrored', 'Mcp-Param-Retries' => '42.0'],
|
||||
));
|
||||
|
||||
$this->assertStringContainsString('does not match', (string) $validator->validate(
|
||||
'tools/call',
|
||||
['name' => 'mirrored', 'arguments' => ['retries' => 42]],
|
||||
['Mcp-Method' => 'tools/call', 'Mcp-Name' => 'mirrored', 'Mcp-Param-Retries' => '43'],
|
||||
));
|
||||
}
|
||||
|
||||
#[TestDox('a numeric-looking string argument keeps its exact-match comparison')]
|
||||
public function testNumericLookingStringArgumentIsNotComparedNumerically(): void
|
||||
{
|
||||
$validator = new StandardHeaderValidator(self::registryWithMirroredTool());
|
||||
|
||||
$this->assertStringContainsString('does not match', (string) $validator->validate(
|
||||
'tools/call',
|
||||
['name' => 'mirrored', 'arguments' => ['retries' => '042']],
|
||||
['Mcp-Method' => 'tools/call', 'Mcp-Name' => 'mirrored', 'Mcp-Param-Retries' => '42'],
|
||||
));
|
||||
|
||||
$this->assertNull($validator->validate(
|
||||
'tools/call',
|
||||
['name' => 'mirrored', 'arguments' => ['retries' => '042']],
|
||||
['Mcp-Method' => 'tools/call', 'Mcp-Name' => 'mirrored', 'Mcp-Param-Retries' => '042'],
|
||||
));
|
||||
}
|
||||
|
||||
#[TestDox('a scientific-notation header is not accepted as a decimal number')]
|
||||
public function testScientificNotationHeaderIsRejected(): void
|
||||
{
|
||||
$validator = new StandardHeaderValidator(self::registryWithMirroredTool());
|
||||
|
||||
$this->assertStringContainsString('does not match', (string) $validator->validate(
|
||||
'tools/call',
|
||||
['name' => 'mirrored', 'arguments' => ['retries' => 40]],
|
||||
['Mcp-Method' => 'tools/call', 'Mcp-Name' => 'mirrored', 'Mcp-Param-Retries' => '4e1'],
|
||||
));
|
||||
}
|
||||
|
||||
#[TestDox('a nested mirrored argument is read at its exact path')]
|
||||
public function testNestedMirroredArgumentIsChecked(): void
|
||||
{
|
||||
$validator = new StandardHeaderValidator(self::registryWithMirroredTool());
|
||||
|
||||
$this->assertNull($validator->validate(
|
||||
'tools/call',
|
||||
['name' => 'mirrored', 'arguments' => ['target' => ['region' => 'us-west1']]],
|
||||
['Mcp-Method' => 'tools/call', 'Mcp-Name' => 'mirrored', 'Mcp-Param-Region' => 'us-west1'],
|
||||
));
|
||||
|
||||
$this->assertStringContainsString('Missing required Mcp-Param-Region', (string) $validator->validate(
|
||||
'tools/call',
|
||||
['name' => 'mirrored', 'arguments' => ['target' => ['region' => 'us-west1']]],
|
||||
['Mcp-Method' => 'tools/call', 'Mcp-Name' => 'mirrored'],
|
||||
));
|
||||
|
||||
// Absent at that path, so no header is expected.
|
||||
$this->assertNull($validator->validate(
|
||||
'tools/call',
|
||||
['name' => 'mirrored', 'arguments' => []],
|
||||
['Mcp-Method' => 'tools/call', 'Mcp-Name' => 'mirrored'],
|
||||
));
|
||||
}
|
||||
|
||||
private static function registryWithMirroredTool(): RegistryInterface
|
||||
{
|
||||
$registry = new Registry();
|
||||
$registry->registerTool(
|
||||
new Tool(
|
||||
name: 'mirrored',
|
||||
title: null,
|
||||
inputSchema: [
|
||||
'type' => 'object',
|
||||
'properties' => [
|
||||
'retries' => ['type' => 'integer', 'x-mcp-header' => 'Retries'],
|
||||
'target' => [
|
||||
'type' => 'object',
|
||||
'properties' => ['region' => ['type' => 'string', 'x-mcp-header' => 'Region']],
|
||||
],
|
||||
],
|
||||
'required' => null,
|
||||
],
|
||||
description: 'x',
|
||||
annotations: null,
|
||||
),
|
||||
static fn (): string => 'ok',
|
||||
);
|
||||
|
||||
return $registry;
|
||||
}
|
||||
|
||||
#[TestDox('a plain header value decodes to itself')]
|
||||
public function testPlainValuePassesThroughDecode(): void
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in a new issue