Skip to content

Fix: adding "APP_TRUSTED_PROXIES" variable #569

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
LoveSkylark wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Fix: adding "APP_TRUSTED_PROXIES" variable #569

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1b1bc34
Fixing trusted proxies
LoveSkylark May 29, 2026
16d081c
making docker default only
LoveSkylark Jun 1, 2026
a7cc10b
Merge branch 'master' into master
LoveSkylark Jun 30, 2026
253a26e
Remove APP_TRUSTED_PROXIES
LoveSkylark Jul 10, 2026
5a7d8e5
Remove APP_TRUSTED_PROXIES setting
LoveSkylark Jul 10, 2026
792e200
Remove APP_TRUSTED_PROXIES from environment file
LoveSkylark Jul 10, 2026
f34e4ad
Clarify APP_TRUSTED_PROXIES description in README
LoveSkylark Jul 10, 2026
0b3f364
removing default values from APP_TRUSTED_PROXIES
LoveSkylark Jul 17, 2026
7d409f6
Remove unneeded changes
murrant Jul 17, 2026
9cf6d54
Update APP_TRUSTED_PROXIES description in README
murrant Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ linux/s390x
* `REAL_IP_FROM`: Trusted addresses that are known to send correct replacement addresses (default `0.0.0.0/32`)
* `REAL_IP_HEADER`: Request header field whose value will be used to replace the client address (default `X-Forwarded-For`)
* `LOG_IP_VAR`: Use another variable to retrieve the remote IP address for access [log_format](http://nginx.org/en/docs/http/ngx_http_log_module.html#log_format) on Nginx. (default `remote_addr`)
* `APP_TRUSTED_PROXIES`: Trusted proxies for Laravel / LibreNMS proxy awareness (default `172.17.0.0/16`)

@murrant murrant Jul 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your description could be improved here. Calling out Laravel is probably confusing for most users.
"What is a trusted proxy?"

@murrant murrant Jul 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this, but I think my wording still could be improved:
"Allow given IP(s) to inject headers into the HTTP request. This is required to forward the real client IP from reverse proxies. IPs are allowed to inject any header even authentication headers, so take care to restrict this to your actual reverse proxies"

@murrant murrant Jul 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this?
IP(s) or CIDR ranges allowed to set forwarding headers (X-Forwarded-For, X-Forwarded-Proto, etc.) so the real client IP and protocol survive behind a reverse proxy. These IPs can inject any header, including auth headers, so restrict this to your actual proxies only.

* `SESSION_DRIVER`: [Driver to use for session storage](https://github.com/librenms/librenms/blob/master/config/session.php) (default `file`)
* `CACHE_DRIVER`: [Driver to use for cache and locks](https://github.com/librenms/librenms/blob/master/config/cache.php) (default `database`)

Expand Down
1 change: 1 addition & 0 deletions examples/traefik/librenms.env
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ OPCACHE_MEM_SIZE=128
REAL_IP_FROM=0.0.0.0/32
REAL_IP_HEADER=X-Forwarded-For
LOG_IP_VAR=http_x_forwarded_for
APP_TRUSTED_PROXIES=172.17.0.0/16

CACHE_DRIVER=redis
SESSION_DRIVER=redis
Expand Down
2 changes: 2 additions & 0 deletions rootfs/etc/cont-init.d/03-config.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ DB_USER=${DB_USER:-librenms}
DB_TIMEOUT=${DB_TIMEOUT:-30}

LIBRENMS_BASE_URL=${LIBRENMS_BASE_URL:-/}
APP_TRUSTED_PROXIES=${APP_TRUSTED_PROXIES:-"172.17.0.0/16"}

@murrant murrant Jul 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we think about this as a default? I'm unsure since it differs from upstream. But at least you document it.

@LoveSkylark LoveSkylark Jul 10, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your are right this range is wrong, back in the old days I used to do exactly this: everything landed on the default bridge, so whitelisting 172.17.0.0/16 gave you a simple, ready-to-go path between your reverse proxy and any other container you spun up on the host. That's was the intent here.

Looking at the example in this repo, we're not even separating the reverse proxy from the LibreNMS server (Traefik and LibreNMS share one compose network), which will be 172.18.0.0/16 most of the time on a basic host. If we want a default to cover more than just the example scenario, it should be 172.16.0.0/12, which covers all the /16 networks Docker hands out on a host of this size.

That said, I don't have a strong stake in how this lands, I won't be using it myself (I run this in Kubernetes), I was just trying to make this easier for the most basic Docker deployments.

I appreciate your dedication to LibreNMS. Feel free to make whatever adjustments you want to this PR — or close it if that's easier. I don't plan to spend more time on it myself.

This wouldn't be the first fix I've submitted that stalled out in rounds of small tweaks. Honestly, I don't think senior contributors like you should have to wait on a drive-by contributor like me for changes you already know should be in there — just make them directly. I'm fine with whatever the result looks like.

@murrant murrant Jul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I just noticed. REAL_IP_FROM Is this a duplicate? does it clash?
Also, APP_TRUSTED_PROXIES means trust the headers passed to me (not real ip specifcally, but it includes that)

@LoveSkylark LoveSkylark Jul 16, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im baffled,
A change was implement the breaks docker deployment with any kind of proxy in front, and we are tunnel visioning on what is the best default setting instead of implementing the fix.

Strip default away set the description to what ever you like, just get the fix through then we can start tweaking that the bets setting is and how it is best described.

This is like not fixing a broken window because you cant decide what color curtains will go with the new window :)

@Yoyasp Yoyasp Jul 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You do realize nothing is broken right?
Its a setting you change... Set an env var, edit the example compose, do what you need. The reason it changed is because it allowed man in the middle attacks...

I get your frustration with getting the change through, but its better to stay on the side of safely failing on boot rather than exposing company data because someone forgot to change a default setting

@murrant murrant Jul 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better analogy, it is like leaving the window open because the window is broken and you don't want people to get in through the broken window. Also, change the window to a door and add a lock... ok I'm lost now.

Just like this PR. I don't really use the docker in the real world which is why I ask so many questions. I don't want to break user's installs or expose their data. We are trying to walk a fine line. Changing defaults has much more wide reaching impact and we have to be careful.

Not having APP_TRUSTED_PROXIES in the docs was a major oversight and I would have merged that almost right away.

Again, I have no idea what is going on with the docker image and a lot of people come in with incorrect assumptions (me included) and have different environments. We can get silly things like duplicate variables. Which is very hard to undo.

@LoveSkylark LoveSkylark Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You do realize nothing is broken right? Its a setting you change... Set an env var, edit the example compose, do what you need. The reason it changed is because it allowed man in the middle attacks...

I get your frustration with getting the change through, but its better to stay on the side of safely failing on boot rather than exposing company data because someone forgot to change a default setting

@Yoyasp have you tested this?

It's true that APP_TRUSTED_PROXIES gets passed into the containe, you can see it with env from a shell inside. The problem is that PHP-FPM strips environment variables from its worker processes as a security measure (clear_env = yes by default), so the web UI never sees the value even though the container has it.

This can be bypassed with the CLEAR_ENV variable the image exposes, but that disables the scrubbing entirely, exposing every container variable, including DB_PASSWORD, to all web-facing PHP code. Arguably a broader security tradeoff than just setting APP_TRUSTED_PROXIES="*", but it's what I'm currently running to keep my deployments working.

Alternatively, PHP-FPM supports whitelisting individual variables through the security filter by adding env[APP_TRUSTED_PROXIES] = $APP_TRUSTED_PROXIES to the pool config (www.conf). But that config is regenerated on container start, and its path changes with PHP version bumps and third-party modifications, so the customization can silently get lost.

The original image sidesteps this for its core settings by writing them into Laravel's .env at container start via the deployment script (rootfs/etc/cont-init.d/03-config.sh).

cat >${LIBRENMS_PATH}/.env <<EOL
APP_URL=${LIBRENMS_BASE_URL}
DB_HOST=${DB_HOST}
DB_PORT=${DB_PORT}
DB_DATABASE=${DB_NAME}
DB_USERNAME=${DB_USER}
DB_PASSWORD="${DB_PASSWORD}"
EOL

This is where my pull request went off the rails. My first thought was to simply add APP_TRUSTED_PROXIES=${APP_TRUSTED_PROXIES} into the .env file . But then anyone not setting the variable would get APP_TRUSTED_PROXIES= and an empty value there overrides LibreNMS's built-in default rather than falling back to it. My workaround was to ship default values covering the internal IP ranges Docker and Kubernetes use, unreachable from outside the cluster, analogous to trusting 127.0.0.1 on a bare-metal install. That sidetracked the whole PR into a discussion about which internal ranges belong in the defaults.

So in my latest commit I've taken APP_TRUSTED_PROXIES out of the heredoc entirely and instead append it to .env only when the variable is actually set. The script already uses this exact pattern for RRDCACHED_SERVER, so it follows the file's existing convention rather than inventing a new one.


# Timezone
echo "Setting timezone to ${TZ}..."
Expand Down Expand Up @@ -133,6 +134,7 @@ if [ -z "$DB_PASSWORD" ]; then
fi
cat >${LIBRENMS_PATH}/.env <<EOL
APP_URL=${LIBRENMS_BASE_URL}
APP_TRUSTED_PROXIES=${APP_TRUSTED_PROXIES}
DB_HOST=${DB_HOST}
DB_PORT=${DB_PORT}
DB_DATABASE=${DB_NAME}
Expand Down