From 3d3ae4965ab370217e871b3a7f0d15589557ee4b Mon Sep 17 00:00:00 2001 From: "Adam Perry @ OpenAI" Date: Thu, 17 Sep 2026 17:38:10 +0000 Subject: [PATCH] Add filesystem accessors bound to environment permissions (#46268) ## What changed Add `EnvironmentAccess` and `FileSystemEnvironmentAccessor` to expose filesystem operations with a captured sandbox configuration, without allowing consumers to extract the filesystem or select another sandbox. Include a text-reading helper and an explicit unrestricted constructor. Provide opaque cache keys that compare filesystem identity and captured permissions without keeping the filesystem alive. Allow opened read streams to outlive the accessor. Export the new APIs through `codex-exec-server` and add `Environment::filesystem_ref()` for borrowing the shared filesystem. ## Testing Add local and remote coverage for text reads through `EnvironmentAccess`, streams surviving accessor disposal, and cache keys distinguishing changed permissions or a replacement filesystem. GitOrigin-RevId: 4f6787ed9ba0fb94adea2f31716c4d3132fed07d --- codex-rs/exec-server/src/environment.rs | 5 + codex-rs/exec-server/src/lib.rs | 4 + .../exec-server/tests/file_system/shared.rs | 60 +++- .../file-system/src/environment_accessor.rs | 280 ++++++++++++++++++ codex-rs/file-system/src/lib.rs | 5 + 5 files changed, 349 insertions(+), 5 deletions(-) create mode 100644 codex-rs/file-system/src/environment_accessor.rs 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;