From 99668ec0d857e31688f0c6cd81a5e5dbafb4139e Mon Sep 17 00:00:00 2001 From: Masahiko AMANO Date: Mon, 15 Jun 2026 14:51:44 +0300 Subject: [PATCH] feat(backend): trust reverse-proxy X-Forwarded-For for the client IP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The auth rate limiter keys on c.ClientIP(), but the router was built with gin.New() and never called SetTrustedProxies — so Gin trusted all proxies by default. Behind a host reverse proxy that meant the limiter either bucketed every request under the proxy's IP, or (with the port reachable directly) could be bypassed by a forged X-Forwarded-For. NewRouter now takes a trusted-proxy list and configures SetTrustedProxies, returning an error on an invalid list so misconfiguration fails fast at startup. The list comes from a new TRUSTED_PROXIES config (CSV of CIDRs/IPs), defaulting to loopback plus the Docker bridge ranges a host proxy reaches the container through. Co-Authored-By: Claude Opus 4.8 --- backend/cmd/server/main.go | 7 ++++++- backend/internal/config/config.go | 22 +++++++++++++++++++++ backend/internal/handler/router.go | 15 ++++++++++++-- backend/internal/integration/server_test.go | 4 +++- 4 files changed, 44 insertions(+), 4 deletions(-) diff --git a/backend/cmd/server/main.go b/backend/cmd/server/main.go index 1c0068d..4c26121 100644 --- a/backend/cmd/server/main.go +++ b/backend/cmd/server/main.go @@ -114,12 +114,17 @@ func main() { aclHandler := handler.NewACLHandler(aclSvc) auditHandler := handler.NewAuditHandler(auditSvc) - r := handler.NewRouter( + r, err := handler.NewRouter( authMiddleware, authHandler, fileHandler, tagHandler, categoryHandler, poolHandler, userHandler, aclHandler, auditHandler, cfg.StaticDir, + cfg.TrustedProxies, ) + if err != nil { + slog.Error("building router", "err", err) + os.Exit(1) + } // ReadHeaderTimeout bounds slow-header (Slowloris) attacks; body read/write // are left unbounded so large file uploads and downloads can stream. diff --git a/backend/internal/config/config.go b/backend/internal/config/config.go index 5055063..0a7508a 100644 --- a/backend/internal/config/config.go +++ b/backend/internal/config/config.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "strconv" + "strings" "time" "github.com/joho/godotenv" @@ -17,6 +18,13 @@ type Config struct { JWTSecret string JWTAccessTTL time.Duration JWTRefreshTTL time.Duration + // TrustedProxies lists the reverse-proxy hops (CIDRs or IPs) whose + // X-Forwarded-For header is trusted. The auth rate limiter keys on the + // client IP, so this must match the proxy in front of the app — otherwise + // every request appears to come from the proxy (one shared bucket) or a + // direct caller could forge the header. Default covers loopback and the + // Docker bridge ranges a host reverse proxy reaches the container through. + TrustedProxies []string // Initial admin bootstrap (applied on startup if the user does not exist) AdminUsername string @@ -100,6 +108,18 @@ func Load() (*Config, error) { return n } + parseCSV := func(key, def string) []string { + raw := defaultStr(key, def) + parts := strings.Split(raw, ",") + out := make([]string, 0, len(parts)) + for _, p := range parts { + if p = strings.TrimSpace(p); p != "" { + out = append(out, p) + } + } + return out + } + parseInt64 := func(key string, def int64) int64 { raw := os.Getenv(key) if raw == "" { @@ -119,6 +139,8 @@ func Load() (*Config, error) { JWTAccessTTL: parseDuration("JWT_ACCESS_TTL", "15m"), JWTRefreshTTL: parseDuration("JWT_REFRESH_TTL", "720h"), + TrustedProxies: parseCSV("TRUSTED_PROXIES", "127.0.0.1/32,::1/128,172.16.0.0/12"), + AdminUsername: defaultStr("ADMIN_USERNAME", "admin"), AdminPassword: requireStr("ADMIN_PASSWORD"), diff --git a/backend/internal/handler/router.go b/backend/internal/handler/router.go index 2679df6..9d977c1 100644 --- a/backend/internal/handler/router.go +++ b/backend/internal/handler/router.go @@ -1,6 +1,7 @@ package handler import ( + "fmt" "net/http" "time" @@ -32,10 +33,20 @@ func NewRouter( aclHandler *ACLHandler, auditHandler *AuditHandler, staticDir string, -) *gin.Engine { + trustedProxies []string, +) (*gin.Engine, error) { r := gin.New() r.Use(gin.Logger(), gin.Recovery(), securityHeaders()) + // Behind a reverse proxy the client's real IP arrives in X-Forwarded-For. + // Trust only the proxy hop(s) so c.ClientIP() — used by the auth rate + // limiter — reflects the real client and can't be spoofed by a forged + // header from a direct caller. An empty list trusts no proxy (ClientIP is + // the immediate peer). + if err := r.SetTrustedProxies(trustedProxies); err != nil { + return nil, fmt.Errorf("configure trusted proxies: %w", err) + } + // Health check — no auth required. r.GET("/health", func(c *gin.Context) { c.JSON(http.StatusOK, gin.H{"status": "ok"}) @@ -189,5 +200,5 @@ func NewRouter( r.NoRoute(spaHandler(staticDir)) } - return r + return r, nil } diff --git a/backend/internal/integration/server_test.go b/backend/internal/integration/server_test.go index dfd7dea..a08fd45 100644 --- a/backend/internal/integration/server_test.go +++ b/backend/internal/integration/server_test.go @@ -151,12 +151,14 @@ func setupSuite(t *testing.T) *harness { aclHandler := handler.NewACLHandler(aclSvc) auditHandler := handler.NewAuditHandler(auditSvc) - r := handler.NewRouter( + r, err := handler.NewRouter( authMiddleware, authHandler, fileHandler, tagHandler, categoryHandler, poolHandler, userHandler, aclHandler, auditHandler, "", + nil, ) + require.NoError(t, err) srv := httptest.NewServer(r) t.Cleanup(srv.Close)