Add tool name validation for SEP-986

Author: chr-hertel

phpstan.dist.neon

@@ -15,6 +15,10 @@ parameters:
     ignoreErrors:
         -
             message: "#^Method .*::test.*\\(\\) has no return type specified\\.$#"
+        -
+            identifier: missingType.iterableValue
+            path: tests/
+
         # These errors should actually be fixed, but are ignored for now
         -
             identifier: missingType.iterableValue

src/Capability/Registry.php

@@ -18,6 +18,7 @@
 use Mcp\Capability\Registry\ResourceReference;
 use Mcp\Capability\Registry\ResourceTemplateReference;
 use Mcp\Capability\Registry\ToolReference;
+use Mcp\Capability\Tool\NameValidator;
 use Mcp\Event\PromptListChangedEvent;
 use Mcp\Event\ResourceListChangedEvent;
 use Mcp\Event\ResourceTemplateListChangedEvent;
@@ -71,6 +72,7 @@ final class Registry implements ReferenceProviderInterface, ReferenceRegistryInt
     public function __construct(
         private readonly ?EventDispatcherInterface $eventDispatcher = null,
         private readonly LoggerInterface $logger = new NullLogger(),
+        private readonly NameValidator $nameValidator = new NameValidator(),
     ) {
     }
 
@@ -100,12 +102,18 @@ public function registerTool(Tool $tool, callable|array|string $handler, bool $i
 
         if ($existing && !$isManual && $existing->isManual) {
             $this->logger->debug(
-                "Ignoring discovered tool '{$toolName}' as it conflicts with a manually registered one.",
+                \sprintf('Ignoring discovered tool "%s" as it conflicts with a manually registered one.', $toolName),
             );
 
             return;
         }
 
+        if (!$this->nameValidator->isValid($toolName)) {
+            $this->logger->warning(
+                \sprintf('Tool name "%s" is invalid. Tool names should only contain letters from a-z, numbers, dots, hyphens, underscores, and forward slashes.', $toolName),
+            );
+        }
+
         $this->tools[$toolName] = new ToolReference($tool, $handler, $isManual);
 
         $this->eventDispatcher?->dispatch(new ToolListChangedEvent());
@@ -118,7 +126,7 @@ public function registerResource(Resource $resource, callable|array|string $hand
 
         if ($existing && !$isManual && $existing->isManual) {
             $this->logger->debug(
-                "Ignoring discovered resource '{$uri}' as it conflicts with a manually registered one.",
+                \sprintf('Ignoring discovered resource "%s" as it conflicts with a manually registered one.', $uri),
             );
 
             return;
@@ -140,7 +148,7 @@ public function registerResourceTemplate(
 
         if ($existing && !$isManual && $existing->isManual) {
             $this->logger->debug(
-                "Ignoring discovered template '{$uriTemplate}' as it conflicts with a manually registered one.",
+                \sprintf('Ignoring discovered template "%s" as it conflicts with a manually registered one.', $uriTemplate),
             );
 
             return;
@@ -167,7 +175,7 @@ public function registerPrompt(
 
         if ($existing && !$isManual && $existing->isManual) {
             $this->logger->debug(
-                "Ignoring discovered prompt '{$promptName}' as it conflicts with a manually registered one.",
+                \sprintf('Ignoring discovered prompt "%s" as it conflicts with a manually registered one.', $promptName),
             );
 
             return;

src/Capability/Tool/NameValidator.php

@@ -0,0 +1,20 @@
+<?php
+
+/*
+ * This file is part of the official PHP MCP SDK.
+ *
+ * A collaboration between Symfony and the PHP Foundation.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Mcp\Capability\Tool;
+
+final class NameValidator
+{
+    public function isValid(string $name): bool
+    {
+        return 1 === preg_match('/^[a-zA-Z0-9._\/-]{1,64}$/', $name);
+    }
+}

tests/Inspector/InspectorSnapshotTestCase.php

@@ -117,7 +117,6 @@ protected function normalizeTestOutput(string $output, ?string $testName = null)
         return $output;
     }
 
-    /** @return array<string, array<string, mixed>> */
     public static function provideMethods(): array
     {
         return [

tests/Unit/Capability/Registry/RegistryTest.php

@@ -140,7 +140,7 @@ public function testRegisterToolIgnoresDiscoveredWhenManualExists(): void
         $this->logger
             ->expects($this->once())
             ->method('debug')
-            ->with("Ignoring discovered tool 'test_tool' as it conflicts with a manually registered one.");
+            ->with('Ignoring discovered tool "test_tool" as it conflicts with a manually registered one.');
 
         $this->registry->registerTool($discoveredTool, fn () => 'discovered', false);
 
@@ -181,7 +181,7 @@ public function testRegisterResourceIgnoresDiscoveredWhenManualExists(): void
         $this->logger
             ->expects($this->once())
             ->method('debug')
-            ->with("Ignoring discovered resource 'test://resource' as it conflicts with a manually registered one.");
+            ->with('Ignoring discovered resource "test://resource" as it conflicts with a manually registered one.');
 
         $this->registry->registerResource($discoveredResource, fn () => 'discovered', false);
 
@@ -210,7 +210,7 @@ public function testRegisterResourceTemplateIgnoresDiscoveredWhenManualExists():
         $this->logger
             ->expects($this->once())
             ->method('debug')
-            ->with("Ignoring discovered template 'test://{id}' as it conflicts with a manually registered one.");
+            ->with('Ignoring discovered template "test://{id}" as it conflicts with a manually registered one.');
 
         $this->registry->registerResourceTemplate($discoveredTemplate, fn () => 'discovered', [], false);
 
@@ -239,7 +239,7 @@ public function testRegisterPromptIgnoresDiscoveredWhenManualExists(): void
         $this->logger
             ->expects($this->once())
             ->method('debug')
-            ->with("Ignoring discovered prompt 'test_prompt' as it conflicts with a manually registered one.");
+            ->with('Ignoring discovered prompt "test_prompt" as it conflicts with a manually registered one.');
 
         $this->registry->registerPrompt($discoveredPrompt, fn () => 'discovered', [], false);
 

tests/Unit/Capability/Tool/NameValidatorTest.php

@@ -0,0 +1,56 @@
+<?php
+
+/*
+ * This file is part of the official PHP MCP SDK.
+ *
+ * A collaboration between Symfony and the PHP Foundation.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Mcp\Tests\Unit\Capability\Tool;
+
+use Mcp\Capability\Tool\NameValidator;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+final class NameValidatorTest extends TestCase
+{
+    #[DataProvider('provideValidNames')]
+    public function testValidNames(string $name): void
+    {
+        $this->assertTrue((new NameValidator())->isValid($name));
+    }
+
+    public static function provideValidNames(): array
+    {
+        return [
+            ['my_tool'],
+            ['MyTool123'],
+            ['my.tool'],
+            ['my-tool'],
+            ['my/tool'],
+            ['my_tool-01.02'],
+            ['my_long_toolname_that_is_exactly_sixty_four_characters_long_1234'],
+        ];
+    }
+
+    #[DataProvider('provideInvalidNames')]
+    public function testInvalidNames(string $name): void
+    {
+        $this->assertFalse((new NameValidator())->isValid($name));
+    }
+
+    public static function provideInvalidNames(): array
+    {
+        return [
+            [''],
+            ['my tool'],
+            ['my@tool'],
+            ['my!tool'],
+            ['my_tool#1'],
+            ['this_tool_name_is_way_too_long_because_it_exceeds_the_sixty_four_character_limit_set_by_the_validator'],
+        ];
+    }
+}