Enforce the MCP admin gate independently of the adapter (1.4.3) - #30
Open
trueqap wants to merge 2 commits into
Open
Enforce the MCP admin gate independently of the adapter (1.4.3)#30trueqap wants to merge 2 commits into
trueqap wants to merge 2 commits into
Conversation
Adapter 0.6.0 added automattic/jetpack-autoloader as a runtime dependency. It is a Composer plugin, so without an allow-plugins entry composer install aborts — which is why every job on the dependency update PR failed at the install step, not at any test. Verified before bundling it: every adapter hook this plugin uses still exists in 0.6.1, and McpNameSanitizer still produces the mcp-adapter-execute-ability tool name ResultUnwrapper matches on. The release ZIP was rebuilt and all 345 classmap entries resolve inside the artifact, so the 0.6.0 ZIP defect that mapped WP_CLI to an omitted test file does not apply to this Composer-built bundle. The release workflow's bundle step is pinned separately from require-dev, so it is bumped to ^0.6 in the same commit; leaving it at ^0.5 would have shipped a ZIP that disagrees with the repository.
The adapter applies its transport check as
apply_filters( 'mcp_adapter_default_transport_permission_user_capability', 'read', ... )
Mcp\TransportGuard raises that to manage_options, but only for as long as that
one filter is actually applied. If it ever is not — an upstream refactor, a hook
rename, or a different bundled copy of the library winning the autoload race,
which already happens on every WooCommerce store — the endpoint falls back to
'read', a capability every logged-in subscriber holds. A single point of failure
for an administrator-level surface, and the fallback is present in every adapter
version, 0.5.0 included; it was never a weakness of the older copy specifically.
Mcp\RouteGuard enforces the same capability on WordPress's own rest_pre_dispatch,
so the gate holds regardless of which adapter is loaded or whether its filters
fire. Registered before the enabled check on purpose: it only ever denies
requests to our own route, so it costs nothing when the server is off, and it
must not depend on our own bootstrapping having got that far.
Scoped deliberately: it matches this plugin's server route and anything beneath
it, but not another plugin's MCP endpoint (there are four more on the test store
alone) and not a route that merely shares the prefix. It also passes through any
result an earlier filter already produced, so it can only ever add a denial.
Verified live: a subscriber gets 403, an unauthenticated caller 401, an
administrator is unaffected, and FluentCRM's MCP route is untouched.
Covered by tests/Unit/Mcp/RouteGuardTest.php (6 cases, red before, green after).
Bump to 1.4.3.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the last thread from the field report — the one I deliberately left out of #29 because it is a security layer and warranted its own review.
The problem
The adapter applies its transport check like this:
Note the default.
Mcp\TransportGuardraises it tomanage_options, but only for as long as that one filter is actually applied. If it ever is not — an upstream refactor, a hook rename, or a different bundled copy of the library winning the autoload race (which already happens on every WooCommerce store) — the endpoint quietly falls back toread, which every logged-in subscriber holds.One hook not firing = an administrator-level endpoint open to any logged-in user. And this fallback is in every adapter version, 0.5.0 included — the field report framed it as a 0.3.0 weakness, but it is not version-specific.
This matters more now that the MCP server ships enabled by default: the gate applies to every install.
The fix
Mcp\RouteGuardenforces the same capability on WordPress's ownrest_pre_dispatch, so it holds regardless of which adapter is loaded or whether its filters fire.Three deliberate scoping decisions:
…-other).Verified live
readonly — what the adapter default would admit)rest_forbiddenTests
RouteGuardTest— 6 cases, red before, green after, covering both allow and deny, sub-paths, the prefix near-miss, other plugins' routes, and the pass-through.Gate: phpcs clean · PHPStan L5 no errors · 445/445.