diff --git a/src/auth/rate-limit.ts b/src/auth/rate-limit.ts index 6b4e36e268..825d1261e5 100644 --- a/src/auth/rate-limit.ts +++ b/src/auth/rate-limit.ts @@ -221,6 +221,15 @@ 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 +// 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. 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"; + 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" + ); } diff --git a/test/unit/auth.test.ts b/test/unit/auth.test.ts index c826b43f1a..5478e247ba 100644 --- a/test/unit/auth.test.ts +++ b/test/unit/auth.test.ts @@ -112,6 +112,7 @@ describe("private-beta auth and rate limiting", () => { expect(routeClassForPath("/v1/github/webhook")).toBe("strict"); expect(routeClassForPath("/v1/orb/ingest")).toBe("strict"); // open telemetry ingest — abuse-capped per IP expect(routeClassForPath("/v1/auth/github/device/start")).toBe("strict"); + expect(routeClassForPath("/v1/auth/github/token")).toBe("strict"); // #6117: same strict cap as the rest of /v1/auth/* expect(routeClassForPath("/v1/local/branch-analysis")).toBe("expensive"); expect(routeClassForPath("/loopover/shot")).toBe("expensive"); expect(routeClassForPath("/v1/scoring/preview")).toBe("expensive"); @@ -180,6 +181,42 @@ describe("private-beta auth and rate limiting", () => { expect(observedKeys[0]).toMatch(/^normal:\/v1\/public\/github\/repos\/:owner\/:repo\/stats:ip:/); }); + it("keys /v1/auth/github/token by SESSION, not by IP (#6117) -- unlike its pre-auth OAuth-flow siblings", 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/github/token", { authorization: `Bearer ${sessionToken}`, "cf-connecting-ip": "203.0.113.9" }), "strict"), + ).resolves.toBeNull(); + await expect( + enforceRateLimit(fakeContext(env, "/v1/auth/github/token", { 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\/github\/token: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/github/token", { authorization: `Bearer ${otherSessionToken}`, "cf-connecting-ip": "203.0.113.9" }), "strict"), + ).resolves.toBeNull(); + expect(observedKeys[0]).toMatch(/^strict:\/v1\/auth\/github\/token: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/github/token", { "cf-connecting-ip": "203.0.113.9" }), "strict"), + ).resolves.toBeNull(); + expect(observedKeys[0]).toMatch(/^strict:\/v1\/auth\/github\/token:ip:/); + }); + it("ignores proxy fallback headers when cf-connecting-ip is absent", async () => { const observedKeys: string[] = []; const env = rateLimitTestEnv({}, observedKeys);