fancy-dns/AGENTS.md

# AGENTS.md — working on fancy-dns

Project-specific knowledge for anyone (human or agent) changing this repo.
General state and the continuation plan live in `STATUS.md`; read that first for
what is done and what is next. This file is the part that should *not* change
every week: the decisions, the invariants, and the rules that have been learned
the hard way.

## What this is

A recursive DNS server in Go with an embedded React console. Config is YAML plus
RFC 1035 master files, hot-reloaded over SIGHUP. Blocklists in four formats, a
sharded LRU cache with a bbolt snapshot for warm restart, per-client ACLs,
query logging, Prometheus metrics, and optional Valkey/Redis for cross-instance
sync.

## Layout

```
cmd/fancy-dns/main.go          entrypoint: flags, signals, SIGHUP reload
internal/
  config/       Config struct, Duration, Size, Validate, atomic Store + hooks,
                ${VAR} environment expansion in Load
  zonefile/     RFC 1035 master-file parser ($TTL/$ORIGIN, parens, serial math)
  blocklist/    Set (exact/sub/root maps), 4 parsers, Manager (fetch+compile+swap)
  cache/        Key, Entry, sharded LRU, TTL policy, serve-stale, bbolt Store
  resolver/     Upstream pool: health, circuit breakers, parallel/failover/random
  acl/          compiled groups, CIDR/PTR/tag matching, per-group domain policy
  localzone/    authoritative data from master files, wildcards, delegations
  engine/       THE QUERY PIPELINE, reverse-PTR cache
  server/       UDP, TCP, DoT, DoH listeners; all funnel into one handler
  qlog/         bbolt query-log ring + live in-memory ring + SSE fanout
  metrics/      Prometheus collectors + rolling time-series rings
  cluster/      Valkey pub/sub, leader lease, divergence detection
  api/          admin REST + SSE, auth, CORS, rate limiting
  app/          wiring, lifecycle, API hook implementations, memory envelope
  web/          go:embed of the built console
web/            Vite + React + TS + Tailwind console
deploy/         example config, example zones, k8s manifests
```

### Query pipeline order (engine.Handle)

```
validate → ACL deny → per-client concurrency → global concurrency → rate limit
  → group allow/deny domains → local zone → blocklist → cache → upstream
  → cache put → finalize (EDNS0, UDP truncation) → metrics + qlog
```

Cache is a single shard-local lookup; only a miss touches the network.

## Decisions already made — do not relitigate

| Decision | Choice | Why |
|---|---|---|
| Scope | Full production | clustering, metrics, ACLs, query logs, hot reload, Docker, k8s, CI |
| Config store | YAML + DNS master files | reviewable/diffable in git |
| Cluster sync | Valkey (Redis protocol) | simple, mature Go client, easy local dev |
| Secure DNS | DoT + DoH, **no DNSSEC validation** | deliberately out of scope; AD/CD/extended-RCODE are passed through only |
| Hot cache | Sharded LRU in memory, bbolt for snapshots | no I/O on the hot path; warm restart |
| k8s networking | **No hostNetwork**, LoadBalancer Service | the server never sets SO_REUSEPORT, so two host-network pods cannot share :53 |
| Memory | Soft GOMEMLIMIT + cache byte budget | a cgroup kill is an outage; a cache miss is not |
| Scale-out | StatefulSet, **no HPA** | an HPA cannot drive a StatefulSet; scale by adding replicas |
| Ingress | A separate ClusterIP Service | a headless Service has no virtual IP for an Ingress to route to |
| Profiling | pprof refused unless admin is loopback | the handlers sit outside the API's auth middleware |

## Invariants

Break these and you break the design, not just the test.

- **Everything the query path reads is behind an `atomic.Pointer` holder** — ACL,
  blocklist, localzone, upstream pool. Readers load the pointer once and work
  against a value that cannot change under them. **A lock on the hot path is a
  regression.** Prefer building a new immutable snapshot and swapping the
  pointer.
- **Cache entries are packed wire bytes, not `*dns.Msg`.** A hit costs one
  `Unpack` and yields a private copy, so serving never mutates shared state and
  TTL ageing is free. Same trade dnsmasq makes.
- **The cache is bounded by bytes as well as by entry count.** `cache.size` alone
  bounds nothing: one cached ANY or TXT answer can be tens of kilobytes.
- **Serve-stale** answers are served with TTL 1. An expired upstream answer is
  never cached. A truncated answer is never cached — it would be served
  truncated forever.
- **The UDP read deadline is reset every iteration.** A deadline set once expires
  for good and turns the loop into a spin that never reads another packet.
- **Only the Valkey lease holder fetches blocklists**, so N instances do not hit
  a mirror N times. Followers serve their compiled copy.
- **Blocklist changes flush the cache** rather than guessing which subtrees are
  affected (`invalidateBlocklistCache()` in `internal/app/hooks.go`).
