fix: internal header leak (#15718)

florian-lefebvre · web-flow · commit 14f37b80aa2b · 2026-03-02T17:08:36.000+01:00
diff --git a/.changeset/smart-pillows-chew.md b/.changeset/smart-pillows-chew.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes a case where internal headers may leak when rendering error pages
diff --git a/packages/astro/src/core/app/base.ts b/packages/astro/src/core/app/base.ts
@@ -15,10 +15,12 @@ import type { Pipeline } from '../base-pipeline.js';
 import {
 	clientAddressSymbol,
 	DEFAULT_404_COMPONENT,
+	NOOP_MIDDLEWARE_HEADER,
 	REROUTABLE_STATUS_CODES,
 	REROUTE_DIRECTIVE_HEADER,
 	responseSentSymbol,
 	REWRITE_DIRECTIVE_HEADER_KEY,
+	ROUTE_TYPE_HEADER,
 } from '../constants.js';
 import { getSetCookiesFromResponse } from '../cookies/index.js';
 import { AstroError, AstroErrorData } from '../errors/index.js';
@@ -84,9 +86,17 @@ export interface RenderOptions {
 	routeData?: RouteData;
 }
 
-export interface RenderErrorOptions {
-	locals?: App.Locals;
-	routeData?: RouteData;
+type RequiredRenderOptions = Required<RenderOptions>;
+
+interface ResolvedRenderOptions {
+	addCookieHeader: RequiredRenderOptions['addCookieHeader'];
+	clientAddress: RequiredRenderOptions['clientAddress'] | undefined;
+	prerenderedErrorPageFetch: RequiredRenderOptions['prerenderedErrorPageFetch'] | undefined;
+	locals: RequiredRenderOptions['locals'] | undefined;
+	routeData: RequiredRenderOptions['routeData'] | undefined;
+}
+
+export interface RenderErrorOptions extends ResolvedRenderOptions {
 	response?: Response;
 	status: 404 | 500;
 	/**
@@ -97,8 +107,6 @@ export interface RenderErrorOptions {
 	 * Allows passing an error to 500.astro. It will be available through `Astro.props.error`.
 	 */
 	error?: unknown;
-	clientAddress: string | undefined;
-	prerenderedErrorPageFetch: ((url: ErrorPagePath) => Promise<Response>) | undefined;
 }
 
 type ErrorPagePath =
@@ -355,19 +363,23 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
 		return pathname;
 	}
 
-	public async render(request: Request, renderOptions?: RenderOptions): Promise<Response> {
+	public async render(
+		request: Request,
+		{
+			addCookieHeader = false,
+			clientAddress = Reflect.get(request, clientAddressSymbol),
+			locals,
+			prerenderedErrorPageFetch = fetch,
+			routeData,
+		}: RenderOptions = {},
+	): Promise<Response> {
 		const timeStart = performance.now();
-		let routeData: RouteData | undefined = renderOptions?.routeData;
-		let locals: object | undefined;
-		let clientAddress: string | undefined;
-		let addCookieHeader: boolean | undefined;
 		const url = new URL(request.url);
 		const redirect = this.redirectTrailingSlash(url.pathname);
-		const prerenderedErrorPageFetch = renderOptions?.prerenderedErrorPageFetch ?? fetch;
 
 		if (redirect !== url.pathname) {
 			const status = request.method === 'GET' ? 301 : 308;
-			return new Response(
+			const response = new Response(
 				redirectTemplate({
 					status,
 					relativeLocation: url.pathname,
@@ -381,13 +393,10 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
 					},
 				},
 			);
+			this.#prepareResponse(response, { addCookieHeader });
+			return response;
 		}
 
-		addCookieHeader = renderOptions?.addCookieHeader;
-		clientAddress = renderOptions?.clientAddress ?? Reflect.get(request, clientAddressSymbol);
-		routeData = renderOptions?.routeData;
-		locals = renderOptions?.locals;
-
 		if (routeData) {
 			this.logger.debug(
 				'router',
@@ -397,15 +406,26 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
 			this.logger.debug('router', 'RouteData');
 			this.logger.debug('router', routeData);
 		}
+
+		const resolvedRenderOptions: ResolvedRenderOptions = {
+			addCookieHeader,
+			clientAddress,
+			prerenderedErrorPageFetch,
+			locals,
+			routeData,
+		};
+
 		if (locals) {
 			if (typeof locals !== 'object') {
 				const error = new AstroError(AstroErrorData.LocalsNotAnObject);
 				this.logger.error(null, error.stack!);
 				return this.renderError(request, {
+					...resolvedRenderOptions,
+					// If locals are invalid, we don't want to include them when
+					// rendering the error page
+					locals: undefined,
 					status: 500,
 					error,
-					clientAddress,
-					prerenderedErrorPageFetch: prerenderedErrorPageFetch,
 				});
 			}
 		}
@@ -434,10 +454,8 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
 			this.logger.debug('router', "Astro hasn't found routes that match " + request.url);
 			this.logger.debug('router', "Here's the available routes:\n", this.manifestData);
 			return this.renderError(request, {
-				locals,
+				...resolvedRenderOptions,
 				status: 404,
-				clientAddress,
-				prerenderedErrorPageFetch: prerenderedErrorPageFetch,
 			});
 		}
 		const pathname = this.getPathnameFromRequest(request);
@@ -472,11 +490,9 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
 		} catch (err: any) {
 			this.logger.error(null, err.stack || err.message || String(err));
 			return this.renderError(request, {
-				locals,
+				...resolvedRenderOptions,
 				status: 500,
 				error: err,
-				clientAddress,
-				prerenderedErrorPageFetch: prerenderedErrorPageFetch,
 			});
 		} finally {
 			await session?.[PERSIST_SYMBOL]();
@@ -490,30 +506,39 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
 			response.headers.get(REROUTE_DIRECTIVE_HEADER) !== 'no'
 		) {
 			return this.renderError(request, {
-				locals,
+				...resolvedRenderOptions,
 				response,
 				status: response.status as 404 | 500,
 				// We don't have an error to report here. Passing null means we pass nothing intentionally
 				// while undefined means there's no error
 				error: response.status === 500 ? null : undefined,
-				clientAddress,
-				prerenderedErrorPageFetch: prerenderedErrorPageFetch,
 			});
 		}
 
+		this.#prepareResponse(response, { addCookieHeader });
+		return response;
+	}
+
+	#prepareResponse(response: Response, { addCookieHeader }: { addCookieHeader: boolean }): void {
 		// We remove internally-used header before we send the response to the user agent.
-		if (response.headers.has(REROUTE_DIRECTIVE_HEADER)) {
-			response.headers.delete(REROUTE_DIRECTIVE_HEADER);
+		for (const headerName of [
+			REROUTE_DIRECTIVE_HEADER,
+			REWRITE_DIRECTIVE_HEADER_KEY,
+			NOOP_MIDDLEWARE_HEADER,
+			ROUTE_TYPE_HEADER,
+		]) {
+			if (response.headers.has(headerName)) {
+				response.headers.delete(headerName);
+			}
 		}
 
 		if (addCookieHeader) {
-			for (const setCookieHeaderValue of BaseApp.getSetCookieFromResponse(response)) {
+			for (const setCookieHeaderValue of getSetCookiesFromResponse(response)) {
 				response.headers.append('set-cookie', setCookieHeaderValue);
 			}
 		}
 
 		Reflect.set(response, responseSentSymbol, true);
-		return response;
 	}
 
 	setCookieHeaders(response: Response) {
@@ -540,13 +565,11 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
 	public async renderError(
 		request: Request,
 		{
-			locals,
 			status,
 			response: originalResponse,
 			skipMiddleware = false,
 			error,
-			clientAddress,
-			prerenderedErrorPageFetch,
+			...resolvedRenderOptions
 		}: RenderErrorOptions,
 	): Promise<Response> {
 		const errorRoutePath = `/${status}${this.manifest.trailingSlash === 'always' ? '/' : ''}`;
@@ -556,8 +579,13 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
 			if (errorRouteData.prerender) {
 				const maybeDotHtml = errorRouteData.route.endsWith(`/${status}`) ? '.html' : '';
 				const statusURL = new URL(`${this.baseWithoutTrailingSlash}/${status}${maybeDotHtml}`, url);
-				if (statusURL.toString() !== request.url && prerenderedErrorPageFetch) {
-					const response = await prerenderedErrorPageFetch(statusURL.toString() as ErrorPagePath);
+				if (
+					statusURL.toString() !== request.url &&
+					resolvedRenderOptions.prerenderedErrorPageFetch
+				) {
+					const response = await resolvedRenderOptions.prerenderedErrorPageFetch(
+						statusURL.toString() as ErrorPagePath,
+					);
 
 					// In order for the response of the remote to be usable as a response
 					// for this request, it needs to have our status code in the response
@@ -571,36 +599,38 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
 					// not match the body we provide and need to be removed.
 					const override = { status, removeContentEncodingHeaders: true };
 
-					return this.mergeResponses(response, originalResponse, override);
+					const newResponse = this.mergeResponses(response, originalResponse, override);
+					this.#prepareResponse(newResponse, resolvedRenderOptions);
+					return newResponse;
 				}
 			}
 			const mod = await this.pipeline.getComponentByRoute(errorRouteData);
 			let session: AstroSession | undefined;
 			try {
 				const renderContext = await this.createRenderContext({
-					locals,
+					locals: resolvedRenderOptions.locals,
 					pipeline: this.pipeline,
 					skipMiddleware,
 					pathname: this.getPathnameFromRequest(request),
 					request,
 					routeData: errorRouteData,
 					status,
 					props: { error },
-					clientAddress,
+					clientAddress: resolvedRenderOptions.clientAddress,
 				});
 				session = renderContext.session;
 				const response = await renderContext.render(mod);
-				return this.mergeResponses(response, originalResponse);
+				const newResponse = this.mergeResponses(response, originalResponse);
+				this.#prepareResponse(newResponse, resolvedRenderOptions);
+				return newResponse;
 			} catch {
 				// Middleware may be the cause of the error, so we try rendering 404/500.astro without it.
 				if (skipMiddleware === false) {
 					return this.renderError(request, {
-						locals,
+						...resolvedRenderOptions,
 						status,
 						response: originalResponse,
 						skipMiddleware: true,
-						clientAddress,
-						prerenderedErrorPageFetch,
 					});
 				}
 			} finally {
@@ -609,7 +639,7 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
 		}
 
 		const response = this.mergeResponses(new Response(null, { status }), originalResponse);
-		Reflect.set(response, responseSentSymbol, true);
+		this.#prepareResponse(response, resolvedRenderOptions);
 		return response;
 	}
 
diff --git a/packages/astro/src/core/app/dev/app.ts b/packages/astro/src/core/app/dev/app.ts
@@ -76,7 +76,13 @@ export class DevApp extends BaseApp<NonRunnablePipeline> {
 
 	async renderError(
 		request: Request,
-		{ locals, skipMiddleware = false, error, clientAddress, status }: RenderErrorOptions,
+		{
+			skipMiddleware = false,
+			error,
+			status,
+			response: _response,
+			...resolvedRenderOptions
+		}: RenderErrorOptions,
 	): Promise<Response> {
 		// we always throw when we have Astro errors around the middleware
 		if (
@@ -90,13 +96,13 @@ export class DevApp extends BaseApp<NonRunnablePipeline> {
 			try {
 				const preloadedComponent = await this.pipeline.getComponentByRoute(routeData);
 				const renderContext = await this.createRenderContext({
-					locals,
+					locals: resolvedRenderOptions.locals,
 					pipeline: this.pipeline,
-					pathname: await this.getPathnameFromRequest(request),
+					pathname: this.getPathnameFromRequest(request),
 					skipMiddleware,
 					request,
 					routeData,
-					clientAddress,
+					clientAddress: resolvedRenderOptions.clientAddress,
 					status,
 					shouldInjectCspMetaTags: false,
 				});
@@ -112,8 +118,7 @@ export class DevApp extends BaseApp<NonRunnablePipeline> {
 			} catch (_err) {
 				if (skipMiddleware === false) {
 					return this.renderError(request, {
-						clientAddress: undefined,
-						prerenderedErrorPageFetch: fetch,
+						...resolvedRenderOptions,
 						status: 500,
 						skipMiddleware: true,
 						error: _err,
diff --git a/packages/astro/src/vite-plugin-app/app.ts b/packages/astro/src/vite-plugin-app/app.ts
@@ -234,7 +234,13 @@ export class AstroServerApp extends BaseApp<RunnablePipeline> {
 
 	async renderError(
 		request: Request,
-		{ locals, skipMiddleware = false, error, clientAddress, status }: RenderErrorOptions,
+		{
+			skipMiddleware = false,
+			error,
+			status,
+			response: _response,
+			...resolvedRenderOptions
+		}: RenderErrorOptions,
 	): Promise<Response> {
 		// we always throw when we have Astro errors around the middleware
 		if (
@@ -248,13 +254,13 @@ export class AstroServerApp extends BaseApp<RunnablePipeline> {
 			try {
 				const preloadedComponent = await this.pipeline.getComponentByRoute(routeData);
 				const renderContext = await this.createRenderContext({
-					locals,
+					locals: resolvedRenderOptions.locals,
 					pipeline: this.pipeline,
-					pathname: await this.getPathnameFromRequest(request),
+					pathname: this.getPathnameFromRequest(request),
 					skipMiddleware,
 					request,
 					routeData,
-					clientAddress,
+					clientAddress: resolvedRenderOptions.clientAddress,
 					status,
 					shouldInjectCspMetaTags: !!this.manifest.csp,
 				});
@@ -270,8 +276,7 @@ export class AstroServerApp extends BaseApp<RunnablePipeline> {
 			} catch (_err) {
 				if (skipMiddleware === false) {
 					return this.renderError(request, {
-						clientAddress: undefined,
-						prerenderedErrorPageFetch: fetch,
+						...resolvedRenderOptions,
 						status: 500,
 						skipMiddleware: true,
 						error: _err,
diff --git a/packages/astro/test/units/app/error-pages.test.js b/packages/astro/test/units/app/error-pages.test.js
@@ -45,6 +45,10 @@ describe('App render error pages', () => {
 
 		assert.equal(response.status, 500);
 		assert.equal(response.headers.get('x-debug'), '1234');
+		assert.equal(
+			[...response.headers.keys()].some((e) => e.startsWith('X-Astro-')),
+			false,
+		);
 		assert.match(await response.text(), /oops/);
 	});
 
diff --git a/packages/astro/test/units/app/response.test.js b/packages/astro/test/units/app/response.test.js
@@ -142,4 +142,13 @@ describe('Using Astro.response in SSR', () => {
 		assert.equal(headers.get('four-five'), 'six');
 		assert.equal(headers.get('Cache-Control'), 'max-age=0, s-maxage=86400');
 	});
+
+	it('Removes internal headers', async () => {
+		const request = new Request('http://example.com/some-header');
+		const response = await app.render(request);
+		assert.equal(
+			[...response.headers.keys()].some((e) => e.startsWith('X-Astro-')),
+			false,
+		);
+	});
 });

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'astro': patch
 +---
++
 +Fixes a case where internal headers may leak when rendering error pages