fix(selfhost): stop client-spoofed cf-connecting-ip from bypassing rate limits

[RealDiligent]

Summary

• Root cause: Self-host binds Redis RATE_LIMITER, but clientIp() still trusts cf-connecting-ip. On Node that header is attacker-controlled (Caddy only sets X-Real-IP / X-Forwarded-For). Attackers can rotate it to bypass strict pre-auth buckets; honest clients without it collapse to a shared unknown-ip bucket.
• Fix: At the Node serve({ fetch }) edge, overwrite cf-connecting-ip via withTrustedClientIp: delete any client-supplied value; behind a private/link-local peer (Caddy) prefer X-Real-IP / leftmost XFF; on a public peer use the TCP address. Workers unchanged.
• Impact: Self-host auth/webhook rate limits bind to the real client again.

Test plan

• Unit tests for spoof rejection, Caddy hop preference, IPv4-mapped peers, unknown-ip
• CI validate + codecov/patch

Risk / tradeoffs

• Assumes the only public front is Caddy on a private compose network (matches shipped Caddyfile). Direct public expose correctly uses the TCP peer and ignores proxy headers.

[superagent-security]

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

[codecov]

Codecov Report

❌ Patch coverage is 69.38776% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.79%. Comparing base (f76cdf3) to head (9d87cae).
⚠️ Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
src/selfhost/trusted-client-ip.ts	69.38%	9 Missing and 6 partials ⚠️

