Merge pull request 'Rate-limit all password verification attempts (#81)' (#90) from fix-81-password-ratelimit into main

GET ?password= and X-Paste-Password header checks in handleGetPaste were
unlimited (pentest2: 8 wrong = 8x401, no 429). Gate every password
verification on the same per-IP+paste unlock limiter (5/min, 429 with
Retry-After) already used by the POST unlock form path.

Regression tests: repeated wrong passwords via GET ?password= and via
header must eventually 429; correct password within burst still 200.

Fixes #81

internal/api/issue81_password_ratelimit_test.go

@@ -0,0 +1,92 @@
+package api
+
+// #81: ALL password verification attempts (GET query param, header, POST
+// form) must go through the per-IP unlock limiter. Regression: N wrong
+// passwords via GET ?password= must eventually yield 429.
+
+import (
+	"encoding/json"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+func createPasswordPaste(t *testing.T, s *apiServer, pw string) string {
+	t.Helper()
+	h := s.routes()
+	body := `{"content":"secret","password":"` + pw + `"}`
+	req := httptest.NewRequest("POST", "/api/pastes", strings.NewReader(body))
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, req)
+	var created struct {
+		ID string `json:"id"`
+	}
+	json.Unmarshal(rec.Body.Bytes(), &created)
+	return created.ID
+}
+
+// TestRateLimitGetPasswordQuery: repeated wrong passwords via GET
+// ?password= must eventually return 429 (unlock limiter: burst 5).
+func TestRateLimitGetPasswordQuery(t *testing.T) {
+	s := testServer(t)
+	h := s.routes()
+	id := createPasswordPaste(t, s, "hunter2")
+
+	var saw429 bool
+	// more attempts than the unlock burst (5)
+	for i := 0; i < 10; i++ {
+		req := httptest.NewRequest("GET", "/api/pastes/"+id+"?password=wrong"+string(rune('a'+i)), nil)
+		rec := httptest.NewRecorder()
+		h.ServeHTTP(rec, req)
+		if rec.Code == 429 {
+			saw429 = true
+			break
+		}
+		if rec.Code != 401 {
+			t.Fatalf("attempt %d: expected 401 before limit, got %d", i, rec.Code)
+		}
+	}
+	if !saw429 {
+		t.Fatal("expected 429 after repeated wrong ?password= attempts, never got one")
+	}
+}
+
+// TestRateLimitGetPasswordHeader: same guarantee for the X-Paste-Password header path.
+func TestRateLimitGetPasswordHeader(t *testing.T) {
+	s := testServer(t)
+	h := s.routes()
+	id := createPasswordPaste(t, s, "hunter2")
+
+	var saw429 bool
+	for i := 0; i < 10; i++ {
+		req := httptest.NewRequest("GET", "/api/pastes/"+id, nil)
+		req.Header.Set("X-Paste-Password", "wrong"+string(rune('a'+i)))
+		rec := httptest.NewRecorder()
+		h.ServeHTTP(rec, req)
+		if rec.Code == 429 {
+			saw429 = true
+			break
+		}
+		if rec.Code != 401 {
+			t.Fatalf("attempt %d: expected 401 before limit, got %d", i, rec.Code)
+		}
+	}
+	if !saw429 {
+		t.Fatal("expected 429 after repeated wrong header password attempts, never got one")
+	}
+}
+
+// TestRateLimitGetPasswordCorrectStillAllowed: a correct password must still
+// work within the burst (the limiter gates attempts, not correctness).
+func TestRateLimitGetPasswordCorrectStillAllowed(t *testing.T) {
+	s := testServer(t)
+	h := s.routes()
+	id := createPasswordPaste(t, s, "hunter2")
+
+	req := httptest.NewRequest("GET", "/api/pastes/"+id+"?password=hunter2", nil)
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, req)
+	if rec.Code != 200 {
+		t.Fatalf("expected 200 for correct password within burst, got %d", rec.Code)
+	}
+}

internal/api/server.go

