filter edit silently re-enables disabled rules and resets created_at #225

Open
opened 2026-07-30 00:37:46 +00:00 by KofTwentyTwo · 0 comments
KofTwentyTwo commented 2026-07-30 00:37:46 +00:00 (Migrated from github.com)

Found during the M4-S3 (#192) adversarial review. FiltersGrpcService::update_rule reconstructs the rule by re-parsing NSQL via NsqlParser::parse_rule, which unconditionally sets enabled: true and resets created_at. Consequence: editing a disabled rule (e.g. changing only its name or priority) silently re-enables it and loses its original creation timestamp.

Not a regression — this faithfully matches the retired CLI-local filter edit behavior and create_rule, so it was correctly judged non-blocking for #192. But it is surprising, user-visible behavior worth fixing: update_rule should preserve the existing rule's enabled flag and created_at unless the caller explicitly changes them.

Fix sketch: in update_rule, after re-parsing, copy enabled and created_at from the fetched existing rule (only the fields actually supplied in the request should change). Add a regression test: edit a disabled rule's priority, assert it stays disabled and keeps its original created_at.

Scope: crates/nunciod/src/grpc.rs (update_rule). Small. No proto change.

Found during the M4-S3 (#192) adversarial review. `FiltersGrpcService::update_rule` reconstructs the rule by re-parsing NSQL via `NsqlParser::parse_rule`, which unconditionally sets `enabled: true` and resets `created_at`. Consequence: editing a **disabled** rule (e.g. changing only its name or priority) silently re-enables it and loses its original creation timestamp. Not a regression — this faithfully matches the retired CLI-local `filter edit` behavior and `create_rule`, so it was correctly judged non-blocking for #192. But it is surprising, user-visible behavior worth fixing: `update_rule` should preserve the existing rule's `enabled` flag and `created_at` unless the caller explicitly changes them. Fix sketch: in `update_rule`, after re-parsing, copy `enabled` and `created_at` from the fetched existing rule (only the fields actually supplied in the request should change). Add a regression test: edit a disabled rule's priority, assert it stays disabled and keeps its original `created_at`. Scope: `crates/nunciod/src/grpc.rs` (`update_rule`). Small. No proto change.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#225