handlers: a directory under /s/ answers 404, not a listing (closes #286)

http.FileServer answered /s/ and /s/css/ with an HTML listing of the
embedded files, and /s/css with a redirect to that listing. HandleStatic
in internal/handlers now looks the name up in the embedded files first
and answers 404 when it names a directory; files are served as before.
The routes mount it under /s.

Model: opus-5-5

README.md

@@ -78,8 +78,9 @@ notification endpoint set, changes show only on the dashboard; see
   answer NXDOMAIN, that it does not exist, has no nameservers and is shown as
   not existing (see Web Dashboard and HTTP API). A domain that exists but has no
   delegation of its own, such as `octocat.github.io`, has no nameservers either.
-  When the parent zone's servers do not answer, the check fails and the set from
+  When the parent zone's servers do not answer, or the lookup of the domain's
-  the previous check is kept.
+  own records (below) fails, the check fails and the set from the previous check
+  is kept.
 - Any change triggers a notification:
     - NS added to or removed from that set. A domain that had nameservers on the
       previous check and no longer exists gets one with all of them removed.
@@ -99,11 +100,18 @@ notification endpoint set, changes show only on the dashboard; see
   records, stored per nameserver. Their changes are notified as a hostname's
   are, as a record change, NS query failure, NS recovery, inconsistency or CNAME
   address change, in a message that starts `Domain:` where a hostname's starts
-  `Hostname:`. A domain listed in `DNSWATCHER_SKIP_RECORD_NOTIFICATIONS` gets no
+  `Hostname:`. A nameserver removed from the domain's delegation gets only the
-  record change or inconsistency notification. A domain with no delegation of
+  NS change notification above. A domain listed in
-  its own has these records asked at the servers of the zone it is in, as a
+  `DNSWATCHER_SKIP_RECORD_NOTIFICATIONS` gets no record change or inconsistency
-  hostname has. A domain that does not exist has none: they are not asked for,
+  notification. A domain with no delegation of its own has these records asked
-  and those saved by an earlier check are removed without a notification.
+  at the servers of the zone it is in, as a hostname has. When none of those
+  servers answers, the lookup of its NS record set fails, and they are saved as
+  failed and notified as a hostname's are (see below). A nameserver the domain's
+  records were asked at on the previous check but not on this one gets a
+  hostname's NS change notification (see below), unless it was in the domain's
+  delegation on the previous check, as when that delegation is withdrawn. A
+  domain that does not exist has none: they are not asked for, and those saved
+  by an earlier check are removed without a notification.
 
 ### DNS Hostname Monitoring (Subdomains)
 
@@ -115,22 +123,29 @@ notification endpoint set, changes show only on the dashboard; see
   trace moves from a name to its parent only when the servers asked answer that
   the name has no delegation of its own, or does not exist. When they do not
   answer, the check fails and the hostname's records from the previous check are
-  kept.
+  kept, unless they are the nameservers the previous check saved for the
+  hostname. Each of those is then saved as failed, with the reason, as a
+  domain's nameservers are when none of them answers, so it gets an NS query
+  failure notification, and an NS recovery notification when it answers again
+  (see below). A trace that gets no answer before it reaches them, as when the
+  host running dnswatcher loses its network, fails the check without a
+  notification.
 - Queries **each** authoritative nameserver independently for **all** record
   types: A, AAAA, CNAME, MX, TXT, SRV, CAA, NS.
 - Each record type is a query of its own. When a nameserver answers some types
-  but the query for another gets no usable reply (no reply after two tries, an
+  but the query for another gets no usable reply at any of its addresses (no
-  error reply such as SERVFAIL, a referral, or a reply too large for UDP whose
+  reply after two tries, an error reply such as SERVFAIL, a referral, or a reply
-  retry over TCP fails), the failure is logged with the reason, and the type is
+  too large for UDP whose retry over TCP fails), the failure is logged with the
-  listed in the nameserver's `failedTypes` and keeps the records saved for the
+  reason, and the type is listed in the nameserver's `failedTypes` and keeps the
-  nameserver by the previous check. On that check those records are not compared
+  records saved for the nameserver by the previous check. On that check those
-  with the other nameservers', so no record change or inconsistency is reported
+  records are not compared with the other nameservers', so no record change or
-  for the type; on the next check they are compared with the nameserver's answer
+  inconsistency is reported for the type; on the next check they are compared
-  as usual. When the previous check did not know the type's records either,
+  with the nameserver's answer as usual. When the previous check did not know
-  because the nameserver was new or failing then or the type was already listed
+  the type's records either, because the nameserver was new or failing then or
-  in `unknownTypes`, the type is also listed in `unknownTypes` and left out of
+  the type was already listed in `unknownTypes`, the type is also listed in
-  every comparison until it answers. A nameserver none of whose queries got a
+  `unknownTypes` and left out of every comparison until it answers. A nameserver
-  usable reply has failed (see NS query failure below).
+  none of whose queries got a usable reply has failed (see NS query failure
+  below).
 - Stores results **per nameserver**. The state for a hostname is not a merged
   view — it is a map from nameserver to record set.
 - DNS names inside record values (CNAME, MX, SRV and NS targets) are stored in
@@ -156,6 +171,10 @@ notification endpoint set, changes show only on the dashboard; see
     - **NS recovery**: A previously-unreachable nameserver starts responding
       again. Its records are not compared with those from before it failed, so a
       change made while it was failing is not reported as a record change.
+    - **NS change**: A nameserver of the zone the hostname is in on the previous
+      check is not one now, whether or not it answered. One notification lists
+      every such nameserver on a `Removed:` line. Nothing is sent for a
+      nameserver that joins the zone.
     - **Inconsistency detected**: Two nameservers return different record sets
       for the same hostname and did not already differ on the previous check.
       Every pair of nameservers is compared. The alert is sent once for each
@@ -213,8 +232,14 @@ notification endpoint set, changes show only on the dashboard; see
     - Certificate CN, issuer, or SANs changed (replacement detected, reports old
       and new CN and issuer).
     - TLS connection failure to a previously-reachable IP:443 (handshake error,
-      timeout, connection refused after previously succeeding).
+      timeout, or connection refused when port 443 closed after the port check
+      that found it open).
     - TLS recovery: a previously-failing IP:443 now completes a handshake again.
+- A port check that finds port 443 closed removes the certificate entries for
+  that IP address, without a notification of its own: the port change
+  notification reports the closed port. When the port opens again, the next TLS
+  check saves the certificate as new, without a certificate change or recovery
+  notification.
 
 ### Notifications
 
@@ -237,7 +262,9 @@ includes:
 
 - **DNS record changes**: Which hostname or domain, which nameserver, what
   record type, old values, new values.
-- **DNS NS changes**: Which domain, which nameservers were added/removed.
+- **DNS NS changes**: Which domain, which nameservers were added to or removed
+  from its delegation. For a hostname, or a domain with no delegation of its
+  own, which nameservers left the zone it is in.
 - **NS address changes**: Which domain, which nameserver, its old and new
   addresses.
 - **CNAME address changes**: Which hostname or domain, the old and new addresses
@@ -259,7 +286,7 @@ includes:
 
 Each endpoint is sent each notification on its own, in the background. A
 delivery that fails (a network error, no reply within 10 seconds, or an HTTP
-status of 400 or more) is retried up to 5 times: the first retry after about 1
+status other than 2xx) is retried up to 5 times: the first retry after about 1
 second, each wait after that twice as long up to 60 seconds, every wait varied
 at random by up to 25%. A delivery still failing after that is logged and
 dropped.
@@ -282,8 +309,10 @@ clears them.
   port entry left with no name goes, so the dashboard and `/api/v1/status` no
   longer list or count it. The first check's port checks remove the port entries
   of addresses no configured name has.
-- Each port check also removes the certificate entries for an address their name
+- Each port check also removes, without a notification, the certificate entries
-  no longer resolves to, except while none of the name's nameservers answer.
+  for an address whose port 443 it found closed, and those for an address their
+  name no longer resolves to, except while none of the name's nameservers
+  answer.
 
 ### Web Dashboard
 
@@ -437,7 +466,7 @@ following precedence (highest to lowest):
 | `DNSWATCHER_SENTRY_DSN`                | Sentry DSN for error reporting                                                                                                        | `""`                  |
 | `DNSWATCHER_MAINTENANCE_MODE`          | Only sets `maintenanceMode` in the health check response; changes nothing else                                                        | `false`               |
 | `DNSWATCHER_METRICS_USERNAME`          | Basic auth username for /metrics, which is served only when this is set                                                               | `""`                  |
-| `DNSWATCHER_METRICS_PASSWORD`          | Basic auth password for /metrics                                                                                                      | `""`                  |
+| `DNSWATCHER_METRICS_PASSWORD`          | Basic auth password for /metrics; required when `DNSWATCHER_METRICS_USERNAME` is set, and startup stops without it                    | `""`                  |
 | `DNSWATCHER_SEND_TEST_NOTIFICATION`    | Send a test notification after first scan completes                                                                                   | `false`               |
 
 **`DNSWATCHER_TARGETS` is required.** dnswatcher will refuse to start if no
@@ -521,6 +550,11 @@ name's nameservers and in the lookup of a nameserver's own address. Such a
 lookup can need others in turn; lookups go at most three deep, one inside
 another, so delegations that point at each other still end.
 
+In step 4 each record type is asked at the IPv4 addresses the nameserver's name
+resolves to, one at a time, until one gives a usable reply; the next type is
+asked first at that address. A type's query fails only when none of them gives
+one, and the nameserver has failed only when that is so for every type.
+
 This approach ensures:
 
 - Independence from any upstream resolver's cache or filtering.
@@ -646,10 +680,20 @@ for A, AAAA and CNAME, the previous check's list is kept, or `null` when no
 earlier check saved one. A state file without it loads, and the first check
 after that saves it without a notification.
 
+A port entry's key is its address and port, and a certificate entry's key its
+address, port 443 and hostname, joined by colons. An IPv6 address is written in
+brackets in these keys, as in `[2001:db8::1]:443` and
+`[2001:db8::1]:443:www.example.com`. `/api/v1/status` and the dashboard show the
+same keys.
+
 A port entry's `hostnames` lists every name that resolves to its address,
 domains included. A port entry in the older format, with one `hostname` instead
 of the `hostnames` list, loads as a list of that one name.
 
+A state file that leaves out any of `domains`, `hostnames`, `ports` and
+`certificates`, or holds `null` for one, loads with that section empty. A file
+holding `{}` loads as empty state.
+
 ---
 
 ## Entrypoints
@@ -866,6 +910,7 @@ internal/
     templates/dashboard.html   Dashboard template (embedded)
     status.go                  /api/v1/status
     healthcheck.go             Health check handler
+    static.go                  Static assets under /s/
   server/
     server.go                  HTTP server lifecycle
     routes.go                  Route definitions

TODO.md

@@ -10,15 +10,41 @@
 
 # Status
 
-pre-1.0. No git tags. Work lands on `next` by PR. Open work for 1.0 is tracked
+pre-1.0. Work lands on `next` by PR. Open work for 1.0 is tracked on the 1.0
-on the 1.0 milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
+milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
 
 # Next Step
 
-trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149
+1.0 tracking issue: https://git.eeqj.de/sneak/dnswatcher/issues/66
 
 # Completed Steps
 
