fix: close etappe-0 whole-branch review findings

- Autoloader: add automattic/jetpack-autoloader and load vendor/autoload_packages.php; require wordpress/mcp-adapter directly (^0.5). - ServerRegistrar: make the adapter create_server() call and the OAuthServerRegistrar class_exists() check injectable so createServer()'s unhook-before-create ordering and the "mcp-oauth absent" branch are unit tested; add a smoke-test check that booting REST triggers no _doing_it_wrong(). - mcp-smoke.sh: assert mcp-adapter's default server route 404s, and that the subscriber role-gate denial actually carries our/the adapter's denial text (not just any isError/error). - Guard tests/Integration/mint-token.php and tec-probe.php with `defined('WP_CLI') && WP_CLI || exit;`; add .gitattributes export-ignore for tests/docs-superpowers/.superpowers/phpunit/phpcs config. - Security: add Server/RequestContext, tracking via rest_request_before_callbacks whether the current REST dispatch is our OAuth-protected /mcp/mcp-oauth-server route; AbilityRegistrar's permission_callback now denies (after the role gate) when abilities are invoked outside that route. Wired in Plugin::boot(). - Minors: single Plugin::VERSION constant used by ServerRegistrar; IntegrationRegistry caches its computed abilities list; tec-probe.php checks wp_insert_post's result and cleans up its probe post in a try/finally. composer test: 47/47 green. composer lint: clean. Full mcp-smoke.sh run ends ALL CHECKS PASSED (plus the expected /.well-known WARN). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7
2026-09-25 19:23:09 +02:00
parent 2c03286ba0
commit 0e3332ad3e
17 changed files with 430 additions and 38 deletions
@@ -51,6 +51,18 @@ mcp() { # $1 token, $2 json body, $3 session id (optional) -> prints headers+bod
    "${session_header[@]}" -d "$2"
 }

