Skip to content

Commit 7a4091f

Browse files
authored
fix(api): credentialed CORS for listing claim routes (#2206)
Session-cookie browser clients (claim, promote) were hitting the public wildcard CORS layer (ACAO: *), which browsers reject with credentials: include — claims failed as "Failed to fetch". Carve those paths into authCorsMiddleware, derive the public-cors skip from the mount list so there is a single source of truth, and add a web-source drift test so the next session surface cannot ship without the carve-out. Follow-up for an origin-based single middleware: #2205
1 parent dfb2048 commit 7a4091f

3 files changed

Lines changed: 228 additions & 63 deletions

File tree

tests/api/user-api-keys-cors.test.ts

Lines changed: 154 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,39 @@
11
import { describe, it, expect } from "bun:test";
22
import { Hono } from "hono";
33
import { cors } from "hono/cors";
4-
import { authCorsMiddleware } from "../../workers/api/src/auth/index.js";
4+
import {
5+
authCorsMiddleware,
6+
CREDENTIALED_CORS_MOUNT_PATHS,
7+
isCredentialedCorsPath,
8+
} from "../../workers/api/src/auth/index.js";
59

610
// Mirrors the index.ts CORS wiring: authCorsMiddleware owns credentialed CORS on
7-
// /api/auth/*, /v1/api-keys/*, /v1/me/*, and /v1/workspaces/*; the wildcard public
8-
// cors() runs on every OTHER path. Without the carve-out the wildcard cors overwrites
9-
// the credentialed Access-Control-Allow-Origin on the actual response — which a
10-
// browser rejects for `credentials: "include"` requests (shows as "Failed to fetch").
11+
// every CREDENTIALED_CORS_MOUNT_PATHS entry; the wildcard public cors() runs on
12+
// every OTHER path (guarded by isCredentialedCorsPath). Without the carve-out the
13+
// wildcard cors overwrites the credentialed Access-Control-Allow-Origin on the
14+
// actual response — which a browser rejects for `credentials: "include"` requests
15+
// (shows as "Failed to fetch" / CORS blocked).
1116
function makeApp() {
1217
const app = new Hono();
13-
app.use("/api/auth/*", authCorsMiddleware());
14-
app.use("/v1/api-keys", authCorsMiddleware());
15-
app.use("/v1/api-keys/*", authCorsMiddleware());
16-
app.use("/v1/me/*", authCorsMiddleware());
17-
app.use("/v1/workspaces", authCorsMiddleware());
18-
app.use("/v1/workspaces/*", authCorsMiddleware());
18+
const credentialedCors = authCorsMiddleware();
19+
for (const path of CREDENTIALED_CORS_MOUNT_PATHS) {
20+
app.use(path, credentialedCors);
21+
}
1922
const publicReadCors = cors();
2023
app.use("*", (c, next) =>
21-
c.req.path.startsWith("/api/auth/") ||
22-
c.req.path === "/v1/api-keys" ||
23-
c.req.path.startsWith("/v1/api-keys/") ||
24-
c.req.path.startsWith("/v1/me/") ||
25-
c.req.path === "/v1/workspaces" ||
26-
c.req.path.startsWith("/v1/workspaces/")
27-
? next()
28-
: publicReadCors(c, next),
24+
isCredentialedCorsPath(c.req.path) ? next() : publicReadCors(c, next),
2925
);
3026
app.get("/v1/api-keys", (c) => c.json({ apiKeys: [] }));
3127
app.post("/v1/me/avatar", (c) => c.json({ avatarUrl: "https://media.test/u.png" }));
3228
app.post("/v1/workspaces/:workspaceId/avatar", (c) =>
3329
c.json({ avatarUrl: "https://media.test/w.png" }),
3430
);
31+
app.post("/v1/listing/claim", (c) => c.json({ id: "clm_test" }));
32+
app.post("/v1/listing/claim/verify", (c) => c.json({ verified: true }));
33+
app.get("/v1/listing/claims", (c) => c.json({ claims: [] }));
34+
app.post("/v1/listing/promote", (c) => c.json({ promoted: true }));
35+
// Anonymous public-write listing routes stay on wildcard CORS.
36+
app.post("/v1/listing/validate", (c) => c.json({ ok: true }));
3537
app.get("/v1/orgs", (c) => c.json({ ok: true }));
3638
return app;
3739
}
@@ -104,6 +106,60 @@ describe("session-authed credentialed CORS", () => {
104106
expect(res.headers.get("access-control-allow-origin")).toBe("*");
105107
});
106108

109+
it("keeps wildcard CORS on anonymous listing validate (not session-authed)", async () => {
110+
const res = await makeApp().request(
111+
"/v1/listing/validate",
112+
{
113+
method: "POST",
114+
headers: { Origin: "https://anything.example" },
115+
},
116+
{ ENVIRONMENT: "production" } as never,
117+
);
118+
expect(res.headers.get("access-control-allow-origin")).toBe("*");
119+
expect(res.headers.get("access-control-allow-credentials")).toBeNull();
120+
});
121+
122+
it("reflects the origin with credentials on listing claim/claims/promote", async () => {
123+
const app = makeApp();
124+
for (const path of [
125+
"/v1/listing/claim",
126+
"/v1/listing/claim/verify",
127+
"/v1/listing/claims",
128+
"/v1/listing/promote",
129+
]) {
130+
const res = await app.request(
131+
path,
132+
{
133+
method: path === "/v1/listing/claims" ? "GET" : "POST",
134+
headers: { Origin: "https://releases.sh" },
135+
},
136+
{ ENVIRONMENT: "production" } as never,
137+
);
138+
expect(res.headers.get("access-control-allow-origin")).toBe("https://releases.sh");
139+
expect(res.headers.get("access-control-allow-credentials")).toBe("true");
140+
}
141+
});
142+
143+
it("allows listing claim preflight with content-type (start claim body)", async () => {
144+
const res = await makeApp().request(
145+
"/v1/listing/claim",
146+
{
147+
method: "OPTIONS",
148+
headers: {
149+
Origin: "https://releases.sh",
150+
"Access-Control-Request-Method": "POST",
151+
"Access-Control-Request-Headers": "content-type",
152+
},
153+
},
154+
{ ENVIRONMENT: "production" } as never,
155+
);
156+
expect(res.headers.get("access-control-allow-origin")).toBe("https://releases.sh");
157+
expect(res.headers.get("access-control-allow-credentials")).toBe("true");
158+
expect(res.headers.get("access-control-allow-headers")?.toLowerCase()).toContain(
159+
"content-type",
160+
);
161+
});
162+
107163
it("wildcard CORS on a mistaken double-/v1 path breaks credentialed uploads", async () => {
108164
const res = await makeApp().request(
109165
"/v1/v1/workspaces/org_abc/avatar",
@@ -117,3 +173,82 @@ describe("session-authed credentialed CORS", () => {
117173
expect(res.headers.get("access-control-allow-credentials")).toBeNull();
118174
});
119175
});
176+
177+
describe("isCredentialedCorsPath", () => {
178+
it("matches every session-authed browser surface", () => {
179+
expect(isCredentialedCorsPath("/api/auth/get-session")).toBe(true);
180+
expect(isCredentialedCorsPath("/v1/api-keys")).toBe(true);
181+
expect(isCredentialedCorsPath("/v1/api-keys/ak_1")).toBe(true);
182+
expect(isCredentialedCorsPath("/v1/me/follows")).toBe(true);
183+
expect(isCredentialedCorsPath("/v1/workspaces")).toBe(true);
184+
expect(isCredentialedCorsPath("/v1/workspaces/ws_1/avatar")).toBe(true);
185+
expect(isCredentialedCorsPath("/v1/listing/claim")).toBe(true);
186+
expect(isCredentialedCorsPath("/v1/listing/claim/verify")).toBe(true);
187+
expect(isCredentialedCorsPath("/v1/listing/claims")).toBe(true);
188+
expect(isCredentialedCorsPath("/v1/listing/promote")).toBe(true);
189+
});
190+
191+
it("leaves anonymous public routes on wildcard", () => {
192+
expect(isCredentialedCorsPath("/v1/orgs")).toBe(false);
193+
expect(isCredentialedCorsPath("/v1/listing/validate")).toBe(false);
194+
expect(isCredentialedCorsPath("/v1/listing/activate")).toBe(false);
195+
});
196+
});
197+
198+
/**
199+
* Drift gate: browser clients with `credentials: "include"` (or `meGet`) must
200+
* hit paths covered by CREDENTIALED_CORS_MOUNT_PATHS. Listing claim shipped
201+
* without the carve-out and failed in prod as CORS-blocked "Failed to fetch".
202+
*
203+
* Scans `web/src` for `apiBase()` / `meGet(...)` path templates. Same-origin
204+
* proxies and anonymous listing validate/activate are excluded.
205+
*/
206+
describe("browser credentialed clients stay on credentialed CORS", () => {
207+
it("every web credentials:include client path is covered by isCredentialedCorsPath", async () => {
208+
const { readdir, readFile } = await import("node:fs/promises");
209+
const { join } = await import("node:path");
210+
211+
async function* walk(dir: string): AsyncGenerator<string> {
212+
for (const ent of await readdir(dir, { withFileTypes: true })) {
213+
const p = join(dir, ent.name);
214+
if (ent.isDirectory()) {
215+
if (ent.name === "node_modules" || ent.name === "__generated__") continue;
216+
yield* walk(p);
217+
} else if (/\.(ts|tsx)$/.test(ent.name) && !/\.test\.(ts|tsx)$/.test(ent.name)) {
218+
yield p;
219+
}
220+
}
221+
}
222+
223+
// `${apiBase()}/v1/me/follows` | meGet("/v1/me/settings/…") | …}/v1/listing/claim
224+
const PATH_RE = /(?:apiBase\(\)\s*\}?|meGet\(\s*["'`])(\/(?:v1|api)\/[A-Za-z0-9_./${}`-]*)/g;
225+
const ANONYMOUS_API_PATHS = new Set(["/v1/listing/validate", "/v1/listing/activate"]);
226+
227+
const uncovered: string[] = [];
228+
const seen = new Set<string>();
229+
230+
for await (const file of walk(join(import.meta.dir, "../../web/src"))) {
231+
const src = await readFile(file, "utf8");
232+
// Only session-cookie clients — skip public apiBase() callers (listing validate).
233+
if (!/credentials:\s*["']include["']/.test(src) && !/\bmeGet\s*[<(]/.test(src)) continue;
234+
235+
for (const m of src.matchAll(PATH_RE)) {
236+
const path = m[1]!
237+
.replace(/\$\{[^}]+\}/g, "_")
238+
.replace(/[`'"]/g, "")
239+
.split("?")[0]!
240+
.replace(/\/$/, "");
241+
if (!path || ANONYMOUS_API_PATHS.has(path)) continue;
242+
if (seen.has(path)) continue;
243+
seen.add(path);
244+
if (!isCredentialedCorsPath(path)) {
245+
uncovered.push(`${path} (from ${file.replace(/.*\/web\//, "web/")})`);
246+
}
247+
}
248+
}
249+
250+
// If this is 0 the regex bit-rotted and the gate is useless.
251+
expect(seen.size).toBeGreaterThan(10);
252+
expect(uncovered).toEqual([]);
253+
});
254+
});

workers/api/src/auth/index.ts

Lines changed: 61 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -677,25 +677,25 @@ function webOriginForEmail(env: { WEB_BASE_URL?: string }): string {
677677
}
678678

679679
/**
680-
* Scoped, credentialed CORS for `/api/auth/*` AND the session-authed self-serve
681-
* surface `/v1/api-keys` (see index.ts). The worker's global `cors()` is
682-
* wildcard-origin / no-credentials, which cannot carry `Access-Control-Allow-
683-
* Credentials`; both surfaces need a reflected origin + credentials so the browser
684-
* will send and store the session cookie. MUST be registered BEFORE the global
685-
* `cors()` so it owns the preflight (the first matching CORS middleware
686-
* answers OPTIONS and returns). Allow-list mirrors {@link authTrustedOrigins}: the
687-
* releases.sh/.localhost family (our first-party web surfaces), every operator-
688-
* configured `BETTER_AUTH_TRUSTED_ORIGINS` entry — exact origins OR host wildcards
689-
* (Vercel preview / the portless dev host + its worktree subdomains via a `*.`
690-
* entry; see {@link matchesTrustedOrigin}) — and bare-loopback origins outside
691-
* production. Keeping the two in lockstep means CORS never silently blocks an origin
692-
* Better Auth already trusts.
680+
* Scoped, credentialed CORS for `/api/auth/*` AND every session-authed browser
681+
* surface (see {@link CREDENTIALED_CORS_MOUNT_PATHS} / index.ts). The worker's
682+
* global `cors()` is wildcard-origin / no-credentials, which cannot carry
683+
* `Access-Control-Allow-Credentials`; these surfaces need a reflected origin +
684+
* credentials so the browser will send and store the session cookie. MUST be
685+
* registered BEFORE the global `cors()` so it owns the preflight (the first
686+
* matching CORS middleware answers OPTIONS and returns). Allow-list mirrors
687+
* {@link authTrustedOrigins}: the releases.sh/.localhost family (our first-party
688+
* web surfaces), every operator-configured `BETTER_AUTH_TRUSTED_ORIGINS` entry —
689+
* exact origins OR host wildcards (Vercel preview / the portless dev host + its
690+
* worktree subdomains via a `*.` entry; see {@link matchesTrustedOrigin}) — and
691+
* bare-loopback origins outside production. Keeping the two in lockstep means
692+
* CORS never silently blocks an origin Better Auth already trusts.
693693
*
694694
* `DELETE` is allowed for the `/v1/api-keys/:id` revoke endpoint; `PUT` for
695695
* `/v1/me/digest` cadence writes; `PATCH` for `/v1/me/webhooks/:id` updates
696696
* (pause/resume, filter edits). Better Auth's own `/api/auth/*` routes are
697697
* POST/GET only, so the extra verbs are no-ops there. The allow-list must cover
698-
* every method any `/v1/me/*` handler uses or the browser blocks that preflight.
698+
* every method any credentialed handler uses or the browser blocks that preflight.
699699
*
700700
* The Sentinel client (`sentinelClient`, #1544) stamps every `/api/auth/*` request
701701
* with custom `X-Visitor-Id` / `X-Request-Id` fingerprint headers (and `X-PoW-Solution`
@@ -715,6 +715,53 @@ export const AUTH_CORS_ALLOWED_HEADERS = [
715715
"X-PoW-Solution",
716716
] as const;
717717

718+
/**
719+
* Hono mount patterns for {@link authCorsMiddleware}. **Single source of truth**
720+
* for the path carve-out: {@link isCredentialedCorsPath} is derived from this
721+
* list, and `index.ts` mounts the same patterns. When adding a session-cookie
722+
* browser client (`credentials: "include"`), add a mount here only.
723+
*
724+
* Anonymous public-write listing routes (`/v1/listing/validate`,
725+
* `/v1/listing/activate`) stay on wildcard CORS — do not list them here.
726+
*/
727+
export const CREDENTIALED_CORS_MOUNT_PATHS = [
728+
"/api/auth/*",
729+
"/v1/api-keys",
730+
"/v1/api-keys/*",
731+
"/v1/me/*",
732+
"/v1/workspaces",
733+
"/v1/workspaces/*",
734+
// Ownership claims + self-serve promote (#1947) — signed-in only.
735+
"/v1/listing/claim",
736+
"/v1/listing/claim/*",
737+
"/v1/listing/claims",
738+
"/v1/listing/promote",
739+
] as const;
740+
741+
/**
742+
* Match a request path against a Hono-style mount pattern (`/foo` or `/foo/*`).
743+
* `/foo/*` also matches the exact `/foo` base — same as Hono's middleware
744+
* matcher (session attach on claim routes relies on this).
745+
*/
746+
function matchesCredentialedCorsMount(path: string, pattern: string): boolean {
747+
if (pattern.endsWith("/*")) {
748+
const base = pattern.slice(0, -2);
749+
return path === base || path.startsWith(`${base}/`);
750+
}
751+
return path === pattern;
752+
}
753+
754+
/**
755+
* True when `path` (no query string) is owned by {@link authCorsMiddleware}.
756+
* Derived from {@link CREDENTIALED_CORS_MOUNT_PATHS} so mounts and the public
757+
* `cors()` skip stay in lockstep without a second hand-maintained list.
758+
*/
759+
export function isCredentialedCorsPath(path: string): boolean {
760+
return CREDENTIALED_CORS_MOUNT_PATHS.some((pattern) =>
761+
matchesCredentialedCorsMount(path, pattern),
762+
);
763+
}
764+
718765
export function authCorsMiddleware(): MiddlewareHandler<Env> {
719766
return cors({
720767
origin: (origin, c) => {

workers/api/src/index.ts

Lines changed: 13 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import type { JWTVerifyGetKey } from "@releases/lib/oauth-jwt";
1010
import {
1111
createAuth,
1212
authCorsMiddleware,
13+
CREDENTIALED_CORS_MOUNT_PATHS,
14+
isCredentialedCorsPath,
1315
runAuthWithWaitUntil,
1416
type BetterAuthInstance,
1517
} from "./auth/index.js";
@@ -491,37 +493,18 @@ app.onError((err, c) => {
491493
// the heuristic-caching rationale.
492494
app.use("*", cacheDefaultDeny());
493495

494-
// Better Auth CORS — credentialed, first-party origins only. MUST come before
495-
// the global wildcard `cors()` so it owns the `/api/auth/*` preflight (the first
496-
// matching CORS middleware answers OPTIONS and returns). See src/auth/index.ts.
497-
app.use("/api/auth/*", authCorsMiddleware());
498-
// Session-authed self-serve surface needs the same credentialed, origin-reflecting
499-
// CORS as /api/auth/* so the browser sends the cross-subdomain session cookie.
500-
app.use("/v1/api-keys", authCorsMiddleware());
501-
app.use("/v1/api-keys/*", authCorsMiddleware());
502-
app.use("/v1/me/*", authCorsMiddleware());
503-
app.use("/v1/workspaces", authCorsMiddleware());
504-
app.use("/v1/workspaces/*", authCorsMiddleware());
505-
506-
// Public read CORS — wildcard is fine; these endpoints don't accept credentials.
507-
// SKIP `/api/auth/*`: those routes are owned by `authCorsMiddleware` above, which
508-
// sets a credentialed, origin-reflecting CORS header. If this wildcard `cors()`
509-
// also ran there it would overwrite `Access-Control-Allow-Origin` with `*` on the
510-
// actual (non-preflight) response — which browsers reject for `credentials:
511-
// "include"` requests. The preflight passes (authCorsMiddleware short-circuits
512-
// OPTIONS), but the real GET/POST would be blocked. Keep the two in lockstep.
513-
// SKIP `/v1/api-keys` for the same reason — it is carved out above.
496+
// Credentialed CORS for first-party session-cookie surfaces, then public
497+
// wildcard CORS for everything else. MUST register credentialed first so it
498+
// owns the preflight. Path list is CREDENTIALED_CORS_MOUNT_PATHS (single source
499+
// of truth; isCredentialedCorsPath is derived from it). If the wildcard also
500+
// ran on those paths it would clobber ACAO with `*`, which browsers reject for
501+
// credentials: "include". See auth/index.ts.
502+
const credentialedCors = authCorsMiddleware();
503+
for (const path of CREDENTIALED_CORS_MOUNT_PATHS) {
504+
app.use(path, credentialedCors);
505+
}
514506
const publicReadCors = cors();
515-
app.use("*", (c, next) =>
516-
c.req.path.startsWith("/api/auth/") ||
517-
c.req.path === "/v1/api-keys" ||
518-
c.req.path.startsWith("/v1/api-keys/") ||
519-
c.req.path.startsWith("/v1/me/") ||
520-
c.req.path === "/v1/workspaces" ||
521-
c.req.path.startsWith("/v1/workspaces/")
522-
? next()
523-
: publicReadCors(c, next),
524-
);
507+
app.use("*", (c, next) => (isCredentialedCorsPath(c.req.path) ? next() : publicReadCors(c, next)));
525508
app.use("*", stagingAccessGate());
526509
app.use("*", blockIndexing());
527510

0 commit comments

Comments
 (0)