diff --git a/.changeset/kind-snails-wash.md b/.changeset/kind-snails-wash.md new file mode 100644 index 000000000000..617b736c118f --- /dev/null +++ b/.changeset/kind-snails-wash.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes `Astro.request.url` not reflecting validated `X-Forwarded-Proto`/`X-Forwarded-Host` headers when `security.allowedDomains` is configured. Previously, only `Astro.url` was updated with the forwarded origin while `Astro.request.url` retained the socket-derived URL, causing the two to diverge behind TLS-terminating proxies. diff --git a/packages/astro/src/core/fetch/fetch-state.ts b/packages/astro/src/core/fetch/fetch-state.ts index 3b531477e8c4..d7cf1dfb504b 100644 --- a/packages/astro/src/core/fetch/fetch-state.ts +++ b/packages/astro/src/core/fetch/fetch-state.ts @@ -14,6 +14,7 @@ import type { RouteData, SSRResult } from '../../types/public/internal.js'; import { AstroCookies } from '../cookies/index.js'; import { type Pipeline, Slots } from '../render/index.js'; import { + appSymbol, ASTRO_GENERATOR, fetchStateSymbol, originPathnameSymbol, @@ -278,10 +279,12 @@ export class FetchState implements AstroFetchState { this.#applyForwardedHeaders(); } - // Set origin pathname for rewrite tracking. - if (!Reflect.get(request, originPathnameSymbol)) { + // Set origin pathname for rewrite tracking. Use this.request + // (not the local parameter) because #applyForwardedHeaders() + // may have reconstructed it with a forwarded URL. + if (!Reflect.get(this.request, originPathnameSymbol)) { setOriginPathname( - request, + this.request, this.pathname, pipeline.manifest.trailingSlash, pipeline.manifest.buildFormat, @@ -914,6 +917,23 @@ export class FetchState implements AstroFetchState { this.clientAddress = forwardedFor; } } + + // Reconstruct the Request with the resolved URL so that + // request.url stays in sync with this.url. Request.url is a + // readonly string, so we must create a new Request object. The + // constructor carries over method, headers, body (incl. stream + + // duplex) and signal from the old request. + const oldRequest = this.request; + this.request = new Request(this.url, oldRequest); + // Re-attach `appSymbol`: the rest of the pipeline resolves the app + // via `getApp(state.request)` (see core/fetch/index.ts), so the new + // Request must carry it. We copy only this known Astro symbol. + // Other request-bound state is either already captured on + // `this` (clientAddress) or set after this point (originPathname). + const app = Reflect.get(oldRequest, appSymbol); + if (app !== undefined) { + Reflect.set(this.request, appSymbol, app); + } } /** diff --git a/packages/astro/test/units/fetch/index.test.ts b/packages/astro/test/units/fetch/index.test.ts index 1bf745f7b6e0..b34f9d21679a 100644 --- a/packages/astro/test/units/fetch/index.test.ts +++ b/packages/astro/test/units/fetch/index.test.ts @@ -809,8 +809,6 @@ describe('FetchState X-Forwarded-* header resolution', () => { }); it('handles headers set by user fetch handler before FetchState creation', () => { - // This is the core use case from the issue: user sets forwarded - // headers in their src/app.ts fetch() before creating FetchState. const app = createTestApp([createPage(simplePage, { route: '/' })], { allowedDomains: [{ hostname: 'example.com' }], }); @@ -827,8 +825,6 @@ describe('FetchState X-Forwarded-* header resolution', () => { }); it('renders through the full pipeline with forwarded headers applied', async () => { - // End-to-end: forwarded headers should be visible in Astro.url - // when rendering through the astro() combined handler. const app = createTestApp([createPage(simplePage, { route: '/' })], { allowedDomains: [{ hostname: 'example.com' }], }); @@ -843,13 +839,123 @@ describe('FetchState X-Forwarded-* header resolution', () => { ); const state = new FetchState(request); - // Verify URL was updated before the handler runs assert.equal(state.url.protocol, 'https:'); assert.equal(state.url.hostname, 'example.com'); const response = await astro(state); assert.equal(response.status, 200); }); + + it('updates request.url to match the forwarded URL', () => { + const app = createTestApp([createPage(simplePage, { route: '/' })], { + allowedDomains: [{ hostname: 'example.com' }], + }); + const request = stampApp( + new Request('http://localhost:4321/page', { + headers: { + 'x-forwarded-proto': 'https', + 'x-forwarded-host': 'example.com', + }, + }), + app, + ); + const state = new FetchState(request); + + assert.equal(state.url.protocol, 'https:'); + assert.equal(state.url.hostname, 'example.com'); + + const requestUrl = new URL(state.request.url); + assert.equal(requestUrl.protocol, 'https:'); + assert.equal(requestUrl.hostname, 'example.com'); + assert.equal(requestUrl.pathname, '/page'); + }); + + it('does not reconstruct request when no forwarded headers are validated', () => { + const app = createTestApp([createPage(simplePage, { route: '/' })], { + allowedDomains: [{ hostname: 'trusted.com' }], + }); + const original = new Request('http://localhost:4321/', { + headers: { + 'x-forwarded-host': 'evil.com', + }, + }); + const request = stampApp(original, app); + const state = new FetchState(request); + + // Rejected forwarded host — request should stay unchanged + assert.equal(state.url.hostname, 'localhost'); + assert.equal(new URL(state.request.url).hostname, 'localhost'); + }); + + it('carries appSymbol onto the reconstructed request so the app still resolves', async () => { + const app = createTestApp([createPage(simplePage, { route: '/' })], { + allowedDomains: [{ hostname: 'example.com' }], + }); + const original = new Request('http://localhost:4321/', { + headers: { + 'x-forwarded-proto': 'https', + 'x-forwarded-host': 'example.com', + }, + }); + const request = stampApp(original, app); + const state = new FetchState(request); + + assert.notEqual(state.request, original); + assert.equal(Reflect.get(state.request, appSymbol), app); + const response = await astro(state); + assert.equal(response.status, 200); + }); + + it('preserves method, headers and body when reconstructing for forwarded headers', async () => { + const app = createTestApp([createPage(simplePage, { route: '/api' })], { + allowedDomains: [{ hostname: 'example.com' }], + }); + const request = stampApp( + new Request('http://localhost:4321/api', { + method: 'POST', + headers: { + 'content-type': 'application/x-www-form-urlencoded', + 'x-forwarded-proto': 'https', + 'x-forwarded-host': 'example.com', + }, + body: 'token=abc123', + }), + app, + ); + const state = new FetchState(request); + + assert.equal(new URL(state.request.url).protocol, 'https:'); + assert.equal(new URL(state.request.url).hostname, 'example.com'); + assert.equal(state.request.method, 'POST'); + assert.equal(state.request.headers.get('content-type'), 'application/x-www-form-urlencoded'); + assert.equal(await state.request.text(), 'token=abc123'); + }); + + it('preserves a streaming request body (duplex) when reconstructing', async () => { + const app = createTestApp([createPage(simplePage, { route: '/api' })], { + allowedDomains: [{ hostname: 'example.com' }], + }); + const body = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode('chunked-payload')); + controller.close(); + }, + }); + const init: any = { + method: 'POST', + headers: { + 'x-forwarded-proto': 'https', + 'x-forwarded-host': 'example.com', + }, + body, + duplex: 'half', + }; + const request = stampApp(new Request('http://localhost:4321/api', init), app); + const state = new FetchState(request); + + assert.equal(new URL(state.request.url).hostname, 'example.com'); + assert.equal(await state.request.text(), 'chunked-payload'); + }); }); // #endregion