Compare commits

..
6 Commits
Author SHA1 Message Date
clawbot b23cf43abf resolver: a reply marked recursion available from a root server's address stops the lookup (closes #300)
check / check (push) Waiting to run
dnswatcher never asks for recursion and a root server never offers it,
so a reply marked recursion available from a root server's address
came from a resolver on the network answering in its place. The first
such reply, a refusal included, now stops the lookup at once with
"this network intercepts DNS queries", naming the address, whether or
not that resolver had the name cached. queryDNS returns a refused reply
along with ErrRefused so the mark can be seen; a refusal without the
mark is handled as before. On such a network dnswatcher and its tests
fail with that error, by design, as the owner ruled on the issue.

Model: opus-5-5
2026-10-16 12:26:52 +02:00
clawbot adc8131191 resolver: a query cut short by the caller's context records no reason (closes #298)
check / check (push) Canceled after 0s
When the caller's context ends, the next query returns
ErrContextCanceled before anything is sent. querySingleType recorded
that as a network error, which outranks a timeout, so a nameserver that
had only timed out could be reported as "network error: context
canceled". It now records no reason for it.

A try the context's deadline cuts short ends as a timeout. When the
context has ended before a further try, tryExchange now returns that
try's error instead of ErrContextCanceled, so such a nameserver is
reported as timed out, not as answering.

