Repository navigation
[INFRA-1240] fix(security): trust only private ranges in the AIO proxy - #10136
sriramveeraghanta wants to merge 1 commit into
Conversation
…y by default The AIO Caddyfile trusted forwarded headers from 0.0.0.0/0, so a client's own X-Forwarded-For reached the API and keyed its per-IP throttles and audit IP. Default TRUSTED_PROXIES to private_ranges, matching the web and admin Caddyfiles, and document the override for load balancers and CDNs.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
|
Linked to Plane Work Item(s) References This comment was auto-generated by Plane |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Proxy default and configuration guidance apps/proxy/Caddyfile.aio.ce, deployments/aio/community/README.md |
The AIO proxy defaults to private_ranges while preserving an explicitly configured TRUSTED_PROXIES value. The README describes proxy ranges, their role in rate limiting, and the risk of trusting 0.0.0.0/0. |
Priority:
Estimated code review effort: 2 (Simple) | ~8 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related issues
makeplane/plane#9525: This changes the AIO proxy default requested in the issue; production environment wiring remains outside this change.makeplane/plane#9913: Restricting trusted proxy ranges addresses the reported use of attacker-controlledX-Forwarded-Forvalues for rate-limit bypass.
Suggested labels: 🛡️security
Suggested reviewers: mguptahub, pablohashescobar
Merge Risk: 
Minimal · up to 56df8
-
The AIO proxy now defaults to trusting private ranges. Operators behind public load balancers or CDNs must configure their proxy ranges as documented. No actionable merge-blocking issue was established.
Pre-merge checks: 
5
-
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
Description
The all-in-one (AIO) image's Caddy config trusted forwarded headers from
0.0.0.0/0. Any client's ownX-Forwarded-Fortherefore reached the bundled API unchanged. The API uses that header for its per-IP throttles on the auth endpoints and for the audit IP (get_client_ip), so on a default AIO install a client could pick the IP the API saw.apps/proxy/Caddyfile.aio.ce: the default forTRUSTED_PROXIESis nowprivate_ranges, the same valueapps/web/caddy/Caddyfileandapps/admin/caddy/Caddyfilealready use. Caddy replaces the header from any client outside the trusted ranges with the real client IP before proxying.deployments/aio/community/README.md: documentsTRUSTED_PROXIESfor operators behind a load balancer or CDN.Behavior change: AIO installs behind a load balancer or CDN on public IPs will see the load balancer's IP as the client until they set
TRUSTED_PROXIES(e.g.-e TRUSTED_PROXIES="private_ranges 203.0.113.0/24"). Until then, all users share one auth rate-limit bucket. This needs a release note.Not changed:
TRUSTED_PROXIESis not added tovariables.env.start.shexportsplane.envover the container environment, so adding it there would silently override a-e TRUSTED_PROXIES=...flag.Caddyfile.ce, CLI compose) keeps its0.0.0.0/0default, which INFRA-1189 kept on purpose.Type of Change
Screenshots and Media (if applicable)
Test Scenarios
Checked with the
caddy:2image (v2.11.4):caddy adapton the new config: when unset,trusted_proxiesresolves to the private ranges.TRUSTED_PROXIES=203.0.113.0/24andTRUSTED_PROXIES="private_ranges 203.0.113.0/24"both resolve as expected.X-Forwarded-For: 6.6.6.6to/api/*, routed to an echo backend on:3004. With the old default (0.0.0.0/0) the backend got6.6.6.6, 127.0.0.1. From an untrusted client it got127.0.0.1only.Suggested on a built AIO image:
X-Forwarded-For, and confirm the API logs and rate-limits the real client IP.-e TRUSTED_PROXIES="<lb range>"behind a load balancer and confirm the real client IP still reaches the API.Known limit: where Docker's userland proxy forwards the traffic (Docker Desktop, some IPv6 setups), clients arrive from a private gateway IP and are still trusted. Standard Linux port mapping keeps the real client IP.
References
NUM_PROXIES/TRUSTED_PROXIESfor the CLI compose)Created with Claude Code
Summary by CodeRabbit