[pull] main from withastro:main

.changeset/fix-cloudflare-fetch-circular-import.md

@@ -0,0 +1,5 @@
+---
+'@astrojs/cloudflare': patch
+---
+
+Fixes a build crash when using `experimental.advancedRouting` with a custom `fetchFile` that statically imports `cf` from `@astrojs/cloudflare/fetch`. The circular dependency between `@astrojs/cloudflare/fetch` and `astro/app/entrypoint` caused `createApp` or `createGetEnv` to be `undefined` at module evaluation time. Initialization is now deferred to the first `cf()` call, breaking the cycle.

.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.

packages/astro/src/core/errors/errors-data.ts

@@ -223,8 +223,8 @@ export const NoClientOnlyHint = {
  * ---
  * export async function getStaticPaths() {
  *	return [
- *		{ params: { slug: "blog" } },
- * 		{ params: { slug: "about" } }
+ *		{ params: { id: "blog" } },
+ * 		{ params: { id: "about" } }
  * 	];
  *}
  *---
@@ -247,8 +247,8 @@ export const InvalidGetStaticPathParam = {
  * ```ts title="pages/blog/[id].astro"
  * export async function getStaticPaths() {
  *	return [ // <-- Array
- *		{ params: { slug: "blog" } }, // <-- Object
- * 		{ params: { slug: "about" } }
+ *		{ params: { id: "blog" } }, // <-- Object
+ * 		{ params: { id: "about" } }
  * 	];
  *}
  * ```
@@ -271,8 +271,8 @@ export const InvalidGetStaticPathsEntry = {
  * ```ts title="pages/blog/[id].astro"
  * export async function getStaticPaths() {
  *	return [ // <-- Array
- *		{ params: { slug: "blog" } },
- * 		{ params: { slug: "about" } }
+ *		{ params: { id: "blog" } },
+ * 		{ params: { id: "about" } }
  * 	];
  *}
  * ```

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,
@@ -308,10 +309,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,
@@ -956,6 +959,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);
+		}
 	}
 
 	/**

packages/astro/test/units/fetch/index.test.ts

@@ -831,8 +831,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' }],
 		});
@@ -849,8 +847,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' }],
 		});
@@ -865,13 +861,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

packages/integrations/cloudflare/src/fetch.ts

@@ -31,9 +31,18 @@ import {
 	getClientAddress,
 } from './utils/cf.js';
 
-setGetEnv(createGetEnv(globalEnv));
+// Lazy initialization — `createApp` and `setGetEnv` are deferred to the
+// first `cf()` call so that this module can be statically imported from a
+// custom `fetchFile` without triggering a circular-dependency crash.
+// (The cycle: fetch.ts → astro/app/entrypoint → virtual:astro:fetchable → user worker → fetch.ts)
+let app: ReturnType<typeof createApp> | undefined;
 
-const app = createApp();
+function ensureInitialized() {
+	if (!app) {
+		setGetEnv(createGetEnv(globalEnv));
+		app = createApp();
+	}
+}
 
 /**
  * Applies Cloudflare-specific setup to a `FetchState`:
@@ -50,9 +59,10 @@ export async function cf(
 	env: Env,
 	ctx: ExecutionContext,
 ): Promise<Response | undefined> {
-	injectSessionBinding(app.manifest, env);
+	ensureInitialized();
+	injectSessionBinding(app!.manifest, env);
 
-	const staticAsset = matchStaticAsset(app.manifest, state.request.url, env);
+	const staticAsset = matchStaticAsset(app!.manifest, state.request.url, env);
 	if (staticAsset) return staticAsset;
 
 	if (!state.routeData) {

packages/integrations/cloudflare/test/fetch-lazy-init.test.ts

@@ -0,0 +1,68 @@
+import * as assert from 'node:assert/strict';
+import { readFileSync } from 'node:fs';
+import { describe, it } from 'node:test';
+
+/**
+ * `@astrojs/cloudflare/fetch` must not call `createApp()` or `setGetEnv()`
+ * at module top-level. Eager calls cause a circular-dependency crash when
+ * a user's custom `fetchFile` (`src/worker.ts`) statically imports `cf`
+ * from `@astrojs/cloudflare/fetch`.
+ *
+ * The cycle: fetch.ts → astro/app/entrypoint → virtual:astro:fetchable
+ *          → user worker → fetch.ts
+ *
+ * See: https://github.com/withastro/astro/issues/16956
+ */
+describe('@astrojs/cloudflare/fetch lazy initialization', () => {
+	const source = readFileSync(new URL('../dist/fetch.js', import.meta.url), 'utf-8');
+
+	it('does not call createApp() at module top-level', () => {
+		// After the fix, createApp() should only appear inside the
+		// ensureInitialized() function body, not as a bare top-level statement.
+		// We check that there is no top-level `createApp()` by verifying the
+		// pattern does NOT appear outside a function body.
+		// A simple heuristic: the call `= createApp()` should not exist as a
+		// top-level const/let/var assignment (the old pattern was `const app = createApp()`).
+		const topLevelCreateApp = /^(?:const|let|var)\s+\w+\s*=\s*createApp\(\)/m;
+		assert.equal(
+			topLevelCreateApp.test(source),
+			false,
+			'createApp() must not be called at module top-level (causes circular import crash)',
+		);
+	});
+
+	it('does not call setGetEnv() at module top-level', () => {
+		// The old pattern was a bare `setGetEnv(createGetEnv(globalEnv))` statement
+		// at the top level. After the fix this should only appear inside ensureInitialized().
+		// A line starting with `setGetEnv(` (not inside a function) is the problem pattern.
+		const lines = source.split('\n');
+		const topLevelSetGetEnv = lines.some((line) => {
+			const trimmed = line.trim();
+			// Skip lines inside function bodies (indented or after opening brace)
+			// A simple heuristic: if the line starts with setGetEnv and is not indented,
+			// it's top-level.
+			return trimmed.startsWith('setGetEnv(') && !line.startsWith('  ') && !line.startsWith('\t');
+		});
+		assert.equal(
+			topLevelSetGetEnv,
+			false,
+			'setGetEnv() must not be called at module top-level (causes circular import crash)',
+		);
+	});
+
+	it('exports ensureInitialized or calls it inside cf()', () => {
+		// Verify that lazy init is wired into the cf() function
+		assert.ok(
+			source.includes('ensureInitialized()'),
+			'cf() should call ensureInitialized() for lazy setup',
+		);
+	});
+
+	it('exports a cf function', () => {
+		// Sanity check: the module still exports cf
+		assert.ok(
+			source.includes('export {') && source.includes('cf'),
+			'Module should export the cf function',
+		);
+	});
+});