diff --git a/crates/cli/src/commands/cp.rs b/crates/cli/src/commands/cp.rs index 070bc66..acbbd6b 100644 --- a/crates/cli/src/commands/cp.rs +++ b/crates/cli/src/commands/cp.rs @@ -7,10 +7,11 @@ use jiff::Timestamp; use rc_core::alias::RetryConfig; use rc_core::{ AliasManager, Error, MetadataDirective, MultipartCopyCancellation, MultipartCopyOptions, - ObjectAttributes, ObjectEncryptionRequest, ObjectInfo, ObjectStore as _, ObjectWriteOptions, - ParsedPath, RemotePath, SseCustomerKey, TransferCancellation, TransferCandidate, - TransferControls, TransferCopyOptions, TransferExecutor, TransferOutcomeState, TransferPlan, - TransferReadOptions, TransferSelection, parse_path, + ObjectAttributes, ObjectEncryptionRequest, ObjectInfo, ObjectKeyPolicy, ObjectStore as _, + ObjectWriteOptions, ParsedPath, RemotePath, SseCustomerKey, TransferCancellation, + TransferCandidate, TransferControls, TransferCopyOptions, TransferExecutor, + TransferOutcomeState, TransferPlan, TransferReadOptions, TransferSelection, + normalize_relative_key, parse_path, relative_local_path_from_key, }; use rc_s3::S3Client; use serde::Serialize; @@ -188,6 +189,10 @@ pub struct CpArgs { /// Print deterministic aggregate transfer counters (human output) #[arg(long)] pub summary: bool, + + /// Reject object keys that cannot be created on Windows filesystems + #[arg(long)] + pub portable_names: bool, } impl fmt::Debug for CpArgs { @@ -232,8 +237,13 @@ impl CpArgs { retry_max_backoff_ms: None, fail_empty: false, summary: false, + portable_names: false, } } + + fn local_key_policy(&self) -> ObjectKeyPolicy { + ObjectKeyPolicy::for_local_destination(self.portable_names) + } } /// Download one remote object through the canonical copy implementation. @@ -834,6 +844,7 @@ async fn execute_transfer_plan( encryption, args.source_customer_key.as_ref(), alias_manager, + args.local_key_policy(), ) .await { @@ -1771,6 +1782,7 @@ fn is_container_target(raw: &str, target: &ParsedPath) -> bool { } } +#[allow(clippy::too_many_arguments)] async fn build_transfer_candidates( sources: &[ParsedPath], target: &ParsedPath, @@ -1779,6 +1791,7 @@ async fn build_transfer_candidates( encryption: Option, source_customer_key: Option<&SseCustomerKey>, alias_manager: &AliasManager, + key_policy: ObjectKeyPolicy, ) -> rc_core::Result>> { let mut candidates = Vec::new(); let mut planning_clients = HashMap::new(); @@ -1807,6 +1820,7 @@ async fn build_transfer_candidates( alias_manager, &mut planning_clients, &mut candidates, + key_policy, ) .await?; } @@ -1904,9 +1918,9 @@ fn build_local_candidates( if metadata.is_file() { let name = local_file_name(source)?; let destination = if target_is_container { - remote_child(target, &name) + remote_child(target, &name, ObjectKeyPolicy::for_remote_destination())? } else { - target.clone() + normalize_remote_target(target, ObjectKeyPolicy::for_remote_destination())? }; candidates.push(local_transfer_candidate( source.to_path_buf(), @@ -1943,7 +1957,11 @@ fn build_local_candidates( .map_or_else(|| relative.clone(), |root| format!("{root}/{relative}")); candidates.push(local_transfer_candidate( path, - remote_child(target, &target_relative), + remote_child( + target, + &target_relative, + ObjectKeyPolicy::for_remote_destination(), + )?, relative, metadata, encryption.clone(), @@ -2026,6 +2044,7 @@ async fn build_remote_candidates( alias_manager: &AliasManager, planning_clients: &mut HashMap>, candidates: &mut Vec>, + key_policy: ObjectKeyPolicy, ) -> rc_core::Result<()> { let is_prefix = source.key.is_empty() || source.key.ends_with('/') || recursive; let client = planning_client(planning_clients, alias_manager, &source.alias).await?; @@ -2073,9 +2092,12 @@ async fn build_remote_candidates( RemotePath::new(&listing_source.alias, &listing_source.bucket, &object.key); match target { ParsedPath::Local(target_root) => { - let relative = - safe_download_relative_path(&object.key, &listing_source.key) - .map_err(Error::InvalidPath)?; + let relative = safe_download_relative_path( + &object.key, + &listing_source.key, + key_policy, + ) + .map_err(Error::InvalidPath)?; let relative_string = relative.to_string_lossy().replace('\\', "/"); let target_relative = if source_root.is_empty() { relative.clone() @@ -2103,6 +2125,7 @@ async fn build_remote_candidates( target, &object.key, multiple_sources, + ObjectKeyPolicy::for_remote_destination(), )?; let size_bytes = object.size_bytes.and_then(|size| u64::try_from(size).ok()); @@ -2161,7 +2184,8 @@ async fn build_remote_candidates( match target { ParsedPath::Local(target) => { let destination = if target_is_container { - let relative = safe_download_relative_path(name, "").map_err(Error::InvalidPath)?; + let relative = safe_download_relative_path(name, "", key_policy) + .map_err(Error::InvalidPath)?; safe_download_destination(target, &relative) .await .map_err(Error::InvalidPath)? @@ -2182,9 +2206,9 @@ async fn build_remote_candidates( } ParsedPath::Remote(target) => { let destination = if target_is_container { - remote_child(target, name) + remote_child(target, name, ObjectKeyPolicy::for_remote_destination())? } else { - target.clone() + normalize_remote_target(target, ObjectKeyPolicy::for_remote_destination())? }; candidates.push(TransferCandidate { payload: CpOperation::RemoteToRemote { @@ -2220,15 +2244,42 @@ async fn planning_client( Ok(client) } -fn remote_child(parent: &RemotePath, relative: &str) -> RemotePath { - let key = if parent.key.is_empty() { - relative.to_string() - } else if parent.key.ends_with('/') { - format!("{}{}", parent.key, relative) +fn remote_child( + parent: &RemotePath, + relative: &str, + policy: ObjectKeyPolicy, +) -> rc_core::Result { + let parent_key = normalize_remote_prefix(&parent.key, policy)?; + let relative = normalize_relative_key(relative, policy)?; + let key = if parent_key.is_empty() { + relative } else { - format!("{}/{}", parent.key, relative) + format!("{parent_key}/{relative}") }; - RemotePath::new(&parent.alias, &parent.bucket, key) + Ok(RemotePath::new(&parent.alias, &parent.bucket, key)) +} + +fn normalize_remote_target( + target: &RemotePath, + policy: ObjectKeyPolicy, +) -> rc_core::Result { + let key = normalize_remote_prefix(&target.key, policy)?; + if target.key.ends_with('/') && !key.is_empty() { + Ok(RemotePath::new( + &target.alias, + &target.bucket, + format!("{key}/"), + )) + } else { + Ok(RemotePath::new(&target.alias, &target.bucket, key)) + } +} + +fn normalize_remote_prefix(key: &str, policy: ObjectKeyPolicy) -> rc_core::Result { + if key.is_empty() { + return Ok(String::new()); + } + normalize_relative_key(key, policy) } fn recursive_listing_source(source: &RemotePath) -> RemotePath { @@ -2259,6 +2310,7 @@ fn recursive_remote_target( target: &RemotePath, object_key: &str, multiple_sources: bool, + policy: ObjectKeyPolicy, ) -> rc_core::Result<(RemotePath, String)> { let relative = object_key.strip_prefix(&source.key).ok_or_else(|| { Error::InvalidPath(format!( @@ -2276,9 +2328,11 @@ fn recursive_remote_target( root if root.is_empty() => relative.to_string(), root => format!("{root}/{relative}"), }; + let normalized_relative = normalize_relative_key(relative, policy)?; + let normalized_destination_relative = normalize_relative_key(&destination_relative, policy)?; Ok(( - remote_child(target, &destination_relative), - relative.to_string(), + remote_child(target, &normalized_destination_relative, policy)?, + normalized_relative, )) } @@ -2674,7 +2728,7 @@ pub(super) async fn download_file( // Determine destination path let dst_path = if dst.is_dir() || dst.to_string_lossy().ends_with('/') { let filename = src.key.rsplit('/').next().unwrap_or(&src.key); - let filename = match safe_download_relative_path(filename, "") { + let filename = match safe_download_relative_path(filename, "", args.local_key_policy()) { Ok(filename) => filename, Err(error) => { return formatter.fail( @@ -2815,7 +2869,11 @@ async fn download_prefix( } // Calculate relative path from prefix - let relative_path = match safe_download_relative_path(&item.key, &src.key) { + let relative_path = match safe_download_relative_path( + &item.key, + &src.key, + args.local_key_policy(), + ) { Ok(path) => path, Err(error) => { error_count += 1; @@ -2888,43 +2946,12 @@ async fn download_prefix( } } -pub(super) fn safe_download_relative_path(key: &str, prefix: &str) -> Result { - let relative = key - .strip_prefix(prefix) - .ok_or_else(|| format!("key is outside requested prefix '{prefix}'"))? - .trim_start_matches('/'); - - let mut path = PathBuf::new(); - for component in relative.split(['/', '\\']) { - if component.is_empty() { - continue; - } - if matches!(component, "." | "..") { - return Err("path traversal components are not allowed".to_string()); - } - if component.contains(':') { - return Err("colon characters are not allowed in download paths".to_string()); - } - if component.ends_with(['.', ' ']) { - return Err("download path components must not end in a dot or space".to_string()); - } - let stem = component.split('.').next().unwrap_or_default(); - let stem = stem.to_ascii_uppercase(); - if matches!(stem.as_str(), "CON" | "PRN" | "AUX" | "NUL") - || (stem.len() == 4 - && (stem.starts_with("COM") || stem.starts_with("LPT")) - && matches!(stem.as_bytes()[3], b'1'..=b'9')) - { - return Err("reserved Windows device names are not allowed".to_string()); - } - path.push(component); - } - - if path.as_os_str().is_empty() { - return Err("object key does not contain a file path".to_string()); - } - - Ok(path) +pub(super) fn safe_download_relative_path( + key: &str, + prefix: &str, + policy: ObjectKeyPolicy, +) -> Result { + relative_local_path_from_key(key, prefix, policy).map_err(|error| error.to_string()) } pub(super) async fn safe_download_destination( @@ -3383,8 +3410,12 @@ mod tests { #[test] fn download_relative_path_preserves_safe_nested_keys() { - let relative = safe_download_relative_path("reports/2026/july/data.csv", "reports/") - .expect("safe key should resolve"); + let relative = safe_download_relative_path( + "reports/2026/july/data.csv", + "reports/", + ObjectKeyPolicy::Logical, + ) + .expect("safe key should resolve"); assert_eq!( relative, @@ -3397,15 +3428,44 @@ mod tests { for key in [ "reports/../../escaped", "reports/..\\..\\escaped", + "/absolute/path", + ] { + assert!( + safe_download_relative_path(key, "reports/", ObjectKeyPolicy::Logical).is_err(), + "unsafe key should be rejected: {key}" + ); + } + } + + #[test] + fn download_relative_path_accepts_colon_keys_on_logical_destinations() { + let relative = safe_download_relative_path( + "loki/fake/deadbeef/19f6abd9af4:19f6abe0e77:499628ff", + "loki/", + ObjectKeyPolicy::Logical, + ) + .expect("colon keys are valid Unix file names"); + + assert_eq!( + relative, + PathBuf::from("fake") + .join("deadbeef") + .join("19f6abd9af4:19f6abe0e77:499628ff") + ); + } + + #[test] + fn download_relative_path_rejects_colon_keys_when_portable_names_requested() { + for key in [ "reports/C:/escaped", "reports/safe:stream", "reports/CON.txt", "reports/trailing.", - "/absolute/path", ] { assert!( - safe_download_relative_path(key, "reports/").is_err(), - "unsafe key should be rejected: {key}" + safe_download_relative_path(key, "reports/", ObjectKeyPolicy::WindowsPortable) + .is_err(), + "portable-unsafe key should be rejected: {key}" ); } } @@ -3736,9 +3796,14 @@ mod tests { let source = RemotePath::new("shared", "source", "src/"); let target = RemotePath::new("shared", "destination", "archive/"); - let (destination, relative) = - recursive_remote_target(&source, &target, "src/nested/report.csv", false) - .expect("map recursive object"); + let (destination, relative) = recursive_remote_target( + &source, + &target, + "src/nested/report.csv", + false, + ObjectKeyPolicy::for_remote_destination(), + ) + .expect("map recursive object"); assert_eq!(relative, "nested/report.csv"); assert_eq!(destination.key, "archive/nested/report.csv"); @@ -3749,14 +3814,70 @@ mod tests { let source = RemotePath::new("shared", "source", ""); let target = RemotePath::new("shared", "destination", "archive/"); - let (destination, relative) = - recursive_remote_target(&source, &target, "nested/report.csv", false) - .expect("map bucket object"); + let (destination, relative) = recursive_remote_target( + &source, + &target, + "nested/report.csv", + false, + ObjectKeyPolicy::for_remote_destination(), + ) + .expect("map bucket object"); assert_eq!(relative, "nested/report.csv"); assert_eq!(destination.key, "archive/nested/report.csv"); } + #[test] + fn recursive_remote_mapping_rejects_unsafe_listed_keys() { + let source = RemotePath::new("shared", "source", "src/"); + let target = RemotePath::new("shared", "destination", "archive/"); + + for object_key in [ + "/absolute.txt", + "src/../escape.txt", + "src\\escape.txt", + "src/control\u{0007}.txt", + ] { + assert!( + recursive_remote_target( + &source, + &target, + object_key, + false, + ObjectKeyPolicy::for_remote_destination(), + ) + .is_err(), + "unsafe listed key should be rejected: {object_key:?}" + ); + } + } + + #[test] + fn remote_child_rejects_unsafe_relative_keys() { + let target = RemotePath::new("shared", "destination", "archive/"); + + for relative in [ + "../escape.txt", + "nested\\escape.txt", + "nested/control\u{0007}.txt", + ] { + assert!( + remote_child(&target, relative, ObjectKeyPolicy::for_remote_destination(),) + .is_err(), + "unsafe relative key should be rejected: {relative:?}" + ); + } + } + + #[test] + fn remote_target_rejects_absolute_prefixes() { + let target = RemotePath::new("shared", "destination", "/archive/"); + + assert!( + normalize_remote_target(&target, ObjectKeyPolicy::for_remote_destination()).is_err() + ); + } + #[test] fn recursive_remote_overlap_is_boundary_aware_and_symmetric() { let source = RemotePath::new("shared", "bucket", "src/"); @@ -3840,6 +3961,7 @@ mod tests { None, None, &alias_manager, + ObjectKeyPolicy::Logical, ) .await .expect("sources can be expanded before selection"); diff --git a/crates/cli/src/commands/mirror.rs b/crates/cli/src/commands/mirror.rs index 60fd2ab..bea53ba 100644 --- a/crates/cli/src/commands/mirror.rs +++ b/crates/cli/src/commands/mirror.rs @@ -9,10 +9,10 @@ use clap::{Args, ValueEnum}; use jiff::Timestamp; use rc_core::alias::RetryConfig; use rc_core::{ - AliasManager, Error, ListOptions, ObjectAttributes, ObjectInfo, ObjectStore as _, - ObjectWriteOptions, ParsedPath, RemotePath, TransferCandidate, TransferControls, - TransferExecutor, TransferOutcomeState, TransferPlan, TransferReport, TransferSelection, - TransferSummary, parse_path, + AliasManager, Error, ListOptions, ObjectAttributes, ObjectInfo, ObjectKeyPolicy, + ObjectStore as _, ObjectWriteOptions, ParsedPath, RemotePath, TransferCandidate, + TransferControls, TransferExecutor, TransferOutcomeState, TransferPlan, TransferReport, + TransferSelection, TransferSummary, normalize_relative_key, parse_path, }; use rc_s3::S3Client; use serde::Serialize; @@ -116,6 +116,10 @@ pub struct MirrorArgs { /// Suppress non-error mirror output (legacy command-local alias) #[arg(long)] pub quiet: bool, + + /// Reject object keys that cannot be created on Windows filesystems + #[arg(long)] + pub portable_names: bool, } #[derive(Debug, Serialize)] @@ -180,15 +184,25 @@ struct MirrorManifest { #[derive(Debug, Clone)] enum MirrorEndpointSpec { - Local(PathBuf), + Local { + root: PathBuf, + key_policy: ObjectKeyPolicy, + }, Remote(RemotePath), } impl MirrorEndpointSpec { + fn local(root: impl Into, key_policy: ObjectKeyPolicy) -> Self { + Self::Local { + root: root.into(), + key_policy, + } + } + fn location_for(&self, relative_path: &str) -> rc_core::Result { - let relative_path = normalize_relative_path(relative_path)?; match self { - Self::Local(root) => { + Self::Local { root, key_policy } => { + let relative_path = normalize_relative_path(relative_path, *key_policy)?; let mut target = root.clone(); for component in relative_path.split('/') { target.push(component); @@ -196,6 +210,8 @@ impl MirrorEndpointSpec { Ok(MirrorLocation::Local(target)) } Self::Remote(root) => { + let relative_path = + normalize_relative_path(relative_path, ObjectKeyPolicy::Logical)?; let prefix = normalized_remote_root_prefix(&root.key)?; Ok(MirrorLocation::Remote(RemotePath::new( &root.alias, @@ -310,6 +326,7 @@ struct MirrorOperationReports { enum RuntimeEndpoint { Local { root: PathBuf, + key_policy: ObjectKeyPolicy, }, Remote { root: RemotePath, @@ -320,7 +337,9 @@ enum RuntimeEndpoint { impl RuntimeEndpoint { fn spec(&self) -> MirrorEndpointSpec { match self { - Self::Local { root } => MirrorEndpointSpec::Local(root.clone()), + Self::Local { root, key_policy } => { + MirrorEndpointSpec::local(root.clone(), *key_policy) + } Self::Remote { root, .. } => MirrorEndpointSpec::Remote(root.clone()), } } @@ -328,8 +347,8 @@ impl RuntimeEndpoint { async fn current_entry(&self, relative_path: &str) -> rc_core::Result> { let location = self.spec().location_for(relative_path)?; match (&location, self) { - (MirrorLocation::Local(path), Self::Local { root }) => { - inspect_local_entry(root, relative_path, path).await + (MirrorLocation::Local(path), Self::Local { root, key_policy }) => { + inspect_local_entry(root, relative_path, path, *key_policy).await } (MirrorLocation::Remote(path), Self::Remote { client, .. }) => { match client.head_object(path).await { @@ -389,8 +408,9 @@ impl MirrorIo for LiveMirrorIo { } match (¤t.location, &self.target) { - (MirrorLocation::Local(path), RuntimeEndpoint::Local { root }) => { - let safe_path = secure_local_path(root, &operation.relative_path, false).await?; + (MirrorLocation::Local(path), RuntimeEndpoint::Local { root, key_policy }) => { + let safe_path = + secure_local_path(root, &operation.relative_path, false, *key_policy).await?; if &safe_path != path { return Err(Error::InvalidPath(format!( "Removal target escaped mirror root: {}", @@ -533,7 +553,7 @@ impl LiveMirrorIo { "Remote mirror source client is unavailable".to_string(), )); }; - let RuntimeEndpoint::Local { root } = &self.target else { + let RuntimeEndpoint::Local { root, key_policy } = &self.target else { return Err(Error::General( "Local mirror target root is unavailable".to_string(), )); @@ -548,7 +568,8 @@ impl LiveMirrorIo { TargetDisposition::Ready => {} } - let destination = secure_local_path(root, &operation.relative_path, true).await?; + let destination = + secure_local_path(root, &operation.relative_path, true, *key_policy).await?; if destination != target_path { return Err(Error::InvalidPath(format!( "Download target escaped mirror root: {}", @@ -783,26 +804,42 @@ pub async fn execute(args: MirrorArgs, mut output_config: OutputConfig) -> ExitC return formatter.fail(exit_code_for_error(&error), &error.to_string()); } - let (source_runtime, source_manifest) = - match prepare_endpoint(&source, MissingRootPolicy::Error, &alias_manager).await { - Ok(prepared) => prepared, - Err(error) => { - return formatter.fail( - exit_code_for_error(&error), - &format!("Failed to enumerate mirror source: {error}"), - ); - } - }; - let (target_runtime, mut target_manifest) = - match prepare_endpoint(&target, MissingRootPolicy::Empty, &alias_manager).await { - Ok(prepared) => prepared, - Err(error) => { - return formatter.fail( - exit_code_for_error(&error), - &format!("Failed to enumerate mirror destination: {error}"), - ); - } - }; + let dest_key_policy = match &target { + ParsedPath::Local(_) => ObjectKeyPolicy::for_local_destination(args.portable_names), + ParsedPath::Remote(_) => ObjectKeyPolicy::Logical, + }; + let (source_runtime, source_manifest) = match prepare_endpoint( + &source, + MissingRootPolicy::Error, + &alias_manager, + ObjectKeyPolicy::Logical, + ) + .await + { + Ok(prepared) => prepared, + Err(error) => { + return formatter.fail( + exit_code_for_error(&error), + &format!("Failed to enumerate mirror source: {error}"), + ); + } + }; + let (target_runtime, mut target_manifest) = match prepare_endpoint( + &target, + MissingRootPolicy::Empty, + &alias_manager, + dest_key_policy, + ) + .await + { + Ok(prepared) => prepared, + Err(error) => { + return formatter.fail( + exit_code_for_error(&error), + &format!("Failed to enumerate mirror destination: {error}"), + ); + } + }; if let Err(error) = enrich_destination_identity( &source_manifest, &mut target_manifest, @@ -951,11 +988,18 @@ async fn prepare_endpoint( parsed: &ParsedPath, missing_root: MissingRootPolicy, alias_manager: &AliasManager, + key_policy: ObjectKeyPolicy, ) -> rc_core::Result<(RuntimeEndpoint, MirrorManifest)> { match parsed { ParsedPath::Local(root) => { let manifest = enumerate_local_manifest(root, missing_root)?; - Ok((RuntimeEndpoint::Local { root: root.clone() }, manifest)) + Ok(( + RuntimeEndpoint::Local { + root: root.clone(), + key_policy, + }, + manifest, + )) } ParsedPath::Remote(root) => { let alias = alias_manager @@ -1099,7 +1143,7 @@ async fn enumerate_remote_manifest( if raw_relative.is_empty() { continue; } - let relative_path = normalize_relative_path(raw_relative)?; + let relative_path = normalize_relative_path(raw_relative, ObjectKeyPolicy::Logical)?; let snapshot = snapshot_from_object(&object)?; insert_manifest_entry( &mut manifest.entries, @@ -1440,54 +1484,8 @@ fn output_outcomes(formatter: &Formatter, marker: &str, report: &TransferRepo } } -fn normalize_relative_path(value: &str) -> rc_core::Result { - if value.starts_with(['/', '\\']) || value.contains('\\') { - return Err(Error::InvalidPath(format!( - "Mirror path must be relative and use '/' separators: {value}" - ))); - } - let mut normalized = Vec::new(); - for component in value.split('/') { - if component.is_empty() || component == "." { - continue; - } - if component == ".." { - return Err(Error::InvalidPath( - "Mirror paths must not contain traversal components".to_string(), - )); - } - validate_portable_component(component)?; - normalized.push(component); - } - if normalized.is_empty() { - return Err(Error::InvalidPath( - "Mirror path does not contain a file name".to_string(), - )); - } - Ok(normalized.join("/")) -} - -fn validate_portable_component(component: &str) -> rc_core::Result<()> { - if component.chars().any(|character| { - character.is_control() || matches!(character, ':' | '<' | '>' | '"' | '|' | '?' | '*') - }) || component.ends_with(['.', ' ']) - { - return Err(Error::InvalidPath(format!( - "Mirror path component is not portable: {component}" - ))); - } - let stem = component.split('.').next().unwrap_or_default(); - let stem = stem.to_ascii_uppercase(); - if matches!(stem.as_str(), "CON" | "PRN" | "AUX" | "NUL") - || (stem.len() == 4 - && (stem.starts_with("COM") || stem.starts_with("LPT")) - && matches!(stem.as_bytes()[3], b'1'..=b'9')) - { - return Err(Error::InvalidPath(format!( - "Mirror path uses a reserved device name: {component}" - ))); - } - Ok(()) +fn normalize_relative_path(value: &str, policy: ObjectKeyPolicy) -> rc_core::Result { + normalize_relative_key(value, policy) } fn local_relative_path(root: &Path, path: &Path) -> rc_core::Result { @@ -1510,7 +1508,7 @@ fn local_relative_path(root: &Path, path: &Path) -> rc_core::Result { })?; components.push(component); } - normalize_relative_path(&components.join("/")) + normalize_relative_path(&components.join("/"), ObjectKeyPolicy::Logical) } fn normalized_remote_root_prefix(key: &str) -> rc_core::Result { @@ -1523,7 +1521,10 @@ fn normalized_remote_root_prefix(key: &str) -> rc_core::Result { if key.is_empty() { return Ok(String::new()); } - Ok(format!("{}/", normalize_relative_path(key)?)) + Ok(format!( + "{}/", + normalize_relative_path(key, ObjectKeyPolicy::Logical)? + )) } fn snapshot_from_metadata(metadata: &std::fs::Metadata) -> MirrorSnapshot { @@ -1823,8 +1824,9 @@ async fn inspect_local_entry( root: &Path, relative_path: &str, expected_path: &Path, + key_policy: ObjectKeyPolicy, ) -> rc_core::Result> { - let path = secure_local_path(root, relative_path, false).await?; + let path = secure_local_path(root, relative_path, false, key_policy).await?; if path != expected_path { return Err(Error::InvalidPath(format!( "Local mirror target escaped its root: {}", @@ -1854,8 +1856,9 @@ async fn secure_local_path( root: &Path, relative_path: &str, create_parents: bool, + key_policy: ObjectKeyPolicy, ) -> rc_core::Result { - let relative_path = normalize_relative_path(relative_path)?; + let relative_path = normalize_relative_path(relative_path, key_policy)?; match tokio::fs::symlink_metadata(root).await { Ok(metadata) if metadata.file_type().is_symlink() => { return Err(Error::InvalidPath(format!( diff --git a/crates/cli/src/commands/mirror/roadmap_tests.rs b/crates/cli/src/commands/mirror/roadmap_tests.rs index 948f745..4f85e43 100644 --- a/crates/cli/src/commands/mirror/roadmap_tests.rs +++ b/crates/cli/src/commands/mirror/roadmap_tests.rs @@ -56,18 +56,48 @@ fn manifest(entries: impl IntoIterator) -> MirrorManifest { #[test] fn relative_paths_are_normalized_and_traversal_is_rejected() { assert_eq!( - normalize_relative_path("nested//./report.txt").expect("normal relative path"), + normalize_relative_path("nested//./report.txt", ObjectKeyPolicy::Logical) + .expect("normal relative path"), "nested/report.txt" ); for value in [ "../secret", "nested/../../secret", "/absolute", + "nested/control\u{0007}.txt", + ] { + assert!( + normalize_relative_path(value, ObjectKeyPolicy::Logical).is_err(), + "accepted {value}" + ); + } +} + +#[test] +fn logical_relative_paths_accept_colon_object_keys() { + assert_eq!( + normalize_relative_path( + "fake/deadbeef/19f6abd9af4:19f6abe0e77:499628ff", + ObjectKeyPolicy::Logical + ) + .expect("colon keys are valid S3 object names"), + "fake/deadbeef/19f6abd9af4:19f6abe0e77:499628ff" + ); +} + +#[test] +fn windows_portable_relative_paths_reject_reserved_names() { + for value in [ "C:/windows", "nested/bad?.txt", - "nested/control\u{0007}.txt", + "safe:stream", + "CON.txt", + "trailing.", ] { - assert!(normalize_relative_path(value).is_err(), "accepted {value}"); + assert!( + normalize_relative_path(value, ObjectKeyPolicy::WindowsPortable).is_err(), + "accepted portable-unsafe {value}" + ); } } @@ -86,7 +116,7 @@ fn planners_map_all_supported_directions_without_changing_relative_paths() { "nested/report.txt", "etag-1", )]), - MirrorEndpointSpec::Local(PathBuf::from("/target")), + MirrorEndpointSpec::local(PathBuf::from("/target"), ObjectKeyPolicy::Logical), MirrorLocation::Local(PathBuf::from("/target").join("nested").join("report.txt")), ), ( @@ -320,9 +350,11 @@ async fn a_removal_retry_treats_an_already_absent_target_as_complete() { let io = LiveMirrorIo { source: RuntimeEndpoint::Local { root: root.path().to_path_buf(), + key_policy: ObjectKeyPolicy::Logical, }, target: RuntimeEndpoint::Local { root: root.path().to_path_buf(), + key_policy: ObjectKeyPolicy::Logical, }, compare: CompareMode::Auto, }; @@ -386,7 +418,7 @@ fn empty_manifests_produce_empty_deterministic_plans() { let copy = build_copy_plan( &MirrorManifest::default(), &MirrorManifest::default(), - &MirrorEndpointSpec::Local(PathBuf::from("/target")), + &MirrorEndpointSpec::local(PathBuf::from("/target"), ObjectKeyPolicy::Logical), &TransferSelection::default(), true, CompareMode::Auto, @@ -443,11 +475,62 @@ fn filtered_nested_tree_is_stably_sorted() { #[test] fn target_mapping_rejects_non_normal_relative_paths() { - let target = MirrorEndpointSpec::Local(PathBuf::from("/target")); + let target = MirrorEndpointSpec::local(PathBuf::from("/target"), ObjectKeyPolicy::Logical); assert!(target.location_for("../outside").is_err()); assert!(normalized_remote_root_prefix("/absolute").is_err()); } +#[test] +fn local_logical_target_maps_colon_object_keys() { + let source = manifest([remote_entry( + "src", + "loki/fake/deadbeef/19f6abd9af4:19f6abe0e77:499628ff", + "fake/deadbeef/19f6abd9af4:19f6abe0e77:499628ff", + "etag-1", + )]); + let plan = build_copy_plan( + &source, + &MirrorManifest::default(), + &MirrorEndpointSpec::local(PathBuf::from("/restore"), ObjectKeyPolicy::Logical), + &TransferSelection::default(), + false, + CompareMode::Auto, + ) + .expect("colon keys are planned onto Unix destinations"); + + assert_eq!(plan.items.len(), 1); + assert_eq!( + plan.items[0].payload.target, + MirrorLocation::Local( + PathBuf::from("/restore") + .join("fake") + .join("deadbeef") + .join("19f6abd9af4:19f6abe0e77:499628ff") + ) + ); +} + +#[test] +fn windows_portable_local_target_rejects_colon_object_keys() { + let source = manifest([remote_entry( + "src", + "loki/fake/deadbeef/19f6abd9af4:19f6abe0e77:499628ff", + "fake/deadbeef/19f6abd9af4:19f6abe0e77:499628ff", + "etag-1", + )]); + let error = build_copy_plan( + &source, + &MirrorManifest::default(), + &MirrorEndpointSpec::local(PathBuf::from("/restore"), ObjectKeyPolicy::WindowsPortable), + &TransferSelection::default(), + false, + CompareMode::Auto, + ) + .expect_err("Windows-portable destinations reject colon keys"); + + assert!(error.to_string().contains("portable")); +} + #[test] fn manifest_rejects_normalization_collisions() { let mut entries = BTreeMap::new(); @@ -1220,7 +1303,7 @@ async fn missing_multilevel_local_target_root_is_created_one_directory_at_a_time std::fs::create_dir(&existing).expect("create existing ancestor"); let root = existing.join("a/b/new-root"); - let destination = secure_local_path(&root, "nested/file.txt", true) + let destination = secure_local_path(&root, "nested/file.txt", true, ObjectKeyPolicy::Logical) .await .expect("create safe target directories"); @@ -1248,7 +1331,7 @@ async fn plain_relative_multilevel_target_uses_the_current_directory_as_its_ance drop(placeholder); let root = relative_base.join("foo/bar"); - let result = secure_local_path(&root, "nested/file.txt", true).await; + let result = secure_local_path(&root, "nested/file.txt", true, ObjectKeyPolicy::Logical).await; let destination = result.expect("create plain relative target tree"); assert_eq!(destination, root.join("nested/file.txt")); @@ -1347,6 +1430,7 @@ fn mirror_arguments_keep_legacy_parallel_alias_and_safe_defaults() { assert_eq!(defaults.retry_attempts, 3); assert!(!defaults.remove); assert!(!defaults.overwrite); + assert!(!defaults.portable_names); let legacy = MirrorArgumentParser::try_parse_from([ "test", diff --git a/crates/cli/src/commands/mod.rs b/crates/cli/src/commands/mod.rs index 11cd192..e408f4c 100644 --- a/crates/cli/src/commands/mod.rs +++ b/crates/cli/src/commands/mod.rs @@ -1691,6 +1691,40 @@ mod tests { } } + #[test] + fn cli_accepts_portable_names_on_mirror_and_copy() { + let mirror = Cli::try_parse_from([ + "rc", + "mirror", + "local/loki/", + "./restore/", + "--portable-names", + ]) + .expect("parse mirror portable-names"); + match mirror.command { + Commands::Mirror(args) => assert!(args.portable_names), + other => panic!("expected mirror command, got {other:?}"), + } + + let copy = Cli::try_parse_from([ + "rc", + "object", + "copy", + "local/loki/", + "./restore/", + "--recursive", + "--portable-names", + ]) + .expect("parse object copy portable-names"); + match copy.command { + Commands::Object(args) => match args.command { + object::ObjectCommands::Copy(args) => assert!(args.portable_names), + other => panic!("expected object copy command, got {other:?}"), + }, + other => panic!("expected object command, got {other:?}"), + } + } + #[test] fn version_selector_rejects_ambiguous_or_empty_values() { assert!(validate_version_selector(Some("v1"), None).is_ok()); diff --git a/crates/cli/src/commands/mv.rs b/crates/cli/src/commands/mv.rs index ade757e..81100a0 100644 --- a/crates/cli/src/commands/mv.rs +++ b/crates/cli/src/commands/mv.rs @@ -303,7 +303,11 @@ async fn move_s3_prefix_to_local( let mut errors = 0usize; for item in objects { - let relative = match cp::safe_download_relative_path(&item.key, &src.key) { + let relative = match cp::safe_download_relative_path( + &item.key, + &src.key, + rc_core::ObjectKeyPolicy::for_local_destination(false), + ) { Ok(relative) => relative, Err(error) => { errors += 1; diff --git a/crates/cli/tests/help_contract.rs b/crates/cli/tests/help_contract.rs index 888da9f..c7208a3 100644 --- a/crates/cli/tests/help_contract.rs +++ b/crates/cli/tests/help_contract.rs @@ -352,6 +352,7 @@ fn top_level_command_help_contract() { "--retry-attempts", "--fail-empty", "--summary", + "--portable-names", "Examples:", "rc object copy ./report.json local/my-bucket/reports/", ], @@ -439,6 +440,7 @@ fn top_level_command_help_contract() { "--retry-initial-backoff-ms", "--retry-max-backoff-ms", "--summary", + "--portable-names", "--compare", ], }, @@ -997,6 +999,7 @@ fn nested_subcommand_help_contract() { "--retry-attempts", "--fail-empty", "--summary", + "--portable-names", "Examples:", "rc object copy ./report.json local/my-bucket/reports/", ], diff --git a/crates/core/src/lib.rs b/crates/core/src/lib.rs index cf69c2c..cc66131 100644 --- a/crates/core/src/lib.rs +++ b/crates/core/src/lib.rs @@ -17,6 +17,7 @@ pub mod encryption; pub mod error; pub mod lifecycle; pub mod multipart_copy; +pub mod object_key; pub mod object_lock; pub mod ops; pub mod path; @@ -47,6 +48,7 @@ pub use multipart_copy::{ S3_MAX_OBJECT_SIZE, S3_MULTIPART_COPY_MAX_PART_SIZE, S3_MULTIPART_COPY_MAX_PARTS, S3_MULTIPART_COPY_MIN_PART_SIZE, S3_SINGLE_COPY_MAX_SIZE, requires_multipart_copy, }; +pub use object_key::{ObjectKeyPolicy, normalize_relative_key, relative_local_path_from_key}; pub use object_lock::{ BucketObjectLockConfiguration, DefaultRetention, LegalHoldStatus, ObjectLockOptions, ObjectRetention, RetentionDuration, RetentionDurationUnit, RetentionMode, diff --git a/crates/core/src/object_key.rs b/crates/core/src/object_key.rs new file mode 100644 index 0000000..2491bfd --- /dev/null +++ b/crates/core/src/object_key.rs @@ -0,0 +1,273 @@ +//! Object-key normalization and local-name safety. +//! +//! S3 keys may contain characters that some local filesystems reject. Traversal +//! and control-character checks always apply. Windows filename rules apply only +//! when a key is being materialized onto a local filesystem that needs them. + +use std::path::PathBuf; + +use crate::error::{Error, Result}; + +/// How a relative object key should be validated. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ObjectKeyPolicy { + /// Security checks only: relative `/` paths, no traversal, no control characters. + /// + /// Use this for remote-to-remote work and for local destinations on Unix-like + /// filesystems, where characters such as `:` are legal in file names. + Logical, + /// Also reject names that cannot be created portably on Windows filesystems. + WindowsPortable, +} + +impl ObjectKeyPolicy { + /// Policy for writing an object key onto a local filesystem. + /// + /// Windows destinations always use [`Self::WindowsPortable`]. Other platforms + /// stay on [`Self::Logical`] unless the caller requests portable names. + pub fn for_local_destination(force_portable: bool) -> Self { + if force_portable || cfg!(windows) { + Self::WindowsPortable + } else { + Self::Logical + } + } + + /// Policy for remote object keys. S3 does not use Windows filename rules. + pub const fn for_remote_destination() -> Self { + Self::Logical + } +} + +/// Normalize a source-relative object key. +/// +/// The result uses `/` separators, rejects traversal, and optionally applies +/// Windows filename portability rules. +pub fn normalize_relative_key(value: &str, policy: ObjectKeyPolicy) -> Result { + if value.starts_with(['/', '\\']) || value.contains('\\') { + return Err(Error::InvalidPath(format!( + "Object key must be relative and use '/' separators: {value}" + ))); + } + + let mut normalized = Vec::new(); + for component in value.split('/') { + if component.is_empty() || component == "." { + continue; + } + if component == ".." { + return Err(Error::InvalidPath( + "Object keys must not contain traversal components".to_string(), + )); + } + validate_key_component(component, policy)?; + normalized.push(component); + } + + if normalized.is_empty() { + return Err(Error::InvalidPath( + "Object key does not contain a file name".to_string(), + )); + } + + Ok(normalized.join("/")) +} + +/// Strip `prefix` from `key` and return a relative local path. +/// +/// Traversal and other unsafe components are rejected before any filesystem +/// join so a single hostile key cannot escape the destination root. +pub fn relative_local_path_from_key( + key: &str, + prefix: &str, + policy: ObjectKeyPolicy, +) -> Result { + let relative = key + .strip_prefix(prefix) + .ok_or_else(|| Error::InvalidPath(format!("key is outside requested prefix '{prefix}'")))? + .trim_start_matches('/'); + let normalized = normalize_relative_key(relative, policy)?; + Ok(normalized.split('/').collect()) +} + +fn validate_key_component(component: &str, policy: ObjectKeyPolicy) -> Result<()> { + if component.chars().any(char::is_control) { + return Err(Error::InvalidPath(format!( + "Object key component contains a control character: {component}" + ))); + } + + if policy != ObjectKeyPolicy::WindowsPortable { + return Ok(()); + } + + if component + .chars() + .any(|character| matches!(character, ':' | '<' | '>' | '"' | '|' | '?' | '*')) + || component.ends_with(['.', ' ']) + { + return Err(Error::InvalidPath(format!( + "Object key component is not portable: {component}" + ))); + } + + let stem = component.split('.').next().unwrap_or_default(); + let stem = stem.to_ascii_uppercase(); + if matches!(stem.as_str(), "CON" | "PRN" | "AUX" | "NUL") + || (stem.len() == 4 + && (stem.starts_with("COM") || stem.starts_with("LPT")) + && matches!(stem.as_bytes()[3], b'1'..=b'9')) + { + return Err(Error::InvalidPath(format!( + "Object key uses a reserved device name: {component}" + ))); + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn logical_policy_accepts_colon_keys() { + assert_eq!( + normalize_relative_key( + "fake/deadbeef/19f6abd9af4:19f6abe0e77:499628ff", + ObjectKeyPolicy::Logical + ) + .expect("colon key is valid on Unix"), + "fake/deadbeef/19f6abd9af4:19f6abe0e77:499628ff" + ); + } + + #[test] + fn logical_policy_accepts_question_and_asterisk_in_names() { + assert_eq!( + normalize_relative_key("logs/what?.txt", ObjectKeyPolicy::Logical).expect("valid"), + "logs/what?.txt" + ); + assert_eq!( + normalize_relative_key("logs/star*.txt", ObjectKeyPolicy::Logical).expect("valid"), + "logs/star*.txt" + ); + } + + #[test] + fn windows_policy_rejects_colon_and_reserved_names() { + for value in [ + "safe:stream", + "nested/bad?.txt", + "CON.txt", + "com1.log", + "trailing.", + "trailing ", + ] { + let error = + normalize_relative_key(value, ObjectKeyPolicy::WindowsPortable).expect_err(value); + assert!( + error.to_string().contains("portable") + || error.to_string().contains("reserved device name"), + "{value}: {error}" + ); + } + } + + #[test] + fn both_policies_reject_traversal_and_control_characters() { + for policy in [ObjectKeyPolicy::Logical, ObjectKeyPolicy::WindowsPortable] { + for value in [ + "../secret", + "nested/../../secret", + "/absolute", + "nested\\escaped", + "nested/control\u{0007}.txt", + ] { + assert!( + normalize_relative_key(value, policy).is_err(), + "policy {policy:?} accepted {value}" + ); + } + } + } + + #[test] + fn local_destination_policy_is_logical_on_non_windows_by_default() { + let policy = ObjectKeyPolicy::for_local_destination(false); + if cfg!(windows) { + assert_eq!(policy, ObjectKeyPolicy::WindowsPortable); + } else { + assert_eq!(policy, ObjectKeyPolicy::Logical); + } + assert_eq!( + ObjectKeyPolicy::for_local_destination(true), + ObjectKeyPolicy::WindowsPortable + ); + assert_eq!( + ObjectKeyPolicy::for_remote_destination(), + ObjectKeyPolicy::Logical + ); + } + + #[test] + fn relative_local_path_preserves_nested_colon_keys() { + let path = relative_local_path_from_key( + "loki/fake/deadbeef/19f6abd9af4:19f6abe0e77:499628ff", + "loki/", + ObjectKeyPolicy::Logical, + ) + .expect("colon key should map onto a Unix path"); + + assert_eq!( + path, + PathBuf::from("fake") + .join("deadbeef") + .join("19f6abd9af4:19f6abe0e77:499628ff") + ); + } + + #[test] + fn relative_local_path_rejects_keys_outside_prefix() { + let error = + relative_local_path_from_key("other/file.txt", "loki/", ObjectKeyPolicy::Logical) + .expect_err("outside prefix"); + assert!(error.to_string().contains("outside requested prefix")); + } + + #[test] + fn relative_local_path_rejects_colon_keys_when_portable() { + assert!( + relative_local_path_from_key( + "loki/fake/deadbeef/19f6abd9af4:19f6abe0e77:499628ff", + "loki/", + ObjectKeyPolicy::WindowsPortable + ) + .is_err() + ); + } + + #[test] + fn normalize_relative_key_rejects_empty_or_dot_only_keys() { + for policy in [ObjectKeyPolicy::Logical, ObjectKeyPolicy::WindowsPortable] { + for value in ["", ".", "./", "//"] { + assert!( + normalize_relative_key(value, policy).is_err(), + "policy {policy:?} accepted {value:?}" + ); + } + } + } + + #[test] + fn relative_local_path_rejects_traversal_after_prefix() { + assert!( + relative_local_path_from_key( + "reports/../../escaped", + "reports/", + ObjectKeyPolicy::Logical + ) + .is_err() + ); + } +} diff --git a/docs/reference/rc/cp.md b/docs/reference/rc/cp.md index 3c3bb34..da2b07a 100644 --- a/docs/reference/rc/cp.md +++ b/docs/reference/rc/cp.md @@ -37,6 +37,7 @@ rc [GLOBAL OPTIONS] cp [OPTIONS] ... | `--continue-on-error` | Continue eligible work after an item fails; the final exit code remains non-zero. | | `--fail-empty` | Return the not-found exit code when no source passes selection. | | `--summary` | Print deterministic aggregate counters in human output; bulk and recursive copies summarize automatically. | +| `--portable-names` | When downloading to a local filesystem, reject keys that cannot be created on Windows. Unix destinations accept characters such as `:` by default. | ## Examples @@ -106,6 +107,12 @@ The current implementation supports `SSE-S3` and `SSE-KMS`. It does not support When the server returns a source or destination object version ID, JSON copy output uses the output v3 `versioned_objects` envelope with `data.operation` set to `copy`. `data.source_version_id` identifies the copied source version and `data.version_id` identifies the created destination version. Copies for which the backend reports no version information retain the legacy JSON shape. +Recursive downloads map object keys onto the local filesystem using `/` separators. Traversal, absolute keys, backslashes, and control characters are always rejected. Characters such as `:` are accepted on Unix destinations unless `--portable-names` is set. + +### BREAKING object-key portability contract migration + +`--portable-names` is additive. Unix destinations no longer apply Windows filename rules by default, so keys containing `:` are accepted. Remote-to-remote copies keep the original key. This PR must be marked `BREAKING` because `docs/reference/rc/cp.md` is a protected CLI behavior contract. No JSON schema or config `schema_version` bump applies. + ### BREAKING cross-alias copy contract migration Cross-alias `rc cp` is additive and does not change same-alias CopyObject behavior. Destinations on a different alias are copied by download then upload instead of failing as `unsupported_feature`. Source content type and user metadata are preserved unless `--metadata-directive replace` is set. This PR must be marked `BREAKING` because `docs/reference/rc/cp.md` is a protected CLI behavior contract. No JSON schema or config `schema_version` bump applies. diff --git a/docs/reference/rc/mirror.md b/docs/reference/rc/mirror.md index 5029f94..c225b0f 100644 --- a/docs/reference/rc/mirror.md +++ b/docs/reference/rc/mirror.md @@ -32,6 +32,7 @@ rc [GLOBAL OPTIONS] mirror [OPTIONS] | `--summary` | Print deterministic aggregate counts and transferred bytes in human output. | | `--compare ` | Choose how existing destination objects are compared before a copy is skipped. Defaults to `auto`. | | `--quiet` | Suppress non-error command output. The global `--quiet` option has the same effect. | +| `--portable-names` | Reject keys that cannot be created on Windows filesystems. Unix destinations accept characters such as `:` by default. | ## Examples @@ -49,7 +50,7 @@ rc mirror stage/data/ prod/data/ --overwrite --compare auto Both operands are directory-like roots. Every selected source-relative path is appended to the destination root without flattening. Remote prefixes are normalized to one trailing `/`, so `alias/bucket/prefix` and `alias/bucket/prefix/` map the same tree. Remote listing is paginated and the final plan is sorted by normalized relative path. -Remote keys that are absolute, contain traversal, use backslashes, collide after normalization, or cannot be represented portably on supported local platforms are rejected. A new relative local target should be written explicitly, for example `./restore/`, so it cannot be confused with `ALIAS/BUCKET` syntax. +Remote keys that are absolute, contain traversal, use backslashes, contain control characters, or collide after normalization are rejected. Windows filename rules (`:`, reserved device names, trailing dots or spaces) apply only when the destination is a local filesystem that needs them: always on Windows, and on other platforms only when `--portable-names` is set. Remote-to-remote copies keep the original object key, including characters such as `:`. A new relative local target should be written explicitly, for example `./restore/`, so it cannot be confused with `ALIAS/BUCKET` syntax. ### Comparison and restart behavior @@ -83,6 +84,10 @@ Copy and removal phases use the shared transfer controls for filtering, concurre Existing remote-to-remote commands continue to work. `--parallel` is retained as an alias of the shared `--concurrency` option. Mirror no longer falls back to an unconditional byte copy when source metadata lookup fails, and missing remote ETags are no longer treated as proof of equality. Automation that depended on either unsafe fallback must handle explicit network or conflict exits and retry after re-planning. +### BREAKING object-key portability contract migration + +`--portable-names` is additive. Unix destinations no longer apply Windows filename rules by default, so Loki-style keys containing `:` can be mirrored locally. Remote-to-remote copies keep the original object key. This PR must be marked `BREAKING` because `docs/reference/rc/mirror.md` is a protected CLI behavior contract. No JSON schema or config `schema_version` bump applies. + ### BREAKING incremental identity contract migration `--compare auto|etag|size` and destination metadata `x-amz-meta-rc-source-etag` are additive. Default skip-on-matching-ETag behavior is unchanged. Objects copied before this change still recopy once under `auto`, then skip. This PR must be marked `BREAKING` because `docs/reference/rc/mirror.md` is a protected CLI behavior contract. No JSON schema or config `schema_version` bump applies. diff --git a/scripts/regression/object-key-safety.sh b/scripts/regression/object-key-safety.sh new file mode 100755 index 0000000..063703b --- /dev/null +++ b/scripts/regression/object-key-safety.sh @@ -0,0 +1,68 @@ +#!/usr/bin/env bash +# +# Regression tests for object-key path safety (#338). +# +# Usage: +# ./scripts/regression/object-key-safety.sh +# +# These tests do not require a running S3 backend. They cover: +# - Unix destinations accepting ':' in object keys (Loki chunks) +# - Windows-portable mode still rejecting reserved names +# - Traversal and control-character rejection on every policy +# - CLI help/parse contract for --portable-names +# + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PROJECT_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" +cd "$PROJECT_ROOT" + +RED='\033[0;31m' +GREEN='\033[0;32m' +BLUE='\033[0;34m' +NC='\033[0m' + +log_info() { echo -e "${BLUE}[INFO]${NC} $*"; } +log_success() { echo -e "${GREEN}[PASS]${NC} $*"; } +log_error() { echo -e "${RED}[FAIL]${NC} $*"; } + +run_tests() { + local description="$1" + shift + log_info "$description" + if "$@"; then + log_success "$description" + else + log_error "$description" + return 1 + fi +} + +log_info "Object-key safety regression suite" + +run_tests "core object-key policy unit tests" \ + cargo test -p rc-core --lib -- object_key:: + +run_tests "mirror path-policy unit tests" \ + cargo test -p rustfs-cli --lib -- \ + logical_relative_paths_accept_colon_object_keys \ + windows_portable_relative_paths_reject_reserved_names \ + local_logical_target_maps_colon_object_keys \ + windows_portable_local_target_rejects_colon_object_keys \ + relative_paths_are_normalized_and_traversal_is_rejected + +run_tests "copy download path-policy unit tests" \ + cargo test -p rustfs-cli --lib -- \ + download_relative_path_preserves_safe_nested_keys \ + download_relative_path_rejects_traversal_and_absolute_keys \ + download_relative_path_accepts_colon_keys_on_logical_destinations \ + download_relative_path_rejects_colon_keys_when_portable_names_requested + +run_tests "CLI parse contract" \ + cargo test -p rustfs-cli --lib -- cli_accepts_portable_names_on_mirror_and_copy + +run_tests "CLI help contract" \ + cargo test -p rustfs-cli --test help_contract + +log_success "Object-key safety regression suite passed"