From f76a175091f286a2924785dfa36414476c97c31b Mon Sep 17 00:00:00 2001
From: sneak <sneak@sneak.berlin>
Date: Fri, 2 Oct 2026 14:54:59 +0000
Subject: [PATCH] Pin the body cap's order in every page route group (closes
 #93)

A route test now posts an oversized body with no session or CSRF
token to a POST route in each page route group that has one, and
requires 413 with no CSRF cookie. Before, only the login form pinned
the cap ahead of CSRF; reordering the /hooks or /hook groups failed
nothing.

The MaxBodySize doc comment says other methods pass uncapped on
purpose, the middleware test comment names the helper it describes,
and NewRouterForTest says why its hand-built Server is enough. The
README already described the cap's position correctly.

Model: opus-5-5
---
 internal/middleware/middleware.go      |  5 +++-
 internal/middleware/middleware_test.go | 10 ++++---
 internal/server/export_test.go         |  7 +++++
 internal/server/routes_test.go         | 38 ++++++++++++++++++++++++++
 4 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go
index 51c8bbd..a1a312b 100644
--- a/internal/middleware/middleware.go
+++ b/internal/middleware/middleware.go
@@ -600,7 +600,10 @@ func bodyLimitedMethod(method string) bool {
 }
 
 // MaxBodySize returns middleware that limits the size of
-// POST/PUT/PATCH request bodies to maxBytes. It must be registered
+// POST/PUT/PATCH request bodies to maxBytes. A request with any other
+// method passes through uncapped, deliberately: no route behind it
+// reads a body on GET, HEAD or DELETE. A handler that starts to needs
+// its method added to bodyLimitedMethod first. It must be registered
 // before any middleware that parses the body — notably CSRF, which
 // calls r.PostFormValue — so that form parsing happens under this
 // cap rather than net/http's 10 MB default.
diff --git a/internal/middleware/middleware_test.go b/internal/middleware/middleware_test.go
index e576659..4482232 100644
--- a/internal/middleware/middleware_test.go
+++ b/internal/middleware/middleware_test.go
@@ -730,10 +730,8 @@ func TestNoCache_SetsHeaders(t *testing.T) {
 
 const testBodyLimit int64 = 64
 
-// maxBodySizeHandler wraps a sentinel handler in MaxBodySize with
-// testBodyLimit. The sentinel records whether it ran and how much of
-// the body it managed to read, so tests can distinguish "never
-// reached" from "reached but truncated".
+// maxBodySizeResult is what runMaxBodySize's sentinel handler saw,
+// together with the response.
 type maxBodySizeResult struct {
 	called   bool
 	read     int
@@ -741,6 +739,10 @@ type maxBodySizeResult struct {
 	response *httptest.ResponseRecorder
 }
 
+// runMaxBodySize wraps a sentinel handler in MaxBodySize with
+// testBodyLimit and serves req through it. The sentinel records
+// whether it ran and how much of the body it managed to read, so
+// tests can distinguish "never reached" from "reached but truncated".
 func runMaxBodySize(
 	t *testing.T,
 	req *http.Request,
diff --git a/internal/server/export_test.go b/internal/server/export_test.go
index 4c4f645..945abef 100644
--- a/internal/server/export_test.go
+++ b/internal/server/export_test.go
@@ -39,6 +39,13 @@ func SentryClientOptionsForTest(
 // and the HTTP listener. Tests use it so that route-group middleware
 // registration order is exercised exactly as it ships, rather than
 // against a hand-rebuilt chain that could drift from routes.go.
+//
+// The Server itself is built by hand rather than through New, because
+// New registers the fx hooks that start the listener. That is enough
+// only while SetupRoutes reads no fields beyond mw, h, params.Config
+// and sentryEnabled: a field it starts reading must also be set here
+// and in the two probe variants below, or these tests run with it
+// zero.
 func NewRouterForTest(
 	log *slog.Logger,
 	cfg *config.Config,
diff --git a/internal/server/routes_test.go b/internal/server/routes_test.go
index 1d12f11..bd9bce5 100644
--- a/internal/server/routes_test.go
+++ b/internal/server/routes_test.go
@@ -531,6 +531,44 @@ func TestStaticServesOnlyGetAndHead(t *testing.T) {
 	}
 }
 
+// --- every page route group ---
+
+// TestPageRouteGroups_OversizeBody_RejectedBeforeCSRF pins the body
+// cap ahead of CSRF and RequireAuth in every page route group that
+// has a POST route. The requests carry no session and no CSRF token,
+// so if either ran first the answer would be a 403 or a redirect to
+// the login page rather than 413, and CSRF would issue its cookie
+// (see TestPagesLogin_UnderLimit_NoToken_CSRFRejects). The user and
+// webhook in the paths need not exist: nothing after the cap runs.
+func TestPageRouteGroups_OversizeBody_RejectedBeforeCSRF(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	form := url.Values{}
+	form.Set("name", oversizeValue())
+
+	for _, path := range []string{
+		"/pages/login",
+		"/user/nobody/password",
+		"/hooks/new",
+		"/hook/nonexistent/edit",
+	} {
+		w := env.post(path, form, nil)
+
+		assert.Equal(
+			t, http.StatusRequestEntityTooLarge, w.Code, path,
+		)
+		assert.False(
+			t, csrfCookieSet(w),
+			"CSRF middleware must not run for an oversized body to %s",
+			path,
+		)
+	}
+}
+
 // --- /pages group ---
 
 // TestPagesLogin_OversizeBody_RejectedBeforeCSRF proves the cap runs