No test enforces that every mutating route carries checkOrigin #157

closed cmc opened this on 2026-09-05 04:10 UTC · security · milestone v1.14.0

Discussion

cmc 2026-09-05 04:10 UTC

The session cookie is SameSite=Lax (#155). The justification, recorded in the checkOrigin doc comment, is that checkOrigin is the primary CSRF defense because Lax withholds the cookie from cross-site POSTs but not from cross-site top-level GETs.

That rests on every cookie-authenticated mutating route being wrapped. It is true today — verified twice during !261 — but only by inspection. A new Route{Method: "POST", ...} added without checkOrigin would silently weaken the defense the comment claims.

Fix: a test over the route table asserting every mutating route is wrapped, in the spirit of TestViewOnlyHasNoMutatingRoutes. The four POST routes that legitimately lack it — git-upload-pack, git-receive-pack, lfs/objects/batch, /api/v1/cmd — accept no session cookie and belong in an explicit allowlist the test names, so adding to that list is a deliberate act.

referenced in commit 69433232c5 by cmc: httpd: open registration in the checkOrigin route walk so POST /register is reachable

2026-09-05 18:22 UTC

closed by commit e3cd3c4236 by cmc: httpd: pin the login() disabled guard and the checkOrigin invariant

2026-09-05 18:22 UTC