diff --git a/crates/targets/src/config/common.rs b/crates/targets/src/config/common.rs index 12e622352..629a14596 100644 --- a/crates/targets/src/config/common.rs +++ b/crates/targets/src/config/common.rs @@ -23,7 +23,7 @@ use rustfs_config::{ NATS_TLS_CLIENT_KEY, NATS_TOKEN, NATS_USERNAME, PULSAR_AUTH_TOKEN, PULSAR_PASSWORD, PULSAR_QUEUE_DIR, PULSAR_TLS_CA, PULSAR_TOPIC, PULSAR_USERNAME, }; -use rustfs_utils::egress::OutboundPolicy; +use rustfs_utils::egress::{ENV_OUTBOUND_ALLOW_ORIGINS, OutboundPolicy, OutboundPolicyError}; use std::collections::HashSet; use std::path::Path; use std::str::FromStr; @@ -223,7 +223,22 @@ pub(super) fn validate_outbound_http_url(value: &Url, field_label: &str) -> Resu OutboundPolicy::from_env_cached().map_err(|err| TargetError::Configuration(format!("invalid outbound policy: {err}")))?; policy .validate_url(value) - .map_err(|e| TargetError::Configuration(format!("{field_label} is not allowed: {e}"))) + .map_err(|err| TargetError::Configuration(format_outbound_http_url_error(field_label, value, &err))) +} + +/// Formats an outbound-policy rejection and adds an exact-origin hint only when that origin can override the rejection. +pub fn format_outbound_http_url_error(field_label: &str, value: &Url, err: &OutboundPolicyError) -> String { + let base = format!("{field_label} is not allowed: {err}"); + let origin = value.origin().ascii_serialization(); + let can_allow_origin = + OutboundPolicy::from_allowed_origins(&origin).is_ok_and(|allowlisted| allowlisted.validate_url(value).is_ok()); + if can_allow_origin { + format!( + "{base}; add {origin} to {ENV_OUTBOUND_ALLOW_ORIGINS} (comma-separated) and restart RustFS to allow this operator-owned endpoint (origin only, no path)" + ) + } else { + base + } } #[cfg(test)] diff --git a/crates/targets/src/config/mod.rs b/crates/targets/src/config/mod.rs index 843ec7762..dd353889c 100644 --- a/crates/targets/src/config/mod.rs +++ b/crates/targets/src/config/mod.rs @@ -17,6 +17,8 @@ mod instance; mod loader; mod target_args; +pub use common::format_outbound_http_url_error; + pub use instance::{ LegacyTargetInstanceDescriptor, TargetInstanceSourceClass, TargetInstanceSourceHints, TargetPluginInstance, TargetPluginInstanceCompatDescriptor, TargetPluginInstanceRecord, normalize_legacy_target_instances, diff --git a/crates/targets/src/config/target_args.rs b/crates/targets/src/config/target_args.rs index 6f50c5d6e..d96c7b719 100644 --- a/crates/targets/src/config/target_args.rs +++ b/crates/targets/src/config/target_args.rs @@ -624,6 +624,7 @@ mod tests { REDIS_CHANNEL, REDIS_CONNECTION_TIMEOUT, REDIS_MAX_RETRY_DELAY, REDIS_MIN_RETRY_DELAY, REDIS_PIPELINE_BUFFER_SIZE, REDIS_RECONNECT_RETRY_ATTEMPTS, REDIS_RESPONSE_TIMEOUT, REDIS_TLS_ALLOW_INSECURE, REDIS_URL, WEBHOOK_ENDPOINT, }; + use rustfs_utils::egress::ENV_OUTBOUND_ALLOW_ORIGINS; fn absolute_test_path(path: &str) -> String { std::env::temp_dir().join(path).to_string_lossy().into_owned() @@ -780,11 +781,29 @@ mod tests { #[test] fn build_webhook_args_rejects_loopback_endpoint() { let mut config = webhook_base_config(); - config.insert(WEBHOOK_ENDPOINT.to_string(), "https://127.0.0.1/hook".to_string()); + config.insert(WEBHOOK_ENDPOINT.to_string(), "https://127.0.0.1:8443/hook".to_string()); let err = build_webhook_args(&config, "/tmp/webhook-queue", TargetType::NotifyEvent) .expect_err("loopback endpoint should be rejected"); - assert!(err.to_string().contains("not allowed")); + let message = err.to_string(); + assert!(message.contains(&format!("add https://127.0.0.1:8443 to {ENV_OUTBOUND_ALLOW_ORIGINS}"))); + assert!(message.contains("loopback address")); + assert!(message.contains("comma-separated")); + assert!(message.contains("restart RustFS")); + assert!(!message.contains("/hook")); + assert!(message.contains("origin only, no path")); + } + + #[test] + fn build_webhook_args_does_not_offer_allowlist_for_metadata_endpoint() { + let mut config = webhook_base_config(); + config.insert(WEBHOOK_ENDPOINT.to_string(), "http://169.254.169.254/latest/meta-data".to_string()); + + let err = build_webhook_args(&config, "/tmp/webhook-queue", TargetType::NotifyEvent) + .expect_err("metadata endpoint should be rejected"); + let message = err.to_string(); + assert!(message.contains("not allowed")); + assert!(!message.contains(ENV_OUTBOUND_ALLOW_ORIGINS)); } #[test] diff --git a/rustfs/src/admin/handlers/target_descriptor.rs b/rustfs/src/admin/handlers/target_descriptor.rs index bd1b89ef5..dab02d4bc 100644 --- a/rustfs/src/admin/handlers/target_descriptor.rs +++ b/rustfs/src/admin/handlers/target_descriptor.rs @@ -28,13 +28,13 @@ use rustfs_targets::{ check_postgres_server_available, check_pulsar_broker_available, check_redis_server_available, config::{ TargetPluginInstanceCompatDescriptor, TargetPluginInstanceRecord, build_amqp_args, build_kafka_args, build_mysql_args, - build_nats_args, build_postgres_args, build_pulsar_args, build_redis_args, try_normalize_target_plugin_instances, - validate_redis_config, + build_nats_args, build_postgres_args, build_pulsar_args, build_redis_args, format_outbound_http_url_error, + try_normalize_target_plugin_instances, validate_redis_config, }, manifest::builtin_target_manifest, target::{TargetType, mqtt::MQTTTlsConfig}, }; -use rustfs_utils::egress::{ENV_OUTBOUND_ALLOW_ORIGINS, OutboundPolicy}; +use rustfs_utils::egress::OutboundPolicy; use s3s::{Body, S3Response, S3Result, header::CONTENT_TYPE, s3_error}; use serde::Serialize; use std::collections::{HashMap, HashSet}; @@ -708,16 +708,9 @@ async fn validate_webhook_request(kv_map: &HashMap) -> S3Result< } let outbound_policy = OutboundPolicy::from_env_cached().map_err(|e| s3_error!(InvalidArgument, "invalid outbound policy: {}", e))?; - outbound_policy.validate_url(&parsed_endpoint).map_err(|e| { - s3_error!( - InvalidArgument, - "endpoint is not allowed by the outbound policy: {}; review {}; private origins must be listed as exact origins such as {}={} and RustFS restarted (metadata and link-local destinations remain blocked)", - e, - ENV_OUTBOUND_ALLOW_ORIGINS, - ENV_OUTBOUND_ALLOW_ORIGINS, - parsed_endpoint.origin().ascii_serialization() - ) - })?; + outbound_policy + .validate_url(&parsed_endpoint) + .map_err(|err| s3_error!(InvalidArgument, "{}", format_outbound_http_url_error("endpoint", &parsed_endpoint, &err)))?; if let Some(queue_dir) = kv_map.get("queue_dir") { validate_queue_dir(queue_dir.as_str()).await?; } @@ -970,24 +963,40 @@ fn to_kvs(kv_map: &HashMap) -> rustfs_config::server_config::KVS #[cfg(test)] mod webhook_request_tests { use super::validate_webhook_request; + use rustfs_utils::egress::ENV_OUTBOUND_ALLOW_ORIGINS; use std::collections::HashMap; #[tokio::test] async fn private_webhook_request_is_rejected_with_operator_action() { - let config = HashMap::from([("endpoint".to_string(), "http://127.0.0.1:49173/webhook/rustfs".to_string())]); + let config = HashMap::from([("endpoint".to_string(), "http://192.168.1.2:1880/webhook/rustfs".to_string())]); let err = validate_webhook_request(&config) .await - .expect_err("a loopback webhook must require an explicit outbound allowlist"); + .expect_err("a private webhook must require an explicit outbound allowlist"); let message = err .message() .expect("the validation error should explain the operator action"); - assert!(message.contains("RUSTFS_OUTBOUND_ALLOW_ORIGINS=http://127.0.0.1:49173")); - assert!(message.contains("RustFS restarted"), "unexpected message: {message}"); + assert!(message.contains(&format!("add http://192.168.1.2:1880 to {ENV_OUTBOUND_ALLOW_ORIGINS}"))); + assert!(message.contains("private address")); + assert!(message.contains("comma-separated")); + assert!(message.contains("restart RustFS"), "unexpected message: {message}"); assert!(!message.contains("/webhook/rustfs")); } + #[tokio::test] + async fn metadata_webhook_request_does_not_offer_allowlist_override() { + let config = HashMap::from([("endpoint".to_string(), "http://169.254.169.254/latest/meta-data".to_string())]); + + let err = validate_webhook_request(&config) + .await + .expect_err("the metadata endpoint must remain permanently blocked"); + let message = err.message().expect("the validation error should explain the rejection"); + + assert!(message.contains("metadata endpoint")); + assert!(!message.contains(ENV_OUTBOUND_ALLOW_ORIGINS)); + } + #[tokio::test] async fn public_webhook_still_passes_outbound_preflight() { let config = HashMap::from([("endpoint".to_string(), "https://hooks.example/webhook".to_string())]);