clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-05-03 10:56:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add sandbox support to filesystem APIs (#16751)

Browse Source 
## Summary
- add optional `sandboxPolicy` support to the app-server filesystem
request surface
- thread sandbox-aware filesystem options through app-server and
exec-server adapters
- enforce sandboxed read/write access in the filesystem abstraction with
focused local and remote coverage

## Validation
- `cargo test -p codex-app-server-protocol`
- `cargo test -p codex-exec-server file_system`
- `cargo test -p codex-app-server suite::v2::fs`

---------

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
starr-openai
2026-04-08 12:10:48 -07:00
committed by GitHub
parent e003f84e1e
commit f383cc980d
12 changed files with 1271 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -1,6 +1,8 @@
use async_trait::async_trait;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::path::Component;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
@@ -38,10 +40,29 @@ impl ExecutorFileSystem for LocalFileSystem {
tokio::fs::read(path.as_path()).await
}
async fn read_file_with_sandbox_policy(
&self,
path: &AbsolutePathBuf,
sandbox_policy: Option<&SandboxPolicy>,
) -> FileSystemResult<Vec<u8>> {
enforce_read_access(path, sandbox_policy)?;
self.read_file(path).await
}
async fn write_file(&self, path: &AbsolutePathBuf, contents: Vec<u8>) -> FileSystemResult<()> {
tokio::fs::write(path.as_path(), contents).await
}
async fn write_file_with_sandbox_policy(
&self,
path: &AbsolutePathBuf,
contents: Vec<u8>,
sandbox_policy: Option<&SandboxPolicy>,
) -> FileSystemResult<()> {
enforce_write_access(path, sandbox_policy)?;
self.write_file(path, contents).await
}
async fn create_directory(
&self,
path: &AbsolutePathBuf,
@@ -55,6 +76,16 @@ impl ExecutorFileSystem for LocalFileSystem {
Ok(())
}
async fn create_directory_with_sandbox_policy(
&self,
path: &AbsolutePathBuf,
create_directory_options: CreateDirectoryOptions,
sandbox_policy: Option<&SandboxPolicy>,
) -> FileSystemResult<()> {
enforce_write_access(path, sandbox_policy)?;
self.create_directory(path, create_directory_options).await
}
async fn get_metadata(&self, path: &AbsolutePathBuf) -> FileSystemResult<FileMetadata> {
let metadata = tokio::fs::metadata(path.as_path()).await?;
Ok(FileMetadata {
@@ -65,6 +96,15 @@ impl ExecutorFileSystem for LocalFileSystem {
})
}
async fn get_metadata_with_sandbox_policy(
&self,
path: &AbsolutePathBuf,
sandbox_policy: Option<&SandboxPolicy>,
) -> FileSystemResult<FileMetadata> {
enforce_read_access(path, sandbox_policy)?;
self.get_metadata(path).await
}
async fn read_directory(
&self,
path: &AbsolutePathBuf,
@@ -82,6 +122,15 @@ impl ExecutorFileSystem for LocalFileSystem {
Ok(entries)
}
async fn read_directory_with_sandbox_policy(
&self,
path: &AbsolutePathBuf,
sandbox_policy: Option<&SandboxPolicy>,
) -> FileSystemResult<Vec<ReadDirectoryEntry>> {
enforce_read_access(path, sandbox_policy)?;
self.read_directory(path).await
}
async fn remove(&self, path: &AbsolutePathBuf, options: RemoveOptions) -> FileSystemResult<()> {
match tokio::fs::symlink_metadata(path.as_path()).await {
Ok(metadata) => {
@@ -102,6 +151,16 @@ impl ExecutorFileSystem for LocalFileSystem {
}
}
async fn remove_with_sandbox_policy(
&self,
path: &AbsolutePathBuf,
remove_options: RemoveOptions,
sandbox_policy: Option<&SandboxPolicy>,
) -> FileSystemResult<()> {
enforce_write_access_preserving_leaf(path, sandbox_policy)?;
self.remove(path, remove_options).await
}
async fn copy(
&self,
source_path: &AbsolutePathBuf,
@@ -152,6 +211,164 @@ impl ExecutorFileSystem for LocalFileSystem {
.await
.map_err(|err| io::Error::other(format!("filesystem task failed: {err}")))?
}
async fn copy_with_sandbox_policy(
&self,
source_path: &AbsolutePathBuf,
destination_path: &AbsolutePathBuf,
copy_options: CopyOptions,
sandbox_policy: Option<&SandboxPolicy>,
) -> FileSystemResult<()> {
enforce_copy_source_read_access(source_path, sandbox_policy)?;
enforce_write_access(destination_path, sandbox_policy)?;
self.copy(source_path, destination_path, copy_options).await
}
}
fn enforce_read_access(
path: &AbsolutePathBuf,
sandbox_policy: Option<&SandboxPolicy>,
) -> FileSystemResult<()> {
enforce_access_for_current_dir(
path,
sandbox_policy,
FileSystemSandboxPolicy::can_read_path_with_cwd,
"read",
AccessPathMode::ResolveAll,
)
}
fn enforce_write_access(
path: &AbsolutePathBuf,
sandbox_policy: Option<&SandboxPolicy>,
) -> FileSystemResult<()> {
enforce_access_for_current_dir(
path,
sandbox_policy,
FileSystemSandboxPolicy::can_write_path_with_cwd,
"write",
AccessPathMode::ResolveAll,
)
}
fn enforce_write_access_preserving_leaf(
path: &AbsolutePathBuf,
sandbox_policy: Option<&SandboxPolicy>,
) -> FileSystemResult<()> {
enforce_access_for_current_dir(
path,
sandbox_policy,
FileSystemSandboxPolicy::can_write_path_with_cwd,
"write",
AccessPathMode::PreserveLeaf,
)
}
fn enforce_copy_source_read_access(
path: &AbsolutePathBuf,
sandbox_policy: Option<&SandboxPolicy>,
) -> FileSystemResult<()> {
let path_mode = match std::fs::symlink_metadata(path.as_path()) {
Ok(metadata) if metadata.file_type().is_symlink() => AccessPathMode::PreserveLeaf,
_ => AccessPathMode::ResolveAll,
};
enforce_access_for_current_dir(
path,
sandbox_policy,
FileSystemSandboxPolicy::can_read_path_with_cwd,
"read",
path_mode,
)
}
#[cfg(all(test, unix))]
fn enforce_read_access_for_cwd(
path: &AbsolutePathBuf,
sandbox_policy: Option<&SandboxPolicy>,
sandbox_cwd: &AbsolutePathBuf,
) -> FileSystemResult<()> {
enforce_access_for_cwd(
path,
sandbox_policy,
sandbox_cwd,
FileSystemSandboxPolicy::can_read_path_with_cwd,
"read",
AccessPathMode::ResolveAll,
)
}
fn enforce_access_for_current_dir(
path: &AbsolutePathBuf,
sandbox_policy: Option<&SandboxPolicy>,
is_allowed: fn(&FileSystemSandboxPolicy, &Path, &Path) -> bool,
access_kind: &str,
path_mode: AccessPathMode,
) -> FileSystemResult<()> {
let Some(sandbox_policy) = sandbox_policy else {
return Ok(());
};
let cwd = current_sandbox_cwd()?;
enforce_access(
path,
sandbox_policy,
cwd.as_path(),
is_allowed,
access_kind,
path_mode,
)
}
#[cfg(all(test, unix))]
fn enforce_access_for_cwd(
path: &AbsolutePathBuf,
sandbox_policy: Option<&SandboxPolicy>,
sandbox_cwd: &AbsolutePathBuf,
is_allowed: fn(&FileSystemSandboxPolicy, &Path, &Path) -> bool,
access_kind: &str,
path_mode: AccessPathMode,
) -> FileSystemResult<()> {
let Some(sandbox_policy) = sandbox_policy else {
return Ok(());
};
let cwd = resolve_existing_path(sandbox_cwd.as_path())?;
enforce_access(
path,
sandbox_policy,
cwd.as_path(),
is_allowed,
access_kind,
path_mode,
)
}
fn enforce_access(
path: &AbsolutePathBuf,
sandbox_policy: &SandboxPolicy,
sandbox_cwd: &Path,
is_allowed: fn(&FileSystemSandboxPolicy, &Path, &Path) -> bool,
access_kind: &str,
path_mode: AccessPathMode,
) -> FileSystemResult<()> {
let resolved_path = resolve_path_for_access_check(path.as_path(), path_mode)?;
let file_system_policy =
canonicalize_file_system_policy_paths(FileSystemSandboxPolicy::from(sandbox_policy))?;
if is_allowed(&file_system_policy, resolved_path.as_path(), sandbox_cwd) {
Ok(())
} else {
Err(io::Error::new(
io::ErrorKind::InvalidInput,
format!(
"fs/{access_kind} is not permitted for path {}",
path.as_path().display()
),
))
}
}
#[derive(Clone, Copy)]
enum AccessPathMode {
ResolveAll,
PreserveLeaf,
}
fn copy_dir_recursive(source: &Path, target: &Path) -> io::Result<()> {
@@ -178,26 +395,30 @@ fn destination_is_same_or_descendant_of_source(
destination: &Path,
) -> io::Result<bool> {
let source = std::fs::canonicalize(source)?;
let destination = resolve_copy_destination_path(destination)?;
let destination = resolve_path_for_access_check(destination, AccessPathMode::ResolveAll)?;
Ok(destination.starts_with(&source))
}
fn resolve_copy_destination_path(path: &Path) -> io::Result<PathBuf> {
let mut normalized = PathBuf::new();
for component in path.components() {
match component {
Component::Prefix(prefix) => normalized.push(prefix.as_os_str()),
Component::RootDir => normalized.push(component.as_os_str()),
Component::CurDir => {}
Component::ParentDir => {
normalized.pop();
}
Component::Normal(part) => normalized.push(part),
}
fn resolve_path_for_access_check(path: &Path, path_mode: AccessPathMode) -> io::Result<PathBuf> {
match path_mode {
AccessPathMode::ResolveAll => resolve_existing_path(path),
AccessPathMode::PreserveLeaf => preserve_leaf_path_for_access_check(path),
}
}
fn preserve_leaf_path_for_access_check(path: &Path) -> io::Result<PathBuf> {
let Some(file_name) = path.file_name() else {
return resolve_existing_path(path);
};
let parent = path.parent().unwrap_or_else(|| Path::new("/"));
let mut resolved_parent = resolve_existing_path(parent)?;
resolved_parent.push(file_name);
Ok(resolved_parent)
}
fn resolve_existing_path(path: &Path) -> io::Result<PathBuf> {
let mut unresolved_suffix = Vec::new();
let mut existing_path = normalized.as_path();
let mut existing_path = path;
while !existing_path.exists() {
let Some(file_name) = existing_path.file_name() else {
break;
@@ -216,6 +437,33 @@ fn resolve_copy_destination_path(path: &Path) -> io::Result<PathBuf> {
Ok(resolved)
}
fn current_sandbox_cwd() -> io::Result<PathBuf> {
let cwd = std::env::current_dir()
.map_err(|err| io::Error::other(format!("failed to read current dir: {err}")))?;
resolve_existing_path(cwd.as_path())
}
fn canonicalize_file_system_policy_paths(
mut file_system_policy: FileSystemSandboxPolicy,
) -> io::Result<FileSystemSandboxPolicy> {
for entry in &mut file_system_policy.entries {
if let FileSystemPath::Path { path } = &mut entry.path {
*path = canonicalize_absolute_path(path)?;
}
}
Ok(file_system_policy)
}
fn canonicalize_absolute_path(path: &AbsolutePathBuf) -> io::Result<AbsolutePathBuf> {
let resolved = resolve_existing_path(path.as_path())?;
AbsolutePathBuf::from_absolute_path(resolved.as_path()).map_err(|err| {
io::Error::new(
io::ErrorKind::InvalidInput,
format!("path must stay absolute after canonicalization: {err}"),
)
})
}
fn copy_symlink(source: &Path, target: &Path) -> io::Result<()> {
let link_target = std::fs::read_link(source)?;
#[cfg(unix)]
@@ -257,6 +505,79 @@ fn system_time_to_unix_ms(time: SystemTime) -> i64 {
.unwrap_or(0)
}
#[cfg(all(test, unix))]
mod tests {
use super::*;
use codex_protocol::protocol::ReadOnlyAccess;
use pretty_assertions::assert_eq;
use std::os::unix::fs::symlink;
fn absolute_path(path: PathBuf) -> AbsolutePathBuf {
match AbsolutePathBuf::try_from(path) {
Ok(path) => path,
Err(err) => panic!("absolute path: {err}"),
}
}
fn read_only_sandbox_policy(readable_roots: Vec<PathBuf>) -> SandboxPolicy {
SandboxPolicy::ReadOnly {
access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: readable_roots.into_iter().map(absolute_path).collect(),
},
network_access: false,
}
}
#[test]
fn resolve_path_for_access_check_rejects_symlink_parent_dotdot_escape() -> io::Result<()> {
let temp_dir = tempfile::TempDir::new()?;
let allowed_dir = temp_dir.path().join("allowed");
let outside_dir = temp_dir.path().join("outside");
std::fs::create_dir_all(&allowed_dir)?;
std::fs::create_dir_all(&outside_dir)?;
symlink(&outside_dir, allowed_dir.join("link"))?;
let resolved = resolve_path_for_access_check(
allowed_dir
.join("link")
.join("..")
.join("secret.txt")
.as_path(),
AccessPathMode::ResolveAll,
)?;
assert_eq!(
resolved,
resolve_existing_path(temp_dir.path())?.join("secret.txt")
);
Ok(())
}
#[test]
fn enforce_read_access_uses_explicit_sandbox_cwd() -> io::Result<()> {
let temp_dir = tempfile::TempDir::new()?;
let workspace_dir = temp_dir.path().join("workspace");
let other_dir = temp_dir.path().join("other");
let note_path = workspace_dir.join("note.txt");
std::fs::create_dir_all(&workspace_dir)?;
std::fs::create_dir_all(&other_dir)?;
std::fs::write(&note_path, "hello")?;
let sandbox_policy = read_only_sandbox_policy(vec![]);
let sandbox_cwd = absolute_path(workspace_dir);
let other_cwd = absolute_path(other_dir);
let note_path = absolute_path(note_path);
enforce_read_access_for_cwd(&note_path, Some(&sandbox_policy), &sandbox_cwd)?;
let error = enforce_read_access_for_cwd(&note_path, Some(&sandbox_policy), &other_cwd)
.expect_err("read should be rejected outside provided cwd");
assert_eq!(error.kind(), io::ErrorKind::InvalidInput);
Ok(())
}
}
#[cfg(all(test, windows))]
mod tests {
use super::*;