diff --git a/.changeset/full-pillows-greet.md b/.changeset/full-pillows-greet.md new file mode 100644 index 000000000000..a52ebf509b93 --- /dev/null +++ b/.changeset/full-pillows-greet.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes `checkOrigin` CSRF protection in `astro dev` behind a TLS-terminating reverse proxy. The dev server now reads `X-Forwarded-Proto` (gated on `security.allowedDomains`, matching production behaviour) so the constructed request origin matches the `https://` origin the browser sends. Also ensures `security.allowedDomains` and `security.checkOrigin` are respected in dev. diff --git a/packages/astro/src/core/app/node.ts b/packages/astro/src/core/app/node.ts index 6ef16e2f27b1..6dcd8c071f85 100644 --- a/packages/astro/src/core/app/node.ts +++ b/packages/astro/src/core/app/node.ts @@ -9,7 +9,11 @@ import { createOutgoingHttpHeaders } from './createOutgoingHttpHeaders.js'; import type { RenderOptions } from './base.js'; import { App } from './app.js'; import type { NodeAppHeadersJson, SerializedSSRManifest, SSRManifest } from './types.js'; -import { validateForwardedHeaders, validateHost } from './validate-headers.js'; +import { + getFirstForwardedValue, + validateForwardedHeaders, + validateHost, +} from './validate-headers.js'; /** * Allow the request body to be explicitly overridden. For example, this @@ -52,14 +56,6 @@ export function createRequest( const isEncrypted = 'encrypted' in req.socket && req.socket.encrypted; - // Parses multiple header and returns first value if available. - const getFirstForwardedValue = (multiValueHeader?: string | string[]) => { - return multiValueHeader - ?.toString() - ?.split(',') - .map((e) => e.trim())?.[0]; - }; - const providedProtocol = isEncrypted ? 'https' : 'http'; const untrustedHostname = req.headers.host ?? req.headers[':authority']; diff --git a/packages/astro/src/core/app/validate-headers.ts b/packages/astro/src/core/app/validate-headers.ts index c73bcb2d3132..a65552e04fd2 100644 --- a/packages/astro/src/core/app/validate-headers.ts +++ b/packages/astro/src/core/app/validate-headers.ts @@ -1,5 +1,19 @@ import { matchPattern, type RemotePattern } from '@astrojs/internal-helpers/remote'; +/** + * Parses a potentially comma-separated multi-value header (as produced by + * proxy chains) and returns the first value, trimmed of whitespace. + * Returns `undefined` when the header is absent or empty. + */ +export function getFirstForwardedValue( + multiValueHeader: string | string[] | undefined, +): string | undefined { + return multiValueHeader + ?.toString() + .split(',') + .map((e) => e.trim())[0]; +} + /** * Sanitize a hostname by rejecting any with path separators. * Prevents path injection attacks. Invalid hostnames return undefined. diff --git a/packages/astro/src/vite-plugin-app/app.ts b/packages/astro/src/vite-plugin-app/app.ts index 87fac3b54501..cbc7f82d9669 100644 --- a/packages/astro/src/vite-plugin-app/app.ts +++ b/packages/astro/src/vite-plugin-app/app.ts @@ -1,6 +1,7 @@ import type http from 'node:http'; import { removeTrailingForwardSlash } from '@astrojs/internal-helpers/path'; import { BaseApp, type RenderErrorOptions } from '../core/app/entrypoints/index.js'; +import { getFirstForwardedValue, validateForwardedHeaders } from '../core/app/validate-headers.js'; import { shouldAppendForwardSlash } from '../core/build/util.js'; import { clientLocalsSymbol } from '../core/constants.js'; import { @@ -128,10 +129,27 @@ export class AstroServerApp extends BaseApp { incomingResponse, isHttps, }: HandleRequest): Promise { - const origin = `${isHttps ? 'https' : 'http'}://${ - incomingRequest.headers[':authority'] ?? incomingRequest.headers.host - }`; + // When the dev server runs behind a TLS-terminating reverse proxy (e.g. + // Caddy, nginx, Traefik), the proxy connects to Vite over plain HTTP while + // the browser communicates over HTTPS. In that setup isHttps is false, but + // the proxy forwards the original scheme via X-Forwarded-Proto: https. + // We trust that header only when security.allowedDomains is configured — + // the same guard used in production (core/app/node.ts). Without it the + // header is untrusted and we fall back to isHttps. + const validated = validateForwardedHeaders( + getFirstForwardedValue(incomingRequest.headers['x-forwarded-proto']), + getFirstForwardedValue(incomingRequest.headers['x-forwarded-host']), + getFirstForwardedValue(incomingRequest.headers['x-forwarded-port']), + this.manifest.allowedDomains, + ); + + const protocol = validated.protocol ?? (isHttps ? 'https' : 'http'); + const host = + validated.host ?? + (incomingRequest.headers[':authority'] as string | undefined) ?? + incomingRequest.headers.host; + const origin = `${protocol}://${host}`; const url = new URL(origin + incomingRequest.url); let pathname: string; if (this.manifest.trailingSlash === 'never' && !incomingRequest.url) { diff --git a/packages/astro/src/vite-plugin-astro-server/plugin.ts b/packages/astro/src/vite-plugin-astro-server/plugin.ts index 39b2c7c1f96e..ccacdb0e9b29 100644 --- a/packages/astro/src/vite-plugin-astro-server/plugin.ts +++ b/packages/astro/src/vite-plugin-astro-server/plugin.ts @@ -268,8 +268,8 @@ export async function createDevelopmentManifest(settings: AstroSettings): Promis componentMetadata: new Map(), inlinedScripts: new Map(), i18n: i18nManifest, - checkOrigin: - (settings.config.security?.checkOrigin && settings.buildOutput === 'server') ?? false, + checkOrigin: settings.config.security?.checkOrigin ?? false, + allowedDomains: settings.config.security?.allowedDomains, actionBodySizeLimit: settings.config.security?.actionBodySizeLimit ? settings.config.security.actionBodySizeLimit : 1024 * 1024, // 1mb default diff --git a/packages/astro/test/units/app/dev-url-construction.test.js b/packages/astro/test/units/app/dev-url-construction.test.js new file mode 100644 index 000000000000..c273991369c8 --- /dev/null +++ b/packages/astro/test/units/app/dev-url-construction.test.js @@ -0,0 +1,205 @@ +import * as assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import { + getFirstForwardedValue, + validateForwardedHeaders, +} from '../../../dist/core/app/validate-headers.js'; + +/** + * Mirrors the URL construction logic in AstroServerApp.handleRequest so that + * the protocol and host derivation can be exercised in isolation. + * + * @param {object} opts + * @param {Record} opts.headers - Incoming request headers + * @param {boolean} [opts.isHttps=false] - Whether Vite itself is running TLS + * @param {import('../../../dist/core/app/types.js').SSRManifest['allowedDomains']} [opts.allowedDomains] + * @param {string} [opts.requestUrl='/'] + * @returns {URL} + */ +function buildDevUrl({ headers, isHttps = false, allowedDomains, requestUrl = '/' }) { + const validated = validateForwardedHeaders( + getFirstForwardedValue(headers['x-forwarded-proto']), + getFirstForwardedValue(headers['x-forwarded-host']), + getFirstForwardedValue(headers['x-forwarded-port']), + allowedDomains, + ); + + const protocol = validated.protocol ?? (isHttps ? 'https' : 'http'); + const host = validated.host ?? headers[':authority'] ?? headers['host']; + + return new URL(`${protocol}://${host}${requestUrl}`); +} + +describe('Dev server URL construction — X-Forwarded-Proto handling', () => { + it('uses http when isHttps=false and no allowedDomains configured (default)', () => { + const url = buildDevUrl({ + headers: { host: 'localhost:4321' }, + isHttps: false, + }); + assert.equal(url.protocol, 'http:'); + assert.equal(url.origin, 'http://localhost:4321'); + }); + + it('ignores X-Forwarded-Proto when allowedDomains is not configured', () => { + // Without allowedDomains the header must not be trusted — this is the + // security guard that prevents an attacker from forcing the scheme used + // in CSRF origin comparisons. + const url = buildDevUrl({ + headers: { host: 'localhost:4321', 'x-forwarded-proto': 'https' }, + isHttps: false, + }); + assert.equal(url.protocol, 'http:'); + assert.equal(url.origin, 'http://localhost:4321'); + }); + + it('ignores X-Forwarded-Proto when allowedDomains is an empty array', () => { + const url = buildDevUrl({ + headers: { host: 'mre.local', 'x-forwarded-proto': 'https' }, + isHttps: false, + allowedDomains: [], + }); + assert.equal(url.protocol, 'http:'); + }); + + it('uses https from X-Forwarded-Proto when allowedDomains matches hostname', () => { + // Behind a TLS-terminating proxy (Caddy, nginx, Traefik) the browser + // sends Origin: https://host while the proxy connects to Vite over HTTP. + // With allowedDomains configured, the dev server derives the same + // https:// origin, so the CSRF Origin === url.origin comparison passes. + const url = buildDevUrl({ + headers: { host: 'mre.local', 'x-forwarded-proto': 'https' }, + isHttps: false, + allowedDomains: [{ hostname: 'mre.local' }], + }); + assert.equal(url.protocol, 'https:'); + assert.equal(url.origin, 'https://mre.local'); + }); + + it('uses https from X-Forwarded-Proto with wildcard hostname pattern', () => { + const url = buildDevUrl({ + headers: { host: 'app.example.com', 'x-forwarded-proto': 'https' }, + isHttps: false, + allowedDomains: [{ protocol: 'https', hostname: '**.example.com' }], + }); + assert.equal(url.protocol, 'https:'); + assert.equal(url.origin, 'https://app.example.com'); + }); + + it('trusts X-Forwarded-Proto even when host does not match allowedDomains pattern', () => { + // validateForwardedHeaders validates protocol and host independently. + // When allowedDomains is non-empty but has no `protocol` property, + // any http/https value is accepted for the protocol. The host match is + // only required for the X-Forwarded-Host to be trusted; the fallback + // host header is used instead. This mirrors production (node.ts) behaviour. + const url = buildDevUrl({ + headers: { host: 'localhost:4321', 'x-forwarded-proto': 'https' }, + isHttps: false, + allowedDomains: [{ hostname: 'mre.local' }], + }); + // Protocol is trusted (allowedDomains is non-empty); host falls back to + // the Host header value. + assert.equal(url.protocol, 'https:'); + assert.equal(url.origin, 'https://localhost:4321'); + }); + + it('rejects X-Forwarded-Proto that does not match explicit protocol in allowedDomains', () => { + // When allowedDomains specifies a protocol, only that protocol is allowed. + const url = buildDevUrl({ + headers: { host: 'mre.local', 'x-forwarded-proto': 'http' }, + isHttps: false, + allowedDomains: [{ protocol: 'https', hostname: 'mre.local' }], + }); + // 'http' is rejected because the pattern requires 'https' + assert.equal(url.protocol, 'http:'); + }); + + it('falls back to isHttps=true when X-Forwarded-Proto is absent but Vite uses TLS', () => { + // When the user configures Vite's own TLS (vite.server.https) without a + // proxy, isHttps=true should still work. + const url = buildDevUrl({ + headers: { host: 'localhost:4321' }, + isHttps: true, + }); + assert.equal(url.protocol, 'https:'); + }); + + it('uses first value from comma-separated X-Forwarded-Proto', () => { + const url = buildDevUrl({ + headers: { host: 'mre.local', 'x-forwarded-proto': 'https,http' }, + isHttps: false, + allowedDomains: [{ hostname: 'mre.local' }], + }); + assert.equal(url.protocol, 'https:'); + }); + + it('uses first value from comma-separated X-Forwarded-Proto with spaces', () => { + const url = buildDevUrl({ + headers: { host: 'mre.local', 'x-forwarded-proto': ' https , http' }, + isHttps: false, + allowedDomains: [{ hostname: 'mre.local' }], + }); + assert.equal(url.protocol, 'https:'); + }); + + it('rejects malicious X-Forwarded-Proto with URL injection', () => { + const url = buildDevUrl({ + headers: { + host: 'mre.local', + 'x-forwarded-proto': 'https://evil.com/?x=', + }, + isHttps: false, + allowedDomains: [{ hostname: 'mre.local' }], + }); + // validateForwardedHeaders rejects invalid protocol values + assert.equal(url.protocol, 'http:'); + }); + + it('rejects javascript: scheme injection in X-Forwarded-Proto', () => { + const url = buildDevUrl({ + headers: { + host: 'mre.local', + 'x-forwarded-proto': 'javascript:alert(1)//', + }, + isHttps: false, + allowedDomains: [{ hostname: 'mre.local' }], + }); + assert.equal(url.protocol, 'http:'); + }); + + it('rejects empty X-Forwarded-Proto and falls back to isHttps', () => { + const url = buildDevUrl({ + headers: { host: 'mre.local', 'x-forwarded-proto': '' }, + isHttps: false, + allowedDomains: [{ hostname: 'mre.local' }], + }); + assert.equal(url.protocol, 'http:'); + }); + + it('produces an origin that matches the browser Origin header when proxy is configured', () => { + // The CSRF check compares request.headers.origin === url.origin. + // When the dev server runs behind a TLS-terminating proxy and + // allowedDomains is configured, both sides of that comparison must + // resolve to the same https:// origin. + const url = buildDevUrl({ + headers: { host: 'mre.local', 'x-forwarded-proto': 'https' }, + isHttps: false, + allowedDomains: [{ hostname: 'mre.local' }], + }); + const browserOriginHeader = 'https://mre.local'; + assert.equal(url.origin, browserOriginHeader); + }); + + it('produces a mismatched origin behind a proxy when allowedDomains is not configured', () => { + // Without allowedDomains, X-Forwarded-Proto is untrusted and the URL + // gets an http:// origin while the browser sends Origin: https://. + // The CSRF check (Origin === url.origin) therefore returns false and + // blocks the request with 403. + const url = buildDevUrl({ + headers: { host: 'mre.local', 'x-forwarded-proto': 'https' }, + isHttps: false, + // no allowedDomains + }); + const browserOriginHeader = 'https://mre.local'; + assert.notEqual(url.origin, browserOriginHeader); // http:// vs https:// + }); +});