fix(security): trust only configured proxy IPs when deriving client IP - #1306
Open
doncross03 wants to merge 1 commit into
Open
fix(security): trust only configured proxy IPs when deriving client IP#1306doncross03 wants to merge 1 commit into
doncross03 wants to merge 1 commit into
Conversation
getClientIP trusted x-forwarded-for / x-real-ip headers unconditionally when TRUSTED_PROXY_IPS was unset, letting clients spoof their IP to rotate rate-limit buckets. Only trust forwarded headers when the request arrives from a proxy listed in TRUSTED_PROXY_IPS; otherwise ignore them entirely and fall back to a shared sentinel. - Add getTrustedProxyConfig()/parseTrustedProxyIPs() to src/config/environment.ts - Refactor getClientIP() in src/lib/ratelimit.ts to enforce the allowlist - Update tests for the secure default and trusted-proxy scenarios
|
@doncross03 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Contributor
|
Well done on the job done so far! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This PR closes the rate-limit IP-spoofing hole in
getClientIP(). Previously the function trustedx-forwarded-for/x-real-ipheaders unconditionally wheneverTRUSTED_PROXY_IPSwas unset, so any client could forge its IP and rotate rate-limit buckets to bypass limits. Now forwarded headers are trusted only when the request arrives directly from a proxy listed in theTRUSTED_PROXY_IPSallowlist; otherwise they are ignored entirely and every request falls into a shared bucket.Related Issue
Closes #1167
Changes
🔒 Trusted-Proxy Allowlist
src/config/environment.tsgetTrustedProxyConfig()– resolves theTRUSTED_PROXY_IPSallowlist (typed, read at call time so it is testable).parseTrustedProxyIPs()– comma-separated IPv4/IPv6 parsing with trimming, mirroring the existinggetAuthConfig/getJWTConfigpatterns.src/lib/ratelimit.tsgetClientIP()now only honorsx-forwarded-for/x-real-ip/cf-connecting-ipwhen the direct connection comes from a configured trusted proxy.TRUSTED_PROXY_IPSis unset/empty, forwarded headers are never trusted (previous "legacy" fallback removed), so spoofed headers can no longer rotate rate-limit buckets..env.example– clarified that leavingTRUSTED_PROXY_IPSunset means forwarded headers are ignored and all requests share one bucket.src/lib/ratelimit.test.ts,src/app/api/tutorials/__tests__/ratelimit.test.ts,src/app/api/errors/report/__tests__/route.test.ts, andsrc/app/api/courses/__tests__/route.test.tscovering the secure default, trusted-proxy, and spoofing-prevention scenarios.Verification Results
Acceptance Criteria
src/lib/ratelimit.ts,src/config/environment.ts)