+# 0. Booting REST triggers no _doing_it_wrong(). A regression in the
+# unhook-before-create ordering (ServerRegistrar::createServer()) would
+# otherwise surface as a "duplicate server id" _doing_it_wrong() notice when
+# mcp-adapter's own mcp_adapter_init action fires during rest_api_init.
+DOING_IT_WRONG=$(wp eval "
+add_action('doing_it_wrong_run', static function () { echo 'DOING_IT_WRONG'; });
+do_action('rest_api_init');
+" 2>&1)
+echo "$DOING_IT_WRONG" | grep -q 'DOING_IT_WRONG' \
+  && fail "_doing_it_wrong() triggered while booting REST: $DOING_IT_WRONG"
+pass "booting REST triggers no _doing_it_wrong()"
+
 # 1. Discovery documents
 #
 # The local DDEV nginx config denies all dotfile paths (`location ~ /\. { deny
@@ -115,6 +127,22 @@ echo "$CALL" | grep -q '"isError":false' || fail "lit-site-status failed: $CALL"
 echo "$CALL" | grep -q 'the-events-calendar' || fail "site-status lacks components"
 pass "tools/call lit-site-status works"

+# 3b. mcp-adapter's default (unprotected) server must not exist: the route
+# it would register, mcp-adapter-default-server, must 404. Uses a temporary
+# application password (Basic auth) instead of the MCP bearer token, since
+# this checks a plain REST route, not the MCP endpoint. Named like the mint-
+# token.php passwords so the cleanup trap above also removes it.
+APP_PASSWORD=$(wp user application-password create "$ADMIN_LOGIN" lit-mcp-integration-test --porcelain)
+DEFAULT_SERVER_STATUS=$(curl -s --max-time 20 -o /dev/null -w '%{http_code}' \
+  -H "Host: $HOST" -H 'X-Forwarded-Proto: https' \
+  -X POST 'http://localhost/wp-json/mcp/mcp-adapter-default-server' \
+  -u "${ADMIN_LOGIN}:${APP_PASSWORD}" \
+  -H 'Content-Type: application/json' \
+  -d '{"jsonrpc":"2.0","id":1,"method":"tools/list"}')
+[ "$DEFAULT_SERVER_STATUS" = "404" ] \
+  || fail "mcp-adapter's default server is still reachable (HTTP $DEFAULT_SERVER_STATUS)"
+pass "mcp-adapter's default server (mcp-adapter-default-server) is gone (404)"
+
 # 4. Token must not authenticate other REST routes (mcp-oauth issue #62)
 ME=$(curl -s --max-time 20 -o /dev/null -w '%{http_code}' http://localhost/wp-json/wp/v2/users/me -H "Authorization: Bearer $TOKEN")
 [ "$ME" = "401" ] || fail "MCP token authenticates /wp/v2/users/me (HTTP $ME) – scope filter needed"
@@ -127,8 +155,18 @@ SUB_INIT=$(mcp "$SUB_TOKEN" '{"jsonrpc":"2.0","id":1,"method":"initialize","para
 SUB_SESSION=$(echo "$SUB_INIT" | grep -i '^mcp-session-id:' | awk '{print $2}' | tr -d '\r' || true)
 SUB_CALL=$(mcp "$SUB_TOKEN" '{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"lit-site-status","arguments":{}}}' "$SUB_SESSION")
 echo "$SUB_CALL" | grep -Eq '"isError":[[:space:]]*true|"error":[[:space:]]*\{' || fail "subscriber could call lit-site-status: $SUB_CALL"
+# The denial text must actually be our role-gate message, not just any error
+# (a crash or validation failure also sets isError/error and must not pass).
+echo "$SUB_CALL" | grep -q 'nicht freigegeben' \
+  || fail "lit-site-status denial lacks role-gate message: $SUB_CALL"
 SUB_META=$(mcp "$SUB_TOKEN" '{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"mcp-adapter-execute-ability","arguments":{"ability_name":"lit/site-status","parameters":{}}}}' "$SUB_SESSION")
 echo "$SUB_META" | grep -Eq '"isError":[[:space:]]*true|"error":[[:space:]]*\{' || fail "subscriber could use execute-ability: $SUB_META"
+# execute-ability denies at the adapter's own capability gate (mcp_adapter_execute_ability_capability,
+# set to lit_mcp_use by Access) before our ability's permission_callback ever runs, so the adapter's
+# own "insufficient_capability" text ("...lacks required capability...") is what appears here, not
+# our "nicht freigegeben" message.
+echo "$SUB_META" | grep -Eq 'lacks required capability|nicht freigegeben' \
+  || fail "execute-ability denial lacks role-gate message: $SUB_META"
 pass "role gate blocks subscriber (direct tool and meta-tool)"

 echo "ALL CHECKS PASSED"
@@ -11,6 +11,8 @@
 * Confirmed with a minimal repro during Task 7 (see task-7-report.md).
 */

+defined('WP_CLI') && WP_CLI || exit;
+
 use WPMedia\MCP\OAuth\Auth\JWT;
 use WPMedia\MCP\OAuth\Auth\SecretManager;

@@ -9,6 +9,8 @@
 * No strict_types: PHP forbids declare(strict_types) inside eval(), which wp eval-file uses.
 */

+defined('WP_CLI') && WP_CLI || exit;
+
 if (! class_exists('Tribe__Events__Main')) {
    fwrite(STDERR, "The Events Calendar is not active\n");
    exit(1);
@@ -21,25 +23,38 @@ $id = wp_insert_post([
    'post_type' => 'tribe_events',
    'post_status' => 'publish',
    'post_title' => 'LIT Probe Event (wegwerfen)',
-]);
-update_post_meta($id, '_EventStartDate', $start);
-update_post_meta($id, '_EventEndDate', $end);
+], true);

-$derived = [];
-foreach (['_EventStartDateUTC', '_EventEndDateUTC', '_EventDuration', '_EventTimezone'] as $key) {
-    $derived[$key] = get_post_meta($id, $key, true);
+if (is_wp_error($id)) {
+    fwrite(STDERR, 'wp_insert_post failed: ' . $id->get_error_message() . "\n");
+    exit(1);
 }

-$found = tribe_get_events([
-    'start_date' => gmdate('Y-m-d', strtotime('+29 days')),
-    'end_date' => gmdate('Y-m-d', strtotime('+31 days')),
-    'fields' => 'ids',
-]);
+if (! $id) {
+    fwrite(STDERR, "wp_insert_post returned no post id\n");
+    exit(1);
+}

-echo wp_json_encode([
-    'event_id' => $id,
-    'found_by_tribe_get_events' => in_array($id, (array) $found, true),
-    'derived_meta' => $derived,
-], JSON_PRETTY_PRINT) . "\n";
+try {
+    update_post_meta($id, '_EventStartDate', $start);
+    update_post_meta($id, '_EventEndDate', $end);

-wp_delete_post($id, true);
+    $derived = [];
+    foreach (['_EventStartDateUTC', '_EventEndDateUTC', '_EventDuration', '_EventTimezone'] as $key) {
+        $derived[$key] = get_post_meta($id, $key, true);
+    }
+
+    $found = tribe_get_events([
+        'start_date' => gmdate('Y-m-d', strtotime('+29 days')),
+        'end_date' => gmdate('Y-m-d', strtotime('+31 days')),
+        'fields' => 'ids',
+    ]);
+
+    echo wp_json_encode([
+        'event_id' => $id,
+        'found_by_tribe_get_events' => in_array($id, (array) $found, true),
+        'derived_meta' => $derived,
+    ], JSON_PRETTY_PRINT) . "\n";
+} finally {
+    wp_delete_post($id, true);
+}
@@ -0,0 +1,17 @@
+<?php
+
+declare(strict_types=1);
+
+if (! class_exists('WP_REST_Request')) {
+    class WP_REST_Request
+    {
+        public function __construct(private string $route = '')
+        {
+        }
+
+        public function get_route(): string
+        {
+            return $this->route;
+        }
+    }
+}
@@ -9,6 +9,7 @@ use LIT\MCP\Abilities\Ability;
 use LIT\MCP\Abilities\AbilityRegistrar;
 use LIT\MCP\Integrations\Integration;
 use LIT\MCP\Integrations\IntegrationRegistry;
+use LIT\MCP\Server\RequestContext;
 use LIT\MCP\Tests\Unit\TestCase;
 use Mockery;
 use RuntimeException;
@@ -16,6 +17,20 @@ use WP_Error;

 final class AbilityRegistrarTest extends TestCase
 {
+    protected function setUp(): void
+    {
+        parent::setUp();
+        // Default to "inside the OAuth server request" so tests unrelated to this
+        // gate keep exercising the ability's own permission() as before.
+        RequestContext::reset(true);
+    }
+
+    protected function tearDown(): void
+    {
+        RequestContext::reset();
+        parent::tearDown();
+    }
+
    private function registrarFor(Ability $ability): AbilityRegistrar
    {
        $integration = Mockery::mock(Integration::class);
@@ -64,6 +79,19 @@ final class AbilityRegistrarTest extends TestCase
        self::assertSame('lit_forbidden', $result->get_error_code());
    }

+    public function testPermissionDeniedOutsideOAuthServerRequest(): void
+    {
+        Functions\when('current_user_can')->justReturn(true);
+        RequestContext::reset(false);
+        $ability = $this->ability();
+        $ability->shouldNotReceive('permission');
+
+        $result = ($this->registrarFor($ability)->argsFor($ability)['permission_callback'])([]);
+
+        self::assertInstanceOf(WP_Error::class, $result);
+        self::assertSame('lit_forbidden', $result->get_error_code());
+    }
+
    public function testPermissionDelegatesToAbilityWhenGatePasses(): void
    {
        Functions\when('current_user_can')->justReturn(true);
@@ -0,0 +1,61 @@
+<?php
+
+declare(strict_types=1);
+
+namespace LIT\MCP\Tests\Unit\Server;
+
+use Brain\Monkey\Functions;
+use LIT\MCP\Server\RequestContext;
+use LIT\MCP\Tests\Unit\TestCase;
+use WP_REST_Request;
+
+final class RequestContextTest extends TestCase
+{
+    protected function tearDown(): void
+    {
+        RequestContext::reset();
+        parent::tearDown();
+    }
+
+    public function testIsOAuthServerRequestIsFalseByDefault(): void
+    {
+        self::assertFalse(RequestContext::isOAuthServerRequest());
+    }
+
+    public function testRecordRouteMarksTheOAuthServerRoute(): void
+    {
+        $request = new WP_REST_Request('/mcp/mcp-oauth-server');
+
+        RequestContext::recordRoute(null, [], $request);
+
+        self::assertTrue(RequestContext::isOAuthServerRequest());
+    }
+
+    public function testRecordRouteIgnoresOtherRoutes(): void
+    {
+        $request = new WP_REST_Request('/wp/v2/users/me');
+
+        RequestContext::recordRoute(null, [], $request);
+
+        self::assertFalse(RequestContext::isOAuthServerRequest());
+    }
+
+    public function testRecordRouteReturnsResponseUnchanged(): void
+    {
+        $response = new \stdClass();
+        $request = new WP_REST_Request('/mcp/mcp-oauth-server');
+
+        $result = RequestContext::recordRoute($response, [], $request);
+
+        self::assertSame($response, $result);
+    }
+
+    public function testRegisterHooksRecordRouteOnRestRequestBeforeCallbacks(): void
+    {
+        Functions\expect('add_filter')
+            ->once()
+            ->with('rest_request_before_callbacks', [RequestContext::class, 'recordRoute'], 10, 3);
+
+        (new RequestContext())->register();
+    }
+}
@@ -146,4 +146,69 @@ final class ServerRegistrarTest extends TestCase

        (new ServerRegistrar($this->registry(), $this->instructions()))->unhookOAuthDefaultServer();
    }
+
+    public function testUnhookOAuthDefaultServerIsNoOpWhenOAuthPackageIsAbsent(): void
+    {
+        $oauthRegistrar = Mockery::mock(OAuthServerRegistrar::class);
+        $oauthCallback = [$oauthRegistrar, 'register_server'];
+        $hook = new class {
+            /** @var array<int, array<string, array{function: mixed, accepted_args: int}>> */
+            public array $callbacks = [];
+        };
+        $hook->callbacks = [10 => ['oauth-id' => ['function' => $oauthCallback, 'accepted_args' => 0]]];
+        $GLOBALS['wp_filter']['mcp_adapter_init'] = $hook;
+
+        Functions\expect('remove_action')->never();
+
+        // Simulate wp-media/mcp-oauth not being installed via the injectable
+        // classExists probe (class_exists() is a PHP internal and cannot be
+        // stubbed directly through Brain\Monkey/Patchwork).
+        $registrar = new ServerRegistrar(
+            $this->registry(),
+            $this->instructions(),
+            null,
+            null,
+            static fn (): bool => false
+        );
+
+        $registrar->unhookOAuthDefaultServer();
+    }
+
+    public function testCreateServerUnhooksOAuthDefaultServerBeforeCreatingAdapterServer(): void
+    {
+        Functions\when('doing_action')->justReturn(true);
+        Functions\when('is_wp_error')->justReturn(false);
+
+        $oauthRegistrar = Mockery::mock(OAuthServerRegistrar::class);
+        $oauthCallback = [$oauthRegistrar, 'register_server'];
+        $hook = new class {
+            /** @var array<int, array<string, array{function: mixed, accepted_args: int}>> */
+            public array $callbacks = [];
+        };
+        $hook->callbacks = [10 => ['oauth-id' => ['function' => $oauthCallback, 'accepted_args' => 0]]];
+        $GLOBALS['wp_filter']['mcp_adapter_init'] = $hook;
+
+        /** @var list<string> $order */
+        $order = [];
+        Functions\expect('remove_action')
+            ->once()
+            ->with('mcp_adapter_init', $oauthCallback, 10)
+            ->andReturnUsing(static function () use (&$order): void {
+                $order[] = 'unhook';
+            });
+
+        $registrar = new ServerRegistrar(
+            $this->registry(),
+            $this->instructions(),
+            static fn (): bool => true,
+            static function (array $args) use (&$order): bool {
+                $order[] = 'create';
+                return true;
+            }
+        );
+
+        $registrar->createServer();
+
+        self::assertSame(['unhook', 'create'], $order, 'unhook must run before the adapter creates the server');
+    }
 }
@@ -9,3 +9,4 @@ if (! defined('ABSPATH')) {
 require_once __DIR__ . '/../vendor/autoload.php';
 require_once __DIR__ . '/Stubs/WpError.php';
 require_once __DIR__ . '/Stubs/WpUser.php';
+require_once __DIR__ . '/Stubs/WpRestRequest.php';