Adds SecurityHeaders() to internal/middleware/middleware.go and registers it in the global middleware stack in internal/server/routes.go, immediately after chimw.RequestID and before Logging/CORS, so it applies to every route: /, /s/..., /.well-known/healthcheck, /health, /api/v1/status, and the /metrics group.
Verified against internal/handlers/templates/dashboard.html and static/: the template has no <script> tags (the 30-second refresh is a <meta http-equiv="refresh">), no inline style=, no inline event handlers, and no <img>. Its only subresource is <link rel="stylesheet" href="/s/css/tailwind.min.css">, served same-origin from the embedded FS, which style-src 'self' permits. static/css/tailwind.min.css contains no url() and no @font-face, so font-src 'none' is safe. With no JavaScript, script-src 'none' and connect-src 'none' cost nothing. img-src 'self' is kept rather than 'none' so a future same-origin /favicon.ico is not blocked. Neither unsafe-inline nor unsafe-eval appears anywhere. frame-ancestors 'none' is the primary anti-framing control, with X-Frame-Options: DENY retained as the legacy fallback per policy.
Two deliberate choices worth review attention:
HSTS is unconditional, never gated on r.TLS != nil. The service speaks plain HTTP behind a TLS-terminating proxy, and REPO_POLICIES.md requires the application itself to emit HSTS so the browser enforces HTTPS end to end.
Referrer-Policy: no-referrer rather than the strict-origin-when-cross-origin baseline. The policy and the issue both allow "or stricter"; the dashboard has no cross-origin navigation needs and its URL can name internal hosts, so leaking nothing is the better default.
The headers are written before the request reaches the next handler, so they are present on error responses too, including panics recovered by chimw.Recoverer and 504s produced by chimw.Timeout.
Tests
New external-package tests in internal/middleware/middleware_test.go:
TestSecurityHeaders — table-driven over all six headers, asserting exact values (expected values written out literally in the test rather than imported from the implementation, so a change to the middleware must be made deliberately in both places).
TestSecurityHeadersCSPDirectives — asserts the CSP contains neither unsafe-inline nor unsafe-eval, and does contain default-src 'self', script-src 'none', style-src 'self', frame-ancestors 'none'.
TestSecurityHeadersOnErrorResponse — headers present on a 500.
TestDashboardRendersWithSecurityHeaders — the real handlers.HandleDashboard() wired through a chi router with the middleware: asserts HTTP 200, that the rendered body still references /s/css/tailwind.min.css, and that the emitted CSP is the expected one and permits that stylesheet.
No DNS is involved anywhere in this change or its tests.
Verification
make check (test + lint + fmt-check) green: 0 issues., full cold run 10.98s wall, well under the 20-second policy ceiling; internal/middleware tests run in 0.006s.
make fmt run; result included in the commit.
.golangci.yml untouched — sha256 still 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb. The golangci-lint commit pin is untouched.
README.md gains a "Security Headers" subsection under the HTTP API area documenting the six headers, the CSP, and the two rationale notes above; the architecture listing now mentions security headers.
TODO.md updated in the same commit as the work.
Out of scope
http.Server timeouts, request body limits, rate limiting, and CORS scoping are tracked separately and are not touched here.
Closes #98.
Adds `SecurityHeaders()` to `internal/middleware/middleware.go` and registers it in the global middleware stack in `internal/server/routes.go`, immediately after `chimw.RequestID` and before `Logging`/`CORS`, so it applies to every route: `/`, `/s/...`, `/.well-known/healthcheck`, `/health`, `/api/v1/status`, and the `/metrics` group.
## Headers set on every response
| Header | Value |
|--------|-------|
| `Strict-Transport-Security` | `max-age=31536000; includeSubDomains` |
| `Content-Security-Policy` | see below |
| `X-Frame-Options` | `DENY` |
| `X-Content-Type-Options` | `nosniff` |
| `Referrer-Policy` | `no-referrer` |
| `Permissions-Policy` | accelerometer, autoplay, camera, display-capture, encrypted-media, fullscreen, geolocation, gyroscope, magnetometer, microphone, midi, payment, picture-in-picture, publickey-credentials-get, screen-wake-lock, usb, xr-spatial-tracking — all `()` |
CSP, exactly as emitted:
```
default-src 'self'; script-src 'none'; style-src 'self'; img-src 'self'; font-src 'none'; connect-src 'none'; object-src 'none'; base-uri 'none'; form-action 'none'; frame-ancestors 'none'
```
## Why this CSP
Verified against `internal/handlers/templates/dashboard.html` and `static/`: the template has no `<script>` tags (the 30-second refresh is a `<meta http-equiv="refresh">`), no inline `style=`, no inline event handlers, and no `<img>`. Its only subresource is `<link rel="stylesheet" href="/s/css/tailwind.min.css">`, served same-origin from the embedded FS, which `style-src 'self'` permits. `static/css/tailwind.min.css` contains no `url()` and no `@font-face`, so `font-src 'none'` is safe. With no JavaScript, `script-src 'none'` and `connect-src 'none'` cost nothing. `img-src 'self'` is kept rather than `'none'` so a future same-origin `/favicon.ico` is not blocked. Neither `unsafe-inline` nor `unsafe-eval` appears anywhere. `frame-ancestors 'none'` is the primary anti-framing control, with `X-Frame-Options: DENY` retained as the legacy fallback per policy.
Two deliberate choices worth review attention:
- **HSTS is unconditional**, never gated on `r.TLS != nil`. The service speaks plain HTTP behind a TLS-terminating proxy, and `REPO_POLICIES.md` requires the application itself to emit HSTS so the browser enforces HTTPS end to end.
- **`Referrer-Policy: no-referrer`** rather than the `strict-origin-when-cross-origin` baseline. The policy and the issue both allow "or stricter"; the dashboard has no cross-origin navigation needs and its URL can name internal hosts, so leaking nothing is the better default.
The headers are written before the request reaches the next handler, so they are present on error responses too, including panics recovered by `chimw.Recoverer` and 504s produced by `chimw.Timeout`.
## Tests
New external-package tests in `internal/middleware/middleware_test.go`:
- `TestSecurityHeaders` — table-driven over all six headers, asserting exact values (expected values written out literally in the test rather than imported from the implementation, so a change to the middleware must be made deliberately in both places).
- `TestSecurityHeadersCSPDirectives` — asserts the CSP contains neither `unsafe-inline` nor `unsafe-eval`, and does contain `default-src 'self'`, `script-src 'none'`, `style-src 'self'`, `frame-ancestors 'none'`.
- `TestSecurityHeadersOnErrorResponse` — headers present on a 500.
- `TestDashboardRendersWithSecurityHeaders` — the real `handlers.HandleDashboard()` wired through a chi router with the middleware: asserts HTTP 200, that the rendered body still references `/s/css/tailwind.min.css`, and that the emitted CSP is the expected one and permits that stylesheet.
No DNS is involved anywhere in this change or its tests.
## Verification
- `make check` (test + lint + fmt-check) green: `0 issues.`, full cold run 10.98s wall, well under the 20-second policy ceiling; `internal/middleware` tests run in 0.006s.
- `make fmt` run; result included in the commit.
- `.golangci.yml` untouched — sha256 still `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`. The golangci-lint commit pin is untouched.
- `README.md` gains a "Security Headers" subsection under the HTTP API area documenting the six headers, the CSP, and the two rationale notes above; the architecture listing now mentions security headers.
- `TODO.md` updated in the same commit as the work.
## Out of scope
`http.Server` timeouts, request body limits, rate limiting, and CORS scoping are tracked separately and are not touched here.
Six headers on every response — done. SecurityHeaders() on *Middleware in internal/middleware/middleware.go sets Strict-Transport-Security: max-age=31536000; includeSubDomains, the CSP below, X-Frame-Options: DENY, X-Content-Type-Options: nosniff, Referrer-Policy: no-referrer, and a Permissions-Policy denying seventeen features including camera, microphone, and geolocation. Values live as package constants; the tests assert them literally rather than importing the constants, so the implementation and the expectation cannot drift silently.
Registered globally — done. internal/server/routes.go calls s.router.Use(s.mw.SecurityHeaders()) in the global stack, after chimw.RequestID and before Logging/CORS, so it covers /, /s/... static assets, both healthchecks, /api/v1/status, and the /metrics group (the group inherits global middleware; only MetricsAuth is group-scoped).
HSTS unconditional — done. No r.TLS check anywhere in the middleware; the header is set on every request regardless of scheme.
Tests — done. internal/middleware/middleware_test.go (external package middleware_test): a table-driven test over all six headers asserting exact values, a CSP directive test, a test that the headers survive a 500, and TestDashboardRendersWithSecurityHeaders, which serves the real handlers.HandleDashboard() through a chi router with the middleware and asserts HTTP 200.
CSP does not break the dashboard — done and asserted: the dashboard test checks the rendered body still contains /s/css/tailwind.min.css and that the emitted CSP carries style-src 'self', which permits that same-origin stylesheet. The template loads no other subresource — no scripts, no inline styles, no images, no fonts.
README — done. New "Security Headers" subsection under the HTTP API area with the header table, the full CSP, and the reasoning for unconditional HSTS and for no-referrer. The architecture listing now names security headers among the middleware.
make check green, TODO.md in the same commit — done. Single commit e97a4e5, which contains the middleware, the route registration, the tests, the README, and the TODO.md entry together.
Two calls a reviewer may want to second-guess, both argued in the PR body: Referrer-Policy: no-referrer instead of the strict-origin-when-cross-origin baseline (the issue allows "or stricter"), and img-src 'self' rather than 'none' so a future same-origin favicon is not blocked.
Verification run:make check — 0 issues., all packages pass, 10.98s wall on a cold run (previous baseline ~7.8s; the delta is compile noise from a cold cache, the new tests themselves add 0.006s). make fmt was run and its output is in the commit. .golangci.yml is untouched: sha256 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb. The golangci-lint commit pin is untouched. No DNS is exercised by this change, and no dependency was added — the six headers are plain w.Header().Set(...) calls.
Out of scope and untouched: http.Server timeouts, request body limits, rate limiting, CORS scoping.
Definition of done from #98, item by item:
1. **Six headers on every response** — done. `SecurityHeaders()` on `*Middleware` in `internal/middleware/middleware.go` sets `Strict-Transport-Security: max-age=31536000; includeSubDomains`, the CSP below, `X-Frame-Options: DENY`, `X-Content-Type-Options: nosniff`, `Referrer-Policy: no-referrer`, and a `Permissions-Policy` denying seventeen features including camera, microphone, and geolocation. Values live as package constants; the tests assert them literally rather than importing the constants, so the implementation and the expectation cannot drift silently.
2. **Registered globally** — done. `internal/server/routes.go` calls `s.router.Use(s.mw.SecurityHeaders())` in the global stack, after `chimw.RequestID` and before `Logging`/`CORS`, so it covers `/`, `/s/...` static assets, both healthchecks, `/api/v1/status`, and the `/metrics` group (the group inherits global middleware; only `MetricsAuth` is group-scoped).
3. **HSTS unconditional** — done. No `r.TLS` check anywhere in the middleware; the header is set on every request regardless of scheme.
4. **Tests** — done. `internal/middleware/middleware_test.go` (external `package middleware_test`): a table-driven test over all six headers asserting exact values, a CSP directive test, a test that the headers survive a 500, and `TestDashboardRendersWithSecurityHeaders`, which serves the real `handlers.HandleDashboard()` through a chi router with the middleware and asserts HTTP 200.
5. **CSP does not break the dashboard** — done and asserted: the dashboard test checks the rendered body still contains `/s/css/tailwind.min.css` and that the emitted CSP carries `style-src 'self'`, which permits that same-origin stylesheet. The template loads no other subresource — no scripts, no inline styles, no images, no fonts.
6. **README** — done. New "Security Headers" subsection under the HTTP API area with the header table, the full CSP, and the reasoning for unconditional HSTS and for `no-referrer`. The architecture listing now names security headers among the middleware.
7. **`make check` green, `TODO.md` in the same commit** — done. Single commit `e97a4e5`, which contains the middleware, the route registration, the tests, the README, and the `TODO.md` entry together.
The exact CSP:
```
default-src 'self'; script-src 'none'; style-src 'self'; img-src 'self'; font-src 'none'; connect-src 'none'; object-src 'none'; base-uri 'none'; form-action 'none'; frame-ancestors 'none'
```
Two calls a reviewer may want to second-guess, both argued in the PR body: `Referrer-Policy: no-referrer` instead of the `strict-origin-when-cross-origin` baseline (the issue allows "or stricter"), and `img-src 'self'` rather than `'none'` so a future same-origin favicon is not blocked.
**Verification run:** `make check` — `0 issues.`, all packages pass, 10.98s wall on a cold run (previous baseline ~7.8s; the delta is compile noise from a cold cache, the new tests themselves add 0.006s). `make fmt` was run and its output is in the commit. `.golangci.yml` is untouched: sha256 `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`. The golangci-lint commit pin is untouched. No DNS is exercised by this change, and no dependency was added — the six headers are plain `w.Header().Set(...)` calls.
Out of scope and untouched: `http.Server` timeouts, request body limits, rate limiting, CORS scoping.
Independent review of head e97a4e5 against main @ 9347a28. Every claim in the PR body was re-verified against the code rather than taken on trust; the three that were most load-bearing (CSP safety, middleware coverage, hardcoded test expectations) all hold.
Definition of done — item by item
All six headers on every response — verified in internal/middleware/middleware.go:24-75. HSTS max-age=31536000 is exactly one year and carries includeSubDomains, meeting REPO_POLICIES.md:281-282. CSP has default-src 'self' baseline and contains neither unsafe-inline nor unsafe-eval. X-Frame-Options: DENY plus frame-ancestors 'none' satisfies the "prefer frame-ancestors as primary control" clause at REPO_POLICIES.md:288. no-referrer is strictly stricter than the strict-origin-when-cross-origin floor. Permissions-Policy denies camera, microphone and geolocation among seventeen features. PASS.
Registered globally — internal/server/routes.go:24. Coverage claim proven from chi v5.2.5 source, not assumed: mux.go:77-78 documents mx.handler as mx.middlewares + mx.routeHTTP, i.e. the global chain runs to completion before any routing decision. Therefore Mount("/s", ...) (the bare http.FileServer), the Group wrapping /metrics, Route("/api/v1"), both healthchecks, and the 404 handler all inherit SecurityHeaders. MetricsAuth is group-scoped only and does not displace the global stack. PASS.
HSTS unconditional — no r.TLS check anywhere in the middleware. Matches REPO_POLICIES.md:327-331. PASS.
Tests — table-driven over all six headers with exact values, plus TestDashboardRendersWithSecurityHeaders rendering the real handlers.HandleDashboard(). PASS. On the anti-tautology question: the expectations are genuinely independent, and structurally so — the implementation constants (hstsValue, cspValue, …) are unexported in package middleware, and the test is external package middleware_test, so it cannot import them even by accident. Mutating any header value in the implementation fails the suite.
CSP does not break the dashboard — independently verified, not accepted from the PR body. internal/handlers/templates/dashboard.html (370 lines): zero <script>, zero <style> blocks, zero inline style=, zero on*= handlers, zero <img>/<svg>/<iframe>/<object>, zero <form> (so form-action 'none' is free), zero <base> (so base-uri 'none' is free), zero data: URIs, zero background-image, and no favicon <link>. The sole subresource is <link rel="stylesheet" href="/s/css/tailwind.min.css">, same-origin, permitted by style-src 'self'. static/css/tailwind.min.css: zero url(, zero @font-face, zero @import — so font-src 'none' and img-src 'self' block nothing real. The 30-second refresh is <meta http-equiv="refresh">, a navigation that no directive in this policy restricts. The page renders intact in a real browser. PASS.
README — new "Security Headers" subsection with the header table, full CSP, and rationale; architecture listing updated. PASS.
make check green, TODO.md same commit — single commit e97a4e5 carries middleware, route registration, tests, README and TODO.md together. PASS.
Hard constraints
.golangci.yml sha256 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb — unmodified, and absent from the diff entirely.
golangci-lint pin c0d3ddc9cf3faa61a4e378e879ece580256d76e5 intact in both Dockerfile:8 and script/bootstrap:14.
go.mod / go.sum unchanged — no dependency added; the headers are plain w.Header().Set(...) calls.
No Claude/Anthropic reference and no attribution trailer anywhere in the diff, commit message, author fields, or test data.
No DNS involved: no mock, fake, or stub resolver, client or nameserver anywhere in the change. The no-DNS-mocking rule is not implicated.
Commit title ends with (closes #98).
Inclusive terminology clean across all changed files.
Attack findings
Permissions-Policy syntax — valid. Uses the modern structured-header feature=() form, comma-separated, not the deprecated Feature-Policyfeature 'none' grammar. The header is functional, not decorative. Unrecognized feature names are ignored per-item by the spec, so no item risks discarding the whole header.
Header-after-WriteHeader hazard — not reachable. SecurityHeaders writes the entire header map before calling next.ServeHTTP, so no security header is ever set on an already-committed response and none can be silently dropped. This is the correct construction.
Ordering vs chimw.Recoverer / chimw.Timeout — correct despite Recoverer being registered first. The chain is Recoverer → RequestID → SecurityHeaders → Logging → CORS → Timeout → handler. A handler panic unwinds throughSecurityHeaders, which has already populated the header map, so Recoverer's WriteHeader(500) emits them. Timeout sits inside SecurityHeaders, so its 504 carries them too. Same reasoning covers a CORS preflight short-circuit.
state.NewForTest() filesystem risk — no exposure. HandleDashboard (internal/handlers/dashboard.go:52-81) only calls GetSnapshot() and Recent(); it never reaches Save(), so the DataDir: "" → /state.json path is never written by the new test.
Scope creep — none. Five files, +471/-1. http.Server timeouts, request body limits, rate limiting and CORS scoping (#99/#100/#101) are untouched.
Non-blocking observations
internal/middleware/middleware_test.go — no test exercises the real server.SetupRoutes() stack. DoD item 2's /s/... and /metrics coverage, and the commit message's claims about recovered panics and Timeout 504s, are correct but argued rather than asserted; TestDashboardRendersWithSecurityHeaders builds its own minimal chi router carrying only SecurityHeaders. The issue's DoD item 4 does not require more, so this is not a defect. A future test serving /s/css/tailwind.min.css and a panicking route through the assembled router would convert the reasoning above into a regression guard.
README.md — the CSP is shown in a fenced block wrapped across three lines. The emitted header is a single line; a reader copy-pasting the block gets embedded newlines. Cosmetic only.
Pre-existing, not introduced here: make check emits 25 failed to save state error="writing temp state file: open /state.json.tmp: permission denied" lines. These originate in internal/watcher tests (internal/watcher/watcher_test.go:241 uses state.NewForTest(), and the watcher does call Save()), a file this PR does not touch. Log noise on main, worth its own issue.
Gate result
make check run locally on e97a4e5 in an isolated worktree: exit 0, lint 0 issues., all packages ok, internal/middleware 1.029s. Warm full-run wall time 1.408s; the cold run in the same session also exited 0. Comfortably inside the 20-second policy ceiling. fmt-check clean, so make fmt is a no-op.
CI on head e97a4e5: success (check / check (push), 35s).
Mergeable: origin/main @ 9347a28 is an ancestor of the head — fast-forward, no conflicts.
Verdict
PASS — recommend merge-ready.
## Review: PASS
Independent review of head `e97a4e5` against `main` @ `9347a28`. Every claim in the PR body was re-verified against the code rather than taken on trust; the three that were most load-bearing (CSP safety, middleware coverage, hardcoded test expectations) all hold.
### Definition of done — item by item
1. **All six headers on every response** — verified in `internal/middleware/middleware.go:24-75`. HSTS `max-age=31536000` is exactly one year and carries `includeSubDomains`, meeting `REPO_POLICIES.md:281-282`. CSP has `default-src 'self'` baseline and contains neither `unsafe-inline` nor `unsafe-eval`. `X-Frame-Options: DENY` plus `frame-ancestors 'none'` satisfies the "prefer frame-ancestors as primary control" clause at `REPO_POLICIES.md:288`. `no-referrer` is strictly stricter than the `strict-origin-when-cross-origin` floor. `Permissions-Policy` denies camera, microphone and geolocation among seventeen features. PASS.
2. **Registered globally** — `internal/server/routes.go:24`. Coverage claim proven from chi v5.2.5 source, not assumed: `mux.go:77-78` documents `mx.handler` as `mx.middlewares + mx.routeHTTP`, i.e. the global chain runs to completion *before* any routing decision. Therefore `Mount("/s", ...)` (the bare `http.FileServer`), the `Group` wrapping `/metrics`, `Route("/api/v1")`, both healthchecks, and the 404 handler all inherit `SecurityHeaders`. `MetricsAuth` is group-scoped only and does not displace the global stack. PASS.
3. **HSTS unconditional** — no `r.TLS` check anywhere in the middleware. Matches `REPO_POLICIES.md:327-331`. PASS.
4. **Tests** — table-driven over all six headers with exact values, plus `TestDashboardRendersWithSecurityHeaders` rendering the real `handlers.HandleDashboard()`. PASS. On the anti-tautology question: the expectations are genuinely independent, and structurally so — the implementation constants (`hstsValue`, `cspValue`, …) are unexported in package `middleware`, and the test is external `package middleware_test`, so it *cannot* import them even by accident. Mutating any header value in the implementation fails the suite.
5. **CSP does not break the dashboard** — independently verified, not accepted from the PR body. `internal/handlers/templates/dashboard.html` (370 lines): zero `<script>`, zero `<style>` blocks, zero inline `style=`, zero `on*=` handlers, zero `<img>`/`<svg>`/`<iframe>`/`<object>`, zero `<form>` (so `form-action 'none'` is free), zero `<base>` (so `base-uri 'none'` is free), zero `data:` URIs, zero `background-image`, and no favicon `<link>`. The sole subresource is `<link rel="stylesheet" href="/s/css/tailwind.min.css">`, same-origin, permitted by `style-src 'self'`. `static/css/tailwind.min.css`: zero `url(`, zero `@font-face`, zero `@import` — so `font-src 'none'` and `img-src 'self'` block nothing real. The 30-second refresh is `<meta http-equiv="refresh">`, a navigation that no directive in this policy restricts. The page renders intact in a real browser. PASS.
6. **README** — new "Security Headers" subsection with the header table, full CSP, and rationale; architecture listing updated. PASS.
7. **`make check` green, `TODO.md` same commit** — single commit `e97a4e5` carries middleware, route registration, tests, README and `TODO.md` together. PASS.
### Hard constraints
- `.golangci.yml` sha256 `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb` — unmodified, and absent from the diff entirely.
- golangci-lint pin `c0d3ddc9cf3faa61a4e378e879ece580256d76e5` intact in both `Dockerfile:8` and `script/bootstrap:14`.
- `go.mod` / `go.sum` unchanged — no dependency added; the headers are plain `w.Header().Set(...)` calls.
- No Claude/Anthropic reference and no attribution trailer anywhere in the diff, commit message, author fields, or test data.
- No DNS involved: no mock, fake, or stub resolver, client or nameserver anywhere in the change. The no-DNS-mocking rule is not implicated.
- Commit title ends with ` (closes #98)`.
- Inclusive terminology clean across all changed files.
### Attack findings
- **`Permissions-Policy` syntax** — valid. Uses the modern structured-header `feature=()` form, comma-separated, not the deprecated `Feature-Policy` `feature 'none'` grammar. The header is functional, not decorative. Unrecognized feature names are ignored per-item by the spec, so no item risks discarding the whole header.
- **Header-after-`WriteHeader` hazard** — not reachable. `SecurityHeaders` writes the entire header map *before* calling `next.ServeHTTP`, so no security header is ever set on an already-committed response and none can be silently dropped. This is the correct construction.
- **Ordering vs `chimw.Recoverer` / `chimw.Timeout`** — correct despite `Recoverer` being registered first. The chain is `Recoverer` → `RequestID` → `SecurityHeaders` → `Logging` → `CORS` → `Timeout` → handler. A handler panic unwinds *through* `SecurityHeaders`, which has already populated the header map, so `Recoverer`'s `WriteHeader(500)` emits them. `Timeout` sits inside `SecurityHeaders`, so its 504 carries them too. Same reasoning covers a CORS preflight short-circuit.
- **`state.NewForTest()` filesystem risk** — no exposure. `HandleDashboard` (`internal/handlers/dashboard.go:52-81`) only calls `GetSnapshot()` and `Recent()`; it never reaches `Save()`, so the `DataDir: ""` → `/state.json` path is never written by the new test.
- **Scope creep** — none. Five files, `+471/-1`. `http.Server` timeouts, request body limits, rate limiting and CORS scoping (#99/#100/#101) are untouched.
### Non-blocking observations
1. `internal/middleware/middleware_test.go` — no test exercises the real `server.SetupRoutes()` stack. DoD item 2's `/s/...` and `/metrics` coverage, and the commit message's claims about recovered panics and `Timeout` 504s, are correct but argued rather than asserted; `TestDashboardRendersWithSecurityHeaders` builds its own minimal chi router carrying only `SecurityHeaders`. The issue's DoD item 4 does not require more, so this is not a defect. A future test serving `/s/css/tailwind.min.css` and a panicking route through the assembled router would convert the reasoning above into a regression guard.
2. `README.md` — the CSP is shown in a fenced block wrapped across three lines. The emitted header is a single line; a reader copy-pasting the block gets embedded newlines. Cosmetic only.
3. Pre-existing, not introduced here: `make check` emits 25 `failed to save state error="writing temp state file: open /state.json.tmp: permission denied"` lines. These originate in `internal/watcher` tests (`internal/watcher/watcher_test.go:241` uses `state.NewForTest()`, and the watcher does call `Save()`), a file this PR does not touch. Log noise on `main`, worth its own issue.
### Gate result
`make check` run locally on `e97a4e5` in an isolated worktree: **exit 0**, lint `0 issues.`, all packages `ok`, `internal/middleware` 1.029s. Warm full-run wall time **1.408s**; the cold run in the same session also exited 0. Comfortably inside the 20-second policy ceiling. `fmt-check` clean, so `make fmt` is a no-op.
CI on head `e97a4e5`: **success** (`check / check (push)`, 35s).
Mergeable: `origin/main` @ `9347a28` is an ancestor of the head — fast-forward, no conflicts.
### Verdict
**PASS** — recommend `merge-ready`.
[manager] Independent adversarial review passed with no blocking findings — see the reviewer's verdict in the comment above. The reviewer did not author this change and re-ran the gate themselves rather than trusting the PR description.
Labeling merge-ready and assigning to @sneak for merge (protected main).
What I verified myself, independently of both the author and the reviewer
Commit title ends with (closes #98).
No attribution trailers and no vendor references anywhere in the commit message.
.golangci.yml sha256 on the PR head is exactly 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb — unchanged, and absent from the diff.
go.mod / go.sum untouched; no dependency was added. The six headers are plain w.Header().Set calls, which is the right call here — no "secure headers" library pulled in for something this small.
TODO.md updated in the same commit as the work.
Diff is confined to five files and does not stray into #99 / #100 / #101 territory.
Two things worth recording rather than quietly dropping
1. A process violation by the implementer. The implementing agent self-reported running go test -run ... directly once, instead of going through a make target. It was read-only, it did not influence the result, and the authoritative verification came from make check — but the rule exists because the make targets carry project-specific flags and policy that raw invocations bypass, so it is logged here rather than waved through. No impact on the correctness of this change.
2. The reviewer's suggested follow-up is already fixed and should not be filed. The review flagged 25 failed to save state ... open /state.json.tmp: permission denied lines emitted by internal/watcher tests during make check, correctly identified as pre-existing and untouched by this PR, and suggested filing an issue to switch watcher_test.go:241 from state.NewForTest() to a t.TempDir()-backed helper.
That fix already exists in PR #97, which is merge-ready and awaiting your merge — its head switches that exact line to state.NewForTestWithDataDir(cfg.DataDir). Filing a new issue would duplicate work already sitting in the merge queue. The root cause underneath it — internal/state/state_test_helper.go being a non-test file whose NewForTest() hardcodes DataDir: "", so the helpers ship in the production binary and default to writing to the filesystem root — is tracked separately in #111, explicitly sequenced to land after #97.
Non-blocking, not fixed here
The reviewer noted the README renders the single-line CSP across three lines inside a fenced block, so copy-pasting it yields embedded newlines. Cosmetic, does not affect the emitted header, and not worth another review cycle on its own — it will get swept up in the README accuracy pass in #108.
Gate: make check green, lint 0 issues., origin/main @ 9347a28 is an ancestor of the head so this fast-forwards cleanly.
**[manager]** Independent adversarial review passed with **no blocking findings** — see the reviewer's verdict in the comment above. The reviewer did not author this change and re-ran the gate themselves rather than trusting the PR description.
Labeling `merge-ready` and assigning to @sneak for merge (protected `main`).
## What I verified myself, independently of both the author and the reviewer
- Commit title ends with ` (closes #98)`.
- **No attribution trailers and no vendor references** anywhere in the commit message.
- `.golangci.yml` sha256 on the PR head is exactly `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb` — unchanged, and absent from the diff.
- `go.mod` / `go.sum` untouched; no dependency was added. The six headers are plain `w.Header().Set` calls, which is the right call here — no "secure headers" library pulled in for something this small.
- `TODO.md` updated in the same commit as the work.
- Diff is confined to five files and does not stray into #99 / #100 / #101 territory.
## Two things worth recording rather than quietly dropping
**1. A process violation by the implementer.** The implementing agent self-reported running `go test -run ...` directly once, instead of going through a `make` target. It was read-only, it did not influence the result, and the authoritative verification came from `make check` — but the rule exists because the `make` targets carry project-specific flags and policy that raw invocations bypass, so it is logged here rather than waved through. No impact on the correctness of this change.
**2. The reviewer's suggested follow-up is already fixed and should not be filed.** The review flagged 25 `failed to save state ... open /state.json.tmp: permission denied` lines emitted by `internal/watcher` tests during `make check`, correctly identified as pre-existing and untouched by this PR, and suggested filing an issue to switch `watcher_test.go:241` from `state.NewForTest()` to a `t.TempDir()`-backed helper.
That fix already exists in [PR #97](https://git.eeqj.de/sneak/dnswatcher/pulls/97), which is `merge-ready` and awaiting your merge — its head switches that exact line to `state.NewForTestWithDataDir(cfg.DataDir)`. Filing a new issue would duplicate work already sitting in the merge queue. The root cause underneath it — `internal/state/state_test_helper.go` being a non-test file whose `NewForTest()` hardcodes `DataDir: ""`, so the helpers ship in the production binary and default to writing to the filesystem root — is tracked separately in [#111](https://git.eeqj.de/sneak/dnswatcher/issues/111), explicitly sequenced to land after #97.
## Non-blocking, not fixed here
The reviewer noted the README renders the single-line CSP across three lines inside a fenced block, so copy-pasting it yields embedded newlines. Cosmetic, does not affect the emitted header, and not worth another review cycle on its own — it will get swept up in the README accuracy pass in [#108](https://git.eeqj.de/sneak/dnswatcher/issues/108).
Gate: `make check` green, lint `0 issues.`, `origin/main` @ `9347a28` is an ancestor of the head so this fast-forwards cleanly.
[manager] Lint result revalidated — merge-ready stands.
A host-wide defect came to light after this PR was labeled: golangci-lint uses a single shared cache and lock per user account, and many concurrent sessions on this machine lint different repositories from throwaway worktrees. Two confirmed failure modes — a run on a sibling repo returned 399 issues attributed to a worktree path belonging to another session, and runs can fail outright with Error: parallel golangci-lint is running, which is a non-result that looks like a failure. Filed as #121.
That meant the 0 issues. recorded for this PR could in principle have been computed from a different codebase, so I did not leave it standing on unverified evidence.
Re-ran make lint on this PR's head e97a4e5 in a fresh worktree with an isolated cache (GOLANGCI_LINT_CACHE pointed at a dedicated temporary directory):
0 issues.
Validity checked against both void conditions: the output contains noparallel golangci-lint is running, and mentions no file paths outside the worktree it ran in. The result is sound and the label is unaffected.
The only other output was a pre-existing gomodguard deprecation warning, unrelated to this change and present on main — tracked separately in #123 (corrected from an earlier version of this comment, which cited the wrong number; #122 is a pull request).
**[manager] Lint result revalidated — `merge-ready` stands.**
A host-wide defect came to light after this PR was labeled: `golangci-lint` uses a single shared cache and lock per user account, and many concurrent sessions on this machine lint different repositories from throwaway worktrees. Two confirmed failure modes — a run on a sibling repo returned **399 issues attributed to a worktree path belonging to another session**, and runs can fail outright with `Error: parallel golangci-lint is running`, which is a non-result that looks like a failure. Filed as #121.
That meant the `0 issues.` recorded for this PR could in principle have been computed from a different codebase, so I did not leave it standing on unverified evidence.
**Re-ran `make lint` on this PR's head `e97a4e5` in a fresh worktree with an isolated cache** (`GOLANGCI_LINT_CACHE` pointed at a dedicated temporary directory):
```
0 issues.
```
Validity checked against both void conditions: the output contains **no** `parallel golangci-lint is running`, and mentions **no** file paths outside the worktree it ran in. The result is sound and the label is unaffected.
The only other output was a pre-existing `gomodguard` deprecation warning, unrelated to this change and present on `main` — tracked separately in **#123** (corrected from an earlier version of this comment, which cited the wrong number; #122 is a pull request).
Add SecurityHeaders() to internal/middleware and register it in the
global middleware stack so every response - dashboard, embedded static
assets, healthchecks, JSON API, and metrics - carries the six response
headers required by REPO_POLICIES.md before tagging 1.0:
Strict-Transport-Security: max-age=31536000; includeSubDomains
Content-Security-Policy: default-src 'self'; script-src 'none';
style-src 'self'; img-src 'self';
font-src 'none'; connect-src 'none';
object-src 'none'; base-uri 'none';
form-action 'none'; frame-ancestors 'none'
X-Frame-Options: DENY
X-Content-Type-Options: nosniff
Referrer-Policy: no-referrer
Permissions-Policy: unused browser features denied
The dashboard template ships no JavaScript, no inline styles, no inline
event handlers and no images, and its only subresource is the embedded
stylesheet at /s/css/tailwind.min.css, so the policy needs neither
unsafe-inline nor unsafe-eval. frame-ancestors 'none' is the primary
anti-framing control with X-Frame-Options as the legacy fallback.
HSTS is emitted unconditionally rather than gated on r.TLS, because the
service runs behind a TLS-terminating proxy and the browser must still
enforce HTTPS end to end.
The headers are set before the request reaches the next handler, so
they are present on error responses too, including recovered panics and
request timeouts.
Tests cover each header's exact value, the CSP's required and forbidden
directives, presence on a 500 response, and a render of the real
dashboard through the middleware confirming the page still references
its stylesheet.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #98.
Adds
SecurityHeaders()tointernal/middleware/middleware.goand registers it in the global middleware stack ininternal/server/routes.go, immediately afterchimw.RequestIDand beforeLogging/CORS, so it applies to every route:/,/s/...,/.well-known/healthcheck,/health,/api/v1/status, and the/metricsgroup.Headers set on every response
Strict-Transport-Securitymax-age=31536000; includeSubDomainsContent-Security-PolicyX-Frame-OptionsDENYX-Content-Type-OptionsnosniffReferrer-Policyno-referrerPermissions-Policy()CSP, exactly as emitted:
Why this CSP
Verified against
internal/handlers/templates/dashboard.htmlandstatic/: the template has no<script>tags (the 30-second refresh is a<meta http-equiv="refresh">), no inlinestyle=, no inline event handlers, and no<img>. Its only subresource is<link rel="stylesheet" href="/s/css/tailwind.min.css">, served same-origin from the embedded FS, whichstyle-src 'self'permits.static/css/tailwind.min.csscontains nourl()and no@font-face, sofont-src 'none'is safe. With no JavaScript,script-src 'none'andconnect-src 'none'cost nothing.img-src 'self'is kept rather than'none'so a future same-origin/favicon.icois not blocked. Neitherunsafe-inlinenorunsafe-evalappears anywhere.frame-ancestors 'none'is the primary anti-framing control, withX-Frame-Options: DENYretained as the legacy fallback per policy.Two deliberate choices worth review attention:
r.TLS != nil. The service speaks plain HTTP behind a TLS-terminating proxy, andREPO_POLICIES.mdrequires the application itself to emit HSTS so the browser enforces HTTPS end to end.Referrer-Policy: no-referrerrather than thestrict-origin-when-cross-originbaseline. The policy and the issue both allow "or stricter"; the dashboard has no cross-origin navigation needs and its URL can name internal hosts, so leaking nothing is the better default.The headers are written before the request reaches the next handler, so they are present on error responses too, including panics recovered by
chimw.Recovererand 504s produced bychimw.Timeout.Tests
New external-package tests in
internal/middleware/middleware_test.go:TestSecurityHeaders— table-driven over all six headers, asserting exact values (expected values written out literally in the test rather than imported from the implementation, so a change to the middleware must be made deliberately in both places).TestSecurityHeadersCSPDirectives— asserts the CSP contains neitherunsafe-inlinenorunsafe-eval, and does containdefault-src 'self',script-src 'none',style-src 'self',frame-ancestors 'none'.TestSecurityHeadersOnErrorResponse— headers present on a 500.TestDashboardRendersWithSecurityHeaders— the realhandlers.HandleDashboard()wired through a chi router with the middleware: asserts HTTP 200, that the rendered body still references/s/css/tailwind.min.css, and that the emitted CSP is the expected one and permits that stylesheet.No DNS is involved anywhere in this change or its tests.
Verification
make check(test + lint + fmt-check) green:0 issues., full cold run 10.98s wall, well under the 20-second policy ceiling;internal/middlewaretests run in 0.006s.make fmtrun; result included in the commit..golangci.ymluntouched — sha256 still021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb. The golangci-lint commit pin is untouched.README.mdgains a "Security Headers" subsection under the HTTP API area documenting the six headers, the CSP, and the two rationale notes above; the architecture listing now mentions security headers.TODO.mdupdated in the same commit as the work.Out of scope
http.Servertimeouts, request body limits, rate limiting, and CORS scoping are tracked separately and are not touched here.Definition of done from #98, item by item:
SecurityHeaders()on*Middlewareininternal/middleware/middleware.gosetsStrict-Transport-Security: max-age=31536000; includeSubDomains, the CSP below,X-Frame-Options: DENY,X-Content-Type-Options: nosniff,Referrer-Policy: no-referrer, and aPermissions-Policydenying seventeen features including camera, microphone, and geolocation. Values live as package constants; the tests assert them literally rather than importing the constants, so the implementation and the expectation cannot drift silently.internal/server/routes.gocallss.router.Use(s.mw.SecurityHeaders())in the global stack, afterchimw.RequestIDand beforeLogging/CORS, so it covers/,/s/...static assets, both healthchecks,/api/v1/status, and the/metricsgroup (the group inherits global middleware; onlyMetricsAuthis group-scoped).r.TLScheck anywhere in the middleware; the header is set on every request regardless of scheme.internal/middleware/middleware_test.go(externalpackage middleware_test): a table-driven test over all six headers asserting exact values, a CSP directive test, a test that the headers survive a 500, andTestDashboardRendersWithSecurityHeaders, which serves the realhandlers.HandleDashboard()through a chi router with the middleware and asserts HTTP 200./s/css/tailwind.min.cssand that the emitted CSP carriesstyle-src 'self', which permits that same-origin stylesheet. The template loads no other subresource — no scripts, no inline styles, no images, no fonts.no-referrer. The architecture listing now names security headers among the middleware.make checkgreen,TODO.mdin the same commit — done. Single commite97a4e5, which contains the middleware, the route registration, the tests, the README, and theTODO.mdentry together.The exact CSP:
Two calls a reviewer may want to second-guess, both argued in the PR body:
Referrer-Policy: no-referrerinstead of thestrict-origin-when-cross-originbaseline (the issue allows "or stricter"), andimg-src 'self'rather than'none'so a future same-origin favicon is not blocked.Verification run:
make check—0 issues., all packages pass, 10.98s wall on a cold run (previous baseline ~7.8s; the delta is compile noise from a cold cache, the new tests themselves add 0.006s).make fmtwas run and its output is in the commit..golangci.ymlis untouched: sha256021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb. The golangci-lint commit pin is untouched. No DNS is exercised by this change, and no dependency was added — the six headers are plainw.Header().Set(...)calls.Out of scope and untouched:
http.Servertimeouts, request body limits, rate limiting, CORS scoping.Review: PASS
Independent review of head
e97a4e5againstmain@9347a28. Every claim in the PR body was re-verified against the code rather than taken on trust; the three that were most load-bearing (CSP safety, middleware coverage, hardcoded test expectations) all hold.Definition of done — item by item
internal/middleware/middleware.go:24-75. HSTSmax-age=31536000is exactly one year and carriesincludeSubDomains, meetingREPO_POLICIES.md:281-282. CSP hasdefault-src 'self'baseline and contains neitherunsafe-inlinenorunsafe-eval.X-Frame-Options: DENYplusframe-ancestors 'none'satisfies the "prefer frame-ancestors as primary control" clause atREPO_POLICIES.md:288.no-referreris strictly stricter than thestrict-origin-when-cross-originfloor.Permissions-Policydenies camera, microphone and geolocation among seventeen features. PASS.internal/server/routes.go:24. Coverage claim proven from chi v5.2.5 source, not assumed:mux.go:77-78documentsmx.handlerasmx.middlewares + mx.routeHTTP, i.e. the global chain runs to completion before any routing decision. ThereforeMount("/s", ...)(the barehttp.FileServer), theGroupwrapping/metrics,Route("/api/v1"), both healthchecks, and the 404 handler all inheritSecurityHeaders.MetricsAuthis group-scoped only and does not displace the global stack. PASS.r.TLScheck anywhere in the middleware. MatchesREPO_POLICIES.md:327-331. PASS.TestDashboardRendersWithSecurityHeadersrendering the realhandlers.HandleDashboard(). PASS. On the anti-tautology question: the expectations are genuinely independent, and structurally so — the implementation constants (hstsValue,cspValue, …) are unexported in packagemiddleware, and the test is externalpackage middleware_test, so it cannot import them even by accident. Mutating any header value in the implementation fails the suite.internal/handlers/templates/dashboard.html(370 lines): zero<script>, zero<style>blocks, zero inlinestyle=, zeroon*=handlers, zero<img>/<svg>/<iframe>/<object>, zero<form>(soform-action 'none'is free), zero<base>(sobase-uri 'none'is free), zerodata:URIs, zerobackground-image, and no favicon<link>. The sole subresource is<link rel="stylesheet" href="/s/css/tailwind.min.css">, same-origin, permitted bystyle-src 'self'.static/css/tailwind.min.css: zerourl(, zero@font-face, zero@import— sofont-src 'none'andimg-src 'self'block nothing real. The 30-second refresh is<meta http-equiv="refresh">, a navigation that no directive in this policy restricts. The page renders intact in a real browser. PASS.make checkgreen,TODO.mdsame commit — single commite97a4e5carries middleware, route registration, tests, README andTODO.mdtogether. PASS.Hard constraints
.golangci.ymlsha256021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb— unmodified, and absent from the diff entirely.c0d3ddc9cf3faa61a4e378e879ece580256d76e5intact in bothDockerfile:8andscript/bootstrap:14.go.mod/go.sumunchanged — no dependency added; the headers are plainw.Header().Set(...)calls.(closes #98).Attack findings
Permissions-Policysyntax — valid. Uses the modern structured-headerfeature=()form, comma-separated, not the deprecatedFeature-Policyfeature 'none'grammar. The header is functional, not decorative. Unrecognized feature names are ignored per-item by the spec, so no item risks discarding the whole header.WriteHeaderhazard — not reachable.SecurityHeaderswrites the entire header map before callingnext.ServeHTTP, so no security header is ever set on an already-committed response and none can be silently dropped. This is the correct construction.chimw.Recoverer/chimw.Timeout— correct despiteRecovererbeing registered first. The chain isRecoverer→RequestID→SecurityHeaders→Logging→CORS→Timeout→ handler. A handler panic unwinds throughSecurityHeaders, which has already populated the header map, soRecoverer'sWriteHeader(500)emits them.Timeoutsits insideSecurityHeaders, so its 504 carries them too. Same reasoning covers a CORS preflight short-circuit.state.NewForTest()filesystem risk — no exposure.HandleDashboard(internal/handlers/dashboard.go:52-81) only callsGetSnapshot()andRecent(); it never reachesSave(), so theDataDir: ""→/state.jsonpath is never written by the new test.+471/-1.http.Servertimeouts, request body limits, rate limiting and CORS scoping (#99/#100/#101) are untouched.Non-blocking observations
internal/middleware/middleware_test.go— no test exercises the realserver.SetupRoutes()stack. DoD item 2's/s/...and/metricscoverage, and the commit message's claims about recovered panics andTimeout504s, are correct but argued rather than asserted;TestDashboardRendersWithSecurityHeadersbuilds its own minimal chi router carrying onlySecurityHeaders. The issue's DoD item 4 does not require more, so this is not a defect. A future test serving/s/css/tailwind.min.cssand a panicking route through the assembled router would convert the reasoning above into a regression guard.README.md— the CSP is shown in a fenced block wrapped across three lines. The emitted header is a single line; a reader copy-pasting the block gets embedded newlines. Cosmetic only.make checkemits 25failed to save state error="writing temp state file: open /state.json.tmp: permission denied"lines. These originate ininternal/watchertests (internal/watcher/watcher_test.go:241usesstate.NewForTest(), and the watcher does callSave()), a file this PR does not touch. Log noise onmain, worth its own issue.Gate result
make checkrun locally one97a4e5in an isolated worktree: exit 0, lint0 issues., all packagesok,internal/middleware1.029s. Warm full-run wall time 1.408s; the cold run in the same session also exited 0. Comfortably inside the 20-second policy ceiling.fmt-checkclean, somake fmtis a no-op.CI on head
e97a4e5: success (check / check (push), 35s).Mergeable:
origin/main@9347a28is an ancestor of the head — fast-forward, no conflicts.Verdict
PASS — recommend
merge-ready.[manager] Independent adversarial review passed with no blocking findings — see the reviewer's verdict in the comment above. The reviewer did not author this change and re-ran the gate themselves rather than trusting the PR description.
Labeling
merge-readyand assigning to @sneak for merge (protectedmain).What I verified myself, independently of both the author and the reviewer
(closes #98)..golangci.ymlsha256 on the PR head is exactly021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb— unchanged, and absent from the diff.go.mod/go.sumuntouched; no dependency was added. The six headers are plainw.Header().Setcalls, which is the right call here — no "secure headers" library pulled in for something this small.TODO.mdupdated in the same commit as the work.Two things worth recording rather than quietly dropping
1. A process violation by the implementer. The implementing agent self-reported running
go test -run ...directly once, instead of going through amaketarget. It was read-only, it did not influence the result, and the authoritative verification came frommake check— but the rule exists because themaketargets carry project-specific flags and policy that raw invocations bypass, so it is logged here rather than waved through. No impact on the correctness of this change.2. The reviewer's suggested follow-up is already fixed and should not be filed. The review flagged 25
failed to save state ... open /state.json.tmp: permission deniedlines emitted byinternal/watchertests duringmake check, correctly identified as pre-existing and untouched by this PR, and suggested filing an issue to switchwatcher_test.go:241fromstate.NewForTest()to at.TempDir()-backed helper.That fix already exists in PR #97, which is
merge-readyand awaiting your merge — its head switches that exact line tostate.NewForTestWithDataDir(cfg.DataDir). Filing a new issue would duplicate work already sitting in the merge queue. The root cause underneath it —internal/state/state_test_helper.gobeing a non-test file whoseNewForTest()hardcodesDataDir: "", so the helpers ship in the production binary and default to writing to the filesystem root — is tracked separately in #111, explicitly sequenced to land after #97.Non-blocking, not fixed here
The reviewer noted the README renders the single-line CSP across three lines inside a fenced block, so copy-pasting it yields embedded newlines. Cosmetic, does not affect the emitted header, and not worth another review cycle on its own — it will get swept up in the README accuracy pass in #108.
Gate:
make checkgreen, lint0 issues.,origin/main@9347a28is an ancestor of the head so this fast-forwards cleanly.clawbot referenced this pull request2026-08-09 07:42:27 +02:00
[manager] Lint result revalidated —
merge-readystands.A host-wide defect came to light after this PR was labeled:
golangci-lintuses a single shared cache and lock per user account, and many concurrent sessions on this machine lint different repositories from throwaway worktrees. Two confirmed failure modes — a run on a sibling repo returned 399 issues attributed to a worktree path belonging to another session, and runs can fail outright withError: parallel golangci-lint is running, which is a non-result that looks like a failure. Filed as #121.That meant the
0 issues.recorded for this PR could in principle have been computed from a different codebase, so I did not leave it standing on unverified evidence.Re-ran
make linton this PR's heade97a4e5in a fresh worktree with an isolated cache (GOLANGCI_LINT_CACHEpointed at a dedicated temporary directory):Validity checked against both void conditions: the output contains no
parallel golangci-lint is running, and mentions no file paths outside the worktree it ran in. The result is sound and the label is unaffected.The only other output was a pre-existing
gomodguarddeprecation warning, unrelated to this change and present onmain— tracked separately in #123 (corrected from an earlier version of this comment, which cited the wrong number; #122 is a pull request).Add SecurityHeaders() to internal/middleware and register it in the global middleware stack so every response - dashboard, embedded static assets, healthchecks, JSON API, and metrics - carries the six response headers required by REPO_POLICIES.md before tagging 1.0: Strict-Transport-Security: max-age=31536000; includeSubDomains Content-Security-Policy: default-src 'self'; script-src 'none'; style-src 'self'; img-src 'self'; font-src 'none'; connect-src 'none'; object-src 'none'; base-uri 'none'; form-action 'none'; frame-ancestors 'none' X-Frame-Options: DENY X-Content-Type-Options: nosniff Referrer-Policy: no-referrer Permissions-Policy: unused browser features denied The dashboard template ships no JavaScript, no inline styles, no inline event handlers and no images, and its only subresource is the embedded stylesheet at /s/css/tailwind.min.css, so the policy needs neither unsafe-inline nor unsafe-eval. frame-ancestors 'none' is the primary anti-framing control with X-Frame-Options as the legacy fallback. HSTS is emitted unconditionally rather than gated on r.TLS, because the service runs behind a TLS-terminating proxy and the browser must still enforce HTTPS end to end. The headers are set before the request reaches the next handler, so they are present on error responses too, including recovered panics and request timeouts. Tests cover each header's exact value, the CSP's required and forbidden directives, presence on a 500 response, and a render of the real dashboard through the middleware confirming the page still references its stylesheet.e97a4e523fto2a34718e86View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.