From 819516b96d7d2788e0fb1a80c5ac427f6fb080fd Mon Sep 17 00:00:00 2001 From: Michael Date: Tue, 26 May 2026 12:17:57 -0700 Subject: [PATCH] fix(hub): CSRF guard bypasses Bearer-auth requests (legacy JWT) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production users on the legacy JWT auth path were hitting 403 csrf_failed on every mutating REST call (e.g. POST /api/supervisors/:id/scan from SupervisorPage). Root cause: legacy JWT login never issues the csrf_token cookie that the new session-cookie auth path issues, so apiFetch can't echo it back in X-CSRF-Token, so csrfGuard rejects. Fix: csrfGuard now bypasses enforcement when the request carries an Authorization: Bearer header. The bypass is gated by a strict /^Bearer\s+\S+/i regex — empty Authorization, non-Bearer schemes (Basic, etc.), and 'Bearer' without a token all still fall through to normal CSRF enforcement. Threat model (also documented in csrf.ts): - CSRF attacks rely on the browser's ambient-credential behavior. Cookies are attached cross-origin automatically; Authorization headers are NOT. - JS on evil.com cannot read app.remo-code.com's bearer token from localStorage (same-origin policy), so it cannot forge a Bearer-authed request. - Therefore a Bearer-authed request is, by construction, not CSRF-eligible and does not need the double-submit token. - Scope guardrails: ONLY the Bearer scheme bypasses. Custom headers do not qualify — a CSRF attacker can set arbitrary custom headers via fetch() if CORS allows, so 'presence of custom header' is not a safe proxy. - Cookie-auth users continue to use the double-submit pattern unchanged. Tests: 10 new cases in hub/test/csrf.test.ts cover the bypass + every guardrail (empty header, Basic scheme, 'Bearer' with no token, lowercase header name, GET passthrough, cookie-auth still works without Bearer). All 27 csrf tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- hub/src/csrf.ts | 41 ++++++++++++++++++++++++ hub/test/csrf.test.ts | 73 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+) diff --git a/hub/src/csrf.ts b/hub/src/csrf.ts index 8e6e1e0..3caebbb 100644 --- a/hub/src/csrf.ts +++ b/hub/src/csrf.ts @@ -84,14 +84,55 @@ export function verifyCsrfPair(cookieValue: string | null, headerValue: string | return constantTimeEqualHex(cookieValue, headerValue); } +// Bearer-auth bypass: matches `Authorization: Bearer ` (case-insensitive +// scheme, any whitespace, any non-empty token). Empty/missing/non-Bearer schemes +// fall through to normal CSRF enforcement. +const BEARER_RE = /^Bearer\s+\S+/i; + // Hono middleware: enforces CSRF on mutating methods except allowlisted paths. // Pass-through on GET/HEAD/OPTIONS and on allowlisted paths. +// +// Threat model for the Bearer bypass (legacy JWT auth path): +// CSRF attacks exploit the browser's ambient-credential behavior — a victim +// visits evil.com, evil.com triggers a cross-origin POST to app.remo-code.com, +// and the BROWSER attaches the victim's cookies automatically. The attacker +// never needs to read the cookie value; they just ride it. +// +// The double-submit cookie pattern defends against this by requiring the +// attacker to ALSO present the CSRF nonce in a header — which the browser +// does NOT attach automatically; only same-origin JS that can read the +// csrf_token cookie can echo it. +// +// `Authorization: Bearer ` headers are NOT in the browser's +// ambient-credential set. Browsers never attach them automatically — JS +// must explicitly set the header on each request. Cross-origin JS on +// evil.com cannot read the bearer token from app.remo-code.com's +// localStorage (same-origin policy), so it cannot forge a Bearer-authed +// request even if the user is logged in. +// +// Therefore: a request that carries a Bearer token is, by construction, +// not a CSRF-eligible request. The bypass is safe AND necessary — legacy +// JWT-auth users never receive a csrf_token cookie (only the new +// session-cookie auth issues one), so without this bypass every mutating +// call from a legacy-auth client fails closed with 403. +// +// Scope guardrails: +// - ONLY Bearer scheme. Custom headers (X-Auth, etc.) do NOT qualify — +// a CSRF attacker can set arbitrary custom headers via fetch() if CORS +// allows it, so "presence of a custom header" is not a safe proxy. +// - Empty `Authorization:` header does NOT bypass. +// - Cookie-auth users continue to use double-submit. We do not loosen +// enforcement for them. export function csrfGuard() { return async (c: Context, next: Next) => { const method = c.req.method.toUpperCase(); if (!MUTATING_METHODS.has(method)) return next(); if (isCsrfAllowlisted(c.req.path)) return next(); + // Bearer-auth bypass — see threat model above. + const authHeader = c.req.header('Authorization') || c.req.header('authorization'); + if (authHeader && BEARER_RE.test(authHeader)) return next(); + const cookieValue = readCsrfCookie(c); const headerValue = c.req.header(CSRF_HEADER_NAME) || c.req.header(CSRF_HEADER_NAME.toLowerCase()); if (!verifyCsrfPair(cookieValue, headerValue ?? null)) { diff --git a/hub/test/csrf.test.ts b/hub/test/csrf.test.ts index 7ae072e..da0ec0d 100644 --- a/hub/test/csrf.test.ts +++ b/hub/test/csrf.test.ts @@ -129,4 +129,77 @@ describe('csrfGuard middleware', () => { const res = await app.request('/api/foo', { method: 'DELETE' }); expect(res.status).toBe(403); }); + + // Bearer-auth bypass (legacy JWT path) — see threat model in csrf.ts. + test('POST with Authorization: Bearer bypasses CSRF', async () => { + const res = await buildApp().request('/api/foo', { + method: 'POST', + headers: { Authorization: 'Bearer eyJhbGciOiJIUzI1NiJ9.payload.sig' }, + }); + expect(res.status).toBe(200); + }); + + test('POST with Bearer bypass works without any csrf cookie or header', async () => { + const res = await buildApp().request('/api/foo', { + method: 'POST', + headers: { Authorization: 'Bearer token-xyz' }, + }); + expect(res.status).toBe(200); + }); + + test('POST with lowercase authorization header still bypasses (HTTP is case-insensitive)', async () => { + const res = await buildApp().request('/api/foo', { + method: 'POST', + headers: { authorization: 'bearer abc.def.ghi' }, + }); + expect(res.status).toBe(200); + }); + + test('POST with empty Authorization header does NOT bypass → 403', async () => { + const res = await buildApp().request('/api/foo', { + method: 'POST', + headers: { Authorization: '' }, + }); + expect(res.status).toBe(403); + }); + + test('POST with non-Bearer scheme (Basic) does NOT bypass → 403', async () => { + const res = await buildApp().request('/api/foo', { + method: 'POST', + headers: { Authorization: 'Basic dXNlcjpwYXNz' }, + }); + expect(res.status).toBe(403); + }); + + test('POST with "Bearer" but no token does NOT bypass → 403', async () => { + const res = await buildApp().request('/api/foo', { + method: 'POST', + headers: { Authorization: 'Bearer ' }, + }); + expect(res.status).toBe(403); + }); + + test('POST with "Bearer" alone (no space, no token) does NOT bypass → 403', async () => { + const res = await buildApp().request('/api/foo', { + method: 'POST', + headers: { Authorization: 'Bearer' }, + }); + expect(res.status).toBe(403); + }); + + test('Bearer bypass does NOT apply to GET (irrelevant — GET is already passthrough)', async () => { + const res = await buildApp().request('/api/foo', { + method: 'GET', + headers: { Authorization: 'Bearer abc' }, + }); + expect(res.status).toBe(200); + }); + + test('cookie-auth double-submit still works when no Authorization header present', async () => { + const res = await buildApp().request('/api/foo', { + method: 'POST', + headers: { cookie: `${CSRF_COOKIE_NAME}=match-me-123`, [CSRF_HEADER_NAME]: 'match-me-123' }, + }); + expect(res.status).toBe(200); + }); });