Add security response headers middleware (HSTS, CSP, X-Frame-Options, nosniff, Referrer-Policy, Permissions-Policy) #98

Open
opened 2026-08-09 03:35:55 +02:00 by clawbot · 1 comment
Collaborator

REPO_POLICIES.md requires that HTTP/web services be "hardened for production internet exposure before tagging 1.0", and lists six security response headers as mandatory on every response. dnswatcher currently sets none of them.

Current state (audited against origin/main, commit 9347a28)

internal/middleware/middleware.go defines only Logging(), CORS(), and MetricsAuth(). internal/server/routes.go:22-26 registers chimw.Recoverer, chimw.RequestID, Logging(), CORS(), chimw.Timeout — and nothing else. A repo-wide grep for Strict-Transport|Content-Security-Policy|X-Frame-Options|X-Content-Type-Options|Referrer-Policy|Permissions-Policy across all *.go returns zero matches.

This affects every route: the HTML dashboard at /, the embedded static assets at /s/..., the JSON API at /api/v1/status, the healthchecks, and /metrics.

Why a strict CSP is achievable with zero template changes

internal/handlers/templates/dashboard.html (370 lines) has been checked for <style, <script, style=, onclick=, onload=, and javascript:zero matches for all of them. The page uses Tailwind utility classes only, loads exactly one external stylesheet (/s/css/tailwind.min.css, served from the embedded static/ FS), and has no <script> tags at all — the 30-second auto-refresh is a <meta http-equiv="refresh">, not JavaScript.

So the strictest possible policy is reachable today. There is no unsafe-inline / unsafe-eval tradeoff to negotiate.

Definition of done

  1. A new exported middleware (e.g. SecurityHeaders()) on *Middleware in internal/middleware/middleware.go sets all six headers on every response:
    • Strict-Transport-Security: max-age=31536000; includeSubDomains (max-age is at least one year, and includeSubDomains is present).
    • Content-Security-Policy with a restrictive policy that contains neither unsafe-inline nor unsafe-eval. default-src 'self' as the baseline, script-src 'none' (the app ships no JavaScript), and whatever style-src/img-src the dashboard genuinely needs.
    • X-Frame-Options: DENY, and frame-ancestors 'none' in the CSP as the primary control (policy: "Prefer the frame-ancestors CSP directive as the primary control").
    • X-Content-Type-Options: nosniff
    • Referrer-Policy: strict-origin-when-cross-origin or stricter.
    • Permissions-Policy denying the browser features this app does not use (camera, microphone, geolocation, at minimum).
  2. The middleware is registered globally in internal/server/routes.go so it applies to every route including /s/... static assets and /metrics.
  3. HSTS is emitted unconditionally even though the service speaks plain HTTP behind a TLS-terminating proxy — the policy explicitly requires this ("HSTS headers ... must still be set by the application so that the browser enforces HTTPS end-to-end"). Do not gate it on r.TLS != nil.
  4. Table-driven tests in internal/middleware assert every header's exact value on a response, plus at least one test asserting the dashboard route still renders successfully with the CSP applied.
  5. The CSP must not break the dashboard. Verify the rendered page still loads its stylesheet — the CSP has to permit /s/css/tailwind.min.css under style-src 'self'.
  6. README is updated to document the security headers (a short subsection under the HTTP API / dashboard area).
  7. make check is green, and TODO.md is updated in the same commit as the work.

The finishing commit's title must end with (closes #N) referencing this issue.

Out of scope

http.Server timeouts, rate limiting, and CORS scoping are tracked separately — do not fold them into this PR.

