diff --git a/package-lock.json b/package-lock.json index 95fc924..a1e34d5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7,6 +7,7 @@ "": { "name": "stackbilt-mcp-gateway", "version": "0.1.0", + "license": "MIT", "dependencies": { "@cloudflare/workers-oauth-provider": "^0.2.4", "@modelcontextprotocol/sdk": "^1.27.1", diff --git a/src/gateway.ts b/src/gateway.ts index 81f2a2f..2adf657 100644 --- a/src/gateway.ts +++ b/src/gateway.ts @@ -108,12 +108,46 @@ function rpcResult(id: unknown, result: unknown): Response { } // ─── Security: scrub secrets from error messages ────────────── +// M-2 remediation (2026-04-10 audit): service binding names are now +// stripped so backend topology (AUTH_SERVICE, ENGINE, etc.) never reaches +// a caller via leaked error text. Without this, an attacker could probe +// each tool to trigger backend errors and map internal service names. +const SERVICE_BINDING_NAMES = + /\b(AUTH_SERVICE|IMG_FORGE|TAROTSCRIPT|ENGINE|DEPLOYER|VISUAL_QA|TRANSPILER|PLATFORM_EVENTS_QUEUE|OAUTH_KV|OAUTH_PROVIDER)\b/g; + function sanitizeError(message: string): string { - // Strip anything that looks like a token, key, or secret + // Strip tokens, keys, secrets, AND internal service binding names return message .replace(/sb_(live|test)_[a-zA-Z0-9]+/g, '[REDACTED_KEY]') .replace(/Bearer\s+[^\s]+/gi, 'Bearer [REDACTED]') - .replace(/[a-f0-9]{32,}/gi, '[REDACTED_HASH]'); + .replace(/[a-f0-9]{32,}/gi, '[REDACTED_HASH]') + .replace(SERVICE_BINDING_NAMES, '[BINDING]'); +} + +// ─── User-facing error helper (M-2 remediation) ─────────────── +// Every tool error path that returns text to the client must flow through +// this helper. It sanitizes the message (stripping secrets + binding names), +// truncates to a fixed length so no backend can pad responses with internal +// state, appends the trace id for correlated support, and logs the FULL +// original error internally so operators can still debug. Before M-2, tool +// handlers interpolated `err.message` directly into JSON-RPC responses, +// leaking internal identifiers, DB ids, and service binding names. +const USER_ERROR_MAX_LEN = 200; + +function userError( + prefix: string, + err: unknown, + traceId: string, +): { content: Array<{ type: 'text'; text: string }>; isError: true } { + const raw = err instanceof Error ? err.message : String(err); + const safe = sanitizeError(raw).slice(0, USER_ERROR_MAX_LEN); + // Full (unsanitized) error stays in worker logs, tied to the trace id + // so support can correlate without exposing anything to the caller. + console.error(`[${traceId}] ${prefix}: ${raw}`); + return { + content: [{ type: 'text', text: `${prefix}: ${safe} (trace: ${traceId})` }], + isError: true, + }; } // ─── Audit helper: log + queue ───────────────────────────────── @@ -170,6 +204,7 @@ async function proxyRestToolCall( args: unknown, session: GatewaySession, env: GatewayEnv, + traceId: string, ): Promise<{ content: Array<{ type: string; text: string }>; isError?: boolean }> { const a = (args ?? {}) as Record; @@ -614,10 +649,7 @@ async function proxyRestToolCall( content: [{ type: 'text', text: JSON.stringify(result, null, 2) }], }; } catch (err) { - return { - content: [{ type: 'text', text: `scaffold_publish failed: ${err instanceof Error ? err.message : String(err)}` }], - isError: true, - }; + return userError('scaffold_publish failed', err, traceId); } } @@ -653,10 +685,7 @@ async function proxyRestToolCall( isError: !res.ok, }; } catch (err) { - return { - content: [{ type: 'text', text: `Deploy failed: ${err instanceof Error ? err.message : String(err)}` }], - isError: true, - }; + return userError('scaffold_deploy failed', err, traceId); } } @@ -704,10 +733,7 @@ async function proxyRestToolCall( isError: !res.ok, }; } catch (err) { - return { - content: [{ type: 'text', text: `Visual QA error: ${err instanceof Error ? err.message : String(err)}` }], - isError: true, - }; + return userError('visual_qa error', err, traceId); } } @@ -739,10 +765,7 @@ async function proxyRestToolCall( isError: !res.ok, }; } catch (err) { - return { - content: [{ type: 'text', text: `Transpiler error: ${err instanceof Error ? err.message : String(err)}` }], - isError: true, - }; + return userError('scaffold_import error', err, traceId); } } @@ -789,15 +812,12 @@ async function proxyToolCall( // ── REST API backends (e.g. TarotScript) ────────────────────── if (route.restApi) { try { - const result = await proxyRestToolCall(binding, route, backendToolName, args, session, env); + const result = await proxyRestToolCall(binding, route, backendToolName, args, session, env, traceId); audit({ ...auditBase, outcome: 'success', latency_ms: Date.now() - start }, env); return result; } catch (err) { audit({ ...auditBase, outcome: 'backend_error', latency_ms: Date.now() - start }, env); - return { - content: [{ type: 'text', text: `${route.product} error: ${sanitizeError(err instanceof Error ? err.message : String(err))}` }], - isError: true, - }; + return userError(`${route.product} error`, err, traceId); } } @@ -833,10 +853,7 @@ async function proxyToolCall( if (!response.ok) { const text = await response.text().catch(() => 'unknown error'); audit({ ...auditBase, outcome: 'backend_error', latency_ms: Date.now() - start }, env); - return { - content: [{ type: 'text', text: `Backend error (${route.product}): ${sanitizeError(text).slice(0, 500)}` }], - isError: true, - }; + return userError(`Backend error (${route.product})`, text, traceId); } // Backend may respond with JSON or SSE (Streamable HTTP transport). @@ -880,10 +897,7 @@ async function proxyToolCall( if (body.error) { audit({ ...auditBase, outcome: 'backend_error', latency_ms: Date.now() - start }, env); - return { - content: [{ type: 'text', text: `${route.product} error: ${sanitizeError(body.error.message ?? 'unknown')}` }], - isError: true, - }; + return userError(`${route.product} error`, body.error.message ?? 'unknown', traceId); } const fallback = { content: [{ type: 'text' as const, text: 'No result from backend' }], isError: true as const }; @@ -919,16 +933,12 @@ async function proxyToolCall( if (err instanceof DOMException && err.name === 'TimeoutError') { audit({ ...auditBase, outcome: 'backend_error', latency_ms: Date.now() - start }, env); return { - content: [{ type: 'text', text: `Backend timeout (${route.product})` }], + content: [{ type: 'text', text: `Backend timeout (${route.product}) (trace: ${traceId})` }], isError: true, }; } - const msg = err instanceof Error ? err.message : String(err); audit({ ...auditBase, outcome: 'error', latency_ms: Date.now() - start }, env); - return { - content: [{ type: 'text', text: `Gateway proxy error: ${sanitizeError(msg)}` }], - isError: true, - }; + return userError('Gateway proxy error', err, traceId); } } diff --git a/src/oauth-handler.ts b/src/oauth-handler.ts index e360688..846c29e 100644 --- a/src/oauth-handler.ts +++ b/src/oauth-handler.ts @@ -51,8 +51,10 @@ const GOOGLE_ICON = ``; return ` @@ -71,15 +73,18 @@ function renderLoginPage( ${error ? `
${escapeHtml(error)}
` : ''}
+ ${csrfInput}
+ ${csrfInput}
or sign in with email
+ ${csrfInput} @@ -96,8 +101,10 @@ function renderLoginPage( function renderSignupPage( oauthParams: string, + csrfToken: string, error?: string, ): string { + const csrfInput = ``; return ` @@ -116,15 +123,18 @@ function renderSignupPage( ${error ? `
${escapeHtml(error)}
` : ''} + ${csrfInput}
+ ${csrfInput}
or sign up with email
+ ${csrfInput} @@ -222,6 +232,130 @@ export function escapeHtml(str: string): string { .replace(/'/g, '''); } +// --- Cookie helpers --- +// Minimal cookie reader used by M-1 (social OAuth browser binding) and +// HD-2 (CSRF tokens). Returns the first matching cookie value or null. +export function getCookie(request: Request, name: string): string | null { + const header = request.headers.get('Cookie'); + if (!header) return null; + const parts = header.split(';'); + for (const part of parts) { + const eq = part.indexOf('='); + if (eq < 0) continue; + const k = part.slice(0, eq).trim(); + if (k === name) { + return decodeURIComponent(part.slice(eq + 1).trim()); + } + } + return null; +} + +// Build a Set-Cookie header for a short-lived, browser-bound cookie. +// HttpOnly blocks JS access, Secure requires HTTPS, SameSite=Lax allows +// top-level cross-site navigations (needed for OAuth redirect-back) while +// blocking cross-site POSTs that would enable CSRF. +export function buildCookie( + name: string, + value: string, + maxAgeSeconds: number, +): string { + return `${name}=${encodeURIComponent(value)}; HttpOnly; Secure; SameSite=Lax; Path=/; Max-Age=${maxAgeSeconds}`; +} + +// Clear a previously-set cookie by emitting a Max-Age=0 Set-Cookie. +export function clearCookie(name: string): string { + return `${name}=; HttpOnly; Secure; SameSite=Lax; Path=/; Max-Age=0`; +} + +// --- Rate limiting (HD-1 remediation) --- +// KV-backed fixed-window counter. The TTL resets on each successful +// increment, so in practice it behaves like a "sliding fixed window" — +// a caller only clears the limit once they stop making requests for the +// full window. Good enough for brute-force mitigation at the single-digit +// scale we use; a TOCTOU race could let 2-3 extra requests through under +// concurrency but that's a rounding error against a 5-per-5min limit. +// +// Returns: { allowed: true } or { allowed: false, retryAfter: seconds } +export async function checkRateLimit( + kv: KVNamespace, + key: string, + limit: number, + windowSeconds: number, +): Promise<{ allowed: boolean; retryAfter?: number }> { + const raw = await kv.get(key); + const count = (raw ? parseInt(raw, 10) : 0) + 1; + if (count > limit) { + return { allowed: false, retryAfter: windowSeconds }; + } + await kv.put(key, String(count), { expirationTtl: windowSeconds }); + return { allowed: true }; +} + +// Extract the best-effort client IP. Cloudflare Workers populate +// CF-Connecting-IP with the real client address. Falls back to the +// first X-Forwarded-For entry, then to 'unknown' (which still gates +// against rapid-fire attempts from a single connection). +export function getClientIp(request: Request): string { + const cfIp = request.headers.get('CF-Connecting-IP'); + if (cfIp) return cfIp; + const xff = request.headers.get('X-Forwarded-For'); + if (xff) return xff.split(',')[0].trim(); + return 'unknown'; +} + +// --- CSRF tokens (HD-2 remediation) --- +// Double-submit cookie pattern. On GET (form render), we mint a fresh +// random token, stamp it as a hidden field inside the form, AND set it +// as an HttpOnly cookie. On POST, the handler compares the cookie to the +// form-supplied value; mismatch → reject. SameSite=Lax on the cookie +// blocks cross-site POSTs from even reaching the handler with the cookie +// attached, so this is defense-in-depth on top of the SameSite guarantee. +// +// The token rotates on every page render, so even a cross-tab CSRF +// scenario where the attacker somehow reads the cookie would still need +// a form value from the latest render to match. +export function mintCsrf(): string { + return crypto.randomUUID(); +} + +export function verifyCsrf(request: Request, formData: FormData): boolean { + const cookie = getCookie(request, 'csrf_token'); + const formValue = formData.get('csrf_token'); + if (!cookie || typeof formValue !== 'string' || !formValue) return false; + return cookie === formValue; +} + +// --- Form-page response builders (HD-2) --- +// These wrappers mint a fresh CSRF token per render, inject it into the +// page's hidden form fields, and stamp a matching HttpOnly cookie on the +// response. POST handlers call verifyCsrf() to reject any submission +// whose cookie doesn't match the form value. +function loginResponse( + oauthParams: string, + error?: string, + status = 200, + extraSetCookies: string[] = [], +): Response { + const csrfToken = mintCsrf(); + const headers = new Headers({ 'Content-Type': 'text/html' }); + headers.append('Set-Cookie', buildCookie('csrf_token', csrfToken, 600)); + for (const c of extraSetCookies) headers.append('Set-Cookie', c); + return new Response(renderLoginPage(oauthParams, csrfToken, error), { status, headers }); +} + +function signupResponse( + oauthParams: string, + error?: string, + status = 200, + extraSetCookies: string[] = [], +): Response { + const csrfToken = mintCsrf(); + const headers = new Headers({ 'Content-Type': 'text/html' }); + headers.append('Set-Cookie', buildCookie('csrf_token', csrfToken, 600)); + for (const c of extraSetCookies) headers.append('Set-Cookie', c); + return new Response(renderSignupPage(oauthParams, csrfToken, error), { status, headers }); +} + // --- Signed identity token (HMAC-SHA256, short-lived) --- // Carries user identity AND the OAuth request context (clientId, redirectUri) // through login → consent without cookies. H-1 remediation from 2026-04-10 @@ -411,9 +545,7 @@ const handler: ExportedHandler = { // GET /signup -- show signup form if (url.pathname === '/signup' && request.method === 'GET') { const oauthParams = url.searchParams.get('oauth_params') || ''; - return new Response(renderSignupPage(oauthParams), { - headers: { 'Content-Type': 'text/html' }, - }); + return signupResponse(oauthParams); } // POST /signup -- create account via stackbilt-auth RPC @@ -465,9 +597,7 @@ async function handleGetAuthorize(request: Request, env: GatewayEnv): Promise { @@ -521,18 +649,12 @@ async function handlePostAuthorize(request: Request, env: GatewayEnv): Promise const password = formData.get('password') as string; const oauthParams = formData.get('oauth_params') as string; + // HD-2 remediation: verify double-submit CSRF token BEFORE doing anything + // else. A cross-site POST won't carry the csrf_token cookie (SameSite=Lax) + // AND won't know the rotating form value, so it fails here. + if (!verifyCsrf(request, formData)) { + return loginResponse(oauthParams || '', 'Your session has expired. Please try again.', 403); + } + if (!email || !password) { - return new Response( - renderLoginPage(oauthParams || '', 'Email and password are required.'), - { headers: { 'Content-Type': 'text/html' } }, + return loginResponse(oauthParams || '', 'Email and password are required.'); + } + + // HD-1 remediation: rate limit BEFORE hitting the auth RPC so brute-force + // attempts can't burn quota on the auth service or probe timing. Key on + // email primarily (throttles targeted attacks) AND on IP (throttles + // credential-stuffing that iterates email lists). A single gated path + // is enough: the first keyed counter to exhaust returns 429. + const emailKey = `rl:login:email:${email.toLowerCase()}`; + const ipKey = `rl:login:ip:${getClientIp(request)}`; + const emailCheck = await checkRateLimit(env.OAUTH_KV, emailKey, 5, 300); + if (!emailCheck.allowed) { + const res = loginResponse( + oauthParams || '', + 'Too many sign-in attempts. Try again in 5 minutes.', + 429, + ); + res.headers.set('Retry-After', String(emailCheck.retryAfter ?? 300)); + return res; + } + const ipCheck = await checkRateLimit(env.OAUTH_KV, ipKey, 20, 300); + if (!ipCheck.allowed) { + const res = loginResponse( + oauthParams || '', + 'Too many sign-in attempts from your network. Try again in 5 minutes.', + 429, ); + res.headers.set('Retry-After', String(ipCheck.retryAfter ?? 300)); + return res; } // Authenticate via stackbilt-auth RPC const result = await env.AUTH_SERVICE.authenticateUser(email, password); if (!result.valid) { - return new Response( - renderLoginPage(oauthParams || '', 'Invalid email or password.'), - { headers: { 'Content-Type': 'text/html' } }, - ); + return loginResponse(oauthParams || '', 'Invalid email or password.'); } // Auto-provision tenant (idempotent -- returns existing if found) @@ -639,18 +808,34 @@ async function handleSignup(request: Request, env: GatewayEnv): Promise { @@ -720,34 +928,60 @@ async function handleSocialOAuthCallback(request: Request, env: GatewayEnv): Pro const error = url.searchParams.get('error'); if (error) { - return new Response(renderLoginPage('', 'Social sign-in failed. Please try again.'), { - headers: { 'Content-Type': 'text/html' }, - }); + return loginResponse('', 'Social sign-in failed. Please try again.'); } if (!stateKey || !code) { - return new Response(renderLoginPage('', 'Missing authentication data. Please try again.'), { - headers: { 'Content-Type': 'text/html' }, - }); + return loginResponse('', 'Missing authentication data. Please try again.'); } - // Retrieve stored oauth_params - const oauthParams = await env.OAUTH_KV.get(`social_state:${stateKey}`); + // Retrieve stored state (one-time use: delete before doing anything else) + const stored = await env.OAUTH_KV.get(`social_state:${stateKey}`); await env.OAUTH_KV.delete(`social_state:${stateKey}`); - if (!oauthParams) { - return new Response(renderLoginPage('', 'Session expired. Please try again.'), { - headers: { 'Content-Type': 'text/html' }, - }); + if (!stored) { + return loginResponse('', 'Session expired. Please try again.'); + } + + // M-1 remediation: parse the JSON-wrapped state and verify the browser + // cookie matches the binding we stamped at flow start. An attacker who + // holds only the stateKey (leaked URL, KV log, etc.) cannot complete the + // callback without also controlling the victim's cookie jar. + let oauthParams: string; + let storedBinding: string | null = null; + try { + const parsed = JSON.parse(stored) as { oauthParams: string; browserBinding: string }; + oauthParams = parsed.oauthParams; + storedBinding = parsed.browserBinding; + } catch { + // Legacy format (raw oauthParams string) — can appear briefly during + // the first 5 minutes after M-1 deploy while in-flight flows drain. + // Accept but log so we can confirm the migration window closed. + console.warn(`[social-oauth] legacy KV state format for ${stateKey}, binding check skipped`); + oauthParams = stored; + } + + if (storedBinding !== null) { + const cookieBinding = getCookie(request, 'oauth_binding'); + if (!cookieBinding || cookieBinding !== storedBinding) { + return loginResponse( + '', + 'Security check failed. Please start the sign-in flow again.', + 403, + [clearCookie('oauth_binding')], + ); + } } // Exchange the one-time code for user identity via auth service RPC const result = await env.AUTH_SERVICE.exchangeSocialCode(code); if (!result.valid) { - return new Response( - renderLoginPage(oauthParams, 'Social sign-in failed. Please try again.'), - { headers: { 'Content-Type': 'text/html' } }, + return loginResponse( + oauthParams, + 'Social sign-in failed. Please try again.', + 200, + [clearCookie('oauth_binding')], ); } @@ -773,5 +1007,12 @@ async function handleSocialOAuthCallback(request: Request, env: GatewayEnv): Pro redirectUri: oauthReqInfo.redirectUri, }); - return Response.redirect(buildAuthorizeRedirect(env, oauthReqInfo, identityToken), 302); + // Clear the browser-binding cookie now that the flow has completed. + return new Response(null, { + status: 302, + headers: { + Location: buildAuthorizeRedirect(env, oauthReqInfo, identityToken), + 'Set-Cookie': clearCookie('oauth_binding'), + }, + }); } diff --git a/test/oauth-handler.test.ts b/test/oauth-handler.test.ts index d9a04dd..dd95fc8 100644 --- a/test/oauth-handler.test.ts +++ b/test/oauth-handler.test.ts @@ -3,6 +3,13 @@ import { escapeHtml, signIdentityToken, verifyIdentityToken, + getCookie, + buildCookie, + clearCookie, + mintCsrf, + verifyCsrf, + checkRateLimit, + getClientIp, } from '../src/oauth-handler.js'; import type { GatewayEnv } from '../src/types.js'; import type { AuthRequest } from '@cloudflare/workers-oauth-provider'; @@ -413,6 +420,184 @@ describe('OAuth handler — scope labels', () => { }); }); +describe('Cookie helpers (M-1 / HD-2)', () => { + it('getCookie returns named value from multi-cookie header', () => { + const req = new Request('https://x/', { + headers: { Cookie: 'other=1; csrf_token=abc123; foo=bar' }, + }); + expect(getCookie(req, 'csrf_token')).toBe('abc123'); + expect(getCookie(req, 'other')).toBe('1'); + expect(getCookie(req, 'foo')).toBe('bar'); + }); + + it('getCookie returns null for missing cookie or missing header', () => { + const withHeader = new Request('https://x/', { headers: { Cookie: 'a=1' } }); + expect(getCookie(withHeader, 'missing')).toBeNull(); + + const noHeader = new Request('https://x/'); + expect(getCookie(noHeader, 'anything')).toBeNull(); + }); + + it('getCookie decodes URL-encoded values', () => { + const req = new Request('https://x/', { + headers: { Cookie: 'x=hello%20world%21' }, + }); + expect(getCookie(req, 'x')).toBe('hello world!'); + }); + + it('buildCookie sets HttpOnly, Secure, SameSite=Lax, Path, Max-Age', () => { + const cookie = buildCookie('csrf_token', 'abc123', 600); + expect(cookie).toContain('csrf_token=abc123'); + expect(cookie).toContain('HttpOnly'); + expect(cookie).toContain('Secure'); + expect(cookie).toContain('SameSite=Lax'); + expect(cookie).toContain('Path=/'); + expect(cookie).toContain('Max-Age=600'); + }); + + it('clearCookie emits Max-Age=0', () => { + const cookie = clearCookie('oauth_binding'); + expect(cookie).toContain('oauth_binding='); + expect(cookie).toContain('Max-Age=0'); + expect(cookie).toContain('HttpOnly'); + expect(cookie).toContain('SameSite=Lax'); + }); +}); + +describe('CSRF tokens (HD-2)', () => { + it('mintCsrf produces a non-empty random token', () => { + const a = mintCsrf(); + const b = mintCsrf(); + expect(a).toBeTruthy(); + expect(b).toBeTruthy(); + expect(a).not.toBe(b); + }); + + it('verifyCsrf accepts matching cookie and form value', () => { + const token = mintCsrf(); + const formData = new FormData(); + formData.set('csrf_token', token); + const req = new Request('https://x/', { + headers: { Cookie: `csrf_token=${token}` }, + }); + expect(verifyCsrf(req, formData)).toBe(true); + }); + + it('verifyCsrf rejects when form value is missing', () => { + const token = mintCsrf(); + const formData = new FormData(); + const req = new Request('https://x/', { + headers: { Cookie: `csrf_token=${token}` }, + }); + expect(verifyCsrf(req, formData)).toBe(false); + }); + + it('verifyCsrf rejects when cookie is missing', () => { + const token = mintCsrf(); + const formData = new FormData(); + formData.set('csrf_token', token); + const req = new Request('https://x/'); + expect(verifyCsrf(req, formData)).toBe(false); + }); + + it('verifyCsrf rejects when cookie and form value differ', () => { + const formData = new FormData(); + formData.set('csrf_token', 'attacker-guessed-value'); + const req = new Request('https://x/', { + headers: { Cookie: 'csrf_token=real-token' }, + }); + expect(verifyCsrf(req, formData)).toBe(false); + }); + + it('verifyCsrf rejects when cookie is empty string', () => { + const formData = new FormData(); + formData.set('csrf_token', ''); + const req = new Request('https://x/', { + headers: { Cookie: 'csrf_token=' }, + }); + expect(verifyCsrf(req, formData)).toBe(false); + }); +}); + +describe('Rate limiting (HD-1)', () => { + function makeKv() { + const store = new Map(); + return { + put: vi.fn(async (key: string, value: string) => { + store.set(key, value); + }), + get: vi.fn(async (key: string) => store.get(key) ?? null), + delete: vi.fn(async (key: string) => { store.delete(key); }), + list: vi.fn(), + getWithMetadata: vi.fn(), + } as unknown as KVNamespace; + } + + it('allows up to the limit and denies after', async () => { + const kv = makeKv(); + const key = 'rl:login:email:test@test.com'; + + for (let i = 0; i < 5; i++) { + const result = await checkRateLimit(kv, key, 5, 300); + expect(result.allowed).toBe(true); + } + const denied = await checkRateLimit(kv, key, 5, 300); + expect(denied.allowed).toBe(false); + expect(denied.retryAfter).toBe(300); + }); + + it('increments across separate keys independently', async () => { + const kv = makeKv(); + const a = await checkRateLimit(kv, 'rl:k1', 1, 60); + const b = await checkRateLimit(kv, 'rl:k2', 1, 60); + expect(a.allowed).toBe(true); + expect(b.allowed).toBe(true); + }); + + it('stores counter with TTL on each increment', async () => { + const kv = makeKv(); + await checkRateLimit(kv, 'rl:test', 5, 300); + expect(kv.put).toHaveBeenCalledWith('rl:test', '1', { expirationTtl: 300 }); + }); + + it('denies cleanly without re-incrementing once over the limit', async () => { + const kv = makeKv(); + for (let i = 0; i < 2; i++) { + await checkRateLimit(kv, 'rl:small', 2, 60); + } + const putCountBeforeDeny = (kv.put as ReturnType).mock.calls.length; + const denied = await checkRateLimit(kv, 'rl:small', 2, 60); + expect(denied.allowed).toBe(false); + // Denied path must NOT write a new counter value (otherwise attackers + // extend their own lockout window by continuing to hit the endpoint). + expect((kv.put as ReturnType).mock.calls.length).toBe(putCountBeforeDeny); + }); +}); + +describe('getClientIp', () => { + it('prefers CF-Connecting-IP header', () => { + const req = new Request('https://x/', { + headers: { + 'CF-Connecting-IP': '203.0.113.10', + 'X-Forwarded-For': '10.0.0.1, 203.0.113.10', + }, + }); + expect(getClientIp(req)).toBe('203.0.113.10'); + }); + + it('falls back to first X-Forwarded-For entry', () => { + const req = new Request('https://x/', { + headers: { 'X-Forwarded-For': '198.51.100.7, 10.0.0.1' }, + }); + expect(getClientIp(req)).toBe('198.51.100.7'); + }); + + it('returns unknown when no identifying header is present', () => { + const req = new Request('https://x/'); + expect(getClientIp(req)).toBe('unknown'); + }); +}); + describe('OAuth handler — security invariants', () => { it('identity tokens use HMAC-SHA256 (not plaintext)', async () => { const token = await signIdentityToken(TEST_SECRET, {