Trust reverse proxies at private addresses by default (closes #198)

With TRUSTED_PROXIES unset, the server now trusts reverse proxies in the
private ranges of RFC 1918. A proxy such as traefik on a Docker network is
then believed. /ip/, the home page's curl card and the request log give the
client's address from X-Forwarded-For instead of the proxy's address.

A list in TRUSTED_PROXIES replaces the default. TRUSTED_PROXIES set but
empty trusts no proxy, so the config package reads it with os.LookupEnv.
A direct client at a public address still cannot choose its address with
a forged header.

Model: opus-5-5

internal/server/middleware_internal_test.go

@@ -1,25 +1,38 @@
 package server
 
 import (
+	"bytes"
 	"encoding/json"
+	"io"
+	"log/slog"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"net/netip"
+	"os"
 	"strings"
 	"testing"
 	"time"
+
+	"git.eeqj.de/sneak/routewatch/internal/config"
+	"git.eeqj.de/sneak/routewatch/internal/database"
+	"git.eeqj.de/sneak/routewatch/internal/logger"
+	"git.eeqj.de/sneak/routewatch/internal/metrics"
+	"git.eeqj.de/sneak/routewatch/internal/streamer"
 )
 
 // The addresses the forwarding tests use: the client a proxy names in
 // X-Forwarded-For, the range of trusted proxies, a peer inside it, and a peer
 // outside it, which lies in testPrefixV4 so that an IP lookup of it finds a
-// route.
+// route. reverseProxyPeer is a reverse proxy at a private address, as Docker
+// gives one.
 const (
 	forwardedClient   = "203.0.113.9"
 	trustedProxyRange = "10.0.0.0/8"
 	trustedPeer       = "10.0.0.2:1234"
 	untrustedPeerAddr = "192.0.2.1"
 	untrustedPeer     = untrustedPeerAddr + ":1234"
+	reverseProxyPeer  = "172.19.0.14:1234"
 )
 
 // TestResponsesCarrySecurityHeaders checks that a page, a JSON route and a
@@ -183,6 +196,141 @@ func TestForwardedForFromUntrustedPeerIsIgnored(t *testing.T) {
 	}
 }
 
+// TestDefaultTrustedProxiesGiveTheForwardedClient builds the server from the
+// configuration with TRUSTED_PROXIES unset. A request from a reverse proxy at
+// a private address must be looked up by /ip/, shown on the home page's card
+// and logged as the client its X-Forwarded-For names. A direct request from a
+// public address with a forged X-Forwarded-For keeps its own address. Before
+// the fix no proxy was trusted unless TRUSTED_PROXIES named it, so /ip/ gave
+// the proxy's address (https://git.eeqj.de/sneak/routewatch/issues/198).
+func TestDefaultTrustedProxiesGiveTheForwardedClient(t *testing.T) {
+	t.Setenv("PORT", "")
+	t.Setenv("XDG_DATA_HOME", "")
+
+	// t.Setenv puts the variable back when the test ends.
+	t.Setenv("TRUSTED_PROXIES", "")
+
+	err := os.Unsetenv("TRUSTED_PROXIES")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cfg, err := config.New()
+	if err != nil {
+		t.Fatalf("config.New: %v", err)
+	}
+
+	var requestLog bytes.Buffer
+
+	s := newServerWithConfig(t, cfg, &requestLog)
+
+	tests := []struct {
+		name string
+		peer string
+		want string
+	}{
+		{"through a reverse proxy", reverseProxyPeer, forwardedClient},
+		{"direct from a public address", untrustedPeer, untrustedPeerAddr},
+	}
+
+	for _, tt := range tests {
+		requestLog.Reset()
+
+		get := func(path string) *httptest.ResponseRecorder {
+			req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, path, nil)
+			req.RemoteAddr = tt.peer
+			req.Header.Set("X-Forwarded-For", forwardedClient)
+
+			rec := httptest.NewRecorder()
+			s.router.ServeHTTP(rec, req)
+
+			return rec
+		}
+
+		var answer struct {
+			Data IPLookupResponse `json:"data"`
+		}
+
+		rec := get("/ip/")
+
+		err := json.Unmarshal(rec.Body.Bytes(), &answer)
+		if err != nil {
+			t.Fatalf("%s: decoding the answer %q: %v", tt.name, rec.Body, err)
+		}
+
+		if answer.Data.Query != tt.want {
+			t.Errorf("%s: /ip/ looked up %q, want %q", tt.name, answer.Data.Query, tt.want)
+		}
+
+		page := get("/").Body.String()
+		if !strings.Contains(page, `data-client-ip="`+tt.want+`"`) {
+			t.Errorf("%s: the home page's card does not hold %s", tt.name, tt.want)
+		}
+
+		logged := loggedAddresses(requestLog.String())
+		if len(logged) == 0 {
+			t.Errorf("%s: the request log names no address", tt.name)
+		}
+
+		for _, addr := range logged {
+			if addr != tt.want {
+				t.Errorf("%s: the request log gives %s, want %s", tt.name, addr, tt.want)
+			}
+		}
+	}
+}
+
+// newServerWithConfig returns a server built with cfg, on a new database, that
+// reads the routes of newTestTable and logs its requests as JSON to
+// requestLog.
+func newServerWithConfig(
+	t *testing.T, cfg *config.Config, requestLog io.Writer,
+) *Server {
+	t.Helper()
+
+	cfg.StateDir = t.TempDir()
+
+	db, err := database.New(cfg, logger.New())
+	if err != nil {
+		t.Fatalf("database.New: %v", err)
+	}
+
+	t.Cleanup(func() { _ = db.Close() })
+
+	s := New(db, newTestTable(), streamer.New(logger.New(), metrics.New()),
+		&logger.Logger{Logger: slog.New(slog.NewJSONHandler(requestLog, nil))}, cfg)
+	s.resolver = &fakeResolver{}
+
+	return s
+}
+
+// loggedAddresses returns the address each line of the JSON request log
+// names, without the port a direct request is logged with.
+func loggedAddresses(requestLog string) []string {
+	var addrs []string
+
+	for line := range strings.Lines(requestLog) {
+		//nolint:tagliatelle // the field name is the key the request log uses
+		var entry struct {
+			RemoteAddr string `json:"remote_addr"`
+		}
+
+		err := json.Unmarshal([]byte(line), &entry)
+		if err != nil || entry.RemoteAddr == "" {
+			continue
+		}
+
+		addr, _, err := net.SplitHostPort(entry.RemoteAddr)
+		if err != nil {
+			addr = entry.RemoteAddr
+		}
+
+		addrs = append(addrs, addr)
+	}
+
+	return addrs
+}
+
 // TestRealIPMiddlewareTrustsOnlyConfiguredProxies checks which address
 // RealIPMiddleware gives a request: the TCP peer, unless the peer is a trusted
 // proxy; then the last X-Forwarded-For entry that is not a trusted proxy,