fix(nginx): correct real_ip_header typo X-Forward-For → X-Forwarded-For#8935
fix(nginx): correct real_ip_header typo X-Forward-For → X-Forwarded-For#8935MinitJain wants to merge 1 commit intomakeplane:previewfrom
Conversation
X-Forward-For is not a real HTTP header — the standard is X-Forwarded-For. With the typo, Nginx never replaces $remote_addr with the actual client IP, so rate limiting and IP logging see the proxy IP instead of the real client. Affects all three nginx configs (web, admin, space).
📝 WalkthroughWalkthroughThis pull request corrects a typo across three Nginx configuration files. The Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/space/nginx/nginx.conf`:
- Around line 12-15: The configuration currently uses set_real_ip_from 0.0.0.0/0
with real_ip_header X-Forwarded-For, which allows IP spoofing and defeats
limit_req_zone using $binary_remote_addr and $remote_addr; replace the wildcard
trust with the actual proxy/CDN/load-balancer CIDR ranges (or make
set_real_ip_from values configurable) so only trusted upstreams can set
X-Forwarded-For, and document the behavior if you must accept arbitrary sources;
update the same change in the other nginx.conf files that use set_real_ip_from,
real_ip_header, and limit_req_zone to ensure rate limits and logs remain
reliable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8782b2aa-8b26-4e35-80b3-049ca833e2ae
📒 Files selected for processing (3)
apps/admin/nginx/nginx.confapps/space/nginx/nginx.confapps/web/nginx/nginx.conf
| set_real_ip_from 0.0.0.0/0; | ||
| real_ip_recursive on; | ||
| real_ip_header X-Forward-For; | ||
| real_ip_header X-Forwarded-For; | ||
| limit_req_zone $binary_remote_addr zone=mylimit:10m rate=10r/s; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the same permissive trust list exists in all three nginx.conf files touched by this PR.
fd -t f 'nginx\.conf$' apps | xargs -I{} sh -c 'echo "=== {} ==="; grep -nE "set_real_ip_from|real_ip_header|real_ip_recursive|limit_req_zone" {}'Repository: makeplane/plane
Length of output: 756
🏁 Script executed:
cd /tmp && git -C /tmp/makeplane/plane log --oneline -20 -- apps/space/nginx/nginx.conf apps/admin/nginx/nginx.conf apps/web/nginx/nginx.conf 2>/dev/null || echo "Git history unavailable; checking file content instead"Repository: makeplane/plane
Length of output: 114
🏁 Script executed:
# Check if there are any diffs or recent changes to these files
git -C /tmp/makeplane/plane diff HEAD~5 -- apps/space/nginx/nginx.conf apps/admin/nginx/nginx.conf apps/web/nginx/nginx.conf 2>/dev/null | head -100 || echo "Unable to retrieve recent diffs"Repository: makeplane/plane
Length of output: 41
🏁 Script executed:
# Search for 'X-Forward-For' (the alleged typo) in the current codebase to confirm it's been fixed everywhere
rg 'X-Forward-For' apps/Repository: makeplane/plane
Length of output: 41
🏁 Script executed:
# Verify the security concern: check nginx documentation behavior for set_real_ip_from 0.0.0.0/0
# Also confirm all three files use $binary_remote_addr in rate limiting
rg -A 2 -B 2 'limit_req_zone.*\$binary_remote_addr' apps/Repository: makeplane/plane
Length of output: 1000
set_real_ip_from 0.0.0.0/0 enables IP-spoofing attacks across all three Nginx configs.
The fix to real_ip_header X-Forwarded-For is correct, but it interacts dangerously with the trust list on line 12. Nginx is configured to trust every upstream as a legitimate proxy, so any direct client can send X-Forwarded-For: <attacker-ip> and Nginx will rewrite $remote_addr to that value. This defeats the limit_req_zone $binary_remote_addr rate limiting (trivial bypass by rotating spoofed IPs) and pollutes access logs.
This issue exists in all three affected files: apps/admin/nginx/nginx.conf, apps/space/nginx/nginx.conf, and apps/web/nginx/nginx.conf.
Restrict set_real_ip_from to actual proxy/CDN/load-balancer ranges only. For example:
- set_real_ip_from 0.0.0.0/0;
+ set_real_ip_from 10.0.0.0/8; # Docker/internal network
+ # set_real_ip_from 173.245.48.0/20; # e.g. Cloudflare (if applicable)If deployments must accept X-Forwarded-For from arbitrary sources, document this limitation clearly or make the trust list configurable.
📝 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.
| set_real_ip_from 0.0.0.0/0; | |
| real_ip_recursive on; | |
| real_ip_header X-Forward-For; | |
| real_ip_header X-Forwarded-For; | |
| limit_req_zone $binary_remote_addr zone=mylimit:10m rate=10r/s; | |
| set_real_ip_from 10.0.0.0/8; # Docker/internal network | |
| # set_real_ip_from 173.245.48.0/20; # e.g. Cloudflare (if applicable) | |
| real_ip_recursive on; | |
| real_ip_header X-Forwarded-For; | |
| limit_req_zone $binary_remote_addr zone=mylimit:10m rate=10r/s; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/space/nginx/nginx.conf` around lines 12 - 15, The configuration
currently uses set_real_ip_from 0.0.0.0/0 with real_ip_header X-Forwarded-For,
which allows IP spoofing and defeats limit_req_zone using $binary_remote_addr
and $remote_addr; replace the wildcard trust with the actual
proxy/CDN/load-balancer CIDR ranges (or make set_real_ip_from values
configurable) so only trusted upstreams can set X-Forwarded-For, and document
the behavior if you must accept arbitrary sources; update the same change in the
other nginx.conf files that use set_real_ip_from, real_ip_header, and
limit_req_zone to ensure rate limits and logs remain reliable.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
|
@coderabbitai set_real_ip_from 0.0.0.0/0 is pre-existing and predates this PR — this PR only fixes the real_ip_header typo. The wildcard trust range is deployment-specific (correct CIDR depends on whether the user is behind Cloudflare, AWS ELB, custom proxy, etc.) and should be addressed separately as a configurable value per deployment. |
|
Only users with a collaborator, contributor, member, or owner role can interact with CodeRabbit. |
1 participant
Description
Fixes #8934
All three nginx configs had a typo in the
real_ip_headerdirective —X-Forward-Forinstead of the standardX-Forwarded-For.X-Forward-Foris not a real HTTP header, so Nginx silently ignored the directive and never replaced$remote_addrwith the actual client IP.Changes:
real_ip_header X-Forward-For→real_ip_header X-Forwarded-Forinapps/web/nginx/nginx.confreal_ip_header X-Forward-For→real_ip_header X-Forwarded-Forinapps/admin/nginx/nginx.confreal_ip_header X-Forward-For→real_ip_header X-Forwarded-Forinapps/space/nginx/nginx.confOne character added in 3 files, nothing else.
Type of Change
Test Scenarios
X-Forwarded-ForReferences
Closes #8934
Summary by CodeRabbit