diff --git a/specs/milestone-21.md b/specs/milestone-21.md new file mode 100644 index 0000000..0e79cf9 --- /dev/null +++ b/specs/milestone-21.md @@ -0,0 +1,314 @@ +# Milestone 21: ABP exceptions from lists, regex rules for operators + +Goal: honor `@@||domain^` exception lines in downloaded blocklists as allow +entries scoped below operator rules and above list blocks (tier 1), and add a +`regex` rule kind for operator rules backed by a homegrown linear-time engine +(tier 2). Nothing else from the ABP syntax enters scope. + +Design written 2026-08-09 against HEAD `ffc3ca6`. Every `file:line` anchor +below was read at that commit. The milestone amends PLAN §2.2, which currently +rules regex out permanently; the amendment is part of session S3, not a +side effect. + +## Implementation contract (read first) + +- Read `AGENTS.md`, then this spec whole, before session work starts. +- Pure core stays pure: `src/filter/` files that are fuzz-module roots import + only `std` (`src/filter/parsers.zig:1-14`). The new `src/filter/regex.zig` + obeys the same constraint. +- Every new `src/**.zig` file must be listed in `src/tests.zig` + (`build.zig:495-540` fatals otherwise). +- Frozen DDL is frozen: schema changes are new migration steps + (`src/storage/config_schema.zig:1-6`, `src/storage/migrations.zig:21-22`). +- After any API shape change, regenerate the contract samples + (`web/src/lib/contractSamples.gen.ts`; procedure in AGENTS.md). + +## Rulings (binding) + +### 1. Exception lines are `@@||name^` and nothing else, plus one modifier + +`src/filter/parser_abp.zig:22` currently maps every `@@` line to +`.unsupported`. After this milestone, a line is an exception when it is +`@@||name^` or `@@||name` (same trailing-`^` and `rule_tokens` treatment as the +block anchor at parser_abp.zig:26-34), optionally suffixed with the literal +`$important` — that suffix is the common form in AdGuard-authored lists and +changes nothing about the meaning here, because list exceptions already sit +below every operator rule. Any other `@@` form (`@@name` without the anchor, +any other `$` modifier, a path, a scheme) stays `.unsupported`. The +parser-header policy paragraph (parser_abp.zig:5-6) is rewritten to state the +new rule and its precedence justification: a list exception can cancel only +list blocks, never an operator decision, so no downloaded list can open an +allow hole the operator did not open. + +### 2. Precedence: list exceptions sit between operator rules and list blocks + +`matcher.Snapshot.evaluate` (`src/filter/matcher.zig:286-338`) gains one level +between the operator wildcard-block walk (level 4) and the blocklist domain +probe (level 5): for each attached source, an exception match — full name or +parent walk, apex covered — returns `blocked = false`, reason +`.blocklist_exception`, `matched` = the matching entry, `source` = the source +index. The doc comment at matcher.zig:269-285 and PLAN §3.10 (PLAN.md:108-117) +are both updated with the new level. Regex rules (ruling 6) slot in as levels +after the wildcard rules and before list exceptions, allow before block, so +the full order is: exact allow, exact block, wildcard allow, wildcard block, +regex allow, regex block, list exception, list domain, list wildcard. +"Tie-break at same specificity: allow wins" is preserved. + +### 3. The compiled-source format grows a third body, checksum-compatibly + +`compiler.compile` (`src/filter/compiler.zig:50-56`) takes a third writer +(`allow_w`) and `Counts` (compiler.zig:23-35) gains `exceptions: u32 = 0`. +Exception candidates go through the existing `addCandidate` path into a third +`Entries` and emit as a sorted, deduplicated `.allow` body. The shared SHA-256 +covers the bodies in order list, wild, allow — because SHA-256 of +`list ++ wild ++ ""` equals the current SHA-256 of `list ++ wild`, every +already-published checksum stays valid, and `Manager.loadSource` +(`src/filter/manager.zig:543-598`) treats a missing `.allow` file as an +empty body. No refetch is forced by upgrading. `bodyChecksum` +(manager.zig:1564) follows the same order; `source_file_suffixes` +(manager.zig:1581) gains `.allow.tmp` and `.allow` with longest-suffix-first +order preserved; the on-disk header (manager.zig:222-242) gains +`# exceptions {d}` after the `# wildcards` line and the pinning test at +manager.zig:1872-1900 is extended, not weakened. + +### 4. Exception counts persist and surface + +Migration step 3 (`ddl_v3`, appended at `src/storage/migrations.zig:23-26`): +`ALTER TABLE blocklist_sources ADD COLUMN exception_count INTEGER NOT NULL +DEFAULT 0;`. `sources_repo.SourceRow` and `updateSourceStats` +(`src/storage/repositories/sources_repo.zig:91-166`) carry it; +`SourceStatus` rehydration (`manager.zig:1457-1494`) restores it alongside the +existing three counts; `StatusView` (`src/web/handlers/blocklists.zig:52-78`) +gains `exceptions: u32`; the blocklists UI shows it where `skipped_regex` +already shows (`web/src/features/blocklists/SourceStatusSection.tsx`, +`web/src/lib/types.ts:163,192`). + +### 5. The regex engine is a Pike VM, linear-time by construction, `std` only + +New file `src/filter/regex.zig`. Syntax: literal bytes, `.`, character +classes `[...]` with ranges and leading-`^` negation, escapes +`\. \\ \- \d \w`, repetition `* + ? {n} {n,m}`, alternation `|`, +non-capturing grouping `(...)`, anchors `^` and `$`. No backreferences, no +lookaround, no captures. Matching is unanchored unless anchors are written +(POSIX-grep convention, matching Pi-hole user expectations). Input is the +normalized lowercase name, ≤ `types.max_name_len` bytes. Hard limits, each a +distinct error: pattern ≤ 256 bytes (`PatternTooLong`), compiled program +≤ 1024 instructions (`PatternTooComplex`). Public API: + +```zig +pub const Error = error{ OutOfMemory, BadPattern, PatternTooLong, PatternTooComplex }; +pub const Program = struct { ... , pub fn deinit(self: *Program, gpa: Allocator) void }; +pub fn compile(gpa: Allocator, pattern: []const u8) Error!Program; +pub fn matches(prog: *const Program, input: []const u8) bool; +``` + +`matches` is a Pike VM: two thread lists, each program counter admitted at +most once per input position, worst case O(program × input) with zero +allocation at match time (thread lists sized from the program at compile +time). The engine is a fuzz-module root like parsers.zig and imports only +`std`. + +### 6. `regex` is a third rule kind, validated at the edge, memoized by the cache + +Migration step 4 (`ddl_v4`): the 12-step rebuild of `rules` with +`CHECK(kind IN ('exact','wildcard','regex'))` — the frozen v1 DDL +(`src/storage/config_schema.zig:65-72`) cannot be edited. `model.RuleKind` +(`src/config/model.zig:253-269`) gains `.regex`; the exhaustive switches in +`config/validate.zig:1067-1098` (compile the pattern, report +`"... is not a valid regex pattern"` through the existing error path at +validate.zig:862-869) and `src/filter/rules.zig:62-68` extend. `RuleSet` +(`rules.zig:28-36`) grows `regex_allow` and `regex_block` slices holding +compiled `Program`s plus their pattern texts (for `Decision.matched`); +`bucketOf` (rules.zig:133-143) becomes a six-bucket layout; +`max_regex_per_group: usize = 256` with `TooManyRegexRules` mirroring +`max_wildcards_per_group` (rules.zig:26). A pattern that fails to compile is +`error.BadPattern` at snapshot build, never skipped (rules.zig:41-49 doc +holds). Reason tags `rule_allow_regex` and `rule_block_regex` join +`matcher.Reason` (matcher.zig:24-32); `/api/lookup` and the query log pick +them up automatically via `@tagName` (`src/web/handlers/lookup.zig:89`; +`max_reason_len = 32` in `src/storage/logger.zig` fits both at 16 chars). +Regex evaluation runs only after every hash and wildcard level missed, and +answers are memoized by the existing DNS cache like every other decision, so +the per-query cost lands on cache misses only. + +### 7. The web contract names the third kind everywhere it names the first two + +`src/web/handlers/rules.zig`: `toInput` accepts `"regex"`; the 400 string at +rules.zig:42 becomes `"kind must be 'exact', 'wildcard' or 'regex'"`. +`src/web/openapi.yaml:1875-1917`: all three `enum: [exact, wildcard]` become +`[exact, wildcard, regex]`. `web/src/lib/types.ts:195`: +`RuleKind = "exact" | "wildcard" | "regex"`; the rules page kind selector +gains the option. Contract samples regenerated. `nxdns export` / `import` +round-trip the new kind with no extra work once `RuleKind.toDb/fromDb` extend +— the existing round-trip test at model.zig:722-723 is extended to prove it. + +### 8. PLAN amendments land with the code, in S3 + +PLAN.md:36 (§2.2) is rewritten: operator regex rules are in scope, backed by +the linear-time engine of ruling 5; regex lines in downloaded lists stay +counted and skipped; `$` modifiers (except the `$important` suffix of ruling +1), partial-segment wildcards, and browser-syntax honoring stay permanently +out. PLAN.md:26 (§2.1 filtering sentence), PLAN.md:106 (§3.9), PLAN.md:108-117 +(§3.10 precedence) and PLAN.md:700 (decision B) are updated to match rulings +2 and 6. In-code echoes of the old §2.2 move with it: +`src/filter/wildcard.zig:6-8,22-24`, `src/filter/parsers.zig:25`, +`src/filter/parser_abp.zig:5-6`, `src/filter/compiler.zig:129-131`. + +### 9. Fuzz invariants move, never lapse + +`tests/fuzz/blocklist_fuzz.zig` header invariant "covers_apex only on +`.wildcard`" (its stated form at :4-28) becomes "only on `.wildcard` or +`.exception`". `tests/fuzz/compiler_fuzz.zig:43` reads `Format` from +`compile`'s parameter list by index — the new `allow_w` parameter appends +after `wild_w`, leaving index 2 valid; the session touching `compile` runs the +fuzz suite and fixes that line if the assumption fails. A new +`tests/fuzz/regex_fuzz.zig` target asserts: `compile` on arbitrary bytes +never crashes and either errors or produces a program within the ruling-5 +limits; `matches` terminates and its VM step count never exceeds +program length × (input length + 1); compile-then-match is deterministic. + +## Sessions + +Three sessions. S1 and S2 run in parallel — they share no files. S3 starts +after both land. + +### Session S1: list exceptions end to end (tier 1) + +Owns: `src/filter/parsers.zig`, `src/filter/parser_abp.zig`, +`src/filter/compiler.zig`, `src/filter/manager.zig`, `src/filter/matcher.zig`, +`src/filter/domain_set.zig` (only if a helper is needed; expected untouched), +`src/filter/filter_integration_test.zig`, `src/storage/migrations.zig` +(step 3 only), `src/storage/repositories/sources_repo.zig`, +`src/web/handlers/blocklists.zig`, `src/web/handlers/lookup.zig` (doc +sentence only), `src/web/openapi.yaml` (StatusView shape only), +`web/src/features/blocklists/*`, `web/src/lib/types.ts` (source-stat fields +only), `web/src/lib/contractSamples.gen.ts`, +`tests/fuzz/blocklist_fuzz.zig`, `tests/fuzz/compiler_fuzz.zig`. + +- S1.1 `Kind.exception` in parsers.zig; parser_abp emits it per ruling 1; + parser_hosts and parser_domains never emit it (no change beyond the enum). +- S1.2 compiler third body per ruling 3; `Counts.exceptions`. +- S1.3 manager: suffixes, header line, `bodyChecksum`, `loadSource` + missing-file-is-empty, `Snapshot.Compiled.allow_body`, + `prepareRefresh`/`publishRefresh`/`applyLoadOutcomes` carry the count. + `rejectedWithoutEntries` (manager.zig:1559-1562) treats a compile with only + exceptions as loadable, not rejected. +- S1.4 matcher: `SourceSets.exceptions`, `Reason.blocklist_exception`, + the new evaluate level per ruling 2, `memoryBytes` includes the new sets. +- S1.5 storage + API + UI per ruling 4. +- S1.6 tests: parser cases (`@@||x^`, `@@||x`, `@@||x^$important`, + `@@||x^$third-party` → unsupported, `@@x` → unsupported); compiler + three-body + checksum-compat cases (empty allow body reproduces the old + digest byte for byte); manager header pin extended; matcher precedence + cases: operator block beats list exception, list exception beats list + domain and list wildcard, exception parent walk covers apex and + subdomains; an integration case through + `filter_integration_test.zig` with a real ABP fixture carrying `@@` lines. + +Acceptance (S1): +- [ ] `zig build test` passes; fuzz targets build and run. +- [ ] A fixture list with `||ads.example^` and `@@||good.ads.example^` + compiled and loaded blocks `ads.example` and `x.ads.example`, does not + block `good.ads.example` or `y.good.ads.example`, and + `/api/lookup` reports `blocklist_exception` with the source id for the + latter two. +- [ ] A pre-milestone data directory (no `.allow` files, old checksums) + loads with zero checksum mismatches. +- [ ] `POST /api/blocklists/update` response rows carry `exceptions`. + +### Session S2: the regex engine (tier 2, engine only) + +Owns: `src/filter/regex.zig` (new), `tests/fuzz/regex_fuzz.zig` (new), +`src/tests.zig` (one added line), `build.zig` (fuzz-suite wiring for the new +target only). + +- S2.1 the engine per ruling 5: parser → AST → NFA program → Pike VM. +- S2.2 unit tests in-file: every syntax form; anchored and unanchored + matching; negated classes; `{n,m}` bounds; each error case; the + pathological backtracker-killers (`(a+)+b` against `aaaaaaaaaaaaaaaaaaaaX`, + nested alternation) complete within the step bound. +- S2.3 the fuzz target per ruling 9. + +Acceptance (S2): +- [ ] `zig build test` passes with the new file in `src/tests.zig`. +- [ ] The step-bound property holds under the fuzz corpus. +- [ ] `regex.zig` imports nothing but `std`. + +### Session S3: the regex rule kind, wired through (needs S1 + S2) + +Owns: `src/storage/migrations.zig` (step 4), `src/storage/config_schema.zig` +(comment only if needed), `src/config/model.zig`, `src/config/validate.zig`, +`src/filter/rules.zig`, `src/filter/matcher.zig`, +`src/storage/repositories/rules_repo.zig`, `src/web/handlers/rules.zig`, +`src/web/handlers/mutations.zig` (only if `checkRule` needs the kind), +`src/web/openapi.yaml`, `web/src/features/rules/*`, `web/src/lib/types.ts`, +`web/src/lib/contractSamples.gen.ts`, `PLAN.md`, `src/filter/wildcard.zig` +(comments), `src/filter/parsers.zig` (comment), `src/filter/parser_abp.zig` +(comment), `src/filter/compiler.zig` (comment), `tools/bench.zig`. + +- S3.1 migration step 4 per ruling 6; repo and model layers. +- S3.2 validate at both edges (config import, API) per rulings 6 and 7. +- S3.3 `RuleSet` six buckets; matcher levels per ruling 2; reasons. +- S3.4 web + UI + openapi + samples per ruling 7. +- S3.5 PLAN and comment amendments per ruling 8. +- S3.6 bench: the filter suite in `tools/bench.zig` gains a variant with 32 + regex rules loaded; the existing p95 < 1 ms assertion covers it. +- S3.7 tests: rule CRUD with kind `regex` through the API including the 400 + for a bad pattern at insert time; precedence cases regex-allow over + regex-block, wildcard over regex, regex over list entries; export/import + round trip; a migration test upgrading a v3 database. + +Acceptance (S3): +- [ ] `zig build test` and `cd web && npm test` pass. +- [ ] `POST /api/rules` with `{"kind":"regex","pattern":"^ad[0-9]+-"}` + returns 201; with `"pattern":"("` returns 400 naming the pattern. +- [ ] A regex block rule blocks a matching name; `/api/lookup` reports + `rule_block_regex` and `matched` carries the pattern text. +- [ ] `zig build bench -Doptimize=ReleaseFast -- filter` passes its targets + with the regex variant present. +- [ ] PLAN §2.2 no longer forbids operator regex; all listed echoes updated. + +### Orchestrator + +Verify S1 and S2 acceptance before starting S3. After S3: run the full gate +set (`zig build test`, `test-aarch64` if qemu present, `npm test`, +`npm run assert-bundled`), then a live smoke against a scratch server: load +one real ABP list with `@@` lines, add one regex rule, verify both over dig +and `/api/lookup`. Record deviations in `## Recorded (implementation)`. + +## Module layout + +New files: +- `src/filter/regex.zig` — the linear-time engine (ruling 5). +- `tests/fuzz/regex_fuzz.zig` — its fuzz target (ruling 9). + +Deleted surface: none. + +## Acceptance (milestone complete) + +- [ ] All session acceptance boxes above. +- [ ] Schema at version 4; a v2 database migrates cleanly with data intact. +- [ ] A pre-milestone blocklist data directory loads without refetch. +- [ ] The six-level operator precedence plus three list levels behave per + ruling 2, proven by matcher tests that enumerate adjacent-level pairs. +- [ ] No `src/filter/` fuzz-root file imports anything but `std`. +- [ ] Contract samples, openapi.yaml and `web/src/lib/types.ts` agree with + the server (the drift guards pass). + +## Anti-requirements + +- No `$` modifier support beyond tolerating `$important` on exception lines. + `$dnstype`, `$dnsrewrite`, `$client`, `$denyallow` and every browser + modifier stay unsupported and counted. +- No regex from downloaded lists. `.regex` lines stay counted and skipped; + `skipped_regex` keeps its meaning. The engine exists for operator rules + only. +- No partial-segment wildcards (`ads*.example.com`) — writing one as a rule + stays rejected; the regex kind covers the need. +- No backreferences, lookaround, captures, named groups or Unicode classes in + the engine, ever. A pattern needing them is rejected, not approximated. +- No PCRE2, RE2 or any external regex dependency. +- No exception-rule UI editor: exceptions come from lists; operators write + allow rules. +- No re-download forced by the upgrade; checksum compatibility (ruling 3) is + a requirement, not an optimization.