From ecfadbaa808abeac2b3849cfc94c3f8550c92d6e Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Sat, 27 Dec 2025 17:21:47 +0100 Subject: [PATCH 01/10] Use SchemaValidator to validate tool input --- src/Schema/JsonRpc/Error.php | 10 ++++- .../Handler/Request/CallToolHandler.php | 39 ++++++++++++++++--- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/Schema/JsonRpc/Error.php b/src/Schema/JsonRpc/Error.php index d5273eb1..4ae25ca5 100644 --- a/src/Schema/JsonRpc/Error.php +++ b/src/Schema/JsonRpc/Error.php @@ -91,9 +91,15 @@ public static function forMethodNotFound(string $message, string|int $id = ''): return new self($id, self::METHOD_NOT_FOUND, $message); } - public static function forInvalidParams(string $message, string|int $id = ''): self + /** + * @param mixed $data + */ + public static function forInvalidParams(string $message, string|int $id = '', /* mixed $data = null */): self { - return new self($id, self::INVALID_PARAMS, $message); + // $data parameter was added in 0.2.2 and will be default in 0.3.0 + $data = func_get_args()[2] ?? null; + + return new self($id, self::INVALID_PARAMS, $message, data: $data); } public static function forInternalError(string $message, string|int $id = ''): self diff --git a/src/Server/Handler/Request/CallToolHandler.php b/src/Server/Handler/Request/CallToolHandler.php index c19b5b8c..edd124b5 100644 --- a/src/Server/Handler/Request/CallToolHandler.php +++ b/src/Server/Handler/Request/CallToolHandler.php @@ -11,6 +11,7 @@ namespace Mcp\Server\Handler\Request; +use Mcp\Capability\Discovery\SchemaValidator; use Mcp\Capability\Registry\ReferenceHandlerInterface; use Mcp\Capability\RegistryInterface; use Mcp\Exception\ToolCallException; @@ -33,11 +34,16 @@ */ final class CallToolHandler implements RequestHandlerInterface { + private SchemaValidator $schemaValidator; + public function __construct( private readonly RegistryInterface $registry, private readonly ReferenceHandlerInterface $referenceHandler, private readonly LoggerInterface $logger = new NullLogger(), + /* ?SchemaValidator $schemaValidator = null */ ) { + // $schemaValidator parameter was added in 0.2.2 and will be default in 0.3.0 + $this->schemaValidator = \func_get_args()[3] ?? new SchemaValidator($logger); } public function supports(Request $request): bool @@ -59,10 +65,35 @@ public function handle(Request $request, SessionInterface $session): Response|Er try { $reference = $this->registry->getTool($toolName); + } catch (ToolNotFoundException $e) { + $this->logger->error('Tool not found', ['name' => $toolName]); + + return new Error($request->getId(), Error::METHOD_NOT_FOUND, $e->getMessage()); + } + + $inputSchema = $reference->tool->inputSchema; + $validationErrors = $this->schemaValidator->validateAgainstJsonSchema($arguments, $inputSchema); + if (!empty($validationErrors)) { + $errorMessages = []; + + foreach ($validationErrors as $errorDetail) { + $pointer = $errorDetail['pointer'] ?? ''; + $message = $errorDetail['message'] ?? 'Unknown validation error'; + $errorMessages[] = ('/' !== $pointer && '' !== $pointer ? "Property '{$pointer}': " : '').$message; + } - $arguments['_session'] = $session; - $arguments['_request'] = $request; + $summaryMessage = "Invalid parameters for tool '{$toolName}': ".implode('; ', \array_slice($errorMessages, 0, 3)); + if (\count($errorMessages) > 3) { + $summaryMessage .= '; ...and more errors.'; + } + + return Error::forInvalidParams($summaryMessage, $request->getId(), ['validation_errors' => $validationErrors]); + } + $arguments['_session'] = $session; + $arguments['_request'] = $request; + + try { $result = $this->referenceHandler->handle($reference, $arguments); $structuredContent = null; @@ -87,10 +118,6 @@ public function handle(Request $request, SessionInterface $session): Response|Er $errorContent = [new TextContent($e->getMessage())]; return new Response($request->getId(), CallToolResult::error($errorContent)); - } catch (ToolNotFoundException $e) { - $this->logger->error('Tool not found', ['name' => $toolName]); - - return new Error($request->getId(), Error::METHOD_NOT_FOUND, $e->getMessage()); } catch (\Throwable $e) { $this->logger->error('Unhandled error during tool execution', [ 'name' => $toolName, From 33e4d14e63e15a5bc32f2827b6c7526209507ad8 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Sat, 27 Dec 2025 17:30:09 +0100 Subject: [PATCH 02/10] add test --- .../Handler/Request/CallToolHandlerTest.php | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php b/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php index 90af0c84..a46eac52 100644 --- a/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php +++ b/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php @@ -466,6 +466,43 @@ public function testHandleReturnsCallToolResult(): void $this->assertArrayNotHasKey('structuredContent', $response->result->jsonSerialize()); } + public function testValidationError(): void + { + $schema = [ + 'type' => 'object', + 'properties' => [ + 'favorite_number' => [ + 'type' => 'number', + 'description' => 'Your favorite number', + ], + ], + 'required' => [ + 'favorite_number', + ], + ]; + + $request = $this->createCallToolRequest('result_tool', ['query' => 'php']); + $toolReference = $this->getMockBuilder(ToolReference::class) + ->setConstructorArgs([new Tool('simple_tool', $schema, null, null), function () {}]) + ->getMock(); + + $this->registry + ->expects($this->once()) + ->method('getTool') + ->with('result_tool') + ->willReturn($toolReference); + + $this->referenceHandler + ->expects($this->never()) + ->method('handle'); + + $response = $this->handler->handle($request, $this->session); + + $this->assertInstanceOf(Error::class, $response); + $this->assertEquals($request->getId(), $response->id); + $this->assertEquals(Error::INVALID_PARAMS, $response->code); + } + /** * @param array $arguments */ From 0312c91642ec26abd15a13f18c208e67e64a9a0d Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Sat, 27 Dec 2025 17:37:17 +0100 Subject: [PATCH 03/10] add changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66238624..cb34bbd3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,11 @@ All notable changes to `mcp/sdk` will be documented in this file. -0.3 +0.3.0 ----- * Add output schema support to MCP tools +* Add validation of the input parameters given to a Tool. 0.2.2 ----- From dc8fa4d5045d011d2cba2a8baec5ffead02122d6 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Sat, 27 Dec 2025 17:49:26 +0100 Subject: [PATCH 04/10] fix cs --- phpstan-baseline.neon | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 3f8e6676..d8e4e2e8 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,5 +1,11 @@ parameters: ignoreErrors: + - + message: '#^PHPDoc tag @param references unknown parameter\: \$data$#' + identifier: parameter.notFound + count: 1 + path: src/Schema/JsonRpc/Error.php + - message: '#^Method Mcp\\Schema\\Result\\ReadResourceResult\:\:jsonSerialize\(\) should return array\{contents\: array\\} but returns array\{contents\: array\\}\.$#' identifier: return.type From 6e52b88776aefe2b877c715410ffb7952da58fa7 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Sat, 27 Dec 2025 18:10:45 +0100 Subject: [PATCH 05/10] Improve error message --- src/Capability/Discovery/SchemaValidator.php | 2 +- tests/Inspector/Http/HttpDiscoveryUserProfileTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Capability/Discovery/SchemaValidator.php b/src/Capability/Discovery/SchemaValidator.php index cc6be11d..a6a37b35 100644 --- a/src/Capability/Discovery/SchemaValidator.php +++ b/src/Capability/Discovery/SchemaValidator.php @@ -220,7 +220,7 @@ private function formatValidationError(ValidationError $error): string break; case 'type': $expected = implode('|', (array) ($args['expected'] ?? [])); - $used = $args['used'] ?? 'unknown'; + $used = $error->data()->type() ?? 'unknown'; $message = "Invalid type. Expected `{$expected}`, but received `{$used}`."; break; case 'enum': diff --git a/tests/Inspector/Http/HttpDiscoveryUserProfileTest.php b/tests/Inspector/Http/HttpDiscoveryUserProfileTest.php index 80114396..985b01b7 100644 --- a/tests/Inspector/Http/HttpDiscoveryUserProfileTest.php +++ b/tests/Inspector/Http/HttpDiscoveryUserProfileTest.php @@ -21,7 +21,7 @@ public static function provideMethods(): array 'method' => 'tools/call', 'options' => [ 'toolName' => 'send_welcome', - 'toolArgs' => ['userId' => '101', 'customMessage' => 'Welcome to our platform!'], + 'toolArgs' => ['userId' => '"101"', 'customMessage' => 'Welcome to our platform!'], ], 'testName' => 'send_welcome', ], From def47a3c1a238da5a84cb39e24468588d6d925e9 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Sat, 27 Dec 2025 18:20:27 +0100 Subject: [PATCH 06/10] fix tests --- src/Schema/JsonRpc/Error.php | 7 ++----- .../Server/Handler/Request/CallToolHandlerTest.php | 12 +++++++++++- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/Schema/JsonRpc/Error.php b/src/Schema/JsonRpc/Error.php index 4ae25ca5..e3fcf49f 100644 --- a/src/Schema/JsonRpc/Error.php +++ b/src/Schema/JsonRpc/Error.php @@ -91,13 +91,10 @@ public static function forMethodNotFound(string $message, string|int $id = ''): return new self($id, self::METHOD_NOT_FOUND, $message); } - /** - * @param mixed $data - */ - public static function forInvalidParams(string $message, string|int $id = '', /* mixed $data = null */): self + public static function forInvalidParams(string $message, string|int $id = '' /* mixed $data = null */): self { // $data parameter was added in 0.2.2 and will be default in 0.3.0 - $data = func_get_args()[2] ?? null; + $data = \func_get_args()[2] ?? null; return new self($id, self::INVALID_PARAMS, $message, data: $data); } diff --git a/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php b/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php index a46eac52..ce44f276 100644 --- a/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php +++ b/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php @@ -525,7 +525,17 @@ private function createToolReference( ?array $outputSchema = null, array $methodsToMock = ['formatResult'], ): ToolReference&MockObject { - $tool = new Tool($name, ['type' => 'object', 'properties' => [], 'required' => null], null, null, null, null, $outputSchema); + $schema = [ + 'type' => 'object', + 'properties' => [ + 'example' => [ + 'type' => 'string', + 'description' => 'This is just a dummy', + ], + ], + 'required' => [], + ]; + $tool = new Tool($name, $schema, null, null, null, null, $outputSchema); $builder = $this->getMockBuilder(ToolReference::class) ->setConstructorArgs([$tool, $handler]); From fd53dd21b64b1890e2c61a32781b3f0e64865956 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Sat, 27 Dec 2025 18:29:37 +0100 Subject: [PATCH 07/10] fix tests --- .../server/complex-tool-schema/Model/EventPriority.php | 8 ++++---- tests/Inspector/InspectorSnapshotTestCase.php | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/examples/server/complex-tool-schema/Model/EventPriority.php b/examples/server/complex-tool-schema/Model/EventPriority.php index eb046eff..97bdb70e 100644 --- a/examples/server/complex-tool-schema/Model/EventPriority.php +++ b/examples/server/complex-tool-schema/Model/EventPriority.php @@ -11,9 +11,9 @@ namespace Mcp\Example\Server\ComplexToolSchema\Model; -enum EventPriority: int +enum EventPriority: string { - case Low = 0; - case Normal = 1; - case High = 2; + case Low = 'low'; + case Normal = 'normal'; + case High = 'high'; } diff --git a/tests/Inspector/InspectorSnapshotTestCase.php b/tests/Inspector/InspectorSnapshotTestCase.php index 292670ae..18bb7f45 100644 --- a/tests/Inspector/InspectorSnapshotTestCase.php +++ b/tests/Inspector/InspectorSnapshotTestCase.php @@ -50,7 +50,7 @@ public function testOutputMatchesSnapshot( if (\is_array($value)) { $args[] = \sprintf('%s=%s', $key, json_encode($value)); } elseif (\is_bool($value)) { - $args[] = \sprintf('%s=%s', $key, $value ? '1' : '0'); + $args[] = \sprintf('%s=%s', $key, $value ? 'true' : 'false'); } else { $args[] = \sprintf('%s=%s', $key, $value); } @@ -73,7 +73,7 @@ public function testOutputMatchesSnapshot( if (\is_array($value)) { $args[] = \sprintf('%s=%s', $key, json_encode($value)); } elseif (\is_bool($value)) { - $args[] = \sprintf('%s=%s', $key, $value ? '1' : '0'); + $args[] = \sprintf('%s=%s', $key, $value ? 'true' : 'false'); } else { $args[] = \sprintf('%s=%s', $key, $value); } From 706185d5fb9727112cee14db2f0e25e2fe6d85b2 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Sat, 27 Dec 2025 18:30:20 +0100 Subject: [PATCH 08/10] fix baseline --- phpstan-baseline.neon | 6 ------ 1 file changed, 6 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index d8e4e2e8..3f8e6676 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,11 +1,5 @@ parameters: ignoreErrors: - - - message: '#^PHPDoc tag @param references unknown parameter\: \$data$#' - identifier: parameter.notFound - count: 1 - path: src/Schema/JsonRpc/Error.php - - message: '#^Method Mcp\\Schema\\Result\\ReadResourceResult\:\:jsonSerialize\(\) should return array\{contents\: array\\} but returns array\{contents\: array\\}\.$#' identifier: return.type From 61f3062853b30ccccf96f1f9b2d91313408b6cf8 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Sat, 27 Dec 2025 18:37:52 +0100 Subject: [PATCH 09/10] update snapshot files --- ...est-tools_call-schedule_event_all_day_reminder.json | 4 ++-- ...maTest-tools_call-schedule_event_high_priority.json | 4 ++-- ...emaTest-tools_call-schedule_event_low_priority.json | 4 ++-- .../HttpComplexToolSchemaTest-tools_list.json | 10 +++++----- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_all_day_reminder.json b/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_all_day_reminder.json index 5f6e553f..26888e22 100644 --- a/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_all_day_reminder.json +++ b/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_all_day_reminder.json @@ -2,7 +2,7 @@ "content": [ { "type": "text", - "text": "{\n \"success\": true,\n \"message\": \"Event \\\"Project Deadline\\\" scheduled successfully for \\\"2024-12-15\\\".\",\n \"event_details\": {\n \"title\": \"Project Deadline\",\n \"date\": \"2024-12-15\",\n \"type\": \"reminder\",\n \"time\": \"All day\",\n \"priority\": \"Normal\",\n \"attendees\": [],\n \"invites_will_be_sent\": false\n }\n}" + "text": "{\n \"success\": true,\n \"message\": \"Event \\\"Project Deadline\\\" scheduled successfully for \\\"2024-12-15\\\".\",\n \"event_details\": {\n \"title\": \"Project Deadline\",\n \"date\": \"2024-12-15\",\n \"type\": \"reminder\",\n \"time\": \"All day\",\n \"priority\": \"High\",\n \"attendees\": [],\n \"invites_will_be_sent\": false\n }\n}" } ], "structuredContent": { @@ -13,7 +13,7 @@ "date": "2024-12-15", "type": "reminder", "time": "All day", - "priority": "Normal", + "priority": "High", "attendees": [], "invites_will_be_sent": false } diff --git a/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_high_priority.json b/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_high_priority.json index 8a702475..23ee2994 100644 --- a/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_high_priority.json +++ b/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_high_priority.json @@ -2,7 +2,7 @@ "content": [ { "type": "text", - "text": "{\n \"success\": true,\n \"message\": \"Event \\\"Client Call\\\" scheduled successfully for \\\"2024-12-02\\\".\",\n \"event_details\": {\n \"title\": \"Client Call\",\n \"date\": \"2024-12-02\",\n \"type\": \"call\",\n \"time\": \"14:30\",\n \"priority\": \"Normal\",\n \"attendees\": [\n \"client@example.com\"\n ],\n \"invites_will_be_sent\": false\n }\n}" + "text": "{\n \"success\": true,\n \"message\": \"Event \\\"Client Call\\\" scheduled successfully for \\\"2024-12-02\\\".\",\n \"event_details\": {\n \"title\": \"Client Call\",\n \"date\": \"2024-12-02\",\n \"type\": \"call\",\n \"time\": \"14:30\",\n \"priority\": \"High\",\n \"attendees\": [\n \"client@example.com\"\n ],\n \"invites_will_be_sent\": false\n }\n}" } ], "structuredContent": { @@ -13,7 +13,7 @@ "date": "2024-12-02", "type": "call", "time": "14:30", - "priority": "Normal", + "priority": "High", "attendees": [ "client@example.com" ], diff --git a/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_low_priority.json b/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_low_priority.json index 97c2340e..36980277 100644 --- a/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_low_priority.json +++ b/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_low_priority.json @@ -2,7 +2,7 @@ "content": [ { "type": "text", - "text": "{\n \"success\": true,\n \"message\": \"Event \\\"Office Party\\\" scheduled successfully for \\\"2024-12-20\\\".\",\n \"event_details\": {\n \"title\": \"Office Party\",\n \"date\": \"2024-12-20\",\n \"type\": \"other\",\n \"time\": \"18:00\",\n \"priority\": \"Normal\",\n \"attendees\": [\n \"team@company.com\"\n ],\n \"invites_will_be_sent\": true\n }\n}" + "text": "{\n \"success\": true,\n \"message\": \"Event \\\"Office Party\\\" scheduled successfully for \\\"2024-12-20\\\".\",\n \"event_details\": {\n \"title\": \"Office Party\",\n \"date\": \"2024-12-20\",\n \"type\": \"other\",\n \"time\": \"18:00\",\n \"priority\": \"Low\",\n \"attendees\": [\n \"team@company.com\"\n ],\n \"invites_will_be_sent\": true\n }\n}" } ], "structuredContent": { @@ -13,7 +13,7 @@ "date": "2024-12-20", "type": "other", "time": "18:00", - "priority": "Normal", + "priority": "Low", "attendees": [ "team@company.com" ], diff --git a/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_list.json b/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_list.json index 5f47adca..58815df7 100644 --- a/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_list.json +++ b/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_list.json @@ -33,13 +33,13 @@ "default": null }, "priority": { - "type": "integer", + "type": "string", "description": "The priority of the event. Defaults to Normal.", - "default": 1, + "default": "normal", "enum": [ - 0, - 1, - 2 + "low", + "normal", + "high" ] }, "attendees": { From 04781f9fcc5b0f2aff080db2d77c32a2c3efe6a5 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Sun, 11 Jan 2026 21:41:41 +0100 Subject: [PATCH 10/10] Remove BC compat while switching to v0.3.0 --- src/Schema/JsonRpc/Error.php | 7 ++----- src/Server/Handler/Request/CallToolHandler.php | 5 ++--- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Schema/JsonRpc/Error.php b/src/Schema/JsonRpc/Error.php index e3fcf49f..422b295f 100644 --- a/src/Schema/JsonRpc/Error.php +++ b/src/Schema/JsonRpc/Error.php @@ -91,12 +91,9 @@ public static function forMethodNotFound(string $message, string|int $id = ''): return new self($id, self::METHOD_NOT_FOUND, $message); } - public static function forInvalidParams(string $message, string|int $id = '' /* mixed $data = null */): self + public static function forInvalidParams(string $message, string|int $id = '', mixed $data = null): self { - // $data parameter was added in 0.2.2 and will be default in 0.3.0 - $data = \func_get_args()[2] ?? null; - - return new self($id, self::INVALID_PARAMS, $message, data: $data); + return new self($id, self::INVALID_PARAMS, $message, $data); } public static function forInternalError(string $message, string|int $id = ''): self diff --git a/src/Server/Handler/Request/CallToolHandler.php b/src/Server/Handler/Request/CallToolHandler.php index edd124b5..ab92239d 100644 --- a/src/Server/Handler/Request/CallToolHandler.php +++ b/src/Server/Handler/Request/CallToolHandler.php @@ -40,10 +40,9 @@ public function __construct( private readonly RegistryInterface $registry, private readonly ReferenceHandlerInterface $referenceHandler, private readonly LoggerInterface $logger = new NullLogger(), - /* ?SchemaValidator $schemaValidator = null */ + ?SchemaValidator $schemaValidator = null, ) { - // $schemaValidator parameter was added in 0.2.2 and will be default in 0.3.0 - $this->schemaValidator = \func_get_args()[3] ?? new SchemaValidator($logger); + $this->schemaValidator = $schemaValidator ?? new SchemaValidator($logger); } public function supports(Request $request): bool