Goal: make the operator-facing contract true. Four decided items — a real per-query deadline, the upstream editor milestone 8 promised, an opt-in trusted-proxy setting, and a field-level contract guard between the server and the frontend — plus BADVERS, the validator holes, and the doc claims the code contradicts. TECH_DEBT.md Theme 4. All four decisions are made; do not re-litigate them.

PROVISIONAL. Written before milestones 15-16 were built. m15 touches docs_drift_test.zig and cli.zig; m16 touches pool.zig's neighbors, the listeners and the settings handler. Re-verify line references before each session starts.

Rulings (binding)

1. `total_timeout_ms` becomes a real per-query deadline

Today the key feeds Pool.attempt_timeout (app.zig:283-288 → pool.zig:110) and bounds one attempt; with N upstreams down a query takes N × 5000 ms. The reference docs contradict each other (configuration.md says "per-query budget", cli.md says "per-attempt") and PLAN promises a total budget. Decision: the total budget becomes real.

model.Upstream gains attempt_timeout_ms: u32 = 2500; total_timeout_ms keeps its name and default (5000) and becomes the whole-exchange deadline. read_timeout_ms is untouched (it feeds the forward-zone client only, app.zig:412).
pool.zig: attempt keeps its race, now budgeted by the new key. The two-pass failover loop is extracted into a private exchangeLoopLen(self, io, query, response_buf) transport.ExchangeError!usize (the exchangeLen precedent, pool.zig:690-693: Io.concurrent stores the future's return value, so the raced loop returns a length and exchange rebuilds the slice). A new outer race wraps that helper with the total budget, using the select idiom already in the file. The outer cancel unwinds an in-flight attempt — the entry.busy lock is taken cancelably on purpose (:169-174) and released by defer. Outer expiry returns error.Timeout, unconditionally: the fast-failure paths (all upstreams disabled → ConnectFailed) finish long before the budget, so no attempted-marker is needed, and a test pins that an all-disabled pool still fails immediately rather than waiting out the deadline.
validate.zig: checkTimeout both keys; the cross-check becomes attempt_timeout_ms <= total_timeout_ms (the old total >= read check, which related knobs of different subsystems, is deleted).
Settings plumbing for the new key (the reflective walk carries it; the hand-written mirrors do not): expected_keys (model.zig:449, sorted), the round-trip test with a non-default value, the SettingsView upstream struct in web_integration_test.zig:528, types.ts Settings.upstream, openapi.yaml Settings + SettingsPatch, SettingsPage SECTIONS[0], the configuration.md row (drift-guarded), and the cli.md/configuration.md prose — both now say: total is the per-query budget, attempt bounds one upstream try.
Tests: a pool test with two stalling upstreams proves the exchange returns within the total budget, not 2 × attempt; the existing per-attempt tests keep passing against the new key.

2. The validator closes its two verified holes

Hostname tls:// upstreams. Endpoint.parse accepts any host text; the DoT client then refuses non-literals on every dial (resolveAddress, dot_client.zig:74-80), so a hostname tls:// config validates clean and fails at query time as a peer fault. Milestone 3 explicitly ordered this check carried into the validator and it was dropped. Correction to the audit: the function to mirror is not parseResolver itself (validate.zig:257 — that is the forward-zone resolver check); the upstream block in checkCollections (validate.zig:603-639) gains, for .dot endpoints, net.IpAddress.parse(endpoint.host, endpoint.port) catch → a new error.UpstreamHostNotIpLiteral diagnostic ("tls:// upstreams take an IP literal host"). faults.zig picks the variant up automatically. Fix the broken example at configuration.md:348 — the hostname NextDNS tls:// config can never complete an exchange; show an IP-literal tls:// example and move the hostname form to https://.
Boot-allocation bounds. query_log_buffer_max is floor-checked only (an Entry is ~420 bytes; maxInt(u32) asks for ~1.8 TB at boot), and cache.size is not validated at all (grep confirms). Both get the file's ceiling-only idiom: 1..1_000_000 with the "must be at most" message shape. No full memory-fit guarantee — out of reach, not needed.

Recorded (implementation): three consequences this ruling did not call out. (a) cache.size = 0 was a documented "disables caching" mode; the 1.. floor removes it — configuration.md now says there is no off value (use 1). (b) The configuration.md NextDNS-DoT redaction passage lost its premise (a hostname tls:// URL is now unconfigurable) and was rewritten. (c) Three config fixtures used tls://dot.example:853 and now use an IP literal with tls_name; the back-up-and-restore.md export transcript grew to head -10 with real re-captured output. Also: Pool.init takes a named pool.Timeouts { attempt, total } struct, not two positional durations — two same-typed adjacent parameters would be silently swappable.

3. The upstream editor is built

Milestone 8 ruling 9 requires upstream CRUD in the UI ("the Settings page must edit them"); milestone 9 dropped the obligation when the SPA was built. The entire data layer exists with zero importers (upstreamsQuery, three mutation factories at queries.ts:251-264, four api.ts wrappers). Decision: build it. Placement deviates from m8's literal text: a dedicated Upstreams page (nav entry), matching the house resource-page pattern — record the deviation beside m8 ruling 9.

New web/src/features/upstreams/UpstreamsPage.tsx + UpstreamForm.tsx, modeled on the blocklists pair (the closest analogue; copy its named idioms: editing-state-as-row, two mutation instances of one factory so row-toggle state never bleeds into the form, mutateAsync submit, full-row resend on toggle because PUT is a replace, key={editing?.id ?? "add"} remount, window.confirm delete, split error routing).
Fields: url, priority (number), enabled (toggle), tls_name. 400s carry the real validator's text (the handler validates via mutations.checkUpstream); surface the three 409 conflicts verbatim ("an upstream with that url already exists", "the last enabled upstream cannot be disabled/removed").
Every mutation response carries restart_required: true and today nothing raises the banner outside SettingsPage. Generalize the restart banner copy to "Changes saved. Restart nxdns to apply." and raise it on upstream create/update/delete success.
Wiring is three edits: createRoute in routes.tsx, the routeTree entry, the AppShell nav item. The health table stays on the Dashboard (health rows have no id — url is the only join key — and reflect the running pool, so a new upstream appears there only after restart; the page states this beside the restart banner).
Dead-layer cleanup in the same stroke (the same finding): delete the seven unused single-row get* wrappers (correction: seven, not five — getGroup and getUpstream are shadowed by longer names in grep), plus getMetrics, getOpenapiYaml, and ruleUpdateMutation. The editor reads rows from the list query per house idiom, so none gains a caller. Git history keeps them.
Page tests in the stubbed-fetch house style: list render, add, toggle resends the full row, delete confirms, a 409 renders inline, the banner raises.

4. Trusted proxies restore real client identity

Behind the documented same-box reverse proxy every request is loopback: localhost_exempt = true (default) then disables the API limiter — the only brake on argon2 brute force — and all remote users share one address's 3-stream SSE cap. No X-Forwarded-For handling exists anywhere (verified repo-wide). Decision: opt-in trusted-proxy setting.

New setting web.trusted_proxies: []const u8 = "" — a comma-separated list of IP literals in one string (the settings codec supports scalars only; a list type is a compile error at model.zig:358). Empty means off: behavior today.
Validation: each comma-separated element must parse as an IP literal → new error.BadTrustedProxy diagnostic in the web block of checkScalars.
Mechanics: when the socket peer is in the trusted set, the effective client address is the last valid IP literal in the request's X-Forwarded-For (the entry our proxy appended); a missing header falls back to the socket peer. Implementation follows the copy-before-dispatch constraint (http_util.zig:10-14): a new max_xff_len = 256 budget, a new Conn buffer, and a new Request field client_addr: address.NetAddress computed once in handleRequest. The copy keeps the tail, not the head: a new bounded suffix-copy helper beside copyHeader, because copyHeader returns empty on overflow (server.zig:608) and an empty result would fall back to the socket peer — loopback behind the intended same-box proxy, which api_localhost_exempt = true (model.zig:89, the default) then exempts. That is the exact bypass this ruling closes: a client ships an oversized XFF through the proxy and regains the exemption. The proxy appends , <client-ip> last, so the real entry always fits in the 256-byte tail. Fail closed on the residue: header present but no valid last IP literal in the tail means the trusted proxy violated its contract — respond 400, never fall back to the exemptible peer. Only a misconfigured proxy can trigger that arm; a spoofed prefix cannot, because the proxy's appended entry always terminates the chain. bucketLimit (server.zig:201-207) and the SSE acquire/release in live.zig:82-87 both read client_addr — the SSE pair must use the same key or releases leak. api_limiter.Config is untouched: WebState.web already carries every web field (app.zig:472), so the trust check reads state.web.
With trust configured, a proxied remote address is not loopback, so the exemption no longer swallows it — the limiter and the per-address SSE cap bind per real client. Requests arriving at the socket from non-trusted, non-loopback peers are unaffected.
Settings plumbing checklist (same list as ruling 1, plus the hand-written web mirrors the fact-finding flagged): expected_keys, round-trip test, WebView + view() in handlers/settings.zig:207-248, restart_required_keys balance test, SettingsView web struct in the integration test, types.ts Settings.web, openapi web schemas (including the required array), SettingsPage web section, the configuration.md row, the api.md rate-limiting section, and a warning in the proxy/authentication how-to: when proxying without trusted_proxies, set api_localhost_exempt = false.
Tests: unit tests on the effective-address derivation (trusted peer + XFF chain, untrusted peer + spoofed XFF ignored, trusted peer + no XFF, trusted peer + an XFF over 256 bytes whose valid last entry is still honored from the tail, trusted peer + a present-but-invalid chain → 400); an integration test proving a proxied remote address is rate-limited while the proxy itself stays exempt.

Recorded (implementation of ruling 4): Request.client_addr is std.Io.net.IpAddress, not address.NetAddress — http_util.zig is a module root for the fuzz target and cannot import nxdns modules; any future ruling that wants a domain type on Request hits this. clientAddr reads only the final chain entry, never scanning backwards: an earlier entry is client-controlled, so a fallback would hand the request an attacker-chosen identity. Known residual, out of this ruling's list: the login log line in handlers/auth.zig still prints the socket peer, which reads as loopback behind a trusted proxy — carried to milestone 19.

5. A field-level contract guard between server and frontend

The REST contract lives in three hand-synced copies. The Zig side is genuinely well guarded (56-entry contract table parsing live responses with .ignore_unknown_fields = false; route-level openapi guards — which live in web_integration_test.zig:1494-1590 and openapi.zig, not in docs_drift_test.zig as the audit said). TypeScript is guarded by nothing: every frontend test stubs fetch, and types.ts is strictly narrower than the wire in places (literal unions like Health.status: "ok" | "degraded"), so re-parsing into Zig structs can never catch an out-of-union string. Decision: one integration layer asserting the frontend's consumed shapes against real responses.

Mechanism — captured samples validated by tsc, no new dependencies:

A new -Dintegration test beside the contract walk drives the real Env server through every route whose contract-table kind is .json and which the frontend consumes through an api.ts wrapper — the GETs and the JSON-returning POST/PUT wrappers (login, the group/client/rule/upstream/blocklist writes, refresh, pause, the settings PUT), driven with deterministic seed payloads so the mutating responses are stable too (plus one sample per shared error response class). GET-only would leave every write-path response contract unguarded. Explicitly excluded: the .raw routes (/metrics, /api/openapi.yaml — not JSON, and ruling 3 deletes their unused wrappers), the .sse route, and the .none deletes. The endpoint-to-type mapping is derived from api.ts, not invented. Captured bodies are canonicalized: keys sorted, every number replaced with 0 (type-preserving; strings and booleans kept — they come from the deterministic seed). Canonicalization is what makes the output byte-stable across runs.
The canonical samples render into a committed file web/src/lib/contractSamples.gen.ts: for each endpoint, export const sample_<name>: <TsType> = <json>; with the matching import from types.ts. TypeScript object literals get excess-property checking, so a server field missing from types.ts, a types.ts field missing from the wire, and an out-of-union literal all fail npm run typecheck — which already runs in CI's frontend job. Recorded (implementation): canonicalization gained two rules beyond "keys sorted, numbers→0": repeated array elements dedupe by canonical text (60 identical zeroed timeseries buckets collapse to one; differing rows all stay), and two build-identity strings (Version.git_commit, Version.zig_version) redact to "<build>" so the golden is not pinned to one machine — the list is the named constant volatile_string_keys. The renderer emits prettier's own fixpoint output, because CI runs format:check on the tree. 43 samples: 38 endpoint + 5 error classes. The regen command is documented in AGENTS.md.
Embedding: an anonymous-import root must be Zig, not TypeScript, so a wrapper web/src/lib/contract_samples.zig exports pub const bytes = @embedFile("contractSamples.gen.ts"); and build.zig registers it — the exact docs/docs.zig pattern. The Zig test byte-compares the embedded bytes against the freshly rendered content and fails on mismatch.
Regeneration is explicit, not write-beside-the-embed (embedded bytes carry no source-tree path): a build option -Dcontract-samples-out (absolute path, wired through build_options) makes the test write the fresh content there instead of comparing. The pinned command, stated in the failure message and the docs: zig build test -Dintegration -Dcontract-samples-out="$PWD/web/src/lib/contractSamples.gen.ts". Golden-file discipline: the server side keeps the samples current; tsc keeps types.ts honest against them.
types.ts gains ErrorEnvelope matching the shared error responses (verify the exact envelope shape against the handlers before writing it) so the error samples are typed too.
Recorded residual risk: the server↔openapi seam stays route-level only. The yaml is served verbatim and never parsed; field-level yaml drift remains possible and is accepted — revisit only if it bites.

6. BADVERS becomes expressible

RFC 6891 §6.1.3: a version-1 EDNS query must get RCODE 16 (BADVERS) with version 0 in the reply OPT. Nothing checks opt.version anywhere (verified), and the write path cannot express it: addOptEcho hardcodes extended_rcode = 0 (packet.zig:354) and synthesize takes a 4-bit types.Rcode. Negligible operational impact; the mechanism is the finding.

edns.zig gains the write-side splitter beside the read-side extendedRcode (:272): a 12-bit rcode splits into the header's four bits and the OPT's upper eight. extendedRcode stays — it is the read-side pair (milestone 19 assumes it survives).
packet.zig gains addOptWithRcode(request_opt, do_bit, extended_rcode: u8); addOptEcho becomes a call with 0.
handler.zig: the version check sits directly after the successful parseOpt (:227-233), before the opcode check — if (o.version != 0) → a BADVERS synthesis (header .no_error + OPT extended_rcode = 1, version = 0), counted in a new Handler.Stats.badvers (the metrics reflection exports it; extend the name test).
Tests: a version-1 query yields composite rcode 16 (assert via edns.extendedRcode) and version 0 in the reply OPT; a version-0 query is unaffected.

Recorded (implementation): the write-side splitter is edns.splitRcode with pub const badvers: u12 = 16; the BADVERS reply echoes the question when qdcount == 1, following the neighboring FORMERR sites, so a client can bind the reply to its query; a malformed OPT whose version byte is 1 answers FORMERR, because parseOpt fails before the version check can read the byte.

7. The stale phase comments state the as-built contract

rate_limiter.zig:5-6 ("Phase 7 decides the locking"): rewrite to the as-built truth — the handler serializes with Handler.limiter_mutex (handler.zig:124), uncancelable on the query path, cancelable in the runMaintenance sweep (app.zig:702-731 already states the rationale to adopt).
shutdown.zig trigger doc ("what a Phase 8 restart endpoint would call"): no restart endpoint exists or is planned — m8 shipped restart_required echoes instead. Rewrite to "what a test uses".
pool.zig:83-84: tense only — the health endpoint exists; "Feeds GET /api/upstream/health."

8. Doc transcripts stop hardcoding the version

Four sites print 0.1.0-dev (tutorial/first-run.md:108, install-with-docker.md:126, install-with-systemd.md:206, upgrade.md:142 — three how-to plus one tutorial, two different transcript shapes). Correct today; silently wrong at the first tag. Milestone 14 mandates a placeholder and a guard but names neither; the milestone-13 executed-transcript rule is in tension.

Ruling on the tension, explicit: milestone-13 ruling 3 constrains command lines; pasted output lines may carry placeholders. The orchestrator adds one clarifying sentence to milestone-13.md beside ruling 3.
The four output lines replace the version with <version> (e.g. nxdns <version> serving on ...).
The guard: docs/docs.zig additionally embeds the tutorial and how-to pages; a new test in docs_drift_test.zig asserts no embedded doc page contains the string 0.1.0-dev and that the four transcript pages contain nxdns <version>. (Note m15 rewrote this file's CLI test — rebase, don't collide.)

9. The log docs stop claiming append mode

files-and-directories.md:143 claims the log file is "opened for append". It is not: .mode = .write_only plus seekTo(state.file_pos) per line, offset seeded once at open — the source comment at logging.zig:502 says so outright ("0.16.0 has no append mode"). Under external logrotate the divergence is destructive (copytruncate → sparse NUL-prefixed file; rename+create → unbounded writes to the renamed inode, max_size_mb unenforceable).

Docs only: correct the row (positional writes, offset tracked in-process), and add an operator warning — nxdns owns rotation for this file; do not point external logrotate at it. A per-line stat re-check was considered and rejected: a syscall per log line on the hot path to defend against a misconfiguration the docs now forbid. Record that decision here.

Sessions

Dependency graph: S1 → S2 → S5 (they share model.zig, validate.zig, types.ts, openapi.yaml, SettingsPage.tsx, web_integration_test.zig — the later session rebases on the earlier). S3, S4, S6 run in parallel with the chain and each other.

Session S1: deadline and validator

Owns src/upstream/pool.zig, src/config/model.zig, src/config/validate.zig, src/app.zig, plus the ruling-1 plumbing trail (types.ts, openapi.yaml, SettingsPage SECTIONS[0], the integration-test SettingsView, configuration.md, cli.md). Rulings 1, 2, and the pool comment from ruling 7.

Session S2: trusted proxy (after S1)

Owns src/web/server.zig, src/web/http_util.zig, src/web/handlers/live.zig, src/web/handlers/settings.zig, and its plumbing trail through the same shared files S1 touched. Ruling 4.

Session S3: upstream editor

Owns web/src/features/upstreams/* (new), web/src/routes.tsx, web/src/shell/AppShell.tsx, web/src/lib/api.ts, web/src/lib/queries.ts, web/src/features/settings/RestartBanner.tsx and restartBanner.ts, specs/milestone-8.md (the deviation note). Ruling 3.

Session S4: BADVERS

Owns src/dns/packet.zig, src/dns/edns.zig, src/server/handler.zig, the metrics name test in src/web/metrics.zig. Ruling 6.

Session S5: contract samples (after S1 and S2)

Owns the new generator test in src/web/web_integration_test.zig, build.zig (the anonymous import), src/tests.zig (if a new file needs an entry), web/src/lib/contractSamples.gen.ts (committed golden), web/src/lib/types.ts (ErrorEnvelope only). Ruling 5.

Session S6: docs and comments

Owns src/server/rate_limiter.zig and src/server/shutdown.zig (headers only), docs/docs.zig, src/docs_drift_test.zig, the four transcript pages, docs/reference/files-and-directories.md, specs/milestone-13.md (the clarifying sentence). Rulings 7 (except the pool comment), 8, 9.

Orchestrator

Strikes the closed findings in TECH_DEBT.md; confirms PLAN's total-budget promise now matches the code; records the m8 placement deviation.

Module layout

New files: web/src/features/upstreams/UpstreamsPage.tsx, web/src/features/upstreams/UpstreamForm.tsx, web/src/lib/contractSamples.gen.ts (generated, committed).

Deleted surface: the seven unused get* wrappers, getMetrics, getOpenapiYaml, ruleUpdateMutation.

Acceptance (milestone complete)

Two stalled upstreams: the exchange returns within total_timeout_ms (test). attempt_timeout_ms exists end to end: settings key, validator range + cross-check, docs row, SettingsPage field, SettingsView, types.ts, openapi.
A hostname tls:// upstream fails nxdns check with the new diagnostic; the configuration.md example connects.
query_log_buffer_max = 4000000000 and cache.size = 0 both fail validation with ceiling/floor messages.
The Upstreams page lists, adds, edits, toggles (full-row resend), deletes; the three 409 texts render; the restart banner raises with the generalized copy; nav and route exist; page tests pass.
The ten dead wrappers/factories are gone; npm run typecheck and lint pass.
With trusted_proxies set, a proxied remote address is rate-limited and holds its own SSE budget while the proxy stays exempt; a spoofed XFF from an untrusted peer is ignored; an oversized XFF through the trusted proxy does not regain the loopback exemption (tests). The how-to carries the no-trust warning.
contractSamples.gen.ts is committed and contains samples for the JSON-returning POST/PUT wrappers, not GETs only; the Zig generator test passes against it; deliberately removing a field from a types.ts type fails npm run typecheck (proven, then reverted).
A version-1 EDNS query answers composite RCODE 16 with version 0; nxdns_dns_badvers_total appears in /metrics.
"Phase 7"/"Phase 8" no longer appear in rate_limiter.zig, shutdown.zig, or pool.zig; the rewritten headers state the mutex and the maintenance task. (Narrowed during implementation: the original repo-wide grep exceeded ruling 7's three files — 24 stale phase references remain in files this milestone's sessions do not own; the sweep moved to milestone 19.)
Grep: 0.1.0-dev appears in no docs/ page; the new drift test passes and was proven able to fail (reinsert the literal, watch it fail, revert).
files-and-directories.md describes positional writes and warns off external logrotate.
Full suite green: zig build test -Dintegration, npm run test, npm run typecheck.

Anti-requirements

No restart endpoint — the restart_required echo model stands.
No hostname resolution for tls:// upstreams — the validator rejects; the client's own check stays as defense in depth.
No openapi codegen and no runtime schema validation library — the contract guard is captured samples + tsc, nothing more.
No proxy protocol (PROXY/haproxy) support; X-Forwarded-For only.
No per-line stat re-check in the logger (ruling 9 records why).
No upstream health join in the editor beyond the stated restart note.
No Forwarded (RFC 7239) header parsing — XFF only, recorded.

25 KiB Raw Permalink Blame History Unescape Escape

Milestone 17: contract repairs