d1a687de267af3212ea64c27c8ddffc43a41929e
120
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 sneak
|
d1a687de26 |
ci: a push cancels its branch's older run, checkout keeps no token (closes #216)
check / check (push) Canceled after 0s
Every push queues a run on the shared runner, and a branch pushed again left its older run queued for a head nobody needed. The workflow now puts each branch's runs in one concurrency group with cancel-in-progress, so a new push cancels that branch's older run, queued or running. The group is keyed on the branch, so pushes to other branches never cancel runs on `next` or `main`; a push to `next` itself does cancel the older `next` run. The checkout step no longer writes the token into `.git/config`; `script/cibuild` does not need it. Model: opus-5-5 |
||
 clawbot
|
82836b41fd |
resolver: try servers in a random order on each resolution (closes #138)
check / check (push) Successful in 1m42s
Every resolution walked the root servers in a fixed order, so a.root-servers.net got every first query and its timeouts were paid on every lookup. Each list of servers the resolver walks is now walked in a random order from rand.Shuffle, chosen anew each time; a server that does not reply, refuses, or gives an error reply or a referral that leads no closer is still passed over for the next. When a referral names a zone's nameservers without their addresses, all of them are now looked up, not only the first that resolves, so the zone is not given up because the first nameserver whose address was found gave no usable reply. No test fails if the walk stops shuffling: which server a live query reached is not observable. Model: opus-5-5 |
||
|
clawbot
|
af81f2ac76 |
README: correct claims the code does not bear out (closes #108)
check / check (push) Failing after 2m12s
Checked every README claim against the code on next and fixed the ones that were wrong or missing: what /metrics serves and when, what DNSWATCHER_MAINTENANCE_MODE does, CORS on the public routes, notification retries and the in-memory alert history, the certificate error field and old port entries in the state file, and the Design tree's missing files. Also corrected: CNAMEs are not followed for watched names, the root server list is never refreshed, the NS set is the delegation from the domain's parent zone, notification contents, and the system resolver being used for webhooks. Code problems found are filed separately. Model: opus-5-5 |
||
|
clawbot
|
56c4395a39 |
config: watch a target listed twice only once (closes #207)
check / check (push) Successful in 1m50s
ClassifyTargets kept a name every time it appeared in DNSWATCHER_TARGETS, so example.com,Example.com. put example.com in the domain list twice and every check looked it up and checked its certificates twice, sending two expiry warnings per TLS check (port checks were already grouped by address and port). It now skips a name it has already kept, comparing after the lower-casing and trailing-dot removal it already did; the list keeps the order of first appearance. Model: opus-5-5 |
||
|
clawbot
|
c9510a986c |
watcher: warn of an expiring certificate on every TLS check (closes #204)
check / check (push) Failing after 2m1s
An expiry warning was skipped when the last one for that hostname and address was sent less than DNSWATCHER_TLS_INTERVAL ago. Each TLS check runs after a DNS pass of varying length, so two checks can be less than the interval apart, and a certificate about to expire was warned about on every check or every other check, at random. TLS checks already start once per interval, so the in-memory record of when each warning was sent is removed and every check warns, as the README says. The test that expected the second check to stay silent is replaced by one that runs TLS checks on state built in the test, with no DNS. Model: opus-5-5 |
||
|
clawbot
|
1f1640d4cd |
resolver: take a domain's NS set from its delegation (closes #200)
check / check (push) Failing after 2m2s
A domain's NS set was taken from whichever of its own servers answered first, so when they disagree (during a move between DNS providers, or with a stale secondary) the set could change between checks and send an NS change notification with nothing changed. The walk now stops at the referral to the domain from its parent zone's servers and returns that delegation, which those servers all hold alike; the domain's own servers are no longer asked for it. The NS records in an answer are still used where no such referral comes first, as from a server that holds both the parent zone and the domain. Hostnames get their zone's servers the same way. Model: opus-5-5 |
||
|
clawbot
|
11ce1b249b |
docs: add the README sections policy requires (closes #173)
check / check (push) Failing after 2m30s
REPO_POLICIES.md requires Getting Started, Rationale, Design and TODO sections in the README, and it had none of them. Getting Started clones the repository, builds the image and runs it watching example.com and www.example.com; DNSWATCHER_TARGETS is the only setting it requires. Rationale is drawn from what the README already says. The Architecture section moves below Entrypoints and is renamed Design, its text unchanged, so the required sections come in policy order. TODO points to TODO.md and the 1.0 milestone instead of copying the list. Model: opus-5-5 |
||
|
clawbot
|
d09822562d |
resolver: pass over a server that answers SERVFAIL or refers no closer (closes #197)
check / check (push) Failing after 2m8s
When the resolver walks from the root servers towards a name, a server that answered SERVFAIL, or referred the query back to its own zone, up or sideways, ended the step, so finding a zone's servers gave up on the zone though its other servers would answer. Such a reply is now passed over for the zone's next server, as a timeout or a refusal already was. To tell a referral that leads closer to the name from one that does not, each walk keeps the zone of the servers it is asking. Other error replies, such as FORMERR, are passed over too. The walk that finds a nameserver's address shares the same server loop, so it changes too. Model: opus-5-5 |
||
|
clawbot
|
97c8138c85 |
watcher: keep port state when no nameserver of a name answered (closes #193)
check / check (push) Failing after 2m22s
The port check removed the saved port state of every address no configured name resolves to. A name whose nameservers all timed out or failed is saved with no records, so its addresses looked gone and lost their port state; when the nameservers answered again it was recorded afresh, and a port that opened or closed meanwhile was not notified. An entry is now kept when one of the names saved on it is configured and none of its nameservers answered on its last check, and such a name stays on the entry when the port is checked again for another name. A name whose nameservers answer with no addresses still loses it, and so does a name no longer configured. Model: opus-5-5 |
||
|
clawbot
|
d2f154b2cf |
resolver: error from ResolveIPAddresses when no nameserver answered (closes #190)
check / check (push) Failing after 2m7s
ResolveIPAddresses now returns an error, not no addresses, when no nameserver of the name's zone answered. A nameserver with status timeout or error is not an answer; one answer, even NXDOMAIN, is enough for an empty result without an error. When every server of a zone fails, FindAuthoritativeNameservers moves on to the parent name, whose servers only refer the query onward. Such a referral now has status error, so it is no answer either, and a hostname's saved records show it as error. The only caller, the nameserver address lookup, already keeps the previous addresses on an error; its comment no longer says the resolver hides this case. Model: opus-5-5 |
||
|
clawbot
|
4c2932d6d6 |
fmt: format and check Markdown with prettier in a container (closes #119)
check / check (push) Failing after 2m11s
make fmt and make fmt-check now cover every Markdown file with prettier (4-space tabs, proseWrap always), as template-app-go does: prettier is pinned by package.json and yarn.lock and runs in a docker build on a digest-pinned node image, never on the host. The check is forced to run with --no-cache-filter, as script/lint is. script/fmt-check is split into a Go half and a Markdown half because the Dockerfile lint stage cannot run docker: that stage now runs the Go half and script/cibuild runs the Markdown half after the build. *.md leaves .dockerignore so documents reach the build context. README.md, TESTING.md and TODO.md are reformatted by make fmt; apart from the README Entrypoints entries, its make fmt line under Building and the TODO entry, that diff is mechanical. Model: opus-5-5 |
||
|
clawbot
|
b5814b2451 |
fmt: check goimports in fmt-check, run it at its pinned commit (#119)
check / check (push) Failing after 2m28s
script/fmt-check now runs goimports in list mode and fails naming any file it would change, and checks gofmt with -s, as script/fmt applies it. Both scripts run goimports with `go run` at the commit that script/bootstrap used to install, so a goimports on PATH is never used and bootstrap no longer installs it. The pin is written in both scripts; change them together. The first run on a machine, and every Dockerfile lint stage run, downloads and builds goimports. The Markdown half of the issue (prettier) is not done here: it needs node in the lint image or a separate build, a decision for the owner. Model: opus-5-5 |
||
|
clawbot
|
797c936c48 |
resolver: query a hostname at its own zone's servers (closes #189)
check / check (push) Failing after 2m4s
A hostname's nameservers came from its last two labels, so a name under co.uk was asked at the co.uk servers and a name in a delegated subdomain at the parent's servers; both only refer onward. The hostname now goes through FindAuthoritativeNameservers, which follows delegations for the name and walks up its labels until it finds the zone it is in. followDelegation now stops at an authoritative reply: that server holds the zone, so its reply is not a referral. Without this, a CNAME answer that also lists the zone's NS records in its authority section, as many servers send, was followed as a referral until the delegation limit. Model: opus-5-5 |
||
|
clawbot
|
c247f6bcf5 |
watcher: notify nameserver address changes (closes #105)
check / check (push) Failing after 2m11s
Each domain check now looks up the addresses every nameserver's name resolves to, with the resolver's ResolveIPAddresses, and saves them sorted in the domain's state. A nameserver that stays in the delegation and resolves to different addresses sends one NS Address Change notification naming the domain, the nameserver and the old and new addresses. Added or removed nameservers get only the NS change notification. A failed or empty lookup keeps the previous addresses, because the resolver returns no address without an error when every server it asks times out. State files without the field load, and the next check fills it in silently. Watcher tests that run domain checks use example.com, which has two nameservers, to stay within the per-attempt limit. Model: opus-5-5 |
||
|
clawbot
|
f6567df2d0 |
watcher: save state when it stops and wait for that save (closes #114)
check / check (push) Successful in 1m24s
The final save at shutdown came from the state's own stop hook, while the watcher's stop hook only cancelled its run loop, so a check under way could change state after that save or be cut off at exit. Run now saves state as it returns, and the watcher's stop hook waits for Run, bounded by the shutdown deadline. The state's own save stays; Save holds the state lock for the whole write, so the two cannot overlap. The start hook derives the watcher's context with WithoutCancel, so the linter needs no exception. A new test stops a watcher built by New and reads the change back from the state file, with no DNS. Model: opus-5-5 |
||
|
clawbot
|
c0ea9b96f2 |
server: report HTTP handler panics to Sentry (closes #107)
check / check (push) Successful in 1m35s
DNSWATCHER_SENTRY_DSN was read but never used. This ports the Sentry integration from gohttpserver with sentry-go v0.49.0. The server's start hook calls sentry.Init when the DSN is set; a DSN Sentry cannot parse fails the hook, so startup stops with the parse error. sentryhttp, with Repanic, reports handler panics and passes them on to chi's Recoverer. Shutdown sends queued reports once the HTTP server has stopped. The client uses the older transport (DisableTelemetryBuffer): with the default one, Flush can return before sending a report made just before it. Client reports are off, so only panics are sent. The DSN is checked at server start, not in config, so the config test is unchanged. sentry-go raises several golang.org/x modules and moves go-spew and go-difflib to untagged commits. Model: opus-5-5 |
||
|
clawbot
|
fe01cdda1e |
watcher: save and notify nothing for a cut-short port or TLS check (closes #185)
check / check (push) Successful in 1m8s
When shutdown cancels a check that is under way, the rest of the check still runs with the cancelled context. The resolver already drops a lookup the context cut short, but a cancelled connection attempt was saved as a closed port or a failed certificate check and notified as Port Change or TLS Failure. The watcher now drops a port or TLS check result when its context was cancelled, the same way. The test runs a check with the context already cancelled, using the real resolver and the real port and TLS checkers; no query is sent and no connection is made. Model: opus-5-5 |
||
|
clawbot
|
a8f9a64600 |
middleware: take the client address from the right of X-Forwarded-For (closes #181)
check / check (push) Successful in 1m22s
realIP took the first X-Forwarded-For entry, which the client itself can write, so behind a proxy that appends to the header a client chose the address dnswatcher logs and the /metrics rate limit counts. It now walks the entries from the right past trusted proxies, using the existing trusted-proxy check, and takes the first that is not one; the leftmost when all are. All X-Forwarded-For header lines are read as one list, since a proxy may add its own line instead of appending to the client's. An empty entry where the client address belongs falls back to the peer address, as an empty first entry did before. X-Real-IP is unchanged. Model: opus-5-5 |
||
|
clawbot
|
8f11ef0038 |
watcher: notify NS query failure and recovery (closes #104)
check / check (push) Successful in 1m31s
LookupAllRecords now returns each nameserver's response, so the watcher saves its status: ok when it answered, NXDOMAIN and no records included, and error with the reason when it timed out, answered SERVFAIL or REFUSED, or could not be reached. A nameserver that starts failing sends NS Failure and one that answers again sends NS Recovery. A failing nameserver is left out of the record change and inconsistency comparisons. The resolver used to report REFUSED and network errors as an answer with no records; they are now errors. A lookup cut short by its context now returns an error instead of a failure of the nameserver it was querying. Model: opus-5-5 |
||
|
clawbot
|
fcd4f7e2c2 |
config: stop startup on an invalid DNS or TLS interval (closes #177)
check / check (push) Successful in 1m3s
DNSWATCHER_DNS_INTERVAL and DNSWATCHER_TLS_INTERVAL were parsed with time.ParseDuration and silently replaced by the default when that failed, so a value like 5 or 1d gave hourly checks with no hint why, and zero or negative values were accepted. Both now go through parseInterval, which returns an error naming the variable and the value, and startup stops the same way it does for invalid targets. An unset or empty variable still gets its default from setupViper. The three tests that pinned the old fallback are replaced. The README says what a valid value looks like. Model: opus-5-5 |
||
|
clawbot
|
ed0f56f144 |
metrics: rate limit /metrics per client address before Basic Auth (closes #101)
check / check (push) Successful in 1m18s
/metrics is behind a password, and REPO_POLICIES.md requires rate limiting on password logins. Each client address may now send it 30 requests a minute, counted by httprate before Basic Auth, so failed logins use up the allowance and a request over it gets 429 without the password being checked. The address is the one the existing trusted-proxy logic in internal/middleware works out, with IPv6 addresses grouped by /64; an IPv4 address a proxy reports in IPv6-mapped form counts as the plain IPv4 address. A Prometheus server scraping every 15 seconds sends 4 requests a minute. Model: opus-5-5 |
||
|
clawbot
|
bea9a3b2f2 |
docker: report the real version in the image (closes #109)
check / check (push) Successful in 1m17s
The Dockerfile builder stage takes ARG VERSION (default `dev`) and passes it to `make build` on the command line, which overrides the Makefile's `git describe` default. script/docker computes the version from `git describe` on the host and passes it as --build-arg VERSION, because .dockerignore leaves .git out of the build context and `git describe` inside the build only ever produced `dev`. A build that passes no argument, such as script/cibuild, still reports `dev`. `logger.Identify`, which logs `starting` with the version, was never called; `main` now calls it first, so the version is in the startup log. Model: opus-5-5 |
||
|
clawbot
|
e93c2664b8 |
notify: release held deliveries so shutdown tests fail, not hang (closes #176)
check / check (push) Successful in 1m5s
Two shutdown tests hold a delivery inside the test server's handler and release it from a timer. The deferred timer stop ran before the server was closed, so a drain that returned early left the handler blocked and the server's close waited on it until the package timed out. Each test now defers a release, guarded so the timer and the defer can both call it, ahead of closing the server. The watchdog comment no longer names a 30-second timeout the test script does not use. Model: opus-5-5 |
||
|
clawbot
|
bde047f2a3 |
script/install-precommit: work where .git is a file (closes #129)
check / check (push) Successful in 1m9s
The script wrote the hook to .git/hooks, which fails when .git is a file rather than a directory, as in a clone made with --separate-git-dir. It now asks git for the repository's own git directory with `git rev-parse --git-common-dir`, creates its hooks directory if missing, and writes the hook there. In an ordinary clone that is .git/hooks, so nothing moves. Before writing anything it stops with an error when its top directory is not the top of the checkout git finds, so a copy inside another repository cannot replace that repository's hook. git's core.hooksPath setting is not followed; where it is in force, git does not run the installed hook, as before. Model: opus-5-5 |
||
|
clawbot
|
6070356676 |
docs: bring TODO.md up to date (closes #146)
check / check (push) Successful in 1m6s
Next Step and Future Steps list the open issues by full URL, in the order of the review on #144; issues the review does not name sit next to the entries they relate to, and #144 itself comes last. Left out: #146, which this change closes; #59, DNSSEC, ruled post-1.0; issues assigned to sneak, which wait on the owner. Shipped work, the dropped domains and hostnames endpoints and the line about mocked resolver tests are gone. Every Completed Steps entry is at most two lines; none was dropped. Workflow branches from `next` and opens PRs against it. Wrapped by hand at 80 columns: `make fmt` does not format Markdown yet (#119). The old Next Step is now #173. Model: opus-5-5 |
||
|
clawbot
|
3390d7065e |
docker: set up the data directory in an entrypoint (closes #166)
check / check (push) Successful in 1m16s
The runtime image no longer sets USER. Its new entrypoint, deploy/docker-entrypoint.sh, runs as root: it creates the data directory if needed, gives it and everything in it to the dnswatcher user (uid 10001) with mode 700 on the directory, then runs dnswatcher as that user with su-exec. A bind-mounted host directory, whether empty and root-owned or holding a state file left by another uid, no longer has to be chowned first, and the README's upaas section now says only which path to mount. The startup check that the data directory is writable stays. Model: opus-5-5 |
||
|
clawbot
|
f7cc6b42e0 |
server: limit wildcard CORS to the public routes (closes #100)
check / check (push) Successful in 1m13s
The CORS wildcard was global, so it also covered the Basic-Auth protected /metrics, which REPO_POLICIES.md forbids, and it allowed POST, PUT and DELETE, which no route serves, plus the Authorization and X-CSRF-Token headers. CORS now sits on a router holding only the public routes and allows GET and OPTIONS with the Accept and Content-Type headers. /metrics gets no CORS at all. Both are mounted routers rather than a Group: chi answers OPTIONS on a Group's route with 405 before its middleware runs, and any method /metrics does not register would otherwise fall through to the public router. So every method on /metrics now meets Basic Auth first, and /metrics/ is served like /metrics. Model: opus-5-5 |
||
|
clawbot
|
db94c903df |
tests: move the test-only constructors into export_test.go (closes #111)
check / check (push) Successful in 1m37s
state.NewForTest, state.NewForTestWithDataDir and watcher.NewForTest were compiled into and exported by the production packages internal/state and internal/watcher. NewForTestWithDataDir and watcher.NewForTest now live in their package's export_test.go. Tests in other packages cannot see those files, so the watcher and middleware tests build their State with state.New and a temporary data directory; the watcher tests no longer try to save to /state.json. state.NewForTest, whose State saved to /, is deleted: the state tests that used it pass t.TempDir() to NewForTestWithDataDir, and the test of the helper itself is gone. Model: opus-5-5 |
||
|
clawbot
|
5493e28480 |
notify: make the shutdown tests fail with the right message (closes #116)
check / check (push) Successful in 1m27s
drainSlack stood for three things: the deadline given to a drain that should finish early, the watchdog on a drain that should time out, and the wait for a delivery to reach the test server. It is now three constants, each commented with what it bounds and why it is 2s; no value changed. The idle-drain failure printed that deadline instead of idleDrainBound, the bound it checks. The cancelled-context test now also requires the drain to return within idleDrainBound and to log its debug line, so a drain that logs nothing no longer passes; newLoggingService records debug level for this. Model: opus-5-5 |
||
|
clawbot
|
651429137f |
tests: rename internal/livedns to livednstest and deny it outside tests (closes #164)
check / check (push) Successful in 1m39s
The live-DNS retry and concurrency limit is only for tests, but nothing stopped program code from importing it and compiling it into the binary. Its directory name now ends in test, and its import path is on the test-support deny list in .golangci.yml, so make lint fails when program code imports it. Every import and mention is updated to the new name. Model: opus-5-5 |
||
|
clawbot
|
93c1fe15e3 |
golangci: re-vendor the org config with gomodguard_v2 (closes #123)
check / check (push) Successful in 1m10s
The org .golangci.yml now uses gomodguard_v2 in place of the deprecated gomodguard, which made every lint run print a deprecation warning. The file is copied unchanged from sneak/prompts. It also turns on depguard with the org test-support rule, which rejects net/http/httptest except in test files and in files under a directory whose name ends in test. This repo's previous copy had no deny entries of its own, so there were none to carry forward. Model: opus-5-5 |
||
|
clawbot
|
6dd6043534 |
tests: remove the DNS stand-ins from the watcher and resolver tests (closes #159)
check / check (push) Successful in 1m21s
The watcher tests used a stand-in resolver and the resolver timeout test a stand-in DNS client, against the rule that DNS is never mocked. Watcher tests that look something up in DNS now run the real resolver against live servers, each attempt on a new watcher. A DNS change is tested by saving values live DNS never returns (names under .invalid, 192.0.2.1) in the state a check starts from, or by marking a real nameserver failed. The timeout test queries 192.0.2.1, where nothing answers. The live-DNS retry and concurrency limit moved from the resolver tests to internal/livedns, so both packages share them. NewFromLoggerWithClient had no other use and is gone. TESTING.md now states the README's rule. Model: opus-5-5 |
||
|
clawbot
|
a93389e1a0 |
watcher: send the inconsistency alert once per disagreement (closes #158)
check / check (push) Successful in 1m5s
detectInconsistencies alerted for neighbouring pairs of nameservers whose records differed, on every DNS check, for as long as they differed. It now also takes the previous hostname state, compares every pair of nameservers, and alerts for a pair that differs unless both were in that state and already differed there, so a nameserver new on a check that answers differently is reported once. The state loaded at startup is the previous state for the first check, so a disagreement saved before a restart is not reported again. The choice of pairs is tested on record data, and the alert through the hostname change detection with the notifier stand-in and no resolver. The README describes the new behaviour. Model: opus-5-5 |
||
|
clawbot
|
95b017eb3e |
resolver: lower-case DNS names in record values (closes #157)
check / check (push) Successful in 58s
Nameservers may answer with DNS names in any letter case. For eeqj.de, y.ns.joker.com answers in upper case while its peers answer in lower case, so the inconsistency check fired on every cycle. extractRecordValue now lower-cases CNAME, MX, SRV and NS targets, so the inconsistency check and the record-change check both compare names regardless of case. A, AAAA, TXT and CAA values are formatted as before. State saved before this change can hold upper-case names, which report a one-time record change on the first check after upgrading. Model: opus-5-5 |
||
|
clawbot
|
1ab0b9f61d |
script: force lint and test to run in cibuild and docker (closes #115)
check / check (push) Successful in 59s
script/cibuild and script/docker were plain docker build. On an unchanged tree the lint stage and the builder stage, which runs make test, came from the layer cache, so the build passed without linting or querying live DNS. Both scripts now pass --no-cache-filter=lint,builder so those stages run on every build, as script/lint already does for its own lint stage. Dependency downloads inside those stages re-run each build. Each of the two stages in the Dockerfile now notes that the scripts name it. README and TODO.md updated to match. Model: opus-4-8 (implementation); opus-5-5 (rework) |
||
|
clawbot
|
19f282c8b3 |
server: assert timeouts on the served http.Server, not just the constructor (closes #120)
check / check (push) Successful in 6s
The timeout tests called newHTTPServer directly, so a Run that built its http.Server inline would drop every timeout with the suite still green. TestRunWiresSocketTimeouts wires a Server as cmd/dnswatcher does, minus the watcher and resolver so no live DNS is touched, drives Run with an unbindable port so it stores its http.Server and returns without listening, and checks that server carries all four timeouts and both required relationships. The addr/handler test, which could not fail, is dropped. The ReadTimeout note now says what net/http does: a request whose headers arrive after ReadTimeout but within ReadHeaderTimeout gets a read deadline that has already passed, so reading its body fails at once. Model: opus-4-8 (implementation); opus-5-5 (rework) |
||
|
clawbot
|
148e47d9c0 |
docker: run as non-root, add health check, document upaas (closes #147)
check / check (push) Successful in 5s
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 |
||
|
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) |
||
|
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 |
||
|
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 |
||
 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 |
||
|
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 |
||
|
clawbotandsneak
|
e77e206fc4 |
next (#136)
check / check (push) Successful in 6s
Long-lived integration branch. One commit per work unit lands here; this PR accumulates them until it is merged to `main`. ## Landed units - **Run all linting in Docker via `Dockerfile.lint` + `script/lint`** — #134 golangci-lint is no longer installed or run on the host. New root `Dockerfile.lint` 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; `script/lint` is reduced to a thin wrapper that builds it. This also works where the docker daemon is remote and bind mounts are impossible. **Pinned digest and how it was verified.** `golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240`, exactly as quoted in the issue. It resolves, and it is genuinely v2.12.2: ``` $ docker buildx imagetools inspect golangci/golangci-lint:v2.12.2 Name: docker.io/golangci/golangci-lint:v2.12.2 MediaType: application/vnd.oci.image.index.v1+json Digest: sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 $ docker run --rm golangci/golangci-lint@sha256:5cceeef04e...ad5240 golangci-lint --version golangci-lint has version 2.12.2 built with go1.26.2 from c0d3ddc9 on 2026-05-06T11:07:58Z ``` The tag's index digest is the quoted digest, and the binary inside reports commit `c0d3ddc9`, matching the org's canonical pin `c0d3ddc9cf3faa61a4e378e879ece580256d76e5`. **Forcing the linter to actually run.** `Dockerfile.lint` is split into a `deps` stage (base image + `go mod download`) and a `lint` stage (source copy + linter run). `script/lint` runs: ``` docker build --progress=plain --no-cache-filter=lint --target lint -f Dockerfile.lint . ``` Caching is explicitly waived for linting, and a cached build lints nothing, so the `lint` stage is invalidated on every invocation. The invalidation is scoped: the `deps` stage stays cached and no global cache wipe is performed. `--progress=plain` keeps the linter's own output visible. **`golangci-lint config verify`: deliberately NOT included.** It fetches its JSON schema over a live, unpinned HTTPS call, which would make linting network-dependent and defeat hash-pinning. Omitted for that reason, and the reason is recorded in a comment at the top of `Dockerfile.lint`. **`script/bootstrap`** no longer installs golangci-lint (and its pinned ref is gone); it warns non-fatally when `docker` is absent instead. The `goimports` install stays, because `script/fmt` and `script/fmt-check` still run on the host. Header comment updated accordingly. **Root `Dockerfile`** — required consequence, not scope creep. Its builder stage ran `make check`, which now calls `script/lint`, which shells out to `docker build`; there is no docker daemon inside a docker build, so `script/cibuild` and `script/docker` would have broken. It gains its own lint stage on the same pinned image (linter invoked directly, with a comment explaining why not `make lint`), with the builder stage depending on it via `COPY --from=lint /src/go.sum /dev/null` and running `make fmt-check`, `make test`, `make build`. The now-unneeded golangci-lint install is gone from the builder stage. **README** `Entrypoints` and `Building` sections now describe linting as a docker-only operation. `TODO.md` updated in the same commit. ### Verification All runs via `make` / `script/` entrypoints only. Two consecutive `make lint` runs on an unchanged tree, both executing the linter: ``` # run 1 #10 [lint 2/2] RUN golangci-lint run --config .golangci.yml ./... #10 10.98 0 issues. #10 DONE 12.0s # run 2, tree untouched #10 [lint 2/2] RUN golangci-lint run --config .golangci.yml ./... #10 14.63 0 issues. ``` A third run shows the cache scoping is working as intended — `deps` served from cache, `lint` re-executed: ``` #6 [deps 2/4] WORKDIR /src #6 CACHED #7 [deps 3/4] COPY go.mod go.sum ./ #7 CACHED #8 [deps 4/4] RUN go mod download #8 CACHED #9 [lint 1/2] COPY . . #10 [lint 2/2] RUN golangci-lint run --config .golangci.yml ./... ``` **Negative control.** A deliberate violation (an unused function containing an ineffectual assignment) was added to `internal/config/config.go`: ``` #10 11.26 internal/config/config.go:29:2: ineffectual assignment to x (ineffassign) #10 11.26 internal/config/config.go:28:6: func negativeControlUnused is unused (unused) #10 11.26 2 issues: #10 ERROR: process "/bin/sh -c golangci-lint run --config .golangci.yml ./..." did not complete successfully: exit code: 1 ERROR: failed to build: failed to solve: process "/bin/sh -c golangci-lint run --config .golangci.yml ./..." did not complete successfully: exit code: 1 make: *** [Makefile:26: lint] Error 1 ``` `make lint` exited non-zero naming both findings and their exact lines. After reverting the file, `make lint` was clean again (`0 issues.`). **`make check`** green end to end (test, lint, fmt-check), exit 0. **`script/cibuild`** green, confirming the `Dockerfile` restructure does not recurse: the lint stage ran (`#16 12.56 0 issues.`), then `#22 [builder 8/9] RUN make test` with `PASS` lines, then `#23 [builder 9/9] RUN make build`. ### Notes for the owner This supersedes two PRs you still have queued for merge, both of which tune host linting that no longer exists after this change: #128 (isolates the host golangci-lint cache and lock) and #131 (always installs the pinned lint tools in `script/bootstrap`). Neither was merged or incorporated here. Made moot by this change: #121 and #130. ### Review outcome Reviewed at #136 (comment) — **PASS**. Three non-blocking comment-accuracy findings were recorded there for the next touch of those files; the reviewer independently confirmed the lint gate is live by negative control from a warm cache. - **Live DNS tests made robust instead of gated; test caps moved to the org-wide 60s/20s/90s values** — #93 The resolver's live-DNS tests failed nondeterministically, a different subset each run. Fixed by engineering the nondeterminism out, not by routing around the network. **Nothing is mocked, faked, stubbed, recorded or replayed; there is no `-short` flag, no build tag, no skip, and no environment-tolerance for restricted egress.** Production resolver behaviour is unchanged. ### Root causes, all test-side 1. **Burst fan-out at one root server.** Every test in `internal/resolver` calls `t.Parallel()` and the build hosts have many cores (48 here), so all ~35 iterative resolutions started within milliseconds of each other, and because `queryServers` walks `rootServerList()` in fixed order they all aimed their first query at `198.41.0.4`. Root servers rate-limit that, which fits the reported symptom of a different arbitrary subset failing each run. 2. **No retry anywhere.** One dropped UDP packet in a delegation chain failed a test outright. 3. **Unanimity assertions.** `TestQueryAllNameservers_AllReturnOK` and `_NXDomainFromAllNS` required *every* nameserver of a domain to answer — four independent chances to fail per run, with no tolerance for one being slow. ### What was built New `internal/resolver/livedns_test.go` holds all the live-DNS machinery, so `resolver_test.go` itself takes only call-site edits: - **Bounded live concurrency.** A package-wide semaphore (`liveConcurrency = 6`) caps how many live resolutions are in flight at once. Tests keep `t.Parallel()`; only their network work is throttled. This is the direct fix for cause 1, and the 60s budget is what makes it affordable. - **Retry with exponential backoff.** Three attempts per live operation, 8s deadline each, 500ms base backoff doubling. The retry predicate is deliberately **transport-level** — "did a nameserver answer at all" — and never the assertion the test is making, so a resolver that answers *incorrectly* still fails on the first attempt rather than being retried into a false green. - **Quorum instead of unanimity, tolerating SILENCE ONLY.** A strict majority of the discovered nameservers must answer as expected, and every individual result must additionally fall inside a closed **allowlist** of statuses the test explicitly sanctions: `ok`/`timeout`/`error` for the all-OK test, `nxdomain`/`timeout`/`error` for the NXDOMAIN test. A nameserver that stays silent is tolerated; one that answers **wrongly** is not, at any count. The allowlist is the load-bearing part — see the rework note below for why a blocklist was not enough. New `internal/resolver/livedns_harness_test.go` tests that machinery directly — quorum arithmetic, status counting, the allowlist, the gate's concurrency bound, per-attempt deadlines, and recovery from a transient failure. It performs no DNS resolution of any kind, so it neither mocks DNS nor depends on it. ### Rework after review — the quorum could not fail on a wrong answer The review at #136 (comment) returned **FAIL** on `9cb2c2b`, correctly. Fixed in `87bce43`. **The defect.** The claim above was, as first written, false for `resolver.StatusNoData`. Each test banned exactly one wrong status — `_AllReturnOK` banned only `nxdomain`, `_NXDomainFromAllNS` banned only `ok` — and `nodata` is neither. It is a **wrong answer, not silence**: `answeredCount` counted it as answered, so it did not even trigger a retry, and with a quorum of 3-of-4 a single wrong nameserver slid through undetected. The pre-change unanimity assertions would have caught it. That is robustness work quietly becoming assertion-loosening, which is exactly what this repo cannot afford. **The fix.** Tolerance is now an allowlist, not a blocklist of one status. New `unsanctionedStatuses()` returns every per-nameserver result whose status the caller did not explicitly sanction, and each test asserts that list is empty in addition to its quorum. A blocklist bans the one wrong answer its author thought of and silently admits everything else, including any status added to the resolver later; an allowlist fails on anything nobody sanctioned. `answeredCount` was reframed the same way — it now counts the closed set `ok`/`nxdomain`/`nodata`, so an unfamiliar status is treated as silence and can only ever cause a retry and then a loud failure, never a quiet pass. **Evidence — the reviewer's exact probe, re-run.** `queryEachNS` in `internal/resolver/iterative.go` was patched to force one of `google.com`'s four nameservers to return `StatusNoData` with empty records. `make test` now goes **red**, naming the offending nameserver and status: ``` exit=2 --- FAIL: TestQueryAllNameservers_AllReturnOK (1.12s) Error: Should be empty, but was [ns1.google.com.=nodata] Messages: 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) Error: Should be empty, but was [ns1.google.com.=nodata] Messages: 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 FAIL sneak.berlin/go/dnswatcher/internal/resolver 1.704s ``` That is the same input that returned `exit=0` with both tests **passing** under the old assertions. Probe reverted, tree clean, suite green again: ``` exit=0 ok sneak.berlin/go/dnswatcher/internal/resolver 4.185s coverage: 77.1% of statements (zero `(cached)` lines) ``` Two harness tests lock the regression in without any probe: three OK plus one `nodata` (quorum satisfied, no NXDOMAIN present — the exact input that used to pass) is reported as unsanctioned, and an unknown status is neither counted as answered nor tolerated. Also fixed from the review: the per-attempt deadline assertion in `livedns_harness_test.go` had no lower bound, so it passed for a deadline far shorter than intended. It now asserts the remaining time exceeds `liveAttemptTimeout/2` as well. Deliberately **not** done in this rework, per the review and the owner: no `-count=1` in `script/test` (the test-cache issue is real but pre-existing and repo-wide, filed separately); the remaining non-DNS mocks stay for #97; `queryServers` root-ordering stays untouched under #138. ### The `-timeout` backstop value: 90s Per the ruling at sneak/prompts#41 (comment) the cap is org-wide with two tiers: **60s hard cap for CI green, 20s target, and anything between the two must be filed as an improvement bug.** The backstop is **`90s`**, matching sneak/prompts#42 and preserving the 1.5x backstop-to-cap ratio the old 20s/30s pair had. It must strictly exceed the 60s cap or the cap is unreachable — the old `-timeout 30s` would have killed a 60s-capped suite at half its allowance. Applied to `script/test`; nothing else in the repo carried the old `30s`. Worst case for one live operation is 3 attempts x 8s plus ~1.5s of backoff, about 26s — comfortably inside the 90s backstop even if several operations exhaust their attempts at once. ### `REPO_POLICIES.md` is re-vendored, not hand-edited The file is org-canonical, so it was **copied byte-for-byte** from `prompts/REPO_POLICIES.md` on `sneak/prompts` branch `org-wide-60s-test-cap` (commit `52b5192`) rather than reworded to approximately the same thing. Verified: ``` $ cmp prompts/REPO_POLICIES.md dnswatcher/REPO_POLICIES.md && echo identical identical $ sha256sum REPO_POLICIES.md bcf11c312a1bee18a0e937eb412b51914411c1ab23308b8362409f3f88379ff7 ``` **What that byte-identity does and does not certify.** The source branch `org-wide-60s-test-cap` is an **unmerged proposal** — sneak/prompts#42 — not `prompts` `main`. So, precisely: - The **60s hard cap and 20s improvement-bug tier ARE the owner's ruling** (sneak/prompts#41 (comment)). - The **`90s` backstop is our own proposed number and is NOT ratified** (sneak/prompts#41 (comment)). - The vendored text is therefore **the proposed canonical text, pending** sneak/prompts#42. If that PR lands with different numbers, this file must be re-vendored to match; it should not be hand-edited here either way. **Known mismatch with this repo's actual state, recorded not papered over.** Re-vendoring also picked up the paragraph at `REPO_POLICIES.md:266-271` mandating that canonical golangci-lint be installed commit-pinned via `go install ...@c0d3ddc9...`. This repo does **not** comply with that mechanism: `cc86473` in this same PR made linting Docker-only, and `script/bootstrap` now installs golangci-lint nowhere. The **version and commit match** (`v2.12.2` / `c0d3ddc9`); the **installation mechanism does not**. The vendored file is org-canonical and must not be edited downstream, so this is being raised upstream for the org text to accommodate Docker-only linting rather than patched here. `TESTING.md`'s stale "within the 30-second target" follows to 60. That edit is deliberately a single line so it merges cleanly when #97 lands. ### Verification All runs through `make` / `script/` entrypoints only; lint runs in Docker. **Ten consecutive `make check` runs, all green, none served from cache.** Go's test cache will happily report `ok pkg (cached)` without executing anything, which proves nothing about nondeterminism, so every run was forced to actually execute and each log was checked for zero `(cached)` lines: ``` check#1 exit=0 wall=32s resolver=2.895s fails=0 cached=0 check#2 exit=0 wall=27s resolver=2.872s fails=0 cached=0 check#3 exit=0 wall=47s resolver=2.729s fails=0 cached=0 check#4 exit=0 wall=36s resolver=2.885s fails=0 cached=0 check#5 exit=0 wall=32s resolver=2.899s fails=0 cached=0 check#6 exit=0 (harness tests added) fails=0 cached=0 check#7 exit=0 wall=41s resolver=2.891s fails=0 cached=0 check#8 exit=0 wall=36s resolver=2.871s fails=0 cached=0 check#9 exit=0 wall=26s resolver=2.846s fails=0 cached=0 check#10 exit=0 wall=46s resolver=2.833s fails=0 cached=0 ``` After the rework commit `87bce43`, `make check` green again end to end, zero `(cached)` test lines, Docker lint stage demonstrably executed rather than served from cache: ``` exit=0 cached=0 #8 [deps 4/4] RUN go mod download #8 CACHED #10 [lint 2/2] RUN golangci-lint run --config .golangci.yml ./... #10 30.72 0 issues. ``` The Docker lint stage was confirmed to execute rather than cache on each run: ``` #8 [deps 4/4] RUN go mod download #8 CACHED #9 [lint 1/2] COPY . . #9 DONE 0.6s #10 [lint 2/2] RUN golangci-lint run --config .golangci.yml ./... #10 DONE 39.7s ``` **`make test` wall time: 3.6-4.1s** across three timed uncached runs (`4093ms`, `3649ms`, `3729ms`). `internal/resolver` went from 2.0s to ~2.9s — the concurrency gate's cost. That is inside the 20s target, so no improvement bug is owed under the new two-tier rule. **Honest note on what these runs do and do not prove.** Live DNS was healthy throughout: **no live-DNS retry fired even once**, and no flake was observed either before or after the change (six pre-change baseline runs were also clean). So these runs demonstrate the change is not itself flaky and does not slow the suite; they do **not** demonstrate recovery from a real DNS failure, because no real DNS failure occurred. The original flakiness is *not reproduced* rather than *shown fixed*. The retry path is instead proven by `TestRetryLiveRecoversFromTransientFailure`, the only source of the single `retrying in 500ms` line in each log: ``` livedns_harness_test.go:90: transient: attempt 1 of 3 failed (no answer from live DNS), retrying in 500ms ``` ### Observation, not acted on The single most effective remaining lever against root-server rate limiting would be to stop `queryServers` always trying `rootServerList()` in the same order, so that load spreads across all thirteen roots instead of concentrating on `a.root-servers.net`. That is **production** code and this issue scopes the work as test-side, so it was left alone rather than changed quietly. It is now tracked for the owner's decision at #138. ### Interaction with #97 `TESTING.md` and `internal/resolver/resolver_test.go` auto-merge — that PR touches `resolver_test.go` only at the import block and the final timeout-test section, while this change touches the body of the file and adds two new files, and it leaves the mock-`DNSClient` timeout test at the tail of `resolver_test.go` entirely alone since removing it is that PR's job. `TODO.md` does conflict; that PR is already labelled `needs-rebase`, so this adds nothing material to its rebase. - **Go's test cache disabled, so every `make test` actually queries live DNS** — #139 `script/test` did not pass `-count=1`, so on an unchanged tree Go served the whole suite from cache: exit 0 in ~0.2s, 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. It had already misled two agents, each of whom forced uncached runs by hand. `-count=1` now disables caching on every invocation. **The conditional verbose rerun was missing and is added here.** `REPO_POLICIES.md` mandates it; 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. Two properties matter and both are covered: 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`, so a flake that passes the second time cannot turn the build green — the first failure already proved the suite broken. **`-timeout 90s` untouched.** It is a deliberate backstop that must strictly exceed the 60s hard cap. **No special-casing for the Docker build**, which reaches the same script via `RUN make test`: a fresh container's test cache is empty, so `-count=1` is a no-op there, and carving out an exception would only create a second code path that could drift. ### Verification **Proven from a warm cache, not a cold one.** The suite was run first to populate the cache, and the pre-change state confirmed: ``` ok sneak.berlin/go/dnswatcher/internal/config (cached) coverage: 92.6% of statements ok sneak.berlin/go/dnswatcher/internal/resolver (cached) coverage: 77.1% of statements ...8 of 8 packages (cached)... real 0m0.203s ``` With the change applied to that same warm cache, three back-to-back runs on an unchanged tree, **zero `(cached)` markers** in all three: ``` # run 1 # run 2 ok .../internal/config 1.045s ok .../internal/config 1.040s ok .../internal/handlers 1.029s ok .../internal/handlers 1.021s ok .../internal/notify 1.148s ok .../internal/notify 1.252s ok .../internal/portcheck 1.026s ok .../internal/portcheck 1.025s ok .../internal/resolver 3.005s ok .../internal/resolver 2.846s ok .../internal/state 1.078s ok .../internal/state 1.097s ok .../internal/tlscheck 1.067s ok .../internal/tlscheck 1.079s ok .../internal/watcher 1.566s ok .../internal/watcher 1.544s real 0m4.174s real 0m4.015s # run 3: grep -c '(cached)' => 0 real 0m4.519s ``` **Measured uncached wall time: 4.0-4.5s** (was ~0.2s served from cache). Inside the 20s target, so no improvement bug is owed under the two-tier rule at sneak/prompts#41 (comment). **`-count=1` composes with `-race` and `-cover`**: both still present in the primary run, and the per-package coverage percentages above are identical to the pre-change values. **The failure path was exercised, not assumed.** A purpose-built flaky test that fails on its first run and passes every run after (marker file kept outside the module, so the tree stays byte-identical and a cached result would be served if caching were on) was run through the script: ``` exit code: 1 --- FAIL: TestFlaky (0.00s) FAIL flakeproof 0.013s --- Rerunning with -v for details --- --- PASS: TestFlaky (0.00s) ok flakeproof 1.014s ``` Quiet failure, verbose rerun that genuinely re-executed (it passed, so it did not replay the cached `FAIL`), and exit `1` regardless of the rerun passing. Scratch module removed afterwards. **`make check` green**, exit 0, with the Docker lint stage demonstrably executed rather than served from cache: ``` #8 [deps 4/4] RUN go mod download #8 CACHED #10 [lint 2/2] RUN golangci-lint run --config .golangci.yml ./... #10 21.36 0 issues. #10 DONE 26.2s ``` `README.md` and `TESTING.md` record why the cache is waived, and `TODO.md` is updated in the same commit. ### Question for the owner, not filed as a defect The Docker lint run emits `The linter 'gomodguard' is deprecated (since v2.12.0) due to: new major version. Replaced by gomodguard_v2.` It is pre-existing and out of this issue's scope. It is not filed as an issue here because `.golangci.yml` tracks the org-canonical config, so switching to `gomodguard_v2` looks like an upstream `sneak/prompts` decision rather than a per-repo fix. Say the word and it gets filed in whichever place you consider canonical. - **MIT `LICENSE` added; README states the licence** — #102 The repo had no licence file at all, so publicly readable code was all-rights-reserved by default and nobody could legally use it. `LICENSE` was also the last file missing from `REPO_POLICIES.md`'s required minimum. MIT, by standing org policy rather than a per-repo call: any public repo lacking a licence gets MIT, and a private one with no licence is already all-rights-reserved. `sneak/dnswatcher` is public (`private: false` on the Gitea repo record). `README.md`'s first line now names the licence, per the Description requirement, and the License section states MIT and points at the file instead of recording the decision as pending. `TODO.md` updated in the same commit. ### Verification `LICENSE` is the canonical MIT text byte-for-byte with only the copyright line filled in (`Copyright (c) 2026 sneak`) — no clauses added, removed, reworded, or reflowed. It was not typed from memory: the file was copied from an existing verbatim MIT template on disk and only the copyright line edited (`diff` against that template shows that one line and nothing else), then the result was word-diffed against SPDX `MIT.txt` fetched from `spdx/license-list-data`, ignoring only line wrapping and the placeholder — identical. `make fmt` did **not** touch `LICENSE`, and cannot: `script/fmt` runs `gofmt -s -w .` and `goimports -w .` only, with no prettier or markdown step in the repo, so no exclusion was needed. `make check` green, exit 0. Tests executed rather than replayed (zero `(cached)` lines, `internal/resolver 3.098s`), and the Docker lint stage ran rather than cached: ``` #10 [deps 4/4] RUN go mod download #10 CACHED #12 [lint 2/2] RUN golangci-lint run --config .golangci.yml ./... #12 36.44 0 issues. #12 DONE 36.7s ``` - **Comment-only corrections in `script/bootstrap`, `script/cibuild`, and `Dockerfile.lint`** — #137 Follow-up to this PR's own review. Nothing executable changed: the diff touches comment lines, one warning string, and `TODO.md`. - `script/bootstrap`'s header justified the pinned `goimports` install by claiming `script/fmt-check` runs it on the host. Verified against the script: `script/fmt-check` runs `gofmt -l .` and nothing else. The header now credits `script/fmt` alone. That `fmt-check` does not verify goimports at all is #119 and was deliberately left alone. - `script/cibuild`'s header still said the `Dockerfile` runs `make check`. It now describes the current file: lint stage runs `make fmt-check` and `golangci-lint`, builder stage runs `make test` and `make build`. - The `docker`-missing warning was three fragments, each re-prefixed with `bootstrap:` mid-clause. Now one sentence: `bootstrap: WARNING: docker not found; install it to run make lint and make docker.` - `Dockerfile.lint`'s comment explained why `golangci-lint config verify` is omitted but read as though the omission were free. It now states the residual risk: unknown top-level keys in `.golangci.yml` are silently ignored, so a mistyped or wrong-schema key lints clean while applying nothing. `config verify` was **not** added — the network-dependence reasoning stands. ### Verification `make check` green, exit 0. Tests executed rather than replayed (zero `(cached)` lines, `internal/resolver 2.820s`), Docker lint stage executed rather than cached: ``` #8 [deps 4/4] RUN go mod download #8 CACHED #10 [lint 2/2] RUN golangci-lint run --config .golangci.yml ./... #10 13.30 0 issues. ``` Comment-only confirmed by reading the whole diff: no statement, flag, or command changed anywhere. --- ## Issues closed by this merge The commits on `next` each carry a bare `(closes #N)` in their subject, but the references in the prose above are full URLs, which Gitea's auto-close parser does not act on. Listing them here in bare form so the merge to `main` definitively closes them rather than leaving them open to be re-picked up as idle work: Closes #93 Closes #102 Closes #134 Closes #137 Closes #139 Co-authored-by: sneak <sneak@sneak.berlin> Reviewed-on: #136 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org> |
||
|
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 |
||
|
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. |
||
|
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. |
||
|
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. |
||
|
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 |
||
|
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 |
||
|
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. |