Skip to content

fix(auth): ignore spoofable proxy IP headers - #613

Merged
JSONbored merged 1 commit into
mainfrom
codex/fix-spoofable-proxy-headers-vulnerability
Jun 13, 2026
Merged

fix(auth): ignore spoofable proxy IP headers#613
JSONbored merged 1 commit into
mainfrom
codex/fix-spoofable-proxy-headers-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The prior change accepted x-real-ip/x-forwarded-for as fallbacks when cf-connecting-ip was missing, which let unauthenticated clients spoof rate-limit identities by supplying attacker-controlled XFF entries.
  • The goal is to prevent unauthenticated clients from creating arbitrary pre-auth rate-limit buckets by trusting only unspoofable peer signals.

Description

  • Removed the trusted-proxy fallback logic from clientIp() in src/auth/rate-limit.ts so the function now only trusts cf-connecting-ip and otherwise returns "unknown-ip".
  • Deleted helper logic that parsed and validated X-Forwarded-For chains and trusted-proxy configuration so header-controlled values are no longer used for rate-limit identities.
  • Updated test/unit/auth.test.ts to assert that proxy headers and configured trusted-proxy values do not produce distinct pre-auth rate-limit keys when cf-connecting-ip is absent.
  • Left token-based identity behavior unchanged so authenticated requests continue to prefer token keys where applicable.

Testing

  • Ran type checking with tsc --noEmit via npm run typecheck, which completed successfully.
  • Ran the focused unit tests with vitest via npm test -- --run test/unit/auth.test.ts --reporter=verbose, and all tests in that file passed (18 tests passed).
  • The full npm test run was attempted in this environment but did not complete within the session (environmental/time constraints), so only the targeted unit tests and typecheck were verified.

Codex Task

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 12, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui f9f95e5 Commit Preview URL

Branch Preview URL
Jun 13 2026, 05:37 AM

@ghost

ghost commented Jun 12, 2026

Copy link
Copy Markdown

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal Result Evidence Action
Gate result ⚠️ Skipped #613 is no longer open. No action.

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

@ghost ghost added the gittensory:reviewed label Jun 12, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored JSONbored self-assigned this Jun 12, 2026
@ghost

ghost commented Jun 12, 2026

Copy link
Copy Markdown

gittensory · advisory review

Reviewed 2 changed file(s) — two independent AI reviewers.

Suggested action:Safe to merge — both reviewers found no blocking issues.

Reviewer A · gpt-oss-120b — recommends ✅ merge
The PR tightens IP handling for rate limiting by only trusting Cloudflare's cf-connecting-ip header and discarding spoofable proxy headers, removing the trusted‑proxy logic. Tests are updated and expanded to cover the new behavior.

Suggestions

  • Add a brief comment or changelog entry noting that RATE_LIMIT_TRUSTED_PROXY_* env vars are now ignored, to avoid confusion for operators.
  • Consider adding a unit test that ensures requests without cf-connecting-ip consistently generate the same "unknown-ip" key across multiple calls.

Worth double-checking

  • Deployments not behind Cloudflare (or without cf-connecting-ip) will now all share the same "unknown-ip" rate‑limit bucket, potentially causing unintended throttling.
  • The removed trusted‑proxy functions may still be referenced elsewhere; ensure no external imports rely on them.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
The PR removes trust in proxy-forwarded IP headers (x-real-ip, x-forwarded-for) for rate limiting, relying solely on Cloudflare's cf-connecting-ip header to prevent IP spoofing. This is a security hardening appropriate for a Cloudflare Workers deployment. Tests were updated to reflect the new behavior, including validation of IP normalization and edge cases. The change is focused, well-tested, and aligns with the project's Cloudflare-centric architecture.

Suggestions

  • Consider adding a comment in clientIp() explaining why proxy headers are ignored (to prevent future confusion about the intentional removal of fallback logic).
  • Ensure the test suite passes locally to confirm no regressions in rate-limiting behavior for valid Cloudflare IPs.

Worth double-checking

  • If the worker is ever deployed outside Cloudflare (e.g., for local testing), rate limiting would treat all requests as 'unknown-ip', potentially causing unintended blocking. However, this matches the project's deployment context.
  • Verify that normalizeIpAddress() correctly handles all valid IP formats (IPv4/IPv6) and edge cases (like mixed IPv6 notation) as tested in the new test case.

@ghost ghost added the gittensory-review label Jun 12, 2026
@JSONbored
JSONbored force-pushed the codex/fix-spoofable-proxy-headers-vulnerability branch from 4390a20 to b1b2ae5 Compare June 13, 2026 06:25
@JSONbored
JSONbored merged commit d7f7a2e into main Jun 13, 2026
10 checks passed
@JSONbored
JSONbored deleted the codex/fix-spoofable-proxy-headers-vulnerability branch June 13, 2026 06:29
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jun 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant