Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
b425692bfd |
@@ -203,6 +203,46 @@ are read, so a smaller value would sever the connection before a handler
|
|||||||
using its full budget could respond. `IdleTimeout` exceeds common
|
using its full budget could respond. `IdleTimeout` exceeds common
|
||||||
Prometheus scrape intervals so the scraper reuses its connection.
|
Prometheus scrape intervals so the scraper reuses its connection.
|
||||||
|
|
||||||
|
### 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
|
||||||
|
`<meta http-equiv="refresh">`), 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
|
## Architecture
|
||||||
@@ -215,7 +255,8 @@ internal/
|
|||||||
globals/globals.go Build-time variables (version)
|
globals/globals.go Build-time variables (version)
|
||||||
logger/logger.go slog structured logging (TTY detection)
|
logger/logger.go slog structured logging (TTY detection)
|
||||||
healthcheck/healthcheck.go Health check service
|
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
|
handlers/handlers.go HTTP request handlers
|
||||||
server/
|
server/
|
||||||
server.go HTTP server lifecycle
|
server.go HTTP server lifecycle
|
||||||
|
|||||||
@@ -23,13 +23,8 @@ Rationale, Design, TODO, License, Author) if any are still missing.
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
- 2026-09-21: server timeout test now drives `Run` and asserts the
|
|
||||||
served `http.Server` carries the timeouts; corrected the inverted
|
|
||||||
`ReadTimeout` rationale note (#120).
|
|
||||||
|
|
||||||
- 2026-09-21: `go mod tidy` dropped the redundant `golang.org/x/sync`
|
- 2026-09-21: `go mod tidy` dropped the redundant `golang.org/x/sync`
|
||||||
`// indirect` line so `script/bootstrap` leaves a clean tree (#132)
|
`// indirect` line so `script/bootstrap` leaves a clean tree (#132)
|
||||||
|
|
||||||
- 2026-08-10: comment-only corrections to `script/bootstrap`,
|
- 2026-08-10: comment-only corrections to `script/bootstrap`,
|
||||||
`script/cibuild`, and `Dockerfile.lint`. The `goimports` pin in
|
`script/cibuild`, and `Dockerfile.lint`. The `goimports` pin in
|
||||||
`script/bootstrap` was justified by a claim that `script/fmt-check`
|
`script/bootstrap` was justified by a claim that `script/fmt-check`
|
||||||
@@ -116,6 +111,16 @@ Rationale, Design, TODO, License, Author) if any are still missing.
|
|||||||
than the 60s `chimw.Timeout` handler budget so that budget stays
|
than the 60s `chimw.Timeout` handler budget so that budget stays
|
||||||
reachable, and tests in `internal/server` pin both the non-zero
|
reachable, and tests in `internal/server` pin both the non-zero
|
||||||
values and that relationship (#99)
|
values and that relationship (#99)
|
||||||
|
- 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
|
- 2026-08-07: golangci-lint bumped to v2.12.2 (commit-pinned installs
|
||||||
in `Dockerfile` and `script/bootstrap`); `.golangci.yml` set to the
|
in `Dockerfile` and `script/bootstrap`); `.golangci.yml` set to the
|
||||||
org-standard v2-schema config used across the org's repos
|
org-standard v2-schema config used across the org's repos
|
||||||
|
|||||||
@@ -21,6 +21,60 @@ import (
|
|||||||
// corsMaxAge is the maximum age for CORS preflight responses.
|
// corsMaxAge is the maximum age for CORS preflight responses.
|
||||||
const corsMaxAge = 300
|
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.
|
// Params contains dependencies for Middleware.
|
||||||
type Params struct {
|
type Params struct {
|
||||||
fx.In
|
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.
|
// MetricsAuth returns basic auth middleware for /metrics.
|
||||||
func (m *Middleware) MetricsAuth() func(http.Handler) http.Handler {
|
func (m *Middleware) MetricsAuth() func(http.Handler) http.Handler {
|
||||||
if m.params.Config.MetricsUsername == "" {
|
if m.params.Config.MetricsUsername == "" {
|
||||||
|
|||||||
@@ -0,0 +1,334 @@
|
|||||||
|
package middleware_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/go-chi/chi/v5"
|
||||||
|
"go.uber.org/fx/fxtest"
|
||||||
|
|
||||||
|
"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(fxtest.NewLifecycle(t), 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)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -5,20 +5,15 @@ import (
|
|||||||
"time"
|
"time"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// NewHTTPServer exports newHTTPServer for testing.
|
||||||
|
func NewHTTPServer(
|
||||||
|
listenAddr string,
|
||||||
|
handler http.Handler,
|
||||||
|
) *http.Server {
|
||||||
|
return newHTTPServer(listenAddr, handler)
|
||||||
|
}
|
||||||
|
|
||||||
// RequestTimeout exports the handler execution budget applied by
|
// RequestTimeout exports the handler execution budget applied by
|
||||||
// chimw.Timeout in SetupRoutes, so tests can assert the relationship
|
// chimw.Timeout in SetupRoutes, so tests can assert the relationship
|
||||||
// between it and the server's WriteTimeout.
|
// between it and the server's WriteTimeout.
|
||||||
const RequestTimeout time.Duration = requestTimeout
|
const RequestTimeout time.Duration = requestTimeout
|
||||||
|
|
||||||
// SetListenPort overrides the port Run binds. A test uses it to hand
|
|
||||||
// Run an unbindable port so ListenAndServe fails immediately and Run
|
|
||||||
// returns after storing its http.Server.
|
|
||||||
func SetListenPort(s *Server, port int) {
|
|
||||||
s.port = port
|
|
||||||
}
|
|
||||||
|
|
||||||
// HTTPServerOf returns the http.Server that Run built and stored, so a
|
|
||||||
// test can inspect the timeouts the running server actually carries.
|
|
||||||
func HTTPServerOf(s *Server) *http.Server {
|
|
||||||
return s.httpServer
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -21,6 +21,7 @@ func (s *Server) SetupRoutes() {
|
|||||||
// Global middleware
|
// Global middleware
|
||||||
s.router.Use(chimw.Recoverer)
|
s.router.Use(chimw.Recoverer)
|
||||||
s.router.Use(chimw.RequestID)
|
s.router.Use(chimw.RequestID)
|
||||||
|
s.router.Use(s.mw.SecurityHeaders())
|
||||||
s.router.Use(s.mw.Logging())
|
s.router.Use(s.mw.Logging())
|
||||||
s.router.Use(s.mw.CORS())
|
s.router.Use(s.mw.CORS())
|
||||||
s.router.Use(chimw.Timeout(requestTimeout))
|
s.router.Use(chimw.Timeout(requestTimeout))
|
||||||
|
|||||||
+79
-103
@@ -1,136 +1,112 @@
|
|||||||
package server_test
|
package server_test
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"net/http"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/spf13/viper"
|
|
||||||
"go.uber.org/fx"
|
|
||||||
|
|
||||||
"sneak.berlin/go/dnswatcher/internal/config"
|
|
||||||
"sneak.berlin/go/dnswatcher/internal/globals"
|
|
||||||
"sneak.berlin/go/dnswatcher/internal/handlers"
|
|
||||||
"sneak.berlin/go/dnswatcher/internal/healthcheck"
|
|
||||||
"sneak.berlin/go/dnswatcher/internal/logger"
|
|
||||||
"sneak.berlin/go/dnswatcher/internal/middleware"
|
|
||||||
"sneak.berlin/go/dnswatcher/internal/notify"
|
|
||||||
"sneak.berlin/go/dnswatcher/internal/server"
|
"sneak.berlin/go/dnswatcher/internal/server"
|
||||||
"sneak.berlin/go/dnswatcher/internal/state"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// buildServer wires a *server.Server exactly as cmd/dnswatcher does,
|
// noopHandler stands in for the router; newHTTPServer only stores it.
|
||||||
// minus the watcher/resolver subtree that would touch live DNS. fx
|
func noopHandler() http.Handler {
|
||||||
// builds the object graph but the lifecycle is never started, so no
|
return http.HandlerFunc(
|
||||||
// OnStart hook runs and nothing listens or resolves. The caller must
|
func(w http.ResponseWriter, _ *http.Request) {
|
||||||
// first configure viper (config.New reads it), which is also why the
|
w.WriteHeader(http.StatusOK)
|
||||||
// caller cannot run in parallel.
|
},
|
||||||
func buildServer(t *testing.T) *server.Server {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
var srv *server.Server
|
|
||||||
|
|
||||||
app := fx.New(
|
|
||||||
fx.NopLogger,
|
|
||||||
fx.Provide(
|
|
||||||
globals.New,
|
|
||||||
logger.New,
|
|
||||||
config.New,
|
|
||||||
state.New,
|
|
||||||
healthcheck.New,
|
|
||||||
notify.New,
|
|
||||||
middleware.New,
|
|
||||||
handlers.New,
|
|
||||||
server.New,
|
|
||||||
),
|
|
||||||
fx.Populate(&srv),
|
|
||||||
)
|
)
|
||||||
|
|
||||||
err := app.Err()
|
|
||||||
if err != nil {
|
|
||||||
t.Fatalf("building server graph: %v", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
return srv
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestRunWiresSocketTimeouts pins that the http.Server the running
|
// TestHTTPServerTimeoutsAreSet asserts that every socket-level
|
||||||
// server actually serves — the one Run builds and hands to
|
// timeout is configured. A zero value in net/http means "no limit",
|
||||||
// ListenAndServe — carries every socket-level timeout, plus the two
|
// so a refactor that silently drops one of these reintroduces the
|
||||||
// relationships the values must satisfy. Earlier tests asserted these
|
// slowloris / unreaped-keep-alive exposure this guards against.
|
||||||
// on newHTTPServer directly, which left the call site unguarded: a Run
|
|
||||||
// that built its http.Server inline would drop every timeout with the
|
|
||||||
// suite still green (https://git.eeqj.de/sneak/dnswatcher/issues/120).
|
|
||||||
//
|
//
|
||||||
// Run is driven to completion with an unbindable port: it builds and
|
// The assertions are on the configured field values only; nothing
|
||||||
// stores s.httpServer, then ListenAndServe fails at once and Run
|
// here measures elapsed time, so the test cannot flake on timing.
|
||||||
// returns without ever listening. The assertions run in the same
|
func TestHTTPServerTimeoutsAreSet(t *testing.T) {
|
||||||
// goroutine after Run returns, so reading s.httpServer is free of any
|
t.Parallel()
|
||||||
// data race. Nothing here measures elapsed time.
|
|
||||||
//
|
|
||||||
// On the ReadTimeout >= ReadHeaderTimeout relationship: a smaller
|
|
||||||
// ReadTimeout does NOT make the header phase unreachable. net/http's
|
|
||||||
// (*Server).readHeaderTimeout applies ReadHeaderTimeout directly, so
|
|
||||||
// the header read keeps its full budget. What breaks is the
|
|
||||||
// whole-request deadline: once the headers are read, readRequest
|
|
||||||
// installs a read deadline of t0+ReadTimeout, which is already in the
|
|
||||||
// past when ReadTimeout is the smaller value, severing the request.
|
|
||||||
// Verified against the pinned go1.25 net/http (Dockerfile golang
|
|
||||||
// 1.25-alpine; go.mod go 1.25.5): src/net/http/server.go readRequest
|
|
||||||
// and (*Server).readHeaderTimeout.
|
|
||||||
func TestRunWiresSocketTimeouts(t *testing.T) {
|
|
||||||
// Sets an env var and touches viper global state, so like the
|
|
||||||
// config tests it cannot use t.Parallel.
|
|
||||||
viper.Reset()
|
|
||||||
t.Setenv("DNSWATCHER_TARGETS", "example.com")
|
|
||||||
|
|
||||||
srv := buildServer(t)
|
srv := server.NewHTTPServer(":8080", noopHandler())
|
||||||
server.SetListenPort(srv, -1)
|
|
||||||
|
|
||||||
srv.Run()
|
if srv.ReadTimeout <= 0 {
|
||||||
|
|
||||||
hs := server.HTTPServerOf(srv)
|
|
||||||
if hs == nil {
|
|
||||||
t.Fatal("Run did not build an http.Server")
|
|
||||||
}
|
|
||||||
|
|
||||||
if hs.ReadTimeout <= 0 {
|
|
||||||
t.Errorf("ReadTimeout must be non-zero, got %v", hs.ReadTimeout)
|
|
||||||
}
|
|
||||||
|
|
||||||
if hs.ReadHeaderTimeout <= 0 {
|
|
||||||
t.Errorf(
|
t.Errorf(
|
||||||
"ReadHeaderTimeout must be non-zero, got %v",
|
"ReadTimeout must be non-zero, got %v",
|
||||||
hs.ReadHeaderTimeout,
|
srv.ReadTimeout,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
if hs.WriteTimeout <= 0 {
|
if srv.ReadHeaderTimeout <= 0 {
|
||||||
t.Errorf("WriteTimeout must be non-zero, got %v", hs.WriteTimeout)
|
t.Errorf(
|
||||||
|
"ReadHeaderTimeout must be non-zero, got %v",
|
||||||
|
srv.ReadHeaderTimeout,
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
if hs.IdleTimeout <= 0 {
|
if srv.WriteTimeout <= 0 {
|
||||||
t.Errorf("IdleTimeout must be non-zero, got %v", hs.IdleTimeout)
|
t.Errorf(
|
||||||
|
"WriteTimeout must be non-zero, got %v",
|
||||||
|
srv.WriteTimeout,
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
if hs.WriteTimeout <= server.RequestTimeout {
|
if srv.IdleTimeout <= 0 {
|
||||||
|
t.Errorf(
|
||||||
|
"IdleTimeout must be non-zero, got %v",
|
||||||
|
srv.IdleTimeout,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestWriteTimeoutExceedsHandlerBudget pins the one relationship the
|
||||||
|
// values must satisfy. net/http arms the write deadline once request
|
||||||
|
// headers are read, so it covers handler execution plus the response
|
||||||
|
// flush. If WriteTimeout were not greater than the chimw.Timeout
|
||||||
|
// handler budget, the connection would be severed before a handler
|
||||||
|
// that used its full budget could respond, making that budget
|
||||||
|
// unreachable.
|
||||||
|
func TestWriteTimeoutExceedsHandlerBudget(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
srv := server.NewHTTPServer(":8080", noopHandler())
|
||||||
|
|
||||||
|
if srv.WriteTimeout <= server.RequestTimeout {
|
||||||
t.Errorf(
|
t.Errorf(
|
||||||
"WriteTimeout (%v) must exceed handler budget (%v)",
|
"WriteTimeout (%v) must exceed handler budget (%v)",
|
||||||
hs.WriteTimeout,
|
srv.WriteTimeout,
|
||||||
server.RequestTimeout,
|
server.RequestTimeout,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if hs.ReadTimeout < hs.ReadHeaderTimeout {
|
// TestReadTimeoutCoversHeaderTimeout asserts the read deadline for
|
||||||
|
// the whole request is at least as long as the header-only deadline;
|
||||||
|
// a smaller ReadTimeout would make ReadHeaderTimeout unreachable.
|
||||||
|
func TestReadTimeoutCoversHeaderTimeout(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
srv := server.NewHTTPServer(":8080", noopHandler())
|
||||||
|
|
||||||
|
if srv.ReadTimeout < srv.ReadHeaderTimeout {
|
||||||
t.Errorf(
|
t.Errorf(
|
||||||
"ReadTimeout (%v) must be >= ReadHeaderTimeout (%v)",
|
"ReadTimeout (%v) must be >= ReadHeaderTimeout (%v)",
|
||||||
hs.ReadTimeout,
|
srv.ReadTimeout,
|
||||||
hs.ReadHeaderTimeout,
|
srv.ReadHeaderTimeout,
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
if hs.Handler != srv {
|
|
||||||
t.Errorf(
|
|
||||||
"Run wired handler %T, want the *server.Server",
|
|
||||||
hs.Handler,
|
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestHTTPServerAddrAndHandler covers the rest of the constructor so
|
||||||
|
// a future edit cannot drop the listen address or the handler.
|
||||||
|
func TestHTTPServerAddrAndHandler(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
srv := server.NewHTTPServer(":9999", noopHandler())
|
||||||
|
|
||||||
|
if srv.Addr != ":9999" {
|
||||||
|
t.Errorf("Addr = %q, want %q", srv.Addr, ":9999")
|
||||||
|
}
|
||||||
|
|
||||||
|
if srv.Handler == nil {
|
||||||
|
t.Error("Handler must not be nil")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user