queryDNS no longer resends a refused query asking for recursion. On a network that intercepts DNS, that resend let answers come silently from a recursive resolver, against the README's promise that dnswatcher never relies on one. A refusal is now only a refusal: the server is passed over for the next, as since #197.
When every server of a zone refuses, the error says so instead of "all servers failed". When every root server refuses, it is ErrIntercepted ("this network intercepts DNS queries"): root servers refuse no query, so something on the network is answering for them, and FindAuthoritativeNameservers stops there rather than trying each parent name. A nameserver refusing the watched name's queries is still saved as error with "server returned REFUSED".
The live tests do not rely on the resend: no root server refuses queries from the build host or its Docker build network.
The resend is guarded by a live test that asks Quad9, a public recursive resolver, only to see it refuse a query not asking for recursion. ErrIntercepted is tested by passing google.com's nameservers, which refuse a query about cloudflare.com, as the root servers.
Unverified: the stop in FindAuthoritativeNameservers has no test; it always asks the fixed root server list.
Unverified: whether the Gitea Actions runners' network intercepts DNS.
Judgement call: interception is reported only when every root server refuses; a network answering in their place without refusing is not detected.
Judgement call: AdGuard DNS, suggested in review, is not used: it answers some queries not asking for recursion from its cache.
Model: opus-5-5
`queryDNS` no longer resends a refused query asking for recursion. On a network that intercepts DNS, that resend let answers come silently from a recursive resolver, against the README's promise that dnswatcher never relies on one. A refusal is now only a refusal: the server is passed over for the next, as since https://git.eeqj.de/sneak/dnswatcher/issues/197.
When every server of a zone refuses, the error says so instead of "all servers failed". When every root server refuses, it is `ErrIntercepted` ("this network intercepts DNS queries"): root servers refuse no query, so something on the network is answering for them, and `FindAuthoritativeNameservers` stops there rather than trying each parent name. A nameserver refusing the watched name's queries is still saved as `error` with "server returned REFUSED".
The live tests do not rely on the resend: no root server refuses queries from the build host or its Docker build network.
The resend is guarded by a live test that asks Quad9, a public recursive resolver, only to see it refuse a query not asking for recursion. `ErrIntercepted` is tested by passing google.com's nameservers, which refuse a query about cloudflare.com, as the root servers.
- Unverified: the stop in `FindAuthoritativeNameservers` has no test; it always asks the fixed root server list.
- Unverified: whether the Gitea Actions runners' network intercepts DNS.
- Judgement call: interception is reported only when every root server refuses; a network answering in their place without refusing is not detected.
- Judgement call: AdGuard DNS, suggested in review, is not used: it answers some queries not asking for recursion from its cache.
Model: opus-5-5
internal/resolver/iterative.go, queryDNS: no test fails when the old resend (asking the server again with recursion after it refused) is put back, so the main change of #206 is unguarded, and the PR body does not say so. Acceptable: a live test against real servers that refuse a query not asking for recursion but answer one that does (Quad9 at 9.9.9.9 and AdGuard DNS at 94.140.14.14 both do today), asserting the refusal is reported, for example QueryNameserverIP giving status error with "server returned REFUSED".
internal/resolver/iterative.go, queryServers, the ErrIntercepted branch: no test fails when it is deleted. The PR says it cannot be tested without a stand-in, but live DNS reaches it: the google.com nameservers that TestQueryServers_EveryServerRefused already collects all refuse, so passing them to QueryServers with zone . gives this error. Acceptable: that live test, asserting ErrIntercepted and its message. The stop in FindAuthoritativeNameservers can stay untested as disclosed, since it always asks the fixed root server list.
Judgement call: both tests use real servers giving real refusals, so neither is a stand-in; the first uses a public recursive resolver only to see it refuse, never for answers.
Model: opus-5-5
1. `internal/resolver/iterative.go`, `queryDNS`: no test fails when the old resend (asking the server again with recursion after it refused) is put back, so the main change of https://git.eeqj.de/sneak/dnswatcher/issues/206 is unguarded, and the PR body does not say so. Acceptable: a live test against real servers that refuse a query not asking for recursion but answer one that does (Quad9 at `9.9.9.9` and AdGuard DNS at `94.140.14.14` both do today), asserting the refusal is reported, for example `QueryNameserverIP` giving status `error` with "server returned REFUSED".
2. `internal/resolver/iterative.go`, `queryServers`, the `ErrIntercepted` branch: no test fails when it is deleted. The PR says it cannot be tested without a stand-in, but live DNS reaches it: the google.com nameservers that `TestQueryServers_EveryServerRefused` already collects all refuse, so passing them to `QueryServers` with zone `.` gives this error. Acceptable: that live test, asserting `ErrIntercepted` and its message. The stop in `FindAuthoritativeNameservers` can stay untested as disclosed, since it always asks the fixed root server list.
Judgement call: both tests use real servers giving real refusals, so neither is a stand-in; the first uses a public recursive resolver only to see it refuse, never for answers.
Model: opus-5-5
Added TestQueryNameserverIP_RecursiveResolverRefused, asking Quad9 at both its addresses; it fails if the resend is put back. AdGuard DNS is not used: it answers some queries not asking for recursion from its cache.
Added TestQueryServers_EveryRootServerRefused; it fails if the ErrIntercepted branch is deleted. The PR body's "Unverified" lines now name only the untested stop in FindAuthoritativeNameservers and the runners' network.
Model: opus-5-5
1. Added `TestQueryNameserverIP_RecursiveResolverRefused`, asking Quad9 at both its addresses; it fails if the resend is put back. AdGuard DNS is not used: it answers some queries not asking for recursion from its cache.
2. Added `TestQueryServers_EveryRootServerRefused`; it fails if the `ErrIntercepted` branch is deleted. The PR body's "Unverified" lines now name only the untested stop in `FindAuthoritativeNameservers` and the runners' network.
Model: opus-5-5
The branch no longer rebases onto next. next now has the change for #138 (servers tried in a random order). Both changes edit queryServers in internal/resolver/iterative.go (its doc comment and the line that starts the loop over the servers) and the top of the TODO.md Completed Steps list. Acceptable: the branch rebased onto current next, keeping both the random order and the count of refusals, with the queryServers comment covering both, and both TODO.md entries.
Unverified: this change combined with current next. It was reviewed only on its own base.
Model: opus-5-5
1. The branch no longer rebases onto `next`. `next` now has the change for https://git.eeqj.de/sneak/dnswatcher/issues/138 (servers tried in a random order). Both changes edit `queryServers` in `internal/resolver/iterative.go` (its doc comment and the line that starts the loop over the servers) and the top of the `TODO.md` Completed Steps list. Acceptable: the branch rebased onto current `next`, keeping both the random order and the count of refusals, with the `queryServers` comment covering both, and both `TODO.md` entries.
Unverified: this change combined with current `next`. It was reviewed only on its own base.
Model: opus-5-5
Rebased onto next. In queryServers the loop now walks shuffled(servers, rand.Shuffle) and counts refusals as it goes; since the count is compared with len(servers) after the loop, the order does not change it. The comment says the servers are asked in a random order, then what is returned when every server refused. TODO.md keeps both Completed Steps entries, this PR's first. Nothing else changed.
Model: opus-5-5
Rebased onto `next`. In `queryServers` the loop now walks `shuffled(servers, rand.Shuffle)` and counts refusals as it goes; since the count is compared with `len(servers)` after the loop, the order does not change it. The comment says the servers are asked in a random order, then what is returned when every server refused. `TODO.md` keeps both Completed Steps entries, this PR's first. Nothing else changed.
Model: opus-5-5
internal/resolver/resolver_test.go, the QueryServers tests: each one passes a list where every server refuses, so nothing checks a list where they do not all refuse. If queryServers counted a server that does not reply as a refusal, or reported interception when only some servers refused, every test would still pass, and a host that cannot reach the root servers would log "this network intercepts DNS queries". With the servers now asked in a random order, a list that all refuse also cannot show that the count is right whatever the order. Acceptable: a live test that passes google.com's nameservers plus 192.0.2.1 (where nothing answers, as in TestQueryNameserverIP_Timeout) to QueryServers as the servers of ., and asserts the error is not ErrIntercepted and does not say every server refused. Checking errors.Is(err, ErrRefused) will not do for the second part: when the servers failed in different ways the error wraps the last server's error, which may be a refusal.
Model: opus-5-5
1. `internal/resolver/resolver_test.go`, the `QueryServers` tests: each one passes a list where every server refuses, so nothing checks a list where they do not all refuse. If `queryServers` counted a server that does not reply as a refusal, or reported interception when only some servers refused, every test would still pass, and a host that cannot reach the root servers would log "this network intercepts DNS queries". With the servers now asked in a random order, a list that all refuse also cannot show that the count is right whatever the order. Acceptable: a live test that passes google.com's nameservers plus `192.0.2.1` (where nothing answers, as in `TestQueryNameserverIP_Timeout`) to `QueryServers` as the servers of `.`, and asserts the error is not `ErrIntercepted` and does not say every server refused. Checking `errors.Is(err, ErrRefused)` will not do for the second part: when the servers failed in different ways the error wraps the last server's error, which may be a refusal.
Model: opus-5-5
Added TestQueryServers_NotEveryRootServerRefused: google.com's nameservers plus 192.0.2.1 as the servers of ., asserting the error is not ErrIntercepted and does not say every server refused, without errors.Is(err, ErrRefused). It fails when a server that gives no reply is counted as a refusal, and when interception is reported though only some servers refused. Nothing else changed.
Judgement call: the test waits for 192.0.2.1 to time out (two tries of two seconds each), so it is now the slowest test in the resolver package; that wait is what it checks.
Model: opus-5-5
Added `TestQueryServers_NotEveryRootServerRefused`: google.com's nameservers plus `192.0.2.1` as the servers of `.`, asserting the error is not `ErrIntercepted` and does not say every server refused, without `errors.Is(err, ErrRefused)`. It fails when a server that gives no reply is counted as a refusal, and when interception is reported though only some servers refused. Nothing else changed.
Judgement call: the test waits for `192.0.2.1` to time out (two tries of two seconds each), so it is now the slowest test in the resolver package; that wait is what it checks.
Model: opus-5-5
internal/resolver/resolver_test.go, googleNameserverIPv4s: it gets the addresses of google.com's nameservers with ResolveIPAddresses, which for each nameserver name finds google.com's nameservers again and asks every one of them for eight record types, and each of the three QueryServers tests runs it afresh. next already has liveResolveNSIPs in internal/resolver/livedns_test.go for this job (the IPv4 addresses of named nameservers, through ResolveNSIPs), with a small fraction of the queries. As it is, these three are the slowest tests in the resolver package and more than double its run time, and their extra live queries run alongside the watcher tests, which already run out of time in CI (#214). Acceptable: the tests get the addresses with liveResolveNSIPs, so the change adds little to the resolver tests' run time; the wait for 192.0.2.1 can stay.
Judgement call: when every root server refuses, no nameserver is reached, so only the log says so; I read "the nameserver status" in the definition of done as that of a nameserver that refuses, which is still "server returned REFUSED".
Unverified: whether the CI runners' network intercepts DNS.
Model: opus-5-5
1. `internal/resolver/resolver_test.go`, `googleNameserverIPv4s`: it gets the addresses of google.com's nameservers with `ResolveIPAddresses`, which for each nameserver name finds google.com's nameservers again and asks every one of them for eight record types, and each of the three `QueryServers` tests runs it afresh. `next` already has `liveResolveNSIPs` in `internal/resolver/livedns_test.go` for this job (the IPv4 addresses of named nameservers, through `ResolveNSIPs`), with a small fraction of the queries. As it is, these three are the slowest tests in the resolver package and more than double its run time, and their extra live queries run alongside the watcher tests, which already run out of time in CI (https://git.eeqj.de/sneak/dnswatcher/issues/214). Acceptable: the tests get the addresses with `liveResolveNSIPs`, so the change adds little to the resolver tests' run time; the wait for `192.0.2.1` can stay.
Judgement call: when every root server refuses, no nameserver is reached, so only the log says so; I read "the nameserver status" in the definition of done as that of a nameserver that refuses, which is still "server returned REFUSED".
Unverified: whether the CI runners' network intercepts DNS.
Model: opus-5-5
googleNameserverIPv4s now gets the addresses of google.com's nameservers with liveResolveNSIPs, once their names are found as before, and waits until each name has an address, as before. The three QueryServers tests are otherwise unchanged and each still fails when the code it guards is put back. Nothing else changed.
Model: opus-5-5
`googleNameserverIPv4s` now gets the addresses of google.com's nameservers with `liveResolveNSIPs`, once their names are found as before, and waits until each name has an address, as before. The three `QueryServers` tests are otherwise unchanged and each still fails when the code it guards is put back. Nothing else changed.
Model: opus-5-5
queryDNS resent a query that a server refused, this time asking for
recursion, so on a network that intercepts DNS the answers could come
from a recursive resolver without anyone knowing. A refusal is now only
a refusal, and the server is passed over for the next.
When every server of a zone refuses, the error says so. When every root
server refuses, the error is ErrIntercepted: root servers refuse no
query, so something on the network is answering in their place.
FindAuthoritativeNameservers stops at that error instead of trying each
parent name, so the watcher's log line says it.
A live test asks Quad9, which refuses a query not asking for recursion,
so that the resend cannot come back unnoticed.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
queryDNSno longer resends a refused query asking for recursion. On a network that intercepts DNS, that resend let answers come silently from a recursive resolver, against the README's promise that dnswatcher never relies on one. A refusal is now only a refusal: the server is passed over for the next, as since #197.When every server of a zone refuses, the error says so instead of "all servers failed". When every root server refuses, it is
ErrIntercepted("this network intercepts DNS queries"): root servers refuse no query, so something on the network is answering for them, andFindAuthoritativeNameserversstops there rather than trying each parent name. A nameserver refusing the watched name's queries is still saved aserrorwith "server returned REFUSED".The live tests do not rely on the resend: no root server refuses queries from the build host or its Docker build network.
The resend is guarded by a live test that asks Quad9, a public recursive resolver, only to see it refuse a query not asking for recursion.
ErrInterceptedis tested by passing google.com's nameservers, which refuse a query about cloudflare.com, as the root servers.FindAuthoritativeNameservershas no test; it always asks the fixed root server list.Model: opus-5-5
internal/resolver/iterative.go,queryDNS: no test fails when the old resend (asking the server again with recursion after it refused) is put back, so the main change of #206 is unguarded, and the PR body does not say so. Acceptable: a live test against real servers that refuse a query not asking for recursion but answer one that does (Quad9 at9.9.9.9and AdGuard DNS at94.140.14.14both do today), asserting the refusal is reported, for exampleQueryNameserverIPgiving statuserrorwith "server returned REFUSED".internal/resolver/iterative.go,queryServers, theErrInterceptedbranch: no test fails when it is deleted. The PR says it cannot be tested without a stand-in, but live DNS reaches it: the google.com nameservers thatTestQueryServers_EveryServerRefusedalready collects all refuse, so passing them toQueryServerswith zone.gives this error. Acceptable: that live test, assertingErrInterceptedand its message. The stop inFindAuthoritativeNameserverscan stay untested as disclosed, since it always asks the fixed root server list.Judgement call: both tests use real servers giving real refusals, so neither is a stand-in; the first uses a public recursive resolver only to see it refuse, never for answers.
Model: opus-5-5
2c03858092toc3f2a7ab16TestQueryNameserverIP_RecursiveResolverRefused, asking Quad9 at both its addresses; it fails if the resend is put back. AdGuard DNS is not used: it answers some queries not asking for recursion from its cache.TestQueryServers_EveryRootServerRefused; it fails if theErrInterceptedbranch is deleted. The PR body's "Unverified" lines now name only the untested stop inFindAuthoritativeNameserversand the runners' network.Model: opus-5-5
next.nextnow has the change for #138 (servers tried in a random order). Both changes editqueryServersininternal/resolver/iterative.go(its doc comment and the line that starts the loop over the servers) and the top of theTODO.mdCompleted Steps list. Acceptable: the branch rebased onto currentnext, keeping both the random order and the count of refusals, with thequeryServerscomment covering both, and bothTODO.mdentries.Unverified: this change combined with current
next. It was reviewed only on its own base.Model: opus-5-5
c3f2a7ab16to4d8ddaf8e9Rebased onto
next. InqueryServersthe loop now walksshuffled(servers, rand.Shuffle)and counts refusals as it goes; since the count is compared withlen(servers)after the loop, the order does not change it. The comment says the servers are asked in a random order, then what is returned when every server refused.TODO.mdkeeps both Completed Steps entries, this PR's first. Nothing else changed.Model: opus-5-5
internal/resolver/resolver_test.go, theQueryServerstests: each one passes a list where every server refuses, so nothing checks a list where they do not all refuse. IfqueryServerscounted a server that does not reply as a refusal, or reported interception when only some servers refused, every test would still pass, and a host that cannot reach the root servers would log "this network intercepts DNS queries". With the servers now asked in a random order, a list that all refuse also cannot show that the count is right whatever the order. Acceptable: a live test that passes google.com's nameservers plus192.0.2.1(where nothing answers, as inTestQueryNameserverIP_Timeout) toQueryServersas the servers of., and asserts the error is notErrInterceptedand does not say every server refused. Checkingerrors.Is(err, ErrRefused)will not do for the second part: when the servers failed in different ways the error wraps the last server's error, which may be a refusal.Model: opus-5-5
4d8ddaf8e9toacde238d00Added
TestQueryServers_NotEveryRootServerRefused: google.com's nameservers plus192.0.2.1as the servers of., asserting the error is notErrInterceptedand does not say every server refused, withouterrors.Is(err, ErrRefused). It fails when a server that gives no reply is counted as a refusal, and when interception is reported though only some servers refused. Nothing else changed.Judgement call: the test waits for
192.0.2.1to time out (two tries of two seconds each), so it is now the slowest test in the resolver package; that wait is what it checks.Model: opus-5-5
internal/resolver/resolver_test.go,googleNameserverIPv4s: it gets the addresses of google.com's nameservers withResolveIPAddresses, which for each nameserver name finds google.com's nameservers again and asks every one of them for eight record types, and each of the threeQueryServerstests runs it afresh.nextalready hasliveResolveNSIPsininternal/resolver/livedns_test.gofor this job (the IPv4 addresses of named nameservers, throughResolveNSIPs), with a small fraction of the queries. As it is, these three are the slowest tests in the resolver package and more than double its run time, and their extra live queries run alongside the watcher tests, which already run out of time in CI (#214). Acceptable: the tests get the addresses withliveResolveNSIPs, so the change adds little to the resolver tests' run time; the wait for192.0.2.1can stay.Judgement call: when every root server refuses, no nameserver is reached, so only the log says so; I read "the nameserver status" in the definition of done as that of a nameserver that refuses, which is still "server returned REFUSED".
Unverified: whether the CI runners' network intercepts DNS.
Model: opus-5-5
acde238d00to7bd8eddba9googleNameserverIPv4snow gets the addresses of google.com's nameservers withliveResolveNSIPs, once their names are found as before, and waits until each name has an address, as before. The threeQueryServerstests are otherwise unchanged and each still fails when the code it guards is put back. Nothing else changed.Model: opus-5-5
Review passed on
7bd8edd.Model: opus-5-5
7bd8eddba9to690d8ce1a3