From 4d585464f39ca6d0db9b5a207de957d745015b10 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Fri, 8 May 2026 23:17:47 -0400 Subject: [PATCH] fix(server): include Origin in CORS preflight Vary header (#26445) --- .../instance/httpapi/middleware/cors-vary.ts | 29 +++++++ .../server/routes/instance/httpapi/server.ts | 2 + .../test/server/httpapi-cors-vary.test.ts | 82 +++++++++++++++++++ 3 files changed, 113 insertions(+) create mode 100644 packages/opencode/src/server/routes/instance/httpapi/middleware/cors-vary.ts create mode 100644 packages/opencode/test/server/httpapi-cors-vary.test.ts diff --git a/packages/opencode/src/server/routes/instance/httpapi/middleware/cors-vary.ts b/packages/opencode/src/server/routes/instance/httpapi/middleware/cors-vary.ts new file mode 100644 index 0000000000..add533560c --- /dev/null +++ b/packages/opencode/src/server/routes/instance/httpapi/middleware/cors-vary.ts @@ -0,0 +1,29 @@ +import { Effect } from "effect" +import { HttpRouter, HttpServerResponse } from "effect/unstable/http" + +// effect-smol's HttpMiddleware.cors builds OPTIONS preflight responses by +// spreading allowOrigin() and allowHeaders() into the same record. Both set +// the `vary` key, so allowHeaders' `Vary: Access-Control-Request-Headers` +// overwrites allowOrigin's `Vary: Origin`. With dynamic origin echoing, the +// missing `Vary: Origin` lets shared caches reuse a preflight cached for one +// origin against a different origin. +// +// TODO: upstream a fix that merges Vary values in headersFromRequestOptions +// (packages/effect/src/unstable/http/HttpMiddleware.ts ~line 332). +export const corsVaryFix = HttpRouter.middleware( + (effect) => + Effect.gen(function* () { + const response = yield* effect + const allowOrigin = response.headers["access-control-allow-origin"] + if (!allowOrigin || allowOrigin === "*") return response + + const vary = response.headers["vary"] + if (!vary) return HttpServerResponse.setHeader(response, "vary", "Origin") + + const tokens = vary.split(",").map((s) => s.trim().toLowerCase()) + if (tokens.includes("origin") || tokens.includes("*")) return response + + return HttpServerResponse.setHeader(response, "vary", `${vary}, Origin`) + }), + { global: true }, +) diff --git a/packages/opencode/src/server/routes/instance/httpapi/server.ts b/packages/opencode/src/server/routes/instance/httpapi/server.ts index fd7c3ec110..696fcc4c29 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/server.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/server.ts @@ -82,6 +82,7 @@ import { disposeMiddleware } from "./lifecycle" import { memoMap } from "@opencode-ai/core/effect/memo-map" import * as ServerBackend from "@/server/backend" import { compressionLayer } from "./middleware/compression" +import { corsVaryFix } from "./middleware/cors-vary" import { errorLayer } from "./middleware/error" import { fenceLayer } from "./middleware/fence" @@ -176,6 +177,7 @@ export function createRoutes(corsOptions?: CorsOptions) { Layer.provide([ errorLayer, compressionLayer, + corsVaryFix, fenceLayer, cors(corsOptions), runtime, diff --git a/packages/opencode/test/server/httpapi-cors-vary.test.ts b/packages/opencode/test/server/httpapi-cors-vary.test.ts new file mode 100644 index 0000000000..edec8e9f76 --- /dev/null +++ b/packages/opencode/test/server/httpapi-cors-vary.test.ts @@ -0,0 +1,82 @@ +import { afterEach, describe, expect, test } from "bun:test" +import { Flag } from "@opencode-ai/core/flag/flag" +import * as Log from "@opencode-ai/core/util/log" +import { Server } from "../../src/server/server" +import { resetDatabase } from "../fixture/db" +import { disposeAllInstances } from "../fixture/fixture" + +void Log.init({ print: false }) + +const original = Flag.OPENCODE_EXPERIMENTAL_HTTPAPI + +afterEach(async () => { + Flag.OPENCODE_EXPERIMENTAL_HTTPAPI = original + await disposeAllInstances() + await resetDatabase() +}) + +function app(experimental: boolean) { + Flag.OPENCODE_EXPERIMENTAL_HTTPAPI = experimental + return experimental ? Server.Default().app : Server.Legacy().app +} + +const PREFLIGHT_HEADERS = { + origin: "http://localhost:3000", + "access-control-request-method": "POST", + "access-control-request-headers": "content-type, x-opencode-directory", +} + +// effect-smol's HttpMiddleware.cors overwrites `Vary: Origin` with +// `Vary: Access-Control-Request-Headers` on OPTIONS preflight responses +// (the two share the same record key during the spread). With dynamic +// origin echoing, missing Vary: Origin lets shared caches serve a preflight +// cached for one origin against a different origin. corsVaryFixLayer +// restores the merged form. +describe("CORS preflight Vary header", () => { + test("Hono backend preflight Vary contains Origin", async () => { + const response = await app(false).request("/global/config", { + method: "OPTIONS", + headers: PREFLIGHT_HEADERS, + }) + + expect([200, 204]).toContain(response.status) + expect(response.headers.get("access-control-allow-origin")).toBe("http://localhost:3000") + expect((response.headers.get("vary") ?? "").toLowerCase()).toContain("origin") + }) + + test("HTTP API backend preflight Vary contains Origin", async () => { + const response = await app(true).request("/global/config", { + method: "OPTIONS", + headers: PREFLIGHT_HEADERS, + }) + + expect([200, 204]).toContain(response.status) + expect(response.headers.get("access-control-allow-origin")).toBe("http://localhost:3000") + expect((response.headers.get("vary") ?? "").toLowerCase()).toContain("origin") + }) + + test("HTTP API backend preflight Vary still preserves Access-Control-Request-Headers", async () => { + const response = await app(true).request("/global/config", { + method: "OPTIONS", + headers: PREFLIGHT_HEADERS, + }) + + const vary = (response.headers.get("vary") ?? "").toLowerCase() + expect(vary).toContain("origin") + expect(vary).toContain("access-control-request-headers") + }) + + test("HTTP API backend does not duplicate Origin in Vary", async () => { + const response = await app(true).request("/global/config", { + method: "OPTIONS", + headers: PREFLIGHT_HEADERS, + }) + + const vary = response.headers.get("vary") ?? "" + const originCount = vary + .split(",") + .map((s) => s.trim().toLowerCase()) + .filter((s) => s === "origin").length + expect(originCount).toBe(1) + }) +})