From 7622e229a25c9c93114d3434eb94af9880fe42fa Mon Sep 17 00:00:00 2001 From: starr-openai Date: Fri, 29 May 2026 13:44:21 -0700 Subject: [PATCH] exec-server: add bound path rpc helpers --- codex-rs/exec-server/src/client.rs | 17 ++++ codex-rs/exec-server/src/environment_path.rs | 87 ++++++++++++++++++- codex-rs/exec-server/src/local_file_system.rs | 59 +++++++++++++ codex-rs/exec-server/src/protocol.rs | 27 ++++++ .../exec-server/src/remote_file_system.rs | 87 +++++++++++++++---- .../src/server/file_system_handler.rs | 26 ++++++ codex-rs/exec-server/src/server/handler.rs | 20 +++++ codex-rs/exec-server/src/server/registry.rs | 16 ++++ codex-rs/exec-server/tests/file_system.rs | 26 ++++++ codex-rs/file-system/src/lib.rs | 27 ++++++ 10 files changed, 374 insertions(+), 18 deletions(-) diff --git a/codex-rs/exec-server/src/client.rs b/codex-rs/exec-server/src/client.rs index b362238a04..b6c56e9f5e 100644 --- a/codex-rs/exec-server/src/client.rs +++ b/codex-rs/exec-server/src/client.rs @@ -45,6 +45,8 @@ 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; @@ -57,6 +59,10 @@ 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; @@ -424,6 +430,17 @@ impl ExecServerClient { 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 8a2995a600..270651cc5f 100644 --- a/codex-rs/exec-server/src/environment_path.rs +++ b/codex-rs/exec-server/src/environment_path.rs @@ -2,7 +2,6 @@ use std::fmt; use std::hash::Hash; use std::hash::Hasher; use std::io; -use std::path::Component; use std::path::Path; use std::sync::Arc; @@ -63,6 +62,21 @@ impl EnvironmentPathRef { Self::new(Arc::clone(&self.file_system), path) } + pub fn join_relative(&self, relative_path: &Path) -> Option { + self.file_system + .join(&self.path, relative_path) + .ok() + .map(|path| self.with_path(path)) + } + + pub fn parent_dir(&self) -> Option { + self.file_system + .parent(&self.path) + .ok() + .flatten() + .map(|path| self.with_path(path)) + } + /// Best-effort resolves this path through its bound filesystem. pub fn canonicalize_if_exists(&self) -> Self { self.file_system @@ -105,11 +119,14 @@ 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; #[derive(Clone, Debug, Eq, PartialEq)] enum RecordedMethod { Canonicalize, + Join, + Parent, ReadFileText, Metadata, ReadDirectory, @@ -176,6 +193,28 @@ mod tests { Ok(path.parent().unwrap()) } + fn join( + &self, + base_path: &AbsolutePathBuf, + relative_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(relative_path)) + } + + 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, @@ -384,6 +423,29 @@ mod tests { assert_eq!(path_ref.canonicalize_if_exists(), path_ref); } + #[test] + fn join_relative_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_relative(Path::new("demo")), + 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, + }] + ); + } + #[test] fn join_relative_keeps_literal_tilde_under_bound_path() { let path_ref = EnvironmentPathRef::local(std::env::temp_dir().join("skills").abs()); @@ -403,6 +465,29 @@ mod tests { assert_eq!(path_ref.join_relative(Path::new("../outside")), None); } + #[test] + fn parent_dir_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_dir(), + 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)] #[test] fn join_relative_rejects_windows_prefixed_and_rooted_paths() { diff --git a/codex-rs/exec-server/src/local_file_system.rs b/codex-rs/exec-server/src/local_file_system.rs index dd09c9950d..3489186293 100644 --- a/codex-rs/exec-server/src/local_file_system.rs +++ b/codex-rs/exec-server/src/local_file_system.rs @@ -1,5 +1,6 @@ use async_trait::async_trait; use codex_utils_absolute_path::AbsolutePathBuf; +use std::path::Component; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; @@ -83,6 +84,18 @@ impl ExecutorFileSystem for LocalFileSystem { self.unsandboxed.canonicalize(path) } + fn join( + &self, + base_path: &AbsolutePathBuf, + relative_path: &Path, + ) -> FileSystemResult { + self.unsandboxed.join(base_path, relative_path) + } + + fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult> { + self.unsandboxed.parent(path) + } + async fn read_file( &self, path: &AbsolutePathBuf, @@ -160,6 +173,18 @@ impl ExecutorFileSystem for UnsandboxedFileSystem { self.file_system.canonicalize(path) } + fn join( + &self, + base_path: &AbsolutePathBuf, + relative_path: &Path, + ) -> FileSystemResult { + self.file_system.join(base_path, relative_path) + } + + fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult> { + self.file_system.parent(path) + } + async fn read_file( &self, path: &AbsolutePathBuf, @@ -250,6 +275,18 @@ impl ExecutorFileSystem for DirectFileSystem { path.canonicalize() } + fn join( + &self, + base_path: &AbsolutePathBuf, + relative_path: &Path, + ) -> FileSystemResult { + join_bound_path(base_path, relative_path) + } + + fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult> { + Ok(path.parent()) + } + async fn read_file( &self, path: &AbsolutePathBuf, @@ -419,6 +456,28 @@ fn reject_sandbox_context(sandbox: Option<&FileSystemSandboxContext>) -> io::Res Ok(()) } +fn join_bound_path( + base_path: &AbsolutePathBuf, + relative_path: &Path, +) -> FileSystemResult { + for component in relative_path.components() { + match component { + Component::CurDir | Component::Normal(_) => {} + Component::ParentDir | Component::RootDir | Component::Prefix(_) => { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!( + "relative path must not escape or replace the bound root: {}", + relative_path.display() + ), + )); + } + } + } + + AbsolutePathBuf::from_absolute_path_checked(base_path.as_path().join(relative_path)) +} + fn reject_platform_sandbox_context(sandbox: Option<&FileSystemSandboxContext>) -> io::Result<()> { if sandbox.is_some_and(FileSystemSandboxContext::should_run_in_sandbox) { return Err(io::Error::new( diff --git a/codex-rs/exec-server/src/protocol.rs b/codex-rs/exec-server/src/protocol.rs index 2de5c423f3..fa56306c62 100644 --- a/codex-rs/exec-server/src/protocol.rs +++ b/codex-rs/exec-server/src/protocol.rs @@ -24,6 +24,8 @@ 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"; @@ -224,6 +226,31 @@ 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 relative_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 1ad0769bdf..8c3a9ebbe8 100644 --- a/codex-rs/exec-server/src/remote_file_system.rs +++ b/codex-rs/exec-server/src/remote_file_system.rs @@ -2,6 +2,8 @@ use async_trait::async_trait; use base64::Engine as _; use base64::engine::general_purpose::STANDARD; use codex_utils_absolute_path::AbsolutePathBuf; +use std::future::Future; +use std::path::Path; use std::thread; use tokio::io; use tracing::trace; @@ -15,11 +17,14 @@ use crate::FileSystemResult; use crate::FileSystemSandboxContext; use crate::ReadDirectoryEntry; use crate::RemoveOptions; +use crate::client::ExecServerClient; 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; @@ -38,31 +43,79 @@ impl RemoteFileSystem { trace!("remote fs new"); Self { client } } + + fn run_sync_remote_call( + &self, + operation: &'static str, + call: F, + ) -> FileSystemResult + where + T: Send + 'static, + F: FnOnce(ExecServerClient) -> Fut + Send + 'static, + Fut: Future> + Send + 'static, + { + let runtime = tokio::runtime::Handle::try_current().map_err(|err| { + io::Error::other(format!("remote fs {operation} requires runtime: {err}")) + })?; + let client = self.client.clone(); + // ExecutorFileSystem exposes synchronous path operations, so remote callers have to hop + // through the active runtime from a helper thread to reach the JSON-RPC filesystem + // surface without blocking the runtime worker. + thread::spawn(move || { + runtime.block_on(async move { + let client = client.get().await.map_err(map_remote_error)?; + call(client).await + }) + }) + .join() + .map_err(|_| io::Error::other(format!("remote fs {operation} thread panicked")))? + } } #[async_trait] impl ExecutorFileSystem for RemoteFileSystem { fn canonicalize(&self, path: &AbsolutePathBuf) -> FileSystemResult { trace!("remote fs canonicalize"); - let runtime = tokio::runtime::Handle::try_current().map_err(|err| { - io::Error::other(format!("remote fs canonicalize requires runtime: {err}")) - })?; - let client = self.client.clone(); let path = path.clone(); - // ExecutorFileSystem::canonicalize is synchronous, so remote callers have to hop through - // the active runtime from a helper thread to reach the JSON-RPC filesystem surface. - thread::spawn(move || { - runtime.block_on(async move { - let client = client.get().await.map_err(map_remote_error)?; - let response = client - .fs_canonicalize(FsCanonicalizeParams { path }) - .await - .map_err(map_remote_error)?; - Ok(response.path) - }) + self.run_sync_remote_call("canonicalize", move |client| async move { + let response = client + .fs_canonicalize(FsCanonicalizeParams { path }) + .await + .map_err(map_remote_error)?; + Ok(response.path) + }) + } + + fn join( + &self, + base_path: &AbsolutePathBuf, + relative_path: &Path, + ) -> FileSystemResult { + trace!("remote fs join"); + let base_path = base_path.clone(); + let relative_path = relative_path.to_path_buf(); + self.run_sync_remote_call("join", move |client| async move { + let response = client + .fs_join(FsJoinParams { + base_path, + relative_path, + }) + .await + .map_err(map_remote_error)?; + Ok(response.path) + }) + } + + fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult> { + trace!("remote fs parent"); + let path = path.clone(); + self.run_sync_remote_call("parent", move |client| async move { + let response = client + .fs_parent(FsParentParams { path }) + .await + .map_err(map_remote_error)?; + Ok(response.path) }) - .join() - .map_err(|_| io::Error::other("remote fs canonicalize thread panicked"))? } async fn read_file( 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 473ecb7843..b3e4a5cebd 100644 --- a/codex-rs/exec-server/src/server/file_system_handler.rs +++ b/codex-rs/exec-server/src/server/file_system_handler.rs @@ -19,6 +19,10 @@ 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; @@ -119,6 +123,28 @@ impl FileSystemHandler { Ok(FsCanonicalizeResponse { path }) } + pub(crate) async fn join( + &self, + params: FsJoinParams, + ) -> Result { + let path = self + .file_system + .join(¶ms.base_path, ¶ms.relative_path) + .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) + .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 3d4849329c..5456ce41c0 100644 --- a/codex-rs/exec-server/src/server/handler.rs +++ b/codex-rs/exec-server/src/server/handler.rs @@ -24,6 +24,10 @@ 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; @@ -250,6 +254,22 @@ impl ExecServerHandler { 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 44d0562382..74adf90580 100644 --- a/codex-rs/exec-server/src/server/registry.rs +++ b/codex-rs/exec-server/src/server/registry.rs @@ -9,6 +9,8 @@ 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; @@ -17,6 +19,8 @@ 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; @@ -104,6 +108,18 @@ pub(crate) fn build_router() -> RpcRouter { 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 2e59308065..212c6c461e 100644 --- a/codex-rs/exec-server/tests/file_system.rs +++ b/codex-rs/exec-server/tests/file_system.rs @@ -367,6 +367,32 @@ async fn file_system_methods_cover_surface_area(use_remote: bool) -> Result<()> 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"), + ) + .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) + .with_context(|| format!("mode={use_remote}"))?; + assert_eq!( + joined_parent, + Some(absolute_path(source_link.join("nested"))) + ); + let join_error = + match file_system.join(&absolute_path(source_dir.clone()), Path::new("../outside")) { + Ok(path) => panic!( + "join should reject parent traversal, got {}", + path.display() + ), + Err(err) => err, + }; + assert_eq!(join_error.kind(), std::io::ErrorKind::InvalidInput); let canonical_nested = file_system .canonicalize(&absolute_path(source_link.join("nested").join("note.txt"))) .with_context(|| format!("mode={use_remote}"))?; diff --git a/codex-rs/file-system/src/lib.rs b/codex-rs/file-system/src/lib.rs index 1d7ef33d3c..fd10908887 100644 --- a/codex-rs/file-system/src/lib.rs +++ b/codex-rs/file-system/src/lib.rs @@ -144,6 +144,33 @@ pub trait ExecutorFileSystem: Send + Sync { )) } + /// Lexically joins a relative path onto an existing bound path when + /// supported. + /// + /// Implementations that cannot join bound paths may return + /// [`io::ErrorKind::Unsupported`]. + fn join( + &self, + _base_path: &AbsolutePathBuf, + _relative_path: &Path, + ) -> FileSystemResult { + Err(io::Error::new( + io::ErrorKind::Unsupported, + "filesystem does not support joining bound paths", + )) + } + + /// Returns the parent directory of a bound path when supported. + /// + /// Implementations that cannot inspect bound paths may return + /// [`io::ErrorKind::Unsupported`]. + fn parent(&self, _path: &AbsolutePathBuf) -> FileSystemResult> { + Err(io::Error::new( + io::ErrorKind::Unsupported, + "filesystem does not support parent lookup for bound paths", + )) + } + async fn read_file( &self, path: &AbsolutePathBuf,