feat(membership): trust reverse-proxy forwarded client IP for rate limit

The per-IP rate limiter keys on the transport RemoteAddr. Behind the
same-origin Caddy proxy that fronts the control plane, every request
arrives with the proxy's single IP, which collapses the limiter into one
bucket shared by the whole world — a flood from one client throttles all
of them.

Add an opt-in `--trusted-proxies` flag (comma-separated IPs/CIDRs). When
the immediate peer is one of the named proxies, clientIP now believes its
X-Forwarded-For (read right-to-left, skipping trusted hops) or X-Real-IP
and keys on the real client. A direct, non-trusted peer's forwarding
headers are ignored entirely, so this opens no quota-fanning hole: an
attacker connecting straight to the public :8470 cannot spoof a key. The
zero value (no flag) preserves the prior RemoteAddr-only behavior exactly.

Covered by ratelimit_proxy_test.go: trusted vs untrusted peers, XFF
right-to-left precedence, client-prepended forgery, X-Real-IP fallback,
and rejection of malformed proxy entries.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

cmd/membershipd/main.go

@@ -59,6 +59,7 @@ func main() {
 		natsStore = flag.String("nats-store", "./local_files/jetstream", "embedded JetStream store dir")
 		busAuth   = flag.String("bus-auth", "off", "control-plane auth rollout: off|soft|enforce (feature flag bus-auth)")
 		corsOrigins = flag.String("cors-origins", "", "comma-separated CORS allowlist of browser origins permitted to call the control plane (e.g. http://localhost:5173,https://chat.example.com); empty = CORS off. Enables the browser-native uniweb client (issue uniweb/0001)")
+		trustedProxies = flag.String("trusted-proxies", "", "comma-separated IPs/CIDRs of reverse proxies whose X-Forwarded-For/X-Real-IP is trusted for the per-IP rate limit; empty = trust the direct connection only. Set to the same-origin proxy's address (e.g. the Caddy node) so the rate limit stays per-client behind the proxy")
 		tlsCert   = flag.String("tls-cert", "", "PATH to the NATS server certificate (deploy/tls/server.crt); enables TLS on the embedded data plane")
 		tlsKey    = flag.String("tls-key", "", "path to the NATS server private key (deploy/tls/server.key); required with --tls-cert")
 		// Cluster (issue 0003a): empty --cluster-name keeps the server standalone.
@@ -357,6 +358,17 @@ func main() {
 		srv.AllowedOrigins = origins
 		log.Printf("CORS: allowing %d browser origin(s): %s", len(origins), strings.Join(origins, ", "))
 	}
+	// Trusted reverse proxies for the per-IP rate limit. Behind the same-origin
+	// Caddy proxy every request arrives with the proxy's IP, which would collapse
+	// the per-IP rate limit into one bucket for the whole world; naming the proxy
+	// here lets the limiter believe its X-Forwarded-For and key on the real client
+	// instead. Empty flag => trust the direct connection only (unchanged behavior).
+	if proxies := splitRoutes(*trustedProxies); len(proxies) > 0 {
+		if err := srv.SetTrustedProxies(proxies); err != nil {
+			log.Fatalf("invalid --trusted-proxies: %v", err)
+		}
+		log.Printf("rate limit: trusting forwarded client IP from proxies: %s", strings.Join(proxies, ", "))
+	}
 
 	// Replicated anti-replay (issue 0006a, audit 0008 N3): a clustered node MUST
 	// share its nonce store across the cluster, or a request accepted on one node

pkg/membership/ratelimit.go

@@ -1,8 +1,10 @@
 package membership
 
 import (
+	"fmt"
 	"net"
 	"net/http"
+	"strings"
 	"sync"
 	"time"
 
@@ -78,16 +80,117 @@ func (l *ipRateLimiter) reapLocked(now time.Time) {
 	}
 }
 
-// clientIP extracts the source IP of an HTTP request, stripping the port. It
-// trusts the transport's RemoteAddr only (no X-Forwarded-For parsing): a public
-// deployment terminates TLS at this process or behind a proxy that the operator
-// controls, and honoring an attacker-supplied header would let a single IP fan
-// its quota across forged identities. If parsing fails the whole RemoteAddr is
-// used as the key (still a stable per-connection bucket).
-func clientIP(r *http.Request) string {
+// clientIP extracts the rate-limit key for a request: the source IP, with the
+// port stripped. By default it trusts the transport's RemoteAddr ONLY (no
+// X-Forwarded-For parsing): honoring an attacker-supplied header would let a
+// single IP fan its quota across forged identities. When the operator runs the
+// control plane behind a reverse proxy they control (the same-origin Caddy
+// deployment), SetTrustedProxies names that proxy's address(es); only then, and
+// only when the immediate peer is one of them, is the forwarded client IP
+// believed. This keeps the per-IP rate limit meaningful behind the proxy, where
+// every request would otherwise share the proxy's single IP. If parsing fails the
+// whole RemoteAddr is used as the key (still a stable per-connection bucket).
+func (s *Server) clientIP(r *http.Request) string {
 	host, _, err := net.SplitHostPort(r.RemoteAddr)
 	if err != nil {
-		return r.RemoteAddr
+		host = r.RemoteAddr
+	}
+	if !s.trustedProxies.has(host) {
+		return host
+	}
+	if fwd := forwardedClientIP(r, s.trustedProxies); fwd != "" {
+		return fwd
 	}
 	return host
 }
+
+// forwardedClientIP returns the real client IP a trusted proxy reported, or "" if
+// none is present. X-Forwarded-For is read RIGHT-TO-LEFT: the rightmost entry is
+// the one our immediate (trusted) proxy appended and therefore cannot be spoofed
+// by the client, which can only prepend entries to the left. Trusted-proxy hops
+// are skipped so a chain of proxies we own resolves to the first address none of
+// them owns — the actual external client. X-Real-IP is a single-value fallback for
+// proxies that set it instead. A non-trusted immediate peer never reaches here, so
+// a direct attacker's forged header is ignored entirely.
+func forwardedClientIP(r *http.Request, trusted trustedProxyMatcher) string {
+	if xff := r.Header.Get("X-Forwarded-For"); xff != "" {
+		parts := strings.Split(xff, ",")
+		for i := len(parts) - 1; i >= 0; i-- {
+			ip := strings.TrimSpace(parts[i])
+			if ip == "" || trusted.has(ip) {
+				continue
+			}
+			if net.ParseIP(ip) != nil {
+				return ip
+			}
+		}
+	}
+	if xrip := strings.TrimSpace(r.Header.Get("X-Real-IP")); xrip != "" {
+		if net.ParseIP(xrip) != nil {
+			return xrip
+		}
+	}
+	return ""
+}
+
+// trustedProxyMatcher is the set of reverse-proxy addresses whose forwarding
+// headers may be honored. The zero value (nil) matches nothing, so the default
+// behavior is RemoteAddr-only.
+type trustedProxyMatcher []*net.IPNet
+
+// SetTrustedProxies configures the proxies whose X-Forwarded-For / X-Real-IP this
+// server trusts for the per-IP rate limit. Each entry is an IP (treated as a /32
+// or /128) or a CIDR. It returns an error on the first unparseable entry and
+// leaves the previous configuration unchanged. Passing no entries clears the set.
+func (s *Server) SetTrustedProxies(entries []string) error {
+	m, err := parseTrustedProxies(entries)
+	if err != nil {
+		return err
+	}
+	s.trustedProxies = m
+	return nil
+}
+
+// parseTrustedProxies turns a list of IPs/CIDRs into a matcher. A bare IP becomes
+// a host route (/32 for IPv4, /128 for IPv6); blanks are skipped.
+func parseTrustedProxies(entries []string) (trustedProxyMatcher, error) {
+	var m trustedProxyMatcher
+	for _, e := range entries {
+		e = strings.TrimSpace(e)
+		if e == "" {
+			continue
+		}
+		if _, ipnet, err := net.ParseCIDR(e); err == nil {
+			m = append(m, ipnet)
+			continue
+		}
+		ip := net.ParseIP(e)
+		if ip == nil {
+			return nil, fmt.Errorf("trusted proxy %q is not an IP or CIDR", e)
+		}
+		bits := 32
+		if ip.To4() == nil {
+			bits = 128
+		}
+		m = append(m, &net.IPNet{IP: ip, Mask: net.CIDRMask(bits, bits)})
+	}
+	return m, nil
+}
+
+// has reports whether host (an IP string with no port) falls inside any trusted
+// range. A nil matcher and an unparseable host both report false.
+func (m trustedProxyMatcher) has(host string) bool {
+	if len(m) == 0 {
+		return false
+	}
+	ip := net.ParseIP(host)
+	if ip == nil {
+		return false
+	}
+	for _, n := range m {
+		if n.Contains(ip) {
+			return true
+		}
+	}
+	return false
+}

pkg/membership/ratelimit_proxy_test.go

@@ -0,0 +1,113 @@
+package membership
+
+import (
+	"net/http"
+	"testing"
+)
+
+// TestClientIPTrustedProxy covers the rate-limit key extraction behind a reverse
+// proxy: forwarding headers are believed ONLY when the immediate peer is a
+// configured trusted proxy, and never otherwise. This is what keeps the per-IP
+// rate limit per-client once the control plane runs behind the same-origin Caddy
+// proxy, without opening a quota-fanning hole for a direct attacker.
+func TestClientIPTrustedProxy(t *testing.T) {
+	const caddy = "135.125.201.30"
+
+	cases := []struct {
+		name    string
+		proxies []string
+		remote  string
+		xff     string
+		xRealIP string
+		want    string
+	}{
+		{
+			name:   "no trusted proxies ignores XFF",
+			remote: "203.0.113.7:5000",
+			xff:    "1.2.3.4",
+			want:   "203.0.113.7",
+		},
+		{
+			name:    "trusted proxy honors XFF client",
+			proxies: []string{caddy},
+			remote:  caddy + ":4451",
+			xff:     "198.51.100.23",
+			want:    "198.51.100.23",
+		},
+		{
+			name:    "loopback proxy honors XFF (magnus-local hop)",
+			proxies: []string{"127.0.0.1/32", "::1/128"},
+			remote:  "127.0.0.1:33344",
+			xff:     "198.51.100.99",
+			want:    "198.51.100.99",
+		},
+		{
+			name:    "untrusted peer cannot spoof XFF",
+			proxies: []string{caddy},
+			remote:  "203.0.113.7:5000",
+			xff:     "10.0.0.1",
+			want:    "203.0.113.7",
+		},
+		{
+			name:    "XFF read right-to-left, trusted hops skipped",
+			proxies: []string{caddy},
+			remote:  caddy + ":4451",
+			xff:     "198.51.100.23, " + caddy,
+			want:    "198.51.100.23",
+		},
+		{
+			name:    "client-prepended forgery is skipped, real appended wins",
+			proxies: []string{caddy},
+			remote:  caddy + ":4451",
+			xff:     "9.9.9.9, 198.51.100.23",
+			want:    "198.51.100.23",
+		},
+		{
+			name:    "X-Real-IP fallback when no XFF",
+			proxies: []string{caddy},
+			remote:  caddy + ":4451",
+			xRealIP: "198.51.100.77",
+			want:    "198.51.100.77",
+		},
+		{
+			name:    "trusted peer but no forwarding header falls back to peer",
+			proxies: []string{caddy},
+			remote:  caddy + ":4451",
+			want:    caddy,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			s := &Server{}
+			if len(tc.proxies) > 0 {
+				if err := s.SetTrustedProxies(tc.proxies); err != nil {
+					t.Fatalf("SetTrustedProxies(%v): %v", tc.proxies, err)
+				}
+			}
+			r, _ := http.NewRequest(http.MethodGet, "/rooms", nil)
+			r.RemoteAddr = tc.remote
+			if tc.xff != "" {
+				r.Header.Set("X-Forwarded-For", tc.xff)
+			}
+			if tc.xRealIP != "" {
+				r.Header.Set("X-Real-IP", tc.xRealIP)
+			}
+			if got := s.clientIP(r); got != tc.want {
+				t.Fatalf("clientIP = %q, want %q", got, tc.want)
+			}
+		})
+	}
+}
+
+// TestParseTrustedProxiesRejectsGarbage proves a malformed entry is a hard error
+// (the command turns it into a startup failure) rather than a silently ignored
+// misconfiguration that would leave the rate limit collapsed behind the proxy.
+func TestParseTrustedProxiesRejectsGarbage(t *testing.T) {
+	if _, err := parseTrustedProxies([]string{"not-an-ip"}); err == nil {
+		t.Fatal("expected error for non-IP/CIDR entry, got nil")
+	}
+	if _, err := parseTrustedProxies([]string{"10.0.0.0/8", "127.0.0.1"}); err != nil {
+		t.Fatalf("valid entries rejected: %v", err)
+	}
+}

pkg/membership/server.go

@@ -97,6 +97,17 @@ type Server struct {
 	// before — so this is opt-in per deployment. Entries are matched exactly (scheme
 	// + host + port); never use "*" with credentials. Set by the command from a flag.
 	AllowedOrigins []string
+
+	// trustedProxies names the reverse proxies whose forwarding headers
+	// (X-Forwarded-For / X-Real-IP) the rate limiter is allowed to believe. It
+	// exists for the same-origin deployment where a single proxy (Caddy) fronts
+	// the control plane: without it every proxied request would share the proxy's
+	// one IP and collapse the per-IP rate limit into a single bucket for the whole
+	// world. Only when the immediate peer is one of these addresses is the
+	// forwarded client IP trusted; the zero value (nil) trusts nobody, preserving
+	// the RemoteAddr-only behavior that predates the flag. Set by the command via
+	// SetTrustedProxies. See clientIP.
+	trustedProxies trustedProxyMatcher
 }
 
 // Posture describes the security posture a membershipd node runs with. It is
@@ -165,7 +176,7 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	// Per-IP rate limit runs first, ahead of auth and body reads, so a flood is
 	// shed at the cheapest possible point. The health probe is exempt so liveness
 	// checks are never throttled.
-	if !isAuthExempt(r) && !s.limiter.allow(clientIP(r), now) {
+	if !isAuthExempt(r) && !s.limiter.allow(s.clientIP(r), now) {
 		writeErr(w, http.StatusTooManyRequests, "rate limit exceeded")
 		return
 	}