Unset TRUSTED_PROXIES trusts RFC 1918, empty trusts none (closes #129) #136

Merged
clawbot merged 1 commits from issue-129-trusted-proxies-default into next 2026-10-14 14:01:38 +02:00
4 changed files with 89 additions and 14 deletions
+10 -8
View File
@@ -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` | `./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) | | `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 | | `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) | | `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) | | `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_USERNAME` | empty | Basic auth user name for `/metrics`; see [Metrics](#metrics) |
| `METRICS_PASSWORD` | empty | Basic auth password 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) | | `SENTRY_DSN` | empty | DSN of the Sentry project to send errors to; see [Sentry](#sentry) |
`TRUSTED_PROXIES` defaults to `TRUSTED_PROXIES` defaults to `10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`. A list
`127.0.0.1/32,::1/128,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`. The loopback replaces the default; set and empty, the server trusts no proxy. Loopback is not
entries cover a reverse proxy on the same host. A request whose direct peer is in the default, so a reverse proxy on the same host has to be named, as in
outside this set has its forwarded headers ignored, and the direct peer is `TRUSTED_PROXIES=127.0.0.1/32`. A request whose direct peer is outside this set
logged and rate-limited instead. The container image does not use this default; has its forwarded headers ignored, and the direct peer is logged and
see [Deployment](#deployment). 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`, 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 `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 #### Report storage
+8
View File
@@ -21,6 +21,14 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
# Completed Steps # 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` - 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 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 ([#123](https://git.eeqj.de/sneak/netwatch/issues/123)). A failed write puts
+13 -6
View File
@@ -9,6 +9,7 @@ import (
"math" "math"
"net/netip" "net/netip"
"net/url" "net/url"
"os"
"strconv" "strconv"
"strings" "strings"
@@ -21,13 +22,11 @@ import (
) )
// defaultTrustedProxies lists the networks whose forwarded // defaultTrustedProxies lists the networks whose forwarded
// headers are honoured by default: IPv4 and IPv6 loopback, // headers are honoured when TRUSTED_PROXIES is unset: the
// for a reverse proxy on the same host, and the RFC1918 // RFC 1918 ranges. The container image does not use it:
// ranges. The container image does not use it:
// bin/entrypoint.sh gives the server 127.0.0.1/32, since // bin/entrypoint.sh gives the server 127.0.0.1/32, since
// nginx is its only client there. // nginx is its only client there.
const defaultTrustedProxies = "127.0.0.1/32,::1/128," + const defaultTrustedProxies = "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
"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 // Default limits on stored reports; README.md gives the reasons for
// these values. // these values.
@@ -135,6 +134,14 @@ func New(
viper.GetString("DEBUG"), errNotBool) 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{ s := &Config{
BindAddress: viper.GetString("BIND_ADDRESS"), BindAddress: viper.GetString("BIND_ADDRESS"),
CORSAllowedOrigins: splitList(viper.GetString("CORS_ALLOWED_ORIGINS")), CORSAllowedOrigins: splitList(viper.GetString("CORS_ALLOWED_ORIGINS")),
@@ -146,7 +153,7 @@ func New(
Port: port, Port: port,
ReportsPerMinute: viper.GetInt("REPORTS_PER_MINUTE"), ReportsPerMinute: viper.GetInt("REPORTS_PER_MINUTE"),
SentryDSN: viper.GetString("SENTRY_DSN"), SentryDSN: viper.GetString("SENTRY_DSN"),
TrustedProxies: splitList(viper.GetString("TRUSTED_PROXIES")), TrustedProxies: splitList(trustedProxies),
log: log, log: log,
params: &params, params: &params,
} }
+58
View File
@@ -1,6 +1,8 @@
package config_test package config_test
import ( import (
"os"
"slices"
"strings" "strings"
"testing" "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 // TestSettingsLoadAsGiven: valid values pass the checks and are used
// as given. bin/entrypoint.sh starts the server with these // as given. bin/entrypoint.sh starts the server with these
// BIND_ADDRESS and PORT values. // BIND_ADDRESS and PORT values.