fix: address Codex review — app-password collision + PHP_AUTH route scoping

Two real bugs surfaced by Codex's review of the Basic-auth migration:

P1.1: app-password collision (production-breaking, conditional)

On any site where `WP_Application_Passwords::is_in_use()` returns true
(i.e. any merchant has ever provisioned an app password — for any user,
any plugin), `wp_validate_application_password` runs at
`determine_current_user` priority 20 against our `ck_xxx` username.
`get_user_by('login', 'ck_xxx')` returns false → WP fires
`application_password_failed_authentication` with an `invalid_username`
WP_Error → `rest_application_password_collect_status` stores it on
`$wp_rest_application_password_status` → `rest_application_password_check_errors`
returns 401 from `rest_authentication_errors` before our route
permission callback ever runs.

The original smoke test passed because the wp-env had zero app
passwords, so `is_in_use()` returned false and the chain short-
circuited. A ticking time bomb.

Fix: filter `application_password_is_api_request` to return false for
`/wp-json/hey-woo/mcp`. Inside `wp_authenticate_application_password`,
the `! $is_api_request` early-return then prevents the error global
from ever being set. Surgical — every other REST route keeps WP's
normal app-password handling.

Verified against a live wp-env with an app password provisioned:
Basic Auth + read-only WC key + MCP initialize returns 200, not 401.

P1.2: route-scope bypass on mod_php (real, exploitable)

`enforce_setup_key_route_scope` calls `extract_request_credential`,
which only read `HTTP_AUTHORIZATION`. On Apache + mod_php (and many
fastcgi setups) Apache strips the raw header and populates
`PHP_AUTH_USER` / `PHP_AUTH_PW` directly. WC's auth reads the split
form, so it would authenticate the setup credential against any WC
REST surface (e.g. /wc/v3/orders), but our scope filter would see ''
and skip the restriction — defeating the credential's privacy
boundary on those SAPIs.

Fix: add a `PHP_AUTH_USER` / `PHP_AUTH_PW` branch at the top of
`extract_request_credential`, before the raw-header parse.

Tests:

- 2 new tests on `test-rest-api-key.php` pin the deny + allow paths
  for the route-scope filter via the PHP_AUTH split form. These would
  have caught the P1.2 bug; the existing tests only exercised
  HTTP_AUTHORIZATION.
- 4 new tests on `test-wc-auth-route-scoping.php`: pin that
  `enable_wc_auth_for_our_routes` excludes the MCP route (which the
  recent narrowing introduced); pin that
  `exclude_mcp_route_from_app_password_auth` denies the MCP route,
  passes through every other REST route, and is a no-op when the
  incoming value is already false.

288 PHPUnit tests pass (up from 282). PHPCS clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: garysmurray

includes/class-plugin.php

