[Bug]: Pre-auth rate limits collapse to `unknown-ip` when `cf-connecting-ip` is absent, so one client can throttle everyone behind fallback proxies

[galuis116]

Summary

enforceRateLimit keys unauthenticated and pre-auth routes by clientIp(c). That helper reads only Cloudflare's cf-connecting-ip header and otherwise returns the literal "unknown-ip":

// src/auth/rate-limit.ts:135-137
async function rateLimitIdentity(c: Context<{ Bindings: Env }>): Promise<string> {
  const ipIdentity = `ip:${await hashToken(clientIp(c))}`;
  if (isPreAuthRateLimitPath(c.req.path)) return ipIdentity;

// src/auth/rate-limit.ts:148-150
function clientIp(c: Context<{ Bindings: Env }>): string {
  return c.req.header("cf-connecting-ip")?.trim() || "unknown-ip";
}

When cf-connecting-ip is absent, every anonymous request on the same route gets the same durable-object bucket:

strict:/v1/auth/github/session:ip:<hash("unknown-ip")>

That is a real availability bug for local deployments, preview/proxy setups, tests, or any nonstandard edge path that forwards x-forwarded-for / x-real-ip but does not inject cf-connecting-ip: one noisy anonymous client can consume the shared "unknown-ip" bucket and rate-limit unrelated users.

Failure mode

Two different clients hit a pre-auth route, for example /v1/auth/github/session, through a proxy that sets only x-forwarded-for:

• client A: x-forwarded-for: 198.51.100.1
• client B: x-forwarded-for: 198.51.100.2
• no cf-connecting-ip

Current behavior:

1. clientIp(c) ignores x-forwarded-for.
2. Both requests become clientIp = "unknown-ip".
3. rateLimitIdentity hashes "unknown-ip" for both.
4. Both requests hit the same Durable Object bucket.
5. Client A can exhaust the strict auth bucket and cause client B to receive 429 rate_limited.

Expected behavior:

• prefer cf-connecting-ip when present
• otherwise use the first valid IP from x-forwarded-for
• otherwise use x-real-ip
• only fall back to "unknown-ip" when no usable client address exists

Reachability

This affects all pre-auth paths because rateLimitIdentity deliberately returns the IP identity before token validation:

// src/auth/rate-limit.ts:135-140
const ipIdentity = `ip:${await hashToken(clientIp(c))}`;
if (isPreAuthRateLimitPath(c.req.path)) return ipIdentity;

The pre-auth set includes user-facing auth and webhook/open surfaces:

// src/auth/rate-limit.ts:152-154
return path === "/health"
  || path === "/v1/mcp/compatibility"
  || path === "/openapi.json"
  || path === "/mcp"
  || path.startsWith("/v1/auth/")
  || path === "/v1/github/webhook";

The test suite already hints at the intended fallback behavior but does not assert it. In test/unit/auth.test.ts, the fallbackHeaders fixture passes only x-forwarded-for:

// test/unit/auth.test.ts:148-153
const fallbackHeaders = fakeContext(
  createTestEnv({ RATE_LIMITER: rateLimiterNamespace({ status: 200, body: {} }) 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();

But the test only verifies response headers. It never captures the generated key, so it misses that the request was keyed as ip:<hash("unknown-ip")> rather than ip:<hash("198.51.100.2")>.

Why this matters

This is not just a cosmetic header parsing gap. The rate limiter is used before authentication on the exact routes where fair per-client isolation matters most:

• GitHub OAuth/device flow start/poll/session endpoints
• webhooks
• public MCP/compatibility/openapi endpoints

If a proxy path lacks cf-connecting-ip, the strict auth bucket can become global for that route. That turns a per-IP abuse control into a cross-user denial-of-service footgun.

Test status

Not locked in.

Existing tests assert:

• two requests with the same cf-connecting-ip share a key even when bearer tokens differ (auth.test.ts:100-123)
• public repo stat paths normalize owner/repo into one path bucket (auth.test.ts:135-140)
• a request containing only x-forwarded-for succeeds and gets default rate-limit response headers (auth.test.ts:148-156)

No test asserts that:

• x-forwarded-for is used when cf-connecting-ip is absent
• the first forwarded IP is selected from a comma-separated chain
• x-real-ip is used as a secondary fallback
• blank/malformed fallback headers do not create unstable or attacker-controlled bucket keys

Expected behavior

Client identity extraction should be deterministic and proxy-aware:

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";
}

The extractor should trim values, use the first x-forwarded-for entry, and ignore empty values. A conservative implementation can avoid full IP validation and still fix the collapse; a stronger one can validate IPv4/IPv6-ish tokens before trusting them.

Actual behavior

clientIp ignores every proxy/client-address header except cf-connecting-ip. Without that one header, all anonymous/pre-auth requests share unknown-ip.

Suggested fix

• Replace clientIp with a small helper that prefers:
  1. cf-connecting-ip
  2. first non-empty entry from x-forwarded-for
  3. x-real-ip
  4. "unknown-ip"
• Normalize/truncate the selected value before hashing so bucket keys remain stable.
• Add fail-on-revert tests in test/unit/auth.test.ts:
  • two pre-auth requests with different x-forwarded-for values and no cf-connecting-ip produce different observed rate-limit keys
  • x-forwarded-for: "198.51.100.2, 198.51.100.3" keys on 198.51.100.2
  • x-real-ip is used when both cf-connecting-ip and x-forwarded-for are absent

Distinct from prior reports

This is not a duplicate of the existing rate-limit/dashboard reports:

• gittensory-philluiz-issue11.md mentions the maintainer dashboard's rate-limit event metric, not route-level rate-limit identity.
• gittensory-philluiz-issue8.md concerns GitHub API sync segments waiting on GitHub REST rate limits, not browser/API request throttling.
• The existing github-type-label fixture references a commit title about trimming bearer tokens in rate-limit keys, but this bug is unrelated to bearer token trimming; it is about anonymous IP fallback collapsing to one shared bucket.

The fix should have a better token profile than a one-line predicate patch because it naturally touches real TypeScript source logic, adds reusable header parsing helpers, and pins multiple route-limit identity cases in tests.

[RealDiligent]

Reopening request: self-host Node still trusts client-spoofable cf-connecting-ip for rate limiting. PR incoming with trusted-client-ip edge normalization (supersedes closed #6550).

[RealDiligent]

PR #7392 (CI green, codecov/patch pass) implements the self-host Node-edge fix for this bug. The PR was auto-closed because this issue is closed as not_planned. Maintainer: please reopen this issue (or file a replacement gittensor:bug) so the security fix can merge.