From af96ec483be683386af53327cd2868763faa2308 Mon Sep 17 00:00:00 2001 From: overtrue Date: Mon, 9 Mar 2026 04:25:19 +0800 Subject: [PATCH 1/4] feat(phase-2): implement find exec/print behavior with e2e coverage --- .github/workflows/integration.yml | 1 + crates/cli/src/commands/find.rs | 83 ++++++++++++++++++- crates/cli/tests/integration.rs | 131 ++++++++++++++++++++++++++++++ 3 files changed, 212 insertions(+), 3 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 21118f1..f9c1915 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -76,6 +76,7 @@ jobs: "quota_operations::test_bucket_quota_set_info_clear" "option_behavior_operations::test_cp_dry_run_does_not_create_target_object" "option_behavior_operations::test_head_bytes_returns_prefix_bytes" + "option_behavior_operations::test_find_print_outputs_full_remote_path" ) for test_name in "${TESTS[@]}"; do diff --git a/crates/cli/src/commands/find.rs b/crates/cli/src/commands/find.rs index a72454b..dbf2c93 100644 --- a/crates/cli/src/commands/find.rs +++ b/crates/cli/src/commands/find.rs @@ -6,6 +6,8 @@ use clap::Args; use rc_core::{AliasManager, ListOptions, ObjectStore as _, RemotePath}; use rc_s3::S3Client; use serde::Serialize; +use std::io::Write as _; +use std::process::Command; use crate::exit_code::ExitCode; use crate::output::{Formatter, OutputConfig}; @@ -130,6 +132,58 @@ pub async fn execute(args: FindArgs, output_config: OutputConfig) -> ExitCode { } }; + // Execute command for each match if requested. + // This mode is human-output only because command output cannot be embedded in JSON. + if let Some(exec_template) = args.exec.as_deref() { + if formatter.is_json() { + formatter.error("--exec cannot be used with --json output"); + return ExitCode::UsageError; + } + + for m in &matches { + let object_path = full_object_path(&alias_name, &bucket, &m.key); + let command_text = exec_template.replace("{}", &object_path); + let output = match run_shell_command(&command_text) { + Ok(output) => output, + Err(e) => { + formatter.error(&format!("Failed to run command for {}: {}", object_path, e)); + return ExitCode::GeneralError; + } + }; + + if std::io::stdout().write_all(&output.stdout).is_err() { + formatter.error("Failed to write command stdout"); + return ExitCode::GeneralError; + } + if std::io::stderr().write_all(&output.stderr).is_err() { + formatter.error("Failed to write command stderr"); + return ExitCode::GeneralError; + } + + if !output.status.success() { + formatter.error(&format!( + "Command failed for {}: {}", + object_path, command_text + )); + return ExitCode::GeneralError; + } + } + } + + let display_matches: Vec = matches + .iter() + .map(|m| MatchInfo { + key: if args.print { + full_object_path(&alias_name, &bucket, &m.key) + } else { + m.key.clone() + }, + size_bytes: m.size_bytes, + size_human: m.size_human.clone(), + last_modified: m.last_modified.clone(), + }) + .collect(); + // Calculate totals let total_count = matches.len(); let total_size: i64 = matches.iter().filter_map(|m| m.size_bytes).sum(); @@ -153,16 +207,16 @@ pub async fn execute(args: FindArgs, output_config: OutputConfig) -> ExitCode { } } else if formatter.is_json() { let output = FindOutput { - matches, + matches: display_matches, total_count, total_size_bytes: total_size, total_size_human: humansize::format_size(total_size as u64, humansize::BINARY), }; formatter.json(&output); - } else if matches.is_empty() { + } else if display_matches.is_empty() { formatter.println("No matches found."); } else { - for m in &matches { + for m in &display_matches { let size = m.size_human.as_deref().unwrap_or("0B"); let styled_size = formatter.style_size(&format!("{:>10}", size)); let styled_key = formatter.style_file(&m.key); @@ -179,6 +233,21 @@ pub async fn execute(args: FindArgs, output_config: OutputConfig) -> ExitCode { ExitCode::Success } +fn full_object_path(alias: &str, bucket: &str, key: &str) -> String { + format!("{alias}/{bucket}/{key}") +} + +fn run_shell_command(command: &str) -> std::io::Result { + #[cfg(target_family = "windows")] + { + Command::new("cmd").args(["/C", command]).output() + } + #[cfg(not(target_family = "windows"))] + { + Command::new("sh").args(["-c", command]).output() + } +} + /// Filters for find command struct FindFilters { name_pattern: Option, @@ -431,4 +500,12 @@ mod tests { assert!(parse_find_path("").is_err()); assert!(parse_find_path("myalias").is_err()); } + + #[test] + fn test_full_object_path() { + assert_eq!( + full_object_path("test", "bucket", "a/b.txt"), + "test/bucket/a/b.txt" + ); + } } diff --git a/crates/cli/tests/integration.rs b/crates/cli/tests/integration.rs index 010d742..334db25 100644 --- a/crates/cli/tests/integration.rs +++ b/crates/cli/tests/integration.rs @@ -2833,4 +2833,135 @@ mod option_behavior_operations { cleanup_bucket(config_dir.path(), &bucket_name); } + + #[test] + fn test_find_print_outputs_full_remote_path() { + let (config_dir, bucket_name) = match setup_with_alias("findprint") { + Some(v) => v, + None => { + eprintln!("Skipping: S3 test config not available"); + return; + } + }; + + upload_text_object( + config_dir.path(), + &bucket_name, + "dir/sample.txt", + "print me", + ); + + let full_path = format!("test/{}/dir/sample.txt", bucket_name); + let output = run_rc( + &[ + "find", + &format!("test/{}/", bucket_name), + "--name", + "*.txt", + "--print", + "--no-color", + ], + config_dir.path(), + ); + assert!( + output.status.success(), + "find --print failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains(&full_path), + "Expected full remote path in output when using --print, got: {}", + stdout + ); + + cleanup_bucket(config_dir.path(), &bucket_name); + } + + #[test] + fn test_find_exec_runs_command_with_full_remote_path() { + let (config_dir, bucket_name) = match setup_with_alias("findexec") { + Some(v) => v, + None => { + eprintln!("Skipping: S3 test config not available"); + return; + } + }; + + upload_text_object(config_dir.path(), &bucket_name, "exec-a.txt", "a"); + upload_text_object(config_dir.path(), &bucket_name, "exec-b.txt", "b"); + + let path_a = format!("test/{}/exec-a.txt", bucket_name); + let path_b = format!("test/{}/exec-b.txt", bucket_name); + let output = run_rc( + &[ + "find", + &format!("test/{}/", bucket_name), + "--name", + "*.txt", + "--exec", + "echo EXEC:{}", + "--no-color", + ], + config_dir.path(), + ); + assert!( + output.status.success(), + "find --exec failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains(&format!("EXEC:{}", path_a)), + "Expected command output for first match, got: {}", + stdout + ); + assert!( + stdout.contains(&format!("EXEC:{}", path_b)), + "Expected command output for second match, got: {}", + stdout + ); + + cleanup_bucket(config_dir.path(), &bucket_name); + } + + #[test] + fn test_find_exec_rejects_json_output() { + let (config_dir, bucket_name) = match setup_with_alias("findexecjson") { + Some(v) => v, + None => { + eprintln!("Skipping: S3 test config not available"); + return; + } + }; + + upload_text_object(config_dir.path(), &bucket_name, "x.txt", "x"); + + let output = run_rc( + &[ + "find", + &format!("test/{}/", bucket_name), + "--name", + "*.txt", + "--exec", + "echo EXEC:{}", + "--json", + ], + config_dir.path(), + ); + assert!( + !output.status.success(), + "find --exec --json should fail with usage error" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("--exec cannot be used with --json output"), + "Unexpected error output: {}", + stderr + ); + + cleanup_bucket(config_dir.path(), &bucket_name); + } } From 64088428d7d4864482e83a6e8b620b5b4a9af947 Mon Sep 17 00:00:00 2001 From: overtrue Date: Mon, 9 Mar 2026 04:30:51 +0800 Subject: [PATCH 2/4] feat(phase-2): complete option behavior coverage and mirror parallel runtime --- .github/workflows/integration.yml | 2 + crates/cli/src/commands/mirror.rs | 103 +++++++-- crates/cli/tests/integration.rs | 350 ++++++++++++++++++++++++++++++ 3 files changed, 433 insertions(+), 22 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index f9c1915..05a5988 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -77,6 +77,8 @@ jobs: "option_behavior_operations::test_cp_dry_run_does_not_create_target_object" "option_behavior_operations::test_head_bytes_returns_prefix_bytes" "option_behavior_operations::test_find_print_outputs_full_remote_path" + "option_behavior_operations::test_find_exec_rejects_json_output" + "option_behavior_operations::test_mirror_parallel_zero_returns_usage_error" ) for test_name in "${TESTS[@]}"; do diff --git a/crates/cli/src/commands/mirror.rs b/crates/cli/src/commands/mirror.rs index 3cb171a..a8312d7 100644 --- a/crates/cli/src/commands/mirror.rs +++ b/crates/cli/src/commands/mirror.rs @@ -9,6 +9,8 @@ use rc_s3::S3Client; use serde::Serialize; use std::collections::HashMap; use std::sync::Arc; +use tokio::sync::Semaphore; +use tokio::task::JoinSet; use crate::commands::diff::{DiffEntry, DiffStatus}; use crate::exit_code::ExitCode; @@ -66,6 +68,11 @@ struct FileInfo { pub async fn execute(args: MirrorArgs, output_config: OutputConfig) -> ExitCode { let formatter = Formatter::new(output_config); + if args.parallel == 0 { + formatter.error("--parallel must be greater than 0"); + return ExitCode::UsageError; + } + // Parse both paths let source_parsed = parse_path(&args.source); let target_parsed = parse_path(&args.target); @@ -253,7 +260,11 @@ pub async fn execute(args: MirrorArgs, output_config: OutputConfig) -> ExitCode let mut copied = 0; let mut errors = 0; - for (key, _) in &to_copy { + let parallel_limit = args.parallel.max(1); + let copy_semaphore = Arc::new(Semaphore::new(parallel_limit)); + let mut copy_tasks: JoinSet<(String, Result<(), String>)> = JoinSet::new(); + + for (key, _) in to_copy { let source_sep = if source_path.key.is_empty() || source_path.key.ends_with('/') { "" } else { @@ -276,26 +287,46 @@ pub async fn execute(args: MirrorArgs, output_config: OutputConfig) -> ExitCode format!("{}{target_sep}{key}", target_path.key), ); - // Get object content and upload to target - match source_client.get_object(&source_full).await { - Ok(data) => match target_client.put_object(&target_full, data, None).await { - Ok(_) => { - copied += 1; - if !args.quiet && !formatter.is_json() { - formatter.println(&format!("+ {key}")); - } + let key = key.to_string(); + let source_client = Arc::clone(&source_client); + let target_client = Arc::clone(&target_client); + let permit = copy_semaphore + .clone() + .acquire_owned() + .await + .expect("semaphore should not be closed"); + copy_tasks.spawn(async move { + let _permit = permit; + let result = match source_client.get_object(&source_full).await { + Ok(data) => target_client + .put_object(&target_full, data, None) + .await + .map(|_| ()) + .map_err(|e| format!("Failed to upload {key}: {e}")), + Err(e) => Err(format!("Failed to download {key}: {e}")), + }; + (key, result) + }); + } + + while let Some(task_result) = copy_tasks.join_next().await { + match task_result { + Ok((key, Ok(()))) => { + copied += 1; + if !args.quiet && !formatter.is_json() { + formatter.println(&format!("+ {key}")); } - Err(e) => { - errors += 1; - if !formatter.is_json() { - formatter.error(&format!("Failed to upload {key}: {e}")); - } + } + Ok((_, Err(message))) => { + errors += 1; + if !formatter.is_json() { + formatter.error(&message); } - }, - Err(e) => { + } + Err(join_error) => { errors += 1; if !formatter.is_json() { - formatter.error(&format!("Failed to download {key}: {e}")); + formatter.error(&format!("Mirror copy worker failed: {join_error}")); } } } @@ -309,7 +340,10 @@ pub async fn execute(args: MirrorArgs, output_config: OutputConfig) -> ExitCode let mut removed = 0; if args.remove { - for key in &to_remove { + let remove_semaphore = Arc::new(Semaphore::new(parallel_limit)); + let mut remove_tasks: JoinSet<(String, Result<(), String>)> = JoinSet::new(); + + for key in to_remove { let sep = if target_path.key.is_empty() || target_path.key.ends_with('/') { "" } else { @@ -321,17 +355,42 @@ pub async fn execute(args: MirrorArgs, output_config: OutputConfig) -> ExitCode format!("{}{sep}{key}", target_path.key), ); - match target_client.delete_object(&target_full).await { - Ok(_) => { + let key = key.to_string(); + let target_client = Arc::clone(&target_client); + let permit = remove_semaphore + .clone() + .acquire_owned() + .await + .expect("semaphore should not be closed"); + remove_tasks.spawn(async move { + let _permit = permit; + let result = target_client + .delete_object(&target_full) + .await + .map(|_| ()) + .map_err(|e| format!("Failed to remove {key}: {e}")); + (key, result) + }); + } + + while let Some(task_result) = remove_tasks.join_next().await { + match task_result { + Ok((key, Ok(()))) => { removed += 1; if !args.quiet && !formatter.is_json() { formatter.println(&format!("- {key}")); } } - Err(e) => { + Ok((_, Err(message))) => { + errors += 1; + if !formatter.is_json() { + formatter.error(&message); + } + } + Err(join_error) => { errors += 1; if !formatter.is_json() { - formatter.error(&format!("Failed to remove {key}: {e}")); + formatter.error(&format!("Mirror remove worker failed: {join_error}")); } } } diff --git a/crates/cli/tests/integration.rs b/crates/cli/tests/integration.rs index 334db25..9cab0e3 100644 --- a/crates/cli/tests/integration.rs +++ b/crates/cli/tests/integration.rs @@ -2834,6 +2834,31 @@ mod option_behavior_operations { cleanup_bucket(config_dir.path(), &bucket_name); } + #[test] + fn test_share_rejects_expiration_over_seven_days() { + let config_dir = tempfile::tempdir().expect("Failed to create temp dir"); + let output = run_rc( + &[ + "share", + "test/demo-bucket/demo.txt", + "--expire", + "8d", + "--json", + ], + config_dir.path(), + ); + assert!( + !output.status.success(), + "share with expiration > 7 days should fail" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("Expiration cannot exceed 7 days"), + "Unexpected error output: {}", + stderr + ); + } + #[test] fn test_find_print_outputs_full_remote_path() { let (config_dir, bucket_name) = match setup_with_alias("findprint") { @@ -2964,4 +2989,329 @@ mod option_behavior_operations { cleanup_bucket(config_dir.path(), &bucket_name); } + + #[test] + fn test_find_maxdepth_excludes_deeper_matches() { + let (config_dir, bucket_name) = match setup_with_alias("finddepth") { + Some(v) => v, + None => { + eprintln!("Skipping: S3 test config not available"); + return; + } + }; + + upload_text_object(config_dir.path(), &bucket_name, "top.txt", "top"); + upload_text_object(config_dir.path(), &bucket_name, "one/file.txt", "one"); + upload_text_object(config_dir.path(), &bucket_name, "one/two/deep.txt", "deep"); + + let output = run_rc( + &[ + "find", + &format!("test/{}/", bucket_name), + "--name", + "*.txt", + "--maxdepth", + "1", + "--json", + ], + config_dir.path(), + ); + assert!( + output.status.success(), + "find --maxdepth failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("top.txt"), "top.txt should be matched"); + assert!( + stdout.contains("one/file.txt"), + "one/file.txt should be matched" + ); + assert!( + !stdout.contains("one/two/deep.txt"), + "one/two/deep.txt should be excluded by maxdepth=1" + ); + + cleanup_bucket(config_dir.path(), &bucket_name); + } + + #[test] + fn test_mirror_remove_with_parallel_synchronizes_destination() { + let (config_dir, bucket_name) = match setup_with_alias("mirroropt") { + Some(v) => v, + None => { + eprintln!("Skipping: S3 test config not available"); + return; + } + }; + + let target_bucket = format!("{}-dest", bucket_name); + let output = run_rc( + &["mb", &format!("test/{}", target_bucket)], + config_dir.path(), + ); + assert!( + output.status.success(), + "Failed to create destination bucket: {}", + String::from_utf8_lossy(&output.stderr) + ); + + upload_text_object(config_dir.path(), &bucket_name, "src/keep-1.txt", "keep-1"); + upload_text_object( + config_dir.path(), + &bucket_name, + "src/nested/keep-2.txt", + "keep-2", + ); + upload_text_object( + config_dir.path(), + &target_bucket, + "mirror-stale.txt", + "should be removed", + ); + + let output = run_rc( + &[ + "mirror", + &format!("test/{}/src/", bucket_name), + &format!("test/{}/", target_bucket), + "--remove", + "--overwrite", + "--parallel", + "2", + "--json", + ], + config_dir.path(), + ); + assert!( + output.status.success(), + "mirror --remove --parallel failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + let json: serde_json::Value = serde_json::from_str(&stdout).expect("Invalid JSON output"); + assert_eq!(json["errors"], 0); + assert!(json["copied"].as_u64().unwrap_or(0) >= 2); + assert!(json["removed"].as_u64().unwrap_or(0) >= 1); + + let output = run_rc( + &[ + "ls", + "--recursive", + &format!("test/{}/", target_bucket), + "--json", + ], + config_dir.path(), + ); + assert!( + output.status.success(), + "Failed to list destination after mirror: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("keep-1.txt"), + "Expected keep-1.txt in destination" + ); + assert!( + stdout.contains("nested/keep-2.txt") || stdout.contains("keep-2.txt"), + "Expected keep-2.txt in destination" + ); + assert!( + !stdout.contains("mirror-stale.txt"), + "Stale destination file should be removed" + ); + + cleanup_bucket(config_dir.path(), &bucket_name); + cleanup_bucket(config_dir.path(), &target_bucket); + } + + #[test] + fn test_mirror_parallel_zero_returns_usage_error() { + let (config_dir, bucket_name) = match setup_with_alias("mirrorparallelzero") { + Some(v) => v, + None => { + eprintln!("Skipping: S3 test config not available"); + return; + } + }; + + let target_bucket = format!("{}-dest", bucket_name); + let output = run_rc( + &["mb", &format!("test/{}", target_bucket)], + config_dir.path(), + ); + assert!( + output.status.success(), + "Failed to create destination bucket" + ); + + let output = run_rc( + &[ + "mirror", + &format!("test/{}/", bucket_name), + &format!("test/{}/", target_bucket), + "--parallel", + "0", + "--json", + ], + config_dir.path(), + ); + assert!( + !output.status.success(), + "mirror --parallel 0 should fail with usage error" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("--parallel must be greater than 0"), + "Unexpected error output: {}", + stderr + ); + + cleanup_bucket(config_dir.path(), &bucket_name); + cleanup_bucket(config_dir.path(), &target_bucket); + } + + #[test] + fn test_tree_option_combination_filters_expected_nodes() { + let (config_dir, bucket_name) = match setup_with_alias("treeopts") { + Some(v) => v, + None => { + eprintln!("Skipping: S3 test config not available"); + return; + } + }; + + upload_text_object(config_dir.path(), &bucket_name, "dir/a.txt", "A"); + upload_text_object(config_dir.path(), &bucket_name, "dir/b.log", "B"); + upload_text_object(config_dir.path(), &bucket_name, "dir/deep/c.txt", "C"); + + let output = run_rc( + &[ + "tree", + &format!("test/{}/", bucket_name), + "--pattern", + "*.txt", + "--level", + "2", + "--full-path", + "--json", + ], + config_dir.path(), + ); + assert!( + output.status.success(), + "tree option combination failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let stdout = String::from_utf8_lossy(&output.stdout); + let json: serde_json::Value = serde_json::from_str(&stdout).expect("Invalid JSON output"); + assert_eq!(json["name"], format!("test/{}/", bucket_name)); + assert!(stdout.contains("a.txt"), "Expected a.txt to match pattern"); + assert!( + !stdout.contains("b.log"), + "b.log should be filtered by pattern" + ); + assert!( + !stdout.contains("deep/c.txt"), + "deep/c.txt should be excluded by level=2" + ); + + // dirs-only should remove all files from output tree + let output = run_rc( + &[ + "tree", + &format!("test/{}/", bucket_name), + "--dirs-only", + "--json", + ], + config_dir.path(), + ); + assert!( + output.status.success(), + "tree --dirs-only failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(!stdout.contains("a.txt"), "dirs-only should exclude files"); + assert!(!stdout.contains("b.log"), "dirs-only should exclude files"); + + cleanup_bucket(config_dir.path(), &bucket_name); + } + + #[test] + fn test_diff_diff_only_excludes_same_entries() { + let (config_dir, bucket_name) = match setup_with_alias("diffonly") { + Some(v) => v, + None => { + eprintln!("Skipping: S3 test config not available"); + return; + } + }; + + let second_bucket = format!("{}-second", bucket_name); + let output = run_rc( + &["mb", &format!("test/{}", second_bucket)], + config_dir.path(), + ); + assert!(output.status.success(), "Failed to create second bucket"); + + upload_text_object(config_dir.path(), &bucket_name, "same.txt", "same-content"); + upload_text_object( + config_dir.path(), + &second_bucket, + "same.txt", + "same-content", + ); + upload_text_object(config_dir.path(), &bucket_name, "only-first.txt", "first"); + + let base_args = [ + "diff", + &format!("test/{}/", bucket_name), + &format!("test/{}/", second_bucket), + ]; + + let output = run_rc( + &[base_args[0], base_args[1], base_args[2], "--json"], + config_dir.path(), + ); + assert!( + !output.status.success(), + "diff with differences should return non-zero" + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("same.txt"), + "baseline diff should include same entries" + ); + + let output = run_rc( + &[ + "diff", + &format!("test/{}/", bucket_name), + &format!("test/{}/", second_bucket), + "--diff-only", + "--json", + ], + config_dir.path(), + ); + assert!( + !output.status.success(), + "diff --diff-only with differences should return non-zero" + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + !stdout.contains("\"status\":\"same\""), + "diff-only output must exclude same entries" + ); + assert!( + stdout.contains("only-first.txt"), + "diff-only output should include real differences" + ); + + cleanup_bucket(config_dir.path(), &bucket_name); + cleanup_bucket(config_dir.path(), &second_bucket); + } } From 2d89c78c2efcd75d395141da1c1fbb918afb4d8b Mon Sep 17 00:00:00 2001 From: overtrue Date: Mon, 9 Mar 2026 06:37:17 +0800 Subject: [PATCH 3/4] feat(phase-4): normalize help usage for windows binary name --- crates/cli/tests/help_contract.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/cli/tests/help_contract.rs b/crates/cli/tests/help_contract.rs index 61ee0dd..8d4f83b 100644 --- a/crates/cli/tests/help_contract.rs +++ b/crates/cli/tests/help_contract.rs @@ -69,8 +69,9 @@ fn assert_help_case(case: &HelpCase) { ); let stdout = String::from_utf8_lossy(&output.stdout); + let normalized_stdout = stdout.replace("Usage: rc.exe ", "Usage: rc "); assert!( - stdout.contains(case.usage), + normalized_stdout.contains(case.usage), "usage marker `{}` missing for {command_label}\nstdout:\n{}", case.usage, stdout From d00f741b799589abcffb4061ae17e6414cd0c9f2 Mon Sep 17 00:00:00 2001 From: overtrue Date: Mon, 9 Mar 2026 06:49:53 +0800 Subject: [PATCH 4/4] feat(phase-4): harden find --exec execution path --- Cargo.lock | 1 + Cargo.toml | 1 + crates/cli/Cargo.toml | 2 +- crates/cli/src/commands/find.rs | 123 ++++++++++++++++++++++++-------- 4 files changed, 96 insertions(+), 31 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7d31856..8b71443 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2327,6 +2327,7 @@ dependencies = [ "rc-s3", "serde", "serde_json", + "shlex", "tempfile", "thiserror", "tokio", diff --git a/Cargo.toml b/Cargo.toml index 2ad94a4..6a41d83 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -61,6 +61,7 @@ futures = "0.3" async-trait = "0.1" mime_guess = "2.0" glob = "0.3" +shlex = "1.3" # HTTP client for Admin API reqwest = { version = "0.12", default-features = false, features = ["rustls-tls-native-roots", "rustls-tls-webpki-roots", "json"] } diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index 159be52..fa59300 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -49,6 +49,7 @@ jiff.workspace = true humansize.workspace = true mime_guess.workspace = true glob.workspace = true +shlex.workspace = true [features] default = [] @@ -60,4 +61,3 @@ golden = [] [dev-dependencies] tempfile.workspace = true insta.workspace = true - diff --git a/crates/cli/src/commands/find.rs b/crates/cli/src/commands/find.rs index dbf2c93..bc31cc1 100644 --- a/crates/cli/src/commands/find.rs +++ b/crates/cli/src/commands/find.rs @@ -7,7 +7,7 @@ use rc_core::{AliasManager, ListOptions, ObjectStore as _, RemotePath}; use rc_s3::S3Client; use serde::Serialize; use std::io::Write as _; -use std::process::Command; +use std::process::{Command, Output}; use crate::exit_code::ExitCode; use crate::output::{Formatter, OutputConfig}; @@ -140,13 +140,25 @@ pub async fn execute(args: FindArgs, output_config: OutputConfig) -> ExitCode { return ExitCode::UsageError; } + let exec_argv_template = match parse_exec_template(exec_template) { + Ok(template) => template, + Err(e) => { + formatter.error(&e); + return ExitCode::UsageError; + } + }; + for m in &matches { let object_path = full_object_path(&alias_name, &bucket, &m.key); - let command_text = exec_template.replace("{}", &object_path); - let output = match run_shell_command(&command_text) { + let (program, exec_args, command_text) = + render_exec_command(&exec_argv_template, &object_path); + let output = match run_exec_command(&program, &exec_args) { Ok(output) => output, Err(e) => { - formatter.error(&format!("Failed to run command for {}: {}", object_path, e)); + formatter.error(&format!( + "Failed to run command for {}: {} ({})", + object_path, command_text, e + )); return ExitCode::GeneralError; } }; @@ -162,31 +174,24 @@ pub async fn execute(args: FindArgs, output_config: OutputConfig) -> ExitCode { if !output.status.success() { formatter.error(&format!( - "Command failed for {}: {}", - object_path, command_text + "Command failed for {} (status {}): {}", + object_path, output.status, command_text )); return ExitCode::GeneralError; } } } - let display_matches: Vec = matches - .iter() - .map(|m| MatchInfo { - key: if args.print { - full_object_path(&alias_name, &bucket, &m.key) - } else { - m.key.clone() - }, - size_bytes: m.size_bytes, - size_human: m.size_human.clone(), - last_modified: m.last_modified.clone(), - }) - .collect(); + let mut display_matches = matches; + if args.print { + for m in &mut display_matches { + m.key = full_object_path(&alias_name, &bucket, &m.key); + } + } // Calculate totals - let total_count = matches.len(); - let total_size: i64 = matches.iter().filter_map(|m| m.size_bytes).sum(); + let total_count = display_matches.len(); + let total_size: i64 = display_matches.iter().filter_map(|m| m.size_bytes).sum(); if args.count { // Only print count @@ -234,18 +239,35 @@ pub async fn execute(args: FindArgs, output_config: OutputConfig) -> ExitCode { } fn full_object_path(alias: &str, bucket: &str, key: &str) -> String { - format!("{alias}/{bucket}/{key}") + RemotePath::new(alias, bucket, key).to_full_path() } -fn run_shell_command(command: &str) -> std::io::Result { - #[cfg(target_family = "windows")] - { - Command::new("cmd").args(["/C", command]).output() - } - #[cfg(not(target_family = "windows"))] - { - Command::new("sh").args(["-c", command]).output() +fn parse_exec_template(exec_template: &str) -> Result, String> { + let args = shlex::split(exec_template) + .ok_or_else(|| "Invalid --exec template: unbalanced quotes".to_string())?; + if args.is_empty() { + return Err("Invalid --exec template: command cannot be empty".to_string()); } + + Ok(args) +} + +fn render_exec_command( + argv_template: &[String], + object_path: &str, +) -> (String, Vec, String) { + let rendered: Vec = argv_template + .iter() + .map(|arg| arg.replace("{}", object_path)) + .collect(); + let program = rendered[0].clone(); + let args = rendered[1..].to_vec(); + let command_text = rendered.join(" "); + (program, args, command_text) +} + +fn run_exec_command(program: &str, args: &[String]) -> std::io::Result { + Command::new(program).args(args).output() } /// Filters for find command @@ -507,5 +529,46 @@ mod tests { full_object_path("test", "bucket", "a/b.txt"), "test/bucket/a/b.txt" ); + assert_eq!(full_object_path("test", "bucket", ""), "test/bucket"); + } + + #[test] + fn test_parse_exec_template() { + assert_eq!( + parse_exec_template("echo EXEC:{}").unwrap(), + vec!["echo".to_string(), "EXEC:{}".to_string()] + ); + assert_eq!( + parse_exec_template(r#"printf '%s\n' "{}""#).unwrap(), + vec!["printf".to_string(), "%s\\n".to_string(), "{}".to_string()] + ); + } + + #[test] + fn test_parse_exec_template_errors() { + assert!(parse_exec_template("").is_err()); + assert!(parse_exec_template("'unterminated").is_err()); + } + + #[test] + fn test_render_exec_command() { + let template = vec![ + "echo".to_string(), + "prefix:{}".to_string(), + "{}".to_string(), + ]; + let (program, args, text) = render_exec_command(&template, "test/bucket/a.txt"); + assert_eq!(program, "echo"); + assert_eq!( + args, + vec![ + "prefix:test/bucket/a.txt".to_string(), + "test/bucket/a.txt".to_string() + ] + ); + assert_eq!( + text, + "echo prefix:test/bucket/a.txt test/bucket/a.txt".to_string() + ); } }