diff --git a/codex-rs/app-server/src/request_processors/fs_processor.rs b/codex-rs/app-server/src/request_processors/fs_processor.rs index e7912432d5b1..c0d93bb158d8 100644 --- a/codex-rs/app-server/src/request_processors/fs_processor.rs +++ b/codex-rs/app-server/src/request_processors/fs_processor.rs @@ -175,10 +175,13 @@ impl FsRequestProcessor { &self, params: FsCopyParams, ) -> Result { + let source_path = PathUri::from_abs_path(¶ms.source_path).map_err(map_fs_error)?; + let destination_path = + PathUri::from_abs_path(¶ms.destination_path).map_err(map_fs_error)?; self.file_system()? .copy( - ¶ms.source_path, - ¶ms.destination_path, + &source_path, + &destination_path, CopyOptions { recursive: params.recursive, }, diff --git a/codex-rs/config/src/loader/tests.rs b/codex-rs/config/src/loader/tests.rs index 8846122ab275..e349ba0c6856 100644 --- a/codex-rs/config/src/loader/tests.rs +++ b/codex-rs/config/src/loader/tests.rs @@ -79,8 +79,8 @@ impl ExecutorFileSystem for TestFileSystem { async fn copy( &self, - _source_path: &AbsolutePathBuf, - _destination_path: &AbsolutePathBuf, + _source_path: &PathUri, + _destination_path: &PathUri, _copy_options: CopyOptions, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index dbf20de26439..64d5c7081c14 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -1109,8 +1109,8 @@ async fn remote_test_env_copy_preserves_symlink_source() -> Result<()> { let sandbox = workspace_write_sandbox(allowed_dir.clone()); file_system .copy( - &absolute_path(source_symlink), - &absolute_path(copied_symlink.clone()), + &PathUri::from_path(&source_symlink)?, + &PathUri::from_path(&copied_symlink)?, CopyOptions { recursive: false }, Some(&sandbox), ) diff --git a/codex-rs/exec-server/src/fs_helper.rs b/codex-rs/exec-server/src/fs_helper.rs index 1f7b4dbb9070..526e682976ba 100644 --- a/codex-rs/exec-server/src/fs_helper.rs +++ b/codex-rs/exec-server/src/fs_helper.rs @@ -292,10 +292,15 @@ pub(crate) async fn run_direct_request( Ok(FsHelperPayload::Remove(FsRemoveResponse {})) } FsHelperRequest::Copy(params) => { + let source_path = codex_utils_path_uri::PathUri::from_abs_path(¶ms.source_path) + .map_err(map_fs_error)?; + let destination_path = + codex_utils_path_uri::PathUri::from_abs_path(¶ms.destination_path) + .map_err(map_fs_error)?; file_system .copy( - ¶ms.source_path, - ¶ms.destination_path, + &source_path, + &destination_path, CopyOptions { recursive: params.recursive, }, diff --git a/codex-rs/exec-server/src/local_file_system.rs b/codex-rs/exec-server/src/local_file_system.rs index 26469a79e1b9..ee38d7ff9461 100644 --- a/codex-rs/exec-server/src/local_file_system.rs +++ b/codex-rs/exec-server/src/local_file_system.rs @@ -148,8 +148,8 @@ impl ExecutorFileSystem for LocalFileSystem { async fn copy( &self, - source_path: &AbsolutePathBuf, - destination_path: &AbsolutePathBuf, + source_path: &PathUri, + destination_path: &PathUri, options: CopyOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -238,8 +238,8 @@ impl ExecutorFileSystem for UnsandboxedFileSystem { async fn copy( &self, - source_path: &AbsolutePathBuf, - destination_path: &AbsolutePathBuf, + source_path: &PathUri, + destination_path: &PathUri, options: CopyOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -382,14 +382,14 @@ impl ExecutorFileSystem for DirectFileSystem { async fn copy( &self, - source_path: &AbsolutePathBuf, - destination_path: &AbsolutePathBuf, + source_path: &PathUri, + destination_path: &PathUri, options: CopyOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { reject_sandbox_context(sandbox)?; - let source_path = source_path.to_path_buf(); - let destination_path = destination_path.to_path_buf(); + let source_path = source_path.to_abs_path()?.into_path_buf(); + let destination_path = destination_path.to_abs_path()?.into_path_buf(); tokio::task::spawn_blocking(move || -> FileSystemResult<()> { let metadata = std::fs::symlink_metadata(source_path.as_path())?; let file_type = metadata.file_type(); @@ -550,6 +550,10 @@ fn system_time_to_unix_ms(time: SystemTime) -> i64 { .unwrap_or(0) } +#[cfg(all(test, any(unix, windows)))] +#[path = "local_file_system_path_uri_tests.rs"] +mod path_uri_tests; + #[cfg(all(test, unix))] mod tests { use super::*; @@ -605,7 +609,3 @@ mod tests { Ok(()) } } - -#[cfg(all(test, any(unix, windows)))] -#[path = "local_file_system_path_uri_tests.rs"] -mod path_uri_tests; diff --git a/codex-rs/exec-server/src/remote_file_system.rs b/codex-rs/exec-server/src/remote_file_system.rs index eea2bbab85ae..9bcdcc5b0965 100644 --- a/codex-rs/exec-server/src/remote_file_system.rs +++ b/codex-rs/exec-server/src/remote_file_system.rs @@ -1,7 +1,6 @@ use async_trait::async_trait; use base64::Engine as _; use base64::engine::general_purpose::STANDARD; -use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use tokio::io; use tracing::trace; @@ -195,17 +194,19 @@ impl ExecutorFileSystem for RemoteFileSystem { async fn copy( &self, - source_path: &AbsolutePathBuf, - destination_path: &AbsolutePathBuf, + source_path: &PathUri, + destination_path: &PathUri, options: CopyOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { trace!("remote fs copy"); + let source_path = source_path.to_abs_path()?; + let destination_path = destination_path.to_abs_path()?; let client = self.client.get().await.map_err(map_remote_error)?; client .fs_copy(FsCopyParams { - source_path: source_path.clone(), - destination_path: destination_path.clone(), + source_path, + destination_path, recursive: options.recursive, sandbox: remote_sandbox_context(sandbox), }) @@ -239,6 +240,10 @@ fn map_remote_error(error: ExecServerError) -> io::Error { } } +#[cfg(all(test, any(unix, windows)))] +#[path = "remote_file_system_path_uri_tests.rs"] +mod path_uri_tests; + #[cfg(test)] mod tests { use codex_protocol::models::PermissionProfile; @@ -248,6 +253,7 @@ mod tests { use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; + use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use super::*; @@ -328,7 +334,3 @@ mod tests { AbsolutePathBuf::from_absolute_path(&path).expect("absolute path") } } - -#[cfg(all(test, any(unix, windows)))] -#[path = "remote_file_system_path_uri_tests.rs"] -mod path_uri_tests; diff --git a/codex-rs/exec-server/src/sandboxed_file_system.rs b/codex-rs/exec-server/src/sandboxed_file_system.rs index 087c59e6004f..14f26e60b948 100644 --- a/codex-rs/exec-server/src/sandboxed_file_system.rs +++ b/codex-rs/exec-server/src/sandboxed_file_system.rs @@ -2,7 +2,6 @@ use async_trait::async_trait; use base64::Engine as _; use base64::engine::general_purpose::STANDARD; use codex_app_server_protocol::JSONRPCErrorError; -use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use tokio::io; @@ -218,8 +217,8 @@ impl ExecutorFileSystem for SandboxedFileSystem { async fn copy( &self, - source_path: &AbsolutePathBuf, - destination_path: &AbsolutePathBuf, + source_path: &PathUri, + destination_path: &PathUri, options: CopyOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -227,8 +226,8 @@ impl ExecutorFileSystem for SandboxedFileSystem { self.run_sandboxed( sandbox, FsHelperRequest::Copy(FsCopyParams { - source_path: source_path.clone(), - destination_path: destination_path.clone(), + source_path: source_path.to_abs_path()?, + destination_path: destination_path.to_abs_path()?, recursive: options.recursive, sandbox: None, }), diff --git a/codex-rs/exec-server/src/server/file_system_handler.rs b/codex-rs/exec-server/src/server/file_system_handler.rs index 2dcaac78993b..97d028f89b7e 100644 --- a/codex-rs/exec-server/src/server/file_system_handler.rs +++ b/codex-rs/exec-server/src/server/file_system_handler.rs @@ -191,10 +191,13 @@ impl FileSystemHandler { &self, params: FsCopyParams, ) -> Result { + let source_path = PathUri::from_abs_path(¶ms.source_path).map_err(map_fs_error)?; + let destination_path = + PathUri::from_abs_path(¶ms.destination_path).map_err(map_fs_error)?; self.file_system .copy( - ¶ms.source_path, - ¶ms.destination_path, + &source_path, + &destination_path, CopyOptions { recursive: params.recursive, }, diff --git a/codex-rs/exec-server/tests/file_system/shared.rs b/codex-rs/exec-server/tests/file_system/shared.rs index 480d7cf64629..0bef4aa2885b 100644 --- a/codex-rs/exec-server/tests/file_system/shared.rs +++ b/codex-rs/exec-server/tests/file_system/shared.rs @@ -212,8 +212,8 @@ async fn file_system_copy_copies_file(implementation: FileSystemImplementation) file_system .copy( - &absolute_path(&source_file), - &absolute_path(&copied_file), + &PathUri::from_path(&source_file)?, + &PathUri::from_path(&copied_file)?, CopyOptions { recursive: false }, /*sandbox*/ None, ) @@ -243,8 +243,8 @@ async fn file_system_copy_copies_directory_recursively( file_system .copy( - &absolute_path(&source_dir), - &absolute_path(&copied_dir), + &PathUri::from_path(&source_dir)?, + &PathUri::from_path(&copied_dir)?, CopyOptions { recursive: true }, /*sandbox*/ None, ) @@ -373,8 +373,8 @@ async fn file_system_copy_rejects_directory_without_recursive( let error = file_system .copy( - &absolute_path(&source_dir), - &absolute_path(tmp.path().join("dest")), + &PathUri::from_path(&source_dir)?, + &PathUri::from_path(tmp.path().join("dest"))?, CopyOptions { recursive: false }, /*sandbox*/ None, ) @@ -543,8 +543,8 @@ async fn file_system_copy_rejects_copying_directory_into_descendant( let error = file_system .copy( - &absolute_path(&source_dir), - &absolute_path(source_dir.join("nested").join("copy")), + &PathUri::from_path(&source_dir)?, + &PathUri::from_path(source_dir.join("nested").join("copy"))?, CopyOptions { recursive: true }, /*sandbox*/ None, ) diff --git a/codex-rs/exec-server/tests/file_system/support.rs b/codex-rs/exec-server/tests/file_system/support.rs index 80d0cfc862fe..715faa9965c5 100644 --- a/codex-rs/exec-server/tests/file_system/support.rs +++ b/codex-rs/exec-server/tests/file_system/support.rs @@ -1,5 +1,4 @@ use std::fmt; -use std::path::Path; use std::sync::Arc; use anyhow::Result; @@ -71,8 +70,7 @@ pub(crate) async fn create_file_system_context( } } -pub(crate) fn absolute_path(path: impl AsRef) -> AbsolutePathBuf { - let path = path.as_ref().to_path_buf(); +pub(crate) fn absolute_path(path: std::path::PathBuf) -> AbsolutePathBuf { assert!( path.is_absolute(), "path must be absolute: {}", diff --git a/codex-rs/exec-server/tests/file_system_unix.rs b/codex-rs/exec-server/tests/file_system_unix.rs index 6ed8b01fdd00..3538f323662a 100644 --- a/codex-rs/exec-server/tests/file_system_unix.rs +++ b/codex-rs/exec-server/tests/file_system_unix.rs @@ -31,7 +31,6 @@ use test_case::test_case; use crate::common::exec_server::exec_server_with_env; use crate::support::FileSystemImplementation; -use crate::support::absolute_path; use crate::support::create_file_system_context; use crate::support::read_only_sandbox; use crate::support::workspace_write_sandbox; @@ -545,8 +544,8 @@ async fn file_system_copy_rejects_symlink_escape_destination( let sandbox = workspace_write_sandbox(allowed_dir.clone()); let error = match file_system .copy( - &absolute_path(allowed_dir.join("source.txt")), - &absolute_path(&requested_destination), + &PathUri::from_path(allowed_dir.join("source.txt"))?, + &PathUri::from_path(&requested_destination)?, CopyOptions { recursive: false }, Some(&sandbox), ) @@ -623,8 +622,8 @@ async fn file_system_copy_preserves_symlink_source( let sandbox = workspace_write_sandbox(allowed_dir.clone()); file_system .copy( - &absolute_path(&source_symlink), - &absolute_path(&copied_symlink), + &PathUri::from_path(&source_symlink)?, + &PathUri::from_path(&copied_symlink)?, CopyOptions { recursive: false }, Some(&sandbox), ) @@ -701,8 +700,8 @@ async fn file_system_copy_rejects_symlink_escape_source( let sandbox = workspace_write_sandbox(allowed_dir); let error = match file_system .copy( - &absolute_path(&requested_source), - &absolute_path(&requested_destination), + &PathUri::from_path(&requested_source)?, + &PathUri::from_path(&requested_destination)?, CopyOptions { recursive: false }, Some(&sandbox), ) @@ -735,8 +734,8 @@ async fn file_system_copy_preserves_symlinks_in_recursive_copy( file_system .copy( - &absolute_path(&source_dir), - &absolute_path(&copied_dir), + &PathUri::from_path(&source_dir)?, + &PathUri::from_path(&copied_dir)?, CopyOptions { recursive: true }, /*sandbox*/ None, ) @@ -781,8 +780,8 @@ async fn file_system_copy_ignores_unknown_special_files_in_recursive_copy( file_system .copy( - &absolute_path(&source_dir), - &absolute_path(&copied_dir), + &PathUri::from_path(&source_dir)?, + &PathUri::from_path(&copied_dir)?, CopyOptions { recursive: true }, /*sandbox*/ None, ) @@ -820,8 +819,8 @@ async fn file_system_copy_rejects_standalone_fifo_source( let error = file_system .copy( - &absolute_path(&fifo_path), - &absolute_path(tmp.path().join("copied")), + &PathUri::from_path(&fifo_path)?, + &PathUri::from_path(tmp.path().join("copied"))?, CopyOptions { recursive: false }, /*sandbox*/ None, ) diff --git a/codex-rs/ext/skills/tests/executor_file_system_authority.rs b/codex-rs/ext/skills/tests/executor_file_system_authority.rs index 61980475d06b..da7328fa962f 100644 --- a/codex-rs/ext/skills/tests/executor_file_system_authority.rs +++ b/codex-rs/ext/skills/tests/executor_file_system_authority.rs @@ -143,8 +143,8 @@ impl ExecutorFileSystem for SyntheticFileSystem { async fn copy( &self, - _source_path: &AbsolutePathBuf, - _destination_path: &AbsolutePathBuf, + _source_path: &PathUri, + _destination_path: &PathUri, _options: CopyOptions, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { diff --git a/codex-rs/file-system/src/lib.rs b/codex-rs/file-system/src/lib.rs index b56204d3b640..95537402af8b 100644 --- a/codex-rs/file-system/src/lib.rs +++ b/codex-rs/file-system/src/lib.rs @@ -192,8 +192,8 @@ pub trait ExecutorFileSystem: Send + Sync { async fn copy( &self, - source_path: &AbsolutePathBuf, - destination_path: &AbsolutePathBuf, + source_path: &PathUri, + destination_path: &PathUri, copy_options: CopyOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()>;