server: assert timeouts on the served http.Server, not just the constructor (closes #120)

The timeout tests called newHTTPServer directly, leaving Run's call
site unguarded: a Run that built its http.Server inline would drop
every timeout with the suite still green. TestRunWiresSocketTimeouts
now wires a Server as cmd/dnswatcher does (minus watcher and resolver,
so no live DNS), drives Run with an unbindable port so it stores its
http.Server and returns without listening, and asserts the served
server carries all four timeouts and both required relationships.
Reverting Run to an inline http.Server without timeouts fails it. The
near-tautological addr/handler test is dropped; the wiring test covers
the handler end to end.

The read-deadline note is corrected: a too-small ReadTimeout does not
make ReadHeaderTimeout unreachable (net/http applies it directly); it
installs an already-expired whole-request deadline. Verified against
the pinned go1.25 net/http.

Model: opus-4-8

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

@@ -23,6 +23,13 @@ Rationale, Design, TODO, License, Author) if any are still missing.
 
 # Completed Steps
 
+- 2026-09-21: server timeout test now drives `Run` and asserts the
+  served `http.Server` carries the timeouts; corrected the inverted
+  `ReadTimeout` rationale note (#120).
+
+- 2026-09-21: `go mod tidy` dropped the redundant `golang.org/x/sync`
+  `// indirect` line so `script/bootstrap` leaves a clean tree (#132)
+
 - 2026-08-10: comment-only corrections to `script/bootstrap`,
   `script/cibuild`, and `Dockerfile.lint`. The `goimports` pin in
   `script/bootstrap` was justified by a claim that `script/fmt-check`
@@ -101,6 +108,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

go.mod

@@ -40,7 +40,6 @@ require (
 	go.yaml.in/yaml/v2 v2.4.2 // indirect
 	go.yaml.in/yaml/v3 v3.0.4 // indirect
 	golang.org/x/mod v0.32.0 // indirect
-	golang.org/x/sync v0.19.0 // indirect
 	golang.org/x/sys v0.41.0 // indirect
 	golang.org/x/text v0.34.0 // indirect
 	golang.org/x/tools v0.41.0 // indirect

internal/server/export_test.go

@@ -0,0 +1,24 @@
+package server
+
+import (
+	"net/http"
+	"time"
+)
+
+// 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
+
+// SetListenPort overrides the port Run binds. A test uses it to hand
+// Run an unbindable port so ListenAndServe fails immediately and Run
+// returns after storing its http.Server.
+func SetListenPort(s *Server, port int) {
+	s.port = port
+}
+
+// HTTPServerOf returns the http.Server that Run built and stored, so a
+// test can inspect the timeouts the running server actually carries.
+func HTTPServerOf(s *Server) *http.Server {
+	return s.httpServer
+}

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.
-const readHeaderTimeout = 10 * time.Second
+// Socket-level timeouts for the HTTP server.
+//
+// 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{
-		Addr:              listenAddr,
-		Handler:           s,
-		ReadHeaderTimeout: readHeaderTimeout,
-	}
+	s.httpServer = newHTTPServer(listenAddr, s)
 
 	s.log.Info("http server starting", "addr", listenAddr)
 

internal/server/server_test.go

@@ -0,0 +1,136 @@
+package server_test
+
+import (
+	"testing"
+
+	"github.com/spf13/viper"
+	"go.uber.org/fx"
+
+	"sneak.berlin/go/dnswatcher/internal/config"
+	"sneak.berlin/go/dnswatcher/internal/globals"
+	"sneak.berlin/go/dnswatcher/internal/handlers"
+	"sneak.berlin/go/dnswatcher/internal/healthcheck"
+	"sneak.berlin/go/dnswatcher/internal/logger"
+	"sneak.berlin/go/dnswatcher/internal/middleware"
+	"sneak.berlin/go/dnswatcher/internal/notify"
+	"sneak.berlin/go/dnswatcher/internal/server"
+	"sneak.berlin/go/dnswatcher/internal/state"
+)
+
+// buildServer wires a *server.Server exactly as cmd/dnswatcher does,
+// minus the watcher/resolver subtree that would touch live DNS. fx
+// builds the object graph but the lifecycle is never started, so no
+// OnStart hook runs and nothing listens or resolves. The caller must
+// first configure viper (config.New reads it), which is also why the
+// caller cannot run in parallel.
+func buildServer(t *testing.T) *server.Server {
+	t.Helper()
+
+	var srv *server.Server
+
+	app := fx.New(
+		fx.NopLogger,
+		fx.Provide(
+			globals.New,
+			logger.New,
+			config.New,
+			state.New,
+			healthcheck.New,
+			notify.New,
+			middleware.New,
+			handlers.New,
+			server.New,
+		),
+		fx.Populate(&srv),
+	)
+
+	err := app.Err()
+	if err != nil {
+		t.Fatalf("building server graph: %v", err)
+	}
+
+	return srv
+}
+
+// TestRunWiresSocketTimeouts pins that the http.Server the running
+// server actually serves — the one Run builds and hands to
+// ListenAndServe — carries every socket-level timeout, plus the two
+// relationships the values must satisfy. Earlier tests asserted these
+// on newHTTPServer directly, which left the call site unguarded: a Run
+// that built its http.Server inline would drop every timeout with the
+// suite still green (https://git.eeqj.de/sneak/dnswatcher/issues/120).
+//
+// Run is driven to completion with an unbindable port: it builds and
+// stores s.httpServer, then ListenAndServe fails at once and Run
+// returns without ever listening. The assertions run in the same
+// goroutine after Run returns, so reading s.httpServer is free of any
+// data race. Nothing here measures elapsed time.
+//
+// On the ReadTimeout >= ReadHeaderTimeout relationship: a smaller
+// ReadTimeout does NOT make the header phase unreachable. net/http's
+// (*Server).readHeaderTimeout applies ReadHeaderTimeout directly, so
+// the header read keeps its full budget. What breaks is the
+// whole-request deadline: once the headers are read, readRequest
+// installs a read deadline of t0+ReadTimeout, which is already in the
+// past when ReadTimeout is the smaller value, severing the request.
+// Verified against the pinned go1.25 net/http (Dockerfile golang
+// 1.25-alpine; go.mod go 1.25.5): src/net/http/server.go readRequest
+// and (*Server).readHeaderTimeout.
+func TestRunWiresSocketTimeouts(t *testing.T) {
+	// Sets an env var and touches viper global state, so like the
+	// config tests it cannot use t.Parallel.
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com")
+
+	srv := buildServer(t)
+	server.SetListenPort(srv, -1)
+
+	srv.Run()
+
+	hs := server.HTTPServerOf(srv)
+	if hs == nil {
+		t.Fatal("Run did not build an http.Server")
+	}
+
+	if hs.ReadTimeout <= 0 {
+		t.Errorf("ReadTimeout must be non-zero, got %v", hs.ReadTimeout)
+	}
+
+	if hs.ReadHeaderTimeout <= 0 {
+		t.Errorf(
+			"ReadHeaderTimeout must be non-zero, got %v",
+			hs.ReadHeaderTimeout,
+		)
+	}
+
+	if hs.WriteTimeout <= 0 {
+		t.Errorf("WriteTimeout must be non-zero, got %v", hs.WriteTimeout)
+	}
+
+	if hs.IdleTimeout <= 0 {
+		t.Errorf("IdleTimeout must be non-zero, got %v", hs.IdleTimeout)
+	}
+
+	if hs.WriteTimeout <= server.RequestTimeout {
+		t.Errorf(
+			"WriteTimeout (%v) must exceed handler budget (%v)",
+			hs.WriteTimeout,
+			server.RequestTimeout,
+		)
+	}
+
+	if hs.ReadTimeout < hs.ReadHeaderTimeout {
+		t.Errorf(
+			"ReadTimeout (%v) must be >= ReadHeaderTimeout (%v)",
+			hs.ReadTimeout,
+			hs.ReadHeaderTimeout,
+		)
+	}
+
+	if hs.Handler != srv {
+		t.Errorf(
+			"Run wired handler %T, want the *server.Server",
+			hs.Handler,
+		)
+	}
+}