querylog: the aside file's name says why, so a schema change is not called corrupt

CHANGELOG.md

@@ -6,6 +6,10 @@ Sections are written by hand. Nothing here is generated from commit messages: th
 
 ## [Unreleased]
 
+### Fixed
+
+- **A query log set aside by a schema change is no longer named `corrupt`.** Every recreate wrote the old file to `querylog.db.corrupt-<unix seconds>`, whatever sent it there — including the fingerprint mismatch an upgrade causes, where the file is a healthy database this build simply cannot read. The name is the only account of the reason that outlives the log line, so it read as an accusation. This is not hypothetical: on the 0.0.6 upgrade it cost a real query log, deleted during a post-deploy cleanup on the strength of the word `corrupt`. The name now says which of the four cases it hit: `querylog.db.corrupt-…`, `.not-a-database-…`, `.quick-check-failed-…` or `.schema-changed-…`. The 0.0.6 upgrade produces `schema-changed`. Nothing else about the recreate changed, and no existing aside file is renamed.
+
 ## [0.0.6] - 2026-08-17
 
 The period picker now scopes the whole dashboard. The upstream table was the last widget that ignored it, and fixing that meant recording upstream outcomes over time instead of counting them since boot. Read the query-log note below before you upgrade.

docs/reference/files-and-directories.md