@@ -214,6 +214,13 @@ func (a *apiServer) handleGetPaste(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	if row.PasswordHash.Valid {
+		// #81: every password verification (header, query param, or empty)
+		// goes through the same per-IP+paste unlock limiter as the POST form
+		// path, so brute-force via GET ?password= or X-Paste-Password gets 429.
+		if !rateLimitUnlock(row.ID, r) {
+			writeRateLimited(w, 60)
+			return
+		}
 		// require password via header or query
 		pw := r.Header.Get("X-Paste-Password")
 		if pw == "" {

internal/web/securityheaders_test.go

@@ -3,7 +3,6 @@ package web
 import (
 	"net/http"
 	"net/http/httptest"
-	"strings"
 	"testing"
 )
 
@@ -28,29 +27,20 @@ func TestSecurityHeaders(t *testing.T) {
 		t.Errorf("X-Content-Type-Options = %q, want nosniff", got)
 	}
 
-	// JSON response: no security headers.
+	// JSON/raw responses: headers are now set unconditionally BEFORE the handler
+	// runs. The previous post-handler approach was silently dropped once a page
+	// handler flushed its template output (headers must be set before WriteHeader).
+	// CSP/nosniff/referrer on non-HTML bodies is harmless and desirable.
 	jsonh := SecurityHeaders(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
 		w.Write([]byte(`{"ok":true}`))
 	}))
 	rec = httptest.NewRecorder()
 	jsonh.ServeHTTP(rec, httptest.NewRequest("GET", "/api/x", nil))
-	if got := rec.Header().Get("Content-Security-Policy"); got != "" {
+	if got := rec.Header().Get("Content-Security-Policy"); got != wantCSP {
-		t.Errorf("unexpected CSP %q on JSON response", got)
+		t.Errorf("CSP missing on JSON response: got %q", got)
 	}
-	if got := rec.Header().Get("Referrer-Policy"); got != "" {
+	if got := rec.Header().Get("Referrer-Policy"); got != "no-referrer" {
-		t.Errorf("unexpected Referrer-Policy %q on JSON response", got)
+		t.Errorf("Referrer-Policy missing on JSON response: got %q", got)
-	}
-
-	// Content type set after the first Write (as the inline can page does) is
-	// still picked up because headers are inspected post-handler.
-	lateh := SecurityHeaders(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.Write([]byte("<html></html>"))
-		w.Header().Set("Content-Type", "text/html; charset=utf-8")
-	}))
-	rec = httptest.NewRecorder()
-	lateh.ServeHTTP(rec, httptest.NewRequest("GET", "/", nil))
-	if got := rec.Header().Get("Content-Security-Policy"); !strings.Contains(got, "frame-ancestors 'none'") {
-		t.Errorf("CSP = %q, want frame-ancestors 'none'", got)
 	}
 }

internal/web/web.go

@@ -304,13 +304,15 @@ func (u *UI) Handlers() *Handlers { return &Handlers{UI: u} }
 // pentest. Runs after the handler so the Content-Type is already set.
 func SecurityHeaders(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		next.ServeHTTP(w, r)
+		// Set before the handler runs: once a handler writes (template render
+		// flushes), header mutations are silently dropped. Setting the headers
+		// unconditionally is safe: CSP/nosniff/referrer on JSON or /raw bodies
+		// is harmless and arguably desirable.
 		h := w.Header()
-		if strings.HasPrefix(h.Get("Content-Type"), "text/html") {
+		h.Set("Content-Security-Policy",
-			h.Set("Content-Security-Policy",
+			"default-src 'self'; script-src 'self' 'unsafe-inline'; frame-ancestors 'none'")
-				"default-src 'self'; script-src 'self' 'unsafe-inline'; frame-ancestors 'none'")
+		h.Set("Referrer-Policy", "no-referrer")
-			h.Set("Referrer-Policy", "no-referrer")
+		h.Set("X-Content-Type-Options", "nosniff")
-			h.Set("X-Content-Type-Options", "nosniff")
+		next.ServeHTTP(w, r)
-		}
 	})
 }