chore(server): S01 server assembly (server.New + buildMiddleware) (Step 5)

What

Step 5 of the S01 HTTP Server and Routing plan: server assembly. Lands server.New plus the buildMiddleware and deriveProbeAddr helpers in internal/server/. Composes the LabKit v2 httpserver.Server with the S01 middleware chain (#5–#7 (closed)) wrapped around the application *http.ServeMux. The wrapped mux is reached through Server.Mux() so downstream specs can register routes, and *Server is an app.Component via the embedded *httpserver.Server.

No endpoints ship in this MR. The composition root (cmd/artifact-registry/, buildApp, lifecycle integration) is Step 6 (separate MR).

Why

S01 acceptance criteria #1, #2, #3 (closed), #4 (closed), #5, #6 (closed), #7 (closed), #10, #11, #12, #16 (closed), #18 (closed), #19 (closed), #20 (closed) all require an HTTP server with the S01 middleware chain wired against the application mux. Step 5 is the convergence point that wires everything Steps 1–4 produced into the LabKit v2 surface, so Step 6 can stand up the lifecycle and integration test against a real binary.

Spec coverage

Spec: S01 HTTP Server and Routing

Acceptance criteria

#	Criterion	Tests
AC-1	Server starts on configured Address and accepts HTTP connections	TestServer_MiddlewareChain_NosniffPresent (loopback :0 round-trip). Full integration owned by Step 6.
AC-8	Unknown route returns 404 with the standard JSON envelope	TestServer_NotFoundReturnsJSONEnvelope, TestBuildMiddleware_RewritesStdlib404. Primary coverage: Step 3.
AC-13	Request body exceeding MaxBodySize returns 413	TestBuildMiddleware_BodySizeLimitEnforced. Primary coverage: Step 3.
AC-16	X-Content-Type-Options: nosniff on all responses	TestServer_MiddlewareChain_NosniffPresent, TestBuildMiddleware_WrapsMux, TestServer_NotFoundReturnsJSONEnvelope. Primary: Step 3.

ACs #2/#3 (closed)/#4 (closed) (SIGTERM/SIGINT/drain), #5/#6 (closed)/#7 (closed) (health endpoints), #10/#11/#12 (LabKit-owned correlation/access-log/panic-recovery), #18 (closed)/#19 (closed) (HEAD on health + X-Request-ID validation), #20 (closed) (shutdown-budget derivation) are owned by Step 6's composition-root integration test per the plan's AC map. AC #14/#15 are owned by Step 1; AC #17 (closed) by Step 2; AC #21 (closed) by Step 4.

Decisions and additional behaviors (Step 5 scope)

#	Behavior	Tests
D-1	server.New returns a non-nil *Server and exposes the application mux via the Server.Mux() accessor	TestServer_New_HappyPath, TestServer_MuxAcceptsRoutes
D-2	*Server satisfies app.Component via the embedded *httpserver.Server	var _ app.Component = (*Server)(nil) (compile-time assertion at file scope)
D-3	Deps exposes the four fields the plan documents	var _ = Deps{...} (compile-time assertion at file scope)
D-4	buildMiddleware wraps the mux with the S01 chain (#5–#7 (closed)) plus the response interceptor	TestBuildMiddleware_WrapsMux, TestBuildMiddleware_RewritesStdlib404
D-5	deriveProbeAddr mirrors ephemeral ports for tests; falls through to LabKit's default for other input; rejects an application port that collides with LabKit's :9090 probe default via the ErrProbePortCollision sentinel	TestDeriveProbeAddr (11 cases: IPv4/IPv6 loopback, wildcard, fixed ports, empty, malformed, plus three :9090 collision cases asserting errors.Is(err, ErrProbePortCollision))
D-6	server.New rejects a nil deps.Logger at boot so a wiring bug surfaces as a startup panic, not as silently missing access logs (matches NewShutdownDelay's convention)	TestServer_New_PanicsOnNilLogger
D-7	Server.mux is unexported and reachable only through Mux(); an exported field would let callers reassign the pointer (srv.Mux = http.NewServeMux()), silently dropping every later-registered route because the handler chain captures the pointer at construction time	Enforced by Go's visibility rules + the compile-time assertion plus exercise via TestServer_MuxAcceptsRoutes

Notes for reviewers

• LabKit probe-listener spec drift (documented + tracked). The S01 spec was written when LabKit v2 served /-/liveness, /-/readiness, /-/metrics on the application listener. LabKit v2.6.1 (gitlab-org/labkit!402 (merged)) introduced a dedicated probe listener; v2.8.0 in go.mod inherits that split. deriveProbeAddr (internal/server/server.go) adapts so the round-trip test stays deterministic and rejects an application Address that would collide with LabKit's :9090 probe default. The docstring documents the drift; reconciling the spec and adding a server.probe_address ServerConfig field is tracked in #98. No action required for this MR.

• AppSec MR Reviewer findings (all three addressed in 0ca9929). Discussion thread on this MR has the validation per finding. Highlights:

  • A — Server.Mux footgun. Unexported + accessor (Mux() getter). The bot's "security bypass" framing didn't match the failure mode; the actual fault is unreachable routes (replacement mux is dropped because the handler chain captures the original pointer). Defensive fix lands either way.
  • B — Missing nil-Logger guard. server.New now panics on nil with "server.New: deps.Logger is nil", matching NewShutdownDelay's convention. Coverage added in commit 16f4810.
  • C — :9090 port collision. New ErrProbePortCollision sentinel and labkitDefaultProbePort constant; deriveProbeAddr returns (string, error); New plumbs through. Stopgap until #98 moves port-collision validation into Step 1's protovalidate rules.

• Operator-applied plan-file edit. The Status-table backfill in docs/plans/2026-05-20-s01-http-server-and-routing.md (Step 3 → !267 (merged), Step 4 → !265 (merged), Step 5 → !275 (merged)) clears /implement-step's precondition-3 check on this branch — Steps 3 and 4 merged with empty cells.

• Pipeline. The lint:links job is allow_failure: true and may fail transiently on third-party link checks against docs/specs/S01-http-server-and-routing.md (not in this MR's diff); the pipeline status is success overall when this happens.

• Middleware chain order at internal/server/server.go is outer-to-inner: ResponseInterceptor → HeaderHardening(#5) → BodySize(#6) → RoutePattern(#7) → mux. The interceptor sits outermost so stdlib 404/405 plain-text fallbacks are rewritten to the JSON envelope before any other S01 middleware re-touches the response.

Commits

SHA	Subject	Purpose
410664c	test(server): add S01 Step 5 tests and panic-skeleton for server.New	Test floor (panic-skeleton; documented --no-verify carve-out per CLAUDE.md).
78bc7a3	chore(server): S01 server assembly (server.New + buildMiddleware)	Implementation against committed tests.
4b04c98	refactor(server): simplify internal/server tests per code-simplifier pass	Drops redundant helpers; reuses shared package-scope test constants.
e8a72aa	docs(plans): backfill Status table for S01 Steps 3, 4, and 5	Plan Status table reflects current merged-MR state for predecessors and this MR.
b188f8b	chore(server): address /review-branch findings on S01 Step 5	Adds TestDeriveProbeAddr, tightens Deps doc, reorders buildMiddleware lead, documents LabKit probe-port drift with full gitlab-org/labkit!402 cross-project link and v2.6.1 introduction version.
0ca9929	chore(server): address @ai-appsec recommendations on !275	Unexports mux + adds Mux() accessor; adds nil-Logger panic in New; introduces ErrProbePortCollision sentinel + collision-detection branch in deriveProbeAddr. Doc-comments and the runtime error reference follow-up issue #98 via full URL.
16f4810	test(server): add TestServer_New_PanicsOnNilLogger	Closes coverage gap on the nil-Logger panic landed in 0ca9929. Mirrors TestNewShutdownDelay_NilLoggerPanics (shutdown_test.go) for the analogous Step 4 contract.

Test plan

• go build ./... — passes
• go vet ./... — passes
• goimports -l -local gitlab.com/gitlab-org/ops/artifact-registry on changed .go files — clean
• golangci-lint v2.12.0 run ./... — 0 issues
• go test -race -short -count=1 ./... — all packages pass
• All 35 tests in internal/server pass (16 inherited from Steps 3+4, 8 Step 5 originals, 11 TestDeriveProbeAddr subtests).
• AppSec MR Reviewer findings (A, B, C) reproduced manually and addressed with sound fixes; reply posted in MR discussion.

Dependencies

• Plan MR (!247 (merged)) — merged
• Step 1 MR (!254 (merged)) — merged
• Step 2 MR (!261 (merged)) — merged
• Step 3 MR (!267 (merged)) — merged
• Step 4 MR (!265 (merged)) — merged

Follow-ups

• #98 — S01: reconcile spec + add server.probe_address for LabKit v2 dual-listener model. Bundles the spec amendment, the probe_address ServerConfig field, and the constructor cleanup that removes ErrProbePortCollision / labkitDefaultProbePort / the ephemeral-mirror branch of deriveProbeAddr once the field exists.