@@ -18,8 +18,8 @@ Default `/var/lib/nxdns`, overridable with `--data-dir DIR`. `nxdns run` and `nx
 | `config.db-wal`, `config.db-shm` | SQLite write-ahead log and shared-memory index for `config.db`. Created by `run`, `import` and `export` when WAL is enabled, inheriting the main file's permissions. `check` creates neither. | 0600 |
 | `querylog.db` | The query log: every domain every client asked for. Expendable — if it is missing or unusable it is recreated empty. | 0600 |
 | `querylog.db-wal`, `querylog.db-shm` | WAL sidecars for `querylog.db`. | 0600 |
-| `querylog.db.corrupt-<unix-seconds>` | A `querylog.db` that could not be used, moved aside before an empty one was created in its place. Kept, never overwritten. | Whatever the renamed file had — no chmod reaches it |
-| `querylog.db.corrupt-<unix-seconds>-<n>` | The same, when the plain name is taken — `<n>` counts from 1 and rises until the name is free. Two recreates within one second is the case it exists for. | The same |
+| `querylog.db.<reason>-<unix-seconds>` | A `querylog.db` this build could not use, moved aside before an empty one was created in its place. Kept, never overwritten. `<reason>` is one of `corrupt`, `not-a-database`, `quick-check-failed` or `schema-changed`; see [why a query log is moved aside](#why-a-query-log-is-moved-aside). | Whatever the renamed file had — no chmod reaches it |
+| `querylog.db.<reason>-<unix-seconds>-<n>` | The same, when the plain name is taken — `<n>` counts from 1 and rises until the name is free. Two recreates within one second is the case it exists for. | The same |
 | `blocklists/` | Compiled blocklist snapshots, one subdirectory of the data directory. | 0700 |
 | `blocklists/<id>.list` | Exact domains for blocklist source `<id>`, one per line, behind a header. | 0600 |
 | `blocklists/<id>.wild` | Wildcard entries for the same source. | 0600 |
@@ -50,7 +50,20 @@ A failed sweep is a warning, not an outage — leftover bytes do not justify los
 
 The temporaries of a source that still exists are cleaned by the refresh that owns them rather than by the sweep: each refresh deletes its own `.raw.tmp`, `.list.tmp`, `.wild.tmp` and `.allow.tmp` as it finishes, successfully or not.
 
-A `querylog.db` is moved aside when it is missing nothing but usability: SQLite reports it corrupt or not a database, `PRAGMA quick_check` does not answer `ok`, or its `user_version` fingerprint does not match the schema. Only the main file is renamed — its `-wal` and `-shm` are deleted, because a stale WAL would be replayed into the fresh database. A missing `querylog.db` is created without any aside file. The rename happens inside `querylog_schema.open`, before the 0600 chmod, and that chmod names `querylog.db` and its two sidecars only — so an aside file keeps the mode the file had at rename time, which for a `querylog.db` nxdns itself created is 0600 and for one an operator put there is whatever they left it at. Nothing prunes the aside files; they accumulate until an operator removes them, and each one holds the same browsing history the live query log holds.
+### Why a query log is moved aside
+
+A `querylog.db` is moved aside when it is missing nothing but usability, and the name it is given says which of the four cases it hit:
+
+| `<reason>` | What happened |
+| --- | --- |
+| `corrupt` | SQLite reported the file as damaged. |
+| `not-a-database` | The file is not a SQLite database at all. |
+| `quick-check-failed` | `PRAGMA quick_check` did not answer `ok`. |
+| `schema-changed` | Nothing is wrong with the file. Its `user_version` fingerprint does not match this build's schema, so this build cannot read it. Upgrades that touch the query-log schema produce this one, and the file they set aside is a healthy database. |
+
+Only the main file is renamed — its `-wal` and `-shm` are deleted, because a stale WAL would be replayed into the fresh database. A missing `querylog.db` is created without any aside file. The rename happens inside `querylog_schema.open`, before the 0600 chmod, and that chmod names `querylog.db` and its two sidecars only — so an aside file keeps the mode the file had at rename time, which for a `querylog.db` nxdns itself created is 0600 and for one an operator put there is whatever they left it at. Nothing prunes the aside files; they accumulate until an operator removes them, and each one holds the same browsing history the live query log holds.
+
+### Why the databases are 0600
 
 The 0600 modes are not cosmetic. `config.db` holds the argon2id password hash and `querylog.db` holds the browsing history of every client on the LAN, so both are as sensitive as each other, and a WAL file holds the same rows as the database it belongs to. SQLite creates the main database at `0644 & ~umask`; nxdns chmods it to 0600 before enabling WAL, so the sidecars inherit 0600 rather than being created world-readable.
 

src/storage/querylog_schema.zig

@@ -138,7 +138,7 @@ pub fn open(io: std.Io, dir: std.Io.Dir, path: [:0]const u8) Error!OpenResult {
     const aside: ?[]const u8 = if (cause == .missing)
         null
     else
-        try renameAside(io, dir, path, &aside_buf);
+        try renameAside(io, dir, path, cause, &aside_buf);
 
     // Not optional: a stale WAL left beside the renamed database would be
     // replayed into the freshly created file and corrupt it immediately. Any
@@ -178,20 +178,37 @@ fn quickCheck(database: *db.Db) db.Error!bool {
     return std.ascii.eqlIgnoreCase(stmt.columnText(0), "ok");
 }
 
+/// What the aside file's name calls the reason it was set aside.
+///
+/// The name is the only account of the reason an operator gets: the log line
+/// naming it scrolls away, the file stays for months. `fingerprint_mismatch` is
+/// a database with nothing wrong with it — this build's DDL moved — so calling
+/// its file "corrupt" invites the operator to delete evidence of a healthy file.
+fn asideTag(reason: RecreateReason) []const u8 {
+    return switch (reason) {
+        .missing => unreachable, // there is no file to rename
+        .corrupt => "corrupt",
+        .not_a_database => "not-a-database",
+        .quick_check_failed => "quick-check-failed",
+        .fingerprint_mismatch => "schema-changed",
+    };
+}
+
 /// Renames the unusable file out of the way and returns the name it now has.
 ///
 /// `renamePreserve` is `RENAME_NOREPLACE`: it returns `error.PathAlreadyExists`
-/// instead of overwriting. A previously saved corrupt file must never be
-/// destroyed by the next recreate, and two recreates in the same second are not
-/// hypothetical on a boot loop — hence the uniquifying retries.
-fn renameAside(io: std.Io, dir: std.Io.Dir, path: []const u8, buf: []u8) Error![]const u8 {
+/// instead of overwriting. A previously saved file must never be destroyed by
+/// the next recreate, and two recreates in the same second are not hypothetical
+/// on a boot loop — hence the uniquifying retries.
+fn renameAside(io: std.Io, dir: std.Io.Dir, path: []const u8, reason: RecreateReason, buf: []u8) Error![]const u8 {
+    const tag = asideTag(reason);
     const seconds = std.Io.Clock.real.now(io).toSeconds();
     var attempt: u32 = 0;
     while (attempt < 100) : (attempt += 1) {
         const aside = if (attempt == 0)
-            std.fmt.bufPrint(buf, "{s}.corrupt-{d}", .{ path, seconds }) catch return error.NameTooLong
+            std.fmt.bufPrint(buf, "{s}.{s}-{d}", .{ path, tag, seconds }) catch return error.NameTooLong
         else
-            std.fmt.bufPrint(buf, "{s}.corrupt-{d}-{d}", .{ path, seconds, attempt }) catch return error.NameTooLong;
+            std.fmt.bufPrint(buf, "{s}.{s}-{d}-{d}", .{ path, tag, seconds, attempt }) catch return error.NameTooLong;
 
         dir.renamePreserve(path, dir, aside, io) catch |e| switch (e) {
             error.PathAlreadyExists => continue,
@@ -287,6 +304,13 @@ test "recreatable is a whitelist and never selects a resource error" {
     }
 }
 
+test "the aside name says why, and a healthy file is never called corrupt" {
+    try testing.expectEqualStrings("corrupt", asideTag(.corrupt));
+    try testing.expectEqualStrings("not-a-database", asideTag(.not_a_database));
+    try testing.expectEqualStrings("quick-check-failed", asideTag(.quick_check_failed));
+    try testing.expectEqualStrings("schema-changed", asideTag(.fingerprint_mismatch));
+}
+
 test "recreatable selects exactly two of db.Error's members" {
     // Exhaustive over the whole set, so a variant added to `db.Error` later
     // defaults to propagate. The list above only proves the named errors are

src/storage/storage_integration_test.zig

@@ -190,7 +190,7 @@ fn countLines(text: []const u8) usize {
 // querylog aside files
 // ---------------------------------------------------------------------------
 
-const aside_prefix = "querylog.db.corrupt-";
+const aside_prefix = "querylog.db.";
 
 const Names = struct {
     items: std.ArrayList([]u8),
@@ -412,6 +412,10 @@ test "S7 case 3: a wrong user_version recreates and keeps the old file aside" {
     defer asides.deinit();
     try testing.expectEqual(@as(usize, 1), asides.items.items.len);
 
+    // The file was healthy: this build's schema moved, the database did not rot.
+    // An operator who reads "corrupt" here deletes a file that was never broken.
+    try testing.expect(std.mem.startsWith(u8, asides.items.items[0], "querylog.db.schema-changed-"));
+
     const kept = try f.read(asides.items.items[0]);
     defer testing.allocator.free(kept);
     try testing.expectEqualSlices(u8, original, kept);
@@ -441,6 +445,12 @@ test "S7 case 4: a garbage file recreates and the garbage is preserved" {
     defer asides.deinit();
     try testing.expectEqual(@as(usize, 1), asides.items.items.len);
 
+    const expected: []const u8 = if (reason == .corrupt)
+        "querylog.db.corrupt-"
+    else
+        "querylog.db.not-a-database-";
+    try testing.expect(std.mem.startsWith(u8, asides.items.items[0], expected));
+
     const kept = try f.read(asides.items.items[0]);
     defer testing.allocator.free(kept);
     try testing.expectEqualSlices(u8, &garbage, kept);