fix(auth): resolve client IP from proxy headers for pre-auth rate limits

[andriypolanski]

Summary

Fixes #477: pre-auth rate limits no longer collapse every anonymous client into a shared unknown-ip bucket when Cloudflare's cf-connecting-ip header is absent.

clientIp now resolves client identity in proxy-aware order:

1. cf-connecting-ip (unchanged preference for Cloudflare edge)
2. first non-empty entry from x-forwarded-for
3. x-real-ip
4. "unknown-ip" only when no usable address header is present

This restores fair per-client isolation on auth, webhook, MCP, and other pre-auth routes for local deployments, preview proxies, and any path that forwards standard client-address headers without injecting cf-connecting-ip.

Scope

• The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
• This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
• This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
• I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

• git diff --check
• npm run actionlint
• npm run typecheck
• npm run test:coverage locally; global coverage stays at or above 97% for lines, statements, functions, and branches (aim for 98%+ branch coverage locally so CI variance does not fail near the threshold)
• npm run test:workers
• npm run build:mcp
• npm run test:mcp-pack
• npm run ui:openapi:check
• npm run ui:lint
• npm run ui:typecheck
• npm run ui:build
• npm audit --audit-level=moderate
• New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

• UI, MCP pack, and UI build checks are not applicable; this change is limited to Worker rate-limit identity extraction and unit tests.

Safety

• No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
• Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
• Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
• API/OpenAPI/MCP behavior is updated and tested where needed.
• UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
• Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.
• Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

Test plan

• Two pre-auth requests with different x-forwarded-for values and no cf-connecting-ip produce different rate-limit keys.
• x-forwarded-for: "198.51.100.2, 198.51.100.3" keys on the first forwarded address (198.51.100.2).
• x-real-ip is used when both cf-connecting-ip and x-forwarded-for are absent.
• Blank x-forwarded-for / x-real-ip values fall back to unknown-ip instead of using attacker-controlled empty strings as bucket keys.
• Existing behavior preserved: cf-connecting-ip still wins when present; same Cloudflare IP with different bearer tokens still shares one pre-auth bucket.

Notes

Suggested PR title: fix(auth): resolve client IP from proxy headers for pre-auth rate limits

[ghost]

Note

Gittensory Gate skipped

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

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

---

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

[superagent-security]

Superagent found 1 security concern(s).

[JSONbored]

@andriypolanski this is the right pre-auth rate-limit path, but the fallback still accepts unsafe header values.

A few notes:

• The implementation takes the first non-empty x-forwarded-for / x-real-ip value. It does not validate that the value is an actual IP address before using it as the rate-limit key.
• For a pre-auth limiter, malformed or spoof-looking forwarded values should not be allowed to create arbitrary buckets.
• Superagent is also action-required on this PR, but the source-level blocker above is enough on its own.

Required changes:

- Validate cf-connecting-ip, x-forwarded-for candidates, and x-real-ip candidates before accepting them.
- Use the first valid IP in x-forwarded-for, not just the first non-empty token.
- Fall back to the next candidate, then unknown-ip, when a header is blank or malformed.
- Add negative tests for malformed forwarded values and mixed valid/invalid x-forwarded-for chains.

Validation expected:

- focused auth/rate-limit tests
- full validate

[superagent-security]

Superagent found 1 security concern(s).

[JSONbored]

@andriypolanski this is still on the right path, but the main pre-auth safety issue is not fixed yet.

A few notes:

• Falling back from cf-connecting-ip to x-forwarded-for and x-real-ip addresses the shared unknown-ip problem from #477.
• The implementation still accepts the first non-empty forwarded value without validating that it is an IP address.
• For a pre-auth limiter, malformed or attacker-controlled strings should not create arbitrary rate-limit buckets.

Required changes:

• Validate cf-connecting-ip, x-forwarded-for, and x-real-ip candidates as IP addresses before using them in the rate-limit key.
• Ignore malformed forwarded values and continue to the next candidate, falling back to unknown-ip only when no usable IP exists.
• Add negative tests for malformed x-forwarded-for, malformed x-real-ip, comma-separated forwarded values, and fallback ordering.

Validation expected:

• focused test/unit/auth.test.ts
• full validate or at least npm run typecheck plus coverage for the new negative paths

[JSONbored]

Approving — the current head resolves the prior change-requests. The pre-auth rate-limit key now validates client-address headers before use, which was the outstanding blocker.

clientIp (src/auth/rate-limit.ts) now:

• Tries cf-connecting-ip, then each comma-split x-forwarded-for token, then x-real-ip, returning the first value that passes normalizeIpAddress and falling back to unknown-ip only when none is a valid IP.
• Validates IPv4 (4 octets, 0-255) and IPv6 (segment count, single ::, hex-only segments, bracket stripping), so malformed/spoof-looking forwarded strings can no longer mint arbitrary rate-limit buckets — the original #477 concern and the follow-up validation requirement are both met.

Tests cover the fallback ordering, first-valid-token selection within an XFF chain, malformed cf-connecting-ip / XFF / x-real-ip falling through, all-invalid collapsing to a shared unknown-ip bucket, and IPv6 bracket normalization keying identically. CI validate and Superagent are green on the current head.

Two non-blocking notes for a future pass:

• The package-lock.json churn (adding dev: true / removing libc entries) is unrelated lockfile metadata from a local install; consider reverting it to keep the PR focused, though it is harmless.
• isValidIpv4 accepts leading-zero octets (e.g. 01.02.03.04); only a cosmetic key-canonicalization nit, not a security issue, since both forms still validate to a stable bucket.

Note: I'm an external reviewer and cannot merge; leaving the merge decision to @JSONbored, who has the standing review on this PR.

[ghost]

reviewbot · advisory review

Reviewed 4 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 adds robust client IP extraction from Cloudflare and proxy headers, introduces configurable trusted proxy settings, updates type definitions, and expands test coverage. The implementation is clean, well‑tested, and the lockfile changes are benign.

Suggestions

• Consider adding a fallback to the raw request IP when both Cloudflare and trusted proxy checks fail, to avoid all unknown clients sharing a single rate‑limit bucket.
• Update project documentation to describe the new RATE_LIMIT_TRUSTED_PROXIES and RATE_LIMIT_TRUSTED_PROXY_COUNT environment variables.
• Run the full test suite to confirm no unrelated failures from the package‑lock modifications.

Worth double-checking

• If "unknown-ip" is used for many clients, they may unintentionally share a rate‑limit bucket.
• Changes to package‑lock.json could affect reproducible builds if not intended.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
The PR improves client IP resolution for pre-auth rate limits by properly handling proxy headers (cf-connecting-ip, x-real-ip, x-forwarded-for) and introducing trusted proxy configuration via environment variables. The implementation includes thorough IP validation and fallback logic. The package-lock.json changes are dependency update side effects (adding 'dev': true and removing libc arrays) and are harmless. Tests extensively cover the new behavior, including edge cases.

Worth double-checking

• Ensure production deployments set RATE_LIMIT_TRUSTED_PROXIES and RATE_LIMIT_TRUSTED_PROXY_COUNT appropriately when behind trusted proxies to avoid falling back to unknown-IP for all requests.
• Verify IPv6 validation handles edge cases like compressed addresses (e.g., '2001:db8::1') correctly; current implementation seems adequate for filtering malformed inputs.

[superagent-security]

Superagent found 2 security concern(s).

[superagent-security]

Superagent found 1 security concern(s).

[superagent-security]

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