Unset TRUSTED_PROXIES trusts RFC 1918, empty trusts none (closes #129)
check / check (push) Waiting to run
check / check (push) Waiting to run
The backend's own TRUSTED_PROXIES now follows the ruling for every app: unset, it trusts 10.0.0.0/8, 172.16.0.0/12 and 192.168.0.0/16, and no longer loopback; a list replaces that; set and empty, it trusts no proxy. viper reads an empty environment variable as unset, so an empty value got the default. config.New now reads this one setting with os.LookupEnv first. viper's AllowEmptyEnv would apply to every setting, so an empty PORT or DEBUG would stop the start instead of counting as unset, as README.md says it does. bin/entrypoint.sh still starts the backend with TRUSTED_PROXIES=127.0.0.1/32. Model: opus-5-5
This commit was merged in pull request #136.
This commit is contained in:
@@ -199,23 +199,25 @@ image, `make run` builds it and runs it on port 8080.
|
||||
| `DATA_DIR` | `./data/reports` | Directory for compressed reports |
|
||||
| `DATA_DIR_MAX_BYTES` | `1073741824` (1 GiB) | Most bytes of report files kept in `DATA_DIR`, oldest deleted first; see [Report limits](#report-limits) |
|
||||
| `DEBUG` | `false` | Enable debug logging |
|
||||
| `TRUSTED_PROXIES` | loopback + RFC1918 | Comma-separated CIDRs whose `X-Forwarded-For` / `X-Real-IP` headers are trusted for client IP resolution |
|
||||
| `TRUSTED_PROXIES` | RFC 1918 ranges | Comma-separated CIDRs whose `X-Forwarded-For` / `X-Real-IP` headers are trusted for client IP resolution |
|
||||
| `REPORTS_PER_MINUTE` | `60` | Reports each client address may send a minute; see [Report limits](#report-limits) |
|
||||
| `CORS_ALLOWED_ORIGINS` | empty | Comma-separated origins whose pages may call the API; see [CORS](#cors) |
|
||||
| `METRICS_USERNAME` | empty | Basic auth user name for `/metrics`; see [Metrics](#metrics) |
|
||||
| `METRICS_PASSWORD` | empty | Basic auth password for `/metrics`; see [Metrics](#metrics) |
|
||||
| `SENTRY_DSN` | empty | DSN of the Sentry project to send errors to; see [Sentry](#sentry) |
|
||||
|
||||
`TRUSTED_PROXIES` defaults to
|
||||
`127.0.0.1/32,::1/128,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`. The loopback
|
||||
entries cover a reverse proxy on the same host. A request whose direct peer is
|
||||
outside this set has its forwarded headers ignored, and the direct peer is
|
||||
logged and rate-limited instead. The container image does not use this default;
|
||||
see [Deployment](#deployment).
|
||||
`TRUSTED_PROXIES` defaults to `10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`. A list
|
||||
replaces the default; set and empty, the server trusts no proxy. Loopback is not
|
||||
in the default, so a reverse proxy on the same host has to be named, as in
|
||||
`TRUSTED_PROXIES=127.0.0.1/32`. A request whose direct peer is outside this set
|
||||
has its forwarded headers ignored, and the direct peer is logged and
|
||||
rate-limited instead. The container image does not use this default; see
|
||||
[Deployment](#deployment).
|
||||
|
||||
A variable set to a value the server cannot use, such as `PORT=abc`,
|
||||
`DEBUG=maybe` or a `BIND_ADDRESS` that is not an IP address, stops it from
|
||||
starting, with an error naming the variable. An empty variable counts as unset.
|
||||
starting, with an error naming the variable. An empty variable counts as unset,
|
||||
except `TRUSTED_PROXIES`.
|
||||
|
||||
#### Report storage
|
||||
|
||||
|
||||
@@ -21,6 +21,14 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-10-14: unset, the backend's own `TRUSTED_PROXIES` trusts `10.0.0.0/8`,
|
||||
`172.16.0.0/12` and `192.168.0.0/16`, and no longer loopback; a list replaces
|
||||
that; set and empty, it trusts no proxy, where it used to get the default, as
|
||||
viper reads an empty environment variable as unset
|
||||
([#129](https://git.eeqj.de/sneak/netwatch/issues/129)). A value it cannot
|
||||
parse still stops the start, and the image still starts the backend with
|
||||
`TRUSTED_PROXIES=127.0.0.1/32`. Tests load the config with it unset, empty and
|
||||
set to a list
|
||||
- 2026-10-14: while report files cannot be written, `POST /api/v1/reports`
|
||||
answers 500, and storing recovers once writing works again, with no restart
|
||||
([#123](https://git.eeqj.de/sneak/netwatch/issues/123)). A failed write puts
|
||||
|
||||
@@ -9,6 +9,7 @@ import (
|
||||
"math"
|
||||
"net/netip"
|
||||
"net/url"
|
||||
"os"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
@@ -21,13 +22,11 @@ import (
|
||||
)
|
||||
|
||||
// defaultTrustedProxies lists the networks whose forwarded
|
||||
// headers are honoured by default: IPv4 and IPv6 loopback,
|
||||
// for a reverse proxy on the same host, and the RFC1918
|
||||
// ranges. The container image does not use it:
|
||||
// headers are honoured when TRUSTED_PROXIES is unset: the
|
||||
// RFC 1918 ranges. The container image does not use it:
|
||||
// bin/entrypoint.sh gives the server 127.0.0.1/32, since
|
||||
// nginx is its only client there.
|
||||
const defaultTrustedProxies = "127.0.0.1/32,::1/128," +
|
||||
"10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
|
||||
const defaultTrustedProxies = "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
|
||||
|
||||
// Default limits on stored reports; README.md gives the reasons for
|
||||
// these values.
|
||||
@@ -135,6 +134,14 @@ func New(
|
||||
viper.GetString("DEBUG"), errNotBool)
|
||||
}
|
||||
|
||||
// An empty TRUSTED_PROXIES trusts no proxy, but viper reads an
|
||||
// empty environment variable as unset and would give the default,
|
||||
// so the environment is read first.
|
||||
trustedProxies, set := os.LookupEnv("TRUSTED_PROXIES")
|
||||
if !set {
|
||||
trustedProxies = viper.GetString("TRUSTED_PROXIES")
|
||||
}
|
||||
|
||||
s := &Config{
|
||||
BindAddress: viper.GetString("BIND_ADDRESS"),
|
||||
CORSAllowedOrigins: splitList(viper.GetString("CORS_ALLOWED_ORIGINS")),
|
||||
@@ -146,7 +153,7 @@ func New(
|
||||
Port: port,
|
||||
ReportsPerMinute: viper.GetInt("REPORTS_PER_MINUTE"),
|
||||
SentryDSN: viper.GetString("SENTRY_DSN"),
|
||||
TrustedProxies: splitList(viper.GetString("TRUSTED_PROXIES")),
|
||||
TrustedProxies: splitList(trustedProxies),
|
||||
log: log,
|
||||
params: ¶ms,
|
||||
}
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
package config_test
|
||||
|
||||
import (
|
||||
"os"
|
||||
"slices"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
@@ -29,6 +31,62 @@ func requireConfigError(t *testing.T, setting string) {
|
||||
}
|
||||
}
|
||||
|
||||
// requireTrustedProxies builds the config as main does and fails the
|
||||
// test unless its TrustedProxies are want.
|
||||
func requireTrustedProxies(t *testing.T, want []string) {
|
||||
t.Helper()
|
||||
|
||||
var cfg *config.Config
|
||||
|
||||
app := fx.New(
|
||||
fx.NopLogger,
|
||||
fx.Provide(globals.New, logger.New, config.New),
|
||||
fx.Populate(&cfg),
|
||||
)
|
||||
|
||||
err := app.Err()
|
||||
if err != nil {
|
||||
t.Fatalf("config error = %v", err)
|
||||
}
|
||||
|
||||
if !slices.Equal(cfg.TrustedProxies, want) {
|
||||
t.Fatalf("TrustedProxies = %q, want %q", cfg.TrustedProxies, want)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTrustedProxiesUnsetTrustsRFC1918: unset, the server trusts the
|
||||
// RFC 1918 ranges and nothing else, loopback included.
|
||||
func TestTrustedProxiesUnsetTrustsRFC1918(t *testing.T) {
|
||||
// t.Setenv first, so the variable is put back as it was after the
|
||||
// test.
|
||||
t.Setenv("TRUSTED_PROXIES", "")
|
||||
|
||||
err := os.Unsetenv("TRUSTED_PROXIES")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
requireTrustedProxies(t,
|
||||
[]string{"10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16"})
|
||||
}
|
||||
|
||||
// TestTrustedProxiesEmptyTrustsNothing: set and empty, the server
|
||||
// trusts no proxy. viper alone would read the empty variable as unset
|
||||
// and give the default.
|
||||
func TestTrustedProxiesEmptyTrustsNothing(t *testing.T) {
|
||||
t.Setenv("TRUSTED_PROXIES", "")
|
||||
|
||||
requireTrustedProxies(t, []string{})
|
||||
}
|
||||
|
||||
// TestTrustedProxiesListReplacesDefault: a list is used as given, with
|
||||
// none of the default ranges added.
|
||||
func TestTrustedProxiesListReplacesDefault(t *testing.T) {
|
||||
t.Setenv("TRUSTED_PROXIES", "172.17.0.1/32, 2001:db8::/32")
|
||||
|
||||
requireTrustedProxies(t, []string{"172.17.0.1/32", "2001:db8::/32"})
|
||||
}
|
||||
|
||||
// TestSettingsLoadAsGiven: valid values pass the checks and are used
|
||||
// as given. bin/entrypoint.sh starts the server with these
|
||||
// BIND_ADDRESS and PORT values.
|
||||
|
||||
Reference in New Issue
Block a user