From c9510a986c77b95e23d048f9d907ebedd579d3b6 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 2 Oct 2026 01:58:28 +0200 Subject: [PATCH] watcher: warn of an expiring certificate on every TLS check (closes #204) An expiry warning was skipped when the last one for that hostname and address was sent less than DNSWATCHER_TLS_INTERVAL ago. Each TLS check runs after a DNS pass of varying length, so two checks can be less than the interval apart, and a certificate about to expire was warned about on every check or every other check, at random. TLS checks already start once per interval, so the in-memory record of when each warning was sent is removed and every check warns, as the README says. The test that expected the second check to stay silent is replaced by one that runs TLS checks on state built in the test, with no DNS. Model: opus-5-5 --- TODO.md | 2 ++ internal/watcher/export_test.go | 22 +++++++----- internal/watcher/watcher.go | 56 ++++++++++------------------- internal/watcher/watcher_test.go | 61 +++++++++++++++++++++----------- 4 files changed, 74 insertions(+), 67 deletions(-) diff --git a/TODO.md b/TODO.md index aa422dc..6b45c81 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149 # Completed Steps +- 2026-10-01: a certificate within the expiry warning period is warned about on + every TLS check, where some checks used to skip it at random (closes #204). - 2026-10-01: a domain's NS set is its delegation from the parent zone's servers, not whichever of its own servers answered first (closes #200). - 2026-10-01: README has Getting Started, Rationale and TODO sections, and its diff --git a/internal/watcher/export_test.go b/internal/watcher/export_test.go index 557e787..59f0319 100644 --- a/internal/watcher/export_test.go +++ b/internal/watcher/export_test.go @@ -20,15 +20,14 @@ func NewForTest( n Notifier, ) *Watcher { return &Watcher{ - log: slog.Default(), - config: cfg, - state: st, - resolver: res, - portCheck: pc, - tlsCheck: tc, - notify: n, - firstRun: true, - expiryNotified: make(map[string]time.Time), + log: slog.Default(), + config: cfg, + state: st, + resolver: res, + portCheck: pc, + tlsCheck: tc, + notify: n, + firstRun: true, } } @@ -72,6 +71,11 @@ func (w *Watcher) CheckAllPorts(ctx context.Context) { w.checkAllPorts(ctx) } +// RunTLSChecks exports runTLSChecks for testing. +func (w *Watcher) RunTLSChecks(ctx context.Context) { + w.runTLSChecks(ctx) +} + // BuildHostnameState exports buildHostnameState for testing. func BuildHostnameState( results map[string]*resolver.NameserverResponse, diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index 2df33f4..bcee203 100644 --- a/internal/watcher/watcher.go +++ b/internal/watcher/watcher.go @@ -7,7 +7,6 @@ import ( "slices" "sort" "strings" - "sync" "time" "go.uber.org/fx" @@ -49,18 +48,16 @@ type Params struct { // Watcher orchestrates all monitoring checks on a schedule. type Watcher struct { - log *slog.Logger - config *config.Config - state *state.State - resolver DNSResolver - portCheck PortChecker - tlsCheck TLSChecker - notify Notifier - cancel context.CancelFunc - done chan struct{} // closed when Run returns - firstRun bool - expiryNotifiedMu sync.Mutex - expiryNotified map[string]time.Time + log *slog.Logger + config *config.Config + state *state.State + resolver DNSResolver + portCheck PortChecker + tlsCheck TLSChecker + notify Notifier + cancel context.CancelFunc + done chan struct{} // closed when Run returns + firstRun bool } // New creates a new Watcher instance wired into the fx lifecycle. @@ -69,15 +66,14 @@ func New( params Params, ) (*Watcher, error) { w := &Watcher{ - log: params.Logger.Get(), - config: params.Config, - state: params.State, - resolver: params.Resolver, - portCheck: params.PortCheck, - tlsCheck: params.TLSCheck, - notify: params.Notify, - firstRun: true, - expiryNotified: make(map[string]time.Time), + log: params.Logger.Get(), + config: params.Config, + state: params.State, + resolver: params.Resolver, + portCheck: params.PortCheck, + tlsCheck: params.TLSCheck, + notify: params.Notify, + firstRun: true, } lifecycle.Append(fx.Hook{ @@ -1028,22 +1024,6 @@ func (w *Watcher) checkTLSExpiry( return } - // Deduplicate expiry warnings: don't re-notify for the same - // hostname within the TLS check interval. - dedupKey := fmt.Sprintf("expiry:%s:%s", hostname, ip) - - w.expiryNotifiedMu.Lock() - - lastNotified, seen := w.expiryNotified[dedupKey] - if seen && time.Since(lastNotified) < w.config.TLSInterval { - w.expiryNotifiedMu.Unlock() - - return - } - - w.expiryNotified[dedupKey] = time.Now() - w.expiryNotifiedMu.Unlock() - msg := fmt.Sprintf( "Host: %s\nIP: %s\nCN: %s\n"+ "Expires: %s (%.0f days)", diff --git a/internal/watcher/watcher_test.go b/internal/watcher/watcher_test.go index c166a3e..9918fc8 100644 --- a/internal/watcher/watcher_test.go +++ b/internal/watcher/watcher_test.go @@ -615,33 +615,54 @@ func TestTLSExpiryWarning(t *testing.T) { assertNotified(t, deps, "TLS Expiry Warning: "+testHost, "warning") } -func TestTLSExpiryWarningDedup(t *testing.T) { +// TestTLSExpiryWarningEachCheck runs the TLS checks three times in a +// row on hostname and port state built here, for a certificate that +// expires within the warning period. Each check warns once, whether the +// TLS interval is a nanosecond, shorter than the time between two +// checks, or a day, longer than it. +func TestTLSExpiryWarningEachCheck(t *testing.T) { t.Parallel() - cfg := defaultTestConfig(t) - cfg.Hostnames = []string{testHost} - cfg.TLSInterval = 24 * time.Hour + title := "TLS Expiry Warning: " + host - title := "TLS Expiry Warning: " + testHost + for _, interval := range []time.Duration{time.Nanosecond, 24 * time.Hour} { + t.Run(interval.String(), func(t *testing.T) { + t.Parallel() - // The second check comes within the TLS interval of the first, - // so it must not warn again. - var warnings int + cfg := defaultTestConfig(t) + cfg.Hostnames = []string{host} + cfg.TLSInterval = interval - deps := runChecks(t, cfg, expiresInThreeDays, func(deps *testDeps) { - warnings = countNotifications(deps, title) - }) + // The TLS checks read the saved hostname and port state and + // look nothing up, so the watcher has no resolver. + deps := newTestDeps(t, cfg) + w := watcher.NewForTest( + cfg, deps.state, nil, + deps.portChecker, deps.tlsChecker, deps.notifier, + ) - if warnings == 0 { - t.Fatal("expected expiry warnings from the first check") - } + expiresInThreeDays(deps) + 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}, + }) - got := countNotifications(deps, title) - if got != warnings { - t.Errorf( - "expected %d expiry warnings (dedup), got %d", - warnings, got, - ) + for check := 1; check <= 3; check++ { + w.RunTLSChecks(t.Context()) + + got := countNotifications(deps, title) + if got != check { + t.Fatalf( + "after check %d: %d expiry warnings, want %d", + check, got, check, + ) + } + } + }) } }