fix(security): redact target debug logs and remove eval-based bench hook (#2637)

This commit is contained in:
houseme
2026-04-22 05:21:01 +08:00
committed by GitHub
parent 4aafb07173
commit 3ac1d2ab0b
5 changed files with 98 additions and 17 deletions

View File

@@ -15,7 +15,12 @@
use crate::notification_system_subscriber::NotificationSystemSubscriberView;
use crate::notifier::TargetList;
use crate::{
Event, error::NotificationError, notifier::EventNotifier, registry::TargetRegistry, rules::BucketNotificationConfig, stream,
Event,
error::NotificationError,
notifier::EventNotifier,
registry::TargetRegistry,
rules::{BucketNotificationConfig, ParseConfigError},
stream,
};
use hashbrown::HashMap;
use rustfs_config::notify::{
@@ -33,7 +38,7 @@ use std::sync::Arc;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::time::{Duration, Instant};
use tokio::sync::{RwLock, Semaphore, broadcast, mpsc};
use tracing::{debug, error, info, warn};
use tracing::{debug, info, warn};
const MAX_RECENT_LIVE_EVENTS: usize = 1024;
@@ -309,7 +314,10 @@ impl NotificationSystem {
let config = {
let guard = self.config.read().await;
debug!("Initializing notification system with config: {:?}", *guard);
debug!(
subsystem_count = guard.0.len(),
"Initializing notification system with configuration summary"
);
guard.clone()
};
@@ -517,7 +525,10 @@ impl NotificationSystem {
if !changed {
info!("Target {} of type {} not found, no changes made.", target_name, target_type);
}
debug!("Config after remove: {:?}", config);
debug!(
subsystem_count = config.0.len(),
"Target config removal processed and configuration summary updated"
);
changed
})
.await;
@@ -593,7 +604,6 @@ impl NotificationSystem {
bucket: &str,
cfg: &BucketNotificationConfig,
) -> Result<(), NotificationError> {
self.subscriber_view.apply_bucket_config(bucket, cfg);
let arn_list = self.notifier.get_arn_list(&cfg.region).await;
if arn_list.is_empty() {
return Err(NotificationError::Configuration("No targets configured".to_string()));
@@ -602,13 +612,18 @@ impl NotificationSystem {
// Validate the configuration against the available ARNs
if let Err(e) = cfg.validate(&cfg.region, &arn_list) {
debug!("Bucket notification config validation region:{} failed: {}", &cfg.region, e);
if !e.to_string().contains("ARN not found") {
if !matches!(e, ParseConfigError::ArnNotFound(_)) {
return Err(NotificationError::BucketNotification(e.to_string()));
} else {
error!("config validate failed, err: {}", e);
}
warn!(
bucket = %bucket,
region = %cfg.region,
error = %e,
"Bucket notification config references missing target ARN; keeping compatibility and loading remaining rules"
);
}
self.subscriber_view.apply_bucket_config(bucket, cfg);
let rules_map = cfg.get_rules_map();
self.notifier.add_rules_map(bucket, rules_map.clone()).await;
info!("Loaded notification config for bucket: {}", bucket);

View File

@@ -28,6 +28,33 @@ pub fn collect_target_configs(
collect_target_configs_from_env(config, route_prefix, target_type, valid_fields, std::env::vars())
}
fn is_sensitive_target_field(field_name: &str) -> bool {
let field_name = field_name.to_ascii_lowercase();
field_name.contains("password")
|| field_name.contains("secret")
|| field_name.contains("token")
|| field_name.contains("credential")
|| field_name.contains("private_key")
|| field_name.contains("client_key")
|| field_name.contains("access_key")
|| field_name.contains("auth")
}
fn redact_target_field_value(field_name: &str, value: &str) -> String {
if is_sensitive_target_field(field_name) && !value.is_empty() {
return "***redacted***".to_string();
}
value.to_string()
}
fn redacted_target_config(config: &KVS) -> Vec<(String, String)> {
config
.0
.iter()
.map(|kv| (kv.key.clone(), redact_target_field_value(&kv.key, &kv.value)))
.collect()
}
pub fn collect_env_target_instance_ids(route_prefix: &str, target_type: &str, valid_fields: &HashSet<String>) -> HashSet<String> {
collect_env_target_instance_ids_from_env(route_prefix, target_type, valid_fields, std::env::vars())
}
@@ -104,7 +131,7 @@ where
debug!(
instance_id = %if instance_id == DEFAULT_DELIMITER { DEFAULT_DELIMITER } else { &instance_id },
%field_name,
%value,
value = %redact_target_field_value(&field_name, value),
"Parsed target environment override"
);
env_overrides
@@ -137,7 +164,10 @@ where
merged_config.extend(env_instance_cfg.clone());
}
debug!(instance_id = %id, ?merged_config, "Merged target configuration");
if tracing::enabled!(tracing::Level::DEBUG) {
let redacted_config = redacted_target_config(&merged_config);
debug!(instance_id = %id, ?redacted_config, "Merged target configuration");
}
if is_target_enabled(&merged_config) {
merged_configs.push((id, merged_config));
}
@@ -148,7 +178,7 @@ where
#[cfg(test)]
mod tests {
use super::{collect_env_target_instance_ids_from_env, collect_target_configs_from_env};
use super::{collect_env_target_instance_ids_from_env, collect_target_configs_from_env, redact_target_field_value};
use rustfs_config::notify::NOTIFY_ROUTE_PREFIX;
use rustfs_config::{ENABLE_KEY, WEBHOOK_ENDPOINT, WEBHOOK_QUEUE_LIMIT};
use rustfs_ecstore::config::{Config, KVS};
@@ -236,4 +266,17 @@ mod tests {
assert_eq!(ids, HashSet::from(["primary".to_string()]));
}
#[test]
fn redact_target_field_value_redacts_sensitive_fields() {
assert_eq!(redact_target_field_value("password", "secret"), "***redacted***");
assert_eq!(redact_target_field_value("auth_token", "token"), "***redacted***");
assert_eq!(redact_target_field_value("credentials_file", "/tmp/creds"), "***redacted***");
}
#[test]
fn redact_target_field_value_keeps_non_sensitive_fields() {
assert_eq!(redact_target_field_value("endpoint", "https://example.com"), "https://example.com");
assert_eq!(redact_target_field_value("queue_limit", "1000"), "1000");
}
}

View File

@@ -672,7 +672,7 @@ where
bucket = %meta.bucket_name,
object = %meta.object_name,
event = %meta.event_name,
preview = %meta.best_effort_preview(&body, 256),
payload_len = body.len(),
"Sending MQTT payload"
);

View File

@@ -288,7 +288,7 @@ where
bucket = %meta.bucket_name,
object = %meta.object_name,
event = %meta.event_name,
preview = %meta.best_effort_preview(&body, 256),
payload_len = body.len(),
"Sending webhook payload"
);

View File

@@ -34,6 +34,7 @@ SAMPLES=20000
# optional hooks
APPLY_CMD=""
APPLY_CMD_ARR=()
APPLY_WAIT_SECS=20
EXTRA_ARGS=()
@@ -74,7 +75,7 @@ s3bench options:
Hooks:
--apply-cmd Optional command to apply/restart RustFS after profile env switch.
Runs via: eval "$APPLY_CMD"
Executed directly (no shell eval), e.g. "bash scripts/restart.sh"
--apply-wait-secs Wait time after apply cmd (default: 20)
Extra:
@@ -110,6 +111,23 @@ validate_positive_int() {
fi
}
parse_apply_cmd() {
local raw="$1"
if [[ "$raw" == *';'* || "$raw" == *'&&'* || "$raw" == *'||'* || "$raw" == *'|'* || "$raw" == *'<'* || "$raw" == *'>'* || "$raw" == *'`'* || "$raw" == *'$'* ]]; then
echo "ERROR: --apply-cmd does not allow shell operators or expansions; pass a plain command and args only" >&2
exit 1
fi
IFS=$' \t\n' read -r -a APPLY_CMD_ARR <<< "$raw"
if [[ "${#APPLY_CMD_ARR[@]}" -eq 0 ]]; then
echo "ERROR: --apply-cmd must not be empty" >&2
exit 1
fi
require_cmd "${APPLY_CMD_ARR[0]}"
}
parse_args() {
while [[ $# -gt 0 ]]; do
case "$1" in
@@ -171,6 +189,9 @@ validate_args() {
if [[ "$TOOL" == "s3bench" ]]; then
validate_positive_int "$SAMPLES" "--samples"
fi
if [[ -n "$APPLY_CMD" ]]; then
parse_apply_cmd "$APPLY_CMD"
fi
}
setup_out_root() {
@@ -238,15 +259,17 @@ sizes_for_group() {
run_apply_hook_if_needed() {
local group="$1"
if [[ -z "$APPLY_CMD" ]]; then
if [[ "${#APPLY_CMD_ARR[@]}" -eq 0 ]]; then
return
fi
echo "[${group}] running apply command..."
if [[ "$DRY_RUN" == "true" ]]; then
echo "[DRY-RUN] eval \"$APPLY_CMD\""
printf '[DRY-RUN] '
printf '%q ' "${APPLY_CMD_ARR[@]}"
printf '\n'
echo "[DRY-RUN] sleep $APPLY_WAIT_SECS"
else
eval "$APPLY_CMD"
"${APPLY_CMD_ARR[@]}"
echo "[${group}] waiting ${APPLY_WAIT_SECS}s for service readiness..."
sleep "$APPLY_WAIT_SECS"
fi