| @@ -36,8 +36,9 @@ | ||
| 36 | 36 | declare(strict_types=1); |
| 37 | 37 | |
| 38 | 38 | namespace XSpeed\Modules\Mcp; |
| 39 | 39 | |
| 40 | +use XSpeed\Activity_Log; | |
| 40 | 41 | use XSpeed\Module; |
| 41 | 42 | use XSpeed\Onboarding; |
| 42 | 43 | |
| 43 | 44 | defined( 'ABSPATH' ) || exit; |
| @@ -63,20 +64,57 @@ | ||
| 63 | 64 | public const REWRITE_RULES = array( |
| 64 | 65 | '^xspeed/mcp/([a-f0-9]{64})/?$', |
| 65 | 66 | '^xspeed/mcp/?$', |
| 66 | 67 | '^xspeed/mcp/attach/?$', |
| 67 | - // OAuth discovery, root form. RFC 9728 §3.1 / RFC 8414 §3.1 put the | |
| 68 | - // `.well-known` segment BEFORE the resource path. | |
| 69 | - '^\.well-known/oauth-(protected-resource|authorization-server)/?$', | |
| 70 | - // OAuth discovery, path-suffixed form. Real clients (Claude Desktop | |
| 71 | - // among them) request THIS one; serving only the root form 404s them. | |
| 72 | - // It names our own resource path explicitly: a catch-all tail here | |
| 73 | - // also matched other MCP plugins' discovery URLs on the same site and | |
| 74 | - // answered them with our metadata, which broke their connectors. | |
| 68 | + // OAuth discovery. RFC 9728 §3.1 / RFC 8414 §3.1 put the | |
| 69 | + // `.well-known` segment BEFORE the resource/issuer path, and both of | |
| 70 | + // our identifiers are /xspeed/mcp — so this pair of URLs, and only | |
| 71 | + // this pair, is ours. It names our own path explicitly: a catch-all | |
| 72 | + // tail here also matched other MCP plugins' discovery URLs on the | |
| 73 | + // same site and answered them with our metadata, which broke their | |
| 74 | + // connectors. The bare root form is registered conditionally and so | |
| 75 | + // lives apart, in ROOT_DISCOVERY_RULE. | |
| 75 | 76 | '^\.well-known/oauth-(protected-resource|authorization-server)/xspeed/mcp/?$', |
| 76 | 77 | '^xspeed/authorize/?$', |
| 77 | 78 | ); |
| 78 | 79 | |
| 80 | + /** | |
| 81 | + * The root-form discovery rule — registered CONDITIONALLY, which is why | |
| 82 | + * it is not in REWRITE_RULES. | |
| 83 | + * | |
| 84 | + * It is the address a client built to the 2025-03-26 MCP spec looks at, | |
| 85 | + * and the only address it looks at; current clients read the | |
| 86 | + * protected-resource document first and follow it to the path form. So | |
| 87 | + * dropping it outright would cut off older clients on every site, | |
| 88 | + * including the single-plugin sites where the collision #266 exists to | |
| 89 | + * fix never happened. We answer it while it is uncontested and stand | |
| 90 | + * down the moment another plugin's rule claims it — | |
| 91 | + * root_discovery_contested(). | |
| 92 | + */ | |
| 93 | + public const ROOT_DISCOVERY_RULE = '^\.well-known/oauth-(protected-resource|authorization-server)/?$'; | |
| 94 | + | |
| 95 | + /** | |
| 96 | + * Rules earlier builds registered that we never register again under any | |
| 97 | + * condition. The self-heal guard flushes once when it finds OUR copy of | |
| 98 | + * one still in the stored table. | |
| 99 | + * | |
| 100 | + * The catch-all below shipped in an intermediate build and matched every | |
| 101 | + * path-suffixed discovery URL on the site, including other MCP plugins' | |
| 102 | + * (#264). Nothing brings it back, so its removal is unconditional — | |
| 103 | + * unlike ROOT_DISCOVERY_RULE, which is a rule we still register when the | |
| 104 | + * root URL is uncontested and therefore cannot live in this list. | |
| 105 | + * | |
| 106 | + * Ownership is read from the rule's TARGET, never from the regex alone: | |
| 107 | + * a sibling may register the same regex for its own document, its rule | |
| 108 | + * comes back from every flush, and a guard that treated that as stale | |
| 109 | + * would flush on every request forever. | |
| 110 | + * | |
| 111 | + * @var string[] | |
| 112 | + */ | |
| 113 | + public const RETIRED_REWRITE_RULES = array( | |
| 114 | + '^\.well-known/oauth-(protected-resource|authorization-server)(?:/.*)?/?$', | |
| 115 | + ); | |
| 116 | + | |
| 79 | 117 | /** REST namespace shared with Free. Public: Mcp_Server builds the |
| 80 | 118 | * discovery fallback URL from it. */ |
| 81 | 119 | public const NS = 'xspeed/v1'; |
| 82 | 120 | |
| @@ -109,9 +147,9 @@ | ||
| 109 | 147 | public function ui_metadata(): array { |
| 110 | 148 | return array( |
| 111 | 149 | 'label' => __( 'MCP Server', 'xspeed' ), |
| 112 | 150 | 'icon' => 'Sparkles', |
| 113 | - 'description' => __( 'Control this site\'s cache from Claude and other AI agents.', 'xspeed' ), | |
| 151 | + 'description' => __( 'Let Claude or another AI agent run this site — purge, check stats, change settings. This is the only thing you connect an AI to, and it is free with no API key.', 'xspeed' ), | |
| 114 | 152 | 'custom_panel' => 'McpPanel', |
| 115 | 153 | ); |
| 116 | 154 | } |
| 117 | 155 | |
| @@ -135,9 +173,29 @@ | ||
| 135 | 173 | public function boot(): void { |
| 136 | 174 | add_action( 'rest_api_init', array( $this, 'register_rest' ) ); |
| 137 | 175 | |
| 138 | 176 | // Pretty per-site endpoint: /xspeed/mcp → MCP JSON-RPC handler. |
| 139 | - add_action( 'init', array( $this, 'add_rewrite' ) ); | |
| 177 | + // `wp_loaded`, not `init`: add_rewrite() decides whether to claim the | |
| 178 | + // root discovery URL by looking at the rewrite table, and on `init` that | |
| 179 | + // view is incomplete -- a sibling MCP plugin hooked at the same priority | |
| 180 | + // but loaded after us has not registered yet. Root then looks | |
| 181 | + // uncontested, we register our rule, and the self-heal guard concludes | |
| 182 | + // nothing is stale, so it never flushes. That is a fixed point: the | |
| 183 | + // table never converges, and because our rule is the one WordPress | |
| 184 | + // matches, the sibling never sees the request either. | |
| 185 | + // | |
| 186 | + // By `wp_loaded` every init callback on every request type has run, so | |
| 187 | + // the contested check sees the sibling and the guard flushes once. | |
| 188 | + // WP_Rewrite::flush_rules() already defers itself to `wp_loaded`, so | |
| 189 | + // nothing is lost by deciding here, and did_action('wp_loaded') is | |
| 190 | + // truthy inside this callback, so the flush lands in time for | |
| 191 | + // parse_request in the same request. (#266 QA) | |
| 192 | + // Priority 0: still after every `init` callback, but ahead of the | |
| 193 | + // widely copied `add_action( 'wp_loaded', 'flush_rewrite_rules' )` | |
| 194 | + // snippet. If such a plugin flushed first it would write a table | |
| 195 | + // without our rules, our guard would find them missing and flush | |
| 196 | + // again -- two flushes and two option writes on every request. | |
| 197 | + add_action( 'wp_loaded', array( $this, 'add_rewrite' ), 0 ); | |
| 140 | 198 | add_filter( 'query_vars', array( $this, 'register_query_var' ) ); |
| 141 | 199 | // Priority 1: a sibling MCP plugin that also claims /.well-known/ gets |
| 142 | 200 | // to answer first at the default priority 10, and whoever answers |
| 143 | 201 | // first calls exit(). Running early means the URL is decided by WHOSE |
| @@ -239,8 +297,15 @@ | ||
| 239 | 297 | |
| 240 | 298 | // -- Pretty endpoint: /xspeed/mcp -- |
| 241 | 299 | |
| 242 | 300 | public function add_rewrite(): void { |
| 301 | + // The stored table is WordPress's routing table AND the only durable | |
| 302 | + // record of which plugin owns which discovery URL, so both the | |
| 303 | + // conditional registration below and the self-heal guard at the | |
| 304 | + // bottom read the SAME snapshot of it. Deciding twice from two reads | |
| 305 | + // is how a guard ends up flushing away a rule it just registered. | |
| 306 | + $stored_rules = get_option( 'rewrite_rules' ); | |
| 307 | + | |
| 243 | 308 | // Token-in-URL form: /xspeed/mcp/<token> — a single string the user |
| 244 | 309 | // pastes into their AI client (no separate token field). The bare |
| 245 | 310 | // /xspeed/mcp still works with a Bearer/header token. |
| 246 | 311 | add_rewrite_rule( |
| @@ -257,55 +322,238 @@ | ||
| 257 | 322 | // (that rule requires 64 hex chars), so ordering is safe. |
| 258 | 323 | add_rewrite_rule( '^xspeed/mcp/attach/?$', 'index.php?' . self::ATTACH_QUERY_VAR . '=1', 'top' ); |
| 259 | 324 | |
| 260 | 325 | // OAuth discovery documents. RFC 9728 §3.1 / RFC 8414 §3.1 place the |
| 261 | - // `.well-known` segment BEFORE the resource path, so our resource at | |
| 262 | - // /xspeed/mcp is discovered at BOTH: | |
| 263 | - // /.well-known/oauth-protected-resource (root form) | |
| 264 | - // /.well-known/oauth-protected-resource/xspeed/mcp (path-suffixed) | |
| 265 | - // Real clients (Claude Desktop among them) request the path-suffixed | |
| 266 | - // form; serving only the root form 404s them and the connection aborts. | |
| 326 | + // `.well-known` segment BEFORE the resource/issuer path, and both of | |
| 327 | + // our canonical identifiers are the MCP endpoint URL, so our | |
| 328 | + // documents live at: | |
| 329 | + // /.well-known/oauth-protected-resource/xspeed/mcp | |
| 330 | + // /.well-known/oauth-authorization-server/xspeed/mcp | |
| 267 | 331 | // |
| 268 | - // Both are matched EXACTLY. A `(?:/.*)?` tail covers the same two URLs | |
| 269 | - // in one rule, but also matches every OTHER plugin's discovery URL on | |
| 270 | - // the same site — and WordPress matches rewrite rules in table order | |
| 271 | - // rather than by specificity, so a sibling's own exact rule never gets | |
| 272 | - // reached. Its clients then receive OUR metadata, find a resource and | |
| 273 | - // issuer that do not match what they are connecting to, and abort | |
| 274 | - // before the login screen. | |
| 332 | + // Matched EXACTLY. A `(?:/.*)?` tail covers our URLs in one rule, but | |
| 333 | + // also matches every OTHER plugin's discovery URL on the same site — | |
| 334 | + // and WordPress matches rewrite rules in table order rather than by | |
| 335 | + // specificity, so a sibling's own exact rule never gets reached. Its | |
| 336 | + // clients then receive OUR metadata, find a resource and issuer that | |
| 337 | + // do not match what they are connecting to, and abort before the | |
| 338 | + // login screen. | |
| 275 | 339 | add_rewrite_rule( |
| 276 | - '^\\.well-known/oauth-(protected-resource|authorization-server)/?$', | |
| 277 | - 'index.php?' . self::WELLKNOWN_QUERY_VAR . '=$matches[1]', | |
| 278 | - 'top' | |
| 279 | - ); | |
| 280 | - add_rewrite_rule( | |
| 281 | 340 | '^\\.well-known/oauth-(protected-resource|authorization-server)/xspeed/mcp/?$', |
| 282 | 341 | 'index.php?' . self::WELLKNOWN_QUERY_VAR . '=$matches[1]', |
| 283 | 342 | 'top' |
| 284 | 343 | ); |
| 285 | 344 | |
| 345 | + // The bare root form, ONLY while no other plugin claims it. A client | |
| 346 | + // written to the 2025-03-26 MCP spec looks there and nowhere else, so | |
| 347 | + // giving it up unconditionally would break those clients on every | |
| 348 | + // site — including the single-plugin sites where the collision never | |
| 349 | + // happened. When a sibling's rule is present the URL is theirs and we | |
| 350 | + // register nothing, which is the case #266 is about. Current clients | |
| 351 | + // read the protected-resource document first and follow it wherever | |
| 352 | + // it points, so they are unaffected either way. The document served | |
| 353 | + // at root carries the LEGACY | |
| 354 | + // host-only issuer, because that is the identifier a client used to | |
| 355 | + // derive that URL (RFC 8414 §3.3). | |
| 356 | + $root_contested = self::root_discovery_contested( $stored_rules ); | |
| 357 | + if ( ! $root_contested ) { | |
| 358 | + add_rewrite_rule( | |
| 359 | + self::ROOT_DISCOVERY_RULE, | |
| 360 | + 'index.php?' . self::WELLKNOWN_QUERY_VAR . '=$matches[1]', | |
| 361 | + 'top' | |
| 362 | + ); | |
| 363 | + } | |
| 364 | + | |
| 286 | 365 | // Browser-facing OAuth consent page — served OUTSIDE REST so cookie |
| 287 | 366 | // auth (is_user_logged_in) works after the wp-login round-trip. |
| 288 | 367 | add_rewrite_rule( '^xspeed/authorize/?$', 'index.php?' . self::AUTHORIZE_QUERY_VAR . '=1', 'top' ); |
| 289 | 368 | |
| 290 | - // Self-heal: flush once if ANY of our rules is missing from the stored | |
| 291 | - // rewrite table. Checking only the first rule is not enough — a site | |
| 292 | - // flushed under an older build (which had /xspeed/mcp but not the | |
| 293 | - // later /xspeed/authorize + /.well-known rules) keeps that first rule, | |
| 294 | - // so the guard never fires and OAuth discovery 404s forever. Guard on | |
| 295 | - // the full set so any newly-added rule triggers a re-flush. | |
| 296 | - $rules = get_option( 'rewrite_rules' ); | |
| 297 | - if ( is_array( $rules ) ) { | |
| 298 | - foreach ( self::REWRITE_RULES as $rule ) { | |
| 299 | - if ( ! isset( $rules[ $rule ] ) ) { | |
| 300 | - flush_rewrite_rules( false ); | |
| 301 | - break; | |
| 369 | + // Self-heal: flush once if the stored rewrite table disagrees with the | |
| 370 | + // rules we just registered. Checking only the first rule is not | |
| 371 | + // enough — a site flushed under an older build (which had /xspeed/mcp | |
| 372 | + // but not the later /xspeed/authorize + /.well-known rules) keeps that | |
| 373 | + // first rule, so the guard never fires and OAuth discovery 404s | |
| 374 | + // forever. Guard on the full set so any newly-added rule triggers a | |
| 375 | + // re-flush. | |
| 376 | + // | |
| 377 | + // The guard must mirror the registration decisions EXACTLY, or it | |
| 378 | + // never reaches a fixed point: | |
| 379 | + // | |
| 380 | + // - Uncontested root: we register it, so the table must hold it | |
| 381 | + // with OUR target. A flush produces exactly that, and the next | |
| 382 | + // request reads the same table and stays uncontested — our own | |
| 383 | + // target never counts as a sibling's. | |
| 384 | + // - Contested root: we register nothing, so OUR copy must be gone. | |
| 385 | + // A flush regenerates the sibling's rule (they register it every | |
| 386 | + // request) but not ours, so the next request is quiet. | |
| 387 | + // | |
| 388 | + // Ownership is read from the TARGET in both directions. Keying on the | |
| 389 | + // regex alone is what produced a flush on every request forever when | |
| 390 | + // a sibling held that regex: their rule comes back from every flush. | |
| 391 | + // | |
| 392 | + // This runs on `wp_loaded` for every request, so the first request after an | |
| 393 | + // upgrade flushes once and the guard is quiet from then on. It cannot | |
| 394 | + // move to Plugin::maybe_upgrade() — that is admin-only and runs at | |
| 395 | + // plugins_loaded 21, i.e. BEFORE init, so a flush there would write a | |
| 396 | + // table without our rules and this guard would flush a second time. | |
| 397 | + if ( ! is_array( $stored_rules ) ) { | |
| 398 | + return; | |
| 399 | + } | |
| 400 | + | |
| 401 | + $stale = false; | |
| 402 | + $retiring = false; | |
| 403 | + foreach ( self::REWRITE_RULES as $rule ) { | |
| 404 | + if ( ! isset( $stored_rules[ $rule ] ) ) { | |
| 405 | + $stale = true; | |
| 406 | + break; | |
| 407 | + } | |
| 408 | + } | |
| 409 | + | |
| 410 | + // Our copy of the root rule must be present exactly when we register | |
| 411 | + // it. Present-and-unwanted is the #266 upgrade; absent-and-wanted is | |
| 412 | + // an older table, or a sibling that has since gone away. | |
| 413 | + $root_is_ours = isset( $stored_rules[ self::ROOT_DISCOVERY_RULE ] ) | |
| 414 | + && self::is_our_rule_target( $stored_rules[ self::ROOT_DISCOVERY_RULE ] ); | |
| 415 | + if ( $root_is_ours === $root_contested ) { | |
| 416 | + $stale = true; | |
| 417 | + $retiring = $root_contested; | |
| 418 | + } | |
| 419 | + | |
| 420 | + // Not an identity move — nothing a site owner can act on — so this | |
| 421 | + // one flushes quietly. | |
| 422 | + foreach ( self::RETIRED_REWRITE_RULES as $rule ) { | |
| 423 | + if ( isset( $stored_rules[ $rule ] ) && self::is_our_rule_target( $stored_rules[ $rule ] ) ) { | |
| 424 | + $stale = true; | |
| 425 | + break; | |
| 426 | + } | |
| 427 | + } | |
| 428 | + | |
| 429 | + if ( ! $stale ) { | |
| 430 | + return; | |
| 431 | + } | |
| 432 | + | |
| 433 | + if ( $retiring ) { | |
| 434 | + // The one moment the identity move is observable to a site owner, | |
| 435 | + // and it happens on a front-end request with no UI attached. Fires | |
| 436 | + // once: after the flush our rule is gone, so the next request | |
| 437 | + // finds nothing to hand over. | |
| 438 | + Activity_Log::record( | |
| 439 | + 'mcp_discovery_moved', | |
| 440 | + __( 'Another plugin now claims the site-wide OAuth discovery address, so xSpeed handed it over. Its own is /.well-known/oauth-protected-resource/xspeed/mcp — AI assistants already connected may ask for approval once more.', 'xspeed' ), | |
| 441 | + Activity_Log::INFO | |
| 442 | + ); | |
| 443 | + } | |
| 444 | + | |
| 445 | + flush_rewrite_rules( false ); | |
| 446 | + } | |
| 447 | + | |
| 448 | + /** | |
| 449 | + * Whether another plugin's rewrite rule already routes the ROOT discovery | |
| 450 | + * URLs, making them theirs rather than ours. | |
| 451 | + * | |
| 452 | + * Read from two views of the rewrite table, because neither alone is | |
| 453 | + * complete at `init`: | |
| 454 | + * | |
| 455 | + * - the STORED table, which is what WordPress actually routes with and | |
| 456 | + * the only view that survives the request. If a sibling registered | |
| 457 | + * the same regex after us at the last flush, its target is what is | |
| 458 | + * stored, and that is precisely "the sibling owns this URL now". | |
| 459 | + * - the IN-MEMORY rules registered so far this request, which catches a | |
| 460 | + * sibling that hooks `init` earlier than we do and has therefore not | |
| 461 | + * reached the stored table yet. | |
| 462 | + * | |
| 463 | + * A sibling that registers LATER than us used to be the one case neither | |
| 464 | + * view saw, and it did NOT resolve itself: the guard is what triggers a | |
| 465 | + * flush, so a guard reading an incomplete view simply never fires. That | |
| 466 | + * is why add_rewrite() now runs on `wp_loaded` rather than `init` — by | |
| 467 | + * then every plugin has registered, whatever its load order. | |
| 468 | + * | |
| 469 | + * @param mixed $stored The stored rewrite table, if already read. | |
| 470 | + */ | |
| 471 | + public static function root_discovery_contested( $stored = null ): bool { | |
| 472 | + $tables = array(); | |
| 473 | + if ( is_array( $stored ) ) { | |
| 474 | + $tables[] = $stored; | |
| 475 | + } elseif ( null === $stored ) { | |
| 476 | + $option = get_option( 'rewrite_rules' ); | |
| 477 | + if ( is_array( $option ) ) { | |
| 478 | + $tables[] = $option; | |
| 479 | + } | |
| 480 | + } | |
| 481 | + | |
| 482 | + if ( isset( $GLOBALS['wp_rewrite'] ) && is_object( $GLOBALS['wp_rewrite'] ) ) { | |
| 483 | + foreach ( array( 'extra_rules_top', 'extra_rules' ) as $prop ) { | |
| 484 | + if ( isset( $GLOBALS['wp_rewrite']->$prop ) && is_array( $GLOBALS['wp_rewrite']->$prop ) ) { | |
| 485 | + $tables[] = $GLOBALS['wp_rewrite']->$prop; | |
| 302 | 486 | } |
| 303 | 487 | } |
| 304 | 488 | } |
| 489 | + | |
| 490 | + foreach ( $tables as $rules ) { | |
| 491 | + foreach ( $rules as $pattern => $target ) { | |
| 492 | + if ( ! self::is_a_wellknown_rule( (string) $pattern ) || self::is_our_rule_target( $target ) ) { | |
| 493 | + continue; | |
| 494 | + } | |
| 495 | + foreach ( array( 'protected-resource', 'authorization-server' ) as $doc ) { | |
| 496 | + $probe = '.well-known/oauth-' . $doc; | |
| 497 | + if ( preg_match( '#' . str_replace( '#', '\\#', (string) $pattern ) . '#', $probe ) ) { | |
| 498 | + return true; | |
| 499 | + } | |
| 500 | + } | |
| 501 | + } | |
| 502 | + } | |
| 503 | + | |
| 504 | + return false; | |
| 305 | 505 | } |
| 306 | 506 | |
| 307 | 507 | /** |
| 508 | + * Whether a rewrite regex was written FOR a .well-known discovery URL, | |
| 509 | + * as opposed to merely matching one. | |
| 510 | + * | |
| 511 | + * WordPress's own page rule -- `(.?.+?)/?$` => `index.php?pagename=...` | |
| 512 | + * -- is in the stored table of every site using pretty permalinks, and | |
| 513 | + * it matches `.well-known/oauth-protected-resource` exactly as it | |
| 514 | + * matches every other path on the site. Reading that as a sibling's | |
| 515 | + * claim would report root as contested EVERYWHERE: the root document | |
| 516 | + * would be retired on every install, including the single-plugin sites | |
| 517 | + * this change exists to leave alone, and each of them would log a | |
| 518 | + * hand-over that never happened. | |
| 519 | + * | |
| 520 | + * A rule that routes these URLs on purpose spells the segment out, so | |
| 521 | + * that is the signal. Backslashes are stripped first because the regex | |
| 522 | + * carries them as escapes (`^\\.well-known/...`) and a rule is free to | |
| 523 | + * escape the hyphen too. | |
| 524 | + * | |
| 525 | + * @param string $pattern The stored rewrite regex. | |
| 526 | + */ | |
| 527 | + private static function is_a_wellknown_rule( string $pattern ): bool { | |
| 528 | + return false !== stripos( str_replace( '\\', '', $pattern ), 'well-known' ); | |
| 529 | + } | |
| 530 | + | |
| 531 | + /** | |
| 532 | + * Whether a stored rewrite target was written by this module. | |
| 533 | + * | |
| 534 | + * Every rule we register resolves to `index.php?<one of our query | |
| 535 | + * vars>=…`, and no other plugin sets those. Used to tell OUR leftover | |
| 536 | + * copy of a retired rule from a sibling's rule that happens to share the | |
| 537 | + * regex — only the first is ours to flush away. | |
| 538 | + * | |
| 539 | + * @param mixed $target The stored rewrite target. | |
| 540 | + */ | |
| 541 | + private static function is_our_rule_target( $target ): bool { | |
| 542 | + if ( ! is_string( $target ) ) { | |
| 543 | + return false; | |
| 544 | + } | |
| 545 | + | |
| 546 | + foreach ( array( self::QUERY_VAR, self::TOKEN_QUERY_VAR, self::WELLKNOWN_QUERY_VAR, self::AUTHORIZE_QUERY_VAR, self::ATTACH_QUERY_VAR ) as $var ) { | |
| 547 | + if ( false !== strpos( $target, $var . '=' ) ) { | |
| 548 | + return true; | |
| 549 | + } | |
| 550 | + } | |
| 551 | + | |
| 552 | + return false; | |
| 553 | + } | |
| 554 | + | |
| 555 | + /** | |
| 308 | 556 | * True when a request for our discovery URL can reach WordPress at all. |
| 309 | 557 | * |
| 310 | 558 | * Since maybe_handle_pretty_endpoint() claims the document by REQUEST |
| 311 | 559 | * PATH, a sibling plugin winning the rewrite match no longer matters — |
| @@ -326,9 +574,16 @@ | ||
| 326 | 574 | |
| 327 | 575 | // Any rule that routes our discovery path to index.php will do — ours |
| 328 | 576 | // or a sibling's — because the path check inside the handler decides |
| 329 | 577 | // the outcome once the request lands. |
| 330 | - $probe = '.well-known/oauth-protected-resource'; | |
| 578 | + // | |
| 579 | + // Probe the URL the 401 challenge actually advertises: the | |
| 580 | + // path-suffixed form, which is the canonical identity since #266. | |
| 581 | + // Probing root would answer a different question — whether ANY plugin | |
| 582 | + // routes the contested URL — and on a site where a sibling owns it | |
| 583 | + // that answer says nothing about whether our own document is | |
| 584 | + // reachable. | |
| 585 | + $probe = '.well-known/oauth-protected-resource/' . Mcp_Pairing::SITE_ENDPOINT_PATH; | |
| 331 | 586 | foreach ( $rules as $pattern => $target ) { |
| 332 | 587 | if ( preg_match( '#' . str_replace( '#', '\\#', $pattern ) . '#', $probe ) ) { |
| 333 | 588 | return true; |
| 334 | 589 | } |
| @@ -353,23 +608,44 @@ | ||
| 353 | 608 | /** |
| 354 | 609 | * Which discovery document the CURRENT request path asks for, if any. |
| 355 | 610 | * |
| 356 | 611 | * Claims only URLs that are unambiguously ours, mirroring the rewrite |
| 357 | - * rules exactly: the bare root form, and the RFC 9728 §3.1 path-suffixed | |
| 358 | - * form naming our own resource (`/xspeed/mcp`). A suffix belonging to a | |
| 359 | - * sibling plugin is deliberately NOT claimed — answering | |
| 612 | + * rules exactly: the RFC 9728 §3.1 / RFC 8414 §3.1 path-suffixed form | |
| 613 | + * naming our own resource and issuer (`/xspeed/mcp`). A suffix belonging | |
| 614 | + * to a sibling plugin is deliberately NOT claimed — answering | |
| 360 | 615 | * `/.well-known/oauth-protected-resource/betterlinks/mcp` with xSpeed |
| 361 | 616 | * metadata is the same bug that broke this site, just pointed the other |
| 362 | 617 | * way. |
| 363 | 618 | * |
| 364 | - * @return string 'protected-resource', 'authorization-server', or ''. | |
| 619 | + * The bare root form is claimed only while no other plugin's rewrite rule | |
| 620 | + * claims it. Leaving the suffix optional here took the root document from | |
| 621 | + * a sibling even on a build that had stopped registering its own root | |
| 622 | + * rule, so the path check is exact. | |
| 623 | + * | |
| 624 | + * The TABLE is what hands root over -- add_rewrite() stops registering | |
| 625 | + * the rule and flushes it away. This claim only releases it, and only | |
| 626 | + * while a sibling's rule actually owns the URL: once WordPress has | |
| 627 | + * routed the request to OUR query var, no other plugin's handler can see | |
| 628 | + * it, so releasing it would abandon the request to the front page rather | |
| 629 | + * than pass it on. Note that is about the winning rule's TARGET, not its | |
| 630 | + * regex -- a sibling can hold the same pattern. The document served at root carries the LEGACY host-only | |
| 631 | + * issuer, the identifier a client used to derive that URL. (#266) | |
| 632 | + * | |
| 633 | + * @param string $matched_query The query the matched rule resolved to. | |
| 634 | + * @return array{doc:string,issuer:string} Doc name ('' when not ours) | |
| 635 | + * and the identity to stamp on it. | |
| 365 | 636 | */ |
| 366 | - private function wellknown_doc_from_path(): string { | |
| 637 | + private function wellknown_claim_from_path( string $matched_query = '' ): array { | |
| 638 | + $none = array( | |
| 639 | + 'doc' => '', | |
| 640 | + 'issuer' => '', | |
| 641 | + ); | |
| 642 | + | |
| 367 | 643 | $uri = isset( $_SERVER['REQUEST_URI'] ) |
| 368 | 644 | ? sanitize_text_field( wp_unslash( $_SERVER['REQUEST_URI'] ) ) |
| 369 | 645 | : ''; |
| 370 | 646 | if ( '' === $uri ) { |
| 371 | - return ''; | |
| 647 | + return $none; | |
| 372 | 648 | } |
| 373 | 649 | |
| 374 | 650 | $path = (string) wp_parse_url( $uri, PHP_URL_PATH ); |
| 375 | 651 | |
| @@ -380,12 +656,56 @@ | ||
| 380 | 656 | } |
| 381 | 657 | |
| 382 | 658 | $path = trim( $path, '/' ); |
| 383 | 659 | |
| 384 | - $pattern = '#^\.well-known/oauth-(protected-resource|authorization-server)' | |
| 385 | - . '(?:/xspeed/mcp)?$#'; | |
| 660 | + // trim() above already dropped a trailing slash, so `/xspeed/mcp/` | |
| 661 | + // still matches — and so does the root form with one. | |
| 662 | + $ours = '#^\.well-known/oauth-(protected-resource|authorization-server)' | |
| 663 | + . '/' . preg_quote( Mcp_Pairing::SITE_ENDPOINT_PATH, '#' ) . '$#'; | |
| 664 | + if ( preg_match( $ours, $path, $m ) ) { | |
| 665 | + return array( | |
| 666 | + 'doc' => $m[1], | |
| 667 | + 'issuer' => Mcp_OAuth::issuer(), | |
| 668 | + ); | |
| 669 | + } | |
| 386 | 670 | |
| 387 | - return preg_match( $pattern, $path, $m ) ? $m[1] : ''; | |
| 671 | + // Releasing root is only safe when somebody else can pick it up. If | |
| 672 | + // OUR rule is what WordPress matched, nobody can: the sibling's query | |
| 673 | + // var is unset, so its handler never runs, and the request falls | |
| 674 | + // through to the front page -- a 301 to the homepage where dev | |
| 675 | + // returns JSON. Answering with the legacy document is the pre-#266 | |
| 676 | + // behaviour. (#266 QA) | |
| 677 | + // | |
| 678 | + // Keyed on the query the matched rule RESOLVED TO -- not on | |
| 679 | + // $wp->query_vars, and not on which regex matched. | |
| 680 | + // | |
| 681 | + // query_vars is wrong because the var is public and WP::parse_request | |
| 682 | + // lets $_GET override anything a rule set, so `?xspeed_mcp_wellknown=1` | |
| 683 | + // would let anyone force our metadata onto a URL a sibling owns. | |
| 684 | + // | |
| 685 | + // matched_rule is wrong because the rewrite table is keyed BY regex: | |
| 686 | + // a sibling that registered this same pattern replaces our entry and | |
| 687 | + // the key still reads as ours, while the target behind it is theirs. | |
| 688 | + // That is a live case here -- is_our_rule_target() exists for it -- | |
| 689 | + // and keying on the rule would answer for the sibling, which is the | |
| 690 | + // bug this whole change is about. | |
| 691 | + // | |
| 692 | + // matched_query is built from the winning rule's TARGET (class-wp.php, | |
| 693 | + // before the parse_request action) and $_GET never touches it. If it | |
| 694 | + // sets our query var, our rule genuinely won. (#266 QA) | |
| 695 | + $routed_to_us = 1 === preg_match( | |
| 696 | + '#(?:^|&)' . preg_quote( self::WELLKNOWN_QUERY_VAR, '#' ) . '=#', | |
| 697 | + $matched_query | |
| 698 | + ); | |
| 699 | + $root = '#^\.well-known/oauth-(protected-resource|authorization-server)$#'; | |
| 700 | + if ( preg_match( $root, $path, $m ) && ( $routed_to_us || ! self::root_discovery_contested() ) ) { | |
| 701 | + return array( | |
| 702 | + 'doc' => $m[1], | |
| 703 | + 'issuer' => Mcp_OAuth::legacy_issuer(), | |
| 704 | + ); | |
| 705 | + } | |
| 706 | + | |
| 707 | + return $none; | |
| 388 | 708 | } |
| 389 | 709 | |
| 390 | 710 | /** |
| 391 | 711 | * Serve the MCP endpoint on the pretty path. Runs on parse_request so |
| @@ -393,11 +713,11 @@ | ||
| 393 | 713 | * |
| 394 | 714 | * @param \WP $wp The WP request object. |
| 395 | 715 | */ |
| 396 | 716 | public function maybe_handle_pretty_endpoint( $wp ): void { |
| 397 | - // OAuth discovery documents (served at the site root). | |
| 717 | + // OAuth discovery documents. | |
| 398 | 718 | // |
| 399 | - // Read the doc name from the REQUEST PATH, not just our query var. | |
| 719 | + // Read the doc name from the REQUEST PATH, and ONLY from the path. | |
| 400 | 720 | // `add_rewrite_rule( …, 'top' )` only means "top at the moment it |
| 401 | 721 | // runs", so whichever MCP plugin hooks `init` last ends up first in |
| 402 | 722 | // the table — an order set by plugin load order, which no plugin |
| 403 | 723 | // controls. A sibling's catch-all |
| @@ -405,20 +725,28 @@ | ||
| 405 | 725 | // the match and our query var is never set, even though the URL is |
| 406 | 726 | // unambiguously ours. Observed live with two different plugins on one |
| 407 | 727 | // site. parse_request runs AFTER matching, so the path is the one |
| 408 | 728 | // signal no sibling rule can take away from us. |
| 409 | - $doc = $this->wellknown_doc_from_path(); | |
| 410 | - if ( '' === $doc && ! empty( $wp->query_vars[ self::WELLKNOWN_QUERY_VAR ] ) ) { | |
| 411 | - $doc = (string) $wp->query_vars[ self::WELLKNOWN_QUERY_VAR ]; | |
| 412 | - } | |
| 729 | + // | |
| 730 | + // The query VAR is deliberately never consulted. It is public, so | |
| 731 | + // $_GET can set it on any URL, and answering from it would put our | |
| 732 | + // metadata on somebody else's address — the whole bug. Which rewrite | |
| 733 | + // RULE matched is a different thing: WordPress decides it, the query | |
| 734 | + // string cannot influence it, and it is only read to tell "a sibling | |
| 735 | + // owns this URL" from "we own it and nobody else can answer". | |
| 736 | + $matched_query = is_object( $wp ) && isset( $wp->matched_query ) ? (string) $wp->matched_query : ''; | |
| 737 | + $claim = $this->wellknown_claim_from_path( $matched_query ); | |
| 738 | + $doc = $claim['doc']; | |
| 413 | 739 | if ( '' !== $doc ) { |
| 414 | 740 | $data = 'authorization-server' === $doc |
| 415 | - ? Mcp_OAuth::authorization_server_metadata() | |
| 416 | - : Mcp_OAuth::protected_resource_metadata(); | |
| 741 | + ? Mcp_OAuth::authorization_server_metadata( $claim['issuer'] ) | |
| 742 | + : Mcp_OAuth::protected_resource_metadata( $claim['issuer'] ); | |
| 417 | 743 | status_header( 200 ); |
| 418 | 744 | header( 'Content-Type: application/json; charset=utf-8' ); |
| 419 | - // Discovery metadata is public + cacheable. | |
| 420 | - header( 'Cache-Control: public, max-age=3600' ); | |
| 745 | + // Public and cacheable, but short: this document IS the server's | |
| 746 | + // identity, and a cached copy outliving an issuer change is the | |
| 747 | + // one failure a client cannot recover from on its own. (#266) | |
| 748 | + header( 'Cache-Control: public, max-age=300' ); | |
| 421 | 749 | echo wp_json_encode( $data ); |
| 422 | 750 | exit; |
| 423 | 751 | } |
| 424 | 752 | |
| @@ -1024,9 +1352,11 @@ | ||
| 1024 | 1352 | * @return \WP_REST_Response |
| 1025 | 1353 | */ |
| 1026 | 1354 | private function discovery_response( array $data ): \WP_REST_Response { |
| 1027 | 1355 | $response = new \WP_REST_Response( $data, 200 ); |
| 1028 | - $response->header( 'Cache-Control', 'public, max-age=3600' ); | |
| 1356 | + // Same short window as the /.well-known/ emit site, same reason: the | |
| 1357 | + // document carries the server's identity. (#266) | |
| 1358 | + $response->header( 'Cache-Control', 'public, max-age=300' ); | |
| 1029 | 1359 | return $response; |
| 1030 | 1360 | } |
| 1031 | 1361 | |
| 1032 | 1362 | /** |
