fix(api): keep per-client keys distinct under broad trusted_proxies (W2-5)

With trusted_proxies covering client networks (e.g. 10.0.0.0/8 over LAN
clients), the right-to-left XFF walk skipped every entry, exhausted, and
fell back to the proxy's RemoteAddr — collapsing all clients into one
rate-limit/lockout bucket, so one user's failed logins locked out
everyone. On exhaustion the walk now returns the leftmost valid entry
(furthest-upstream hop), the best distinct per-client key such a config
allows. An untrusted RemoteAddr still never gets its headers honoured.

Also (W3-3): the CIDR list parses once per request instead of once per
XFF candidate, config load warns about invalid CIDR entries at startup
(a silently skipped entry silently un-trusts the proxy), and the sample
config documents that trusted_proxies must list only proxy hops.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Server/api/middleware.go

@@ -205,8 +205,12 @@ func clientIPWithProxies(r *http.Request, trustedCIDRs []string) string {
 		return remoteHost
 	}
 
-	trusted, _ := isTrustedProxy(remoteHost, trustedCIDRs)
-	if !trusted {
+	// Parse the CIDR list once per request instead of once per XFF candidate.
+	// ponytail: parse at middleware construction if this ever shows in a
+	// profile — it would mean threading a parsed type through every caller.
+	nets := parseCIDRList(trustedCIDRs)
+
+	if !ipInNets(remoteHost, nets) {
 		return remoteHost
 	}
 
@@ -226,21 +230,62 @@ func clientIPWithProxies(r *http.Request, trustedCIDRs []string) string {
 	// letting it forge per-IP rate-limit and lockout keys.
 	if xff := r.Header.Get("X-Forwarded-For"); xff != "" {
 		parts := strings.Split(xff, ",")
+		leftmostValid := ""
 		for i := len(parts) - 1; i >= 0; i-- {
 			candidate := strings.TrimSpace(parts[i])
 			if candidate == "" || net.ParseIP(candidate) == nil {
 				continue
 			}
-			if trusted, _ := isTrustedProxy(candidate, trustedCIDRs); trusted {
+			leftmostValid = candidate
+			if ipInNets(candidate, nets) {
 				continue // our own proxy hop, keep walking left
 			}
 			return candidate
 		}
+		// Every entry fell inside trustedCIDRs — a config that covers client
+		// networks too (e.g. trusted_proxies: 10.0.0.0/8 with LAN clients).
+		// Falling back to RemoteAddr here would collapse ALL clients behind
+		// the proxy into one rate-limit/lockout bucket, so one user's failed
+		// logins would lock out everyone. The leftmost valid entry is the
+		// furthest-upstream hop — the best distinct per-client key available
+		// under such a config. trusted_proxies must list only proxy hops;
+		// startup validation warns about entries that cannot be proxies.
+		if leftmostValid != "" {
+			return leftmostValid
+		}
 	}
 
 	return remoteHost
 }
 
+// parseCIDRList parses CIDR strings, silently skipping invalid entries — a
+// misconfigured entry must not crash request handling (config load warns
+// about them at startup).
+func parseCIDRList(cidrs []string) []*net.IPNet {
+	nets := make([]*net.IPNet, 0, len(cidrs))
+	for _, c := range cidrs {
+		if _, n, err := net.ParseCIDR(c); err == nil {
+			nets = append(nets, n)
+		}
+	}
+	return nets
+}
+
+// ipInNets reports whether ipStr (a plain IP, no port) falls inside any of
+// the parsed networks.
+func ipInNets(ipStr string, nets []*net.IPNet) bool {
+	ip := net.ParseIP(ipStr)
+	if ip == nil {
+		return false
+	}
+	for _, n := range nets {
+		if n.Contains(ip) {
+			return true
+		}
+	}
+	return false
+}
+
 // isTrustedProxy reports whether remoteIP (a plain IP string, no port) falls
 // within any of the provided CIDR ranges. It returns an error if any CIDR is
 // malformed.

Server/config/config.go

@@ -6,6 +6,7 @@ import (
 	"encoding/hex"
 	"fmt"
 	"log/slog"
+	"net"
 	"os"
 	"strings"
 
@@ -215,7 +216,10 @@ server:
   name: "OwnCord Server"
   data_dir: "data"
   # allowed_origins: []       # empty = deny cross-origin; set to ["*"] for dev or specific origins for prod
-  # trusted_proxies: []       # CIDRs of trusted reverse proxies, e.g. ["10.0.0.0/8"]
+  # trusted_proxies: []       # CIDRs of the reverse-proxy HOPS only (e.g. ["10.0.0.2/32"]).
+  #                           # Never list client networks here: a range that covers
+  #                           # clients degrades per-client rate limiting and lets
+  #                           # covered clients influence their own rate-limit key.
   # admin_allowed_cidrs:      # CIDRs allowed to access /admin (default: private networks only)
   #   - "127.0.0.0/8"
   #   - "::1/128"
@@ -349,9 +353,26 @@ func Load(cfgPath string) (*Config, error) {
 		cfg.Voice.LiveKitAPISecret = ""
 	}
 
+	// Invalid CIDR entries are skipped at request time (they must not crash
+	// handling), which silently un-trusts a misconfigured proxy — warn once
+	// at startup instead. Common mistake: a bare IP without the /32 mask.
+	warnInvalidCIDRs("server.trusted_proxies", cfg.Server.TrustedProxies)
+	warnInvalidCIDRs("server.admin_allowed_cidrs", cfg.Server.AdminAllowedCIDRs)
+
 	return &cfg, nil
 }
 
+// warnInvalidCIDRs logs a startup warning for each list entry that is not
+// valid CIDR notation.
+func warnInvalidCIDRs(key string, cidrs []string) {
+	for _, c := range cidrs {
+		if _, _, err := net.ParseCIDR(c); err != nil {
+			slog.Warn("config: ignoring invalid CIDR entry (use address/prefix notation, e.g. 10.0.0.1/32)",
+				"key", key, "entry", c)
+		}
+	}
+}
+
 // defaultLiveKitAPIKey and defaultLiveKitAPISecret are the well-known dev
 // credentials that ship in the default config. They must never be used in
 // production — NewLiveKitClient rejects them.