fix(filter): surface rule-eval timeouts and instrument engine + webhook egress #369

Merged
KofTwentyTwo merged 1 commits from story/obs-4-filter-observability into dev 2026-07-31 23:34:51 +00:00
KofTwentyTwo commented 2026-07-31 23:33:24 +00:00 (Migrated from github.com)

Summary

  • Fixes a silent security-relevant bug in FilterEngine::evaluate_with_timeout: a ReDoS-guard timeout was consumed with unwrap_or_default(), silently treating a timed-out rule as "no match" with no signal that anything happened. The evaluation is now performed per-rule on a blocking-pool task (so the timeout can actually preempt a long-running synchronous evaluation) and a timeout now emits a warn! carrying the rule's id/name; the batch continues evaluating the remaining rules. Documented semantics: a timed-out rule is treated as non-matching (fail-closed, unchanged from before) but the outcome is now visible, never silent.
  • Adds observability instrumentation to nuncio-filter, which previously had zero tracing calls:
    • DEBUG on every rule match in evaluate() and evaluate_with_timeout() (rule id/name + action kinds only — never destination data like folder names, forward addresses, or webhook URLs).
    • INFO on reload_rules with the reloaded rule count.
    • Webhook egress (webhook.rs): each dispatch runs inside a tracing span carrying the target host and rule id. Every SSRF/CIDR/DNS-rebind block decision is logged: warn! with host + reason for a pass-6 policy block, warn! with host + blocked IP for a DNS-rebind block on the resolved address, debug! for an allowed egress. Network-level dispatch success/failure is also logged at debug/warn. No message bodies, secrets, or full URLs (only hostnames) are ever logged.

Test plan

  • cargo fmt --all -- --check
  • cargo check-all (clippy --all-targets, -D warnings)
  • cargo test-all (full workspace, 0 failed / 0 ignored)
  • New regression test: engine::tests::test_evaluate_with_timeout_logs_warn_and_treats_timeout_as_non_match — a deliberately slow condition evaluation with a 1ms timeout budget deterministically times out (spawn_blocking runs on a real thread so the timer genuinely races it), asserts the result is empty (documented non-match outcome) and that a WARN log carrying the rule id was emitted.
  • New regression tests in webhook.rs: test_blocked_webhook_logs_warn_with_host_and_reason (asserts a blocked SSRF target logs a WARN with host + reason, and that the HMAC secret never appears in any log line) and test_allowed_webhook_logs_debug_not_warn (asserts an allowed dispatch logs DEBUG only, no WARN).
  • Manually grepped the diff for secrets/PII in tracing macros — none found.

Refs #360

## Summary - Fixes a silent security-relevant bug in `FilterEngine::evaluate_with_timeout`: a ReDoS-guard timeout was consumed with `unwrap_or_default()`, silently treating a timed-out rule as "no match" with no signal that anything happened. The evaluation is now performed per-rule on a blocking-pool task (so the timeout can actually preempt a long-running synchronous evaluation) and a timeout now emits a `warn!` carrying the rule's id/name; the batch continues evaluating the remaining rules. Documented semantics: a timed-out rule is treated as non-matching (fail-closed, unchanged from before) but the outcome is now visible, never silent. - Adds observability instrumentation to `nuncio-filter`, which previously had zero `tracing` calls: - `DEBUG` on every rule match in `evaluate()` and `evaluate_with_timeout()` (rule id/name + action *kinds* only — never destination data like folder names, forward addresses, or webhook URLs). - `INFO` on `reload_rules` with the reloaded rule count. - Webhook egress (`webhook.rs`): each dispatch runs inside a `tracing` span carrying the target host and rule id. Every SSRF/CIDR/DNS-rebind block decision is logged: `warn!` with host + reason for a pass-6 policy block, `warn!` with host + blocked IP for a DNS-rebind block on the resolved address, `debug!` for an allowed egress. Network-level dispatch success/failure is also logged at debug/warn. No message bodies, secrets, or full URLs (only hostnames) are ever logged. ## Test plan - [x] `cargo fmt --all -- --check` - [x] `cargo check-all` (clippy --all-targets, `-D warnings`) - [x] `cargo test-all` (full workspace, 0 failed / 0 ignored) - [x] New regression test: `engine::tests::test_evaluate_with_timeout_logs_warn_and_treats_timeout_as_non_match` — a deliberately slow condition evaluation with a 1ms timeout budget deterministically times out (spawn_blocking runs on a real thread so the timer genuinely races it), asserts the result is empty (documented non-match outcome) and that a WARN log carrying the rule id was emitted. - [x] New regression tests in `webhook.rs`: `test_blocked_webhook_logs_warn_with_host_and_reason` (asserts a blocked SSRF target logs a WARN with host + reason, and that the HMAC secret never appears in any log line) and `test_allowed_webhook_logs_debug_not_warn` (asserts an allowed dispatch logs DEBUG only, no WARN). - [x] Manually grepped the diff for secrets/PII in tracing macros — none found. Refs #360
Sign in to join this conversation.