Cookie store picks up cookies from interrupted requests, causing auth/nonce issues

[adamziel]

Summary

When multiple client.goTo() calls happen in quick succession (e.g., redirect to homepage immediately followed by redirect to editor), both requests arrive at the server with no cookies. Each request independently tries to log the user in, setting different session cookies. The cookie store picks up cookies from both requests, including the interrupted one, which corrupts the session state and breaks nonces.

Steps to reproduce

1. Call client.goTo('/wp-admin/')
2. Immediately call client.goTo('/wp-admin/post-new.php') before the first navigation completes
3. Observe that the session cookies get mixed up

Root cause

The data flow in this scenario:

1. First request (homepage redirect) is sent with no cookies
2. Second request (editor redirect) interrupts the first, also sent with no cookies
3. Both requests independently hit the auto-login endpoint
4. Both requests set different session cookies (sec cookie changes between them)
5. The HttpCookieStore in php-request-handler.ts stores cookies from both responses, including the interrupted request
6. This corrupts the session - nonces generated by one request don't match the session established by the other

With regular browser cookies, you'd simply get logged out (clearly indicating something is wrong). But because we use a custom cookie store that accumulates cookies from all responses, we end up with a corrupted but "logged in" state.

Relevant code

• packages/php-wasm/universal/src/lib/http-cookie-store.ts - stores cookies from all responses
• packages/php-wasm/universal/src/lib/php-request-handler.ts:665-668 - calls rememberCookiesFromResponseHeaders after every request

Possible solutions

1. Don't store cookies from interrupted/cancelled requests - Track which requests are still "active" and only persist cookies from completed navigations

2. Await navigation completion after every goTo() call - Callers should wait for the iframe to fully load before issuing another goTo(). This is a usage pattern fix rather than a Playground fix.

3. Implement request sequencing - Queue goTo() calls so they execute one at a time, preventing overlapping requests

4. Add request correlation to cookie storage - Associate cookies with specific request IDs and only keep cookies from the "winning" request in case of races

[brandonpayton]

@adamziel, is there a way we can adjust our autologin code to avoid this problem entirely?

Here's what I'm thinking:

• Our autologin code calls wp_set_auth_cookie() here.
• wp_set_auth_cookie() takes a $token arg for the user session token here.
• If the $token arg is not provided, wp_set_auth_cookie() creates a new token here using the WP_Session_Tokens create() method.
• If we take responsibility for generating and saving the session token to the DB, can we make sure concurrent auto-login attempts respond with the same cookie?

It could work if we do the same as the WP_Session_Tokens create() method but do a conditional update where we only save the session to user meta if the current value is NULL.

If we find that we fail to save the new sessions token, it means another request has updated it first, and if that is the case, we can just query the winning token from the DB and use that to generate and send the login cookie.

At that point, shouldn't either login cookie be considered valid by WordPress because it is based on the user's current session token?

[brandonpayton]

The idea is that concurrent autologin attempts would all return cookies based on the same user session token. I think it would make autologin into a sort of atomic operation, even if there are multiple attempts.

[brandonpayton]

The actual DB update is initiated by WP_User_Meta_Session_Tokens here.

We might extend that session manager and place it using the session_token_manager filter which is called in the WP_Session_Tokens::get_instance() method here.

[adamziel]

Hm, would FileLockManager help us here? It's sort of a tool for atomic operations (if you squint right) after all. Only one PHP instance would hold the lock and then indicate the autologin was already handled.

[brandonpayton]

I see what you mean... kinda like WP locks a maintenance file. I suppose flock() could be used on an auto-login file, and PHP requests that find a locked auto-login file could enter a sleep loop until the auto-login file is unlocked, look up the user session one it is unlocked, and generate a cookie based on that.

I think it could work without getting so involved in WP internals, and we could do it all from the Playground auto-login PHP. Nice, @adamziel :)

[brandonpayton]

@adamziel I don't think that proper file locking is an option here yet because we never solved making it totally async for the asyncify build. Making fd_close async kept leading to crashes no matter what we added to the asyncify lists.

But I think we could use the Web Locks API if PHP could call out to JS somehow. The problem isn't generally with async. It was just with making fd_close operations async.

[adamziel]

Oh yes, you're right @brandonpayton. Good catch!

[adamziel]

@brandonpayton I'm starting to question the atomic token solution. While it's very tempting, it's not what would happen on a hosted site, would it? I'm very nervous about any changes where Playground's behavior diverges from native WordPress behavior. This problem isn't specific to Playground. This issue could also happen on a regular, hosted site. A client could trigger the auto-login twice, get two different cookies, and invalidate the previous nonce. The only nuance here is that the first request gets cancelled and Playground still stores a cookie sent in response to that cancelled request.

At that point, shouldn't either login cookie be considered valid by WordPress because it is based on the user's current session token?

I think they're both considered valid. it. It's just the nonce changes.

[adamziel]

In theory, we should be able to detect a cancelled request:

self.addEventListener("fetch", (event) => {
  event.request.signal.addEventListener("abort", () => {
    // best-effort: may or may not fire depending on browser + scenario
    console.log("client aborted");
  });
});

In practice, browsers support for that event seems flaky at best:

• https://bugs.chromium.org/p/chromium/issues/detail?id=823697
• https://bugzilla.mozilla.org/show_bug.cgi?id=1394102

This is an interesting workaround, though:

As far as I can tell the only workaround is to check the clientId on the request and poll self.clients.get() to check if the client is still present until it either goes away or the request is completed.

[brandonpayton]

I'm very nervous about any changes where Playground's behavior diverges from native WordPress behavior. This problem isn't specific to Playground. This issue could also happen on a regular, hosted site. A client could trigger the auto-login twice, get two different cookies, and invalidate the previous nonce. The only nuance here is that the first request gets cancelled and Playground still stores a cookie sent in response to that cancelled request.

@adamziel I'm not yet sure what I think about this... Is the auto-login flow like a regular login flow? Offhand, I think it is more magical than that.

With regular login, the user is explicitly performing a login action. They login before doing other things.

But with auto-login, Playground just sends cookies with the first request it receives. And if there is a race between those requests, shouldn't both requests just be automatically logged in? It's even possible that two concurrent requests could occur without being cancelled (e.g., opening two tabs to the Playground CLI site simultaneously). In those cases, shouldn't both requests be logged in? I'm not sure it matters whether one of the requests is cancelled or not. Playground is just saying "everybody gets logged in".

In that case, I think we just want both requests to succeed and be lead to logged-in responses.

Does this affect your perspective at all?

[brandonpayton]

Update: I edited an above sentence to say what I intended.
From: It's even possible that two concurrent requests could occur.
To: It's even possible that two concurrent requests could occur without being cancelled.