From c823c83665a99e3204b1b698c162351e77f4dc4f Mon Sep 17 00:00:00 2001 From: loukieluke <84424020+loukieluke@users.noreply.github.com> Date: Tue, 15 Sep 2026 09:09:36 -0500 Subject: [PATCH 01/11] ENG-7204: Re-apply the User Switching logout fix as 1.0.5 Restores the ENG-7125 change reverted in 1.0.4. The code is byte-identical to 1.0.3 apart from the version header; the revert was made on the belief that 1.0.3 had broken normal site access, which a paired harness run disproves. An Automattic build smoke test reported HTTP 401 with no session cookie on the front page and on the wp-admin login flow. Both scenarios return 401 on the 1.0.2 release artifact and on 1.0.4, neither of which contains any 1.0.3 code: force_basic_authentication() runs at plugins_loaded priority 1 and challenges any request arriving without an Authorization: Basic header, wp-login.php included. A smoke test that sends no credentials cannot pass against a site this plugin gates, whatever version is installed. Meanwhile 1.0.4 still fatals on every logout path when User Switching is active, and still lets an anonymous caller with no credentials reach wp_logout(). --- .gitattributes | 4 +- .github/workflows/main.yml | 11 ++ .github/workflows/test.yml | 33 ++++++ pressable-basic-authentication.php | 93 ++++++++++++---- readme.txt | 2 +- tests/hook-registration-test.php | 166 +++++++++++++++++++++++++++++ 6 files changed, 286 insertions(+), 23 deletions(-) create mode 100644 .github/workflows/test.yml create mode 100644 tests/hook-registration-test.php diff --git a/.gitattributes b/.gitattributes index 28a7274..ac24691 100644 --- a/.gitattributes +++ b/.gitattributes @@ -14,4 +14,6 @@ /README.md export-ignore /composer.json export-ignore /composer.lock export-ignore -/.DS_Store export-ignore \ No newline at end of file +/.DS_Store export-ignore +# Tests are not part of the distributed plugin. +/tests export-ignore diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 4ee246e..16032c8 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -18,6 +18,17 @@ jobs: - name: Checkout code uses: actions/checkout@v3 + # The release is published straight from this job, and no ordering exists + # between separate workflow files, so the checks in test.yml cannot gate it + # from outside -- a push that breaks them would still ship a Release. These + # run here, ahead of the archive, so a failure stops the release rather than + # being reported next to one. + - name: Lint PHP files + run: find . -path ./vendor -prune -o -name '*.php' -print0 | xargs -0 -n1 -- php -l + + - name: Hook registration regression test + run: php tests/hook-registration-test.php + - name: Install Ruby uses: ruby/setup-ruby@v1 with: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..237951c --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,33 @@ +name: Tests + +on: + pull_request: + push: + branches: + - main + +permissions: + contents: read + +jobs: + test: + name: Lint and hook registration + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + # The job lints and executes PHP straight from the checked-out pull + # request, so the GITHUB_TOKEN must not be left in the local git + # config where that code could read it. + persist-credentials: false + + - name: Report PHP version + run: php -v + + - name: Lint PHP files + run: find . -path ./vendor -prune -o -name '*.php' -print0 | xargs -0 -n1 -- php -l + + - name: Hook registration regression test + run: php tests/hook-registration-test.php diff --git a/pressable-basic-authentication.php b/pressable-basic-authentication.php index 91e10c5..3cc58e1 100644 --- a/pressable-basic-authentication.php +++ b/pressable-basic-authentication.php @@ -8,7 +8,7 @@ /* Plugin Name: Hosting Basic Authentication Description: Forces all users to authenticate using Basic Authentication before accessing any page. -Version: 1.0.4 +Version: 1.0.5 License: GPL2 Text Domain: hosting-basic-authentication */ @@ -30,6 +30,18 @@ public function __construct() { // Hook into WordPress before anything is outputted. add_action( 'plugins_loaded', array( $this, 'init' ), 1 ); + // Logout is handled on `init`, deliberately later than the rest of `init()`. + // wp_logout() fires the `wp_logout` action, and its subscribers may rely on + // constants their own plugin defines in a `plugins_loaded` callback. Firing it + // from `plugins_loaded` priority 1 races that setup, and which plugin wins the + // race depends on load order -- `active_plugins` ordering, anything filtering + // it, and network-activated plugins, which load earlier still. User Switching + // defines its cookie constants that way, so wherever this plugin happens to run + // first, User Switching's `wp_logout` subscriber fatals on a constant it has + // not defined yet. Hooking to `init` drops the dependency on load order + // entirely: every `plugins_loaded` callback has completed by then. + add_action( 'init', array( $this, 'handle_logout_request' ), 1 ); + // Add filter for logout URL. add_filter( 'logout_url', array( $this, 'modify_logout_url' ), 10, 2 ); @@ -41,36 +53,45 @@ public function __construct() { * Initialize the plugin */ public function init() { - // Skip if we're doing AJAX. - if ( $this->is_ajax_request() ) { + if ( $this->skip_request() ) { return; } - // Skip if we're doing CRON. - if ( $this->is_cron_request() ) { - return; - } + // Redirect from wp-login.php when already authenticated via Basic Auth + $this->maybe_redirect_from_login_page(); + + // Force authentication. + $this->force_basic_authentication(); + } - // Skip if we're in CLI mode. - if ( $this->is_cli_request() ) { + /** + * Handles the Basic Auth logout request. + * + * Hooked to `init` rather than running with the rest of init() on + * `plugins_loaded` -- see the hook registration in the constructor for why. + */ + public function handle_logout_request() { + if ( $this->skip_request() ) { return; } - // Skip requests to excluded endpoints - if ($this->should_skip_auth()) { - return; - } - - // Handle logout request. - if ( isset( $_GET['basic-auth-logout'] ) ) { - $this->handle_basic_auth_logout(); + if ( ! isset( $_GET['basic-auth-logout'] ) ) { + return; } - // Redirect from wp-login.php when already authenticated via Basic Auth - $this->maybe_redirect_from_login_page(); + $this->handle_basic_auth_logout(); + } - // Force authentication. - $this->force_basic_authentication(); + /** + * Whether this request is outside the scope of Basic Authentication. + * + * @return bool + */ + private function skip_request() { + return $this->is_ajax_request() + || $this->is_cron_request() + || $this->is_cli_request() + || $this->should_skip_auth(); } /** @@ -109,6 +130,27 @@ private function force_basic_authentication() { $this->send_auth_headers(); } + // A request asking to log out must still clear the authentication gate above -- + // that is what keeps an anonymous caller from reaching wp_logout() -- but it must + // not be given a session that handle_logout_request() discards moments later on + // `init`. Establishing one fires set_auth_cookie and set_logged_in_cookie on what + // is only ever a logout, which an audit or session-tracking plugin can reasonably + // record as a real login. + // + // Skipping wp_set_current_user() as well as the cookies is correct, not a + // shortcut: execution only reaches here when nobody is logged in -- a live session + // returns above -- so there is no established identity for the following + // wp_logout() to report. It passes whatever get_current_user_id() actually holds, + // which is 0, instead of one this method manufactured moments earlier purely to + // tear it down again. + // + // Placement is load-bearing in both directions. Above the credential handling this + // would skip the 401 as well, readmitting the unauthenticated caller it exists to + // exclude; below the cookie calls it would do nothing at all. + if ( isset( $_GET['basic-auth-logout'] ) ) { + return; + } + // Log the user in programmatically. wp_set_current_user( $user->ID ); wp_set_auth_cookie( $user->ID ); @@ -270,6 +312,15 @@ public function modify_logout_url( $logout_url, $redirect ) { public function maybe_redirect_from_login_page() { global $pagenow; + // A request that asks to log out is never redirected away from the logout. This + // guard only matters on wp-login.php, and only for a logout URL that omits + // `action=logout` -- the URL modify_logout_url() builds always carries it. Since + // the logout moved to `init`, this method now runs first, and without the guard + // such a request would redirect to the home page still logged in, with no error. + if ( isset( $_GET['basic-auth-logout'] ) ) { + return; + } + // Check if we're on the login page and have Basic Auth credentials if ( 'wp-login.php' === $pagenow && ! empty( $_SERVER['PHP_AUTH_USER'] ) && diff --git a/readme.txt b/readme.txt index ebf5922..54f8f9e 100644 --- a/readme.txt +++ b/readme.txt @@ -4,7 +4,7 @@ Tags: pressable, basic auth, authentication, security Requires at least: 6.7 Tested up to: 6.8 Requires PHP: 8.1 -Stable tag: 1.0.4 +Stable tag: 1.0.5 License: GPLv2 or later License URI: http://www.gnu.org/licenses/gpl-2.0.html diff --git a/tests/hook-registration-test.php b/tests/hook-registration-test.php new file mode 100644 index 0000000..024fae2 --- /dev/null +++ b/tests/hook-registration-test.php @@ -0,0 +1,166 @@ + $hook, 'callback' => $callback, 'priority' => $priority ); +} + +function add_filter( $hook, $callback, $priority = 10, $accepted_args = 1 ) { + $GLOBALS['hooks'][] = array( 'hook' => $hook, 'callback' => $callback, 'priority' => $priority ); +} + +require __DIR__ . '/../pressable-basic-authentication.php'; + +$failures = array(); + +/** + * Records a single assertion. + * + * @param bool $passed Whether the assertion held. + * @param string $description What was asserted. + */ +function check( $passed, $description ) { + global $failures; + + if ( $passed ) { + echo " PASS $description\n"; + return; + } + + $failures[] = $description; + echo " FAIL $description\n"; +} + +/** + * Finds the hook a given method of the plugin class was registered against. + * + * @param string $method Method name. + * @return array|null The recorded registration, or null when unregistered. + */ +function registration_for( $method ) { + foreach ( $GLOBALS['hooks'] as $registration ) { + if ( is_array( $registration['callback'] ) && $registration['callback'][1] === $method ) { + return $registration; + } + } + + return null; +} + +/** + * Returns the source of one method of the plugin class. + * + * @param string $method Method name. + * @return string + */ +function source_of( $method ) { + if ( ! method_exists( 'Pressable_Basic_Auth', $method ) ) { + return ''; + } + + $reflected = new ReflectionMethod( 'Pressable_Basic_Auth', $method ); + $lines = file( $reflected->getFileName() ); + + return implode( + '', + array_slice( $lines, $reflected->getStartLine() - 1, $reflected->getEndLine() - $reflected->getStartLine() + 1 ) + ); +} + +echo "Basic Auth hook registration\n"; + +$logout = registration_for( 'handle_logout_request' ); +check( null !== $logout, 'the logout handler is registered' ); +check( null !== $logout && 'init' === $logout['hook'], "the logout handler is hooked to 'init'" ); + +$boot = registration_for( 'init' ); +check( null !== $boot && 'plugins_loaded' === $boot['hook'], "init() is still hooked to 'plugins_loaded'" ); +check( null !== $boot && 1 === $boot['priority'], 'init() still runs at priority 1, so enforcement stays early' ); + +// Asserted from both sides on purpose. The negative check alone would pass +// vacuously if handle_basic_auth_logout() were renamed -- silently losing the +// coverage this test exists for -- so the positive check pins the name as live. +// Both match on the trailing "(" so a rename to a superstring (…_logout_renamed) +// does not satisfy either check. +check( + false !== strpos( source_of( 'handle_logout_request' ), 'handle_basic_auth_logout(' ), + 'the init callback reaches the logout handler' +); + +check( + false === strpos( source_of( 'init' ), 'handle_basic_auth_logout(' ), + 'init() does not invoke the logout handler, so wp_logout() cannot fire on plugins_loaded' +); + +check( + // Short-circuits so a missing method reports a failure rather than throwing a + // ReflectionException, which would abort the run and hide any later check. + method_exists( 'Pressable_Basic_Auth', 'handle_logout_request' ) + && ( new ReflectionMethod( 'Pressable_Basic_Auth', 'handle_logout_request' ) )->isPublic(), + 'the logout handler is public, as a hook callback must be' +); + +// The guard that skips session setup on a logout request is bracketed rather than +// merely ordered, because both neighbours are hazards. Above the credential +// handling it would suppress the spurious session -- so the symptom it was added +// for would look fixed -- while also skipping the 401, readmitting the +// unauthenticated caller the logout move excluded. Below the cookie calls it would +// be inert. Anchoring on the LAST send_auth_headers() rather than wp_authenticate() +// is deliberate: between the two, a guard still clears every ordering check yet lets +// INVALID credentials bypass the challenge. +$force = source_of( 'force_basic_authentication' ); +$guard_at = strpos( $force, "\$_GET['basic-auth-logout']" ); +$last_challenge = strrpos( $force, 'send_auth_headers(' ); +$cookie_at = strpos( $force, 'wp_set_auth_cookie(' ); + +check( + false !== $guard_at && false !== $last_challenge && $guard_at > $last_challenge, + 'the logout guard sits after every credential challenge, so a logout still requires valid credentials' +); + +check( + false !== $guard_at && false !== $cookie_at && $guard_at < $cookie_at, + 'the logout guard sits before wp_set_auth_cookie(), so a logout establishes no session' +); + +// Position alone would be satisfied by a guard whose body no longer returns. +check( + 1 === preg_match( '/if \(\s*isset\(\s*\$_GET\[.basic-auth-logout.\]\s*\)\s*\)\s*\{\s*return;\s*\}/', $force ), + 'the logout guard actually returns, rather than only appearing in the right place' +); + +echo "\n"; + +if ( $failures ) { + echo count( $failures ) . " failure(s)\n"; + exit( 1 ); +} + +echo "All checks passed\n"; +exit( 0 ); From 5c6e1f41281a6c2199c08f1751ddea98fa026056 Mon Sep 17 00:00:00 2001 From: loukieluke <84424020+loukieluke@users.noreply.github.com> Date: Tue, 15 Sep 2026 10:12:44 -0500 Subject: [PATCH 02/11] ENG-7204: Match excluded endpoints on the request path, not the raw URI should_skip_auth() substring-matched its excluded endpoints against the whole REQUEST_URI, which includes the query string. Any caller could therefore waive Basic Authentication on any URL by appending one as a parameter value, with no credentials at all: GET /?x=wp-json/wp/v2 -> 200, full front page GET /?foo=xmlrpc.php -> 200 GET /wp-login.php?x=wp-json/wp/v2 -> 200, login form rendered POST the same with log/pwd -> 302 to /wp-admin/, session cookie issued The last of those bypasses the gate entirely: the login form is exposed and a normal WordPress login succeeds, which also restores unlimited credential brute-forcing against sites whose only protection is this plugin. Matching now runs against the path from parse_url(), with both ends anchored on a slash so a needle matches whole path segments -- `/notwp-json/wp/v2` no longer satisfies `wp-json/wp/v2`. The path is deliberately not anchored at its start, because a subdirectory or multisite subsite install serves these endpoints below a prefix; `/sub1/wp-json/wp/v2/posts` must still be excluded. Present in every released version, so this is not a regression from the logout work on this branch -- it is separable and can be cherry-picked ahead of it. Verified against a WordPress 7.1 harness, single-site and subdirectory multisite: all six bypass shapes now answer 401, and xmlrpc.php, wp-json/wp/v2, wp-json/wp/v3 and wp-json/jetpack stay reachable, including below a subsite prefix. The seven new assertions in the hook-registration test all fail against the old matching. --- pressable-basic-authentication.php | 17 ++++++++- tests/hook-registration-test.php | 55 ++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/pressable-basic-authentication.php b/pressable-basic-authentication.php index 3cc58e1..e22aa21 100644 --- a/pressable-basic-authentication.php +++ b/pressable-basic-authentication.php @@ -194,9 +194,24 @@ private function should_skip_auth() { return true; } + // Match the request PATH only, never the raw REQUEST_URI. A substring test + // against the whole URI also reads the query string, so any caller could + // disable this plugin on any URL by appending an excluded endpoint as a + // parameter value -- `/?x=wp-json/wp/v2` served the front page, and the same + // string on wp-login.php exposed the login form and allowed a full WordPress + // login with no Basic Auth at all. + // + // Both ends are anchored on a slash so the needle matches whole path + // segments: `/notwp-json/wp/v2` must not satisfy `wp-json/wp/v2`. The path is + // not anchored at its start, because a subdirectory or multisite subsite + // install legitimately serves these endpoints below a prefix + // (`/sub1/wp-json/wp/v2/posts`). + $request_path = '/' . ltrim((string) parse_url($request_uri, PHP_URL_PATH), '/'); + $haystack = rtrim($request_path, '/') . '/'; + // Check all excluded endpoints foreach ($excluded_endpoints as $endpoint) { - if (strpos($request_uri, $endpoint) !== false) { + if (strpos($haystack, '/' . trim($endpoint, '/') . '/') !== false) { return true; } } diff --git a/tests/hook-registration-test.php b/tests/hook-registration-test.php index 024fae2..78b664a 100644 --- a/tests/hook-registration-test.php +++ b/tests/hook-registration-test.php @@ -155,6 +155,61 @@ function source_of( $method ) { 'the logout guard actually returns, rather than only appearing in the right place' ); +/** + * Whether should_skip_auth() would waive Basic Authentication for a request URI. + * + * Invoked for real rather than inspected as source: the method touches only + * basename() and parse_url(), so it runs without WordPress, and a behavioural + * assertion cannot be satisfied by a rewrite that merely looks different. + * + * @param string $uri Value to place in REQUEST_URI. + * @return bool + */ +function skips_auth_for( $uri ) { + $_SERVER['REQUEST_URI'] = $uri; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $method = new ReflectionMethod( 'Pressable_Basic_Auth', 'should_skip_auth' ); + $method->setAccessible( true ); + + return (bool) $method->invoke( new Pressable_Basic_Auth() ); +} + +// An excluded endpoint appearing in the QUERY STRING must never waive +// authentication. Matching those needles against the whole REQUEST_URI let any +// caller disable the plugin on any URL -- `/?x=wp-json/wp/v2` served the front +// page, and the same string on wp-login.php exposed the login form and allowed a +// full WordPress login with no Basic Auth at all. +foreach ( array( + '/?x=wp-json/wp/v2', + '/?foo=xmlrpc.php', + '/?x=wp-json/jetpack', + '/?x=wp-json/wp/v3', + '/wp-login.php?x=wp-json/wp/v2', + '/?p=1&x=wp-json/wp/v2', +) as $uri ) { + check( false === skips_auth_for( $uri ), "an excluded endpoint in the query string does not waive auth: $uri" ); +} + +// A needle must match whole path segments, not any substring of one. +check( false === skips_auth_for( '/notwp-json/wp/v2' ), 'a path segment merely ENDING in an excluded endpoint does not waive auth' ); + +// The genuine exclusions still have to work, including below a subdirectory or +// multisite subsite prefix -- which is why the path is not anchored at its start. +foreach ( array( + '/xmlrpc.php', + '/wp-json/wp/v2/posts', + '/wp-json/wp/v2/posts?per_page=1', + '/wp-json/jetpack/v4/whatever', + '/wp-json/wp/v3/anything', + '/sub1/wp-json/wp/v2/posts', + '/sub1/xmlrpc.php', +) as $uri ) { + check( true === skips_auth_for( $uri ), "a genuine excluded endpoint still waives auth: $uri" ); +} + +check( false === skips_auth_for( '/' ), 'an ordinary request is still gated' ); + echo "\n"; if ( $failures ) { From 66dd9b0608b4ccec1869cd5bc7a6ae9b12eeb641 Mon Sep 17 00:00:00 2001 From: loukieluke <84424020+loukieluke@users.noreply.github.com> Date: Tue, 15 Sep 2026 10:13:17 -0500 Subject: [PATCH 03/11] ENG-7204: Pin CI to the supported PHP range, and record what the init hook costs Review follow-ups, no behaviour change to the plugin itself. - Both workflows ran on the runner's ambient PHP, so neither check ever exercised the `Requires PHP: 8.1` floor the readme declares. test.yml now runs a matrix of 8.1 and 8.4; the release gate in main.yml pins the floor, since that is the version most likely to break and the one a release must not ship broken. Closes the gap raised as ENG-7193. - main.yml's checkout kept the default persisted GITHUB_TOKEN while now running repository PHP ahead of publishing. test.yml already set persist-credentials false for that reason; main.yml matches it. The release script still receives the token explicitly, and `git archive main` reads a local ref, so neither step depends on the credential helper. - The `init` hook comment recorded why the logout moved there but not what it costs: output emitted while plugins load leaves the 401 and the cookie clearing unable to send, so the logout degrades to a 200 with no challenge while still destroying the session server-side. Reproduced with Kadence Security under WP_DEBUG display. `plugins_loaded` at PHP_INT_MAX was measured as an alternative and degrades identically, so the comment records the tension rather than a fix. - The test file's guard comment claimed the sapi check stopped the file answering 200. It does not -- exit status is not an HTTP status; it prevents the output, and `/tests export-ignore` is what keeps the file out of the release. - readme.txt had no changelog at all, so 1.0.3, its withdrawal in 1.0.4 and this release left no user-visible trail. Added, and `Tested up to` moved to 7.1, which is what this branch was verified against. --- .github/workflows/main.yml | 13 +++++++++++++ .github/workflows/test.yml | 16 +++++++++++++++- pressable-basic-authentication.php | 12 ++++++++++++ readme.txt | 25 +++++++++++++++++++++++-- tests/hook-registration-test.php | 5 ++++- 5 files changed, 67 insertions(+), 4 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 16032c8..71201c9 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -17,6 +17,19 @@ jobs: steps: - name: Checkout code uses: actions/checkout@v3 + with: + # This job runs repository PHP (below) before publishing, so the + # GITHUB_TOKEN is kept out of the local git config. The release script + # receives it explicitly as an env var instead. + persist-credentials: false + + - name: Set up PHP + uses: shivammathur/setup-php@v2 + with: + # The floor declared in readme.txt -- the gate should run against the + # oldest version the plugin claims to support, not the runner's ambient one. + php-version: '8.1' + coverage: none # The release is published straight from this job, and no ordering exists # between separate workflow files, so the checks in test.yml cannot gate it diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 237951c..8fba562 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -11,9 +11,17 @@ permissions: jobs: test: - name: Lint and hook registration + name: Lint and hook registration (PHP ${{ matrix.php }}) runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + # The floor declared in readme.txt, and a current release. The runner's + # ambient PHP is whatever the image ships and tracks neither, so relying + # on it tested a version the plugin does not claim to support. + php: [ '8.1', '8.4' ] + steps: - name: Checkout code uses: actions/checkout@v4 @@ -23,6 +31,12 @@ jobs: # config where that code could read it. persist-credentials: false + - name: Set up PHP ${{ matrix.php }} + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + coverage: none + - name: Report PHP version run: php -v diff --git a/pressable-basic-authentication.php b/pressable-basic-authentication.php index e22aa21..87d1a35 100644 --- a/pressable-basic-authentication.php +++ b/pressable-basic-authentication.php @@ -40,6 +40,18 @@ public function __construct() { // first, User Switching's `wp_logout` subscriber fatals on a constant it has // not defined yet. Hooking to `init` drops the dependency on load order // entirely: every `plugins_loaded` callback has completed by then. + // + // The cost of running this late is that output may already have been sent: + // anything echoed while plugins load -- a `_doing_it_wrong()` notice under + // WP_DEBUG display, a stray BOM -- makes the 401 header and the cookie + // clearing below fail, leaving a 200 with no challenge. wp_logout() itself + // still runs, so the session is destroyed server-side; what is lost is the + // browser-visible half. `plugins_loaded` at PHP_INT_MAX was measured as an + // alternative and degrades identically, because the output is emitted during + // that same hook. The two requirements are in tension: running after every + // `plugins_loaded` callback necessarily means running after any of them may + // have printed, and priority 1 -- the only position that avoids output -- is + // the position that causes the race above. add_action( 'init', array( $this, 'handle_logout_request' ), 1 ); // Add filter for logout URL. diff --git a/readme.txt b/readme.txt index 54f8f9e..30ff7e0 100644 --- a/readme.txt +++ b/readme.txt @@ -2,7 +2,7 @@ Contributors: pressable Tags: pressable, basic auth, authentication, security Requires at least: 6.7 -Tested up to: 6.8 +Tested up to: 7.1 Requires PHP: 8.1 Stable tag: 1.0.5 License: GPLv2 or later @@ -40,4 +40,25 @@ No manual installation is necessary.​ == Screenshots ==​ -* Initial release​ \ No newline at end of file +* Initial release​ + +== Changelog == + += 1.0.5 = +* Fixed: an excluded endpoint appearing anywhere in a request's query string + disabled Basic Authentication for that request, including on wp-login.php. + Exclusions now match the request path only. +* Fixed: logging out on a site also running User Switching caused a fatal error. +* Fixed: a logout request no longer reaches wp_logout() without valid credentials. +* Fixed: a logout URL without action=logout is no longer redirected away from the + logout while still signed in. + += 1.0.4 = +* Reverted the 1.0.3 changes pending verification. Functionally identical to 1.0.2. + += 1.0.3 = +* Deferred logout handling to init to avoid the User Switching conflict. Withdrawn + in 1.0.4; re-issued, with the exclusion fix above, in 1.0.5. + += 1.0.2 = +* PHP 8.4 compatibility. diff --git a/tests/hook-registration-test.php b/tests/hook-registration-test.php index 78b664a..637187d 100644 --- a/tests/hook-registration-test.php +++ b/tests/hook-registration-test.php @@ -18,7 +18,10 @@ // This file defines ABSPATH itself, so the usual `defined( 'ABSPATH' ) || exit` // plugin guard cannot protect it. It sits inside the plugin directory, which the // web server serves directly without loading WordPress -- so without this guard it -// answers 200 on a site where Basic Authentication returns 401 for everything else. +// would print its own output on a site where Basic Authentication returns 401 for +// everything else. The exit status is not an HTTP status: the request still answers +// 200, just with an empty body. `/tests export-ignore` keeps the file out of the +// release zip entirely; this guard is the second layer, for a checkout served direct. if ( 'cli' !== php_sapi_name() ) { exit( 1 ); } From d930787deb735570ff18758c52c635d3f86d97b6 Mon Sep 17 00:00:00 2001 From: loukieluke <84424020+loukieluke@users.noreply.github.com> Date: Tue, 15 Sep 2026 10:18:38 -0500 Subject: [PATCH 04/11] ENG-7204: Refuse to waive auth for a path carrying a traversal segment The path-matching fix closed the query-string vector but not this one. The server resolves `.` and `..` when it maps a request to a file, so the path this plugin inspects is not the path that gets served: GET /xmlrpc.php/../wp-login.php -> 200, login form rendered POST the same with log/pwd -> 302 to /wp-admin/, session cookie issued REQUEST_URI still reads as the excluded xmlrpc endpoint, so authentication was waived, while Apache served wp-login.php. Same full bypass as before by a different spelling, and the encoded forms (`%2e%2e`, `xmlrpc%2ephp`) reach it too. The path is now compared after rawurldecode(), because the server decodes before it resolves, and endpoint matching is skipped entirely for any path containing a `.` or `..` segment. A genuine excluded endpoint never carries one, so refusing is correct and avoids re-implementing the server's own path resolution here. Only the endpoint matching is skipped, not the XMLRPC_REQUEST / REST_REQUEST constant checks: a real xmlrpc.php request defines XMLRPC_REQUEST before this plugin loads and stays excluded on that evidence, which a caller cannot spell. Verified against the harness: all seven traversal and encoded shapes now answer 401, the login POST issues no session, and xmlrpc.php, wp-json/wp/v2, wp-json/wp/v3 and wp-json/jetpack stay reachable. Seven new assertions cover the class and all seven fail with the guard removed. --- pressable-basic-authentication.php | 26 ++++++++++++++++++++++---- tests/hook-registration-test.php | 16 ++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/pressable-basic-authentication.php b/pressable-basic-authentication.php index 87d1a35..3f6e08c 100644 --- a/pressable-basic-authentication.php +++ b/pressable-basic-authentication.php @@ -218,13 +218,31 @@ private function should_skip_auth() { // not anchored at its start, because a subdirectory or multisite subsite // install legitimately serves these endpoints below a prefix // (`/sub1/wp-json/wp/v2/posts`). - $request_path = '/' . ltrim((string) parse_url($request_uri, PHP_URL_PATH), '/'); + // Compared after decoding, because the server decodes before it resolves the + // path: `/xmlrpc%2ephp` and `/xmlrpc.php` reach the same file. + $request_path = rawurldecode('/' . ltrim((string) parse_url($request_uri, PHP_URL_PATH), '/')); $haystack = rtrim($request_path, '/') . '/'; + // A path carrying a `.` or `..` segment is not the path that gets served -- + // the server resolves those when mapping the request to a file, so + // `/xmlrpc.php/../wp-login.php` reads as the excluded xmlrpc endpoint while + // actually reaching wp-login.php. That served the login form and allowed a + // full WordPress login with no Basic Auth. A genuine excluded endpoint never + // contains a traversal segment, so refuse to waive authentication for one + // rather than trying to re-implement the server's resolution here. + // + // Only the endpoint matching is skipped, not the constant checks below: a + // real xmlrpc.php request defines XMLRPC_REQUEST before this plugin loads and + // stays excluded on that evidence, which cannot be spelled by a caller. + $segments = explode('/', $haystack); + $has_traversal = in_array('..', $segments, true) || in_array('.', $segments, true); + // Check all excluded endpoints - foreach ($excluded_endpoints as $endpoint) { - if (strpos($haystack, '/' . trim($endpoint, '/') . '/') !== false) { - return true; + if (!$has_traversal) { + foreach ($excluded_endpoints as $endpoint) { + if (strpos($haystack, '/' . trim($endpoint, '/') . '/') !== false) { + return true; + } } } diff --git a/tests/hook-registration-test.php b/tests/hook-registration-test.php index 637187d..5bd4e2f 100644 --- a/tests/hook-registration-test.php +++ b/tests/hook-registration-test.php @@ -194,6 +194,22 @@ function skips_auth_for( $uri ) { check( false === skips_auth_for( $uri ), "an excluded endpoint in the query string does not waive auth: $uri" ); } +// A path carrying a traversal segment is not the path the server ends up serving, +// so it must never waive authentication: `/xmlrpc.php/../wp-login.php` resolves to +// wp-login.php while reading as the excluded xmlrpc endpoint, which served the login +// form and allowed a full WordPress login with no Basic Auth at all. +foreach ( array( + '/xmlrpc.php/../wp-login.php', + '/xmlrpc.php/%2e%2e/wp-login.php', + '/xmlrpc%2ephp/../wp-login.php', + '/wp-json/wp/v2/../wp-login.php', + '/wp-json/wp/v2/../../wp-login.php', + '/xmlrpc.php/./../wp-login.php', + '/xmlrpc.php/../', +) as $uri ) { + check( false === skips_auth_for( $uri ), "a traversal segment does not waive auth: $uri" ); +} + // A needle must match whole path segments, not any substring of one. check( false === skips_auth_for( '/notwp-json/wp/v2' ), 'a path segment merely ENDING in an excluded endpoint does not waive auth' ); From 67adb00ed0801d8d057931179ae275c5db435a1c Mon Sep 17 00:00:00 2001 From: loukieluke <84424020+loukieluke@users.noreply.github.com> Date: Tue, 15 Sep 2026 10:48:21 -0500 Subject: [PATCH 05/11] ENG-7204: Gate the release on the same PHP range as pull requests Review follow-ups. No change to the plugin's behaviour. - The release gate covered only PHP 8.1 while pull requests covered 8.1 and 8.4, so a break on 8.4 would have failed every pull request and still shipped. The checks now run as a matrix job in main.yml that the release job `needs:`, so both legs must pass before anything is published. Kept as a separate job rather than matrix steps on the release job itself: a matrix there would run `git archive` and the publish step once per PHP version, racing to create the same tag. The release job is now byte-identical to main except for that `needs:` line -- the earlier `persist-credentials: false` on its checkout is gone, because the job no longer runs repository PHP. That hardening lives on the new check job, which does. `git archive main` is therefore untouched. - test.yml drops its push-to-main trigger, which would otherwise duplicate the new check job on every push. - PHP 8.4 was added to CI without ever having been run against this plugin. It has now been: the full request matrix, the User Switching switch-then-logout flow, both bypass classes and the hook-registration suite all pass on 8.4.25, with no plugin-attributed deprecations. - The changelog described only the query-string half of the bypass, so a reader would have concluded a request path could still get through. It now covers both spellings and what replaced them. - skips_auth_for() overwrote $_SERVER and left it overwritten, and built a fresh plugin instance per call, appending to the recorded hook list each time. Both are invisible today only because every hook-wiring check runs above it. It now restores $_SERVER in a finally block and reuses one instance. --- .github/workflows/main.yml | 54 +++++++++++++++++++++++--------- .github/workflows/test.yml | 6 ++-- readme.txt | 10 ++++-- tests/hook-registration-test.php | 22 +++++++++++-- 4 files changed, 69 insertions(+), 23 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 71201c9..3e7eb5b 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -10,38 +10,64 @@ env: VERSION_FILE_PATH: './pressable-basic-authentication.php' jobs: - build: - name: Package Release Project + # A separate job from the release below, rather than steps inside it, so the + # checks can fan out across the supported PHP range while the release itself + # still happens exactly once. Putting the matrix on the release job would run + # `git archive` and the publish step once per PHP version, racing to create the + # same tag. + # + # The same span as test.yml, deliberately: a version good enough to block a pull + # request is good enough to block a release, and gating the release on a + # narrower range would let a break on the untested version ship precisely + # because nothing stopped it. + check: + name: Lint and hook registration (PHP ${{ matrix.php }}) runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + # The floor declared in readme.txt, and a current release. The runner's + # ambient PHP is whatever the image ships and tracks neither. + php: [ '8.1', '8.4' ] + steps: - name: Checkout code uses: actions/checkout@v3 with: - # This job runs repository PHP (below) before publishing, so the - # GITHUB_TOKEN is kept out of the local git config. The release script - # receives it explicitly as an env var instead. + # This job lints and executes repository PHP, so the GITHUB_TOKEN must + # not be left in the local git config where that code could read it. persist-credentials: false - - name: Set up PHP + - name: Set up PHP ${{ matrix.php }} uses: shivammathur/setup-php@v2 with: - # The floor declared in readme.txt -- the gate should run against the - # oldest version the plugin claims to support, not the runner's ambient one. - php-version: '8.1' + php-version: ${{ matrix.php }} coverage: none - # The release is published straight from this job, and no ordering exists - # between separate workflow files, so the checks in test.yml cannot gate it - # from outside -- a push that breaks them would still ship a Release. These - # run here, ahead of the archive, so a failure stops the release rather than - # being reported next to one. + - name: Report PHP version + run: php -v + - name: Lint PHP files run: find . -path ./vendor -prune -o -name '*.php' -print0 | xargs -0 -n1 -- php -l - name: Hook registration regression test run: php tests/hook-registration-test.php + build: + name: Package Release Project + runs-on: ubuntu-latest + + # Every matrix leg must pass before anything is published. Two separate + # workflow files have no ordering between them, so test.yml alone could never + # stop a broken commit from shipping a Release -- within one workflow, this + # does. + needs: check + + steps: + - name: Checkout code + uses: actions/checkout@v3 + - name: Install Ruby uses: ruby/setup-ruby@v1 with: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8fba562..1442198 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,10 +1,10 @@ name: Tests +# Pull requests only. A push to main runs the same checks as the `check` job in +# main.yml, where they additionally gate the release; running them here too would +# just duplicate that. on: pull_request: - push: - branches: - - main permissions: contents: read diff --git a/readme.txt b/readme.txt index 30ff7e0..0b33ed6 100644 --- a/readme.txt +++ b/readme.txt @@ -45,9 +45,13 @@ No manual installation is necessary.​ == Changelog == = 1.0.5 = -* Fixed: an excluded endpoint appearing anywhere in a request's query string - disabled Basic Authentication for that request, including on wp-login.php. - Exclusions now match the request path only. +* Fixed: Basic Authentication could be bypassed entirely on any URL, with no + credentials, by making the request resemble one of the endpoints excluded from + authentication -- either by naming one in the query string + (`/?x=wp-json/wp/v2`) or by reaching a gated page through one + (`/xmlrpc.php/../wp-login.php`). Both served the login form and allowed a full + WordPress sign-in. Exclusions now match the decoded request path only, and are + refused for any path containing a `.` or `..` segment. * Fixed: logging out on a site also running User Switching caused a fatal error. * Fixed: a logout request no longer reaches wp_logout() without valid credentials. * Fixed: a logout URL without action=logout is no longer redirected away from the diff --git a/tests/hook-registration-test.php b/tests/hook-registration-test.php index 5bd4e2f..96daf45 100644 --- a/tests/hook-registration-test.php +++ b/tests/hook-registration-test.php @@ -169,13 +169,29 @@ function source_of( $method ) { * @return bool */ function skips_auth_for( $uri ) { + // $_SERVER is restored and the plugin instance reused so these checks leave no + // state behind: every hook-wiring check above reads $GLOBALS['hooks'] and + // $_SERVER, and a later one appended below this point would otherwise read + // whatever the last URI here happened to set. + static $plugin = null; + + if ( null === $plugin ) { + $plugin = new Pressable_Basic_Auth(); + } + + $original = $_SERVER; + $_SERVER['REQUEST_URI'] = $uri; $_SERVER['SCRIPT_NAME'] = '/index.php'; - $method = new ReflectionMethod( 'Pressable_Basic_Auth', 'should_skip_auth' ); - $method->setAccessible( true ); + try { + $method = new ReflectionMethod( 'Pressable_Basic_Auth', 'should_skip_auth' ); + $method->setAccessible( true ); - return (bool) $method->invoke( new Pressable_Basic_Auth() ); + return (bool) $method->invoke( $plugin ); + } finally { + $_SERVER = $original; + } } // An excluded endpoint appearing in the QUERY STRING must never waive From 85f92db4fe579771a1fcadfc6cc67578cb9b90ef Mon Sep 17 00:00:00 2001 From: loukieluke <84424020+loukieluke@users.noreply.github.com> Date: Tue, 15 Sep 2026 10:55:27 -0500 Subject: [PATCH 06/11] ENG-7204: Match what the server executes, not what the caller requested A third spelling of the same bypass, found while reviewing the second fix: GET /wp-login.php/wp-json/wp/v2/ -> 200, login form rendered POST the same with log/pwd -> 302 to /wp-admin/, session cookie issued No query string and no traversal segment, so neither previous guard applied. The server executes wp-login.php and hands `/wp-json/wp/v2/` to it as PATH_INFO, while the plugin read that trailing text as an excluded endpoint and waived authentication. Confirmed with a probe script: REQUEST_URI carries the endpoint, SCRIPT_NAME is `/wp-login.php`, PATH_INFO is the rest. Three vectors in three review passes is the signal: matching the requested URI at all was the fault, and each fix addressed a spelling of it rather than the fault. So the method now asks what the server will actually run. - xmlrpc.php is matched on SCRIPT_NAME, which is the resolved script, so it holds however the request was spelled -- including `/sub1/xmlrpc.php`, which the multisite rewrite resolves back to the root script. It is no longer in the string-matched list at all. - The REST endpoints are rewrite targets, so they only mean anything when the request is routed to index.php. Endpoint matching is refused for any path with a segment ending `.php` (the server is executing that script, and the rest is PATH_INFO) or a `.`/`..` segment (the path resolves to something other than what it reads as). Both checked on the decoded path, since the server decodes first. - The XMLRPC_REQUEST / REST_REQUEST constants still apply, and are the strongest signal available: the request's own execution defines them, so a caller cannot spell them. Verified against WordPress 7.1 on PHP 8.4 and 8.1, single-site and subdirectory multisite: all nine bypass shapes across the three classes answer 401, the PATH_INFO login POST issues no session, and xmlrpc.php, wp-json/wp/v2, wp-json/wp/v3 and wp-json/jetpack stay reachable, including below a subsite prefix. The full request matrix and the User Switching flow are unchanged. 38 assertions now cover the method; the three guards mutation-check independently at 6, 5 and 3 failures. --- pressable-basic-authentication.php | 73 ++++++++++++++++-------------- readme.txt | 14 +++--- tests/hook-registration-test.php | 38 ++++++++++++++-- 3 files changed, 81 insertions(+), 44 deletions(-) diff --git a/pressable-basic-authentication.php b/pressable-basic-authentication.php index 3f6e08c..0266e1a 100644 --- a/pressable-basic-authentication.php +++ b/pressable-basic-authentication.php @@ -189,9 +189,10 @@ private function log_failed_auth( $message ) { * @return bool */ private function should_skip_auth() { - // List of endpoints to exclude from Basic Auth + // REST rewrite targets only. xmlrpc.php is deliberately NOT in this list -- + // it is matched on SCRIPT_NAME below, which is authoritative in a way a + // requested path is not. $excluded_endpoints = array( - 'xmlrpc.php', 'wp-json/jetpack', 'wp-json/wp/v2', 'wp-json/wp/v3' @@ -201,44 +202,48 @@ private function should_skip_auth() { $request_uri = $_SERVER['REQUEST_URI'] ?? ''; $script_name = $_SERVER['SCRIPT_NAME'] ?? ''; - // Check if this is a direct xmlrpc.php request + // SCRIPT_NAME is the script the server actually resolved, so this holds + // however the caller spelled the request. if (basename($script_name) === 'xmlrpc.php') { return true; } - // Match the request PATH only, never the raw REQUEST_URI. A substring test - // against the whole URI also reads the query string, so any caller could - // disable this plugin on any URL by appending an excluded endpoint as a - // parameter value -- `/?x=wp-json/wp/v2` served the front page, and the same - // string on wp-login.php exposed the login form and allowed a full WordPress - // login with no Basic Auth at all. + // Everything below matches the REQUESTED path, which is not necessarily + // what the server serves. Three spellings made a gated page look like an + // excluded endpoint, each waiving authentication entirely and allowing a + // full WordPress sign-in with no credentials: // - // Both ends are anchored on a slash so the needle matches whole path - // segments: `/notwp-json/wp/v2` must not satisfy `wp-json/wp/v2`. The path is - // not anchored at its start, because a subdirectory or multisite subsite - // install legitimately serves these endpoints below a prefix - // (`/sub1/wp-json/wp/v2/posts`). - // Compared after decoding, because the server decodes before it resolves the - // path: `/xmlrpc%2ephp` and `/xmlrpc.php` reach the same file. + // /?x=wp-json/wp/v2 query string read as part of the path + // /xmlrpc.php/../wp-login.php `..` resolved by the server afterwards + // /wp-login.php/wp-json/wp/v2/ trailing segments land in PATH_INFO + // + // The guard below is written against the general fault rather than those + // three shapes: the endpoints above are rewrite targets, so they only mean + // anything when the server routes the request to index.php. Decoded first, + // because the server decodes before it resolves. $request_path = rawurldecode('/' . ltrim((string) parse_url($request_uri, PHP_URL_PATH), '/')); $haystack = rtrim($request_path, '/') . '/'; - // A path carrying a `.` or `..` segment is not the path that gets served -- - // the server resolves those when mapping the request to a file, so - // `/xmlrpc.php/../wp-login.php` reads as the excluded xmlrpc endpoint while - // actually reaching wp-login.php. That served the login form and allowed a - // full WordPress login with no Basic Auth. A genuine excluded endpoint never - // contains a traversal segment, so refuse to waive authentication for one - // rather than trying to re-implement the server's resolution here. - // - // Only the endpoint matching is skipped, not the constant checks below: a - // real xmlrpc.php request defines XMLRPC_REQUEST before this plugin loads and - // stays excluded on that evidence, which cannot be spelled by a caller. - $segments = explode('/', $haystack); - $has_traversal = in_array('..', $segments, true) || in_array('.', $segments, true); - - // Check all excluded endpoints - if (!$has_traversal) { + // A `.php` segment means the server is executing that script and handing + // the rest to it as PATH_INFO; a `.` or `..` segment means the path is + // resolved to something other than what it reads as. In neither case is the + // request routed to index.php, so an endpoint appearing in it is decoration, + // not a destination -- refuse rather than re-implement the server's own + // path resolution here. + $serves_another_script = false; + + foreach (explode('/', $haystack) as $segment) { + if ('.' === $segment || '..' === $segment || '.php' === strtolower(substr($segment, -4))) { + $serves_another_script = true; + break; + } + } + + // Check all excluded endpoints. Anchored on a slash at both ends so a needle + // matches whole path segments -- `/notwp-json/wp/v2` must not satisfy + // `wp-json/wp/v2` -- but not anchored at the start of the path, because a + // subdirectory or multisite subsite install serves these below a prefix. + if (!$serves_another_script) { foreach ($excluded_endpoints as $endpoint) { if (strpos($haystack, '/' . trim($endpoint, '/') . '/') !== false) { return true; @@ -246,7 +251,9 @@ private function should_skip_auth() { } } - // Check WordPress constants + // Check WordPress constants. xmlrpc.php and the REST bootstrap define these + // themselves, so they are evidence from the request's own execution rather + // than from how it was spelled. if (defined('XMLRPC_REQUEST') && XMLRPC_REQUEST) { return true; } diff --git a/readme.txt b/readme.txt index 0b33ed6..d1906ba 100644 --- a/readme.txt +++ b/readme.txt @@ -46,12 +46,14 @@ No manual installation is necessary.​ = 1.0.5 = * Fixed: Basic Authentication could be bypassed entirely on any URL, with no - credentials, by making the request resemble one of the endpoints excluded from - authentication -- either by naming one in the query string - (`/?x=wp-json/wp/v2`) or by reaching a gated page through one - (`/xmlrpc.php/../wp-login.php`). Both served the login form and allowed a full - WordPress sign-in. Exclusions now match the decoded request path only, and are - refused for any path containing a `.` or `..` segment. + credentials, by making a request to a gated page resemble one of the endpoints + excluded from authentication -- by naming one in the query string + (`/?x=wp-json/wp/v2`), by reaching the page through one + (`/xmlrpc.php/../wp-login.php`), or by trailing one after it + (`/wp-login.php/wp-json/wp/v2/`). All three served the login form and allowed a + full WordPress sign-in. xmlrpc.php is now matched on the script the server + actually resolved, and the REST endpoints only on a decoded request path that + the server routes to index.php. * Fixed: logging out on a site also running User Switching caused a fatal error. * Fixed: a logout request no longer reaches wp_logout() without valid credentials. * Fixed: a logout URL without action=logout is no longer redirected away from the diff --git a/tests/hook-registration-test.php b/tests/hook-registration-test.php index 96daf45..a277fb8 100644 --- a/tests/hook-registration-test.php +++ b/tests/hook-registration-test.php @@ -165,10 +165,14 @@ function source_of( $method ) { * basename() and parse_url(), so it runs without WordPress, and a behavioural * assertion cannot be satisfied by a rewrite that merely looks different. * - * @param string $uri Value to place in REQUEST_URI. + * @param string $uri Value to place in REQUEST_URI. + * @param string $script_name Value to place in SCRIPT_NAME -- the script the server + * resolved, which is what xmlrpc.php is matched on and + * what a PATH_INFO request leaves pointing at the script + * rather than at the endpoint trailing it. * @return bool */ -function skips_auth_for( $uri ) { +function skips_auth_for( $uri, $script_name = '/index.php' ) { // $_SERVER is restored and the plugin instance reused so these checks leave no // state behind: every hook-wiring check above reads $GLOBALS['hooks'] and // $_SERVER, and a later one appended below this point would otherwise read @@ -182,7 +186,7 @@ function skips_auth_for( $uri ) { $original = $_SERVER; $_SERVER['REQUEST_URI'] = $uri; - $_SERVER['SCRIPT_NAME'] = '/index.php'; + $_SERVER['SCRIPT_NAME'] = $script_name; try { $method = new ReflectionMethod( 'Pressable_Basic_Auth', 'should_skip_auth' ); @@ -210,6 +214,21 @@ function skips_auth_for( $uri ) { check( false === skips_auth_for( $uri ), "an excluded endpoint in the query string does not waive auth: $uri" ); } +// Trailing segments after a real script land in PATH_INFO: the server executes the +// script and hands the rest to it, so an endpoint spelled there is decoration on a +// gated page. `/wp-login.php/wp-json/wp/v2/` served the login form and allowed a +// full WordPress sign-in with no Basic Auth at all. SCRIPT_NAME is passed as the +// server would set it, and the guard holds on the path alone regardless. +foreach ( array( + array( '/wp-login.php/wp-json/wp/v2/', '/wp-login.php' ), + array( '/wp-login.php/xmlrpc.php/', '/wp-login.php' ), + array( '/index.php/wp-json/wp/v2/', '/index.php' ), + array( '/wp-login.PHP/wp-json/wp/v2/', '/wp-login.PHP' ), + array( '/sub1/wp-login.php/wp-json/wp/v2/', '/wp-login.php' ), +) as $case ) { + check( false === skips_auth_for( $case[0], $case[1] ), "a PATH_INFO endpoint after a script does not waive auth: {$case[0]}" ); +} + // A path carrying a traversal segment is not the path the server ends up serving, // so it must never waive authentication: `/xmlrpc.php/../wp-login.php` resolves to // wp-login.php while reading as the excluded xmlrpc endpoint, which served the login @@ -232,17 +251,26 @@ function skips_auth_for( $uri ) { // The genuine exclusions still have to work, including below a subdirectory or // multisite subsite prefix -- which is why the path is not anchored at its start. foreach ( array( - '/xmlrpc.php', '/wp-json/wp/v2/posts', '/wp-json/wp/v2/posts?per_page=1', '/wp-json/jetpack/v4/whatever', '/wp-json/wp/v3/anything', '/sub1/wp-json/wp/v2/posts', - '/sub1/xmlrpc.php', ) as $uri ) { check( true === skips_auth_for( $uri ), "a genuine excluded endpoint still waives auth: $uri" ); } +// xmlrpc.php is matched on SCRIPT_NAME, not on the requested path, so it holds +// however the request was spelled -- including below a multisite subsite prefix, +// which the network rewrite resolves back to the root script. +foreach ( array( + array( '/xmlrpc.php', '/xmlrpc.php' ), + array( '/sub1/xmlrpc.php', '/xmlrpc.php' ), + array( '/xmlrpc.php?for=jetpack', '/xmlrpc.php' ), +) as $case ) { + check( true === skips_auth_for( $case[0], $case[1] ), "xmlrpc.php still waives auth via SCRIPT_NAME: {$case[0]}" ); +} + check( false === skips_auth_for( '/' ), 'an ordinary request is still gated' ); echo "\n"; From 4bd4fe48b81a934e2433c20136de0e86030bead8 Mon Sep 17 00:00:00 2001 From: loukieluke <84424020+loukieluke@users.noreply.github.com> Date: Tue, 15 Sep 2026 11:20:24 -0500 Subject: [PATCH 07/11] ENG-7204: Only refuse a REST endpoint when a script PRECEDES it Codex pre-PR review, P2, accepted. The previous commit scanned the whole request path for a `.php` segment and refused the exclusion if it found one anywhere. That also refuses a valid REST route which merely contains a later `.php` segment, so a request WordPress would dispatch to the REST API was answered with a Basic Auth challenge instead. Verified rather than reasoned: with the plugin inactive, `/wp-json/wp/v2/custom-route.php` returns WordPress's own `rest_no_route` JSON, and a probe reports SCRIPT_NAME `/index.php` with no PATH_INFO -- a genuine REST request. With the plugin active it answered 401. The distinction is position, not presence. A `.php` segment BEFORE the endpoint means the server runs that script and the endpoint arrives as PATH_INFO; one AFTER it is part of the route. Only the prefix is now examined. Two intermediate approaches were measured and discarded: - Keying on SCRIPT_NAME == index.php alone fixes the false positive but reopens `/index.php/wp-json/wp/v2/` and `/wp-login.PHP/wp-json/wp/v2/`, both of which resolve to index.php while carrying the endpoint in PATH_INFO or in a path the rewrite never consumed. - Scanning the whole path is the bug this commit fixes. The prefix rule closes both of those as well, so nothing regressed to gain it. Verified on WordPress 7.1, PHP 8.4 and 8.1, single-site and subdirectory multisite: eleven bypass shapes across four classes all answer 401 (including `/index.php/wp-json/wp/v2/`, the case-variant spellings, and `/index.php/hello-world/wp-json/wp/v2/`); the PATH_INFO login POST issues no session; xmlrpc.php, wp-json/wp/v2, wp-json/wp/v3 and wp-json/jetpack stay reachable, including below a subsite prefix and including Codex's case there. The request matrix, the User Switching flow and multisite are unchanged. 45 assertions; the four guards mutation-check independently at 4, 7, 2 and 3 failures, the first of which reproduces exactly the fault Codex found. --- pressable-basic-authentication.php | 49 ++++++++++++++++++++++-------- readme.txt | 4 +-- tests/hook-registration-test.php | 18 +++++++++++ 3 files changed, 57 insertions(+), 14 deletions(-) diff --git a/pressable-basic-authentication.php b/pressable-basic-authentication.php index 0266e1a..8ee1d7a 100644 --- a/pressable-basic-authentication.php +++ b/pressable-basic-authentication.php @@ -217,24 +217,20 @@ private function should_skip_auth() { // /xmlrpc.php/../wp-login.php `..` resolved by the server afterwards // /wp-login.php/wp-json/wp/v2/ trailing segments land in PATH_INFO // - // The guard below is written against the general fault rather than those + // The guards below are written against the general fault rather than those // three shapes: the endpoints above are rewrite targets, so they only mean // anything when the server routes the request to index.php. Decoded first, // because the server decodes before it resolves. $request_path = rawurldecode('/' . ltrim((string) parse_url($request_uri, PHP_URL_PATH), '/')); $haystack = rtrim($request_path, '/') . '/'; - // A `.php` segment means the server is executing that script and handing - // the rest to it as PATH_INFO; a `.` or `..` segment means the path is - // resolved to something other than what it reads as. In neither case is the - // request routed to index.php, so an endpoint appearing in it is decoration, - // not a destination -- refuse rather than re-implement the server's own - // path resolution here. - $serves_another_script = false; + // A `.` or `..` segment means the path resolves to something other than what + // it reads as, so nothing in it can be trusted to name a destination. + $has_traversal = false; foreach (explode('/', $haystack) as $segment) { - if ('.' === $segment || '..' === $segment || '.php' === strtolower(substr($segment, -4))) { - $serves_another_script = true; + if ('.' === $segment || '..' === $segment) { + $has_traversal = true; break; } } @@ -243,9 +239,11 @@ private function should_skip_auth() { // matches whole path segments -- `/notwp-json/wp/v2` must not satisfy // `wp-json/wp/v2` -- but not anchored at the start of the path, because a // subdirectory or multisite subsite install serves these below a prefix. - if (!$serves_another_script) { + if (!$has_traversal) { foreach ($excluded_endpoints as $endpoint) { - if (strpos($haystack, '/' . trim($endpoint, '/') . '/') !== false) { + $position = strpos($haystack, '/' . trim($endpoint, '/') . '/'); + + if (false !== $position && !$this->path_runs_another_script(substr($haystack, 0, $position))) { return true; } } @@ -265,6 +263,33 @@ private function should_skip_auth() { return false; } + /** + * Whether a path prefix names a script the server would execute. + * + * Only what sits BEFORE an excluded endpoint is asked about. A `.php` segment + * there means the server runs that script and hands the endpoint to it as + * PATH_INFO, so the endpoint is decoration on a gated page: + * `/wp-login.php/wp-json/wp/v2/` served the login form and allowed a full + * WordPress sign-in with no Basic Auth at all. + * + * A `.php` segment AFTER the endpoint is part of the REST route itself -- + * `/wp-json/wp/v2/custom-route.php` is routed to index.php and dispatched to + * the REST API -- so an earlier version of this check, which scanned the whole + * path, wrongly demanded authentication for a valid REST request. + * + * @param string $prefix The portion of the request path preceding the endpoint. + * @return bool + */ + private function path_runs_another_script($prefix) { + foreach (explode('/', $prefix) as $segment) { + if ('.php' === strtolower(substr($segment, -4))) { + return true; + } + } + + return false; + } + /** * Sends authentication headers. */ diff --git a/readme.txt b/readme.txt index d1906ba..47273cb 100644 --- a/readme.txt +++ b/readme.txt @@ -52,8 +52,8 @@ No manual installation is necessary.​ (`/xmlrpc.php/../wp-login.php`), or by trailing one after it (`/wp-login.php/wp-json/wp/v2/`). All three served the login form and allowed a full WordPress sign-in. xmlrpc.php is now matched on the script the server - actually resolved, and the REST endpoints only on a decoded request path that - the server routes to index.php. + actually resolved, and a REST endpoint only when nothing preceding it in the + request path names a script the server would execute instead. * Fixed: logging out on a site also running User Switching caused a fatal error. * Fixed: a logout request no longer reaches wp_logout() without valid credentials. * Fixed: a logout URL without action=logout is no longer redirected away from the diff --git a/tests/hook-registration-test.php b/tests/hook-registration-test.php index a277fb8..aaefd69 100644 --- a/tests/hook-registration-test.php +++ b/tests/hook-registration-test.php @@ -225,10 +225,28 @@ function skips_auth_for( $uri, $script_name = '/index.php' ) { array( '/index.php/wp-json/wp/v2/', '/index.php' ), array( '/wp-login.PHP/wp-json/wp/v2/', '/wp-login.PHP' ), array( '/sub1/wp-login.php/wp-json/wp/v2/', '/wp-login.php' ), + array( '/index.php/wp-json/wp/v2/', '/index.php' ), + array( '/index.PHP/wp-json/wp/v2/', '/index.php' ), + array( '/index.php/hello-world/wp-json/wp/v2/', '/index.php' ), ) as $case ) { check( false === skips_auth_for( $case[0], $case[1] ), "a PATH_INFO endpoint after a script does not waive auth: {$case[0]}" ); } +// The endpoint must not be preceded by a script the server would execute -- but a +// `.php` segment AFTER it is part of the REST route and must still be excluded. +// Scanning the whole path for `.php` instead, as an earlier fix did, wrongly +// demanded authentication for a valid REST request (caught by the Codex pre-PR +// review, verified against a live install: WordPress dispatches +// /wp-json/wp/v2/custom-route.php to the REST API and returns rest_no_route). +foreach ( array( + '/wp-json/wp/v2/custom-route.php', + '/wp-json/wp/v2/media/thing.php', + '/wp-json/jetpack/v4/x.php', + '/wp-json/wp/v3/anything.php', +) as $uri ) { + check( true === skips_auth_for( $uri ), "a .php segment INSIDE a REST route still waives auth: $uri" ); +} + // A path carrying a traversal segment is not the path the server ends up serving, // so it must never waive authentication: `/xmlrpc.php/../wp-login.php` resolves to // wp-login.php while reading as the excluded xmlrpc endpoint, which served the login From cded14e41c9f7823a67b89f9a3fc70db08030e65 Mon Sep 17 00:00:00 2001 From: loukieluke <84424020+loukieluke@users.noreply.github.com> Date: Tue, 15 Sep 2026 11:49:53 -0500 Subject: [PATCH 08/11] ENG-7204: Keep the first path segment on a target beginning with two slashes Codex pre-PR review, P1, accepted. parse_url() reads a request target beginning `//` as a protocol-relative URL and discards the first segment as an authority, so the script named there vanished before the prefix check could object to it: parse_url('//wp-login.php/wp-json/wp/v2/', PHP_URL_PATH) === '/wp-json/wp/v2/' The server does not agree. It preserved the target, executed wp-login.php and passed `/wp-json/wp/v2/` as PATH_INFO, so authentication was waived on a page the plugin exists to gate. Measured before the fix: 200 with the login form rendered, and a POST carrying log/pwd returned 302 to /wp-admin/ with a session cookie -- a full sign-in with no credentials, a fifth spelling of the same bypass. The query and fragment are now cut by hand with strcspn(), which has no opinion about authorities, and the leading slashes are collapsed afterwards so the result is still comparable. `//wp-json/wp/v2` consequently resolves to the REST endpoint it actually names and stays excluded, where parse_url had mangled it to `/wp/v2` and gated it. Also documents a limitation raised as P2 in the same review and deliberately NOT fixed: a WordPress install inside a DIRECTORY named `*.php` has a prefix that reads like a script, so REST beneath it is challenged rather than excluded. Reproduced. Separating a directory from a script needs either the absence of PATH_INFO as evidence -- trusting a variable's absence, which converts this fail-closed edge case into a fail-open one on any SAPI that does not populate it -- or a filesystem lookup that a subdirectory install defeats anyway. Given five fail-open bypasses have been found in this method, challenging a REST request under a pathologically named directory is the cheaper of the two errors. Verified on WordPress 7.1, PHP 8.4 and 8.1, single-site and subdirectory multisite: twelve bypass shapes across five classes answer 401, the `//` login POST issues no session, and xmlrpc.php, wp-json/wp/v2, wp-json/wp/v3, wp-json/jetpack and a REST route containing a later `.php` segment all stay reachable, including below a subsite prefix. The request matrix, the User Switching flow and multisite are unchanged. 50 assertions; restoring parse_url() fails 4 of them. --- pressable-basic-authentication.php | 20 +++++++++++++++++++- readme.txt | 7 ++++--- tests/hook-registration-test.php | 17 +++++++++++++++++ 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/pressable-basic-authentication.php b/pressable-basic-authentication.php index 8ee1d7a..3d05428 100644 --- a/pressable-basic-authentication.php +++ b/pressable-basic-authentication.php @@ -221,7 +221,16 @@ private function should_skip_auth() { // three shapes: the endpoints above are rewrite targets, so they only mean // anything when the server routes the request to index.php. Decoded first, // because the server decodes before it resolves. - $request_path = rawurldecode('/' . ltrim((string) parse_url($request_uri, PHP_URL_PATH), '/')); + // The query and fragment are cut by hand rather than with parse_url(), which + // reads a target beginning `//` as a protocol-relative URL and discards the + // first segment as an authority. `//wp-login.php/wp-json/wp/v2/` parsed to + // `/wp-json/wp/v2/` -- the script vanished, leaving nothing before the + // endpoint to object to -- while the server preserved the target, executed + // wp-login.php and passed the rest as PATH_INFO. That served the login form + // and allowed a full WordPress sign-in with no Basic Auth at all. Collapsing + // the leading slashes afterwards is what makes the resulting path comparable. + $cut = strcspn($request_uri, '?#'); + $request_path = rawurldecode('/' . ltrim(substr($request_uri, 0, $cut), '/')); $haystack = rtrim($request_path, '/') . '/'; // A `.` or `..` segment means the path resolves to something other than what @@ -277,6 +286,15 @@ private function should_skip_auth() { * the REST API -- so an earlier version of this check, which scanned the whole * path, wrongly demanded authentication for a valid REST request. * + * Known limitation, accepted deliberately: a WordPress install inside a + * DIRECTORY named `*.php` has a prefix that reads like a script but is not one, + * so REST under it is challenged rather than excluded. Separating the two needs + * either the absence of PATH_INFO as evidence -- trusting a variable's absence, + * which turns this fail-closed edge case into a fail-open one wherever the SAPI + * does not populate it -- or a filesystem lookup that a subdirectory install + * defeats anyway. Refusing a REST request under a pathologically named directory + * is the cheaper error of the two. + * * @param string $prefix The portion of the request path preceding the endpoint. * @return bool */ diff --git a/readme.txt b/readme.txt index 47273cb..40aa9be 100644 --- a/readme.txt +++ b/readme.txt @@ -51,9 +51,10 @@ No manual installation is necessary.​ (`/?x=wp-json/wp/v2`), by reaching the page through one (`/xmlrpc.php/../wp-login.php`), or by trailing one after it (`/wp-login.php/wp-json/wp/v2/`). All three served the login form and allowed a - full WordPress sign-in. xmlrpc.php is now matched on the script the server - actually resolved, and a REST endpoint only when nothing preceding it in the - request path names a script the server would execute instead. + full WordPress sign-in, as did a request target beginning `//` + (`//wp-login.php/wp-json/wp/v2/`). xmlrpc.php is now matched on the script the + server actually resolved, and a REST endpoint only when nothing preceding it in + the request path names a script the server would execute instead. * Fixed: logging out on a site also running User Switching caused a fatal error. * Fixed: a logout request no longer reaches wp_logout() without valid credentials. * Fixed: a logout URL without action=logout is no longer redirected away from the diff --git a/tests/hook-registration-test.php b/tests/hook-registration-test.php index aaefd69..0d5abf5 100644 --- a/tests/hook-registration-test.php +++ b/tests/hook-registration-test.php @@ -247,6 +247,23 @@ function skips_auth_for( $uri, $script_name = '/index.php' ) { check( true === skips_auth_for( $uri ), "a .php segment INSIDE a REST route still waives auth: $uri" ); } +// A target beginning `//` must not lose its first segment. parse_url() reads such a +// target as protocol-relative and discards that segment as an authority, so +// `//wp-login.php/wp-json/wp/v2/` parsed to `/wp-json/wp/v2/` with nothing left +// before the endpoint -- while the server preserved it, ran wp-login.php and served +// the login form with no Basic Auth at all. Caught by the Codex pre-PR review. +foreach ( array( + array( '//wp-login.php/wp-json/wp/v2/', '/wp-login.php' ), + array( '///wp-login.php/wp-json/wp/v2/', '/wp-login.php' ), + array( '//index.php/wp-json/wp/v2/', '/index.php' ), + array( '//wp-login.php/wp-json%2Fwp%2Fv2', '/wp-login.php' ), +) as $case ) { + check( false === skips_auth_for( $case[0], $case[1] ), "a protocol-relative-looking target keeps its first segment: {$case[0]}" ); +} + +// Collapsing those leading slashes must not break the endpoint underneath them. +check( true === skips_auth_for( '//wp-json/wp/v2' ), 'an endpoint behind a doubled leading slash is still excluded' ); + // A path carrying a traversal segment is not the path the server ends up serving, // so it must never waive authentication: `/xmlrpc.php/../wp-login.php` resolves to // wp-login.php while reading as the excluded xmlrpc endpoint, which served the login From 6a9978fa0c2037c9e12a76aeb3124b2c3da92a37 Mon Sep 17 00:00:00 2001 From: loukieluke <84424020+loukieluke@users.noreply.github.com> Date: Tue, 15 Sep 2026 16:34:05 -0500 Subject: [PATCH 09/11] ENG-7204: Bump the release workflow checkout to v4, don't persist its token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit review on PR #9, two Major quick-wins, both in main.yml: - actions/checkout@v3 pins the removed Node 16 runtime, which GitHub no longer runs on hosted runners. Both jobs move to @v4 (test.yml was already there). - The build job's checkout persisted the GITHUB_TOKEN in git config, though it only runs `git archive` on a local ref and the release script carries its own explicit token. persist-credentials: false, matching the check job and test.yml — closes the zizmor "artipacked" finding. No behaviour change to the plugin. The third CodeRabbit finding (restrict REST exclusions to configured path prefixes) is addressed in a PR reply rather than code: it is infeasible at plugins_loaded (WordPress routing/multisite config is not bootstrapped that early) and the over-broad match grants no access beyond the intended public REST exclusion -- every bogus-prefix request returns a themed 404, never gated content, verified, and identically on 1.0.4. --- .github/workflows/main.yml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 3e7eb5b..ab05024 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -33,7 +33,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: # This job lints and executes repository PHP, so the GITHUB_TOKEN must # not be left in the local git config where that code could read it. @@ -66,7 +66,12 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 + with: + # This job runs `git archive` (a local ref) and the release script, + # which uses its own explicit GITHUB_TOKEN -- so the checkout token is + # not needed after checkout and must not be left in the git config. + persist-credentials: false - name: Install Ruby uses: ruby/setup-ruby@v1 From d29a41ee1d9204b51a2f48382e9c89de7327f9d7 Mon Sep 17 00:00:00 2001 From: loukieluke <84424020+loukieluke@users.noreply.github.com> Date: Tue, 15 Sep 2026 16:57:47 -0500 Subject: [PATCH 10/11] ENG-7204: Don't treat the X-Requested-With header as AJAX (auth bypass) Blocking finding from Mitch's review of PR #9, accepted. is_ajax_request() is the first arm of skip_request(), and it returned true on a caller-supplied request header: ( ! empty( $_SERVER['HTTP_X_REQUESTED_WITH'] ) && 'xmlhttprequest' === strtolower( $_SERVER['HTTP_X_REQUESTED_WITH'] ) ) init() returns before force_basic_authentication() when skip_request() is true, so a single header turned Basic Auth off on any URL: curl -H 'X-Requested-With: XMLHttpRequest' https:/// Verified: the front page and wp-login.php served unauthenticated, and a login POST carrying the header issued a session cookie -- the same full bypass as the path-spelling tricks already fixed here, reached with one header and no knowledge of the rewrite rules. Confirmed live on both production test sites. Pre-existing on main, but in scope for the same reasons this PR already fixes a sibling bypass in this exact disjunction: the PR rewrites skip_request(), adds a second consumer in handle_logout_request(), and the fix follows the rule the rewrite states for itself -- match what the server executes, not what the caller requested. HTTP_X_REQUESTED_WITH is nothing but what the caller requested. is_ajax_request() now matches only DOING_AJAX, which WordPress defines itself when admin-ajax.php runs and a caller cannot forge. The header arm covered nothing the constant does not: real AJAX runs through admin-ajax.php with DOING_AJAX set, and REST is handled by should_skip_auth(). A custom endpoint that needs access sends Basic Auth like anything else. Three new assertions (the suite had none touching is_ajax_request/DOING_AJAX/ X-Requested-With, as Mitch noted); restoring the header arm fails them. Full request matrix, logout flow and exclusions unchanged; admin-ajax.php stays excluded via DOING_AJAX. --- pressable-basic-authentication.php | 15 ++++++++-- tests/hook-registration-test.php | 44 ++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/pressable-basic-authentication.php b/pressable-basic-authentication.php index 3d05428..30c783e 100644 --- a/pressable-basic-authentication.php +++ b/pressable-basic-authentication.php @@ -457,11 +457,22 @@ private function prevent_caching() { /** * Check if the current request is an AJAX request * + * Matched only on the `DOING_AJAX` constant, which WordPress defines itself + * when `admin-ajax.php` runs -- evidence from the request's own execution that + * a caller cannot forge. The `X-Requested-With: XMLHttpRequest` request header + * was deliberately removed: it is set by the caller, so keying an auth waiver + * on it let any anonymous request turn Basic Auth off on any URL, + * `wp-login.php` included, by sending one header -- the same full bypass the + * `should_skip_auth()` rewrite closes for path spellings, reached with a + * header instead. It covered nothing the constant does not: real WordPress + * AJAX runs through admin-ajax.php with `DOING_AJAX` set, and the REST API is + * handled separately by `should_skip_auth()`. A custom endpoint that needs + * access sends Basic Auth like anything else. + * * @return bool */ private function is_ajax_request() { - return ( defined( 'DOING_AJAX' ) && DOING_AJAX ) || - ( ! empty( $_SERVER['HTTP_X_REQUESTED_WITH'] ) && 'xmlhttprequest' === strtolower( $_SERVER['HTTP_X_REQUESTED_WITH'] ) ); + return defined( 'DOING_AJAX' ) && DOING_AJAX; } /** diff --git a/tests/hook-registration-test.php b/tests/hook-registration-test.php index 0d5abf5..12ab148 100644 --- a/tests/hook-registration-test.php +++ b/tests/hook-registration-test.php @@ -308,6 +308,50 @@ function skips_auth_for( $uri, $script_name = '/index.php' ) { check( false === skips_auth_for( '/' ), 'an ordinary request is still gated' ); +// is_ajax_request() is tested directly rather than through skip_request(): this file runs +// under the CLI SAPI, so is_cli_request() (a sibling arm of skip_request()) is always true +// here and would mask everything else. +function is_ajax_for( $x_requested_with ) { + static $plugin = null; + + if ( null === $plugin ) { + $plugin = new Pressable_Basic_Auth(); + } + + $original = isset( $_SERVER['HTTP_X_REQUESTED_WITH'] ) ? $_SERVER['HTTP_X_REQUESTED_WITH'] : null; + if ( null === $x_requested_with ) { + unset( $_SERVER['HTTP_X_REQUESTED_WITH'] ); + } else { + $_SERVER['HTTP_X_REQUESTED_WITH'] = $x_requested_with; + } + + try { + $method = new ReflectionMethod( 'Pressable_Basic_Auth', 'is_ajax_request' ); + $method->setAccessible( true ); + + return (bool) $method->invoke( $plugin ); + } finally { + if ( null === $original ) { + unset( $_SERVER['HTTP_X_REQUESTED_WITH'] ); + } else { + $_SERVER['HTTP_X_REQUESTED_WITH'] = $original; + } + } +} + +// The `X-Requested-With: XMLHttpRequest` request header must NOT count as AJAX. It is +// caller-controlled, so keying an auth waiver on it let any anonymous request turn Basic +// Auth off on any URL, wp-login.php included, by sending one header (caught in review by +// Mitch). is_ajax_request() is the first arm of skip_request(), so a true here is a waiver. +check( false === is_ajax_for( 'XMLHttpRequest' ), 'the X-Requested-With header does not count as AJAX' ); +check( false === is_ajax_for( 'xmlhttprequest' ), 'the X-Requested-With header (lowercased) does not count as AJAX' ); +check( false === is_ajax_for( null ), 'a plain request with no such header is not AJAX' ); + +// Real WordPress AJAX still bypasses: admin-ajax.php defines DOING_AJAX itself, which a +// caller cannot forge. Asserted last, because define() is process-global and irreversible. +define( 'DOING_AJAX', true ); +check( true === is_ajax_for( null ), 'a genuine DOING_AJAX request is still AJAX (bypasses)' ); + echo "\n"; if ( $failures ) { From 97fd44032cc906c92525b0a2a89d57286902c4b9 Mon Sep 17 00:00:00 2001 From: loukieluke <84424020+loukieluke@users.noreply.github.com> Date: Tue, 15 Sep 2026 16:58:18 -0500 Subject: [PATCH 11/11] ENG-7204: Note the X-Requested-With bypass fix in the changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to d29a41e — the readme changelog entry now covers the header vector alongside the path-spelling bypasses it sits with. --- readme.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/readme.txt b/readme.txt index 40aa9be..144252a 100644 --- a/readme.txt +++ b/readme.txt @@ -55,6 +55,10 @@ No manual installation is necessary.​ (`//wp-login.php/wp-json/wp/v2/`). xmlrpc.php is now matched on the script the server actually resolved, and a REST endpoint only when nothing preceding it in the request path names a script the server would execute instead. +* Fixed: sending the `X-Requested-With: XMLHttpRequest` request header waived + Basic Authentication on any URL, a full sign-in included. That header is + caller-supplied, so it no longer counts as AJAX -- only WordPress's own + `DOING_AJAX` constant (set by admin-ajax.php) does. * Fixed: logging out on a site also running User Switching caused a fatal error. * Fixed: a logout request no longer reaches wp_logout() without valid credentials. * Fixed: a logout URL without action=logout is no longer redirected away from the