diff --git a/specs/milestone-20.md b/specs/milestone-20.md new file mode 100644 index 0000000..9278c98 --- /dev/null +++ b/specs/milestone-20.md @@ -0,0 +1,894 @@ +# Milestone 20: declarative configuration for IaC + +Goal: a config file an operator can keep in git and deploy with Ansible, where +the file is the sole declarative source of truth, converged at every boot — +without re-downloading every blocklist on every boot, and without the UI +silently diverging from the file. Two authority modes, selected by the +presence of one flag: bare `run` serves the DB; `run --config=` makes +the file authority. + +Design finalized 2026-08-09 after three adversarial rounds (red team ops 1-16 +and debt 1-14; Codex cross-validation F1-F12; a Codex round on the premise +revision). All findings are folded in below or declined with written reasons; +all file:line anchors were re-verified at the pre-implementation HEAD +(7039a9f). A premise revision replaced the earlier `--config-source` mode +enum with presence-of-`--config` and deleted the persisted authority marker; +rulings 1, 6 and 8 record the reasons — do not reintroduce either. + +## Implementation contract (read first) + +- Read `AGENTS.md`, then this spec whole, before session work starts. +- Session order: R1 → R2 sequential; R3 and R4 parallel to both (Sessions, + below). File ownership is write-exclusivity; the interfaces between + sessions (`reconcile.Summary`, `WebState.authority` + `reconciled_at`, + `RouteInfo.policy`) are fixed in this spec and are not renegotiable + mid-build. +- Gates: `zig build test` and `zig build test -Dintegration` with 0 failed, + plus the drift-guard regenerations the Tests section names. Skip counts are + reported with their reasons (plain-suite skips are integration-gated; the 4 + integration skips are the live-network TLS tests excluded by milestone-1 + design). One `-Dlive` run covers the real download path (Tests). +- No `std.log.err` in new code. No secrets in logs — url redaction goes + through `src/safe_url.zig` as everywhere else. +- Every fix or behaviour claim lands with a test the author watched fail + against the reverted implementation (milestone-13 ruling F-f applies). +- Doc pages touched by R4 follow milestone-13 ruling 3: every command block + in `tutorial/` and `how-to/` is executed on this host by the session that + writes it, or marked in-page as unverified with the reason. +- The final commit is GPG-signed by the user (`git commit -S`, lowercase, + single line); stage the work and hand the command over. + +## Rulings (binding) + +### 1. Authority is the invocation: `--config` present means the file governs + +`nxdns run` — the DB is authority, today's appliance behaviour. +`nxdns run --config=/etc/nxdns/config.zon` — the file is authority. The +flag's presence selects the mode; there is no mode enum and no default path. +`check` keeps its existing surface under the same rule: bare `check` grades +the DB, `check --config ` grades the file. The DB-exists-wins heuristic +in `checkImpl` (cli.zig:559-588) and `CheckArgs.config_explicit` are deleted. + +Two principles, separated deliberately, because conflating them is what +produced the rejected `--config-source` enum: + +- **Authority must be explicit in the invocation.** An operator reads + `ExecStart` and knows which authority is live. Probing `/etc/nxdns` for a + file and switching behaviour on its existence is ambient magic — the same + class of heuristic that made seed-once bootstrap a source of doc lies (the + compose comment, the first-run tutorial) — and stays banned: a file on disk + that no flag names changes nothing. +- **A mode enum is the wrong shape for a two-state choice a path already + expresses.** `--config-source=db` names an implementation, not an operator + intent, and a mode flag beside a path flag manufactures invalid + combinations (path without mode, mode without path) that then need pairing + rules and usage errors to defend. Presence-of-path has no invalid + combinations and nothing to defend. + +Breaking change, named: `run --config ` today means seed-once; the same +syntax now means file authority — reconcile on every boot, UI config writes +rejected. For an operator who seeded once and then configured through the UI, +the first post-upgrade restart converges the DB to that old seed file, +deleting the UI edits. The upgrade doc's breaking-changes section leads with +this and gives the two exits (ruling 9): drop the flag, or re-export to the +file path first. `check --config` keeps its meaning exactly. Greenfield rules +apply — the seed-once interface does not survive for compatibility — but the +break is loud in the docs, never silent-by-omission. + +A rename (`--managed-config`) that would make the old invocation fail loudly +was considered twice and declined: it trades a worse name and a permanent +asymmetry with `check --config` against a one-time hazard whose exposed +population is the pre-change install base of a project whose first release is +days old. The hazard is real and the docs lead with it; the interface does +not carry the scar. This is a judgment, recorded so it is revisited only with +new facts (a real install base would be one). + +### 2. File mode fails closed, and *declarative* failure is exit 2 + +File mode contract: the file is the sole declarative source; the DB stays the +runtime substrate and the effective-config read path. Startup sequence, +replacing the `seedFromFile` call at app.zig:196: + +1. `DataDir.open` → open config DB → `migrate` (unchanged). +2. Read the file (existing 4 MiB cap), ZON parse (arena, never freed — keep + the import.zig discipline), `validate.validate`. +3. Reconcile into the DB in one `BEGIN IMMEDIATE` transaction, `errdefer` + rollback (ruling 3). +4. `config_export.readConfig` (app.zig:206) → serve, unchanged from there on. + +A missing, unreadable, or invalid file fails startup. Never fall back to the +DB: a fallback turns a deploy typo into a silently stale config. + +Exit codes keep the 2am contract — exit 2 means "your config is wrong, run +`nxdns check`"; exit 1 means "the box is wrong". `faults.isConfigFault` +deliberately excludes `FileNotFound`/`AccessDenied` (faults.zig:107-108), and +that stays true in general. The file-mode loader is the seam, and it maps +**path-class open failures only**: `FileNotFound`, `AccessDenied`, +`PermissionDenied`, `NotDir`, `IsDir`, `SymLinkLoop`, `NameTooLong`, +`BadPathName` become `error.ManagedConfigUnreadable` (message includes the +path), which joins the `ConfigFault` set beside `ParseZon`/`ConfigTooLarge`. +Every other member of `ReadFileAllocError` — `SystemResources`, +`ProcessFdQuotaExceeded`, `SystemFdQuotaExceeded`, I/O errors, `OutOfMemory` — +propagates unmapped, exit 1: those are box faults a retry can clear, and once +ruling 9 adds `RestartPreventExitStatus=2 64`, mapping them to exit 2 would +stop the unit permanently on a transient fault. The mapping is an explicit +named error set in the loader, switched exhaustively with +`else => |other| return other`, so a std error added in a Zig bump defaults to +exit 1 rather than silently to exit 2 — the same closed-set discipline +faults.zig already documents. The mapping lives in **one shared helper** used +by both `run` and `check`; two copies would let the two grade the same +unreadable file differently, exactly the divergence faults.zig:1-8 exists to +abolish. + +**Scope of the check/run agreement claim**: `check --config=` +grades exactly the declarative faults `run` would hit — read (path-class), +parse, size, validate — through the same shared loader helper. Reconcile-time +faults (FK violations, `SQLITE_BUSY` from a restart race, disk full) are +runtime faults, exit 1, and structurally invisible to `check`, which needs no +DB. The acceptance encodes the scoped claim, not "check passing guarantees run +converges". The one *systematic* gap the red team found — a group removal +tripping the un-cascaded `clients.group_id` FK against observed rows `check` +cannot see — is eliminated in the engine itself (ruling 3's reassign rule), +not papered over in `check`. + +Reconcile-time diagnostics include the SQLite error (`SQLITE_FULL` by name +when that is the cause) so a full SD card reads as "disk", not as a bare +exit 1. Diagnostics render to `r.err` and flush immediately, keeping the +current `seedFromFile` discipline (app.zig:137-178) — `serve` never returns, +so a buffered error line is a lost error line. + +Because every boot revalidates the file, a latent file error is no longer a +first-boot-only hazard: a bad push that skips its restart handler detonates at +the next power blip. Two mitigations, both in ruling 9: the shipped unit gains +`RestartPreventExitStatus=2 64` (retrying a config fault every 2 s is pure +loop; the journal holds the diagnostics), and the deployment docs mandate +`nxdns check --config=` as the pre-restart gate in any Ansible +handler. + +Db mode: steps 2-3 are skipped entirely; the DB is truth exactly as today. +Bare `check` on a box with no `config.db` exits 2: `no config database at ` +plus the ruling-6 hint line — the deleted heuristic's "nothing to check" +branch (cli.zig:582-587) is replaced, not dropped. + +### 3. Reconcile engine: replace declarative state, preserve runtime state by stable identity + +New module `src/config/reconcile.zig`, replacing `applyToDb`'s wipe+reinsert. +The defect it exists to fix: today's import deletes and reinserts +`blocklist_sources` including runtime columns (checksum, `last_updated`, +counters — config_schema.zig:46-56), and compiled blocklists are keyed by +source row id (`.list`/`.wild`). A naive re-import every boot would +force a full re-download and recompile of every blocklist on every restart. + +Contract, per table: match rows by identity key ⇒ UPDATE declarative columns +in place (row id survives); present in DB but absent from the file ⇒ DELETE; +present in file but not DB ⇒ INSERT. Never wipe. One transaction. The +`clients` table refines the match rule with promotion (below): an observed row +whose IP the file declares is matched and updated, never deleted. + +**Identity matching is on canonical forms.** Import already canonicalizes +client IPs and prefixes before insert (import.zig:226-237; `FD00:0:0:0:0:0:0:1` +stores as `fd00::1`) and the schema documents the columns as canonical text. +The engine canonicalizes file values *before* matching; matching the raw file +string against the canonical column would churn ids under an unchanged +non-canonical file, violating ruling 5 in a way an exported-config test +(export emits canonical forms) cannot catch. The idempotence test includes a +non-canonical file. + +**Writes only on difference.** A matched row whose declarative columns already +equal the file's values gets no UPDATE. This is stronger than byte-stability: +reconciling an unchanged file performs **zero writes**, so a no-op boot needs +no WAL headroom and a querylog-full SD card cannot brick a file-mode restart +that db mode would survive. Testable: the second reconcile returns an all-zero +summary and `sqlite3_total_changes` does not move. + +| Table | Identity key | Declarative columns | Runtime-owned (preserved on match) | +|---|---|---|---| +| `blocklist_sources` | `url` | `name`, `enabled`, `is_suggested` | **`id`**, `checksum`, `last_updated`, `domain_count`, `wildcard_count`, `skipped_regex_count` | +| `groups` | `name` (`default` pinned to id 1, existing assert kept) | `safe_search` | `id` | +| `clients` | canonical `ip` | `name`, `group_id`, `hand_edited=1` | `first_seen`, `last_seen`; observed rows (`hand_edited=0`) are kept wholesale; an observed row whose IP is now declared in the file is **promoted in place** (UPDATE `name`, `group_id`, `hand_edited` 0→1 — `first_seen`/`last_seen` untouched, row id survives, counts as `updated`); observed rows whose group is deleted are **reassigned to the default group** (below) | +| `rules` | the full tuple `(group_id, pattern, kind, action)` — no natural key exists; the identity is a **multiset**, exact duplicate tuples pair off by count | (the tuple) | `id`, `created_at` (unmatched inserts stamp `now`) | +| `upstreams` / `client_prefixes` / `forward_zones` / `local_records` | `url` / canonical `prefix` / `zone` / `(name, rtype, value)` | everything else | `id` — preserved as a consequence of the idempotence invariant (ruling 5), **not** for FK stability: nothing references these ids by FK and id-based REST mutations are rejected in file mode | +| `group_sources` | `(group_id, source_id)`, resolved via the name/url maps | whole row | — | +| `settings` | key | value via `settings_repo.putSetting` upsert, on difference only | delete keys not produced by `toSettings`, **except** `web.password_hash`, which the reconciler owns directly (ruling 4) | + +**Pass order (binding).** `delete_order` alone under-specifies the engine: +inserts need parents before children, and a group DELETE cascades through +`rules`, `client_prefixes`, `group_sources` (config_schema.zig:35, :60, :67), +which could silently undo child-table work and corrupt the diff counts if +deletes ran first or interleaved. The order is: + +- **Phase A — upserts, parents first**: `groups`, `blocklist_sources`, then + the referrers (`clients`, `client_prefixes`, `rules`, `group_sources`, + `upstreams`, `local_records`, `forward_zones`, `settings`), with the + name→id / url→id maps built after the parent passes. Client promotion + happens here, in the `clients` pass. +- **Phase B — delete-absent, child first**, in `delete_order` + (config_schema.zig:99). Because every declarative child of a dying group is + itself absent from the file (validate guarantees file rules/prefixes + reference file groups), the child passes have already deleted and counted + them by the time the parent DELETE runs; the FK cascades become a safety + net, never the accountant. +- Immediately before the `groups` delete pass: observed clients + (`hand_edited=0`) whose `group_id` belongs to a dying group are reassigned + to the default group (id 1). `clients.group_id` has **no** ON DELETE clause + (config_schema.zig:26) — without this step, removing or renaming a group + that observed devices had been assigned to trips the FK mid-transaction: + exit 1, restart loop, and a `check` that said the file was fine. The + reassignment is the semantics we want anyway: the operator un-declared the + group, not the devices. + +Consequences that make the fix complete: an unchanged URL keeps id **and** +checksum + counters + `last_updated` together, so `loadSource` never sees +`.never_fetched`, `needsRefresh` does not fire spuriously (preserving +`last_updated` matters — checksum alone is not enough), `sweepOrphans` never +orphans `.list`/`.wild`, and a restart costs zero downloads. A changed +URL is a new identity: new row, new id, fresh download — consistent with +artifacts keyed by id. **Accepted trade, stated**: the old row's compiled +`.list`/`.wild` are orphaned at commit and swept before the +new source's first fetch succeeds, so a URL edit whose new host is down leaves +that list unenforced until a fetch lands. Deferring the sweep until a +successor fetch would need a cross-artifact lifecycle for an event that is +rare, operator-initiated, and bounded by the scheduler's retry — not worth the +machinery on a household box. The REST `updateSource` keeps runtime columns +across a URL edit; in file mode that route is rejected (ruling 7), so the +divergence is unreachable; in db mode the reconciler only runs via explicit +`import`. + +Related clock fix, same subsystem: `needsRefresh` is wall-clock arithmetic +(`now - last >= interval`, manager.zig:1201-1202) and the Pi has no RTC. A +fetch stamped while the clock was ahead (pre-NTP boot, restored image) +suspends refresh until real time catches the future timestamp — and this +design's idempotence would faithfully preserve the poison forever, having +deleted the wipe that used to be the accidental reset lever. The engine's +sibling fix: `needsRefresh` treats `last_updated > now` as refresh-due. One +comparison, with a test. + +Client promotion in place makes the `saved_clients` lift/merge/restore +scaffolding (import.zig:268-361, including `merge_observed_timestamps_sql`) +unnecessary: that machinery exists only because the wipe destroyed +`first_seen`/`last_seen` and had to smuggle them across. With no wipe, the +semantics it encodes — observed history survives declaration — are a plain +UPDATE that never touches the timestamp columns. The scaffolding dies with +the wipe (Deletions); its tests' semantics move to the promotion tests. + +**Ordering invariant**: reconcile commits before `manager.reload` +(app.zig:400) runs and before the scheduler's `sweepOrphans` can fire, so +preserved ids and checksums are visible to the filter layer before any +pruning. This holds by construction — reconcile completes inside `serve()` +before the manager exists — and is enforced *behaviorally*, not by a +statement-order unit test with nothing to grip: the restart-no-redownload +integration test (Tests, below) fails if anything between reconcile and +reload re-orders or wipes. No `serve()` restructuring is chartered for this. + +The engine returns a summary (ruling 8's input and the cross-session +contract): + +```zig +pub const TableCounts = struct { inserted: u32, updated: u32, deleted: u32 }; +pub const Summary = struct { + groups: TableCounts, sources: TableCounts, clients: TableCounts, + client_prefixes: TableCounts, rules: TableCounts, group_sources: TableCounts, + upstreams: TableCounts, local_records: TableCounts, forward_zones: TableCounts, + settings: TableCounts, + auth_transition: enum { none, enabled, disabled, rotated }, +}; +``` + +`updated` counts only rows actually written (writes-on-difference); +observed-client preservation counts nothing; promotion and reassignment count +as `updated` on `clients`. `deleted` on `clients` means exactly one thing: a +formerly declared (`hand_edited=1`) row absent from the file. An unchanged +file yields an all-zero summary. + +New repo verbs carry the engine (`sources_repo.upsertByUrl`, +per-table `deleteWhereNotIn`-style helpers), not new call ordering. `applyToDb` +and its wipe loop are deleted; import.zig keeps its parse/validate/diagnostics +plumbing. + +### 4. Password: the file overrides when it speaks, and only then + +`model.Web` changes to `password: ?[]const u8 = null` and +`password_hash: ?[]const u8 = null`. The settings bridge splits its skip +policy by direction — `isSkipped` becomes two functions, because encode and +decode need different sets: + +- `isEncodeSkipped` = {`web.password`, `web.password_hash`}: `toSettings` + stops emitting `web.password_hash` (the reconciler owns that settings row + directly) and continues to never emit `web.password`. +- `isDecodeSkipped` = {`web.password`} only: `fromSettings` **still loads** + `web.password_hash` from the settings table — the reconciler owning the + write does not mean the read path stops seeing it. Skipping it on decode + would leave `cfg.web.password_hash` null on every read path + (`config_export.readConfig` at export.zig:52, `mutations.loadConfig`), turn + `authEnabled` false, and silently disable auth in both modes. +- `decodeValue` gains an `.optional => try decodeValue(child, text)` arm + (model.zig:408-421 has none today; an unskipped optional field is currently + a `@compileError`). Absent key ⇒ default `null`; present ⇒ non-null. + +`auth.authEnabled` becomes `(web.password_hash orelse "").len != 0` +(auth.zig:62-63); app.zig:475's `.live_hash = .init(...)` unwraps with +`orelse ""`. The cases: + +- **Both set**: existing error (`PasswordAndHashBothSet`, import.zig:250, + validate.zig:395 — the check ports to `!= null` on both fields). +- **`password` present and empty**: rejected by `validate` with a new + diagnostic naming the remedy — `password_hash = ""` is how auth is disabled + declaratively. Without this rejection, `.password = ""` would hash the + empty string into a non-empty PHC (`authEnabled` true) while auth.zig:90 + refuses every empty-password login: auth on, unreachable. A declarative + fault, exit 2, so ruling 2's check/run agreement holds. +- **`password` set (non-empty plaintext)**: verify against the stored + `web.password_hash` with argon2; on match keep the stored hash, on mismatch + or absent stored hash, hash fresh. The justification is ruling 5 alone: + hashing unconditionally generates a fresh salt per apply and breaks + byte-stability. It is **not** a cost saving — argon2 verification recomputes + the full function (same t=2, m=19 MiB) with the stored salt, so + verify-and-keep costs exactly what hashing costs. Do not "optimize" the + verify away with any cached-plaintext scheme; that would be a security bug. +- **`password_hash` set**: written verbatim. An explicit `password_hash = ""` + is the declarative way to disable auth (auth.zig's documented empty-hash + state). +- **Neither set**: the stored hash is untouched. This is the deliberate + carve-out from file-as-sole-truth, because the naive alternative is a trap + the red team walked straight into: `toSettings` today always emits + `web.password_hash` with default `""` (model.zig:534, :93), so an operator + who hand-trims the ugly PHC string out of an exported file — meaning "keep + the current password" — would silently reconcile `""` over the stored hash + and open the admin UI to the LAN (`authEnabled` is `len != 0`, auth.zig:63). + Silence must mean "keep", and disabling auth must require the explicit + empty string. + +**Export's canonical form**: `password = null`, `password_hash = `. A non-null `password` is never emitted — the current +`cfg.web.password = "";` at export.zig:71 ported literally would make every +export carry a present-empty password beside a stored hash, tripping +`PasswordAndHashBothSet` on re-import: export's own output failing its own +rule, breaking ruling 9's adoption walkthrough and ruling 5's round trip. The +comment at export.zig:67-70 and the header sample change with it. With +`password = null`, the empty-plaintext rejection above does not fire and the +"hash written verbatim" branch keeps idempotence. + +Any auth transition (enabled/disabled/rotated) is reported in the summary and +printed by ruling 8 — an auth change is never a silent line item in a count. + +### 5. Idempotence is the invariant + +Reconciling the same file twice produces a byte-identical database — ids, +checksums, timestamps, `created_at`, password hash, the whole settings +table — **and** the second pass performs zero writes (all-zero summary, +`total_changes` unmoved). The unit test asserts +byte-stability via the `dump()` helper (import.zig:469), which is rewritten to +iterate an explicit all-tables list (`config_schema.table_names`, all ten +content-bearing tables) because its current driver, `content_tables`, is +deleted (ruling 6). Anything that churns under an unchanged file — including a +*non-canonical but equivalent* file — is a bug in the engine, by definition. +(This invariant is what forces ruling 3's rules-multiset, canonical matching, +and writes-on-difference, and ruling 4's password handling — the places a +naive design silently violates it. It is also half of why ruling 8 persists +no authority state at all.) + +### 6. `import` becomes a thin wrapper over reconcile; the guard becomes diff-gated + +`nxdns import` is db mode's one-shot apply and the restore tool, reimplemented +on the reconcile engine. Reconcile is non-destructive of *runtime* state, but +deletion of declarative rows absent from the file is still a first-class +outcome (ruling 3) — a mistaken `nxdns import ./wrong.zon` against a +configured DB would still remove every group, rule, upstream, and source not +in that file. So the guard is **retargeted, not deleted**: emptiness-gating +(`isEmpty`) becomes diff-gating. + +- After the reconcile passes, still inside the same `BEGIN IMMEDIATE` — + preserving import.zig:199-203's deliberate check-inside-the-write-lock + property, no TOCTOU — if any table's `deleted != 0` and no override flag, + roll back and fail exit 2 with the per-table delete counts in the message. + Observed-client reassignment is not declarative data and never trips the + gate (and under ruling 3's promotion rule, observed rows are never deleted + by reconcile at all). +- The flag is `--allow-delete` (`Options.allow_delete`), the renamed + `--force`; the error is `error.DestructiveImport`, the renamed + `DatabaseNotEmpty`, exit-2-mapped where cli.zig:533 maps today. This is + strictly better than the emptiness guard: additive and edit-only re-imports + stop needing a flag at all, and the flag now names what it permits. + "Edit-only" means edits to declarative columns on a matched identity; an + edit that *changes an identity column* — a group name, a source or upstream + url, a prefix, a zone, a rule tuple, a local-record identity — is a delete + plus an insert to the engine (ruling 3) and needs the flag. cli.md states + the distinction. +- `import.isEmpty` and `content_tables` still die — the diff-gate needs no + table list. + +`import` does not detect a file-managed DB, because nothing records one: +authority lives in the invocation (ruling 1) and the DB carries no marker +(ruling 8). On a box whose unit runs file mode, `import` behaves like any +other import — the diff-gate guards deletion, and the next boot's reconcile +converges the DB back to the file, its summary reporting what it corrected. +The docs own this story plainly: `import` is a **stop-first operation**, on a +file-mode box doubly so — against a running instance its effect is partial +(the runtime divergence below) and lasts only until the next restart. The +file-authority contract is stated with the same precision everywhere: the +file is the sole declarative source, **converged at every boot** — not a +lock on the database between boots. A detect-and-warn variant was +designed and deleted (ruling 8) — a per-invocation warning cannot *prevent* +db-side writes anyway, the CLI user is root on their own box, and the +asymmetry with the web layer's 403 is intentional: the web UI has +anonymous-ish LAN users, the CLI has the operator. + +What a mid-run import against a *running* file-mode instance actually does — +documented, because the divergence is partial, not merely deferred: +`Manager.reload` re-reads `blocklist_sources`, `groups`, `group_sources`, +`rules`, `clients`, `client_prefixes` from the DB at runtime +(manager.zig:405-489) and the scheduler runs `sweepOrphans` before every pass +(manager.zig:1095, :1115), so filtering follows the imported rows on the next +reload and can unlink the compiled `.list`/`.wild` of sources the +import deleted — while upstreams, listeners, and settings stay at boot +values, and the web UI shows the imported state under the file-authority +banner. The next restart's reconcile re-inserts deleted sources from the file +with new ids: a full re-download, the exact cost this design exists to +prevent. That is why the docs name the restart requirement and the +re-download cost. The settings envelope's `reconciled_at` cannot see a CLI +import — ruling 7's weakened claim covers exactly this. + +The startup-vs-import race needs no code: both paths take `BEGIN IMMEDIATE` +under the 5 s busy timeout (db.zig:252), so the outcome is ordering, not +corruption — an import racing a restart may be reverted by the reconcile that +wins the lock second. Documented, not engineered around. + +First-run story in db mode, after bootstrap dies: a fresh empty DB fails +validation naturally (`NoUsableUpstreams`, exit 2). The remediation hint — +one fixed line naming `nxdns import` and `run --config` — is owned by +**cli.zig's db-source fault renderer** (one arm, beside the exit-code mapping; +Zig errors carry no text and validate.zig must stay mode-blind), and the same +line serves bare `check` with no DB (ruling 2). One location, R2's charter. + +`export` is unchanged in role: the diagnostic/capture tool in both modes, and +the file-mode adoption tool (its canonical password form changes per +ruling 4). + +### 7. Web layer: policy as data, rejected after auth, plain 403 + +`WebState` gains `authority: union(enum) { database, managed_file: []const u8 }` +and `reconciled_at: ?i64`, built where `WebState` is assembled (app.zig:468). +The `managed_file` path slice is owned by `serve`'s arena, which outlives +`WebState` — R2 provides it, R3 consumes it, neither copies. `reconciled_at` +is stamped by `serve` immediately after the reconcile commits — same process, +same frame, a local — and is `null` in db mode, which never reconciles. + +`RouteInfo` gains `policy: enum { read, config_write, runtime_action }` beside +`auth` and `rate_limit` — policy-as-data, matching the table's existing style. +No default value: all 56 route entries state their class explicitly, and +router.zig's `test_table` (:198) gains the field too. Classification is +per-route, not per-prefix: `POST /api/blocklists/update` is a +`runtime_action`; its CRUD siblings are `config_write`. + +- Config writes, rejected in file mode: all group / blocklist / rule / + local-record / forward-zone / upstream / client-prefix mutations, + `PUT /api/settings` (password changes go through the file; its + `live_hash.installAndRevoke` side effect never fires in file mode), and + client PUT — naming or regrouping an observed client is declarative drift + (open question 1). +- Client DELETE is a **runtime action**: deleting an observed + (`hand_edited=0`) row discards runtime state the file never declared — + without this, a mis-identified or departed device's row is immortal in file + mode, since the file can only promote IPs, never remove them. Deleting a + *declared* client contradicts the file: the handler answers the same 403 + envelope. This is the one policy decision that needs a row read; it lives + in the client handler, not the router. (After ruling 3's promotion, a + declared IP's row *is* declared — DELETE answers 403, the intended + reading.) +- Runtime actions, always live: pause, blocklist refresh, cert reload, + login/logout. + +Enforcement lives in `router.dispatch` **after** `check_auth`, before the +handler — match → rate limit → auth → policy. Pre-auth rejection would leak +route existence; the codebase answers 401 first and this design keeps that. + +Rejection is **403**, body the existing single-field envelope: +`{"error":"configuration is managed by /etc/nxdns/config.zon; edit the file and restart"}`. +Not 409 — that status already means constraint conflict four ways in +openapi.yaml — and **no `code` field**: 403 is unused today, so the status +alone is machine-readable; the UI learns authority declaratively from +`GET /api/settings`, not by probing errors; and the single-property `Error` +schema, golden contract samples, and `api.ts` stay untouched. An optional field +with no consumer is machinery, not a contract. + +The envelope must survive its own path: `respondError` today builds into a +fixed 512-byte buffer and **silently downgrades to `text/plain`** on overflow +(http_util.zig:315-327) — a long managed-file path (nested bind mounts) would +demote the documented JSON envelope. Fix at the root: `respondError` gets the +arena treatment `respondJson` already uses (`Writer.Allocating` over +`request.arena`, :337-339) and the `respondPlain` silent-downgrade path is +deleted — which fixes every long error message, not just this one. +`src/web/http_util.zig` joins R3's ownership. + +Authority discovery: the `GET /api/settings` envelope gains +`authority: {mode, path, reconciled_at}` — an authenticated route, so the +filesystem path never leaks through open `/api/version`/`/api/health`. All +three come from `WebState` (live truth); `path` is present in file mode only +and `reconciled_at` is nullable — `null` in db mode. Its meaning is exactly +"**this process loaded the file at T**" — restart-pending detection: an mtime +newer than `reconciled_at` means the running process has not loaded the +current file. The comparison is one-directional and non-authoritative — a +stepped clock (pre-NTP boot stamping the future, ruling 3's clock fix +territory) or a preserved mtime (`git checkout`, `rsync -a`) can make a newer +file look older, and the DB can move without either timestamp moving +(ruling 6's `import`, this ruling's `runtime_action` routes). It does not +answer "is the file what the server uses"; answering that would take content +hashing, which the anti-requirements refuse. The UI renders a read-only +banner (RestartBanner slot precedent) and disables mutation controls, with +the 403 as backstop. + +### 8. The operator can see what happened + +- `logStartup` (after `logging.install`) logs the authority: + `authority: database` / `authority: file (/etc/nxdns/config.zon)`. +- In file mode, the reconcile summary (ruling 3's `Summary`) prints through + the Runner at startup, matching `seedFromFile`'s existing output discipline: + per-table inserted/updated/deleted counts, the changed settings **keys** + (never values), and the auth transition when there is one. It is the answer + to "what did that restart change" without opening sqlite. +- **Authority is never persisted.** The DB carries no record of which mode + wrote it: authority lives in the invocation (ruling 1), per-process state + (`WebState.authority`, `reconciled_at`) serves the API (ruling 7), and the + journal holds the history. A persisted marker was designed twice and + deleted twice. A per-boot `reconciled_at` settings row breaks ruling 5 + outright — `dump()` iterates the settings table, so any row rewritten per + reconcile forfeits byte-identity, and `putSetting` moves `total_changes`, + forfeiting zero-writes. The write-on-difference `authority.mode`/`path` + variant survived idempotence but cost a reserved key namespace, a + `fromSettings` decode filter, and a reconciler sweep exemption — three + mechanisms whose only consumer was one `import` warning (ruling 6). State + that exists to power a courtesy message is not worth a namespace. The + settings sweep's exemption list is therefore exactly one key: + `web.password_hash` (ruling 4). + +No `std.log.err` in any new code, per the standing spec rule for new code +(the pre-existing calls in db.zig/tls_server.zig are out of scope). + +### 9. Deployment and migration + +- **systemd**: the shipped unit stays flagless (db default) and gains two + lines. `RestartPreventExitStatus=2 64` — a config fault or usage error must + not restart-loop every 2 s until StartLimitBurst; the journal holds the + diagnostics and the fix is a file edit, not a retry. (Correct in db mode + too: exit 2 means the config is wrong in either mode. And it is why + ruling 2 keeps exit 2 to *declarative* faults only — a transient box fault + mapped to exit 2 would stop the unit permanently.) And + `ReadOnlyPaths=/etc/nxdns` — `ConfigurationDirectory=nxdns` makes systemd + create the directory owned by the service user, so without this line the + source-of-truth file is writable by the very process whose mutation routes + file mode exists to disable; nxdns never writes `/etc/nxdns` in either mode, + so the base unit ships the enforcement rather than recommending it. Docs + show a drop-in for file mode: `ExecStart=` reset plus + `--config=/etc/nxdns/config.zon`. +- **Ansible / config-management**: the docs' deployment guidance mandates + `nxdns check --config=` as the handler precondition — validate + the pushed file *before* restarting, so a typo is a failed deploy at noon, + not a dead resolver at the next 3am power blip. +- **docker**: compose ships file mode as its example — + `command: ["run", "--config=/etc/nxdns/config.zon"]` (resolving old open + question 4; the `:ro` mount at compose.yaml:15 already suggests it). This keeps the + fresh-install and volume-loss stories working after bootstrap dies: a + recreated `nxdns-data` volume reconciles from the mounted file on next + start, which is *better* than the old seed-once self-heal. The binary's + default stays db. The seed-once comment (compose.yaml:9-12) is rewritten. + For db-mode-in-docker, the docs show the recovery one-liner + (`docker compose run --rm nxdns import /etc/nxdns/config.zon` — the file is + positional, `ImportArgs.file` at cli.zig:61, plus `--allow-delete` when the + diff deletes) — without it, `restart: unless-stopped` plus exit 2 is an + infinite crash loop (docker has no start limit) with no documented way out. +- **Adopt file mode** on a UI-configured box: **stop the service first**, then + `nxdns export --out /etc/nxdns/config.zon` → + `nxdns check --config=/etc/nxdns/config.zon` → add the flag → start. + Stop-first is load-bearing twice: `export` opens the DB immutable and + refuses with `WalPending` against a live instance's steady-state WAL + (db.zig:222-226, cli.md:121), and any UI edit landing between a live export + and the restart would be silently reverted by the first reconcile. Stopped, + the first reconcile's summary is all-zero and writes nothing — blocklist + state, compiled files, and client history all survive. Every unchanged boot + after it writes nothing either. +- **Leave file mode**: drop the flag (remove the drop-in), restart. The DB + already holds the last reconciled state; nothing else needed. +- **Binary downgrade from file mode**: no unit edit is needed — the old + binary accepts `run --config` with seed-once semantics, and against the + already-configured DB it ignores the file and serves the last-reconciled + state. The rollback note states the consequence: file edits stop applying + until the binary is upgraded again. +- **Restore from backup**: db mode is stop → restore `config.db` → start, + with the WAL caveat db.zig itself documents: take backups only from a + stopped instance (or use `export`), and on restore delete any stale + `config.db-wal`/`config.db-shm` beside the target — a mismatched WAL is + silently discarded by SQLite, which turns "restore" into "lose the tail". + Restoring an *exported file* into a populated DB via `import` is exactly + the deleting case: back-up-and-restore.md documents `--allow-delete` there + (ruling 6) — the flag choreography is rewritten, not removed. In file mode + `config.db` is not the config backup — the file is; restore = redeploy the + file. The query-log DB restores independently in both modes. + +Migration honesty, replacing the earlier blanket claim: a db-mode install +that never passed `--config` needs nothing. Anyone whose unit, wrapper, or +compose `command` carries `run --config` (the documented seed-once invocation +in first-run.md and three how-to labs) gets file authority at the first +post-upgrade start — the seed file becomes the config, and UI edits made +since seeding are deleted by the first reconcile. The upgrade doc's +breaking-changes section leads with this and gives the two exits: drop the +flag to keep the DB, or re-export to the file path first to adopt file mode +cleanly (the walkthrough above). `check --config` keeps its meaning. Docker +db-mode fresh installs must use the new compose or run `import` once. No +schema migration. + +### 10. Documentation is part of the change + +`explanation/configuration-model.md` is rewritten around the two-mode model +(its "why the database wins" argument becomes mode-scoped, not superseded). +Seed-once claims are corrected in: the first-run tutorial, install-with-systemd, +install-with-docker, upgrade (breaking-changes + rollback sections, +ruling 9), back-up-and-restore (new restore semantics, WAL sidecars, +`--allow-delete`), set-up-admin-authentication (the ruling 4 +absence/empty-string rule), troubleshoot, cli.md +(`run`/`check`/`import`/`export`, `--allow-delete`), configuration.md +(optional password fields), files-and-directories.md, api.md (settings +envelope, 403, per-route rejectability), the compose.yaml comment, +README/INSTALL. + +## Deletions (complete list) + +`config/bootstrap.zig` (+2 tests, S7 cases 15-18, + app.zig import/call sites +:35/:142/:146 and tests :1124/:1237) · `import.isEmpty` (+6 tests, + the +storage_integration_test.zig call sites :763/:795/:894, + the clients_repo.zig +doc comments :6/:129) · `config_schema.content_tables` (+its test; `dump()` +re-pointed to the new `table_names` list) · `app.seedFromFile` (+its app.zig +tests) · `CheckArgs.config_explicit` · the `checkImpl` source heuristic · +`applyToDb`'s wipe loop, including the `saved_clients` lift/merge/restore +scaffolding (import.zig:268-361 — superseded by ruling 3's +promotion-in-place) · `toSettings`'s `web.password_hash` emission (+its +key-list test row; the model.zig:665-670 "skipped in both directions" test is +rewritten for the encode/decode split) · `http_util.respondPlain`'s +silent-downgrade path (`respondError` rebuilt on the request arena). + +Renamed, not deleted (ruling 6): `Options.force` → `Options.allow_delete` +(`--force` → `--allow-delete`), `error.DatabaseNotEmpty` → +`error.DestructiveImport` (cli.zig:533 arm retargeted, faults.zig exclusion +and tests follow the name). + +## Sessions + +R1 lands first and R2 rewires onto it and carries every deletion — they are +ordered, not parallel (the old plan had R1 deleting `bootstrap.zig` out from +under R2-owned app.zig call sites). R1 is **additive engine plus one +coordinated model change**, not purely additive: the `model.Web` optional +fields force same-session edits at every site that reads them, or the tree +stops building. R3 and R4 run parallel to both; their interfaces +(`reconcile.Summary`, `WebState.authority` + `reconciled_at`, +`RouteInfo.policy`) are fixed here. + +### Session R1: reconcile engine + the coordinated model change + +Owns `src/config/reconcile.zig` (engine + `Summary` as specified), the new +repo verbs in `src/storage/repositories/`, the `needsRefresh` clock clamp in +`src/filter/manager.zig`, registration in `src/tests.zig`. Rulings 3, 4, 5. + +Owns `src/config/model.zig` whole: the optional `Web.password`/`password_hash` +fields, the `isEncodeSkipped`/`isDecodeSkipped` split, `decodeValue`'s +`.optional` arm, and the bridge tests. And the sites the +optional fields break, which no session previously owned: `validate.zig` +(:395 both-set port, the new empty-password rejection, test :1966-1968), +`auth.zig` (`authEnabled` orelse), `export.zig` (canonical `password = null`, +comment and header sample), `src/web/handlers/settings.zig` (`newPassword`'s +`orelse` chain at :184-186, and `FieldType` collapsing `?T` so `Partial`'s +`?FieldType(...)` at :114 does not generate a double optional — landing this +in R1 keeps R3's `web/` work unblocked). R1 also makes the one-line +`orelse ""` edit at app.zig:475 so the tree builds; app.zig otherwise stays +R2's, and R2 absorbs that line into its startup rewire. + +### Session R2: CLI + startup + deletions + +Owns `src/cli.zig`, `src/app.zig`, `src/config/faults.zig`, +`src/config/import.zig` (thin-wrapper rewrite, diff-gate, `--allow-delete`), +the shared loader-fault mapping helper (ruling 2, used by `run` and `check`), +and the entire Deletions list plus the renames. Rulings 1, 2, 6, 8. Threads authority and +`reconciled_at` into `WebState` (arena-owned path) but does not touch the +router. Owns the cli.zig hint-line renderer arm. + +### Session R3: web enforcement + UI + +Owns `src/web/router.zig` (dispatch policy step, `test_table` gains the +policy column), `src/web/routes.zig` (all 56 entries state `policy` +explicitly — no default), `src/web/http_util.zig` (`respondError` arena +rewrite, `respondPlain` downgrade deletion), `src/web/openapi.yaml`, the +client-DELETE observed/declared branch in the clients handler, `web/` +frontend (banner, disabled controls, settings envelope with nullable +`reconciled_at`). Ruling 7. Consumes `WebState.authority`/`reconciled_at` as +fixed above. + +### Session R4: deployment + docs + +Owns `deploy/**`, `docs/**`, `README.md`, `INSTALL`, compose file + comment, +the systemd unit's two new lines. Rulings 9, 10. + +### Orchestrator + +This spec, R1→R2 sequencing, cross-session integration, the drift-guard +regenerations that span sessions (contract samples, openapi route counts, +api.md/cli.md rows), and the `-Dlive` acceptance run. + +## Tests + +- **Reconcile unit tests** (R1, `:memory:` + migrate): idempotence via the + rewritten `dump()` byte-stability across all tables after applying the same + file twice — including a file carrying a plaintext password **and a + non-canonical but equivalent file** (`FD00::1`-style) — plus the zero-writes + assertion (all-zero `Summary`, `total_changes` unmoved) on the second pass; + per-table preservation via `sources_repo.SourceRow` after seeding stats with + `updateSourceStats`; URL change ⇒ new id; source removal; observed-client + survival, **promote-on-declare (asserting `first_seen`/`last_seen` survive + the promotion and the row id is stable)**, and **reassign-to-default when + their group is removed or renamed** (the un-cascaded FK case); + `safe_search` edit on an existing group converges; rules `created_at` + stability including duplicate tuples; password verify-keeps-hash, + mismatch-rehashes, absent-keeps-stored, explicit-empty-disables; rollback + on mid-tx failure; default-group id 1 pin; `needsRefresh` + future-`last_updated` clamp. +- **Model/bridge tests** (R1, model.zig test neighbourhood at :659): + `decodeValue` on an optional field (absent ⇒ + null, present ⇒ value); `web.password_hash` decodes from settings but is + not encoded (the encode/decode split); `validate` rejects present-and-empty + `password` with the `password_hash = ""` remedy in the diagnostic; export ⇒ + import round trip with the canonical `password = null` form. +- **Import gate tests** (R2): a file whose diff deletes rows, without + `--allow-delete` ⇒ rollback, exit 2, per-table delete counts in the + message; with the flag ⇒ applied; additive/edit-only import needs no flag. +- **Loader-fault mapping** (R2, on the shared helper directly): each + path-class open error ⇒ `ManagedConfigUnreadable`; a non-path member of the + set propagates unmapped. +- **Restart-no-redownload** in filter_integration_test.zig: reconcile, then a + `Manager` restart reuses `.list`/`.wild` with no refetch (fixtures + already assert reuse by id) — this is also the behavioral enforcement of + ruling 3's ordering invariant. Plus **one `-Dlive` run of the real download + path** — hermetic suites have hidden a process-killing bug on the real + network path before (MEMORY), so the acceptance includes the real path once. +- **Router**: policy classification unit tests via `test_table`/`matchPath`, + no sockets; the contract table in web_integration_test.zig gains a `policy` + column and the existing 1:1 coverage assertion widens so classification + cannot drift; one socketed test per class in file mode (config write → 403, + runtime action → 2xx, read → 200) plus observed-vs-declared client DELETE, + via `EnvOptions` authority; `respondError` with a message longer than the + old 512-byte buffer stays `application/json`. Route count stays 56 — no new + routes. +- **CLI**: `parseArgs` tests: `run` without `--config` selects db authority; + `run --config=` selects file authority and the path lands in the run + args; `--allow-delete` parsing; `usage_text` test; exit-2 for a missing + managed file and for bare `check` with no DB (hint line present) — via the + `Captured` runner (in-process; remember the buffered-writer caveat — flush + through a real `File.Writer` where the test asserts delivery). +- **Drift guards knowingly tripped and regenerated**: openapi.yaml (settings + envelope, 403 responses), golden contract samples (`-Dcontract-samples-out`), + api.md rows, cli.md sections, docs_drift_test, `toSettings` key-list test. + +## Acceptance (design complete when implemented) + +- [ ] `nxdns run --config=` on a DB with fetched blocklists (any boot + after adoption): restart performs zero downloads **and zero DB writes**; + source ids, checksums, `last_updated`, and compiled + `.list`/`.wild` files are identical before and after. Proven + once with `-Dlive` against a real source. +- [ ] Reconciling an unchanged exported config twice yields a byte-identical + `dump()` and an all-zero summary — including with a plaintext + `password` in the file, and with non-canonical addresses. +- [ ] Removing (and renaming) a group that observed clients were assigned to + converges: clients land in the default group, no FK error, counts + reported. Declaring an observed client's IP promotes the row in place: + `first_seen`/`last_seen` and row id survive, counted as `updated`. +- [ ] A file with neither `password` nor `password_hash` leaves the stored + hash — and auth — intact; `password_hash = ""` disables auth and the + startup summary says so; a present-but-empty `password` is refused at + validate with a diagnostic naming `password_hash = ""` as the disable + path. +- [ ] `run --config=` with a missing file exits 2 with the path in + the message; with an invalid file exits 2 with diagnostics; never serves + from the DB. An open failure outside the path class (fd exhaustion, I/O + error) exits 1, not 2. `check --config=` agrees with `run` + on every parse/validate/path fault via the shared helper (the scoped + claim of ruling 2). +- [ ] A config file present at `/etc/nxdns/config.zon` with no `--config` + flag changes nothing: bare `run` serves the DB and never reads the + file. +- [ ] `nxdns import` whose diff would delete rows fails exit 2 without + `--allow-delete`, printing per-table delete counts, and rolls back; + with the flag it applies; an additive import needs no flag. +- [ ] Fresh empty DB in db mode exits 2 (`NoUsableUpstreams`) with the hint + line; bare `check` with no `config.db` exits 2 with the + same hint; neither restart-loops under the shipped unit + (`RestartPreventExitStatus=2 64`). +- [ ] The shipped compose file boots a fresh container (empty volume, mounted + config.zon) into file mode successfully; the db-mode import recovery + one-liner is documented and works. +- [ ] In file mode: every `config_write` route answers 403 with the + single-field error envelope — `application/json` even when the managed + path is long; every `runtime_action` and `read` route behaves as in db + mode; DELETE of an observed client succeeds, of a declared client + answers 403; unauthenticated requests to protected routes still answer + 401, not 403. +- [ ] `GET /api/settings` reports `authority` with `reconciled_at` (null in + db mode); the UI shows the read-only banner and disables mutation + controls in file mode. +- [ ] `rg -n 'import\.isEmpty|content_tables|config/bootstrap|seedFromFile|config_explicit' src/` + returns nothing (historical specs exempt; pattern chosen so + fetcher.zig's `host.isEmpty()` and validate.zig's "bootstrap problem" + prose cannot false-positive). +- [ ] Adopt-file-mode walkthrough (stop → `export` → `check --config` → add + the flag → start) run end to end on a UI-configured instance: the first + reconcile summary is all-zero and writes nothing, as does every + unchanged boot after it; leave-file-mode (drop the flag, restart) + serves identically; `export` against the *running* instance refuses + with the WalPending message, as documented. +- [ ] All existing gates pass; tripped drift guards are regenerated, not + suppressed. + +## Anti-requirements + +- No file watcher, no inotify, no SIGHUP reload — restart is the reload. +- No UI write-back to the file, no partial/merge authority, no per-table + hybrid modes, no multi-file config. +- No persisted authority state, no config hashing, no content-based change + detection — authority lives in the invocation; last-load time is + per-process state in the settings envelope, honest about what it can and + cannot answer (ruling 7). +- No deferred deletion of a replaced source's compiled artifacts — the URL-edit + gap is an accepted trade (ruling 3), bounded by the scheduler retry. +- No etag/conditional GET for blocklist fetches. +- No `code` field or richer error envelope — the status is the machine + contract. +- No rule-identity schema change (no synthetic rule key column). +- No preservation of rule `created_at` across pattern edits — an edited rule + is a new rule. +- No CLI-side blocking or detection of `import` against a file-mode box's + DB — the diff-gate guards deletion; the next boot converges and its + summary reports what it corrected. +- No fallback from file mode to db mode under any failure. + +## Resolved defaults (were open questions; the user may overrule before R3/R4) + +1. **UI naming and regrouping of observed (`hand_edited=0`) clients in file + mode: rejected as declarative drift** (client PUT stays `config_write`, + ruling 7). The household user adds the client to the file instead. The + alternative — carving naming/grouping out as a runtime action — + reintroduces two-way merge for one table, which the anti-requirements + refuse. Affects R3. +2. **`export` output is byte-identical in both modes** — no file-mode + annotation. An annotation would make export → file → adopt produce a + different file than the one checked, for a label the operator already has + in the unit file. Affects R4's round-trip docs. +3. **File mode ships as a documented systemd drop-in**, not a second + commented `ExecStart` in the packaged unit. The packaged unit stays + flagless and correct by itself; a commented alternative line in a unit + file is a doc pretending to be config. Affects R4. + +## Cross-validation findings rejected + +Round 1: none — all twelve findings (F1-F12) were confirmed against the repo +and are folded into the rulings above. The F1 remedy has since been +superseded: the premise revision deleted the persisted marker entirely +(ruling 8) instead of keeping it write-on-difference. + +Round 2 (on the premise revision) returned three important findings and one +minor. Folded: the docker recovery one-liner used a `--config` flag `import` +does not have (fixed in ruling 9, file is positional); `import` is now +stated everywhere as a stop-first operation and the file-authority contract +as "converged at every boot" (ruling 6); identity-column edits are named as +delete-plus-insert needing `--allow-delete` (ruling 6). Declined: renaming +`run --config` to `--managed-config` — reasons recorded in ruling 1. + +## Red-team findings rejected + +None rejected outright — every finding checked out against the repo. Two +proposed remedies were declined while their findings were accepted: + +- **ops 10's mitigation** (keep the old compiled artifact until the successor + URL's first successful fetch): declined — a cross-artifact lifecycle for a + rare, operator-initiated event on a household box; the trade is now stated + in ruling 3 and the anti-requirements instead. +- **ops 14's db-mode warning** ("a config file exists but authority is + database"): declined — db mode is given no file path, so warning would mean + probing a well-known location, which is precisely the ambient inference + ruling 1 bans; the accepted half (drift visibility) is served by the + settings envelope's `authority` block and per-process `reconciled_at`.