diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..a55a796 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,5 @@ +/tests export-ignore +/docs/superpowers export-ignore +/.superpowers export-ignore +/phpunit.xml.dist export-ignore +/phpcs.xml.dist export-ignore diff --git a/composer.json b/composer.json index f7ed2a4..e7c6ee8 100644 --- a/composer.json +++ b/composer.json @@ -5,6 +5,8 @@ "license": "GPL-2.0-or-later", "require": { "php": ">=8.1", + "automattic/jetpack-autoloader": "^3.0", + "wordpress/mcp-adapter": "^0.5", "wp-media/mcp-oauth": "^1.1" }, "require-dev": { @@ -30,6 +32,7 @@ "sort-packages": true, "platform": { "php": "8.1" }, "allow-plugins": { + "automattic/jetpack-autoloader": true, "dealerdirect/phpcodesniffer-composer-installer": true } } diff --git a/composer.lock b/composer.lock index b8fef02..c953e95 100644 --- a/composer.lock +++ b/composer.lock @@ -4,8 +4,72 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "538b1dbb81faf068bc0ff5f6a99efde9", + "content-hash": "f61708b5ca7588f7b1c5b47774383472", "packages": [ + { + "name": "automattic/jetpack-autoloader", + "version": "v3.1.3", + "source": { + "type": "git", + "url": "https://github.com/Automattic/jetpack-autoloader.git", + "reference": "e7e49a4e2f16cb2dfd3e58c478499a60d7d51839" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/Automattic/jetpack-autoloader/zipball/e7e49a4e2f16cb2dfd3e58c478499a60d7d51839", + "reference": "e7e49a4e2f16cb2dfd3e58c478499a60d7d51839", + "shasum": "" + }, + "require": { + "composer-plugin-api": "^1.1 || ^2.0", + "php": ">=7.0" + }, + "require-dev": { + "automattic/jetpack-changelogger": "^4.2.8", + "composer/composer": "^1.1 || ^2.0", + "yoast/phpunit-polyfills": "^1.1.1" + }, + "type": "composer-plugin", + "extra": { + "class": "Automattic\\Jetpack\\Autoloader\\CustomAutoloaderPlugin", + "autotagger": true, + "mirror-repo": "Automattic/jetpack-autoloader", + "branch-alias": { + "dev-trunk": "3.1.x-dev" + }, + "changelogger": { + "link-template": "https://github.com/Automattic/jetpack-autoloader/compare/v${old}...v${new}" + }, + "version-constants": { + "::VERSION": "src/AutoloadGenerator.php" + } + }, + "autoload": { + "psr-4": { + "Automattic\\Jetpack\\Autoloader\\": "src" + }, + "classmap": [ + "src/AutoloadGenerator.php" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "GPL-2.0-or-later" + ], + "description": "Creates a custom autoloader for a plugin or theme.", + "keywords": [ + "autoload", + "autoloader", + "composer", + "jetpack", + "plugin", + "wordpress" + ], + "support": { + "source": "https://github.com/Automattic/jetpack-autoloader/tree/v3.1.3" + }, + "time": "2024-11-04T09:23:56+00:00" + }, { "name": "wordpress/mcp-adapter", "version": "v0.5.0", diff --git a/src/Abilities/AbilityRegistrar.php b/src/Abilities/AbilityRegistrar.php index 031074e..4e0ceba 100644 --- a/src/Abilities/AbilityRegistrar.php +++ b/src/Abilities/AbilityRegistrar.php @@ -6,6 +6,7 @@ namespace LIT\MCP\Abilities; use LIT\MCP\Access; use LIT\MCP\Integrations\IntegrationRegistry; +use LIT\MCP\Server\RequestContext; use Throwable; use WP_Error; @@ -55,6 +56,10 @@ final class AbilityRegistrar return Errors::forbidden('Dein Benutzerkonto ist für LIT MCP nicht freigegeben.'); } + if (! RequestContext::isOAuthServerRequest()) { + return Errors::forbidden('LIT MCP ist nur über den OAuth-geschützten MCP-Server nutzbar.'); + } + return $ability->permission(is_array($input) ? $input : []); }, 'execute_callback' => static function (mixed $input = null) use ($ability): mixed { diff --git a/src/Integrations/IntegrationRegistry.php b/src/Integrations/IntegrationRegistry.php index 4b75be3..6364c0f 100644 --- a/src/Integrations/IntegrationRegistry.php +++ b/src/Integrations/IntegrationRegistry.php @@ -11,6 +11,9 @@ final class IntegrationRegistry /** @var list */ private array $integrations; + /** @var list|null */ + private ?array $abilities = null; + public function __construct(Integration ...$integrations) { $this->integrations = array_values($integrations); @@ -28,12 +31,16 @@ final class IntegrationRegistry /** @return list */ public function abilities(): array { - $abilities = []; - foreach ($this->available() as $integration) { - array_push($abilities, ...$integration->abilities()); + if ($this->abilities === null) { + $abilities = []; + foreach ($this->available() as $integration) { + array_push($abilities, ...$integration->abilities()); + } + + $this->abilities = $abilities; } - return $abilities; + return $this->abilities; } /** @return list */ diff --git a/src/Plugin.php b/src/Plugin.php index b3b19ca..bfb740c 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -8,6 +8,7 @@ use LIT\MCP\Abilities\AbilityRegistrar; use LIT\MCP\Guides\InstructionsBuilder; use LIT\MCP\Integrations\Core\CoreIntegration; use LIT\MCP\Integrations\IntegrationRegistry; +use LIT\MCP\Server\RequestContext; use LIT\MCP\Server\ServerRegistrar; /** @@ -15,6 +16,8 @@ use LIT\MCP\Server\ServerRegistrar; */ final class Plugin { + public const VERSION = '0.1.0'; + private static string $pluginFile = ''; public static function boot(string $pluginFile): void @@ -28,6 +31,7 @@ final class Plugin } (new Access())->register(); + (new RequestContext())->register(); $registry = new IntegrationRegistry( new CoreIntegration(self::pluginDir() . '/docs/guides') diff --git a/src/Server/RequestContext.php b/src/Server/RequestContext.php new file mode 100644 index 0000000..15c3e6c --- /dev/null +++ b/src/Server/RequestContext.php @@ -0,0 +1,62 @@ + $handler + * @return mixed + */ + public static function recordRoute(mixed $response, array $handler, WP_REST_Request $request): mixed + { + if ($request->get_route() === self::route()) { + self::$isOAuthServerRequest = true; + } + + return $response; + } + + public static function isOAuthServerRequest(): bool + { + return self::$isOAuthServerRequest; + } + + /** + * Test-only: resets (or forces) the recorded state between unit tests. + */ + public static function reset(bool $isOAuthServerRequest = false): void + { + self::$isOAuthServerRequest = $isOAuthServerRequest; + } + + private static function route(): string + { + return '/' . ServerRegistrar::NAMESPACE . '/' . ServerRegistrar::ROUTE; + } +} diff --git a/src/Server/ServerRegistrar.php b/src/Server/ServerRegistrar.php index abec288..e950559 100644 --- a/src/Server/ServerRegistrar.php +++ b/src/Server/ServerRegistrar.php @@ -6,6 +6,7 @@ namespace LIT\MCP\Server; use LIT\MCP\Guides\InstructionsBuilder; use LIT\MCP\Integrations\IntegrationRegistry; +use LIT\MCP\Plugin; use WP\MCP\Core\McpAdapter; use WP\MCP\Infrastructure\ErrorHandling\ErrorLogMcpErrorHandler; use WPMedia\MCP\OAuth\Context as OAuthContext; @@ -41,13 +42,38 @@ final class ServerRegistrar /** @var callable(): bool */ private $oauthEnabled; + /** @var callable(array): mixed */ + private $createAdapterServer; + + /** @var callable(string): bool */ + private $classExists; + public function __construct( private IntegrationRegistry $registry, private InstructionsBuilder $instructions, - ?callable $oauthEnabled = null + ?callable $oauthEnabled = null, + ?callable $createAdapterServer = null, + ?callable $classExists = null ) { $this->oauthEnabled = $oauthEnabled ?? static fn (): bool => (new OAuthContext())->is_enabled(); + $this->classExists = $classExists + ?? static fn (string $class): bool => class_exists($class); + $this->createAdapterServer = $createAdapterServer + ?? static function (array $args): mixed { + return McpAdapter::instance()->create_server( + $args['id'], + $args['namespace'], + $args['route'], + $args['name'], + $args['description'], + $args['version'], + [OAuthHttpTransport::class], + ErrorLogMcpErrorHandler::class, + (new OAuthContext())->observability_handler(), + $args['tools'] + ); + }; } public function register(): void @@ -75,7 +101,7 @@ final class ServerRegistrar 'route' => self::ROUTE, 'name' => 'LIT MCP', 'description' => $this->instructions->build(), - 'version' => '0.1.0', + 'version' => Plugin::VERSION, 'tools' => array_merge($this->registry->directAbilityNames(), self::META_TOOLS), ]; } @@ -97,18 +123,7 @@ final class ServerRegistrar $this->unhookOAuthDefaultServer(); - $result = McpAdapter::instance()->create_server( - $args['id'], - $args['namespace'], - $args['route'], - $args['name'], - $args['description'], - $args['version'], - [OAuthHttpTransport::class], - ErrorLogMcpErrorHandler::class, - (new OAuthContext())->observability_handler(), - $args['tools'] - ); + $result = ($this->createAdapterServer)($args); if (is_wp_error($result)) { $message = '[wp-lit-mcp] MCP server registration failed: ' . $result->get_error_message(); @@ -129,7 +144,7 @@ final class ServerRegistrar */ public function unhookOAuthDefaultServer(): void { - if (! class_exists(OAuthServerRegistrar::class)) { + if (! ($this->classExists)(OAuthServerRegistrar::class)) { return; } diff --git a/tests/Integration/mcp-smoke.sh b/tests/Integration/mcp-smoke.sh index b0cfaca..3b1c4af 100644 --- a/tests/Integration/mcp-smoke.sh +++ b/tests/Integration/mcp-smoke.sh @@ -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" diff --git a/tests/Integration/mint-token.php b/tests/Integration/mint-token.php index 6d7907d..f80a174 100644 --- a/tests/Integration/mint-token.php +++ b/tests/Integration/mint-token.php @@ -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; diff --git a/tests/Integration/tec-probe.php b/tests/Integration/tec-probe.php index 63111e7..da8120c 100644 --- a/tests/Integration/tec-probe.php +++ b/tests/Integration/tec-probe.php @@ -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); +} diff --git a/tests/Stubs/WpRestRequest.php b/tests/Stubs/WpRestRequest.php new file mode 100644 index 0000000..ccca454 --- /dev/null +++ b/tests/Stubs/WpRestRequest.php @@ -0,0 +1,17 @@ +route; + } + } +} diff --git a/tests/Unit/Abilities/AbilityRegistrarTest.php b/tests/Unit/Abilities/AbilityRegistrarTest.php index cf7de1f..3e22e6c 100644 --- a/tests/Unit/Abilities/AbilityRegistrarTest.php +++ b/tests/Unit/Abilities/AbilityRegistrarTest.php @@ -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); diff --git a/tests/Unit/Server/RequestContextTest.php b/tests/Unit/Server/RequestContextTest.php new file mode 100644 index 0000000..4630109 --- /dev/null +++ b/tests/Unit/Server/RequestContextTest.php @@ -0,0 +1,61 @@ +once() + ->with('rest_request_before_callbacks', [RequestContext::class, 'recordRoute'], 10, 3); + + (new RequestContext())->register(); + } +} diff --git a/tests/Unit/Server/ServerRegistrarTest.php b/tests/Unit/Server/ServerRegistrarTest.php index 215f5b2..5e2fba0 100644 --- a/tests/Unit/Server/ServerRegistrarTest.php +++ b/tests/Unit/Server/ServerRegistrarTest.php @@ -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> */ + 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> */ + public array $callbacks = []; + }; + $hook->callbacks = [10 => ['oauth-id' => ['function' => $oauthCallback, 'accepted_args' => 0]]]; + $GLOBALS['wp_filter']['mcp_adapter_init'] = $hook; + + /** @var list $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'); + } } diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 5162f52..e77c426 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -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'; diff --git a/wp-lit-mcp.php b/wp-lit-mcp.php index dd0edbd..c089e3b 100644 --- a/wp-lit-mcp.php +++ b/wp-lit-mcp.php @@ -16,7 +16,7 @@ declare(strict_types=1); defined('ABSPATH') || exit; -$lit_mcp_autoload = __DIR__ . '/vendor/autoload.php'; +$lit_mcp_autoload = __DIR__ . '/vendor/autoload_packages.php'; if (! is_readable($lit_mcp_autoload)) { add_action('admin_notices', static function (): void {