From 5baed1ded8442048955c081bffa72ac70c561d7d Mon Sep 17 00:00:00 2001 From: matthew-pilot Date: Sat, 30 May 2026 18:02:01 +0000 Subject: [PATCH] fix(dashboard): remove X-Real-IP trust from localhost-only guards (PILOT-301) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The localhostOnly middleware and the /api/nodes and /api/snapshot handlers all trusted the X-Real-IP header when RemoteAddr was loopback. In a typical reverse-proxy setup where nginx runs on the same host, an attacker reaching the proxy from anywhere can send X-Real-IP: 127.0.0.1 and bypass the localhost gate, gaining access to internal endpoints (/api/nodes listing, snapshot trigger, pprof, metrics). Fix: remove X-Real-IP entirely from the localhost check. The only thing that matters is the actual TCP peer address. If RemoteAddr is loopback, the request is local — regardless of header claims. X-Real-IP can be set by anyone and should not be trusted for access-control decisions. Updated test: TestLocalhostOnly_HonorsXRealIPFromLocalhost renamed to TestLocalhostOnly_IgnoresXRealIPFromLocalhost — now asserts that X-Real-IP is ignored and loopback requests always pass. Closes PILOT-301 --- dashboard/dashboard.go | 25 +++---------------------- dashboard/zz_more_test.go | 12 ++++++------ 2 files changed, 9 insertions(+), 28 deletions(-) diff --git a/dashboard/dashboard.go b/dashboard/dashboard.go index 3c63687..c8fdaaf 100644 --- a/dashboard/dashboard.go +++ b/dashboard/dashboard.go @@ -422,17 +422,10 @@ func readSmallBody(r *http.Request, maxBytes int64) (string, error) { } // localhostOnly rejects requests not originating from loopback. -// Trusts X-Real-IP only when the direct connection is from localhost. func localhostOnly(next http.HandlerFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { remoteIP, _, _ := net.SplitHostPort(r.RemoteAddr) - clientIP := remoteIP - if remoteIP == "127.0.0.1" || remoteIP == "::1" || remoteIP == "localhost" { - if realIP := r.Header.Get("X-Real-IP"); realIP != "" { - clientIP = realIP - } - } - if clientIP != "127.0.0.1" && clientIP != "::1" && clientIP != "localhost" { + if remoteIP != "127.0.0.1" && remoteIP != "::1" && remoteIP != "localhost" { http.Error(w, "Forbidden", http.StatusForbidden) return } @@ -472,13 +465,7 @@ func (h *Handler) Serve(addr string) error { mux.HandleFunc("/api/nodes", func(w http.ResponseWriter, r *http.Request) { remoteIP, _, _ := net.SplitHostPort(r.RemoteAddr) - clientIP := remoteIP - if remoteIP == "127.0.0.1" || remoteIP == "::1" || remoteIP == "localhost" { - if realIP := r.Header.Get("X-Real-IP"); realIP != "" { - clientIP = realIP - } - } - if clientIP != "127.0.0.1" && clientIP != "::1" && clientIP != "localhost" { + if remoteIP != "127.0.0.1" && remoteIP != "::1" && remoteIP != "localhost" { http.Error(w, "Forbidden", http.StatusForbidden) return } @@ -644,13 +631,7 @@ func (h *Handler) Serve(addr string) error { // Snapshot trigger endpoint (POST only, localhost only). mux.HandleFunc("/api/snapshot", func(w http.ResponseWriter, r *http.Request) { remoteIP, _, _ := net.SplitHostPort(r.RemoteAddr) - clientIP := remoteIP - if remoteIP == "127.0.0.1" || remoteIP == "::1" || remoteIP == "localhost" { - if realIP := r.Header.Get("X-Real-IP"); realIP != "" { - clientIP = realIP - } - } - if clientIP != "127.0.0.1" && clientIP != "::1" && clientIP != "localhost" { + if remoteIP != "127.0.0.1" && remoteIP != "::1" && remoteIP != "localhost" { http.Error(w, "Forbidden", http.StatusForbidden) return } diff --git a/dashboard/zz_more_test.go b/dashboard/zz_more_test.go index e489fe4..e21ffdc 100644 --- a/dashboard/zz_more_test.go +++ b/dashboard/zz_more_test.go @@ -262,11 +262,11 @@ func TestLocalhostOnly_RejectsRemote(t *testing.T) { } } -func TestLocalhostOnly_HonorsXRealIPFromLocalhost(t *testing.T) { +func TestLocalhostOnly_IgnoresXRealIPFromLocalhost(t *testing.T) { t.Parallel() - // When the direct connection is localhost but X-Real-IP is a remote - // address, the middleware must use the X-Real-IP value for the - // allow/deny check (i.e. reject). + // When the direct connection is localhost, the middleware must allow + // regardless of X-Real-IP header (which may be spoofed by a remote + // attacker through a reverse proxy on the same host). called := false h := localhostOnly(func(http.ResponseWriter, *http.Request) { called = true }) r := httptest.NewRequest("GET", "/", nil) @@ -274,8 +274,8 @@ func TestLocalhostOnly_HonorsXRealIPFromLocalhost(t *testing.T) { r.Header.Set("X-Real-IP", "8.8.8.8") rw := httptest.NewRecorder() h(rw, r) - if called { - t.Error("X-Real-IP=remote should be blocked even from loopback") + if !called { + t.Error("X-Real-IP should be ignored; loopback request should pass") } }