From 223537e9863ca88232606d576f1ee9dfe7ccab67 Mon Sep 17 00:00:00 2001 From: matthew-pilot Date: Tue, 30 Jun 2026 02:30:10 +0000 Subject: [PATCH 1/2] test(csrf): add regression test for query-param admin_token on mutation endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestAdminToken_CSRF_MutationRejectsQueryParam verifies that POST /api/admin/runtime/gc with ?admin_token=... returns 401, while X-Admin-Token header continues to work. This is the repro case for PILOT-238 — the requireAdminToken middleware accepts query-param tokens on all HTTP methods, creating a CSRF vector for any password-leak (URL in logs, referer, etc). The test is written to fail before the fix and pass after. --- dashboard/zz_stats_auth_test.go | 37 +++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/dashboard/zz_stats_auth_test.go b/dashboard/zz_stats_auth_test.go index 4187ef7..53641ee 100644 --- a/dashboard/zz_stats_auth_test.go +++ b/dashboard/zz_stats_auth_test.go @@ -3,6 +3,7 @@ package dashboard import ( + "bytes" "net/http" "net/http/httptest" "testing" @@ -99,3 +100,39 @@ func TestStatsAuth_UnconfiguredLocksShut(t *testing.T) { t.Errorf("unconfigured: status %d, want 401", resp.StatusCode) } } + +// TestAdminToken_CSRF_MutationRejectsQueryParam confirms that +// query-param admin_token is REJECTED on mutation endpoints (POST/PUT/...) +// — these require the X-Admin-Token header to prevent CSRF from leaked URLs. +func TestAdminToken_CSRF_MutationRejectsQueryParam(t *testing.T) { + t.Parallel() + cb := minimalCallbacks() + cb.GetAdminToken = func() string { return "op-secret" } + h := NewHandler(cb) + srv := httptest.NewServer(h.buildMux()) + defer srv.Close() + + // POST /api/admin/runtime/gc with query-param token — must 401. + body := bytes.NewBufferString("{}") + req, _ := http.NewRequest("POST", srv.URL+"/api/admin/runtime/gc?admin_token=op-secret", body) + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("POST /api/admin/runtime/gc (query-param): %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusUnauthorized { + t.Fatalf("query-param token on POST got %d, want 401", resp.StatusCode) + } + + // POST with proper X-Admin-Token header — must succeed. + req2, _ := http.NewRequest("POST", srv.URL+"/api/admin/runtime/gc", body) + req2.Header.Set("X-Admin-Token", "op-secret") + resp2, err := http.DefaultClient.Do(req2) + if err != nil { + t.Fatalf("POST /api/admin/runtime/gc (header): %v", err) + } + defer resp2.Body.Close() + if resp2.StatusCode != http.StatusOK { + t.Fatalf("header token on POST got %d, want 200", resp2.StatusCode) + } +} From cee9343caa22dde9fe9792be7e007075d70e4b65 Mon Sep 17 00:00:00 2001 From: matthew-pilot Date: Tue, 30 Jun 2026 02:30:17 +0000 Subject: [PATCH 2/2] fix(dashboard): restrict requireAdminToken query-param to GET only (PILOT-238) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The requireAdminToken middleware accepted admin_token= as a query parameter for all HTTP methods — GET, POST, PUT, DELETE, etc. This creates a CSRF vector: if an operator copies a URL containing admin_token=... into chat/browser-history/logs, an attacker can auto-submit a hidden form pointing at that URL and trigger mutations like /api/admin/runtime/gc?admin_token=..., /api/admin/runtime/log-level, or /api/admin/whitelist mutations. Fix: the query-param fallback is restricted to GET requests only. POST/PUT/DELETE must present the admin token via the X-Admin-Token header, which cannot be set by cross-site form submissions. The /api/banner handler already had an inline guard for this (merged in PR #7), but requireAdminToken — used by /api/stats, /api/pulse, /api/badge/*, /api/admin/runtime/*, /api/admin/whitelist, /api/admin/ networks/*, and /api/admin/backups — was still unconditionally accepting query-param tokens on all methods. Closes PILOT-238 --- dashboard/dashboard.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/dashboard/dashboard.go b/dashboard/dashboard.go index 7bd2be0..2b35d68 100644 --- a/dashboard/dashboard.go +++ b/dashboard/dashboard.go @@ -739,9 +739,10 @@ func (l *ipRateLimiter) middleware(maxReqs int, window time.Duration, next http. // requireAdminToken wraps next so it returns 401 Unauthorized to anyone // that doesn't present the operator-configured admin token. The token // may be supplied via X-Admin-Token header (preferred for write paths) -// or admin_token=... query parameter (read-only convenience). When no -// admin token is configured on the server the wrapped path is locked -// shut entirely — operators must set -admin-token to enable access. +// or admin_token=... query parameter (GET only — read-only convenience; +// mutations require the header to prevent CSRF from leaked URLs). +// When no admin token is configured on the server the wrapped path is +// locked shut entirely — operators must set -admin-token to enable access. // // Used to lock down the previously-public stats / pulse / badge / // snapshot endpoints so polo.pilotprotocol.network can expose only the @@ -750,7 +751,10 @@ func (l *ipRateLimiter) middleware(maxReqs int, window time.Duration, next http. func (h *Handler) requireAdminToken(next http.HandlerFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { token := r.Header.Get("X-Admin-Token") - if token == "" { + // Query-param fallback only for GET (read-only, no CSRF risk). + // POST/PUT/DELETE require the header so a leaked URL cannot + // be used for cross-site mutation attacks. + if token == "" && r.Method == http.MethodGet { token = r.URL.Query().Get("admin_token") } adminToken := h.cb.GetAdminToken()