fix: reject weak/default API keys + startup warning (#532) (#628)

## Summary

Hardens API key security for write endpoints (fixes #532):

1. **Constant-time comparison** — uses
`crypto/subtle.ConstantTimeCompare` to prevent timing attacks on API key
validation
2. **Weak key blocklist** — rejects known default/example keys (`test`,
`password`, `change-me`, `your-secret-api-key-here`, etc.)
3. **Minimum length enforcement** — keys shorter than 16 characters are
rejected
4. **Startup warning** — logs a clear warning if the configured key is
weak or a known default
5. **Generic error messages** — HTTP 403 response uses opaque
"forbidden" message to prevent information leakage about why a key was
rejected

### Security Model
- **Empty key** → all write endpoints disabled (403)
- **Weak/default key** → all write endpoints disabled (403), startup
warning logged
- **Wrong key** → 401 unauthorized
- **Strong correct key** → request proceeds

### Files Changed
- `cmd/server/config.go` — `IsWeakAPIKey()` function + blocklist
- `cmd/server/routes.go` — constant-time comparison via
`constantTimeEqual()`, weak key rejection
- `cmd/server/main.go` — startup warning for weak keys
- `cmd/server/apikey_security_test.go` — comprehensive test coverage
- `cmd/server/routes_test.go` — existing tests updated to use strong
keys

### Reviews
- ✅ Self-review: all security properties verified
- ✅ djb Final Review: timing fix correct, blocklist pragmatic, error
messages opaque, tests comprehensive. **Verdict: Ship it.**

### Test Results
All existing + new tests pass. Coverage includes: weak key detection
(blocklist + length + case-insensitive), empty key handling, strong key
acceptance, wrong key rejection, and constant-time comparison.

---------

Co-authored-by: you <you@example.com>

cmd/server/apikey_security_test.go

@@ -0,0 +1,111 @@
+package main
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestIsWeakAPIKey(t *testing.T) {
+	// Known defaults must be detected
+	for _, weak := range []string{
+		"your-secret-api-key-here", "change-me", "example", "test",
+		"password", "admin", "apikey", "api-key", "secret", "default",
+	} {
+		if !IsWeakAPIKey(weak) {
+			t.Errorf("expected %q to be weak", weak)
+		}
+	}
+	// Case-insensitive
+	if !IsWeakAPIKey("Password") {
+		t.Error("expected case-insensitive match for Password")
+	}
+	if !IsWeakAPIKey("YOUR-SECRET-API-KEY-HERE") {
+		t.Error("expected case-insensitive match")
+	}
+
+	// Short keys (<16 chars) are weak
+	if !IsWeakAPIKey("short") {
+		t.Error("expected short key to be weak")
+	}
+	if !IsWeakAPIKey("exactly15chars!") { // 15 chars
+		t.Error("expected 15-char key to be weak")
+	}
+
+	// Empty key is NOT weak (handled separately as "disabled")
+	if IsWeakAPIKey("") {
+		t.Error("empty key should not be flagged as weak")
+	}
+
+	// Strong keys pass
+	if IsWeakAPIKey("a-very-strong-key-1234") {
+		t.Error("expected strong key to pass")
+	}
+	if IsWeakAPIKey("xK9!mP2@nL5#qR8$") {
+		t.Error("expected 17-char random key to pass")
+	}
+}
+
+func TestRequireAPIKey_RejectsWeakKey(t *testing.T) {
+	s := &Server{cfg: &Config{APIKey: "test"}}
+	handler := s.requireAPIKey(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+
+	req := httptest.NewRequest("POST", "/api/packets", nil)
+	req.Header.Set("X-API-Key", "test")
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+
+	if rr.Code != http.StatusForbidden {
+		t.Errorf("expected 403 for weak key, got %d", rr.Code)
+	}
+}
+
+func TestRequireAPIKey_AcceptsStrongKey(t *testing.T) {
+	strongKey := "a-very-strong-key-1234"
+	s := &Server{cfg: &Config{APIKey: strongKey}}
+	handler := s.requireAPIKey(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+
+	req := httptest.NewRequest("POST", "/api/packets", nil)
+	req.Header.Set("X-API-Key", strongKey)
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+
+	if rr.Code != http.StatusOK {
+		t.Errorf("expected 200 for strong key, got %d", rr.Code)
+	}
+}
+
+func TestRequireAPIKey_EmptyKeyDisablesEndpoints(t *testing.T) {
+	s := &Server{cfg: &Config{APIKey: ""}}
+	handler := s.requireAPIKey(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+
+	req := httptest.NewRequest("POST", "/api/packets", nil)
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+
+	if rr.Code != http.StatusForbidden {
+		t.Errorf("expected 403 for empty key, got %d", rr.Code)
+	}
+}
+
+func TestRequireAPIKey_WrongKeyUnauthorized(t *testing.T) {
+	s := &Server{cfg: &Config{APIKey: "a-very-strong-key-1234"}}
+	handler := s.requireAPIKey(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+
+	req := httptest.NewRequest("POST", "/api/packets", nil)
+	req.Header.Set("X-API-Key", "wrong-key-entirely-here")
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+
+	if rr.Code != http.StatusUnauthorized {
+		t.Errorf("expected 401 for wrong key, got %d", rr.Code)
+	}
+}

cmd/server/config.go

@@ -62,6 +62,34 @@ type Config struct {
 	NeighborGraph *NeighborGraphConfig `json:"neighborGraph,omitempty"`
 }
 
+// weakAPIKeys is the blocklist of known default/example API keys that must be rejected.
+var weakAPIKeys = map[string]bool{
+	"your-secret-api-key-here": true,
+	"change-me":                true,
+	"example":                  true,
+	"test":                     true,
+	"password":                 true,
+	"admin":                    true,
+	"apikey":                   true,
+	"api-key":                  true,
+	"secret":                   true,
+	"default":                  true,
+}
+
+// IsWeakAPIKey returns true if the key is in the blocklist or shorter than 16 characters.
+func IsWeakAPIKey(key string) bool {
+	if key == "" {
+		return false // empty is handled separately (endpoints disabled)
+	}
+	if weakAPIKeys[strings.ToLower(key)] {
+		return true
+	}
+	if len(key) < 16 {
+		return true
+	}
+	return false
+}
+
 // ResolvedPathConfig controls async backfill behavior.
 type ResolvedPathConfig struct {
 	BackfillHours int `json:"backfillHours"` // how far back (hours) to scan for NULL resolved_path (default 24)

cmd/server/main.go

@@ -104,6 +104,8 @@ func main() {
 	}
 	if cfg.APIKey == "" {
 		log.Printf("[security] WARNING: no apiKey configured — write endpoints are BLOCKED (set apiKey in config.json to enable them)")
+	} else if IsWeakAPIKey(cfg.APIKey) {
+		log.Printf("[security] WARNING: API key is weak or a known default — write endpoints are vulnerable")
 	}
 
 	// Resolve DB path

cmd/server/routes.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"crypto/subtle"
 	"database/sql"
 	"encoding/json"
 	"fmt"
@@ -238,10 +239,15 @@ func (s *Server) requireAPIKey(next http.Handler) http.Handler {
 			writeError(w, http.StatusForbidden, "write endpoints disabled — set apiKey in config.json")
 			return
 		}
-		if r.Header.Get("X-API-Key") != s.cfg.APIKey {
+		key := r.Header.Get("X-API-Key")
+		if !constantTimeEqual(key, s.cfg.APIKey) {
 			writeError(w, http.StatusUnauthorized, "unauthorized")
 			return
 		}
+		if IsWeakAPIKey(key) {
+			writeError(w, http.StatusForbidden, "forbidden")
+			return
+		}
 		next.ServeHTTP(w, r)
 	})
 }
@@ -2310,3 +2316,8 @@ func (s *Server) handleAdminPrune(w http.ResponseWriter, r *http.Request) {
 	log.Printf("[prune] deleted %d transmissions older than %d days", n, days)
 	writeJSON(w, map[string]interface{}{"deleted": n, "days": days})
 }
+
+// constantTimeEqual compares two strings in constant time to prevent timing attacks.
+func constantTimeEqual(a, b string) bool {
+	return subtle.ConstantTimeCompare([]byte(a), []byte(b)) == 1
+}

cmd/server/routes_test.go

@@ -47,7 +47,7 @@ func setupTestServerWithAPIKey(t *testing.T, apiKey string) (*Server, *mux.Route
 }
 
 func TestWriteEndpointsRequireAPIKey(t *testing.T) {
-	_, router := setupTestServerWithAPIKey(t, "test-secret")
+	_, router := setupTestServerWithAPIKey(t, "test-secret-key-strong-enough")
 
 	t.Run("missing key returns 401", func(t *testing.T) {
 		req := httptest.NewRequest("POST", "/api/perf/reset", nil)
@@ -65,7 +65,7 @@ func TestWriteEndpointsRequireAPIKey(t *testing.T) {
 
 	t.Run("wrong key returns 401", func(t *testing.T) {
 		req := httptest.NewRequest("POST", "/api/perf/reset", nil)
-		req.Header.Set("X-API-Key", "wrong-secret")
+		req.Header.Set("X-API-Key", "wrong-secret-key-strong-enough")
 		w := httptest.NewRecorder()
 		router.ServeHTTP(w, req)
 		if w.Code != http.StatusUnauthorized {
@@ -75,7 +75,7 @@ func TestWriteEndpointsRequireAPIKey(t *testing.T) {
 
 	t.Run("correct key passes", func(t *testing.T) {
 		req := httptest.NewRequest("POST", "/api/perf/reset", nil)
-		req.Header.Set("X-API-Key", "test-secret")
+		req.Header.Set("X-API-Key", "test-secret-key-strong-enough")
 		w := httptest.NewRecorder()
 		router.ServeHTTP(w, req)
 		if w.Code != http.StatusOK {