nginx: trust X-Forwarded-For only from TRUSTED_PROXIES (closes #64)

nginx trusted X-Forwarded-For from every RFC1918 address, so a client
reaching it from one could write a new address on each request and
get a fresh rate-limit allowance. The container's TRUSTED_PROXIES now
names the reverse proxies nginx trusts, none by default.
bin/entrypoint.sh makes each entry a CIDR, checks it with the new
"netwatch-server check-cidr", which runs the server's own
TRUSTED_PROXIES parsing, and writes one set_real_ip_from line per
entry into /etc/nginx/trusted-proxies.conf, which nginx.conf includes.
The backend is started with TRUSTED_PROXIES=127.0.0.1/32, since nginx
is its only client. The viewport test mounts an empty file there.

Model: opus-5-5

README.md

@@ -184,8 +184,8 @@ container: nginx serves the built frontend and passes `/api/` and
 only inside the container, on `127.0.0.1:8081`. The image:
 
 - Listens on port 8080 by default (override with `PORT` env var)
-- Trusts `X-Forwarded-For` from RFC1918 reverse proxies (10/8, 172.16/12,
-  192.168/16)
+- Takes the client address from `X-Forwarded-For` only on requests from the
+  reverse proxies named in `TRUSTED_PROXIES`, and by default from none
 - Sends access logs to stdout
 - Caches static assets with immutable headers
 - Stores reports in `DATA_DIR`, `/data/reports` by default, on the `/data`
@@ -224,10 +224,16 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for netwatch needs:
     - `DEBUG`, default `false`: debug logging
     - `DATA_DIR`, default `/data/reports`: leave unset; reports kept outside
       `/data` do not survive a redeploy
-    - `TRUSTED_PROXIES`, default loopback and RFC1918: leave unset. The
-      backend's only client is nginx, on loopback, which passes on the client
-      address; nginx takes it from `X-Forwarded-For` only from RFC1918
-      addresses.
+    - `TRUSTED_PROXIES`, default empty: set it to the address the reverse proxy
+      in front of the container connects from, as an IP address or CIDR; several
+      are separated by commas. nginx takes the client address from
+      `X-Forwarded-For` only on a request from one of them, and the rate limit
+      counts that address. Unset, `X-Forwarded-For` is ignored and every client
+      behind the proxy shares the proxy's one allowance of `REPORTS_PER_MINUTE`.
+      Name only addresses nothing but the proxy connects from: any client that
+      connects from one can write its own `X-Forwarded-For`, and through a port
+      Docker publishes, every client may connect from the Docker network's
+      gateway, such as `172.17.0.1`.
 - **Health check:** the image's `HEALTHCHECK` requests
   `/.well-known/healthcheck` through nginx every 30 seconds, so it fails unless
   both nginx and the backend answer. upaas reads the container's health 60

TODO.md

@@ -23,6 +23,22 @@ latest run passes.
 
 # Completed Steps
 
+- 2026-09-29: nginx takes the client address from `X-Forwarded-For` only on
+  requests from the reverse proxies named in the container's `TRUSTED_PROXIES`
+  (issue #64), and by default from none, where it trusted every RFC1918 address
+  before, so a client could write a new address on each request and escape the
+  rate limit. `bin/entrypoint.sh` writes one `set_real_ip_from` line per entry
+  into `/etc/nginx/trusted-proxies.conf`, which `nginx.conf` includes, refusing
+  an entry that is not an IP address or CIDR, as `netwatch-server check-cidr`
+  finds; it starts the backend with `TRUSTED_PROXIES=127.0.0.1/32`, since nginx
+  is its only client
+- 2026-09-29: report file names can no longer collide (issue #61): each is
+  `reports-<timestamp>-<number>.jsonl.zst`, where the number goes up by one for
+  each file the server starts to write, so two flushes in the same millisecond,
+  such as a flush for size and the final flush at shutdown, each get a file of
+  their own instead of the second one failing. A failed write uses up its
+  number, leaving a gap if the file could not be created and otherwise a file
+  under that number that may be incomplete.
 - 2026-09-29: ready to run under upaas (issue #59): the image has a
   `HEALTHCHECK` that requests `/.well-known/healthcheck` through nginx on the
   port from `PORT`. The backend no longer reads a bad `PORT` as 0 or a bad

backend/README.md

@@ -87,9 +87,10 @@ Internal packages in `internal/` follow standard Go project layout:
 | `CORS_ALLOWED_ORIGINS` | empty                | Comma-separated origins whose pages may call the API; see [CORS](#cors)                                  |
 
 `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 the reverse proxy that shares the container; the
-RFC1918 ranges match `nginx.conf`. A request whose direct peer is outside this
-set has its forwarded headers ignored, and the direct peer is logged instead.
+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 [Container image](#container-image).
 
 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
@@ -101,14 +102,30 @@ The root `Dockerfile` builds one image in which nginx listens on the public port
 8080, serves the frontend, and proxies `/api/` and `/.well-known/healthcheck` to
 this server. The image's entrypoint, `bin/entrypoint.sh`, starts the server as
 user `netwatch` (uid 1000) with `BIND_ADDRESS=127.0.0.1` and `PORT=8081`, so
-only nginx reaches it. `DATA_DIR` is `/data/reports`, on the `/data` volume,
-which `netwatch` owns.
+only nginx reaches it, and with `TRUSTED_PROXIES=127.0.0.1/32`, so it takes the
+client address nginx passes on and no other. `DATA_DIR` is `/data/reports`, on
+the `/data` volume, which `netwatch` owns.
+
+The container's own `TRUSTED_PROXIES` goes to nginx instead: IP addresses or
+CIDRs, separated by commas, of the reverse proxies in front of the container.
+nginx takes the client address from `X-Forwarded-For` only on a request from one
+of them. Unset or empty, nginx trusts no proxy, and the client address is the
+one each request comes from, so every client behind a proxy shares one rate
+limit. An entry that is not an IP address or CIDR, such as a hostname or
+`1.2.3`, stops the container at start with an error naming `TRUSTED_PROXIES`:
+the entrypoint checks each entry with `netwatch-server check-cidr`, which parses
+it as this server parses its own `TRUSTED_PROXIES`.
 
 ### Report storage
 
-Reports are written as `reports-<timestamp>.jsonl.zst` files in `DATA_DIR`.
-Each file contains one JSON object per line, compressed with zstd. Files are
-created with `O_EXCL` to prevent overwrites.
+Reports are written as `reports-<timestamp>-<number>.jsonl.zst` files in
+`DATA_DIR`. The timestamp is in UTC to the millisecond, so the names sort by
+time. The number starts at 1 when the server starts and goes up by one for each
+file the server starts to write, so two files written in the same millisecond
+still get different names. A failed write uses up its number, leaving a gap in
+the numbers if the file could not be created and otherwise a file under that
+number that may be incomplete. Each file contains one JSON object per line,
+compressed with zstd. Files are created with `O_EXCL` to prevent overwrites.
 
 ### Report limits
 

backend/cmd/netwatch-server/main.go

@@ -2,6 +2,9 @@
 package main
 
 import (
+	"fmt"
+	"os"
+
 	"sneak.berlin/go/netwatch/internal/config"
 	"sneak.berlin/go/netwatch/internal/globals"
 	"sneak.berlin/go/netwatch/internal/handlers"
@@ -22,6 +25,19 @@ var (
 )
 
 func main() {
+	// "netwatch-server check-cidr CIDR" exits 1, with the error, if
+	// this server would refuse CIDR in its TRUSTED_PROXIES.
+	// bin/entrypoint.sh runs it on each entry it gives nginx.
+	if len(os.Args) == 3 && os.Args[1] == "check-cidr" {
+		_, err := middleware.ParseTrustedProxies(os.Args[2:])
+		if err != nil {
+			fmt.Fprintln(os.Stderr, err)
+			os.Exit(1)
+		}
+
+		return
+	}
+
 	globals.Appname = Appname
 	globals.Version = Version
 	globals.Buildarch = Buildarch

backend/internal/config/config.go

@@ -21,10 +21,11 @@ import (
 )
 
 // defaultTrustedProxies lists the networks whose forwarded
-// headers are honoured by default. It covers the RFC1918
-// ranges (to match nginx.conf) plus IPv4 and IPv6 loopback,
-// because the reverse proxy shares the container and reaches
-// the backend over loopback.
+// 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:
+// 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"
 

backend/internal/middleware/export_test.go

@@ -29,7 +29,3 @@ func ClientIP(
 ) string {
 	return clientIP(remoteAddr, header, trusted)
 }
-
-func ParseTrustedProxies(cidrs []string) ([]netip.Prefix, error) {
-	return parseTrustedProxies(cidrs)
-}

backend/internal/middleware/middleware.go

@@ -64,7 +64,7 @@ func New(
 	_ fx.Lifecycle,
 	params Params,
 ) (*Middleware, error) {
-	trusted, err := parseTrustedProxies(params.Config.TrustedProxies)
+	trusted, err := ParseTrustedProxies(params.Config.TrustedProxies)
 	if err != nil {
 		return nil, err
 	}
@@ -77,9 +77,11 @@ func New(
 	return s, nil
 }
 
-// parseTrustedProxies converts the TRUSTED_PROXIES entries into
-// prefixes, failing fast on any malformed entry.
-func parseTrustedProxies(cidrs []string) ([]netip.Prefix, error) {
+// ParseTrustedProxies converts the TRUSTED_PROXIES entries into
+// prefixes, failing fast on any malformed entry. Each entry must be
+// a CIDR; a lone address is refused. "netwatch-server check-cidr"
+// runs it too.
+func ParseTrustedProxies(cidrs []string) ([]netip.Prefix, error) {
 	prefixes := make([]netip.Prefix, 0, len(cidrs))
 
 	for _, cidr := range cidrs {

backend/internal/middleware/middleware_test.go

@@ -36,15 +36,32 @@ func mustPrefixes(t *testing.T, cidrs ...string) []netip.Prefix {
 	return prefixes
 }
 
+// TestParseTrustedProxiesRejectsMalformed includes entries nginx would
+// read as another address or look up as a hostname, in the CIDR form
+// bin/entrypoint.sh gives "netwatch-server check-cidr".
 func TestParseTrustedProxiesRejectsMalformed(t *testing.T) {
 	t.Parallel()
 
-	_, err := middleware.ParseTrustedProxies([]string{"not-a-cidr"})
-	if err == nil || !strings.Contains(err.Error(), "TRUSTED_PROXIES") {
-		t.Fatalf("error = %v, want one naming TRUSTED_PROXIES", err)
+	for _, cidr := range []string{
+		"not-a-cidr", "10.0.0.1", "1.2.3/32", "172.30/32", "10/32",
+		"cafe/32", "999.1.1.1/32", "10.0.0.0/33", "::1/129",
+		"fe80::1%eth0/128",
+	} {
+		_, err := middleware.ParseTrustedProxies([]string{cidr})
+		if err == nil || !strings.Contains(err.Error(), "TRUSTED_PROXIES") {
+			t.Errorf("%q: error = %v, want one naming TRUSTED_PROXIES",
+				cidr, err)
+		}
 	}
 }
 
+func TestParseTrustedProxiesAcceptsCIDRs(t *testing.T) {
+	t.Parallel()
+
+	mustPrefixes(t, "172.17.0.1/32", "10.0.0.0/8", "2001:db8::1/128",
+		"2001:db8::/32", "::ffff:192.0.2.1/128")
+}
+
 type clientIPCase struct {
 	name       string
 	remoteAddr string

backend/internal/reportbuf/export_test.go

@@ -1,7 +1,15 @@
 package reportbuf
 
+import "time"
+
 // Flush writes the buffered reports to a file now, as the periodic
 // flush does, so tests need not wait a minute for it.
 func (b *Buffer) Flush() error {
 	return b.flushLocked()
 }
+
+// StopClock makes every report file the buffer writes from now on
+// carry the timestamp at, as if all were written in one millisecond.
+func (b *Buffer) StopClock(at time.Time) {
+	b.now = func() time.Time { return at }
+}

backend/internal/reportbuf/reportbuf.go

@@ -14,6 +14,7 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"sneak.berlin/go/netwatch/internal/config"
@@ -30,7 +31,8 @@ const (
 	dirPerms           fs.FileMode = 0o750
 	filePerms          fs.FileMode = 0o640
 
-	// Report files are named filePrefix + timestamp + fileSuffix.
+	// Report files are named filePrefix + timestamp + "-" + number +
+	// fileSuffix; see writeFile.
 	filePrefix = "reports-"
 	fileSuffix = ".jsonl.zst"
 )
@@ -56,6 +58,12 @@ type Buffer struct {
 	log      *slog.Logger
 	maxBytes int64
 	mu       sync.Mutex
+	// now is the clock report files are named by: time.Now, except
+	// in tests that need two flushes to share a timestamp.
+	now func() time.Time
+	// seq numbers the report files, so that two named in the same
+	// millisecond still get different names.
+	seq      atomic.Uint64
 	stopOnce sync.Once
 	// usedBytes is what Append checks against maxBytes: the size
 	// of the report files in dataDir, plus the reports not yet
@@ -79,6 +87,7 @@ func New(
 		done:     make(chan struct{}),
 		log:      params.Logger.Get(),
 		maxBytes: params.Config.DataDirMaxBytes,
+		now:      time.Now,
 	}
 
 	lc.Append(fx.Hook{
@@ -211,11 +220,14 @@ func (b *Buffer) drainBuf() []byte {
 // writeFile creates a timestamped zstd-compressed JSONL file
 // in the data directory.
 func (b *Buffer) writeFile(data []byte) error {
-	ts := time.Now().UTC().Format("2006-01-02T15-04-05.000Z")
-	path := filepath.Join(b.dataDir, filePrefix+ts+fileSuffix)
+	// The timestamp comes first, so the names sort by time; the number
+	// after it tells apart files named in the same millisecond.
+	ts := b.now().UTC().Format("2006-01-02T15-04-05.000Z")
+	name := fmt.Sprintf("%s%s-%d%s", filePrefix, ts, b.seq.Add(1), fileSuffix)
+	path := filepath.Join(b.dataDir, name)
 
 	// path is built from the operator-supplied dataDir plus a
-	// generated timestamp, so it carries no external input.
+	// generated timestamp and number, so it carries no external input.
 	f, err := os.OpenFile( //nolint:gosec // see comment above
 		path,
 		os.O_WRONLY|os.O_CREATE|os.O_EXCL,

backend/internal/reportbuf/reportbuf_test.go

@@ -3,9 +3,11 @@ package reportbuf_test
 import (
 	"encoding/json"
 	"errors"
+	"fmt"
 	"io/fs"
 	"os"
 	"path/filepath"
+	"slices"
 	"strconv"
 	"strings"
 	"sync"
@@ -18,6 +20,7 @@ import (
 	"sneak.berlin/go/netwatch/internal/logger"
 	"sneak.berlin/go/netwatch/internal/reportbuf"
 
+	"github.com/klauspost/compress/zstd"
 	"go.uber.org/fx"
 	"go.uber.org/fx/fxtest"
 )
@@ -210,10 +213,6 @@ func TestWrittenReportsCountAtFileSize(t *testing.T) {
 		t.Fatalf("flush: %v", err)
 	}
 
-	// The second report is written at shutdown, and must not land in
-	// the first file's millisecond (see TestWrittenReportsKeepCounting).
-	time.Sleep(time.Millisecond)
-
 	err = buf.Append(report)
 	if err != nil {
 		t.Fatalf("second report, after the first was written: %v", err)
@@ -254,10 +253,6 @@ func TestWrittenReportsKeepCounting(t *testing.T) {
 			t.Fatalf("with %d bytes of report files: %v", used, err)
 		}
 
-		// Report files are named to the millisecond; two in the same
-		// one collide (https://git.eeqj.de/sneak/netwatch/issues/61).
-		time.Sleep(time.Millisecond)
-
 		err = buf.Flush()
 		if err != nil {
 			t.Fatalf("flush: %v", err)
@@ -315,6 +310,43 @@ func TestConcurrentAppendsStopAtCap(t *testing.T) {
 	}
 }
 
+// TestTwoFlushesInOneMillisecond flushes twice within one millisecond,
+// as a flush for size and the final flush at shutdown can: each flush
+// must write a file of its own, and the files must hold every report.
+func TestTwoFlushesInOneMillisecond(t *testing.T) {
+	const flushes = 2
+
+	dir := t.TempDir()
+	t.Setenv("DATA_DIR", dir)
+
+	buf := startBuffer(t)
+	buf.StopClock(time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC))
+
+	for id := 1; id <= flushes; id++ {
+		err := buf.Append(map[string]int{"id": id})
+		if err != nil {
+			t.Fatalf("append report %d: %v", id, err)
+		}
+
+		err = buf.Flush()
+		if err != nil {
+			t.Fatalf("flush %d: %v", id, err)
+		}
+	}
+
+	files := readReportFiles(t, dir)
+	if len(files) != flushes {
+		t.Fatalf("%d report files after %d flushes", len(files), flushes)
+	}
+
+	for id := 1; id <= flushes; id++ {
+		want := fmt.Sprintf(`{"id":%d}`+"\n", id)
+		if !slices.Contains(files, want) {
+			t.Fatalf("no report file holds report %d alone", id)
+		}
+	}
+}
+
 // reportFilesBytes returns the total size of the report files in dir.
 func reportFilesBytes(t *testing.T, dir string) int64 {
 	t.Helper()
@@ -338,6 +370,43 @@ func reportFilesBytes(t *testing.T, dir string) int64 {
 	return total
 }
 
+// readReportFiles returns the decompressed contents of each report
+// file in dir.
+func readReportFiles(t *testing.T, dir string) []string {
+	t.Helper()
+
+	files := os.DirFS(dir)
+
+	names, err := fs.Glob(files, "reports-*.jsonl.zst")
+	if err != nil {
+		t.Fatalf("list report files: %v", err)
+	}
+
+	dec, err := zstd.NewReader(nil)
+	if err != nil {
+		t.Fatalf("create zstd decoder: %v", err)
+	}
+	defer dec.Close()
+
+	contents := make([]string, 0, len(names))
+
+	for _, name := range names {
+		compressed, readErr := fs.ReadFile(files, name)
+		if readErr != nil {
+			t.Fatalf("read %s: %v", name, readErr)
+		}
+
+		data, decErr := dec.DecodeAll(compressed, nil)
+		if decErr != nil {
+			t.Fatalf("decompress %s: %v", name, decErr)
+		}
+
+		contents = append(contents, string(data))
+	}
+
+	return contents
+}
+
 func writeBytes(t *testing.T, path string, n int) {
 	t.Helper()
 

bin/entrypoint.sh

@@ -32,16 +32,47 @@ if [ "$PORT" -eq 8081 ]; then
     exit 1
 fi
 
+# TRUSTED_PROXIES names the reverse proxies in front of the container,
+# as IP addresses or CIDRs separated by commas. nginx takes the client
+# address from X-Forwarded-For only on a request from one of them, so
+# unset or empty, it trusts no one. nginx.conf includes the file written
+# here, one set_real_ip_from line per entry.
+#
+# nginx looks up an entry it cannot read as an address as a hostname,
+# and trusts what it finds (1.2.3 is found as 1.2.0.3). So each entry
+# is made a CIDR, a lone address getting /128 if it is IPv6 and /32 if
+# not, and netwatch-server checks it with the parsing it gives its own
+# TRUSTED_PROXIES. Its error, naming the CIDR, is dropped for the one
+# below, naming the entry as written. set -f keeps a * in an entry from
+# becoming a list of file names.
+TRUSTED_PROXIES="${TRUSTED_PROXIES:-}"
+set -f
+for proxy in $(printf '%s' "$TRUSTED_PROXIES" | tr ',' ' '); do
+    case "$proxy" in
+        */*) cidr="$proxy" ;;
+        *:*) cidr="$proxy/128" ;;
+        *) cidr="$proxy/32" ;;
+    esac
+    if ! netwatch-server check-cidr "$cidr" 2> /dev/null; then
+        echo "entrypoint: TRUSTED_PROXIES must be IP addresses or CIDRs" \
+            "separated by commas; '$proxy' is neither" >&2
+        exit 1
+    fi
+    echo "set_real_ip_from $cidr;"
+done > /etc/nginx/trusted-proxies.conf
+
 # A stop signal is only noted here; the loop below acts on it.
 stop_requested=""
 trap 'stop_requested=yes' TERM INT
 
 # netwatch-server runs as the netwatch user and listens on loopback
 # only, on a port other than the public one; nginx.conf proxies to this
-# address. The netwatch user has no login shell, hence -s /bin/sh.
-# busybox su replaces itself with the command instead of staying on as
-# its parent, so $! is the server's own PID.
-BIND_ADDRESS=127.0.0.1 PORT=8081 \
+# address. Its only client is nginx, so it takes the client address
+# nginx passes on from 127.0.0.1 alone, whatever TRUSTED_PROXIES the
+# container has. The netwatch user has no login shell, hence -s
+# /bin/sh. busybox su replaces itself with the command instead of
+# staying on as its parent, so $! is the server's own PID.
+BIND_ADDRESS=127.0.0.1 PORT=8081 TRUSTED_PROXIES=127.0.0.1/32 \
     su -s /bin/sh netwatch -c 'exec netwatch-server' &
 backend=$!
 

nginx.conf

@@ -11,10 +11,12 @@ server {
     root /usr/share/nginx/html;
     index index.html;
 
-    # Trust RFC1918 reverse proxies for X-Forwarded-For
-    set_real_ip_from 10.0.0.0/8;
-    set_real_ip_from 172.16.0.0/12;
-    set_real_ip_from 192.168.0.0/16;
+    # The client address comes from X-Forwarded-For only on a request
+    # from the reverse proxies in TRUSTED_PROXIES: bin/entrypoint.sh
+    # writes one set_real_ip_from line for each into this file, and
+    # leaves it empty when TRUSTED_PROXIES is unset, so that by default
+    # the client address is the one each request comes from.
+    include /etc/nginx/trusted-proxies.conf;
     real_ip_header X-Forwarded-For;
     real_ip_recursive on;
 

script/frontend-viewport-test

@@ -63,11 +63,14 @@ main() {
 
     # nginx.conf is a template: the image renders it over its own
     # default.conf, with the same port and limit bin/entrypoint.sh uses.
+    # The empty file it includes trusts no proxy, as bin/entrypoint.sh
+    # writes it when TRUSTED_PROXIES is unset.
     docker run -d --rm --name "$SERVER" \
         --network "$NETWORK" --network-alias netwatch \
         -e PORT=8080 -e NGINX_ENVSUBST_FILTER='^PORT$' \
         -v "$ROOT/dist:/usr/share/nginx/html:ro" \
         -v "$ROOT/nginx.conf:/etc/nginx/templates/default.conf.template:ro" \
+        -v /dev/null:/etc/nginx/trusted-proxies.conf:ro \
         "$SERVER_IMAGE" > /dev/null
 
     # The image's own entrypoint already exposes CDP on 9222 and passes