openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-05 15:18:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

exec-server: make bound path operations async

Browse Source
This commit is contained in:
starr-openai
2026-05-29 14:06:44 -07:00
parent 385010e276
commit 111adfbdb6
12 changed files with 306 additions and 269 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
codex-rs/config/src/loader/tests.rs
View File

@@ -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<AbsolutePathBuf> {
path.canonicalize()
}
async fn join(
&self,
base_path: &AbsolutePathBuf,
path: &Path,
) -> FileSystemResult<AbsolutePathBuf> {
Ok(base_path.join(path))
}
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
Ok(path.parent())
}
async fn read_file(
&self,
path: &AbsolutePathBuf,

4
codex-rs/exec-server/src/client.rs
View File

@@ -214,10 +214,6 @@ impl LazyRemoteExecServerClient {
}
}
pub(crate) fn new_connection_client(&self) -> Self {
Self::new(self.transport_params.clone())
}
pub(crate) async fn get(&self) -> Result<ExecServerClient, ExecServerError> {
if let Some(client) = self.connected_client() {
return Ok(client);

143
codex-rs/exec-server/src/environment_path.rs
View File

@@ -62,26 +62,28 @@ impl EnvironmentPathRef {
Self::new(Arc::clone(&self.file_system), path)
}
pub fn join_relative(&self, relative_path: &Path) -> Option<Self> {
pub async fn join<P: AsRef<Path>>(&self, path: P) -> io::Result<Self> {
self.file_system
.join(&self.path, relative_path)
.ok()
.join(&self.path, path.as_ref())
.await
.map(|path| self.with_path(path))
}
pub fn parent_dir(&self) -> Option<Self> {
pub async fn parent(&self) -> io::Result<Option<Self>> {
self.file_system
.parent(&self.path)
.ok()
.flatten()
.map(|path| self.with_path(path))
.await
.map(|path| path.map(|path| self.with_path(path)))
}
/// Best-effort resolves this path through its bound filesystem.
pub fn canonicalize_if_exists(&self) -> Self {
pub async fn canonicalize(
&self,
sandbox: Option<&FileSystemSandboxContext>,
) -> io::Result<Self> {
self.file_system
.canonicalize(&self.path)
.map_or_else(|_| self.clone(), |path| self.with_path(path))
.canonicalize(&self.path, sandbox)
.await
.map(|path| self.with_path(path))
}
}
@@ -122,6 +124,8 @@ mod tests {
use std::path::Path;
use std::sync::Mutex;
use crate::LOCAL_FS;
#[derive(Clone, Debug, Eq, PartialEq)]
enum RecordedMethod {
Canonicalize,
@@ -141,26 +145,17 @@ mod tests {
struct RecordingFileSystem {
calls: Mutex<Vec<RecordedCall>>,
canonicalize_supported: bool,
}
impl Default for RecordingFileSystem {
fn default() -> Self {
Self {
calls: Mutex::new(Vec::new()),
canonicalize_supported: true,
}
}
}
impl RecordingFileSystem {
fn without_canonicalize() -> Self {
Self {
calls: Mutex::new(Vec::new()),
canonicalize_supported: false,
}
}
fn recorded_calls(&self) -> Vec<RecordedCall> {
match self.calls.lock() {
Ok(calls) => calls.clone(),
@@ -176,37 +171,39 @@ mod tests {
}
}
fn local_path_ref(path: AbsolutePathBuf) -> EnvironmentPathRef {
EnvironmentPathRef::new(Arc::clone(&LOCAL_FS), path)
}
#[async_trait]
impl ExecutorFileSystem for RecordingFileSystem {
fn canonicalize(&self, path: &AbsolutePathBuf) -> io::Result<AbsolutePathBuf> {
async fn canonicalize(
&self,
path: &AbsolutePathBuf,
_sandbox: Option<&FileSystemSandboxContext>,
) -> io::Result<AbsolutePathBuf> {
self.push_call(RecordedCall {
method: RecordedMethod::Canonicalize,
path: path.clone(),
sandbox: None,
});
if !self.canonicalize_supported {
return Err(io::Error::new(
io::ErrorKind::Unsupported,
"filesystem does not support canonicalization",
));
}
Ok(path.parent().unwrap())
}
fn join(
async fn join(
&self,
base_path: &AbsolutePathBuf,
relative_path: &Path,
path: &Path,
) -> io::Result<AbsolutePathBuf> {
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))
AbsolutePathBuf::from_absolute_path_checked(base_path.as_path().join(path))
}
fn parent(&self, path: &AbsolutePathBuf) -> io::Result<Option<AbsolutePathBuf>> {
async fn parent(&self, path: &AbsolutePathBuf) -> io::Result<Option<AbsolutePathBuf>> {
self.push_call(RecordedCall {
method: RecordedMethod::Parent,
path: path.clone(),
@@ -391,13 +388,16 @@ mod tests {
let set = HashSet::from([left, same, different_path, different_fs]);
assert_eq!(set.len(), 3);
}
#[test]
fn canonicalize_if_exists_keeps_bound_file_system_identity() {
#[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_if_exists();
let canonicalized = path_ref
.canonicalize(/*sandbox*/ None)
.await
.expect("canonicalize");
assert_eq!(canonicalized.path(), &path.parent().unwrap());
assert_eq!(
@@ -414,23 +414,14 @@ mod tests {
);
}
#[test]
fn canonicalize_if_exists_keeps_unsupported_path() {
let path = std::env::temp_dir().join("skills/demo").abs();
let path_ref =
EnvironmentPathRef::new(Arc::new(RecordingFileSystem::without_canonicalize()), path);
assert_eq!(path_ref.canonicalize_if_exists(), path_ref);
}
#[test]
fn join_relative_keeps_bound_file_system_identity() {
#[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_relative(Path::new("demo")),
path_ref.join(Path::new("demo")).await.ok(),
Some(EnvironmentPathRef::new(
file_system.clone(),
std::env::temp_dir().join("skills/demo").abs(),
@@ -446,33 +437,43 @@ mod tests {
);
}
#[test]
fn join_relative_keeps_literal_tilde_under_bound_path() {
let path_ref = EnvironmentPathRef::local(std::env::temp_dir().join("skills").abs());
#[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_relative(Path::new("~"))
.join(Path::new("~"))
.await
.ok()
.map(|path_ref| path_ref.path().clone()),
Some(std::env::temp_dir().join("skills/~").abs())
Some(path_ref.path().join(Path::new("~")))
);
}
#[test]
fn join_relative_rejects_parent_dirs() {
let path_ref = EnvironmentPathRef::local(std::env::temp_dir().join("skills").abs());
#[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_relative(Path::new("../outside")), None);
assert_eq!(
path_ref
.join(Path::new("../outside"))
.await
.expect("join")
.path()
.clone(),
path_ref.path().join(Path::new("../outside"))
);
}
#[test]
fn parent_dir_keeps_bound_file_system_identity() {
#[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_dir(),
path_ref.parent().await.expect("parent"),
Some(EnvironmentPathRef::new(
file_system.clone(),
std::env::temp_dir().join("skills").abs(),
@@ -489,11 +490,27 @@ mod tests {
}
#[cfg(windows)]
#[test]
fn join_relative_rejects_windows_prefixed_and_rooted_paths() {
let path_ref = EnvironmentPathRef::local(std::env::temp_dir().join("skills").abs());
#[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_relative(Path::new(r"C:temp")), None);
assert_eq!(path_ref.join_relative(Path::new(r"\temp")), None);
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"))
);
}
}

27
codex-rs/exec-server/src/fs_helper.rs
View File

@@ -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<FsCanonicalizeResponse, JSONRPCErrorError> {
match self {
Self::Canonicalize(response) => Ok(response),
other => Err(unexpected_response(
FS_CANONICALIZE_METHOD,
other.operation(),
)),
}
}
pub(crate) fn expect_read_directory(
self,
) -> Result<FsReadDirectoryResponse, JSONRPCErrorError> {
@@ -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(&params.path, /*sandbox*/ None)
.await
.map_err(map_fs_error)?;
Ok(FsHelperPayload::Canonicalize(FsCanonicalizeResponse {
path,
}))
}
FsHelperRequest::ReadDirectory(params) => {
let entries = file_system
.read_directory(&params.path, /*sandbox*/ None)

6
codex-rs/exec-server/src/lib.rs
View File

@@ -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;

78
codex-rs/exec-server/src/local_file_system.rs
View File

@@ -1,6 +1,5 @@
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;
@@ -80,20 +79,25 @@ impl LocalFileSystem {
#[async_trait]
impl ExecutorFileSystem for LocalFileSystem {
fn canonicalize(&self, path: &AbsolutePathBuf) -> FileSystemResult<AbsolutePathBuf> {
self.unsandboxed.canonicalize(path)
async fn canonicalize(
&self,
path: &AbsolutePathBuf,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<AbsolutePathBuf> {
let (file_system, sandbox) = self.file_system_for(sandbox)?;
file_system.canonicalize(path, sandbox).await
}
fn join(
async fn join(
&self,
base_path: &AbsolutePathBuf,
relative_path: &Path,
path: &Path,
) -> FileSystemResult<AbsolutePathBuf> {
self.unsandboxed.join(base_path, relative_path)
self.unsandboxed.join(base_path, path).await
}
fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
self.unsandboxed.parent(path)
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
self.unsandboxed.parent(path).await
}
async fn read_file(
@@ -169,20 +173,25 @@ impl ExecutorFileSystem for LocalFileSystem {
#[async_trait]
impl ExecutorFileSystem for UnsandboxedFileSystem {
fn canonicalize(&self, path: &AbsolutePathBuf) -> FileSystemResult<AbsolutePathBuf> {
self.file_system.canonicalize(path)
async fn canonicalize(
&self,
path: &AbsolutePathBuf,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<AbsolutePathBuf> {
reject_platform_sandbox_context(sandbox)?;
self.file_system.canonicalize(path, /*sandbox*/ None).await
}
fn join(
async fn join(
&self,
base_path: &AbsolutePathBuf,
relative_path: &Path,
path: &Path,
) -> FileSystemResult<AbsolutePathBuf> {
self.file_system.join(base_path, relative_path)
self.file_system.join(base_path, path).await
}
fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
self.file_system.parent(path)
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
self.file_system.parent(path).await
}
async fn read_file(
@@ -271,19 +280,24 @@ impl ExecutorFileSystem for UnsandboxedFileSystem {
#[async_trait]
impl ExecutorFileSystem for DirectFileSystem {
fn canonicalize(&self, path: &AbsolutePathBuf) -> FileSystemResult<AbsolutePathBuf> {
path.canonicalize()
async fn canonicalize(
&self,
path: &AbsolutePathBuf,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<AbsolutePathBuf> {
reject_sandbox_context(sandbox)?;
AbsolutePathBuf::from_absolute_path(tokio::fs::canonicalize(path.as_path()).await?)
}
fn join(
async fn join(
&self,
base_path: &AbsolutePathBuf,
relative_path: &Path,
path: &Path,
) -> FileSystemResult<AbsolutePathBuf> {
join_bound_path(base_path, relative_path)
Ok(base_path.join(path))
}
fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
Ok(path.parent())
}
@@ -456,28 +470,6 @@ fn reject_sandbox_context(sandbox: Option<&FileSystemSandboxContext>) -> io::Res
Ok(())
}
fn join_bound_path(
base_path: &AbsolutePathBuf,
relative_path: &Path,
) -> FileSystemResult<AbsolutePathBuf> {
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(

3
codex-rs/exec-server/src/protocol.rs
View File

@@ -218,6 +218,7 @@ pub struct FsGetMetadataResponse {
#[serde(rename_all = "camelCase")]
pub struct FsCanonicalizeParams {
pub path: AbsolutePathBuf,
pub sandbox: Option<FileSystemSandboxContext>,
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
@@ -230,7 +231,7 @@ pub struct FsCanonicalizeResponse {
#[serde(rename_all = "camelCase")]
pub struct FsJoinParams {
pub base_path: AbsolutePathBuf,
pub relative_path: PathBuf,
pub path: PathBuf,
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]

152
codex-rs/exec-server/src/remote_file_system.rs
View File

@@ -2,13 +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::sync::OnceLock;
use std::thread;
use tokio::io;
use tokio::runtime::Builder;
use tokio::runtime::Runtime;
use tracing::trace;
use crate::CopyOptions;
@@ -20,7 +15,6 @@ 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;
@@ -37,124 +31,60 @@ const INVALID_REQUEST_ERROR_CODE: i64 = -32600;
const NOT_FOUND_ERROR_CODE: i64 = -32004;
pub(crate) struct RemoteFileSystem {
async_client: LazyRemoteExecServerClient,
sync_client: LazyRemoteExecServerClient,
sync_call_runtime: SyncCallRuntime,
client: LazyRemoteExecServerClient,
}
impl RemoteFileSystem {
pub(crate) fn new(client: LazyRemoteExecServerClient) -> Self {
trace!("remote fs new");
Self {
async_client: client,
sync_client: client.new_connection_client(),
sync_call_runtime: SyncCallRuntime::default(),
}
}
fn run_sync_remote_call<T, F, Fut>(
&self,
operation: &'static str,
call: F,
) -> FileSystemResult<T>
where
T: Send + 'static,
F: FnOnce(ExecServerClient) -> Fut + Send + 'static,
Fut: Future<Output = FileSystemResult<T>> + Send + 'static,
{
let runtime = self.sync_call_runtime.get()?;
let client = self.sync_client.clone();
// ExecutorFileSystem exposes synchronous path operations, so remote callers have to hop
// through a dedicated exec-server client/runtime from a helper thread. Reusing the caller
// runtime can deadlock current-thread runtimes because the JSON-RPC reader also needs that
// runtime to make progress.
thread::scope(|scope| {
scope
.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")))?
}
}
#[derive(Default)]
struct SyncCallRuntime {
runtime: OnceLock<Result<Runtime, String>>,
}
impl SyncCallRuntime {
fn get(&self) -> FileSystemResult<&Runtime> {
self.runtime
.get_or_init(|| {
Builder::new_multi_thread()
.worker_threads(1)
.thread_name("codex-remote-fs")
.enable_all()
.build()
.map_err(|err| err.to_string())
})
.as_ref()
.map_err(|err| io::Error::other(format!("failed to start remote fs runtime: {err}")))
}
}
impl Drop for SyncCallRuntime {
fn drop(&mut self) {
if let Some(Ok(runtime)) = self.runtime.take() {
runtime.shutdown_background();
}
Self { client }
}
}
#[async_trait]
impl ExecutorFileSystem for RemoteFileSystem {
fn canonicalize(&self, path: &AbsolutePathBuf) -> FileSystemResult<AbsolutePathBuf> {
async fn canonicalize(
&self,
path: &AbsolutePathBuf,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<AbsolutePathBuf> {
trace!("remote fs canonicalize");
let path = path.clone();
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)
})
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)
}
fn join(
async fn join(
&self,
base_path: &AbsolutePathBuf,
relative_path: &Path,
path: &Path,
) -> FileSystemResult<AbsolutePathBuf> {
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)
})
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)
}
fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
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)
})
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(
@@ -163,7 +93,7 @@ impl ExecutorFileSystem for RemoteFileSystem {
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<Vec<u8>> {
trace!("remote fs read_file");
let client = self.async_client.get().await.map_err(map_remote_error)?;
let client = self.client.get().await.map_err(map_remote_error)?;
let response = client
.fs_read_file(FsReadFileParams {
path: path.clone(),
@@ -186,7 +116,7 @@ impl ExecutorFileSystem for RemoteFileSystem {
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
trace!("remote fs write_file");
let client = self.async_client.get().await.map_err(map_remote_error)?;
let client = self.client.get().await.map_err(map_remote_error)?;
client
.fs_write_file(FsWriteFileParams {
path: path.clone(),
@@ -205,7 +135,7 @@ impl ExecutorFileSystem for RemoteFileSystem {
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
trace!("remote fs create_directory");
let client = self.async_client.get().await.map_err(map_remote_error)?;
let client = self.client.get().await.map_err(map_remote_error)?;
client
.fs_create_directory(FsCreateDirectoryParams {
path: path.clone(),
@@ -223,7 +153,7 @@ impl ExecutorFileSystem for RemoteFileSystem {
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<FileMetadata> {
trace!("remote fs get_metadata");
let client = self.async_client.get().await.map_err(map_remote_error)?;
let client = self.client.get().await.map_err(map_remote_error)?;
let response = client
.fs_get_metadata(FsGetMetadataParams {
path: path.clone(),
@@ -246,7 +176,7 @@ impl ExecutorFileSystem for RemoteFileSystem {
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<Vec<ReadDirectoryEntry>> {
trace!("remote fs read_directory");
let client = self.async_client.get().await.map_err(map_remote_error)?;
let client = self.client.get().await.map_err(map_remote_error)?;
let response = client
.fs_read_directory(FsReadDirectoryParams {
path: path.clone(),
@@ -272,7 +202,7 @@ impl ExecutorFileSystem for RemoteFileSystem {
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
trace!("remote fs remove");
let client = self.async_client.get().await.map_err(map_remote_error)?;
let client = self.client.get().await.map_err(map_remote_error)?;
client
.fs_remove(FsRemoveParams {
path: path.clone(),
@@ -293,7 +223,7 @@ impl ExecutorFileSystem for RemoteFileSystem {
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
trace!("remote fs copy");
let client = self.async_client.get().await.map_err(map_remote_error)?;
let client = self.client.get().await.map_err(map_remote_error)?;
client
.fs_copy(FsCopyParams {
source_path: source_path.clone(),

34
codex-rs/exec-server/src/sandboxed_file_system.rs
View File

@@ -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<AbsolutePathBuf> {
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<AbsolutePathBuf> {
Ok(base_path.join(path))
}
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
Ok(path.parent())
}
async fn read_file(
&self,
path: &AbsolutePathBuf,

7
codex-rs/exec-server/src/server/file_system_handler.rs
View File

@@ -118,7 +118,8 @@ impl FileSystemHandler {
) -> Result<FsCanonicalizeResponse, JSONRPCErrorError> {
let path = self
.file_system
.canonicalize(&params.path)
.canonicalize(&params.path, params.sandbox.as_ref())
.await
.map_err(map_fs_error)?;
Ok(FsCanonicalizeResponse { path })
}
@@ -129,7 +130,8 @@ impl FileSystemHandler {
) -> Result<FsJoinResponse, JSONRPCErrorError> {
let path = self
.file_system
.join(&params.base_path, &params.relative_path)
.join(&params.base_path, &params.path)
.await
.map_err(map_fs_error)?;
Ok(FsJoinResponse { path })
}
@@ -141,6 +143,7 @@ impl FileSystemHandler {
let path = self
.file_system
.parent(&params.path)
.await
.map_err(map_fs_error)?;
Ok(FsParentResponse { path })
}

51
codex-rs/exec-server/tests/file_system.rs
View File

@@ -372,6 +372,7 @@ async fn file_system_methods_cover_surface_area(use_remote: bool) -> Result<()>
&absolute_path(source_link.clone()),
Path::new("nested/note.txt"),
)
.await
.with_context(|| format!("mode={use_remote}"))?;
assert_eq!(
joined_nested,
@@ -379,22 +380,26 @@ async fn file_system_methods_cover_surface_area(use_remote: bool) -> Result<()>
);
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 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 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")))
.canonicalize(
&absolute_path(source_link.join("nested").join("note.txt")),
/*sandbox*/ None,
)
.await
.with_context(|| format!("mode={use_remote}"))?;
assert_eq!(
canonical_nested,
@@ -568,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)]

49
codex-rs/file-system/src/lib.rs
View File

@@ -133,43 +133,22 @@ pub type FileSystemResult<T> = io::Result<T>;
/// a remote environment.
#[async_trait]
pub trait ExecutorFileSystem: Send + Sync {
/// Resolves a path within this filesystem when supported.
///
/// Implementations that cannot resolve bound paths may return
/// [`io::ErrorKind::Unsupported`].
fn canonicalize(&self, _path: &AbsolutePathBuf) -> FileSystemResult<AbsolutePathBuf> {
Err(io::Error::new(
io::ErrorKind::Unsupported,
"filesystem does not support canonicalization",
))
}
/// 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(
/// Resolves a path within this filesystem.
async fn canonicalize(
&self,
_base_path: &AbsolutePathBuf,
_relative_path: &Path,
) -> FileSystemResult<AbsolutePathBuf> {
Err(io::Error::new(
io::ErrorKind::Unsupported,
"filesystem does not support joining bound paths",
))
}
path: &AbsolutePathBuf,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<AbsolutePathBuf>;
/// 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<Option<AbsolutePathBuf>> {
Err(io::Error::new(
io::ErrorKind::Unsupported,
"filesystem does not support parent lookup for bound paths",
))
}
/// Lexically joins a path onto an existing bound path.
async fn join(
&self,
base_path: &AbsolutePathBuf,
path: &Path,
) -> FileSystemResult<AbsolutePathBuf>;
/// Returns the parent directory of a bound path.
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>>;
async fn read_file(
&self,