Merge pull request 'ratelimit: rightmost untrusted-hop-safe client IP (fixes #85)' (#89) from fix-85-xff 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

docs/API.md

@@ -22,6 +22,10 @@ curl -X POST http://localhost:8080/api/pastes \
 
 - `expires_in` is a Go duration string (`90m`, `6h`, `336h`). Omit for no expiry.
 - `visibility` is `public` or `unlisted`.
+- Alternatively (or additionally), `public` may be sent as a boolean (#83):
+  `false` maps to `unlisted` and `true` maps to `public`. When both fields are
+  present, the boolean `public` takes precedence over the string `visibility`.
+  Omitting both defaults to `public`.
 - `burn_after_reads` sets how many reads the paste survives (default 1 when
   `burn_after_read` is true). A read is counted per unique viewer session;
   the same viewer returning within 15 minutes does not count again.

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/publicbool_test.go

@@ -0,0 +1,97 @@
+package api
+
+// #83 regression tests: `public` boolean in the create payload must map to
+// visibility (false -> unlisted, true -> public); string `visibility` still works.
+
+import (
+	"encoding/json"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+func createPasteBody(t *testing.T, h *apiServer, body string) map[string]any {
+	req := httptest.NewRequest("POST", "/api/pastes", strings.NewReader(body))
+	rec := httptest.NewRecorder()
+	h.routes().ServeHTTP(rec, req)
+	if rec.Code != 201 {
+		t.Fatalf("create: got %d: %s", rec.Code, rec.Body.String())
+	}
+	var resp map[string]any
+	if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("bad json: %v", err)
+	}
+	return resp
+}
+
+func getVis(t *testing.T, h *apiServer, id string) string {
+	req := httptest.NewRequest("GET", "/api/pastes/"+id, nil)
+	rec := httptest.NewRecorder()
+	h.routes().ServeHTTP(rec, req)
+	if rec.Code != 200 {
+		t.Fatalf("get %s: got %d", id, rec.Code)
+	}
+	var resp struct {
+		Visibility string `json:"visibility"`
+	}
+	if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("bad json: %v", err)
+	}
+	return resp.Visibility
+}
+
+// TestPublicBooleanFalseMapsToUnlisted: {"public": false} must create an unlisted paste.
+func TestPublicBooleanFalseMapsToUnlisted(t *testing.T) {
+	s := testServer(t)
+	resp := createPasteBody(t, s, `{"content":"x","public":false}`)
+	if v := getVis(t, s, resp["id"].(string)); v != "unlisted" {
+		t.Fatalf("public:false -> got visibility %q, want unlisted", v)
+	}
+}
+
+// TestPublicBooleanTrueMapsToPublic: {"public": true} must create a public paste.
+func TestPublicBooleanTrueMapsToPublic(t *testing.T) {
+	s := testServer(t)
+	resp := createPasteBody(t, s, `{"content":"x","public":true}`)
+	if v := getVis(t, s, resp["id"].(string)); v != "public" {
+		t.Fatalf("public:true -> got visibility %q, want public", v)
+	}
+}
+
+// TestPublicBooleanOverridesString: boolean wins when both fields are sent.
+func TestPublicBooleanOverridesString(t *testing.T) {
+	s := testServer(t)
+	resp := createPasteBody(t, s, `{"content":"x","visibility":"public","public":false}`)
+	if v := getVis(t, s, resp["id"].(string)); v != "unlisted" {
+		t.Fatalf("boolean override -> got %q, want unlisted", v)
+	}
+}
+
+// TestVisibilityStringStillWorks: existing string contract unchanged.
+func TestVisibilityStringStillWorks(t *testing.T) {
+	s := testServer(t)
+	resp := createPasteBody(t, s, `{"content":"x","visibility":"unlisted"}`)
+	if v := getVis(t, s, resp["id"].(string)); v != "unlisted" {
+		t.Fatalf("string field -> got %q, want unlisted", v)
+	}
+	resp = createPasteBody(t, s, `{"content":"y","visibility":"public"}`)
+	if v := getVis(t, s, resp["id"].(string)); v != "public" {
+		t.Fatalf("string field -> got %q, want public", v)
+	}
+}
+
+// TestPublicListExcludesPublicFalse: {"public":false} pastes stay out of /api/public.
+func TestPublicListExcludesPublicFalse(t *testing.T) {
+	s := testServer(t)
+	createPasteBody(t, s, `{"content":"hidden","public":false}`)
+	req := httptest.NewRequest("GET", "/api/public", nil)
+	rec := httptest.NewRecorder()
+	s.routes().ServeHTTP(rec, req)
+	var resp struct {
+		Total int `json:"total"`
+	}
+	json.Unmarshal(rec.Body.Bytes(), &resp)
+	if resp.Total != 0 {
+		t.Fatalf("public:false paste leaked into /api/public: total=%d", resp.Total)
+	}
+}

internal/api/ratelimit.go

@@ -48,8 +48,28 @@ func (l *limiter) allow(key string, rate, burst float64) bool {
 	return true
 }
 
-// clientIP extracts the request IP (no reverse proxy header by default).
+// clientIP extracts the client IP for rate-limit keying (#85).
+//
+// Trust boundary: palette runs behind exactly ONE trusted reverse proxy
+// (Traefik in the k3s pod network). Traefik APPENDS the real client IP to
+// X-Forwarded-For, so the RIGHTMOST entry is the last value the trusted
+// proxy observed and is unspoofable by the client (a client-supplied fake
+// entry only lands on the LEFT and is ignored). This matches chi's
+// middleware.RealIP semantics for a single trusted proxy hop.
+//
+// Direct connections (no XFF header) fall back to RemoteAddr. Directly
+// reachable deployments must NOT expose the app to untrusted networks
+// without a proxy in front, or attackers could forge the rightmost entry.
 func clientIP(r *http.Request) string {
+	if xff := r.Header.Get("X-Forwarded-For"); xff != "" {
+		if i := strings.LastIndex(xff, ","); i >= 0 {
+			return strings.TrimSpace(xff[i+1:])
+		}
+		return strings.TrimSpace(xff)
+	}
+	if xr := r.Header.Get("X-Real-Ip"); xr != "" {
+		return strings.TrimSpace(xr)
+	}
 	host := r.RemoteAddr
 	if i := strings.LastIndex(host, ":"); i > 0 {
 		host = host[:i]

internal/api/ratelimit_xff_test.go

@@ -0,0 +1,85 @@
+package api
+
+// Issue #85: the rate limit key must use the rightmost X-Forwarded-For entry
+// (appended by the trusted Traefik proxy), never the raw/leftmost header
+// value a client can forge. A spoofed FIRST XFF entry must not bypass the
+// limit or rotate buckets.
+
+import (
+	"bytes"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestClientIPTakesRightmostXFF(t *testing.T) {
+	r := httptest.NewRequest("POST", "/", nil)
+	r.RemoteAddr = "10.42.0.7:51000" // trusted Traefik pod
+	r.Header.Set("X-Forwarded-For", "1.2.3.4, 1.2.3.5, 203.0.113.9")
+	if got := clientIP(r); got != "203.0.113.9" {
+		t.Fatalf("clientIP = %q, want rightmost 203.0.113.9", got)
+	}
+}
+
+func TestClientIPXRealIPFallback(t *testing.T) {
+	r := httptest.NewRequest("POST", "/", nil)
+	r.RemoteAddr = "10.42.0.7:51000"
+	r.Header.Set("X-Real-Ip", "203.0.113.10")
+	if got := clientIP(r); got != "203.0.113.10" {
+		t.Fatalf("clientIP = %q, want 203.0.113.10", got)
+	}
+}
+
+func TestClientIPDirectFallback(t *testing.T) {
+	r := httptest.NewRequest("POST", "/", nil)
+	r.RemoteAddr = "198.51.100.5:51000"
+	if got := clientIP(r); got != "198.51.100.5" {
+		t.Fatalf("clientIP = %q, want 198.51.100.5", got)
+	}
+}
+
+// TestRateLimitSpoofedFirstXFFDoesNotBypass: an attacker rotating a fake
+// leftmost XFF entry stays limited on their real (rightmost) IP.
+func TestRateLimitSpoofedFirstXFFDoesNotBypass(t *testing.T) {
+	srv := newTestServer(t)
+	h := srv.routes()
+	for i := 0; i < 5; i++ {
+		req := httptest.NewRequest("POST", "/api/pastes", bytes.NewReader([]byte(`{"content":"hi"}`)))
+		req.RemoteAddr = "10.42.0.7:51000"
+		// each request spoofs a DIFFERENT leftmost entry
+		req.Header.Set("X-Forwarded-For", spoofN(i)+", 203.0.113.9")
+		rr := httptest.NewRecorder()
+		h.ServeHTTP(rr, req)
+		if rr.Code != 201 {
+			t.Fatalf("req %d: want 201, got %d", i, rr.Code)
+		}
+	}
+	// 6th request, still the same real IP, new spoofed prefix: must 429
+	req := httptest.NewRequest("POST", "/api/pastes", bytes.NewReader([]byte(`{"content":"hi"}`)))
+	req.RemoteAddr = "10.42.0.7:51000"
+	req.Header.Set("X-Forwarded-For", "9.9.9.9, 203.0.113.9")
+	rr := httptest.NewRecorder()
+	h.ServeHTTP(rr, req)
+	if rr.Code != 429 {
+		t.Fatalf("spoofed 6th req: want 429, got %d", rr.Code)
+	}
+}
+
+func spoofN(i int) string {
+	return "1.2.3." + string(rune('0'+i))
+}
+
+// Distinct real IPs must still get distinct buckets (no over-limiting).
+func TestRateLimitDistinctRightmostIPsIndependent(t *testing.T) {
+	srv := newTestServer(t)
+	h := srv.routes()
+	for _, ip := range []string{"203.0.113.20", "203.0.113.21"} {
+		req := httptest.NewRequest("POST", "/api/pastes", bytes.NewReader([]byte(`{"content":"hi"}`)))
+		req.RemoteAddr = "10.42.0.7:51000"
+		req.Header.Set("X-Forwarded-For", "6.6.6.6, "+ip)
+		rr := httptest.NewRecorder()
+		h.ServeHTTP(rr, req)
+		if rr.Code != 201 {
+			t.Fatalf("ip %s: want 201, got %d", ip, rr.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/store/store.go

@@ -30,6 +30,8 @@ type Paste struct {
 	BurnAfterRead bool  `json:"burn_after_read,omitempty"`
 	BurnAfterReads *int `json:"burn_after_reads,omitempty"` // #49: readable N times (default 1)
 	Visibility  string  `json:"visibility"`
+	// #83: accept "public": true/false as an alias for visibility.
+	Public *bool `json:"public,omitempty"`
 	CanID       *string `json:"can_id,omitempty"`
 	CreatedAt   int64   `json:"created_at"`
 	DeletedAt   *int64  `json:"deleted_at,omitempty"`
@@ -215,6 +217,14 @@ func (s *Store) CreatePaste(p *Paste) (*Paste, error) {
 	}
 
 	visibility := p.Visibility
+	// #83: "public": false -> unlisted, true -> public; overrides string field
+	if p.Public != nil {
+		if *p.Public {
+			visibility = "public"
+		} else {
+			visibility = "unlisted"
+		}
+	}
 	if visibility == "" {
 		visibility = "public"
 	}

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 != "" {
-		t.Errorf("unexpected CSP %q on JSON response", got)
+	if got := rec.Header().Get("Content-Security-Policy"); got != wantCSP {
+		t.Errorf("CSP missing on JSON response: got %q", got)
 	}
-	if got := rec.Header().Get("Referrer-Policy"); got != "" {
-		t.Errorf("unexpected Referrer-Policy %q on JSON response", 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)
+	if got := rec.Header().Get("Referrer-Policy"); got != "no-referrer" {
+		t.Errorf("Referrer-Policy missing on JSON response: got %q", 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",
-				"default-src 'self'; script-src 'self' 'unsafe-inline'; frame-ancestors 'none'")
-			h.Set("Referrer-Policy", "no-referrer")
-			h.Set("X-Content-Type-Options", "nosniff")
-		}
+		h.Set("Content-Security-Policy",
+			"default-src 'self'; script-src 'self' 'unsafe-inline'; frame-ancestors 'none'")
+		h.Set("Referrer-Policy", "no-referrer")
+		h.Set("X-Content-Type-Options", "nosniff")
+		next.ServeHTTP(w, r)
 	})
 }