Skip to content

Site Health: Send only WordPress cookies with the REST API test - #12952

Open
itzmekhokan wants to merge 1 commit into
WordPress:trunkfrom
itzmekhokan:fix/65839-site-health-rest-cookies
Open

Site Health: Send only WordPress cookies with the REST API test#12952
itzmekhokan wants to merge 1 commit into
WordPress:trunkfrom
itzmekhokan:fix/65839-site-health-rest-cookies

Conversation

@itzmekhokan

Copy link
Copy Markdown

Restricts the cookies sent with the Site Health REST API availability test to the cookie names WordPress itself defines, instead of forwarding the visitor's entire browser cookie jar to the loopback request.

What the problem was:

  • WP_Site_Health::get_test_rest_availability() passed wp_unslash( $_COOKIE ) straight to wp_remote_get(), so every cookie in the visitor's browser — analytics, CDN, cache, commerce, and others — was forwarded to the site's own REST endpoint.

  • None of those cookies play any part in authenticating the request, which is done with the WordPress cookies plus the X-WP-Nonce header.

  • On the reporting site this produced a false (403) Forbidden with rest_cookie_invalid_nonce, while the same request limited to the WordPress cookies returned 200.

What the fix does:

  • Filters $cookies through array_intersect_key() so only AUTH_COOKIE, SECURE_AUTH_COOKIE, LOGGED_IN_COOKIE, and TEST_COOKIE are forwarded.

  • Adds a test asserting that unrelated cookies are dropped and the WordPress cookies are kept.

Approach and why:

  • The allowlist uses core's cookie constants rather than a wordpress_ name prefix. A prefix would keep wp-settings-*, which is not an authentication cookie, and would break on sites that redefine the cookie constants in wp-config.php. The test covers the wp-settings-* case specifically.

  • This narrows what is sent, so it is worth stating plainly: a site whose edge or host requires some other cookie to reach the endpoint (for example Cloudflare Access) can restore it through the existing http_request_args filter, which runs after this code. No new filter is introduced.

  • Only the method named in the ticket is changed. WP_Site_Health::can_perform_loopback(), WP_Automatic_Updater, and wp-admin/includes/file.php use the same wp_unslash( $_COOKIE ) pattern for different loopback targets and need their own analysis; they are left for a separate ticket.

Testing instructions:

  1. Log in as an administrator and add several unrelated cookies for the site in the browser (for example _ga, wp-settings-time-1, and a large dummy cookie of a few kilobytes).

  2. Visit Tools > Site Health. On trunk, inspect the outgoing loopback request and confirm the whole cookie jar is sent; on a site sensitive to this the REST API test reports (403) Forbidden with rest_cookie_invalid_nonce.

  3. Apply the patch, reload Site Health, and confirm the REST API test reports "The REST API is available" and that only the WordPress cookies are sent.

  4. Run phpunit --filter test_get_test_rest_availability_sends_only_wordpress_cookies tests/phpunit/tests/admin/wpSiteHealth.php. The test fails on trunk and passes with the patch.

Trac ticket: https://core.trac.wordpress.org/ticket/65839

Use of AI Tools

AI assistance: Yes

Tool(s): Claude Code

Model(s): Claude Opus 5

Used for: Ticket analysis, code implementation, and tests. All changes were reviewed and validated by me.


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

The REST API availability test forwarded the visitor's entire browser
cookie jar to the loopback request. Only WordPress' own cookies have any
bearing on that request, and unrelated cookies have been reported to fail
it with a 403 rest_cookie_invalid_nonce on sites where the same request
succeeds when limited to the WordPress cookies.

Restrict the forwarded cookies to the cookie names WordPress defines.
Sites whose edge or host requires another cookie to reach the endpoint
can restore it through the existing http_request_args filter.

Fixes #65839.
@github-actions

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props khokansardar.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions

Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant