fix(filter): use unique rule ids to prevent silent overwrite #267

2026-07-31T16:30:58Z

KofTwentyTwo commented

2026-07-31 16:30:58 +00:00

(Migrated from github.com)

Summary

CreateRule derived a rule's id deterministically from sha256(name + nsql), so two rules with identical name and NSQL text produced the SAME id.
save_filter_rule persists via INSERT OR REPLACE with no existence check, so the second CreateRule silently overwrote the first rule with no warning.
Rule ids are now generated from a CSPRNG (the same OsRng nuncio-store already uses to source key material) instead of the rule's content, so identical name+NSQL always yields distinct ids. UpdateRule and list_filter_rules already reassign the id from the persisted row after parse_rule, so they are unaffected.

Test plan

New test create_rule_with_duplicate_name_and_nsql_does_not_overwrite_the_first (crates/nunciod/src/grpc.rs): creates two rules with identical name+NSQL, asserts distinct ids and that ListRules returns both.
Existing test update_rule_applies_partial_overrides_and_reloads_the_live_engine still passes, confirming UpdateRule preserves the original id across an edit.
cargo fmt --all -- --check
cargo check-all (clippy --all-targets -D warnings, 0 warnings)
cargo test-all (0 failed, 0 ignored)

Refs #249

## Summary - `CreateRule` derived a rule's id deterministically from `sha256(name + nsql)`, so two rules with identical name and NSQL text produced the SAME id. - `save_filter_rule` persists via `INSERT OR REPLACE` with no existence check, so the second `CreateRule` silently overwrote the first rule with no warning. - Rule ids are now generated from a CSPRNG (the same `OsRng` `nuncio-store` already uses to source key material) instead of the rule's content, so identical name+NSQL always yields distinct ids. `UpdateRule` and `list_filter_rules` already reassign the id from the persisted row after `parse_rule`, so they are unaffected. ## Test plan - [x] New test `create_rule_with_duplicate_name_and_nsql_does_not_overwrite_the_first` (crates/nunciod/src/grpc.rs): creates two rules with identical name+NSQL, asserts distinct ids and that `ListRules` returns both. - [x] Existing test `update_rule_applies_partial_overrides_and_reloads_the_live_engine` still passes, confirming `UpdateRule` preserves the original id across an edit. - [x] `cargo fmt --all -- --check` - [x] `cargo check-all` (clippy `--all-targets -D warnings`, 0 warnings) - [x] `cargo test-all` (0 failed, 0 ignored) Refs #249

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#267