Ensure cookies set by middleware are included in error page responses

.changeset/harden-merge-responses-cookies.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes cookie handling during error page rendering to ensure cookies set by middleware are consistently included in the response

packages/astro/src/core/app/base.ts

@@ -23,7 +23,12 @@ import {
 	REWRITE_DIRECTIVE_HEADER_KEY,
 	ROUTE_TYPE_HEADER,
 } from '../constants.js';
-import { getSetCookiesFromResponse } from '../cookies/index.js';
+import {
+	AstroCookies,
+	attachCookiesToResponse,
+	getSetCookiesFromResponse,
+} from '../cookies/index.js';
+import { getCookiesFromResponse } from '../cookies/response.js';
 import { AstroError, AstroErrorData } from '../errors/index.js';
 import { consoleLogDestination } from '../logger/console.js';
 import { AstroIntegrationLogger, Logger } from '../logger/core.js';
@@ -695,16 +700,24 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
 			// this function could throw an error...
 			originalResponse.headers.delete('Content-type');
 		} catch {}
-		// we use a map to remove duplicates
-		const mergedHeaders = new Map([
-			...Array.from(newResponseHeaders),
-			...Array.from(originalResponse.headers),
-		]);
+		// Build merged headers using append() to preserve multi-value headers (e.g. Set-Cookie).
+		// Headers from the original response take priority over new response headers for
+		// single-value headers, but we use append to avoid collapsing multi-value entries.
 		const newHeaders = new Headers();
