From 4310c35eaa8b2b093f75d70f0fd83fd97f7c74d9 Mon Sep 17 00:00:00 2001 From: Ronaldo Martins Date: Mon, 10 Aug 2026 23:58:54 -0300 Subject: [PATCH] fix(api): strip stale Access-Control-Allow-Origin before setting in addCorsHeaders Follow-up to #427. addCorsHeaders re-applied corsHeadersFor via Headers.set, but when the evaluated origin is not allowed no ACAO is computed, so a stale/upstream Access-Control-Allow-Origin (and Allow-Credentials) survived on the response and could leak. Delete both CORS headers before re-applying, guaranteeing at most one correct value and no ACAO for disallowed origins. --- packages/api/src/core/cors.test.ts | 43 ++++++++++++++++++++++++++++++ packages/api/src/core/cors.ts | 9 ++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/packages/api/src/core/cors.test.ts b/packages/api/src/core/cors.test.ts index 3bca97c..505ec31 100644 --- a/packages/api/src/core/cors.test.ts +++ b/packages/api/src/core/cors.test.ts @@ -97,5 +97,48 @@ describe("ENG-1666 CORS conformance", () => { ); expect(res.headers.get("Access-Control-Allow-Origin")).toBeNull(); }); + + it("overwrites a stale Allow-Origin with the correct value (no duplicates)", () => { + process.env.ALLOWED_ORIGINS = "https://app.example.com"; + const upstream = new Response(JSON.stringify({ ok: true }), { + headers: { + "Content-Type": "application/json", + "Access-Control-Allow-Origin": "https://stale.example.com", + }, + }); + const res = addCorsHeaders(upstream, makeReq("https://app.example.com")); + // Only the correct value, emitted exactly once. + expect(res.headers.get("Access-Control-Allow-Origin")).toBe( + "https://app.example.com", + ); + }); + + it("strips a stale Allow-Origin when the origin is not allowed", () => { + process.env.ALLOWED_ORIGINS = "https://app.example.com"; + const upstream = new Response(JSON.stringify({ ok: true }), { + headers: { + "Content-Type": "application/json", + "Access-Control-Allow-Origin": "https://stale.example.com", + }, + }); + const res = addCorsHeaders(upstream, makeReq("https://evil.example.com")); + expect(res.headers.get("Access-Control-Allow-Origin")).toBeNull(); + }); + + it("strips a stale Allow-Credentials header", () => { + process.env.ALLOWED_ORIGINS = "https://app.example.com"; + const upstream = new Response(JSON.stringify({ ok: true }), { + headers: { + "Content-Type": "application/json", + "Access-Control-Allow-Origin": "https://stale.example.com", + "Access-Control-Allow-Credentials": "true", + }, + }); + const res = addCorsHeaders(upstream, makeReq("https://evil.example.com")); + expect(res.headers.get("Access-Control-Allow-Origin")).toBeNull(); + expect( + res.headers.get("Access-Control-Allow-Credentials"), + ).toBeNull(); + }); }); }); diff --git a/packages/api/src/core/cors.ts b/packages/api/src/core/cors.ts index ba69913..6ebd0e4 100644 --- a/packages/api/src/core/cors.ts +++ b/packages/api/src/core/cors.ts @@ -34,7 +34,14 @@ export function corsHeadersFor(req: Request): Record { export function addCorsHeaders(response: Response, req: Request): Response { const newHeaders = new Headers(response.headers); - for (const [key, value] of Object.entries(corsHeadersFor(req))) { + // Remove any pre-existing/stale CORS headers so an upstream + // Access-Control-Allow-Origin can never leak or be emitted twice. + // corsHeadersFor is re-applied below with the values for the evaluated origin; + // when the origin is not allowed, ACAO stays absent (secure by default). + newHeaders.delete("Access-Control-Allow-Origin"); + newHeaders.delete("Access-Control-Allow-Credentials"); + const corsHeaders = corsHeadersFor(req); + for (const [key, value] of Object.entries(corsHeaders)) { newHeaders.set(key, value); } return new Response(response.body, {