fix: add siteUrl to resolve origin mismatch behind reverse proxies#382
fix: add siteUrl to resolve origin mismatch behind reverse proxies#382UpperM wants to merge 4 commits intoemdash-cms:mainfrom
Conversation
Add `siteUrl` to EmDashConfig, replacing `passkeyPublicOrigin`. Create `getPublicOrigin(url, config)` pure helper in api/public-url.ts that resolves the public origin from config, EMDASH_SITE_URL / SITE_URL env vars (at runtime via process.env), or falls back to url.origin. Both config and env var paths validate http/https protocol only. Extend checkPublicCsrf() with dual-origin matching so the Origin header can match either the internal or public origin behind a reverse proxy. Disable Astro's checkOrigin (EmDash's CSRF handles origin validation with dual-origin and runtime siteUrl support that Astro's build-time check cannot provide). When siteUrl is known at build time, also set allowedDomains so Astro.url reflects the public origin in templates. Discussion: emdash-cms#315
Replace url.origin with getPublicOrigin() across 25 files: - Auth middleware: CSRF checks, login redirects, WWW-Authenticate - Setup wizard + dev-bypass: store public origin as emdash:site_url - Passkey routes (8 files): use siteUrl for rpId and origin - OAuth: provider redirects, callback, authorize, device code - Well-known endpoints: RFC 8414 and RFC 9728 metadata - Snapshot export, theme preview HMAC, sitemap, robots.txt - Page context + JSON-LD: pass siteUrl through for structured data OAuth Secure cookie flag now checks siteUrl protocol when set, preserving the existing fallback for non-proxy deployments.
Update public docs, skills reference, and demo config to document siteUrl replacing passkeyPublicOrigin. Add EMDASH_SITE_URL / SITE_URL to env vars table. Changeset: minor bump with breaking-change note.
🦋 Changeset detectedLatest commit: 3cb6035 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
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 |
|
All contributors have signed the CLA ✍️ ✅ |
Scope checkThis PR changes 552 lines across 37 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
|
I have read the CLA Document and I hereby sign the CLA |
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
What does this PR do?
Adds a
siteUrlconfig option that fixes a class of reverse-proxy origin mismatches affecting Node.js self-hosted deployments. When EmDash runs behind a TLS-terminating proxy (nginx, Traefik, Caddy -- typical for Coolify, Dokploy, or any Docker setup), the server seeshttp://localhost:4321while browsers reach it viahttps://mysite.example.com. PR #225 fixed this for passkeys; this PR extends the same fix to all affected call sites.Affected subsystems (before this fix):
Securecookie flag (omitted when TLS is terminated by the proxy)What this adds:
A single
getPublicOrigin(url, config)helper wraps every affectedurl.origincall. WhensiteUrlis set, it returns that; otherwise it falls back tourl.originunchanged, so non-proxied deployments are not affected. The public origin comes from operator config or environment variables, not from request headers.Also supports
EMDASH_SITE_URL/SITE_URLenv vars for container deployments where the domain is only known at runtime (usingprocess.envrather thanimport.meta.envso it works after the build).Breaking:
passkeyPublicOriginis removed. Since this is pre-release, renaming felt cleaner than maintaining both. Migration is just renaming the key inastro.config.mjs.Decision point:
checkOrigin: falseThis PR disables Astro's built-in
security.checkOriginand relies on EmDash's own CSRF layer (checkPublicCsrf+X-EmDash-Requestheader). I want to be transparent about why and open this to discussion.The problem: Astro's
checkOriginandallowedDomainsare baked into the build manifest. Behind a reverse proxy,url.originis the internal address and Astro's origin check blocks form-like POSTs and requests without content-type with "Cross-site POST form submissions are forbidden" -- before EmDash middleware runs. (Note:application/jsonrequests are not affected by Astro's check, but some EmDash flows use form-like content types.) The only Astro-native solutions are:allowedDomainsscoped to a hostname -- works, but requires the domain at build time, which breaks Docker "build once, deploy anywhere" images whereSITE_URLis set at runtimeallowedDomains: [{}]wildcard -- enables host header poisoning on GET responses (login redirects, OAuth, WWW-Authenticate) whenSITE_URLisn't configuredThe approach: Disable
checkOriginand let EmDash handle CSRF entirely. EmDash's CSRF layer handles the proxy case that Astro's check cannot: dual-origin support and runtime siteUrl resolution via env vars. WhensiteUrlis known at build time,allowedDomainsis still set soAstro.urlreflects the public origin for user template code.The gap: User-authored form-handling routes on the same Astro site would lose Astro's CSRF protection for form submissions. (Astro's
checkOriginonly covers form-like content types and requests without content-type -- it does not protectapplication/jsonAPI routes regardless.) In practice, EmDash sites typically don't add custom form POST endpoints (all state changes go through EmDash's API), but this is worth flagging.Alternatives I considered but rejected:
unshiftbefore user middleware in the pipeline)If you'd prefer a different approach -- keeping
checkOriginenabled and documenting that Docker users must setsiteUrlat build time, or another path I haven't considered -- I'm happy to rework this.Related: #210, #225, #253, #393
Type of change
Checklist
pnpm typecheckpassespnpm --silent lint:json | jq '.diagnostics | length'returns 0pnpm testpasses (or targeted tests for my change)pnpm formathas been runAI-generated code disclosure
Screenshots / test output
Test output