From 11b3bab9fe9b25279d687f910a678036b1d03e2f Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 15 Jul 2026 05:37:04 -0700 Subject: [PATCH] fix(auth): rate-limit /v1/auth/extension/session by session, not IP The same pre-auth-vs-session-authenticated gap #6117 fixed for /v1/auth/github/token also applies to /v1/auth/extension/session (#556): isPreAuthRateLimitPath's broad /v1/auth/ prefix match classified it as pre-auth, keying its rate limit by client IP. But this endpoint always requires (and validates) a real session bearer token to mint a new extension-scoped session from it -- it is not a pre-auth flow. IP-keying meant a caller with a stolen session token could bypass the strict 10/min cap by rotating source IPs, and unrelated sessions behind a shared IP (office NAT, CI infra) would throttle each other. Generalized the #6117 fix's single-path exclusion into a small Set of session-authenticated /v1/auth/* paths and added this one to it. Falls through to token-based rate-limit keying when a valid bearer is present, falling back to IP-keying only when no valid bearer is supplied, matching every other authenticated route. --- src/auth/rate-limit.ts | 12 ++++++++---- test/unit/auth.test.ts | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/auth/rate-limit.ts b/src/auth/rate-limit.ts index 825d1261e5..86e04f8b62 100644 --- a/src/auth/rate-limit.ts +++ b/src/auth/rate-limit.ts @@ -221,15 +221,19 @@ function isValidIpv6(value: string): boolean { return hasHexSegment; } -// /v1/auth/github/token (#6114/#6115/#6117) is excluded from the broad /v1/auth/ prefix match below: unlike -// the OAuth start/callback/device-poll flows it sits alongside, it always requires (and validates) a real -// session bearer token to do anything useful, so it should rate-limit per SESSION like any other authenticated +// These /v1/auth/* paths are excluded from the broad /v1/auth/ prefix match below: unlike the OAuth +// start/callback/device-poll flows they sit alongside, each always requires (and validates) a real session +// bearer token to do anything useful, so they should rate-limit per SESSION like any other authenticated // route -- not per IP, which would let a caller with a stolen session token bypass the strict 10/min cap by // rotating source IPs, and would let unrelated sessions behind one NAT (a shared office network, CI infra) // throttle each other. +// /v1/auth/github/token (#6114/#6115/#6117): fetches the session's live GitHub token. +// /v1/auth/extension/session (#556): mints a new extension-scoped session from an existing one. +const SESSION_AUTHENTICATED_AUTH_PATHS = new Set(["/v1/auth/github/token", "/v1/auth/extension/session"]); + function isPreAuthRateLimitPath(path: string): boolean { return ( (path === "/health" || path === "/v1/mcp/compatibility" || path === "/openapi.json" || path === "/mcp" || path.startsWith("/v1/auth/") || path === "/v1/github/webhook") && - path !== "/v1/auth/github/token" + !SESSION_AUTHENTICATED_AUTH_PATHS.has(path) ); } diff --git a/test/unit/auth.test.ts b/test/unit/auth.test.ts index 5478e247ba..e45a745257 100644 --- a/test/unit/auth.test.ts +++ b/test/unit/auth.test.ts @@ -217,6 +217,42 @@ describe("private-beta auth and rate limiting", () => { expect(observedKeys[0]).toMatch(/^strict:\/v1\/auth\/github\/token:ip:/); }); + it("keys /v1/auth/extension/session by SESSION, not by IP -- same pre-existing gap as #6117, fixed the same way", async () => { + const observedKeys: string[] = []; + const env = rateLimitTestEnv({}, observedKeys); + const { token: sessionToken } = await createSessionForGitHubUser(env, { login: "jsonbored", id: 42 }); + + // The same session's token from two DIFFERENT IPs shares one bucket -- a stolen token can't be used to + // bypass the strict cap by rotating source IPs. + await expect( + enforceRateLimit(fakeContext(env, "/v1/auth/extension/session", { authorization: `Bearer ${sessionToken}`, "cf-connecting-ip": "203.0.113.9" }), "strict"), + ).resolves.toBeNull(); + await expect( + enforceRateLimit(fakeContext(env, "/v1/auth/extension/session", { authorization: `Bearer ${sessionToken}`, "cf-connecting-ip": "198.51.100.50" }), "strict"), + ).resolves.toBeNull(); + expect(observedKeys).toHaveLength(2); + expect(observedKeys[0]).toBe(observedKeys[1]); + expect(observedKeys[0]).toMatch(/^strict:\/v1\/auth\/extension\/session:token:/); + const firstSessionKey = observedKeys[0]; + + // A DIFFERENT session's token from the SAME IP gets its own independent bucket -- unrelated sessions + // behind one NAT/CI-runner IP don't throttle each other. + observedKeys.length = 0; + const { token: otherSessionToken } = await createSessionForGitHubUser(env, { login: "other-user", id: 43 }); + await expect( + enforceRateLimit(fakeContext(env, "/v1/auth/extension/session", { authorization: `Bearer ${otherSessionToken}`, "cf-connecting-ip": "203.0.113.9" }), "strict"), + ).resolves.toBeNull(); + expect(observedKeys[0]).toMatch(/^strict:\/v1\/auth\/extension\/session:token:/); + expect(observedKeys[0]).not.toBe(firstSessionKey); + + // No/invalid bearer still falls back to IP-keying (the pre-auth default), matching every other route. + observedKeys.length = 0; + await expect( + enforceRateLimit(fakeContext(env, "/v1/auth/extension/session", { "cf-connecting-ip": "203.0.113.9" }), "strict"), + ).resolves.toBeNull(); + expect(observedKeys[0]).toMatch(/^strict:\/v1\/auth\/extension\/session:ip:/); + }); + it("ignores proxy fallback headers when cf-connecting-ip is absent", async () => { const observedKeys: string[] = []; const env = rateLimitTestEnv({}, observedKeys);