- **API keys are compared by SHA-256 hash, constant-time**, and
  `X-Forwarded-For` is honoured *only* from `trusted_proxies`. Honouring it from
  anyone lets a client spoof its identity past the ACL and the rate limiter.
- **The API response field names are shared with the frontend.** If you rename
  one, grep `web/src` too.
- **`PatternKind.Pattern` is the display form; `WireName` is the wildcard form.**
  The internal `*.` / `**.` prefixes must not leak into the UI, but the API needs
  them to re-add a rule unchanged.
- **The Go heap does not account for goroutine stacks, runtime metadata, or the
  bbolt mmap.** `memory.reserve` exists to cover them; the cache snapshot and
  query log are mmapped and live outside the heap entirely.
- **Unknown config keys are silently ignored** (a known open decision —
  `yaml` `KnownFields(true)` would reject them). If a config key seems to have no
  effect, check the spelling against the struct tags before anything else.

## Working rules

**Never leave the tree red.** You have a hard budget and no way to detect you are
close to it. Whatever is on disk at the end of any round must compile and pass.
Get a few tests running early and keep them running. If you are running long,
stop adding tests and make the existing ones pass — a smaller green suite is worth
far more than a larger red one.

**Scope your edits.** Check `git status` before you start; other work may be in
flight. Stage only the packages you touched. `.codesearch/` is tracked but
gitignored — never stage it. If you edit a package someone else is also editing,
say so in your report rather than silently overwriting.

**Verify before you claim.** `make check-go` is the fast loop (gofmt, vet,
compile, ~1s). `make check` adds the full test suite. Push only a green tree.

**Commit and push when the task says so.** Read `git log --oneline -5` and
`git show --stat HEAD` first and match the style: an imperative subject line, then
one paragraph per bug covering root cause and fix. A commit that says *what* was
changed without saying *why* is not finished work.

**When a test fails, decide which of three things is wrong before editing
anything:** the test, the implementation, or the specification. All three happen
here. Say which one you concluded and why. Do not relax an assertion to make a
suite pass — if an existing assertion pins buggy behaviour, change it and say so
explicitly in the commit message, because that decision belongs in the history.

**Report honestly.** If you did not finish, say what is unfinished and what state
the tree is in. A partial result that is accurately described is usable; a
partial result described as done is not.

## Test-hygiene traps

- `dns.NewRR` needs a TTL column: `dns.NewRR("host.example.com")` does not parse.
  A failure there is a bad fixture, not a product bug.
- A test that closes a stop channel *after* `wg.Wait()` deadlocks the whole
  package and looks like "the suite hangs". Close first, then wait.
- Assert on invariants, not on what a concurrent goroutine happened to observe.
  "A reader must have seen address X" is a flake; "every list a reader sees is one
  of the published ones" is a real property and cannot flake.
- Spin up `dns.Server` fakes on `127.0.0.1:0`: `net.Listen`/`net.ListenPacket` to
  find the port, `ActivateAndServe` in a goroutine, stop in `t.Cleanup`.
- `internal/app/integration_test.go` already builds a fully wired app. Reuse its
  harness rather than rebuilding one.
- `tsc -b` replays **cached diagnostics** from `node_modules/.tmp/*.tsbuildinfo`.
  After a large frontend edit: `rm node_modules/.tmp/*.tsbuildinfo`.
- `vite build` empties `outDir` and **deletes `internal/web/dist/.gitkeep`**.
  Restore it or the directory disappears from git.
- The Go binary embeds `internal/web/dist`, so `npm ci && npm run build` must run
  before a fresh clone will build a binary with the console in it.
- `make build` depends on `web` and shells out to npm. `make go-build` skips the
  console and is what you want in an inner loop.

## Security-sensitive areas

These have been wrong before. Changes here need a test that fails without the fix.

- **Auth** (`internal/api/auth.go`): API key comparison must stay constant-time;
  the key is hashed before comparison so the plaintext is not in a long-lived
  string.
- **Client identity**: `X-Forwarded-For` is only trusted from `trusted_proxies`,
  and `httpRemoteAddr` (used by the DoH handler) deliberately does **not**
  consult it at all. Behind an Ingress every DoH client therefore lands in the
  proxy's ACL group — fail-closed if that group is restrictive, fail-open if it
  is permissive. Pin this behaviour with a test before changing it.
- **Config expansion**: `${VAR}` is expanded in `Load` and an unset variable
  with no default is fatal, so a missing Secret is a crash loop rather than an
  unauthenticated console. The expansion works inside quoted scalars — that is
  the form operators write — and skips YAML comments. Do not "simplify" it into a
  plain `os.ExpandEnv`; that silently breaks the API key path.
- **pprof** is loopback-only by construction, so no config flag can turn it into
  a network-reachable unauthenticated endpoint. Keep it that way.
- **Amplification**: the DoH/UDP handlers cap the client's advertised EDNS0
  buffer. Do not remove the cap.