Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/auth/rate-limit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
}
36 changes: 36 additions & 0 deletions test/unit/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down