fix(effect): preserve Vary: Origin in CORS preflight when Access-Control-Request-Headers is present

.changeset/fix-cors-vary-merge.md

@@ -0,0 +1,5 @@
+---
+"effect": patch
+---
+
+Fix `HttpMiddleware.cors` overwriting the `Vary: Origin` header on preflight responses. When `allowedOrigins` is a predicate or has multiple entries and the preflight request includes `Access-Control-Request-Headers`, the middleware previously emitted only `Vary: Access-Control-Request-Headers`, dropping `Origin`. This could let a shared cache serve a preflight response cached for one origin to a request from a different origin. Vary entries are now merged into a single `Vary: Origin, Access-Control-Request-Headers` header.

packages/effect/src/unstable/http/HttpMiddleware.ts

@@ -313,21 +313,21 @@ export const cors = (options?: {
     ? opts.allowedOrigins
     : (origin: string) => (opts.allowedOrigins as ReadonlyArray<string>).includes(origin)
 
+  const originVaries = typeof opts.allowedOrigins === "function" || opts.allowedOrigins.length > 0
+
   const allowOrigin = typeof opts.allowedOrigins === "function" || opts.allowedOrigins.length > 1
     ? ((originHeader: string) => {
       if (!isAllowedOrigin(originHeader)) return undefined
       return {
-        "access-control-allow-origin": originHeader,
-        vary: "Origin"
+        "access-control-allow-origin": originHeader
       }
     })
     : opts.allowedOrigins.length === 0
     ? constant({
       "access-control-allow-origin": "*"
     })
     : constant({
-      "access-control-allow-origin": opts.allowedOrigins[0],
-      vary: "Origin"
+      "access-control-allow-origin": opts.allowedOrigins[0]
     })
 
   const allowMethods = opts.allowedMethods.length > 0
@@ -343,7 +343,6 @@ export const cors = (options?: {
   ): ReadonlyRecord<string, string> | undefined => {
     if (opts.allowedHeaders.length === 0 && accessControlRequestHeaders) {
       return {
-        vary: "Access-Control-Request-Headers",
         "access-control-allow-headers": accessControlRequestHeaders
       }
     }
@@ -367,23 +366,35 @@ export const cors = (options?: {
 
   const headersFromRequest = (request: HttpServerRequest) => {
     const origin = request.headers["origin"]
+    const allowOriginHeaders = allowOrigin(origin)
     return Headers.fromRecordUnsafe({
-      ...allowOrigin(origin),
+      ...allowOriginHeaders,
       ...allowCredentials,
-      ...exposeHeaders
+      ...exposeHeaders,
+      ...(originVaries ? { vary: "Origin" } : undefined)
     })
   }
 
   const headersFromRequestOptions = (request: HttpServerRequest) => {
     const origin = request.headers["origin"]
     const accessControlRequestHeaders = request.headers["access-control-request-headers"]
+    const allowOriginHeaders = allowOrigin(origin)
+    const allowHeadersHeaders = allowHeaders(accessControlRequestHeaders)
+    const varyParts: Array<string> = []
+    if (originVaries) {
+      varyParts.push("Origin")
+    }
+    if (opts.allowedHeaders.length === 0 && accessControlRequestHeaders) {
+      varyParts.push("Access-Control-Request-Headers")
+    }
     return Headers.fromRecordUnsafe({
-      ...allowOrigin(origin),
+      ...allowOriginHeaders,
       ...allowCredentials,
       ...exposeHeaders,
       ...allowMethods,
-      ...allowHeaders(accessControlRequestHeaders),
-      ...maxAge
+      ...allowHeadersHeaders,
+      ...maxAge,
+      ...(varyParts.length > 0 ? { vary: varyParts.join(", ") } : undefined)
     })
   }
 

packages/effect/test/unstable/http/HttpMiddleware.test.ts

@@ -2,6 +2,7 @@ import { assert, describe, it } from "@effect/vitest"
 import * as Effect from "effect/Effect"
 import * as Logger from "effect/Logger"
 import * as References from "effect/References"
+import * as HttpEffect from "effect/unstable/http/HttpEffect"
 import * as HttpMiddleware from "effect/unstable/http/HttpMiddleware"
 import * as HttpServerRequest from "effect/unstable/http/HttpServerRequest"
 import * as HttpServerResponse from "effect/unstable/http/HttpServerResponse"
@@ -54,4 +55,120 @@ describe("HttpMiddleware", () => {
         assert.deepStrictEqual(spans, [["http.span"], ["http.span"]])
       }))
   })
+
+  describe("cors", () => {
+    it.effect("preflight Vary header includes both Origin and Access-Control-Request-Headers when origin is dynamic", () =>
+      Effect.gen(function*() {
+        const request = HttpServerRequest.fromWeb(
+          new Request("http://api.example.com/resource", {
+            method: "OPTIONS",
+            headers: {
+              "origin": "https://app-a.example.com",
+              "access-control-request-method": "POST",
+              "access-control-request-headers": "content-type"
+            }
+          })
+        )
+
+        const response = yield* HttpMiddleware.cors({
+          allowedOrigins: ["https://app-a.example.com", "https://app-b.example.com"]
+        })(Effect.succeed(HttpServerResponse.empty({ status: 200 }))).pipe(
+          Effect.provideService(HttpServerRequest.HttpServerRequest, request)
+        )
+
+        assert.strictEqual(response.headers["access-control-allow-origin"], "https://app-a.example.com")
+        // Bug: vary is just "Access-Control-Request-Headers" — the "Origin" entry
+        // set by allowOrigin is overwritten by the spread from allowHeaders.
+        // Without `Vary: Origin`, a shared cache may serve a preflight cached
+        // for app-a.example.com to a request from app-b.example.com.
+        const vary = response.headers["vary"] ?? ""
+        assert.include(vary, "Origin", `expected Vary to include "Origin", got: ${vary}`)
+        assert.include(
+          vary,
+          "Access-Control-Request-Headers",
+          `expected Vary to include "Access-Control-Request-Headers", got: ${vary}`
+        )
+      }))
+
+    it.effect("preflight Vary is just Origin when allowedHeaders is configured statically", () =>
+      Effect.gen(function*() {
+        const request = HttpServerRequest.fromWeb(
+          new Request("http://api.example.com/resource", {
+            method: "OPTIONS",
+            headers: {
+              "origin": "https://app-a.example.com",
+              "access-control-request-method": "POST",
+              "access-control-request-headers": "content-type"
+            }
+          })
+        )
+
+        const response = yield* HttpMiddleware.cors({
+          allowedOrigins: ["https://app-a.example.com", "https://app-b.example.com"],
+          allowedHeaders: ["content-type", "authorization"]
+        })(Effect.succeed(HttpServerResponse.empty({ status: 200 }))).pipe(
+          Effect.provideService(HttpServerRequest.HttpServerRequest, request)
+        )
+
+        assert.strictEqual(response.headers["vary"], "Origin")
+      }))
+
+    it.effect("preflight Vary includes Origin when a dynamic origin is rejected", () =>
+      Effect.gen(function*() {
+        const request = HttpServerRequest.fromWeb(
+          new Request("http://api.example.com/resource", {
+            method: "OPTIONS",
+            headers: {
+              "origin": "https://denied.example.com",
+              "access-control-request-method": "POST",
+              "access-control-request-headers": "content-type"
+            }
+          })
+        )
+
+        const response = yield* HttpMiddleware.cors({
+          allowedOrigins: ["https://app-a.example.com", "https://app-b.example.com"]
+        })(Effect.succeed(HttpServerResponse.empty({ status: 200 }))).pipe(
+          Effect.provideService(HttpServerRequest.HttpServerRequest, request)
+        )
+
+        assert.strictEqual(response.headers["access-control-allow-origin"], undefined)
+        assert.strictEqual(response.headers["vary"], "Origin, Access-Control-Request-Headers")
+      }))
+
+    it("response Vary includes Origin when a dynamic origin is rejected", async () => {
+      const response = await HttpEffect.toWebHandler(
+        HttpMiddleware.cors({
+          allowedOrigins: (origin) => origin === "https://app-a.example.com"
+        })(Effect.succeed(HttpServerResponse.empty({ status: 200 })))
+      )(
+        new Request("http://api.example.com/resource", {
+          headers: { origin: "https://denied.example.com" }
+        })
+      )
+
+      assert.strictEqual(response.headers.get("access-control-allow-origin"), null)
+      assert.strictEqual(response.headers.get("vary"), "Origin")
+    })
+
+    it.effect("preflight has no Vary header for wildcard origin", () =>
+      Effect.gen(function*() {
+        const request = HttpServerRequest.fromWeb(
+          new Request("http://api.example.com/resource", {
+            method: "OPTIONS",
+            headers: {
+              "origin": "https://app-a.example.com",
+              "access-control-request-method": "POST"
+            }
+          })
+        )
+
+        const response = yield* HttpMiddleware.cors()(
+          Effect.succeed(HttpServerResponse.empty({ status: 200 }))
+        ).pipe(Effect.provideService(HttpServerRequest.HttpServerRequest, request))
+
+        assert.strictEqual(response.headers["access-control-allow-origin"], "*")
+        assert.strictEqual(response.headers["vary"], undefined)
+      }))
+  })
 })