."`, built at comptime from the field names, so a new field cannot drift out of the mapping: ```zig pub const SettingPair = struct { key: []const u8, value: []const u8 }; /// Writes every scalar field of `cfg` as a key/value pair into `out`. Keys are comptime strings /// (never freed); values are allocated from `gpa`. pub fn toSettings(cfg: Config, gpa: std.mem.Allocator, out: *std.ArrayList(SettingPair)) error{OutOfMemory}!void; pub const SettingsError = error{ BadSettingValue, OutOfMemory }; /// Applies pairs onto `cfg`, which the caller has initialized to `.{}` (all defaults). /// An absent key keeps the default — that is how a migration adds a setting with no data step. /// An unknown key is logged at `warn` and counted in `unknown_keys`; it is never an error, because /// downgrading a binary must not brick a config database. pub fn fromSettings(pairs: []const SettingPair, cfg: *Config, unknown_keys: *usize) SettingsError!void; ``` Both are implemented with `inline for` over `@typeInfo(Section).@"struct".fields` for each section, so the section list appears exactly once. Value encoding, one function per field type: - `bool` → `"true"` / `"false"`; anything else → `error.BadSettingValue`. - integers → `std.fmt.parseInt` / decimal formatting, on the field's declared type, so an out-of-range stored value is `error.BadSettingValue` rather than a truncating cast. - enums → `std.meta.stringToEnum` on the DB text form; an unknown tag is `error.BadSettingValue`. - `[]const u8` → verbatim; allocated from `gpa` on the way out of the DB. `web.password` is skipped in **both** directions — it is never a settings row (S2.5). The full key list is 47 entries and is asserted by a test: `toSettings` on a default `Config` produces exactly the expected sorted key list, written out literally in the test. A field added without updating that literal breaks the test — which is the point. ### S2.4 no-panic arithmetic Every unit conversion the rest of the program needs lives here as a named function, and every one is guarded by a `comptime` assertion that the field type's maximum cannot overflow the destination: ```zig pub fn connectTimeout(u: Upstream) std.Io.Duration; // ms → ns pub fn readTimeout(u: Upstream) std.Io.Duration; pub fn totalTimeout(u: Upstream) std.Io.Duration; pub fn sessionTtlSeconds(w: Web) i64; // hours → seconds pub fn retentionSeconds(l: Logging) i64; // days → seconds pub fn maxLogBytes(l: Logging) u64; // MiB → bytes pub fn minFreeBytes(d: Disk) u64; pub fn warnFreeBytes(d: Disk) u64; pub fn updateIntervalSeconds(b: BlocklistUpdate) i64; // hours → seconds comptime { assertFits(u32, std.time.ns_per_ms, i96); // timeouts assertFits(u16, 3600, i64); // session ttl assertFits(u16, 86400, i64); // retention, update interval assertFits(u32, 1024 * 1024, u64); // MiB conversions } ``` `assertFits(FieldType, factor, Dest)` is a comptime helper that `@compileError`s unless `@as(u128, std.math.maxInt(FieldType)) * factor <= std.math.maxInt(Dest)`. No conversion in this file uses a `@intCast` on a value whose range has not been proven this way, and no conversion can return an error — the types make overflow impossible, which is stronger than checking at runtime. `std.Io.Duration` is nanoseconds in an `i96` (verified above), so the timeout conversions are trivially safe; the assertion is there so a later widening of a field type breaks the build. ### S2.5 password handling - `web.password` is operator input in a hand-written bootstrap or import file. - `web.password_hash` is the argon2id PHC string actually stored in `settings` under `web.password_hash`. - Import/bootstrap rule (implemented in S5, specified here because it is model semantics): - `password != "" and password_hash != ""` → `error.PasswordAndHashBothSet`. Ambiguity in a security setting is refused, not guessed. - `password != ""` → hash it, store the hash, discard the plaintext. - `password == ""` → store `password_hash` verbatim (possibly `""`, which disables auth per PLAN §3.11). - Export always emits `.password = ""` and the stored `.password_hash`. This is exactly what makes the round trip byte-stable: the second import takes the `password == ""` branch and stores the same hash, so the third export is identical to the second. - Hashing uses `std.crypto.pwhash.argon2.strHash(password, .{ .allocator = gpa, .params = .owasp_2id, .mode = .argon2id, .encoding = .phc }, out_buf, io)` (verified: argon2.zig:591; `Params.owasp_2id` is `t=2, m=19 MiB, p=1`, argon2.zig:96). `owasp_2id` rather than `interactive_2id` (64 MiB) because PLAN §18 budgets under 100 MB total on a Pi 5. `out_buf` is 256 bytes, which comfortably holds a PHC-encoded argon2id string. - **Not in this milestone:** verification, sessions, cookies, any login flow. Only the hash is produced and stored. ### S2.6 the validator ```zig pub const Problem = struct { /// Dotted path into the config, e.g. "upstreams[2].url" or "web.port". Owned by Diagnostics. path: []const u8, /// Human-readable, e.g. "unknown group 'kids'". Owned by Diagnostics. message: []const u8, err: ValidateError, }; pub const Diagnostics = struct { gpa: std.mem.Allocator, problems: std.ArrayList(Problem), pub fn init(gpa: std.mem.Allocator) Diagnostics; pub fn deinit(self: *Diagnostics) void; // frees every path and message pub fn add(self: *Diagnostics, err: ValidateError, comptime fmt: []const u8, args: anytype) error{OutOfMemory}!void; /// Writes one "path: message" line per problem. pub fn writeAll(self: *const Diagnostics, w: *std.Io.Writer) std.Io.Writer.Error!void; }; pub const ValidateError = error{ NoUpstreams, BadUpstreamUrl, DuplicateUpstreamUrl, MissingDefaultGroup, DuplicateGroupName, UnknownGroup, EmptyGroupName, BadClientIp, DuplicateClientIp, BadClientPrefix, DuplicateClientPrefix, BadSourceUrl, DuplicateSourceUrl, EmptySourceName, UnknownSource, DuplicateGroupSource, BadRulePattern, BadLocalRecordName, BadLocalRecordValue, DuplicateLocalRecord, BadForwardZone, DuplicateForwardZone, BadResolverUrl, BadPort, BadTimeout, BadTtl, BadRetention, BadLogRotation, BadDiskThresholds, BadRateLimit, BadBindAddress, MissingCertPath, MissingKeyPath, MissingLogPath, PasswordAndHashBothSet, OutOfMemory, }; /// Collects EVERY problem into `diags`, then returns the first one's `err`. /// Pure: no Io, no SQLite, no clock. The allocator is only for diagnostic text. pub fn validate(cfg: Config, diags: *Diagnostics) ValidateError!void; ``` The controlling requirement: **the validator must reject everything the schema would reject.** An import that passes validation and then dies on a `Constraint` mid-transaction gives the operator a SQLite error message instead of a line number. Concretely: *Uniqueness* — one check per `UNIQUE` constraint in §11.2, each implemented with a `std.StringHashMapUnmanaged(void)` over the already-seen values: | Constraint | Check | |---|---| | `groups.name UNIQUE` | `DuplicateGroupName`; also `EmptyGroupName` for `""` | | `upstreams.url UNIQUE` | `DuplicateUpstreamUrl` | | `clients.ip UNIQUE` | `DuplicateClientIp`, compared on the **canonical** form (see below) | | `client_prefixes.prefix UNIQUE` | `DuplicateClientPrefix`, canonical form | | `blocklist_sources.url UNIQUE` | `DuplicateSourceUrl` | | `group_sources PRIMARY KEY (group,source)` | `DuplicateGroupSource` | | `local_records UNIQUE(name,rtype,value)` | `DuplicateLocalRecord` | | `forward_zones.zone UNIQUE` | `DuplicateForwardZone` | `rules` has no `UNIQUE` constraint, so duplicate rules are accepted; export sorts them deterministically, so a duplicate still round-trips byte-stably. Client IPs and prefixes are compared **after canonicalization** through `NetAddress.parse` + `format` / `Prefix.parse` + `format`, because `fd00::1` and `FD00:0:0:0:0:0:0:1` are the same row to a human but two distinct rows to `UNIQUE`. Import writes the canonical form, so the validator must detect the collision the same way. *Foreign keys* — every `group` reference in **all four** referencing collections (`clients`, `client_prefixes`, `group_sources`, `rules`) must name a group present in `cfg.groups` (`UnknownGroup`), and every `group_sources.source_url` must name a source present in `cfg.blocklist_sources` (`UnknownSource`). `cfg.groups` must contain a group named `"default"` (`MissingDefaultGroup`) — §11.2 seeds it, §7.2 depends on it, and an import that dropped it would leave the server with nowhere to assign an unknown client. *Parsing* — reuse, do not reimplement: - upstream URLs → `transport.Endpoint.parse` (`BadUpstreamUrl`). - `clients.ip` → `NetAddress.parse` (`BadClientIp`). - `client_prefixes.prefix` → `Prefix.parse` (`BadClientPrefix`). - `dns.bind_ipv4` must parse as an `.ip4`, `dns.bind_ipv6` and `web.bind`, `doh_server.bind`, `dot_server.bind` must parse as either family (`BadBindAddress`). - every domain-shaped string — `rules.pattern` (with `*` segments removed first), `local_records.name`, a `cname` record's `value`, `forward_zones.zone` — through `dns.name.fromText`. - `local_records.value` for `a` must parse as `.ip4`, for `aaaa` as `.ip6` (`BadLocalRecordValue`). - `blocklist_sources.url` must be `http://` or `https://` with a non-empty host (`BadSourceUrl`); `blocklist_sources.name` non-empty (`EmptySourceName`). - `forward_zones.resolver` through a `pub fn parseResolver(text: []const u8) ResolverError!Resolver` declared **in this file**: scheme `udp://` or `tcp://` (PLAN §6.5 permits plain transports for local infra), host an IP literal via `NetAddress.parse`, port 1–65535, nothing after the authority. `transport.Endpoint.parse` cannot be used — it rejects these schemes by design. Phase 5's `local/forward_zones.zig` imports `parseResolver` rather than writing a second one. - Wildcard rule patterns: `kind == .wildcard` requires at least one label that is exactly `"*"`; `kind == .exact` requires no `*` at all. Every other label must satisfy `dns.name.fromText` when the `*` labels are substituted with a placeholder label. Matching semantics are Phase 5's; this is syntax only. *Ranges* — all `BadPort` / `BadTimeout` / … as listed: - `dns.port`, `web.port`, `doh_server.port`, `dot_server.port` in 1–65535 (0 is rejected). - `upstream.connect_timeout_ms`, `read_timeout_ms`, `total_timeout_ms` each ≥ 100 and ≤ 120_000; `total_timeout_ms >= connect_timeout_ms` and `>= read_timeout_ms`. - `dns.rate_limit` ≥ 1, `dns.rate_window_seconds` in 1–3600, `web.api_rate_limit_per_min` ≥ 1, `web.sse_max_connections_per_ip` ≥ 1 (`BadRateLimit`). - `blocking.ttl` ≤ 86400, `cache.negative_ttl_max` ≤ 86400, `local_records.ttl` in 1–604800 (`BadTtl`). - `logging.retention_days` ≥ 1, `logging.query_log_buffer_max` ≥ 1 (`BadRetention`). - `logging.max_size_mb` ≥ 1 and `logging.max_files` ≥ 1 (`BadLogRotation`). - `disk.min_free_mb <= disk.warn_free_mb`, both ≥ 1 (`BadDiskThresholds`). - `web.session_ttl_hours` ≥ 1, `blocklist_update.interval_hours` ≥ 1. - `cfg.upstreams` must contain at least one entry with `enabled = true` (`NoUpstreams`) — PLAN §12.2. *Conditional* — `doh_server.enabled` or `dot_server.enabled` requires non-empty `cert_path` and `key_path` (`MissingCertPath`, `MissingKeyPath`); readability is `nxdns check`'s job, not the pure validator's. `logging.output == .file` requires a non-empty absolute `file_path` (`MissingLogPath`). `web.password != "" and web.password_hash != ""` → `PasswordAndHashBothSet`. `validate` never stops at the first problem. It runs every check, appends every failure, and only then returns `diags.problems.items[0].err`. On success it returns without touching `diags`. ### S2.7 Tests (in-file) - A default `Config` with one enabled upstream and a `default` group validates cleanly. - One named test per `ValidateError` member (except `OutOfMemory`), each asserting the exact error and that the diagnostic path names the offending element (e.g. `"clients[1].ip"`). - A config with **five** distinct problems yields five diagnostics and returns the first one's error. - Duplicate detection across canonical forms: `clients` holding both `"fd00::1"` and `"FD00:0:0:0:0:0:0:1"` is `DuplicateClientIp`. - Unknown group referenced from each of the four collections in turn. - Unknown source referenced from `group_sources`. - `toSettings`/`fromSettings` round-trip on a non-default `Config` reproduces every scalar field. - `toSettings` on a default `Config` produces exactly the literal expected key list. - An unknown settings key increments `unknown_keys` and does not error; a malformed integer value is `error.BadSettingValue`. - `LogLevel.err` encodes as `"error"` and decodes back. - Every `toDb`/`fromDb` enum pair round-trips over all tags. - The `parseResolver` table: `udp://192.168.1.1:53` ok; `tcp://[fd00::1]:53` ok; `https://x/` → error; `udp://host.name:53` → error (IP literal required); `udp://1.1.1.1` → error (port required); `udp://1.1.1.1:0` → error. ### S2.8 Acceptance criteria - [ ] `zig fmt --check` and `zig ast-check` clean on both files. - [ ] Neither file imports `std.Io` for anything but `std.Io.Writer` (diagnostics) and `std.Io.Duration` (the timeout conversions). No `Io` value is a parameter anywhere. - [ ] Neither file imports `storage/db.zig`. - [ ] Every bullet in S2.7 exists as a named test. - [ ] The comptime `assertFits` block is present and every conversion function is covered by it. --- ## Session S3: `src/storage/config_schema.zig`, `src/storage/migrations.zig`, `src/storage/querylog_schema.zig` ### S3.1 `config_schema.zig` Holds the DDL, verbatim from PLAN §11.2, as the first migration step: ```sql CREATE TABLE schema_version (version INTEGER NOT NULL); CREATE TABLE groups ( id INTEGER PRIMARY KEY, name TEXT NOT NULL UNIQUE, safe_search INTEGER NOT NULL DEFAULT 0 ); INSERT OR IGNORE INTO groups (id, name) VALUES (1, 'default'); CREATE TABLE clients ( id INTEGER PRIMARY KEY, ip TEXT NOT NULL UNIQUE, -- canonical text form (v4 dotted / v6 RFC 5952) name TEXT, group_id INTEGER NOT NULL REFERENCES groups(id), hand_edited INTEGER NOT NULL DEFAULT 0, first_seen INTEGER NOT NULL, last_seen INTEGER NOT NULL ); CREATE TABLE client_prefixes ( id INTEGER PRIMARY KEY, prefix TEXT NOT NULL UNIQUE, -- "192.168.1.0/24", "fd00:abcd::/48" group_id INTEGER NOT NULL REFERENCES groups(id) ON DELETE CASCADE, priority INTEGER NOT NULL DEFAULT 100 ); CREATE TABLE upstreams ( id INTEGER PRIMARY KEY, url TEXT NOT NULL UNIQUE, priority INTEGER NOT NULL DEFAULT 100, enabled INTEGER NOT NULL DEFAULT 1 ); CREATE TABLE blocklist_sources ( id INTEGER PRIMARY KEY, url TEXT NOT NULL UNIQUE, name TEXT NOT NULL, enabled INTEGER NOT NULL DEFAULT 1, is_suggested INTEGER NOT NULL DEFAULT 0, last_updated INTEGER, domain_count INTEGER NOT NULL DEFAULT 0, wildcard_count INTEGER NOT NULL DEFAULT 0, skipped_regex_count INTEGER NOT NULL DEFAULT 0, checksum TEXT ); CREATE TABLE group_sources ( group_id INTEGER NOT NULL REFERENCES groups(id) ON DELETE CASCADE, source_id INTEGER NOT NULL REFERENCES blocklist_sources(id) ON DELETE CASCADE, PRIMARY KEY (group_id, source_id) ); CREATE TABLE rules ( id INTEGER PRIMARY KEY, group_id INTEGER NOT NULL REFERENCES groups(id) ON DELETE CASCADE, pattern TEXT NOT NULL, kind TEXT NOT NULL CHECK(kind IN ('exact','wildcard')), action TEXT NOT NULL CHECK(action IN ('allow','block')), created_at INTEGER NOT NULL ); CREATE TABLE local_records ( id INTEGER PRIMARY KEY, name TEXT NOT NULL, rtype TEXT NOT NULL CHECK(rtype IN ('A','AAAA','CNAME')), value TEXT NOT NULL, ttl INTEGER NOT NULL DEFAULT 300, UNIQUE(name, rtype, value) ); CREATE TABLE forward_zones ( id INTEGER PRIMARY KEY, zone TEXT NOT NULL UNIQUE, resolver TEXT NOT NULL -- "udp://192.168.1.1:53" ); CREATE TABLE settings (key TEXT PRIMARY KEY, value TEXT NOT NULL); ``` Exported as `pub const ddl_v1: [:0]const u8 = @embedFile(...)` or a multiline string literal — either is fine, but the SQL text must be byte-identical to the block above. Also in this file, because they belong with the schema and every other session needs them: ```zig /// Child-before-parent. Used by import's wipe step; correct under `foreign_keys = ON`. /// `upstreams`, `local_records`, `forward_zones` and `settings` have no foreign keys, so their /// position is free; `groups` and `blocklist_sources` must come last, after every referrer. pub const delete_order = [_][]const u8{ "group_sources", "rules", "client_prefixes", "clients", "upstreams", "local_records", "forward_zones", "settings", "blocklist_sources", "groups", }; /// Every table whose emptiness defines "the database has never been configured" (S5.2). pub const content_tables = [_][]const u8{ "clients", "client_prefixes", "upstreams", "blocklist_sources", "group_sources", "rules", "local_records", "forward_zones", "settings", }; ``` Check both lists against the DDL before using them and report any correction in the completion report — a wrong delete order surfaces as a `Constraint` error inside import's transaction, which is exactly the failure mode this milestone is meant to make impossible. ### S3.2 `migrations.zig` ```zig pub const Step = struct { version: u32, sql: [:0]const u8 }; pub const steps = [_]Step{ .{ .version = 1, .sql = config_schema.ddl_v1 }, }; pub const target_version: u32 = steps[steps.len - 1].version; pub const Error = db.Error || error{ SchemaTooNew, SchemaCorrupt }; /// Reads the stamped version, applies every newer step in ONE transaction, stamps the result. /// `db` must already have had `applyPragmas` called (foreign_keys is a no-op inside a transaction). pub fn migrate(database: *db.Db) Error!u32; /// Seam for tests: same logic against an injected step list. pub fn migrateSteps(database: *db.Db, list: []const Step) Error!u32; ``` Algorithm: 1. Assert at comptime that `steps` versions are `1, 2, 3, …` with no gaps and strictly increasing. 2. Read the current version: `SELECT count(*) FROM sqlite_schema WHERE type='table' AND name='schema_version'`. Zero → current version is `0`. Otherwise `SELECT version FROM schema_version` — it must yield **exactly one row**; zero rows or more than one row is `error.SchemaCorrupt`, never a guess. 3. `current > target` → `error.SchemaTooNew`. Distinct from every other error so the CLI can print "config.db is at schema version N; this nxdns binary supports N-1. Install a newer nxdns." A database from the future is never silently accepted, and never downgraded. 4. `current == target` → return `current`. No transaction, no write. 5. `var tx = try Tx.begin(database); errdefer tx.rollback();` then, for each step with `version > current`, in ascending order, `database.exec(step.sql)`. Then `DELETE FROM schema_version;` and `INSERT INTO schema_version (version) VALUES (?)` with the target. Then `tx.commit()`. SQLite runs DDL transactionally, so a failing step leaves the file exactly as it was. 6. Return the new version. Reading the version happens **outside** the transaction in step 2 and is re-read **inside** it before applying, so two processes starting simultaneously cannot both apply step N (the second sees the stamped version under `BEGIN IMMEDIATE` and finds nothing to do). Tests (in-file, `:memory:`): - Fresh database → `migrate` returns `target_version`; every table in the DDL exists (assert by counting `sqlite_schema` rows of `type='table'` matching a literal name list); `groups` holds exactly one row, `id = 1`, `name = 'default'`. - `migrate` twice is idempotent: the second call writes nothing (`sqlite3_changes` after it, or a before/after comparison of `sqlite_schema`) and returns the same version. - A database stamped `target_version + 1` → `error.SchemaTooNew`, and the stamped version is unchanged afterwards. - `schema_version` with two rows → `error.SchemaCorrupt`. - Rollback proof through `migrateSteps`: a two-step list whose second step is invalid SQL leaves the stamped version at the value before the call and leaves the second step's table absent. - Stepwise upgrade through `migrateSteps`: apply a one-step list, then a two-step list with the same first step, and assert the second step's effect is present and the version is 2. This is the "upgrade = install binary, restart" proof PLAN §20.11 asks for. ### S3.3 `querylog_schema.zig` The DDL, verbatim from PLAN §11.3: ```sql CREATE TABLE domains ( id INTEGER PRIMARY KEY, domain TEXT NOT NULL UNIQUE ); CREATE TABLE query_log ( id INTEGER PRIMARY KEY, timestamp INTEGER NOT NULL, domain_id INTEGER NOT NULL REFERENCES domains(id), client_ip TEXT NOT NULL, -- text, not a FK: log rows are immutable facts qtype INTEGER, blocked INTEGER NOT NULL, block_reason TEXT, response_time_us INTEGER, cache_hit INTEGER, upstream TEXT ); CREATE INDEX idx_query_log_ts ON query_log(timestamp); CREATE INDEX idx_query_log_client ON query_log(client_ip); CREATE INDEX idx_query_log_domain ON query_log(domain_id); ``` The fingerprint is derived from the DDL text, not maintained by hand: ```zig pub const ddl: [:0]const u8 = ...; /// PRAGMA user_version is a signed 32-bit field. Deriving the fingerprint from the DDL means /// editing the schema automatically invalidates every existing file — which is exactly the /// policy (PLAN §3.7: querylog.db is never migrated). pub const fingerprint: i32 = @bitCast(std.hash.Crc32.hash(ddl)); ``` `std.hash.Crc32.hash(bytes) u32` verified at `lib/std/hash/crc/impl.zig:96` via `lib/std/hash.zig:10`. ```zig pub const RecreateReason = enum { missing, corrupt, not_a_database, quick_check_failed, fingerprint_mismatch }; pub const OpenResult = struct { database: db.Db, recreated: ?RecreateReason, // non-null feeds a counter and the /api/health rollup in Phase 8 }; pub const Error = db.Error || error{AsideNameCollision} || std.Io.Dir.RenameError || std.Io.Dir.DeleteFileError || std.Io.Dir.AccessError; /// Opens `/querylog.db`, recreating it if and only if it is genuinely unusable. pub fn open(io: std.Io, dir: std.Io.Dir, path: [:0]const u8) Error!OpenResult; ``` Open sequence: 1. `dir.access(io, path, .{})`. `error.FileNotFound` → create fresh (reason `.missing`, which is not an anomaly — do not log it as a warning on first start; log at `info`). 2. Open with `.read_write_existing`, `applyPragmas`. 3. `PRAGMA quick_check` — a single row whose text must be `"ok"`. Anything else → `.quick_check_failed`. `quick_check` rather than `integrity_check` because it skips the (expensive) index-vs-table cross-check while still catching structural damage, and a damaged index on an expendable log is not worth a multi-second startup scan. 4. `PRAGMA user_version` must equal `fingerprint`, else `.fingerprint_mismatch`. 5. Otherwise return the handle with `recreated = null`. **The recreate predicate is a whitelist, not a fallback.** Recreate only for: `error.Corrupt` → `.corrupt`; `error.NotADb` → `.not_a_database`; a failed `quick_check`; a fingerprint mismatch; a missing file. Every other error **propagates unchanged and the file is not touched**. Named explicitly, because each one has destroyed data in some other project: `error.Busy` and `error.Locked` (another process holds the write lock — waiting is right, deleting is catastrophic), `error.OutOfMemory` (this process's problem), `error.CantOpen` (usually a permissions or missing-directory problem — recreating would not help and would mask it), `error.ReadOnly`, `error.IoErr`, `error.Full`, `error.Perm`, `error.Auth`, `error.Canceled`. A test exists for `Busy`/`Locked`/`OutOfMemory` asserting the file's bytes are unchanged. Recreate sequence, in this order: 1. Close the handle (if one was opened) so SQLite checkpoints and drops `-wal`/`-shm` where it can. 2. Build the aside name `querylog.db.corrupt-` from `std.Io.Clock.real.now(io).toSeconds()`. Rename with **`dir.renamePreserve(path, dir, aside, io)`** (verified: `RENAME_NOREPLACE`, returns `error.PathAlreadyExists` when taken). On `error.PathAlreadyExists`, retry with `--1`, `-2`, … up to 100 attempts, then `error.AsideNameCollision`. The aside name must never overwrite a previously saved corrupt file — two recreates in the same second are not hypothetical on a boot loop. 3. `dir.deleteFile(io, "querylog.db-wal")` and `dir.deleteFile(io, "querylog.db-shm")`, tolerating **only** `error.FileNotFound`. This step is not optional: a stale WAL left beside the renamed database would be replayed into the freshly created file and corrupt it immediately. If either delete fails for any other reason, propagate — do not create the new database on top of a half-cleaned state. 4. Create fresh with `.read_write_create`, `applyPragmas`, `exec(ddl)`, and `PRAGMA user_version = ` inside one transaction. 5. `std.log.warn` with the reason and the aside path. `.missing` logs at `info` instead. Tests — these need real files, so they live in S7's integration file; S3 writes them there is **not** permitted (S7 owns that file). Instead S3 exposes the seams the tests need and lists the required cases in its completion report: - fresh directory → created, `recreated == .missing`, `user_version == fingerprint`; - reopen → `recreated == null`; - stamp a wrong `user_version` → recreated with `.fingerprint_mismatch`, aside file exists; - write garbage bytes over the file → recreated with `.not_a_database` or `.corrupt`; - two recreates within one second produce two distinct aside files; - a pre-existing stale `querylog.db-wal` is removed by the recreate. In-file tests S3 *can* write (`:memory:` only): `fingerprint` is stable across calls; `ddl` executes cleanly against a fresh memory database and creates `domains`, `query_log` and the three indexes. ### S3.4 Acceptance criteria - [ ] `zig fmt --check` and `zig ast-check` clean on all three files. - [ ] The DDL in `config_schema.zig` is byte-identical to PLAN §11.2; the DDL in `querylog_schema.zig` is byte-identical to PLAN §11.3. - [ ] `migrate` applies everything in one transaction and refuses a newer database with `error.SchemaTooNew`. - [ ] Every `migrations.zig` test bullet exists as a named test. - [ ] The recreate predicate is a positive whitelist; `Busy`, `Locked` and `OutOfMemory` propagate. - [ ] `renamePreserve` is used for the aside, with uniquifying retries. - [ ] `-wal` and `-shm` removal precedes creating the replacement. --- ## Session S4: `src/storage/repositories/*.zig` Seven files, one per PLAN §5 name that is in scope. `queries_repo.zig` is Phase 6 and is **not** created here. | File | Tables | |---|---| | `groups_repo.zig` | `groups`, `group_sources` | | `clients_repo.zig` | `clients`, `client_prefixes` | | `upstreams_repo.zig` | `upstreams` | | `sources_repo.zig` | `blocklist_sources` | | `rules_repo.zig` | `rules` | | `local_repo.zig` | `local_records`, `forward_zones` | | `settings_repo.zig` | `settings` | ### S4.1 Uniform API Each repository exposes, for each of its tables: ```zig pub fn listX(database: *db.Db, gpa: std.mem.Allocator) db.Error!std.ArrayList(model.X); pub fn freeX(gpa: std.mem.Allocator, items: []const model.X) void; pub fn insertX(database: *db.Db, item: model.X, ctx: InsertContext) db.Error!void; pub fn deleteAllX(database: *db.Db) db.Error!void; pub fn countX(database: *db.Db) db.Error!i64; ``` `InsertContext` carries what the model deliberately omits: `now: i64` (for `first_seen`, `last_seen`, `created_at`) and the id lookups (`group_ids: *const std.StringHashMapUnmanaged(i64)`, `source_ids: …`) that turn a name reference into the `group_id`/`source_id` column. Building those maps is the caller's job (S5), because only the caller knows the ids it just inserted. Only these five operations exist. Update-by-id, delete-by-id and paged reads are Phase 8's REST surface; adding them now would be untested, unused generality (AGENTS.md). ### S4.2 Ordering Every `list` uses an `ORDER BY` whose trailing columns are unique, so export is byte-stable: | List | `ORDER BY` | |---|---| | groups | `name` | | group_sources (joined to names) | `g.name, s.url` | | clients | `ip` | | client_prefixes | `prefix` | | upstreams | `priority, url` | | blocklist_sources | `url` | | rules | `group_id, kind, action, pattern, id` | | local_records | `name, rtype, value` | | forward_zones | `zone` | | settings | `key` | `rules` has no unique tuple, hence the trailing `id`. `upstreams` sorts by `priority` first because that is the operationally meaningful order (it matches `Pool.init`'s expectation), and `url` breaks ties uniquely. `group_sources` and the `group`-referencing collections are read through a join so the list yields model values holding **names**, not ids: ```sql SELECT g.name, s.url FROM group_sources gs JOIN groups g ON g.id = gs.group_id JOIN blocklist_sources s ON s.id = gs.source_id ORDER BY g.name, s.url ``` `clients` list applies `WHERE hand_edited = 1` (S2.2's rationale: auto-materialized clients are runtime state and must not appear in an export). `countClients` counts **all** rows — the emptiness predicate in S5 needs the true count. ### S4.3 Memory discipline - Returned strings are heap-owned copies via `Stmt.columnTextAlloc`. `columnText` borrows from SQLite and is invalidated by the next `step`; a repository that returns a borrowed slice is a use-after-free waiting for the second row. - Every `list` builds into a `std.ArrayList(T)` with an `errdefer` that frees **both** every element already appended and every string of the partially-built element: ```zig var out: std.ArrayList(model.Group) = .empty; errdefer freeGroups(gpa, out.items); errdefer out.deinit(gpa); while (try stmt.step()) { const name = try stmt.columnTextAlloc(gpa, 0); errdefer gpa.free(name); try out.append(gpa, .{ .name = name, .safe_search = stmt.columnBool(1) }); } ``` - `freeX` frees every heap string in every element and is idempotent against an empty slice. - The callers of `list` may pass an arena; `freeX` must still be correct against a general-purpose allocator, because the tests use `std.testing.allocator`. ### S4.4 Tests (in-file, `:memory:`) Each repository gets, at minimum: - Round trip: `migrate` a memory database, insert three rows, `list` them, assert the exact order produced by the specified `ORDER BY`, assert every field survived. - `deleteAll` empties the table and `count` reflects it. - A leak-safety test through `std.testing.checkAllAllocationFailures(std.testing.allocator, testListImpl, .{})`, where `testListImpl` seeds rows and calls `list` + `free`. This is the runnable form of "leak-safe error paths"; a missing `errdefer` fails it. - `clients_repo`: a `hand_edited = 0` row is absent from `listClients` but counted by `countClients`. - `settings_repo`: keys sort ascending; a value containing a `'` round-trips (proving parameter binding, not string concatenation, is used). - `groups_repo`: `listGroupSources` yields names, not ids, and rejects nothing — the FK guarantees the join is total. ### S4.5 Acceptance criteria - [ ] `zig fmt --check` and `zig ast-check` clean on all seven files. - [ ] Every list statement's `ORDER BY` matches the table in S4.2 exactly. - [ ] No repository returns a slice borrowed from SQLite. - [ ] Every repository has a passing `checkAllAllocationFailures` test. - [ ] No SQL is built by string concatenation of a value; every value is bound. - [ ] No update-by-id / delete-by-id / pagination functions exist. --- ## Session S5: `src/config/bootstrap.zig`, `src/config/import.zig`, `src/config/export.zig` ### S5.1 `std.zon` ownership — verified verdict, and the rule that follows **Verified in `/home/mokhtar/app/zig/lib/std/zon/parse.zig` at tag 0.16.0:** `Parser.parseStruct` fills absent fields from the struct's default value by copying the default straight through (`parse.zig:874–888`): ```zig inline for (field_found, 0..) |found, i| { if (!found) { const field_info = field_infos[i]; if (field_info.default_value_ptr) |default| { const typed: *const field_info.type = @ptrCast(@alignCast(default)); @field(result, field_info.name) = typed.*; } else { ... } } } ``` For a `[]const u8` field with a default like `"0.0.0.0"`, the resulting slice points into the binary's read-only data. `std.zon.parse.free` (`parse.zig:412–456`) recurses over **every** field of a struct with no record of which were parsed and which were defaulted, and its slice arm is `for (value) |item| free(gpa, item); gpa.free(value);`. `std.mem.Allocator.free` (`lib/std/mem/Allocator.zig`) returns early only for zero length; for a non-empty slice it does `@memset(bytes, undefined)` and then `rawFree`. Applied to a defaulted string that lives in rodata, that is a write to read-only memory followed by a free of a pointer the allocator never owned. **Verdict: calling `std.zon.parse.free` on a parsed `Config` with defaulted string fields is a crash, and the nxdns config model has many non-empty string defaults.** (The `free_on_error` path at `parse.zig:830` is *not* affected — its `errdefer` walks only `fields.names[0..initialized]`, the fields actually present in the source. The hazard is exclusively in the caller-invoked `free`.) **Rule, binding on this milestone and every later one:** parse into an `std.heap.ArenaAllocator`, free the arena, **never** call `std.zon.parse.free` on a `Config`. A comment stating this and citing `parse.zig:874` goes at the parse site in `import.zig`. A test asserts the shape of the rule by parsing a source that omits every optional field and then destroying the arena — under `std.testing.allocator` a stray `parse.free` would fail the test. ### S5.2 the emptiness predicate Shared by `bootstrap` and `import --force`-less mode; lives in `import.zig` and is exported: ```zig /// A database is "never configured" when migrations have run and nothing else has. /// Migrations themselves create `schema_version` and seed `groups(1,'default')`, so /// "no rows anywhere" is the wrong test. pub fn isEmpty(database: *db.Db) db.Error!bool; ``` True iff **all** of the following hold: 1. Every table in `config_schema.content_tables` has `count(*) = 0`. (That is `clients`, `client_prefixes`, `upstreams`, `blocklist_sources`, `group_sources`, `rules`, `local_records`, `forward_zones`, `settings`.) 2. `SELECT count(*) FROM groups` is exactly 1. 3. That row satisfies `id = 1 AND name = 'default' AND safe_search = 0`. Any deviation makes the database non-empty. Note that `countClients` here counts every row including auto-materialized ones — a server that has answered one query is configured enough that a bootstrap file must not overwrite it. ### S5.3 `import.zig` ```zig pub const Options = struct { force: bool = false }; pub const Error = db.Error || validate.ValidateError || std.Io.Dir.ReadFileAllocError || error{ DatabaseNotEmpty, ConfigTooLarge, ParseZon, PasswordAndHashBothSet }; pub const max_config_bytes = 4 * 1024 * 1024; /// Reads, parses, validates, then replaces the database contents. Nothing is written to the /// database and no file is created until every check has passed. pub fn importFile( io: std.Io, gpa: std.mem.Allocator, database: *db.Db, dir: std.Io.Dir, path: []const u8, options: Options, diags: *validate.Diagnostics, ) Error!void; /// The half that bootstrap reuses: an already-parsed, already-validated config into the database, /// all or nothing. `now` is the caller's timestamp for the runtime columns. pub fn applyToDb( io: std.Io, gpa: std.mem.Allocator, database: *db.Db, cfg: model.Config, now: i64, options: Options, ) Error!void; ``` `importFile` order — the order is the specification: 1. `dir.readFileAllocOptions(io, path, gpa, .limited(max_config_bytes), .of(u8), 0)` → a `[:0]u8`. `std.zon.parse` requires the sentinel; `readFileAlloc` cannot supply one. `error.StreamTooLong` maps to `error.ConfigTooLarge`. 2. `var arena = std.heap.ArenaAllocator.init(gpa); defer arena.deinit();` then `std.zon.parse.fromSliceAlloc(model.Config, arena.allocator(), source, &zon_diag, .{})`. Never `parse.free`. `zon_diag` is a `std.zon.parse.Diagnostics`; on `error.ParseZon` its `{f}` rendering goes into `diags` verbatim so the operator sees line and column. 3. `validate.validate(cfg, diags)` — every problem collected before returning. 4. `applyToDb`. `applyToDb`: 1. `var tx = try db.Tx.begin(database); errdefer tx.rollback();` 2. **Inside** the transaction, `if (!options.force and !try isEmpty(database)) return error.DatabaseNotEmpty;`. Checking before `BEGIN IMMEDIATE` would be a TOCTOU window against a concurrently starting process; `BEGIN IMMEDIATE` already holds the write lock, so the check and the writes are one atomic unit. 3. Delete every table in `config_schema.delete_order` (children first, so `foreign_keys = ON` never fires). 4. Insert `groups`, `"default"` **first and with an explicit `id = 1`**, then the rest in list order. §11.2 seeds group 1 as `default` and §7.2's fallback assignment depends on it; letting an import renumber it would silently move every unassigned client. Build the `name → id` map from `sqlite3_last_insert_rowid` as you go. 5. Insert `blocklist_sources`, building the `url → id` map. 6. Insert `clients` (`hand_edited = 1`, `first_seen = last_seen = now`), `client_prefixes`, `upstreams`, `group_sources`, `rules` (`created_at = now`), `local_records`, `forward_zones` — each resolving names through the maps built above. 7. Resolve the password per S2.5 (`strHash` when `password != ""`), then `toSettings` and insert every pair. `web.password` is never a settings row. 8. `tx.commit()`. No filesystem write happens anywhere in `applyToDb`, and `importFile` opens the input read-only. The only file this milestone creates outside the data directory is `export --out`. ### S5.4 `export.zig` ```zig /// Reads every repository into `arena` and assembles a Config. Deterministic by construction: /// every list is ordered per S4.2 and every scalar comes from the settings map. pub fn readConfig(database: *db.Db, arena: std.mem.Allocator) Error!model.Config; /// Canonical ZON. Deterministic for a given Config — no timestamps, no host names, no counters. pub fn writeConfig(cfg: model.Config, w: *std.Io.Writer) std.Io.Writer.Error!void; /// Atomic, owner-only. The exported file contains `web.password_hash`, so 0600 is not optional. pub fn writeToFile(io: std.Io, gpa: std.mem.Allocator, database: *db.Db, dir: std.Io.Dir, path: []const u8) Error!void; pub fn writeToWriter(gpa: std.mem.Allocator, database: *db.Db, w: *std.Io.Writer) Error!void; ``` `writeConfig` emits a fixed two-line header comment and then the value: ``` // nxdns configuration // generated by `nxdns export` — the database is the source of truth ``` The header carries **no** timestamp, version or hostname. Any of those would break the byte-stable round trip and would turn a config diff into noise. ZON permits comments, and `std.zon.parse` reads through them. Serialization: `std.zon.stringify.serialize(cfg, .{ .whitespace = true, .emit_default_optional_fields = true }, w)`. Emitting defaults explicitly makes the exported file a complete, self-documenting record of the running configuration and makes the round trip independent of a later change to a default value. `writeToFile` uses the verified atomic helper: ```zig var af = try dir.createFileAtomic(io, path, .{ .permissions = .fromMode(0o600), .replace = true }); defer af.deinit(io); var fw = af.file.writer(io, &buf); try writeConfig(cfg, &fw.interface); try fw.interface.flush(); try af.file.sync(io); try af.replace(io); ``` The session must read `Io/Threaded.zig`'s `dirCreateFileAtomic` implementation and **confirm that the temporary file is created in the destination's directory** — a temporary on another filesystem cannot be renamed atomically into place. Record the finding in the completion report; if it is not, fall back to an explicit temp-plus-`rename` in the same directory (`rename` replaces, verified Dir.zig:1085) and say so. ### S5.5 `bootstrap.zig` ```zig pub const Outcome = enum { seeded, db_already_configured, no_config_file }; /// First-start seeding (PLAN §3.5). Called by `nxdns run` before serving. pub fn bootstrap( io: std.Io, gpa: std.mem.Allocator, database: *db.Db, dir: std.Io.Dir, config_path: []const u8, diags: *validate.Diagnostics, ) Error!Outcome; ``` 1. `dir.access(io, config_path, .{})` → `error.FileNotFound` returns `.no_config_file`. An absent bootstrap file is the normal steady state, not a problem: log at `info` and continue. Any other access error propagates. 2. `if (!try import.isEmpty(database)) return .db_already_configured;` — log at `info` ("configuration file ignored; the database is already configured"), do not read the file. PLAN §3.5: subsequent starts ignore the file. 3. Otherwise `import.importFile(..., .{ .force = false }, diags)`. A file that is present but unreadable, unparseable, or invalid is an **error** — the operator wrote it, meant it, and starting with silent defaults instead would be the exact silent-failure mode PLAN §1.3 exists to prevent. Return `.seeded` on success. There is no second copy of the seeding logic: bootstrap is a policy wrapper over `importFile`. ### S5.6 Tests In-file (`:memory:`, no filesystem): - `isEmpty` on a freshly migrated database is true; after one `settings` row it is false; after one auto-materialized `clients` row (`hand_edited = 0`) it is false; after renaming group 1 it is false. - `applyToDb` on a non-empty database without `force` returns `error.DatabaseNotEmpty` and leaves every row untouched (compare a full dump before and after). - `applyToDb` with a config whose insert fails mid-way (inject via a group name exceeding no constraint but a `local_records` row duplicating an earlier one — validation would catch it, so instead call `applyToDb` directly with an invalid config that bypasses `validate`) leaves the database exactly as it was. This is the all-or-nothing proof and it must not go through `importFile`. - `readConfig` → `writeConfig` → `parse` → `applyToDb` → `readConfig` produces an equal `Config`. - **Byte-stable round trip**: seed a memory database, `writeToWriter` into buffer A, create a second memory database, import buffer A, `writeToWriter` into buffer B, `try std.testing.expectEqualStrings(A, B)`. This is PLAN §20.8 as a runnable check. - Parse a ZON source omitting every optional field into an arena, destroy the arena, and assert no leak under `std.testing.allocator` (the S5.1 rule, as a test). - `password` set → the stored `web.password_hash` starts with `"$argon2id$"` and `settings` has no `web.password` key; `password` and `password_hash` both set → `error.PasswordAndHashBothSet`. - `import` of a config with a validation error writes nothing: assert `isEmpty` still true. Filesystem cases (`writeToFile` permissions and atomicity, `bootstrap`'s three outcomes against real files) belong to S7. ### S5.7 Acceptance criteria - [ ] `zig fmt --check` and `zig ast-check` clean on all three files. - [ ] `std.zon.parse.free` appears nowhere in the repository (`grep` it). - [ ] The arena rule is documented at the parse site with the `parse.zig:874` citation. - [ ] The non-empty check is inside the transaction. - [ ] Validation completes before the first `Tx.begin` and before any file is created. - [ ] The byte-stable round-trip test passes. - [ ] `bootstrap` has one code path into the database, and it is `importFile`. --- ## Session S6: `src/cli.zig`, `src/main.zig` `main.zig` becomes a thin shell: parse arguments, dispatch, return an exit code. Everything testable lives in `cli.zig`. ### S6.1 argument parsing (pure) ```zig pub const Command = union(enum) { run: Paths, check: CheckArgs, export_: ExportArgs, import_: ImportArgs, version, help, }; pub const Paths = struct { data_dir: []const u8 = "/var/lib/nxdns", // PLAN §3.13 config: []const u8 = "/etc/nxdns/config.zon", }; pub const CheckArgs = struct { paths: Paths = .{}, config_explicit: bool = false }; pub const ExportArgs = struct { paths: Paths = .{}, out: ?[]const u8 = null }; pub const ImportArgs = struct { paths: Paths = .{}, file: []const u8, force: bool = false }; pub const ParseError = error{ UnknownCommand, UnknownFlag, MissingValue, MissingArgument, TooManyArguments }; /// `argv` excludes the program name. Slices borrow from `argv`. pub fn parseArgs(argv: []const []const u8) ParseError!Command; ``` Accepted forms: ``` nxdns run [--data-dir DIR] [--config FILE] nxdns check [--data-dir DIR] [--config FILE] nxdns export [--data-dir DIR] [--out FILE] nxdns import FILE [--force] [--data-dir DIR] nxdns version nxdns help | --help | -h ``` `--flag=value` and `--flag value` both work. `config_explicit` records whether `--config` was given, because `check` needs to distinguish "the operator named a file" from "the default path happens to exist" (S6.4). Unknown flags and missing values are `ParseError`, not silent ignores. Exit codes: `0` success, `1` a runtime failure (I/O, database), `2` a validation or check failure, `64` a usage error (matching the existing milestone-1 convention). ### S6.2 path assembly ```zig pub const DataDir = struct { dir: std.Io.Dir, config_db_path: [:0]const u8, // "/config.db" querylog_db_path: [:0]const u8, // "/querylog.db" /// Creates `` (and parents) with mode 0700 if absent, then opens it. pub fn open(io: std.Io, gpa: std.mem.Allocator, data_dir: []const u8, create: bool) !DataDir; pub fn close(self: *DataDir, io: std.Io, gpa: std.mem.Allocator) void; }; ``` - Directory creation uses `Dir.cwd().createDirPathStatus(io, data_dir, .fromMode(0o700))`. Plain `createDirPath` hardcodes `0o777` (verified Dir.zig:843) and must not be used here — the data directory holds `config.db`, which holds `web.password_hash`. - `config.db` must end up mode 0600. SQLite creates it itself, at `0644 & ~umask`. The order matters: 1. `db.open(config_db_path, .{ .mode = .read_write_create })` — this creates the file. 2. `dir.setFilePermissions(io, "config.db", .fromMode(0o600), .{})`. 3. `applyPragmas` — this is what creates `config.db-wal` and `config.db-shm`, and SQLite copies the **main database file's** permissions onto them. Setting 0600 first therefore gets the sidecars for free; doing it in the other order leaves them world-readable. Encode this ordering in `DataDir.open` with a comment stating the reason, and cover it in S7. - `export --out` is written relative to the current working directory (an operator running `nxdns export --out backup.zon` means the shell's directory), not the data directory. ### S6.3 entry functions ```zig pub const Runner = struct { io: std.Io, gpa: std.mem.Allocator, out: *std.Io.Writer, err: *std.Io.Writer }; pub fn runCheck(r: Runner, args: CheckArgs, probe: bool) u8; pub fn runExport(r: Runner, args: ExportArgs) u8; pub fn runImport(r: Runner, args: ImportArgs) u8; pub fn runVersion(r: Runner) u8; pub fn usage(w: *std.Io.Writer) void; ``` Every one takes its writers as parameters so tests can capture output into `std.Io.Writer.Allocating`. `main.zig` builds the `Runner` from `std.process.Init` (`init.io`, `init.gpa`, `std.Io.File.stdout()/stderr()`), calls `parseArgs`, dispatches, and returns the code. `main.zig` contains no logic beyond that — no database calls, no formatting decisions. `run` prints `not implemented` and returns 2, unchanged from milestone 1. Wiring the server to the configuration is Phase 7. ### S6.4 `nxdns check` The point of `check` is that an operator sees **every** problem in one run. It never returns early. 1. Choose what to check: - `--config FILE` given → validate that file (parse it into an arena, then `validate`). - otherwise, if `/config.db` exists → migrate it (a `check` on a database a version behind should still work), `export.readConfig`, then `validate`. - otherwise, if the default `/etc/nxdns/config.zon` exists → validate that file. - otherwise → print "nothing to check: no config.db in and no " and return 2. Print which source was used, so the answer is never ambiguous. 2. Run `validate` and print **every** diagnostic through `Diagnostics.writeAll`, one `path: message` line each. Do not stop at the first. 3. Certificates: for each of `doh_server` and `dot_server` that is `enabled`, `dir.access(io, cert_path, .{})` and the same for `key_path`; print a `FAIL` line per unreadable path. Then `dir.statFile(io, key_path, .{})` and, if `stat.permissions.toMode() & 0o077 != 0`, print a `WARN` line — PLAN §19 requires TLS keys readable by the service user only. A warning does not change the exit code; an unreadable file does. 4. Upstream probe (only when `probe` is true — `main` passes true, unit tests pass false): for **each** enabled upstream, build its `transport.Endpoint`, its `DohClient` or `DotClient`, and a `pool.Pool` with that **single** entry, then `exchange` a hand-built `A example.com` query with `attempt_timeout` from `model.totalTimeout(cfg.upstream)`. One `Pool` per upstream rather than one pool over all of them, because the pool's job is failover — it would report success as soon as any upstream answered, and a broken upstream would stay invisible. Reusing the pool (rather than calling the client directly) keeps the deadline, cancellation and health machinery identical to what the server will do. Read `Pool.snapshot` for the error text and print one `OK`/`FAIL` line per upstream with the endpoint URL and, on failure, `snapshot.last_error`. 5. Exit `0` if there were no `FAIL` lines and validation passed, else `2`. Warnings alone keep `0`. The probe leaves the machine, so it is `-Dlive` territory: `runCheck(r, args, false)` is what the default test suite exercises, and the probing path gets one `-Dlive` test in S7. ### S6.5 Tests (in-file, pure) - `parseArgs` table: every accepted form above; `--data-dir` with and without `=`; `import` without a file → `MissingArgument`; `--out` without a value → `MissingValue`; `--nope` → `UnknownFlag`; `nxdns frobnicate` → `UnknownCommand`; `export extra` → `TooManyArguments`. - `usage` writes non-empty text. - `runCheck` with `probe = false` against an in-memory-backed config: a clean config returns 0 and a config with three problems returns 2 and prints three lines (assert on captured output). `runExport` / `runImport` need real files; those tests live in S7. ### S6.6 Acceptance criteria - [ ] `zig fmt --check` and `zig ast-check` clean on both files. - [ ] `main.zig` contains argument dispatch and nothing else; every command body is in `cli.zig` and takes its writers as parameters. - [ ] `nxdns version` behaviour is unchanged from milestone 1. - [ ] `check` prints every diagnostic before exiting. - [ ] `check` builds one `Pool` per upstream. - [ ] The `config.db` permission ordering (create → chmod 0600 → WAL) is implemented and commented. - [ ] Every `parseArgs` bullet exists as a named test. --- ## Session S7: `src/storage/storage_integration_test.zig` One file, everything real: real temporary directories, real database files, real exports. Guarded by `if (!build_options.integration) return error.SkipZigTest;` at the top of every test, and one `build_options.live` test for the upstream probe. Use `std.testing.tmpDir(.{})` for isolation — verified `pub fn tmpDir(opts: Io.Dir.OpenOptions) TmpDir` (`lib/std/testing.zig:634`). It uses `std.testing`'s own `io`, so the tests must drive the code under test with the same `Io` instance the returned `TmpDir` was created against; read `lib/std/testing.zig` around line 634 for how that `io` is obtained and report it. Capture command output with `std.Io.Writer.Allocating` (`lib/std/Io/Writer.zig:2502`). ### S7.1 Required cases *querylog recreate policy* (the S3.3 list, which S3 could not write): 1. Fresh directory → `open` creates the file, `recreated == .missing`, `PRAGMA user_version` equals `fingerprint`, `domains` and `query_log` exist. 2. Reopen the same file → `recreated == null`, no aside file was created. 3. Stamp a wrong `user_version`, reopen → `recreated == .fingerprint_mismatch`, exactly one `querylog.db.corrupt-*` file exists, and its bytes equal the original file's bytes. 4. Overwrite the file with 4096 bytes of garbage, reopen → recreated with `.not_a_database` or `.corrupt`; the garbage is preserved in the aside file. 5. Two recreates without an intervening second produce **two** aside files with distinct names (drive it by recreating twice in a loop; the uniquifier, not the clock, must make them distinct). 6. Create a stale `querylog.db-wal` beside a fingerprint-mismatched database; after the recreate, the stale `-wal` is gone. 7. A `querylog.db` whose file permissions deny reading (`0o000`) → the error propagates and the file still exists with its original bytes. This is the "never destroy a healthy file" guarantee. *config.db and permissions*: 8. `DataDir.open` on a fresh path creates the directory with mode `0o700` (assert via `statFile` → `permissions.toMode() & 0o777 == 0o700`). 9. After `DataDir.open` + `migrate`, `config.db` is `0o600` and `config.db-wal` (if present) is `0o600`. 10. `migrate` on a database stamped one version ahead → `error.SchemaTooNew`; the file is unchanged. *export / import / bootstrap*: 11. Seed a database, `export.writeToFile`, assert the output file is mode `0o600` and its first line is the header comment. 12. Byte-stable round trip **through real files**: export to `a.zon`, import into a second data directory, export to `b.zon`, `expectEqualStrings` on the two file contents. 13. `import` into a non-empty database without `--force` → `error.DatabaseNotEmpty` and the existing rows are unchanged; with `--force` → replaced. 14. `import` of a file with two validation errors → both diagnostics are produced, the database is still empty, and no file was created. 15. `bootstrap` with no config file → `.no_config_file`, database still empty. 16. `bootstrap` with a valid config file on an empty database → `.seeded`, and the rows match. 17. `bootstrap` on an already-configured database → `.db_already_configured` and the file is not even read (prove it by making the file invalid ZON: the call must still succeed). 18. `bootstrap` with a present-but-invalid config file on an empty database → an error, and the database is still empty. 19. `export.writeToFile` over an existing file replaces it atomically: assert the old content is gone and the mode is still `0o600`. *CLI end to end*: 20. `cli.runImport` then `cli.runExport` through the public entry functions, capturing stdout, reproducing case 12 at the CLI level and asserting the exit codes are 0. 21. `cli.runCheck` with `probe = false` against a seeded data directory returns 0; against a database seeded with a config carrying two problems returns 2 with two diagnostic lines. *live*: 22. Guarded by `build_options.live`: `cli.runCheck` with `probe = true` against a config naming `https://cloudflare-dns.com/dns-query` prints an `OK` line. A live failure is an environment finding, not a gate (milestone 1 and 3 convention). ### S7.2 Acceptance criteria - [ ] `zig fmt --check` and `zig ast-check` clean. - [ ] All 21 hermetic cases exist as named tests and pass under `zig build test -Dintegration`. - [ ] No test leaves a file behind outside its `tmpDir`. - [ ] No hermetic test opens a socket or resolves a name. --- ## Module Layout ``` src/storage/db.zig S1 sqlite3 wrapper: errors, Stmt, Tx, pragmas src/storage/config_schema.zig S3 PLAN §11.2 DDL, delete order, content tables src/storage/migrations.zig S3 numbered steps, one-transaction runner src/storage/querylog_schema.zig S3 PLAN §11.3 DDL, fingerprint, recreate policy src/storage/repositories/groups_repo.zig S4 groups, group_sources src/storage/repositories/clients_repo.zig S4 clients, client_prefixes src/storage/repositories/upstreams_repo.zig S4 upstreams src/storage/repositories/sources_repo.zig S4 blocklist_sources src/storage/repositories/rules_repo.zig S4 rules src/storage/repositories/local_repo.zig S4 local_records, forward_zones src/storage/repositories/settings_repo.zig S4 settings src/storage/storage_integration_test.zig S7 -Dintegration, real files src/config/model.zig S2 Config, settings mapping, unit conversions src/config/validate.zig S2 pure validator + Diagnostics + parseResolver src/config/bootstrap.zig S5 first-start seeding src/config/import.zig S5 read → parse → validate → replace src/config/export.zig S5 DB → canonical ZON, atomic 0600 write src/cli.zig S6 parseArgs, DataDir, runCheck/runExport/runImport src/main.zig S6 thin dispatch shell (rewritten) ``` ## File Ownership | Files | Owner | Notes | |---|---|---| | `src/storage/db.zig` | S1 | frozen after S1 verifies | | `src/config/model.zig`, `src/config/validate.zig` | S2 | frozen after S2 verifies | | `src/storage/config_schema.zig`, `src/storage/migrations.zig`, `src/storage/querylog_schema.zig` | S3 | | | `src/storage/repositories/*.zig` (7 files) | S4 | | | `src/config/bootstrap.zig`, `src/config/import.zig`, `src/config/export.zig` | S5 | | | `src/cli.zig`, `src/main.zig` | S6 | `main.zig` is rewritten; milestone-1 CLI behaviour preserved | | `src/storage/storage_integration_test.zig` | S7 | | | `build.zig`, `build.zig.zon`, `src/tests.zig` | orchestrator | no session edits these | | `PLAN.md` §12.1 | orchestrator | amended per S2.1 after S2 verifies | No session touches milestone 1, 2 or 3 files. A needed change there is reported, not made. In particular `src/upstream/*` and `src/server/*` are read-only in this milestone. ## Acceptance Criteria (Milestone 4 Complete) - [ ] `zig build test` exits 0 with every new file wired into `src/tests.zig`. - [ ] `zig build test -Dintegration` exits 0: milestone 1's loopback TLS echo, milestone 3's listener and resolver tests, and all 21 hermetic storage cases pass. - [ ] `zig build test -Dintegration -Dlive` exits 0 locally, or live failures are reported as environment findings with the exact error. - [ ] `zig build cross` still produces two statically linked executables. - [ ] `grep -rn "std.zon.parse.free" src/` returns nothing. - [ ] `grep -rn "@cImport" src/` returns nothing. - [ ] Byte-stable round trip: `nxdns import a.zon && nxdns export --out b.zon` on a fresh data directory, then `cmp a.zon b.zon` after the first export — expressed as test case 12 and reproducible by hand with: ``` ./zig-out/bin/nxdns --help >/dev/null ./zig-out/bin/nxdns import --data-dir /tmp/nx1 fixture.zon ./zig-out/bin/nxdns export --data-dir /tmp/nx1 --out /tmp/a.zon ./zig-out/bin/nxdns import --data-dir /tmp/nx2 /tmp/a.zon ./zig-out/bin/nxdns export --data-dir /tmp/nx2 --out /tmp/b.zon cmp /tmp/a.zon /tmp/b.zon ``` - [ ] `stat -c %a /tmp/nx1` reports `700`; `stat -c %a /tmp/nx1/config.db` and `/tmp/a.zon` report `600`. - [ ] `nxdns check --config ` prints 3 diagnostic lines and exits 2. - [ ] `nxdns import` of an invalid file leaves the database empty (`sqlite3 config.db "select count(*) from settings"` is 0 — or the equivalent assertion in test case 14, since the repo ships no sqlite3 CLI). - [ ] A `config.db` stamped one version ahead makes startup fail with the "schema too new" message, not a crash and not a silent downgrade. - [ ] `zig fmt --check` clean repo-wide; GPG-signed lowercase commits. ## Anti-Requirements - **No web server, no REST API, no SSE, no `/metrics`, no auth flow.** Phase 8. The argon2id hash is computed and stored; nothing verifies it yet. - **No blocklist downloading, parsing, or compiling.** Phase 5. `blocklist_sources` rows are metadata; no `.list`/`.wild` file is created, no HTTP fetch happens. - **No filtering, rule matching, wildcard matching, safe-search, CNAME uncloaking.** Phase 5. The validator checks rule *syntax* only. - **No query-log writing.** Phase 6. `querylog.db` is created and its schema is verified; nothing inserts a row, there is no `queries_repo.zig`, no buffer, no flush loop, no retention job, no VACUUM, no disk monitor. - **No DNS serving changes.** `src/dns/`, `src/server/`, `src/upstream/` are untouched. `nxdns run` still prints `not implemented`; wiring the config into the servers is Phase 7. - **No local records or conditional forwarding behaviour.** Phase 5. Their rows are stored and validated, nothing consumes them. - **No prepared-statement cache, no connection pool, no ORM, no query builder.** One handle per database, statements prepared where used. - **No `querylog.db` migrations, ever.** That is the design (PLAN §3.7), not a gap. - **No CRUD beyond list/insert/deleteAll/count** in the repositories. - **No config file watcher, no auto-regeneration, no reload signal.** PLAN §3.5. - **No third-party Zig packages.** stdlib plus the two pinned C libraries. ## As built The implementation matches the spec with these evaluation- and review-driven refinements (three review rounds; findings went 8 → 3 → 1 low): - **Logging policy (binding, repo-wide):** a condition that is returned as a typed error logs at `warn` at most; `err` is reserved for failures the code swallows (the logged-then-discarded ROLLBACK failure). The zig test runner fails any test that emits `err` logs, and the negative tests exercise these paths. - `db.zig`: `c.Destructor` is `?*anyopaque`, not a fn-pointer type — the `SQLITE_TRANSIENT` sentinel (-1) is not a valid function address and aarch64 fn pointers require alignment, so the fn-pointer form fails `zig build cross`. `queryInt` now enforces its contract: exactly one column, integer-typed, exactly one row. - `querylog_schema.zig`: the DDL fingerprint's comptime CRC needs `@setEvalBranchQuota(2_000_000)` (the stdlib lookup-table generation runs under the caller's quota). Behavior-level integration tests prove `open()` preserves the file under `Busy`/`Locked` (a concurrent exclusive transaction) and only whitelisted corruption recreates. - `rules_repo.zig`: rule listing orders by group NAME (subselect), not `group_id` — ids permute across import into a fresh database and would break the byte-stable round trip. - `validate.zig`: one shared `parseAuthority` (strict brackets, no userinfo/query/fragment, printable-ASCII-only, port 1–65535) backs both the source-URL check and `parseResolver`. `Problem.err` is `ProblemError` (`ValidateError || error{ParseZon}`) so import's ZON line/column diagnostics travel the same channel the CLI renders; `validate` returns the first problem it recorded itself. - `cli.zig`: certificate/key checks do a real readability probe, not an existence check; an OutOfMemory during diagnostics recording takes the runtime exit code, never the config-problem exit; `help` rejects trailing arguments like every other command. - **Discovered stdlib limitation** (recorded in specs/research/zig-0.16-api-notes.md): `Certificate.Parsed.verifyHostName` ignores `iPAddress` SANs, so DoT to an IP literal cannot pass verification on stock 0.16 — the `-Dlive` DoT case fails with `CertificateHostMismatch` against 1.1.1.1 while live DoH passes. Planned follow-up (own commit): a per-upstream `tls_name` for SNI + verification while dialing the IP.