`REPO_POLICIES.md` requires that HTTP/web services be "hardened for production internet exposure **before tagging 1.0**", and lists six security response headers as mandatory on every response. dnswatcher currently sets **none** of them. ## Current state (audited against `origin/main`, commit `9347a28`) `internal/middleware/middleware.go` defines only `Logging()`, `CORS()`, and `MetricsAuth()`. `internal/server/routes.go:22-26` registers `chimw.Recoverer`, `chimw.RequestID`, `Logging()`, `CORS()`, `chimw.Timeout` — and nothing else. A repo-wide grep for `Strict-Transport|Content-Security-Policy|X-Frame-Options|X-Content-Type-Options|Referrer-Policy|Permissions-Policy` across all `*.go` returns **zero matches**. This affects every route: the HTML dashboard at `/`, the embedded static assets at `/s/...`, the JSON API at `/api/v1/status`, the healthchecks, and `/metrics`. ## Why a strict CSP is achievable with zero template changes `internal/handlers/templates/dashboard.html` (370 lines) has been checked for `<style`, `<script`, `style=`, `onclick=`, `onload=`, and `javascript:` — **zero matches for all of them**. The page uses Tailwind utility classes only, loads exactly one external stylesheet (`/s/css/tailwind.min.css`, served from the embedded `static/` FS), and has no `<script>` tags at all — the 30-second auto-refresh is a `<meta http-equiv="refresh">`, not JavaScript. So the strictest possible policy is reachable today. There is no `unsafe-inline` / `unsafe-eval` tradeoff to negotiate. ## Definition of done 1. A new exported middleware (e.g. `SecurityHeaders()`) on `*Middleware` in `internal/middleware/middleware.go` sets **all six** headers on every response: - `Strict-Transport-Security: max-age=31536000; includeSubDomains` (max-age is at least one year, and `includeSubDomains` is present). - `Content-Security-Policy` with a restrictive policy that contains **neither** `unsafe-inline` **nor** `unsafe-eval`. `default-src 'self'` as the baseline, `script-src 'none'` (the app ships no JavaScript), and whatever `style-src`/`img-src` the dashboard genuinely needs. - `X-Frame-Options: DENY`, **and** `frame-ancestors 'none'` in the CSP as the primary control (policy: "Prefer the `frame-ancestors` CSP directive as the primary control"). - `X-Content-Type-Options: nosniff` - `Referrer-Policy: strict-origin-when-cross-origin` or stricter. - `Permissions-Policy` denying the browser features this app does not use (camera, microphone, geolocation, at minimum). 2. The middleware is registered globally in `internal/server/routes.go` so it applies to **every** route including `/s/...` static assets and `/metrics`. 3. HSTS is emitted unconditionally even though the service speaks plain HTTP behind a TLS-terminating proxy — the policy explicitly requires this ("HSTS headers ... must still be set by the application so that the browser enforces HTTPS end-to-end"). Do not gate it on `r.TLS != nil`. 4. Table-driven tests in `internal/middleware` assert every header's exact value on a response, plus at least one test asserting the dashboard route still renders successfully with the CSP applied. 5. The CSP must not break the dashboard. Verify the rendered page still loads its stylesheet — the CSP has to permit `/s/css/tailwind.min.css` under `style-src 'self'`. 6. README is updated to document the security headers (a short subsection under the HTTP API / dashboard area). 7. `make check` is green, and `TODO.md` is updated in the same commit as the work. The finishing commit's title must end with ` (closes #N)` referencing this issue. ## Out of scope `http.Server` timeouts, rate limiting, and CORS scoping are tracked separately — do not fold them into this PR.
clawbot added this to the 1.0 milestone 2026-08-09 03:35:55 +02:00
Author
Collaborator

Implementation plan (branch fix/98-security-headers, from origin/main @ 9347a28):

1. internal/middleware/middleware.go — new SecurityHeaders() method on *Middleware

Package-level string constants plus a middleware that sets six headers on every response before calling next.ServeHTTP, so they are emitted regardless of status code (including chimw.Recoverer 500s and chimw.Timeout 504s):

  • Strict-Transport-Security: max-age=31536000; includeSubDomains — emitted unconditionally, never gated on r.TLS != nil, per the policy note about TLS-terminating proxies.
  • 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 denying camera, microphone, geolocation and the rest of the feature set the app does not use.

CSP reasoning, verified against internal/handlers/templates/dashboard.html and static/: the template has zero <script> tags, zero inline style= / event handlers, and no <img>; the only subresource is <link rel="stylesheet" href="/s/css/tailwind.min.css">, which style-src 'self' permits. static/css/tailwind.min.css contains no url() and no @font-face, so font-src 'none' is safe. No JavaScript means script-src 'none' and connect-src 'none' cost nothing. img-src 'self' is kept rather than 'none' so a future /favicon.ico is not blocked. Neither unsafe-inline nor unsafe-eval appears. frame-ancestors 'none' is the primary anti-framing control, with X-Frame-Options: DENY as the legacy fallback. Referrer-Policy: no-referrer is chosen as strictly stricter than strict-origin-when-cross-origin — the dashboard URL can contain internal hostnames and the app has no cross-origin navigation needs.

