Honour forwarded headers only from trusted proxies (closes #259)

A new ClientIP middleware sets request.RemoteAddr to the client's
address before the request is logged. X-Forwarded-For and X-Real-IP
count only when the peer is in TRUSTED_PROXIES, which defaults to the
RFC 1918 ranges. The login rate limit, the session's IP address and
hostname, and the request log all use that address.

Model: opus-5-5

README.md

@@ -1821,7 +1821,7 @@ over HTTPS. The proxy must:
       default does not trust them, so set `TRUSTED_PROXIES=127.0.0.1,::1`.
     - Through a published Docker port, that address is the Docker bridge
       gateway, such as `172.17.0.1`. The default trusts it.
-- Add the client's address to `X-Forwarded-For`.
+- Add the client's address to `X-Forwarded-For`, or set `X-Real-IP`.
 - Wait at least 60 seconds for a response. A long poll can take 30 seconds, and
   the server's own write timeout is 60 seconds.
 
@@ -1890,21 +1890,10 @@ The IRC listener has no TLS.
 
 The server logs with Go's `slog`, one line per event, to standard output. The
 lines are JSON, or plain text when standard output is a terminal. Every HTTP
-request is logged with:
-
-- start time
-- method
-- URL
-- protocol
-- status
-- latency
-- client address
-- user agent
-- referrer
-- request ID
-
-The client address is the one found as described in
-[Reverse proxy](#reverse-proxy). `DEBUG=true` adds debug-level lines.
+request is logged with its method, URL, protocol, status, latency, client
+address, user agent, referrer and request ID. The client address is the one
+found as described in [Reverse proxy](#reverse-proxy). `DEBUG=true` adds
+debug-level lines.
 
 ### The database
 

internal/config/config.go

@@ -43,8 +43,6 @@ var errOutOfRange = errors.New("setting out of range")
 
 var errNeverMatches = errors.New("entry can never match a connection")
 
-var errNotString = errors.New("not a string")
-
 // Params defines the dependencies for creating a Config.
 type Params struct {
 	fx.In
@@ -191,19 +189,11 @@ func parseSettings(cfg *Config) error {
 // counts an empty environment variable as unset, so the environment is
 // read directly as well.
 func parseTrustedProxies(value **netipx.IPSet) error {
-	setting := viper.Get("TRUSTED_PROXIES")
+	raw := viper.GetString("TRUSTED_PROXIES")
 
 	fromEnv, set := os.LookupEnv("TRUSTED_PROXIES")
 	if set {
-		setting = fromEnv
-	}
-
-	// A YAML list or map in the config file is not a string, and
-	// viper.GetString would turn it into the empty string.
-	raw, isString := setting.(string)
-	if !isString {
-		return fmt.Errorf("cannot parse TRUSTED_PROXIES: %v is %w",
-			setting, errNotString)
+		raw = fromEnv
 	}
 
 	var builder netipx.IPSetBuilder

internal/config/config_test.go

@@ -3,12 +3,10 @@ package config_test
 import (
 	"net/netip"
 	"os"
-	"path/filepath"
 	"strings"
 	"testing"
 	"time"
 
-	"github.com/spf13/viper"
 	"sneak.berlin/go/neoirc/internal/config"
 	"sneak.berlin/go/neoirc/internal/globals"
 	"sneak.berlin/go/neoirc/internal/logger"
@@ -143,7 +141,8 @@ func settingsOf(cfg *config.Config) settings {
 }
 
 func TestBadSettingStopsStartup(t *testing.T) {
-	// Each value is invalid for its setting.
+	// For each setting, the first value does not parse and the others
+	// parse but are out of range.
 	tests := []struct {
 		key    string
 		values []string
@@ -286,50 +285,3 @@ func TestTrustedProxies(t *testing.T) {
 		})
 	}
 }
-
-func TestTrustedProxiesNotAStringInYAMLStopsStartup(t *testing.T) {
-	// YAML reads the first value as a list and the second as a map.
-	values := []string{"[10.0.0.1]", "{proxy: 10.0.0.1}"}
-
-	for _, value := range values {
-		t.Run(value, func(t *testing.T) {
-			// t.Setenv puts back whatever value was there after the
-			// test, including when Unsetenv removes it.
-			t.Setenv("TRUSTED_PROXIES", "")
-
-			err := os.Unsetenv("TRUSTED_PROXIES")
-			if err != nil {
-				t.Fatal(err)
-			}
-
-			// loadConfig names the app neoirc-test, so config.New reads
-			// $HOME/.config/neoirc-test/neoirc-test.yaml.
-			home := t.TempDir()
-			t.Setenv("HOME", home)
-
-			dir := filepath.Join(home, ".config", "neoirc-test")
-
-			err = os.MkdirAll(dir, 0o700)
-			if err != nil {
-				t.Fatal(err)
-			}
-
-			err = os.WriteFile(
-				filepath.Join(dir, "neoirc-test.yaml"),
-				[]byte("TRUSTED_PROXIES: "+value+"\n"), 0o600,
-			)
-			if err != nil {
-				t.Fatal(err)
-			}
-
-			// Viper keeps the values it read from this file after the
-			// file is gone. Reset clears them for later tests.
-			t.Cleanup(viper.Reset)
-
-			_, err = loadConfig(t)
-			if err == nil || !strings.Contains(err.Error(), "TRUSTED_PROXIES") {
-				t.Errorf("error = %v, want one naming TRUSTED_PROXIES", err)
-			}
-		})
-	}
-}