Skip to content

refactor(mcp): dedupe security and validator gates - #8457

Open
soyuka wants to merge 1 commit into
api-platform:4.3from
soyuka:refactor/mcp-single-security-validator-gate
Open

refactor(mcp): dedupe security and validator gates#8457
soyuka wants to merge 1 commit into
api-platform:4.3from
soyuka:refactor/mcp-single-security-validator-gate

Conversation

@soyuka

@soyuka soyuka commented Aug 16, 2026

Copy link
Copy Markdown
Member

Follow-up to #8435.

That PR fixed a real hole — with use_symfony_listeners: true, MCP tool calls bypassed security and validation entirely, because api_platform.mcp.handler was wired to the bare CallableProvider instead of a decorated chain. To decide whether to wire the new mcp/security.php and mcp/validator.php, it re-derived two gates inside load():

  • isset($bundles['SecurityBundle']) — canonical version already in registerSecurityConfiguration()
  • interface_exists(ValidatorInterface::class) — canonical version already in registerValidatorConfiguration()

They agree today. But if either canonical gate ever changes, MCP silently loses security again — which is exactly the bug class #8435 just closed. A duplicated security gate is a latent version of the same defect.

What changed

The MCP service files are now loaded from inside the two methods that already own those decisions:

  • mcp/security.php loads after registerSecurityConfiguration()'s SecurityBundle early return, so it is structurally unreachable when security is absent — there is no second condition left to desync.
  • mcp/validator.php and mcp/security_validator.php load inside isValidatorAvailable() branches, the same branches that own validator/*.php and state/security_validator.php.

A single $mcpProviderChain flag, derived once in load() from the existing MCP-enabled condition, is threaded into both methods.

Gate occurrences before after
isset($bundles['SecurityBundle']) 2 1
interface_exists(ValidatorInterface::class) 3 1

All three load conditions are preserved exactly; the block in load() collapses from 24 lines to 4.

Testing

Two DI tests added. Written first, then verified red by deliberately desyncing the duplicated gate against the original code — reproducing the #8435 failure mode:

Service "api_platform.mcp.state_provider.access_checker" not found.

The negative test asserts api_platform.mcp.handler is still registered without SecurityBundle, so it cannot pass trivially by MCP being disabled outright.

tests/Functional/McpSecurityTest.php and McpTest.php pass in both listener modes (22 tests, 172 assertions each). Note AppKernel::getCacheDir() is not keyed by USE_SYMFONY_LISTENERS, so the cache was cleared between modes to avoid replaying a stale compiled container.

Notes

  • Load order changes: mcp/validator.php / mcp/security.php now load before mcp/mcp.php and mcp/events.php. Safe — all six files use distinct service ids and only ->decorate(), which DecoratorServicePass resolves at compile time independent of definition order. Verified against the compiled container.
  • isValidatorAvailable() also collapses a pre-existing double interface_exists in registerValidatorConfiguration() / registerSecurityConfiguration().
  • $config['use_symfony_listeners'] still appears twice, selecting different file sets for different subsystems. Not the same drift hazard, left alone.

Branch 4.3 CI is currently broken for an unrelated reason (symfony/mercure v0.8.0 added two methods to HubInterface that the TestHub fixture does not implement, killing every job at container warmup). This PR's CI will not be meaningful until that lands.

PR api-platform#8435 re-derived isset($bundles['SecurityBundle']) and interface_exists(ValidatorInterface)
inside load() to decide whether to wire mcp/security.php and mcp/validator.php. Both gates already
exist in registerSecurityConfiguration() and registerValidatorConfiguration(). They agree today, but
changing a canonical gate would silently strip MCP of security again -- the exact bug api-platform#8435 fixed.

Load the MCP files from inside those two methods instead, guarded by a single $mcpProviderChain flag
derived once from the existing MCP-enabled condition. One place now decides "security available",
one decides "validator available".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant