Astro: Redirects cause adapter to fail

[blaine-arcjet]

It seems that redirects defined in astro.config.mjs cause our adapter to fail. This is due to the context.isPrerendered flag being set to false yet context.clientAddress throws when accessed. This generally makes sense because a redirect doesn't produce a static page itself.

Astro doesn't seem to have anything like isRedirect on the context but you can discover redirects during the build process. I experimented with using codegen to make an isDynamic function that consumed the discovered redirects but I'm not sure I'm happy with users needed to import the utility explictly.

Instead, we could use this generated logic to warn and produce an Allow decision on static pages. Alternatively, we could produce an Error decision like we do when IP isn't defined. I'm not sure which is better.

[davidmytton]

How would this work?

warn and produce an Allow decision on static pages

Client hits the redirect URL, we detect it and Allow the request and the client gets redirected?

If so, that seems like the right choice because there's not really anything for us to process on a redirect. Arcjet would likely run again on the next request anyway.

[blaine-arcjet]

Client hits the redirect URL, we detect it and Allow the request and the client gets redirected?

With the Vercel adapter, the redirects are just added to the vercel.json file so there is nothing happening on the server. However, you make a good point for something like the Node adapter. I'll have to look into that a bit.

[davidmytton]

Makes sense. Then warning probably doesn't add much since there's nothing for us to protect. A redirect should take priority over executing any code because the real relevant context is wherever the redirect destination is.

[blaine-arcjet]

Then warning probably doesn't add much since there's nothing for us to protect

I guess I'm worried that someone will add arcjet in their middleware, generate a website in static mode, and think their routes are protected via bot protection. Adding the warning would at least alert them during generation. Alternatively, I could design the reporting functionality during build 🤔

This could also be a problem in an Astro component if it is marked as prerendered since protect() would be executed during build but not at runtime.

[blaine-arcjet]

The more I think about things like the above, the more I think we need to provide a utility function like isDynamicRoute() because it allows our protect() function to keep throwing an error if someone uses it wrong and specifically calls out that Arcjet is only running on dynamic routes.

[blaine-arcjet]

This doesn't actually seem to be a problem with @astrojs/node. Even though Arcjet is executed on both the original route and the redirected route (not ideal), there is never a thrown error when trying to access the ctx.clientAddress because it doesn't try to resolve the route to a static file.

If we don't want to call this a bug with the Vercel adapter, then it is just a problem specific to that adapter and makes me lean even further to adding an opt-in workaround just for that platform.

[blaine-arcjet]

I set up an example with the Vercel adapter and it works completely normally. This leads me to believe the bug is within Starlight 😬

[blaine-arcjet]

I can't actually figure out where this would happen in Starlight and I traced the actual failure into Astro; however, I don't know what combination of Vercel Adapter, Starlight, Arcjet, or other integrations cause this.

Instead, I'm working on a feature request into Astro that we can recommend users leverage.

[blaine-arcjet]

Astro PR: withastro/astro#13143