-		for (const [name, value] of mergedHeaders) {
-			newHeaders.set(name, value);
+		const seen = new Set<string>();
+		// Add original response headers first (they take priority)
+		for (const [name, value] of originalResponse.headers) {
+			newHeaders.append(name, value);
+			seen.add(name.toLowerCase());
+		}
+		// Add new response headers that weren't already set by the original response,
+		// but skip content-type since the error page must return text/html
+		for (const [name, value] of newResponseHeaders) {
+			if (!seen.has(name.toLowerCase())) {
+				newHeaders.append(name, value);
+			}
 		}
-		return new Response(newResponse.body, {
+		const mergedResponse = new Response(newResponse.body, {
 			status,
 			statusText: status === 200 ? newResponse.statusText : originalResponse.statusText,
 			// If you're looking at here for possible bugs, it means that it's not a bug.
@@ -714,6 +727,24 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
 			// Although, we don't want it to replace the content-type, because the error page must return `text/html`
 			headers: newHeaders,
 		});
+
+		// Transfer AstroCookies from the original or new response so that
+		// #prepareResponse can read them when addCookieHeader is true.
+		const originalCookies = getCookiesFromResponse(originalResponse);
+		const newCookies = getCookiesFromResponse(newResponse);
+		if (originalCookies) {
+			// If both responses have cookies, merge new response cookies into original
+			if (newCookies) {
+				for (const cookieValue of AstroCookies.consume(newCookies)) {
+					originalResponse.headers.append('set-cookie', cookieValue);
+				}
+			}
+			attachCookiesToResponse(mergedResponse, originalCookies);
+		} else if (newCookies) {
+			attachCookiesToResponse(mergedResponse, newCookies);
+		}
+
+		return mergedResponse;
 	}
 
 	getDefaultStatusCode(routeData: RouteData, pathname: string): number {

packages/astro/test/units/middleware/middleware-app.test.js

@@ -585,6 +585,182 @@ describe('Middleware via App.render()', () => {
 		});
 	});
 
+	describe('cookies on error pages', () => {
+		it('should preserve cookies set by middleware when returning Response(null, { status: 404 })', async () => {
+			// Middleware sets a cookie and returns 404 with null body (common auth guard pattern)
+			const onRequest = async (ctx, next) => {
+				ctx.cookies.set('session', 'abc123', { path: '/' });
+				if (ctx.url.pathname.startsWith('/api/guarded')) {
+					return new Response(null, { status: 404 });
+				}
+				return next();
+			};
+
+			const guardedRouteData = createRouteData({
+				route: '/api/guarded/[...path]',
+				pathname: undefined,
+				segments: undefined,
+			});
+			// Override for spread route
+			guardedRouteData.params = ['...path'];
+			guardedRouteData.pattern = /^\/api\/guarded(?:\/(.*))?$/;
+			guardedRouteData.pathname = undefined;
+			guardedRouteData.segments = [
+				[{ content: 'api', dynamic: false, spread: false }],
+				[{ content: 'guarded', dynamic: false, spread: false }],
+				[{ content: '...path', dynamic: true, spread: true }],
+			];
+
+			const pageMap = new Map([
+				[
+					guardedRouteData.component,
+					async () => ({
+						page: async () => ({
+							default: simplePage(),
+						}),
+					}),
+				],
+				[
+					notFoundRouteData.component,
+					async () => ({ page: async () => ({ default: notFoundPage }) }),
+				],
+			]);
+			const app = createAppWithMiddleware({
+				onRequest,
+				routes: [{ routeData: guardedRouteData }, { routeData: notFoundRouteData }],
+				pageMap,
+			});
+
+			const response = await app.render(new Request('http://localhost/api/guarded/secret'), {
+				addCookieHeader: true,
+			});
+
+			assert.equal(response.status, 404);
+			const setCookie = response.headers.get('set-cookie');
+			assert.ok(setCookie, 'Expected Set-Cookie header to be present on 404 error page response');
+			assert.match(setCookie, /session=abc123/);
+		});
+
+		it('should preserve cookies set by middleware when returning Response(null, { status: 500 })', async () => {
+			const onRequest = async (ctx, next) => {
+				ctx.cookies.set('csrf', 'token456', { path: '/' });
+				if (ctx.url.pathname.startsWith('/api/error')) {
+					return new Response(null, { status: 500 });
+				}
+				return next();
+			};
+
+			const errorRouteData = createRouteData({
+				route: '/api/error/[...path]',
+				pathname: undefined,
+				segments: undefined,
+			});
+			errorRouteData.params = ['...path'];
+			errorRouteData.pattern = /^\/api\/error(?:\/(.*))?$/;
+			errorRouteData.pathname = undefined;
+			errorRouteData.segments = [
+				[{ content: 'api', dynamic: false, spread: false }],
+				[{ content: 'error', dynamic: false, spread: false }],
+				[{ content: '...path', dynamic: true, spread: true }],
+			];
+
+			const pageMap = new Map([
+				[
+					errorRouteData.component,
+					async () => ({
+						page: async () => ({
+							default: simplePage(),
+						}),
+					}),
+				],
+				[
+					serverErrorRouteData.component,
+					async () => ({ page: async () => ({ default: serverErrorPage }) }),
+				],
+			]);
+			const app = createAppWithMiddleware({
+				onRequest,
+				routes: [{ routeData: errorRouteData }, { routeData: serverErrorRouteData }],
+				pageMap,
+			});
+
+			const response = await app.render(new Request('http://localhost/api/error/test'), {
+				addCookieHeader: true,
+			});
+
+			assert.equal(response.status, 500);
+			const setCookie = response.headers.get('set-cookie');
+			assert.ok(setCookie, 'Expected Set-Cookie header to be present on 500 error page response');
+			assert.match(setCookie, /csrf=token456/);
+		});
+
+		it('should preserve multiple cookies from sequenced middleware during error page rerouting', async () => {
+			const onRequest = async (ctx, next) => {
+				ctx.cookies.set('session', 'abc123', { path: '/' });
+				ctx.cookies.set('csrf', 'token456', { path: '/' });
+				if (ctx.url.pathname.startsWith('/api/guarded')) {
+					ctx.cookies.set('auth_attempt', 'failed', { path: '/' });
+					return new Response(null, { status: 404 });
+				}
+				return next();
+			};
+
+			const guardedRouteData = createRouteData({
+				route: '/api/guarded/[...path]',
+				pathname: undefined,
+				segments: undefined,
+			});
+			guardedRouteData.params = ['...path'];
+			guardedRouteData.pattern = /^\/api\/guarded(?:\/(.*))?$/;
+			guardedRouteData.pathname = undefined;
+			guardedRouteData.segments = [
+				[{ content: 'api', dynamic: false, spread: false }],
+				[{ content: 'guarded', dynamic: false, spread: false }],
+				[{ content: '...path', dynamic: true, spread: true }],
+			];
+
+			const pageMap = new Map([
+				[
+					guardedRouteData.component,
+					async () => ({
+						page: async () => ({
+							default: simplePage(),
+						}),
+					}),
+				],
+				[
+					notFoundRouteData.component,
+					async () => ({ page: async () => ({ default: notFoundPage }) }),
+				],
+			]);
+			const app = createAppWithMiddleware({
+				onRequest,
+				routes: [{ routeData: guardedRouteData }, { routeData: notFoundRouteData }],
+				pageMap,
+			});
+
+			const response = await app.render(new Request('http://localhost/api/guarded/secret'), {
+				addCookieHeader: true,
+			});
+
+			assert.equal(response.status, 404);
+			const setCookies = response.headers.getSetCookie();
+			const cookieValues = setCookies.join(', ');
+			assert.ok(
+				cookieValues.includes('session=abc123'),
+				'Expected session cookie in Set-Cookie headers',
+			);
+			assert.ok(
+				cookieValues.includes('csrf=token456'),
+				'Expected csrf cookie in Set-Cookie headers',
+			);
+			assert.ok(
+				cookieValues.includes('auth_attempt=failed'),
+				'Expected auth_attempt cookie in Set-Cookie headers',
+			);
+		});
+	});
+
 	describe('middleware with custom headers', () => {
 		it('should correctly set custom headers in middleware', async () => {
 			const onRequest = async (_ctx, next) => {