fix(frontend): allow the deployment's own origin in the CSRF allowlist - #701
Conversation
Self-hosted deployments on custom domains got 401 "Not authenticated" on every cookie-authenticated mutation (creating API tokens, group management) because the CSRF origin allowlist defaulted to the hosted domains and the CSRF_ALLOWED_ORIGINS escape hatch was undocumented. The origin derived from NEXT_PUBLIC_URL β which self-hosters already set for OAuth redirects β is now always included in the allowlist, even when CSRF_ALLOWED_ORIGINS overrides the defaults. Both variables are now documented in .env.example. Fixes #695 Constraint: CSRF check must never widen to attacker-controllable input (request Host/X-Forwarded-* headers) Rejected: same-origin comparison against the request Host header | Host can be wrong behind misconfigured proxies and is not operator-declared intent Rejected: docs-only fix | NEXT_PUBLIC_URL already declares the deployment origin, requiring a second var for the same value is a footgun Confidence: high Scope-risk: narrow Directive: NEXT_PUBLIC_URL origin is intentionally allowed even when CSRF_ALLOWED_ORIGINS is set β do not make the env var fully replace it
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
π‘ Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a916889167
βΉοΈ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with π.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const origin = new URL(publicUrl).origin; | ||
| if (!origins.includes(origin)) { | ||
| origins.push(origin); |
There was a problem hiding this comment.
Reject non-HTTP NEXT_PUBLIC_URL origins
When NEXT_PUBLIC_URL is a URL that parses but is not an HTTP(S) deployment URL, such as mailto:admin@example.com or file:///tmp/app, new URL(...).origin returns the literal string "null"; this adds "null" to the CSRF allowlist, so cookie-authenticated mutating requests with Origin: null are accepted instead of falling back to the explicit/default origins. Please only append origins for http:/https: URLs and ignore opaque origins.
Useful? React with πΒ / π.
There was a problem hiding this comment.
Valid catch β fixed in 8bfa36e. The origin is now only appended when url.protocol is http: or https:, so opaque origins (mailto:, file:, etc. β "null") never enter the allowlist. Added a regression test that NEXT_PUBLIC_URL=mailto:admin@example.com + Origin: null is rejected.
There was a problem hiding this comment.
1 issue found across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The domain is not ours, so allowlisting it lets whoever registers it mount CSRF attacks against cookie sessions. The existing allow-test now asserts rejection instead. Confidence: high Scope-risk: narrow
new URL("mailto:...").origin is the literal string "null", and
browsers send Origin: null from sandboxed iframes, so a non-HTTP
NEXT_PUBLIC_URL would have allowlisted a CSRF vector. Non-http(s)
schemes are now ignored like malformed URLs.
Constraint: the opaque origin "null" must never enter the allowlist
Confidence: high
Scope-risk: narrow
junhoyeo#701) * fix(frontend): allow the deployment's own origin in the CSRF allowlist Self-hosted deployments on custom domains got 401 "Not authenticated" on every cookie-authenticated mutation (creating API tokens, group management) because the CSRF origin allowlist defaulted to the hosted domains and the CSRF_ALLOWED_ORIGINS escape hatch was undocumented. The origin derived from NEXT_PUBLIC_URL β which self-hosters already set for OAuth redirects β is now always included in the allowlist, even when CSRF_ALLOWED_ORIGINS overrides the defaults. Both variables are now documented in .env.example. Fixes junhoyeo#695 Constraint: CSRF check must never widen to attacker-controllable input (request Host/X-Forwarded-* headers) Rejected: same-origin comparison against the request Host header | Host can be wrong behind misconfigured proxies and is not operator-declared intent Rejected: docs-only fix | NEXT_PUBLIC_URL already declares the deployment origin, requiring a second var for the same value is a footgun Confidence: high Scope-risk: narrow Directive: NEXT_PUBLIC_URL origin is intentionally allowed even when CSRF_ALLOWED_ORIGINS is set β do not make the env var fully replace it * fix(frontend): drop nonexistent tokscale.dev from CSRF defaults The domain is not ours, so allowlisting it lets whoever registers it mount CSRF attacks against cookie sessions. The existing allow-test now asserts rejection instead. Confidence: high Scope-risk: narrow * fix(frontend): only derive CSRF origin from http(s) NEXT_PUBLIC_URL new URL("mailto:...").origin is the literal string "null", and browsers send Origin: null from sandboxed iframes, so a non-HTTP NEXT_PUBLIC_URL would have allowlisted a CSRF vector. Non-http(s) schemes are now ignored like malformed URLs. Constraint: the opaque origin "null" must never enter the allowlist Confidence: high Scope-risk: narrow
junhoyeo#701) * fix(frontend): allow the deployment's own origin in the CSRF allowlist Self-hosted deployments on custom domains got 401 "Not authenticated" on every cookie-authenticated mutation (creating API tokens, group management) because the CSRF origin allowlist defaulted to the hosted domains and the CSRF_ALLOWED_ORIGINS escape hatch was undocumented. The origin derived from NEXT_PUBLIC_URL β which self-hosters already set for OAuth redirects β is now always included in the allowlist, even when CSRF_ALLOWED_ORIGINS overrides the defaults. Both variables are now documented in .env.example. Fixes junhoyeo#695 Constraint: CSRF check must never widen to attacker-controllable input (request Host/X-Forwarded-* headers) Rejected: same-origin comparison against the request Host header | Host can be wrong behind misconfigured proxies and is not operator-declared intent Rejected: docs-only fix | NEXT_PUBLIC_URL already declares the deployment origin, requiring a second var for the same value is a footgun Confidence: high Scope-risk: narrow Directive: NEXT_PUBLIC_URL origin is intentionally allowed even when CSRF_ALLOWED_ORIGINS is set β do not make the env var fully replace it * fix(frontend): drop nonexistent tokscale.dev from CSRF defaults The domain is not ours, so allowlisting it lets whoever registers it mount CSRF attacks against cookie sessions. The existing allow-test now asserts rejection instead. Confidence: high Scope-risk: narrow * fix(frontend): only derive CSRF origin from http(s) NEXT_PUBLIC_URL new URL("mailto:...").origin is the literal string "null", and browsers send Origin: null from sandboxed iframes, so a non-HTTP NEXT_PUBLIC_URL would have allowlisted a CSRF vector. Non-http(s) schemes are now ignored like malformed URLs. Constraint: the opaque origin "null" must never enter the allowlist Confidence: high Scope-risk: narrow
Fixes #695
Problem
Self-hosted deployments on custom domains get 401 "Not authenticated" on every cookie-authenticated mutating request β creating API tokens, managing groups, renaming devices β even with a valid session (thanks @liyuerich for the report and for tracking it down to
CSRF_ALLOWED_ORIGINS).The CSRF origin allowlist in
getSessionFromRequest()defaults to the hosted domains, so any other origin is rejected before the session cookie is even looked at. TheCSRF_ALLOWED_ORIGINSescape hatch existed but was documented nowhere.Fix
NEXT_PUBLIC_URLβ which self-hosters already must set for OAuth redirects β is now always included in the allowlist. Setting up a custom domain no longer requires any extra configuration.CSRF_ALLOWED_ORIGINSoverrides the defaults (the deployment's own origin is always a legitimate request source).NEXT_PUBLIC_URLvalues are ignored gracefully.NEXT_PUBLIC_URLandCSRF_ALLOWED_ORIGINSare now documented in.env.example.https://tokscale.devis removed from the default allowlist β the domain doesn't exist and isn't ours, so allowlisting it would let whoever registers it mount CSRF attacks against cookie sessions. The default list is nowhttps://tokscale.ai+http://localhost:3000.Deliberately not done: comparing Origin against the request
Host/X-Forwarded-Hostheaders β those are not operator-declared intent and can be wrong behind misconfigured proxies.NEXT_PUBLIC_URLis an explicit operator setting, so trusting its origin doesn't widen the CSRF surface.Verification
requestSessionCsrf.test.ts: custom-domain origin allowed, origin derived correctly from URL with path/trailing slash, still allowed alongsideCSRF_ALLOWED_ORIGINS, unknown origins still rejected, malformedNEXT_PUBLIC_URLignored. The former tokscale.dev allow-test now asserts rejection.ViewSelector.tsxpredates this change).