diff --git a/docs/TODO.md b/docs/TODO.md index a2acafa..a05b670 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -64,8 +64,8 @@ Do these in order. Each task is a thin end-to-end slice: test first, implement, - [x] Tests: use a stub limiter binding to check both branches and the disabled flag ## 9. CORS -- [ ] Hono `cors()` driven by `CORS_ORIGINS` (`*` or an allowlist); methods, allowed headers and exposed headers as in api.md -- [ ] Tests: preflight on `/bins` and `/bins/:id`; a disallowed origin gets no ACAO header; `X-Bin-*` headers are exposed +- [x] Hono `cors()` driven by `CORS_ORIGINS` (`*` or an allowlist); methods, allowed headers and exposed headers as in api.md +- [x] Tests: preflight on `/bins` and `/bins/:id`; a disallowed origin gets no ACAO header; `X-Bin-*` headers are exposed ## 10. OpenAPI - [ ] Every route is defined with `createRoute` (request headers, bodies and all error responses described) diff --git a/docs/api.md b/docs/api.md index 45b5841..65ccbb7 100644 --- a/docs/api.md +++ b/docs/api.md @@ -74,7 +74,7 @@ There is no listing endpoint (ADR 0002), and no PATCH, ETag/If-Match, version hi | `X-Bin-Updated-At` | ISO timestamp | | `X-Bin-Expires-At` | ISO timestamp. Left out when the Bin has no Expiry. | -Browsers can read all of these via `Access-Control-Expose-Headers`, along with `Location`. +Browsers can read all of these via `Access-Control-Expose-Headers`, along with `Location` and `Retry-After` (see [CORS](#cors)). ## Validation and errors @@ -122,7 +122,15 @@ Reads have no limit in the MVP. Setting `RATE_LIMIT_ENABLED=false` turns both of ## CORS -`CORS_ORIGINS` is `*` (the default) or a comma-separated allowlist. Allowed methods: `GET, POST, PUT, DELETE, OPTIONS`. Allowed headers: `Authorization, Content-Type, X-Bin-Private, X-Bin-TTL`. The docs must warn that a Write Key embedded in frontend code gives every visitor write access. +`CORS_ORIGINS` is `*` (the default) or a comma-separated allowlist. Every response, errors included, carries the CORS headers. + +- `*` → `Access-Control-Allow-Origin: *`. An allowlist → the request's `Origin` is echoed back only if it exactly matches an entry; otherwise no `Access-Control-Allow-Origin` is sent. Responses send `Vary: Origin`. +- Allowed methods: `GET, POST, PUT, DELETE, OPTIONS`. Allowed headers: `Authorization, Content-Type, X-Bin-Private, X-Bin-TTL`. +- Exposed headers: `X-Bin-Id, X-Bin-Private, X-Bin-Created-At, X-Bin-Updated-At, X-Bin-Expires-At, Location, Retry-After`. `Retry-After` isn't CORS-safelisted, so without it a browser couldn't read how long to wait after a `429`. +- No `Access-Control-Allow-Credentials`: keys are sent in the `Authorization` header, not in cookies. +- Preflight `OPTIONS` requests get `204` with the allow headers. They aren't rate limited and don't touch D1. + +The docs must warn that a Write Key embedded in frontend code gives every visitor write access. ## Instance configuration (`vars`) diff --git a/src/bins/routes.ts b/src/bins/routes.ts index 5d2a378..9513bd2 100644 --- a/src/bins/routes.ts +++ b/src/bins/routes.ts @@ -1,5 +1,5 @@ import { OpenAPIHono } from "@hono/zod-openapi"; -import { type AppEnv, withConfig } from "../context"; +import type { AppEnv } from "../context"; import { ApiError } from "../errors"; import { requireKey } from "./auth"; import { parseBinBody } from "./body"; @@ -11,8 +11,6 @@ import { type Bin, findLive, insert, remove, replaceContent } from "./repository export const binRoutes = new OpenAPIHono(); -binRoutes.use(withConfig); - // Rate limiting comes first, before the id, auth and body checks (docs/api.md → Validation and errors): it protects // the Worker, so a limited client shouldn't get D1 lookups, key hashing or body parsing. Reads aren't limited. binRoutes.post("/", rateLimit("CREATE_LIMITER")); diff --git a/src/cors.ts b/src/cors.ts new file mode 100644 index 0000000..1bda9ff --- /dev/null +++ b/src/cors.ts @@ -0,0 +1,37 @@ +import type { Context } from "hono"; +import { cors } from "hono/cors"; +import type { AppEnv } from "./context"; + +/** The request headers a browser may send (docs/api.md → CORS). */ +export const CORS_ALLOWED_HEADERS = ["Authorization", "Content-Type", "X-Bin-Private", "X-Bin-TTL"]; + +/** + * The response headers a browser may read (docs/api.md → CORS): the `X-Bin-*` metadata, `Location` on create, and + * `Retry-After` on `429`, none of which are CORS-safelisted. + */ +export const CORS_EXPOSED_HEADERS = [ + "X-Bin-Id", + "X-Bin-Private", + "X-Bin-Created-At", + "X-Bin-Updated-At", + "X-Bin-Expires-At", + "Location", + "Retry-After", +]; + +/** + * CORS for every route, driven by `CORS_ORIGINS` (docs/api.md → CORS). Needs `withConfig` first. `*` allows any + * origin; an allowlist echoes the request's `Origin` only on an exact match and otherwise sends no + * `Access-Control-Allow-Origin`. Preflights are answered here with `204`, before rate limiting and D1. No + * credentials: keys travel in the `Authorization` header, not in cookies. + */ +export const withCors = cors({ + origin: (origin: string, c: Context) => { + const allowed = c.var.config.corsOrigins; + if (allowed === "*") return "*"; + return allowed.includes(origin) ? origin : null; + }, + allowMethods: ["GET", "POST", "PUT", "DELETE", "OPTIONS"], + allowHeaders: CORS_ALLOWED_HEADERS, + exposeHeaders: CORS_EXPOSED_HEADERS, +}); diff --git a/src/index.ts b/src/index.ts index ba66c8f..5c9b414 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,7 +1,8 @@ import { OpenAPIHono } from "@hono/zod-openapi"; import { sweepExpired } from "./bins/expiry"; import { binRoutes } from "./bins/routes"; -import type { AppEnv } from "./context"; +import { type AppEnv, withConfig } from "./context"; +import { withCors } from "./cors"; import { ApiError, errorResponse } from "./errors"; export const app = new OpenAPIHono(); @@ -12,6 +13,11 @@ app.use(async (c, next) => { c.header("Cache-Control", "no-store"); }); +// Before the routes, so every response carries the CORS headers, errors from notFound and onError included, and +// preflights are answered before any route-level middleware such as the rate limiter. +app.use(withConfig); +app.use(withCors); + app.get("/health", (c) => c.json({ status: "ok" })); app.route("/bins", binRoutes); diff --git a/test/cors.test.ts b/test/cors.test.ts new file mode 100644 index 0000000..ad4624d --- /dev/null +++ b/test/cors.test.ts @@ -0,0 +1,256 @@ +import { createExecutionContext, waitOnExecutionContext } from "cloudflare:test"; +import { env } from "cloudflare:workers"; +import { beforeEach, describe, expect, it } from "vitest"; +import { generateBinId } from "../src/bins/keys"; +import { RATE_LIMIT_PERIOD_SECONDS } from "../src/bins/rate-limit"; +import { app } from "../src/index"; + +const BASE_URL = "https://keepjson.test"; +const ALLOWED_ORIGIN = "https://b.example"; +const OTHER_ORIGIN = "https://evil.example"; +const ALLOWLIST = "https://a.example, https://b.example"; + +const ALLOWED_METHODS = ["GET", "POST", "PUT", "DELETE", "OPTIONS"]; +const ALLOWED_HEADERS = ["Authorization", "Content-Type", "X-Bin-Private", "X-Bin-TTL"]; +const EXPOSED_HEADERS = [ + "X-Bin-Id", + "X-Bin-Private", + "X-Bin-Created-At", + "X-Bin-Updated-At", + "X-Bin-Expires-At", + "Location", + "Retry-After", +]; + +/** A limiter binding that denies every call. */ +const denyingLimiter: RateLimit = { + async limit() { + return { success: false }; + }, +}; + +/** + * Calls the app directly, so `CORS_ORIGINS` and the limiter bindings can be overridden per test. With no + * `corsOrigins`, the test worker's own value (`*`) applies. + */ +async function send( + path: string, + init: RequestInit = {}, + options: { corsOrigins?: string; rateLimited?: boolean } = {}, +): Promise { + const testEnv: Env = { ...env }; + if (options.corsOrigins !== undefined) testEnv.CORS_ORIGINS = options.corsOrigins; + if (options.rateLimited) { + testEnv.RATE_LIMIT_ENABLED = "true"; + testEnv.CREATE_LIMITER = denyingLimiter; + testEnv.WRITE_LIMITER = denyingLimiter; + } + const ctx = createExecutionContext(); + const res = await app.fetch(new Request(`${BASE_URL}${path}`, init), testEnv, ctx); + await waitOnExecutionContext(ctx); + return res; +} + +function preflight(path: string, origin: string, method: string, options?: { corsOrigins?: string }) { + return send( + path, + { + method: "OPTIONS", + headers: { + Origin: origin, + "Access-Control-Request-Method": method, + "Access-Control-Request-Headers": "authorization, content-type", + }, + }, + options, + ); +} + +function postBin(origin: string, options?: { corsOrigins?: string; rateLimited?: boolean }) { + return send( + "/bins", + { method: "POST", headers: { Origin: origin, "Content-Type": "application/json" }, body: '{"keep": true}' }, + options, + ); +} + +/** The comma-separated values of a header, trimmed and lowercased (header names are case-insensitive). */ +function listHeader(res: Response, name: string): string[] { + return (res.headers.get(name) ?? "") + .split(",") + .map((value) => value.trim().toLowerCase()) + .filter(Boolean); +} + +function expectExposedHeaders(res: Response) { + const exposed = listHeader(res, "Access-Control-Expose-Headers"); + for (const header of EXPOSED_HEADERS) expect(exposed).toContain(header.toLowerCase()); +} + +function expectCommonHeaders(res: Response) { + expect(res.headers.has("Access-Control-Allow-Credentials")).toBe(false); + expect(res.headers.get("Cache-Control")).toBe("no-store"); +} + +async function countBins(): Promise { + const row = await env.DB.prepare("SELECT COUNT(*) AS n FROM bins").first<{ n: number }>(); + return row?.n ?? 0; +} + +beforeEach(async () => { + await env.DB.exec("DELETE FROM bins"); +}); + +describe("CORS with CORS_ORIGINS=*", () => { + it("runs with the default CORS_ORIGINS", () => { + expect(env.CORS_ORIGINS).toBe("*"); + }); + + it.each([ + ["/bins", "POST"], + ["/bins/V1StGXR8_Z5jdHi6B-myT", "PUT"], + ["/bins/V1StGXR8_Z5jdHi6B-myT", "DELETE"], + ["/bins/V1StGXR8_Z5jdHi6B-myT", "GET"], + ])("answers a preflight on %s (%s) with 204 and the allow headers", async (path, method) => { + const res = await preflight(path, OTHER_ORIGIN, method); + expect(res.status).toBe(204); + expect(res.headers.get("Access-Control-Allow-Origin")).toBe("*"); + const methods = listHeader(res, "Access-Control-Allow-Methods"); + for (const allowed of ALLOWED_METHODS) expect(methods).toContain(allowed.toLowerCase()); + const headers = listHeader(res, "Access-Control-Allow-Headers"); + for (const allowed of ALLOWED_HEADERS) expect(headers).toContain(allowed.toLowerCase()); + expectCommonHeaders(res); + expect(await res.text()).toBe(""); + }); + + it("answers preflights without rate limiting them or touching the Bins", async () => { + const res = await preflight("/bins", OTHER_ORIGIN, "POST", undefined); + expect(res.status).toBe(204); + const limited = await send( + "/bins", + { method: "OPTIONS", headers: { Origin: OTHER_ORIGIN, "Access-Control-Request-Method": "POST" } }, + { rateLimited: true }, + ); + expect(limited.status).toBe(204); + expect(limited.headers.has("Retry-After")).toBe(false); + expect(await countBins()).toBe(0); + }); + + it("sends ACAO * and exposes the metadata headers on POST and GET", async () => { + const created = await postBin(OTHER_ORIGIN); + expect(created.status).toBe(201); + expect(created.headers.get("Access-Control-Allow-Origin")).toBe("*"); + expectExposedHeaders(created); + expectCommonHeaders(created); + const { id } = (await created.json()) as { id: string }; + + const read = await send(`/bins/${id}`, { headers: { Origin: OTHER_ORIGIN } }); + expect(read.status).toBe(200); + expect(read.headers.get("Access-Control-Allow-Origin")).toBe("*"); + expectExposedHeaders(read); + expectCommonHeaders(read); + }); + + it("sends the CORS headers on GET /health", async () => { + const res = await send("/health", { headers: { Origin: OTHER_ORIGIN } }); + expect(res.status).toBe(200); + expect(res.headers.get("Access-Control-Allow-Origin")).toBe("*"); + expectCommonHeaders(res); + }); + + it("sends the CORS headers on 404 responses", async () => { + for (const path of [`/bins/${generateBinId()}`, "/bins/not-an-id", "/nowhere"]) { + const res = await send(path, { headers: { Origin: OTHER_ORIGIN } }); + expect(res.status).toBe(404); + expect(res.headers.get("Access-Control-Allow-Origin")).toBe("*"); + expectExposedHeaders(res); + expectCommonHeaders(res); + } + }); + + it("sends the CORS headers on 401 responses", async () => { + const created = await send("/bins", { + method: "POST", + headers: { "Content-Type": "application/json", "X-Bin-Private": "true" }, + body: "{}", + }); + const { id } = (await created.json()) as { id: string }; + const res = await send(`/bins/${id}`, { headers: { Origin: OTHER_ORIGIN } }); + expect(res.status).toBe(401); + expect(res.headers.get("Access-Control-Allow-Origin")).toBe("*"); + expectExposedHeaders(res); + expectCommonHeaders(res); + }); + + it("sends the CORS headers, exposing Retry-After, on 429 responses", async () => { + const res = await postBin(OTHER_ORIGIN, { rateLimited: true }); + expect(res.status).toBe(429); + expect(res.headers.get("Retry-After")).toBe(String(RATE_LIMIT_PERIOD_SECONDS)); + expect(res.headers.get("Access-Control-Allow-Origin")).toBe("*"); + expectExposedHeaders(res); + expectCommonHeaders(res); + }); +}); + +describe("CORS with an allowlist", () => { + const options = { corsOrigins: ALLOWLIST }; + + it("echoes an allowed origin on a preflight", async () => { + const res = await preflight(`/bins/${generateBinId()}`, ALLOWED_ORIGIN, "PUT", options); + expect(res.status).toBe(204); + expect(res.headers.get("Access-Control-Allow-Origin")).toBe(ALLOWED_ORIGIN); + expect(listHeader(res, "Vary")).toContain("origin"); + expect(listHeader(res, "Access-Control-Allow-Methods")).toContain("put"); + expectCommonHeaders(res); + }); + + it("sends no ACAO header to a disallowed origin on a preflight", async () => { + const res = await preflight("/bins", OTHER_ORIGIN, "POST", options); + expect(res.status).toBe(204); + expect(res.headers.has("Access-Control-Allow-Origin")).toBe(false); + expect(listHeader(res, "Vary")).toContain("origin"); + expectCommonHeaders(res); + }); + + it("echoes an allowed origin on actual requests", async () => { + const created = await postBin(ALLOWED_ORIGIN, options); + expect(created.status).toBe(201); + expect(created.headers.get("Access-Control-Allow-Origin")).toBe(ALLOWED_ORIGIN); + expect(listHeader(created, "Vary")).toContain("origin"); + expectExposedHeaders(created); + expectCommonHeaders(created); + const { id } = (await created.json()) as { id: string }; + + const read = await send(`/bins/${id}`, { headers: { Origin: "https://a.example" } }, options); + expect(read.status).toBe(200); + expect(read.headers.get("Access-Control-Allow-Origin")).toBe("https://a.example"); + expect(listHeader(read, "Vary")).toContain("origin"); + }); + + it("sends no ACAO header to a disallowed origin on actual requests", async () => { + const created = await postBin(OTHER_ORIGIN, options); + expect(created.status).toBe(201); + expect(created.headers.has("Access-Control-Allow-Origin")).toBe(false); + expect(listHeader(created, "Vary")).toContain("origin"); + expectCommonHeaders(created); + const { id } = (await created.json()) as { id: string }; + + for (const origin of [OTHER_ORIGIN, "https://b.example.evil.example", "http://b.example", null]) { + const headers: Record = origin === null ? {} : { Origin: origin }; + const read = await send(`/bins/${id}`, { headers }, options); + expect(read.status).toBe(200); + expect(read.headers.has("Access-Control-Allow-Origin")).toBe(false); + } + }); + + it("echoes an allowed origin on error responses", async () => { + const notFound = await send(`/bins/${generateBinId()}`, { headers: { Origin: ALLOWED_ORIGIN } }, options); + expect(notFound.status).toBe(404); + expect(notFound.headers.get("Access-Control-Allow-Origin")).toBe(ALLOWED_ORIGIN); + + const limited = await postBin(ALLOWED_ORIGIN, { ...options, rateLimited: true }); + expect(limited.status).toBe(429); + expect(limited.headers.get("Access-Control-Allow-Origin")).toBe(ALLOWED_ORIGIN); + expectExposedHeaders(limited); + }); +});