Stale _astroActionPayload cookie breaks all SSR pages

[mlafeldt]

Astro Info

Astro                    v4.15.9
Node                     v22.8.0
System                   macOS (arm64)
Package Manager          bun
Output                   hybrid
Adapter                  none
Integrations             none

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

As the title says, returning NOT_FOUND from actions breaks SSR when you have a static 404 page.

This happens because the _astroActionPayload cookie never gets deleted in that case (I guess because there's no server). As a result, all SSR routes will return the 404 page until you manually clear that cookie.

See https://github.com/mlafeldt/extraterrestrial-equator for a small repo.

What's the expected result?

Reloading SSR routes should work even with a static 404 page.

Link to Minimal Reproducible Example

https://github.com/mlafeldt/extraterrestrial-equator

Participation

• [X] I am willing to submit a pull request for this issue.

[mlafeldt]

Another interesting fact: With the Cloudflare adapter, server-rendering 404.astro via export const prerender = false will not fix the problem. However, doing the same with the default Node adapter will fix it (see repro). 🤔

[mlafeldt]

This can happen with other status codes too.

One of my apps is stuck/broken with a 500 error because the _astroActionPayload cookie contains this after one action returned a 500 (and Astro won't delete the cookie):

{"actionName":"user.deleteUser","actionResult":{"type":"error","status":500,"contentType":"application/json","body":"{\"type\":\"AstroActionError\",\"code\":\"INTERNAL_SERVER_ERROR\",\"status\":500,\"message\":\"User not found\"}"}}

Using a single cookie for all things SSR without scoping seems problematic.

[mlafeldt]

In fact, you can break all SSR routes of an Astro site by doing this:

document.cookie = "_astroActionPayload=boom"

This will even affect routes that don't use Astro Actions at all, which shows a possible unintended consequence of the current design.

[michaelpayne02]

I encountered this when creating a repro for #11675 (and #11973). This error also occurs if the action call fails schema validation regardless if the handler returns an error code or not.

https://stackblitz.com/edit/github-g63nyt-zhjdjc?file=README.md

In 14.5.9 this is the stack trace

  Stack trace:
    at JSON.parse (<anonymous>)
    at eval (/home/projects/github-g63nyt-zhjdjc/node_modules/astro/dist/actions/runtime/middleware.js:27:103)
    at eval (/home/projects/github-g63nyt-zhjdjc/node_modules/astro/dist/core/middleware/sequence.js:21:12)
    at eval (file:///home/projects/github-g63nyt-zhjdjc/node_modules/astro/dist/core/middleware/sequence.js:58:18)
    at applyHandle (file:///home/projects/github-g63nyt-zhjdjc/node_modules/astro/dist/core/middleware/sequence.js:33:22)

After #12016 the result is base64-encoded for size savings and parsing still throws

01:07:00 [ERROR] Invalid character
  Stack trace:
    at decodeBase64_internal (file:///home/projects/github-g63nyt-zhjdjc/node_modules/@oslojs/encoding/dist/base64.js:90:23)
    at eval (/home/projects/github-g63nyt-zhjdjc/node_modules/astro/dist/actions/runtime/middleware.js:33:75)
    at eval (/home/projects/github-g63nyt-zhjdjc/node_modules/astro/dist/core/middleware/sequence.js:21:12)
    at eval (file:///home/projects/github-g63nyt-zhjdjc/node_modules/astro/dist/core/middleware/sequence.js:58:18)
    at applyHandle (file:///home/projects/github-g63nyt-zhjdjc/node_modules/astro/dist/core/middleware/sequence.js:33:22)

So two things:

1. Somehow the cookie is not actually being removed on the server when set to deleted in the middleware chain and is leaking to the client.
2. Because the parsing errors are server-side, we should be able to redirect to the correct page using the Referrer header with the payload cleared

[mlafeldt]

In general, I wonder how using a single cookie for all actions is supposed to work when, for example, two or more actions happen simultaneously.

[bholmesdev]

Fair point @mlafeldt! However, it's worth noting cookies are only used for server form requests. It should only be possible to handle and render a single form request at a time using the standard form request model. If you submit multiple actions at once via client-side JS, everything works as expected.

As for the issue at hand: we're considering a model that would make cookie forwarding an opt-in at the action level. This work may negate the bug entirely, but we'll be sure to capture in an integration test.

[jamesknelson]

A possibly related issue: middleware redirects can cause an action's cookie to not be deleted, and in turn to prevent future actions from being processed.

The particular steps required in my case:

• I have a logout action, which is available in every page via a button in the layout
• I also have a middleware that redirects logged out users to a login screen:

      const isAuthenticationRequired =
        !context.url.pathname.startsWith('/lobby');
      if (isAuthenticationRequired) {
        if (!context.locals.identity) {
          return context.redirect(`/lobby/login`);
        }
      }

The redirect middleware and the login screen worked fine until I introduced the logout button. From what I can tell, the reason is that the redirect middleware runs in the same request as the logout action is processed, redirecting to the login screen with the logout button's action cookie still in place, which somehow prevents login actions from being submitted at all.

Here's the stale cookie I get after decoding, which stays in place after reloads/form submissions. The only way to get the app to start responding again is to manually deleted it.

{"actionName":"logout","actionResult":{"type":"data","status":200,"contentType":"application/json+devalue","body":"[true]"}}

FWIW, I got around this by handling logout as an endpoint function instead of an action.

I've noticed that you don't have a redirect function available on ActionAPIContext, and wonder if you don't want people redirecting to prevent precisely this behavior?

[bholmesdev]

Alright, we spent some time reevaluating how form actions should be handled. We landed on a strategy that we'll be shipping in the next 5.0 Beta:

• Cookie redirects will no longer be built-in. Due to the 4 KB size limit and nuanced behavior for hybrid apps, we don't think it's a best practice we should endorse. This means that action results will render as a standard form POST without redirects. Confirmed the issue reported here will go away using this new default.
• If you do want to implement cookie redirects, or build your own behavior, you will have access to action results from middleware. We will add a new getActionContext() utility to check for inbound action requests and handle results however you wish. Store in your own session storage, store as a cookie, add your own gated security layer, etc. Currently in PR.

There's a few features I'd like to see Astro address in the future to make our defaults simpler:

• Built-in session storage to plug in your favorite storage provider. Now in stage 2 discussion
• Some primitive to check whether a given route is pre-rendered or not. In the case of this bug, we should only redirect with a cookie when the destination route is server-rendered. Otherwise, the cookie cannot be deleted.

This will be shipped in Beta, so we're open to feedback! Thanks to @mlafeldt @jamesknelson and @michaelpayne02 for sharing examples with us.