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
This commit is contained in:
Adam Perry @ OpenAI
2026-09-17 17:59:19 +00:00
committed by copyberry
parent 608825d511
commit 3d3ae4965a
5 changed files with 349 additions and 5 deletions
+5
View File
@@ -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<dyn ExecutorFileSystem> {
&self.filesystem
}
/// Returns a filesystem view that fails instead of starting or waiting for a connection.
pub fn get_filesystem_without_reconnect(&self) -> Arc<dyn ExecutorFileSystem> {
match &self.remote_client {
+4
View File
@@ -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;
@@ -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<dyn ExecutorFileSystem> = Arc::new(LocalFileSystem::unsandboxed());
let replacement = FileSystemEnvironmentAccessor::new(&other_file_system, sandbox);
assert_ne!(key, replacement.cache_key());
assert_eq!(
stream.try_collect::<Vec<_>>().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<()> {
@@ -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<u8>>;
/// 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<u8>,
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<ReadDirectoryEntry>>;
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<T: EnvironmentAccess + ?Sized> 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<dyn ExecutorFileSystem>,
// Cache keys are cloned per root; share the permission and path lists instead of copying them.
sandbox: Option<Arc<FileSystemSandboxContext>>,
}
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<H: Hasher>(&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<dyn ExecutorFileSystem>,
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<dyn ExecutorFileSystem>) -> 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<u8>> {
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<u8>,
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<ReadDirectoryEntry>> {
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(),
)
}
}
+5
View File
@@ -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;