findings of the security review of stages 2-6

The review of 4433688..0808779 (read-only, a separate agent) found one
medium and some lows; all fixed here.

Medium, proven by the reviewer: A1's userKey put the typed username into
the limiter map whole, and the public forms had no body bound beyond
net/http's 10 MiB for a parsed form - each refused request kept up to
10 MiB in memory for fifteen minutes. userKey now holds a 96-bit hash of
the folded name, and sign-in, reset, a reset link's page and setup read at
most 64 KiB (limitForm, http.MaxBytesReader).
TestPublicFormsAreBoundedAndKeysShort.

Lows:
- Refund skipped the request that filled a bucket (count < max), so a
  right password at the ceiling left the shared address blocked; Refund
  now always undoes the Allow it follows.
- The reset form did not give the address its attempt back when the
  name's bucket refused (sign-in did); it does.
- A3's day of five links is spent only by a link that goes out, and the
  price of the pause - anyone knowing a username can pause that owner's
  reset for a day - is recorded in security.md.
- longHeaderLine counted a trailing CR on a header's first line.
- test.yml declares permissions: contents: read.

Not changed, as the review's nits: the store matches names exactly while
the limiter folds them (stricter, said in the comment now); a session from
a longer window is honoured until its next Touch (the next request).

go test ./internal/web/auth ./internal/milter.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

.github/workflows/test.yml

@@ -5,6 +5,11 @@ on:
     branches: [main]
   pull_request:
 
+# The job reads the repository and nothing else (security audit 2026-10-17,
+# review of stages 2-6).
+permissions:
+  contents: read
+
 jobs:
   test:
     runs-on: ubuntu-latest

docs/security.md

@@ -154,7 +154,9 @@ The panel is exposed to the internet — the items below are **not optional**.
   security audit 2026-10-17, A1, A9). When the first requests after a start
   all come from one private peer with `X-Forwarded-For` and no proxy is
   trusted, the log says so with the value to set. A refused sign-in and a
-  lockout are logged, five lines per address in 15 minutes (A8). The Account form's
+  lockout are logged, five lines per address in 15 minutes (A8). A public
+  form - sign-in, reset, a link's page, setup - is read only up to 64 KiB,
+  and the limiters keep a hash of a typed name, not the name. The Account form's
   current-password check has a limit of its own, ten in 15 minutes per user,
   so guesses there do not lock the address out of signing in (`2.5.4`).
 - An unknown username is refused in the time a wrong password is: the sign-in
@@ -200,7 +202,11 @@ The panel is exposed to the internet — the items below are **not optional**.
   hour per account and five reset links a day - after which the owner is
   told once that further requests are ignored for 24 hours, and nothing
   more is sent (`2.14.4`; audit A3: anyone who knew a username kept a live
-  link in that mailbox five times an hour) - and a link's page to 30 requests in 15 minutes per link
+  link in that mailbox five times an hour). The price is recorded: whoever
+  knows a username can spend those five links and so pause the owner's own
+  reset for a day; signing in, and a global administrator setting a
+  password on Server › Users, still work, and only a link that went out
+  counts - and a link's page to 30 requests in 15 minutes per link
   (300 per address alone), so one stranger behind a shared proxy cannot stop
   the others asking for or opening a link (`2.14.4`; audit A1). A link's page changes nothing
   on `GET` (a mail scanner opens links): the button does. Setting a password

internal/milter/sign.go

