feat: blocked_networks config and extended SSRF ranges (closes #67)

Add a blocked_networks config key: a list of CIDRs parsed with net/netip,
added to (not replacing) the built-in SSRF blocklist. An invalid CIDR
aborts startup naming the key and the offending value.

Extend the built-in blocklist to CGNAT 100.64.0.0/10, IETF protocol
assignments 192.0.0.0/24, benchmark 198.18.0.0/15, and NAT64 64:ff9b::/96,
unmapping IPv4-mapped IPv6 so the IPv4 ranges are caught in both forms.
Enforcement stays in the dial-time re-resolution (dialSSRFSafe), which now
also consults the operator-supplied prefixes, so the DNS-rebinding window
remains closed.

Model: opus-4-8

Dockerfile

@@ -67,9 +67,8 @@ RUN adduser -D -H -s /sbin/nologin pixad && \
     mkdir -p /var/lib/pixa /etc/pixa && \
     chown pixad:pixad /var/lib/pixa
 
-# Copy the image config; signing_key comes from PIXA_SIGNING_KEY.
-# Mount a file over /etc/pixa/config.yml to override anything else.
-COPY config.docker.yml /etc/pixa/config.yml
+# Copy default config (edit signing_key before use)
+COPY config.example.yml /etc/pixa/config.yml
 
 USER pixad
 WORKDIR /var/lib/pixa

README.md

@@ -15,25 +15,14 @@ git clone https://git.eeqj.de/sneak/pixa.git
 cd pixa
 make build
 
-# run with a config file: copy the example and set a real signing key
-# (the example placeholder is refused at startup), e.g. with
-# openssl rand -base64 32
-cp config.example.yml config.yml
-$EDITOR config.yml   # replace the signing_key placeholder
-./bin/pixad --config config.yml
+# run with a config file
+./bin/pixad --config config.example.yml
 
 # or build and run via Docker
 make docker
-docker run -p 8080:8080 -e PIXA_SIGNING_KEY="$(openssl rand -base64 32)" pixa:latest
+docker run -p 8080:8080 pixad:latest
 ```
 
-A container is configured two ways. The signing key comes from the
-`PIXA_SIGNING_KEY` environment variable, which the baked-in config
-reads; if it is unset the container exits at startup naming the
-variable. Everything else uses built-in defaults, so to change any
-other setting mount your own file over `/etc/pixa/config.yml` (see
-`config.example.yml` for the full set of keys).
-
 ## Rationale
 
 Image-heavy web applications need a fast, caching reverse proxy that
@@ -126,18 +115,6 @@ Configured via YAML file (`--config`). Key settings:
   added to the always-enforced built-in ranges (loopback, private,
   link-local, CGNAT, benchmark, NAT64, and the like); an invalid CIDR
   aborts startup
-- `trusted_proxies` — list of CIDR ranges of the reverse proxies in front
-  of pixa. `X-Forwarded-For` is believed only when the direct peer falls
-  inside one of these ranges; the logged and login-recorded client
-  address is then the rightmost forwarded entry that is not itself a
-  trusted proxy. Otherwise the direct peer address is used and the header
-  is ignored, so a client connecting directly cannot spoof its address.
-  An omitted key defaults to the RFC 1918 private ranges (`10.0.0.0/8`,
-  `172.16.0.0/12`, `192.168.0.0/16`), since pixa is deployed behind a
-  proxy on a private network; an explicitly empty list (`[]`) trusts no
-  one, and an explicit list replaces the default. An invalid CIDR aborts
-  startup. Set this to your proxy's address range if it is not already
-  covered by the defaults
 - `upstream_fetch_timeout` — timeout for origin requests
 - `upstream_max_response_size` — max origin response size
 - `downstream_timeout` — client response timeout

TODO.md

@@ -30,19 +30,6 @@ exhaustion
 
 # Completed Steps
 
-- 2026-09-21 trusted-proxy client IP resolution (closes #94): a
-  `trusted_proxies` config key taking a list of CIDRs, parsed by the same
-  `net/netip` list parser as `blocked_networks` (an invalid entry aborts
-  startup naming the key and value; an omitted key defaults to the RFC 1918
-  private ranges, an explicitly empty list trusts no one, and an explicit
-  list replaces the default); a new
-  `internal/clientip` package resolves the client address by honoring
-  `X-Forwarded-For` only when the direct peer is a trusted proxy, walking
-  the chain right-to-left to the rightmost non-proxy entry, so a client
-  connecting directly cannot spoof its address; the resolved address is
-  stored in the request context by a new middleware and used by the
-  request-logging middleware and the login-attempt logs in place of the
-  raw peer address; documented in `README.md` and `config.example.yml`.
 - 2026-09-21 blocked networks configuration extending SSRF protection: a
   `blocked_networks` config key taking a list of CIDRs (parsed with
   `net/netip`, an invalid entry aborts startup naming the key and value),
@@ -52,14 +39,6 @@ exhaustion
   (IPv4-mapped forms covered); enforcement stays in the dial-time
   re-resolution so the DNS-rebinding window remains closed; documented in
   `README.md` and `config.example.yml`.
-- 2026-09-21 http.Server hardening (closes #92): added
-  `HTTPReadHeaderTimeout` (10s, bounds the slowloris header dribble) and
-  `HTTPIdleTimeout` (120s, bounds keep-alive reuse) alongside the
-  existing timeouts and wired them onto the server; added a `LimitBody`
-  middleware capping the two form POST bodies (`POST /`, `POST /generate`)
-  at `MaxFormBytes` (1 MiB) and returning 413, applied ahead of the CSRF
-  middleware so an oversized body is refused as 413 rather than being read
-  as a missing CSRF token (403); left `WriteTimeout` at 60s unchanged
 - 2026-08-07 update golangci-lint to v2.12.2 with the canonical
   `.golangci.yml` (v2 schema, `default: all` minus six disabled
   linters, `lll` 88, tests included): bumped the pinned

config.docker.yml

@@ -1,11 +0,0 @@
-# Pixa configuration baked into the Docker image.
-#
-# The signing key is read from the PIXA_SIGNING_KEY environment
-# variable; startup aborts naming it when it is unset. Every other key
-# is omitted so its default applies. Operators who need more (an
-# allowlist, metrics, and so on) mount their own file over
-# /etc/pixa/config.yml.
-
-signing_key: "${ENV:PIXA_SIGNING_KEY}"
-state_dir: /var/lib/pixa
-port: 8080

config.example.yml

@@ -31,21 +31,6 @@ allowlist_hosts:
 #   - 100.64.0.0/10
 #   - 2001:db8::/32
 
-# CIDR ranges of the reverse proxies in front of pixa. X-Forwarded-For
-# is believed only when the direct peer is inside one of these ranges;
-# the client address in the access log and login records is then the
-# rightmost forwarded entry that is not itself a trusted proxy. A client
-# connecting directly (peer outside these ranges) cannot spoof its
-# address: the header is ignored and the peer address is used. When
-# omitted, this defaults to the RFC 1918 private ranges (10.0.0.0/8,
-# 172.16.0.0/12, 192.168.0.0/16), since pixa is deployed behind a proxy on
-# a private network. An explicitly empty list ([]) trusts no one; an
-# explicit list replaces the default. An invalid CIDR aborts startup.
-# Uncomment to override the defaults with your proxy's address range.
-# trusted_proxies:
-#   - 10.0.0.0/8
-#   - 2001:db8::/32
-
 # Allow HTTP upstream (only for testing, always use HTTPS in production)
 allow_http: false
 

internal/clientip/clientip.go

@@ -1,119 +0,0 @@
-// Package clientip resolves the real client IP address of an HTTP request
-// when pixa runs behind a reverse proxy. Forwarding headers are believed
-// only when the immediate peer is a configured trusted proxy, so an
-// untrusted client cannot spoof its address by sending the header.
-package clientip
-
-import (
-	"context"
-	"net"
-	"net/netip"
-	"slices"
-	"strings"
-)
-
-// ForwardedForHeader is the request header carrying the proxy chain. It is
-// honored only when the immediate peer is a trusted proxy.
-const ForwardedForHeader = "X-Forwarded-For"
-
-// Resolver determines the client IP of a request against a fixed set of
-// trusted proxy networks.
-type Resolver struct {
-	trusted []netip.Prefix
-}
-
-// NewResolver returns a Resolver that trusts forwarding headers only from
-// peers inside the given CIDR ranges. A nil or empty list trusts no one,
-// so the peer address is always used.
-func NewResolver(trusted []netip.Prefix) *Resolver {
-	return &Resolver{trusted: trusted}
-}
-
-// Resolve returns the client IP for a request whose direct peer is
-// remoteAddr (a "host:port" string as in http.Request.RemoteAddr) and
-// whose X-Forwarded-For header lines are forwardedFor (as returned by
-// http.Header.Values). When the peer is not a trusted proxy, the peer
-// address is returned and the header is ignored entirely. When the peer is
-// trusted, the header is walked right to left and the first address that is
-// not itself a trusted proxy is returned; this is the client the outermost
-// trusted proxy observed, and entries an untrusted client may have prepended
-// sit to its left and are never reached.
-func (r *Resolver) Resolve(remoteAddr string, forwardedFor []string) string {
-	peer := hostOnly(remoteAddr)
-
-	peerAddr, err := netip.ParseAddr(peer)
-	if err != nil || !r.isTrusted(peerAddr) {
-		return peer
-	}
-
-	for _, hop := range slices.Backward(forwardedForChain(forwardedFor)) {
-		hopAddr, err := netip.ParseAddr(hop)
-		if err != nil || r.isTrusted(hopAddr) {
-			continue
-		}
-
-		return hopAddr.String()
-	}
-
-	return peerAddr.String()
-}
-
-// isTrusted reports whether addr falls inside one of the trusted proxy
-// ranges. Addresses are unmapped first so an IPv4-mapped IPv6 form matches
-// an IPv4 range, matching the fetcher's blocklist comparison.
-func (r *Resolver) isTrusted(addr netip.Addr) bool {
-	if !addr.IsValid() {
-		return false
-	}
-
-	unmapped := addr.Unmap()
-
-	return slices.ContainsFunc(r.trusted, func(prefix netip.Prefix) bool {
-		return prefix.Contains(unmapped)
-	})
-}
-
-// hostOnly strips the port from a "host:port" address. A value without a
-// port (already a bare host) is returned unchanged.
-func hostOnly(remoteAddr string) string {
-	host, _, err := net.SplitHostPort(remoteAddr)
-	if err != nil {
-		return remoteAddr
-	}
-
-	return host
-}
-
-// forwardedForChain flattens the comma-separated entries of every
-// X-Forwarded-For header line into a single ordered, trimmed list.
-func forwardedForChain(values []string) []string {
-	var chain []string
-
-	for _, value := range values {
-		for part := range strings.SplitSeq(value, ",") {
-			trimmed := strings.TrimSpace(part)
-			if trimmed != "" {
-				chain = append(chain, trimmed)
-			}
-		}
-	}
-
-	return chain
-}
-
-// contextKey is the private key type under which the resolved client IP is
-// stored in a request context.
-type contextKey struct{}
-
-// WithClientIP returns a copy of ctx carrying the resolved client IP.
-func WithClientIP(ctx context.Context, ip string) context.Context {
-	return context.WithValue(ctx, contextKey{}, ip)
-}
-
-// FromContext returns the resolved client IP stored in ctx, or an empty
-// string if none was set.
-func FromContext(ctx context.Context) string {
-	ip, _ := ctx.Value(contextKey{}).(string)
-
-	return ip
-}

internal/clientip/clientip_test.go

@@ -1,189 +0,0 @@
-package clientip_test
-
-import (
-	"net/netip"
-	"testing"
-
-	"sneak.berlin/go/pixa/internal/clientip"
-)
-
-// Addresses reused across the resolver cases.
-const (
-	trustedRangeV4 = "10.0.0.0/8"
-	forwardedV4    = "203.0.113.7"
-	untrustedV4    = "198.51.100.9"
-	trustedPeer    = "10.0.0.1:5000"
-)
-
-// mustPrefixes parses CIDR strings into prefixes for building a resolver.
-func mustPrefixes(t *testing.T, cidrs ...string) []netip.Prefix {
-	t.Helper()
-
-	prefixes := make([]netip.Prefix, 0, len(cidrs))
-
-	for _, c := range cidrs {
-		p, err := netip.ParsePrefix(c)
-		if err != nil {
-			t.Fatalf("netip.ParsePrefix(%q) error = %v", c, err)
-		}
-
-		prefixes = append(prefixes, p)
-	}
-
-	return prefixes
-}
-
-type resolveCase struct {
-	name         string
-	trusted      []string
-	remoteAddr   string
-	forwardedFor []string
-	want         string
-}
-
-// runResolveCases runs each case against a resolver built from its trusted
-// list and checks the resolved address.
-func runResolveCases(t *testing.T, cases []resolveCase) {
-	t.Helper()
-
-	for _, tt := range cases {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			r := clientip.NewResolver(mustPrefixes(t, tt.trusted...))
-
-			got := r.Resolve(tt.remoteAddr, tt.forwardedFor)
-			if got != tt.want {
-				t.Errorf("Resolve(%q, %v) = %q, want %q",
-					tt.remoteAddr, tt.forwardedFor, got, tt.want)
-			}
-		})
-	}
-}
-
-// TestResolvePeerTrust covers the trust decision on the direct peer: a
-// forwarded header is believed only from a trusted peer, and a client
-// connecting directly cannot spoof its address.
-func TestResolvePeerTrust(t *testing.T) {
-	t.Parallel()
-
-	runResolveCases(t, []resolveCase{
-		{
-			name:         "trusted peer honors forwarded client",
-			trusted:      []string{trustedRangeV4},
-			remoteAddr:   trustedPeer,
-			forwardedFor: []string{forwardedV4},
-			want:         forwardedV4,
-		},
-		{
-			name:         "untrusted peer ignores forwarded header",
-			trusted:      []string{trustedRangeV4},
-			remoteAddr:   untrustedV4 + ":33333",
-			forwardedFor: []string{forwardedV4},
-			want:         untrustedV4,
-		},
-		{
-			name:         "spoofed chain from untrusted peer cannot influence result",
-			trusted:      []string{trustedRangeV4},
-			remoteAddr:   untrustedV4 + ":33333",
-			forwardedFor: []string{"1.2.3.4, 10.9.9.9, 127.0.0.1"},
-			want:         untrustedV4,
-		},
-		{
-			name:         "empty trusted list always uses peer",
-			trusted:      nil,
-			remoteAddr:   forwardedV4 + ":80",
-			forwardedFor: []string{"10.0.0.5"},
-			want:         forwardedV4,
-		},
-		{
-			name:         "trusted peer with no forwarded header uses peer",
-			trusted:      []string{trustedRangeV4},
-			remoteAddr:   trustedPeer,
-			forwardedFor: nil,
-			want:         "10.0.0.1",
-		},
-		{
-			name:         "unparseable peer is returned unchanged",
-			trusted:      []string{trustedRangeV4},
-			remoteAddr:   "garbage",
-			forwardedFor: []string{forwardedV4},
-			want:         "garbage",
-		},
-	})
-}
-
-// TestResolveChainWalk covers walking the X-Forwarded-For chain from a
-// trusted peer to the rightmost entry that is not itself a trusted proxy.
-func TestResolveChainWalk(t *testing.T) {
-	t.Parallel()
-
-	runResolveCases(t, []resolveCase{
-		{
-			name:         "rightmost untrusted entry across a mixed chain",
-			trusted:      []string{trustedRangeV4, "192.168.0.0/16"},
-			remoteAddr:   trustedPeer,
-			forwardedFor: []string{forwardedV4 + ", 192.168.1.1, 10.0.0.2"},
-			want:         forwardedV4,
-		},
-		{
-			name:         "spoofed client behind a trusted proxy is not believed",
-			trusted:      []string{trustedRangeV4},
-			remoteAddr:   trustedPeer,
-			forwardedFor: []string{"1.2.3.4, " + untrustedV4},
-			want:         untrustedV4,
-		},
-		{
-			name:         "chain split across multiple header lines",
-			trusted:      []string{trustedRangeV4},
-			remoteAddr:   trustedPeer,
-			forwardedFor: []string{forwardedV4, "10.0.0.2"},
-			want:         forwardedV4,
-		},
-		{
-			name:         "garbage entries are skipped",
-			trusted:      []string{trustedRangeV4},
-			remoteAddr:   trustedPeer,
-			forwardedFor: []string{forwardedV4 + ", not-an-ip"},
-			want:         forwardedV4,
-		},
-		{
-			name:         "all-trusted chain falls back to peer",
-			trusted:      []string{trustedRangeV4},
-			remoteAddr:   trustedPeer,
-			forwardedFor: []string{"10.0.0.9, 10.0.0.2"},
-			want:         "10.0.0.1",
-		},
-		{
-			name:         "trusted IPv6 peer honors forwarded client",
-			trusted:      []string{"2001:db8::/32"},
-			remoteAddr:   "[2001:db8::1]:9000",
-			forwardedFor: []string{forwardedV4},
-			want:         forwardedV4,
-		},
-		{
-			name:         "IPv4-mapped peer matches IPv4 trusted range",
-			trusted:      []string{trustedRangeV4},
-			remoteAddr:   "[::ffff:10.0.0.1]:5000",
-			forwardedFor: []string{forwardedV4},
-			want:         forwardedV4,
-		},
-	})
-}
-
-func TestContextRoundTrip(t *testing.T) {
-	t.Parallel()
-
-	ctx := clientip.WithClientIP(t.Context(), forwardedV4)
-	if got := clientip.FromContext(ctx); got != forwardedV4 {
-		t.Errorf("FromContext = %q, want %q", got, forwardedV4)
-	}
-}
-
-func TestFromContextAbsent(t *testing.T) {
-	t.Parallel()
-
-	if got := clientip.FromContext(t.Context()); got != "" {
-		t.Errorf("FromContext with no value = %q, want empty", got)
-	}
-}

internal/config/config.go

@@ -44,15 +44,8 @@ const (
 	keyUpstreamConnectionsPerHost = "upstream_connections_per_host"
 	keyCacheMaxBytes              = "cache_max_bytes"
 	keyBlockedNetworks            = "blocked_networks"
-	keyTrustedProxies             = "trusted_proxies"
 )
 
-// placeholderSigningKey is the dummy signing_key shipped in
-// config.example.yml. It is 45 characters, so it passes the length
-// check, but it is public in this repository and must be rejected at
-// startup so no deployment ever signs URLs with it.
-const placeholderSigningKey = "CHANGE_ME_generate_with_openssl_rand_base64_32"
-
 // Static validation errors. Each use site attaches the offending key
 // and value by wrapping these with fmt.Errorf and %w.
 var (
@@ -71,9 +64,6 @@ var (
 	errPortOutOfRange    = errors.New("outside the valid port range")
 	errTooFewConnections = errors.New("must be at least 1")
 	errValueTooShort     = errors.New("value too short")
-	errPlaceholderKey    = errors.New(
-		"is the placeholder from config.example.yml; " +
-			"generate a real key with: openssl rand -base64 32")
 	errMustBeSetTogether = errors.New("must be set together")
 	errMustNotBeNegative = errors.New("must not be negative")
 	errOverflowsInt64    = errors.New("overflows a 64-bit integer")
@@ -118,17 +108,6 @@ type Config struct {
 	// fetcher's dialer; the built-in ranges always apply.
 	BlockedNetworks []netip.Prefix
 
-	// TrustedProxies are the CIDR ranges of reverse proxies whose
-	// forwarding headers may be believed. Forwarded headers are honored
-	// only when the immediate peer falls inside one of these ranges;
-	// otherwise the peer address is used and the headers are ignored, so
-	// an untrusted client cannot spoof its address. An omitted key
-	// defaults to the RFC 1918 private ranges (see defaultTrustedProxies),
-	// since pixa is deployed behind a proxy on a private network; an
-	// explicitly empty list trusts nothing and always uses the peer
-	// address, and an explicit list replaces the default.
-	TrustedProxies []netip.Prefix
-
 	// CacheMaxBytes is the disk cache size limit in bytes. Zero
 	// disables the disk cache entirely. When cache_max_bytes is
 	// omitted from the configuration, this holds the computed default
@@ -197,24 +176,11 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
 		}
 	}
 
-	blockedNetworks, err := parseCIDRList(sc, keyBlockedNetworks)
+	blockedNetworks, err := getBlockedNetworks(sc)
 	if err != nil {
 		return nil, err
 	}
 
-	trustedProxies, err := parseCIDRList(sc, keyTrustedProxies)
-	if err != nil {
-		return nil, err
-	}
-
-	// parseCIDRList returns a nil slice only when the key is absent; an
-	// explicitly empty list ([]) comes back non-nil and empty. An omitted
-	// key takes the RFC 1918 default, while an explicit empty list is left
-	// as trust-nothing.
-	if trustedProxies == nil {
-		trustedProxies = defaultTrustedProxies()
-	}
-
 	loader := &strictLoader{sc: sc}
 
 	c := &Config{
@@ -232,7 +198,6 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
 			keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost),
 		CacheMaxBytes:   loader.int64Val(keyCacheMaxBytes, 0),
 		BlockedNetworks: blockedNetworks,
-		TrustedProxies:  trustedProxies,
 	}
 
 	// The computed default for cache_max_bytes needs a validated
@@ -348,8 +313,7 @@ func isKnownConfigKey(key string) bool {
 	switch key {
 	case keyDebug, keyMaintenanceMode, keyPort, keyStateDir, keySentryDSN,
 		keyDBURL, keyMetrics, keySigningKey, keyAllowlistHosts, keyAllowHTTP,
-		keyUpstreamConnectionsPerHost, keyCacheMaxBytes, keyBlockedNetworks,
-		keyTrustedProxies, "env":
+		keyUpstreamConnectionsPerHost, keyCacheMaxBytes, keyBlockedNetworks, "env":
 		return true
 	}
 
@@ -391,10 +355,10 @@ func (c *Config) ensureStateDirWritable() error {
 	return nil
 }
 
-// validateSigningKey checks that the signing key is present, long
-// enough, and not the public placeholder from config.example.yml. The
-// key value itself is never echoed in error messages.
-func (c *Config) validateSigningKey() error {
+// validate checks that all required configuration values are set and
+// that every value is within its valid range.
+func (c *Config) validate() error {
+	// The signing key value is never echoed in error messages.
 	if c.SigningKey == "" {
 		return fmt.Errorf("config key %q: %w", keySigningKey, errValueRequired)
 	}
@@ -406,21 +370,6 @@ func (c *Config) validateSigningKey() error {
 			keySigningKey, errValueTooShort, minKeyLength, len(c.SigningKey))
 	}
 
-	if c.SigningKey == placeholderSigningKey {
-		return fmt.Errorf("config key %q: %w", keySigningKey, errPlaceholderKey)
-	}
-
-	return nil
-}
-
-// validate checks that all required configuration values are set and
-// that every value is within its valid range.
-func (c *Config) validate() error {
-	err := c.validateSigningKey()
-	if err != nil {
-		return err
-	}
-
 	const maxPort = 65535
 	if c.Port < 1 || c.Port > maxPort {
 		return fmt.Errorf("config key %q: value %d is %w 1-%d",
@@ -844,42 +793,27 @@ func getStringSlice(sc *smartconfig.Config) []string {
 	return nil
 }
 
-// defaultTrustedProxies returns the trusted_proxies default: the three RFC
-// 1918 private ranges. pixa is always deployed behind a TLS-terminating
-// reverse proxy, which in practice sits on a private network, so its
-// forwarding headers are believed unless the operator says otherwise.
-// Loopback is deliberately excluded: it is not an RFC 1918 range, and no
-// deployment reaches pixa over it. A fresh slice is returned on each call so
-// callers may hold it without aliasing shared state.
-func defaultTrustedProxies() []netip.Prefix {
-	return []netip.Prefix{
-		netip.MustParsePrefix("10.0.0.0/8"),
-		netip.MustParsePrefix("172.16.0.0/12"),
-		netip.MustParsePrefix("192.168.0.0/16"),
-	}
-}
-
-// parseCIDRList parses the value of the named config key into CIDR
-// prefixes, or returns nil if the key is omitted. It accepts a YAML list
-// of strings or a comma-separated string. An explicitly null value, a
-// wrong type, an empty entry, a non-string entry, or an unparseable CIDR
-// aborts startup naming the key and the offending value; the default
-// (an empty list) applies only to an omitted key.
-func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) {
+// getBlockedNetworks parses the blocked_networks value into CIDR prefixes,
+// or returns nil if the key is omitted. It accepts a YAML list of strings
+// or a comma-separated string. An explicitly null value, a wrong type, an
+// empty entry, a non-string entry, or an unparseable CIDR aborts startup
+// naming the key and the offending value; a default (the built-in
+// blocklist alone) applies only to an omitted key.
+func getBlockedNetworks(sc *smartconfig.Config) ([]netip.Prefix, error) {
 	if sc == nil {
 		return nil, nil
 	}
 
-	raw, ok := sc.Get(key)
+	raw, ok := sc.Get(keyBlockedNetworks)
 	if !ok {
 		return nil, nil
 	}
 
 	if raw == nil {
-		return nil, errNullConfigValue(key)
+		return nil, errNullConfigValue(keyBlockedNetworks)
 	}
 
-	entries, err := cidrListEntries(raw, key)
+	entries, err := blockedNetworkEntries(raw)
 	if err != nil {
 		return nil, err
 	}
@@ -890,7 +824,7 @@ func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) {
 		prefix, err := netip.ParsePrefix(entry)
 		if err != nil {
 			return nil, fmt.Errorf("config key %q: value %q is %w",
-				key, entry, errNotAValidCIDR)
+				keyBlockedNetworks, entry, errNotAValidCIDR)
 		}
 
 		prefixes = append(prefixes, prefix)
@@ -899,10 +833,10 @@ func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) {
 	return prefixes, nil
 }
 
-// cidrListEntries extracts the raw entries of the named CIDR-list key as
+// blockedNetworkEntries extracts the raw blocked_networks entries as
 // trimmed, non-empty strings, from either a YAML list of strings or a
 // comma-separated string. Any other shape is a configuration error.
-func cidrListEntries(raw any, key string) ([]string, error) {
+func blockedNetworkEntries(raw any) ([]string, error) {
 	switch val := raw.(type) {
 	case []any:
 		entries := make([]string, 0, len(val))
@@ -911,12 +845,12 @@ func cidrListEntries(raw any, key string) ([]string, error) {
 			str, ok := item.(string)
 			if !ok {
 				return nil, fmt.Errorf("config key %q: list entry %v (%T) is %w",
-					key, item, item, errNotAString)
+					keyBlockedNetworks, item, item, errNotAString)
 			}
 
 			if strings.TrimSpace(str) == "" {
 				return nil, fmt.Errorf("config key %q: %w",
-					key, errEmptyListEntry)
+					keyBlockedNetworks, errEmptyListEntry)
 			}
 
 			entries = append(entries, strings.TrimSpace(str))
@@ -930,7 +864,7 @@ func cidrListEntries(raw any, key string) ([]string, error) {
 			trimmed := strings.TrimSpace(part)
 			if trimmed == "" {
 				return nil, fmt.Errorf("config key %q: value %q %w",
-					key, val, errEmptyEntry)
+					keyBlockedNetworks, val, errEmptyEntry)
 			}
 
 			entries = append(entries, trimmed)
@@ -939,6 +873,6 @@ func cidrListEntries(raw any, key string) ([]string, error) {
 		return entries, nil
 	default:
 		return nil, fmt.Errorf("config key %q: value %v (%T) is %w",
-			key, raw, raw, errNotAStringList)
+			keyBlockedNetworks, raw, raw, errNotAStringList)
 	}
 }

internal/config/config_validation_internal_test.go

@@ -303,11 +303,6 @@ func invalidHostAndCredentialCases() []abortCase {
 			yaml:              "signing_key: short\n",
 			wantErrSubstrings: []string{keySigningKey},
 		},
-		{
-			name:              "signing_key is the documented placeholder",
-			yaml:              "signing_key: " + placeholderSigningKey + "\n",
-			wantErrSubstrings: []string{keySigningKey},
-		},
 		{
 			name:              "signing_key missing",
 			yaml:              "port: 8080\n",

internal/config/trusted_proxies_internal_test.go

@@ -1,85 +0,0 @@
-package config
-
-import (
-	"strings"
-	"testing"
-)
-
-// TestTrustedProxiesConfig checks the trusted_proxies key wiring: an
-// explicit CIDR list lands in TrustedProxies in order and replaces the
-// default, an omitted key falls back to the RFC 1918 private ranges, and an
-// explicitly empty list trusts no one. The list parser itself is shared with
-// blocked_networks and is exercised in depth by that key's tests.
-func TestTrustedProxiesConfig(t *testing.T) {
-	t.Parallel()
-
-	t.Run("explicit list replaces the default in order", func(t *testing.T) {
-		t.Parallel()
-
-		c, err := configFromYAML(t,
-			signingKeyLine+`trusted_proxies: ["10.0.0.0/8", "2001:db8::/32"]`+"\n")
-		if err != nil {
-			t.Fatalf("valid trusted_proxies should load: %v", err)
-		}
-
-		got := make([]string, len(c.TrustedProxies))
-		for i, p := range c.TrustedProxies {
-			got[i] = p.String()
-		}
-
-		if joined := strings.Join(got, ","); joined != "10.0.0.0/8,2001:db8::/32" {
-			t.Errorf("TrustedProxies = %v, want the two ranges in order", got)
-		}
-	})
-
-	t.Run("omitted key defaults to the RFC 1918 ranges", func(t *testing.T) {
-		t.Parallel()
-
-		c, err := configFromYAML(t, signingKeyLine)
-		if err != nil {
-			t.Fatalf("minimal config should load: %v", err)
-		}
-
-		got := make([]string, len(c.TrustedProxies))
-		for i, p := range c.TrustedProxies {
-			got[i] = p.String()
-		}
-
-		want := "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
-		if joined := strings.Join(got, ","); joined != want {
-			t.Errorf("TrustedProxies = %v, want the RFC 1918 ranges %q", got, want)
-		}
-	})
-
-	t.Run("explicitly empty list trusts no one", func(t *testing.T) {
-		t.Parallel()
-
-		c, err := configFromYAML(t, signingKeyLine+"trusted_proxies: []\n")
-		if err != nil {
-			t.Fatalf("empty trusted_proxies should load: %v", err)
-		}
-
-		if len(c.TrustedProxies) != 0 {
-			t.Errorf("TrustedProxies = %v, want empty", c.TrustedProxies)
-		}
-	})
-}
-
-// TestTrustedProxiesInvalidAbortsStartup checks that an invalid or null
-// value aborts startup with an error naming the key and the offending value.
-func TestTrustedProxiesInvalidAbortsStartup(t *testing.T) {
-	t.Parallel()
-
-	runAbortCases(t, []abortCase{
-		{
-			name:              "invalid cidr",
-			yaml:              signingKeyLine + `trusted_proxies: ["999.0.0.0/8"]` + "\n",
-			wantErrSubstrings: []string{keyTrustedProxies, "999.0.0.0/8"},
-		},
-		{
-			name:              "null value",
-			yaml:              signingKeyLine + "trusted_proxies:\n",
-			wantErrSubstrings: []string{keyTrustedProxies, nullValueText},
-		},
-	})
-}

internal/handlers/auth.go

@@ -8,7 +8,6 @@ import (
 	"strconv"
 	"time"
 
-	"sneak.berlin/go/pixa/internal/clientip"
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/imgcache"
 	"sneak.berlin/go/pixa/internal/templates"
@@ -48,8 +47,7 @@ func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) {
 
 	// Constant-time comparison to prevent timing attacks
 	if subtle.ConstantTimeCompare([]byte(submittedKey), []byte(s.config.SigningKey)) != 1 {
-		s.log.Warn("failed login attempt",
-			"remote_addr", clientip.FromContext(r.Context()))
+		s.log.Warn("failed login attempt", "remote_addr", r.RemoteAddr)
 		s.renderLogin(w, r, "Invalid signing key")
 
 		return
@@ -64,8 +62,7 @@ func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	s.log.Info("successful login",
-		"remote_addr", clientip.FromContext(r.Context()))
+	s.log.Info("successful login", "remote_addr", r.RemoteAddr)
 
 	// Redirect to generator page
 	http.Redirect(w, r, "/", http.StatusSeeOther)

internal/handlers/auth_clientip_internal_test.go

@@ -1,41 +0,0 @@
-package handlers
-
-import (
-	"bytes"
-	"log/slog"
-	"net/http"
-	"net/http/httptest"
-	"net/url"
-	"strings"
-	"testing"
-
-	"sneak.berlin/go/pixa/internal/clientip"
-	"sneak.berlin/go/pixa/internal/config"
-)
-
-// TestFailedLoginLogsResolvedClientIP verifies the failed-login record
-// carries the resolved client IP from the request context, not the raw
-// proxy peer address.
-func TestFailedLoginLogsResolvedClientIP(t *testing.T) {
-	t.Parallel()
-
-	var buf bytes.Buffer
-
-	h := &Handlers{
-		log:    slog.New(slog.NewJSONHandler(&buf, nil)),
-		config: &config.Config{SigningKey: testSigningKey},
-	}
-
-	form := url.Values{loginKeyField: {"wrong-key"}}
-	req := httptest.NewRequestWithContext(
-		t.Context(), http.MethodPost, "/",
-		strings.NewReader(form.Encode()))
-	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-	req = req.WithContext(clientip.WithClientIP(req.Context(), "203.0.113.7"))
-
-	h.handleLoginPost(httptest.NewRecorder(), req)
-
-	if !strings.Contains(buf.String(), `"remote_addr":"203.0.113.7"`) {
-		t.Errorf("failed-login log missing resolved client IP; got %q", buf.String())
-	}
-}

internal/handlers/bodylimit.go

@@ -1,45 +0,0 @@
-package handlers
-
-import (
-	"errors"
-	"net/http"
-)
-
-// MaxFormBytes bounds the request body accepted on the HTML form POST
-// routes (POST / and POST /generate). The forms carry a handful of short
-// fields, so 1 MiB is generous while making the bound explicit rather than
-// resting on ParseForm's incidental 10 MB cap.
-const MaxFormBytes = 1 << 20 // 1 MiB
-
-// LimitBody returns middleware that caps the request body on POST requests
-// at maxBytes and rejects an oversized body with 413 Request Entity Too
-// Large.
-//
-// It parses the form here, before the CSRF middleware reads the token from
-// it. The CSRF middleware reads the token with PostFormValue, which
-// swallows a parse error, so if the body were only capped there an
-// oversized body would read as a missing token and be refused as 403. By
-// parsing under the cap first, an oversized body is refused as 413. A
-// successful parse is cached on the request, so the CSRF check and the
-// handler reuse it rather than reading the body again.
-func (s *Handlers) LimitBody(maxBytes int64) func(http.Handler) http.Handler {
-	return func(next http.Handler) http.Handler {
-		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			if r.Method == http.MethodPost {
-				r.Body = http.MaxBytesReader(w, r.Body, maxBytes)
-
-				err := r.ParseForm()
-
-				var tooLarge *http.MaxBytesError
-				if errors.As(err, &tooLarge) {
-					http.Error(w, "Request body too large",
-						http.StatusRequestEntityTooLarge)
-
-					return
-				}
-			}
-
-			next.ServeHTTP(w, r)
-		})
-	}
-}

internal/handlers/bodylimit_internal_test.go

@@ -1,177 +0,0 @@
-package handlers
-
-import (
-	"log/slog"
-	"net/http"
-	"net/url"
-	"strings"
-	"testing"
-
-	"github.com/go-chi/chi/v5"
-
-	"sneak.berlin/go/pixa/internal/config"
-	"sneak.berlin/go/pixa/internal/encurl"
-	"sneak.berlin/go/pixa/internal/session"
-)
-
-// Form field names and a throwaway source image URL for the body-limit
-// tests.
-const (
-	sourceURLField = "url"
-	testSourceURL  = "https://example.com/a.jpg"
-)
-
-// newBodyLimitTestRouter mirrors the production wiring for the form POST
-// routes (see server.SetupRoutes): LimitBody sits in front of the CSRF
-// middleware, which sits in front of the handlers. maxBytes is the body
-// cap under test, so a test can trip the limit with a small body.
-func newBodyLimitTestRouter(
-	t *testing.T, maxBytes int64,
-) (*Handlers, http.Handler) {
-	t.Helper()
-
-	cfg := &config.Config{SigningKey: testSigningKey, Debug: true}
-
-	sessMgr, err := session.NewManager(testSigningKey)
-	if err != nil {
-		t.Fatalf("session.NewManager() error = %v", err)
-	}
-
-	encGen, err := encurl.NewGenerator(testSigningKey)
-	if err != nil {
-		t.Fatalf("encurl.NewGenerator() error = %v", err)
-	}
-
-	protect, err := newCSRFProtect(testSigningKey, cfg.Debug)
-	if err != nil {
-		t.Fatalf("newCSRFProtect() error = %v", err)
-	}
-
-	h := &Handlers{
-		log:         slog.New(slog.DiscardHandler),
-		config:      cfg,
-		sessMgr:     sessMgr,
-		encGen:      encGen,
-		csrfProtect: protect,
-	}
-
-	r := chi.NewRouter()
-	r.Group(func(r chi.Router) {
-		r.Use(h.LimitBody(maxBytes))
-		r.Use(h.CSRF())
-		r.Get("/", h.HandleRoot())
-		r.Post("/", h.HandleRoot())
-		r.Post("/generate", h.HandleGenerateURL())
-	})
-
-	return h, r
-}
-
-// TestOversizedLoginPostRejectedBeforeCSRF is the core regression: an
-// oversized POST / carrying an otherwise valid CSRF cookie and token must
-// be rejected with 413. If the body limit ran after CSRF, the truncated
-// body would read as a missing token and return 403; if it ran after the
-// handler, a valid token would return 303. Getting 413 proves the limit
-// fires before CSRF parses the form.
-func TestOversizedLoginPostRejectedBeforeCSRF(t *testing.T) {
-	t.Parallel()
-
-	_, srv := newBodyLimitTestRouter(t, 16)
-
-	cookies, token := csrfCredentials(t, srv, nil)
-
-	rec := postForm(srv, "/", cookies, url.Values{
-		loginKeyField:  {testSigningKey},
-		csrfTokenField: {token},
-	})
-
-	if rec.Code != http.StatusRequestEntityTooLarge {
-		t.Errorf("oversized POST / status = %d, want %d",
-			rec.Code, http.StatusRequestEntityTooLarge)
-	}
-}
-
-// TestOversizedGeneratePostRejectedBeforeCSRF is the same regression for
-// POST /generate, which also parses a form behind CSRF.
-func TestOversizedGeneratePostRejectedBeforeCSRF(t *testing.T) {
-	t.Parallel()
-
-	h, srv := newBodyLimitTestRouter(t, 16)
-
-	sessionCookie := newSessionCookie(t, h)
-
-	cookies, token := csrfCredentials(t, srv, []*http.Cookie{sessionCookie})
-	cookies = append(cookies, sessionCookie)
-
-	rec := postForm(srv, "/generate", cookies, url.Values{
-		sourceURLField: {testSourceURL},
-		csrfTokenField: {token},
-	})
-
-	if rec.Code != http.StatusRequestEntityTooLarge {
-		t.Errorf("oversized POST /generate status = %d, want %d",
-			rec.Code, http.StatusRequestEntityTooLarge)
-	}
-}
-
-// TestWithinLimitLoginPostSucceeds verifies the limit does not disturb a
-// normal request: under the production cap, a valid login still parses and
-// establishes a session (303). This guards against the body limit
-// consuming or corrupting the form the CSRF check and handler depend on.
-func TestWithinLimitLoginPostSucceeds(t *testing.T) {
-	t.Parallel()
-
-	_, srv := newBodyLimitTestRouter(t, MaxFormBytes)
-
-	cookies, token := csrfCredentials(t, srv, nil)
-
-	rec := postForm(srv, "/", cookies, url.Values{
-		loginKeyField:  {testSigningKey},
-		csrfTokenField: {token},
-	})
-
-	if rec.Code != http.StatusSeeOther {
-		t.Fatalf("within-limit POST / status = %d, want %d",
-			rec.Code, http.StatusSeeOther)
-	}
-
-	var authed bool
-
-	for _, c := range rec.Result().Cookies() {
-		if c.Name == session.CookieName && c.Value != "" {
-			authed = true
-		}
-	}
-
-	if !authed {
-		t.Error("within-limit valid login did not set a session cookie")
-	}
-}
-
-// TestWithinLimitGeneratePostSucceeds is the same non-regression check for
-// POST /generate.
-func TestWithinLimitGeneratePostSucceeds(t *testing.T) {
-	t.Parallel()
-
-	h, srv := newBodyLimitTestRouter(t, MaxFormBytes)
-
-	sessionCookie := newSessionCookie(t, h)
-
-	cookies, token := csrfCredentials(t, srv, []*http.Cookie{sessionCookie})
-	cookies = append(cookies, sessionCookie)
-
-	rec := postForm(srv, "/generate", cookies, url.Values{
-		sourceURLField: {testSourceURL},
-		"format":       {"jpeg"},
-		csrfTokenField: {token},
-	})
-
-	if rec.Code != http.StatusOK {
-		t.Fatalf("within-limit POST /generate status = %d, want %d",
-			rec.Code, http.StatusOK)
-	}
-
-	if !strings.Contains(rec.Body.String(), "/v1/e/") {
-		t.Error("within-limit generate response did not contain a generated URL")
-	}
-}

internal/middleware/client_ip_internal_test.go

@@ -1,118 +0,0 @@
-package middleware
-
-import (
-	"bytes"
-	"log/slog"
-	"net/http"
-	"net/http/httptest"
-	"net/netip"
-	"strings"
-	"testing"
-
-	"sneak.berlin/go/pixa/internal/clientip"
-	"sneak.berlin/go/pixa/internal/config"
-)
-
-// testForwardedClient is the client address the proxy forwards.
-const testForwardedClient = "203.0.113.7"
-
-// newTestMiddleware builds a Middleware whose resolver trusts the given
-// CIDRs and whose logger writes JSON to buf.
-func newTestMiddleware(t *testing.T, buf *bytes.Buffer, trusted ...string) *Middleware {
-	t.Helper()
-
-	prefixes := make([]netip.Prefix, 0, len(trusted))
-
-	for _, c := range trusted {
-		p, err := netip.ParsePrefix(c)
-		if err != nil {
-			t.Fatalf("netip.ParsePrefix(%q) error = %v", c, err)
-		}
-
-		prefixes = append(prefixes, p)
-	}
-
-	return &Middleware{
-		log:      slog.New(slog.NewJSONHandler(buf, nil)),
-		config:   &config.Config{TrustedProxies: prefixes},
-		clientIP: clientip.NewResolver(prefixes),
-	}
-}
-
-// TestClientIPMiddlewareStoresResolvedIP verifies the ClientIP middleware
-// puts the resolved address into the request context for a trusted and an
-// untrusted peer.
-func TestClientIPMiddlewareStoresResolvedIP(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name       string
-		remoteAddr string
-		forwarded  string
-		want       string
-	}{
-		{
-			name:       "trusted peer honors forwarded client",
-			remoteAddr: "10.0.0.1:5000",
-			forwarded:  testForwardedClient,
-			want:       testForwardedClient,
-		},
-		{
-			name:       "untrusted peer ignores forwarded header",
-			remoteAddr: "198.51.100.9:5000",
-			forwarded:  testForwardedClient,
-			want:       "198.51.100.9",
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			mw := newTestMiddleware(t, &bytes.Buffer{}, "10.0.0.0/8")
-
-			var got string
-
-			handler := mw.ClientIP()(http.HandlerFunc(
-				func(_ http.ResponseWriter, r *http.Request) {
-					got = clientip.FromContext(r.Context())
-				}))
-
-			req := httptest.NewRequestWithContext(
-				t.Context(), http.MethodGet, "/", nil)
-			req.RemoteAddr = tt.remoteAddr
-			req.Header.Set("X-Forwarded-For", tt.forwarded)
-
-			handler.ServeHTTP(httptest.NewRecorder(), req)
-
-			if got != tt.want {
-				t.Errorf("client IP in context = %q, want %q", got, tt.want)
-			}
-		})
-	}
-}
-
-// TestLoggingUsesResolvedClientIP verifies the logging middleware records
-// the resolved forwarded client IP rather than the proxy peer address.
-func TestLoggingUsesResolvedClientIP(t *testing.T) {
-	t.Parallel()
-
-	var buf bytes.Buffer
-
-	mw := newTestMiddleware(t, &buf, "10.0.0.0/8")
-
-	handler := mw.ClientIP()(mw.Logging()(http.HandlerFunc(
-		func(w http.ResponseWriter, _ *http.Request) {
-			w.WriteHeader(http.StatusOK)
-		})))
-
-	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
-	req.RemoteAddr = "10.0.0.1:5000"
-	req.Header.Set("X-Forwarded-For", testForwardedClient)
-
-	handler.ServeHTTP(httptest.NewRecorder(), req)
-
-	if !strings.Contains(buf.String(), `"remoteIP":"`+testForwardedClient+`"`) {
-		t.Errorf("log output missing resolved client IP; got %q", buf.String())
-	}
-}

internal/middleware/middleware.go

@@ -3,6 +3,7 @@ package middleware
 
 import (
 	"log/slog"
+	"net"
 	"net/http"
 	"time"
 
@@ -13,7 +14,6 @@ import (
 	ghmm "github.com/slok/go-http-metrics/middleware"
 	"github.com/slok/go-http-metrics/middleware/std"
 	"go.uber.org/fx"
-	"sneak.berlin/go/pixa/internal/clientip"
 	"sneak.berlin/go/pixa/internal/config"
 	"sneak.berlin/go/pixa/internal/logger"
 )
@@ -21,33 +21,6 @@ import (
 // CORSMaxAgeSeconds is the max age for CORS preflight cache (24 hours).
 const CORSMaxAgeSeconds = 86400
 
-// HSTSValue is the Strict-Transport-Security header value: one year with
-// includeSubDomains. Emitted unconditionally even though pixa listens plain
-// HTTP behind a TLS-terminating proxy; browsers ignore an HSTS header received
-// over plaintext (RFC 6797 section 8.1), so it never lies about the connection,
-// and emitting it here avoids trusting a forwarded-proto header.
-const HSTSValue = "max-age=31536000; includeSubDomains"
-
-// ContentSecurityPolicyValue is the Content-Security-Policy header value.
-// default-src 'self' is the baseline and frame-ancestors 'none' is the primary
-// clickjacking control. 'unsafe-inline' is required in script-src and style-src
-// because the served templates carry inline onclick handlers (generator page)
-// and the bundled Tailwind asset injects a runtime <style> element; dropping it
-// needs template changes outside this issue's scope.
-const ContentSecurityPolicyValue = "default-src 'self'; " +
-	"script-src 'self' 'unsafe-inline'; " +
-	"style-src 'self' 'unsafe-inline'; " +
-	"object-src 'none'; " +
-	"base-uri 'self'; " +
-	"form-action 'self'; " +
-	"frame-ancestors 'none'"
-
-// PermissionsPolicyValue is the Permissions-Policy header value. Every listed
-// feature is denied because pixa uses none of them.
-const PermissionsPolicyValue = "accelerometer=(), autoplay=(), camera=(), " +
-	"display-capture=(), geolocation=(), gyroscope=(), magnetometer=(), " +
-	"microphone=(), payment=(), usb=()"
-
 // Params defines dependencies for Middleware.
 type Params struct {
 	fx.In
@@ -60,7 +33,6 @@ type Params struct {
 type Middleware struct {
 	log    *slog.Logger
 	config *config.Config
-	clientIP *clientip.Resolver
 }
 
 // New creates a new Middleware instance.
@@ -68,24 +40,22 @@ func New(_ fx.Lifecycle, params Params) (*Middleware, error) {
 	s := &Middleware{
 		log:    params.Logger.Get(),
 		config: params.Config,
-		clientIP: clientip.NewResolver(params.Config.TrustedProxies),
 	}
 
 	return s, nil
 }
 
-// ClientIP returns a middleware that resolves the real client IP,
-// honoring X-Forwarded-For only from trusted proxies, and stores it in
-// the request context for the logging middleware and handlers to read.
-func (s *Middleware) ClientIP() func(http.Handler) http.Handler {
-	return func(next http.Handler) http.Handler {
-		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			ip := s.clientIP.Resolve(
-				r.RemoteAddr, r.Header.Values(clientip.ForwardedForHeader))
-			ctx := clientip.WithClientIP(r.Context(), ip)
-			next.ServeHTTP(w, r.WithContext(ctx))
-		})
+func ipFromHostPort(hp string) string {
+	h, _, err := net.SplitHostPort(hp)
+	if err != nil {
+		return ""
 	}
+
+	if len(h) > 0 && h[0] == '[' {
+		return h[1 : len(h)-1]
+	}
+
+	return h
 }
 
 type loggingResponseWriter struct {
@@ -130,7 +100,7 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler {
 					"request_id", reqID,
 					"referer", r.Referer(),
 					"proto", r.Proto,
-					"remoteIP", clientip.FromContext(ctx),
+					"remoteIP", ipFromHostPort(r.RemoteAddr),
 					"status", lrw.statusCode,
 					"response_bytes", lrw.bytesWritten,
 					"latency_ms", latency.Milliseconds(),
@@ -194,16 +164,6 @@ func (s *Middleware) SecurityHeaders() func(http.Handler) http.Handler {
 			// Disable XSS filtering (modern browsers don't need it, can cause issues)
 			w.Header().Set("X-XSS-Protection", "0")
 
-			// Force HTTPS on future visits (ignored by browsers over plaintext)
-			w.Header().Set("Strict-Transport-Security", HSTSValue)
-
-			// Restrict content sources; frame-ancestors is the primary
-			// clickjacking control, X-Frame-Options the legacy fallback
-			w.Header().Set("Content-Security-Policy", ContentSecurityPolicyValue)
-
-			// Deny browser features pixa does not use
-			w.Header().Set("Permissions-Policy", PermissionsPolicyValue)
-
 			next.ServeHTTP(w, r)
 		})
 	}

internal/middleware/middleware_internal_test.go

@@ -56,61 +56,6 @@ func TestSecurityHeaders(t *testing.T) {
 	}
 }
 
-func TestSecurityHeaders_PolicyHeaders(t *testing.T) {
-	t.Parallel()
-
-	cfg := &config.Config{}
-	mw := &Middleware{
-		log:    slog.Default(),
-		config: cfg,
-	}
-
-	testHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
-		w.WriteHeader(http.StatusOK)
-	})
-
-	handler := mw.SecurityHeaders()(testHandler)
-
-	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/test", nil)
-	rec := httptest.NewRecorder()
-
-	handler.ServeHTTP(rec, req)
-
-	tests := []struct {
-		header string
-		want   string
-	}{
-		{"Strict-Transport-Security", "max-age=31536000; includeSubDomains"},
-		{
-			"Content-Security-Policy",
-			"default-src 'self'; " +
-				"script-src 'self' 'unsafe-inline'; " +
-				"style-src 'self' 'unsafe-inline'; " +
-				"object-src 'none'; " +
-				"base-uri 'self'; " +
-				"form-action 'self'; " +
-				"frame-ancestors 'none'",
-		},
-		{
-			"Permissions-Policy",
-			"accelerometer=(), autoplay=(), camera=(), " +
-				"display-capture=(), geolocation=(), gyroscope=(), " +
-				"magnetometer=(), microphone=(), payment=(), usb=()",
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.header, func(t *testing.T) {
-			t.Parallel()
-
-			got := rec.Header().Get(tt.header)
-			if got != tt.want {
-				t.Errorf("%s = %q, want %q", tt.header, got, tt.want)
-			}
-		})
-	}
-}
-
 func TestSecurityHeaders_PreservesExistingHeaders(t *testing.T) {
 	t.Parallel()
 

internal/server/http.go

@@ -10,39 +10,23 @@ import (
 // HTTP server configuration constants.
 const (
 	HTTPReadTimeout    = 30 * time.Second
-	// HTTPReadHeaderTimeout bounds the request-header read on its own,
-	// short, so a slowloris client dribbling headers is dropped well
-	// before it ties up a connection for the whole ReadTimeout window.
-	HTTPReadHeaderTimeout = 10 * time.Second
 	HTTPWriteTimeout   = 60 * time.Second
-	// HTTPIdleTimeout bounds how long an idle keep-alive connection is
-	// held open, so idle connections cannot accumulate without limit on a
-	// service targeting high concurrency.
-	HTTPIdleTimeout    = 120 * time.Second
 	HTTPMaxHeaderBytes = 8 << 10 // 8KB
 )
 
-// newHTTPServer builds the http.Server with the hardening timeouts and
-// limits applied. It is separate from serveUntilShutdown so the
-// configuration can be asserted in a test without binding a listener.
-func (s *Server) newHTTPServer() *http.Server {
-	return &http.Server{
-		Addr:              fmt.Sprintf(":%d", s.config.Port),
+func (s *Server) serveUntilShutdown() {
+	listenAddr := fmt.Sprintf(":%d", s.config.Port)
+	s.httpServer = &http.Server{
+		Addr:           listenAddr,
 		ReadTimeout:    HTTPReadTimeout,
-		ReadHeaderTimeout: HTTPReadHeaderTimeout,
 		WriteTimeout:   HTTPWriteTimeout,
-		IdleTimeout:       HTTPIdleTimeout,
 		MaxHeaderBytes: HTTPMaxHeaderBytes,
 		Handler:        s,
 	}
-}
-
-func (s *Server) serveUntilShutdown() {
-	s.httpServer = s.newHTTPServer()
 
 	s.SetupRoutes()
 
-	s.log.Info("http begin listen", "listenaddr", s.httpServer.Addr)
+	s.log.Info("http begin listen", "listenaddr", listenAddr)
 
 	err := s.httpServer.ListenAndServe()
 	if err != nil && !errors.Is(err, http.ErrServerClosed) {

internal/server/http_internal_test.go

@@ -1,65 +0,0 @@
-package server
-
-import (
-	"testing"
-	"time"
-
-	"sneak.berlin/go/pixa/internal/config"
-)
-
-// TestNewHTTPServerTimeouts verifies that the constructed http.Server
-// carries every hardening timeout wired onto it, including the slowloris
-// defense (ReadHeaderTimeout) and the keep-alive bound (IdleTimeout). This
-// guards against a field being defined but never set on the server, so
-// each assertion compares the server field to its constant.
-func TestNewHTTPServerTimeouts(t *testing.T) {
-	t.Parallel()
-
-	s := &Server{config: &config.Config{Port: 8080}}
-
-	srv := s.newHTTPServer()
-
-	fields := []struct {
-		name string
-		got  time.Duration
-		want time.Duration
-	}{
-		{"ReadTimeout", srv.ReadTimeout, HTTPReadTimeout},
-		{"ReadHeaderTimeout", srv.ReadHeaderTimeout, HTTPReadHeaderTimeout},
-		{"WriteTimeout", srv.WriteTimeout, HTTPWriteTimeout},
-		{"IdleTimeout", srv.IdleTimeout, HTTPIdleTimeout},
-	}
-
-	for _, f := range fields {
-		if f.got != f.want {
-			t.Errorf("%s = %v, want %v", f.name, f.got, f.want)
-		}
-	}
-
-	if srv.MaxHeaderBytes != HTTPMaxHeaderBytes {
-		t.Errorf("MaxHeaderBytes = %d, want %d",
-			srv.MaxHeaderBytes, HTTPMaxHeaderBytes)
-	}
-
-	if srv.Handler != s {
-		t.Error("Handler is not the server")
-	}
-}
-
-// TestHardeningTimeoutValues pins the intent behind the two new timeouts
-// without hard-coding brittle exact durations: the header-read phase is
-// bounded strictly shorter than the whole-request read (the slowloris
-// dribble), and idle keep-alive connections are bounded rather than held
-// open forever.
-func TestHardeningTimeoutValues(t *testing.T) {
-	t.Parallel()
-
-	if HTTPReadHeaderTimeout <= 0 || HTTPReadHeaderTimeout > HTTPReadTimeout {
-		t.Errorf("ReadHeaderTimeout = %v, want positive and <= ReadTimeout %v",
-			HTTPReadHeaderTimeout, HTTPReadTimeout)
-	}
-
-	if HTTPIdleTimeout <= 0 {
-		t.Errorf("IdleTimeout = %v, want positive bound", HTTPIdleTimeout)
-	}
-}

internal/server/routes.go

@@ -8,7 +8,6 @@ import (
 	"github.com/go-chi/chi/v5/middleware"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 
-	"sneak.berlin/go/pixa/internal/handlers"
 	"sneak.berlin/go/pixa/internal/static"
 )
 
@@ -18,7 +17,6 @@ func (s *Server) SetupRoutes() {
 
 	s.router.Use(middleware.Recoverer)
 	s.router.Use(middleware.RequestID)
-	s.router.Use(s.mw.ClientIP())
 	s.router.Use(s.mw.SecurityHeaders())
 	s.router.Use(s.mw.Logging())
 
@@ -48,10 +46,8 @@ func (s *Server) SetupRoutes() {
 
 	// Login/generator UI. The form routes carry CSRF protection; the
 	// token cookie is independent of the session cookie, so it also
-	// covers the login POST, where no session exists yet. LimitBody caps
-	// the POST body ahead of CSRF, which reads its token from that body.
+	// covers the login POST, where no session exists yet.
 	s.router.Group(func(r chi.Router) {
-		r.Use(s.h.LimitBody(handlers.MaxFormBytes))
 		r.Use(s.h.CSRF())
 		r.Get("/", s.h.HandleRoot())
 		r.Post("/", s.h.HandleRoot())