Merge pull request 'Bound title and language at create time (#86)' (#91) from fix-86-field-bounds into main

Titles are trimmed and truncated to 200 characters; language must be
blank or match ^[a-zA-Z0-9+#-]{1,40}$ (max 40 chars) or the create is
rejected with a clear 400. Regression tests cover truncation, malformed
and oversized language values, valid identifiers, and blank metadata.

internal/api/issue86_field_bounds_test.go

@@ -0,0 +1,152 @@
+package api
+
+// Regression tests for #86: title and language are bounded at create time.
+// Titles over 200 chars are truncated; language must match
+// ^[a-zA-Z0-9+#-]{1,40}$ or the create is rejected with a clear 400.
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+// pasteMeta fetches a created paste's stored metadata via the API.
+func pasteMeta(t *testing.T, h http.Handler, id string) map[string]any {
+	t.Helper()
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, httptest.NewRequest("GET", "/api/pastes/"+id, nil))
+	if rec.Code != 200 {
+		t.Fatalf("get paste %s: got %d: %s", id, rec.Code, rec.Body.String())
+	}
+	var m map[string]any
+	if err := json.Unmarshal(rec.Body.Bytes(), &m); err != nil {
+		t.Fatal(err)
+	}
+	return m
+}
+
+// A 5000-char title is truncated to 200 characters at create time (#86).
+func TestCreatePasteTitleTruncated(t *testing.T) {
+	s := testServer(t)
+	h := s.routes()
+
+	title := strings.Repeat("t", 5000)
+	body, _ := json.Marshal(map[string]any{"content": "hi", "title": title})
+	rec := createPasteRaw(t, h, string(body))
+	if rec.Code != http.StatusCreated {
+		t.Fatalf("got %d want 201: %s", rec.Code, rec.Body.String())
+	}
+	var resp struct {
+		ID string `json:"id"`
+	}
+	json.Unmarshal(rec.Body.Bytes(), &resp)
+	meta := pasteMeta(t, h, resp.ID)
+	got, _ := meta["title"].(string)
+	if got != strings.Repeat("t", 200) {
+		t.Fatalf("title not truncated to 200 chars: len=%d", len(got))
+	}
+}
+
+// A title within the 200-char bound is stored verbatim (minus surrounding
+// whitespace, which is trimmed).
+func TestCreatePasteTitleWithinBoundKept(t *testing.T) {
+	s := testServer(t)
+	h := s.routes()
+
+	title := "  " + strings.Repeat("x", 200) + "  "
+	body, _ := json.Marshal(map[string]any{"content": "hi", "title": title})
+	rec := createPasteRaw(t, h, string(body))
+	if rec.Code != http.StatusCreated {
+		t.Fatalf("got %d want 201: %s", rec.Code, rec.Body.String())
+	}
+	var resp struct {
+		ID string `json:"id"`
+	}
+	json.Unmarshal(rec.Body.Bytes(), &resp)
+	meta := pasteMeta(t, h, resp.ID)
+	if got, _ := meta["title"].(string); got != strings.Repeat("x", 200) {
+		t.Fatalf("title changed unexpectedly: len=%d", len(got))
+	}
+}
+
+// A language longer than 40 chars is rejected with a clear 400 (#86).
+func TestCreatePasteLanguageTooLong400(t *testing.T) {
+	s := testServer(t)
+	h := s.routes()
+
+	body, _ := json.Marshal(map[string]any{"content": "hi", "language": strings.Repeat("a", 41)})
+	rec := createPasteRaw(t, h, string(body))
+	if rec.Code != http.StatusBadRequest {
+		t.Fatalf("got %d want 400: %s", rec.Code, rec.Body.String())
+	}
+	if !strings.Contains(rec.Body.String(), "language") {
+		t.Fatalf("unclear error message: %s", rec.Body.String())
+	}
+}
+
+// Language strings outside ^[a-zA-Z0-9+#-]{1,40}$ are rejected with 400.
+func TestCreatePasteLanguageBadFormat400(t *testing.T) {
+	s := testServer(t)
+	h := s.routes()
+
+	for _, bad := range []string{
+		"<img src=x onerror=alert(1)>",
+		"java script",
+		"c++ extra!",
+		"py_thon",
+		"go.lang",
+	} {
+		body, _ := json.Marshal(map[string]any{"content": "hi", "language": bad})
+		rec := createPasteRaw(t, h, string(body))
+		if rec.Code != http.StatusBadRequest {
+			t.Errorf("language %q: got %d want 400: %s", bad, rec.Code, rec.Body.String())
+			continue
+		}
+		if !strings.Contains(rec.Body.String(), "language must match") {
+			t.Errorf("language %q: unclear error: %s", bad, rec.Body.String())
+		}
+	}
+}
+
+// Valid languages (letters, digits, #, +, -) within 40 chars are accepted.
+func TestCreatePasteLanguageValidAccepted(t *testing.T) {
+	for _, ok := range []string{"go", "c#", "f#", "c++", "objective-c", "ECMAScript-2023", strings.Repeat("a", 40)} {
+		s2 := testServer(t) // fresh rate limiter per case
+		h2 := s2.routes()
+		body, _ := json.Marshal(map[string]any{"content": "hi", "language": ok})
+		rec := createPasteRaw(t, h2, string(body))
+		if rec.Code != http.StatusCreated {
+			t.Errorf("language %q: got %d want 201: %s", ok, rec.Code, rec.Body.String())
+		}
+	}
+}
+
+// An absent or blank language still creates fine and stores NULL, and a
+// blank title is stored NULL rather than an empty string.
+func TestCreatePasteBlankMetadataOK(t *testing.T) {
+	s := testServer(t)
+	h := s.routes()
+
+	for _, body := range []string{
+		`{"content":"hi"}`,
+		`{"content":"hi","language":"","title":"  "}`,
+	} {
+		rec := createPasteRaw(t, h, body)
+		if rec.Code != http.StatusCreated {
+			t.Fatalf("body %s: got %d want 201: %s", body, rec.Code, rec.Body.String())
+		}
+		var resp struct {
+			ID string `json:"id"`
+		}
+		json.Unmarshal(rec.Body.Bytes(), &resp)
+		meta := pasteMeta(t, h, resp.ID)
+		if lang, ok := meta["language"]; ok && lang != nil && lang != "" {
+			t.Fatalf("body %s: language not null: %v", body, lang)
+		}
+		if title, ok := meta["title"]; ok && title != nil && title != "" {
+			t.Fatalf("body %s: title not null: %v", body, title)
+		}
+	}
+}

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

@@ -165,6 +165,27 @@ func (a *apiServer) handleCreatePaste(w http.ResponseWriter, r *http.Request) {
 		writeErr(w, status, msg)
 		return
 	}
+	// #86: bound free-form metadata at create time
+	if p.Title != nil {
+		t, err := checkTitle(*p.Title)
+		if err != nil {
+			writeErr(w, 400, err.Error())
+			return
+		}
+		p.Title = &t
+	}
+	if p.Language != nil {
+		l, err := checkLanguage(*p.Language)
+		if err != nil {
+			writeErr(w, 400, err.Error())
+			return
+		}
+		if l == "" {
+			p.Language = nil
+		} else {
+			p.Language = &l
+		}
+	}
 	if p.BurnAfterReads != nil { // #68: reject negative read budgets
 		if err := parseBurnAfterReads(*p.BurnAfterReads); err != nil {
 			writeErr(w, 400, err.Error())

internal/api/validate.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"regexp"
 	"strconv"
 	"strings"
 )
@@ -90,3 +91,46 @@ func parseBurnAfterReads(n int) error {
 	}
 	return nil
 }
+
+// #86: bounds for free-form metadata fields on create.
+const (
+	maxTitleLen    = 200
+	maxLanguageLen = 40
+)
+
+// languageRe restricts language to identifiers like go, c#, f#, c++, objc.
+var languageRe = regexp.MustCompile(`^[a-zA-Z0-9+#-]{1,40}$`)
+
+// checkTitle validates the paste title (#86): over-max titles are truncated
+// to 200 characters so a bloated listing entry can't be stored; whitespace
+// is trimmed first.
+func checkTitle(title string) (string, error) {
+	title = strings.TrimSpace(title)
+	if len(title) > maxTitleLen {
+		return truncateRunes(title, maxTitleLen), nil
+	}
+	return title, nil
+}
+
+// checkLanguage validates the language field (#86): optional, max 40 chars,
+// and must match ^[a-zA-Z0-9+#-]{1,40}$. Returns "" for absent/blank values.
+// Anything else malformed is a 400.
+func checkLanguage(lang string) (string, error) {
+	lang = strings.TrimSpace(lang)
+	if lang == "" {
+		return "", nil
+	}
+	if len(lang) > maxLanguageLen || !languageRe.MatchString(lang) {
+		return "", fmt.Errorf("language must match ^[a-zA-Z0-9+#-]{1,40}$ (max %d chars)", maxLanguageLen)
+	}
+	return lang, nil
+}
+
+// truncateRunes cuts s to at most max runes, keeping the prefix intact.
+func truncateRunes(s string, max int) string {
+	runes := []rune(s)
+	if len(runes) <= max {
+		return s
+	}
+	return string(runes[:max])
+}