@@ -380,7 +380,7 @@ func signLines(headers []field) ([]string, []byte) {
 func longHeaderLine(headers []field) string {
 	for _, h := range headers {
 		lines := strings.Split(h.value, "\n")
-		if len(h.name)+1+len(lines[0]) > smtpLineLimit {
+		if len(h.name)+1+len(strings.TrimRight(lines[0], "\r")) > smtpLineLimit {
 			return h.name
 		}
 		for _, l := range lines[1:] {

internal/web/auth/handlers.go

@@ -115,6 +115,7 @@ func (m *Module) submitLogin(w http.ResponseWriter, r *http.Request) {
 		m.renderLogin(w, http.StatusTooManyRequests, "Too many attempts. Please wait and try again.")
 		return
 	}
+	limitForm(w, r)
 	if err := r.ParseForm(); err != nil {
 		m.renderLogin(w, http.StatusBadRequest, "Invalid form submission.")
 		return
@@ -250,6 +251,7 @@ func (m *Module) renderSetupForm(w http.ResponseWriter, status int, token, formE
 }
 
 func (m *Module) submitSetup(w http.ResponseWriter, r *http.Request, token string) {
+	limitForm(w, r)
 	if err := r.ParseForm(); err != nil {
 		m.renderSetupForm(w, http.StatusBadRequest, token, "Invalid form submission.")
 		return

internal/web/auth/limitkeys.go

@@ -35,10 +35,24 @@ func limitAddr(r *http.Request, trusted []*net.IPNet) string {
 }
 
 // userKey is a limiter key for one username tried from one address. The
-// name is folded as the store folds it (usernames are matched without
-// regard to case).
+// name is the stranger's text, of any length a form carries: the key holds
+// its hash, not the name (the review of stages 2-6 kept a 5 MB name for
+// fifteen minutes per request). Case is folded, so "Admin" and "admin" share
+// a bucket - stricter than the store, which matches names exactly.
 func userKey(addr, username string) string {
-	return addr + "|" + strings.ToLower(strings.TrimSpace(username))
+	sum := sha256.Sum256([]byte(strings.ToLower(strings.TrimSpace(username))))
+	return addr + "|" + hex.EncodeToString(sum[:12])
+}
+
+// maxPublicForm bounds the body of a form a stranger can send - sign-in,
+// reset, a link's page, setup - far below net/http's 10 MiB for a parsed
+// form (net/http, parsePostForm: 10 << 20 when the body is not already a
+// MaxBytesReader).
+const maxPublicForm = 64 << 10
+
+// limitForm caps the body ParseForm will read on a public form.
+func limitForm(w http.ResponseWriter, r *http.Request) {
+	r.Body = http.MaxBytesReader(w, r.Body, maxPublicForm)
 }
 
 // tokenKey is a limiter key for one link: its hash, so a limiter's map holds

internal/web/auth/limitkeys_test.go

@@ -132,3 +132,19 @@ func TestDNSRechecksArePerUser(t *testing.T) {
 		t.Error("another user's first re-check was refused")
 	}
 }
+
+// A sign-in form past 64 KiB is refused before it is parsed, and the limiter
+// keeps a short hash of a typed name, not the name (the review of stages
+// 2-6: a 5 MB username stayed in the map for fifteen minutes).
+func TestPublicFormsAreBoundedAndKeysShort(t *testing.T) {
+	m := moduleWithAdmin(t)
+	if rec := postLogin(m, "203.0.113.70:1", strings.Repeat("a", maxPublicForm), "x"); rec.Code != http.StatusBadRequest {
+		t.Errorf("a sign-in form past the bound = %d, want 400", rec.Code)
+	}
+	if k := userKey("203.0.113.70", strings.Repeat("a", 1<<20)); len(k) > 64 {
+		t.Errorf("the key of a 1 MiB name is %d bytes", len(k))
+	}
+	if userKey("a", "Admin") != userKey("a", " admin ") || userKey("a", "admin") == userKey("a", "admin2") {
+		t.Error("userKey does not fold case and space, or merges two names")
+	}
+}

internal/web/auth/links.go

@@ -150,12 +150,14 @@ func (m *Module) HandleReset(w http.ResponseWriter, r *http.Request) {
 			tooMany()
 			return
 		}
+		limitForm(w, r)
 		if err := r.ParseForm(); err != nil {
 			m.view.Render(w, http.StatusBadRequest, "login-reset", view.NewReset(m.cfg.Hostname, "Invalid form submission."))
 			return
 		}
 		username := strings.TrimSpace(r.PostFormValue("username"))
 		if !m.resetLimiter.Allow(userKey(addr, username)) {
+			m.resetAddrLimiter.Refund(addr) // counted once, against the name
 			tooMany()
 			return
 		}
@@ -204,6 +206,14 @@ func (m *Module) sendReset(username string) {
 		}
 		return
 	}
+	// A day's place is spent only on a link that goes out (the review of
+	// stages 2-6).
+	sent := false
+	defer func() {
+		if !sent {
+			m.resetDayLimiter.Refund(key)
+		}
+	}()
 	if !m.mailLimiter.Allow(key) {
 		logf("panel: user %d: reset links asked for too often; none was sent", u.ID)
 		return
@@ -221,6 +231,7 @@ func (m *Module) sendReset(username string) {
 		logf("panel: user %d: reset link not sent: %v", u.ID, err)
 		return
 	}
+	sent = true
 	logf("panel: user %d: a reset link was sent", u.ID)
 }
 
@@ -307,6 +318,7 @@ func (m *Module) setPasswordFromLink(w http.ResponseWriter, r *http.Request, tok
 	fail := func(status int, msg string) {
 		m.view.Render(w, status, "login-reset-set", view.NewResetSet(m.cfg.Hostname, u.Username, token, msg))
 	}
+	limitForm(w, r)
 	if err := r.ParseForm(); err != nil {
 		fail(http.StatusBadRequest, "Invalid form submission.")
 		return

internal/web/auth/ratelimit.go

@@ -75,12 +75,14 @@ func (r *rateLimiter) Allow(key string) bool {
 }
 
 // Refund gives back the attempt Allow took for key, for an attempt that
-// turned out not to be a guess (a right password). A bucket already at its
-// limit stays there: a refund never unblocks.
+// turned out not to be a guess (a right password). It is called only after
+// an Allow that admitted the attempt, so it undoes that one: the request that
+// filled the bucket gives its own place back too (the review of stages 2-6:
+// a right password at the ceiling used to leave the address blocked).
 func (r *rateLimiter) Refund(key string) {
 	r.mu.Lock()
 	defer r.mu.Unlock()
-	if b := r.buckets[key]; b != nil && b.count > 0 && b.count < r.max {
+	if b := r.buckets[key]; b != nil && b.count > 0 {
 		b.count--
 	}
 }