From 44f5c550e17ef76dbc4eff588a57b0ee75ed04a8 Mon Sep 17 00:00:00 2001 From: Arnas Donauskas Date: Mon, 1 Jun 2026 14:26:01 +0300 Subject: [PATCH] harden(invite): optional ALLOWED_INVITE_HOSTS allow-list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Defense-in-depth for the request-derived branch of `getBaseUrl()` in /api/account/invitations. The threat model - On a typical proxied deploy (Vercel / Hostinger / Cloudflare / nginx with a fixed server_name), the proxy overwrites the `Host` and `X-Forwarded-Host` headers with the deployment's canonical hostname, so the request-derived path produces the right URL. NEXT_PUBLIC_SITE_URL pinning is also unnecessary in that case. - On a *bare* deployment exposed to the public internet (no proxy in front, no SITE_URL set), an attacker can POST directly to /api/account/invitations with `Host: phishing.example` and receive an invite URL pointing at their site. They'd still need admin role on *some* account to call the endpoint, and they'd need to set up `phishing.example/join/` to actually phish anyone — but the resolution chain shouldn't trust un-validated client input by default. The fix - New env var `ALLOWED_INVITE_HOSTS` (comma-separated hostnames). - When set, `getBaseUrl` validates each candidate host against the list and falls through to wacrm.tech if neither matches. - When unset, behavior is unchanged — the request-derived host is trusted (preserves the "just works" experience for the typical proxied case). - Distinct console.warn for the rejection path vs the no-Host-at-all path so operators can tell their allow-list is actually catching something. Documented in .env.local.example with a clear explanation of when operators should set it (bare deploys + multi-tenant hosting) and when they probably don't need to (the typical case where NEXT_PUBLIC_SITE_URL already pins the host). Severity: MEDIUM in the final review, downgraded in practice because (a) the attack window is narrow, (b) requires admin role + a phishing site, and (c) most deployments are behind a proxy that strips the spoofable headers. Still worth the small amount of code to give security-conscious operators the knob. Verification - `npm run lint` → 0 errors (same 19 pre-existing warnings) - `npm run typecheck` → clean - `npm test` → 366/366 pass Co-Authored-By: Claude Opus 4.7 (1M context) --- .env.local.example | 25 ++++++++++ src/app/api/account/invitations/route.ts | 58 ++++++++++++++++++++++-- 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/.env.local.example b/.env.local.example index 5e59fe1..39162d0 100644 --- a/.env.local.example +++ b/.env.local.example @@ -40,6 +40,31 @@ NEXT_PUBLIC_SITE_URL=https://crm.example.com # OPTIONAL — only needed if you use the feature. # ============================================================ +# Defense-in-depth allow-list for the hostnames that +# /api/account/invitations is willing to publish in invite URLs. +# Comma-separated, no scheme, no port (just hostnames). +# +# Why this exists: when NEXT_PUBLIC_SITE_URL is unset, invite URLs +# are derived from the incoming request's `Host` / `X-Forwarded- +# Host` header. On a typical proxied deploy the proxy sets these +# to your canonical hostname and they're trustworthy. On a *bare* +# deployment exposed to the public internet, an attacker could +# POST to the API directly with a spoofed `Host: phishing.example` +# and receive an invite URL pointing at their site. +# +# When this var is set, hostnames not on the list are rejected +# (the request falls through to the wacrm.tech fallback with a +# console.warn). When unset, behavior is unchanged from earlier +# versions — the request-derived host is trusted. +# +# Most operators don't need this: setting NEXT_PUBLIC_SITE_URL to +# your canonical URL already pins invite links there. This is for +# operators who want belt-and-braces or run multi-tenant setups +# where one app instance serves several hostnames. +# +# Example: +# ALLOWED_INVITE_HOSTS=crm.example.com,crm-staging.example.com + # Shared secret protecting GET /api/automations/cron. Required if you # use Wait steps in automations (a scheduled pinger drains pending # executions). Generate any long random string: diff --git a/src/app/api/account/invitations/route.ts b/src/app/api/account/invitations/route.ts index 5ce3de5..4173299 100644 --- a/src/app/api/account/invitations/route.ts +++ b/src/app/api/account/invitations/route.ts @@ -46,15 +46,51 @@ import { isAccountRole } from "@/lib/auth/roles"; // impossible from a real browser. Logs a warning so the // operator can spot the misconfig. // +// Defense-in-depth: `ALLOWED_INVITE_HOSTS` +// +// The request-header path (#2 and #3 above) trusts whatever +// hostname the client (or proxy) puts in the header. On a +// typical proxied deploy (Vercel / Hostinger / Cloudflare) the +// proxy overwrites these so they're trustworthy. On a bare +// deployment exposed to the public internet, an attacker could +// POST directly with a crafted `Host: phishing.example` and +// receive an invite URL pointing at their site. +// +// When `ALLOWED_INVITE_HOSTS` is set (comma-separated hostnames), +// we validate the derived host against the list. Anything not +// on the list falls through to the wacrm.tech fallback with a +// loud console.warn. Operators who care about this attack +// surface should set this to their canonical hostnames; everyone +// else gets today's permissive behavior. +// // Previous implementation hard-defaulted to `https://wacrm.tech` // (the docs/marketing site, a different repo). Forks that didn't // set `NEXT_PUBLIC_SITE_URL` got invite links pointing at the // marketing site, which 404s on `/join/`. This resolution // chain removes the foot-gun. +function parseAllowedHosts(): readonly string[] | null { + const raw = process.env.ALLOWED_INVITE_HOSTS?.trim(); + if (!raw) return null; + const list = raw + .split(",") + .map((h) => h.trim().toLowerCase()) + .filter(Boolean); + return list.length > 0 ? list : null; +} + +function isHostAllowed( + hostname: string, + allowList: readonly string[] | null, +): boolean { + if (!allowList) return true; // No allow-list → permissive (legacy behavior). + return allowList.includes(hostname.toLowerCase()); +} + function getBaseUrl(request: Request): string { const explicit = process.env.NEXT_PUBLIC_SITE_URL?.trim(); if (explicit) return explicit.replace(/\/+$/, ""); + const allowList = parseAllowedHosts(); const forwardedHost = request.headers .get("x-forwarded-host") ?.split(",")[0] @@ -63,21 +99,33 @@ function getBaseUrl(request: Request): string { .get("x-forwarded-proto") ?.split(",")[0] ?.trim(); - if (forwardedHost) { + if (forwardedHost && isHostAllowed(forwardedHost, allowList)) { return `${forwardedProto || "https"}://${forwardedHost}`; } const host = request.headers.get("host")?.trim(); - if (host) { + if (host && isHostAllowed(host, allowList)) { // The protocol on `request.url` is whatever the framework saw — // reliable for bare deployments where no proxy is rewriting it. const reqProto = new URL(request.url).protocol.replace(":", ""); return `${reqProto}://${host}`; } - console.warn( - "[POST /api/account/invitations] could not derive base URL from request; falling back to marketing domain", - ); + // We fall through here when EITHER no Host header was present at + // all (essentially impossible from a real browser) OR an + // ALLOWED_INVITE_HOSTS list was set and neither candidate matched + // it. The warning is the operator's signal that someone is + // probing the API with a spoofed Host header. + if (allowList && (forwardedHost || host)) { + console.warn( + "[POST /api/account/invitations] rejected non-allow-listed host:", + { forwardedHost, host, allowList }, + ); + } else { + console.warn( + "[POST /api/account/invitations] could not derive base URL from request; falling back to marketing domain", + ); + } return "https://wacrm.tech"; }