fix: enforce DNS-first ordering for port and TLS checks #64

clawbot · 2026-03-02T00:03:30+01:00

clawbot commented

2026-03-02 00:03:30 +01:00

Summary

DNS checks now always complete before port or TLS checks begin, ensuring those checks use freshly resolved IP addresses instead of potentially stale ones from a previous cycle.

Problem

Port and TLS checks read IP addresses from state that was populated during the most recent DNS check. If DNS changes between cycles, port/TLS checks may target stale IPs. In particular, when the TLS ticker fired (every 12h), it ran runTLSChecks without refreshing DNS first — meaning TLS checks could use IPs that were up to 12 hours old.

Changes

Extract runDNSChecks() from the former runDNSAndPortChecks() so DNS resolution can be invoked independently as a prerequisite for any check type.
TLS ticker now runs DNS first: When the TLS ticker fires, DNS checks run before TLS checks, ensuring fresh IPs.
RunOnce uses explicit 3-phase ordering: DNS → ports → TLS. Port checks must complete before TLS because TLS checks only target IPs where port 443 is open.
New test TestDNSRunsBeforePortAndTLSChecks: Verifies that when DNS IPs change between cycles, port and TLS checks pick up the new IPs.
README updated: Monitoring lifecycle section now documents the DNS-first ordering guarantee.

Check ordering

Trigger	Phase 1	Phase 2	Phase 3
Startup (`RunOnce`)	DNS	Ports	TLS
DNS ticker	DNS	Ports	—
TLS ticker	DNS	—	TLS

closes #58

## Summary DNS checks now always complete before port or TLS checks begin, ensuring those checks use freshly resolved IP addresses instead of potentially stale ones from a previous cycle. ## Problem Port and TLS checks read IP addresses from state that was populated during the most recent DNS check. If DNS changes between cycles, port/TLS checks may target stale IPs. In particular, when the TLS ticker fired (every 12h), it ran `runTLSChecks` without refreshing DNS first — meaning TLS checks could use IPs that were up to 12 hours old. ## Changes - **Extract `runDNSChecks()`** from the former `runDNSAndPortChecks()` so DNS resolution can be invoked independently as a prerequisite for any check type. - **TLS ticker now runs DNS first**: When the TLS ticker fires, DNS checks run before TLS checks, ensuring fresh IPs. - **`RunOnce` uses explicit 3-phase ordering**: DNS → ports → TLS. Port checks must complete before TLS because TLS checks only target IPs where port 443 is open. - **New test `TestDNSRunsBeforePortAndTLSChecks`**: Verifies that when DNS IPs change between cycles, port and TLS checks pick up the new IPs. - **README updated**: Monitoring lifecycle section now documents the DNS-first ordering guarantee. ## Check ordering | Trigger | Phase 1 | Phase 2 | Phase 3 | |---------|---------|---------|----------| | Startup (`RunOnce`) | DNS | Ports | TLS | | DNS ticker | DNS | Ports | — | | TLS ticker | DNS | — | TLS | closes https://git.eeqj.de/sneak/dnswatcher/issues/58

clawbot added the needs-review bot labels 2026-03-02 00:03:30 +01:00

clawbot added 1 commit 2026-03-02 00:03:30 +01:00

fix: enforce DNS-first ordering for port and TLS checks

check / check (push) Successful in 46s

Details

461ab7e06c

DNS checks now always complete before port or TLS checks begin,
ensuring those checks use freshly resolved IP addresses instead of
potentially stale ones from a previous cycle.

Changes:
- Extract runDNSChecks() from runDNSAndPortChecks() so DNS resolution
  can be invoked independently
- Run DNS before TLS on the TLS ticker (previously TLS ran alone with
  whatever IPs were in state from the last DNS cycle, up to 12h stale)
- RunOnce uses explicit 3-phase ordering: DNS → ports → TLS
- Add TestDNSRunsBeforePortAndTLSChecks to verify fresh IPs propagate
- Update README monitoring lifecycle to document DNS-first ordering

closes #58

clawbot commented

2026-03-02 00:06:59 +01:00

✅ Review: PASS

Summary

Clean, well-structured fix that enforces DNS-first ordering for port and TLS checks.

What was checked

DNS-first ordering verified in all three execution paths:
- RunOnce(): DNS → Ports → TLS (explicit 3-phase)
- DNS ticker: runDNSChecks() → checkAllPorts() → saveState()
- TLS ticker: runDNSChecks() → runTLSChecks() → saveState()
Fresh IPs confirmed: collectIPs() reads from state.GetHostnameState() which is updated by runDNSChecks() via checkHostname() → SetHostnameState(). Port and TLS checks always see the IPs that DNS just resolved.
TLS ticker path: Intentionally runs DNS but not ports. TLS checks filter on port state (from last DNS ticker, ≤1h stale). This is a reasonable trade-off — new IPs get port-checked within 1h on the next DNS ticker.
New test TestDNSRunsBeforePortAndTLSChecks: Correctly verifies that when DNS IPs change between cycles, both port and TLS checks pick up the new IPs. Uses the established mock infrastructure (consistent with all other watcher tests).
README updated per sneak's request — monitoring lifecycle section now documents the DNS-first ordering guarantee.
No test weakening, no config changes, no unrelated changes.
docker build . passes — all tests, lint, and format checks green.

## ✅ Review: PASS ### Summary Clean, well-structured fix that enforces DNS-first ordering for port and TLS checks. ### What was checked 1. **DNS-first ordering verified** in all three execution paths: - `RunOnce()`: DNS → Ports → TLS (explicit 3-phase) - DNS ticker: `runDNSChecks()` → `checkAllPorts()` → `saveState()` - TLS ticker: `runDNSChecks()` → `runTLSChecks()` → `saveState()` 2. **Fresh IPs confirmed**: `collectIPs()` reads from `state.GetHostnameState()` which is updated by `runDNSChecks()` via `checkHostname()` → `SetHostnameState()`. Port and TLS checks always see the IPs that DNS just resolved. 3. **TLS ticker path**: Intentionally runs DNS but not ports. TLS checks filter on port state (from last DNS ticker, ≤1h stale). This is a reasonable trade-off — new IPs get port-checked within 1h on the next DNS ticker. 4. **New test `TestDNSRunsBeforePortAndTLSChecks`**: Correctly verifies that when DNS IPs change between cycles, both port and TLS checks pick up the new IPs. Uses the established mock infrastructure (consistent with all other watcher tests). 5. **README updated** per sneak's request — monitoring lifecycle section now documents the DNS-first ordering guarantee. 6. **No test weakening**, no config changes, no unrelated changes. 7. **`docker build .` passes** — all tests, lint, and format checks green.

clawbot added merge-ready and removed needs-review bot labels 2026-03-02 00:07:14 +01:00

sneak was assigned by clawbot

2026-03-02 00:07:24 +01:00

sneak merged commit ee14bd01ae into main

2026-03-02 00:10:49 +01:00

sneak deleted branch fix/issue-58-check-ordering

2026-03-02 00:10:50 +01:00

sneak referenced this issue from a commit

2026-03-02 00:10:51 +01:00

fix: enforce DNS-first ordering for port and TLS checks (#64)

clawbot referenced this pull request

2026-03-02 00:58:13 +01:00