docs: cover the proxy trust allowlist and the closed CORS default - #145
Merged
Merged
Conversation
The reverse-proxy guide put an outer proxy in front of the container but never mentioned FORWARDED_ALLOW_IPS, so gunicorn kept trusting only the bundled nginx and every visitor behind that proxy was read as one client: one shared rate-limit bucket, one web device fingerprint. The CORS paragraph also still described an empty ROMM_CORS_ALLOWED_ORIGINS as allowing every origin. It now allows none, and a wildcard is served without credentials. Both describe rommapp/romm#4699. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gantoine
approved these changes
Sep 24, 2026
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Two unresolved documentation and security-guidance issues must be addressed.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Updates the reverse-proxy guide for proxy trust configuration and the closed CORS default.
Changes:
- Documents
FORWARDED_ALLOW_IPSand security implications. - Corrects CORS guidance and the example origin.
Review findings remain unresolved regarding Uvicorn middleware attribution and overly broad CIDR trust guidance.
| File | Description |
|---|---|
docs/install/reverse-proxy.md |
Adds proxy trust guidance and revises CORS documentation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| ## Set `FORWARDED_ALLOW_IPS` for your proxy | ||
|
|
||
| Inside the container, a bundled nginx sits in front of gunicorn, and by default gunicorn only trusts that nginx. When you put your own reverse proxy in front, the bundled nginx appends your proxy's address to `X-Forwarded-For` before passing the request on. Gunicorn reads the header from right to left and takes the first address it doesn't trust as the client. That address is your proxy, so every request looks like it came from the proxy instead of the real visitor. |
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.

Description
Two statements in the reverse-proxy guide describe behaviour that
rommapp/romm#4699changes.FORWARDED_ALLOW_IPSwas undocumentedThe guide tells you to put a proxy in front of the container, but never mentions that gunicorn only trusts the bundled nginx by default. Requests reaching it carry an
X-Forwarded-Forlist that nginx built by appending the address the connection came from, so with an outer proxy the last entry is that proxy. Gunicorn reads the list from the right and stops at the first address it does not trust, which is the outer proxy, so every visitor behind it is read as one client. The client IP keys the rate limits on metadata heartbeats, device pairing and client tokens, and it is part of the fingerprint that groups browsers into web devices.The new section covers appending the proxy's address (or the CIDR it connects from) to the default, keeping
127.0.0.1, and why*undoes the protection. This applies to all four proxies documented above: Caddy, nginx, Traefik and NPM all append the client address, and none of them can make gunicorn trust the result.The CORS paragraph described the old default
It said an empty
ROMM_CORS_ALLOWED_ORIGINSallows any origin. Empty now allows none, which is what a normal deployment wants, since the UI is same-origin with the API. A wildcard is still usable, but is served without credentials, so it only reaches endpoints that need no login. The compose example in that section pointed at the instance's own URL, which no longer needs listing, so it now shows a companion origin.A note on the generated table
docs/resources/snippets/env-vars.mdis generated from the ref pinned inscripts/sources.toml, currently5.3.0-beta.1, so the committed table still reads "empty allows all" forROMM_CORS_ALLOWED_ORIGINSand has noFORWARDED_ALLOW_IPSrow at all. Both correct themselves the next time that pin is bumped to a release containing #4699 and the generators are re-run, and the deploy workflow regenerates it anyway, so the snippet is deliberately untouched here.Dependency
This documents
rommapp/romm#4699, which is open. Merge this with or after it, not before.Verification
uv run mkdocs buildsucceeds, and the rendered page carries the new section with both admonitions.trunk checkandtrunk fmtare clean on the changed file.uvicorn'sProxyHeadersMiddlewarewalks right to left,gunicorn'sforwarded_allow_ipsaccepts CIDR ranges, and the bundled nginx proxies to127.0.0.1:${DEV_PORT}, which is why the loopback entry stays.AI assistance disclosure
Drafted with AI assistance (Claude Code), on behalf of a RomM maintainer. Every claim above was checked against the current
rommapp/rommsource, the pinnedenv.template, and the installeduvicornandgunicornversions.🤖 Generated with Claude Code