From 1e38c8cc9734525d755074adc829e92f6baee33b Mon Sep 17 00:00:00 2001 From: andriypolanski Date: Mon, 8 Jun 2026 10:46:54 -0400 Subject: [PATCH 1/5] fix(auth): resolve client IP from proxy headers for pre-auth rate limits --- src/auth/rate-limit.ts | 22 +++++++++++++++++- test/unit/auth.test.ts | 53 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/src/auth/rate-limit.ts b/src/auth/rate-limit.ts index c30fd7904e..17c782302d 100644 --- a/src/auth/rate-limit.ts +++ b/src/auth/rate-limit.ts @@ -146,7 +146,27 @@ async function validateBearerForRateLimit(c: Context<{ Bindings: Env }>, token: } function clientIp(c: Context<{ Bindings: Env }>): string { - return c.req.header("cf-connecting-ip")?.trim() || "unknown-ip"; + return ( + firstUsableIp([ + c.req.header("cf-connecting-ip"), + firstForwardedFor(c.req.header("x-forwarded-for")), + c.req.header("x-real-ip"), + ]) ?? "unknown-ip" + ); +} + +function firstUsableIp(candidates: Array): string | undefined { + for (const candidate of candidates) { + const trimmed = candidate?.trim(); + if (trimmed) return trimmed; + } + return undefined; +} + +function firstForwardedFor(header: string | undefined): string | undefined { + if (!header?.trim()) return undefined; + const first = header.split(",")[0]?.trim(); + return first || undefined; } function isPreAuthRateLimitPath(path: string): boolean { diff --git a/test/unit/auth.test.ts b/test/unit/auth.test.ts index e7051d885d..0f90f32fdc 100644 --- a/test/unit/auth.test.ts +++ b/test/unit/auth.test.ts @@ -140,17 +140,68 @@ describe("private-beta auth and rate limiting", () => { expect(observedKeys[0]).toMatch(/^normal:\/v1\/public\/github\/repos\/:owner\/:repo\/stats:ip:/); }); + it("keys pre-auth routes by proxy fallback headers when cf-connecting-ip is absent", async () => { + const observedKeys: string[] = []; + const env = createTestEnv({ RATE_LIMITER: rateLimiterNamespace({ status: 200, body: {} }, observedKeys) as unknown as DurableObjectNamespace }); + + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { "x-forwarded-for": "198.51.100.1" }), + "strict", + ), + ).resolves.toBeNull(); + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { "x-forwarded-for": "198.51.100.2" }), + "strict", + ), + ).resolves.toBeNull(); + expect(observedKeys).toHaveLength(2); + expect(observedKeys[0]).not.toBe(observedKeys[1]); + expect(observedKeys[0]).toMatch(/^strict:\/v1\/auth\/github\/session:ip:/); + + observedKeys.length = 0; + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { "x-forwarded-for": "198.51.100.2, 198.51.100.3" }), + "strict", + ), + ).resolves.toBeNull(); + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { "x-forwarded-for": "198.51.100.2" }), + "strict", + ), + ).resolves.toBeNull(); + expect(observedKeys).toHaveLength(2); + expect(observedKeys[0]).toBe(observedKeys[1]); + + observedKeys.length = 0; + await expect( + enforceRateLimit(fakeContext(env, "/v1/auth/github/session", { "x-real-ip": "203.0.113.44" }), "strict"), + ).resolves.toBeNull(); + await expect( + enforceRateLimit(fakeContext(env, "/v1/auth/github/session", { "x-forwarded-for": "", "x-real-ip": " " }), "strict"), + ).resolves.toBeNull(); + expect(observedKeys).toHaveLength(2); + expect(observedKeys[0]).not.toBe(observedKeys[1]); + expect(observedKeys[1]).toMatch(/^strict:\/v1\/auth\/github\/session:ip:/); + }); + it("enforces route limits with session and IP keys plus retry headers", async () => { const env = createTestEnv(); const noLimiter = fakeContext(env, "/v1/repos/123/pulls/456", { authorization: "Bearer session-token" }); await expect(enforceRateLimit(noLimiter, "normal")).resolves.toBeNull(); + const fallbackObservedKeys: string[] = []; const fallbackHeaders = fakeContext( - createTestEnv({ RATE_LIMITER: rateLimiterNamespace({ status: 200, body: {} }) as unknown as DurableObjectNamespace }), + createTestEnv({ RATE_LIMITER: rateLimiterNamespace({ status: 200, body: {} }, fallbackObservedKeys) as unknown as DurableObjectNamespace }), "/v1/repos/JSONbored/gittensory", { "x-forwarded-for": "198.51.100.2, 198.51.100.3" }, ); await expect(enforceRateLimit(fallbackHeaders, "normal")).resolves.toBeNull(); + expect(fallbackObservedKeys).toHaveLength(1); + expect(fallbackObservedKeys[0]).toMatch(/^normal:\/v1\/repos\/JSONbored\/gittensory:ip:/); expect(fallbackHeaders.res.headers.get("x-ratelimit-limit")).toBe("120"); expect(fallbackHeaders.res.headers.get("x-ratelimit-remaining")).toBe("120"); expect(fallbackHeaders.res.headers.get("x-ratelimit-reset")).toBeNull(); From 2bc2530757e28e645a3fa4265907ffc465b122cd Mon Sep 17 00:00:00 2001 From: andriypolanski Date: Wed, 10 Jun 2026 14:41:14 -0400 Subject: [PATCH 2/5] fix(auth): validate proxy client IPs for pre-auth rate limits --- package-lock.json | 155 +++++++---------------------------------- src/auth/rate-limit.ts | 62 +++++++++++++---- test/unit/auth.test.ts | 120 +++++++++++++++++++++++++++++++ 3 files changed, 194 insertions(+), 143 deletions(-) diff --git a/package-lock.json b/package-lock.json index 59d27d7fab..ee07d1fa69 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1498,6 +1498,7 @@ "cpu": [ "ppc64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1514,6 +1515,7 @@ "cpu": [ "arm" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1530,6 +1532,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1546,6 +1549,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1562,6 +1566,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1578,6 +1583,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1594,6 +1600,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1610,6 +1617,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1626,6 +1634,7 @@ "cpu": [ "arm" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1642,6 +1651,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1658,6 +1668,7 @@ "cpu": [ "ia32" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1674,6 +1685,7 @@ "cpu": [ "loong64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1690,6 +1702,7 @@ "cpu": [ "mips64el" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1706,6 +1719,7 @@ "cpu": [ "ppc64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1722,6 +1736,7 @@ "cpu": [ "riscv64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1738,6 +1753,7 @@ "cpu": [ "s390x" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1754,6 +1770,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1770,6 +1787,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1786,6 +1804,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1802,6 +1821,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1818,6 +1838,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1834,6 +1855,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1850,6 +1872,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1866,6 +1889,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1882,6 +1906,7 @@ "cpu": [ "ia32" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1898,6 +1923,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2314,9 +2340,6 @@ "arm" ], "dev": true, - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -2334,9 +2357,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -2354,9 +2374,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -2374,9 +2391,6 @@ "riscv64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -2394,9 +2408,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -2414,9 +2425,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -2434,9 +2442,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -2454,9 +2459,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -2474,9 +2476,6 @@ "arm" ], "dev": true, - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -2500,9 +2499,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -2526,9 +2522,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -2552,9 +2545,6 @@ "riscv64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -2578,9 +2568,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -2604,9 +2591,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -2630,9 +2614,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -2656,9 +2637,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -4905,9 +4883,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -4924,9 +4899,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -4943,9 +4915,6 @@ "cpu": [ "ppc64" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -4962,9 +4931,6 @@ "cpu": [ "s390x" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -4981,9 +4947,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -5000,9 +4963,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -5206,9 +5166,6 @@ "arm" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -5223,9 +5180,6 @@ "arm" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -5240,9 +5194,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -5257,9 +5208,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -5274,9 +5222,6 @@ "loong64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -5291,9 +5236,6 @@ "loong64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -5308,9 +5250,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -5325,9 +5264,6 @@ "ppc64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -5342,9 +5278,6 @@ "riscv64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -5359,9 +5292,6 @@ "riscv64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -5376,9 +5306,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -5393,9 +5320,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -5410,9 +5334,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -5689,9 +5610,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -5708,9 +5626,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -5727,9 +5642,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -5746,9 +5658,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -10237,9 +10146,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -10260,9 +10166,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -10283,9 +10186,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -10306,9 +10206,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ diff --git a/src/auth/rate-limit.ts b/src/auth/rate-limit.ts index 17c782302d..d5722748df 100644 --- a/src/auth/rate-limit.ts +++ b/src/auth/rate-limit.ts @@ -146,27 +146,61 @@ async function validateBearerForRateLimit(c: Context<{ Bindings: Env }>, token: } function clientIp(c: Context<{ Bindings: Env }>): string { - return ( - firstUsableIp([ - c.req.header("cf-connecting-ip"), - firstForwardedFor(c.req.header("x-forwarded-for")), - c.req.header("x-real-ip"), - ]) ?? "unknown-ip" - ); + const candidates: Array = [ + c.req.header("cf-connecting-ip"), + ...forwardedForCandidates(c.req.header("x-forwarded-for")), + c.req.header("x-real-ip"), + ]; + return firstValidIp(candidates) ?? "unknown-ip"; +} + +function forwardedForCandidates(header: string | undefined): string[] { + if (!header?.trim()) return []; + return header.split(",").map((part) => part.trim()).filter(Boolean); } -function firstUsableIp(candidates: Array): string | undefined { +function firstValidIp(candidates: Array): string | undefined { for (const candidate of candidates) { - const trimmed = candidate?.trim(); - if (trimmed) return trimmed; + const valid = normalizeIpAddress(candidate); + if (valid) return valid; } return undefined; } -function firstForwardedFor(header: string | undefined): string | undefined { - if (!header?.trim()) return undefined; - const first = header.split(",")[0]?.trim(); - return first || undefined; +function normalizeIpAddress(value: string | undefined): string | undefined { + const trimmed = value?.trim(); + if (!trimmed || !isValidIpAddress(trimmed)) return undefined; + if (trimmed.startsWith("[") && trimmed.endsWith("]")) return trimmed.slice(1, -1); + return trimmed; +} + +function isValidIpAddress(value: string): boolean { + return isValidIpv4(value) || isValidIpv6(value); +} + +function isValidIpv4(value: string): boolean { + const parts = value.split("."); + if (parts.length !== 4) return false; + for (const part of parts) { + if (!/^\d{1,3}$/.test(part)) return false; + const octet = Number(part); + if (octet < 0 || octet > 255) return false; + } + return true; +} + +function isValidIpv6(value: string): boolean { + let candidate = value; + if (candidate.startsWith("[") && candidate.endsWith("]")) candidate = candidate.slice(1, -1); + if (!candidate.includes(":") || !/^[0-9a-fA-F:.]+$/.test(candidate)) return false; + if (candidate.split("::").length > 2) return false; + const segments = candidate.split(":"); + if (segments.length > 8) return false; + for (const segment of segments) { + if (segment === "") continue; + if (!/^[0-9a-fA-F]{1,4}$/.test(segment)) return false; + } + return true; } function isPreAuthRateLimitPath(path: string): boolean { diff --git a/test/unit/auth.test.ts b/test/unit/auth.test.ts index 0f90f32fdc..ac4456530d 100644 --- a/test/unit/auth.test.ts +++ b/test/unit/auth.test.ts @@ -188,6 +188,126 @@ describe("private-beta auth and rate limiting", () => { expect(observedKeys[1]).toMatch(/^strict:\/v1\/auth\/github\/session:ip:/); }); + it("ignores malformed client address headers when building rate-limit keys", async () => { + const observedKeys: string[] = []; + const env = createTestEnv({ RATE_LIMITER: rateLimiterNamespace({ status: 200, body: {} }, observedKeys) as unknown as DurableObjectNamespace }); + + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { + "cf-connecting-ip": "not-an-ip", + "x-forwarded-for": "198.51.100.2, 198.51.100.3", + }), + "strict", + ), + ).resolves.toBeNull(); + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { + "x-forwarded-for": "198.51.100.2", + }), + "strict", + ), + ).resolves.toBeNull(); + expect(observedKeys).toHaveLength(2); + expect(observedKeys[0]).toBe(observedKeys[1]); + + observedKeys.length = 0; + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { + "x-forwarded-for": "garbage, also-not-ip", + "x-real-ip": "203.0.113.44", + }), + "strict", + ), + ).resolves.toBeNull(); + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { + "x-real-ip": "203.0.113.44", + }), + "strict", + ), + ).resolves.toBeNull(); + expect(observedKeys).toHaveLength(2); + expect(observedKeys[0]).toBe(observedKeys[1]); + + observedKeys.length = 0; + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { + "cf-connecting-ip": "999.999.999.999", + "x-forwarded-for": "still-not-ip", + "x-real-ip": "also-invalid", + }), + "strict", + ), + ).resolves.toBeNull(); + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { + "x-forwarded-for": "attacker-controlled-bucket", + }), + "strict", + ), + ).resolves.toBeNull(); + expect(observedKeys).toHaveLength(2); + expect(observedKeys[0]).toBe(observedKeys[1]); + expect(observedKeys[0]).toMatch(/^strict:\/v1\/auth\/github\/session:ip:/); + + observedKeys.length = 0; + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { + "cf-connecting-ip": "203.0.113.9", + "x-forwarded-for": "198.51.100.1", + "x-real-ip": "198.51.100.99", + }), + "strict", + ), + ).resolves.toBeNull(); + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { + "x-forwarded-for": "not-an-ip, 198.51.100.55", + }), + "strict", + ), + ).resolves.toBeNull(); + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { + "x-forwarded-for": "198.51.100.55", + }), + "strict", + ), + ).resolves.toBeNull(); + expect(observedKeys).toHaveLength(3); + expect(observedKeys[0]).not.toBe(observedKeys[1]); + expect(observedKeys[1]).toBe(observedKeys[2]); + + observedKeys.length = 0; + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { + "x-real-ip": "[2001:db8::1]", + }), + "strict", + ), + ).resolves.toBeNull(); + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { + "x-real-ip": "2001:db8::1", + }), + "strict", + ), + ).resolves.toBeNull(); + expect(observedKeys).toHaveLength(2); + expect(observedKeys[0]).toBe(observedKeys[1]); + }); + it("enforces route limits with session and IP keys plus retry headers", async () => { const env = createTestEnv(); const noLimiter = fakeContext(env, "/v1/repos/123/pulls/456", { authorization: "Bearer session-token" }); From 69ff97a1fdb2b9ad3217b37b00bdfbb25d31a8e4 Mon Sep 17 00:00:00 2001 From: andriypolanski Date: Thu, 11 Jun 2026 08:21:51 -0400 Subject: [PATCH 3/5] fix(auth): validate proxy client IPs for pre-auth rate limits --- src/auth/rate-limit.ts | 4 +++- test/unit/auth.test.ts | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/auth/rate-limit.ts b/src/auth/rate-limit.ts index 6e8f844d56..b707c7f36b 100644 --- a/src/auth/rate-limit.ts +++ b/src/auth/rate-limit.ts @@ -197,11 +197,13 @@ function isValidIpv6(value: string): boolean { if (candidate.split("::").length > 2) return false; const segments = candidate.split(":"); if (segments.length > 8) return false; + let hasHexSegment = false; for (const segment of segments) { if (segment === "") continue; if (!/^[0-9a-fA-F]{1,4}$/.test(segment)) return false; + hasHexSegment = true; } - return true; + return hasHexSegment; } function isPreAuthRateLimitPath(path: string): boolean { diff --git a/test/unit/auth.test.ts b/test/unit/auth.test.ts index da5d5fb451..d750e23571 100644 --- a/test/unit/auth.test.ts +++ b/test/unit/auth.test.ts @@ -307,6 +307,34 @@ describe("private-beta auth and rate limiting", () => { ).resolves.toBeNull(); expect(observedKeys).toHaveLength(2); expect(observedKeys[0]).toBe(observedKeys[1]); + + observedKeys.length = 0; + await expect(enforceRateLimit(fakeContext(env, "/v1/auth/github/session"), "strict")).resolves.toBeNull(); + const unknownIpKey = observedKeys[0]; + + observedKeys.length = 0; + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { + "cf-connecting-ip": "1.2.3.abc", + "x-forwarded-for": "256.0.0.1, 1.2.3", + "x-real-ip": "1::2::3", + }), + "strict", + ), + ).resolves.toBeNull(); + expect(observedKeys[0]).toBe(unknownIpKey); + + observedKeys.length = 0; + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { + "x-forwarded-for": "1:2:3:4:5:6:7:8:9:0, xyz::1", + }), + "strict", + ), + ).resolves.toBeNull(); + expect(observedKeys[0]).toBe(unknownIpKey); }); it("enforces route limits with session and IP keys plus retry headers", async () => { From 8babdbd4b5777510ef21a8af6b753eb5f282b5f5 Mon Sep 17 00:00:00 2001 From: andriypolanski Date: Thu, 11 Jun 2026 08:45:54 -0400 Subject: [PATCH 4/5] fix: gate forwarded IP haders on trusted proxy signals --- src/auth/rate-limit.ts | 47 +++++++++++++-- src/env.d.ts | 2 + test/unit/auth.test.ts | 126 +++++++++++++++++++++++++++++++---------- 3 files changed, 139 insertions(+), 36 deletions(-) diff --git a/src/auth/rate-limit.ts b/src/auth/rate-limit.ts index b707c7f36b..c57222695d 100644 --- a/src/auth/rate-limit.ts +++ b/src/auth/rate-limit.ts @@ -147,12 +147,47 @@ async function validateBearerForRateLimit(c: Context<{ Bindings: Env }>, token: } function clientIp(c: Context<{ Bindings: Env }>): string { - const candidates: Array = [ - c.req.header("cf-connecting-ip"), - ...forwardedForCandidates(c.req.header("x-forwarded-for")), - c.req.header("x-real-ip"), - ]; - return firstValidIp(candidates) ?? "unknown-ip"; + const cloudflareIp = normalizeIpAddress(c.req.header("cf-connecting-ip")); + if (cloudflareIp) return cloudflareIp; + + if (!isTrustedProxyRequest(c)) return "unknown-ip"; + + const proxyCount = trustedProxyCount(c.env.RATE_LIMIT_TRUSTED_PROXY_COUNT); + return ( + firstValidIp([ + c.req.header("x-real-ip"), + trustedForwardedForIp(c.req.header("x-forwarded-for"), proxyCount), + ]) ?? "unknown-ip" + ); +} + +function isTrustedProxyRequest(c: Context<{ Bindings: Env }>): boolean { + if (c.req.header("cf-ray")?.trim()) return true; + const trustedProxies = parseTrustedProxyList(c.env.RATE_LIMIT_TRUSTED_PROXIES); + if (trustedProxies.length === 0) return false; + const chain = forwardedForCandidates(c.req.header("x-forwarded-for")) + .map((entry) => normalizeIpAddress(entry)) + .filter((entry): entry is string => Boolean(entry)); + const peer = chain[chain.length - 1]; + return peer ? trustedProxies.includes(peer) : false; +} + +function parseTrustedProxyList(value: string | undefined): string[] { + return (value ?? "") + .split(",") + .map((entry) => normalizeIpAddress(entry)) + .filter((entry): entry is string => Boolean(entry)); +} + +function trustedProxyCount(value: string | undefined): number { + const parsed = Number(value?.trim()); + return Number.isInteger(parsed) && parsed > 0 ? parsed : 1; +} + +function trustedForwardedForIp(header: string | undefined, trustedProxyCountValue: number): string | undefined { + const chain = forwardedForCandidates(header); + if (chain.length < trustedProxyCountValue) return undefined; + return chain[chain.length - trustedProxyCountValue]; } function forwardedForCandidates(header: string | undefined): string[] { diff --git a/src/env.d.ts b/src/env.d.ts index d1950955e2..f8c1094c0b 100644 --- a/src/env.d.ts +++ b/src/env.d.ts @@ -31,6 +31,8 @@ declare global { GITTENSORY_API_TOKEN: string; GITTENSORY_MCP_TOKEN: string; INTERNAL_JOB_TOKEN: string; + RATE_LIMIT_TRUSTED_PROXIES?: string; + RATE_LIMIT_TRUSTED_PROXY_COUNT?: string; } } diff --git a/test/unit/auth.test.ts b/test/unit/auth.test.ts index d750e23571..3bbc60d651 100644 --- a/test/unit/auth.test.ts +++ b/test/unit/auth.test.ts @@ -147,13 +147,13 @@ describe("private-beta auth and rate limiting", () => { await expect( enforceRateLimit( - fakeContext(env, "/v1/auth/github/session", { "x-forwarded-for": "198.51.100.1" }), + fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-forwarded-for": "198.51.100.1" })), "strict", ), ).resolves.toBeNull(); await expect( enforceRateLimit( - fakeContext(env, "/v1/auth/github/session", { "x-forwarded-for": "198.51.100.2" }), + fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-forwarded-for": "198.51.100.2" })), "strict", ), ).resolves.toBeNull(); @@ -164,13 +164,13 @@ describe("private-beta auth and rate limiting", () => { observedKeys.length = 0; await expect( enforceRateLimit( - fakeContext(env, "/v1/auth/github/session", { "x-forwarded-for": "198.51.100.2, 198.51.100.3" }), + fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-forwarded-for": "198.51.100.2, 198.51.100.3" })), "strict", ), ).resolves.toBeNull(); await expect( enforceRateLimit( - fakeContext(env, "/v1/auth/github/session", { "x-forwarded-for": "198.51.100.2" }), + fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-forwarded-for": "198.51.100.3" })), "strict", ), ).resolves.toBeNull(); @@ -179,25 +179,42 @@ describe("private-beta auth and rate limiting", () => { observedKeys.length = 0; await expect( - enforceRateLimit(fakeContext(env, "/v1/auth/github/session", { "x-real-ip": "203.0.113.44" }), "strict"), + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-real-ip": "203.0.113.44", "x-forwarded-for": "198.51.100.99" })), + "strict", + ), ).resolves.toBeNull(); await expect( - enforceRateLimit(fakeContext(env, "/v1/auth/github/session", { "x-forwarded-for": "", "x-real-ip": " " }), "strict"), + enforceRateLimit(fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-real-ip": "203.0.113.44" })), "strict"), ).resolves.toBeNull(); expect(observedKeys).toHaveLength(2); - expect(observedKeys[0]).not.toBe(observedKeys[1]); - expect(observedKeys[1]).toMatch(/^strict:\/v1\/auth\/github\/session:ip:/); + expect(observedKeys[0]).toBe(observedKeys[1]); + + observedKeys.length = 0; + await expect( + enforceRateLimit(fakeContext(env, "/v1/auth/github/session", { "x-forwarded-for": "198.51.100.1" }), "strict"), + ).resolves.toBeNull(); + await expect( + enforceRateLimit(fakeContext(env, "/v1/auth/github/session", { "x-forwarded-for": "198.51.100.2" }), "strict"), + ).resolves.toBeNull(); + expect(observedKeys).toHaveLength(2); + expect(observedKeys[0]).toBe(observedKeys[1]); + expect(observedKeys[0]).toMatch(/^strict:\/v1\/auth\/github\/session:ip:/); }); - it("ignores malformed client address headers when building rate-limit keys", async () => { + it("keys pre-auth routes by configured trusted proxy IPs without cf-ray", async () => { const observedKeys: string[] = []; - const env = createTestEnv({ RATE_LIMITER: rateLimiterNamespace({ status: 200, body: {} }, observedKeys) as unknown as DurableObjectNamespace }); + const env = createTestEnv({ + RATE_LIMITER: rateLimiterNamespace({ status: 200, body: {} }, observedKeys) as unknown as DurableObjectNamespace, + RATE_LIMIT_TRUSTED_PROXIES: "198.51.100.99", + RATE_LIMIT_TRUSTED_PROXY_COUNT: "2", + }); await expect( enforceRateLimit( fakeContext(env, "/v1/auth/github/session", { - "cf-connecting-ip": "not-an-ip", - "x-forwarded-for": "198.51.100.2, 198.51.100.3", + "x-forwarded-for": "198.51.100.2, 198.51.100.99", + "x-real-ip": "198.51.100.2", }), "strict", ), @@ -205,29 +222,71 @@ describe("private-beta auth and rate limiting", () => { await expect( enforceRateLimit( fakeContext(env, "/v1/auth/github/session", { - "x-forwarded-for": "198.51.100.2", + "x-forwarded-for": "198.51.100.2, 198.51.100.99", }), "strict", ), ).resolves.toBeNull(); expect(observedKeys).toHaveLength(2); expect(observedKeys[0]).toBe(observedKeys[1]); + expect(observedKeys[0]).toMatch(/^strict:\/v1\/auth\/github\/session:ip:/); + }); + + it("ignores forwarded headers when configured trusted proxy peer is absent", async () => { + const observedKeys: string[] = []; + const env = createTestEnv({ + RATE_LIMITER: rateLimiterNamespace({ status: 200, body: {} }, observedKeys) as unknown as DurableObjectNamespace, + RATE_LIMIT_TRUSTED_PROXIES: "198.51.100.99", + }); + + await expect(enforceRateLimit(fakeContext(env, "/v1/auth/github/session"), "strict")).resolves.toBeNull(); + const unknownIpKey = observedKeys[0]; observedKeys.length = 0; await expect( enforceRateLimit( - fakeContext(env, "/v1/auth/github/session", { + fakeContext(env, "/v1/auth/github/session", { "x-forwarded-for": "198.51.100.1, 198.51.100.2", "x-real-ip": "198.51.100.3" }), + "strict", + ), + ).resolves.toBeNull(); + expect(observedKeys[0]).toBe(unknownIpKey); + }); + + it("ignores malformed client address headers when building rate-limit keys", async () => { + const observedKeys: string[] = []; + const env = createTestEnv({ RATE_LIMITER: rateLimiterNamespace({ status: 200, body: {} }, observedKeys) as unknown as DurableObjectNamespace }); + + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ + "cf-connecting-ip": "not-an-ip", + "x-forwarded-for": "198.51.100.2, 198.51.100.3", + })), + "strict", + ), + ).resolves.toBeNull(); + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-forwarded-for": "198.51.100.3" })), + "strict", + ), + ).resolves.toBeNull(); + expect(observedKeys).toHaveLength(2); + expect(observedKeys[0]).toBe(observedKeys[1]); + + observedKeys.length = 0; + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-forwarded-for": "garbage, also-not-ip", "x-real-ip": "203.0.113.44", - }), + })), "strict", ), ).resolves.toBeNull(); await expect( enforceRateLimit( - fakeContext(env, "/v1/auth/github/session", { - "x-real-ip": "203.0.113.44", - }), + fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-real-ip": "203.0.113.44" })), "strict", ), ).resolves.toBeNull(); @@ -270,17 +329,15 @@ describe("private-beta auth and rate limiting", () => { ).resolves.toBeNull(); await expect( enforceRateLimit( - fakeContext(env, "/v1/auth/github/session", { + fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-forwarded-for": "not-an-ip, 198.51.100.55", - }), + })), "strict", ), ).resolves.toBeNull(); await expect( enforceRateLimit( - fakeContext(env, "/v1/auth/github/session", { - "x-forwarded-for": "198.51.100.55", - }), + fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-forwarded-for": "198.51.100.55" })), "strict", ), ).resolves.toBeNull(); @@ -291,17 +348,13 @@ describe("private-beta auth and rate limiting", () => { observedKeys.length = 0; await expect( enforceRateLimit( - fakeContext(env, "/v1/auth/github/session", { - "x-real-ip": "[2001:db8::1]", - }), + fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-real-ip": "[2001:db8::1]" })), "strict", ), ).resolves.toBeNull(); await expect( enforceRateLimit( - fakeContext(env, "/v1/auth/github/session", { - "x-real-ip": "2001:db8::1", - }), + fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-real-ip": "2001:db8::1" })), "strict", ), ).resolves.toBeNull(); @@ -312,6 +365,15 @@ describe("private-beta auth and rate limiting", () => { await expect(enforceRateLimit(fakeContext(env, "/v1/auth/github/session"), "strict")).resolves.toBeNull(); const unknownIpKey = observedKeys[0]; + observedKeys.length = 0; + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-forwarded-for": "", "x-real-ip": " " })), + "strict", + ), + ).resolves.toBeNull(); + expect(observedKeys[0]).toBe(unknownIpKey); + observedKeys.length = 0; await expect( enforceRateLimit( @@ -346,7 +408,7 @@ describe("private-beta auth and rate limiting", () => { const fallbackHeaders = fakeContext( createTestEnv({ RATE_LIMITER: rateLimiterNamespace({ status: 200, body: {} }, fallbackObservedKeys) as unknown as DurableObjectNamespace }), "/v1/repos/JSONbored/gittensory", - { "x-forwarded-for": "198.51.100.2, 198.51.100.3" }, + trustedProxyHeaders({ "x-forwarded-for": "198.51.100.2, 198.51.100.3" }), ); await expect(enforceRateLimit(fallbackHeaders, "normal")).resolves.toBeNull(); expect(fallbackObservedKeys).toHaveLength(1); @@ -748,3 +810,7 @@ function fakeContext(env: Env, path: string, headers: Record = { }, } as unknown as import("hono").Context<{ Bindings: Env }> & { res: { headers: Headers } }; } + +function trustedProxyHeaders(headers: Record = {}): Record { + return { "cf-ray": "test-trusted-edge", ...headers }; +} From a5dff2624703d76818192416497faa92fe6db272 Mon Sep 17 00:00:00 2001 From: andriypolanski Date: Thu, 11 Jun 2026 17:01:59 -0400 Subject: [PATCH 5/5] fix(auth): require explicit trusted proxies for forwarded client IPs --- src/auth/rate-limit.ts | 1 - test/unit/auth.test.ts | 59 ++++++++++++++++++++++++++++++++++++------ 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/auth/rate-limit.ts b/src/auth/rate-limit.ts index c57222695d..99ae735a46 100644 --- a/src/auth/rate-limit.ts +++ b/src/auth/rate-limit.ts @@ -162,7 +162,6 @@ function clientIp(c: Context<{ Bindings: Env }>): string { } function isTrustedProxyRequest(c: Context<{ Bindings: Env }>): boolean { - if (c.req.header("cf-ray")?.trim()) return true; const trustedProxies = parseTrustedProxyList(c.env.RATE_LIMIT_TRUSTED_PROXIES); if (trustedProxies.length === 0) return false; const chain = forwardedForCandidates(c.req.header("x-forwarded-for")) diff --git a/test/unit/auth.test.ts b/test/unit/auth.test.ts index 3bbc60d651..6730afd088 100644 --- a/test/unit/auth.test.ts +++ b/test/unit/auth.test.ts @@ -143,7 +143,7 @@ describe("private-beta auth and rate limiting", () => { it("keys pre-auth routes by proxy fallback headers when cf-connecting-ip is absent", async () => { const observedKeys: string[] = []; - const env = createTestEnv({ RATE_LIMITER: rateLimiterNamespace({ status: 200, body: {} }, observedKeys) as unknown as DurableObjectNamespace }); + const env = rateLimitTestEnv({}, observedKeys); await expect( enforceRateLimit( @@ -164,13 +164,13 @@ describe("private-beta auth and rate limiting", () => { observedKeys.length = 0; await expect( enforceRateLimit( - fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-forwarded-for": "198.51.100.2, 198.51.100.3" })), + fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-real-ip": "198.51.100.3", "x-forwarded-for": "198.51.100.2, 198.51.100.3" })), "strict", ), ).resolves.toBeNull(); await expect( enforceRateLimit( - fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-forwarded-for": "198.51.100.3" })), + fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-real-ip": "198.51.100.3" })), "strict", ), ).resolves.toBeNull(); @@ -202,6 +202,30 @@ describe("private-beta auth and rate limiting", () => { expect(observedKeys[0]).toMatch(/^strict:\/v1\/auth\/github\/session:ip:/); }); + it("does not treat spoofed cf-ray as trusted proxy proof", async () => { + const observedKeys: string[] = []; + const env = createTestEnv({ + RATE_LIMITER: rateLimiterNamespace({ status: 200, body: {} }, observedKeys) as unknown as DurableObjectNamespace, + RATE_LIMIT_TRUSTED_PROXIES: TEST_TRUSTED_PROXY, + }); + + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { "cf-ray": "attacker-controlled", "x-forwarded-for": "198.51.100.1" }), + "strict", + ), + ).resolves.toBeNull(); + await expect( + enforceRateLimit( + fakeContext(env, "/v1/auth/github/session", { "cf-ray": "attacker-controlled", "x-forwarded-for": "198.51.100.2" }), + "strict", + ), + ).resolves.toBeNull(); + expect(observedKeys).toHaveLength(2); + expect(observedKeys[0]).toBe(observedKeys[1]); + expect(observedKeys[0]).toMatch(/^strict:\/v1\/auth\/github\/session:ip:/); + }); + it("keys pre-auth routes by configured trusted proxy IPs without cf-ray", async () => { const observedKeys: string[] = []; const env = createTestEnv({ @@ -254,12 +278,13 @@ describe("private-beta auth and rate limiting", () => { it("ignores malformed client address headers when building rate-limit keys", async () => { const observedKeys: string[] = []; - const env = createTestEnv({ RATE_LIMITER: rateLimiterNamespace({ status: 200, body: {} }, observedKeys) as unknown as DurableObjectNamespace }); + const env = rateLimitTestEnv({}, observedKeys); await expect( enforceRateLimit( fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "cf-connecting-ip": "not-an-ip", + "x-real-ip": "198.51.100.2", "x-forwarded-for": "198.51.100.2, 198.51.100.3", })), "strict", @@ -267,7 +292,7 @@ describe("private-beta auth and rate limiting", () => { ).resolves.toBeNull(); await expect( enforceRateLimit( - fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-forwarded-for": "198.51.100.3" })), + fakeContext(env, "/v1/auth/github/session", trustedProxyHeaders({ "x-real-ip": "198.51.100.2" })), "strict", ), ).resolves.toBeNull(); @@ -406,9 +431,9 @@ describe("private-beta auth and rate limiting", () => { const fallbackObservedKeys: string[] = []; const fallbackHeaders = fakeContext( - createTestEnv({ RATE_LIMITER: rateLimiterNamespace({ status: 200, body: {} }, fallbackObservedKeys) as unknown as DurableObjectNamespace }), + rateLimitTestEnv({}, fallbackObservedKeys), "/v1/repos/JSONbored/gittensory", - trustedProxyHeaders({ "x-forwarded-for": "198.51.100.2, 198.51.100.3" }), + trustedProxyHeaders({ "x-real-ip": "198.51.100.3", "x-forwarded-for": "198.51.100.2, 198.51.100.3" }), ); await expect(enforceRateLimit(fallbackHeaders, "normal")).resolves.toBeNull(); expect(fallbackObservedKeys).toHaveLength(1); @@ -794,6 +819,15 @@ function rateLimiterNamespace(decision: { status: number; body: Record = {}, observedKeys?: string[]) { + return createTestEnv({ + RATE_LIMITER: rateLimiterNamespace({ status: 200, body: {} }, observedKeys) as unknown as DurableObjectNamespace, + RATE_LIMIT_TRUSTED_PROXIES: TEST_TRUSTED_PROXY, + RATE_LIMIT_TRUSTED_PROXY_COUNT: "2", + ...overrides, + }); +} + function fakeContext(env: Env, path: string, headers: Record = {}) { const responseHeaders = new Headers(); return { @@ -811,6 +845,15 @@ function fakeContext(env: Env, path: string, headers: Record = { } as unknown as import("hono").Context<{ Bindings: Env }> & { res: { headers: Headers } }; } +const TEST_TRUSTED_PROXY = "198.51.100.99"; + function trustedProxyHeaders(headers: Record = {}): Record { - return { "cf-ray": "test-trusted-edge", ...headers }; + const next = { ...headers }; + const chain = (next["x-forwarded-for"] ?? "") + .split(",") + .map((part) => part.trim()) + .filter(Boolean); + if (!chain.includes(TEST_TRUSTED_PROXY)) chain.push(TEST_TRUSTED_PROXY); + next["x-forwarded-for"] = chain.join(", "); + return next; }