From e97a4e523f16dd9a62bbaf05143b9c8ad7901958 Mon Sep 17 00:00:00 2001 From: clawbot Date: Sun, 9 Aug 2026 01:47:47 +0000 Subject: [PATCH] feat: add security response headers middleware (closes #98) 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. --- README.md | 43 +++- TODO.md | 10 + internal/middleware/middleware.go | 85 +++++++ internal/middleware/middleware_test.go | 333 +++++++++++++++++++++++++ internal/server/routes.go | 1 + 5 files changed, 471 insertions(+), 1 deletion(-) create mode 100644 internal/middleware/middleware_test.go diff --git a/README.md b/README.md index 83f9226..b429749 100644 --- a/README.md +++ b/README.md @@ -182,6 +182,46 @@ dnswatcher exposes a lightweight HTTP API for operational visibility: | `GET /api/v1/status` | Current monitoring state | | `GET /metrics` | Prometheus metrics (optional) | +### Security Headers + +Every response — the dashboard, the static assets under `/s/...`, the +healthchecks, the JSON API, and `/metrics` — carries the following +headers, set by a global middleware: + +| 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` | all unused browser features denied | + +The content security policy is: + +``` +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' +``` + +The dashboard ships no JavaScript (the 30-second refresh is a +``), no inline styles, no inline event +handlers, and no images; its only subresource is the embedded stylesheet +at `/s/css/tailwind.min.css`, which `style-src 'self'` permits. The +policy therefore needs neither `unsafe-inline` nor `unsafe-eval`. +`frame-ancestors 'none'` is the primary anti-framing control, with +`X-Frame-Options: DENY` retained as the legacy fallback. + +HSTS is emitted unconditionally, including over plain HTTP. dnswatcher is +expected to run behind a TLS-terminating reverse proxy, and the browser +must still be told to enforce HTTPS end to end, so the header is never +gated on whether the request itself arrived over TLS. + +`Referrer-Policy: no-referrer` is stricter than the +`strict-origin-when-cross-origin` baseline: the dashboard has no +cross-origin navigation needs, and its URL may name internal hosts. + --- ## Architecture @@ -194,7 +234,8 @@ internal/ globals/globals.go Build-time variables (version) logger/logger.go slog structured logging (TTY detection) healthcheck/healthcheck.go Health check service - middleware/middleware.go HTTP middleware (logging, CORS, metrics auth) + middleware/middleware.go HTTP middleware (logging, CORS, security + headers, metrics auth) handlers/handlers.go HTTP request handlers server/ server.go HTTP server lifecycle diff --git a/TODO.md b/TODO.md index bc4c519..8174d50 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,16 @@ confirm make check still passes. # Completed Steps +- 2026-08-09: security response headers middleware + (`SecurityHeaders()` in `internal/middleware/middleware.go`) + registered globally in `internal/server/routes.go`, so HSTS, CSP, + `X-Frame-Options`, `X-Content-Type-Options`, `Referrer-Policy`, and + `Permissions-Policy` are set on every response including `/s/...` and + `/metrics`; the CSP needs no `unsafe-inline`/`unsafe-eval` because the + dashboard ships no JavaScript and no inline styles; HSTS is emitted + unconditionally per policy (TLS-terminating proxy in front). Remaining + 1.0 hardening items — `http.Server` timeouts, request body limits, + rate limiting, CORS scoping — are tracked separately - 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 diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 0a05dd5..03f435e 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -21,6 +21,60 @@ import ( // corsMaxAge is the maximum age for CORS preflight responses. const corsMaxAge = 300 +// Security response header values applied to every response. +// +// The CSP is as strict as the dashboard allows: the 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, which style-src 'self' permits. Neither +// unsafe-inline nor unsafe-eval is used. frame-ancestors 'none' is the +// primary anti-framing control; X-Frame-Options is the legacy fallback. +const ( + // hstsValue is emitted unconditionally, including over plain HTTP, + // because the service runs behind a TLS-terminating proxy and the + // browser must still enforce HTTPS end to end. + hstsValue = "max-age=31536000; includeSubDomains" + + cspValue = "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'" + + frameOptionsValue = "DENY" + + contentTypeOptionsValue = "nosniff" + + // referrerPolicyValue is stricter than the policy minimum of + // strict-origin-when-cross-origin: the dashboard has no + // cross-origin navigation needs and its URL may name internal + // hosts. + referrerPolicyValue = "no-referrer" + + permissionsPolicyValue = "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=()" +) + // Params contains dependencies for Middleware. type Params struct { fx.In @@ -186,6 +240,37 @@ func (m *Middleware) CORS() func(http.Handler) http.Handler { }) } +// SecurityHeaders returns middleware that sets the security response +// headers required for production internet exposure on every response. +// +// The headers are set before the request reaches the next handler so +// that they are present on every response, including panics recovered +// by chi's Recoverer and timeouts produced by chi's Timeout. +func (m *Middleware) SecurityHeaders() func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func( + writer http.ResponseWriter, + request *http.Request, + ) { + header := writer.Header() + header.Set("Strict-Transport-Security", hstsValue) + header.Set("Content-Security-Policy", cspValue) + header.Set("X-Frame-Options", frameOptionsValue) + header.Set( + "X-Content-Type-Options", + contentTypeOptionsValue, + ) + header.Set("Referrer-Policy", referrerPolicyValue) + header.Set( + "Permissions-Policy", + permissionsPolicyValue, + ) + + next.ServeHTTP(writer, request) + }) + } +} + // MetricsAuth returns basic auth middleware for /metrics. func (m *Middleware) MetricsAuth() func(http.Handler) http.Handler { if m.params.Config.MetricsUsername == "" { diff --git a/internal/middleware/middleware_test.go b/internal/middleware/middleware_test.go new file mode 100644 index 0000000..9ccadfe --- /dev/null +++ b/internal/middleware/middleware_test.go @@ -0,0 +1,333 @@ +package middleware_test + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/go-chi/chi/v5" + + "sneak.berlin/go/dnswatcher/internal/config" + "sneak.berlin/go/dnswatcher/internal/globals" + "sneak.berlin/go/dnswatcher/internal/handlers" + "sneak.berlin/go/dnswatcher/internal/logger" + "sneak.berlin/go/dnswatcher/internal/middleware" + "sneak.berlin/go/dnswatcher/internal/notify" + "sneak.berlin/go/dnswatcher/internal/state" +) + +// Expected security header values, spelled out literally so that any +// change to the middleware has to be made deliberately here as well. +const ( + wantHSTS = "max-age=31536000; includeSubDomains" + + wantCSP = "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'" + + wantFrameOptions = "DENY" + + wantContentTypeOptions = "nosniff" + + wantReferrerPolicy = "no-referrer" + + wantPermissionsPolicy = "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=()" +) + +// stylesheetPath is the only subresource the dashboard loads. +const stylesheetPath = "/s/css/tailwind.min.css" + +// newTestLogger builds a logger for direct component construction. +func newTestLogger(t *testing.T) *logger.Logger { + t.Helper() + + glob, err := globals.New(nil) + if err != nil { + t.Fatalf("globals.New: %v", err) + } + + log, err := logger.New(nil, logger.Params{Globals: glob}) + if err != nil { + t.Fatalf("logger.New: %v", err) + } + + return log +} + +// newTestMiddleware builds a Middleware without an fx application. +func newTestMiddleware(t *testing.T) *middleware.Middleware { + t.Helper() + + glob, err := globals.New(nil) + if err != nil { + t.Fatalf("globals.New: %v", err) + } + + mw, err := middleware.New(nil, middleware.Params{ + Logger: newTestLogger(t), + Globals: glob, + Config: &config.Config{}, + }) + if err != nil { + t.Fatalf("middleware.New: %v", err) + } + + return mw +} + +// serveWithSecurityHeaders runs a GET through SecurityHeaders and +// returns the recorded response. +func serveWithSecurityHeaders( + t *testing.T, + target string, + handler http.Handler, +) *httptest.ResponseRecorder { + t.Helper() + + mw := newTestMiddleware(t) + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext( + t.Context(), http.MethodGet, target, nil, + ) + + mw.SecurityHeaders()(handler).ServeHTTP(rec, req) + + return rec +} + +// okHandler writes a trivial 200 response. +func okHandler() http.Handler { + return http.HandlerFunc(func( + writer http.ResponseWriter, + _ *http.Request, + ) { + writer.WriteHeader(http.StatusOK) + }) +} + +func TestSecurityHeaders(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + header string + want string + }{ + { + "hsts", + "Strict-Transport-Security", + wantHSTS, + }, + { + "csp", + "Content-Security-Policy", + wantCSP, + }, + { + "frame options", + "X-Frame-Options", + wantFrameOptions, + }, + { + "content type options", + "X-Content-Type-Options", + wantContentTypeOptions, + }, + { + "referrer policy", + "Referrer-Policy", + wantReferrerPolicy, + }, + { + "permissions policy", + "Permissions-Policy", + wantPermissionsPolicy, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + rec := serveWithSecurityHeaders(t, "/", okHandler()) + + got := rec.Header().Get(tt.header) + if got != tt.want { + t.Errorf( + "%s = %q, want %q", + tt.header, got, tt.want, + ) + } + }) + } +} + +// TestSecurityHeadersCSPDirectives guards the properties the repo +// policy requires of the content security policy itself. +func TestSecurityHeadersCSPDirectives(t *testing.T) { + t.Parallel() + + rec := serveWithSecurityHeaders(t, "/", okHandler()) + csp := rec.Header().Get("Content-Security-Policy") + + forbidden := []string{"unsafe-inline", "unsafe-eval"} + for _, directive := range forbidden { + if strings.Contains(csp, directive) { + t.Errorf("CSP must not contain %q: %q", directive, csp) + } + } + + required := []string{ + "default-src 'self'", + "script-src 'none'", + "style-src 'self'", + "frame-ancestors 'none'", + } + for _, directive := range required { + if !strings.Contains(csp, directive) { + t.Errorf("CSP must contain %q: %q", directive, csp) + } + } +} + +// TestSecurityHeadersOnErrorResponse verifies the headers are emitted +// even when the wrapped handler fails, since they are set before the +// handler runs. +func TestSecurityHeadersOnErrorResponse(t *testing.T) { + t.Parallel() + + failing := http.HandlerFunc(func( + writer http.ResponseWriter, + _ *http.Request, + ) { + http.Error( + writer, + "boom", + http.StatusInternalServerError, + ) + }) + + rec := serveWithSecurityHeaders(t, "/api/v1/status", failing) + + if rec.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want 500", rec.Code) + } + + if got := rec.Header().Get( + "X-Content-Type-Options", + ); got != wantContentTypeOptions { + t.Errorf( + "X-Content-Type-Options = %q, want %q", + got, wantContentTypeOptions, + ) + } + + if got := rec.Header().Get( + "Strict-Transport-Security", + ); got != wantHSTS { + t.Errorf( + "Strict-Transport-Security = %q, want %q", + got, wantHSTS, + ) + } +} + +// newTestHandlers builds real Handlers with empty monitoring state. +func newTestHandlers(t *testing.T) *handlers.Handlers { + t.Helper() + + glob, err := globals.New(nil) + if err != nil { + t.Fatalf("globals.New: %v", err) + } + + log := newTestLogger(t) + + notifier, err := notify.New(nil, notify.Params{ + Logger: log, + Config: &config.Config{}, + }) + if err != nil { + t.Fatalf("notify.New: %v", err) + } + + hnd, err := handlers.New(nil, handlers.Params{ + Logger: log, + Globals: glob, + State: state.NewForTest(), + Notify: notifier, + }) + if err != nil { + t.Fatalf("handlers.New: %v", err) + } + + return hnd +} + +// TestDashboardRendersWithSecurityHeaders renders the real dashboard +// through the middleware and checks that the policy still permits the +// one stylesheet the page loads. +func TestDashboardRendersWithSecurityHeaders(t *testing.T) { + t.Parallel() + + mw := newTestMiddleware(t) + hnd := newTestHandlers(t) + + router := chi.NewRouter() + router.Use(mw.SecurityHeaders()) + router.Get("/", hnd.HandleDashboard()) + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext( + t.Context(), http.MethodGet, "/", nil, + ) + + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rec.Code) + } + + body := rec.Body.String() + if !strings.Contains(body, stylesheetPath) { + t.Errorf("dashboard does not reference %q", stylesheetPath) + } + + if !strings.Contains(body, "dnswatcher") { + t.Errorf("dashboard body looks empty: %d bytes", len(body)) + } + + csp := rec.Header().Get("Content-Security-Policy") + if csp != wantCSP { + t.Errorf("CSP = %q, want %q", csp, wantCSP) + } + + // The stylesheet is same-origin, so style-src 'self' allows it. + if !strings.Contains(csp, "style-src 'self'") { + t.Errorf("CSP would block %q: %q", stylesheetPath, csp) + } +} diff --git a/internal/server/routes.go b/internal/server/routes.go index fa99177..5c71d84 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -21,6 +21,7 @@ func (s *Server) SetupRoutes() { // Global middleware s.router.Use(chimw.Recoverer) s.router.Use(chimw.RequestID) + s.router.Use(s.mw.SecurityHeaders()) s.router.Use(s.mw.Logging()) s.router.Use(s.mw.CORS()) s.router.Use(chimw.Timeout(requestTimeout))