Model: opus-5-5
2026-10-15 19:48:08 +02:00
clawbot 06ae357165 resolver: drop the unused timeout multiplier and the comment claiming it (closes #296)
check / check (push) Canceled after 0s
The resolver declared timeoutMultiplier and passed each try's number to
exchangeWithTimeout, which threw it away with a comment saying the
client config grew the timeout. Nothing did: every try waits the same
2-second queryTimeoutDuration. The constant goes, and exchangeWithTimeout
is folded into its one caller, tryExchange, whose comment now says the
timeout is fixed. Behaviour is unchanged.

Model: opus-5-5
2026-10-15 18:31:07 +02:00
clawbot f6b7aefcbf docs: TESTING.md constraints match the resolver and the test-time policy (closes #289)
check / check (push) Canceled after 0s
The Constraints list named a query timeout calibrated to round-trip time and a
limited root-server fan-out; the resolver has neither. It now gives the
2-second timeout and the second try over UDP, says a zone's servers are asked
one at a time in a random order, and gives the 60-second cap and 20-second
target for make test from REPO_POLICIES.md. The line on flaky failures now
says, as the README does, that a flaky live test is fixed with robustness,
never with mocks or skip flags.

Model: opus-5-5
2026-10-15 18:01:51 +02:00
clawbot db3b09c5d8 handlers: a directory under /s/ answers 404, not a listing (closes #286)
check / check (push) Canceled after 0s
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
2026-10-15 17:01:52 +02:00
clawbot caa3aa3c12 notify: a reply other than 2xx is a failed delivery (closes #287)
check / check (push) Canceled after 0s
A notification endpoint that answered with a redirect was counted as
delivered: the request is not sent through a client, so the redirect
is never followed, and only a status of 400 or more counted as a
failure. Now any status outside 200-299 fails the delivery for ntfy,
Slack and Mattermost, so it is retried and logged like the others.

Model: opus-5-5
2026-10-15 16:14:20 +02:00
10 changed files with 321 additions and 48 deletions
+13
View File
@@ -561,6 +561,18 @@ This approach ensures:
- Ability to detect split-horizon or inconsistent responses across authoritative
servers.
A root server never offers recursion, so a reply marked "recursion available"
from a root server's address came from a recursive resolver on the network
answering in its place, as on a network where a router rule, a VPN or a DNS
filter sends all DNS to one resolver. The first such reply from a root server's
address, a refusal included, stops the lookup at once with the error "this
network intercepts DNS queries", naming that address, and no other root server
is asked. A lookup that every root server refuses without that mark fails with
the same error. Every lookup starts at the root servers, so on such a network
every lookup fails with that error, whether or not that resolver has the name
cached, and dnswatcher's tests fail there too. That is by design: nothing asks
again another way or otherwise works around the interception.
A watched name's records are stored as its nameservers return them, CNAME
included. When they return a CNAME and no address, the chain of every CNAME
target they gave is followed (with a depth limit to prevent loops) to the A and
@@ -910,6 +922,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
+9 -7
View File
@@ -19,16 +19,18 @@ works correctly in production.
### Constraints
- Tests hit real DNS infrastructure and require network access
- Test duration depends on network conditions; timeout tuning keeps the suite
within the 60-second target
- Query timeout is calibrated to 3× maximum antipodal RTT (~300ms) plus
processing margin
- Root server fan-out is limited to reduce parallel query load
- Test duration depends on network conditions. `make test` must finish in under
60 seconds, the hard cap in `REPO_POLICIES.md`; under 20 seconds is the target
- Each query to a nameserver has a 2-second timeout, and a query over UDP that
gets no reply is sent once more (`queryTimeoutDuration` and `maxRetries` in
`internal/resolver/iterative.go`)
- In the walk to a name's nameservers, the servers of each zone are asked one at
a time in a random order, as the README's DNS Resolution Strategy describes
- Live lookups that expect an answer go through `internal/livednstest`, which
limits how many run at once in a test binary and retries a lookup that got
none
- Flaky failures from transient network issues are acceptable and should be
investigated as potential resolver bugs, not papered over with mocks or skip
- A flaky live test is a robustness problem, fixed with retries with backoff,
several independent nameservers or longer timeouts, never with mocks or skip
flags
### What NOT to do
+17
View File
@@ -19,6 +19,23 @@ milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
# Completed Steps
- 2026-10-16: the first reply marked "recursion available" from a root server's
address, a refusal included, stops the lookup with "this network intercepts
DNS queries", so on a network that sends DNS to a resolver dnswatcher and its
tests fail, by design (closes #300).
- 2026-10-15: a query its caller's context ends before it is sent records no
reason about the nameserver, and a try the context's deadline cuts short is a
timeout, so a nameserver whose only failures were timeouts is `timeout`, not a
network error or `ok` (closes #298).
- 2026-10-15: the resolver no longer says its query timeout grows on each try;
every try waits the same 2 seconds, and the unused `timeoutMultiplier` is gone
(closes #296).
- 2026-10-15: `TESTING.md` Constraints give the resolver's 2-second query
timeout and second try, say a zone's servers are asked one at a time in a
random order, give the 60-second cap and 20-second target for `make test`, and
say a flaky live test is fixed with robustness, not with mocks (closes #289).
- 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
+36
View File
@@ -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)
}
}
+7 -3
View File
@@ -53,9 +53,13 @@ var (
"reply truncated and its retry over TCP failed",
)
// ErrIntercepted is returned when every root server refused a
// query. Root servers refuse no query, so the refusals came from
// something on the network answering in their place.
// ErrIntercepted is returned when a root server's address gave a
// reply marked recursion available, a refusal included, which ends
// the lookup at once, or when every root server refused a query.
// Root servers do neither, so those replies came from something on
// the network answering in their place. On such a network
// dnswatcher fails with it, by design: nothing works around the
// interception.
ErrIntercepted = errors.New("this network intercepts DNS queries")
// ErrCNAMEDepthExceeded is returned when a CNAME chain
+38 -25
View File
@@ -19,7 +19,6 @@ const (
queryTimeoutDuration = 2 * time.Second
maxRetries = 2
maxDelegation = 20
timeoutMultiplier = 2
// maxLookupDepth is how many lookups of nameserver addresses may be
// under way one inside another. Looking up a nameserver's address
@@ -62,19 +61,11 @@ func checkCtx(ctx context.Context) error {
return nil
}
func (r *Resolver) exchangeWithTimeout(
ctx context.Context,
msg *dns.Msg,
addr string,
attempt int,
) (*dns.Msg, error) {
_ = attempt // timeout escalation handled by client config
resp, _, err := r.client.ExchangeContext(ctx, msg, addr)
return resp, err
}
// tryExchange sends msg to addr over UDP, up to maxRetries times, until
// a reply comes. Every try waits the same queryTimeoutDuration, or less
// when ctx's deadline comes first. When ctx has ended before a further
// try, it returns the error of the try already made, such as a timeout,
// or ErrContextCanceled when none was made.
func (r *Resolver) tryExchange(
ctx context.Context,
msg *dns.Msg,
@@ -84,14 +75,16 @@ func (r *Resolver) tryExchange(
var err error
for attempt := range maxRetries {
for range maxRetries {
if checkCtx(ctx) != nil {
return nil, ErrContextCanceled
if err == nil {
return nil, ErrContextCanceled
}
return nil, err
}
resp, err = r.exchangeWithTimeout(
ctx, msg, addr, attempt,
)
resp, _, err = r.client.ExchangeContext(ctx, msg, addr)
if err == nil {
break
}
@@ -122,7 +115,8 @@ func (r *Resolver) retryTCP(
}
// queryDNS sends a DNS query to a specific server IP, never asking it
// for recursion. A reply of REFUSED is returned as ErrRefused.
// for recursion. A reply of REFUSED is returned together with
// ErrRefused, so that the caller can still read the reply's flags.
func (r *Resolver) queryDNS(
ctx context.Context,
serverIP string,
@@ -146,7 +140,7 @@ func (r *Resolver) queryDNS(
}
if resp.Rcode == dns.RcodeRefused {
return nil, fmt.Errorf(
return resp, fmt.Errorf(
"query %s @%s: %w", name, serverIP, ErrRefused,
)
}
@@ -291,10 +285,15 @@ func shuffled(
}
// queryServers asks servers, the servers of zone, about name in a random
// order until one gives a usable reply. A server that times out, refuses
// or gives a reply that is not usable is passed over for the next. When
// every server refused, the error says so, and when they are the root
// servers it is ErrIntercepted.
// order until one gives a usable reply. When they are the root servers,
// the first reply marked recursion available, a refusal included, ends
// the lookup with ErrIntercepted, naming the address it came from, and
// no other root server is asked: root servers offer no recursion, so
// that reply came from something on the network answering in their
// place. Otherwise a server that times out, refuses or gives a reply
// that is not usable is passed over for the next. When every server
// refused, the error says so, and when they are the root servers it is
// ErrIntercepted.
func (r *Resolver) queryServers(
ctx context.Context,
servers []string,
@@ -311,7 +310,17 @@ func (r *Resolver) queryServers(
return nil, ErrContextCanceled
}
// queryDNS returns a refusal together with its reply, so a
// refusal's mark is checked here too.
resp, err := r.queryDNS(ctx, ip, name, qtype)
if resp != nil && zone == "." && resp.RecursionAvailable {
return nil, fmt.Errorf(
"query %s @%s: reply marked recursion available, "+
"so not from a root server: %w",
name, ip, ErrIntercepted,
)
}
if err == nil && !usableReply(resp, zone, name) {
err = fmt.Errorf(
"query %s @%s: %w", name, ip, ErrUnusableReply,
@@ -877,6 +886,10 @@ func (r *Resolver) querySingleType(
if err != nil {
err = fmt.Errorf("query %s @%s: %w", hostname, nsIP, err)
}
case errors.Is(err, ErrContextCanceled):
// The caller's context ended before the query was sent.
// That says nothing about the nameserver, so no reason is
// recorded.
case isTimeout(err):
state.gotTimeout = true
case errors.Is(err, ErrRefused):
@@ -1,6 +1,8 @@
package resolver
import (
"context"
"log/slog"
"strconv"
"syscall"
"testing"
@@ -132,3 +134,33 @@ func TestReadReply(t *testing.T) {
})
}
}
// TestQuerySingleType_ContextEnded asks a nameserver about one record
// type with a context that has already ended, so no query is sent. A
// query the context ended before it was sent says nothing about the
// nameserver and records no reason: a nameserver whose earlier query
// timed out is still reported as timed out, not as a network error.
func TestQuerySingleType_ContextEnded(t *testing.T) {
t.Parallel()
r := NewFromLogger(slog.New(slog.DiscardHandler))
ctx, cancel := context.WithCancel(t.Context())
cancel()
resp := &NameserverResponse{Records: map[string][]string{}, Status: StatusOK}
// The query for an earlier record type timed out.
state := queryState{gotTimeout: true}
_, err := r.querySingleType(
ctx, []string{"192.0.2.1"}, "example.com.", dns.TypeA, resp, &state,
)
require.ErrorIs(t, err, ErrContextCanceled)
assert.Equal(t, queryState{gotTimeout: true}, state)
classifyResponse(resp, state)
assert.Equal(t, StatusTimeout, resp.Status)
assert.Equal(t, "all queries timed out", resp.Error)
}
+128 -4
View File
@@ -850,6 +850,74 @@ func TestQueryServers_NotEveryRootServerRefused(t *testing.T) {
require.NotContains(t, err.Error(), "refused a query for")
}
// TestQueryServers_RecursiveResolversAsRootServers passes public
// recursive resolvers to QueryServers as the servers of the root zone,
// as a network that sends every DNS query to a resolver would. Their
// replies are marked recursion available, which no root server's is:
// SERVFAIL, REFUSED, or an answer from a resolver's cache, as 1.1.1.1
// gives for com., which would be used if a root server gave it. The
// first such reply ends the lookup with ErrIntercepted, naming the
// resolver that gave it. In the first group the replies differ; every
// resolver in the second refuses a query that does not ask for
// recursion, as 9.9.9.9 does, so there a refusal ends the lookup
// instead of being passed over. Each group has resolvers of four
// operators, and a resolver that gives no reply is passed over for the
// next, so one of them answering is enough.
func TestQueryServers_RecursiveResolversAsRootServers(t *testing.T) {
t.Parallel()
r := newTestResolver(t)
groups := [][]string{
{"1.1.1.1", "8.8.8.8", "9.9.9.9", "208.67.222.222"},
{"64.6.64.6", "185.222.222.222", "4.2.2.1", "94.140.14.14"},
}
for _, resolvers := range groups {
for _, name := range []string{"com.", "desec.io."} {
var err error
livednstest.Retry(
t,
"QueryServers("+strings.Join(resolvers, ", ")+
" as root servers, "+name+")",
func(ctx context.Context) error {
_, err = r.QueryServers(
ctx, resolvers, ".", name, dns.TypeNS,
)
// When no resolver gave a reply marked recursion
// available, none may have replied at all, so the
// attempt is tried again.
if err != nil && !strings.Contains(
err.Error(), "marked recursion available",
) {
return fmt.Errorf(
"%w: %w", livednstest.ErrNoAnswer, err,
)
}
return nil
},
)
require.ErrorIs(t, err, resolver.ErrIntercepted)
// The error names the resolver whose reply ended the
// lookup.
messages := make([]string, 0, len(resolvers))
for _, ip := range resolvers {
messages = append(messages,
"query "+name+" @"+ip+": reply marked recursion "+
"available, so not from a root server: "+
"this network intercepts DNS queries",
)
}
require.Contains(t, messages, err.Error())
}
}
}
func TestQueryNameserver_RecordsSorted(t *testing.T) {
t.Parallel()
@@ -1380,10 +1448,10 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
r := newTestResolver(t)
// Nothing answers at 192.0.2.1, a documentation address. The
// resolver tries each query twice, and the first try gives up
// after two seconds. A deadline that ends during the first try
// makes the status vary from run to run between error and
// timeout, so the deadline must outlast the first try.
// resolver tries each query twice, and each try gives up after two
// seconds. This deadline ends during the second try of the first
// record type; TestQueryNameserverIP_DeadlineDuringFirstTry covers
// one that ends during the first.
ctx, cancel := context.WithTimeout(
context.Background(), 3*time.Second,
)
@@ -1399,6 +1467,62 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
assert.NotEmpty(t, resp.Error)
}
// TestQueryNameserverIP_DeadlineDuringFirstTry gives a query to
// 192.0.2.1, where nothing answers, a deadline that ends during its
// first try. That try ends at the deadline as a timeout, so the
// nameserver is reported as timed out, not as answering.
func TestQueryNameserverIP_DeadlineDuringFirstTry(t *testing.T) {
t.Parallel()
r := newTestResolver(t)
// Well under the two seconds one try waits.
ctx, cancel := context.WithTimeout(
context.Background(), 500*time.Millisecond,
)
t.Cleanup(cancel)
resp, err := r.QueryNameserverIP(
ctx, "unreachable.test.", "192.0.2.1",
"example.com",
)
require.NoError(t, err)
assert.Equal(t, resolver.StatusTimeout, resp.Status)
assert.NotEmpty(t, resp.Error)
}
// TestQueryNameserverIP_CancelDuringFirstTry cancels the context while
// the first try of a query to 192.0.2.1, where nothing answers, waits
// for a reply. That try's timeout is all that is learned about the
// nameserver, so it is reported as timed out, not as answering.
func TestQueryNameserverIP_CancelDuringFirstTry(t *testing.T) {
t.Parallel()
r := newTestResolver(t)
// The first try starts at once and waits two seconds for a reply;
// the cancel comes a second in. The DNS client cuts a try short at
// the context's deadline but not at a cancel, so the try still
// waits its full two seconds and ends as a timeout a second after
// the cancel. By then the context has certainly ended, so no second
// try is made. With a deadline instead, the try ends at the moment
// the context does, and whether a second try is made depends on
// which the resolver sees first.
ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)
time.AfterFunc(time.Second, cancel)
resp, err := r.QueryNameserverIP(
ctx, "unreachable.test.", "192.0.2.1",
"example.com",
)
require.NoError(t, err)
assert.Equal(t, resolver.StatusTimeout, resp.Status)
assert.NotEmpty(t, resp.Error)
}
// TestQueryNameserverIP_CancelledLogsNothing cancels the context while
// a query to 192.0.2.1, where nothing answers, is waiting for a reply,
// as shutdown does. The query was cut short, not failed, so nothing is
+1 -9
View File
@@ -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(
"/s",
http.StripPrefix(
"/s",
http.FileServer(http.FS(static.Static)),
),
)
public.Mount("/s", http.StripPrefix("/s", s.handlers.HandleStatic()))
// Health check (standard well-known path)
public.Get(
+40
View File
@@ -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,
)
}
}