1 Commits

Author SHA1 Message Date
520ce79709 build: isolate golangci-lint cache and lock per checkout (closes #121)
All checks were successful
check / check (push) Successful in 54s
script/lint used golangci-lint's per-user global state, which breaks
when several checkouts on one host lint concurrently. Two independent
failure modes, two causes:

- Cross-contamination. The analysis cache (GOLANGCI_LINT_CACHE,
  default ~/.cache/golangci-lint) is keyed by content, not by
  checkout, so a hit written by another checkout is replayed with
  that checkout's file paths. A run reports findings for files it
  never linted.

- Lock collision. golangci-lint locks os.TempDir()/golangci-lint.lock
  (pkg/commands/run.go, acquireFileLock), which is NOT in the cache
  directory, with a 5s timeout. Peers that hold it longer make the
  run abort with "parallel golangci-lint is running" - a non-result
  that reads as a lint failure. Isolating the cache does not move it.

Point GOLANGCI_LINT_CACHE and TMPDIR at .lint-cache/ under the
checkout root, using the existing $ROOT idiom. TMPDIR is what makes
the lock per-checkout, so the lock keeps serialising the runs that
actually share a cache instead of being disabled. .lint-cache/ is
git-ignored and Docker-ignored, and the cache persists across runs in
a checkout, so caching is not lost.

Reproduced both modes on the unfixed script across 12 copies of this
tree: 10 of 12 concurrent runs void with the lock error, and 11 of 12
sequential runs reported findings at ../w1/... after an identical
lint-failing file was added to every copy. After the fix, 20-way
concurrency gives 0 void and 0 foreign paths, and each copy reports
only its own relative path.
2026-08-09 14:35:42 +00:00
8 changed files with 45 additions and 472 deletions

View File

@@ -1,5 +1,6 @@
.git/
bin/
.lint-cache/
*.md
LICENSE
.editorconfig

1
.gitignore vendored
View File

@@ -1,6 +1,7 @@
bin/
vendor/
data/
.lint-cache/
.env
*.exe
/dnswatcher

View File

@@ -182,46 +182,6 @@ 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
`<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
@@ -234,8 +194,7 @@ 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, security
headers, metrics auth)
middleware/middleware.go HTTP middleware (logging, CORS, metrics auth)
handlers/handlers.go HTTP request handlers
server/
server.go HTTP server lifecycle
@@ -428,7 +387,10 @@ them. We provide:
- `script/projectname` — print the project name (used for the Docker
image tag)
- `script/test` — run the test suite (race detector, coverage)
- `script/lint` — run golangci-lint
- `script/lint` — run golangci-lint, with its cache and its lock file
isolated to this checkout (under the git-ignored `.lint-cache/`) so
concurrent checkouts on one host cannot share cache entries or
contend on a single lock
- `script/fmt` — format all code (gofmt -s, goimports)
- `script/fmt-check` — check formatting (read-only)
- `script/check` — run test, lint, and fmt-check

19
TODO.md
View File

@@ -25,16 +25,15 @@ 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-09: `script/lint` now isolates golangci-lint's per-user global
state to the checkout (#121): `GOLANGCI_LINT_CACHE` and `TMPDIR` are
both pointed at the git-ignored, Docker-ignored `.lint-cache/`. The
cache fixes cross-contamination; `TMPDIR` is what moves the lock,
which lives at `$TMPDIR/golangci-lint.lock` and not in the cache
directory. Reproduced both failure modes on the unfixed script (10 of
12 concurrent runs void with `parallel golangci-lint is running`; 11
of 12 reporting another checkout's paths) and both are gone at 20-way
concurrency after the fix
- 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

View File

@@ -21,60 +21,6 @@ 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
@@ -240,37 +186,6 @@ 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 == "" {

View File

@@ -1,333 +0,0 @@
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)
}
}

View File

@@ -21,7 +21,6 @@ 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))

View File

@@ -1,11 +1,40 @@
#!/bin/sh
# script/lint: run the linter.
#
# golangci-lint keeps two pieces of per-user global state, and both of
# them break when several checkouts on one host lint concurrently:
#
# 1. Its analysis cache (GOLANGCI_LINT_CACHE, default
# ~/.cache/golangci-lint). Entries are keyed by content, not by
# checkout, so a hit written by another checkout is replayed
# verbatim - including that checkout's file paths. The run then
# reports findings for files it never linted.
#
# 2. Its "one runner at a time" lock, which does NOT live in the
# cache directory: golangci-lint locks
# $(os.TempDir())/golangci-lint.lock, i.e.
# "$TMPDIR"/golangci-lint.lock (pkg/commands/run.go,
# acquireFileLock). It waits 5s, then aborts with "parallel
# golangci-lint is running" - a non-result that looks like a lint
# failure. Setting GOLANGCI_LINT_CACHE alone does not move it.
#
# So both are pinned under the checkout root. The cache is never shared,
# and TMPDIR makes the lock file per-checkout, which keeps the lock
# doing its actual job (serialising runs that share one cache) at the
# right scope. .lint-cache/ is git-ignored and Docker-ignored, and
# caching still works: it persists across runs in this checkout.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() {
cd "$ROOT"
GOLANGCI_LINT_CACHE="$ROOT/.lint-cache/cache"
TMPDIR="$ROOT/.lint-cache/tmp"
export GOLANGCI_LINT_CACHE TMPDIR
mkdir -p "$GOLANGCI_LINT_CACHE" "$TMPDIR"
golangci-lint run --config .golangci.yml ./...
}