diff --git a/.changeset/many-flies-shine.md b/.changeset/many-flies-shine.md new file mode 100644 index 000000000000..ea7aa10291cd --- /dev/null +++ b/.changeset/many-flies-shine.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Enable checkOrigin when astro is behind a reverse proxy diff --git a/packages/astro/src/core/app/node.ts b/packages/astro/src/core/app/node.ts index 716ec5205f76..314b7ff09cf1 100644 --- a/packages/astro/src/core/app/node.ts +++ b/packages/astro/src/core/app/node.ts @@ -19,6 +19,17 @@ interface NodeRequest extends IncomingMessage { body?: unknown; } +function findRequestPort(req: NodeRequest, proto: string): string | undefined { + const hostheader = req.headers['host']; + if (hostheader) { + const hosturl = new URL(`${proto}://${hostheader}`) + if (hosturl.port) { + return hosturl.port + } + } + return undefined +} + /** * Converts a NodeJS IncomingMessage into a web standard Request. * ```js @@ -76,7 +87,7 @@ export function createRequest( allowedDomains, ); const hostname = validated.host ?? validatedHostname ?? 'localhost'; - const port = validated.port; + const port = validated.port ?? req.socket?.localPort?.toString() ?? findRequestPort(req, protocol); let url: URL; try { diff --git a/packages/astro/src/core/app/validate-headers.ts b/packages/astro/src/core/app/validate-headers.ts index fe2138cc7a61..da2ef7042f4a 100644 --- a/packages/astro/src/core/app/validate-headers.ts +++ b/packages/astro/src/core/app/validate-headers.ts @@ -1,4 +1,4 @@ -import { matchPattern, type RemotePattern } from '@astrojs/internal-helpers/remote'; +import { matchPattern, matchHostname, type RemotePattern } from '@astrojs/internal-helpers/remote'; /** * Sanitize a hostname by rejecting any with path separators. @@ -85,57 +85,82 @@ export function validateForwardedHeaders( ): { protocol?: string; host?: string; port?: string } { const result: { protocol?: string; host?: string; port?: string } = {}; - // Validate protocol - if (forwardedProtocol) { - if (allowedDomains && allowedDomains.length > 0) { - const hasProtocolPatterns = allowedDomains.some((pattern) => pattern.protocol !== undefined); - if (hasProtocolPatterns) { - // Validate against allowedDomains patterns - try { - const testUrl = new URL(`${forwardedProtocol}://example.com`); - const isAllowed = allowedDomains.some((pattern) => matchPattern(testUrl, pattern)); - if (isAllowed) { - result.protocol = forwardedProtocol; - } - } catch { - // Invalid protocol, omit from result - } - } else if (/^https?$/.test(forwardedProtocol)) { - // allowedDomains exist but no protocol patterns, allow http/https - result.protocol = forwardedProtocol; - } - } else if (/^https?$/.test(forwardedProtocol)) { - // No allowedDomains, only allow http/https - result.protocol = forwardedProtocol; - } + // Require allowed domains to validate any forwarded headers - prevents trusting unvalidated headers + if (!allowedDomains || allowedDomains.length === 0) { + return result } - // Validate port first - if (forwardedPort && allowedDomains && allowedDomains.length > 0) { - const hasPortPatterns = allowedDomains.some((pattern) => pattern.port !== undefined); - if (hasPortPatterns) { - // Validate against allowedDomains patterns - const isAllowed = allowedDomains.some((pattern) => pattern.port === forwardedPort); - if (isAllowed) { - result.port = forwardedPort; - } - } - // If no port patterns, reject the header (strict security default) + // require a hostname + if (!forwardedHost || forwardedHost.length === 0) { + return result; } // Validate host (extract port from hostname for validation) // Reject empty strings and sanitize to prevent path injection - if (forwardedHost && forwardedHost.length > 0 && allowedDomains && allowedDomains.length > 0) { - const protoForValidation = result.protocol || 'https'; - const sanitized = sanitizeHost(forwardedHost); - if (sanitized) { - const { hostname, port: portFromHost } = parseHost(sanitized); - const portForValidation = result.port || portFromHost; - if (matchesAllowedDomains(hostname, protoForValidation, portForValidation, allowedDomains)) { - result.host = sanitized; + const sanitized = sanitizeHost(forwardedHost); + if (!sanitized) { + return result; + } + const { hostname, port } = parseHost(sanitized); + if (!hostname) { + return result; + } + + const testUrl = new URL(`https://${hostname}/`); + // do not match anything other than hostname here. + const found = allowedDomains.find((pattern) => matchHostname(testUrl, pattern.hostname, true)); + if (!found) { + return result; + } + + // we might need to set the port... + // if allowedDomain specified a port, but none was found, reject all the headers. + if (found.port) { + if (port && found.port !== port) { + return result; + } + if (forwardedPort && found.port !== forwardedPort) { + return result; + } + } + + if (forwardedPort || port) { + if (found.port) { + if (port) { + if (forwardedPort && forwardedPort !== port) { + // weird case. + return result; + } + if (found.port === port) { + result.port = port; + } else { + return result + } + } else if (found.port === forwardedPort) { + result.port = forwardedPort; + } else { + + // found port but no match + return result } + } else { + // If no port patterns, reject the header (strict security default) } } - return result; + // if a protocol is configured, ensure it is correct. + if (found.protocol) { + if (found.protocol !== forwardedProtocol) { + return result; + } + + // all good + result.protocol = found.protocol; + } else if (forwardedProtocol && /^https?$/.test(forwardedProtocol)) { + // fallback to allow if it is not specified in the allowedDomains, but only if it is http or https + result.protocol = forwardedProtocol; + } + + result.host = hostname; + return result } diff --git a/packages/astro/test/units/app/node.test.js b/packages/astro/test/units/app/node.test.js index f461e13d3e09..d567a1e3e87d 100644 --- a/packages/astro/test/units/app/node.test.js +++ b/packages/astro/test/units/app/node.test.js @@ -324,6 +324,7 @@ describe('node', () => { ...mockNodeRequest, headers: { host: 'example.com', + 'x-forwarded-host': 'example.com', 'x-forwarded-proto': 'http', 'x-forwarded-port': '80', }, @@ -339,6 +340,7 @@ describe('node', () => { ...mockNodeRequest, headers: { host: 'example.com', + 'x-forwarded-host': 'example.com', 'x-forwarded-proto': 'http,https', 'x-forwarded-port': '80,443', }, @@ -354,6 +356,7 @@ describe('node', () => { ...mockNodeRequest, headers: { host: 'example.com', + 'x-forwarded-host': 'example.com', }, }, { allowedDomains: [{ hostname: 'example.com' }] }, @@ -362,6 +365,20 @@ describe('node', () => { }); it('rejects malicious x-forwarded-proto with URL injection (https://www.malicious-url.com/?tank=)', () => { + const result = createRequest( + { + ...mockNodeRequest, + headers: { + host: 'example.com', + 'x-forwarded-host': 'example.com', + 'x-forwarded-proto': 'https://www.malicious-url.com/?tank=', + }, + }, + { allowedDomains: [{ hostname: 'example.com' }] }, + ); + assert.equal(result.url, 'https://example.com/'); + }); + it('rejects malicious x-forwarded-proto with URL injection (https://www.malicious-url.com/?tank=) when host is missing', () => { const result = createRequest( { ...mockNodeRequest, @@ -381,6 +398,7 @@ describe('node', () => { ...mockNodeRequest, headers: { host: 'example.com', + 'x-forwarded-host': 'example.com', 'x-forwarded-proto': 'x:admin?', }, }, @@ -395,6 +413,7 @@ describe('node', () => { ...mockNodeRequest, headers: { host: 'example.com', + 'x-forwarded-host': 'example.com', 'x-forwarded-proto': 'https://localhost/vulnerable?', }, }, @@ -409,6 +428,7 @@ describe('node', () => { ...mockNodeRequest, headers: { host: 'example.com', + 'x-forwarded-host': 'example.com', 'x-forwarded-proto': 'javascript:alert(document.cookie)//', }, }, @@ -423,6 +443,7 @@ describe('node', () => { ...mockNodeRequest, headers: { host: 'example.com', + 'x-forwarded-host': 'example.com', 'x-forwarded-proto': '', }, }, @@ -430,6 +451,131 @@ describe('node', () => { ); assert.equal(result.url, 'https://example.com/'); }); + it('accepts x-forwarded-proto when allowedDomains has protocol and hostname', () => { + const result = createRequest( + { + ...mockNodeRequest, + socket: { encrypted: false, remoteAddress: '2.2.2.2' }, + headers: { + host: 'myapp.example.com', + 'x-forwarded-host': 'myapp.example.com', + 'x-forwarded-proto': 'https', + }, + }, + { allowedDomains: [{ protocol: 'https', hostname: 'myapp.example.com' }] }, + ); + // Without the fix, protocol validation fails due to hostname mismatch + // and falls back to socket.encrypted (false → http) + assert.equal(result.url, 'https://myapp.example.com/'); + }); + + it('rejects x-forwarded-proto when it does not match protocol in allowedDomains', () => { + const result = createRequest( + { + ...mockNodeRequest, + socket: { encrypted: false, remoteAddress: '2.2.2.2' }, + headers: { + host: 'myapp.example.com', + 'x-forwarded-proto': 'http', + }, + }, + { allowedDomains: [{ protocol: 'https', hostname: 'myapp.example.com' }] }, + ); + // http is not in allowedDomains (only https), protocol falls back to socket (false → http) + // Host validation also fails because http doesn't match the pattern's protocol: 'https' + assert.equal(result.url, 'http://localhost/'); + }); + + it('accepts x-forwarded-proto with wildcard hostname pattern in allowedDomains', () => { + const result = createRequest( + { + ...mockNodeRequest, + socket: { encrypted: false, remoteAddress: '2.2.2.2' }, + headers: { + 'x-forwarded-host': 'joel.example.com', + 'x-forwarded-proto': 'https', + }, + }, + { allowedDomains: [{ protocol: 'https', hostname: '**.example.com' }] }, + ); + assert.equal(result.url, 'https://joel.example.com/'); + }); + + it('constructs correct URL behind reverse proxy with all forwarded headers', () => { + // Simulates: Reverse proxy terminates TLS, connects to Astro via HTTP, + // forwards original protocol/host/port via X-Forwarded-* headers + const result = createRequest( + { + ...mockNodeRequest, + socket: { encrypted: false, remoteAddress: '2.2.2.2' }, + headers: { + host: 'myapp.example.com', + 'x-forwarded-proto': 'https', + 'x-forwarded-host': 'myapp.example.com', + }, + }, + { allowedDomains: [{ protocol: 'https', hostname: 'myapp.example.com' }] }, + ); + assert.equal(result.url, 'https://myapp.example.com/'); + }); + + it('hmmm', () => { + const result = createRequest( + { + ...mockNodeRequest, + socket: { encrypted: false, remoteAddress: '2.2.2.2' }, + headers: { + host: 'something-other.com', + 'x-forwarded-proto': 'https', + 'x-forwarded-host': 'myapp.example.com', + }, + }, + { allowedDomains: [{ protocol: 'https', hostname: 'another.example.com' }] }, + ); + // should be 'http://something-other.com/' no? + assert.equal(result.url, 'http://localhost/'); + }); + + it('trick validate-headers', () => { + const result = createRequest( + { + ...mockNodeRequest, + socket: { encrypted: false, remoteAddress: '2.2.2.2' }, + headers: { + host: 'this-header-does-nothing.com', + 'x-forwarded-host': 'myapp.example.com', + }, + }, + { + allowedDomains: [ + { protocol: 'http', hostname: 'local.domain' }, + { protocol: 'https', hostname: 'myapp.example.com' } + ] + }, + ); + + assert.equal(result.url, 'http://localhost/'); + }); + + it('trick validate-headers 2', () => { + const result = createRequest( + { + ...mockNodeRequest, + socket: { encrypted: false, remoteAddress: '2.2.2.2' }, + headers: { + 'x-forwarded-proto': 'https', + }, + }, + { + allowedDomains: [ + { protocol: 'https', hostname: 'myapp.example.com' } + ] + }, + ); + + assert.equal(result.url, 'http://localhost/'); + }); + }); describe('x-forwarded-port', () => { @@ -439,12 +585,12 @@ describe('node', () => { ...mockNodeRequest, headers: { host: 'example.com', + 'x-forwarded-host': 'example.com', 'x-forwarded-port': '8443', }, }, { allowedDomains: [ - { hostname: 'example.com' }, { hostname: 'example.com', port: '8443' }, ], }, @@ -458,12 +604,12 @@ describe('node', () => { ...mockNodeRequest, headers: { host: 'example.com', + 'x-forwarded-host': 'example.com', 'x-forwarded-port': '8443,3000', }, }, { allowedDomains: [ - { hostname: 'example.com' }, { hostname: 'example.com', port: '8443' }, ], }, @@ -477,6 +623,7 @@ describe('node', () => { ...mockNodeRequest, headers: { host: 'example.com', + 'x-forwarded-host': 'example.com', 'x-forwarded-port': '8443', }, }, @@ -491,6 +638,7 @@ describe('node', () => { ...mockNodeRequest, headers: { host: 'example.com:3000', + 'x-forwarded-host': 'example.com', 'x-forwarded-port': '443', }, }, @@ -508,7 +656,7 @@ describe('node', () => { 'x-forwarded-host': 'example.com:3000', }, }, - { allowedDomains: [{ hostname: 'example.com' }] }, + { allowedDomains: [{ hostname: 'example.com', port:'3000' }] }, ); assert.equal(result.url, 'https://example.com:3000/'); }); diff --git a/packages/integrations/node/test/url.test.js b/packages/integrations/node/test/url.test.js index c3e90ead56f9..96b1449f6b2a 100644 --- a/packages/integrations/node/test/url.test.js +++ b/packages/integrations/node/test/url.test.js @@ -48,7 +48,7 @@ describe('URL', () => { it('return http when the X-Forwarded-Proto header is set to http', async () => { const { handler } = await import('./fixtures/url/dist/server/entry.mjs'); const { req, res, text } = createRequestAndResponse({ - headers: { 'X-Forwarded-Proto': 'http' }, + headers: { 'X-Forwarded-Proto': 'http', 'X-Forwarded-Host': 'legitimate.example.com' }, url: '/', }); @@ -62,7 +62,7 @@ describe('URL', () => { it('return https when the X-Forwarded-Proto header is set to https', async () => { const { handler } = await import('./fixtures/url/dist/server/entry.mjs'); const { req, res, text } = createRequestAndResponse({ - headers: { 'X-Forwarded-Proto': 'https' }, + headers: { 'X-Forwarded-Proto': 'https', 'X-Forwarded-Host': 'legitimate.example.com' }, url: '/', }); @@ -71,6 +71,7 @@ describe('URL', () => { const html = await text(); assert.equal(html.includes('https:'), true); + }); it('includes forwarded host and port in the url', async () => { @@ -132,7 +133,7 @@ describe('URL', () => { const $ = cheerio.load(html); // Should use the Host header, not X-Forwarded-Host when allowedDomains is not configured - assert.equal($('body').text(), 'https://legitimate.example.com/'); + assert.equal($('body').text(), 'http://legitimate.example.com/'); }); it('rejects port in forwarded host when port not in allowedDomains', async () => { @@ -153,7 +154,7 @@ describe('URL', () => { const $ = cheerio.load(html); // Port 8080 not in allowedDomains (only 444), so should fall back to Host header - assert.equal($('body').text(), 'https://localhost:3000/'); + assert.equal($('body').text(), 'http://localhost:3000/'); }); it('rejects empty X-Forwarded-Host with allowedDomains configured', async () => { @@ -174,7 +175,7 @@ describe('URL', () => { const $ = cheerio.load(html); // Empty X-Forwarded-Host should be rejected and fall back to Host header - assert.equal($('body').text(), 'https://legitimate.example.com/'); + assert.equal($('body').text(), 'http://legitimate.example.com/'); }); it('rejects X-Forwarded-Host with path injection attempt', async () => { @@ -195,6 +196,6 @@ describe('URL', () => { const $ = cheerio.load(html); // Path injection attempt should be rejected and fall back to Host header - assert.equal($('body').text(), 'https://localhost:3000/'); + assert.equal($('body').text(), 'http://localhost:3000/'); }); });