diff --git a/PLAN.md b/PLAN.md index 0e2ed71..6fe437f 100644 --- a/PLAN.md +++ b/PLAN.md @@ -98,7 +98,7 @@ Two SQLite files with opposite write profiles, isolated from each other: Blocklist domains are **not** stored in SQLite — they are a cache of re-downloadable remote artifacts, not config or state: -- Each source compiles to `/var/lib/nxdns/blocklists/.list`: normalized, one domain per line, small header (source URL, fetch time, counts, checksum). Wildcard/regex/exception-flavored lines: wildcards go to `.wild`, ABP exceptions (`@@||name^`) to `.allow`; regex lines and browser-syntax lines nxdns cannot translate into a DNS decision are counted + skipped, both counts in metadata → UI. The checksum covers the three bodies in that order, so a source with no exceptions keeps the digest it had when only two existed and no upgrade forces a refetch. +- Each source compiles to `/var/lib/nxdns/blocklists/.list`: normalized, one domain per line, small header (source URL, fetch time, counts, checksum). Wildcard/regex/exception-flavored lines: wildcards go to `.wild`, ABP exceptions (`@@||name^`) to `.allow`; regex lines and browser-syntax lines nxdns cannot translate into a DNS decision are counted + skipped, both counts in metadata → UI. The checksum covers the three bodies in that order, each followed by a separator byte so that moving a name between bodies — an upstream switching `a.example` to `*.a.example` — changes the digest and forces a republish. - `config.db` keeps source **metadata only** (`blocklist_sources`). - Startup + post-update: parse files into the immutable in-memory matcher (RCU swap, §9.5). - Corruption recovery is per-file: checksum mismatch → re-download one list. diff --git a/specs/milestone-24.md b/specs/milestone-24.md index e207b9b..6772d90 100644 --- a/specs/milestone-24.md +++ b/specs/milestone-24.md @@ -118,9 +118,13 @@ the only thing a restart reads — rehydration never reparses headers. Both skip counters take `compiled.result.counts` on this path: `.skipped_regex_count = compiled.result.counts.skipped_regex` and `.skipped_unsupported_count = compiled.result.counts.skipped_unsupported`. - `domain_count`, `wildcard_count` and `exception_count` keep reading from + ~~`domain_count`, `wildcard_count` and `exception_count` keep reading from `row` — they count written entries, so an unchanged checksum does mean an - unchanged value for them. + unchanged value for them.~~ **Corrected post-commit:** that claim was false. + The unframed digest could not distinguish an entry in `.list` from the same + entry in `.wild`, so an unchanged checksum did not vouch for the entry + counts either. The addendum below frames the hash and makes all five stat + fields read fresh on this path. This needs a test the author has watched fail with the fix reverted: two compiles of bodies whose written entries are identical but whose skipped @@ -495,3 +499,83 @@ New files: none. Deleted surface: none. - No new columns beyond the one. See ruling 7 for what that leaves open and why — the reason is a scope decision, not a claim that the other counters are already visible. + +## Addendum (post-`1bce81e`): the body checksum is framed + +A filtering correctness bug found by the implementation review, predating this +milestone. Fixed as a follow-up commit; this addendum is its design record — +the user ruled it a follow-up, not a milestone 25. + +### The defect + +`bodyChecksum` (manager.zig:1660) and the compiler's incremental hashing +(compiler.zig:105-108) both digest the unframed concatenation +`list ++ wild ++ allow`. The compiler strips `*.` from a wildcard candidate +(compiler.zig:130-133), so upstream `a.example` (list `a.example\n`, wild +empty) and upstream `*.a.example` (list empty, wild `a.example\n`) hash the +same bytes. `diskBodiesMatch` (manager.zig:1085) recomputes with the same +function, so the refresh takes the unchanged-checksum branch and a list that +switches an exact block to a wildcard block never takes effect. Ruling 3's +entry-count argument rested on the digest distinguishing bodies; it does not, +and the strikethrough above records that. + +### The fix: a `0x00` separator after each body + +The digest becomes `SHA-256(list ‖ 00 ‖ wild ‖ 00 ‖ allow ‖ 00)` — one zero +byte fed to the hasher **after each of the three bodies**, same order as +today. Soundness: a compiled body holds only validated name bytes and `\n`; +`addCandidate` rejects any byte ≥ `0x80` or control byte +(compiler.zig:151-155), so `0x00` cannot occur in a body and the three +boundaries are unambiguous. Two distinct `(list, wild, allow)` triples cannot +produce one digest short of SHA-256 itself. + +A separator, not a length prefix, because the compiler hashes while it emits +and does not know a body's length up front; a trailing byte needs no pre-pass. + +Both producers move together or every refresh republishes forever: +`compiler.compile` feeds the byte after each `emit` call, and +`manager.bodyChecksum` feeds it after each body slice. Export the separator as +a `pub const` from `compiler.zig` and have `bodyChecksum` use it — two literal +`0`s in two files is how the next drift starts. + +### Consequences, accepted + +- Every stored checksum changes once. Zero installs; a development data + directory fails its startup checksum verification and the startup refresh + pass re-downloads and repairs it (or delete the data directory — the ruling + 1 stance). +- Checksum compatibility with pre-exception digests — milestone 21 ruling 3's + "empty allow body reproduces the old digest" property — is dead, and its + rationale comments go with it: `Result.checksum` (compiler.zig:44-53), the + `Header` doc and `bodyChecksum` doc (manager.zig:218-225, 1645-1648), + `SourceStats.checksum` (sources_repo.zig), and the PLAN §3.8 sentence + "keeps the digest it had when only two existed" (PLAN.md:101). All are + rewritten to state the framed digest. The body order stays list, wild, + allow. + +### The branch reads all five fresh + +On the checksum-unchanged path, all five stat fields — +`domain_count`, `wildcard_count`, `exception_count`, `skipped_regex_count`, +`skipped_unsupported_count` — now read from `compiled.result.counts`; +`checksum` keeps passing `stored`. With framing, fresh and stored entry counts +are provably equal, so this is not a correctness requirement — it removes the +per-field vouching argument from the code entirely, and any future digest +weakness then degrades to consistent stats rather than a split between +database and status table. + +### Regression tests (each watched failing with the framing reverted) + +- [ ] Compiler: compiling `a.example` and compiling `*.a.example` produce + **different** checksums. This is the collision itself. +- [ ] Manager: publish a source whose body is `a.example`; refresh it with + upstream bytes `*.a.example`. Assert the unchanged-checksum branch is + **not** taken: the on-disk `.wild` stripped body is `a.example\n`, the + `.list` body is empty, and the database row reads `domain_count = 0`, + `wildcard_count = 1`. +- [ ] Agreement: `bodyChecksum` over the three stripped on-disk bodies equals + `compile`'s reported checksum for a fixture where **all three bodies are + non-empty** (extend the existing agreement coverage if it exists; the + empty-allow case no longer exercises the third frame). + +Report the observed failure output for each, per the ruling 3 convention. diff --git a/src/filter/compiler.zig b/src/filter/compiler.zig index 2dfc2f1..a68c7e4 100644 --- a/src/filter/compiler.zig +++ b/src/filter/compiler.zig @@ -43,16 +43,30 @@ pub const Counts = struct { pub const Result = struct { counts: Counts, - /// Lowercase hex sha256 over the `.list` body, then the `.wild` body, then - /// the `.allow` body. + /// Lowercase hex sha256 over the `.list` body, the `.wild` body and the + /// `.allow` body in that order, each followed by `body_separator`. /// - /// The `.allow` body came last for a reason: hashing an empty one adds - /// nothing, so a list with no exceptions keeps the digest it had when only - /// two bodies existed. Every checksum published before exceptions were - /// honoured therefore stays valid, and upgrading forces no refetch. + /// The separator is what makes the digest identify a compile. Without it + /// the three bodies concatenate ambiguously: a wildcard is stored with its + /// `*.` stripped, so an upstream that changes `a.example` to `*.a.example` + /// moves the same bytes from the `.list` body to the `.wild` body and + /// hashes to the same digest. `Manager.diskBodiesMatch` would then accept + /// the stale files, the refresh would keep them, and the wildcard would + /// never take effect. checksum: [64]u8, }; +/// Fed to the checksum hasher after each of the three bodies, so the digest +/// reads them as three fields rather than one run of bytes. +/// +/// `0x00` is sound as a separator because it can never occur inside a body: +/// `addCandidate` rejects every control byte and every byte at or above `0x80`, +/// so a body holds none. Any producer of this digest must use this constant — +/// `compiler.compile` hashes while it emits and `Manager.bodyChecksum` hashes +/// three finished buffers, and a one-byte disagreement between them would make +/// every refresh republish for ever. +pub const body_separator = "\x00"; + pub const Error = error{ OutOfMemory, TooManyDomains, ReadFailed, WriteFailed }; /// Reads `r` to end of stream and writes the three compiled bodies. @@ -100,12 +114,15 @@ pub fn compile( } } - // The `.allow` body is hashed last so that a list with no exceptions - // reproduces the digest a two-body compile of the same bytes produced. + // Each body is followed by `body_separator`, which is what keeps the digest + // from confusing a name in one body with the same name in another. var hasher = Sha256.init(.{}); counts.domains = try emit(&bodies.list, list_w, &hasher, &counts.duplicates); + hasher.update(body_separator); counts.wildcards = try emit(&bodies.wild, wild_w, &hasher, &counts.duplicates); + hasher.update(body_separator); counts.exceptions = try emit(&bodies.allow, allow_w, &hasher, &counts.duplicates); + hasher.update(body_separator); var digest: [Sha256.digest_length]u8 = undefined; hasher.final(&digest); @@ -433,22 +450,23 @@ test "the allow body is sorted, deduplicated and normalized like the others" { try testing.expectEqualStrings("ads.example\n", c.wild()); } -test "an empty allow body reproduces the checksum of a two-body compile" { - var c = try compileText(testing.allocator, hosts_fixture, .hosts); - defer c.deinit(); - try testing.expectEqualStrings("", c.allow()); +test "moving a name between bodies changes the checksum" { + // The collision the separator exists to prevent. A wildcard is stored with + // its `*.` stripped, so both compiles write the bytes `a.example\n` — one + // into the `.list` body, one into the `.wild` body. Unframed, the two hash + // identically, `diskBodiesMatch` accepts the stale files, and an upstream + // that switched a name to a wildcard never takes effect. + var exact = try compileText(testing.allocator, "a.example\n", .domains); + defer exact.deinit(); + var wild = try compileText(testing.allocator, "*.a.example\n", .domains); + defer wild.deinit(); - // What the digest was before the `.allow` body existed: the `.list` body - // followed by the `.wild` body and nothing else. Every checksum stored by an - // older nxdns was taken this way, and this is the equality that keeps them - // valid — without it, every source on every installation would report - // `ChecksumMismatch` at the first reload after the upgrade and re-download. - var hasher = Sha256.init(.{}); - hasher.update(c.list()); - hasher.update(c.wild()); - var digest: [Sha256.digest_length]u8 = undefined; - hasher.final(&digest); - try testing.expectEqualStrings(&std.fmt.bytesToHex(digest, .lower), &c.result.checksum); + try testing.expectEqualStrings("a.example\n", exact.list()); + try testing.expectEqualStrings("", exact.wild()); + try testing.expectEqualStrings("", wild.list()); + try testing.expectEqualStrings("a.example\n", wild.wild()); + + try testing.expect(!std.mem.eql(u8, &exact.result.checksum, &wild.result.checksum)); } test "the checksum of a source with exceptions covers all three bodies in order" { @@ -461,8 +479,11 @@ test "the checksum of a source with exceptions covers all three bodies in order" var hasher = Sha256.init(.{}); hasher.update(c.list()); + hasher.update(body_separator); hasher.update(c.wild()); + hasher.update(body_separator); hasher.update(c.allow()); + hasher.update(body_separator); var digest: [Sha256.digest_length]u8 = undefined; hasher.final(&digest); try testing.expectEqualStrings(&std.fmt.bytesToHex(digest, .lower), &c.result.checksum); @@ -620,15 +641,17 @@ test "carriage returns are stripped" { try testing.expectEqualStrings("a.example.com\nb.example.com\n", c.list()); } -test "empty input produces empty bodies and the sha256 of the empty string" { +test "empty input produces empty bodies and the digest of three separators" { var c = try compileText(testing.allocator, "", .domains); defer c.deinit(); try testing.expectEqualStrings("", c.list()); try testing.expectEqualStrings("", c.wild()); try testing.expectEqualStrings("", c.allow()); + // Three empty bodies still hash their three separators, so an empty compile + // has a digest of its own rather than the sha256 of the empty string. try testing.expectEqualStrings( - "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + "709e80c88487a2411e1ee4dfb9f22a861492d20c4765150c0c794abd70f8147c", &c.result.checksum, ); } diff --git a/src/filter/filter_integration_test.zig b/src/filter/filter_integration_test.zig index 24d9a91..859972b 100644 --- a/src/filter/filter_integration_test.zig +++ b/src/filter/filter_integration_test.zig @@ -258,11 +258,17 @@ fn snapshotOver( }); } +/// The third producer of this digest, beside `compiler.compile` and +/// `Manager.bodyChecksum`. All three must frame the bodies the same way or the +/// manager reads its own files as damaged. fn bodyChecksum(list_body: []const u8, wild_body: []const u8, allow_body: []const u8) [64]u8 { var hasher = Sha256.init(.{}); hasher.update(list_body); + hasher.update(compiler.body_separator); hasher.update(wild_body); + hasher.update(compiler.body_separator); hasher.update(allow_body); + hasher.update(compiler.body_separator); var digest: [Sha256.digest_length]u8 = undefined; hasher.final(&digest); return std.fmt.bytesToHex(digest, .lower); @@ -2149,7 +2155,7 @@ test "19: a downloaded list's exceptions lift its own blocks and nothing else" { } } -test "20: a data directory written before exceptions existed loads with no checksum mismatch" { +test "20: a data directory with no .allow file loads, and an unframed checksum does not" { if (!build_options.integration) return error.SkipZigTest; const gpa = testing.allocator; @@ -2159,8 +2165,8 @@ test "20: a data directory written before exceptions existed loads with no check const id = try seedSource(&env.database, source_url); - // Exactly what an older nxdns left behind: two compiled files, no `.allow` - // file, and a checksum taken over the two bodies alone. + // Two compiled files and no `.allow` file, which is what a source that + // published before exceptions existed left on disk. const list_body = "aaa.example.com\nbbb.example.com\n"; const wild_body = "ccc.example.com\n"; @@ -2177,33 +2183,55 @@ test "20: a data directory written before exceptions existed loads with no check .data = wild_body, }); - var hasher = Sha256.init(.{}); - hasher.update(list_body); - hasher.update(wild_body); - var digest: [Sha256.digest_length]u8 = undefined; - hasher.final(&digest); - const legacy_checksum = std.fmt.bytesToHex(digest, .lower); - - try sources_repo.updateSourceStats(&env.database, id, .{ + var stats: sources_repo.SourceStats = .{ .last_updated = 1_700_000_000, .domain_count = 2, .wildcard_count = 1, .exception_count = 0, .skipped_regex_count = 0, .skipped_unsupported_count = 0, - .checksum = &legacy_checksum, - }); + .checksum = undefined, + }; - try env.mgr.reload(io); + // The digest those two files carried before the bodies were framed. It is + // not the digest of any body layout the compiler produces now, so the load + // must report the mismatch instead of serving the files. + { + var hasher = Sha256.init(.{}); + hasher.update(list_body); + hasher.update(wild_body); + var digest: [Sha256.digest_length]u8 = undefined; + hasher.final(&digest); + const unframed = std.fmt.bytesToHex(digest, .lower); - const status = try env.status(id); - try testing.expectEqual(manager.State.ok, status.state); - try testing.expect(status.loaded); - try testing.expectEqualStrings("", status.errorText()); + stats.checksum = &unframed; + try sources_repo.updateSourceStats(&env.database, id, stats); + try env.mgr.reload(io); - const decision, _ = try env.evaluate("aaa.example.com"); - try testing.expect(decision.blocked); - try testing.expect((try env.evaluate("x.ccc.example.com"))[0].blocked); + const status = try env.status(id); + try testing.expectEqual(manager.State.load_failed, status.state); + try testing.expect(!status.loaded); + try testing.expectEqualStrings("ChecksumMismatch", status.errorText()); + } + + // The same two files under the framed digest of three bodies, the third of + // them empty. The absent `.allow` file is that empty body, so the source + // loads and both entries filter. + { + const framed = bodyChecksum(list_body, wild_body, ""); + stats.checksum = &framed; + try sources_repo.updateSourceStats(&env.database, id, stats); + try env.mgr.reload(io); + + const status = try env.status(id); + try testing.expectEqual(manager.State.ok, status.state); + try testing.expect(status.loaded); + try testing.expectEqualStrings("", status.errorText()); + + const decision, _ = try env.evaluate("aaa.example.com"); + try testing.expect(decision.blocked); + try testing.expect((try env.evaluate("x.ccc.example.com"))[0].blocked); + } } // --------------------------------------------------------------------------- @@ -2267,6 +2295,42 @@ const churn_after = "||blocked.example^\n" ++ "tracker.example\n"; +const boundary_before = "a.example\n"; +const boundary_after = "*.a.example\n"; + +test "23: a name moving from the list body to the wild body forces a republish" { + if (!build_options.integration) return error.SkipZigTest; + + const gpa = testing.allocator; + const env = try Env.create(gpa); + defer env.destroy(); + const io = env.io(); + + var fixture = try HttpFixture.init(io, boundary_before); + fixture.changed_body = boundary_after; + defer fixture.deinit(io); + var group: std.Io.Group = .init; + defer group.cancel(io); + try group.concurrent(io, HttpFixture.serve, .{ &fixture, io }); + + var url_buf: [64]u8 = undefined; + const url = try fixture.url(&url_buf); + _ = try seedSource(&env.database, url); + + try testing.expect(try refreshOnce(env, url)); + + var first_checksum: [64]u8 = undefined; + { + var rows = try listRows(&env.database); + defer rows.deinit(); + const row = try rows.byUrl(url); + try testing.expectEqual(@as(i64, 1), row.domain_count); + try testing.expectEqual(@as(i64, 0), row.wildcard_count); + @memcpy(&first_checksum, row.checksum orelse return error.TestNoChecksum); + } + +} + test "22: a list that changed only its skipped lines still updates both skip counters" { if (!build_options.integration) return error.SkipZigTest; diff --git a/src/filter/manager.zig b/src/filter/manager.zig index 212580f..b073832 100644 --- a/src/filter/manager.zig +++ b/src/filter/manager.zig @@ -220,9 +220,8 @@ pub const SourceStatus = struct { /// **not** the header, so it stays stable across a refetch of unchanged content /// while `fetched_at` moves. /// -/// The `.allow` body is hashed last so that a source with no exceptions keeps -/// the digest it had when only two bodies existed: every checksum written before -/// exceptions were honoured stays valid, and no upgrade forces a refetch. +/// Each body is followed by a separator byte, so the digest identifies which +/// body a name sits in rather than only which names were written. pub const Header = struct { url: []const u8, format: parsers.Format, @@ -591,11 +590,10 @@ pub const Manager = struct { }; bodies.appendAssumeCapacity(wild_bytes); - // A missing `.allow` file is an empty allow body, not a failure. Two - // sources are in that state and both are ordinary: one compiled before - // exceptions were honoured, and one whose list carries no `@@` line. - // Because the empty body contributes nothing to the checksum, the - // stored digest of either still matches. + // A missing `.allow` file is an empty allow body, not a failure. The + // digest still covers three bodies, the third of them empty, so a + // source whose list carries no `@@` line matches whether its empty + // `.allow` file survived or not. const allow_bytes: []const u8 = blk: { const read = dir.readFileAlloc(io, allow_name, self.gpa, .limited(max_compiled_bytes)) catch |err| { if (err == error.OutOfMemory) return error.OutOfMemory; @@ -822,19 +820,19 @@ pub const Manager = struct { if (std.mem.eql(u8, stored, &compiled.result.checksum) and self.diskBodiesMatch(io, dir, row.id, stored)) { - // The three written counts come from the row: the checksum - // covers the three bodies, so an unchanged checksum means an - // unchanged number of entries in each. The two skip counts do - // not: a skipped line lands in no body, so a list that changed - // only its regex or browser-syntax lines arrives here with a - // stale row and a fresh compile. Taking them from the row would - // show the operator one number and restore another after a - // restart. + // Every count comes from the compile that just ran, not from + // the row. The two skip counts have to: a skipped line lands in + // no body, so a list that changed only its regex or + // browser-syntax lines reaches here with a stale row. The three + // written counts equal the row's anyway once the digest is + // framed, so reading them from the compile costs nothing and + // leaves no field whose freshness rests on an argument about + // what the checksum covers. try sources_repo.updateSourceStats(self.database, row.id, .{ .last_updated = now, - .domain_count = row.domain_count, - .wildcard_count = row.wildcard_count, - .exception_count = row.exception_count, + .domain_count = compiled.result.counts.domains, + .wildcard_count = compiled.result.counts.wildcards, + .exception_count = compiled.result.counts.exceptions, .skipped_regex_count = compiled.result.counts.skipped_regex, .skipped_unsupported_count = compiled.result.counts.skipped_unsupported, .checksum = stored, @@ -1078,10 +1076,9 @@ pub const Manager = struct { /// the rewrite path — the only path that can repair it. /// /// A missing `.allow` file is the one exception, and it is the same one - /// `loadSource` makes: a source compiled before exceptions were honoured has - /// no such file, and its stored checksum was taken over an empty allow body. - /// Answering false there would rewrite every list on the first refresh after - /// an upgrade for no change in content. + /// `loadSource` makes: it reads as an empty allow body, which is what a list + /// with no `@@` line compiles to anyway. Answering false there would rewrite + /// such a list on every refresh for no change in content. fn diskBodiesMatch(self: *Manager, io: std.Io, dir: std.Io.Dir, id: i64, expected: []const u8) bool { var list_buf: [name_buf_len]u8 = undefined; var wild_buf: [name_buf_len]u8 = undefined; @@ -1653,15 +1650,20 @@ fn rejectedWithoutEntries(counts: compiler.Counts) bool { return counts.invalid != 0 or counts.skipped_unsupported != 0 or counts.long_lines != 0; } -/// The digest the `.list`, `.wild` and `.allow` bodies share, in that order. -/// The allow body comes last so that hashing an empty one leaves the digest of -/// the two-body form untouched, which is what keeps every checksum stored before -/// exceptions were honoured valid. +/// The digest the `.list`, `.wild` and `.allow` bodies share, in that order, +/// each followed by `compiler.body_separator`. +/// +/// Must stay byte-for-byte what `compiler.compile` produces, separators +/// included: this is the other half of the same digest, and the two are +/// compared against each other on every refresh. fn bodyChecksum(list_body: []const u8, wild_body: []const u8, allow_body: []const u8) [64]u8 { var hasher = Sha256.init(.{}); hasher.update(list_body); + hasher.update(compiler.body_separator); hasher.update(wild_body); + hasher.update(compiler.body_separator); hasher.update(allow_body); + hasher.update(compiler.body_separator); var digest: [Sha256.digest_length]u8 = undefined; hasher.final(&digest); return std.fmt.bytesToHex(digest, .lower); @@ -2412,23 +2414,33 @@ test "compiledBodiesMatch verifies the bodies, not the presence of the files" { try testing.expect(!compiledBodiesMatch("", "", "", &expected)); } -test "a source with no exceptions keeps the checksum it had before the allow body existed" { +test "bodyChecksum separates the three bodies" { const list_body = "a.example.com\nb.example.com\n"; const wild_body = "c.example.com\n"; - // What an older nxdns stored: the digest of the two bodies alone. It is what - // sits in `blocklist_sources.checksum` on every installation being upgraded, - // and the files on disk are the two it was taken over. var hasher = Sha256.init(.{}); hasher.update(list_body); + hasher.update(compiler.body_separator); hasher.update(wild_body); + hasher.update(compiler.body_separator); + hasher.update(compiler.body_separator); var digest: [Sha256.digest_length]u8 = undefined; hasher.final(&digest); - const stored = std.fmt.bytesToHex(digest, .lower); + const expected = std.fmt.bytesToHex(digest, .lower); - try testing.expectEqualStrings(&stored, &bodyChecksum(list_body, wild_body, "")); + try testing.expectEqualStrings(&expected, &bodyChecksum(list_body, wild_body, "")); // No `.allow` file: what `loadSource` and `diskBodiesMatch` pass for one. - try testing.expect(compiledBodiesMatch(list_body, wild_body, "", &stored)); + // It is an empty body, and an empty body still gets its separator. + try testing.expect(compiledBodiesMatch(list_body, wild_body, "", &expected)); + + // The framing itself: the same bytes in a different body is a different + // digest. Unframed these two are equal, and a stale `.list` survives an + // upstream that switched the name to a wildcard. + try testing.expect(!std.mem.eql( + u8, + &bodyChecksum("a.example\n", "", ""), + &bodyChecksum("", "a.example\n", ""), + )); } test "rejectedWithoutEntries fails a compile that produced nothing usable" { @@ -2552,7 +2564,12 @@ test "collectSample steps over a line that does not fit the reader buffer" { test "bodyChecksum covers the list body, then the wild body, then the allow body" { const all = bodyChecksum("a.example.com\n", "b.example.com\n", "c.example.com\n"); var hasher = Sha256.init(.{}); - hasher.update("a.example.com\nb.example.com\nc.example.com\n"); + hasher.update("a.example.com\n"); + hasher.update(compiler.body_separator); + hasher.update("b.example.com\n"); + hasher.update(compiler.body_separator); + hasher.update("c.example.com\n"); + hasher.update(compiler.body_separator); var digest: [Sha256.digest_length]u8 = undefined; hasher.final(&digest); try testing.expectEqualStrings(&std.fmt.bytesToHex(digest, .lower), &all); diff --git a/src/storage/repositories/sources_repo.zig b/src/storage/repositories/sources_repo.zig index 992b3b8..4c8026b 100644 --- a/src/storage/repositories/sources_repo.zig +++ b/src/storage/repositories/sources_repo.zig @@ -111,9 +111,8 @@ pub const SourceStats = struct { skipped_regex_count: i64, skipped_unsupported_count: i64, /// Lowercase hex sha256 over the `.list` body, then the `.wild` body, then - /// the `.allow` body. The allow body is hashed last so an empty one leaves - /// the digest of a two-body compile unchanged, which is what keeps a - /// checksum written before exceptions were honoured valid. + /// the `.allow` body, each followed by `compiler.body_separator` so the + /// digest cannot confuse a name in one body with the same name in another. checksum: []const u8, };