fix(mcp): evaluate security when listing tools and resources

[Amoifr]

Q	A
Branch?	4.3
Tickets	Fix #8455
License	MIT
Doc PR	/

ListHandler returned every registered element, so an anonymous caller could read the name, the description and the full input schema of a tool it was not allowed to invoke. Invocation itself was already denied since #8435; only discovery leaked.

Elements whose operation-level security denies the current caller are now dropped from tools/list and resources/list.

The two design questions from the issue

Only security is evaluated. securityPostDenormalize and securityPostValidation need arguments and an object that do not exist at list time, so they cannot take part. The filter is therefore best-effort on security alone, which is what the class docblock now says.

Expressions that need call-time variables leave the element listed. An expression reading object, previous_object or a uri variable cannot be evaluated before the tool runs: the expression language rejects the unknown name, and the element stays visible. tools/call still enforces it, so nothing is granted that was not granted before. This mirrors what AccessCheckerProvider already does when it skips the pre_read stage for expressions using the object, so I did not add a configuration flag: hiding those elements would hide tools the caller is perfectly allowed to call, and no flag is needed to describe a behaviour that is already the codebase's convention. Happy to turn it into an option if you would rather have it explicit.

Notes

• Filtering happens after paging, so a page can hold fewer elements than the page size. The cursor still walks the whole registry, so no element is skipped, and the MCP spec puts no constraint on page size.
• The new constructor arguments are nullable and come last, so ListHandler keeps working unfiltered when security is not installed (ignoreOnInvalid() on the checker).
• SyntaxError is only referenced in a catch, and api-platform/mcp does not gain a dependency on symfony/expression-language, the same way Handler already uses RequestStack without requiring symfony/http-foundation.

Tests

• Unit: a denied tool is dropped, a granted one is kept, an expression needing call-time variables keeps the tool listed, and the same for resources/list.
• Functional, on the existing McpSecuredTools fixture: an anonymous caller no longer sees secured_tool but still sees secured_post_denormalize_tool, secured_post_validation_tool and secured_uri_variable_tool; an admin sees all of them. Both fail on 4.3 without the fix.

[soyuka]

We usually throw a RuntimeException instead of using assert

[Amoifr]

Done — isGranted() throws ApiPlatform\Metadata\Exception\RuntimeException when the metadata factory or the access checker is missing, instead of asserting.

I left the \assert($request instanceof ListToolsRequest) in handle() alone, since it predates this PR and Handler does the same thing a few lines away. Say the word if you would rather have both converted.

[soyuka]

let's use a loop to reduce complexity

[Amoifr]

Done, filterGranted() builds the list with a plain foreach now, no array_filter + array_values.

[soyuka]

Thanks for this! can you check my review?

[Amoifr]

Both applied, thanks for the review @soyuka. Unit tests and the functional McpSecurityTest are green, PHPStan clean on src/Mcp.

[soyuka]

I pushed three commits here, and opened the matching upstream PR: modelcontextprotocol/php-sdk#494.

Why the code moved

The original ListHandler re-implemented the SDK's ListToolsHandler and ListResourcesHandler verbatim. It only existed because Builder::build() eagerly loads a registry passed through setRegistry(), which both of our integrations do — so setLazyLoading(true) never applied and a cold-cache boot under a persistent runtime could freeze the registry empty.

Since custom handlers are prepended and dispatch is first-match, that copy also shadowed the SDK's handlers with a hardcoded page size of 20, silently overriding the configured mcp.pagination_limit. Replacing it with a RegistryInterface decorator hands tools/list back to the SDK and restores the configured page size.

The security decision then moved behind ElementAccessCheckerInterface, because the Symfony semantics do not port: Laravel reads policy, not security, and evaluates it through Gate, which never raises a SyntaxError. Its signal for "cannot decide at list time" is an ArgumentCountError — Gate::callPolicyMethod drops the class-string argument and calls the policy method with the user alone, so a method that needs a model instance throws instead of answering. Hence ExpressionAccessChecker and PolicyAccessChecker.

That third commit closes the actual gap: Laravel registered no list handler at all, so tools/list there was leaking policy-protected tools. #8455 was only half fixed.

What upstream removes, and what it does not

php-sdk#494 lets the builder defer loading into an empty custom registry. That removes the lazy-load half of SecureRegistry — the $loaded flag, the LoaderInterface argument, and the TODO.

It does not remove the decorator. Filtering listings by security still needs a seam, and today the only ones the SDK offers are a request handler (what we just deleted) or a registry decorator. Getting rid of it properly needs a second upstream step: a reference/list filter consulted inside Registry::getTools() while it fills a page. That would also fix the artefact we currently document — we filter after paging, so a page can come back short. I'd rather propose that separately than bundle it into #494.

The branch constraint

This targets 4.3, which pins mcp/sdk: ^0.6 || ^0.7. #494 would land in 0.8.x at the earliest, so nothing here can consume it on this branch — the lazy load stays for 4.3's lifetime regardless of how fast upstream moves.

So the choice is: merge now and let 4.4/5.0 shed the lazy-load half once the SDK floor moves, or hold a security fix waiting on an upstream release. I lean towards merging, but happy to be told otherwise.

[soyuka]

Thanks @Amoifr !