Add origin checks for UI route submissions

[brophdawg11]

No description provided.

[changeset-bot]

🦋 Changeset detected

Latest commit: 7b01c8b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages

Name	Type
@react-router/dev	Major
react-router	Major
@react-router/fs-routes	Major
@react-router/remix-routes-option-adapter	Major
@react-router/architect	Major
@react-router/cloudflare	Major
react-router-dom	Major
@react-router/express	Major
@react-router/node	Major
@react-router/serve	Major
create-react-router	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

🤖 Hello there,

We just published version 7.12.0-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[github-actions]

🤖 Hello there,

We just published version 7.12.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[krissrex]

This documentation could be better.

The host header includes protocol (https://), but the comparison happens without.
The docs could state that origins should be provided without protocol.

Also, globs wont match if the origin is "null": *, ** and n* doesn't match.

I'm not sure how to allow if the origin header is completely absent.

[brophdawg11]

We can get the docs updated to be more specific there - how about "The allowed origins (excluding protocol) for actions / mutations on UI routes (i.e., those with a UI component). Micromatch glob patterns are supported."?

We can also look into the glob matching for null - I would think that should be matched by *.

I'm not sure how to allow if the origin header is completely absent.

If the header is absent, this check won't even happen?

[rossipedia]

This caught me off-guard today too. Omitting the protocol from the origin feels like it might be a Bad Idea™, and I'm wondering if RR should be fairly strict about this considering it's meant to mitigate a security issue.

[krissrex]

This config is baked during npm run build, so I can't have dynamic values and change it from process.env.ALLOWED_ORIGINS or similar. I wanted two different values based on staging and production, and also null which comes up in local development. I can't, with this approach, and have to use a workaround with

export default {
  allowedActionOrigins:
    process.env.NODE_ENV === 'development'
      ? ['null']
      : ['mylogin.staging.com', 'mylogin.prod.com'],
} satisfies Config;

It also broke the login of our site (using EntraID with redirect), so it's not a minor change.

[simply-arjen]

This broke our production as well, we deploy our app to 30 domains so we'd need to include all of them here.

[brophdawg11]

Do your applications have external origins posting to UI routes? action handlers on UI routes are intended to be posted to from <Form>/useFetcher in the UI components within that route (or it's children). Resource routes are the intended way to open up API endpoints for REST submissions.

I think extending the check to support runtime specification is a probably good idea so we will look into a patch update to permit that.

[brophdawg11]

we deploy our app to 30 domains so we'd need to include all of them here

You could always use a glob? allowedActionOrigins: ["*"] should make it behave the same as before where any origin can submit, but that is not recommended for security reasons.

[simply-arjen]

You could always use a glob? allowedActionOrigins: ["*"] should make it behave the same as before where any origin can submit, but that is not recommended for security reasons.

Yes I'll just add the complete list for now. If it will be runtime configurable in the future 👍

Our app is behind a k8s/ingress that causes the origin to be different than the x-forwarded-for. We're looking to fix that, but it caught us by surprise since it used to work. I should have read the change logs 😅 but I got a bit trigger happy updating to the latest with the announcement of the CVE fixes on bluesky.

[wethegreenpeople]

We had the same issue in our production environment for the same reason - our app lives behind k8s that cause the origin and the x-forwarded-for to differ.

I don't have much extra to add, I just wanted to leave a comment saying that we got this error in our logs:
Error: x-forwarded-host header does not match origin header from a forwarded action request. Aborting the action

Just in case anyone else googles this issue.

FWIW though I would agree that this was not a minor change

[carbonrobot]

This is currently breaking applications deployed on Vercel and Netlify, even when using the wildcard workaround allowedActionOrigins: ["*"]. The origin and host don't always match because you can assign vanity URLs to domains.

Here is an example of the combinations of headers that can be expected on those platforms (from log data, obfuscated).

Host:             core.unicorns.dev
Origin:           https://www.unicorns.io
X-Forwarded-Host: core.unicorns.dev

[brophdawg11]

Sorry folks - I didn't look close enough at the glob implementation and missed 2 key details:

• * only matches 1 segment, not multiple - you need ** for multiple`
• There is a hard limitation that doesn't permit ** to be a global wildcard so allowedActionOrigins: ["**"] still throws

For now, you can use allowedActionOrigins: ["**.*"] to bypass that limitation if you want to permit any domain - but we would recommend limiting it to known trusted domains.

I'll talk to @jacob-ebey and see if we should just remove that limitation so allowedActionOrigins: ["**"] works as you would expect - but we will make sure to get better docs on this.

[brophdawg11]

@krissrex If you need to set these dynamically at runtime, you can do so on the ServerBuild via a custom server - here's an example using express:

import "react-router";
import { createRequestHandler } from "@react-router/express";
import express from "express";
import type { ServerBuild } from "react-router";

export const app = express();

async function getBuild() {
  let build: ServerBuild = await import("virtual:react-router/server-build");
  return {
    ...build,
    allowedActionOrigins: process.env.NODE_ENV === 'development'
      ? undefined
      : ['*.staging.com', '*.prod.com'],
  };
}

app.use(createRequestHandler({ build: getBuild }));

[brophdawg11]

Hey folks - just a head sup that I opened #14722 to improve the docs and make allowedActionOrigins:["**"] work as expected. Thanks for the feedback here!

[krissrex]

Hi, a coworker said he experienced exceptions from cases where Origin was not a valid URL.

I'm not sure what cases this happen in, but it happened.

TypeError with code: ERR_INVALID_URL.