+- 2026-10-15: a request under `/s/` that names a directory, such as `/s/` or
+  `/s/css`, answers 404 instead of a listing of its files (closes #286).
+- 2026-10-15: a notification endpoint that answers with a redirect, or any
+  status other than 2xx, is a failed delivery, retried and logged (closes #287).
+- 2026-10-15: a nameserver is asked at each of its IPv4 addresses in turn, so it
+  has failed only when none of them gives a usable reply (closes #285).
+- 2026-10-15: the log names a notification endpoint (`slack`, `mattermost`,
+  `ntfy`) and never shows its URL's path or query (closes #288).
+- 2026-10-13: a hostname or a domain whose zone's nameservers all stop answering
+  gets an NS Failure for each, then an NS Recovery (closes #284).
+- 2026-10-13: an IPv6 address is written in brackets before its port, as in
+  `[2001:db8::1]:443`, in keys and Port Change notifications (closes #270).
+- 2026-10-13: `TODO.md` names only open issues as next and future steps, and its
+  Status says nothing about git tags (closes #281).
+- 2026-10-13: a port check that finds port 443 closed removes that address's
+  certificate entries, so none is left shown as `ok` (closes #265).
+- 2026-10-13: a state file holding `{}`, or leaving out a section, loads with
+  that section empty; the first check no longer crashes on it (closes #267).
+- 2026-10-13: a stop before the first scan completes sends no startup
+  notification; a scan that ran to the end still sends it (closes #271).
+- 2026-10-13: a nameserver leaving a domain's delegation sends only NS Change;
+  one leaving a hostname's zone sends NS Change, not NS Failure (closes #262).
+- 2026-10-13: `DNSWATCHER_METRICS_USERNAME` set without
+  `DNSWATCHER_METRICS_PASSWORD` stops startup (closes #268).
+- 2026-10-13: a save syncs the state file to disk before renaming it into place,
+  so a crash cannot leave it empty and stop the next start (closes #264).
 - 2026-10-13: a `PORT` that cannot be bound stops startup with an error naming
   the address, instead of running without the HTTP server (closes #266).
 - 2026-10-13: a bad `PORT`, `DNSWATCHER_TLS_EXPIRY_WARNING` or true/false
@@ -189,6 +215,4 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149
 
 # Future Steps
 
-- 1.0 readiness: run it with a real config and read the logs:
+- DNSSEC validation, after 1.0: https://git.eeqj.de/sneak/dnswatcher/issues/59
-  https://git.eeqj.de/sneak/dnswatcher/issues/66
-- review toward 1.0: https://git.eeqj.de/sneak/dnswatcher/issues/144

internal/config/config.go

@@ -61,6 +61,13 @@ var ErrInvalidTLSExpiryWarning = errors.New(
 // unset and means the default.
 var ErrInvalidBool = errors.New("value must be true or false")
 
+// ErrNoMetricsPassword is returned when DNSWATCHER_METRICS_USERNAME is set and
+// DNSWATCHER_METRICS_PASSWORD is empty. Basic Auth on /metrics would otherwise
+// let in anyone who sends the username with an empty password.
+var ErrNoMetricsPassword = errors.New(
+	"DNSWATCHER_METRICS_PASSWORD must be set when DNSWATCHER_METRICS_USERNAME is set",
+)
+
 // Params contains dependencies for Config.
 type Params struct {
 	fx.In
@@ -108,6 +115,10 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) {
 		return nil, err
 	}
 
+	if cfg.MetricsUsername != "" && cfg.MetricsPassword == "" {
+		return nil, ErrNoMetricsPassword
+	}
+
 	configureDebugLogging(cfg, params)
 
 	return cfg, nil

internal/config/config_test.go

@@ -354,6 +354,24 @@ func TestNew_SkipRecordNotificationsNotInTargetsStopsStartup(t *testing.T) {
 	require.ErrorContains(t, err, `"www.example.net"`)
 }
 
+// TestNew_MetricsUsernameWithoutPasswordStopsStartup checks an unset and an
+// empty password; TestNew_EnvironmentOverrides checks a username with one.
+func TestNew_MetricsUsernameWithoutPasswordStopsStartup(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com")
+	t.Setenv("DNSWATCHER_METRICS_USERNAME", "prom")
+
+	_, err := config.New(nil, newTestParams(t))
+	require.ErrorIs(t, err, config.ErrNoMetricsPassword, "password unset")
+	require.ErrorContains(t, err, "DNSWATCHER_METRICS_PASSWORD")
+
+	viper.Reset()
+	t.Setenv("DNSWATCHER_METRICS_PASSWORD", "")
+
+	_, err = config.New(nil, newTestParams(t))
+	require.ErrorIs(t, err, config.ErrNoMetricsPassword, "password empty")
+}
+
 func TestNew_CustomDNSIntervalDuration(t *testing.T) {
 	viper.Reset()
 	t.Setenv("DNSWATCHER_TARGETS", "example.com")

internal/handlers/static.go

@@ -0,0 +1,36 @@
+package handlers
+
+import (
+	"io/fs"
+	"net/http"
+	"strings"
+
+	"sneak.berlin/go/dnswatcher/static"
+)
+
+// HandleStatic returns the handler for the embedded static assets. It
+// is mounted under /s, with that prefix stripped from the path. A
+// request that names a directory answers 404: http.FileServer would
+// answer it with a listing of the directory, or with a redirect to
+// that listing when the trailing slash is missing.
+func (h *Handlers) HandleStatic() http.HandlerFunc {
+	files := http.FileServer(http.FS(static.Static))
+
+	return func(
+		writer http.ResponseWriter,
+		request *http.Request,
+	) {
+		// fs.Stat refuses a name that is empty or ends in a slash,
+		// so /s/ and /s/css/ answer 404 here along with /s/css.
+		name := strings.TrimPrefix(request.URL.Path, "/")
+
+		info, err := fs.Stat(static.Static, name)
+		if err != nil || info.IsDir() {
+			http.NotFound(writer, request)
+
+			return
+		}
+
+		files.ServeHTTP(writer, request)
+	}
+}

internal/notify/delivery_test.go

@@ -6,10 +6,12 @@ import (
 	"encoding/json"
 	"errors"
 	"io"
+	"log/slog"
 	"maps"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"slices"
 	"strings"
 	"sync"
 	"testing"
@@ -38,6 +40,13 @@ const (
 	prioUrgent  = "urgent"
 )
 
+// Endpoint names, as the log gives them, used across multiple tests.
+const (
+	endpointNtfy       = "ntfy"
+	endpointSlack      = "slack"
+	endpointMattermost = "mattermost"
+)
+
 // testHost is the hostname used in request construction tests.
 const testHost = "example.com"
 
@@ -1094,6 +1103,182 @@ func TestSendNotificationErrorNamesEndpoint(t *testing.T) {
 	}
 }
 
+// ── webhook URLs kept out of the log ──────────────────────
+
+// webhookPathAndQuery follows the server's address in every
+// endpoint's URL in the tests below. A webhook URL's path or query
+// lets anyone post to the channel or topic, so neither word marked
+// secret in it may reach the log.
+const webhookPathAndQuery = "/hooks/secret-in-path?token=secret-in-query"
+
+// assertNoWebhookSecret fails the test if text holds either word
+// marked secret in webhookPathAndQuery.
+func assertNoWebhookSecret(t *testing.T, text string) {
+	t.Helper()
+
+	for _, secret := range []string{"secret-in-path", "secret-in-query"} {
+		if strings.Contains(text, secret) {
+			t.Errorf("%q from the webhook URL is in:\n%s", secret, text)
+		}
+	}
+}
+
+// logRecord is the part of a JSON log line the tests below read.
+type logRecord struct {
+	Level    string `json:"level"`
+	Msg      string `json:"msg"`
+	Endpoint string `json:"endpoint"`
+	Title    string `json:"title"`
+	Error    string `json:"error"`
+}
+
+// parseLog decodes each line of a JSON log.
+func parseLog(t *testing.T, logs string) []logRecord {
+	t.Helper()
+
+	var records []logRecord
+
+	for line := range strings.Lines(logs) {
+		var record logRecord
+
+		err := json.Unmarshal([]byte(line), &record)
+		if err != nil {
+			t.Fatalf("decoding log line %q: %v", line, err)
+		}
+
+		records = append(records, record)
+	}
+
+	return records
+}
+
+// sendToEachEndpoint sends one notification with the given title to
+// ntfy, Slack and Mattermost, each at baseURL followed by
+// webhookPathAndQuery, and retries a failed delivery once. When
+// every delivery has finished it returns the JSON log written,
+// debug lines included.
+func sendToEachEndpoint(t *testing.T, baseURL, title string) string {
+	t.Helper()
+
+	target, err := url.Parse(baseURL + webhookPathAndQuery)
+	if err != nil {
+		t.Fatalf("parsing webhook URL: %v", err)
+	}
+
+	svc, logs := newLoggingService(http.DefaultTransport)
+	svc.SetNtfyURL(target)
+	svc.SetSlackWebhookURL(target)
+	svc.SetMattermostWebhookURL(target)
+	svc.SetSleepFunc(instantSleep)
+	svc.SetRetryConfig(notify.RetryConfig{
+		MaxRetries: 1,
+		BaseDelay:  time.Millisecond,
+		MaxDelay:   time.Millisecond,
+	})
+
+	svc.SendNotification(context.Background(), title, "m", prioInfo)
+
+	waitForCondition(t, func() bool {
+		return svc.OutstandingDeliveries() == 0
+	})
+
+	return logs.String()
+}
+
+// assertEachEndpointRetriedThenFailed fails the test unless, for each
+// of ntfy, Slack and Mattermost, the lines of logs whose error
+// contains reason are the retry line and then the final failure line.
+func assertEachEndpointRetriedThenFailed(
+	t *testing.T, logs, reason string,
+) {
+	t.Helper()
+
+	// For each endpoint, the messages of its lines whose error
+	// contains reason, in the order they were logged.
+	got := map[string][]string{}
+
+	for _, record := range parseLog(t, logs) {
+		if strings.Contains(record.Error, reason) {
+			got[record.Endpoint] = append(
+				got[record.Endpoint], record.Msg,
+			)
+		}
+	}
+
+	lines := []string{
+		"notification delivery failed, retrying",
+		"failed to send notification after retries",
+	}
+	want := map[string][]string{
+		endpointNtfy:       lines,
+		endpointSlack:      lines,
+		endpointMattermost: lines,
+	}
+
+	if !maps.EqualFunc(got, want, slices.Equal) {
+		t.Errorf("lines whose error contains %q = %v, want %v",
+			reason, got, want)
+	}
+}
+
+// TestSendNotificationDebugLogOmitsWebhookURL checks that, with debug
+// logging on, a delivery to each endpoint logs the endpoint's name and
+// the title, and no part of the URL's path or query.
+func TestSendNotificationDebugLogOmitsWebhookURL(t *testing.T) {
+	t.Parallel()
+
+	const title = "Port Change"
+
+	srv := httptest.NewServer(
+		http.HandlerFunc(
+			func(w http.ResponseWriter, _ *http.Request) {
+				w.WriteHeader(http.StatusOK)
+			}),
+	)
+	defer srv.Close()
+
+	logs := sendToEachEndpoint(t, srv.URL, title)
+
+	assertNoWebhookSecret(t, logs)
+
+	got := map[string]string{}
+
+	for _, record := range parseLog(t, logs) {
+		if record.Level == slog.LevelDebug.String() {
+			got[record.Endpoint] = record.Title
+		}
+	}
+
+	want := map[string]string{
+		endpointNtfy:       title,
+		endpointSlack:      title,
+		endpointMattermost: title,
+	}
+
+	if !maps.Equal(got, want) {
+		t.Errorf("debug lines' endpoint and title = %v, want %v", got, want)
+	}
+}
+
+// TestSendNotificationErrorLogOmitsWebhookURL checks that a delivery
+// to each endpoint that cannot connect logs the transport's error,
+// which names the refused connection, on its retry line and on its
+// final failure line, and that no line holds any part of the URL's
+// path or query.
+func TestSendNotificationErrorLogOmitsWebhookURL(t *testing.T) {
+	t.Parallel()
+
+	// Closed before anything is sent, so every attempt fails to
+	// connect.
+	srv := httptest.NewServer(http.NotFoundHandler())
+	srv.Close()
+
+	logs := sendToEachEndpoint(t, srv.URL, "t")
+
+	assertNoWebhookSecret(t, logs)
+	assertEachEndpointRetriedThenFailed(t, logs, "connection refused")
+}
+
 // ── SlackPayload JSON marshaling ──────────────────────────
 
 func TestSlackPayloadJSON(t *testing.T) {

internal/notify/export_test.go

@@ -81,7 +81,8 @@ func (svc *Service) SendNtfy(
 	return svc.sendNtfy(ctx, topicURL, title, message, priority)
 }
 
-// SendSlack exports sendSlack for testing.
+// SendSlack exports sendSlack for testing. It sends as the
+// "slack" endpoint.
 func (svc *Service) SendSlack(
 	ctx context.Context,
 	webhookURL *url.URL,
@@ -89,7 +90,7 @@ func (svc *Service) SendSlack(
 	title, message, priority string,
 ) error {
 	return svc.sendSlack(
-		ctx, webhookURL, failed, title, message, priority,
+		ctx, webhookURL, "slack", failed, title, message, priority,
 	)
 }
 

internal/notify/notify.go

@@ -25,9 +25,6 @@ import (
 // HTTP client timeout.
 const httpClientTimeout = 10 * time.Second
 
-// HTTP status code thresholds.
-const httpStatusClientError = 400
-
 // Sentinel errors for notification failures.
 var (
 	// ErrNtfyFailed indicates the ntfy request failed.
@@ -56,6 +53,15 @@ func IsAllowedScheme(scheme string) bool {
 func ValidateWebhookURL(raw string) (*url.URL, error) {
 	u, err := url.ParseRequestURI(raw)
 	if err != nil {
+		// A *url.Error quotes the whole URL, and this error is
+		// logged at startup. A webhook URL's path or query lets
+		// anyone post to the channel or topic, so keep only the
+		// reason.
+		var urlErr *url.Error
+		if errors.As(err, &urlErr) {
+			err = urlErr.Err
+		}
+
 		return nil, fmt.Errorf("invalid URL: %w", err)
 	}
 
@@ -277,7 +283,7 @@ func (svc *Service) dispatchSlack(
 
 	svc.dispatch(ctx, "slack", func(c context.Context) error {
 		return svc.sendSlack(
-			c, svc.slackWebhookURL, ErrSlackFailed,
+			c, svc.slackWebhookURL, "slack", ErrSlackFailed,
 			title, message, priority,
 		)
 	})
@@ -295,8 +301,8 @@ func (svc *Service) dispatchMattermost(
 		ctx, "mattermost",
 		func(c context.Context) error {
 			return svc.sendSlack(
-				c, svc.mattermostWebhookURL, ErrMattermostFailed,
+				c, svc.mattermostWebhookURL, "mattermost",
-				title, message, priority,
+				ErrMattermostFailed, title, message, priority,
 			)
 		},
 	)
@@ -307,9 +313,11 @@ func (svc *Service) sendNtfy(
 	topicURL *url.URL,
 	title, message, priority string,
 ) error {
+	// The topic URL is left out of the log: it lets anyone who has
+	// it read and post to the topic.
 	svc.log.Debug(
 		"sending ntfy notification",
-		"topic", topicURL.String(),
+		"endpoint", "ntfy",
 		"title", title,
 	)
 
@@ -333,7 +341,10 @@ func (svc *Service) sendNtfy(
 
 	defer func() { _ = resp.Body.Close() }()
 
-	if resp.StatusCode >= httpStatusClientError {
+	// Only a 2xx reply means ntfy took the notification. A
+	// redirect is not followed, so it is a failure too.
+	if resp.StatusCode < http.StatusOK ||
+		resp.StatusCode >= http.StatusMultipleChoices {
 		return fmt.Errorf(
 			"%w: status %d",
 			ErrNtfyFailed, resp.StatusCode,
@@ -372,12 +383,15 @@ type SlackAttachment struct {
 }
 
 // sendSlack posts to a Slack or Mattermost incoming webhook, which
-// take the same payload. An HTTP error status is returned wrapped in
+// take the same payload. endpoint, "slack" or "mattermost", names
-// failed, ErrSlackFailed or ErrMattermostFailed, so the error names
+// the webhook in the log in place of its URL, which lets anyone who
-// the endpoint.
+// has it post to the channel. A reply status other than 2xx is
+// returned wrapped in failed, ErrSlackFailed or ErrMattermostFailed,
+// so the error names the endpoint.
 func (svc *Service) sendSlack(
 	ctx context.Context,
 	webhookURL *url.URL,
+	endpoint string,
 	failed error,
 	title, message, priority string,
 ) error {
@@ -388,7 +402,7 @@ func (svc *Service) sendSlack(
 
 	svc.log.Debug(
 		"sending webhook notification",
-		"url", webhookURL.String(),
+		"endpoint", endpoint,
 		"title", title,
 	)
 
@@ -423,7 +437,10 @@ func (svc *Service) sendSlack(
 
 	defer func() { _ = resp.Body.Close() }()
 
-	if resp.StatusCode >= httpStatusClientError {
+	// Only a 2xx reply means the webhook took the notification. A
+	// redirect is not followed, so it is a failure too.
+	if resp.StatusCode < http.StatusOK ||
+		resp.StatusCode >= http.StatusMultipleChoices {
 		return fmt.Errorf(
 			"%w: status %d",
 			failed, resp.StatusCode,

internal/notify/notify_test.go

@@ -79,6 +79,23 @@ func TestValidateWebhookURLInvalid(t *testing.T) {
 	}
 }
 
+// TestValidateWebhookURLErrorOmitsURL checks that the error for a
+// webhook URL that does not parse, here because a carriage return
+// was left at its end, leaves out the URL's path and query: the
+// error is logged at startup.
+func TestValidateWebhookURLErrorOmitsURL(t *testing.T) {
+	t.Parallel()
+
+	_, err := notify.ValidateWebhookURL(
+		"https://hooks.example.com" + webhookPathAndQuery + "\r",
+	)
+	if err == nil {
+		t.Fatal("expected an error for a URL ending in a carriage return")
+	}
+
+	assertNoWebhookSecret(t, err.Error())
+}
+
 func TestIsAllowedScheme(t *testing.T) {
 	t.Parallel()
 

internal/notify/retry_test.go

@@ -536,3 +536,33 @@ func TestSendNotificationPermanentFailureLogsError(
 		return attempts >= 3
 	})
 }
+
+// TestSendNotificationRedirectFails checks that a 302 reply is a
+// failed delivery at each endpoint: the server is sent the first
+// attempt and the retry, and each endpoint logs a retry line and
+// then the final failure line, both naming the 302.
+func TestSendNotificationRedirectFails(t *testing.T) {
+	t.Parallel()
+
+	var requests atomic.Int64
+
+	srv := httptest.NewServer(
+		http.HandlerFunc(
+			func(w http.ResponseWriter, r *http.Request) {
+				requests.Add(1)
+				http.Redirect(w, r, "/moved", http.StatusFound)
+			}),
+	)
+	defer srv.Close()
+
+	logs := sendToEachEndpoint(t, srv.URL, "t")
+
+	// ntfy, Slack and Mattermost each send once and retry once.
+	const wantRequests = 3 * 2
+
+	if sent := requests.Load(); sent != wantRequests {
+		t.Errorf("server got %d requests, want %d", sent, wantRequests)
+	}
+
+	assertEachEndpointRetriedThenFailed(t, logs, "status 302")
+}

internal/resolver/errors.go

@@ -2,6 +2,23 @@ package resolver
 
 import "errors"
 
+// NoAnswerError is returned when the walk to a name's nameservers
+// reached a zone through a referral and none of that zone's
+// nameservers, Nameservers as the referral names them, answered. Err
+// says why they did not.
+type NoAnswerError struct {
+	Nameservers []string
+	Err         error
+}
+
+func (e *NoAnswerError) Error() string {
+	return e.Err.Error()
+}
+
+func (e *NoAnswerError) Unwrap() error {
+	return e.Err
+}
+
 // Sentinel errors returned by the resolver.
 var (
 	// ErrNoNameservers is returned when no authoritative NS
@@ -19,6 +36,10 @@ var (
 	// so whether the name has addresses is unknown.
 	ErrNoNameserverAnswered = errors.New("no nameserver answered")
 
+	// ErrNoNameserverAddress is returned when a nameserver is to be
+	// asked and no address for it was given.
+	ErrNoNameserverAddress = errors.New("nameserver has no address")
+
 	// ErrUnusableReply is returned when a server replied with an
 	// error such as SERVFAIL, or with a referral that leads no
 	// closer to the name asked about.

internal/resolver/export_test.go

@@ -31,9 +31,11 @@ func NewWithQueryTimeout(log *slog.Logger, timeout time.Duration) *Resolver {
 func (r *Resolver) FollowDelegation(
 	ctx context.Context,
 	domain string,
+	zone string,
+	nameservers []string,
 	servers []string,
 ) ([]string, error) {
-	return r.followDelegation(ctx, domain, servers)
+	return r.followDelegation(ctx, domain, zone, nameservers, servers)
 }
 
 // FindAuthoritativeNameserversFrom exports findAuthoritativeNameservers
@@ -94,6 +96,17 @@ func (r *Resolver) QueryServers(
 	return r.queryServers(ctx, servers, zone, name, qtype)
 }
 
+// QueryTypes exports queryTypes for testing.
+func (r *Resolver) QueryTypes(
+	ctx context.Context,
+	nsHostname string,
+	nsIPs []string,
+	hostname string,
+	qtypes []uint16,
+) (*NameserverResponse, error) {
+	return r.queryTypes(ctx, nsHostname, nsIPs, hostname, qtypes)
+}
+
 // QueryEachNS exports queryEachNS for testing.
 func (r *Resolver) QueryEachNS(
 	ctx context.Context,

internal/resolver/iterative.go

@@ -204,20 +204,24 @@ func glueIPs(nsNames []string, glue map[string][]net.IP) []string {
 	return ips
 }
 
-// followDelegation follows referrals from servers, the root servers, to
+// followDelegation follows referrals to domain from servers, the
-// domain and returns the NS set of domain's delegation. When the servers
+// addresses of the servers of zone, and returns the NS set of domain's
-// of the zone domain is in answer that domain does not exist, the error
+// delegation. nameservers are the names a referral gave the servers of
-// is ErrNXDomain. When they answer that it has no delegation of its own,
+// zone; the walk to a name's nameservers starts at the root servers, the
+// servers of zone ".", which no referral names. When the servers of the
+// zone domain is in answer that domain does not exist, the error is
+// ErrNXDomain. When they answer that it has no delegation of its own,
 // because it is not the zone's apex, the set is empty and there is no
-// error. Any other error means that no such answer came.
+// error. Any other error means that no such answer came; it is a
+// *NoAnswerError when the servers that did not answer are those of a
+// zone a referral named.
 func (r *Resolver) followDelegation(
 	ctx context.Context,
 	domain string,
+	zone string,
+	nameservers []string,
 	servers []string,
 ) ([]string, error) {
-	// servers are the root servers, the servers of zone ".".
-	zone := "."
-
 	var withoutAddresses []string
 
 	for range maxDelegation {
@@ -228,6 +232,12 @@ func (r *Resolver) followDelegation(
 		resp, err := r.queryZone(
 			ctx, servers, withoutAddresses, zone, domain, dns.TypeNS, 0,
 		)
+
+		// A query that ctx cut short says nothing about the servers.
+		if err != nil && len(nameservers) > 0 && checkCtx(ctx) == nil {
+			return nil, &NoAnswerError{Nameservers: nameservers, Err: err}
+		}
+
 		if err != nil {
 			return nil, err
 		}
@@ -256,6 +266,7 @@ func (r *Resolver) followDelegation(
 			return r.resolveNSIterative(ctx, domain)
 		}
 
+		nameservers = authNS
 		servers, withoutAddresses = referralNameservers(resp)
 		zone = referralZone(resp)
 	}
@@ -626,7 +637,8 @@ func (r *Resolver) resolveARecord(
 // them. When the servers asked answer that the name has no delegation
 // of its own, or does not exist, it tries each parent name in turn, so
 // it returns the nameservers of the zone the name is in. When they do
-// not answer, it returns the error and tries no parent name.
+// not answer, it returns the error and tries no parent name; when a
+// referral named them, the error is a *NoAnswerError naming them.
 func (r *Resolver) FindAuthoritativeNameservers(
 	ctx context.Context,
 	domain string,
@@ -655,7 +667,7 @@ func (r *Resolver) findAuthoritativeNameservers(
 
 		candidate := strings.Join(labels[i:], ".") + "."
 
-		nsNames, err := r.followDelegation(ctx, candidate, servers)
+		nsNames, err := r.followDelegation(ctx, candidate, ".", nil, servers)
 		if err != nil && !errors.Is(err, ErrNXDomain) {
 			return nil, err
 		}
@@ -696,8 +708,9 @@ func (r *Resolver) QueryNameserver(
 	return r.queryNameserver(ctx, nsHostname, hostname, recordTypes())
 }
 
-// queryNameserver queries a specific nameserver for the record types
+// queryNameserver queries a specific nameserver, at the IPv4 addresses
-// in qtypes and builds a NameserverResponse.
+// its name resolves to, for the record types in qtypes and builds a
+// NameserverResponse.
 func (r *Resolver) queryNameserver(
 	ctx context.Context,
 	nsHostname string,
@@ -715,7 +728,7 @@ func (r *Resolver) queryNameserver(
 
 	hostname = dns.Fqdn(hostname)
 
-	return r.queryTypes(ctx, nsHostname, nsIPs[0], hostname, qtypes)
+	return r.queryTypes(ctx, nsHostname, nsIPs, hostname, qtypes)
 }
 
 // QueryNameserverIP queries a nameserver by its IP address directly,
@@ -732,23 +745,33 @@ func (r *Resolver) QueryNameserverIP(
 
 	hostname = dns.Fqdn(hostname)
 
-	return r.queryTypes(ctx, nsHostname, nsIP, hostname, recordTypes())
+	return r.queryTypes(
+		ctx, nsHostname, []string{nsIP}, hostname, recordTypes(),
+	)
 }
 
+// queryTypes asks the nameserver nsHostname, at its addresses nsIPs,
+// about hostname's records of each type in qtypes and builds a
+// NameserverResponse. With no address there is nothing to ask, and it
+// returns ErrNoNameserverAddress.
 func (r *Resolver) queryTypes(
 	ctx context.Context,
 	nsHostname string,
-	nsIP string,
+	nsIPs []string,
 	hostname string,
 	qtypes []uint16,
 ) (*NameserverResponse, error) {
+	if len(nsIPs) == 0 {
+		return nil, fmt.Errorf("%w: %s", ErrNoNameserverAddress, nsHostname)
+	}
+
 	resp := &NameserverResponse{
 		Nameserver: nsHostname,
 		Records:    make(map[string][]string),
 		Status:     StatusOK,
 	}
 
-	state := r.queryEachType(ctx, nsIP, hostname, qtypes, resp)
+	state := r.queryEachType(ctx, nsIPs, hostname, qtypes, resp)
 	classifyResponse(resp, state)
 
 	return resp, nil
@@ -766,28 +789,37 @@ type queryState struct {
 	answered      bool
 }
 
-// queryEachType asks the nameserver at nsIP about hostname once for each
+// queryEachType asks the nameserver about hostname's records of each type
-// record type in qtypes, and lists in resp.FailedTypes the types whose
+// in qtypes, at its addresses nsIPs in turn until one gives a usable
-// query got no usable reply, logging each with the reason unless ctx was
+// reply, and lists in resp.FailedTypes the types that got none at any of
-// cancelled: shutdown cancels it, and a query it cut short did not fail.
+// them, logging each with the reasons unless ctx was cancelled: shutdown
+// cancels it, and a query it cut short did not fail.
 func (r *Resolver) queryEachType(
 	ctx context.Context,
-	nsIP string,
+	nsIPs []string,
 	hostname string,
 	qtypes []uint16,
 	resp *NameserverResponse,
 ) queryState {
 	var state queryState
 
+	// Reordered below; the caller's list is left as it was.
+	nsIPs = slices.Clone(nsIPs)
+
 	for _, qtype := range qtypes {
 		if checkCtx(ctx) != nil {
 			break
 		}
 
-		err := r.querySingleType(ctx, nsIP, hostname, qtype, resp, &state)
+		answeredAt, err := r.querySingleType(
+			ctx, nsIPs, hostname, qtype, resp, &state,
+		)
 		if err == nil {
 			state.answered = true
 
+			// The next type is asked first at the address that answered.
+			nsIPs[0], nsIPs[answeredAt] = nsIPs[answeredAt], nsIPs[0]
+
 			continue
 		}
 
@@ -821,20 +853,30 @@ func (r *Resolver) queryEachType(
 	return state
 }
 
-// querySingleType asks the nameserver at nsIP about hostname's records
+// querySingleType asks the nameserver about hostname's records of type
-// of type qtype. It returns nil when the nameserver answered: with
+// qtype at its addresses nsIPs, of which there is at least one, one at a
-// records, with none, or with NXDOMAIN; otherwise it returns why not.
+// time, until one answers: with records, with none, or with NXDOMAIN.
+// It returns the index in nsIPs of the address that answered; when none
+// did, it returns why each did not, each reason naming its address.
 func (r *Resolver) querySingleType(
 	ctx context.Context,
-	nsIP string,
+	nsIPs []string,
 	hostname string,
 	qtype uint16,
 	resp *NameserverResponse,
 	state *queryState,
-) error {
+) (int, error) {
-	msg, err := r.queryDNS(ctx, nsIP, hostname, qtype)
+	var errs []error
-	if err != nil {
+
+	for i, nsIP := range nsIPs {
+		msg, err := r.queryDNS(ctx, nsIP, hostname, qtype)
+
 		switch {
+		case err == nil:
+			err = readReply(msg, resp, state)
+			if err != nil {
+				err = fmt.Errorf("query %s @%s: %w", hostname, nsIP, err)
+			}
 		case isTimeout(err):
 			state.gotTimeout = true
 		case errors.Is(err, ErrRefused):
@@ -843,10 +885,14 @@ func (r *Resolver) querySingleType(
 			state.netErr = err
 		}
 
-		return err
+		if err == nil {
+			return i, nil
+		}
+
+		errs = append(errs, err)
 	}
 
-	return readReply(msg, resp, state)
+	return 0, errors.Join(errs...)
 }
 
 // readReply adds to resp the records in msg, a nameserver's reply to a
@@ -940,17 +986,15 @@ func isTimeout(err error) bool {
 }
 
 // classifyResponse sets the nameserver's status. One that answered no
-// record type has failed, and Error says why; one that answered some has
+// record type has failed, and Error says why; it says all queries timed
-// the status of those answers. It has no data only when every type
+// out only when every query, at every address, did. One that answered
+// some has the status of those answers. It has no data only when every type
 // answered with no records: a type in FailedTypes may have records, so a
 // nameserver with one stays ok.
 func classifyResponse(resp *NameserverResponse, state queryState) {
 	switch {
 	case state.gotNXDomain && !state.hasRecords:
 		resp.Status = StatusNXDomain
-	case state.gotTimeout && !state.answered:
-		resp.Status = StatusTimeout
-		resp.Error = "all queries timed out"
 	case state.gotErrorReply && !state.answered:
 		resp.Status = StatusError
 		resp.Error = "server returned " + state.errorReply
@@ -963,6 +1007,9 @@ func classifyResponse(resp *NameserverResponse, state queryState) {
 	case state.gotReferral && !state.answered:
 		resp.Status = StatusError
 		resp.Error = "server returned a referral"
+	case state.gotTimeout && !state.answered:
+		resp.Status = StatusTimeout
+		resp.Error = "all queries timed out"
 	// An NXDOMAIN reply with no records was taken by the first case.
 	case !state.hasRecords && len(resp.FailedTypes) == 0:
 		resp.Status = StatusNoData
@@ -1067,7 +1114,7 @@ func (r *Resolver) LookupNS(
 	}
 
 	return r.followDelegation(
-		ctx, dns.Fqdn(strings.ToLower(domain)), rootServerList(),
+		ctx, dns.Fqdn(strings.ToLower(domain)), ".", nil, rootServerList(),
 	)
 }
 

internal/resolver/iterative_internal_test.go

@@ -15,8 +15,10 @@ import (
 // that answered some record types, even with no records, has not failed
 // when its query for another type got no usable reply, whatever the
 // reason, and is ok, not nodata: that type may have records. One whose
-// every query got none has failed. Only one whose every type answered
+// every query got none has failed, and its error says all queries timed
-// with no records is nodata.
+// out only when every one did: a reply such as a refusal, or a network
+// error, at one of its addresses is given instead. Only one whose every
+// type answered with no records is nodata.
 func TestClassifyResponse(t *testing.T) {
 	t.Parallel()
 
@@ -27,63 +29,46 @@ func TestClassifyResponse(t *testing.T) {
 		wantStatus  string
 		wantError   string
 	}{
-		{
+		{"every type answered with no records",
-			"every type answered with no records",
 			queryState{answered: true},
-			nil,
+			nil, StatusNoData, ""},
-			StatusNoData, "",
+		{"some types answered with no records, another timed out",
-		},
-		{
-			"some types answered with no records, another timed out",
 			queryState{answered: true, gotTimeout: true},
-			[]string{"A"},
+			[]string{"A"}, StatusOK, ""},
-			StatusOK, "",
+		{"some types answered with no records, another got SERVFAIL",
-		},
+			queryState{answered: true, gotErrorReply: true, errorReply: "SERVFAIL"},
-		{
+			[]string{"A"}, StatusOK, ""},
-			"some types answered with no records, another got SERVFAIL",
+		{"some types answered with no records, another was refused",
-			queryState{
-				answered: true, gotErrorReply: true, errorReply: "SERVFAIL",
-			},
-			[]string{"A"},
-			StatusOK, "",
-		},
-		{
-			"some types answered with no records, another was refused",
 			queryState{answered: true, gotRefused: true},
-			[]string{"A"},
+			[]string{"A"}, StatusOK, ""},
-			StatusOK, "",
+		{"some types answered with no records, another got a network error",
-		},
-		{
-			"some types answered with no records, another got a network error",
 			queryState{answered: true, netErr: syscall.ECONNREFUSED},
-			[]string{"A"},
+			[]string{"A"}, StatusOK, ""},
-			StatusOK, "",
+		{"some types answered with no records, another's reply was " +
-		},
+			"truncated and its retry over TCP failed",
-		{
-			"some types answered with no records, another's reply was " +
-				"truncated and its retry over TCP failed",
 			queryState{answered: true, netErr: ErrTruncated},
-			[]string{"TXT"},
+			[]string{"TXT"}, StatusOK, ""},
-			StatusOK, "",
+		{"some types answered with no records, another got a referral",
-		},
-		{
-			"some types answered with no records, another got a referral",
 			queryState{answered: true, gotReferral: true},
-			[]string{"A"},
+			[]string{"A"}, StatusOK, ""},
-			StatusOK, "",
+		{"every query timed out",
-		},
-		{
-			"every query timed out",
 			queryState{gotTimeout: true},
-			[]string{"A", "AAAA", "CNAME"},
+			[]string{"A", "AAAA", "CNAME"}, StatusTimeout, "all queries timed out"},
-			StatusTimeout, "all queries timed out",
+		{"every query got NOTIMP",
-		},
-		{
-			"every query got NOTIMP",
 			queryState{gotErrorReply: true, errorReply: "NOTIMP"},
-			[]string{"A", "AAAA", "CNAME"},
+			[]string{"A", "AAAA", "CNAME"}, StatusError, "server returned NOTIMP"},
-			StatusError, "server returned NOTIMP",
+		{"every query got SERVFAIL at one address, timed out at another",
-		},
+			queryState{gotErrorReply: true, errorReply: "SERVFAIL", gotTimeout: true},
+			[]string{"A"}, StatusError, "server returned SERVFAIL"},
+		{"every query was refused at one address, timed out at another",
+			queryState{gotRefused: true, gotTimeout: true},
+			[]string{"A"}, StatusError, "server returned REFUSED"},
+		{"every query got a network error at one address, timed out at another",
+			queryState{netErr: syscall.ECONNREFUSED, gotTimeout: true},
+			[]string{"A"}, StatusError, "network error: connection refused"},
+		{"every query got a referral at one address, timed out at another",
+			queryState{gotReferral: true, gotTimeout: true},
+			[]string{"A"}, StatusError, "server returned a referral"},
 	}
 
 	for _, tt := range tests {

internal/resolver/resolver_test.go

@@ -121,7 +121,9 @@ func TestFindAuthoritativeNameservers_DelegatedSubdomain(
 // delegation of its own, so the lookup returns that walk's error, about
 // www.google.com, and tries no parent name: trying google.com and com
 // would end in ErrNoNameservers, or in the error of a walk for one of
-// them.
+// them. The servers that did not answer are those the walk started at,
+// which no referral named, so the error is no NoAnswerError, as when
+// the host running dnswatcher has lost its network.
 func TestFindAuthoritativeNameservers_NoAnswer(t *testing.T) {
 	t.Parallel()
 
@@ -134,6 +136,9 @@ func TestFindAuthoritativeNameservers_NoAnswer(t *testing.T) {
 	require.NotErrorIs(t, err, resolver.ErrNoNameservers)
 	assert.Contains(t, err.Error(), "query www.google.com. @"+noAnswerAddress)
 	assert.Empty(t, nameservers)
+
+	var noAnswer *resolver.NoAnswerError
+	assert.NotErrorAs(t, err, &noAnswer)
 }
 
 func TestFindAuthoritativeNameservers_ReturnsSorted(
@@ -355,6 +360,124 @@ func TestQueryNameserver_ZoneDelegatedWithoutAddresses(t *testing.T) {
 	assert.Equal(t, resolver.StatusOK, resp.Status)
 }
 
+// TestQueryTypes_FirstAddressNeverAnswers asks a google.com nameserver
+// about google.com's A and AAAA records at a list of addresses that puts
+// 192.0.2.1, where nothing answers, before the nameserver's own. The
+// nameserver answers at its own address, so it is ok, with its records.
+// Each attempt has time for one round of tries at 192.0.2.1, two tries
+// of two seconds, and not for two, so AAAA must be asked first at the
+// address that answered A.
+func TestQueryTypes_FirstAddressNeverAnswers(t *testing.T) {
+	t.Parallel()
+
+	r := newTestResolver(t)
+	ns := findOneNSForDomain(t, r, "google.com")
+	nsIPs := append(
+		[]string{noAnswerAddress}, liveResolveNSIPs(t, r, []string{ns}, 1)...,
+	)
+
+	var resp *resolver.NameserverResponse
+
+	livednstest.Retry(
+		t,
+		"QueryTypes("+ns+" at "+strings.Join(nsIPs, ", ")+", google.com)",
+		func(ctx context.Context) error {
+			ctx, cancel := context.WithTimeout(ctx, 6*time.Second)
+			defer cancel()
+
+			var err error
+
+			resp, err = r.QueryTypes(
+				ctx, ns, nsIPs, "google.com.",
+				[]uint16{dns.TypeA, dns.TypeAAAA},
+			)
+			if err != nil {
+				return err
+			}
+
+			if len(resp.FailedTypes) > 0 {
+				return fmt.Errorf(
+					"%w: %s gave no usable reply for %s",
+					livednstest.ErrNoAnswer, ns,
+					strings.Join(resp.FailedTypes, ", "),
+				)
+			}
+
+			return nil
+		},
+	)
+
+	assert.Equal(t, resolver.StatusOK, resp.Status)
+	assert.NotEmpty(t, resp.Records["A"])
+	assert.NotEmpty(t, resp.Records["AAAA"])
+}
+
+// TestQueryTypes_EveryAddressFails asks a google.com nameserver about
+// cloudflare.com, a zone it does not serve, at 192.0.2.1, where nothing
+// answers, and then at its own address, which refuses. The nameserver
+// has failed with the refusal, not a timeout, and the warning about the
+// type gives the reason at each address.
+func TestQueryTypes_EveryAddressFails(t *testing.T) {
+	t.Parallel()
+
+	var logs bytes.Buffer
+
+	r := resolver.NewFromLogger(slog.New(slog.NewTextHandler(&logs, nil)))
+	ns := findOneNSForDomain(t, r, "google.com")
+	nsIPs := append(
+		[]string{noAnswerAddress}, liveResolveNSIPs(t, r, []string{ns}, 1)...,
+	)
+
+	var resp *resolver.NameserverResponse
+
+	livednstest.Retry(
+		t,
+		"QueryTypes("+ns+" at "+strings.Join(nsIPs, ", ")+", cloudflare.com)",
+		func(ctx context.Context) error {
+			logs.Reset()
+
+			var err error
+
+			resp, err = r.QueryTypes(
+				ctx, ns, nsIPs, "cloudflare.com.", []uint16{dns.TypeA},
+			)
+			if err != nil {
+				return err
+			}
+
+			// A timeout or a network error at its own address is no
+			// reply at all.
+			if resp.Status == resolver.StatusTimeout ||
+				strings.HasPrefix(resp.Error, "network error") {
+				return fmt.Errorf(
+					"%w: %s: %s", livednstest.ErrNoAnswer, ns, resp.Error,
+				)
+			}
+
+			return nil
+		},
+	)
+
+	assert.Equal(t, resolver.StatusError, resp.Status)
+	assert.Equal(t, "server returned REFUSED", resp.Error)
+	assert.Contains(t, logs.String(), "@"+noAnswerAddress+": ")
+	assert.Contains(t, logs.String(), "@"+nsIPs[1]+": ")
+}
+
+// TestQueryTypes_NoAddress asks a nameserver given no address. There is
+// nothing to ask it at, so it has failed, with an error that says it has
+// no address; it is not ok.
+func TestQueryTypes_NoAddress(t *testing.T) {
+	t.Parallel()
+
+	resp, err := newTestResolver(t).QueryTypes(
+		t.Context(), "ns1.example.net.", nil, "www.example.net.",
+		[]uint16{dns.TypeA},
+	)
+	require.ErrorIs(t, err, resolver.ErrNoNameserverAddress)
+	assert.Nil(t, resp)
+}
+
 func TestQueryNameserver_AAAA(t *testing.T) {
 	t.Parallel()
 
@@ -419,8 +542,9 @@ func TestQueryNameserver_TXT(t *testing.T) {
 // nameserver about google.com with a resolver whose retries over TCP
 // fail. google.com's TXT records do not fit in a reply over UDP, so TXT
 // is reported as failed, holding none of the records that fit, and
-// logged with the reason, while the nameserver, which answered the other
+// logged with the reason, which names the address whose reply was
-// types, is ok.
+// truncated, while the nameserver, which answered the other types, is
+// ok.
 func TestQueryNameserver_TruncatedReplyWhoseTCPRetryFails(t *testing.T) {
 	t.Parallel()
 
@@ -437,6 +561,13 @@ func TestQueryNameserver_TruncatedReplyWhoseTCPRetryFails(t *testing.T) {
 	assert.Contains(t, logs.String(),
 		"hostname=google.com. nameserver="+ns+" type=TXT error=",
 	)
+
+	nsIPs := liveResolveNSIPs(t, newTestResolver(t), []string{ns}, 1)
+	for _, ip := range nsIPs {
+		assert.Contains(t, logs.String(),
+			"@"+ip+": "+resolver.ErrTruncated.Error(),
+		)
+	}
 }
 
 func TestQueryNameserver_NXDomain(t *testing.T) {
@@ -490,6 +621,57 @@ func TestQueryNameserver_Refused(t *testing.T) {
 	assert.Equal(t, "server returned REFUSED", resp.Error)
 }
 
+// TestQueryNameserver_EveryAddressRefuses asks ada.ns.cloudflare.com,
+// whose name has several IPv4 addresses, about google.com, a zone it
+// does not serve, which it refuses. A record type is asked at each of
+// the nameserver's addresses before it fails, so the warning about it
+// names every one.
+func TestQueryNameserver_EveryAddressRefuses(t *testing.T) {
+	t.Parallel()
+
+	const ns = "ada.ns.cloudflare.com."
+
+	var logs bytes.Buffer
+
+	r := resolver.NewFromLogger(slog.New(slog.NewTextHandler(&logs, nil)))
+	nsIPs := liveResolveNSIPs(t, r, []string{ns}, 2)
+
+	var resp *resolver.NameserverResponse
+
+	livednstest.Retry(
+		t,
+		"QueryNameserver("+ns+", google.com)",
+		func(ctx context.Context) error {
+			logs.Reset()
+
+			var err error
+
+			resp, err = r.QueryNameserver(ctx, ns, "google.com")
+			if err != nil {
+				return err
+			}
+
+			// A timeout or a network error at every address is no
+			// reply at all.
+			if resp.Status == resolver.StatusTimeout ||
+				strings.HasPrefix(resp.Error, "network error") {
+				return fmt.Errorf(
+					"%w: %s: %s", livednstest.ErrNoAnswer, ns, resp.Error,
+				)
+			}
+
+			return nil
+		},
+	)
+
+	assert.Equal(t, resolver.StatusError, resp.Status)
+	assert.Equal(t, "server returned REFUSED", resp.Error)
+
+	for _, ip := range nsIPs {
+		assert.Contains(t, logs.String(), "@"+ip+": ")
+	}
+}
+
 // TestQueryServers_RecursiveResolverRefused passes a public recursive
 // resolver to QueryServers as the server of google.com. These resolvers
 // refuse a query that does not ask for recursion and answer one that
@@ -954,12 +1136,34 @@ func TestFollowDelegation_NoAnswer(t *testing.T) {
 	r := resolver.NewWithQueryTimeout(slog.Default(), 100*time.Millisecond)
 
 	nameservers, err := r.FollowDelegation(
-		t.Context(), "google.com.", []string{noAnswerAddress},
+		t.Context(), "google.com.", ".", nil, []string{noAnswerAddress},
 	)
 	require.Error(t, err)
 	assert.Empty(t, nameservers)
 }
 
+// TestFollowDelegation_NoAnswerAfterReferral starts the walk for
+// www.example.net at the servers of example.net as a referral names
+// them, ns1.example.net and ns2.example.net, at 192.0.2.1, where nothing
+// answers. The error is a NoAnswerError naming them, which the watcher
+// compares with the nameservers it saved for the name.
+func TestFollowDelegation_NoAnswerAfterReferral(t *testing.T) {
+	t.Parallel()
+
+	r := resolver.NewWithQueryTimeout(slog.Default(), 100*time.Millisecond)
+	nameservers := []string{"ns1.example.net.", "ns2.example.net."}
+
+	_, err := r.FollowDelegation(
+		t.Context(), "www.example.net.", "example.net.", nameservers,
+		[]string{noAnswerAddress},
+	)
+
+	var noAnswer *resolver.NoAnswerError
+	require.ErrorAs(t, err, &noAnswer)
+	assert.Equal(t, nameservers, noAnswer.Nameservers)
+	assert.Contains(t, err.Error(), "query www.example.net. @"+noAnswerAddress)
+}
+
 // TestResolveNSIterative_NoDelegationOfItsOwn walks to the nameservers
 // of www.google.com as the fallback walk does. As in
 // TestLookupNS_NoDelegationOfItsOwn, the set is empty, with no error.

internal/server/routes.go

@@ -8,8 +8,6 @@ import (
 	"github.com/go-chi/chi/v5"
 	chimw "github.com/go-chi/chi/v5/middleware"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
-
-	"sneak.berlin/go/dnswatcher/static"
 )
 
 // requestTimeout is the maximum duration for handling a request.
@@ -47,13 +45,7 @@ func (s *Server) SetupRoutes() {
 	public.Get("/", s.handlers.HandleDashboard())
 
 	// Static assets (embedded CSS/JS)
-	public.Mount(
+	public.Mount("/s", http.StripPrefix("/s", s.handlers.HandleStatic()))
-		"/s",
-		http.StripPrefix(
-			"/s",
-			http.FileServer(http.FS(static.Static)),
-		),
-	)
 
 	// Health check (standard well-known path)
 	public.Get(

internal/server/routes_test.go

@@ -1,6 +1,7 @@
 package server_test
 
 import (
+	"mime"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -288,3 +289,42 @@ func TestMetricsRateLimitComesBeforeAuth(t *testing.T) {
 		t.Errorf("another address: status = %d, want 200", rec.Code)
 	}
 }
+
+// TestStaticDirectoriesAnswerNotFound checks that a request naming a
+// directory of the static assets, with or without the trailing slash,
+// answers 404 rather than a listing of it or a redirect to one, and
+// that the stylesheet is still served as CSS.
+func TestStaticDirectoriesAnswerNotFound(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com")
+
+	srv := routedServer(t)
+
+	for _, path := range []string{"/s", "/s/", "/s/css", "/s/css/"} {
+		rec := serve(srv, httptest.NewRequestWithContext(
+			t.Context(), http.MethodGet, path, nil,
+		))
+
+		if rec.Code != http.StatusNotFound {
+			t.Errorf("GET %s: status = %d, want 404", path, rec.Code)
+		}
+	}
+
+	rec := serve(srv, httptest.NewRequestWithContext(
+		t.Context(), http.MethodGet, "/s/css/tailwind.min.css", nil,
+	))
+
+	if rec.Code != http.StatusOK {
+		t.Errorf("GET stylesheet: status = %d, want 200", rec.Code)
+	}
+
+	contentType := rec.Header().Get("Content-Type")
+
+	mediaType, _, err := mime.ParseMediaType(contentType)
+	if err != nil || mediaType != "text/css" {
+		t.Errorf(
+			"GET stylesheet: Content-Type = %q, want text/css",
+			contentType,
+		)
+	}
+}

internal/state/state.go

@@ -4,6 +4,7 @@ package state
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log/slog"
 	"maps"
@@ -215,6 +216,24 @@ func (s *State) Load() error {
 		return fmt.Errorf("parsing state file: %w", err)
 	}
 
+	// A section the file leaves out, or holds as null, loads empty: the
+	// setters write into these maps, and writing into a nil map panics.
+	if snapshot.Domains == nil {
+		snapshot.Domains = make(map[string]*DomainState)
+	}
+
+	if snapshot.Hostnames == nil {
+		snapshot.Hostnames = make(map[string]*HostnameState)
+	}
+
+	if snapshot.Ports == nil {
+		snapshot.Ports = make(map[string]*PortState)
+	}
+
+	if snapshot.Certificates == nil {
+		snapshot.Certificates = make(map[string]*CertificateState)
+	}
+
 	// A state file saved before each record value was stored once can
 	// hold a hostname's CNAME once for every record type asked for.
 	// Each value is kept once, so the first check does not see a
@@ -253,10 +272,20 @@ func (s *State) Save() error {
 		return fmt.Errorf("creating data directory: %w", err)
 	}
 
-	// Atomic write: write to temp file, then rename
+	// Atomic write: write to temp file, then rename. The temp file is
+	// synced before the rename and the directory after it, or a crash
+	// can leave the renamed state file empty.
 	tmpPath := path + ".tmp"
 
-	err = os.WriteFile(tmpPath, data, filePermissions)
+	//nolint:gosec // path is from trusted config
+	tmpFile, err := os.OpenFile(
+		tmpPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, filePermissions,
+	)
+	if err == nil {
+		_, err = tmpFile.Write(data)
+		err = errors.Join(err, tmpFile.Sync(), tmpFile.Close())
+	}
+
 	if err != nil {
 		return fmt.Errorf("writing temp state file: %w", err)
 	}
@@ -266,6 +295,15 @@ func (s *State) Save() error {
 		return fmt.Errorf("renaming state file: %w", err)
 	}
 
+	dir, err := os.Open(filepath.Dir(path))
+	if err == nil {
+		err = errors.Join(dir.Sync(), dir.Close())
+	}
+
+	if err != nil {
+		return fmt.Errorf("syncing data directory: %w", err)
+	}
+
 	s.log.Debug("state saved to disk", "path", path)
 
 	return nil

internal/state/state_test.go

@@ -618,6 +618,53 @@ func TestSaveAtomicWrite(t *testing.T) {
 	}
 }
 
+// TestSaveOverLeftoverTempFile verifies that a temp file left by a
+// crash, longer than the state Save writes, leaves nothing of itself in
+// the state file, and that no temp file remains after Save.
+func TestSaveOverLeftoverTempFile(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	statePath := filepath.Join(dir, "state.json")
+	tmpPath := statePath + ".tmp"
+
+	err := os.WriteFile(tmpPath, []byte(strings.Repeat("x", 64*1024)), 0o600)
+	if err != nil {
+		t.Fatalf("writing leftover temp file: %v", err)
+	}
+
+	s := state.NewForTestWithDataDir(dir)
+
+	populateState(t, s)
+
+	err = s.Save()
+	if err != nil {
+		t.Fatalf("Save() error: %v", err)
+	}
+
+	//nolint:gosec // test file with known path
+	data, err := os.ReadFile(statePath)
+	if err != nil {
+		t.Fatalf("reading state file: %v", err)
+	}
+
+	var snap state.Snapshot
+
+	err = json.Unmarshal(data, &snap)
+	if err != nil {
+		t.Fatalf("state file is not complete JSON: %v", err)
+	}
+
+	if len(snap.Domains) != 2 {
+		t.Errorf("state file has %d domains, want 2", len(snap.Domains))
+	}
+
+	_, err = os.Stat(tmpPath)
+	if !os.IsNotExist(err) {
+		t.Error("temp file should not exist after save")
+	}
+}
+
 // TestLoadMissingFile verifies that loading a nonexistent file is not an error.
 func TestLoadMissingFile(t *testing.T) {
 	t.Parallel()
@@ -648,6 +695,58 @@ func TestLoadMissingFile(t *testing.T) {
 	}
 }
 
+// TestLoadFileWithoutSections loads a state file holding {}, and files
+// that each leave out one section. Every section loads as an empty map.
+// TestFirstCheckAfterLoadingFileWithoutSections in the watcher tests
+// runs the first check after such a load.
+func TestLoadFileWithoutSections(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		file string
+	}{
+		{"no sections", `{}`},
+		{"no domains", `{"hostnames": {}, "ports": {}, "certificates": {}}`},
+		{"no hostnames", `{"domains": {}, "ports": {}, "certificates": {}}`},
+		{"no ports", `{"domains": {}, "hostnames": {}, "certificates": {}}`},
+		{"no certificates", `{"domains": {}, "hostnames": {}, "ports": {}}`},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			dir := t.TempDir()
+
+			err := os.WriteFile(
+				filepath.Join(dir, "state.json"), []byte(tt.file), 0o600,
+			)
+			if err != nil {
+				t.Fatalf("writing state file: %v", err)
+			}
+
+			s := state.NewForTestWithDataDir(dir)
+
+			err = s.Load()
+			if err != nil {
+				t.Fatalf("Load() error: %v", err)
+			}
+
+			want := state.Snapshot{
+				Domains:      map[string]*state.DomainState{},
+				Hostnames:    map[string]*state.HostnameState{},
+				Ports:        map[string]*state.PortState{},
+				Certificates: map[string]*state.CertificateState{},
+			}
+
+			if got := s.GetSnapshot(); !reflect.DeepEqual(got, want) {
+				t.Errorf("loaded %#v, want %#v", got, want)
+			}
+		})
+	}
+}
+
 // TestLoadCorruptFile verifies that a corrupt state file returns an error.
 func TestLoadCorruptFile(t *testing.T) {
 	t.Parallel()

internal/watcher/cancelled_test.go

@@ -83,6 +83,30 @@ func TestCancelledCheckSavesNothing(t *testing.T) {
 	}
 }
 
+// TestStopBeforeFirstScanSendsNoStartupNotification runs a watcher with
+// the startup notification on and its context already cancelled, so the
+// first scan is cut short before its lookup is sent. The scan did not
+// complete, so no startup notification may be sent.
+func TestStopBeforeFirstScanSendsNoStartupNotification(t *testing.T) {
+	t.Parallel()
+
+	cfg := defaultTestConfig(t)
+	cfg.Hostnames = []string{testHost}
+	cfg.SendTestNotification = true
+
+	w, deps := newTestWatcher(t, cfg)
+
+	ctx, cancel := context.WithCancel(t.Context())
+	cancel()
+
+	w.Run(ctx)
+
+	notifications := deps.notifier.getNotifications()
+	if len(notifications) != 0 {
+		t.Errorf("sent %v, want no notifications", notifications)
+	}
+}
+
 // newLoggingWatcher returns a watcher for a domain and a hostname, with
 // the real resolver, that writes what it logs at warning level or above
 // into the returned buffer.

internal/watcher/export_test.go

@@ -122,6 +122,34 @@ func (w *Watcher) RunTLSChecks(ctx context.Context) {
 	w.runTLSChecks(ctx)
 }
 
+// CheckHostnameResult exports checkHostnameResult for testing.
+func (w *Watcher) CheckHostnameResult(
+	ctx context.Context,
+	hostname string,
+	results map[string]*resolver.NameserverResponse,
+	err error,
+) bool {
+	return w.checkHostnameResult(ctx, hostname, results, err)
+}
+
+// CheckDomainResult exports checkDomainResult for testing.
+func (w *Watcher) CheckDomainResult(
+	ctx context.Context,
+	domain string,
+	nameservers []string,
+	err error,
+) {
+	w.checkDomainResult(ctx, domain, nameservers, err)
+}
+
+// FailedNameservers exports failedNameservers for testing.
+func FailedNameservers(
+	err error,
+	prev *state.HostnameState,
+) map[string]*resolver.NameserverResponse {
+	return failedNameservers(err, prev)
+}
+
 // BuildHostnameState exports buildHostnameState for testing.
 func BuildHostnameState(
 	results map[string]*resolver.NameserverResponse,

internal/watcher/interfaces.go

@@ -20,7 +20,10 @@ type DNSResolver interface {
 	) ([]string, error)
 
 	// LookupAllRecords queries all record types for a hostname,
-	// returning each nameserver's response keyed by nameserver.
+	// returning each nameserver's response keyed by nameserver. When
+	// the walk to the nameservers reached a zone through a referral and
+	// none of that zone's nameservers answered, the error is a
+	// *resolver.NoAnswerError naming them.
 	LookupAllRecords(
 		ctx context.Context,
 		hostname string,

internal/watcher/ipv6_test.go

@@ -0,0 +1,83 @@
+package watcher_test
+
+import (
+	"maps"
+	"slices"
+	"testing"
+
+	"sneak.berlin/go/dnswatcher/internal/state"
+	"sneak.berlin/go/dnswatcher/internal/watcher"
+)
+
+// TestIPv6AddressInBrackets runs the port checks, not the watcher's
+// first, on hostname state built here for a hostname at an IPv6 address
+// whose port 443 the previous check found closed. It is open now. Then
+// it runs the TLS checks and the port checks again. The port and
+// certificate keys and the Port Change notification write the address
+// in brackets, so its last group cannot be read as the port, and the
+// second port checks keep the certificate entry, whose address they read
+// back from its key. When a last port check finds port 443 closed, the
+// certificate entry is removed. Nothing is looked up.
+func TestIPv6AddressInBrackets(t *testing.T) {
+	t.Parallel()
+
+	const aaaaType = "AAAA"
+
+	cfg := defaultTestConfig(t)
+	cfg.Hostnames = []string{host}
+
+	deps := newTestDeps(t, cfg)
+	w := watcher.NewForTest(
+		cfg, deps.state, nil,
+		deps.portChecker, deps.tlsChecker, deps.notifier,
+	)
+	w.SetFirstRun(false)
+
+	deps.state.SetHostnameState(host, saved(
+		map[string]*state.NameserverRecordState{
+			nsA: answered(map[string][]string{aaaaType: {"2001:db8::1"}}),
+		},
+	))
+	deps.state.SetPortState("[2001:db8::1]:443", &state.PortState{
+		Hostnames: []string{host},
+	})
+
+	w.CheckAllPorts(t.Context())
+	w.RunTLSChecks(t.Context())
+	w.CheckAllPorts(t.Context())
+
+	snap := deps.state.GetSnapshot()
+
+	gotPorts := slices.Sorted(maps.Keys(snap.Ports))
+	wantPorts := []string{"[2001:db8::1]:443", "[2001:db8::1]:80"}
+
+	if !slices.Equal(gotPorts, wantPorts) {
+		t.Errorf("port entries %v, want %v", gotPorts, wantPorts)
+	}
+
+	gotCerts := slices.Sorted(maps.Keys(snap.Certificates))
+	wantCerts := []string{"[2001:db8::1]:443:www.example.net"}
+
+	if !slices.Equal(gotCerts, wantCerts) {
+		t.Errorf("certificate entries %v, want %v", gotCerts, wantCerts)
+	}
+
+	title := "Port Change: [2001:db8::1]:443"
+	want := `Hostnames: www.example.net
+Address: [2001:db8::1]:443
+Port now open`
+
+	got := deps.notifier.getNotifications()
+	if len(got) != 1 || got[0].Title != title || got[0].Message != want {
+		t.Errorf("sent %v, want one %q with message:\n%s", got, title, want)
+	}
+
+	deps.portChecker.closed = true
+
+	w.CheckAllPorts(t.Context())
+
+	gotCerts = slices.Sorted(maps.Keys(deps.state.GetSnapshot().Certificates))
+	if len(gotCerts) != 0 {
+		t.Errorf("certificate entries %v with port 443 closed, want none", gotCerts)
+	}
+}

internal/watcher/message_test.go

@@ -5,7 +5,6 @@ import (
 	"strings"
 	"testing"
 
-	"sneak.berlin/go/dnswatcher/internal/config"
 	"sneak.berlin/go/dnswatcher/internal/state"
 	"sneak.berlin/go/dnswatcher/internal/watcher"
 )
@@ -74,21 +73,19 @@ b.ns.example.net.: 192.0.2.2`,
 func TestDomainRecordNotificationsNameTheDomain(t *testing.T) {
 	t.Parallel()
 
-	notifier := &mockNotifier{}
+	cfg := defaultTestConfig(t)
-	w := watcher.NewForTest(
+	cfg.Domains = []string{domain}
-		&config.Config{Domains: []string{domain}},
+
-		nil, nil, nil, nil, notifier,
+	deps := newTestDeps(t, cfg)
-	)
+	w := watcher.NewForTest(cfg, deps.state, nil, nil, nil, deps.notifier)
 
 	// nsA's address changes, which also makes it differ from nsC; nsB
-	// fails; nsC answers again; nsD is gone.
+	// fails; nsC answers again.
-	nsD := "d.ns.example.net."
 	w.DetectHostnameChanges(t.Context(), domain,
 		saved(map[string]*state.NameserverRecordState{
 			nsA: answered(map[string][]string{"A": {ip1}}),
 			nsB: answered(map[string][]string{"A": {ip1}}),
 			nsC: failed(),
-			nsD: answered(map[string][]string{"A": {ip1}}),
 		}),
 		saved(map[string]*state.NameserverRecordState{
 			nsA: answered(map[string][]string{"A": {ip2}}),
@@ -102,18 +99,17 @@ func TestDomainRecordNotificationsNameTheDomain(t *testing.T) {
 		t.Context(), domain, cnameState(ip1), cnameState(ip2),
 	)
 
-	// NS Failure is sent for nsB failing and for nsD being gone.
 	want := map[string]int{
 		"Record Change":        1,
 		"Inconsistency":        1,
-		"NS Failure":           2,
+		"NS Failure":           1,
 		"NS Recovery":          1,
 		"CNAME Address Change": 1,
 	}
 
 	sent := make(map[string]int)
 
-	for _, n := range notifier.getNotifications() {
+	for _, n := range deps.notifier.getNotifications() {
 		kind, _, _ := strings.Cut(n.Title, ":")
 		sent[kind]++
 
@@ -128,6 +124,89 @@ func TestDomainRecordNotificationsNameTheDomain(t *testing.T) {
 	}
 }
 
+// nsB, which answered, and nsC, which did not, are no longer among the
+// nameservers. One NS Change lists both, for the hostname and for the
+// configured apex domain with no delegation of its own. Nothing is sent
+// for the domain whose delegation on the previous check listed them:
+// the check of its delegation sends its NS Change.
+func TestRemovedNameserversAreAnNSChange(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name   string
+		target string
+		// The domain's nameservers saved by the previous check.
+		delegation []string
+		// The NS Change sent, or "" for none.
+		want string
+	}{
+		{
+			name:   "hostname",
+			target: host,
+			want: `Hostname: www.example.net
+Removed: b.ns.example.net., c.ns.example.net.`,
+		},
+		{
+			name:       "domain with no delegation of its own",
+			target:     domain,
+			delegation: []string{},
+			want: `Domain: example.net
+Removed: b.ns.example.net., c.ns.example.net.`,
+		},
+		{
+			name:       "domain whose delegation listed them",
+			target:     domain,
+			delegation: []string{nsA, nsB, nsC},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			cfg := defaultTestConfig(t)
+			cfg.Domains = []string{domain}
+
+			deps := newTestDeps(t, cfg)
+			w := watcher.NewForTest(
+				cfg, deps.state, nil, nil, nil, deps.notifier,
+			)
+
+			deps.state.SetDomainState(domain, &state.DomainState{
+				Nameservers: tt.delegation,
+			})
+
+			prev := saved(map[string]*state.NameserverRecordState{
+				nsA: answered(map[string][]string{"A": {ip1}}),
+				nsB: answered(map[string][]string{"A": {ip1}}),
+				nsC: failed(),
+			})
+			current := saved(map[string]*state.NameserverRecordState{
+				nsA: answered(map[string][]string{"A": {ip1}}),
+			})
+
+			w.DetectHostnameChanges(t.Context(), tt.target, prev, current)
+
+			got := deps.notifier.getNotifications()
+
+			if tt.want == "" {
+				if len(got) != 0 {
+					t.Errorf("sent %v, want nothing", got)
+				}
+
+				return
+			}
+
+			title := "NS Change: " + tt.target
+			if len(got) != 1 || got[0].Title != title ||
+				got[0].Message != tt.want {
+				t.Errorf("sent %v, want one %q with message:\n%s",
+					got, title, tt.want)
+			}
+		})
+	}
+}
+
 // The startup notification counts the configured domains and hostnames,
 // although the state's hostnames also hold the apex domain's own
 // records. Nothing is looked up: the watcher has no resolver.

internal/watcher/nsfailure_test.go

@@ -2,6 +2,7 @@ package watcher_test
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"log/slog"
 	"strings"
@@ -14,6 +15,10 @@ import (
 	"sneak.berlin/go/dnswatcher/internal/watcher"
 )
 
+// errNoReply is why a walk that got no reply from a zone's servers
+// failed.
+var errNoReply = errors.New("all servers failed: i/o timeout")
+
 // answered is what a check saves for a nameserver that answered with
 // these records.
 func answered(records map[string][]string) *state.NameserverRecordState {
@@ -183,6 +188,134 @@ func TestNSFailureAlertNamesHostnameNameserverAndReason(t *testing.T) {
 	}
 }
 
+// TestZoneNameserversAllStopAnswering runs three checks of host from
+// the point where its lookup has returned. On the first, nsA and nsB,
+// the nameservers of its zone, answer. On the second, the lookup fails
+// with the error the walk to them returns when neither answers (see
+// the resolver's TestFollowDelegation_NoAnswerAfterReferral), given
+// here because no live zone has nameservers that never answer. For a
+// hostname it is the lookup of its records that fails, and for a domain
+// with no delegation of its own the lookup of its delegation. Both are
+// saved as failed, with the reason, and each gets an NS Failure. On the
+// third, both answer, and each gets an NS Recovery. A domain's records
+// are checked as a hostname's, so its first and third checks are too.
+func TestZoneNameserversAllStopAnswering(t *testing.T) {
+	t.Parallel()
+
+	records := map[string][]string{"A": {ip1}}
+	bothAnswer := map[string]*resolver.NameserverResponse{
+		nsA: response(records), nsB: response(records),
+	}
+	noAnswer := &resolver.NoAnswerError{
+		Nameservers: []string{nsA, nsB}, Err: errNoReply,
+	}
+
+	tests := []struct {
+		name     string
+		isDomain bool
+	}{
+		{"hostname", false},
+		{"domain with no delegation of its own", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			cfg := defaultTestConfig(t)
+			if tt.isDomain {
+				cfg.Domains = []string{host}
+			} else {
+				cfg.Hostnames = []string{host}
+			}
+
+			// The watcher is given each lookup's result, so it has no
+			// resolver.
+			deps := newTestDeps(t, cfg)
+			w := watcher.NewForTest(
+				cfg, deps.state, nil,
+				deps.portChecker, deps.tlsChecker, deps.notifier,
+			)
+			w.SetFirstRun(false)
+
+			w.CheckHostnameResult(t.Context(), host, bothAnswer, nil)
+
+			if tt.isDomain {
+				w.CheckDomainResult(t.Context(), host, nil, noAnswer)
+			} else {
+				w.CheckHostnameResult(t.Context(), host, nil, noAnswer)
+			}
+
+			current, _ := deps.state.GetHostnameState(host)
+
+			for _, ns := range []string{nsA, nsB} {
+				got := current.RecordsByNameserver[ns]
+				if got.Status != failed().Status || got.Error != errNoReply.Error() {
+					t.Errorf(
+						"%s saved with status %q, error %q; want %q, %q",
+						ns, got.Status, got.Error, failed().Status, errNoReply,
+					)
+				}
+			}
+
+			if n := countNotifications(deps, "NS Failure: "+host); n != 2 {
+				t.Errorf("sent %d NS Failure notifications, want 2", n)
+			}
+
+			w.CheckHostnameResult(t.Context(), host, bothAnswer, nil)
+
+			if n := countNotifications(deps, "NS Recovery: "+host); n != 2 {
+				t.Errorf("sent %d NS Recovery notifications, want 2", n)
+			}
+		})
+	}
+}
+
+// TestFailedNameserversOnlyForThoseOfThePreviousCheck gives
+// FailedNameservers lookup errors after a check in which nsA and nsB
+// answered. Only an error that names both makes them failed.
+func TestFailedNameserversOnlyForThoseOfThePreviousCheck(t *testing.T) {
+	t.Parallel()
+
+	records := map[string][]string{"A": {ip1}}
+	bothAnswered := saved(map[string]*state.NameserverRecordState{
+		nsA: answered(records), nsB: answered(records),
+	})
+	noAnswer := func(nameservers ...string) error {
+		return &resolver.NoAnswerError{Nameservers: nameservers, Err: errNoReply}
+	}
+
+	tests := []struct {
+		name       string
+		err        error
+		prev       *state.HostnameState
+		wantFailed bool
+	}{
+		{"both did not answer", noAnswer(nsA, nsB), bothAnswered, true},
+		{
+			"the servers of a parent zone did not answer",
+			noAnswer(nsC), bothAnswered, false,
+		},
+		{
+			"the zone's delegation lists only nsA now",
+			noAnswer(nsA), bothAnswered, false,
+		},
+		{"no root server answered", errNoReply, bothAnswered, false},
+		{"no previous check", noAnswer(nsA, nsB), nil, false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			got := watcher.FailedNameservers(tt.err, tt.prev)
+			if (got != nil) != tt.wantFailed {
+				t.Errorf("returned %v, want failed: %v", got, tt.wantFailed)
+			}
+		})
+	}
+}
+
 // TestNameserverThatNeverAnswers asks a nameserver address where
 // nothing answers, 192.0.2.1, and checks what the watcher saves for it.
 // The deadline outlasts the resolver's first two-second try, as in the

internal/watcher/removed_test.go

@@ -2,7 +2,9 @@ package watcher_test
 
 import (
 	"maps"
+	"net"
 	"slices"
+	"strings"
 	"testing"
 
 	"sneak.berlin/go/dnswatcher/internal/state"
@@ -172,7 +174,7 @@ func TestCertificateStateForAnAddressGone(t *testing.T) {
 					"A": {ip1}, "AAAA": {ip6},
 				}),
 			}),
-			[]string{ip1 + ":443:" + host, ip6 + ":443:" + host},
+			[]string{ip1 + ":443:" + host, "[" + ip6 + "]:443:" + host},
 		},
 		{
 			"no nameserver answered",
@@ -182,7 +184,7 @@ func TestCertificateStateForAnAddressGone(t *testing.T) {
 			[]string{
 				ip1 + ":443:" + host,
 				ip2 + ":443:" + host,
-				ip6 + ":443:" + host,
+				"[" + ip6 + "]:443:" + host,
 			},
 		},
 	}
@@ -205,7 +207,8 @@ func TestCertificateStateForAnAddressGone(t *testing.T) {
 
 			for _, ip := range []string{ip1, ip2, ip6} {
 				deps.state.SetCertificateState(
-					ip+":443:"+host, &state.CertificateState{Status: "ok"},
+					net.JoinHostPort(ip, "443")+":"+host,
+					&state.CertificateState{Status: "ok"},
 				)
 			}
 
@@ -222,3 +225,67 @@ func TestCertificateStateForAnAddressGone(t *testing.T) {
 		})
 	}
 }
+
+// TestCertificateStateForAPortClosed runs the port checks on hostname
+// and port state built here for a configured hostname at ip1, whose
+// port 443 the previous check found open and whose certificate entry
+// is ok. The port is closed now: the certificate entry is removed, and
+// only the Port Change is notified. When the port opens again, the next
+// TLS check saves the certificate as new and notifies nothing about it.
+// Nothing is looked up.
+func TestCertificateStateForAPortClosed(t *testing.T) {
+	t.Parallel()
+
+	cfg := defaultTestConfig(t)
+	cfg.Hostnames = []string{host}
+
+	deps := newTestDeps(t, cfg)
+	w := watcher.NewForTest(
+		cfg, deps.state, nil,
+		deps.portChecker, deps.tlsChecker, deps.notifier,
+	)
+	w.SetFirstRun(false)
+
+	certKey := ip1 + ":443:" + host
+
+	deps.state.SetHostnameState(host, saved(
+		map[string]*state.NameserverRecordState{
+			nsA: answered(map[string][]string{"A": {ip1}}),
+		},
+	))
+	deps.state.SetPortState(ip1+":443", &state.PortState{
+		Open: true, Hostnames: []string{host},
+	})
+	deps.state.SetCertificateState(certKey, &state.CertificateState{
+		CommonName: host, Status: "ok",
+	})
+	deps.portChecker.closed = true
+
+	w.CheckAllPorts(t.Context())
+
+	if _, ok := deps.state.GetCertificateState(certKey); ok {
+		t.Errorf("certificate entry %s kept with port 443 closed", certKey)
+	}
+
+	sent := deps.notifier.getNotifications()
+	if title := "Port Change: " + ip1 + ":443"; len(sent) != 1 ||
+		sent[0].Title != title {
+		t.Errorf("sent %v, want one %q", sent, title)
+	}
+
+	deps.portChecker.closed = false
+
+	w.CheckAllPorts(t.Context())
+	w.RunTLSChecks(t.Context())
+
+	cert, ok := deps.state.GetCertificateState(certKey)
+	if !ok || cert.Status != "ok" {
+		t.Errorf("certificate entry %s after port 443 opened: %+v", certKey, cert)
+	}
+
+	for _, n := range deps.notifier.getNotifications() {
+		if strings.HasPrefix(n.Title, "TLS") {
+			t.Errorf("sent %v, want no TLS notification", n)
+		}
+	}
+}

internal/watcher/watcher.go

@@ -6,8 +6,10 @@ import (
 	"fmt"
 	"log/slog"
 	"maps"
+	"net"
 	"slices"
 	"sort"
+	"strconv"
 	"strings"
 	"time"
 
@@ -133,7 +135,12 @@ func (w *Watcher) Run(ctx context.Context) {
 
 	w.cleanupRemovedTargets()
 	w.RunOnce(ctx)
-	w.maybeSendTestNotification(ctx)
+
+	// RunOnce returns as usual when shutdown cuts the scan short, and
+	// the startup notification says the scan completed.
+	if ctx.Err() == nil {
+		w.maybeSendTestNotification(ctx)
+	}
 
 	dnsTicker := time.NewTicker(w.config.DNSInterval)
 	tlsTicker := time.NewTicker(w.config.TLSInterval)
@@ -284,12 +291,25 @@ func (w *Watcher) logFailedLookup(
 	w.log.Error(msg, args...)
 }
 
+// checkDomain looks up domain's delegation and checks domain with it
+// (see checkDomainResult).
 func (w *Watcher) checkDomain(
 	ctx context.Context,
 	domain string,
 ) {
 	nameservers, err := w.resolver.LookupNS(ctx, domain)
 
+	w.checkDomainResult(ctx, domain, nameservers, err)
+}
+
+// checkDomainResult finishes a check of domain from nameservers and err,
+// what the lookup of its delegation returned.
+func (w *Watcher) checkDomainResult(
+	ctx context.Context,
+	domain string,
+	nameservers []string,
+	err error,
+) {
 	// A domain that does not exist has no nameservers.
 	nxdomain := errors.Is(err, resolver.ErrNXDomain)
 	if nxdomain {
@@ -304,6 +324,12 @@ func (w *Watcher) checkDomain(
 			"error", err,
 		)
 
+		// For a domain with no delegation of its own, the servers that
+		// did not answer can be those of the zone it is in, at which
+		// its records are asked. They are then saved as failed, as a
+		// hostname's are.
+		w.checkHostnameResult(ctx, domain, nil, err)
+
 		return
 	}
 
@@ -322,6 +348,20 @@ func (w *Watcher) checkDomain(
 		ctx, nameservers, prevAddresses,
 	)
 
+	// A domain that does not exist has no records of its own: none are
+	// asked for, and those saved by an earlier check are removed.
+	// Otherwise the apex domain's records are also checked and saved as
+	// a hostname's, so that the port and TLS checks find its addresses.
+	// Notifications about them name it as a domain (see nameLine). When
+	// their lookup fails, the check fails as when the NS lookup does, so
+	// the saved delegation and the saved records come from the same
+	// check, as detectNSDisappearances needs.
+	if nxdomain {
+		w.state.DeleteHostnameState(domain)
+	} else if !w.checkHostname(ctx, domain) {
+		return
+	}
+
 	if hasPrev && !w.firstRun {
 		w.detectNSChanges(ctx, domain, prev.Nameservers, nameservers)
 		w.detectNSAddressChanges(ctx, domain, prevAddresses, addresses)
@@ -333,19 +373,6 @@ func (w *Watcher) checkDomain(
 		NXDomain:            nxdomain,
 		LastChecked:         now,
 	})
-
-	// A domain that does not exist has no records of its own: none are
-	// asked for, and those saved by an earlier check are removed.
-	if nxdomain {
-		w.state.DeleteHostnameState(domain)
-
-		return
-	}
-
-	// The apex domain's records are also checked and saved as a
-	// hostname's, so that the port and TLS checks find its addresses.
-	// Notifications about them name it as a domain (see nameLine).
-	w.checkHostname(ctx, domain)
 }
 
 func (w *Watcher) detectNSChanges(
@@ -456,10 +483,12 @@ func (w *Watcher) detectNSAddressChanges(
 	}
 }
 
+// checkHostname looks up hostname's records and checks them (see
+// checkHostnameResult). It reports whether it saved them.
 func (w *Watcher) checkHostname(
 	ctx context.Context,
 	hostname string,
-) {
+) bool {
 	results, err := w.resolver.LookupAllRecords(ctx, hostname)
 	if err != nil {
 		w.logFailedLookup(
@@ -468,15 +497,70 @@ func (w *Watcher) checkHostname(
 			"hostname", hostname,
 			"error", err,
 		)
-
-		return
 	}
 
+	return w.checkHostnameResult(ctx, hostname, results, err)
+}
+
+// checkHostnameResult finishes a check of hostname from results and err,
+// what the lookup of its records returned, and reports whether it saved
+// them. When the lookup failed, it saves nothing, unless the nameservers
+// that did not answer are those of the previous check (see
+// failedNameservers).
+func (w *Watcher) checkHostnameResult(
+	ctx context.Context,
+	hostname string,
+	results map[string]*resolver.NameserverResponse,
+	err error,
+) bool {
 	prev, _ := w.state.GetHostnameState(hostname)
 
+	if err != nil {
+		results = failedNameservers(err, prev)
+		if results == nil {
+			return false
+		}
+	}
+
 	w.updateHostnameState(
 		ctx, hostname, buildHostnameState(results, prev, time.Now().UTC()),
 	)
+
+	return true
+}
+
+// failedNameservers returns a failed response for each nameserver in
+// prev, a name's state from the previous check, with err as the reason,
+// when err says that the walk to the name's nameservers reached their
+// zone and none of them answered. For any other error, as when the walk
+// got no answer before it reached their zone, or when prev is nil, it
+// returns nil.
+func failedNameservers(
+	err error,
+	prev *state.HostnameState,
+) map[string]*resolver.NameserverResponse {
+	var noAnswer *resolver.NoAnswerError
+	if !errors.As(err, &noAnswer) || prev == nil {
+		return nil
+	}
+
+	saved := slices.Sorted(maps.Keys(prev.RecordsByNameserver))
+	if !slices.Equal(noAnswer.Nameservers, saved) {
+		return nil
+	}
+
+	results := make(map[string]*resolver.NameserverResponse)
+
+	for _, ns := range saved {
+		results[ns] = &resolver.NameserverResponse{
+			Nameserver: ns,
+			Records:    make(map[string][]string),
+			Status:     resolver.StatusError,
+			Error:      err.Error(),
+		}
+	}
+
+	return results
 }
 
 // updateHostnameState finishes a check of hostname from newState, built
@@ -772,28 +856,53 @@ func (w *Watcher) detectRecordChanges(
 	}
 }
 
+// detectNSDisappearances sends one NS Change listing the nameservers of
+// the previous check that are no longer those of the zone hostname is
+// in. For a configured apex domain it leaves out those of the domain's
+// saved delegation, which detectNSChanges notifies when they are removed
+// from it. checkDomain saves a new delegation only after this runs, and
+// only together with the records, so the saved delegation and the
+// nameservers in prev come from the same check.
 func (w *Watcher) detectNSDisappearances(
 	ctx context.Context,
 	hostname string,
 	prev, current *state.HostnameState,
 ) {
-	for ns, prevNS := range prev.RecordsByNameserver {
+	var delegation []string
-		if _, ok := current.RecordsByNameserver[ns]; ok || prevNS.Status != statusOK {
+
-			continue
+	if w.isDomain(hostname) {
+		if domainState, ok := w.state.GetDomainState(hostname); ok {
+			delegation = domainState.Nameservers
 		}
-
-		msg := fmt.Sprintf(
-			"%s\nNameserver: %s disappeared",
-			w.nameLine(hostname), ns,
-		)
-
-		w.notify.SendNotification(
-			ctx,
-			"NS Failure: "+hostname,
-			msg,
-			"error",
-		)
 	}
+
+	var removed []string
+
+	for ns := range prev.RecordsByNameserver {
+		_, ok := current.RecordsByNameserver[ns]
+		if !ok && !slices.Contains(delegation, ns) {
+			removed = append(removed, ns)
+		}
+	}
+
+	if len(removed) == 0 {
+		return
+	}
+
+	slices.Sort(removed)
+
+	msg := fmt.Sprintf(
+		"%s\nRemoved: %s",
+		w.nameLine(hostname),
+		strings.Join(removed, ", "),
+	)
+
+	w.notify.SendNotification(
+		ctx,
+		"NS Change: "+hostname,
+		msg,
+		"warning",
+	)
 }
 
 // detectNSFailures notifies when a nameserver that answered on the
@@ -943,7 +1052,8 @@ func (w *Watcher) checkAllPorts(ctx context.Context) {
 
 	// Phase 3: Remove port state entries that no longer have
 	// any hostname referencing them, and certificate entries for
-	// an address their name no longer has.
+	// an address whose port 443 is closed or that their name no
+	// longer has.
 	w.cleanupStalePorts(associations)
 	w.cleanupStaleCertificates()
 }
@@ -964,7 +1074,7 @@ func (w *Watcher) buildPortAssociations() map[string][]string {
 		ips := w.collectIPs(name)
 		for _, ip := range ips {
 			for _, port := range monitoredPorts {
-				key := fmt.Sprintf("%s:%d", ip, port)
+				key := net.JoinHostPort(ip, strconv.Itoa(port))
 				if assoc[key] == nil {
 					assoc[key] = make(map[string]bool)
 				}
@@ -989,18 +1099,15 @@ func (w *Watcher) buildPortAssociations() map[string][]string {
 	return result
 }
 
-// parsePortKey splits an "ip:port" key into its components.
+// parsePortKey splits an "ip:port" key, written by net.JoinHostPort,
+// into its components.
 func parsePortKey(key string) (string, int) {
-	lastColon := strings.LastIndex(key, ":")
+	ip, port, err := net.SplitHostPort(key)
-	if lastColon < 0 {
+	if err != nil {
 		return key, 0
 	}
 
-	ip := key[:lastColon]
+	p, err := strconv.Atoi(port)
-
-	var p int
-
-	_, err := fmt.Sscanf(key[lastColon+1:], "%d", &p)
 	if err != nil {
 		return ip, 0
 	}
@@ -1030,13 +1137,21 @@ func (w *Watcher) cleanupStalePorts(
 }
 
 // cleanupStaleCertificates removes the certificate entries for an
-// address their name no longer resolves to. An entry saved for a name
+// address whose port 443 was found closed, and for an address their
-// none of whose nameservers answered is kept: that name's addresses are
+// name no longer resolves to. An entry saved for a name none of whose
-// not known, not gone.
+// nameservers answered is kept unless its port is closed: that name's
+// addresses are not known, not gone.
 func (w *Watcher) cleanupStaleCertificates() {
 	for _, key := range w.state.GetAllCertificateKeys() {
 		ip, hostname := parseCertKey(key)
 
+		ps, ok := w.state.GetPortState(net.JoinHostPort(ip, strconv.Itoa(tlsPort)))
+		if ok && !ps.Open {
+			w.state.DeleteCertificateState(key)
+
+			continue
+		}
+
 		if slices.Contains(w.collectIPs(hostname), ip) ||
 			w.noNameserverAnswered(hostname) {
 			continue
@@ -1145,7 +1260,7 @@ func (w *Watcher) checkSinglePort(
 		return
 	}
 
-	key := fmt.Sprintf("%s:%d", ip, port)
+	key := net.JoinHostPort(ip, strconv.Itoa(port))
 	now := time.Now().UTC()
 	prev, hasPrev := w.state.GetPortState(key)
 
@@ -1207,7 +1322,7 @@ func (w *Watcher) checkTLSForHostname(
 	ips := w.collectIPs(hostname)
 
 	for _, ip := range ips {
-		portKey := fmt.Sprintf("%s:%d", ip, tlsPort)
+		portKey := net.JoinHostPort(ip, strconv.Itoa(tlsPort))
 
 		ps, ok := w.state.GetPortState(portKey)
 		if !ok || !ps.Open {
@@ -1231,7 +1346,7 @@ func (w *Watcher) checkTLSCert(
 		return
 	}
 
-	certKey := fmt.Sprintf("%s:%d:%s", ip, tlsPort, hostname)
+	certKey := net.JoinHostPort(ip, strconv.Itoa(tlsPort)) + ":" + hostname
 	now := time.Now().UTC()
 	prev, hasPrev := w.state.GetCertificateState(certKey)
 

internal/watcher/watcher_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"log/slog"
+	"net"
 	"os"
 	"slices"
 	"strings"
@@ -139,6 +140,22 @@ func (m *mockNotifier) getNotifications() []notification {
 	return result
 }
 
+// cancelOnNotify is a mockNotifier that calls cancel at each
+// notification, as if shutdown cut the check short there.
+type cancelOnNotify struct {
+	*mockNotifier
+
+	cancel context.CancelFunc
+}
+
+func (n cancelOnNotify) SendNotification(
+	ctx context.Context,
+	title, message, priority string,
+) {
+	n.mockNotifier.SendNotification(ctx, title, message, priority)
+	n.cancel()
+}
+
 // --- Helpers to build a Watcher and run its checks against live DNS ---
 
 type testDeps struct {
@@ -461,6 +478,54 @@ func TestDomainPortAndTLSChecks(t *testing.T) {
 	}
 }
 
+// TestFirstCheckAfterLoadingFileWithoutSections loads a state file
+// holding {}, and one that leaves out the certificates section, and runs
+// the first check. It saves domain, hostname, port and certificate
+// entries; a section loaded as nil made it panic.
+func TestFirstCheckAfterLoadingFileWithoutSections(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		file string
+	}{
+		{"no sections", `{}`},
+		{"no certificates", `{"domains": {}, "hostnames": {}, "ports": {}}`},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			cfg := defaultTestConfig(t)
+			cfg.Domains = []string{testSmallDomain}
+
+			_, deps := runChecks(t, cfg, func(deps *testDeps) {
+				err := os.WriteFile(cfg.StatePath(), []byte(tt.file), 0o600)
+				if err != nil {
+					t.Fatalf("writing the state file: %v", err)
+				}
+
+				err = deps.state.Load()
+				if err != nil {
+					t.Fatalf("loading the state file: %v", err)
+				}
+			})
+
+			snap := deps.state.GetSnapshot()
+			if len(snap.Domains) == 0 || len(snap.Hostnames) == 0 ||
+				len(snap.Ports) == 0 || len(snap.Certificates) == 0 {
+				t.Errorf(
+					"saved %d domain, %d hostname, %d port and %d "+
+						"certificate entries, want at least one of each",
+					len(snap.Domains), len(snap.Hostnames),
+					len(snap.Ports), len(snap.Certificates),
+				)
+			}
+		})
+	}
+}
+
 func TestNSChangeDetection(t *testing.T) {
 	t.Parallel()
 
@@ -558,6 +623,11 @@ func TestDomainThatDoesNotExist(t *testing.T) {
 // nameservers and without nxdomain, and its records, asked at the
 // codeberg.page servers, are saved. Those are testSmallDomain's two
 // nameservers; github.io, the zone of the README's example, has eight.
+// The previous check saved a delegation to oldNS1, now withdrawn, and
+// records asked at oldNS1. A first check is cut short at its first
+// notification, as shutdown would, and a second runs in full: across
+// them the NS Change of its delegation removes oldNS1, and no other NS
+// Change lists it again.
 func TestDomainWithNoDelegationOfItsOwn(t *testing.T) {
 	t.Parallel()
 
@@ -569,9 +639,30 @@ func TestDomainWithNoDelegationOfItsOwn(t *testing.T) {
 	var deps *testDeps
 
 	livednstest.Retry(t, "watcher checks", func(ctx context.Context) error {
-		var w *watcher.Watcher
+		deps = newTestDeps(t, cfg)
 
-		w, deps = newTestWatcher(t, cfg)
+		firstCtx, cancel := context.WithCancel(ctx)
+		defer cancel()
+
+		w := watcher.NewForTest(
+			cfg,
+			deps.state,
+			resolver.NewFromLogger(slog.Default()),
+			deps.portChecker,
+			deps.tlsChecker,
+			cancelOnNotify{mockNotifier: deps.notifier, cancel: cancel},
+		)
+
+		deps.state.SetDomainState(domain, &state.DomainState{
+			Nameservers: []string{oldNS1},
+		})
+		deps.state.SetHostnameState(domain, saved(
+			map[string]*state.NameserverRecordState{
+				oldNS1: answered(map[string][]string{"A": {oldIP}}),
+			},
+		))
+
+		w.RunOnce(firstCtx)
 
 		err := checkOnce(ctx, w, deps)
 
@@ -593,6 +684,22 @@ func TestDomainWithNoDelegationOfItsOwn(t *testing.T) {
 	if _, ok := deps.state.GetHostnameState(domain); !ok {
 		t.Errorf("no records saved for %s", domain)
 	}
+
+	title := "NS Change: " + domain
+	want := "Domain: " + domain + "\nAdded: \nRemoved: " + oldNS1
+
+	var changes []notification
+
+	for _, n := range deps.notifier.getNotifications() {
+		if n.Title == title {
+			changes = append(changes, n)
+		}
+	}
+
+	if len(changes) != 1 || changes[0].Message != want {
+		t.Errorf("sent %v, want one %q with message:\n%s",
+			changes, title, want)
+	}
 }
 
 func TestNSAddressChangeDetection(t *testing.T) {
@@ -718,7 +825,8 @@ func TestPortStateChange(t *testing.T) {
 
 	hs, _ := deps.state.GetHostnameState(testHost)
 	assertNotified(
-		t, deps, "Port Change: "+addresses(hs)[0]+":443", "warning",
+		t, deps, "Port Change: "+net.JoinHostPort(addresses(hs)[0], "443"),
+		"warning",
 	)
 }
 
@@ -925,11 +1033,12 @@ func TestDNSRunsBeforePortAndTLSChecks(t *testing.T) {
 
 	// Port and TLS checks must use the addresses this check found.
 	for _, ip := range addresses(snap.Hostnames[testHost]) {
-		if _, ok := snap.Ports[ip+":80"]; !ok {
+		portKey := net.JoinHostPort(ip, "80")
-			t.Error("port check used stale DNS: missing " + ip + ":80")
+		if _, ok := snap.Ports[portKey]; !ok {
+			t.Error("port check used stale DNS: missing " + portKey)
 		}
 
-		certKey := ip + ":443:" + testHost
+		certKey := net.JoinHostPort(ip, "443") + ":" + testHost
 		if _, ok := snap.Certificates[certKey]; !ok {
 			t.Error("TLS check used stale DNS: missing " + certKey)
 		}
@@ -1049,7 +1158,7 @@ func TestSendTestNotification_Disabled(t *testing.T) {
 	}
 }
 
-func TestNSFailureAndRecovery(t *testing.T) {
+func TestNSChangeAndRecovery(t *testing.T) {
 	t.Parallel()
 
 	cfg := defaultTestConfig(t)
@@ -1059,7 +1168,7 @@ func TestNSFailureAndRecovery(t *testing.T) {
 
 	// The saved state has every nameserver live DNS lists as one that
 	// did not answer, and, as answering, one that live DNS does not
-	// list, which then disappears.
+	// list, which the check then finds removed.
 	_, deps := runChecks(t, cfg, func(deps *testDeps) {
 		byNameserver := map[string]*state.NameserverRecordState{
 			oldNS1: answered(map[string][]string{"A": {oldIP}}),
@@ -1071,7 +1180,7 @@ func TestNSFailureAndRecovery(t *testing.T) {
 		deps.state.SetHostnameState(testHost, saved(byNameserver))
 	})
 
-	assertNotified(t, deps, "NS Failure: "+testHost, "error")
+	assertNotified(t, deps, "NS Change: "+testHost, "warning")
 	assertNotified(t, deps, "NS Recovery: "+testHost, "success")
 
 	// A nameserver that did not answer has no records to compare, so