server: set all four http.Server socket timeouts (#118)

The http.Server was built with only ReadHeaderTimeout set; the other three
timeouts were zero, which in net/http means no limit, so a peer could hold a
connection open past the header phase, responses had no write deadline, and
keep-alive connections were never reaped. ReadTimeout 15s, WriteTimeout 75s,
and IdleTimeout 120s now join ReadHeaderTimeout 10s as named constants.
WriteTimeout must stay above the 60s chimw.Timeout handler budget, because
net/http arms the write deadline once request headers are read; a test fails
the build if either number moves alone. The server literal moved into
newHTTPServer so the configuration can be asserted without binding a socket
(closes #99)

Model: opus-5

README.md

@@ -182,6 +182,27 @@ dnswatcher exposes a lightweight HTTP API for operational visibility:
 | `GET /api/v1/status`                  | Current monitoring state       |
 | `GET /metrics`                        | Prometheus metrics (optional)  |
 
+#### Server timeouts
+
+The HTTP server sets all four socket-level timeouts. These are compile-time
+constants in `internal/server/server.go`, not configurable via environment
+variables.
+
+| Timeout             | Value | Purpose                                       |
+|---------------------|-------|-----------------------------------------------|
+| `ReadHeaderTimeout` | 10s   | Bounds the request header read (slowloris)    |
+| `ReadTimeout`       | 15s   | Bounds the whole request read, headers + body |
+| `WriteTimeout`      | 75s   | Bounds handler execution plus response flush  |
+| `IdleTimeout`       | 120s  | Reaps idle keep-alive connections             |
+
+These are distinct from the 60s per-request handler budget applied by
+`chimw.Timeout` in `internal/server/routes.go`, which cancels the request
+context but does not touch the socket. `WriteTimeout` is deliberately
+larger than that budget: the write deadline is armed once request headers
+are read, so a smaller value would sever the connection before a handler
+using its full budget could respond. `IdleTimeout` exceeds common
+Prometheus scrape intervals so the scraper reuses its connection.
+
 ---
 
 ## Architecture

TODO.md

@@ -101,6 +101,14 @@ Rationale, Design, TODO, License, Author) if any are still missing.
   so shutdown cannot be extended indefinitely; an `OnStop` context that
   is already expired on entry with nothing outstanding drains quietly
   rather than warning about deliveries that were never abandoned
+- 2026-08-09: `http.Server` now sets all four socket-level timeouts
+  (`ReadTimeout` 15s, `ReadHeaderTimeout` 10s, `WriteTimeout` 75s,
+  `IdleTimeout` 120s) as named constants in `internal/server/server.go`,
+  closing the slowloris / unreaped-keep-alive exposure required by
+  `REPO_POLICIES.md` before 1.0; `WriteTimeout` is deliberately greater
+  than the 60s `chimw.Timeout` handler budget so that budget stays
+  reachable, and tests in `internal/server` pin both the non-zero
+  values and that relationship (#99)
 - 2026-08-07: golangci-lint bumped to v2.12.2 (commit-pinned installs
   in `Dockerfile` and `script/bootstrap`); `.golangci.yml` set to the
   org-standard v2-schema config used across the org's repos

internal/server/export_test.go

@@ -0,0 +1,19 @@
+package server
+
+import (
+	"net/http"
+	"time"
+)
+
+// NewHTTPServer exports newHTTPServer for testing.
+func NewHTTPServer(
+	listenAddr string,
+	handler http.Handler,
+) *http.Server {
+	return newHTTPServer(listenAddr, handler)
+}
+
+// RequestTimeout exports the handler execution budget applied by
+// chimw.Timeout in SetupRoutes, so tests can assert the relationship
+// between it and the server's WriteTimeout.
+const RequestTimeout time.Duration = requestTimeout

internal/server/server.go

@@ -33,8 +33,52 @@ type Params struct {
 // shutdownTimeout is how long to wait for graceful shutdown.
 const shutdownTimeout = 30 * time.Second
 
-// readHeaderTimeout is the max duration for reading request headers.
+// Socket-level timeouts for the HTTP server.
-const readHeaderTimeout = 10 * time.Second
+//
+// These bound time spent on the connection itself and are a distinct
+// control from the per-request handler budget enforced by
+// chimw.Timeout(requestTimeout) in routes.go: that one cancels the
+// request context after requestTimeout but never touches the socket,
+// so without the values below a peer can hold a connection open
+// forever (slowloris, unreaped keep-alives).
+//
+// The one hard constraint between the two controls is
+// writeTimeout > requestTimeout. net/http arms the write deadline
+// once the request headers have been read, so on a plaintext
+// connection it covers handler execution AND the response flush. If
+// writeTimeout were <= requestTimeout the server would sever the
+// connection before a handler that legitimately consumed its full
+// budget could emit anything, making the 60s budget unreachable in
+// practice. The margin between them is the response-flush allowance.
+//
+// The only clients of this service are browsers loading the dashboard
+// and a Prometheus scraper; the values are sized for those.
+const (
+	// readHeaderTimeout is the max duration for reading request
+	// headers.
+	readHeaderTimeout = 10 * time.Second
+
+	// readTimeout bounds reading the entire request, headers plus
+	// body. Every route here is a GET with no body, so this only
+	// ever needs to cover headers; the extra 5s over
+	// readHeaderTimeout is slack, not a real allowance, and keeps a
+	// body dribbled one byte at a time from holding the read side
+	// open indefinitely.
+	readTimeout = 15 * time.Second
+
+	// writeTimeout must exceed the requestTimeout handler budget
+	// (60s) per the note above. The 15s difference is the allowance
+	// for flushing a completed response to a slow client.
+	writeTimeout = 75 * time.Second
+
+	// idleTimeout reaps keep-alive connections between requests. It
+	// is deliberately longer than the common Prometheus scrape
+	// intervals (15s/30s/60s) so the scraper reuses its connection
+	// rather than reconnecting every cycle, while a browser tab
+	// left open on the dashboard stops occupying a connection
+	// within two minutes of going quiet.
+	idleTimeout = 120 * time.Second
+)
 
 // Server is the HTTP server.
 type Server struct {
@@ -76,16 +120,29 @@ func New(
 	return srv, nil
 }
 
+// newHTTPServer builds the listening http.Server with every
+// socket-level timeout set. All four are set deliberately: a zero
+// value in net/http means "no limit", not "some default".
+func newHTTPServer(
+	listenAddr string,
+	handler http.Handler,
+) *http.Server {
+	return &http.Server{
+		Addr:              listenAddr,
+		Handler:           handler,
+		ReadTimeout:       readTimeout,
+		ReadHeaderTimeout: readHeaderTimeout,
+		WriteTimeout:      writeTimeout,
+		IdleTimeout:       idleTimeout,
+	}
+}
+
 // Run starts the HTTP server.
 func (s *Server) Run() {
 	s.SetupRoutes()
 
 	listenAddr := fmt.Sprintf(":%d", s.port)
-	s.httpServer = &http.Server{
+	s.httpServer = newHTTPServer(listenAddr, s)
-		Addr:              listenAddr,
-		Handler:           s,
-		ReadHeaderTimeout: readHeaderTimeout,
-	}
 
 	s.log.Info("http server starting", "addr", listenAddr)
 

internal/server/server_test.go

@@ -0,0 +1,112 @@
+package server_test
+
+import (
+	"net/http"
+	"testing"
+
+	"sneak.berlin/go/dnswatcher/internal/server"
+)
+
+// noopHandler stands in for the router; newHTTPServer only stores it.
+func noopHandler() http.Handler {
+	return http.HandlerFunc(
+		func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		},
+	)
+}
+
+// TestHTTPServerTimeoutsAreSet asserts that every socket-level
+// timeout is configured. A zero value in net/http means "no limit",
+// so a refactor that silently drops one of these reintroduces the
+// slowloris / unreaped-keep-alive exposure this guards against.
+//
+// The assertions are on the configured field values only; nothing
+// here measures elapsed time, so the test cannot flake on timing.
+func TestHTTPServerTimeoutsAreSet(t *testing.T) {
+	t.Parallel()
+
+	srv := server.NewHTTPServer(":8080", noopHandler())
+
+	if srv.ReadTimeout <= 0 {
+		t.Errorf(
+			"ReadTimeout must be non-zero, got %v",
+			srv.ReadTimeout,
+		)
+	}
+
+	if srv.ReadHeaderTimeout <= 0 {
+		t.Errorf(
+			"ReadHeaderTimeout must be non-zero, got %v",
+			srv.ReadHeaderTimeout,
+		)
+	}
+
+	if srv.WriteTimeout <= 0 {
+		t.Errorf(
+			"WriteTimeout must be non-zero, got %v",
+			srv.WriteTimeout,
+		)
+	}
+
+	if srv.IdleTimeout <= 0 {
+		t.Errorf(
+			"IdleTimeout must be non-zero, got %v",
+			srv.IdleTimeout,
+		)
+	}
+}
+
+// TestWriteTimeoutExceedsHandlerBudget pins the one relationship the
+// values must satisfy. net/http arms the write deadline once request
+// headers are read, so it covers handler execution plus the response
+// flush. If WriteTimeout were not greater than the chimw.Timeout
+// handler budget, the connection would be severed before a handler
+// that used its full budget could respond, making that budget
+// unreachable.
+func TestWriteTimeoutExceedsHandlerBudget(t *testing.T) {
+	t.Parallel()
+
+	srv := server.NewHTTPServer(":8080", noopHandler())
+
+	if srv.WriteTimeout <= server.RequestTimeout {
+		t.Errorf(
+			"WriteTimeout (%v) must exceed handler budget (%v)",
+			srv.WriteTimeout,
+			server.RequestTimeout,
+		)
+	}
+}
+
+// TestReadTimeoutCoversHeaderTimeout asserts the read deadline for
+// the whole request is at least as long as the header-only deadline;
+// a smaller ReadTimeout would make ReadHeaderTimeout unreachable.
+func TestReadTimeoutCoversHeaderTimeout(t *testing.T) {
+	t.Parallel()
+
+	srv := server.NewHTTPServer(":8080", noopHandler())
+
+	if srv.ReadTimeout < srv.ReadHeaderTimeout {
+		t.Errorf(
+			"ReadTimeout (%v) must be >= ReadHeaderTimeout (%v)",
+			srv.ReadTimeout,
+			srv.ReadHeaderTimeout,
+		)
+	}
+}
+
+// TestHTTPServerAddrAndHandler covers the rest of the constructor so
+// a future edit cannot drop the listen address or the handler.
+func TestHTTPServerAddrAndHandler(t *testing.T) {
+	t.Parallel()
+
+	srv := server.NewHTTPServer(":9999", noopHandler())
+
+	if srv.Addr != ":9999" {
+		t.Errorf("Addr = %q, want %q", srv.Addr, ":9999")
+	}
+
+	if srv.Handler == nil {
+		t.Error("Handler must not be nil")
+	}
+}