1 Commits

Author SHA1 Message Date
a535ae864b Remove DNS mocking from tests; use live DNS everywhere
All checks were successful
check / check (push) Successful in 53s
DNS is never mocked in this repository: tests exercise live DNS,
and robustness comes from handling real-world DNS behavior with
tolerant assertions and sensible timeouts, not from mocks.

watcher: drop mockResolver and wire the real iterative resolver
into the tests, querying stable public names (example.com,
www.example.com). Change detection is exercised by seeding the
state store with a synthetic previous observation that live DNS
cannot match (reserved .invalid nameserver names and RFC 5737
documentation addresses); DNS stays live in every run. The port
checker, TLS checker, and notifier remain test doubles since they
are not DNS, keeping notification and state assertions
deterministic against whatever addresses live DNS returns.

resolver: drop the timeoutClient fake DNSClient and the
NewFromLoggerWithClient mock constructor. The timeout test is
replaced by a live query against an RFC 5737 documentation
address where no nameserver can exist, asserting a classified
non-OK response with no records.

TESTING.md: extend the live-DNS policy to every package and
remove the carve-out that permitted DNS mocks in packages that
consume the resolver.

TODO.md: update stale references to hermetic mocked-DNS work to
reflect the no-mocking policy and the current state of
feature/resolver.

Intentionally dropped coverage: the exact StatusTimeout
classification (previously forced by the fake client) is no
longer asserted, because a genuinely unreachable server may fail
fast instead of timing out depending on the network path; the
live test tolerantly accepts any failure classification.
2026-08-07 23:40:38 +00:00
10 changed files with 455 additions and 1013 deletions

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

View File

@@ -2,8 +2,10 @@
## DNS Resolution Tests
All resolver tests **MUST** use live queries against real DNS servers.
No mocking of the DNS client layer is permitted.
All tests that involve DNS resolution — in every package, including
consumers of the resolver such as the watcher — **MUST** use live
queries against real DNS servers. No mocking, faking, or stubbing of
DNS at any layer is permitted.
### Rationale
@@ -12,6 +14,8 @@ the full delegation chain. Mocked responses cannot faithfully represent
the variety of real-world DNS behavior (truncation, referrals, glue
records, DNSSEC, varied response times, EDNS, etc.). Testing against
real servers ensures the resolver works correctly in production.
Robustness comes from handling real-world DNS behavior with tolerant
assertions and sensible timeouts, not from mocks.
### Constraints
@@ -24,11 +28,14 @@ real servers ensures the resolver works correctly in production.
- Flaky failures from transient network issues are acceptable and
should be investigated as potential resolver bugs, not papered over
with mocks or skip flags
- Watcher change-detection tests seed a synthetic *previous state*
and compare it against fresh live lookups; the DNS side is never
faked
### What NOT to do
- **Do not mock `DNSClient`** for resolver tests (the mock constructor
exists for unit-testing other packages that consume the resolver)
- **Do not mock `DNSClient`**, the watcher's `DNSResolver` interface,
or any other DNS abstraction — in any package, for any reason
- **Do not add `-short` flags** to skip slow tests
- **Do not increase `-timeout`** to hide hanging queries
- **Do not modify linter configuration** to suppress findings

34
TODO.md
View File

