Blocks 1.0: http.Server missing IdleTimeout and ReadHeaderTimeout (slowloris), no request body size limits #92

New Issue

clawbot · 2026-08-09T07:02:42+02:00

clawbot commented

2026-08-09 07:02:42 +02:00

From the audit against the canonical REPO_POLICIES.md. Independently verified against main at 61f42e6.

1. Missing server timeouts. The policy requires:

> ReadTimeout and ReadHeaderTimeout on the http.Server to defend against slowloris attacks. ... IdleTimeout on the http.Server.

internal/server/http.go:16-24 constructs the server with Addr, ReadTimeout (30 s), WriteTimeout (60 s), MaxHeaderBytes (8 KiB), and Handler — and nothing else. Grep for IdleTimeout|ReadHeaderTimeout across internal/ and cmd/ returns zero matches — verified.

This is not covered by the existing settings. ReadTimeout bounds the whole request read but does not specifically bound the header phase, which is the slowloris dribble the policy names. With no IdleTimeout, a keep-alive connection is held open indefinitely — so a trivial number of idle connections can exhaust the accept path on a service explicitly targeting high concurrency.

2. No application-enforced request body limit. The policy requires:

> Maximum request body size enforced on all endpoints (e.g. Go http.MaxBytesReader). Choose a sane default per-route; never accept unbounded input.

Grep for MaxBytesReader returns zero matches. POST / and POST /generate both call r.ParseForm() (internal/handlers/auth.go:38,86) with no wrapper.

Stated honestly: these routes are not literally unbounded — Go's ParseForm applies its own 10 MB cap to URL-encoded bodies. The divergence is that the limit is incidental rather than chosen, and it would silently evaporate if a handler ever switched to io.ReadAll or multipart. Lower severity than item 1, but it belongs in the same pass since both are HTTP-server hardening.

Definition of done

HTTPIdleTimeout and HTTPReadHeaderTimeout constants added alongside the existing ones (internal/server/http.go:10-14) and set on the http.Server. Choose values deliberately — ReadHeaderTimeout should be short (seconds), IdleTimeout longer than typical keep-alive reuse but bounded.
http.MaxBytesReader (or middleware) applies an explicit per-route body limit on POST / and POST /generate; exceeding it returns 413, not a generic 400.
Consider whether WriteTimeout of 60 s is right given large image responses over slow links — flag if it needs raising, but do not change it without saying why.
Tests asserting the configured timeout values and that an oversized body is rejected.
make check green.

Priority

Blocks 1.0.0 for item 1 per the policy's pre-1.0 hardening section. Item 2 is cleanup. internal/server/http.go is untouched by both open PRs.

From the audit against the canonical `REPO_POLICIES.md`. Independently verified against `main` at `61f42e6`. **1. Missing server timeouts.** The policy requires: > `ReadTimeout` and `ReadHeaderTimeout` on the `http.Server` to defend against slowloris attacks. ... `IdleTimeout` on the `http.Server`. `internal/server/http.go:16-24` constructs the server with `Addr`, `ReadTimeout` (30 s), `WriteTimeout` (60 s), `MaxHeaderBytes` (8 KiB), and `Handler` — and nothing else. Grep for `IdleTimeout|ReadHeaderTimeout` across `internal/` and `cmd/` returns zero matches — verified. This is not covered by the existing settings. `ReadTimeout` bounds the whole request read but does not specifically bound the header phase, which is the slowloris dribble the policy names. With no `IdleTimeout`, a keep-alive connection is held open indefinitely — so a trivial number of idle connections can exhaust the accept path on a service explicitly targeting high concurrency. **2. No application-enforced request body limit.** The policy requires: > Maximum request body size enforced on all endpoints (e.g. Go `http.MaxBytesReader`). Choose a sane default per-route; never accept unbounded input. Grep for `MaxBytesReader` returns zero matches. `POST /` and `POST /generate` both call `r.ParseForm()` (`internal/handlers/auth.go:38,86`) with no wrapper. **Stated honestly:** these routes are not literally unbounded — Go's `ParseForm` applies its own 10 MB cap to URL-encoded bodies. The divergence is that the limit is incidental rather than chosen, and it would silently evaporate if a handler ever switched to `io.ReadAll` or multipart. Lower severity than item 1, but it belongs in the same pass since both are HTTP-server hardening. ## Definition of done 1. `HTTPIdleTimeout` and `HTTPReadHeaderTimeout` constants added alongside the existing ones (`internal/server/http.go:10-14`) and set on the `http.Server`. Choose values deliberately — `ReadHeaderTimeout` should be short (seconds), `IdleTimeout` longer than typical keep-alive reuse but bounded. 2. `http.MaxBytesReader` (or middleware) applies an explicit per-route body limit on `POST /` and `POST /generate`; exceeding it returns 413, not a generic 400. 3. Consider whether `WriteTimeout` of 60 s is right given large image responses over slow links — flag if it needs raising, but do not change it without saying why. 4. Tests asserting the configured timeout values and that an oversized body is rejected. 5. `make check` green. ## Priority **Blocks 1.0.0** for item 1 per the policy's pre-1.0 hardening section. Item 2 is cleanup. `internal/server/http.go` is untouched by both open PRs.

clawbot added this to the 1.0.0 milestone 2026-08-09 07:02:42 +02:00

clawbot referenced this issue

2026-08-09 16:10:39 +02:00

Tracking: 1.0.0 sequencing and critical path #103

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/pixa#92