Compare commits

...
13 Commits
Author SHA1 Message Date
sneak daef29e432 docker: run as non-root, add health check, document upaas (closes #147)
check / check (push) Successful in 53s
The runtime image runs as uid 10001, which owns /var/lib/dnswatcher. The
working directory is /, so config loading finds no .env or dnswatcher
config file there; the binary lives in /usr/local/bin. A Docker
HEALTHCHECK probes /.well-known/healthcheck every 10 seconds with busybox
wget, well inside the 60 seconds upaas waits.

Startup now fails with an error naming the data directory when it cannot
be written, instead of running with every save failing. The check creates
the directory if needed and writes and removes the temp file Save uses;
tests cover the create and the write failing.

README gains "Running under upaas": the prod branch, host directory
setup, network and port, environment and health check.

Model: opus-5-5
2026-09-28 17:51:31 +00:00
clawbot 8aaa103956 middleware: add security response headers (closes #98)
check / check (push) Successful in 54s
Adds a SecurityHeaders middleware and registers it globally, right after the request ID middleware, so every route gets the headers, including static files, /metrics and error responses.

It sets Strict-Transport-Security (one year, includeSubDomains), a Content-Security-Policy with default-src 'self', no scripts and frame-ancestors 'none', X-Frame-Options DENY, X-Content-Type-Options nosniff, Referrer-Policy no-referrer and a Permissions-Policy that turns every listed feature off.

HSTS is sent on every response, not only over TLS: the service runs behind a TLS-terminating proxy and REPO_POLICIES.md requires the application to send it. Referrer-Policy is stricter than the policy baseline because dashboard URLs can name internal hosts.

model: claude-opus-4-8 (implementation); claude-fable-5 (commit message)
2026-09-22 00:52:48 +02:00
clawbot c2a07ce690 test: add tests for globals, healthcheck and logger (closes #110)
check / check (push) Failing after 0s
Tests for the three packages that had none, written from outside each package, each able to fail on a plausible break:

- globals: values set are read back through New, and New returns an independent copy. One sequential test function with a disclosed paralleltest suppression, because it changes shared package variables.
- healthcheck: Check returns status "ok", the documented JSON fields, an RFC3339Nano timestamp, the maintenance flag from config in both states, and version and appname from globals.
- logger: New gives a usable *slog.Logger, debug output is off by default and EnableDebugLogging turns it on.

No production code changed. The terminal output format is not asserted.

Model: opus-4-8
2026-09-21 10:05:38 +02:00
clawbot b351a2350c gomod: drop stale golang.org/x/sync indirect line (closes #132)
check / check (push) Failing after 0s
go.mod listed golang.org/x/sync v0.19.0 twice: once in the direct
require block and once as // indirect. The module is a genuine direct
dependency (internal/portcheck/portcheck.go imports
golang.org/x/sync/errgroup), so the indirect entry was redundant and
stale. script/bootstrap ends with go mod download, which dropped that
line as a side effect and left every fresh checkout with a dirty
go.mod. Running go mod tidy removes the line for good; no dependency
and no version changed, and go.sum is unaffected.

Model: opus-4-8
2026-09-21 09:44:49 +02:00
clawbot 1ffe303a6e Merge main into next, resolving TODO.md (closes #145)
check / check (push) Failing after 1s
The last milestone landed on main as a squash, so main was no longer an
ancestor of next and git merge-tree reported a TODO.md conflict: both
branches added the same block of Completed Steps entries, and next added
one more (the #99 http.Server timeouts entry). This real merge commit
makes main an ancestor of next again. TODO.md is resolved by hand to keep
every entry from both sides exactly once, which is next's version since
next's entries are a superset of main's. Every other file already equals
next. No new TODO.md entry is added, per the task brief.

Model: opus-4-8
2026-09-21 07:22:17 +00:00
clawbot fc43f893a5 server: set all four http.Server socket timeouts (#118)
check / check (push) Successful in 3m50s
The http.Server was built with only ReadHeaderTimeout set; the other three
timeouts were zero, which in net/http means no limit, so a peer could hold a
connection open past the header phase, responses had no write deadline, and
keep-alive connections were never reaped. ReadTimeout 15s, WriteTimeout 75s,
and IdleTimeout 120s now join ReadHeaderTimeout 10s as named constants.
WriteTimeout must stay above the 60s chimw.Timeout handler budget, because
net/http arms the write deadline once request headers are read; a test fails
the build if either number moves alone. The server literal moved into
newHTTPServer so the configuration can be asserted without binding a socket
(closes #99)

Model: opus-5
2026-09-09 15:17:43 +02:00
clawbot ae06f7e3a1 notify: drain in-flight deliveries at shutdown (closes #106)
check / check (push) Successful in 1m22s
Shutdown waits for deliveries already in flight instead of dropping them.

Reviewed and green; squashed to next by the dispatcher, dnswatcher having no manager.

Model: opus-5
2026-09-09 14:26:53 +02:00
sneak b8662b8a9c docs: correct stale script headers and record the config-verify cost (closes #137)
check / check (push) Successful in 51s
script/bootstrap credited the goimports pin to script/fmt-check, which
runs gofmt only; the header now credits script/fmt. script/cibuild
still described the Dockerfile as running make check, which stopped
being true once linting moved to its own stage. The docker-missing
warning in script/bootstrap is one sentence instead of three fragments
each carrying the bootstrap: prefix. Dockerfile.lint now states the
residual risk of skipping golangci-lint config verify: unknown
top-level keys in .golangci.yml are ignored silently, so a mistyped key
lints clean and applies nothing.

Comment and message text only; no behaviour changes.
2026-08-10 14:09:20 +00:00
sneak 168281ad60 docs: add MIT LICENSE and state the licence in the README (closes #102)
check / check (push) Has been cancelled
The repository had no licence file at all, which makes publicly readable
code all-rights-reserved by default: nobody may legally use it. That is a
1.0 blocker rather than a nicety, and `LICENSE` was the only file from
`REPO_POLICIES.md`'s required minimum still missing here.

The choice is standing org policy rather than a per-repo call: any public
repo lacking a licence gets MIT, while a private repo with no licence is
already all-rights-reserved and needs nothing. `sneak/dnswatcher` is
public, so MIT.

`LICENSE` carries the canonical MIT text byte-for-byte with only the
copyright line filled in; no clauses added, removed, reworded, or
reflowed. `README.md`'s first line now names the licence, which the
Description requirement in `REPO_POLICIES.md` calls for, and the License
section states MIT and points at the file instead of recording the
decision as pending.
2026-08-10 14:04:31 +00:00
sneak 6f6bf3a65b test: disable Go's test cache so every run queries live DNS (closes #139)
check / check (push) Successful in 1m34s
`script/test` did not pass `-count=1`, so on an unchanged tree Go
served the whole suite from its test cache: exit 0 in ~0.2s with every
package marked `(cached)` and not one DNS query made. This repo's suite
exists to exercise live resolution on every run (`TESTING.md`), so that
green asserted nothing — and it is exactly the green used as evidence
that a flakiness fix works, since "run it a few times" stops being
runs after the first.

`-count=1` now disables caching on every invocation.

The conditional verbose rerun that `REPO_POLICIES.md` mandates was
missing at the same spot and is added here rather than left broken: the
primary run had been unconditionally `-v`, which is the failure mode
the policy exists to prevent (unreadable CI and `docker build` logs on
success). Tests now run quiet, and only a failure triggers the `-v`
rerun. The rerun carries `-count=1` too, so it cannot replay a cached
copy of the failure it is meant to diagnose, and its exit status is
discarded in favour of a forced 1: the first failure already proved the
suite broken, so a flake that passes the second time must not turn the
build green.

`-timeout 90s` is untouched. It is a deliberate backstop that must
strictly exceed the 60s hard cap on suite duration.

No special-casing for the Docker build, which also reaches this script
via `RUN make test`: a fresh container's test cache is empty, so
`-count=1` changes nothing there and carving out an exception would
only create a second code path that could drift.

Verified: three back-to-back `make test` runs on an unchanged tree,
zero `(cached)` markers, ~4.0-4.5s wall each (was ~0.2s cached),
comfortably inside the 20s target with `-race` and `-cover` both still
working and coverage percentages unchanged. The rerun-and-still-fail
path was exercised against a purpose-built flaky test that fails once
then passes: quiet failure, verbose rerun that genuinely re-executed,
exit 1 regardless. `make check` green.
2026-08-10 13:48:15 +00:00
sneak 87bce43f8d test: rework live-DNS quorum unit — tolerate silence, never a wrong answer
check / check (push) Successful in 1m23s
Rework of the unit at #93
(commit 9cb2c2b), against the review at
#136 (comment).

Review of 9cb2c2b found the quorum assertions could not fail on a
class of wrong answer. Each test banned exactly one bad status —
_AllReturnOK banned only nxdomain, _NXDomainFromAllNS banned only ok
— so resolver.StatusNoData passed both. nodata is a wrong answer, not
silence, and answeredCount counted it as answered, so it did not even
trigger a retry; with a quorum of 3 of 4 a single wrong nameserver
slid through undetected. That is assertion-loosening beyond what the
quorum change requires.

Tolerance is now a closed allowlist rather than a blocklist of one
status. unsanctionedStatuses() reports every per-nameserver result
whose status the caller did not explicitly sanction: ok/timeout/error
for the all-OK test, nxdomain/timeout/error for the NXDOMAIN test.
Silence (timeout, error) is the only thing quorum exists to tolerate;
any other status, including one added to the resolver later, fails by
name. answeredCount is likewise an allowlist of ok/nxdomain/nodata, so
an unknown status counts as silence and can only cause a retry and
then a loud failure, never a quiet pass.

Two harness tests cover the regression directly: three OK plus one
nodata (quorum satisfied, no nxdomain present — the input that used
to pass) is now reported as unsanctioned, and an unknown status is
neither counted as answered nor tolerated.

Verified by re-running the reviewer's probe: queryEachNS patched to
force one of google.com's four nameservers to return StatusNoData
turns both tests red, naming the offending nameserver and status —

    --- FAIL: TestQueryAllNameservers_AllReturnOK (1.12s)
        Should be empty, but was [ns1.google.com.=nodata]
        every nameserver must answer OK or not answer at all:
        ns1.google.com.=nodata ns2.google.com.=ok ns3.google.com.=ok
        ns4.google.com.=ok
    --- FAIL: TestQueryAllNameservers_NXDomainFromAllNS (1.34s)
        Should be empty, but was [ns1.google.com.=nodata]
        every nameserver must report NXDOMAIN or not answer at all:
        ns1.google.com.=nodata ns2.google.com.=nxdomain
        ns3.google.com.=nxdomain ns4.google.com.=nxdomain

— and green with the probe reverted. Also fixes the review's nit: the
per-attempt deadline assertion had no lower bound, so it passed for a
deadline far shorter than intended.

No production code changed; DNS is still never mocked.
2026-08-10 13:33:53 +00:00
sneak 9cb2c2b7e0 test: make live DNS tests robust instead of gated (closes #93)
check / check (push) Successful in 1m18s
The resolver's live-DNS tests failed nondeterministically, a different
subset each run. Three structural causes, all test-side:

- Burst fan-out. Every test in the package is parallel and the build
  hosts have many cores, so all ~35 iterative resolutions started at
  the same instant and, because queryServers walks rootServerList() in
  fixed order, hit the same root server within milliseconds. Root
  servers rate-limit that.
- No retry anywhere. One dropped UDP packet in a delegation chain
  failed a test outright.
- Unanimity assertions. TestQueryAllNameservers_AllReturnOK and
  _NXDomainFromAllNS required every one of a domain's nameservers to
  answer, with no tolerance for one being slow.

New internal/resolver/livedns_test.go addresses each: a package-wide
gate bounds how many live resolutions are in flight at once, every
live operation gets three attempts with exponential backoff and its
own deadline, and multi-nameserver assertions now need a strict
majority rather than unanimity. The retry predicate is deliberately
transport-level -- "did a nameserver answer at all" -- never the
assertion under test, so a resolver that answers incorrectly still
fails on the first attempt. A nameserver that stays silent is
tolerated; one that answers wrongly is not.

livedns_harness_test.go tests that machinery directly: quorum
arithmetic, status counting, the gate's concurrency bound, per-attempt
deadlines, and recovery from a transient failure. It touches no DNS.

Nothing is mocked, faked, stubbed, recorded, skipped or build-tagged,
and production resolver behaviour is unchanged.

Test caps move to the new org-wide values ruled at prompts issue 41:
60s hard cap, 20s target, 90s -timeout backstop. REPO_POLICIES.md is
re-vendored byte-identical from sneak/prompts rather than hand-edited,
which also picks up the golangci-lint paragraph this copy had drifted
behind on. TESTING.md's stale 30-second target follows to 60.

#93
2026-08-10 13:09:17 +00:00
sneak cc86473410 build: run all linting in Docker via Dockerfile.lint (closes #134)
check / check (push) Successful in 1m17s
golangci-lint is no longer installed or run on the host. script/lint is
now a thin wrapper that builds the new root Dockerfile.lint, which COPYs
the repo into the digest-pinned golangci/golangci-lint:v2.12.2 image and
lints as a build step, so a successful build is a clean lint. This works
even where the docker daemon is remote and bind mounts are impossible.

Dockerfile.lint is split into a deps stage (base image, go mod download)
and a lint stage (source copy, linter run). script/lint passes
--no-cache-filter=lint so the lint stage executes on every invocation:
caching is explicitly waived for linting, and a cached build lints
nothing. The deps stage stays cached and no global cache invalidation is
performed. --progress=plain keeps the linter's own output visible.

golangci-lint config verify is deliberately omitted: it fetches its JSON
schema over a live, unpinned HTTPS call, which would make linting
network-dependent and defeat hash-pinning.

script/bootstrap no longer installs golangci-lint and warns instead when
docker is absent. The goimports install stays, since script/fmt and
script/fmt-check still run it on the host.

The root Dockerfile ran make check in its builder stage, which would now
recurse into script/lint and shell out to docker build with no daemon
available. It gains its own lint stage on the same pinned image, invoked
directly, with the builder depending on it via COPY --from=lint and
running make fmt-check, make test and make build.
2026-08-10 12:37:47 +00:00
15 changed files with 1162 additions and 18 deletions
+24 -6
View File
@@ -41,15 +41,33 @@ FROM alpine@sha256:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4
RUN apk add --no-cache ca-certificates tzdata RUN apk add --no-cache ca-certificates tzdata
WORKDIR /app COPY --from=builder /src/bin/dnswatcher /usr/local/bin/dnswatcher
COPY --from=builder /src/bin/dnswatcher /app/dnswatcher # Run as an unprivileged user that owns the data directory. A fresh named
# volume inherits this ownership; a bind-mounted host directory must be
# Create data directory # owned by uid 10001 (see "Running under upaas" in README.md), or startup
RUN mkdir -p /var/lib/dnswatcher # fails.
RUN addgroup -S -g 10001 dnswatcher \
&& adduser -S -G dnswatcher -u 10001 dnswatcher \
&& mkdir -p /var/lib/dnswatcher \
&& chown dnswatcher:dnswatcher /var/lib/dnswatcher
ENV DNSWATCHER_DATA_DIR=/var/lib/dnswatcher ENV DNSWATCHER_DATA_DIR=/var/lib/dnswatcher
# Config loading also reads a `.env` file and a file named `dnswatcher`
# (any config extension, or none) from the working directory. `/` holds
# neither, so every setting comes from the environment. Do not make the
# data directory, or the binary's directory, the working directory.
WORKDIR /
USER dnswatcher
EXPOSE 8080 EXPOSE 8080
ENTRYPOINT ["/app/dnswatcher"] # busybox wget (already in alpine) probes the health endpoint every 10
# seconds, so the container is healthy well before upaas reads its health
# 60 seconds after a deploy and fails the deploy unless it is healthy.
HEALTHCHECK --interval=10s --timeout=5s --start-period=10s --retries=3 \
CMD wget -q -O /dev/null "http://127.0.0.1:${PORT:-8080}/.well-known/healthcheck" || exit 1
ENTRYPOINT ["/usr/local/bin/dnswatcher"]
+112 -4
View File
@@ -182,6 +182,67 @@ dnswatcher exposes a lightweight HTTP API for operational visibility:
| `GET /api/v1/status` | Current monitoring state | | `GET /api/v1/status` | Current monitoring state |
| `GET /metrics` | Prometheus metrics (optional) | | `GET /metrics` | Prometheus metrics (optional) |
#### Server timeouts
The HTTP server sets all four socket-level timeouts. These are compile-time
constants in `internal/server/server.go`, not configurable via environment
variables.
| Timeout | Value | Purpose |
|---------------------|-------|-----------------------------------------------|
| `ReadHeaderTimeout` | 10s | Bounds the request header read (slowloris) |
| `ReadTimeout` | 15s | Bounds the whole request read, headers + body |
| `WriteTimeout` | 75s | Bounds handler execution plus response flush |
| `IdleTimeout` | 120s | Reaps idle keep-alive connections |
These are distinct from the 60s per-request handler budget applied by
`chimw.Timeout` in `internal/server/routes.go`, which cancels the request
context but does not touch the socket. `WriteTimeout` is deliberately
larger than that budget: the write deadline is armed once request headers
are read, so a smaller value would sever the connection before a handler
using its full budget could respond. `IdleTimeout` exceeds common
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
@@ -194,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
@@ -446,11 +508,57 @@ docker run -d \
--- ---
## Running under upaas
[upaas](https://git.eeqj.de/sneak/upaas) builds the image from this
repository's `Dockerfile` and runs it. The app needs:
- **Branch:** `prod`. `prod` is cut from `main`, and merging a `main` to
`prod` pull request is a deploy.
- **Volume:** one host directory mounted at `/var/lib/dnswatcher`, where
the state file lives. upaas bind-mounts the host path it is given and
does not create it. The container runs as uid 10001 and does not start
unless it can write there. Create the directory before the first
deploy:
```sh
mkdir -p /path/to/data
chown 10001:10001 /path/to/data
chmod 700 /path/to/data
```
- **Network and port:** the dashboard is unauthenticated and shows every
watched name and recent alert, and upaas publishes every mapped port on
all interfaces of the host
([upaas issue 113](https://git.eeqj.de/sneak/upaas/issues/113)). Add a
port mapping to container port `8080` only if the dashboard should be
public. Otherwise add none: set the app's Docker network in upaas to
your reverse proxy's Docker network, and the proxy reaches the app at
`upaas-` followed by the app name, port `8080`.
- **Required environment:** `DNSWATCHER_TARGETS`, a comma-separated list
of the domains and hostnames to watch. dnswatcher refuses to start
without it.
- **Recommended environment:** at least one notification endpoint
(`DNSWATCHER_SLACK_WEBHOOK`, `DNSWATCHER_MATTERMOST_WEBHOOK`,
`DNSWATCHER_NTFY_TOPIC`); without one, changes show only on the
dashboard. `DNSWATCHER_METRICS_USERNAME` and
`DNSWATCHER_METRICS_PASSWORD` serve `/metrics` behind basic auth.
- **Leave unset:** `DNSWATCHER_DATA_DIR`, which the image sets to
`/var/lib/dnswatcher`, and `PORT`, which defaults to `8080`. Every
setting comes from the environment; the image holds no config file.
- **Health check:** the image's own, which requests
`/.well-known/healthcheck` every 10 seconds. upaas reads the
container's health 60 seconds after a deploy and marks the deploy
failed unless it is `healthy`.
---
## Monitoring Lifecycle ## Monitoring Lifecycle
1. **Startup**: Load state from disk. If no state file exists, start 1. **Startup**: Check that the data directory can be written, and exit
with empty state (first check will establish baseline without with an error naming it if not. Load state from disk. If no state
triggering change notifications). file exists, start with empty state (first check will establish
baseline without triggering change notifications).
2. **Initial check**: Immediately perform all DNS, port, and TLS checks 2. **Initial check**: Immediately perform all DNS, port, and TLS checks
on startup. on startup.
3. **Periodic checks** (DNS always runs first): 3. **Periodic checks** (DNS always runs first):
+25
View File
@@ -23,6 +23,13 @@ Rationale, Design, TODO, License, Author) if any are still missing.
# Completed Steps # Completed Steps
- 2026-09-28: upaas deploy readiness — runtime image runs as unprivileged
`dnswatcher`, Docker `HEALTHCHECK`, startup fails when the data directory is
not writable, README "Running under upaas" (closes #147).
- 2026-09-21: added behavioural tests for `internal/globals`,
`internal/healthcheck`, and `internal/logger` (closes #110).
- 2026-09-21: `go mod tidy` dropped the redundant `golang.org/x/sync`
`// 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`
@@ -101,6 +108,24 @@ Rationale, Design, TODO, License, Author) if any are still missing.
so shutdown cannot be extended indefinitely; an `OnStop` context that so shutdown cannot be extended indefinitely; an `OnStop` context that
is already expired on entry with nothing outstanding drains quietly is already expired on entry with nothing outstanding drains quietly
rather than warning about deliveries that were never abandoned rather than warning about deliveries that were never abandoned
- 2026-08-09: `http.Server` now sets all four socket-level timeouts
(`ReadTimeout` 15s, `ReadHeaderTimeout` 10s, `WriteTimeout` 75s,
`IdleTimeout` 120s) as named constants in `internal/server/server.go`,
closing the slowloris / unreaped-keep-alive exposure required by
`REPO_POLICIES.md` before 1.0; `WriteTimeout` is deliberately greater
than the 60s `chimw.Timeout` handler budget so that budget stays
reachable, and tests in `internal/server` pin both the non-zero
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
-1
View File
@@ -40,7 +40,6 @@ require (
go.yaml.in/yaml/v2 v2.4.2 // indirect go.yaml.in/yaml/v2 v2.4.2 // indirect
go.yaml.in/yaml/v3 v3.0.4 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect
golang.org/x/mod v0.32.0 // indirect golang.org/x/mod v0.32.0 // indirect
golang.org/x/sync v0.19.0 // indirect
golang.org/x/sys v0.41.0 // indirect golang.org/x/sys v0.41.0 // indirect
golang.org/x/text v0.34.0 // indirect golang.org/x/text v0.34.0 // indirect
golang.org/x/tools v0.41.0 // indirect golang.org/x/tools v0.41.0 // indirect
+50
View File
@@ -0,0 +1,50 @@
package globals_test
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/dnswatcher/internal/globals"
)
// TestGlobals exercises the package-level version and appname
// variables through their setters and read-back via New. These are
// shared package state, so the test mutates a global and must run
// sequentially; it cannot use t.Parallel().
//
//nolint:paralleltest // mutates shared package-level globals, must run sequentially
func TestGlobals(t *testing.T) {
versions := []string{"v1.2.3", "dev", "", "v1.2.3-4-gabcdef"}
for _, want := range versions {
globals.SetVersion(want)
g, err := globals.New(nil)
require.NoError(t, err)
assert.Equal(t, want, g.Version,
"New must surface the version set by SetVersion")
}
names := []string{"dnswatcher", "other", ""}
for _, want := range names {
globals.SetAppname(want)
g, err := globals.New(nil)
require.NoError(t, err)
assert.Equal(t, want, g.Appname,
"New must surface the appname set by SetAppname")
}
// New returns a snapshot: a later SetVersion must not mutate a
// Globals handed out earlier.
globals.SetVersion("first")
g, err := globals.New(nil)
require.NoError(t, err)
globals.SetVersion("second")
assert.Equal(t, "first", g.Version,
"a Globals returned by New must not change when the "+
"package variable is set again")
}
+134
View File
@@ -0,0 +1,134 @@
package healthcheck_test
import (
"context"
"encoding/json"
"testing"
"time"
"go.uber.org/fx"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/dnswatcher/internal/config"
"sneak.berlin/go/dnswatcher/internal/globals"
"sneak.berlin/go/dnswatcher/internal/healthcheck"
"sneak.berlin/go/dnswatcher/internal/logger"
)
// recordingLifecycle is a minimal fx.Lifecycle that records the hooks
// appended to it, so healthcheck.New can be exercised through its real
// constructor without standing up a whole fx application.
type recordingLifecycle struct {
hooks []fx.Hook
}
func (l *recordingLifecycle) Append(hook fx.Hook) {
l.hooks = append(l.hooks, hook)
}
// newHealthcheck builds a Healthcheck through the real constructor and
// runs the registered OnStart hook so StartupTime is set the same way
// the fx lifecycle would set it.
func newHealthcheck(
t *testing.T,
maintenance bool,
version string,
) *healthcheck.Healthcheck {
t.Helper()
g := &globals.Globals{Appname: "dnswatcher", Version: version}
log, err := logger.New(nil, logger.Params{Globals: g})
require.NoError(t, err)
lifecycle := &recordingLifecycle{}
hc, err := healthcheck.New(lifecycle, healthcheck.Params{
Globals: g,
Config: &config.Config{MaintenanceMode: maintenance},
Logger: log,
})
require.NoError(t, err)
require.Len(t, lifecycle.hooks, 1,
"New must register exactly one lifecycle hook")
require.NotNil(t, lifecycle.hooks[0].OnStart)
require.NoError(t, lifecycle.hooks[0].OnStart(context.Background()))
return hc
}
func TestCheckStatusAndPayloadShape(t *testing.T) {
t.Parallel()
hc := newHealthcheck(t, false, "v9.9.9")
resp := hc.Check()
assert.Equal(t, "ok", resp.Status)
// The JSON shape and field names are part of the contract for the
// /health and /.well-known/healthcheck routes, so assert on the
// exact set of keys the response marshals to.
raw, err := json.Marshal(resp)
require.NoError(t, err)
var fields map[string]json.RawMessage
require.NoError(t, json.Unmarshal(raw, &fields))
wantKeys := []string{
"status",
"now",
"uptimeSeconds",
"uptimeHuman",
"version",
"appname",
"maintenanceMode",
}
assert.Len(t, fields, len(wantKeys),
"response must marshal to exactly the documented fields")
for _, key := range wantKeys {
assert.Contains(t, fields, key, "missing JSON field %q", key)
}
// The Now field is documented as RFC3339Nano; a change to the
// format constant should turn this red.
_, err = time.Parse(time.RFC3339Nano, resp.Now)
assert.NoError(t, err, "Now must be RFC3339Nano")
}
func TestCheckMaintenanceModeReflectsConfig(t *testing.T) {
t.Parallel()
tests := []struct {
name string
maintenance bool
}{
{"maintenance off", false},
{"maintenance on", true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
hc := newHealthcheck(t, tt.maintenance, "test")
resp := hc.Check()
assert.Equal(t, tt.maintenance, resp.Maintenance,
"maintenanceMode must mirror Config.MaintenanceMode")
})
}
}
func TestCheckSurfacesVersionAndAppname(t *testing.T) {
t.Parallel()
hc := newHealthcheck(t, false, "surfaced-version-123")
resp := hc.Check()
assert.Equal(t, "surfaced-version-123", resp.Version,
"version from globals must appear in the payload")
assert.Equal(t, "dnswatcher", resp.Appname)
}
+66
View File
@@ -0,0 +1,66 @@
package logger_test
import (
"context"
"log/slog"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/dnswatcher/internal/globals"
"sneak.berlin/go/dnswatcher/internal/logger"
)
func newTestLogger(t *testing.T) *logger.Logger {
t.Helper()
g := &globals.Globals{Appname: "dnswatcher", Version: "test"}
l, err := logger.New(nil, logger.Params{Globals: g})
require.NoError(t, err)
return l
}
// TestNewReturnsUsableLogger checks that the constructor yields a
// working *slog.Logger.
func TestNewReturnsUsableLogger(t *testing.T) {
t.Parallel()
l := newTestLogger(t)
require.NotNil(t, l.Get(), "Get must return a non-nil logger")
}
// TestDefaultLevelExcludesDebug verifies the default configuration
// logs at info: debug records are suppressed, info records pass.
func TestDefaultLevelExcludesDebug(t *testing.T) {
t.Parallel()
log := newTestLogger(t).Get()
ctx := context.Background()
assert.False(t, log.Enabled(ctx, slog.LevelDebug),
"debug must be suppressed at the default level")
assert.True(t, log.Enabled(ctx, slog.LevelInfo),
"info must be enabled at the default level")
}
// TestEnableDebugLoggingChangesLevel verifies the debug and non-debug
// configurations differ as intended: enabling debug makes debug
// records pass where they previously did not.
func TestEnableDebugLoggingChangesLevel(t *testing.T) {
t.Parallel()
l := newTestLogger(t)
log := l.Get()
ctx := context.Background()
require.False(t, log.Enabled(ctx, slog.LevelDebug),
"debug must start disabled")
l.EnableDebugLogging()
assert.True(t, log.Enabled(ctx, slog.LevelDebug),
"debug must be enabled after EnableDebugLogging")
}
+85
View File
@@ -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 == "" {
+334
View File
@@ -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)
}
}
+19
View File
@@ -0,0 +1,19 @@
package server
import (
"net/http"
"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
// chimw.Timeout in SetupRoutes, so tests can assert the relationship
// between it and the server's WriteTimeout.
const RequestTimeout time.Duration = requestTimeout
+1
View File
@@ -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))
+64 -7
View File
@@ -33,8 +33,52 @@ type Params struct {
// shutdownTimeout is how long to wait for graceful shutdown. // shutdownTimeout is how long to wait for graceful shutdown.
const shutdownTimeout = 30 * time.Second const shutdownTimeout = 30 * time.Second
// readHeaderTimeout is the max duration for reading request headers. // Socket-level timeouts for the HTTP server.
const readHeaderTimeout = 10 * time.Second //
// These bound time spent on the connection itself and are a distinct
// control from the per-request handler budget enforced by
// chimw.Timeout(requestTimeout) in routes.go: that one cancels the
// request context after requestTimeout but never touches the socket,
// so without the values below a peer can hold a connection open
// forever (slowloris, unreaped keep-alives).
//
// The one hard constraint between the two controls is
// writeTimeout > requestTimeout. net/http arms the write deadline
// once the request headers have been read, so on a plaintext
// connection it covers handler execution AND the response flush. If
// writeTimeout were <= requestTimeout the server would sever the
// connection before a handler that legitimately consumed its full
// budget could emit anything, making the 60s budget unreachable in
// practice. The margin between them is the response-flush allowance.
//
// The only clients of this service are browsers loading the dashboard
// and a Prometheus scraper; the values are sized for those.
const (
// readHeaderTimeout is the max duration for reading request
// headers.
readHeaderTimeout = 10 * time.Second
// readTimeout bounds reading the entire request, headers plus
// body. Every route here is a GET with no body, so this only
// ever needs to cover headers; the extra 5s over
// readHeaderTimeout is slack, not a real allowance, and keeps a
// body dribbled one byte at a time from holding the read side
// open indefinitely.
readTimeout = 15 * time.Second
// writeTimeout must exceed the requestTimeout handler budget
// (60s) per the note above. The 15s difference is the allowance
// for flushing a completed response to a slow client.
writeTimeout = 75 * time.Second
// idleTimeout reaps keep-alive connections between requests. It
// is deliberately longer than the common Prometheus scrape
// intervals (15s/30s/60s) so the scraper reuses its connection
// rather than reconnecting every cycle, while a browser tab
// left open on the dashboard stops occupying a connection
// within two minutes of going quiet.
idleTimeout = 120 * time.Second
)
// Server is the HTTP server. // Server is the HTTP server.
type Server struct { type Server struct {
@@ -76,16 +120,29 @@ func New(
return srv, nil return srv, nil
} }
// newHTTPServer builds the listening http.Server with every
// socket-level timeout set. All four are set deliberately: a zero
// value in net/http means "no limit", not "some default".
func newHTTPServer(
listenAddr string,
handler http.Handler,
) *http.Server {
return &http.Server{
Addr: listenAddr,
Handler: handler,
ReadTimeout: readTimeout,
ReadHeaderTimeout: readHeaderTimeout,
WriteTimeout: writeTimeout,
IdleTimeout: idleTimeout,
}
}
// Run starts the HTTP server. // Run starts the HTTP server.
func (s *Server) Run() { func (s *Server) Run() {
s.SetupRoutes() s.SetupRoutes()
listenAddr := fmt.Sprintf(":%d", s.port) listenAddr := fmt.Sprintf(":%d", s.port)
s.httpServer = &http.Server{ s.httpServer = newHTTPServer(listenAddr, s)
Addr: listenAddr,
Handler: s,
ReadHeaderTimeout: readHeaderTimeout,
}
s.log.Info("http server starting", "addr", listenAddr) s.log.Info("http server starting", "addr", listenAddr)
+112
View File
@@ -0,0 +1,112 @@
package server_test
import (
"net/http"
"testing"
"sneak.berlin/go/dnswatcher/internal/server"
)
// noopHandler stands in for the router; newHTTPServer only stores it.
func noopHandler() http.Handler {
return http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
},
)
}
// TestHTTPServerTimeoutsAreSet asserts that every socket-level
// timeout is configured. A zero value in net/http means "no limit",
// so a refactor that silently drops one of these reintroduces the
// slowloris / unreaped-keep-alive exposure this guards against.
//
// The assertions are on the configured field values only; nothing
// here measures elapsed time, so the test cannot flake on timing.
func TestHTTPServerTimeoutsAreSet(t *testing.T) {
t.Parallel()
srv := server.NewHTTPServer(":8080", noopHandler())
if srv.ReadTimeout <= 0 {
t.Errorf(
"ReadTimeout must be non-zero, got %v",
srv.ReadTimeout,
)
}
if srv.ReadHeaderTimeout <= 0 {
t.Errorf(
"ReadHeaderTimeout must be non-zero, got %v",
srv.ReadHeaderTimeout,
)
}
if srv.WriteTimeout <= 0 {
t.Errorf(
"WriteTimeout must be non-zero, got %v",
srv.WriteTimeout,
)
}
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(
"WriteTimeout (%v) must exceed handler budget (%v)",
srv.WriteTimeout,
server.RequestTimeout,
)
}
}
// 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(
"ReadTimeout (%v) must be >= ReadHeaderTimeout (%v)",
srv.ReadTimeout,
srv.ReadHeaderTimeout,
)
}
}
// 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")
}
}
+29
View File
@@ -148,6 +148,11 @@ func New(
lifecycle.Append(fx.Hook{ lifecycle.Append(fx.Hook{
OnStart: func(_ context.Context) error { OnStart: func(_ context.Context) error {
err := state.checkDataDirWritable()
if err != nil {
return err
}
return state.Load() return state.Load()
}, },
OnStop: func(_ context.Context) error { OnStop: func(_ context.Context) error {
@@ -345,3 +350,27 @@ func (s *State) GetCertificateState(
return cs, ok return cs, ok
} }
// checkDataDirWritable creates the data directory if needed, then writes
// and removes the temp file that Save uses. It runs at startup so that an
// unwritable directory stops the process, instead of the process running
// with every save failing and only logged.
func (s *State) checkDataDirWritable() error {
dir := s.config.DataDir
tmpPath := s.config.StatePath() + ".tmp"
err := os.MkdirAll(dir, dirPermissions)
if err == nil {
err = os.WriteFile(tmpPath, nil, filePermissions)
}
if err == nil {
err = os.Remove(tmpPath)
}
if err != nil {
return fmt.Errorf("data directory %s is not writable: %w", dir, err)
}
return nil
}
+107
View File
@@ -4,10 +4,16 @@ import (
"encoding/json" "encoding/json"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"sync" "sync"
"testing" "testing"
"time" "time"
"go.uber.org/fx/fxtest"
"sneak.berlin/go/dnswatcher/internal/config"
"sneak.berlin/go/dnswatcher/internal/globals"
"sneak.berlin/go/dnswatcher/internal/logger"
"sneak.berlin/go/dnswatcher/internal/state" "sneak.berlin/go/dnswatcher/internal/state"
) )
@@ -493,6 +499,107 @@ func TestSaveWritePermissionError(t *testing.T) {
} }
} }
// startState builds a State through the real constructor and runs its
// startup hook against dataDir, returning the startup error.
func startState(t *testing.T, dataDir string) error {
t.Helper()
g, err := globals.New(nil)
if err != nil {
t.Fatalf("globals.New: %v", err)
}
log, err := logger.New(nil, logger.Params{Globals: g})
if err != nil {
t.Fatalf("logger.New: %v", err)
}
lifecycle := fxtest.NewLifecycle(t)
_, err = state.New(lifecycle, state.Params{
Logger: log,
Config: &config.Config{DataDir: dataDir},
})
if err != nil {
t.Fatalf("state.New: %v", err)
}
return lifecycle.Start(t.Context())
}
// TestStartupFailsWhenDataDirNotWritable verifies that startup stops
// with an error naming the data directory when it cannot be written.
// The directory's parent is a regular file, which also fails as root.
func TestStartupFailsWhenDataDirNotWritable(t *testing.T) {
t.Parallel()
parent := filepath.Join(t.TempDir(), "file")
err := os.WriteFile(parent, nil, 0o600)
if err != nil {
t.Fatalf("writing file: %v", err)
}
dataDir := filepath.Join(parent, "data")
err = startState(t, dataDir)
if err == nil {
t.Fatal("startup should fail when the data directory is not writable")
}
want := "data directory " + dataDir + " is not writable"
if !strings.Contains(err.Error(), want) {
t.Errorf("startup error %q does not contain %q", err, want)
}
}
// TestStartupFailsWhenExistingDataDirNotWritable verifies that startup
// stops when the data directory exists but the temp file that saving uses
// cannot be written in it. A directory sitting at the temp file's path
// makes that write fail, which also holds as root.
func TestStartupFailsWhenExistingDataDirNotWritable(t *testing.T) {
t.Parallel()
dataDir := t.TempDir()
err := os.Mkdir(filepath.Join(dataDir, "state.json.tmp"), 0o700)
if err != nil {
t.Fatalf("creating directory: %v", err)
}
err = startState(t, dataDir)
if err == nil {
t.Fatal("startup should fail when the data directory is not writable")
}
want := "data directory " + dataDir + " is not writable"
if !strings.Contains(err.Error(), want) {
t.Errorf("startup error %q does not contain %q", err, want)
}
}
// TestStartupCreatesDataDir verifies that startup creates a missing
// data directory and leaves nothing behind in it.
func TestStartupCreatesDataDir(t *testing.T) {
t.Parallel()
dataDir := filepath.Join(t.TempDir(), "data")
err := startState(t, dataDir)
if err != nil {
t.Fatalf("startup error: %v", err)
}
entries, err := os.ReadDir(dataDir)
if err != nil {
t.Fatalf("reading data directory: %v", err)
}
if len(entries) != 0 {
t.Errorf("startup left %d entries in the data directory", len(entries))
}
}
// TestPortStateUnmarshalJSON_NewFormat verifies deserialization of the // TestPortStateUnmarshalJSON_NewFormat verifies deserialization of the
// current multi-hostname format. // current multi-hostname format.
func TestPortStateUnmarshalJSON_NewFormat(t *testing.T) { func TestPortStateUnmarshalJSON_NewFormat(t *testing.T) {