diff --git a/crates/openshell-cli/src/ssh.rs b/crates/openshell-cli/src/ssh.rs index 4b284bff..89d63653 100644 --- a/crates/openshell-cli/src/ssh.rs +++ b/crates/openshell-cli/src/ssh.rs @@ -447,22 +447,31 @@ pub(crate) async fn sandbox_exec_without_exec( sandbox_exec_with_mode(server, name, command, tty, tls, false).await } -/// Push a list of files from a local directory into a sandbox using tar-over-SSH. -/// -/// This replaces the old rsync-based sync. Files are streamed as a tar archive -/// to `ssh ... tar xf - -C ` on the sandbox side. -pub async fn sandbox_sync_up_files( +/// What to pack into the tar archive streamed to the sandbox. +enum UploadSource { + /// A single local file or directory. `tar_name` controls the entry name + /// inside the archive (e.g. the target basename for file-to-file uploads). + SinglePath { + local_path: PathBuf, + tar_name: std::ffi::OsString, + }, + /// A set of files relative to a base directory (git-filtered uploads). + FileList { + base_dir: PathBuf, + files: Vec, + }, +} + +/// Core tar-over-SSH upload: streams a tar archive into `dest_dir` on the +/// sandbox. Callers are responsible for splitting the destination path so +/// that `dest_dir` is always a directory. +async fn ssh_tar_upload( server: &str, name: &str, - base_dir: &Path, - files: &[String], - dest: &str, + dest_dir: &str, + source: UploadSource, tls: &TlsOptions, ) -> Result<()> { - if files.is_empty() { - return Ok(()); - } - let session = ssh_session_config(server, name, tls).await?; let mut ssh = ssh_base_command(&session.proxy_command); @@ -472,8 +481,8 @@ pub async fn sandbox_sync_up_files( .arg("sandbox") .arg(format!( "mkdir -p {} && cat | tar xf - -C {}", - shell_escape(dest), - shell_escape(dest) + shell_escape(dest_dir), + shell_escape(dest_dir) )) .stdin(Stdio::piped()) .stdout(Stdio::inherit()) @@ -486,22 +495,43 @@ pub async fn sandbox_sync_up_files( .ok_or_else(|| miette::miette!("failed to open stdin for ssh process"))?; // Build the tar archive in a blocking task since the tar crate is synchronous. - let base_dir = base_dir.to_path_buf(); - let files = files.to_vec(); tokio::task::spawn_blocking(move || -> Result<()> { let mut archive = tar::Builder::new(stdin); - for file in &files { - let full_path = base_dir.join(file); - if full_path.is_file() { - archive - .append_path_with_name(&full_path, file) - .into_diagnostic() - .wrap_err_with(|| format!("failed to add {file} to tar archive"))?; - } else if full_path.is_dir() { - archive - .append_dir_all(file, &full_path) - .into_diagnostic() - .wrap_err_with(|| format!("failed to add directory {file} to tar archive"))?; + match source { + UploadSource::SinglePath { + local_path, + tar_name, + } => { + if local_path.is_file() { + archive + .append_path_with_name(&local_path, &tar_name) + .into_diagnostic()?; + } else if local_path.is_dir() { + archive.append_dir_all(".", &local_path).into_diagnostic()?; + } else { + return Err(miette::miette!( + "local path does not exist: {}", + local_path.display() + )); + } + } + UploadSource::FileList { base_dir, files } => { + for file in &files { + let full_path = base_dir.join(file); + if full_path.is_file() { + archive + .append_path_with_name(&full_path, file) + .into_diagnostic() + .wrap_err_with(|| format!("failed to add {file} to tar archive"))?; + } else if full_path.is_dir() { + archive + .append_dir_all(file, &full_path) + .into_diagnostic() + .wrap_err_with(|| { + format!("failed to add directory {file} to tar archive") + })?; + } + } } } archive.finish().into_diagnostic()?; @@ -524,7 +554,55 @@ pub async fn sandbox_sync_up_files( Ok(()) } +/// Split a sandbox path into (parent_directory, basename). +/// +/// Examples: +/// `"/sandbox/.bashrc"` -> `("/sandbox", ".bashrc")` +/// `"/sandbox/sub/file"` -> `("/sandbox/sub", "file")` +/// `"file.txt"` -> `(".", "file.txt")` +fn split_sandbox_path(path: &str) -> (&str, &str) { + match path.rfind('/') { + Some(0) => ("/", &path[1..]), + Some(pos) => (&path[..pos], &path[pos + 1..]), + None => (".", path), + } +} + +/// Push a list of files from a local directory into a sandbox using tar-over-SSH. +/// +/// Files are streamed as a tar archive to `ssh ... tar xf - -C ` on +/// the sandbox side. +pub async fn sandbox_sync_up_files( + server: &str, + name: &str, + base_dir: &Path, + files: &[String], + dest: &str, + tls: &TlsOptions, +) -> Result<()> { + if files.is_empty() { + return Ok(()); + } + ssh_tar_upload( + server, + name, + dest, + UploadSource::FileList { + base_dir: base_dir.to_path_buf(), + files: files.to_vec(), + }, + tls, + ) + .await +} + /// Push a local path (file or directory) into a sandbox using tar-over-SSH. +/// +/// When uploading a single file to a destination that does not end with `/`, +/// the destination is treated as a file path: the parent directory is created +/// and the file is written with the destination's basename. This matches +/// `cp` / `scp` semantics and avoids creating a directory at the destination +/// path. pub async fn sandbox_sync_up( server: &str, name: &str, @@ -532,64 +610,47 @@ pub async fn sandbox_sync_up( sandbox_path: &str, tls: &TlsOptions, ) -> Result<()> { - let session = ssh_session_config(server, name, tls).await?; - - let mut ssh = ssh_base_command(&session.proxy_command); - ssh.arg("-T") - .arg("-o") - .arg("RequestTTY=no") - .arg("sandbox") - .arg(format!( - "mkdir -p {} && cat | tar xf - -C {}", - shell_escape(sandbox_path), - shell_escape(sandbox_path) - )) - .stdin(Stdio::piped()) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()); + // When uploading a single file to a path that looks like a file (does not + // end with '/'), split into parent directory + target basename so that + // `mkdir -p` creates the parent and tar extracts the file with the right + // name. For directories or paths ending in '/', keep the original + // behavior where sandbox_path is the extraction directory. + if local_path.is_file() && !sandbox_path.ends_with('/') { + let (parent, target_name) = split_sandbox_path(sandbox_path); + return ssh_tar_upload( + server, + name, + parent, + UploadSource::SinglePath { + local_path: local_path.to_path_buf(), + tar_name: target_name.into(), + }, + tls, + ) + .await; + } - let mut child = ssh.spawn().into_diagnostic()?; - let stdin = child - .stdin - .take() - .ok_or_else(|| miette::miette!("failed to open stdin for ssh process"))?; + let tar_name = if local_path.is_file() { + local_path + .file_name() + .ok_or_else(|| miette::miette!("path has no file name"))? + .to_os_string() + } else { + // For directories the tar_name is unused — append_dir_all uses "." + ".".into() + }; - let local_path = local_path.to_path_buf(); - tokio::task::spawn_blocking(move || -> Result<()> { - let mut archive = tar::Builder::new(stdin); - if local_path.is_file() { - let file_name = local_path - .file_name() - .ok_or_else(|| miette::miette!("path has no file name"))?; - archive - .append_path_with_name(&local_path, file_name) - .into_diagnostic()?; - } else if local_path.is_dir() { - archive.append_dir_all(".", &local_path).into_diagnostic()?; - } else { - return Err(miette::miette!( - "local path does not exist: {}", - local_path.display() - )); - } - archive.finish().into_diagnostic()?; - Ok(()) - }) + ssh_tar_upload( + server, + name, + sandbox_path, + UploadSource::SinglePath { + local_path: local_path.to_path_buf(), + tar_name, + }, + tls, + ) .await - .into_diagnostic()??; - - let status = tokio::task::spawn_blocking(move || child.wait()) - .await - .into_diagnostic()? - .into_diagnostic()?; - - if !status.success() { - return Err(miette::miette!( - "ssh tar extract exited with status {status}" - )); - } - - Ok(()) } /// Pull a path from a sandbox to a local destination using tar-over-SSH. @@ -1149,4 +1210,25 @@ mod tests { assert!(message.contains("Forwarding port 3000 to sandbox demo")); assert!(message.contains("Access at: http://localhost:3000/")); } + + #[test] + fn split_sandbox_path_separates_parent_and_basename() { + assert_eq!( + split_sandbox_path("/sandbox/.bashrc"), + ("/sandbox", ".bashrc") + ); + assert_eq!( + split_sandbox_path("/sandbox/sub/file"), + ("/sandbox/sub", "file") + ); + assert_eq!(split_sandbox_path("/a/b/c/d.txt"), ("/a/b/c", "d.txt")); + } + + #[test] + fn split_sandbox_path_handles_root_and_bare_names() { + // File directly under root + assert_eq!(split_sandbox_path("/.bashrc"), ("/", ".bashrc")); + // No directory component at all + assert_eq!(split_sandbox_path("file.txt"), (".", "file.txt")); + } }