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
11 changes: 10 additions & 1 deletion src/auth/rate-limit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
}
37 changes: 37 additions & 0 deletions test/unit/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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);
Expand Down