diff --git a/codex-rs/config/src/loader/tests.rs b/codex-rs/config/src/loader/tests.rs index 2c87e1381d1..9d4511a8a6a 100644 --- a/codex-rs/config/src/loader/tests.rs +++ b/codex-rs/config/src/loader/tests.rs @@ -8,12 +8,33 @@ use codex_file_system::FileSystemSandboxContext; use codex_file_system::ReadDirectoryEntry; use codex_file_system::RemoveOptions; use pretty_assertions::assert_eq; +use std::path::Path; use tempfile::tempdir; struct TestFileSystem; #[async_trait] impl ExecutorFileSystem for TestFileSystem { + async fn canonicalize( + &self, + path: &AbsolutePathBuf, + _sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult { + path.canonicalize() + } + + async fn join( + &self, + base_path: &AbsolutePathBuf, + path: &Path, + ) -> FileSystemResult { + Ok(base_path.join(path)) + } + + async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult> { + Ok(path.parent()) + } + async fn read_file( &self, path: &AbsolutePathBuf, diff --git a/codex-rs/exec-server/src/client.rs b/codex-rs/exec-server/src/client.rs index e06a736c174..b6c56e9f5e2 100644 --- a/codex-rs/exec-server/src/client.rs +++ b/codex-rs/exec-server/src/client.rs @@ -41,19 +41,28 @@ use crate::protocol::ExecExitedNotification; use crate::protocol::ExecOutputDeltaNotification; use crate::protocol::ExecParams; use crate::protocol::ExecResponse; +use crate::protocol::FS_CANONICALIZE_METHOD; use crate::protocol::FS_COPY_METHOD; use crate::protocol::FS_CREATE_DIRECTORY_METHOD; use crate::protocol::FS_GET_METADATA_METHOD; +use crate::protocol::FS_JOIN_METHOD; +use crate::protocol::FS_PARENT_METHOD; use crate::protocol::FS_READ_DIRECTORY_METHOD; use crate::protocol::FS_READ_FILE_METHOD; use crate::protocol::FS_REMOVE_METHOD; use crate::protocol::FS_WRITE_FILE_METHOD; +use crate::protocol::FsCanonicalizeParams; +use crate::protocol::FsCanonicalizeResponse; use crate::protocol::FsCopyParams; use crate::protocol::FsCopyResponse; use crate::protocol::FsCreateDirectoryParams; use crate::protocol::FsCreateDirectoryResponse; use crate::protocol::FsGetMetadataParams; use crate::protocol::FsGetMetadataResponse; +use crate::protocol::FsJoinParams; +use crate::protocol::FsJoinResponse; +use crate::protocol::FsParentParams; +use crate::protocol::FsParentResponse; use crate::protocol::FsReadDirectoryParams; use crate::protocol::FsReadDirectoryResponse; use crate::protocol::FsReadFileParams; @@ -414,6 +423,24 @@ impl ExecServerClient { self.call(FS_GET_METADATA_METHOD, ¶ms).await } + pub async fn fs_canonicalize( + &self, + params: FsCanonicalizeParams, + ) -> Result { + self.call(FS_CANONICALIZE_METHOD, ¶ms).await + } + + pub async fn fs_join(&self, params: FsJoinParams) -> Result { + self.call(FS_JOIN_METHOD, ¶ms).await + } + + pub async fn fs_parent( + &self, + params: FsParentParams, + ) -> Result { + self.call(FS_PARENT_METHOD, ¶ms).await + } + pub async fn fs_read_directory( &self, params: FsReadDirectoryParams, diff --git a/codex-rs/exec-server/src/environment_path.rs b/codex-rs/exec-server/src/environment_path.rs index 49e2cef91f7..4ee022a6ed0 100644 --- a/codex-rs/exec-server/src/environment_path.rs +++ b/codex-rs/exec-server/src/environment_path.rs @@ -2,6 +2,7 @@ use std::fmt; use std::hash::Hash; use std::hash::Hasher; use std::io; +use std::path::Path; use std::sync::Arc; use codex_utils_absolute_path::AbsolutePathBuf; @@ -60,6 +61,30 @@ impl EnvironmentPathRef { pub fn with_path(&self, path: AbsolutePathBuf) -> Self { Self::new(Arc::clone(&self.file_system), path) } + + pub async fn join>(&self, path: P) -> io::Result { + self.file_system + .join(&self.path, path.as_ref()) + .await + .map(|path| self.with_path(path)) + } + + pub async fn parent(&self) -> io::Result> { + self.file_system + .parent(&self.path) + .await + .map(|path| path.map(|path| self.with_path(path))) + } + + pub async fn canonicalize( + &self, + sandbox: Option<&FileSystemSandboxContext>, + ) -> io::Result { + self.file_system + .canonicalize(&self.path, sandbox) + .await + .map(|path| self.with_path(path)) + } } impl PartialEq for EnvironmentPathRef { @@ -96,10 +121,16 @@ mod tests { use codex_utils_absolute_path::test_support::PathBufExt; use pretty_assertions::assert_eq; use std::collections::HashSet; + use std::path::Path; use std::sync::Mutex; + use crate::LOCAL_FS; + #[derive(Clone, Debug, Eq, PartialEq)] enum RecordedMethod { + Canonicalize, + Join, + Parent, ReadFileText, Metadata, ReadDirectory, @@ -140,8 +171,47 @@ mod tests { } } + fn local_path_ref(path: AbsolutePathBuf) -> EnvironmentPathRef { + EnvironmentPathRef::new(Arc::clone(&LOCAL_FS), path) + } + #[async_trait] impl ExecutorFileSystem for RecordingFileSystem { + async fn canonicalize( + &self, + path: &AbsolutePathBuf, + _sandbox: Option<&FileSystemSandboxContext>, + ) -> io::Result { + self.push_call(RecordedCall { + method: RecordedMethod::Canonicalize, + path: path.clone(), + sandbox: None, + }); + Ok(path.parent().unwrap()) + } + + async fn join( + &self, + base_path: &AbsolutePathBuf, + path: &Path, + ) -> io::Result { + self.push_call(RecordedCall { + method: RecordedMethod::Join, + path: base_path.clone(), + sandbox: None, + }); + AbsolutePathBuf::from_absolute_path_checked(base_path.as_path().join(path)) + } + + async fn parent(&self, path: &AbsolutePathBuf) -> io::Result> { + self.push_call(RecordedCall { + method: RecordedMethod::Parent, + path: path.clone(), + sandbox: None, + }); + Ok(path.parent()) + } + async fn read_file( &self, path: &AbsolutePathBuf, @@ -318,4 +388,129 @@ mod tests { let set = HashSet::from([left, same, different_path, different_fs]); assert_eq!(set.len(), 3); } + #[tokio::test] + async fn canonicalize_keeps_bound_file_system_identity() { + let path = std::env::temp_dir().join("skills/demo").abs(); + let file_system = Arc::new(RecordingFileSystem::default()); + let path_ref = EnvironmentPathRef::new(file_system.clone(), path.clone()); + + let canonicalized = path_ref + .canonicalize(/*sandbox*/ None) + .await + .expect("canonicalize"); + + assert_eq!(canonicalized.path(), &path.parent().unwrap()); + assert_eq!( + canonicalized, + EnvironmentPathRef::new(file_system.clone(), path.parent().unwrap()) + ); + assert_eq!( + file_system.recorded_calls(), + vec![RecordedCall { + method: RecordedMethod::Canonicalize, + path, + sandbox: None, + }] + ); + } + + #[tokio::test] + async fn join_keeps_bound_file_system_identity() { + let path = std::env::temp_dir().join("skills").abs(); + let file_system = Arc::new(RecordingFileSystem::default()); + let path_ref = EnvironmentPathRef::new(file_system.clone(), path.clone()); + + assert_eq!( + path_ref.join(Path::new("demo")).await.ok(), + Some(EnvironmentPathRef::new( + file_system.clone(), + std::env::temp_dir().join("skills/demo").abs(), + )) + ); + assert_eq!( + file_system.recorded_calls(), + vec![RecordedCall { + method: RecordedMethod::Join, + path, + sandbox: None, + }] + ); + } + + #[tokio::test] + async fn join_matches_absolute_path_buf_for_tilde_paths() { + let path_ref = local_path_ref(std::env::temp_dir().join("skills").abs()); + + assert_eq!( + path_ref + .join(Path::new("~")) + .await + .ok() + .map(|path_ref| path_ref.path().clone()), + Some(path_ref.path().join(Path::new("~"))) + ); + } + + #[tokio::test] + async fn join_matches_absolute_path_buf_for_parent_dirs() { + let path_ref = local_path_ref(std::env::temp_dir().join("skills").abs()); + + assert_eq!( + path_ref + .join(Path::new("../outside")) + .await + .expect("join") + .path() + .clone(), + path_ref.path().join(Path::new("../outside")) + ); + } + + #[tokio::test] + async fn parent_keeps_bound_file_system_identity() { + let path = std::env::temp_dir().join("skills/demo").abs(); + let file_system = Arc::new(RecordingFileSystem::default()); + let path_ref = EnvironmentPathRef::new(file_system.clone(), path.clone()); + + assert_eq!( + path_ref.parent().await.expect("parent"), + Some(EnvironmentPathRef::new( + file_system.clone(), + std::env::temp_dir().join("skills").abs(), + )) + ); + assert_eq!( + file_system.recorded_calls(), + vec![RecordedCall { + method: RecordedMethod::Parent, + path, + sandbox: None, + }] + ); + } + + #[cfg(windows)] + #[tokio::test] + async fn join_matches_absolute_path_buf_for_windows_prefixed_and_rooted_paths() { + let path_ref = local_path_ref(std::env::temp_dir().join("skills").abs()); + + assert_eq!( + path_ref + .join(Path::new(r"C:temp")) + .await + .expect("join") + .path() + .clone(), + path_ref.path().join(Path::new(r"C:temp")) + ); + assert_eq!( + path_ref + .join(Path::new(r"\temp")) + .await + .expect("join") + .path() + .clone(), + path_ref.path().join(Path::new(r"\temp")) + ); + } } diff --git a/codex-rs/exec-server/src/fs_helper.rs b/codex-rs/exec-server/src/fs_helper.rs index 8d912248039..0a210175b27 100644 --- a/codex-rs/exec-server/src/fs_helper.rs +++ b/codex-rs/exec-server/src/fs_helper.rs @@ -10,6 +10,7 @@ use crate::CreateDirectoryOptions; use crate::ExecutorFileSystem; use crate::RemoveOptions; use crate::local_file_system::DirectFileSystem; +use crate::protocol::FS_CANONICALIZE_METHOD; use crate::protocol::FS_COPY_METHOD; use crate::protocol::FS_CREATE_DIRECTORY_METHOD; use crate::protocol::FS_GET_METADATA_METHOD; @@ -17,6 +18,8 @@ use crate::protocol::FS_READ_DIRECTORY_METHOD; use crate::protocol::FS_READ_FILE_METHOD; use crate::protocol::FS_REMOVE_METHOD; use crate::protocol::FS_WRITE_FILE_METHOD; +use crate::protocol::FsCanonicalizeParams; +use crate::protocol::FsCanonicalizeResponse; use crate::protocol::FsCopyParams; use crate::protocol::FsCopyResponse; use crate::protocol::FsCreateDirectoryParams; @@ -49,6 +52,8 @@ pub(crate) enum FsHelperRequest { CreateDirectory(FsCreateDirectoryParams), #[serde(rename = "fs/getMetadata")] GetMetadata(FsGetMetadataParams), + #[serde(rename = "fs/canonicalize")] + Canonicalize(FsCanonicalizeParams), #[serde(rename = "fs/readDirectory")] ReadDirectory(FsReadDirectoryParams), #[serde(rename = "fs/remove")] @@ -75,6 +80,8 @@ pub(crate) enum FsHelperPayload { CreateDirectory(FsCreateDirectoryResponse), #[serde(rename = "fs/getMetadata")] GetMetadata(FsGetMetadataResponse), + #[serde(rename = "fs/canonicalize")] + Canonicalize(FsCanonicalizeResponse), #[serde(rename = "fs/readDirectory")] ReadDirectory(FsReadDirectoryResponse), #[serde(rename = "fs/remove")] @@ -90,6 +97,7 @@ impl FsHelperPayload { Self::WriteFile(_) => FS_WRITE_FILE_METHOD, Self::CreateDirectory(_) => FS_CREATE_DIRECTORY_METHOD, Self::GetMetadata(_) => FS_GET_METADATA_METHOD, + Self::Canonicalize(_) => FS_CANONICALIZE_METHOD, Self::ReadDirectory(_) => FS_READ_DIRECTORY_METHOD, Self::Remove(_) => FS_REMOVE_METHOD, Self::Copy(_) => FS_COPY_METHOD, @@ -132,6 +140,16 @@ impl FsHelperPayload { } } + pub(crate) fn expect_canonicalize(self) -> Result { + match self { + Self::Canonicalize(response) => Ok(response), + other => Err(unexpected_response( + FS_CANONICALIZE_METHOD, + other.operation(), + )), + } + } + pub(crate) fn expect_read_directory( self, ) -> Result { @@ -219,6 +237,15 @@ pub(crate) async fn run_direct_request( modified_at_ms: metadata.modified_at_ms, })) } + FsHelperRequest::Canonicalize(params) => { + let path = file_system + .canonicalize(¶ms.path, /*sandbox*/ None) + .await + .map_err(map_fs_error)?; + Ok(FsHelperPayload::Canonicalize(FsCanonicalizeResponse { + path, + })) + } FsHelperRequest::ReadDirectory(params) => { let entries = file_system .read_directory(¶ms.path, /*sandbox*/ None) diff --git a/codex-rs/exec-server/src/lib.rs b/codex-rs/exec-server/src/lib.rs index 01b530ba341..dea039c2883 100644 --- a/codex-rs/exec-server/src/lib.rs +++ b/codex-rs/exec-server/src/lib.rs @@ -64,12 +64,18 @@ pub use protocol::ExecOutputDeltaNotification; pub use protocol::ExecOutputStream; pub use protocol::ExecParams; pub use protocol::ExecResponse; +pub use protocol::FsCanonicalizeParams; +pub use protocol::FsCanonicalizeResponse; pub use protocol::FsCopyParams; pub use protocol::FsCopyResponse; pub use protocol::FsCreateDirectoryParams; pub use protocol::FsCreateDirectoryResponse; pub use protocol::FsGetMetadataParams; pub use protocol::FsGetMetadataResponse; +pub use protocol::FsJoinParams; +pub use protocol::FsJoinResponse; +pub use protocol::FsParentParams; +pub use protocol::FsParentResponse; pub use protocol::FsReadDirectoryEntry; pub use protocol::FsReadDirectoryParams; pub use protocol::FsReadDirectoryResponse; diff --git a/codex-rs/exec-server/src/local_file_system.rs b/codex-rs/exec-server/src/local_file_system.rs index c7d628ce8f1..3f410f926de 100644 --- a/codex-rs/exec-server/src/local_file_system.rs +++ b/codex-rs/exec-server/src/local_file_system.rs @@ -79,6 +79,27 @@ impl LocalFileSystem { #[async_trait] impl ExecutorFileSystem for LocalFileSystem { + async fn canonicalize( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult { + let (file_system, sandbox) = self.file_system_for(sandbox)?; + file_system.canonicalize(path, sandbox).await + } + + async fn join( + &self, + base_path: &AbsolutePathBuf, + path: &Path, + ) -> FileSystemResult { + self.unsandboxed.join(base_path, path).await + } + + async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult> { + self.unsandboxed.parent(path).await + } + async fn read_file( &self, path: &AbsolutePathBuf, @@ -152,6 +173,27 @@ impl ExecutorFileSystem for LocalFileSystem { #[async_trait] impl ExecutorFileSystem for UnsandboxedFileSystem { + async fn canonicalize( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult { + reject_platform_sandbox_context(sandbox)?; + self.file_system.canonicalize(path, /*sandbox*/ None).await + } + + async fn join( + &self, + base_path: &AbsolutePathBuf, + path: &Path, + ) -> FileSystemResult { + self.file_system.join(base_path, path).await + } + + async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult> { + self.file_system.parent(path).await + } + async fn read_file( &self, path: &AbsolutePathBuf, @@ -238,6 +280,27 @@ impl ExecutorFileSystem for UnsandboxedFileSystem { #[async_trait] impl ExecutorFileSystem for DirectFileSystem { + async fn canonicalize( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult { + reject_sandbox_context(sandbox)?; + AbsolutePathBuf::from_absolute_path(tokio::fs::canonicalize(path.as_path()).await?) + } + + async fn join( + &self, + base_path: &AbsolutePathBuf, + path: &Path, + ) -> FileSystemResult { + Ok(base_path.join(path)) + } + + async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult> { + Ok(path.parent()) + } + async fn read_file( &self, path: &AbsolutePathBuf, diff --git a/codex-rs/exec-server/src/protocol.rs b/codex-rs/exec-server/src/protocol.rs index e801a7f4379..9ac80c188ed 100644 --- a/codex-rs/exec-server/src/protocol.rs +++ b/codex-rs/exec-server/src/protocol.rs @@ -23,6 +23,9 @@ pub const FS_READ_FILE_METHOD: &str = "fs/readFile"; pub const FS_WRITE_FILE_METHOD: &str = "fs/writeFile"; pub const FS_CREATE_DIRECTORY_METHOD: &str = "fs/createDirectory"; pub const FS_GET_METADATA_METHOD: &str = "fs/getMetadata"; +pub const FS_CANONICALIZE_METHOD: &str = "fs/canonicalize"; +pub const FS_JOIN_METHOD: &str = "fs/join"; +pub const FS_PARENT_METHOD: &str = "fs/parent"; pub const FS_READ_DIRECTORY_METHOD: &str = "fs/readDirectory"; pub const FS_REMOVE_METHOD: &str = "fs/remove"; pub const FS_COPY_METHOD: &str = "fs/copy"; @@ -211,6 +214,44 @@ pub struct FsGetMetadataResponse { pub modified_at_ms: i64, } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct FsCanonicalizeParams { + pub path: AbsolutePathBuf, + pub sandbox: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct FsCanonicalizeResponse { + pub path: AbsolutePathBuf, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct FsJoinParams { + pub base_path: AbsolutePathBuf, + pub path: PathBuf, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct FsJoinResponse { + pub path: AbsolutePathBuf, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct FsParentParams { + pub path: AbsolutePathBuf, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct FsParentResponse { + pub path: Option, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct FsReadDirectoryParams { diff --git a/codex-rs/exec-server/src/remote_file_system.rs b/codex-rs/exec-server/src/remote_file_system.rs index 4251fa33ecd..54198c93bac 100644 --- a/codex-rs/exec-server/src/remote_file_system.rs +++ b/codex-rs/exec-server/src/remote_file_system.rs @@ -2,6 +2,7 @@ use async_trait::async_trait; use base64::Engine as _; use base64::engine::general_purpose::STANDARD; use codex_utils_absolute_path::AbsolutePathBuf; +use std::path::Path; use tokio::io; use tracing::trace; @@ -15,9 +16,12 @@ use crate::FileSystemSandboxContext; use crate::ReadDirectoryEntry; use crate::RemoveOptions; use crate::client::LazyRemoteExecServerClient; +use crate::protocol::FsCanonicalizeParams; use crate::protocol::FsCopyParams; use crate::protocol::FsCreateDirectoryParams; use crate::protocol::FsGetMetadataParams; +use crate::protocol::FsJoinParams; +use crate::protocol::FsParentParams; use crate::protocol::FsReadDirectoryParams; use crate::protocol::FsReadFileParams; use crate::protocol::FsRemoveParams; @@ -26,7 +30,6 @@ use crate::protocol::FsWriteFileParams; const INVALID_REQUEST_ERROR_CODE: i64 = -32600; const NOT_FOUND_ERROR_CODE: i64 = -32004; -#[derive(Clone)] pub(crate) struct RemoteFileSystem { client: LazyRemoteExecServerClient, } @@ -40,6 +43,50 @@ impl RemoteFileSystem { #[async_trait] impl ExecutorFileSystem for RemoteFileSystem { + async fn canonicalize( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult { + trace!("remote fs canonicalize"); + let client = self.client.get().await.map_err(map_remote_error)?; + let response = client + .fs_canonicalize(FsCanonicalizeParams { + path: path.clone(), + sandbox: remote_sandbox_context(sandbox), + }) + .await + .map_err(map_remote_error)?; + Ok(response.path) + } + + async fn join( + &self, + base_path: &AbsolutePathBuf, + path: &Path, + ) -> FileSystemResult { + trace!("remote fs join"); + let client = self.client.get().await.map_err(map_remote_error)?; + let response = client + .fs_join(FsJoinParams { + base_path: base_path.clone(), + path: path.to_path_buf(), + }) + .await + .map_err(map_remote_error)?; + Ok(response.path) + } + + async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult> { + trace!("remote fs parent"); + let client = self.client.get().await.map_err(map_remote_error)?; + let response = client + .fs_parent(FsParentParams { path: path.clone() }) + .await + .map_err(map_remote_error)?; + Ok(response.path) + } + async fn read_file( &self, path: &AbsolutePathBuf, diff --git a/codex-rs/exec-server/src/sandboxed_file_system.rs b/codex-rs/exec-server/src/sandboxed_file_system.rs index 133a3b73273..00b6966c3bd 100644 --- a/codex-rs/exec-server/src/sandboxed_file_system.rs +++ b/codex-rs/exec-server/src/sandboxed_file_system.rs @@ -3,6 +3,7 @@ use base64::Engine as _; use base64::engine::general_purpose::STANDARD; use codex_app_server_protocol::JSONRPCErrorError; use codex_utils_absolute_path::AbsolutePathBuf; +use std::path::Path; use tokio::io; use crate::CopyOptions; @@ -17,6 +18,7 @@ use crate::RemoveOptions; use crate::fs_helper::FsHelperPayload; use crate::fs_helper::FsHelperRequest; use crate::fs_sandbox::FileSystemSandboxRunner; +use crate::protocol::FsCanonicalizeParams; use crate::protocol::FsCopyParams; use crate::protocol::FsCreateDirectoryParams; use crate::protocol::FsGetMetadataParams; @@ -51,6 +53,38 @@ impl SandboxedFileSystem { #[async_trait] impl ExecutorFileSystem for SandboxedFileSystem { + async fn canonicalize( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult { + let sandbox = require_platform_sandbox(sandbox)?; + let response = self + .run_sandboxed( + sandbox, + FsHelperRequest::Canonicalize(FsCanonicalizeParams { + path: path.clone(), + sandbox: None, + }), + ) + .await? + .expect_canonicalize() + .map_err(map_sandbox_error)?; + Ok(response.path) + } + + async fn join( + &self, + base_path: &AbsolutePathBuf, + path: &Path, + ) -> FileSystemResult { + Ok(base_path.join(path)) + } + + async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult> { + Ok(path.parent()) + } + async fn read_file( &self, path: &AbsolutePathBuf, 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 bef30e975f6..b60589d0200 100644 --- a/codex-rs/exec-server/src/server/file_system_handler.rs +++ b/codex-rs/exec-server/src/server/file_system_handler.rs @@ -11,12 +11,18 @@ use crate::ExecutorFileSystem; use crate::RemoveOptions; use crate::local_file_system::LocalFileSystem; use crate::protocol::FS_WRITE_FILE_METHOD; +use crate::protocol::FsCanonicalizeParams; +use crate::protocol::FsCanonicalizeResponse; use crate::protocol::FsCopyParams; use crate::protocol::FsCopyResponse; use crate::protocol::FsCreateDirectoryParams; use crate::protocol::FsCreateDirectoryResponse; use crate::protocol::FsGetMetadataParams; use crate::protocol::FsGetMetadataResponse; +use crate::protocol::FsJoinParams; +use crate::protocol::FsJoinResponse; +use crate::protocol::FsParentParams; +use crate::protocol::FsParentResponse; use crate::protocol::FsReadDirectoryEntry; use crate::protocol::FsReadDirectoryParams; use crate::protocol::FsReadDirectoryResponse; @@ -106,6 +112,42 @@ impl FileSystemHandler { }) } + pub(crate) async fn canonicalize( + &self, + params: FsCanonicalizeParams, + ) -> Result { + let path = self + .file_system + .canonicalize(¶ms.path, params.sandbox.as_ref()) + .await + .map_err(map_fs_error)?; + Ok(FsCanonicalizeResponse { path }) + } + + pub(crate) async fn join( + &self, + params: FsJoinParams, + ) -> Result { + let path = self + .file_system + .join(¶ms.base_path, ¶ms.path) + .await + .map_err(map_fs_error)?; + Ok(FsJoinResponse { path }) + } + + pub(crate) async fn parent( + &self, + params: FsParentParams, + ) -> Result { + let path = self + .file_system + .parent(¶ms.path) + .await + .map_err(map_fs_error)?; + Ok(FsParentResponse { path }) + } + pub(crate) async fn read_directory( &self, params: FsReadDirectoryParams, diff --git a/codex-rs/exec-server/src/server/handler.rs b/codex-rs/exec-server/src/server/handler.rs index d0645724c4f..5456ce41c0b 100644 --- a/codex-rs/exec-server/src/server/handler.rs +++ b/codex-rs/exec-server/src/server/handler.rs @@ -16,12 +16,18 @@ use crate::client::http_client::PendingReqwestHttpBodyStream; use crate::client::http_client::ReqwestHttpRequestRunner; use crate::protocol::ExecParams; use crate::protocol::ExecResponse; +use crate::protocol::FsCanonicalizeParams; +use crate::protocol::FsCanonicalizeResponse; use crate::protocol::FsCopyParams; use crate::protocol::FsCopyResponse; use crate::protocol::FsCreateDirectoryParams; use crate::protocol::FsCreateDirectoryResponse; use crate::protocol::FsGetMetadataParams; use crate::protocol::FsGetMetadataResponse; +use crate::protocol::FsJoinParams; +use crate::protocol::FsJoinResponse; +use crate::protocol::FsParentParams; +use crate::protocol::FsParentResponse; use crate::protocol::FsReadDirectoryParams; use crate::protocol::FsReadDirectoryResponse; use crate::protocol::FsReadFileParams; @@ -240,6 +246,30 @@ impl ExecServerHandler { self.file_system.get_metadata(params).await } + pub(crate) async fn fs_canonicalize( + &self, + params: FsCanonicalizeParams, + ) -> Result { + self.require_initialized_for("filesystem")?; + self.file_system.canonicalize(params).await + } + + pub(crate) async fn fs_join( + &self, + params: FsJoinParams, + ) -> Result { + self.require_initialized_for("filesystem")?; + self.file_system.join(params).await + } + + pub(crate) async fn fs_parent( + &self, + params: FsParentParams, + ) -> Result { + self.require_initialized_for("filesystem")?; + self.file_system.parent(params).await + } + pub(crate) async fn fs_read_directory( &self, params: FsReadDirectoryParams, diff --git a/codex-rs/exec-server/src/server/registry.rs b/codex-rs/exec-server/src/server/registry.rs index 87dee6aa580..74adf905805 100644 --- a/codex-rs/exec-server/src/server/registry.rs +++ b/codex-rs/exec-server/src/server/registry.rs @@ -5,16 +5,22 @@ use crate::protocol::EXEC_READ_METHOD; use crate::protocol::EXEC_TERMINATE_METHOD; use crate::protocol::EXEC_WRITE_METHOD; use crate::protocol::ExecParams; +use crate::protocol::FS_CANONICALIZE_METHOD; use crate::protocol::FS_COPY_METHOD; use crate::protocol::FS_CREATE_DIRECTORY_METHOD; use crate::protocol::FS_GET_METADATA_METHOD; +use crate::protocol::FS_JOIN_METHOD; +use crate::protocol::FS_PARENT_METHOD; use crate::protocol::FS_READ_DIRECTORY_METHOD; use crate::protocol::FS_READ_FILE_METHOD; use crate::protocol::FS_REMOVE_METHOD; use crate::protocol::FS_WRITE_FILE_METHOD; +use crate::protocol::FsCanonicalizeParams; use crate::protocol::FsCopyParams; use crate::protocol::FsCreateDirectoryParams; use crate::protocol::FsGetMetadataParams; +use crate::protocol::FsJoinParams; +use crate::protocol::FsParentParams; use crate::protocol::FsReadDirectoryParams; use crate::protocol::FsReadFileParams; use crate::protocol::FsRemoveParams; @@ -96,6 +102,24 @@ pub(crate) fn build_router() -> RpcRouter { handler.fs_get_metadata(params).await }, ); + router.request( + FS_CANONICALIZE_METHOD, + |handler: Arc, params: FsCanonicalizeParams| async move { + handler.fs_canonicalize(params).await + }, + ); + router.request( + FS_JOIN_METHOD, + |handler: Arc, params: FsJoinParams| async move { + handler.fs_join(params).await + }, + ); + router.request( + FS_PARENT_METHOD, + |handler: Arc, params: FsParentParams| async move { + handler.fs_parent(params).await + }, + ); router.request( FS_READ_DIRECTORY_METHOD, |handler: Arc, params: FsReadDirectoryParams| async move { diff --git a/codex-rs/exec-server/tests/file_system.rs b/codex-rs/exec-server/tests/file_system.rs index 3da796df245..b8d1c218943 100644 --- a/codex-rs/exec-server/tests/file_system.rs +++ b/codex-rs/exec-server/tests/file_system.rs @@ -365,6 +365,49 @@ async fn file_system_methods_cover_surface_area(use_remote: bool) -> Result<()> .await .with_context(|| format!("mode={use_remote}"))?; + let source_link = tmp.path().join("source-link"); + symlink(&source_dir, &source_link)?; + let joined_nested = file_system + .join( + &absolute_path(source_link.clone()), + Path::new("nested/note.txt"), + ) + .await + .with_context(|| format!("mode={use_remote}"))?; + assert_eq!( + joined_nested, + absolute_path(source_link.join("nested").join("note.txt")) + ); + let joined_parent = file_system + .parent(&joined_nested) + .await + .with_context(|| format!("mode={use_remote}"))?; + assert_eq!( + joined_parent, + Some(absolute_path(source_link.join("nested"))) + ); + let joined_parent_traversal = file_system + .join(&absolute_path(source_dir.clone()), Path::new("../outside")) + .await + .with_context(|| format!("mode={use_remote}"))?; + assert_eq!( + joined_parent_traversal, + absolute_path(source_dir.join("../outside")) + ); + let canonical_nested = file_system + .canonicalize( + &absolute_path(source_link.join("nested").join("note.txt")), + /*sandbox*/ None, + ) + .await + .with_context(|| format!("mode={use_remote}"))?; + assert_eq!( + canonical_nested, + absolute_path(std::fs::canonicalize( + source_dir.join("nested").join("note.txt") + )?) + ); + let nested_file_contents = file_system .read_file(&absolute_path(nested_file.clone()), /*sandbox*/ None) .await @@ -530,6 +573,32 @@ async fn file_system_sandboxed_read_allows_readable_root(use_remote: bool) -> Re Ok(()) } +#[test_case(false ; "local")] +#[test_case(true ; "remote")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn file_system_sandboxed_canonicalize_allows_readable_root(use_remote: bool) -> Result<()> { + let context = create_file_system_context(use_remote).await?; + let file_system = context.file_system; + + let tmp = TempDir::new()?; + let allowed_dir = tmp.path().join("allowed"); + let file_path = allowed_dir.join("note.txt"); + std::fs::create_dir_all(&allowed_dir)?; + std::fs::write(&file_path, "sandboxed hello")?; + let sandbox = read_only_sandbox(allowed_dir); + + let canonical_path = file_system + .canonicalize(&absolute_path(file_path.clone()), Some(&sandbox)) + .await + .with_context(|| format!("mode={use_remote}"))?; + assert_eq!( + canonical_path, + absolute_path(std::fs::canonicalize(file_path)?) + ); + + Ok(()) +} + #[test_case(false ; "local")] #[test_case(true ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] diff --git a/codex-rs/file-system/src/lib.rs b/codex-rs/file-system/src/lib.rs index 606f987cbd1..8fad1f5b62b 100644 --- a/codex-rs/file-system/src/lib.rs +++ b/codex-rs/file-system/src/lib.rs @@ -133,6 +133,23 @@ pub type FileSystemResult = io::Result; /// a remote environment. #[async_trait] pub trait ExecutorFileSystem: Send + Sync { + /// Resolves a path within this filesystem. + async fn canonicalize( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult; + + /// Lexically joins a path onto an existing bound path. + async fn join( + &self, + base_path: &AbsolutePathBuf, + path: &Path, + ) -> FileSystemResult; + + /// Returns the parent directory of a bound path. + async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult>; + async fn read_file( &self, path: &AbsolutePathBuf,