internal/server has no tests: deleting the SecurityHeaders router registration fails nothing #142

New Issue

2026-09-04T01:25:27+02:00

clawbot commented

2026-09-04 01:25:27 +02:00

Found during review of PR #112 (security response headers middleware). Non-blocking there — the PR meets every DoD item of issue #98 — but the gap is real and outlives that PR.

The gap

internal/server/ has no test file at all, and nothing in the tree references SetupRoutes.

The middleware itself is well covered: internal/middleware has table-driven tests, and the dashboard test asserts the headers on a rendered response. But that dashboard test builds its own router and calls the middleware directly. Nothing exercises the production wiring.

Mutation-verified during the #112 review: deleting

router.Use(s.mw.SecurityHeaders())

from SetupRoutes — which silently strips all six security headers from every production response — causes no test to fail. Seven other mutations (dropping each header, weakening CSP two ways, weakening HSTS, weakening Referrer-Policy) were each caught by an on-point assertion. Only the registration itself is unguarded.

Definition of done

internal/server has a test file covering SetupRoutes.
A test asserts that a response served through the router actually built by SetupRoutes carries all six security headers with their expected values (HSTS max-age=31536000; includeSubDomains, CSP with no unsafe-inline/unsafe-eval and with frame-ancestors 'none', X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy).
Mutation check recorded in the PR: removing the router.Use(s.mw.SecurityHeaders()) line makes the new test fail.
Coverage is not limited to one route — assert on at least a non-dashboard path (e.g. /api/v1/status and a 404) so the headers are proven to apply router-wide, not per-handler.
No mocking of the DNS client anywhere in the new tests.
make check and docker build . both green.

Related: issue #110 (other packages with no tests at all) and issue #120 (timeout tests protect the constructor but not the call site) — same shape of defect, different packages. Keep them separate.

Found during review of [PR #112](https://git.eeqj.de/sneak/dnswatcher/pulls/112) (security response headers middleware). Non-blocking there — the PR meets every DoD item of [issue #98](https://git.eeqj.de/sneak/dnswatcher/issues/98) — but the gap is real and outlives that PR. ## The gap `internal/server/` has **no test file at all**, and nothing in the tree references `SetupRoutes`. The middleware itself is well covered: `internal/middleware` has table-driven tests, and the dashboard test asserts the headers on a rendered response. But that dashboard test **builds its own router** and calls the middleware directly. Nothing exercises the production wiring. Mutation-verified during the #112 review: deleting ```go router.Use(s.mw.SecurityHeaders()) ``` from `SetupRoutes` — which silently strips all six security headers from every production response — **causes no test to fail**. Seven other mutations (dropping each header, weakening CSP two ways, weakening HSTS, weakening Referrer-Policy) were each caught by an on-point assertion. Only the registration itself is unguarded. ## Definition of done 1. `internal/server` has a test file covering `SetupRoutes`. 2. A test asserts that a response served through the router actually built by `SetupRoutes` carries all six security headers with their expected values (HSTS `max-age=31536000; includeSubDomains`, CSP with no `unsafe-inline`/`unsafe-eval` and with `frame-ancestors 'none'`, X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy). 3. Mutation check recorded in the PR: removing the `router.Use(s.mw.SecurityHeaders())` line makes the new test fail. 4. Coverage is not limited to one route — assert on at least a non-dashboard path (e.g. `/api/v1/status` and a 404) so the headers are proven to apply router-wide, not per-handler. 5. No mocking of the DNS client anywhere in the new tests. 6. `make check` and `docker build .` both green. Related: [issue #110](https://git.eeqj.de/sneak/dnswatcher/issues/110) (other packages with no tests at all) and [issue #120](https://git.eeqj.de/sneak/dnswatcher/issues/120) (timeout tests protect the constructor but not the call site) — same shape of defect, different packages. Keep them separate.

~~clawbot referenced this issue 2026-09-04 01:25:39 +02:00~~

feat: add security response headers middleware (closes #98) #112

clawbot commented

2026-09-05 11:32:51 +02:00

Closed at sneak's instruction, 2026-09-05: this was opened by an agent running on another machine outside the managed fleet, under superseded rules. Reopen if the content is wanted.

Model: fable-5-1

Closed at sneak's instruction, 2026-09-05: this was opened by an agent running on another machine outside the managed fleet, under superseded rules. Reopen if the content is wanted. Model: fable-5-1

clawbot closed this issue

2026-09-05 11:32:51 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/dnswatcher#142