mirror of
https://github.com/kevinveenbirkenbach/computer-playbook.git
synced 2026-11-06 10:59:59 +00:00
fix(wordpress): refuse an MCP request the site cannot attribute to a user
The mu-plugin header describes the split as 'the adapter authorises the transport, the callback authorises the operation'. The second half held, every ability carries infinito_mcp_may_read. The first did not: the adapter answered the initialize handshake to anyone, so the server and its protocol version were readable without a credential. Measured in run 34653754175, the swarm WordPress#0: test-mcp-guest.js posts an anonymous initialize and expects at least 400, and the error context reads 'Expected: >= 400, Received: 200'. Its neighbour addons/mcp-adapter.spec.js posts an anonymous tools/list and passed, so the tool call was already refused and only the handshake was not. The filter hangs on rest_pre_dispatch and refuses every route under the mcp namespace while the site cannot attribute the request. It leaves an already short-circuited response alone rather than overwriting it, scopes itself by route prefix so the rest of the REST API stays as open as it was, and reaches is_user_logged_in, which triggers determine_current_user and with it wp_validate_application_password, so a client presenting its application password passes. A WP_Error renders no jsonrpc key, which is the second thing the spec asserts. The new unit test reads the guarded prefix out of the PHP and the endpoint out of the spec and requires the endpoint to sit under the prefix. Without it an adapter that moves namespace would leave the guard protecting nothing while both negative specs keep passing. A positive spec was tried and reverted: handing the MCP credential to the Playwright env is forbidden by tests/lint/ansible/services/test_mcp_secret_not_in_playwright.py, because the trace travels in the CI artifact. No role probes its MCP endpoint with the credential at deploy time either, so the authenticated direction stays unproven. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
849b29015c
commit
604843bb87
3 files changed
+82
No files matched your search
@@ -28,6 +28,34 @@ function infinito_mcp_may_read() {
|
||||
return is_user_logged_in() && current_user_can( 'read' );
|
||||
}
|
||||
|
||||
/**
|
||||
* Refuse an MCP transport request the site cannot attribute to a user.
|
||||
*
|
||||
* The adapter gates JSON-RPC methods that touch an ability, but answers the
|
||||
* `initialize` handshake to anyone, so the server and its protocol version are
|
||||
* readable without a credential.
|
||||
*
|
||||
* Args:
|
||||
* $result: the short-circuited response, or null while none was produced.
|
||||
* $server: the REST server handling the request.
|
||||
* $request: the request being dispatched.
|
||||
*/
|
||||
function infinito_mcp_guard_transport( $result, $server, $request ) {
|
||||
if ( null !== $result || is_user_logged_in() ) {
|
||||
return $result;
|
||||
}
|
||||
if ( 0 !== strpos( ltrim( $request->get_route(), '/' ), 'mcp/' ) ) {
|
||||
return $result;
|
||||
}
|
||||
return new WP_Error(
|
||||
'infinito_mcp_unauthorized',
|
||||
'Authentication is required to reach the MCP server.',
|
||||
array( 'status' => 401 )
|
||||
);
|
||||
}
|
||||
|
||||
add_filter( 'rest_pre_dispatch', 'infinito_mcp_guard_transport', 10, 3 );
|
||||
|
||||
/**
|
||||
* @param bool $available Whether core already considers them available.
|
||||
*/
|
||||
|
||||
@@ -0,0 +1,3 @@
|
||||
from pathlib import Path
|
||||
|
||||
PROJECT_ROOT: Path = Path(__file__).resolve().parents[5]
|
||||
@@ -0,0 +1,51 @@
|
||||
"""The MCP transport guard covers the endpoint the specs probe.
|
||||
|
||||
``infinito-mcp-abilities.php`` refuses every REST route under one namespace
|
||||
prefix that the site cannot attribute to a user. The specs probe one concrete
|
||||
endpoint. If the adapter ever moves out of that namespace the guard stops
|
||||
guarding anything, and both negative specs keep passing on a surface nobody
|
||||
protects, so the two are read against each other here.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
import unittest
|
||||
|
||||
from utils.cache.files import PROJECT_ROOT, read_text
|
||||
|
||||
_ROLE = PROJECT_ROOT / "roles/web-app-wordpress"
|
||||
_PLUGIN = _ROLE / "files/mu-plugins/infinito-mcp-abilities.php"
|
||||
_SPECS = (_ROLE / "files/playwright/test-mcp-guest.js",)
|
||||
|
||||
_GUARDED_PREFIX = re.compile(r"get_route\(\),\s*'/'\s*\),\s*'([\w-]+/)'")
|
||||
_ENDPOINT = re.compile(r'MCP_ENDPOINT\s*=\s*"([^"]+)"')
|
||||
|
||||
|
||||
class TestMcpTransportGuard(unittest.TestCase):
|
||||
def test_the_guarded_prefix_covers_every_endpoint_the_specs_probe(self) -> None:
|
||||
guarded = _GUARDED_PREFIX.search(read_text(str(_PLUGIN)))
|
||||
self.assertIsNotNone(
|
||||
guarded,
|
||||
"no route prefix found in infinito-mcp-abilities.php; the regex no "
|
||||
"longer matches how the transport guard scopes itself, so this "
|
||||
"test has stopped guarding anything",
|
||||
)
|
||||
prefix = guarded.group(1)
|
||||
|
||||
for spec in _SPECS:
|
||||
with self.subTest(spec=spec.name):
|
||||
endpoint = _ENDPOINT.search(read_text(str(spec)))
|
||||
self.assertIsNotNone(endpoint, f"no MCP_ENDPOINT in {spec.name}")
|
||||
route = endpoint.group(1).removeprefix("/wp-json/")
|
||||
self.assertTrue(
|
||||
route.startswith(prefix),
|
||||
f"{spec.name} probes {endpoint.group(1)}, which does not sit "
|
||||
f"under the guarded prefix '{prefix}'. The guard would let "
|
||||
"an anonymous initialize through while the negative specs "
|
||||
"still pass against whatever else answers",
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__": # pragma: no cover
|
||||
unittest.main()
|
||||
Reference in new issue
Block a user