❌ Your patch check has failed because the patch coverage (69.38%) is below the target coverage (99.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6547      +/-   ##
==========================================
+ Coverage   93.59%   95.79%   +2.19%     
==========================================
  Files         672      591      -81     
  Lines       67622    47163   -20459     
  Branches    18591    15016    -3575     
==========================================
- Hits        63289    45178   -18111     
+ Misses       3360     1185    -2175     
+ Partials      973      800     -173     

Flag	Coverage Δ
rees	?
shard-1	43.93% <69.38%> (-0.11%)	⬇️
shard-2	37.08% <0.00%> (+0.10%)	⬆️
shard-3	32.41% <0.00%> (-0.13%)	⬇️
shard-4	34.86% <0.00%> (+0.20%)	⬆️
shard-5	31.29% <0.00%> (+0.09%)	⬆️
shard-6	45.68% <0.00%> (+0.20%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/selfhost/trusted-client-ip.ts	69.38% <69.38%> (ø)

... and 83 files with indirect coverage changes

[loopover-orb]

Caution

🛑 LoopOver review result - reject/close recommended

_{Review updated: 2026-07-16 13:12:59 UTC}

4 files · 1 AI reviewer · 3 blockers · CI failing · blocked

🛑 Suggested Action - Reject/Close

Review summary
This adds a Node-only trusted-client-ip resolver and wires it into server.ts's fetch handler to overwrite cf-connecting-ip before the Worker fetch runs, closing a real spoofing hole for self-host Redis-backed rate limiting. The isPrivateOrLinkLocal/IP-parsing logic itself is careful and well-tested (IPv4-mapped stripping, RFC1918/link-local/loopback classification, malformed-input rejection), but the whole fix hinges on `nodeEnv?.incoming?.socket?.remoteAddress` actually being the shape the runtime's `serve({fetch})` passes as the second callback argument, and that wiring is untested and unverifiable from this diff alone. If that shape is wrong, every request behind Caddy silently falls into the single shared 'unknown-ip' bucket — the same collapse-to-shared-bucket failure the PR claims to fix, just now guaranteed for everyone instead of only header-less clients.

Blockers

• src/server.ts: the fix's entire self-host benefit depends on `nodeEnv?.incoming?.socket?.remoteAddress` matching the actual second-argument shape of the underlying `serve({fetch})` implementation; there is no test or visible import confirming that field path exists, and if it's wrong every Caddy-fronted request resolves to peer=undefined → resolveTrustedClientIp returns 'unknown-ip' → cf-connecting-ip is never set, collapsing ALL self-host traffic into one shared rate-limit bucket rather than just header-less clients as before.

Nits — 5 non-blocking

• The X-Forwarded-For leftmost-hop trust (trusted-client-ip.ts:20) assumes Caddy always *replaces* the header with a single value from `{remote_host}` rather than appending to a client-supplied XFF; worth a comment/test confirming the Caddyfile config actually overwrites rather than appends, since an appended XFF would let a client's own injected first hop survive.
• codecov/patch is at 69.38% against a 99% target — the new server.ts wiring (the nodeEnv extraction and the branch where clientIp stays undefined) appears to be the main uncovered surface, which is exactly the risky part flagged above.
• External brief flags several unexplained numeric literals (4, 8, 127, 192/168, 172/16-31, 169/254) in trusted-client-ip.ts — these are standard RFC1918/octet-count constants and match the existing safe-url.ts convention, so named constants aren't necessary, but a short inline comment per range (as safe-url.ts already does) would be more consistent.
• cf-workers-shim.ts comment update is accurate but purely descriptive churn alongside the real fix — fine, just noting it's not itself a functional change.
• Add a smoke/integration test that exercises the actual `serve({fetch})` call path in server.ts (or at least assert the shape of the second callback argument against the runtime library's types) so a library upgrade or shape mismatch fails CI instead of silently degrading rate-limiting.

Why this is blocked

• src/server.ts: the fix's entire self-host benefit depends on `nodeEnv?.incoming?.socket?.remoteAddress` matching the actual second-argument shape of the underlying `serve({fetch})` implementation; there is no test or visible import confirming that field path exists, and if it's wrong every Caddy-fronted request resolves to peer=undefined → resolveTrustedClientIp returns 'unknown-ip' → cf-connecting-ip is never set, collapsing ALL self-host traffic into one shared rate-limit bucket rather than just header-less clients as before.
• No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
• Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

📋 Copy for AI agents — paste into your coding agent

Fix the following blocker(s) from this PR review:

1. src/server.ts: the fix's entire self-host benefit depends on \`nodeEnv?.incoming?.socket?.remoteAddress\` matching the actual second-argument shape of the underlying \`serve\(\{fetch\}\)\` implementation; there is no test or visible import confirming that field path exists, and if it's wrong every Caddy-fronted request resolves to peer=undefined → resolveTrustedClientIp returns 'unknown-ip' → cf-connecting-ip is never set, collapsing ALL self-host traffic into one shared rate-limit bucket rather than just header-less clients as before.

2. No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.

3. Maintainer requires a linked issue — Link the relevant issue (for example `Closes #123`) before opening the PR.

CI checks failing

• codecov/patch — 69.38% of diff hit (target 99.00%)
• validate
• validate-code

Decision drivers

• ❌ Code review — 3 blockers (1 reviewer)
• ❌ Gate result — Blocking (Repo-configured hard blocker found.)

Context & advisory signals — never blocks the verdict

Signal	Result	Evidence
Linked issue	⚠️ Missing	No linked issue or no-issue rationale found.
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (no linked issue context).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 342 registered-repo PR(s), 166 merged, 31 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	RealDiligent; Gittensor profile; 342 PR(s), 31 issue(s).
Improvement	✅ Minor	risk: clean · value: minor · LLM: significant

Review context

• Author: RealDiligent
• Role context: outside_contributor
• Public audience mode: oss maintainer
• Lane context: Repository is configured for direct PR review.
• Public profile languages: not available
• Official Gittensor activity: 342 PR(s), 31 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Start here: Explain no-issue PR.
• Then work through the remaining 1 step in the Signals table above.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
• Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
• Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

• @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
• @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
• A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

---

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

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

• Re-run LoopOver review

[loopover-orb]

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (codecov/patch, validate, validate-code); No linked issue detected; Maintainer requires a linked issue; AI reviewers agree on a likely critical defect: src/server.ts: the fix's entire self-host benefit depends on `nodeEnv?.incoming?.socket?.remoteAddress` matching the actual second-argument shape of the underlying `serve({fetch})` implementation; there is no test or visible import confirming that field path exists, and if it's wrong every Caddy-fronted request resolves to peer=undefined → resolveTrustedClientIp returns 'unknown-ip' → cf-connecting-ip is never set, collapsing ALL self-host traffic into one shared rate-limit bucket rather than just header-less clients as before.). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.