fix(filter): correct account-field evaluation and reject header conditions #223

Merged
KofTwentyTwo merged 1 commits from story/m4-filter-correctness into dev 2026-07-30 00:19:37 +00:00
KofTwentyTwo commented 2026-07-30 00:18:23 +00:00 (Migrated from github.com)

Fixes #191.

Summary

Two confirmed bugs in CompiledFilter::eval_leaf (crates/nuncio-filter/src/engine.rs):

  1. account condition field read the wrong column. FilterField::Folder | FilterField::Account was a single match arm comparing both against email.folder_id, so WHERE account = 'acct-a' would false-positive on any message sharing a folder with an acct-a message, regardless of its real account. Split into two arms: Folder still compares email.folder_id; Account now compares email.account_id. This is unrelated to the rule-level ON ACCOUNT scope in FilterRule::matches_account (ast.rs), which was already correct and untouched.

  2. header[...] field silently evaluated to false forever. nuncio_core::model::Email has no parsed-headers field, so there was genuinely nothing to match against. Rather than fake header matching against Subject/From (fabrication), NsqlValidator::pass1_field_types now rejects any header[...] condition with ValidationError::FieldTypeMismatch("header field conditions are not yet supported: message headers are not indexed"). Since validate is called from CreateRule/ValidateRule/PreviewRule, all three RPCs (and their CLI commands) now honestly reject header rules instead of accepting a rule that can never fire. The runtime FilterField::Header(_) => false arm is kept only for match exhaustiveness and its doc comment now states it is an unreachable defensive branch.

No proto change. Engine-only (crates/nuncio-filter/{engine.rs,validator.rs}).

Tests added

  • engine::tests::test_account_condition_field_matches_account_id_not_folder_id — constructs the exact "same folder, different account" collision and proves WHERE account = 'acct-a' matches only by account_id.
  • validator::tests::test_pass1_rejects_unsupported_header_field_conditions — proves header['X-Spam-Score'] = '10' is rejected via FieldTypeMismatch.

Sanity-checked the existing grpc.rs filter tests (create_rule_rejects_invalid_nsql_and_persists_nothing, validate_rule_reports_honest_ok_and_error_verdicts_without_persisting, preview_rule_and_validate_rule_reject_invalid_nsql_syntax) — unaffected, still passing.

Gate output

$ cargo fmt --all -- --check
(no output — clean)

$ cargo check-all
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 3.04s
(zero clippy warnings across workspace)

$ cargo test-all
nuncio-filter unit tests: 26 passed; 0 failed; 0 ignored
nuncio-filter webhook_integration_test: 2 passed; 0 failed; 0 ignored
Full workspace (cargo test --workspace): 388 passed; 0 failed; 0 ignored across all suites

Pre-commit hook (fmt + clippy --all-targets -D warnings + full test suite) ran on commit and passed.

Fixes #191. ## Summary Two confirmed bugs in `CompiledFilter::eval_leaf` (`crates/nuncio-filter/src/engine.rs`): 1. **`account` condition field read the wrong column.** `FilterField::Folder | FilterField::Account` was a single match arm comparing both against `email.folder_id`, so `WHERE account = 'acct-a'` would false-positive on any message sharing a folder with an acct-a message, regardless of its real account. Split into two arms: `Folder` still compares `email.folder_id`; `Account` now compares `email.account_id`. This is unrelated to the rule-level `ON ACCOUNT` scope in `FilterRule::matches_account` (`ast.rs`), which was already correct and untouched. 2. **`header[...]` field silently evaluated to `false` forever.** `nuncio_core::model::Email` has no parsed-headers field, so there was genuinely nothing to match against. Rather than fake header matching against Subject/From (fabrication), `NsqlValidator::pass1_field_types` now rejects any `header[...]` condition with `ValidationError::FieldTypeMismatch("header field conditions are not yet supported: message headers are not indexed")`. Since `validate` is called from `CreateRule`/`ValidateRule`/`PreviewRule`, all three RPCs (and their CLI commands) now honestly reject header rules instead of accepting a rule that can never fire. The runtime `FilterField::Header(_) => false` arm is kept only for match exhaustiveness and its doc comment now states it is an unreachable defensive branch. No proto change. Engine-only (`crates/nuncio-filter/{engine.rs,validator.rs}`). ## Tests added - `engine::tests::test_account_condition_field_matches_account_id_not_folder_id` — constructs the exact "same folder, different account" collision and proves `WHERE account = 'acct-a'` matches only by `account_id`. - `validator::tests::test_pass1_rejects_unsupported_header_field_conditions` — proves `header['X-Spam-Score'] = '10'` is rejected via `FieldTypeMismatch`. Sanity-checked the existing `grpc.rs` filter tests (`create_rule_rejects_invalid_nsql_and_persists_nothing`, `validate_rule_reports_honest_ok_and_error_verdicts_without_persisting`, `preview_rule_and_validate_rule_reject_invalid_nsql_syntax`) — unaffected, still passing. ## Gate output ``` $ cargo fmt --all -- --check (no output — clean) $ cargo check-all Finished `dev` profile [unoptimized + debuginfo] target(s) in 3.04s (zero clippy warnings across workspace) $ cargo test-all nuncio-filter unit tests: 26 passed; 0 failed; 0 ignored nuncio-filter webhook_integration_test: 2 passed; 0 failed; 0 ignored Full workspace (cargo test --workspace): 388 passed; 0 failed; 0 ignored across all suites ``` Pre-commit hook (fmt + clippy --all-targets -D warnings + full test suite) ran on commit and passed.
Sign in to join this conversation.