diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 29c1502b5c..de77ff3efc 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -1123,6 +1123,11 @@ impl Environment { Arc::clone(&self.filesystem) } + /// Borrows the shared filesystem identity without extending the environment's lifetime. + pub fn filesystem_ref(&self) -> &Arc { + &self.filesystem + } + /// Returns a filesystem view that fails instead of starting or waiting for a connection. pub fn get_filesystem_without_reconnect(&self) -> Arc { match &self.remote_client { diff --git a/codex-rs/exec-server/src/lib.rs b/codex-rs/exec-server/src/lib.rs index c01d5368fb..c512d0d035 100644 --- a/codex-rs/exec-server/src/lib.rs +++ b/codex-rs/exec-server/src/lib.rs @@ -71,10 +71,14 @@ pub use codex_exec_server_protocol::ExecutorCapabilityDiscoverySnapshot; pub use codex_exec_server_protocol::ProcessId; pub use codex_file_system::CopyOptions; pub use codex_file_system::CreateDirectoryOptions; +pub use codex_file_system::EnvironmentAccess; +pub use codex_file_system::EnvironmentAccessExt; +pub use codex_file_system::EnvironmentAccessKey; pub use codex_file_system::ExecutorFileSystem; pub use codex_file_system::ExecutorFileSystemFuture; pub use codex_file_system::FILE_READ_CHUNK_SIZE; pub use codex_file_system::FileMetadata; +pub use codex_file_system::FileSystemEnvironmentAccessor; pub use codex_file_system::FileSystemReadStream; pub use codex_file_system::FileSystemResult; pub use codex_file_system::FileSystemSandboxContext; diff --git a/codex-rs/exec-server/tests/file_system/shared.rs b/codex-rs/exec-server/tests/file_system/shared.rs index 4b3ecd0b9a..cc9553113b 100644 --- a/codex-rs/exec-server/tests/file_system/shared.rs +++ b/codex-rs/exec-server/tests/file_system/shared.rs @@ -2,11 +2,14 @@ use anyhow::Context; use anyhow::Result; use codex_exec_server::CopyOptions; use codex_exec_server::CreateDirectoryOptions; +use codex_exec_server::EnvironmentAccess; +use codex_exec_server::EnvironmentAccessExt; #[cfg(unix)] use codex_exec_server::ExecServerRuntimePaths; use codex_exec_server::ExecutorFileSystem; use codex_exec_server::FILE_READ_CHUNK_SIZE; use codex_exec_server::FileMetadata; +use codex_exec_server::FileSystemEnvironmentAccessor; use codex_exec_server::FileSystemSandboxContext; use codex_exec_server::LocalFileSystem; use codex_exec_server::ReadDirectoryEntry; @@ -34,6 +37,7 @@ use codex_utils_path_uri::PathUri; use futures::TryStreamExt; use pretty_assertions::assert_eq; use std::path::Path; +use std::sync::Arc; use tempfile::TempDir; use test_case::test_case; @@ -324,17 +328,23 @@ async fn file_system_read_file_text_returns_string( let tmp = TempDir::new()?; let file_path = tmp.path().join("note.txt"); std::fs::write(&file_path, "hello from trait")?; + let file_path = PathUri::from_host_native_path(file_path)?; let contents = file_system - .read_file_text( - &PathUri::from_host_native_path(&file_path)?, - Default::default(), - /*sandbox*/ None, - ) + .read_file_text(&file_path, Default::default(), /*sandbox*/ None) .await .with_context(|| format!("mode={implementation}"))?; assert_eq!(contents, "hello from trait"); + let accessor = FileSystemEnvironmentAccessor::unrestricted(&file_system); + let access: &dyn EnvironmentAccess = &accessor; + assert_eq!( + access + .read_file_text(&file_path, Default::default()) + .await?, + contents + ); + Ok(()) } @@ -1041,6 +1051,46 @@ async fn file_system_full_disk_read_skips_sandbox_only_for_reads() -> Result<()> Ok(()) } +/// A stream already opened under one environment remains usable after its accessor is dropped; +/// cache keys still distinguish a new filesystem or changed captured permissions. +#[test_case(FileSystemImplementation::Local ; "local")] +#[test_case(FileSystemImplementation::Remote ; "remote")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn environment_accessor_stream_outlives_permissions_snapshot( + implementation: FileSystemImplementation, +) -> Result<()> { + let context = create_file_system_context(implementation).await?; + let tmp = TempDir::new()?; + let file = tmp.path().join("note.txt"); + std::fs::write(&file, b"opened in the previous turn")?; + let cwd = PathUri::from_host_native_path(tmp.path())?; + let file = PathUri::from_host_native_path(file)?; + let sandbox = FileSystemSandboxContext::from_permission_profile( + PermissionProfile::read_only(), + cwd.clone(), + ); + + let (stream, key) = { + let access = FileSystemEnvironmentAccessor::new(&context.file_system, sandbox.clone()); + (access.read_file_stream(&file).await?, access.cache_key()) + }; + let same = FileSystemEnvironmentAccessor::new(&context.file_system, sandbox.clone()); + assert_eq!(key, same.cache_key()); + let changed = FileSystemEnvironmentAccessor::new( + &context.file_system, + FileSystemSandboxContext::from_permission_profile(PermissionProfile::Disabled, cwd), + ); + assert_ne!(key, changed.cache_key()); + let other_file_system: Arc = Arc::new(LocalFileSystem::unsandboxed()); + let replacement = FileSystemEnvironmentAccessor::new(&other_file_system, sandbox); + assert_ne!(key, replacement.cache_key()); + assert_eq!( + stream.try_collect::>().await?.concat(), + b"opened in the previous turn" + ); + Ok(()) +} + /// A read deny keeps both ordinary and streaming reads from falling back to direct access. #[tokio::test] async fn file_system_restricted_reads_require_sandbox() -> Result<()> { diff --git a/codex-rs/file-system/src/environment_accessor.rs b/codex-rs/file-system/src/environment_accessor.rs new file mode 100644 index 0000000000..159edeb446 --- /dev/null +++ b/codex-rs/file-system/src/environment_accessor.rs @@ -0,0 +1,280 @@ +//! Filesystem access bound to one environment configuration without exposing its authority. + +use crate::CopyOptions; +use crate::CreateDirectoryOptions; +use crate::ExecutorFileSystem; +use crate::ExecutorFileSystemFuture; +use crate::FileMetadata; +use crate::FileSystemReadStream; +use crate::FileSystemSandboxContext; +use crate::GetMetadataOptions; +use crate::ReadDirectoryEntry; +use crate::ReadFileOptions; +use crate::RemoveOptions; +use crate::WalkOptions; +use crate::WalkOutcome; +use crate::WriteFileOptions; +use codex_utils_path_uri::PathUri; +use std::fmt; +use std::hash::Hash; +use std::hash::Hasher; +use std::sync::Arc; +use std::sync::Weak; + +/// Filesystem operations authorized by the environment that supplied this borrowed accessor. +/// +/// Callers cannot extract the underlying filesystem or choose another sandbox. They may retain +/// returned data, cache keys, and already-opened read streams after the accessor is gone. +pub trait EnvironmentAccess: Send + Sync { + /// Identifies the filesystem and captured permissions without granting access to either. + fn cache_key(&self) -> EnvironmentAccessKey; + + fn canonicalize<'a>(&'a self, path: &'a PathUri) -> ExecutorFileSystemFuture<'a, PathUri>; + + fn read_file<'a>( + &'a self, + path: &'a PathUri, + options: ReadFileOptions, + ) -> ExecutorFileSystemFuture<'a, Vec>; + + /// Opens an owned stream; subsequent reads do not borrow or reauthorize through the accessor. + fn read_file_stream<'a>( + &'a self, + path: &'a PathUri, + ) -> ExecutorFileSystemFuture<'a, FileSystemReadStream>; + + fn write_file<'a>( + &'a self, + path: &'a PathUri, + contents: Vec, + options: WriteFileOptions, + ) -> ExecutorFileSystemFuture<'a, ()>; + + fn create_directory<'a>( + &'a self, + path: &'a PathUri, + options: CreateDirectoryOptions, + ) -> ExecutorFileSystemFuture<'a, ()>; + + fn get_metadata<'a>( + &'a self, + path: &'a PathUri, + options: GetMetadataOptions, + ) -> ExecutorFileSystemFuture<'a, FileMetadata>; + + fn read_directory<'a>( + &'a self, + path: &'a PathUri, + ) -> ExecutorFileSystemFuture<'a, Vec>; + + fn walk<'a>( + &'a self, + path: &'a PathUri, + options: WalkOptions, + ) -> ExecutorFileSystemFuture<'a, WalkOutcome>; + + fn remove<'a>( + &'a self, + path: &'a PathUri, + options: RemoveOptions, + ) -> ExecutorFileSystemFuture<'a, ()>; + + fn copy<'a>( + &'a self, + source_path: &'a PathUri, + destination_path: &'a PathUri, + options: CopyOptions, + ) -> ExecutorFileSystemFuture<'a, ()>; +} + +/// Convenience operations available to every implementation of [`EnvironmentAccess`]. +pub trait EnvironmentAccessExt: EnvironmentAccess { + fn read_file_text<'a>( + &'a self, + path: &'a PathUri, + options: ReadFileOptions, + ) -> ExecutorFileSystemFuture<'a, String>; +} + +impl EnvironmentAccessExt for T { + fn read_file_text<'a>( + &'a self, + path: &'a PathUri, + options: ReadFileOptions, + ) -> ExecutorFileSystemFuture<'a, String> { + Box::pin(async move { + let bytes = self.read_file(path, options).await?; + String::from_utf8(bytes) + .map_err(|error| std::io::Error::new(std::io::ErrorKind::InvalidData, error)) + }) + } +} + +/// Opaque identity for reusing data discovered with the same filesystem and permissions. +/// +/// Holding this key keeps neither the underlying filesystem nor an accessor alive. +#[derive(Clone)] +pub struct EnvironmentAccessKey { + file_system: Weak, + // Cache keys are cloned per root; share the permission and path lists instead of copying them. + sandbox: Option>, +} + +impl fmt::Debug for EnvironmentAccessKey { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("EnvironmentAccessKey(..)") + } +} + +impl PartialEq for EnvironmentAccessKey { + fn eq(&self, other: &Self) -> bool { + self.file_system.ptr_eq(&other.file_system) && self.sandbox == other.sandbox + } +} + +impl Eq for EnvironmentAccessKey {} + +impl Hash for EnvironmentAccessKey { + fn hash(&self, state: &mut H) { + self.file_system.as_ptr().cast::<()>().hash(state); + // Sandbox contexts compare by value but are not Hash. Collisions for different policies + // on the same filesystem are harmless; equality still compares the complete context. + self.sandbox.is_some().hash(state); + } +} + +/// Borrows a filesystem and forwards every operation with its captured sandbox configuration. +/// The underlying filesystem remains responsible for choosing sandboxed or direct access. +pub struct FileSystemEnvironmentAccessor<'environment> { + file_system: &'environment dyn ExecutorFileSystem, + key: EnvironmentAccessKey, +} + +impl<'environment> FileSystemEnvironmentAccessor<'environment> { + /// Binds operations and cache identity to the captured environment permissions. + pub fn new( + file_system: &'environment Arc, + sandbox: FileSystemSandboxContext, + ) -> Self { + Self { + file_system: file_system.as_ref(), + key: EnvironmentAccessKey { + file_system: Arc::downgrade(file_system), + sandbox: Some(Arc::new(sandbox)), + }, + } + } + + /// Bypasses the filesystem sandbox. Use only for: + /// + /// 1. Tests. + /// 2. Callers pending migration to sandboxed access. + /// 3. Codex-internal operations that should never be sandboxed and that the model cannot trigger. + /// + /// Model-turn discovery must use the accessor supplied by its selected environment. + pub fn unrestricted(file_system: &'environment Arc) -> Self { + Self { + file_system: file_system.as_ref(), + key: EnvironmentAccessKey { + file_system: Arc::downgrade(file_system), + sandbox: None, + }, + } + } +} + +impl EnvironmentAccess for FileSystemEnvironmentAccessor<'_> { + fn cache_key(&self) -> EnvironmentAccessKey { + self.key.clone() + } + + fn canonicalize<'a>(&'a self, path: &'a PathUri) -> ExecutorFileSystemFuture<'a, PathUri> { + self.file_system + .canonicalize(path, self.key.sandbox.as_deref()) + } + + fn read_file<'a>( + &'a self, + path: &'a PathUri, + options: ReadFileOptions, + ) -> ExecutorFileSystemFuture<'a, Vec> { + self.file_system + .read_file(path, options, self.key.sandbox.as_deref()) + } + + fn read_file_stream<'a>( + &'a self, + path: &'a PathUri, + ) -> ExecutorFileSystemFuture<'a, FileSystemReadStream> { + self.file_system + .read_file_stream(path, self.key.sandbox.as_deref()) + } + + fn write_file<'a>( + &'a self, + path: &'a PathUri, + contents: Vec, + options: WriteFileOptions, + ) -> ExecutorFileSystemFuture<'a, ()> { + self.file_system + .write_file(path, contents, options, self.key.sandbox.as_deref()) + } + + fn create_directory<'a>( + &'a self, + path: &'a PathUri, + options: CreateDirectoryOptions, + ) -> ExecutorFileSystemFuture<'a, ()> { + self.file_system + .create_directory(path, options, self.key.sandbox.as_deref()) + } + + fn get_metadata<'a>( + &'a self, + path: &'a PathUri, + options: GetMetadataOptions, + ) -> ExecutorFileSystemFuture<'a, FileMetadata> { + self.file_system + .get_metadata(path, options, self.key.sandbox.as_deref()) + } + + fn read_directory<'a>( + &'a self, + path: &'a PathUri, + ) -> ExecutorFileSystemFuture<'a, Vec> { + self.file_system + .read_directory(path, self.key.sandbox.as_deref()) + } + + fn walk<'a>( + &'a self, + path: &'a PathUri, + options: WalkOptions, + ) -> ExecutorFileSystemFuture<'a, WalkOutcome> { + self.file_system + .walk(path, options, self.key.sandbox.as_deref()) + } + + fn remove<'a>( + &'a self, + path: &'a PathUri, + options: RemoveOptions, + ) -> ExecutorFileSystemFuture<'a, ()> { + self.file_system + .remove(path, options, self.key.sandbox.as_deref()) + } + + fn copy<'a>( + &'a self, + source_path: &'a PathUri, + destination_path: &'a PathUri, + options: CopyOptions, + ) -> ExecutorFileSystemFuture<'a, ()> { + self.file_system.copy( + source_path, + destination_path, + options, + self.key.sandbox.as_deref(), + ) + } +} diff --git a/codex-rs/file-system/src/lib.rs b/codex-rs/file-system/src/lib.rs index b8ddeddce2..79049a34f0 100644 --- a/codex-rs/file-system/src/lib.rs +++ b/codex-rs/file-system/src/lib.rs @@ -1,3 +1,4 @@ +mod environment_accessor; mod exec_permission_profile_serde; mod find_up; @@ -18,6 +19,10 @@ use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; use codex_utils_path_uri::LegacyAppPathString; use codex_utils_path_uri::PathUri; +pub use environment_accessor::EnvironmentAccess; +pub use environment_accessor::EnvironmentAccessExt; +pub use environment_accessor::EnvironmentAccessKey; +pub use environment_accessor::FileSystemEnvironmentAccessor; pub use find_up::FindUpErrorPolicy; pub use find_up::find_nearest_ancestor_with_markers; pub use find_up::find_nearest_native_ancestor_with_markers;