diff --git a/crates/targets/src/config/loader.rs b/crates/targets/src/config/loader.rs index add0bc73d..990597d49 100644 --- a/crates/targets/src/config/loader.rs +++ b/crates/targets/src/config/loader.rs @@ -113,6 +113,25 @@ fn redacted_target_config(config: &KVS) -> Vec<(String, String)> { .collect() } +/// Scrubs a free-form error message against an instance's merged configuration: +/// every non-empty config value whose field is redacted in debug logs (secrets, +/// endpoint URLs, DSNs) has its occurrences in the message replaced by the same +/// redacted form, so a construction error can surface its underlying detail +/// without leaking credential-bearing configuration values. +pub(crate) fn redact_error_detail_with_config(message: &str, config: &KVS) -> String { + let mut redacted = message.to_string(); + for kv in &config.0 { + if kv.value.is_empty() { + continue; + } + let replacement = redact_target_field_value(&kv.key, &kv.value); + if replacement != kv.value { + redacted = redacted.replace(&kv.value, &replacement); + } + } + redacted +} + #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct MergedTargetConfigRecord { pub instance_id: String, @@ -392,7 +411,7 @@ where mod tests { use super::{ collect_env_target_instance_ids_from_env, collect_target_config_results_from_env, collect_target_configs_from_env, - redact_target_field_value, redacted_target_config, try_collect_target_configs_from_env, + redact_error_detail_with_config, redact_target_field_value, redacted_target_config, try_collect_target_configs_from_env, }; use crate::TargetError; use rustfs_config::notify::{ @@ -635,6 +654,33 @@ mod tests { assert_eq!(configs[0].1.lookup(ENABLE_KEY).as_deref(), Some(" on ")); } + #[test] + fn redact_error_detail_with_config_scrubs_sensitive_values_from_message() { + let mut config = KVS::new(); + config.insert("endpoint".to_string(), "https://example.com/private/hook?sig=hunter2".to_string()); + config.insert("auth_token".to_string(), "hook-secret-token".to_string()); + config.insert("queue_limit".to_string(), "1000".to_string()); + + let detail = redact_error_detail_with_config( + "webhook endpoint is not allowed: https://example.com/private/hook?sig=hunter2 (auth_token hook-secret-token, queue_limit 1000)", + &config, + ); + + assert_eq!( + detail, + "webhook endpoint is not allowed: https://example.com (auth_token ***redacted***, queue_limit 1000)" + ); + } + + #[test] + fn redact_error_detail_with_config_leaves_untainted_message_intact() { + let mut config = KVS::new(); + config.insert("auth_token".to_string(), "hook-secret-token".to_string()); + + let detail = redact_error_detail_with_config("queue store open failed: permission denied", &config); + assert_eq!(detail, "queue store open failed: permission denied"); + } + #[test] fn redact_target_field_value_redacts_sensitive_fields() { assert_eq!(redact_target_field_value("password", "secret"), "***redacted***"); diff --git a/crates/targets/src/config/mod.rs b/crates/targets/src/config/mod.rs index dd353889c..b7db6dc4c 100644 --- a/crates/targets/src/config/mod.rs +++ b/crates/targets/src/config/mod.rs @@ -25,6 +25,7 @@ pub use instance::{ normalize_legacy_target_instances_from_env, normalize_target_plugin_instances, normalize_target_plugin_instances_from_env, try_normalize_target_plugin_instances, try_normalize_target_plugin_instances_from_env, }; +pub(crate) use loader::redact_error_detail_with_config; pub use loader::{ collect_env_target_instance_ids, collect_env_target_instance_ids_from_env, collect_target_config_results, collect_target_config_results_from_env, collect_target_configs, collect_target_configs_from_env, try_collect_target_configs, diff --git a/crates/targets/src/plugin.rs b/crates/targets/src/plugin.rs index 6c72590a7..43e5ba8ad 100644 --- a/crates/targets/src/plugin.rs +++ b/crates/targets/src/plugin.rs @@ -14,7 +14,7 @@ use crate::{ PluginRuntimeAdapter, RuntimeActivation, Target, TargetError, - config::collect_target_config_results, + config::{collect_target_config_results, redact_error_detail_with_config}, manifest::{TargetPluginManifest, builtin_target_manifest}, target::with_deferred_queue_store_open, }; @@ -368,9 +368,14 @@ where info!(target_type = %target.id().name, instance_id = %id, "Create target successfully"); successful_targets.push(target); } - Err(_) => { - failures.push(format!("{target_type}/{id}: target construction failed")); - error!(target_type = %target_type, instance_id = %id, reason = "construction_failed", "Failed to create target"); + Err(err) => { + // The underlying error names the root cause (egress policy + // rejection, queue-store open failure, ...); scrub it against + // the instance config so credential-bearing values never + // reach the log or the Admin-visible failure summary. + let detail = redact_error_detail_with_config(&err.to_string(), &merged_config); + failures.push(format!("{target_type}/{id}: target construction failed: {detail}")); + error!(target_type = %target_type, instance_id = %id, reason = "construction_failed", detail = %detail, "Failed to create target"); } } } @@ -571,4 +576,51 @@ mod tests { assert_eq!(failures.len(), 1); assert!(failures[0].contains("alpha/bad"), "unexpected failure summary: {}", failures[0]); } + + // Regression (#5115 debugging): a construction failure must carry the + // underlying error detail (e.g. an egress-policy rejection) in the failure + // summary instead of an opaque "target construction failed", while + // credential-bearing config values stay redacted. + #[tokio::test] + async fn construction_failure_surfaces_redacted_error_detail() { + let mut registry = TargetPluginRegistry::::new(); + registry.register(TargetPluginDescriptor::new( + "gamma", + &[ENABLE_KEY, "endpoint", "auth_token"], + |_config| Ok(()), + |_id, config| { + let endpoint = config.lookup("endpoint").unwrap_or_default(); + let token = config.lookup("auth_token").unwrap_or_default(); + Err(TargetError::Configuration(format!( + "webhook endpoint is not allowed: {endpoint} (auth_token {token})" + ))) + }, + )); + + let mut cfg = Config(HashMap::new()); + let mut section = HashMap::new(); + let mut primary = KVS::new(); + primary.insert(ENABLE_KEY.to_string(), "on".to_string()); + primary.insert("endpoint".to_string(), "https://example.com/private/hook?sig=hunter2".to_string()); + primary.insert("auth_token".to_string(), "hook-secret-token".to_string()); + section.insert("primary".to_string(), primary); + cfg.0.insert("notify_gamma".to_string(), section); + + let (targets, failures) = registry + .create_dormant_targets_from_config(&cfg, "notify_") + .await + .expect("a failing instance must not abort target creation"); + + assert!(targets.is_empty()); + assert_eq!(failures.len(), 1); + let failure = &failures[0]; + assert!(failure.contains("gamma/primary"), "unexpected failure summary: {failure}"); + // The root cause is surfaced instead of an opaque generic message. + assert!(failure.contains("webhook endpoint is not allowed"), "missing error detail: {failure}"); + // The endpoint is reduced to its origin; path, query, and token are gone. + assert!(failure.contains("https://example.com"), "endpoint origin should stay visible: {failure}"); + assert!(!failure.contains("/private/hook"), "endpoint path must be redacted: {failure}"); + assert!(!failure.contains("hunter2"), "endpoint query must be redacted: {failure}"); + assert!(!failure.contains("hook-secret-token"), "auth token must be redacted: {failure}"); + } }