Merge pull request '#280: remove PALETTE_TRUSTED_IP_HEADER, rate limits key on peer address only' (#291) from fix-280-r2 into dev

README.md

@@ -70,7 +70,6 @@ go build -o palette ./cmd/palette
 | `PALETTE_ADMIN_KEY` | generated | Admin key; if unset a 32-char hex key is generated and persisted to `<db-dir>/admin-key` (0600) |
 | `PALETTE_DEFAULT_DARK` | dark on | Default dark mode for new visitors. Set `false`, `0`, or `off` to default to light mode. Visitors who toggle dark mode keep their choice in their browser. |
 | `PALETTE_UNLOCK_SECRET` | random per start | HMAC secret for password-unlock cookies. Set a fixed value to keep unlock sessions across restarts or across replicas. |
-| `PALETTE_TRUSTED_IP_HEADER` | unset | Name of a proxy-controlled client-IP header to key API rate limits on (e.g. `CF-Connecting-IP` when Cloudflare is the ingress; Cloudflare strips any client-supplied value). Unset: rate limits key on the peer address only, and all client-supplied IP headers (X-Forwarded-For, X-Real-Ip) are ignored. (#280) |
 
 An `/admin` page exists for runtime settings, protected by a key set at
 install (`PALETTE_ADMIN_KEY` env var) and resettable locally. See

internal/api/clientip.go

@@ -7,48 +7,18 @@
 // per request and the limit was unenforceable (pentest H1: 6x201 across 8
 // rotating-XFF creates).
 //
-// Default: key on the actual peer address (RemoteAddr) only. Behind any
+// The bucket key is the actual peer address (RemoteAddr) only. Behind any
 // reverse proxy this is the proxy's address, so all clients share one bucket
-// per endpoint — coarse, but safe.
-//
-// Proxy-honoring mode: a deployment in front of a proxy that OVERWRITES (not
-// appends to) a client-IP header can set PALETTE_TRUSTED_IP_HEADER (e.g.
-// CF-Connecting-IP when Cloudflare is the ingress; Cloudflare strips any
-// client-supplied value). The header is honored ONLY when explicitly
-// configured at startup, and X-Forwarded-For / X-Real-Ip are never trusted.
+// per endpoint — coarse, but safe. Client-supplied IP headers
+// (X-Forwarded-For, X-Real-Ip) are never trusted.
 package api
 
 import (
 	"net"
 	"net/http"
-	"sync"
 )
 
-var (
-	trustedIPMu     sync.RWMutex
-	trustedIPHeader string // empty = never trust any client-IP header
-)
-
-// SetTrustedIPHeader configures the single proxy-controlled header whose
-// value may key rate-limit buckets. Called at startup; tests may reset it.
-func SetTrustedIPHeader(name string) {
-	trustedIPMu.Lock()
-	defer trustedIPMu.Unlock()
-	trustedIPHeader = name
-}
-
-func getTrustedIPHeader() string {
-	trustedIPMu.RLock()
-	defer trustedIPMu.RUnlock()
-	return trustedIPHeader
-}
-
 func clientIP(r *http.Request) string {
-	if name := getTrustedIPHeader(); name != "" {
-		if v := r.Header.Get(name); v != "" {
-			return v
-		}
-	}
 	host := r.RemoteAddr
 	if h, _, err := net.SplitHostPort(r.RemoteAddr); err == nil {
 		host = h

internal/api/ratelimit_xff_test.go

@@ -7,8 +7,6 @@ import (
 )
 
 func TestClientIPUsesRemoteAddrNotXFF(t *testing.T) {
-	SetTrustedIPHeader("")
-	defer SetTrustedIPHeader("")
 	r := httptest.NewRequest("POST", "/api/pastes", nil)
 	r.RemoteAddr = "203.0.113.7:4432"
 	r.Header.Set("X-Forwarded-For", "1.2.3.4, 1.2.3.5, 203.0.113.9")
@@ -18,18 +16,15 @@ func TestClientIPUsesRemoteAddrNotXFF(t *testing.T) {
 	}
 }
 
-func TestClientIPTrustedHeaderOnlyWhenConfigured(t *testing.T) {
-	SetTrustedIPHeader("")
-	defer SetTrustedIPHeader("")
+// A proxy-controlled header is not honored even when set: #280 revision
+// removed the PALETTE_TRUSTED_IP_HEADER mechanism per owner decision, so the
+// bucket key is the peer address only.
+func TestClientIPNeverTrustsHeaders(t *testing.T) {
 	r := httptest.NewRequest("POST", "/api/pastes", nil)
 	r.RemoteAddr = "10.0.1.47:9999"
 	r.Header.Set("CF-Connecting-IP", "198.51.100.9")
 	if got := clientIP(r); got != "10.0.1.47" {
-		t.Fatalf("unconfigured: clientIP = %q, want peer 10.0.1.47", got)
-	}
-	SetTrustedIPHeader("CF-Connecting-IP")
-	if got := clientIP(r); got != "198.51.100.9" {
-		t.Fatalf("configured: clientIP = %q, want CF-Connecting-IP value", got)
+		t.Fatalf("clientIP = %q, want peer 10.0.1.47", got)
 	}
 }
 
@@ -37,9 +32,7 @@ func TestClientIPTrustedHeaderOnlyWhenConfigured(t *testing.T) {
 // repro was 8 creates with rotating XFF -> 6x201.
 func TestRotatingXFFDoesNotResetBucket(t *testing.T) {
 	globalLimiter = newLimiter()
-	defer SetTrustedIPHeader("")
-	SetTrustedIPHeader("")
-	s := defaultSettings(Config{}) // burst/limit defaults; any header values are ignored anyway
+	s := defaultSettings(Config{}) // burst/limit defaults; header values are ignored anyway
 	var allowed, limited int
 	for i := 0; i < 8; i++ {
 		r := httptest.NewRequest("POST", "/api/pastes", nil)

internal/api/server.go

@@ -28,10 +28,6 @@ type Config struct {
 	DBPath       string
 	MaxTextBytes int64
 	MaxItemBytes int64
-	// TrustedIPHeader optionally names a proxy-controlled client-IP header
-	// (e.g. CF-Connecting-IP behind Cloudflare) to key rate limits on. Empty
-	// (default) keys on the peer address only. See clientip.go (#280).
-	TrustedIPHeader string
 }
 
 type apiServer struct {
@@ -43,7 +39,6 @@ type apiServer struct {
 }
 
 func NewServer(st *store.Store, cfg Config, ui *web.UI, ss *settingsStore, adminKey string) *apiServer {
-	SetTrustedIPHeader(cfg.TrustedIPHeader) // #280
 	return &apiServer{store: st, cfg: cfg, ui: ui, settings: ss, adminKey: adminKey}
 }