From 9811191648bf9215ebf5b2ae46277608c64cb323 Mon Sep 17 00:00:00 2001 From: palette-agent Date: Wed, 9 Sep 2026 09:15:01 -0500 Subject: [PATCH] admin: per-IP rate limit (5/min) on admin key attempts (#66) --- internal/api/admin.go | 5 ++ internal/api/adminratelimit_test.go | 80 +++++++++++++++++++++++++++++ internal/api/ratelimit.go | 6 +++ 3 files changed, 91 insertions(+) create mode 100644 internal/api/adminratelimit_test.go diff --git a/internal/api/admin.go b/internal/api/admin.go index bdde0da..b88d9dc 100644 --- a/internal/api/admin.go +++ b/internal/api/admin.go @@ -172,6 +172,11 @@ func (a *apiServer) adminKeyOK(r *http.Request, key string) bool { func (a *apiServer) adminAuth(next http.HandlerFunc, key string) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + if !rateLimitAdmin(r) { + log.Printf("admin auth RATE LIMITED: %s %s from %s", r.Method, r.URL.Path, r.RemoteAddr) + writeRateLimited(w, 60) + return + } if !a.adminKeyOK(r, key) { log.Printf("admin auth FAILURE: %s %s from %s", r.Method, r.URL.Path, r.RemoteAddr) writeErr(w, 401, "unauthorized") diff --git a/internal/api/adminratelimit_test.go b/internal/api/adminratelimit_test.go new file mode 100644 index 0000000..677ce4b --- /dev/null +++ b/internal/api/adminratelimit_test.go @@ -0,0 +1,80 @@ +package api + +// #66: admin key attempts must be rate limited per IP (5/min), constant-time +// compared, and failures logged. Hammering bad keys must yield 429s. + +import ( + "net/http/httptest" + "strings" + "testing" +) + +// TestAdminKeyRateLimited: burst of 5 bad-key attempts allowed (401), the 6th +// gets 429, and even the correct key is blocked from that IP until refill. +func TestAdminKeyRateLimited(t *testing.T) { + srv := newTestServer(t) + h := srv.routes() + reqIP := "10.7.7.1:1234" + + var got429, retryAfter bool + var lastCode int + for i := 0; i < 10; i++ { + req := httptest.NewRequest("POST", "/admin/api/settings", nil) + req.RemoteAddr = reqIP + req.Header.Set("X-Admin-Key", "wrong-key") + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + lastCode = rec.Code + if rec.Code == 429 { + got429 = true + retryAfter = rec.Header().Get("Retry-After") != "" + break + } + } + if !got429 { + t.Fatalf("expected 429 after hammering bad keys, last status %d", lastCode) + } + if !retryAfter { + t.Error("429 missing Retry-After header") + } + + // Correct key from the same IP is also locked out. + req := httptest.NewRequest("POST", "/admin/api/settings", nil) + req.RemoteAddr = reqIP + req.Header.Set("X-Admin-Key", srv.adminKey) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + if rec.Code != 429 { + t.Errorf("correct key after lockout: got %d, want 429", rec.Code) + } + + // A different IP is unaffected. + req2 := httptest.NewRequest("POST", "/admin/api/settings", strings.NewReader(`{"rate_limit_burst":5,"rate_limit_per_minute":60,"max_content_bytes":1048576,"custom_slug_reservation_days":30,"burn_viewer_window_minutes":15}`)) + req2.RemoteAddr = "203.0.113.9:1234" + req2.Header.Set("X-Admin-Key", srv.adminKey) + rec2 := httptest.NewRecorder() + h.ServeHTTP(rec2, req2) + if rec2.Code != 200 { + t.Errorf("correct key from another IP: got %d, want 200", rec2.Code) + } +} + +// TestAdminKeyConstantTimeCompare: sanity check that the comparison is +// constant-time (uses subtle.ConstantTimeCompare, not ==). +func TestAdminKeyConstantTimeCompare(t *testing.T) { + srv := newTestServer(t) + r := httptest.NewRequest("GET", "/", nil) + r.Header.Set("X-Admin-Key", "test-admin-key") + if !srv.adminKeyOK(r, srv.adminKey) { + t.Fatal("correct key rejected") + } + r.Header.Set("X-Admin-Key", "wrong") + if srv.adminKeyOK(r, srv.adminKey) { + t.Fatal("wrong key accepted") + } + // differ in length: must not panic/mismatch unexpectedly + r.Header.Set("X-Admin-Key", "test-admin-key-longer") + if srv.adminKeyOK(r, srv.adminKey) { + t.Fatal("longer wrong key accepted") + } +} diff --git a/internal/api/ratelimit.go b/internal/api/ratelimit.go index c31aec0..17a5a8b 100644 --- a/internal/api/ratelimit.go +++ b/internal/api/ratelimit.go @@ -84,6 +84,12 @@ func rateLimitUnlock(id string, r *http.Request) bool { return globalLimiter.allow("unlock:"+id+":"+clientIP(r), 5.0/60.0, 5) } +// rateLimitAdmin: 5 attempts per minute per IP on the admin key check (#66), +// same pattern as the unlock limiter (#34). +func rateLimitAdmin(r *http.Request) bool { + return globalLimiter.allow("admin:"+clientIP(r), 5.0/60.0, 5) +} + // writeRateLimited responds 429 with Retry-After based on refill rate. func writeRateLimited(w http.ResponseWriter, retryAfterSecs int) { w.Header().Set("Retry-After", strconv.Itoa(retryAfterSecs))