Merge pull request #30 from techquestsdev/andre.nogueira/fix-ci-realip-deprecation

fix(ci): replace deprecated chi RealIP + rename gomodguard linter

Author: aanogueira

.golangci.yml

@@ -14,9 +14,12 @@ output:
 
 linters:
   default: all
+  enable:
+    - gomodguard_v2 # replaces deprecated gomodguard
   disable:
     # Deprecated
     - wsl
+    - gomodguard # superseded by gomodguard_v2
 
     # Too noisy / impractical
     - exhaustruct

cmd/api/server/server.go

@@ -3,6 +3,7 @@ package server
 import (
 	"context"
 	_ "embed"
+	"net"
 	"net/http"
 	"time"
 
@@ -97,7 +98,7 @@ func newRouter(
 
 	// Middleware
 	r.Use(middleware.RequestID)
-	r.Use(middleware.RealIP)
+	r.Use(authmw.RealIP(mustParseTrustedProxies(cfg.Server.TrustedProxies)))
 	r.Use(middleware.Logger)
 	r.Use(middleware.Recoverer)
 	r.Use(middleware.Timeout(60 * time.Second))
@@ -286,6 +287,18 @@ func isNoOpAuthenticator(a authmw.Authenticator) bool {
 	return ok
 }
 
+// mustParseTrustedProxies parses the configured CIDRs and panics on invalid
+// input. Misconfiguration here is a deployment error that should fail fast at
+// startup rather than silently disabling client-IP resolution.
+func mustParseTrustedProxies(in []string) []*net.IPNet {
+	nets, err := authmw.ParseTrustedProxies(in)
+	if err != nil {
+		panic("invalid server.trusted_proxies: " + err.Error())
+	}
+
+	return nets
+}
+
 func corsMiddleware(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// When an Origin header is present, echo it back and allow credentials

config.example.yaml

@@ -32,6 +32,12 @@ server:
   addr: ":8080"
   read_timeout: 15s
   write_timeout: 60s
+  # CIDRs (or bare IPs) whose X-Forwarded-For headers will be honored to
+  # set r.RemoteAddr. Leave empty to keep the actual TCP peer address.
+  # Set this to your ingress / load balancer CIDR when running behind one.
+  # trusted_proxies:
+  #   - 10.0.0.0/8
+  #   - 172.16.0.0/12
 
 # Database configuration
 # Env: CS_DATABASE_DRIVER, CS_DATABASE_URL, CS_DATABASE_MAX_OPEN_CONNS, CS_DATABASE_MAX_IDLE_CONNS, CS_DATABASE_CONN_MAX_LIFETIME

internal/config/config.go

@@ -62,6 +62,11 @@ type ServerConfig struct {
 	Addr         string        `mapstructure:"addr"`
 	ReadTimeout  time.Duration `mapstructure:"read_timeout"`
 	WriteTimeout time.Duration `mapstructure:"write_timeout"`
+	// TrustedProxies is the list of CIDRs (or bare IPs) whose X-Forwarded-For
+	// headers will be honored to populate r.RemoteAddr. Leave empty to keep
+	// the actual TCP peer address — the safe default for direct exposure.
+	// Set this to the ingress/load-balancer CIDR when running behind one.
+	TrustedProxies []string `mapstructure:"trusted_proxies"`
 }
 
 type DatabaseConfig struct {

internal/indexer/server.go

@@ -65,7 +65,6 @@ func (s *Server) Start() error {
 
 	// Middleware
 	r.Use(middleware.RequestID)
-	r.Use(middleware.RealIP)
 	r.Use(middleware.Recoverer)
 	r.Use(middleware.Timeout(60 * time.Second))
 

internal/middleware/realip.go

@@ -0,0 +1,111 @@
+package middleware
+
+import (
+	"fmt"
+	"net"
+	"net/http"
+	"slices"
+	"strings"
+)
+
+// RealIP returns chi-compatible middleware that rewrites r.RemoteAddr to the
+// real client IP by walking the X-Forwarded-For chain — but ONLY when the
+// immediate TCP peer is in trustedProxies. This closes the spoofing vector
+// that caused chi's own middleware.RealIP to be deprecated
+// (GHSA-3fxj-6jh8-hvhx, GHSA-rjr7-jggh-pgcp, GHSA-9g5q-2w5x-hmxf).
+//
+// If trustedProxies is empty, the middleware is a no-op — r.RemoteAddr keeps
+// the actual TCP peer address. That is the safe default: better to log the
+// proxy IP than to trust a client-supplied header from an unknown network.
+func RealIP(trustedProxies []*net.IPNet) func(http.Handler) http.Handler {
+	if len(trustedProxies) == 0 {
+		return func(next http.Handler) http.Handler { return next }
+	}
+
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if client := realClientIP(r, trustedProxies); client != "" {
+				r.RemoteAddr = client
+			}
+
+			next.ServeHTTP(w, r)
+		})
+	}
+}
+
+func realClientIP(r *http.Request, trusted []*net.IPNet) string {
+	peerHost, _, err := net.SplitHostPort(r.RemoteAddr)
+	if err != nil {
+		peerHost = r.RemoteAddr
+	}
+
+	peerIP := net.ParseIP(peerHost)
+	if peerIP == nil || !ipInAny(peerIP, trusted) {
+		return ""
+	}
+
+	xff := r.Header.Get("X-Forwarded-For")
+	if xff == "" {
+		return ""
+	}
+
+	parts := strings.Split(xff, ",")
+	for _, raw := range slices.Backward(parts) {
+		candidate := strings.TrimSpace(raw)
+
+		ip := net.ParseIP(candidate)
+		if ip == nil {
+			continue
+		}
+
+		if !ipInAny(ip, trusted) {
+			return candidate
+		}
+	}
+
+	return ""
+}
+
+func ipInAny(ip net.IP, nets []*net.IPNet) bool {
+	for _, n := range nets {
+		if n.Contains(ip) {
+			return true
+		}
+	}
+
+	return false
+}
+
+// ParseTrustedProxies parses CIDR notation strings into []*net.IPNet.
+// Bare IPv4/IPv6 addresses are accepted and converted to single-host CIDRs.
+func ParseTrustedProxies(in []string) ([]*net.IPNet, error) {
+	out := make([]*net.IPNet, 0, len(in))
+
+	for _, s := range in {
+		s = strings.TrimSpace(s)
+		if s == "" {
+			continue
+		}
+
+		if _, n, err := net.ParseCIDR(s); err == nil {
+			out = append(out, n)
+			continue
+		}
+
+		if ip := net.ParseIP(s); ip != nil {
+			suffix := "/32"
+			if ip.To4() == nil {
+				suffix = "/128"
+			}
+
+			if _, n, err := net.ParseCIDR(s + suffix); err == nil {
+				out = append(out, n)
+				continue
+			}
+		}
+
+		return nil, fmt.Errorf("invalid trusted_proxy value %q: expected CIDR or IP", s)
+	}
+
+	return out, nil
+}

internal/middleware/realip_test.go

@@ -0,0 +1,130 @@
+package middleware
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestRealIP_NoTrustedProxies_IsNoOp(t *testing.T) {
+	captured := ""
+	h := RealIP(nil)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+		captured = r.RemoteAddr
+	}))
+
+	r := httptest.NewRequest(http.MethodGet, "/", nil)
+	r.RemoteAddr = "1.2.3.4:5555"
+	r.Header.Set("X-Forwarded-For", "9.9.9.9")
+	h.ServeHTTP(httptest.NewRecorder(), r)
+
+	if captured != "1.2.3.4:5555" {
+		t.Fatalf("expected RemoteAddr untouched, got %q", captured)
+	}
+}
+
+func TestRealIP_TrustedPeer_UsesXFF(t *testing.T) {
+	trusted, err := ParseTrustedProxies([]string{"10.0.0.0/8"})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	captured := ""
+	h := RealIP(trusted)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+		captured = r.RemoteAddr
+	}))
+
+	r := httptest.NewRequest(http.MethodGet, "/", nil)
+	r.RemoteAddr = "10.0.5.6:5555"
+	r.Header.Set("X-Forwarded-For", "203.0.113.7")
+	h.ServeHTTP(httptest.NewRecorder(), r)
+
+	if captured != "203.0.113.7" {
+		t.Fatalf("expected client IP from XFF, got %q", captured)
+	}
+}
+
+func TestRealIP_UntrustedPeer_IgnoresXFF(t *testing.T) {
+	trusted, err := ParseTrustedProxies([]string{"10.0.0.0/8"})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	captured := ""
+	h := RealIP(trusted)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+		captured = r.RemoteAddr
+	}))
+
+	r := httptest.NewRequest(http.MethodGet, "/", nil)
+	r.RemoteAddr = "8.8.8.8:5555"
+	r.Header.Set("X-Forwarded-For", "203.0.113.7")
+	h.ServeHTTP(httptest.NewRecorder(), r)
+
+	if captured != "8.8.8.8:5555" {
+		t.Fatalf("expected RemoteAddr untouched for untrusted peer, got %q", captured)
+	}
+}
+
+func TestRealIP_ChainOfTrustedProxies(t *testing.T) {
+	trusted, err := ParseTrustedProxies([]string{"10.0.0.0/8", "172.16.0.0/12"})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	captured := ""
+	h := RealIP(trusted)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+		captured = r.RemoteAddr
+	}))
+
+	r := httptest.NewRequest(http.MethodGet, "/", nil)
+	r.RemoteAddr = "10.0.5.6:5555"
+	// Chain: client → ext LB (untrusted) → internal LB (trusted) → peer (trusted)
+	r.Header.Set("X-Forwarded-For", "203.0.113.7, 172.16.0.1, 10.0.5.1")
+	h.ServeHTTP(httptest.NewRecorder(), r)
+
+	if captured != "203.0.113.7" {
+		t.Fatalf("expected first untrusted IP from right, got %q", captured)
+	}
+}
+
+func TestRealIP_AllForwardersTrusted_NoMatch(t *testing.T) {
+	trusted, err := ParseTrustedProxies([]string{"10.0.0.0/8"})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	captured := ""
+	h := RealIP(trusted)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+		captured = r.RemoteAddr
+	}))
+
+	r := httptest.NewRequest(http.MethodGet, "/", nil)
+	r.RemoteAddr = "10.0.5.6:5555"
+	r.Header.Set("X-Forwarded-For", "10.0.5.1, 10.0.5.2")
+	h.ServeHTTP(httptest.NewRecorder(), r)
+
+	if captured != "10.0.5.6:5555" {
+		t.Fatalf("expected RemoteAddr untouched when no untrusted hop, got %q", captured)
+	}
+}
+
+func TestParseTrustedProxies(t *testing.T) {
+	cases := []struct {
+		in     []string
+		wantOK bool
+	}{
+		{[]string{"10.0.0.0/8"}, true},
+		{[]string{"::1/128"}, true},
+		{[]string{"192.168.1.1"}, true}, // bare IP → /32
+		{[]string{"2001:db8::1"}, true}, // bare IPv6 → /128
+		{[]string{""}, true},            // empty entries skipped
+		{[]string{"not-an-ip"}, false},
+		{[]string{"10.0.0.0/99"}, false},
+	}
+
+	for _, c := range cases {
+		_, err := ParseTrustedProxies(c.in)
+		if (err == nil) != c.wantOK {
+			t.Errorf("ParseTrustedProxies(%v): err=%v, wantOK=%v", c.in, err, c.wantOK)
+		}
+	}
+}