fix: OAuth bearer resolver fatal before init (#127) (#128)

* test(oauth): pin bearer resolve fatal before wp_rewrite exists

Any plugin calling is_user_logged_in() during plugins_loaded fires
determine_current_user before wp-settings.php has created $wp_rewrite.
On a genuine Bearer MCP request the resolver reaches its RFC 8707
audience check, resource_id() calls rest_url(), and get_rest_url()
fatals on the missing global — killing the request and the connector.

Three tests, committed red with the exact customer stack
(using_index_permalinks() on null via class-saddle-oauth.php:183):
resolution must work early, resource_id() must be byte-identical
before and after init on all three permalink shapes, and being early
must not skip the audience check.

Refs #127

* fix(oauth): resolve bearer tokens before wp_rewrite exists

resource_id() now routes through rest_url_early(): with the rewrite
global present it defers to rest_url() untouched; without it, it
replicates core get_rest_url(null, $path, 'rest') exactly from
pre-init-available state. The replica must be byte-identical because
the value is compared against the RFC 8707 resource stored on every
access token — divergence would refuse every token as invalid_token.
The only substitution is using_index_permalinks(), a pure function of
the permalink_structure option and the hard-coded index.php default.

Store::find() had already hardened the DB half of this window with raw
wpdb; this closes the URL half. issuer()/endpoint() stay on plain
rest_url() — audit shows no pre-init caller.

Refs #127

Author: ifahimreza

includes/oauth/class-saddle-oauth.php

@@ -180,7 +180,77 @@ public static function set_enabled( $enabled ) {
 	 * @return string
 	 */
 	public static function resource_id() {
-		return untrailingslashit( rest_url( ltrim( Saddle_MCP::REST_NAMESPACE . Saddle_MCP::ROUTE, '/' ) ) );
+		return untrailingslashit( self::rest_url_early( ltrim( Saddle_MCP::REST_NAMESPACE . Saddle_MCP::ROUTE, '/' ) ) );
+	}
+
+	/**
+	 * `rest_url()` that also works before `init`.
+	 *
+	 * The bearer resolver runs on `determine_current_user`, which another
+	 * plugin can fire during `plugins_loaded` by calling `is_user_logged_in()`
+	 * — before wp-settings.php has created `$GLOBALS['wp_rewrite']`, which
+	 * core's `get_rest_url()` dereferences unconditionally whenever a
+	 * permalink structure is set. This is the same pre-init window
+	 * {@see Saddle_OAuth_Store::find()} documents for the DB path; this
+	 * hardens the URL path. Any new pre-init caller of `rest_url()` must come
+	 * through here.
+	 *
+	 * When the global exists this defers to core untouched. When it does not,
+	 * it replicates core's `get_rest_url( null, $path, 'rest' )` exactly —
+	 * including the `rest_url` filter with core's argument shape — because
+	 * the result is compared against the RFC 8707 `resource` stored on every
+	 * access token, and any divergence would refuse every token on the site
+	 * as `invalid_token`. The only substitution is
+	 * `WP_Rewrite::using_index_permalinks()`, which is a pure function of the
+	 * `permalink_structure` option and the hard-coded `index.php` default;
+	 * anything customizing `WP_Rewrite::$index` does so on the real instance,
+	 * which only exists once the branch below defers to core anyway.
+	 *
+	 * @param string $path REST route below the REST prefix.
+	 * @return string Full URL to the endpoint.
+	 */
+	private static function rest_url_early( $path ) {
+		if ( ( $GLOBALS['wp_rewrite'] ?? null ) instanceof WP_Rewrite ) {
+			return rest_url( $path );
+		}
+
+		$path = '/' . ltrim( (string) $path, '/' );
+
+		// Core's multisite leg, get_blog_option( null, … ), reads the current
+		// site's option — identical to get_option() here. Truthiness, not
+		// '' !==, to mirror core.
+		$structure = get_option( 'permalink_structure' );
+
+		if ( $structure ) {
+			$prefix = rest_get_url_prefix();
+			if ( preg_match( '#^/*index\.php#', (string) $structure ) ) {
+				$prefix = 'index.php/' . $prefix;
+			}
+			$url = get_home_url( null, $prefix, 'rest' ) . $path;
+		} else {
+			$url = trailingslashit( get_home_url( null, '', 'rest' ) );
+			// Core appends index.php to dodge an nginx redirect that only
+			// allows HTTP/1.0 methods; substr instead of str_ends_with for
+			// the PHP 7.4 floor.
+			if ( 'index.php' !== substr( $url, -9 ) ) {
+				$url .= 'index.php';
+			}
+			$url = add_query_arg( 'rest_route', $path, $url );
+		}
+
+		if ( is_ssl() && isset( $_SERVER['SERVER_NAME'] ) ) {
+			// phpcs:ignore WordPress.Security.ValidatedSanitizedInput -- Mirrors core's get_rest_url() host comparison byte for byte; the strict equality against the parsed home host is the validation.
+			if ( wp_parse_url( get_home_url( null ), PHP_URL_HOST ) === $_SERVER['SERVER_NAME'] ) {
+				$url = set_url_scheme( $url, 'https' );
+			}
+		}
+
+		if ( is_admin() && force_ssl_admin() ) {
+			$url = set_url_scheme( $url, 'https' );
+		}
+
+		/** This filter is documented in wp-includes/rest-api.php */
+		return apply_filters( 'rest_url', $url, $path, null, 'rest' );
 	}
 
 	/**

readme.txt

@@ -143,6 +143,7 @@ If you happen to have the separate MCP Adapter plugin active, Saddle notices and
 
 = 1.0.0 =
 * Initial public release.
+* Fixed: on sites where another plugin checks who is signed in very early in the request — several SEO plugins do — every ChatGPT request crashed before Saddle could examine its token, so the connection failed with a sign-in error forever while the same build worked elsewhere. The token check now works no matter how early in the request it runs.
 * Fixed: the connection check could report that sign-ins were working on a server that was actually blocking half of them. Apps you connect with a pasted key send one kind of sign-in header and apps that sign in through Saddle — ChatGPT is the one that can only connect that way — send another, and some servers pass the first and drop the second. The check now tests both, says which one is being blocked, and offers the same one-click fix, which always covered both.
 * Connection details and health now shows how each request signed in, so "the key was rejected" and "no key ever arrived" stop looking identical. They are the same error message and they need opposite fixes — one is reconnecting the app, the other is a word with your host.
 * Fixed: the request recorder was logging its own screen refreshing, which pushed the requests you were trying to capture out of the list within about two minutes. It now records only real app traffic, and keeps four times as much of it.

tests/oauth-bearer-test.php

@@ -169,6 +169,103 @@ public function test_cleartext_transport_refuses_the_token() {
 		remove_filter( 'saddle_oauth_readiness_ssl', '__return_false' );
 	}
 
+	/* ------------------------------------------------------------------
+	 * Pre-init resolution
+	 *
+	 * determine_current_user is not a REST-time hook: any plugin calling
+	 * is_user_logged_in() during plugins_loaded fires it before
+	 * wp-settings.php has created $GLOBALS['wp_rewrite'] — the same window
+	 * Saddle_OAuth_Store::find() documents for the DB path. The resolver
+	 * must authenticate there too, not just avoid the fatal: core memoizes
+	 * whatever user it resolves for the rest of the request, so a refusal
+	 * here is a dead connector exactly like a crash.
+	 * --------------------------------------------------------------- */
+
+	/**
+	 * Run $fn in the state determine_current_user fires in when another
+	 * plugin checks the user during plugins_loaded — before wp-settings.php
+	 * has created $wp_rewrite.
+	 *
+	 * try/finally, per call: WP_UnitTestCase never restores this global, so
+	 * a failing assertion must not leak a nulled instance into later tests.
+	 *
+	 * @param callable $fn What to run without the rewrite global.
+	 * @return mixed Whatever $fn returns.
+	 */
+	private function before_wp_rewrite_exists( $fn ) {
+		$saved                 = $GLOBALS['wp_rewrite'];
+		$GLOBALS['wp_rewrite'] = null;
+		try {
+			return $fn();
+		} finally {
+			$GLOBALS['wp_rewrite'] = $saved;
+		}
+	}
+
+	public function test_resolve_works_before_wp_rewrite_exists() {
+		$this->issue_token();
+
+		$user = $this->before_wp_rewrite_exists(
+			function () {
+				return Saddle_OAuth_Bearer::resolve( false );
+			}
+		);
+
+		$this->assertSame(
+			$this->admin,
+			$user,
+			'is_user_logged_in() during plugins_loaded fires determine_current_user before $wp_rewrite exists; a valid token must resolve there, not fatal or 401.'
+		);
+	}
+
+	public function test_resource_id_is_identical_before_and_after_init() {
+		$structures = array(
+			'pretty' => '/%postname%/',
+			'index'  => '/index.php/%postname%/',
+			'plain'  => '',
+		);
+
+		foreach ( $structures as $label => $structure ) {
+			// set_permalink_structure(), not update_option(): rest_url()
+			// reads the live instance, and a stale one would make this
+			// compare the replica against the wrong baseline.
+			$this->set_permalink_structure( $structure );
+
+			$post_init = Saddle_OAuth::resource_id();
+			$pre_init  = $this->before_wp_rewrite_exists(
+				function () {
+					return Saddle_OAuth::resource_id();
+				}
+			);
+
+			$this->assertSame(
+				$post_init,
+				$pre_init,
+				sprintf( 'resource_id() diverged on %s permalinks — every stored token audience would stop matching.', $label )
+			);
+		}
+
+		// Leave the live instance matching what set_up put in the option,
+		// so state cannot leak into whichever test runs next.
+		$this->set_permalink_structure( '/%postname%/' );
+	}
+
+	public function test_a_foreign_token_is_still_refused_before_wp_rewrite_exists() {
+		$this->issue_token();
+
+		$record = Saddle_OAuth_Store::get_access_token( $this->token );
+		update_post_meta( $record['id'], '_saddle_resource', 'https://elsewhere.example/wp-json/saddle/v1/mcp' );
+
+		$this->assertFalse(
+			$this->before_wp_rewrite_exists(
+				function () {
+					return Saddle_OAuth_Bearer::resolve( false );
+				}
+			),
+			'Being early is not a reason to skip the RFC 8707 audience check.'
+		);
+	}
+
 	/* ------------------------------------------------------------------
 	 * Confinement
 	 * --------------------------------------------------------------- */