@@ -14,7 +14,10 @@ pre-1.0. No git tags. Core resolver work in flight on feature/resolver
(dirty: internal/resolver/resolver_test.go). Local checkout has diverged
from origin: origin/main is 8 commits ahead (watcher orchestrator,
unified TARGETS) and origin/feature/resolver already contains the full
iterative resolver implementation with hermetic mocked tests.
iterative resolver implementation. DNS mocking is banned in this repo
(see `TESTING.md`): all tests use live DNS only. The hermetic mocked
tests previously noted on `feature/resolver` are gone from its current
tip, which carries a live-DNS suite against `*.dns.sneak.cloud`.
# Next Step
@@ -25,16 +28,10 @@ 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: DNS mocking removed from the entire test suite; watcher
tests now drive the real iterative resolver against live DNS and
`TESTING.md` bans DNS mocks in every package (`remove-dns-mocking`
branch)
- 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
@@ -46,7 +43,8 @@ confirm make check still passes.
- 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints,
Makefile shims, README Entrypoints section
- 2026-02-20: iterative DNS resolver implemented; tests made hermetic
with mocked DNS (origin/feature/resolver, unmerged)
with mocked DNS (origin/feature/resolver, unmerged; superseded — DNS
mocking is banned, see `TESTING.md`)
- 2026-02-20: CI actions and go install refs pinned to commit SHAs;
Gitea Actions workflow for make check (origin/ci/make-check, unmerged)
- 2026-02-20: watcher monitoring orchestrator merged to main (#8)
@@ -73,8 +71,9 @@ Branch reconciliation:
- Sync local checkout with origin: local main is 8 commits behind
origin/main; local feature/resolver has diverged from
origin/feature/resolver, which already implements the resolver
- Merge in-flight branches to main once green: feature/resolver,
ci/make-check, feature/portcheck-implementation,
- Merge in-flight branches to main once green: feature/resolver
(confirm its tests remain live-DNS — DNS mocking is banned, see
`TESTING.md`), ci/make-check, feature/portcheck-implementation,
feature/tlscheck-implementation
Resolver (plan from untracked TODO.md; largely implemented on
@@ -158,6 +157,7 @@ Infrastructure notes (from untracked TODO.md):
- Module path sneak.berlin/go/dnswatcher differs from the git.eeqj.de
remote intentionally; do not "fix" it
- Dependencies: github.com/miekg/dns, golang.org/x/net/publicsuffix
- Resolver tests originally used live DNS against *.dns.sneak.cloud
(required records documented in the test file header); origin now has
mocked hermetic tests, keep them hermetic
- Resolver tests originally used live DNS against `*.dns.sneak.cloud`
(required records documented in the test file header); `main` now
tests against live public DNS. DNS mocking is banned (see
`TESTING.md`); never reintroduce hermetic mocked DNS tests

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

@@ -7,8 +7,8 @@ import (
"github.com/miekg/dns"
)
// DNSClient abstracts DNS wire-protocol exchanges so the resolver
// can be tested without hitting real nameservers.
// DNSClient abstracts DNS wire-protocol exchanges over a single
// transport, letting the resolver switch between UDP and TCP.
type DNSClient interface {
ExchangeContext(
ctx context.Context,

View File

@@ -67,17 +67,4 @@ func NewFromLogger(log *slog.Logger) *Resolver {
}
}
// NewFromLoggerWithClient creates a Resolver with a custom DNS
// client, useful for testing with mock DNS responses.
func NewFromLoggerWithClient(
log *slog.Logger,
client DNSClient,
) *Resolver {
return &Resolver{
log: log,
client: client,
tcp: client,
}
}
// Method implementations are in iterative.go.

View File

@@ -10,7 +10,6 @@ import (
"testing"
"time"
"github.com/miekg/dns"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -624,58 +623,41 @@ func TestQueryAllNameservers_ContextCanceled(t *testing.T) {
}
// ----------------------------------------------------------------
// Timeout tests
// Unreachable nameserver tests
// ----------------------------------------------------------------
func TestQueryNameserverIP_Timeout(t *testing.T) {
func TestQueryNameserverIP_UnreachableServer(t *testing.T) {
t.Parallel()
log := slog.New(slog.NewTextHandler(
os.Stderr,
&slog.HandlerOptions{Level: slog.LevelDebug},
))
r := resolver.NewFromLoggerWithClient(
log, &timeoutClient{},
)
r := newTestResolver(t)
ctx, cancel := context.WithTimeout(
context.Background(), 10*time.Second,
)
t.Cleanup(cancel)
// Query any IP — the client always returns a timeout error.
// 192.0.2.1 is an RFC 5737 documentation address: no
// nameserver can exist there. Depending on the network
// path the queries either time out (silent drop) or fail
// fast (ICMP unreachable), so accept any non-OK status;
// the resolver must return a classified response with no
// records rather than an error or a hang.
resp, err := r.QueryNameserverIP(
ctx, "unreachable.test.", "192.0.2.1",
"example.com",
)
require.NoError(t, err)
assert.Equal(t, resolver.StatusTimeout, resp.Status)
assert.NotEmpty(t, resp.Error)
}
assert.NotEqual(t, resolver.StatusOK, resp.Status)
// timeoutClient simulates DNS timeout errors for testing.
type timeoutClient struct{}
func (c *timeoutClient) ExchangeContext(
_ context.Context,
_ *dns.Msg,
_ string,
) (*dns.Msg, time.Duration, error) {
return nil, 0, &net.OpError{
Op: "read",
Net: "udp",
Err: &timeoutError{},
totalRecords := 0
for _, values := range resp.Records {
totalRecords += len(values)
}
assert.Zero(t, totalRecords)
}
type timeoutError struct{}
func (e *timeoutError) Error() string { return "i/o timeout" }
func (e *timeoutError) Timeout() bool { return true }
func (e *timeoutError) Temporary() bool { return true }
func TestResolveIPAddresses_ContextCanceled(t *testing.T) {
t.Parallel()

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))

File diff suppressed because it is too large Load Diff