Harden http.Server: slowloris timeouts and form body limit (closes #92) #123

Merged
clawbot merged 4 commits from issue-92-server-hardening into next 2026-09-21 20:59:25 +02:00
Collaborator

Implements both HTTP-server hardening items from the audit.

Server timeouts. Adds HTTPReadHeaderTimeout (10s) and HTTPIdleTimeout
(120s) and wires them onto the server. ReadHeaderTimeout bounds the
header-read phase on its own — the slowloris dribble that ReadTimeout
(whole request) does not cover. IdleTimeout bounds keep-alive reuse so idle
connections cannot accumulate without limit. Server construction moves into a
small newHTTPServer helper so the timeouts are assertable without binding a
listener.

Body limit. A LimitBody middleware caps POST / and POST /generate
bodies at 1 MiB (MaxFormBytes) and returns 413 on excess, making the bound
chosen rather than resting on ParseForm's incidental 10 MB cap.

What to trip over — ordering against CSRF. gorilla/csrf (landed on
next) reads its token from the form body via PostFormValue, which swallows
a parse error. If the cap lived only inside CSRF, an oversized body would
surface as a missing token (403), not 413. So LimitBody parses the form
under the cap ahead of CSRF; a successful parse is cached and reused by the
CSRF check and handler. A test asserts an oversized body carrying an
otherwise-valid token still returns 413.

Judgement call (DoD item 3): left WriteTimeout at 60s. It also caps
image-response write time; whether it should rise for large images over slow
links is a load decision for the owner, out of scope here.

make check passes via the local Docker builder gate; hosted CI is down for
an infrastructure reason.

Model: opus-4-8

Implements both HTTP-server hardening items from the audit. **Server timeouts.** Adds `HTTPReadHeaderTimeout` (10s) and `HTTPIdleTimeout` (120s) and wires them onto the server. `ReadHeaderTimeout` bounds the header-read phase on its own — the slowloris dribble that `ReadTimeout` (whole request) does not cover. `IdleTimeout` bounds keep-alive reuse so idle connections cannot accumulate without limit. Server construction moves into a small `newHTTPServer` helper so the timeouts are assertable without binding a listener. **Body limit.** A `LimitBody` middleware caps `POST /` and `POST /generate` bodies at 1 MiB (`MaxFormBytes`) and returns 413 on excess, making the bound chosen rather than resting on `ParseForm`'s incidental 10 MB cap. **What to trip over — ordering against CSRF.** gorilla/csrf (landed on `next`) reads its token from the form body via `PostFormValue`, which swallows a parse error. If the cap lived only inside CSRF, an oversized body would surface as a missing token (403), not 413. So `LimitBody` parses the form under the cap ahead of CSRF; a successful parse is cached and reused by the CSRF check and handler. A test asserts an oversized body carrying an otherwise-valid token still returns 413. **Judgement call (DoD item 3):** left `WriteTimeout` at 60s. It also caps image-response write time; whether it should rise for large images over slow links is a load decision for the owner, out of scope here. `make check` passes via the local Docker builder gate; hosted CI is down for an infrastructure reason. Model: opus-4-8
clawbot added 4 commits 2026-09-21 20:29:18 +02:00
Failing test first: the server-hardening policy requires a slowloris
defense (ReadHeaderTimeout) and a keep-alive bound (IdleTimeout) on the
http.Server, neither of which is set today. The test asserts every
timeout field is wired onto the constructed server.

Model: opus-4-8
Add HTTPReadHeaderTimeout (10s) and HTTPIdleTimeout (120s) constants and
wire them onto the server. ReadHeaderTimeout bounds the header-read phase
specifically, dropping a slowloris client that dribbles headers; ReadTimeout
alone bounds the whole request but not that phase. IdleTimeout bounds
keep-alive reuse so idle connections cannot accumulate without limit.

Server construction moves into newHTTPServer so the timeout configuration
is assertable without binding a listener.

Model: opus-4-8
Failing test first for the per-route body limit. The key regression:
an oversized POST / or POST /generate carrying an otherwise valid CSRF
cookie and token must return 413. CSRF landed on next and reads its token
from the form body, so if the limit ran after CSRF the truncated body
would read as a missing token (403) and if it ran after the handler a
valid token would succeed (303). 413 proves the limit fires first. Two
within-limit tests guard against the limit disturbing a normal request.

Model: opus-4-8
Add a LimitBody middleware that caps the request body at MaxFormBytes
(1 MiB) on POST / and POST /generate and rejects an oversized body with
413. It parses the form under the cap before the CSRF middleware, which
reads its token from the body with PostFormValue and would otherwise see a
truncated body as a missing token (403); a successful parse is cached, so
the CSRF check and handler reuse it. Wired ahead of CSRF in SetupRoutes.

This makes the limit explicit rather than resting on ParseForm's incidental
10 MB cap, which would silently vanish if a handler switched to io.ReadAll
or multipart.

Model: opus-4-8
clawbot added the needs-review label 2026-09-21 20:29:26 +02:00
clawbot self-assigned this 2026-09-21 20:29:26 +02:00
Author
Collaborator

Reviewed against the definition of done and the current next: the ReadHeaderTimeout and IdleTimeout values are set and asserted by tests, the 1 MiB form-POST body limit parses under the cap ahead of the CSRF token read so an oversized body returns 413 (not 403/400), WriteTimeout is flagged and deliberately left at 60s, no existing test was changed, the branch rebases cleanly onto next, and the local Docker builder gate (lint and tests) is green — approving for #92.

Model: opus-4-8

Reviewed against the definition of done and the current `next`: the `ReadHeaderTimeout` and `IdleTimeout` values are set and asserted by tests, the 1 MiB form-POST body limit parses under the cap ahead of the CSRF token read so an oversized body returns 413 (not 403/400), `WriteTimeout` is flagged and deliberately left at 60s, no existing test was changed, the branch rebases cleanly onto `next`, and the local Docker builder gate (lint and tests) is green — approving for https://git.eeqj.de/sneak/pixa/issues/92. Model: opus-4-8
clawbot merged commit 37d49ade11 into next 2026-09-21 20:59:25 +02:00
clawbot deleted branch issue-92-server-hardening 2026-09-21 20:59:26 +02:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/pixa#123