From bdc9c72d5dcdeff476d7831f63f442cd7301ed6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois-Xavier=20THIRY?= Date: Fri, 17 Apr 2026 10:35:13 +0200 Subject: [PATCH] chore(release): v2.0.1 hardening patch (secrets, migration UX, gauge debounce) Three post-v2.0.0 fixes bundled into one coherent "durability" patch. - Notifier config secrets (bot_token, webhook_url, Webhook headers, SMTP password) are now SecretString, so a format!("{:?}", config) or a tracing context carrying the parsed config renders [REDACTED] instead of leaking the secret. Regression-guard test notifier_config_debug_never_leaks_secrets runs canaries through each notifier config's Debug output. - The migration error emitted when a v1.x config is loaded against v2.x now leads with a human-readable incompatibility line, includes the before/after YAML diff, a direct link to MIGRATION.md, and a rollback hint. - valerter_vl_source_up gauge is now debounced to 3 consecutive failures before flipping to 0, via VL_SOURCE_UP_FAILURE_THRESHOLD. Transient 5xx / EOF / timeout no longer page. Flip back to 1 on any single success, counter resets. No breaking changes. No new features. Dashboards and alerts are unchanged (the gauge contract for a persistently-down source is identical). --- CHANGELOG.md | 14 ++++++ Cargo.lock | 2 +- Cargo.toml | 2 +- src/config/notifiers.rs | 75 +++++++++++++++++++++++++++---- src/config/types.rs | 6 +-- src/notify/email.rs | 12 ++--- src/notify/registry.rs | 27 +++++------ src/notify/telegram.rs | 15 ++++--- src/notify/tests.rs | 47 ++++++++++--------- src/notify/webhook.rs | 55 +++++++++++++---------- src/tail.rs | 90 +++++++++++++++++++++++++++++-------- tests/integration_notify.rs | 7 ++- 12 files changed, 248 insertions(+), 104 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa0bc61..28d31d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,20 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.0.1] - 2026-04-17 + +Hardening patch after the v2.0.0 release. No new features, no breaking changes. Three fixes bundled into one coherent "post-v2.0.0 durability" release. + +### Security + +- **Notifier config secrets wrapped in `SecretString`**. `TelegramNotifierConfig.bot_token`, `MattermostNotifierConfig.webhook_url`, `WebhookNotifierConfig.url`, `WebhookNotifierConfig.headers` values, and `SmtpConfig.password` are now `SecretString` rather than raw `String`. A `format!("{:?}", config)` or a tracing context that captures the parsed config renders `[REDACTED]` instead of the actual value. Prior to this, the token lived as a plain `String` until notifier construction time, so any debug-log or error context that carried the config body would leak the secret. A regression-guard unit test (`notifier_config_debug_never_leaks_secrets`) runs canary values through each notifier config's `Debug` output and asserts none appear. + +### Fixed + +- **Migration error on v1.x configs now points to MIGRATION.md and covers rollback** (D-doc-1). Loading a v1.x `victorialogs.url`-shape config against v2.x used to emit a serde-flavoured error that referenced the CHANGELOG only. The new message leads with a human-readable "Configuration incompatible with valerter v2.0.0", includes the before/after YAML diff, a direct link to `MIGRATION.md`, and a rollback hint pointing at the GitHub releases page. + +- **`valerter_vl_source_up` gauge debounced to 3 consecutive failures** (D-vl-obs-1). The gauge used to flip to `0` on any single HTTP 5xx, connection error, or mid-stream EOF, which made sources behind a flaky load balancer flap their reachability state and page operators on transient events. The flip is now gated by `VL_SOURCE_UP_FAILURE_THRESHOLD = 3` (fixed, not configurable): three consecutive failures before `0`, any single success resets the counter back to `1` and re-arms the debounce. Contract unchanged for persistently-down sources. + ## [2.0.0] - 2026-04-16 ### Security advisory diff --git a/Cargo.lock b/Cargo.lock index 942ab98..d09f706 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2334,7 +2334,7 @@ dependencies = [ [[package]] name = "valerter" -version = "2.0.0" +version = "2.0.1" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index 0f1e8cd..03009a1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "valerter" -version = "2.0.0" +version = "2.0.1" edition = "2024" description = "Real-time log alerting for VictoriaLogs" license = "Apache-2.0" diff --git a/src/config/notifiers.rs b/src/config/notifiers.rs index def5f85..7b1e2bb 100644 --- a/src/config/notifiers.rs +++ b/src/config/notifiers.rs @@ -1,5 +1,6 @@ //! Notifier configurations (Mattermost, Webhook, Email). +use super::secret::SecretString; use serde::Deserialize; use std::collections::HashMap; @@ -26,7 +27,7 @@ pub enum NotifierConfig { #[serde(deny_unknown_fields)] pub struct MattermostNotifierConfig { /// Webhook URL (supports `${ENV_VAR}` substitution). - pub webhook_url: String, + pub webhook_url: SecretString, #[serde(default)] pub channel: Option, #[serde(default)] @@ -40,11 +41,11 @@ pub struct MattermostNotifierConfig { #[serde(deny_unknown_fields)] pub struct WebhookNotifierConfig { /// Target URL (supports `${ENV_VAR}` substitution). - pub url: String, + pub url: SecretString, #[serde(default = "default_post")] pub method: String, #[serde(default)] - pub headers: HashMap, + pub headers: HashMap, #[serde(default)] pub body_template: Option, } @@ -68,7 +69,7 @@ pub struct EmailNotifierConfig { #[serde(deny_unknown_fields)] pub struct TelegramNotifierConfig { /// Bot API token (supports `${ENV_VAR}` substitution). - pub bot_token: String, + pub bot_token: SecretString, /// One or more Telegram chat IDs to send messages to. Must be non-empty. pub chat_ids: Vec, /// Telegram parse mode for the message text. Defaults to `HTML`. @@ -94,7 +95,7 @@ pub struct SmtpConfig { #[serde(default)] pub username: Option, #[serde(default)] - pub password: Option, + pub password: Option, #[serde(default)] pub tls: TlsMode, #[serde(default = "default_true")] @@ -133,7 +134,7 @@ mod tests { let config: NotifierConfig = serde_yaml::from_str(yaml).unwrap(); match config { NotifierConfig::Mattermost(cfg) => { - assert_eq!(cfg.webhook_url, "https://example.com/hooks/test"); + assert_eq!(cfg.webhook_url.expose(), "https://example.com/hooks/test"); assert!(cfg.channel.is_none()); } _ => panic!("Expected Mattermost variant"), @@ -194,7 +195,7 @@ mod tests { let config: NotifierConfig = serde_yaml::from_str(yaml).unwrap(); match config { NotifierConfig::Telegram(cfg) => { - assert_eq!(cfg.bot_token, "${TELEGRAM_BOT_TOKEN}"); + assert_eq!(cfg.bot_token.expose(), "${TELEGRAM_BOT_TOKEN}"); assert_eq!(cfg.chat_ids.len(), 2); assert_eq!(cfg.parse_mode.as_deref(), Some("HTML")); assert_eq!(cfg.disable_notification, Some(false)); @@ -215,7 +216,7 @@ mod tests { let config: NotifierConfig = serde_yaml::from_str(yaml).unwrap(); match config { NotifierConfig::Telegram(cfg) => { - assert_eq!(cfg.bot_token, "token123"); + assert_eq!(cfg.bot_token.expose(), "token123"); assert_eq!(cfg.chat_ids, vec!["-100".to_string()]); assert!(cfg.parse_mode.is_none()); assert!(cfg.disable_notification.is_none()); @@ -305,7 +306,7 @@ mod tests { let config: NotifierConfig = serde_yaml::from_str(yaml).unwrap(); match config { NotifierConfig::Webhook(cfg) => { - assert_eq!(cfg.url, "https://api.example.com/alerts"); + assert_eq!(cfg.url.expose(), "https://api.example.com/alerts"); assert_eq!(cfg.method, "PUT"); assert_eq!(cfg.headers.len(), 2); assert!(cfg.body_template.is_some()); @@ -556,4 +557,60 @@ mod tests { assert_eq!(config.destinations.len(), 1); assert_eq!(config.destinations[0], "mattermost-infra"); } + + /// Regression guard: notifier configs must never leak secrets through + /// `Debug` (which is what `tracing::debug!`, `anyhow::Error` contexts, + /// and support-ticket config dumps all end up calling). + #[test] + fn notifier_config_debug_never_leaks_secrets() { + const CANARY_TOKEN: &str = "CANARY_BOT_TOKEN_abc123xyz"; + const CANARY_URL: &str = "https://canary.example.com/hooks/SECRET-hooks-path"; + const CANARY_HEADER: &str = "CANARY_BEARER_SECRET_xyz789"; + const CANARY_PASSWORD: &str = "CANARY_SMTP_PASSWORD_qwerty"; + + let telegram_yaml = format!( + "type: telegram\nbot_token: \"{}\"\nchat_ids: [\"-100\"]\n", + CANARY_TOKEN + ); + let mattermost_yaml = format!("type: mattermost\nwebhook_url: \"{}\"\n", CANARY_URL); + let webhook_yaml = format!( + "type: webhook\nurl: \"{}\"\nheaders:\n Authorization: \"Bearer {}\"\n", + CANARY_URL, CANARY_HEADER + ); + let email_yaml = format!( + "type: email\nsmtp:\n host: smtp.example.com\n port: 587\n username: u\n password: \"{}\"\nfrom: f@example.com\nto: [t@example.com]\nsubject_template: s\n", + CANARY_PASSWORD + ); + + let configs: Vec<(&str, NotifierConfig)> = vec![ + ("telegram", serde_yaml::from_str(&telegram_yaml).unwrap()), + ( + "mattermost", + serde_yaml::from_str(&mattermost_yaml).unwrap(), + ), + ("webhook", serde_yaml::from_str(&webhook_yaml).unwrap()), + ("email", serde_yaml::from_str(&email_yaml).unwrap()), + ]; + + let canaries = [CANARY_TOKEN, CANARY_URL, CANARY_HEADER, CANARY_PASSWORD]; + + for (kind, cfg) in &configs { + let dbg = format!("{:?}", cfg); + for canary in &canaries { + assert!( + !dbg.contains(canary), + "{} Debug leaked secret canary '{}': {}", + kind, + canary, + dbg + ); + } + assert!( + dbg.contains("REDACTED"), + "{} Debug should render secrets as [REDACTED]: {}", + kind, + dbg + ); + } + } } diff --git a/src/config/types.rs b/src/config/types.rs index 066dd4a..3e4fb84 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -83,7 +83,7 @@ struct LegacyVictoriaLogsConfig { /// Migration error text pointing users from the v1 single-URL shape to the /// v2 map shape. Exposed so tests can assert wording. -pub(crate) const LEGACY_VL_MIGRATION_MESSAGE: &str = "`victorialogs` is now a map of named sources (breaking change in v2.0.0).\n\nMigrate from:\n victorialogs:\n url: \"http://...\"\n basic_auth:\n username: \"u\"\n password: \"p\"\nTo:\n victorialogs:\n default:\n url: \"http://...\"\n basic_auth:\n username: \"u\"\n password: \"p\"\n\nThen optionally target sources per rule via `vl_sources: [default]` (or omit to fan out across all sources). See CHANGELOG v2.0.0 for the full migration note."; +pub(crate) const LEGACY_VL_MIGRATION_MESSAGE: &str = "\nConfiguration incompatible with valerter v2.0.0.\n\nYour config uses the v1.x single-URL shape (`victorialogs.url`), replaced by a map of named sources in v2.\n\nMigrate from:\n victorialogs:\n url: \"http://...\"\n basic_auth:\n username: \"u\"\n password: \"p\"\nTo:\n victorialogs:\n default:\n url: \"http://...\"\n basic_auth:\n username: \"u\"\n password: \"p\"\n\nThen optionally target sources per rule via `vl_sources: [default]` (or omit to fan out across all sources).\n\nFull migration guide: https://github.com/fxthiry/valerter/blob/main/MIGRATION.md\nRollback: install the last v1.x release from https://github.com/fxthiry/valerter/releases"; /// Deserialize `victorialogs` as `BTreeMap`, but /// emit a migration-oriented error when the legacy single-object shape @@ -615,7 +615,7 @@ impl Config { for (name, notifier) in notifiers { match notifier { super::notifiers::NotifierConfig::Mattermost(cfg) => { - if let Err(e) = validate_url(&cfg.webhook_url) { + if let Err(e) = validate_url(cfg.webhook_url.expose()) { errors.push(ConfigError::ValidationError(format!( "notifier '{}': webhook_url: {}", name, e @@ -623,7 +623,7 @@ impl Config { } } super::notifiers::NotifierConfig::Webhook(cfg) => { - if let Err(e) = validate_url(&cfg.url) { + if let Err(e) = validate_url(cfg.url.expose()) { errors.push(ConfigError::ValidationError(format!( "notifier '{}': url: {}", name, e diff --git a/src/notify/email.rs b/src/notify/email.rs index ac6ffae..d311cdb 100644 --- a/src/notify/email.rs +++ b/src/notify/email.rs @@ -160,7 +160,7 @@ impl EmailNotifier { .smtp .password .as_ref() - .map(|p| resolve_env_vars(p)) + .map(|p| resolve_env_vars(p.expose())) .transpose() .map_err(|e| ConfigError::InvalidNotifier { name: name.to_string(), @@ -661,7 +661,7 @@ impl std::fmt::Debug for EmailNotifier { #[cfg(test)] mod tests { use super::*; - use crate::config::{EmailNotifierConfig, SmtpConfig, TlsMode}; + use crate::config::{EmailNotifierConfig, SecretString, SmtpConfig, TlsMode}; use crate::template::RenderedMessage; use serial_test::serial; use std::sync::Mutex; @@ -938,7 +938,7 @@ mod tests { fn from_config_fails_with_password_without_username() { let mut config = make_test_config(); config.smtp.username = None; - config.smtp.password = Some("pass".to_string()); + config.smtp.password = Some(SecretString::new("pass".to_string())); let result = EmailNotifier::from_config("no-username", &config, &test_config_dir()); @@ -964,7 +964,7 @@ mod tests { || { let mut config = make_test_config(); config.smtp.username = Some("${TEST_SMTP_USER}".to_string()); - config.smtp.password = Some("${TEST_SMTP_PASS}".to_string()); + config.smtp.password = Some(SecretString::new("${TEST_SMTP_PASS}".to_string())); let result = EmailNotifier::from_config("env-creds", &config, &test_config_dir()); @@ -985,7 +985,7 @@ mod tests { temp_env::with_var("UNDEFINED_SMTP_VAR", None::<&str>, || { let mut config = make_test_config(); config.smtp.username = Some("${UNDEFINED_SMTP_VAR}".to_string()); - config.smtp.password = Some("somepass".to_string()); + config.smtp.password = Some(SecretString::new("somepass".to_string())); let result = EmailNotifier::from_config("bad-env", &config, &test_config_dir()); @@ -1168,7 +1168,7 @@ mod tests { || { let mut config = make_test_config(); config.smtp.username = Some("${TEST_DBG_USER}".to_string()); - config.smtp.password = Some("${TEST_DBG_PASS}".to_string()); + config.smtp.password = Some(SecretString::new("${TEST_DBG_PASS}".to_string())); let notifier = EmailNotifier::from_config("cred-email", &config, &test_config_dir()).unwrap(); diff --git a/src/notify/registry.rs b/src/notify/registry.rs index e71570c..0a6da4e 100644 --- a/src/notify/registry.rs +++ b/src/notify/registry.rs @@ -158,19 +158,20 @@ impl NotifierRegistry { match config { NotifierConfig::Mattermost(mm_config) => { // Resolve environment variables in webhook_url - let resolved_url = resolve_env_vars(&mm_config.webhook_url).map_err(|e| { - // Track env var resolution failures for monitoring (Fix M1) - metrics::counter!( - "valerter_notifier_config_errors_total", - "notifier" => name.to_string(), - "error_type" => "env_var_resolution" - ) - .increment(1); - ConfigError::InvalidNotifier { - name: name.to_string(), - message: format!("webhook_url: {}", e), - } - })?; + let resolved_url = + resolve_env_vars(mm_config.webhook_url.expose()).map_err(|e| { + // Track env var resolution failures for monitoring (Fix M1) + metrics::counter!( + "valerter_notifier_config_errors_total", + "notifier" => name.to_string(), + "error_type" => "env_var_resolution" + ) + .increment(1); + ConfigError::InvalidNotifier { + name: name.to_string(), + message: format!("webhook_url: {}", e), + } + })?; let notifier = MattermostNotifier::with_options( name.to_string(), diff --git a/src/notify/telegram.rs b/src/notify/telegram.rs index 2f21849..80c7f7e 100644 --- a/src/notify/telegram.rs +++ b/src/notify/telegram.rs @@ -247,11 +247,12 @@ impl TelegramNotifier { }); } - let resolved_token = - resolve_env_vars(&config.bot_token).map_err(|e| ConfigError::InvalidNotifier { + let resolved_token = resolve_env_vars(config.bot_token.expose()).map_err(|e| { + ConfigError::InvalidNotifier { name: name.to_string(), message: format!("bot_token: {}", e), - })?; + } + })?; if resolved_token.trim().is_empty() { return Err(ConfigError::InvalidNotifier { name: name.to_string(), @@ -537,7 +538,7 @@ mod tests { fn config_with(chat_ids: Vec) -> TelegramNotifierConfig { TelegramNotifierConfig { - bot_token: "fake-token".to_string(), + bot_token: SecretString::new("fake-token".to_string()), chat_ids, parse_mode: None, disable_notification: None, @@ -603,7 +604,7 @@ mod tests { fn from_config_fails_fast_on_unresolved_env_var() { let client = reqwest::Client::new(); let mut cfg = config_with(vec!["-100".to_string()]); - cfg.bot_token = "${VALERTER_TELEGRAM_TEST_MISSING_VAR}".to_string(); + cfg.bot_token = SecretString::new("${VALERTER_TELEGRAM_TEST_MISSING_VAR}".to_string()); let err = TelegramNotifier::from_config("tg", &cfg, client).unwrap_err(); assert!(matches!(err, ConfigError::InvalidNotifier { .. })); assert!(err.to_string().contains("bot_token")); @@ -644,7 +645,7 @@ mod tests { fn debug_impl_does_not_leak_token_or_endpoint() { let client = reqwest::Client::new(); let mut cfg = config_with(vec!["-100".to_string()]); - cfg.bot_token = "SUPER_SECRET_TOKEN".to_string(); + cfg.bot_token = SecretString::new("SUPER_SECRET_TOKEN".to_string()); let notifier = TelegramNotifier::from_config("tg", &cfg, client).unwrap(); let dbg = format!("{:?}", notifier); assert!(dbg.contains("TelegramNotifier")); @@ -756,7 +757,7 @@ mod tests { fn from_config_rejects_empty_resolved_bot_token() { let client = reqwest::Client::new(); let mut cfg = config_with(vec!["-100".to_string()]); - cfg.bot_token = " ".to_string(); + cfg.bot_token = SecretString::new(" ".to_string()); let err = TelegramNotifier::from_config("tg", &cfg, client).unwrap_err(); assert!(matches!(err, ConfigError::InvalidNotifier { .. })); assert!(err.to_string().contains("bot_token")); diff --git a/src/notify/tests.rs b/src/notify/tests.rs index b7a88c0..7c05ca0 100644 --- a/src/notify/tests.rs +++ b/src/notify/tests.rs @@ -8,7 +8,7 @@ use std::sync::Arc; use tokio::sync::broadcast; use crate::config::{ - EmailNotifierConfig, MattermostNotifierConfig, NotifierConfig, SmtpConfig, + EmailNotifierConfig, MattermostNotifierConfig, NotifierConfig, SecretString, SmtpConfig, TelegramNotifierConfig, TlsMode, WebhookNotifierConfig, }; use crate::error::NotifyError; @@ -234,7 +234,7 @@ fn registry_from_config_creates_mattermost_notifiers() { notifiers_config.insert( "mattermost-infra".to_string(), NotifierConfig::Mattermost(MattermostNotifierConfig { - webhook_url: "${TEST_MM_WEBHOOK}".to_string(), + webhook_url: SecretString::new("${TEST_MM_WEBHOOK}".to_string()), channel: Some("infra-alerts".to_string()), username: Some("valerter".to_string()), icon_url: None, @@ -243,7 +243,9 @@ fn registry_from_config_creates_mattermost_notifiers() { notifiers_config.insert( "mattermost-ops".to_string(), NotifierConfig::Mattermost(MattermostNotifierConfig { - webhook_url: "https://static.example.com/hooks/static".to_string(), + webhook_url: SecretString::new( + "https://static.example.com/hooks/static".to_string(), + ), channel: None, username: None, icon_url: None, @@ -282,7 +284,7 @@ fn registry_from_config_fails_on_undefined_env_var() { notifiers_config.insert( "bad-notifier".to_string(), NotifierConfig::Mattermost(MattermostNotifierConfig { - webhook_url: "${UNDEFINED_WEBHOOK_VAR}".to_string(), + webhook_url: SecretString::new("${UNDEFINED_WEBHOOK_VAR}".to_string()), channel: None, username: None, icon_url: None, @@ -324,7 +326,7 @@ fn registry_from_config_collects_all_errors() { notifiers_config.insert( "bad-notifier-1".to_string(), NotifierConfig::Mattermost(MattermostNotifierConfig { - webhook_url: "${UNDEFINED_VAR_1}".to_string(), + webhook_url: SecretString::new("${UNDEFINED_VAR_1}".to_string()), channel: None, username: None, icon_url: None, @@ -333,7 +335,7 @@ fn registry_from_config_collects_all_errors() { notifiers_config.insert( "bad-notifier-2".to_string(), NotifierConfig::Mattermost(MattermostNotifierConfig { - webhook_url: "${UNDEFINED_VAR_2}".to_string(), + webhook_url: SecretString::new("${UNDEFINED_VAR_2}".to_string()), channel: None, username: None, icon_url: None, @@ -374,15 +376,18 @@ fn registry_from_config_creates_webhook_notifiers() { notifiers_config.insert( "webhook-alerts".to_string(), NotifierConfig::Webhook(WebhookNotifierConfig { - url: "https://api.example.com/alerts".to_string(), + url: SecretString::new("https://api.example.com/alerts".to_string()), method: "POST".to_string(), headers: { let mut h = std::collections::HashMap::new(); h.insert( "Authorization".to_string(), - "Bearer ${TEST_WEBHOOK_TOKEN}".to_string(), + SecretString::new("Bearer ${TEST_WEBHOOK_TOKEN}".to_string()), + ); + h.insert( + "Content-Type".to_string(), + SecretString::new("application/json".to_string()), ); - h.insert("Content-Type".to_string(), "application/json".to_string()); h }, body_template: Some(r#"{"alert": "{{ title }}"}"#.to_string()), @@ -414,7 +419,7 @@ fn registry_from_config_webhook_with_defaults() { notifiers_config.insert( "simple-webhook".to_string(), NotifierConfig::Webhook(WebhookNotifierConfig { - url: "https://api.example.com/hook".to_string(), + url: SecretString::new("https://api.example.com/hook".to_string()), method: "POST".to_string(), headers: std::collections::HashMap::new(), body_template: None, @@ -441,13 +446,13 @@ fn registry_from_config_webhook_fails_on_undefined_env_var() { notifiers_config.insert( "bad-webhook".to_string(), NotifierConfig::Webhook(WebhookNotifierConfig { - url: "https://api.example.com/alerts".to_string(), + url: SecretString::new("https://api.example.com/alerts".to_string()), method: "POST".to_string(), headers: { let mut h = std::collections::HashMap::new(); h.insert( "Authorization".to_string(), - "Bearer ${UNDEFINED_WEBHOOK_TOKEN}".to_string(), + SecretString::new("Bearer ${UNDEFINED_WEBHOOK_TOKEN}".to_string()), ); h }, @@ -489,7 +494,7 @@ fn registry_from_config_mixed_notifiers() { notifiers_config.insert( "mattermost-1".to_string(), NotifierConfig::Mattermost(MattermostNotifierConfig { - webhook_url: "${TEST_MM_WEBHOOK_MIXED}".to_string(), + webhook_url: SecretString::new("${TEST_MM_WEBHOOK_MIXED}".to_string()), channel: None, username: None, icon_url: None, @@ -498,13 +503,13 @@ fn registry_from_config_mixed_notifiers() { notifiers_config.insert( "webhook-1".to_string(), NotifierConfig::Webhook(WebhookNotifierConfig { - url: "https://api.example.com/alerts".to_string(), + url: SecretString::new("https://api.example.com/alerts".to_string()), method: "POST".to_string(), headers: { let mut h = std::collections::HashMap::new(); h.insert( "X-API-Key".to_string(), - "${TEST_WH_TOKEN_MIXED}".to_string(), + SecretString::new("${TEST_WH_TOKEN_MIXED}".to_string()), ); h }, @@ -767,7 +772,7 @@ fn registry_from_config_email_with_auth() { host: "smtp.example.com".to_string(), port: 587, username: Some("${TEST_SMTP_USER_REG}".to_string()), - password: Some("${TEST_SMTP_PASS_REG}".to_string()), + password: Some(SecretString::new("${TEST_SMTP_PASS_REG}".to_string())), tls: TlsMode::Starttls, tls_verify: true, }, @@ -806,7 +811,7 @@ fn registry_from_config_email_fails_on_undefined_env_var() { host: "smtp.example.com".to_string(), port: 587, username: Some("${UNDEFINED_SMTP_VAR_REG}".to_string()), - password: Some("somepass".to_string()), + password: Some(SecretString::new("somepass".to_string())), tls: TlsMode::Starttls, tls_verify: true, }, @@ -886,7 +891,7 @@ fn registry_from_config_all_three_notifier_types() { notifiers_config.insert( "mattermost".to_string(), NotifierConfig::Mattermost(MattermostNotifierConfig { - webhook_url: "${TEST_MM_ALL_TYPES}".to_string(), + webhook_url: SecretString::new("${TEST_MM_ALL_TYPES}".to_string()), channel: None, username: None, icon_url: None, @@ -897,7 +902,7 @@ fn registry_from_config_all_three_notifier_types() { notifiers_config.insert( "webhook".to_string(), NotifierConfig::Webhook(WebhookNotifierConfig { - url: "https://api.example.com/alerts".to_string(), + url: SecretString::new("https://api.example.com/alerts".to_string()), method: "POST".to_string(), headers: std::collections::HashMap::new(), body_template: None, @@ -953,7 +958,7 @@ fn registry_from_config_creates_telegram_notifier() { notifiers_config.insert( "telegram-infra".to_string(), NotifierConfig::Telegram(TelegramNotifierConfig { - bot_token: "fake-token-123".to_string(), + bot_token: SecretString::new("fake-token-123".to_string()), chat_ids: vec!["-100123".to_string(), "-100456".to_string()], parse_mode: Some("HTML".to_string()), disable_notification: None, @@ -987,7 +992,7 @@ fn registry_from_config_propagates_telegram_validation_errors() { notifiers_config.insert( "telegram-broken".to_string(), NotifierConfig::Telegram(TelegramNotifierConfig { - bot_token: "fake-token".to_string(), + bot_token: SecretString::new("fake-token".to_string()), chat_ids: vec![], parse_mode: None, disable_notification: None, diff --git a/src/notify/webhook.rs b/src/notify/webhook.rs index ca16e6d..b24288f 100644 --- a/src/notify/webhook.rs +++ b/src/notify/webhook.rs @@ -149,7 +149,7 @@ impl WebhookNotifier { ) -> Result { // Resolve environment variables in URL let resolved_url = - resolve_env_vars(&config.url).map_err(|e| ConfigError::InvalidNotifier { + resolve_env_vars(config.url.expose()).map_err(|e| ConfigError::InvalidNotifier { name: name.to_string(), message: format!("url: {}", e), })?; @@ -174,7 +174,7 @@ impl WebhookNotifier { let mut headers = HeaderMap::new(); for (key, value) in &config.headers { let resolved_value = - resolve_env_vars(value).map_err(|e| ConfigError::InvalidNotifier { + resolve_env_vars(value.expose()).map_err(|e| ConfigError::InvalidNotifier { name: name.to_string(), message: format!("header '{}': {}", key, e), })?; @@ -412,15 +412,18 @@ mod tests { fn from_config_with_all_fields() { temp_env::with_var("TEST_WEBHOOK_TOKEN", Some("secret-token-123"), || { let config = WebhookNotifierConfig { - url: "https://api.example.com/alerts".to_string(), + url: SecretString::new("https://api.example.com/alerts".to_string()), method: "PUT".to_string(), headers: { let mut h = HashMap::new(); h.insert( "Authorization".to_string(), - "Bearer ${TEST_WEBHOOK_TOKEN}".to_string(), + SecretString::new("Bearer ${TEST_WEBHOOK_TOKEN}".to_string()), + ); + h.insert( + "Content-Type".to_string(), + SecretString::new("application/json".to_string()), ); - h.insert("Content-Type".to_string(), "application/json".to_string()); h }, body_template: Some(r#"{"alert": "{{ title }}"}"#.to_string()), @@ -440,7 +443,7 @@ mod tests { #[test] fn from_config_with_defaults() { let config = WebhookNotifierConfig { - url: "https://api.example.com/alerts".to_string(), + url: SecretString::new("https://api.example.com/alerts".to_string()), method: "POST".to_string(), headers: HashMap::new(), body_template: None, @@ -462,7 +465,7 @@ mod tests { Some("https://resolved.example.com/hook"), || { let config = WebhookNotifierConfig { - url: "${TEST_WEBHOOK_URL}".to_string(), + url: SecretString::new("${TEST_WEBHOOK_URL}".to_string()), method: "POST".to_string(), headers: HashMap::new(), body_template: None, @@ -482,7 +485,7 @@ mod tests { fn from_config_fails_on_undefined_url_env_var() { temp_env::with_var("UNDEFINED_WEBHOOK_URL", None::<&str>, || { let config = WebhookNotifierConfig { - url: "${UNDEFINED_WEBHOOK_URL}".to_string(), + url: SecretString::new("${UNDEFINED_WEBHOOK_URL}".to_string()), method: "POST".to_string(), headers: HashMap::new(), body_template: None, @@ -509,13 +512,13 @@ mod tests { fn from_config_fails_on_undefined_header_env_var() { temp_env::with_var("UNDEFINED_TOKEN", None::<&str>, || { let config = WebhookNotifierConfig { - url: "https://api.example.com/alerts".to_string(), + url: SecretString::new("https://api.example.com/alerts".to_string()), method: "POST".to_string(), headers: { let mut h = HashMap::new(); h.insert( "Authorization".to_string(), - "Bearer ${UNDEFINED_TOKEN}".to_string(), + SecretString::new("Bearer ${UNDEFINED_TOKEN}".to_string()), ); h }, @@ -542,7 +545,7 @@ mod tests { fn from_config_rejects_unsupported_methods() { // AC6: Only POST and PUT are supported let config = WebhookNotifierConfig { - url: "https://api.example.com/alerts".to_string(), + url: SecretString::new("https://api.example.com/alerts".to_string()), method: "PATCH".to_string(), headers: HashMap::new(), body_template: None, @@ -567,7 +570,7 @@ mod tests { #[test] fn from_config_accepts_put_method() { let config = WebhookNotifierConfig { - url: "https://api.example.com/alerts".to_string(), + url: SecretString::new("https://api.example.com/alerts".to_string()), method: "PUT".to_string(), headers: HashMap::new(), body_template: None, @@ -584,7 +587,7 @@ mod tests { #[test] fn from_config_rejects_delete_method() { let config = WebhookNotifierConfig { - url: "https://api.example.com/alerts".to_string(), + url: SecretString::new("https://api.example.com/alerts".to_string()), method: "DELETE".to_string(), headers: HashMap::new(), body_template: None, @@ -599,7 +602,7 @@ mod tests { #[test] fn from_config_fails_on_invalid_body_template() { let config = WebhookNotifierConfig { - url: "https://api.example.com/alerts".to_string(), + url: SecretString::new("https://api.example.com/alerts".to_string()), method: "POST".to_string(), headers: HashMap::new(), body_template: Some("{% if unclosed".to_string()), @@ -688,7 +691,7 @@ mod tests { #[test] fn webhook_notifier_properties() { let config = WebhookNotifierConfig { - url: "https://api.example.com/alerts".to_string(), + url: SecretString::new("https://api.example.com/alerts".to_string()), method: "POST".to_string(), headers: HashMap::new(), body_template: None, @@ -704,7 +707,7 @@ mod tests { #[tokio::test] async fn notifier_trait_is_object_safe() { let config = WebhookNotifierConfig { - url: "https://api.example.com/alerts".to_string(), + url: SecretString::new("https://api.example.com/alerts".to_string()), method: "POST".to_string(), headers: HashMap::new(), body_template: None, @@ -730,7 +733,7 @@ mod tests { Some("https://secret.example.com/hook/abc123"), || { let config = WebhookNotifierConfig { - url: "${TEST_SECRET_URL}".to_string(), + url: SecretString::new("${TEST_SECRET_URL}".to_string()), method: "POST".to_string(), headers: HashMap::new(), body_template: None, @@ -756,13 +759,13 @@ mod tests { fn debug_output_does_not_expose_headers() { temp_env::with_var("TEST_AUTH_TOKEN", Some("Bearer super-secret-token"), || { let config = WebhookNotifierConfig { - url: "https://api.example.com/alerts".to_string(), + url: SecretString::new("https://api.example.com/alerts".to_string()), method: "POST".to_string(), headers: { let mut h = HashMap::new(); h.insert( "Authorization".to_string(), - "${TEST_AUTH_TOKEN}".to_string(), + SecretString::new("${TEST_AUTH_TOKEN}".to_string()), ); h }, @@ -839,12 +842,15 @@ mod tests { #[test] fn from_config_rejects_invalid_header_name() { let config = WebhookNotifierConfig { - url: "https://api.example.com/alerts".to_string(), + url: SecretString::new("https://api.example.com/alerts".to_string()), method: "POST".to_string(), headers: { let mut h = HashMap::new(); // Header names cannot contain spaces or special chars - h.insert("Invalid Header Name".to_string(), "value".to_string()); + h.insert( + "Invalid Header Name".to_string(), + SecretString::new("value".to_string()), + ); h }, body_template: None, @@ -867,12 +873,15 @@ mod tests { #[test] fn from_config_rejects_invalid_header_value() { let config = WebhookNotifierConfig { - url: "https://api.example.com/alerts".to_string(), + url: SecretString::new("https://api.example.com/alerts".to_string()), method: "POST".to_string(), headers: { let mut h = HashMap::new(); // Header values cannot contain control characters (e.g., newlines) - h.insert("X-Custom".to_string(), "value\nwith\nnewlines".to_string()); + h.insert( + "X-Custom".to_string(), + SecretString::new("value\nwith\nnewlines".to_string()), + ); h }, body_template: None, diff --git a/src/tail.rs b/src/tail.rs index e2872d1..ebd62cc 100644 --- a/src/tail.rs +++ b/src/tail.rs @@ -54,6 +54,22 @@ pub const BACKOFF_MAX: Duration = Duration::from_secs(60); /// TCP keepalive interval to detect dead connections through firewalls/NAT. pub const TCP_KEEPALIVE: Duration = Duration::from_secs(60); +/// Number of consecutive connection / stream failures a task must observe +/// before flipping the per-source `valerter_vl_source_up` gauge to 0. +/// Debounces transient errors (EOF, isolated 5xx, timeout) so a flaky source +/// does not generate spurious `ValerterVlSourceDown` Prometheus alerts. Fixed +/// value (not configurable) to keep the contract simple; operators who want +/// a different threshold should use Prometheus `for:` on the alert rule. +pub const VL_SOURCE_UP_FAILURE_THRESHOLD: u32 = 3; + +/// Gate for the per-source reachability gauge flip. Extracted so the debounce +/// decision is covered by a deterministic unit test without requiring a live +/// Prometheus recorder. +#[inline] +pub(crate) fn should_report_source_down(consecutive_failures: u32) -> bool { + consecutive_failures >= VL_SOURCE_UP_FAILURE_THRESHOLD +} + /// Configuration for connecting to VictoriaLogs tail endpoint. #[derive(Debug, Clone)] pub struct TailConfig { @@ -317,6 +333,7 @@ impl TailClient { { let mut attempt: u32 = 0; let mut had_failure = false; + let mut consecutive_failures: u32 = 0; loop { let url = self.build_url(); @@ -357,15 +374,22 @@ impl TailClient { } attempt = 0; had_failure = false; + consecutive_failures = 0; resp } Ok(resp) => { - // HTTP error (4xx, 5xx) - mark this source as down. - metrics::gauge!( - "valerter_vl_source_up", - "vl_source" => vl_source.to_string(), - ) - .set(0.0); + // HTTP error (4xx, 5xx). Debounced flip via + // `should_report_source_down`: only flips to 0 once we + // reach `VL_SOURCE_UP_FAILURE_THRESHOLD` consecutive + // failures, so transient 5xx do not page. + consecutive_failures = consecutive_failures.saturating_add(1); + if should_report_source_down(consecutive_failures) { + metrics::gauge!( + "valerter_vl_source_up", + "vl_source" => vl_source.to_string(), + ) + .set(0.0); + } had_failure = true; let delay = backoff_delay_with_jitter(attempt); @@ -381,12 +405,16 @@ impl TailClient { continue; } Err(e) => { - // Connection error - mark this source as down. - metrics::gauge!( - "valerter_vl_source_up", - "vl_source" => vl_source.to_string(), - ) - .set(0.0); + // Connection error (DNS, timeout, refused, ...). Same + // debounce gate as the HTTP-error branch above. + consecutive_failures = consecutive_failures.saturating_add(1); + if should_report_source_down(consecutive_failures) { + metrics::gauge!( + "valerter_vl_source_up", + "vl_source" => vl_source.to_string(), + ) + .set(0.0); + } had_failure = true; let delay = backoff_delay_with_jitter(attempt); @@ -467,12 +495,16 @@ impl TailClient { } } Err(e) => { - // Stream error - mark this source as down and reconnect. - metrics::gauge!( - "valerter_vl_source_up", - "vl_source" => vl_source.to_string(), - ) - .set(0.0); + // Mid-stream error (EOF, broken pipe, ...). Same + // debounce gate as the connect branches above. + consecutive_failures = consecutive_failures.saturating_add(1); + if should_report_source_down(consecutive_failures) { + metrics::gauge!( + "valerter_vl_source_up", + "vl_source" => vl_source.to_string(), + ) + .set(0.0); + } had_failure = true; let delay = backoff_delay_with_jitter(attempt); @@ -939,6 +971,28 @@ mod tests { assert!(result.is_ok()); } + #[test] + fn should_report_source_down_debounces_until_threshold() { + // Threshold-1 failures in a row must not flip the gauge; the Nth does. + for below in 0..VL_SOURCE_UP_FAILURE_THRESHOLD { + assert!( + !should_report_source_down(below), + "{} consecutive failures must not report the source as down (threshold is {})", + below, + VL_SOURCE_UP_FAILURE_THRESHOLD + ); + } + assert!( + should_report_source_down(VL_SOURCE_UP_FAILURE_THRESHOLD), + "{} consecutive failures must report the source as down", + VL_SOURCE_UP_FAILURE_THRESHOLD + ); + assert!( + should_report_source_down(VL_SOURCE_UP_FAILURE_THRESHOLD + 5), + "sustained failures past the threshold must keep reporting down" + ); + } + // Test that StreamBuffer integration works correctly #[test] fn test_buffer_integration_simulation() { diff --git a/tests/integration_notify.rs b/tests/integration_notify.rs index 2aefb26..773f1d1 100644 --- a/tests/integration_notify.rs +++ b/tests/integration_notify.rs @@ -479,9 +479,12 @@ fn make_webhook_notifier( body_template: Option, ) -> WebhookNotifier { let config = WebhookNotifierConfig { - url: url.to_string(), + url: SecretString::new(url.to_string()), method: method_str.to_string(), - headers, + headers: headers + .into_iter() + .map(|(k, v)| (k, SecretString::new(v))) + .collect(), body_template, }; let client = make_client();