@@ -166,9 +166,23 @@ private function init_hooks() {
 
 		// Register our own MCP server when the adapter initializes. Owns the
 		// `/wp-json/hey-woo/mcp` endpoint with a curated tool/resource/prompt
-		// surface and a custom auth callback that handles X-MCP-API-Key (the
-		// header `mcp-wordpress-remote` sends).
+		// surface and a custom auth callback that authenticates ck:cs Basic
+		// Auth against `wp_woocommerce_api_keys`.
 		add_action( 'mcp_adapter_init', array( $this, 'register_mcp_server' ) );
+
+		// Tell WP not to run application-password auth on our MCP route.
+		// Without this, on any site where any app password exists
+		// (`WP_Application_Passwords::is_in_use()` returns true), WP's
+		// app-password handler runs at `determine_current_user` priority
+		// 20, fails with `invalid_username` for our `ck_xxx` user, stores
+		// the error on `$wp_rest_application_password_status`, and
+		// `rest_application_password_check_errors` returns 401 before our
+		// route permission callback ever runs. Excluding the MCP route
+		// from `application_password_is_api_request` makes the early
+		// return inside `wp_authenticate_application_password` fire — no
+		// error global, no 401 — so our own callback can authenticate
+		// the WC API key cleanly.
+		add_filter( 'application_password_is_api_request', array( $this, 'exclude_mcp_route_from_app_password_auth' ) );
 	}
 
 	/**
@@ -266,6 +280,40 @@ public function enable_wc_auth_for_our_routes( $is_request ) {
 		return $is_request;
 	}
 
+	/**
+	 * Tell WP not to treat the Hey Woo MCP route as an "API request" for
+	 * the purposes of application-password auth. See the rationale on
+	 * the `application_password_is_api_request` filter registration in
+	 * `init_hooks()` for the full chain — short version: the WC consumer
+	 * key (`ck_xxx`) is not a WP user login, and the app-password
+	 * handler turns that into a 401 on `rest_authentication_errors`
+	 * before our route permission callback runs.
+	 *
+	 * Filters very narrowly — only `/wp-json/hey-woo/mcp` and any
+	 * sub-routes the transport may add. Every other REST route still
+	 * gets WP's normal app-password handling.
+	 *
+	 * @param bool $is_api_request What WP would otherwise consider this request.
+	 * @return bool
+	 */
+	public function exclude_mcp_route_from_app_password_auth( $is_api_request ) {
+		if ( ! $is_api_request ) {
+			return $is_api_request;
+		}
+
+		$route = $this->current_rest_route();
+		if ( '' === $route ) {
+			return $is_api_request;
+		}
+
+		$mcp_prefix = self::MCP_SERVER_NS . '/' . self::MCP_SERVER_ROUTE;
+		if ( 0 === strpos( $route, $mcp_prefix ) ) {
+			return false;
+		}
+
+		return $is_api_request;
+	}
+
 	/**
 	 * Resolve the REST route the current request targets, regardless of
 	 * whether the install uses pretty (/wp-json/<route>) or plain

includes/setup/class-setup-page.php

@@ -541,7 +541,17 @@ private static function extract_request_credential() {
 			return wp_unslash( (string) $_SERVER['HTTP_X_MCP_API_KEY'] );
 		}
 
-		// 2. HTTP Basic auth — username:password = ck_xxx:cs_xxx.
+		// 2a. PHP_AUTH_USER + PHP_AUTH_PW — the split form Apache/mod_php
+		// (and many fastcgi setups) expose Basic auth as. WC's auth reads
+		// these directly, so the route-scope filter must too — otherwise
+		// a request bearing our setup credential against a non-MCP route
+		// (e.g. /wc/v3/orders) would slip past the scope check on any
+		// SAPI that doesn't surface HTTP_AUTHORIZATION.
+		if ( ! empty( $_SERVER['PHP_AUTH_USER'] ) && isset( $_SERVER['PHP_AUTH_PW'] ) ) {
+			return wp_unslash( (string) $_SERVER['PHP_AUTH_USER'] ) . ':' . wp_unslash( (string) $_SERVER['PHP_AUTH_PW'] );
+		}
+
+		// 2b. HTTP Basic auth (raw header form) — username:password = ck_xxx:cs_xxx.
 		$auth = isset( $_SERVER['HTTP_AUTHORIZATION'] ) ? wp_unslash( (string) $_SERVER['HTTP_AUTHORIZATION'] ) : '';
 		if ( 0 === stripos( $auth, 'Basic ' ) ) {
 			// phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.obfuscation_base64_decode -- decoding HTTP Basic auth header per RFC 7617; strict mode rejects invalid input.

tests/integration/test-rest-api-key.php

@@ -554,6 +554,69 @@ public function test_setup_key_is_allowed_on_mcp_route() {
 		}
 	}
 
+	/**
+	 * Apache + mod_php (and many fastcgi setups) expose Basic auth via
+	 * `PHP_AUTH_USER` / `PHP_AUTH_PW` and strip `HTTP_AUTHORIZATION`.
+	 * WC's auth reads the split form directly, so the route-scope
+	 * filter must too — otherwise a request bearing the setup
+	 * credential as Basic auth against a non-MCP route (e.g.
+	 * /wc/v3/orders) would be authenticated by WC and slip past the
+	 * scope check on those SAPIs, defeating the credential's privacy
+	 * boundary.
+	 *
+	 * Pin the deny-by-PHP_AUTH path explicitly so the regression
+	 * surfaces if the credential extractor is ever narrowed back to
+	 * HTTP_AUTHORIZATION-only.
+	 */
+	public function test_setup_key_is_rejected_on_non_mcp_routes_with_php_auth_basic() {
+		$helper                      = new RestApiKey();
+		$state                       = $helper->get_or_create();
+		list( $username, $password ) = RestApiKey::split_credential( $state['credential'] );
+
+		$original_server = $_SERVER;
+		try {
+			// phpcs:disable WordPress.Security.NonceVerification.Recommended -- test fixture mutates $_SERVER directly.
+			unset( $_SERVER['HTTP_X_MCP_API_KEY'], $_SERVER['HTTP_AUTHORIZATION'] );
+			$_SERVER['PHP_AUTH_USER'] = $username;
+			$_SERVER['PHP_AUTH_PW']   = $password;
+			$_SERVER['REQUEST_URI']   = '/wp-json/wc/v3/orders';
+			$result                   = SetupPage::enforce_setup_key_route_scope( null );
+
+			$this->assertInstanceOf( \WP_Error::class, $result );
+			$this->assertSame( 'hey_woo_route_restricted', $result->get_error_code() );
+			$this->assertSame( 403, $result->get_error_data()['status'] ?? 0 );
+		} finally {
+			$_SERVER = $original_server;
+			// phpcs:enable WordPress.Security.NonceVerification.Recommended
+		}
+	}
+
+	/**
+	 * Companion to the deny test above — the credential is allowed on
+	 * the MCP route when delivered as PHP_AUTH_USER/PHP_AUTH_PW too.
+	 * Pins the same SAPI shape on the allow side.
+	 */
+	public function test_setup_key_is_allowed_on_mcp_route_with_php_auth_basic() {
+		$helper                      = new RestApiKey();
+		$state                       = $helper->get_or_create();
+		list( $username, $password ) = RestApiKey::split_credential( $state['credential'] );
+
+		$original_server = $_SERVER;
+		try {
+			// phpcs:disable WordPress.Security.NonceVerification.Recommended -- test fixture mutates $_SERVER directly.
+			unset( $_SERVER['HTTP_X_MCP_API_KEY'], $_SERVER['HTTP_AUTHORIZATION'] );
+			$_SERVER['PHP_AUTH_USER'] = $username;
+			$_SERVER['PHP_AUTH_PW']   = $password;
+			$_SERVER['REQUEST_URI']   = '/wp-json/hey-woo/mcp';
+			$result                   = SetupPage::enforce_setup_key_route_scope( null );
+
+			$this->assertNull( $result, 'No restriction error on the allowed MCP route.' );
+		} finally {
+			$_SERVER = $original_server;
+			// phpcs:enable WordPress.Security.NonceVerification.Recommended
+		}
+	}
+
 	/**
 	 * The route-scope filter is a no-op when the request isn't using
 	 * our credential at all. Anything else would inadvertently

tests/integration/test-wc-auth-route-scoping.php

@@ -229,4 +229,92 @@ public function test_query_string_with_owned_substring_does_not_grant_scope() {
 			'A query string mentioning a hey-woo/ path must not opt the underlying /wp/v2/posts request into WC auth.'
 		);
 	}
+
+	/**
+	 * The MCP route is intentionally *outside* WC's auth scope. WC's
+	 * `check_user_permissions` enforces a per-method read/write split
+	 * that would 401 every MCP POST against a read-only key, so the
+	 * MCP transport handles auth itself via its own permission
+	 * callback. This pins the narrowing — if the prefix match ever
+	 * widens back to `hey-woo/`, MCP requests with read-only keys
+	 * would start 401-ing.
+	 */
+	public function test_mcp_route_is_outside_wc_auth_scope() {
+		$this->set_pretty_permalink_request( '/hey-woo/mcp' );
+		$this->assertFalse(
+			$this->check(),
+			'Pretty permalinks: hey-woo/mcp must NOT opt into WC auth (the MCP transport authenticates itself).'
+		);
+
+		$this->set_plain_permalink_request( '/hey-woo/mcp' );
+		$this->assertFalse(
+			$this->check(),
+			'Plain permalinks: hey-woo/mcp must NOT opt into WC auth.'
+		);
+	}
+
+	/**
+	 * `Plugin::exclude_mcp_route_from_app_password_auth` is the second
+	 * half of the auth picture. Without it, on any site where a
+	 * `WP_Application_Passwords` row exists,
+	 * `wp_authenticate_application_password()` runs at
+	 * `determine_current_user` priority 20 against our `ck_xxx`
+	 * username, fails with `invalid_username`, stores the error on the
+	 * `$wp_rest_application_password_status` global, and
+	 * `rest_application_password_check_errors` returns 401 before our
+	 * route permission callback ever runs. Returning false here makes
+	 * `wp_authenticate_application_password()` early-return without
+	 * touching the global, so the MCP transport's own callback runs
+	 * cleanly.
+	 *
+	 * Pin the deny path so a regression doesn't silently re-open the
+	 * 401 cliff for any merchant who has app passwords in use.
+	 */
+	public function test_mcp_route_excluded_from_app_password_auth() {
+		$plugin = \HeyWoo\Plugin::instance();
+
+		$this->set_pretty_permalink_request( '/hey-woo/mcp' );
+		$this->assertFalse(
+			$plugin->exclude_mcp_route_from_app_password_auth( true ),
+			'Pretty permalinks: hey-woo/mcp must opt out of app-password auth.'
+		);
+
+		$this->set_plain_permalink_request( '/hey-woo/mcp' );
+		$this->assertFalse(
+			$plugin->exclude_mcp_route_from_app_password_auth( true ),
+			'Plain permalinks: hey-woo/mcp must opt out of app-password auth.'
+		);
+	}
+
+	/**
+	 * The app-password exclusion must be surgical — every other REST
+	 * route (including our own `hey-woo/v1/...` REST controllers) keeps
+	 * WP's normal app-password handling. Anything else would silently
+	 * disable a documented WP feature on unrelated routes.
+	 */
+	public function test_app_password_auth_passes_through_for_non_mcp_routes() {
+		$plugin = \HeyWoo\Plugin::instance();
+
+		foreach ( array( '/hey-woo/v1/store/profile', '/wp/v2/posts', '/wc/v3/orders', '/wp-abilities/v1/abilities/wc-analytics/get-revenue-summary/run' ) as $route ) {
+			$this->set_pretty_permalink_request( $route );
+			$this->assertTrue(
+				$plugin->exclude_mcp_route_from_app_password_auth( true ),
+				"Route {$route} must keep WP's app-password handling."
+			);
+		}
+	}
+
+	/**
+	 * Pass-through when WP would not have considered the request an
+	 * API request anyway. We only ever flip true → false; we never
+	 * coerce false → true.
+	 */
+	public function test_app_password_filter_passes_through_when_already_false() {
+		$plugin = \HeyWoo\Plugin::instance();
+		$this->set_pretty_permalink_request( '/hey-woo/mcp' );
+		$this->assertFalse(
+			$plugin->exclude_mcp_route_from_app_password_auth( false ),
+			'When the incoming value is already false, the filter is a no-op pass-through.'
+		);
+	}
 }