support more proxy header #458
Conversation
|
|
📝 WalkthroughWalkthroughThe changes replace a Cloudflare-specific proxy header mechanism with a generic, configurable approach supporting multiple proxy providers. The new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
docs/docs/selfhosting/configuring.md (1)
37-37: Clarify the documentation for CLIENT_IP_HEADER.The description should be more specific about:
- Which header name to provide (e.g., "Set to the name of the proxy header containing the client IP, such as
cf-connecting-iporx-forwarded-for")- What happens when not set (automatic detection using common proxy headers)
- Whether the default is truly
nullor empty/unsetSuggested improvement
-| `CLIENT_IP_HEADER` | `null` | If set - Swetrix will get IP address information from header provided by Proxy. | +| `CLIENT_IP_HEADER` | | The name of the proxy header to use for determining client IP addresses (e.g., `cf-connecting-ip`, `x-forwarded-for`). If not set, Swetrix will automatically detect the IP from common proxy headers. |backend/.env.example (1)
84-88: Fix typo in comment."Cloudfront" should be "CloudFront" (capital F) for consistency with the official product name.
🔎 Proposed fix
# Configure proxy header for client IP # Some well know proxies # Cloudflare: cf-connecting-ip -# Cloudfront: cloudFront-viewer-address +# CloudFront: cloudfront-viewer-address CLIENT_IP_HEADER=cf-connecting-ipbackend/apps/community/src/common/utils.ts (1)
1184-1190: Consider improving forwarded header parsing.The regex handles basic IPv4 and IPv6 patterns, but the extracted IPv6 address may include brackets that should be stripped for consistency.
🔎 Proposed improvement
if (header === 'forwarded') { const match = ip.match(/for=(\[?[0-9a-fA-F:.]+\]?)/) if (match) { - return match[1] + // Remove brackets from IPv6 addresses + return match[1].replace(/[\[\]]/g, '') } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/.env.examplebackend/apps/community/src/common/constants.tsbackend/apps/community/src/common/utils.tsdocs/docs/selfhosting/configuring.mddocs/docs/selfhosting/how-to.md
💤 Files with no reviewable changes (1)
- backend/apps/community/src/common/constants.ts
🔇 Additional comments (1)
backend/apps/community/src/common/utils.ts (1)
122-136: Good coverage of proxy headers.The list provides comprehensive coverage of common proxy and CDN headers with reasonable precedence ordering.
| SENTRY_DSN='' | ||
|
|
||
| # Cloudflare Proxy | ||
| CLOUDFLARE_PROXY_ENABLED=false |
There was a problem hiding this comment.
Remove the deprecated CLOUDFLARE_PROXY_ENABLED variable.
Since this PR replaces CLOUDFLARE_PROXY_ENABLED with the more generic CLIENT_IP_HEADER approach, this line should be removed to avoid confusion.
🔎 Proposed fix
-# Cloudflare Proxy
-CLOUDFLARE_PROXY_ENABLED=false
-Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/.env.example at line 82, remove the deprecated
CLOUDFLARE_PROXY_ENABLED=false entry because the project now uses the
CLIENT_IP_HEADER approach; delete that line from the file to avoid confusion and
ensure the env example only contains current configuration variables.
| const getIPFromHeaders = (headers: any) => { | ||
| if (isProxiedByCloudflare && headers['cf-connecting-ip']) { | ||
| return headers['cf-connecting-ip'] | ||
| const customHeader = process.env.CLIENT_IP_HEADER | ||
|
|
||
| // only use customer header if not present in IP_ADDRESS_HEADERS | ||
| // since we might need some extra process based on proxy | ||
| if ( | ||
| !!customHeader && | ||
| !IP_ADDRESS_HEADERS.includes(customHeader) && | ||
| headers.get(customHeader) | ||
| ) { | ||
| return headers.get(customHeader) | ||
| } |
There was a problem hiding this comment.
Fix typo in comment.
"customer header" should be "custom header".
🔎 Proposed fix
const customHeader = process.env.CLIENT_IP_HEADER
- // only use customer header if not present in IP_ADDRESS_HEADERS
+ // only use custom header if not present in IP_ADDRESS_HEADERS
// since we might need some extra process based on proxy
if (
!!customHeader &&📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const getIPFromHeaders = (headers: any) => { | |
| if (isProxiedByCloudflare && headers['cf-connecting-ip']) { | |
| return headers['cf-connecting-ip'] | |
| const customHeader = process.env.CLIENT_IP_HEADER | |
| // only use customer header if not present in IP_ADDRESS_HEADERS | |
| // since we might need some extra process based on proxy | |
| if ( | |
| !!customHeader && | |
| !IP_ADDRESS_HEADERS.includes(customHeader) && | |
| headers.get(customHeader) | |
| ) { | |
| return headers.get(customHeader) | |
| } | |
| const getIPFromHeaders = (headers: any) => { | |
| const customHeader = process.env.CLIENT_IP_HEADER | |
| // only use custom header if not present in IP_ADDRESS_HEADERS | |
| // since we might need some extra process based on proxy | |
| if ( | |
| !!customHeader && | |
| !IP_ADDRESS_HEADERS.includes(customHeader) && | |
| headers.get(customHeader) | |
| ) { | |
| return headers.get(customHeader) | |
| } |
🤖 Prompt for AI Agents
backend/apps/community/src/common/utils.ts around lines 1157 to 1168, the inline
comment contains a typo "customer header" which should read "custom header";
update the comment text to "custom header" (or rephrase to "custom header" where
appropriate) so the comment correctly describes the customHeader variable usage.
| const header = IP_ADDRESS_HEADERS.find(name => { | ||
| return headers.get(name) | ||
| }) | ||
|
|
||
| if (ip) { | ||
| return ip | ||
| if (!header) { | ||
| return null | ||
| } | ||
|
|
||
| ip = headers['x-forwarded-for'] || null | ||
| const ip = headers.get(header) | ||
|
|
||
| if (!ip) { | ||
| return null | ||
| if (header === 'x-forwarded-for') { | ||
| return ip.split(',')[0].trim() | ||
| } | ||
|
|
||
| return _split(ip, ',')[0] | ||
| if (header === 'forwarded') { | ||
| const match = ip.match(/for=(\[?[0-9a-fA-F:.]+\]?)/) | ||
|
|
||
| if (match) { | ||
| return match[1] | ||
| } | ||
| } | ||
| if (header === 'cloudfront-viewer-address') { | ||
| const lastColonIndex = ip.lastIndexOf(':') | ||
| if (lastColonIndex > 0) { | ||
| return ip.substring(0, lastColonIndex) | ||
| } | ||
| } | ||
| return ip | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n backend/apps/community/src/common/utils.ts | sed -n '1150,1210p'Repository: Swetrix/swetrix
Length of output: 1731
🏁 Script executed:
rg -n "getIPFromHeaders" backend/apps/community/src/ -g '!**/test/**' -g '!**/*.spec.ts' -A 3 -B 1Repository: Swetrix/swetrix
Length of output: 8733
🏁 Script executed:
rg -n "validateIP\|isValidIP\|ipv4\|ipv6" backend/apps/community/src/ -g '!**/test/**' -g '!**/*.spec.ts' -iRepository: Swetrix/swetrix
Length of output: 41
🏁 Script executed:
rg -n "const checkRateLimit\|function checkRateLimit" backend/apps/community/src/common/utils.ts -A 20Repository: Swetrix/swetrix
Length of output: 41
🏁 Script executed:
rg -n "checkRateLimit" backend/apps/community/src/ --type=ts -g '!**/test/**' -g '!**/*.spec.ts' | head -5Repository: Swetrix/swetrix
Length of output: 580
🏁 Script executed:
rg -n "checkRateLimit" backend/apps/community/src/common/utils.ts -B 2 -A 15Repository: Swetrix/swetrix
Length of output: 1155
🏁 Script executed:
grep -n "export.*checkRateLimit" backend/apps/community/src/common/utils.tsRepository: Swetrix/swetrix
Length of output: 41
🏁 Script executed:
rg -n "getRateLimitHash" backend/apps/community/src/common/utils.ts -B 2 -A 8Repository: Swetrix/swetrix
Length of output: 806
🏁 Script executed:
rg -n "IP_ADDRESS_HEADERS" backend/apps/community/src/common/utils.ts -B 2 -A 2Repository: Swetrix/swetrix
Length of output: 642
The custom header case returns completely unvalidated IP values; consider sanitizing or validating before use.
The function lacks explicit IP format validation. Most critically, at line 1167, a custom CLIENT_IP_HEADER value is returned raw without any sanitization. Additionally, the regex pattern for the 'forwarded' header (line 1185) does not properly match standard IPv4 addresses (e.g., "192.168.1.1" contains no colons and would not match). The extracted IP is passed directly to checkRateLimit(), which only hashes it without validation—acceptable for rate limiting by hash, but the unvalidated custom header poses a risk if the header is attacker-controlled.
🤖 Prompt for AI Agents
In backend/apps/community/src/common/utils.ts around lines 1170 to 1198, the
function returns header-derived IPs without validation (including the custom
CLIENT_IP_HEADER) and the 'forwarded' regex only matches addresses with colons;
update the code to validate and sanitize any extracted IP before returning it:
use Node's net.isIP (or equivalent) to accept only valid IPv4/IPv6 addresses
(return null or fallback when invalid), normalize/trim values, enhance the
'forwarded' regex to also capture IPv4 (e.g., for=192.168.0.1 and
for="[::1]":port), properly strip optional port information (including bracketed
IPv6 ports), and apply the same validation to the cloudfront and x-forwarded-for
branches so attacker-controlled headers can't produce unvalidated IPs.
| if (header === 'cloudfront-viewer-address') { | ||
| const lastColonIndex = ip.lastIndexOf(':') | ||
| if (lastColonIndex > 0) { | ||
| return ip.substring(0, lastColonIndex) | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix IPv6 handling for cloudfront-viewer-address.
The current logic assumes "ip:port" format, but CloudFront uses "[ipv6]:port" for IPv6 addresses. Using lastIndexOf(':') on an IPv6 address without brackets will incorrectly strip part of the IP address.
🔎 Proposed fix
if (header === 'cloudfront-viewer-address') {
+ // CloudFront format: "ipv4:port" or "[ipv6]:port"
+ // Handle IPv6 in brackets
+ const ipv6Match = ip.match(/^\[([^\]]+)\]:\d+$/)
+ if (ipv6Match) {
+ return ipv6Match[1]
+ }
+
+ // Handle IPv4:port
const lastColonIndex = ip.lastIndexOf(':')
if (lastColonIndex > 0) {
return ip.substring(0, lastColonIndex)
}
}🤖 Prompt for AI Agents
In backend/apps/community/src/common/utils.ts around lines 1191-1196, the
current logic uses lastIndexOf(':') and strips everything after it which breaks
bracketed IPv6 like "[2001:db8::1]:1234" and also can corrupt unbracketed IPv6;
change the handling to: if header === 'cloudfront-viewer-address' then detect
bracketed IPv6 (ip.startsWith('[')); if bracketed, find the closing ']' and if
the next character is ':' strip the port after ']' only; otherwise, for
non-bracketed addresses only strip on the last colon when the string contains
exactly one ':' (i.e., IPv4:port), leaving multi-colon IPv6 addresses intact.
Ensure no other behavior changes.
| If you use reverse proxy like Nginx, please set it up to pass the request IP address as the `x-real-ip` header, otherwise it may cause the problems related to API route rate limiting and analytics sessions. If the `x-real-ip` header is undefined, Swetrix will use `x-forwarded-for` or the request IP address as a backup. | ||
|
|
||
| If you are using Cloudflare as a proxy for your self-hosted Swetrix instance, you can set the `CLOUDFLARE_PROXY_ENABLED` environment variable to `true`. This will force Swetrix to check the `cf-connecting-ip` header for the IP address. | ||
| If you are using proxy for your self-hosted Swetrix instance, you should set the `CLIENT_IP_HEADER` environment variable. This will force Swetrix to use header added by proxy for determine the IP address of client. |
There was a problem hiding this comment.
Fix grammatical error.
"for determine" should be "to determine".
🔎 Proposed fix
-If you are using proxy for your self-hosted Swetrix instance, you should set the `CLIENT_IP_HEADER` environment variable. This will force Swetrix to use header added by proxy for determine the IP address of client.
+If you are using proxy for your self-hosted Swetrix instance, you should set the `CLIENT_IP_HEADER` environment variable. This will force Swetrix to use the header added by proxy to determine the IP address of the client.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| If you are using proxy for your self-hosted Swetrix instance, you should set the `CLIENT_IP_HEADER` environment variable. This will force Swetrix to use header added by proxy for determine the IP address of client. | |
| If you are using proxy for your self-hosted Swetrix instance, you should set the `CLIENT_IP_HEADER` environment variable. This will force Swetrix to use the header added by proxy to determine the IP address of the client. |
🤖 Prompt for AI Agents
In docs/docs/selfhosting/how-to.md around line 77, the sentence currently reads
"This will force Swetrix to use header added by proxy for determine the IP
address of client." Fix the grammar by changing "for determine" to "to
determine" and also add the missing articles to read smoothly: "This will force
Swetrix to use the header added by the proxy to determine the IP address of the
client."
|
good PR. I'll review & merge it later, it will be included in the next release |
3 participants
Changes
If applicable, please describe what changes were made in this pull request.
Community Edition support
Database migrations
Documentation
This resolve issue #457
Summary by CodeRabbit
Configuration Changes
CLIENT_IP_HEADERenvironment variable to specify which proxy header contains the client IP address (e.g., cf-connecting-ip, cloudfront-viewer-address).Documentation
✏️ Tip: You can customize this high-level summary in your review settings.