2. internal/server/routes.go — register s.router.Use(s.mw.SecurityHeaders()) in the global middleware stack (immediately after chimw.RequestID, before CORS), so it covers /, /s/..., the healthchecks, /api/v1/status, and the /metrics group.

3. Tests — new internal/middleware/middleware_test.go (external package middleware_test)

  • Table-driven test asserting each of the six headers' exact values on a response through the middleware.
  • Test asserting the CSP contains neither unsafe-inline nor unsafe-eval and does contain frame-ancestors 'none'.
  • Test that the headers are present on a non-200 response as well.
  • Dashboard test: real handlers.HandleDashboard() wired through a chi router with SecurityHeaders(), asserting HTTP 200, that the body still references /s/css/tailwind.min.css, and that the CSP header is set on the rendered page. No DNS is involved anywhere in this issue.

4. Docs/bookkeeping — a short "Security headers" subsection in the README under the HTTP API area documenting the six headers and the CSP rationale; TODO.md updated in the same commit; make fmt run and make check green before the PR. .golangci.yml and the golangci-lint pin are untouched.

Out of scope and not touched: http.Server timeouts, rate limiting, CORS scoping, request body limits.

Implementation plan (branch `fix/98-security-headers`, from `origin/main` @ `9347a28`): **1. `internal/middleware/middleware.go` — new `SecurityHeaders()` method on `*Middleware`** Package-level string constants plus a middleware that sets six headers on every response before calling `next.ServeHTTP`, so they are emitted regardless of status code (including `chimw.Recoverer` 500s and `chimw.Timeout` 504s): - `Strict-Transport-Security: max-age=31536000; includeSubDomains` — emitted unconditionally, never gated on `r.TLS != nil`, per the policy note about TLS-terminating proxies. - `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` denying camera, microphone, geolocation and the rest of the feature set the app does not use. CSP reasoning, verified against `internal/handlers/templates/dashboard.html` and `static/`: the template has zero `<script>` tags, zero inline `style=` / event handlers, and no `<img>`; the only subresource is `<link rel="stylesheet" href="/s/css/tailwind.min.css">`, which `style-src 'self'` permits. `static/css/tailwind.min.css` contains no `url()` and no `@font-face`, so `font-src 'none'` is safe. No JavaScript means `script-src 'none'` and `connect-src 'none'` cost nothing. `img-src 'self'` is kept rather than `'none'` so a future `/favicon.ico` is not blocked. Neither `unsafe-inline` nor `unsafe-eval` appears. `frame-ancestors 'none'` is the primary anti-framing control, with `X-Frame-Options: DENY` as the legacy fallback. `Referrer-Policy: no-referrer` is chosen as strictly stricter than `strict-origin-when-cross-origin` — the dashboard URL can contain internal hostnames and the app has no cross-origin navigation needs. **2. `internal/server/routes.go`** — register `s.router.Use(s.mw.SecurityHeaders())` in the global middleware stack (immediately after `chimw.RequestID`, before `CORS`), so it covers `/`, `/s/...`, the healthchecks, `/api/v1/status`, and the `/metrics` group. **3. Tests — new `internal/middleware/middleware_test.go` (external `package middleware_test`)** - Table-driven test asserting each of the six headers' exact values on a response through the middleware. - Test asserting the CSP contains neither `unsafe-inline` nor `unsafe-eval` and does contain `frame-ancestors 'none'`. - Test that the headers are present on a non-200 response as well. - Dashboard test: real `handlers.HandleDashboard()` wired through a chi router with `SecurityHeaders()`, asserting HTTP 200, that the body still references `/s/css/tailwind.min.css`, and that the CSP header is set on the rendered page. No DNS is involved anywhere in this issue. **4. Docs/bookkeeping** — a short "Security headers" subsection in the README under the HTTP API area documenting the six headers and the CSP rationale; `TODO.md` updated in the same commit; `make fmt` run and `make check` green before the PR. `.golangci.yml` and the golangci-lint pin are untouched. Out of scope and not touched: `http.Server` timeouts, rate limiting, CORS scoping, request body limits.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/dnswatcher#98