diff --git a/composer.json b/composer.json index 5c04813fcca..0295a010325 100644 --- a/composer.json +++ b/composer.json @@ -142,7 +142,7 @@ "jangregor/phpstan-prophecy": "^2.1.11", "justinrainbow/json-schema": "^6.5.2", "laravel/framework": "^11.0 || ^12.0 || ^13.0", - "mcp/sdk": "^0.6 || ^0.7", + "mcp/sdk": "^0.8", "orchestra/testbench": "^10.9 || ^11.0", "phpspec/prophecy-phpunit": "^2.2", "phpstan/extension-installer": "^1.1", @@ -176,7 +176,7 @@ "symfony/intl": "^6.4 || ^7.0 || ^8.0", "symfony/json-streamer": "^7.4 || ^8.0", "symfony/maker-bundle": "^1.24", - "symfony/mcp-bundle": "^0.12", + "symfony/mcp-bundle": "^0.13", "symfony/mercure-bundle": "^0.4.3|^0.5", "symfony/messenger": "^6.4 || ^7.0 || ^8.0", "symfony/object-mapper": "^7.4 || ^8.0", diff --git a/src/Laravel/ApiPlatformProvider.php b/src/Laravel/ApiPlatformProvider.php index 8eb0b5d4277..09aa4f9f335 100644 --- a/src/Laravel/ApiPlatformProvider.php +++ b/src/Laravel/ApiPlatformProvider.php @@ -109,9 +109,11 @@ use ApiPlatform\Laravel\State\SwaggerUiProvider; use ApiPlatform\Laravel\State\ValidateProvider; use ApiPlatform\Mcp\Capability\Registry\Loader as McpLoader; +use ApiPlatform\Mcp\Capability\Registry\SecureRegistry; use ApiPlatform\Mcp\JsonSchema\SchemaFactory as McpSchemaFactory; use ApiPlatform\Mcp\Metadata\Operation\Factory\OperationMetadataFactory as McpOperationMetadataFactory; use ApiPlatform\Mcp\Routing\IriConverter as McpIriConverter; +use ApiPlatform\Mcp\Security\PolicyAccessChecker; use ApiPlatform\Mcp\Server\Handler; use ApiPlatform\Mcp\State\StructuredContentProcessor; use ApiPlatform\Metadata\IdentifiersExtractor; @@ -180,6 +182,7 @@ use Illuminate\Support\Facades\Event; use Illuminate\Support\ServiceProvider; use Mcp\Capability\Registry; +use Mcp\Capability\RegistryInterface; use Mcp\Server; use Mcp\Server\Builder; use Mcp\Server\Session\InMemorySessionStore; @@ -1173,6 +1176,21 @@ private function registerMcp(): void }); $this->app->tag(McpLoader::class, 'mcp.loader'); + $this->app->singleton(PolicyAccessChecker::class, static function (Application $app) { + return new PolicyAccessChecker( + $app->make(McpOperationMetadataFactory::class), + $app->make(ResourceAccessCheckerInterface::class) + ); + }); + + $this->app->singleton(RegistryInterface::class, static function (Application $app) { + return new SecureRegistry( + $app->make(Registry::class), + $app->make(McpLoader::class), + $app->make(PolicyAccessChecker::class) + ); + }); + // TODO: add more stores? $this->app->singleton('mcp.session.store', static function () { return new InMemorySessionStore(3600); @@ -1190,7 +1208,7 @@ private function registerMcp(): void null // website_url todo ) ->setPaginationLimit(100) - ->setRegistry($app->make(Registry::class)) + ->setRegistry($app->make(RegistryInterface::class)) ->setSession($app->make('mcp.session.store')); foreach ($app->tagged('mcp.loader') as $loader) { diff --git a/src/Laravel/Tests/McpPolicyTest.php b/src/Laravel/Tests/McpPolicyTest.php new file mode 100644 index 00000000000..68c19bde2d2 --- /dev/null +++ b/src/Laravel/Tests/McpPolicyTest.php @@ -0,0 +1,139 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Laravel\Tests; + +use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Gate; +use Orchestra\Testbench\Concerns\WithWorkbench; +use Orchestra\Testbench\TestCase; +use Symfony\AI\McpBundle\McpBundle; +use Workbench\App\ApiResource\McpSecuredTools; + +class McpPolicyTest extends TestCase +{ + use RefreshDatabase; + use WithWorkbench; + + protected function defineEnvironment($app): void + { + Gate::guessPolicyNamesUsing(static function (string $modelClass) { + return McpSecuredTools::class === $modelClass ? + McpSecuredToolsPolicy::class : + null; + }); + } + + private function isPsr17FactoryAvailable(): bool + { + try { + if (!class_exists('Http\Discovery\Psr17FactoryDiscovery')) { + return false; + } + + \Http\Discovery\Psr17FactoryDiscovery::findServerRequestFactory(); + + return true; + } catch (\Throwable) { + return false; + } + } + + private function initializeMcpSession(): string + { + $response = $this->postJson('/mcp', [ + 'jsonrpc' => '2.0', + 'id' => 1, + 'method' => 'initialize', + 'params' => [ + 'protocolVersion' => '2024-11-05', + 'clientInfo' => [ + 'name' => 'ApiPlatform Test Suite', + 'version' => '1.0', + ], + 'capabilities' => [], + ], + ], [ + 'Accept' => 'application/json, text/event-stream', + 'Content-Type' => 'application/json', + ]); + + $response->assertStatus(200); + + return $response->headers->get('mcp-session-id'); + } + + /** + * @return list + */ + private function listToolNames(): array + { + $sessionId = $this->initializeMcpSession(); + $response = $this->postJson('/mcp', [ + 'jsonrpc' => '2.0', + 'id' => 2, + 'method' => 'tools/list', + ], [ + 'Accept' => 'application/json, text/event-stream', + 'Content-Type' => 'application/json', + 'mcp-session-id' => $sessionId, + ]); + + $response->assertStatus(200); + + return array_column($response->json('result.tools'), 'name'); + } + + public function testToolDeniedByPolicyIsNotListed(): void + { + if (!class_exists(McpBundle::class)) { + $this->markTestSkipped('MCP bundle is not installed'); + } + + if (!$this->isPsr17FactoryAvailable()) { + $this->markTestSkipped('PSR-17 HTTP factory implementation not available (required for MCP)'); + } + + $this->assertNotContains('secured_denied_tool', $this->listToolNames()); + } + + public function testToolGrantedByPolicyIsListed(): void + { + if (!class_exists(McpBundle::class)) { + $this->markTestSkipped('MCP bundle is not installed'); + } + + if (!$this->isPsr17FactoryAvailable()) { + $this->markTestSkipped('PSR-17 HTTP factory implementation not available (required for MCP)'); + } + + $this->assertContains('secured_granted_tool', $this->listToolNames()); + } + + public function testToolWhosePolicyNeedsTheModelStaysListed(): void + { + if (!class_exists(McpBundle::class)) { + $this->markTestSkipped('MCP bundle is not installed'); + } + + if (!$this->isPsr17FactoryAvailable()) { + $this->markTestSkipped('PSR-17 HTTP factory implementation not available (required for MCP)'); + } + + // Gate::callPolicyMethod shifts off a string first argument ("this policy already knows + // what type of models it can authorize") and then calls $policy->view($user), so a policy + // method requiring a model instance throws instead of answering, see + // vendor/laravel/framework/src/Illuminate/Auth/Access/Gate.php:825-839 + $this->assertContains('secured_model_tool', $this->listToolNames()); + } +} diff --git a/src/Laravel/Tests/McpSecuredToolsPolicy.php b/src/Laravel/Tests/McpSecuredToolsPolicy.php new file mode 100644 index 00000000000..d509b0c78c1 --- /dev/null +++ b/src/Laravel/Tests/McpSecuredToolsPolicy.php @@ -0,0 +1,35 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Laravel\Tests; + +use Illuminate\Foundation\Auth\User; +use Workbench\App\ApiResource\McpSecuredTools; + +class McpSecuredToolsPolicy +{ + public function viewAny(?User $user): bool + { + return false; + } + + public function create(?User $user): bool + { + return true; + } + + public function view(?User $user, McpSecuredTools $resource): bool + { + return true; + } +} diff --git a/src/Laravel/composer.json b/src/Laravel/composer.json index c33c55461fd..5f68b516064 100644 --- a/src/Laravel/composer.json +++ b/src/Laravel/composer.json @@ -65,7 +65,7 @@ "phpstan/phpdoc-parser": "^1.29 || ^2.0", "phpunit/phpunit": "^11.5 || ^12.2", "symfony/http-client": "^7.4 || ^8.0", - "symfony/mcp-bundle": "^0.12", + "symfony/mcp-bundle": "^0.13", "symfony/object-mapper": "^7.4 || ^8.0" }, "autoload": { diff --git a/src/Laravel/workbench/app/ApiResource/McpSecuredTools.php b/src/Laravel/workbench/app/ApiResource/McpSecuredTools.php new file mode 100644 index 00000000000..5e80142d697 --- /dev/null +++ b/src/Laravel/workbench/app/ApiResource/McpSecuredTools.php @@ -0,0 +1,60 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Workbench\App\ApiResource; + +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\McpTool; +use Mcp\Schema\Content\TextContent; +use Mcp\Schema\Result\CallToolResult; + +#[ApiResource( + shortName: 'McpSecuredTools', + operations: [], + mcp: [ + 'secured_denied_tool' => new McpTool( + processor: [self::class, 'process'], + policy: 'viewAny', + ), + 'secured_model_tool' => new McpTool( + processor: [self::class, 'process'], + policy: 'view', + ), + 'secured_granted_tool' => new McpTool( + processor: [self::class, 'process'], + policy: 'create', + ), + ] +)] +class McpSecuredTools +{ + public function __construct( + private ?string $text = null, + ) { + } + + public function getText(): ?string + { + return $this->text; + } + + public function setText(?string $text): void + { + $this->text = $text; + } + + public static function process(self $data): CallToolResult + { + return new CallToolResult([new TextContent('processed: '.$data->getText())]); + } +} diff --git a/src/Mcp/Capability/Registry/SecureRegistry.php b/src/Mcp/Capability/Registry/SecureRegistry.php new file mode 100644 index 00000000000..946d9181411 --- /dev/null +++ b/src/Mcp/Capability/Registry/SecureRegistry.php @@ -0,0 +1,259 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Mcp\Capability\Registry; + +use ApiPlatform\Mcp\Security\ElementAccessCheckerInterface; +use Mcp\Capability\Registry\Loader\LoaderInterface; +use Mcp\Capability\Registry\PromptReference; +use Mcp\Capability\Registry\ResourceReference; +use Mcp\Capability\Registry\ResourceTemplateReference; +use Mcp\Capability\Registry\ToolReference; +use Mcp\Capability\RegistryInterface; +use Mcp\Schema\Page; +use Mcp\Schema\Prompt; +use Mcp\Schema\ResourceDefinition; +use Mcp\Schema\ResourceTemplate; +use Mcp\Schema\Tool; + +/** + * Decorates the SDK registry, loading API Platform elements into it on first read. + * + * The SDK populates the registry once, when mcp.server is built. Under a persistent runtime + * (e.g. FrankenPHP worker mode) that single build can capture an empty registry (cold metadata + * cache) and stays empty for the whole process, so tools/list returns [] while tools/call keeps + * working through the request-time handler. Loading the API Platform elements lazily here heals + * that: it runs once per process (registrations are idempotent by name) and reads back through + * the shared registry, so runtime registrations and other registry decorators are preserved. + * + * Elements the configured ElementAccessCheckerInterface denies to the current caller are omitted from + * getTools()/getResources(), so a caller cannot discover the name, description and input schema of + * a tool it is not allowed to invoke. Nothing else is filtered: has*() is read by + * Builder::detectCapabilities() at build time, where there is no request to check against, and + * getTool()/getResource() are read by DiscoveryLoader during load for its identity check, plus + * AccessCheckerProvider already enforces security on tools/call and resources/read. + * + * @experimental + * TODO: drop the lazy load once the SDK can hand its loader to a registry passed to Builder::setRegistry() + */ +final class SecureRegistry implements RegistryInterface +{ + private bool $loaded = false; + + public function __construct( + private readonly RegistryInterface $inner, + private readonly LoaderInterface $loader, + private readonly ?ElementAccessCheckerInterface $accessChecker = null, + ) { + } + + public function registerTool(Tool $tool, callable|array|string $handler): ToolReference + { + return $this->inner->registerTool($tool, $handler); + } + + public function registerResource(ResourceDefinition $resource, callable|array|string $handler): ResourceReference + { + return $this->inner->registerResource($resource, $handler); + } + + public function registerResourceTemplate( + ResourceTemplate $template, + callable|array|string $handler, + array $completionProviders = [], + ): ResourceTemplateReference { + return $this->inner->registerResourceTemplate($template, $handler, $completionProviders); + } + + public function registerPrompt( + Prompt $prompt, + callable|array|string $handler, + array $completionProviders = [], + ): PromptReference { + return $this->inner->registerPrompt($prompt, $handler, $completionProviders); + } + + public function unregisterTool(string $name): void + { + $this->inner->unregisterTool($name); + } + + public function unregisterResource(string $uri): void + { + $this->inner->unregisterResource($uri); + } + + public function unregisterResourceTemplate(string $uriTemplate): void + { + $this->inner->unregisterResourceTemplate($uriTemplate); + } + + public function unregisterPrompt(string $name): void + { + $this->inner->unregisterPrompt($name); + } + + public function hasTool(string $name): bool + { + $this->load(); + + return $this->inner->hasTool($name); + } + + public function hasResource(string $uri): bool + { + $this->load(); + + return $this->inner->hasResource($uri); + } + + public function hasResourceTemplate(string $uriTemplate): bool + { + $this->load(); + + return $this->inner->hasResourceTemplate($uriTemplate); + } + + public function hasPrompt(string $name): bool + { + $this->load(); + + return $this->inner->hasPrompt($name); + } + + public function hasTools(): bool + { + $this->load(); + + return $this->inner->hasTools(); + } + + public function getTools(?int $limit = null, ?string $cursor = null): Page + { + $this->load(); + + $page = $this->inner->getTools($limit, $cursor); + $references = $this->filterGranted($page->references, static fn (Tool $tool): string => $tool->name); + + return new Page($references, $page->nextCursor); + } + + public function getTool(string $name): ToolReference + { + $this->load(); + + return $this->inner->getTool($name); + } + + public function hasResources(): bool + { + $this->load(); + + return $this->inner->hasResources(); + } + + public function getResources(?int $limit = null, ?string $cursor = null): Page + { + $this->load(); + + $page = $this->inner->getResources($limit, $cursor); + $references = $this->filterGranted($page->references, static fn (ResourceDefinition $resource): string => $resource->uri); + + return new Page($references, $page->nextCursor); + } + + public function getResource(string $uri, bool $includeTemplates = true): ResourceReference|ResourceTemplateReference + { + $this->load(); + + return $this->inner->getResource($uri, $includeTemplates); + } + + public function hasResourceTemplates(): bool + { + $this->load(); + + return $this->inner->hasResourceTemplates(); + } + + public function getResourceTemplates(?int $limit = null, ?string $cursor = null): Page + { + $this->load(); + + return $this->inner->getResourceTemplates($limit, $cursor); + } + + public function getResourceTemplate(string $uriTemplate): ResourceTemplateReference + { + $this->load(); + + return $this->inner->getResourceTemplate($uriTemplate); + } + + public function hasPrompts(): bool + { + $this->load(); + + return $this->inner->hasPrompts(); + } + + public function getPrompts(?int $limit = null, ?string $cursor = null): Page + { + $this->load(); + + return $this->inner->getPrompts($limit, $cursor); + } + + public function getPrompt(string $name): PromptReference + { + $this->load(); + + return $this->inner->getPrompt($name); + } + + private function load(): void + { + if (!$this->loaded) { + $this->loader->load($this->inner); + $this->loaded = true; + } + } + + /** + * Filtering happens after paging, so a page may hold fewer elements than the page size. The + * cursor still walks the whole registry, so no element is skipped. + * + * @template T of Tool|ResourceDefinition + * + * @param array $references + * @param callable(T): string $identify returns the operation name the reference maps to + * + * @return list + */ + private function filterGranted(array $references, callable $identify): array + { + if (null === $this->accessChecker) { + return array_values($references); + } + + $granted = []; + + foreach ($references as $reference) { + if ($this->accessChecker->isGranted($identify($reference))) { + $granted[] = $reference; + } + } + + return $granted; + } +} diff --git a/src/Mcp/Security/ElementAccessCheckerInterface.php b/src/Mcp/Security/ElementAccessCheckerInterface.php new file mode 100644 index 00000000000..d595ae53e43 --- /dev/null +++ b/src/Mcp/Security/ElementAccessCheckerInterface.php @@ -0,0 +1,24 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Mcp\Security; + +/** + * Decides whether the current caller may see a tool or resource in list results. + * + * @experimental + */ +interface ElementAccessCheckerInterface +{ + public function isGranted(string $operationName): bool; +} diff --git a/src/Mcp/Security/ExpressionAccessChecker.php b/src/Mcp/Security/ExpressionAccessChecker.php new file mode 100644 index 00000000000..71fa6dd236c --- /dev/null +++ b/src/Mcp/Security/ExpressionAccessChecker.php @@ -0,0 +1,57 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Mcp\Security; + +use ApiPlatform\Metadata\Operation\Factory\OperationMetadataFactoryInterface; +use ApiPlatform\Metadata\ResourceAccessCheckerInterface; +use Symfony\Component\ExpressionLanguage\SyntaxError; +use Symfony\Component\HttpFoundation\RequestStack; + +/** + * Evaluates the operation-level "security" expression, as used by the Symfony integration. + * + * @experimental + */ +final class ExpressionAccessChecker implements ElementAccessCheckerInterface +{ + public function __construct( + private readonly OperationMetadataFactoryInterface $operationMetadataFactory, + private readonly ?ResourceAccessCheckerInterface $resourceAccessChecker = null, + private readonly ?RequestStack $requestStack = null, + ) { + } + + public function isGranted(string $operationName): bool + { + if (null === $this->resourceAccessChecker) { + return true; + } + + $operation = $this->operationMetadataFactory->create($operationName); + + if (null === $operation || null === ($security = $operation->getSecurity())) { + return true; + } + + try { + return $this->resourceAccessChecker->isGranted($operation->getClass() ?? '', $security, ['request' => $this->requestStack?->getCurrentRequest()]); + } catch (SyntaxError) { + // The expression reads variables that only exist once the element is called (object, + // previous_object, uri variables). Listing cannot decide, so the element stays visible + // and the expression is enforced on tools/call and resources/read, as + // AccessCheckerProvider already defers the pre_read stage in that case. + return true; + } + } +} diff --git a/src/Mcp/Security/PolicyAccessChecker.php b/src/Mcp/Security/PolicyAccessChecker.php new file mode 100644 index 00000000000..c6901208238 --- /dev/null +++ b/src/Mcp/Security/PolicyAccessChecker.php @@ -0,0 +1,54 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Mcp\Security; + +use ApiPlatform\Metadata\Operation\Factory\OperationMetadataFactoryInterface; +use ApiPlatform\Metadata\ResourceAccessCheckerInterface; + +/** + * Evaluates the operation-level "policy", as used by the Laravel integration. + * + * @experimental + */ +final class PolicyAccessChecker implements ElementAccessCheckerInterface +{ + public function __construct( + private readonly OperationMetadataFactoryInterface $operationMetadataFactory, + private readonly ?ResourceAccessCheckerInterface $resourceAccessChecker = null, + ) { + } + + public function isGranted(string $operationName): bool + { + if (null === $this->resourceAccessChecker) { + return true; + } + + $operation = $this->operationMetadataFactory->create($operationName); + + if (null === $operation || null === ($policy = $operation->getPolicy())) { + return true; + } + + try { + return $this->resourceAccessChecker->isGranted($operation->getClass() ?? '', $policy, []); + } catch (\ArgumentCountError) { + // Gate::callPolicyMethod shifts off the policy name and calls $policy->{$method}($user), + // so a policy method that requires a model instance throws instead of answering. Listing + // cannot decide, so the element stays visible and the policy is enforced on tools/call + // and resources/read by AccessCheckerProvider. + return true; + } + } +} diff --git a/src/Mcp/Server/Handler.php b/src/Mcp/Server/Handler.php index 60de8cfa6d5..e6ea7dab268 100644 --- a/src/Mcp/Server/Handler.php +++ b/src/Mcp/Server/Handler.php @@ -14,6 +14,7 @@ namespace ApiPlatform\Mcp\Server; use ApiPlatform\Mcp\State\ToolProvider; +use ApiPlatform\Metadata\Exception\HttpExceptionInterface; use ApiPlatform\Metadata\HttpOperation; use ApiPlatform\Metadata\Operation\Factory\OperationMetadataFactoryInterface; use ApiPlatform\State\ProcessorInterface; @@ -131,7 +132,15 @@ public function handle(Request $request, SessionInterface $session): Response|Er $operation = $operation->withDeserialize(false); } - $body = $this->provider->provide($operation, $uriVariables, $context); + // The MCP transport has no HTTP response to carry a status code, so a caller-facing + // HttpExceptionInterface (e.g. access denied, validation) is converted into a JSON-RPC + // error carrying its message; anything else stays uncaught and reaches the SDK's own + // generic handler, which does not leak arbitrary exception messages to the client. + try { + $body = $this->provider->provide($operation, $uriVariables, $context); + } catch (HttpExceptionInterface $e) { + return Error::forInternalError($e->getMessage(), $request->getId()); + } if (!$isResource && null !== ($httpRequest = $context['request'] ?? null)) { $context['previous_data'] = $httpRequest->attributes->get('previous_data'); @@ -148,6 +157,10 @@ public function handle(Request $request, SessionInterface $session): Response|Er $operation = $operation->withSerialize(false); } - return $this->processor->process($body, $operation, $uriVariables, $context); + try { + return $this->processor->process($body, $operation, $uriVariables, $context); + } catch (HttpExceptionInterface $e) { + return Error::forInternalError($e->getMessage(), $request->getId()); + } } } diff --git a/src/Mcp/Server/ListHandler.php b/src/Mcp/Server/ListHandler.php deleted file mode 100644 index 3ac710dc8d1..00000000000 --- a/src/Mcp/Server/ListHandler.php +++ /dev/null @@ -1,82 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -declare(strict_types=1); - -namespace ApiPlatform\Mcp\Server; - -use Mcp\Capability\Registry\Loader\LoaderInterface; -use Mcp\Capability\RegistryInterface; -use Mcp\Schema\JsonRpc\Request; -use Mcp\Schema\JsonRpc\Response; -use Mcp\Schema\Request\ListResourcesRequest; -use Mcp\Schema\Request\ListToolsRequest; -use Mcp\Schema\Result\ListResourcesResult; -use Mcp\Schema\Result\ListToolsResult; -use Mcp\Server\Handler\Request\RequestHandlerInterface; -use Mcp\Server\Session\SessionInterface; - -/** - * Serves tools/list and resources/list from the MCP registry, loading API Platform elements - * into it on first use. - * - * The SDK populates the registry once, when mcp.server is built. Under a persistent runtime - * (e.g. FrankenPHP worker mode) that single build can capture an empty registry (cold metadata - * cache) and stays empty for the whole process, so tools/list returns [] while tools/call keeps - * working through the request-time {@see Handler}. Loading the API Platform elements lazily here - * heals that: it runs once per process (registrations are idempotent by name) and reads back - * through the shared registry, so runtime registrations and registry decorators are preserved. - * - * Tagged mcp.request_handler, it takes precedence over the SDK's registry-backed list handlers. - * - * @experimental - * TODO: remove once php-sdk:^0.7 has https://github.com/modelcontextprotocol/php-sdk/pull/389/changes - * - * @implements RequestHandlerInterface - */ -final class ListHandler implements RequestHandlerInterface -{ - private bool $loaded = false; - - public function __construct( - private readonly RegistryInterface $registry, - private readonly LoaderInterface $loader, - private readonly int $pageSize = 20, - ) { - } - - public function supports(Request $request): bool - { - return $request instanceof ListToolsRequest || $request instanceof ListResourcesRequest; - } - - /** - * @return Response - */ - public function handle(Request $request, SessionInterface $session): Response - { - if (!$this->loaded) { - $this->loader->load($this->registry); - $this->loaded = true; - } - - if ($request instanceof ListResourcesRequest) { - $page = $this->registry->getResources($this->pageSize, $request->cursor); - $result = new ListResourcesResult($page->references, $page->nextCursor); - } else { - \assert($request instanceof ListToolsRequest); - $page = $this->registry->getTools($this->pageSize, $request->cursor); - $result = new ListToolsResult($page->references, $page->nextCursor); - } - - return new Response($request->getId(), $result); - } -} diff --git a/src/Mcp/Tests/Capability/Registry/SecureRegistryTest.php b/src/Mcp/Tests/Capability/Registry/SecureRegistryTest.php new file mode 100644 index 00000000000..e01bf390b9f --- /dev/null +++ b/src/Mcp/Tests/Capability/Registry/SecureRegistryTest.php @@ -0,0 +1,251 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Mcp\Tests\Capability\Registry; + +use ApiPlatform\JsonSchema\Schema; +use ApiPlatform\JsonSchema\SchemaFactoryInterface; +use ApiPlatform\Mcp\Capability\Registry\Loader; +use ApiPlatform\Mcp\Capability\Registry\SecureRegistry; +use ApiPlatform\Mcp\Security\ElementAccessCheckerInterface; +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\McpResource; +use ApiPlatform\Metadata\McpTool; +use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; +use ApiPlatform\Metadata\Resource\Factory\ResourceNameCollectionFactoryInterface; +use ApiPlatform\Metadata\Resource\ResourceMetadataCollection; +use ApiPlatform\Metadata\Resource\ResourceNameCollection; +use Mcp\Capability\Registry; +use Mcp\Capability\Registry\Loader\LoaderInterface; +use Mcp\Capability\RegistryInterface; +use Mcp\Schema\Page; +use Mcp\Schema\Tool; +use PHPUnit\Framework\TestCase; + +class SecureRegistryTest extends TestCase +{ + public function testToolsAreLoadedIntoTheRegistryOnFirstRead(): void + { + $inputSchema = new Schema(Schema::VERSION_JSON_SCHEMA); + unset($inputSchema['$schema']); + $inputSchema['type'] = 'object'; + $inputSchema['properties'] = ['query' => ['type' => 'string']]; + + $schemaFactory = $this->createMock(SchemaFactoryInterface::class); + $schemaFactory->method('buildSchema')->willReturn($inputSchema); + + $mcpTool = new McpTool( + name: 'search', + description: 'Search things', + structuredContent: false, + class: \stdClass::class, + ); + + $resource = (new ApiResource(class: \stdClass::class))->withMcp(['search' => $mcpTool]); + + $registry = new SecureRegistry(new Registry(), $this->createLoader($resource, $schemaFactory)); + + $page = $registry->getTools(); + + $this->assertSame(['search'], array_map(static fn (Tool $tool): string => $tool->name, array_values($page->references))); + } + + public function testResourcesAreLoadedIntoTheRegistryOnFirstRead(): void + { + $mcpResource = new McpResource( + uri: 'dummy://docs', + name: 'docs', + description: 'Documentation resource', + mimeType: 'text/plain', + class: \stdClass::class, + ); + + $resource = (new ApiResource(class: \stdClass::class))->withMcp(['docs' => $mcpResource]); + + $registry = new SecureRegistry(new Registry(), $this->createLoader($resource, $this->createMock(SchemaFactoryInterface::class))); + + $page = $registry->getResources(); + + $this->assertSame(['dummy://docs'], array_column($page->references, 'uri')); + } + + public function testToolRegisteredAtRuntimeIsReturned(): void + { + $inner = new Registry(); + $inner->registerTool(new Tool(name: 'runtime_tool', title: null, inputSchema: ['type' => 'object', 'properties' => [], 'required' => null], description: null, annotations: null), 'runtime_handler'); + + $registry = new SecureRegistry($inner, $this->createMock(LoaderInterface::class)); + + $page = $registry->getTools(); + + $names = array_map(static fn (Tool $tool): string => $tool->name, array_values($page->references)); + $this->assertContains('runtime_tool', $names); + } + + public function testElementsAreLoadedExactlyOnce(): void + { + $inner = $this->createMock(RegistryInterface::class); + $inner->method('getTools')->willReturn(new Page([], null)); + + $loader = $this->createMock(LoaderInterface::class); + $loader->expects($this->once())->method('load'); + + $registry = new SecureRegistry($inner, $loader); + $registry->getTools(); + $registry->getTools(); + } + + public function testToolDeniedBySecurityIsOmittedFromGetTools(): void + { + $secured = new McpTool(name: 'secured', description: 'Secured', structuredContent: false, class: \stdClass::class, security: "is_granted('ROLE_ADMIN')"); + $public = new McpTool(name: 'public', description: 'Public', structuredContent: false, class: \stdClass::class); + + $accessChecker = $this->createMock(ElementAccessCheckerInterface::class); + $accessChecker->method('isGranted')->willReturnMap([ + ['secured', false], + ['public', true], + ]); + + $page = $this->buildToolRegistry([$secured, $public], $accessChecker)->getTools(); + + $this->assertSame(['public'], array_map(static fn (Tool $tool): string => $tool->name, array_values($page->references))); + } + + public function testToolGrantedBySecurityIsKept(): void + { + $secured = new McpTool(name: 'secured', description: 'Secured', structuredContent: false, class: \stdClass::class, security: "is_granted('ROLE_ADMIN')"); + + $accessChecker = $this->createMock(ElementAccessCheckerInterface::class); + $accessChecker->expects($this->once())->method('isGranted')->with('secured')->willReturn(true); + + $page = $this->buildToolRegistry([$secured], $accessChecker)->getTools(); + + $this->assertSame(['secured'], array_map(static fn (Tool $tool): string => $tool->name, array_values($page->references))); + } + + public function testToolStaysListedWhenAccessCheckerGrants(): void + { + $secured = new McpTool(name: 'secured', description: 'Secured', structuredContent: false, class: \stdClass::class, security: 'object.owner == user'); + + $accessChecker = $this->createMock(ElementAccessCheckerInterface::class); + $accessChecker->method('isGranted')->willReturn(true); + + $page = $this->buildToolRegistry([$secured], $accessChecker)->getTools(); + + $this->assertSame(['secured'], array_map(static fn (Tool $tool): string => $tool->name, array_values($page->references))); + } + + public function testResourceDeniedBySecurityIsOmittedFromGetResources(): void + { + $secured = new McpResource(uri: 'dummy://secured', name: 'secured', description: 'Secured', mimeType: 'text/plain', class: \stdClass::class, security: "is_granted('ROLE_ADMIN')"); + $public = new McpResource(uri: 'dummy://public', name: 'public', description: 'Public', mimeType: 'text/plain', class: \stdClass::class); + + $accessChecker = $this->createMock(ElementAccessCheckerInterface::class); + $accessChecker->method('isGranted')->willReturnMap([ + ['dummy://secured', false], + ['dummy://public', true], + ]); + + $apiResource = (new ApiResource(class: \stdClass::class))->withMcp(['secured' => $secured, 'public' => $public]); + $registry = new SecureRegistry( + new Registry(), + $this->createLoader($apiResource, $this->createMock(SchemaFactoryInterface::class)), + $accessChecker, + ); + + $page = $registry->getResources(); + + $this->assertSame(['dummy://public'], array_column($page->references, 'uri')); + } + + public function testNoFilteringWhenAccessCheckerIsNull(): void + { + $secured = new McpTool(name: 'secured', description: 'Secured', structuredContent: false, class: \stdClass::class, security: "is_granted('ROLE_ADMIN')"); + + $inputSchema = new Schema(Schema::VERSION_JSON_SCHEMA); + unset($inputSchema['$schema']); + $inputSchema['type'] = 'object'; + $inputSchema['properties'] = []; + + $schemaFactory = $this->createMock(SchemaFactoryInterface::class); + $schemaFactory->method('buildSchema')->willReturn($inputSchema); + + $resource = (new ApiResource(class: \stdClass::class))->withMcp(['secured' => $secured]); + + $registry = new SecureRegistry(new Registry(), $this->createLoader($resource, $schemaFactory)); + + $page = $registry->getTools(); + + $this->assertSame(['secured'], array_map(static fn (Tool $tool): string => $tool->name, array_values($page->references))); + } + + public function testGetToolStillReturnsReferenceForToolDeniedBySecurity(): void + { + $secured = new McpTool(name: 'secured', description: 'Secured', structuredContent: false, class: \stdClass::class, security: "is_granted('ROLE_ADMIN')"); + + $accessChecker = $this->createMock(ElementAccessCheckerInterface::class); + $accessChecker->method('isGranted')->willReturn(false); + + $reference = $this->buildToolRegistry([$secured], $accessChecker)->getTool('secured'); + + $this->assertSame('secured', $reference->tool->name); + } + + public function testHasToolsIsTrueEvenWhenEveryToolIsDenied(): void + { + $secured = new McpTool(name: 'secured', description: 'Secured', structuredContent: false, class: \stdClass::class, security: "is_granted('ROLE_ADMIN')"); + + $accessChecker = $this->createMock(ElementAccessCheckerInterface::class); + $accessChecker->method('isGranted')->willReturn(false); + + $this->assertTrue($this->buildToolRegistry([$secured], $accessChecker)->hasTools()); + } + + private function createLoader(ApiResource $resource, SchemaFactoryInterface $schemaFactory): Loader + { + $nameCollectionFactory = $this->createMock(ResourceNameCollectionFactoryInterface::class); + $nameCollectionFactory->method('create')->willReturn(new ResourceNameCollection([\stdClass::class])); + + $metadataCollectionFactory = $this->createMock(ResourceMetadataCollectionFactoryInterface::class); + $metadataCollectionFactory->method('create')->willReturn(new ResourceMetadataCollection(\stdClass::class, [$resource])); + + return new Loader($nameCollectionFactory, $metadataCollectionFactory, $schemaFactory); + } + + /** + * @param list $tools + */ + private function buildToolRegistry(array $tools, ElementAccessCheckerInterface $accessChecker): SecureRegistry + { + $inputSchema = new Schema(Schema::VERSION_JSON_SCHEMA); + unset($inputSchema['$schema']); + $inputSchema['type'] = 'object'; + $inputSchema['properties'] = []; + + $schemaFactory = $this->createMock(SchemaFactoryInterface::class); + $schemaFactory->method('buildSchema')->willReturn($inputSchema); + + $mcp = []; + foreach ($tools as $tool) { + $mcp[$tool->getName()] = $tool; + } + + $resource = (new ApiResource(class: \stdClass::class))->withMcp($mcp); + + return new SecureRegistry( + new Registry(), + $this->createLoader($resource, $schemaFactory), + $accessChecker, + ); + } +} diff --git a/src/Mcp/Tests/Security/ExpressionAccessCheckerTest.php b/src/Mcp/Tests/Security/ExpressionAccessCheckerTest.php new file mode 100644 index 00000000000..1c14ab6dd04 --- /dev/null +++ b/src/Mcp/Tests/Security/ExpressionAccessCheckerTest.php @@ -0,0 +1,91 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Mcp\Tests\Security; + +use ApiPlatform\Mcp\Security\ExpressionAccessChecker; +use ApiPlatform\Metadata\McpTool; +use ApiPlatform\Metadata\Operation\Factory\OperationMetadataFactoryInterface; +use ApiPlatform\Metadata\ResourceAccessCheckerInterface; +use PHPUnit\Framework\TestCase; +use Symfony\Component\ExpressionLanguage\SyntaxError; + +class ExpressionAccessCheckerTest extends TestCase +{ + public function testGrantedWhenOperationIsNotFound(): void + { + $operationMetadataFactory = $this->createMock(OperationMetadataFactoryInterface::class); + $operationMetadataFactory->method('create')->willReturn(null); + + $checker = new ExpressionAccessChecker($operationMetadataFactory, $this->createMock(ResourceAccessCheckerInterface::class)); + + $this->assertTrue($checker->isGranted('unknown')); + } + + public function testGrantedWhenOperationHasNoSecurity(): void + { + $operation = new McpTool(name: 'public', description: 'Public', structuredContent: false, class: \stdClass::class); + + $operationMetadataFactory = $this->createMock(OperationMetadataFactoryInterface::class); + $operationMetadataFactory->method('create')->willReturn($operation); + + $checker = new ExpressionAccessChecker($operationMetadataFactory, $this->createMock(ResourceAccessCheckerInterface::class)); + + $this->assertTrue($checker->isGranted('public')); + } + + public function testGrantedWhenAccessCheckerGrants(): void + { + $operation = new McpTool(name: 'secured', description: 'Secured', structuredContent: false, class: \stdClass::class, security: "is_granted('ROLE_ADMIN')"); + + $operationMetadataFactory = $this->createMock(OperationMetadataFactoryInterface::class); + $operationMetadataFactory->method('create')->willReturn($operation); + + $resourceAccessChecker = $this->createMock(ResourceAccessCheckerInterface::class); + $resourceAccessChecker->expects($this->once())->method('isGranted')->with(\stdClass::class, "is_granted('ROLE_ADMIN')")->willReturn(true); + + $checker = new ExpressionAccessChecker($operationMetadataFactory, $resourceAccessChecker); + + $this->assertTrue($checker->isGranted('secured')); + } + + public function testDeniedWhenAccessCheckerDenies(): void + { + $operation = new McpTool(name: 'secured', description: 'Secured', structuredContent: false, class: \stdClass::class, security: "is_granted('ROLE_ADMIN')"); + + $operationMetadataFactory = $this->createMock(OperationMetadataFactoryInterface::class); + $operationMetadataFactory->method('create')->willReturn($operation); + + $resourceAccessChecker = $this->createMock(ResourceAccessCheckerInterface::class); + $resourceAccessChecker->method('isGranted')->willReturn(false); + + $checker = new ExpressionAccessChecker($operationMetadataFactory, $resourceAccessChecker); + + $this->assertFalse($checker->isGranted('secured')); + } + + public function testGrantedWhenAccessCheckerThrowsSyntaxError(): void + { + $operation = new McpTool(name: 'secured', description: 'Secured', structuredContent: false, class: \stdClass::class, security: 'object.owner == user'); + + $operationMetadataFactory = $this->createMock(OperationMetadataFactoryInterface::class); + $operationMetadataFactory->method('create')->willReturn($operation); + + $resourceAccessChecker = $this->createMock(ResourceAccessCheckerInterface::class); + $resourceAccessChecker->method('isGranted')->willThrowException(new SyntaxError('Variable "object" is not valid')); + + $checker = new ExpressionAccessChecker($operationMetadataFactory, $resourceAccessChecker); + + $this->assertTrue($checker->isGranted('secured')); + } +} diff --git a/src/Mcp/Tests/Security/PolicyAccessCheckerTest.php b/src/Mcp/Tests/Security/PolicyAccessCheckerTest.php new file mode 100644 index 00000000000..b1abe082d73 --- /dev/null +++ b/src/Mcp/Tests/Security/PolicyAccessCheckerTest.php @@ -0,0 +1,105 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Mcp\Tests\Security; + +use ApiPlatform\Mcp\Security\PolicyAccessChecker; +use ApiPlatform\Metadata\McpTool; +use ApiPlatform\Metadata\Operation\Factory\OperationMetadataFactoryInterface; +use ApiPlatform\Metadata\ResourceAccessCheckerInterface; +use PHPUnit\Framework\TestCase; + +class PolicyAccessCheckerTest extends TestCase +{ + public function testGrantedWhenOperationIsNotFound(): void + { + $operationMetadataFactory = $this->createMock(OperationMetadataFactoryInterface::class); + $operationMetadataFactory->method('create')->willReturn(null); + + $checker = new PolicyAccessChecker($operationMetadataFactory, $this->createMock(ResourceAccessCheckerInterface::class)); + + $this->assertTrue($checker->isGranted('unknown')); + } + + public function testGrantedWhenOperationHasNoPolicy(): void + { + $operation = new McpTool(name: 'public', description: 'Public', structuredContent: false, class: \stdClass::class); + + $operationMetadataFactory = $this->createMock(OperationMetadataFactoryInterface::class); + $operationMetadataFactory->method('create')->willReturn($operation); + + $checker = new PolicyAccessChecker($operationMetadataFactory, $this->createMock(ResourceAccessCheckerInterface::class)); + + $this->assertTrue($checker->isGranted('public')); + } + + public function testGrantedWhenAccessCheckerGrants(): void + { + $operation = new McpTool(name: 'secured', description: 'Secured', structuredContent: false, class: \stdClass::class, policy: 'view'); + + $operationMetadataFactory = $this->createMock(OperationMetadataFactoryInterface::class); + $operationMetadataFactory->method('create')->willReturn($operation); + + $resourceAccessChecker = $this->createMock(ResourceAccessCheckerInterface::class); + $resourceAccessChecker->expects($this->once())->method('isGranted')->with(\stdClass::class, 'view', [])->willReturn(true); + + $checker = new PolicyAccessChecker($operationMetadataFactory, $resourceAccessChecker); + + $this->assertTrue($checker->isGranted('secured')); + } + + public function testDeniedWhenAccessCheckerDenies(): void + { + $operation = new McpTool(name: 'secured', description: 'Secured', structuredContent: false, class: \stdClass::class, policy: 'view'); + + $operationMetadataFactory = $this->createMock(OperationMetadataFactoryInterface::class); + $operationMetadataFactory->method('create')->willReturn($operation); + + $resourceAccessChecker = $this->createMock(ResourceAccessCheckerInterface::class); + $resourceAccessChecker->method('isGranted')->willReturn(false); + + $checker = new PolicyAccessChecker($operationMetadataFactory, $resourceAccessChecker); + + $this->assertFalse($checker->isGranted('secured')); + } + + public function testGrantedWhenAccessCheckerThrowsArgumentCountError(): void + { + $operation = new McpTool(name: 'secured', description: 'Secured', structuredContent: false, class: \stdClass::class, policy: 'view'); + + $operationMetadataFactory = $this->createMock(OperationMetadataFactoryInterface::class); + $operationMetadataFactory->method('create')->willReturn($operation); + + $resourceAccessChecker = $this->createMock(ResourceAccessCheckerInterface::class); + $resourceAccessChecker->method('isGranted')->willThrowException(new \ArgumentCountError('Too few arguments')); + + $checker = new PolicyAccessChecker($operationMetadataFactory, $resourceAccessChecker); + + $this->assertTrue($checker->isGranted('secured')); + } + + public function testSecurityIsIgnoredWhenPolicyIsAbsent(): void + { + $operation = new McpTool(name: 'secured', description: 'Secured', structuredContent: false, class: \stdClass::class, security: "is_granted('ROLE_ADMIN')"); + + $operationMetadataFactory = $this->createMock(OperationMetadataFactoryInterface::class); + $operationMetadataFactory->method('create')->willReturn($operation); + + $resourceAccessChecker = $this->createMock(ResourceAccessCheckerInterface::class); + $resourceAccessChecker->expects($this->never())->method('isGranted'); + + $checker = new PolicyAccessChecker($operationMetadataFactory, $resourceAccessChecker); + + $this->assertTrue($checker->isGranted('secured')); + } +} diff --git a/src/Mcp/Tests/Server/ListHandlerTest.php b/src/Mcp/Tests/Server/ListHandlerTest.php deleted file mode 100644 index a8bfd4bdf4a..00000000000 --- a/src/Mcp/Tests/Server/ListHandlerTest.php +++ /dev/null @@ -1,140 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -declare(strict_types=1); - -namespace ApiPlatform\Mcp\Tests\Server; - -use ApiPlatform\JsonSchema\Schema; -use ApiPlatform\JsonSchema\SchemaFactoryInterface; -use ApiPlatform\Mcp\Capability\Registry\Loader; -use ApiPlatform\Mcp\Server\ListHandler; -use ApiPlatform\Metadata\ApiResource; -use ApiPlatform\Metadata\McpResource; -use ApiPlatform\Metadata\McpTool; -use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; -use ApiPlatform\Metadata\Resource\Factory\ResourceNameCollectionFactoryInterface; -use ApiPlatform\Metadata\Resource\ResourceMetadataCollection; -use ApiPlatform\Metadata\Resource\ResourceNameCollection; -use Mcp\Capability\Registry; -use Mcp\Capability\Registry\Loader\LoaderInterface; -use Mcp\Capability\RegistryInterface; -use Mcp\Schema\Request\ListResourcesRequest; -use Mcp\Schema\Request\ListToolsRequest; -use Mcp\Schema\Result\ListResourcesResult; -use Mcp\Schema\Result\ListToolsResult; -use Mcp\Schema\Tool; -use Mcp\Server\Session\SessionInterface; -use PHPUnit\Framework\TestCase; - -class ListHandlerTest extends TestCase -{ - public function testListToolsLoadsApiPlatformElementsIntoTheRegistry(): void - { - $inputSchema = new Schema(Schema::VERSION_JSON_SCHEMA); - unset($inputSchema['$schema']); - $inputSchema['type'] = 'object'; - $inputSchema['properties'] = ['query' => ['type' => 'string']]; - - $schemaFactory = $this->createMock(SchemaFactoryInterface::class); - $schemaFactory->method('buildSchema')->willReturn($inputSchema); - - $mcpTool = new McpTool( - name: 'search', - description: 'Search things', - structuredContent: false, - class: \stdClass::class, - ); - - $resource = (new ApiResource(class: \stdClass::class))->withMcp(['search' => $mcpTool]); - - $registry = new Registry(); - $handler = new ListHandler($registry, $this->createLoader($resource, $schemaFactory)); - - $result = $handler->handle((new ListToolsRequest())->withId(1), $this->createMock(SessionInterface::class))->result; - - $this->assertInstanceOf(ListToolsResult::class, $result); - $this->assertCount(1, $result->tools); - $this->assertSame('search', $result->tools[0]->name); - } - - public function testListResourcesLoadsApiPlatformElementsIntoTheRegistry(): void - { - $mcpResource = new McpResource( - uri: 'dummy://docs', - name: 'docs', - description: 'Documentation resource', - mimeType: 'text/plain', - class: \stdClass::class, - ); - - $resource = (new ApiResource(class: \stdClass::class))->withMcp(['docs' => $mcpResource]); - - $registry = new Registry(); - $handler = new ListHandler($registry, $this->createLoader($resource, $this->createMock(SchemaFactoryInterface::class))); - - $result = $handler->handle((new ListResourcesRequest())->withId(1), $this->createMock(SessionInterface::class))->result; - - $this->assertInstanceOf(ListResourcesResult::class, $result); - $this->assertCount(1, $result->resources); - $this->assertSame('dummy://docs', $result->resources[0]->uri); - } - - /** - * Reading through the shared registry (rather than a private one) keeps tools registered at - * runtime — e.g. dynamically discovered affordances — visible in tools/list. - */ - public function testListToolsIncludesToolsRegisteredAtRuntime(): void - { - $registry = new Registry(); - $registry->registerTool(new Tool(name: 'runtime_tool', title: null, inputSchema: ['type' => 'object', 'properties' => [], 'required' => null], description: null, annotations: null), 'runtime_handler'); - - $loader = $this->createMock(LoaderInterface::class); - $handler = new ListHandler($registry, $loader); - - $result = $handler->handle((new ListToolsRequest())->withId(1), $this->createMock(SessionInterface::class))->result; - - $names = array_map(static fn (Tool $t): string => $t->name, $result->tools); - $this->assertContains('runtime_tool', $names); - } - - public function testElementsAreLoadedOncePerProcess(): void - { - $registry = $this->createMock(RegistryInterface::class); - $registry->method('getTools')->willReturn(new \Mcp\Schema\Page([], null)); - - $loader = $this->createMock(LoaderInterface::class); - $loader->expects($this->once())->method('load'); - - $handler = new ListHandler($registry, $loader); - $handler->handle((new ListToolsRequest())->withId(1), $this->createMock(SessionInterface::class)); - $handler->handle((new ListToolsRequest())->withId(2), $this->createMock(SessionInterface::class)); - } - - public function testSupportsListRequests(): void - { - $handler = new ListHandler($this->createMock(RegistryInterface::class), $this->createMock(LoaderInterface::class)); - - $this->assertTrue($handler->supports(new ListToolsRequest())); - $this->assertTrue($handler->supports(new ListResourcesRequest())); - } - - private function createLoader(ApiResource $resource, SchemaFactoryInterface $schemaFactory): Loader - { - $nameCollectionFactory = $this->createMock(ResourceNameCollectionFactoryInterface::class); - $nameCollectionFactory->method('create')->willReturn(new ResourceNameCollection([\stdClass::class])); - - $metadataCollectionFactory = $this->createMock(ResourceMetadataCollectionFactoryInterface::class); - $metadataCollectionFactory->method('create')->willReturn(new ResourceMetadataCollection(\stdClass::class, [$resource])); - - return new Loader($nameCollectionFactory, $metadataCollectionFactory, $schemaFactory); - } -} diff --git a/src/Mcp/composer.json b/src/Mcp/composer.json index f6531135ea0..7c2f893551c 100644 --- a/src/Mcp/composer.json +++ b/src/Mcp/composer.json @@ -30,7 +30,7 @@ "php": ">=8.2", "api-platform/metadata": "^4.3", "api-platform/json-schema": "^4.3", - "mcp/sdk": "^0.6 || ^0.7", + "mcp/sdk": "^0.8", "symfony/object-mapper": "^7.4 || ^8.0", "symfony/polyfill-php85": "^1.32" }, diff --git a/src/Symfony/Bundle/ApiPlatformBundle.php b/src/Symfony/Bundle/ApiPlatformBundle.php index 3b034ecbfef..f887630f0d5 100644 --- a/src/Symfony/Bundle/ApiPlatformBundle.php +++ b/src/Symfony/Bundle/ApiPlatformBundle.php @@ -23,6 +23,7 @@ use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\GraphQlResolverPass; use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\GraphQlTypePass; use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\JsonStreamerTransformerPass; +use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\McpRegistryPass; use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\MetadataAwareNameConverterPass; use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\MutatorPass; use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\PropertyInfoTagPass; @@ -63,6 +64,7 @@ public function build(ContainerBuilder $container): void $container->addCompilerPass(new SerializerMappingLoaderPass()); $container->addCompilerPass(new ErrorResourceAttributeLoaderPass()); $container->addCompilerPass(new MutatorPass()); + $container->addCompilerPass(new McpRegistryPass()); $container->addCompilerPass(new PropertyInfoTagPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, -100); // Must run after Symfony's TransformerPass so we can rely on the value_object_transformer tag being processed. $container->addCompilerPass(new JsonStreamerTransformerPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, -10); diff --git a/src/Symfony/Bundle/DependencyInjection/Compiler/McpRegistryPass.php b/src/Symfony/Bundle/DependencyInjection/Compiler/McpRegistryPass.php new file mode 100644 index 00000000000..55b83955cba --- /dev/null +++ b/src/Symfony/Bundle/DependencyInjection/Compiler/McpRegistryPass.php @@ -0,0 +1,69 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler; + +use ApiPlatform\Mcp\Capability\Registry\SecureRegistry; +use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Reference; + +/** + * symfony/mcp-bundle 0.13 registers one registry per configured server under a dynamic id + * (mcp.server..registry), so decorating it can no longer be done with a static + * decorate() call in configuration: this pass discovers every server via its builder tag + * and decorates its registry individually. + * + * Decoration keeps the SDK's own list handlers in charge (they receive the configured + * mcp.pagination_limit, which a custom handler would silently override), while loading + * API Platform elements on first read heals a persistent runtime (e.g. FrankenPHP worker + * mode) where the SDK builds the registry once and may capture an empty state. + */ +final class McpRegistryPass implements CompilerPassInterface +{ + public function process(ContainerBuilder $container): void + { + if (!$container->hasDefinition('api_platform.mcp.loader') || !$container->hasDefinition('api_platform.mcp.security.expression_access_checker')) { + return; + } + + foreach ($container->findTaggedServiceIds('mcp.server.builder') as $tags) { + foreach ($tags as $tag) { + $server = $tag['server'] ?? null; + + if (null === $server) { + continue; + } + + $registryId = \sprintf('mcp.server.%s.registry', $server); + + if (!$container->hasDefinition($registryId)) { + continue; + } + + $decoratorId = \sprintf('api_platform.mcp.secure_registry.%s', $server); + + $definition = new Definition(SecureRegistry::class); + $definition->setDecoratedService($registryId); + $definition->setArguments([ + new Reference($decoratorId.'.inner'), + new Reference('api_platform.mcp.loader'), + new Reference('api_platform.mcp.security.expression_access_checker'), + ]); + + $container->setDefinition($decoratorId, $definition); + } + } + } +} diff --git a/src/Symfony/Bundle/Resources/config/mcp/mcp.php b/src/Symfony/Bundle/Resources/config/mcp/mcp.php index 4740dde1f38..4f3d9f0973e 100644 --- a/src/Symfony/Bundle/Resources/config/mcp/mcp.php +++ b/src/Symfony/Bundle/Resources/config/mcp/mcp.php @@ -17,7 +17,7 @@ use ApiPlatform\Mcp\JsonSchema\SchemaFactory; use ApiPlatform\Mcp\Metadata\Operation\Factory\OperationMetadataFactory; use ApiPlatform\Mcp\Routing\IriConverter; -use ApiPlatform\Mcp\Server\ListHandler; +use ApiPlatform\Mcp\Security\ExpressionAccessChecker; use ApiPlatform\Mcp\State\ToolProvider; return static function (ContainerConfigurator $container) { @@ -36,17 +36,12 @@ ]) ->tag('mcp.loader'); - // Serves tools/list and resources/list, loading API Platform elements into the registry on - // first use. This heals a persistent runtime (e.g. FrankenPHP worker mode) where the SDK - // builds the registry once and may capture an empty state. Reads back through the shared - // registry so runtime registrations and decorators are preserved. Takes precedence over the - // SDK's registry-backed list handlers. - $services->set('api_platform.mcp.list_handler', ListHandler::class) + $services->set('api_platform.mcp.security.expression_access_checker', ExpressionAccessChecker::class) ->args([ - service('mcp.registry'), - service('api_platform.mcp.loader'), - ]) - ->tag('mcp.request_handler'); + service('api_platform.mcp.metadata.operation.mcp_factory'), + service('api_platform.security.resource_access_checker')->ignoreOnInvalid(), + service('request_stack'), + ]); $services->set('api_platform.mcp.iri_converter', IriConverter::class) ->decorate('api_platform.iri_converter', null, 300) diff --git a/tests/Fixtures/app/config/config_common.yml b/tests/Fixtures/app/config/config_common.yml index 48da39cf1b4..c2e5bbc4867 100644 --- a/tests/Fixtures/app/config/config_common.yml +++ b/tests/Fixtures/app/config/config_common.yml @@ -95,15 +95,15 @@ api_platform: include_type: true mcp: - client_transports: - http: true - stdio: false - http: - path: '/mcp' - session: - store: 'file' - directory: '%kernel.cache_dir%/mcp' - ttl: 3600 + servers: + default: + http: + path: '/mcp' + session: + store: 'file' + directory: '%kernel.cache_dir%/mcp' + ttl: 3600 + registry: '*' services: test.client: diff --git a/tests/Functional/McpSecurityTest.php b/tests/Functional/McpSecurityTest.php index cbeaa26ad58..8316a869f86 100644 --- a/tests/Functional/McpSecurityTest.php +++ b/tests/Functional/McpSecurityTest.php @@ -85,6 +85,34 @@ public function testAdminCanCallSecuredTool(string $tool, array $arguments): voi self::assertStringContainsString('Secured: hello', $result['result']['content'][0]['text'] ?? ''); } + public function testAnonymousCannotDiscoverToolsItCannotCall(): void + { + $this->skipUnlessMcpIsAvailable(); + + $client = self::createClient(); + $names = $this->listToolNames($client, $this->initializeMcpSession($client)); + + self::assertNotContains('secured_tool', $names, 'An anonymous caller discovered a tool it cannot invoke.'); + + // Only the operation level "security" can be evaluated before the tool runs: the other + // expressions need arguments, an object or uri variables, so those tools stay listed and + // are enforced on tools/call. + self::assertContains('secured_post_denormalize_tool', $names); + self::assertContains('secured_post_validation_tool', $names); + self::assertContains('secured_uri_variable_tool', $names); + } + + public function testAdminDiscoversSecuredTool(): void + { + $this->skipUnlessMcpIsAvailable(); + + $client = self::createClient(); + $sessionId = $this->initializeMcpSession($client); + $names = $this->listToolNames($client, $sessionId, ['Authorization' => self::ADMIN_AUTH]); + + self::assertContains('secured_tool', $names); + } + private function skipUnlessMcpIsAvailable(): void { if (!class_exists(McpBundle::class)) { @@ -152,4 +180,30 @@ private function callTool($client, string $sessionId, string $toolName, array $a ], ]); } + + /** + * @param array $headers + * + * @return list + */ + private function listToolNames($client, string $sessionId, array $headers = []): array + { + $result = $client->request('POST', '/mcp', [ + 'headers' => $headers + [ + 'Accept' => 'application/json, text/event-stream', + 'Content-Type' => 'application/json', + 'mcp-session-id' => $sessionId, + ], + 'json' => [ + 'jsonrpc' => '2.0', + 'id' => 2, + 'method' => 'tools/list', + 'params' => [], + ], + ])->toArray(false); + + self::assertArrayNotHasKey('error', $result, 'MCP error: '.json_encode($result['error'] ?? null)); + + return array_column($result['result']['tools'] ?? [], 'name'); + } }