mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-06 13:27:43 +00:00
fix(targets): surface redacted construction error detail in target failures (#5729)
A failed target construction previously logged only reason=construction_failed and pushed an opaque "target construction failed" summary, discarding the underlying TargetError. Debugging rustfs#5115 showed the log repeated every 5s with no root cause, even though the error carried an actionable egress-policy rejection (RUSTFS_OUTBOUND_ALLOW_ORIGINS hint). The Err branch now includes the error detail in both the error! log (detail field) and the returned failures summary. The detail is scrubbed against the instance's merged config via the loader's existing per-field redaction (secrets -> ***redacted***, endpoint URLs -> origin only, DSNs -> password masked) so credential-bearing values never reach logs or Admin-visible summaries.
This commit is contained in:
@@ -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***");
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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::<String>::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}");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user