Gary HansenandClaude Fable 5
af15c9c2d4
Reconstructed behaviour spec of dns.squish.net / Ruby dnstraverse 0.1.14 (inputs, traversal semantics, probability model, verbatim output formats, sourced from the live site, Wayback captures, and the Ruby source), the engine rework design that maps it onto Go, a point-in-time codebase review, golden reference captures, and tools/golden/run-reference.sh for running the reference Ruby engine locally (clone is gitignored, GPL-3 dev-only). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
92 lines
16 KiB
Markdown
92 lines
16 KiB
Markdown
# ExploreDNS Codebase Review
|
||
|
||
## 1. What it is and how it's architected
|
||
|
||
ExploreDNS (module `gitea.hansenits.com.au/hits/ExploreDNS`) is a Go rewrite of the Ruby `dnstraverse` tool. Its stated purpose: walk the DNS delegation tree from the root servers down, exploring *every* resolution path a real iterative resolver might take, assigning each branch a probability, and reporting per-path answers/failures plus server software fingerprints. It ships two binaries.
|
||
|
||
**CLI path.** `cmd/exploredns/main.go` parses ~30 stdlib flags into `config.Config` (`internal/config/config.go:25-51`), validates ranges (config.go:112-140), then fans out into three engine configs — `dns.QueryConfig`, `dns.RootDiscoveryConfig`, `traverse.TraverserConfig` (main.go:150-171) — plus an `output.Config` (main.go:199-214). `output.RunTraversal` (internal/output/runner.go:12-45) attaches progress hooks, runs the traversal, fingerprints every encountered server IP via `version.bind` CHAOS TXT probes (internal/fingerprint/fingerprint.go), and renders either a streaming colored text tree (internal/output/text.go) or a buffered JSON document (internal/output/json.go).
|
||
|
||
**The engine.** `internal/traverse/traverser.go` runs a LIFO work-stack loop (Traverse, traverser.go:88-202): discover roots via an upstream recursive resolver (internal/dns/roots.go, hardcoded IANA hints as fallback), pop a referral, query its addresses (`processReferral`, traverser.go:221-274), classify the response (internal/dns/decode.go:86-120), and on a referral push one child branch per NS name with probability = parent/N (internal/traverse/response.go:143-185, 248-256). Glue-less NS names are resolved first via a hardcoded local resolver at 127.0.0.1:53, then via a secondary from-root traversal (`ResolveNS`, traverser.go:276-410). `internal/dns/query.go` does the wire work: UDP with exponential backoff, TCP fallback on truncation, EDNS0 at 2048 bytes.
|
||
|
||
**Web path.** `cmd/server` + `web/api` wrap the same engine as an async job service: POST `/api/traverse` spawns a goroutine and returns a job ID (handler.go:206-243), GET polls, GET `.../stream` delivers SSE progress, and an embedded single-file SPA (`web/api/static/index.html`, served via `go:embed`) is the UI. Jobs live in an in-memory map with a 1-hour TTL. Importantly, the web path builds its **own** hooks and serialization (handler.go:357-450) and bypasses `internal/output` and `internal/fingerprint` entirely — no dedup, no summary, no fingerprints in web results.
|
||
|
||
## 2. Current health
|
||
|
||
Green on the surface. `go build ./...`, `go vet ./...`, and the full test suite (`-race`, all seven packages) pass cleanly on go 1.26.4; `make build`/`build-server` produce working binaries and the repo stays clean. Live CLI runs work end-to-end (`./bin/exploredns example.com` produces a correct tree, fingerprints, and a 100%-answered summary in ~15-30s; NXDOMAIN and `--json` runs behave; exit 0). The web server serves the SPA, health endpoint, job creation, polling, and SSE correctly against real domains.
|
||
|
||
CI (`.gitea/workflows/ci.yml`) runs vet/test/build with a pinned toolchain and Docker builds of both images; Dockerfiles, module path, and port config are all consistent. Two soft spots: the "coverage check" step (ci.yml:29-32) echoes an average but enforces nothing — it can never fail; and several tests hit the real network or 127.0.0.1:53 with self-skip guards (internal/traverse/coverage_test.go:328, internal/dns/resolver_test.go:466-471), so offline pass/skip counts silently differ. `internal/config/config_test.go:291-310` fails gofmt, uncaught because `make lint` is only `go vet` (Makefile:25).
|
||
|
||
The critical caveat: the tests pass because **every traverse test injects a mock exchange**, which routes through a different code path (RD=0) than production traffic takes (RD=1) — see below. CI green does not mean the core behaviour is correct.
|
||
|
||
## 3. Where reality diverges from the README (ranked by user impact)
|
||
|
||
1. **"Full iterative traversal" is false — production queries are recursive.** `buildQuery` hardcodes `RecursionDesired=true` (internal/dns/query.go:221) and `dns.Query` never clears it. The RD=0 path (`IterativeQueryWithExchange`, query.go:146-155) is reached only when a test exchange is injected (traverser.go:416-419). `ensureRDFalse` (traverser.go:445-459) just flips the RD bit on the *response* — cosmetic — and its `t.exchange != nil` branch is unreachable. Any recursion-capable server on the path returns a final recursive answer, which `classify()` accepts as legitimate (decode.go:103-107), silently collapsing the traversal into a plain recursive lookup. This directly contradicts README:3-18 and the package doc (query.go:4-6), and is the single strongest candidate for "doesn't quite work the way I want."
|
||
|
||
2. **"Follows every referral exhaustively" — only per NS *name*, and fragile per IP.** `processReferral` iterates a referral's addresses in order and returns the **first non-SERVFAIL** response (traverser.go:263-268). Only SERVFAIL advances to the next IP; a timeout or network error on IP #1 kills the whole branch even if IP #2 works. There is no per-address branching at all.
|
||
|
||
3. **"Query all 13 root server sets in parallel" (README:19, 195-196) — neither all, nor parallel.** All root IPs are packed into ONE initial referral (traverser.go:96-97), so with `--all-root-servers` typically exactly one root is queried (first non-SERVFAIL wins). Nothing in the engine is concurrent — the mutexes at traverser.go:103-106 guard a single-threaded loop. Default runs use a single root chosen as `nsSet[0]` by the upstream resolver (roots.go:110-127), a different one each run.
|
||
|
||
4. **`--root-server <IP>` is broken end-to-end.** Config accepts only IP literals (config.go:149-158), but `discoverRootOverride` treats the value as a DNS *name* and looks up the IP-as-hostname (roots.go:100-107), always failing with "no addresses for root server 198.41.0.4" — verified live. Unlike the other paths, this one has no hints fallback (roots.go:54-56), so the run aborts.
|
||
|
||
5. **Fast mode "shares glue across branches" (README:26-27, 352-355) — a no-op beyond the root.** Non-root referrals get a throwaway `rootCache.Child()` (traverser.go:120-127) and `InfoCache` writes never propagate to the parent (cache.go:66-81), so glue learned in one branch is discarded, not shared. `--fast=true` vs `false` differ only marginally.
|
||
|
||
6. **"No-glue resolution" works but is contaminated and lossy.** It first asks a hardcoded recursive resolver at `127.0.0.1:53`, A-records only (resolveGlueViaSystem, traverser.go:461-501, literal at :484), ignoring `--dns-upstream` and `/etc/resolv.conf`. The fallback `ResolveNS` queries TypeA only (traverser.go:305 — IPv6-only nameservers unresolvable) and its visited-guard (traverser.go:290, 385-387) prunes *every* glue-less child in the sub-traversal, so any NS whose resolution path itself contains a no-glue delegation fails with "resolution exhausted without answer" (traverser.go:406-409).
|
||
|
||
7. **`--follow-aaaa` (README:110) is a dead flag.** Parsed and stored (main.go:23,74) but never read anywhere in the repo. Related: `discoverRoots` hardcodes IPv4-only (traverser.go:216), discarding AAAA roots even with `--root-aaaa`.
|
||
|
||
8. **"Configurable... timeouts" (README:25) — no timeout is configurable.** No `--timeout` flag exists; `QueryConfig.Timeout` is hardcoded to 5s at main.go:152 and is *dead* anyway — `realExchange` uses its own hardcoded 5s timeouts (query.go:53-74).
|
||
|
||
9. **JSON output (README:254-264) is an object, not the documented array, and duplicates every result.** With defaults, each terminal result is appended twice — once by `WriteResult` (json.go:90-96), again by `WriteSummary` with no dedup (json.go:99-103) — so answer probabilities in `results[]` sum to ~2.0 while `summary` sums to 1.0 in the same document (verified: each example.com answer appears 26×).
|
||
|
||
10. **Smaller doc breaks:** `--quiet` suppresses only the banner line, not "supplementary information" (README:127; `output.Config.Quiet` is never read — formatter.go:40); `-dd` is behaviourally identical to `-d` (Debug>0 is the only check anywhere: main.go:175,224, formatter.go:89); "Requires Go 1.21" (README:43) is wrong — go.mod pins 1.24.0; README omits `--dns-upstream` while built-in `--help` omits `--json`; `--retries` means total *attempts*, and `--retries 0` (allowed per README and validation, config.go:125) makes every query fail with the malformed error `failed after 0 retries: %!w(<nil>)` (query.go:97,133).
|
||
|
||
What *does* match the README: fingerprinting (CLI only), the web endpoints/SSE replay/done semantics, the 1-hour job TTL, and the CLI flag inventory itself — every documented flag parses.
|
||
|
||
## 4. Notable issues, half-finished pieces, surprises (ranked)
|
||
|
||
**Correctness — high impact:**
|
||
1. Resolve sub-traversal results pollute main output: `AttachHooks` fires `WriteResult` on every EventComplete with no `IsResolve` guard (internal/output/formatter.go:101), so NS hosts' own A records appear as answers in both text and JSON — verified live on hansenits.com.au, where answer probabilities summed to 2.29.
|
||
2. `Referral.Bailiwick` is overloaded to carry the NS **hostname**, not a zone (response.go:173-180; acknowledged comment at traverser.go:230-232), while the intended `NSName` field (referral.go:44) is never assigned yet is read by output (text.go:275, stats.go:182). This makes in-bailiwick filtering meaningless (response.go:154, masked by the accept-all fallback at :160-166), leaks "bailiwick":"m.gtld-servers.net" into the JSON/SSE public API, and lists root servers under their bare IPs.
|
||
3. `Traverser.visited` is never written and `Traverser.depth` never incremented (traverser.go:60-73, 223-228), so nested no-glue resolutions have no loop protection — circular NS dependencies recurse unboundedly, contained only by an accidental pruning bug and ctx timeouts. The seeding workaround (traverser.go:311-313) is a no-op because `StoreGlue` ignores empty slices (cache.go:67-69).
|
||
4. CNAME follows re-query the **same servers** instead of restarting from the root (response.go:200-204); out-of-zone targets hit the wrong authoritative servers. `--type CNAME` also misbehaves: a correct CNAME answer is classified as follow-me (response.go:129-141) and chased instead of reported. Loop detection checks only the final chain target (response.go:191).
|
||
5. `queryServer` swallows query errors entirely — a failed server yields `Response{Type: RespError}` with no message or server recorded (traverser.go:425-431); depth-limit rejections synthesize the same empty error with no hook events (traverser.go:157-167). Output can never say *why* a branch failed.
|
||
6. Root discovery silently degrades: any upstream failure falls back to hardcoded hints with the error discarded (roots.go:58-73); the discovery query path itself has no EDNS0, no retries, no TCP fallback (roots.go:204-224).
|
||
|
||
**Web stack:**
|
||
7. Frontend type `<select>` offers SRV and CAA (index.html:490-491) but `ParseQueryType` rejects both (config.go:53-77) → HTTP 400 in the UI. A likely "doesn't work" candidate.
|
||
8. SSE subscribe-then-snapshot race duplicates events (handler.go:303-314); slow clients silently *lose* events (32-slot drop-on-full, handler.go:93-96). Frontend `onerror` renders a running job as "Complete — no results" and re-enables the form (index.html:725-729, 790-806); Enter bypasses the disabled button and starts a second job (index.html:668).
|
||
9. Cancellation is half-built: `job.cancel` is plumbed (handler.go:228,236) but no endpoint calls it, shutdown doesn't cancel jobs, traversals have no timeout, never-finishing jobs leak forever (cleanup requires `DoneAt`, handler.go:150-157), and wildcard CORS with no auth or job cap lets any website spawn unbounded traversals (server.go:99).
|
||
10. The result "tree" indentation never renders — plain spaces collapse in HTML (index.html:829 vs :338); progress count is ~2× steps because start and complete both emit events.
|
||
11. The 127.0.0.1:53 hardcode hits the Docker web image hardest: alpine has no local resolver, so every glue lookup burns a timeout before falling back.
|
||
|
||
**Surprises and paper cuts:**
|
||
12. `--show-X=false` is silently ignored — the truthiness override at main.go:86-120 only honours `--no-show-X` — inconsistent with `--fast=false`, which README documents as the disable syntax.
|
||
13. Flags after the positional domain are silently dropped (stdlib flag behaviour): `exploredns example.com --type NS` queries type A with no warning (verified live).
|
||
14. `--help` prints its flag groups in random order (map iteration, config.go:218; observed 3 orderings in 4 runs).
|
||
15. ANSI color is gated solely on `NO_COLOR` with no TTY check (main.go:212, formatter.go:59) — escape codes end up in piped files. Server list sorts descending, almost certainly a `>` for `<` typo (text.go:89-91).
|
||
16. All exits are 0/1/2: NXDOMAIN, SERVFAIL, and success all exit 0 — useless for scripting.
|
||
17. Fingerprinting fires by default against every server encountered (runner.go:33-35) — chatty; and `--show-versions --no-show-servers` silently does nothing (runner.go:33).
|
||
18. Dead-code inventory suggesting abandoned plans: `dns.Resolver`/`BasicResolver`/`CachingResolver` (resolver.go, zero production callers), `Referral.Resolve` (~130-line duplicate of ResolveNS, referral.go:125-257), `ParseUDPSize`/`ParseMaxDepth`/`ParseRetries` (config.go:79-110), vestigial mutexes implying a shelved concurrency design, empty `dns.go`, unused `MinEDNS0UDPSize`.
|
||
|
||
## 5. External context
|
||
|
||
Commit history references a Jira project (HAN-*): HAN-410 (Go toolchain pin, PR #25), HAN-400 (host selection), HAN-388/389 (web SPA/API with SSE), HAN-384 (fingerprinting), HAN-383 (output formatting). PR #23 ("Fix NS resolution: NS name bug, glue bypass, FormatRecord duplicate header, result deduplication") shows the NS-name/Bailiwick area has already been patched once — the overload survives it. The repo is hosted on a private Gitea (gitea.hansenits.com.au/hits), with three stale `agent/go-expert-developer/*` remote branches; recent work has been housekeeping (module rename, CI pinning, untracking binaries) rather than engine correctness.
|
||
|
||
## 6. Clarifying questions for the owner
|
||
|
||
1. **Recursion:** When you run a traversal against a server that offers recursion, do you expect referrals or are you seeing final answers appear "too early"? Production queries currently go out with RD=1 (query.go:221; traverser.go:419) — should every traversal query be non-recursive like dnstraverse, making the fix "use IterativeQuery and delete ensureRDFalse"?
|
||
|
||
2. **Per-IP behaviour:** Should each nameserver *address* get its own branch with probability split across IPs (dnstraverse-style), or is first-responsive-IP acceptable? Today it's neither consistently — SERVFAIL falls through to the next IP but a timeout kills the branch (traverser.go:263-268) — and `--all-root-servers` effectively queries one root.
|
||
|
||
3. **The 127.0.0.1:53 hardcodes:** Are the local-resolver shortcuts in glue resolution (traverser.go:484) and root discovery fallback (roots.go:199) intentional speed hacks or placeholders? On machines/containers without localhost DNS they add 5s stalls per lookup, and they contaminate the "pure traversal" result — should they honour `--dns-upstream` / resolv.conf, or be removed?
|
||
|
||
4. **Fast mode:** What did you intend "shared glue cache" to mean? For sibling branches to actually reuse glue, writes must land in the shared root cache instead of throwaway `Child()` layers (traverser.go:120-127, cache.go:66-81) — is cross-branch sharing the goal, or per-branch isolation?
|
||
|
||
5. **CNAME follows:** When a target is out-of-zone (www.example.com → cdn.other.net), should the traversal restart from the root for the new name (dnstraverse behaviour) rather than re-asking the current servers (response.go:200-204)?
|
||
|
||
6. **Output shape:** In JSON, which is canonical — the streamed per-query results or the deduplicated terminal set (currently both are appended, doubling everything, json.go:90-103)? And should resolve sub-traversals be hidden, indented under their parent, or shown flat as now? Relatedly, should `--quiet` mean "results only"?
|
||
|
||
7. **Web scope:** Should the web API expose the CLI's knobs (max-depth, retries, all-roots variants, dns-upstream) and reuse the CLI's summary/dedup/fingerprint pipeline, or stay minimal? Concretely: the UI offers SRV/CAA that the backend rejects (index.html:490-491 vs config.go:53-77), and web results have no summary or fingerprints — which side is wrong?
|
||
|
||
8. **Scripting contract:** Do you need distinct exit codes (answer vs NXDOMAIN vs infrastructure failure)? Everything currently exits 0 on any completed traversal, which makes the CLI hard to use in automation.
|