Просмотр исходного кода

fix(server): include Origin in CORS preflight Vary header (#26445)

Kit Langton 3 месяцев назад
Родитель
Сommit
4d585464f3

+ 29 - 0
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 },
+)

+ 2 - 0
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,

+ 82 - 0
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)
+  })
+})