openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-19 17:28:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Route filesystem reads and writes by their own sandbox permissions (#46122)

Browse Source 
## Why

Filesystem reads previously required a sandbox whenever writes were restricted, even with full-disk read permission. This made permitted reads depend on sandbox availability.

## What changed

- Select sandboxing independently for reads and writes, allowing full-disk reads directly while keeping restricted operations sandboxed.
- Use the executor's path convention when evaluating full-disk access, including `:slash_tmp` denials.
- Apply read-specific checks to capability discovery and skill resource reads, allowing unrestricted reads on executors without sandboxed discovery support.

## Testing

Add regression tests for direct read APIs, restricted writes and reads, executor-specific permission rules, and capability discovery without sandbox support. Update Windows tests to verify sandbox enforcement through writes.

GitOrigin-RevId: a03844bd1f0ea583bb54326683cddbfdd050119f
This commit is contained in:
Adam Perry @ OpenAI
2026-09-17 05:09:03 +00:00
committed by copyberry
parent 70e8fe1be3
commit a4ee536f01
15 changed files with 635 additions and 151 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/tools/handlers/view_image.rs
+11 -1
View File
@@ -279,6 +279,10 @@ mod tests {
use crate::tools::context::ToolInvocation;
use crate::turn_diff_tracker::TurnDiffTracker;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use core_test_support::TempDirExt;
@@ -373,7 +377,13 @@ mod tests {
panic!("primary environment should be ready");
};
environment.config_mut().permission_profile =
PermissionProfileSnapshot::legacy(PermissionProfile::read_only());
PermissionProfileSnapshot::legacy(PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new(
image_cwd.into(),
FileSystemAccessMode::Read,
)]),
NetworkSandboxPolicy::Restricted,
));
let turn = Arc::new(turn);
let result = ViewImageHandler::default()
codex-rs/exec-server/src/capability_discovery.rs
+1 -1
View File
@@ -85,7 +85,7 @@ async fn discover_root(
#[cfg(target_os = "windows")]
if sandbox.is_some_and(|context| {
context.should_run_in_sandbox() && !context.windows_sandbox_is_requested()
context.should_read_from_sandbox() && !context.windows_sandbox_is_requested()
}) {
discovery.error = Some("filesystem sandbox is unavailable on this executor".to_string());
return discovery;
codex-rs/exec-server/src/environment.rs
+1 -1
View File
@@ -989,7 +989,7 @@ impl Environment {
if params.roots.iter().any(|root| {
root.sandbox
.as_ref()
.is_some_and(crate::FileSystemSandboxContext::should_run_in_sandbox)
.is_some_and(crate::FileSystemSandboxContext::should_read_from_sandbox)
}) && !client
.environment_info()
.await?
codex-rs/exec-server/src/local_file_system.rs
+39 -18
View File
@@ -92,7 +92,7 @@ impl LocalFileSystem {
})
}
fn file_system_for<'a>(
fn file_system_for_reads<'a>(
&'a self,
sandbox: Option<&'a FileSystemSandboxContext>,
) -> io::Result<(
@@ -102,10 +102,27 @@ impl LocalFileSystem {
if let Some(sandbox) = sandbox {
sandbox.validate_file_system_paths_for_current_host()?;
}
if sandbox.is_some_and(FileSystemSandboxContext::should_run_in_sandbox) {
if sandbox.is_some_and(FileSystemSandboxContext::should_read_from_sandbox) {
Ok((self.sandboxed()?, sandbox))
} else {
Ok((&self.unsandboxed, sandbox))
Ok((&self.unsandboxed, None))
}
}
fn file_system_for_writes<'a>(
&'a self,
sandbox: Option<&'a FileSystemSandboxContext>,
) -> io::Result<(
&'a dyn ExecutorFileSystem,
Option<&'a FileSystemSandboxContext>,
)> {
if let Some(sandbox) = sandbox {
sandbox.validate_file_system_paths_for_current_host()?;
}
if sandbox.is_some_and(FileSystemSandboxContext::should_write_into_sandbox) {
Ok((self.sandboxed()?, sandbox))
} else {
Ok((&self.unsandboxed, None))
}
}
}
@@ -119,10 +136,12 @@ impl LocalFileSystem {
if let Some(sandbox) = sandbox {
sandbox.validate_file_system_paths_for_current_host()?;
}
if sandbox.is_some_and(FileSystemSandboxContext::should_run_in_sandbox) {
if sandbox.is_some_and(FileSystemSandboxContext::should_read_from_sandbox) {
return self.sandboxed()?.open_file_for_read(path, sandbox).await;
}
self.unsandboxed.open_file_for_read(path, sandbox).await
self.unsandboxed
.open_file_for_read(path, /*sandbox*/ None)
.await
}
async fn canonicalize(
@@ -130,14 +149,14 @@ impl LocalFileSystem {
path: &PathUri,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<PathUri> {
let (file_system, sandbox) = self.file_system_for(sandbox)?;
let (file_system, sandbox) = self.file_system_for_reads(sandbox)?;
file_system.canonicalize(path, sandbox).await
}
#[tracing::instrument(
name = "fs.read_file",
skip_all,
fields(sandboxed = sandbox.is_some_and(FileSystemSandboxContext::should_run_in_sandbox))
fields(sandboxed = sandbox.is_some_and(FileSystemSandboxContext::should_read_from_sandbox))
)]
async fn read_file(
&self,
@@ -145,7 +164,7 @@ impl LocalFileSystem {
options: ReadFileOptions,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<Vec<u8>> {
let (file_system, sandbox) = self.file_system_for(sandbox)?;
let (file_system, sandbox) = self.file_system_for_reads(sandbox)?;
file_system.read_file(path, options, sandbox).await
}
@@ -154,7 +173,7 @@ impl LocalFileSystem {
path: &PathUri,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<FileSystemReadStream> {
let (file_system, sandbox) = self.file_system_for(sandbox)?;
let (file_system, sandbox) = self.file_system_for_reads(sandbox)?;
file_system.read_file_stream(path, sandbox).await
}
@@ -165,7 +184,7 @@ impl LocalFileSystem {
options: WriteFileOptions,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
let (file_system, sandbox) = self.file_system_for(sandbox)?;
let (file_system, sandbox) = self.file_system_for_writes(sandbox)?;
file_system
.write_file(path, contents, options, sandbox)
.await
@@ -177,14 +196,14 @@ impl LocalFileSystem {
options: CreateDirectoryOptions,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
let (file_system, sandbox) = self.file_system_for(sandbox)?;
let (file_system, sandbox) = self.file_system_for_writes(sandbox)?;
file_system.create_directory(path, options, sandbox).await
}
#[tracing::instrument(
name = "fs.get_metadata",
skip_all,
fields(sandboxed = sandbox.is_some_and(FileSystemSandboxContext::should_run_in_sandbox))
fields(sandboxed = sandbox.is_some_and(FileSystemSandboxContext::should_read_from_sandbox))
)]
async fn get_metadata(
&self,
@@ -192,7 +211,7 @@ impl LocalFileSystem {
options: GetMetadataOptions,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<FileMetadata> {
let (file_system, sandbox) = self.file_system_for(sandbox)?;
let (file_system, sandbox) = self.file_system_for_reads(sandbox)?;
file_system.get_metadata(path, options, sandbox).await
}
@@ -201,7 +220,7 @@ impl LocalFileSystem {
path: &PathUri,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<Vec<ReadDirectoryEntry>> {
let (file_system, sandbox) = self.file_system_for(sandbox)?;
let (file_system, sandbox) = self.file_system_for_reads(sandbox)?;
file_system.read_directory(path, sandbox).await
}
@@ -211,7 +230,7 @@ impl LocalFileSystem {
options: WalkOptions,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<WalkOutcome> {
let (file_system, sandbox) = self.file_system_for(sandbox)?;
let (file_system, sandbox) = self.file_system_for_reads(sandbox)?;
file_system.walk(path, options, sandbox).await
}
@@ -221,7 +240,7 @@ impl LocalFileSystem {
options: RemoveOptions,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
let (file_system, sandbox) = self.file_system_for(sandbox)?;
let (file_system, sandbox) = self.file_system_for_writes(sandbox)?;
file_system.remove(path, options, sandbox).await
}
@@ -232,7 +251,7 @@ impl LocalFileSystem {
options: CopyOptions,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
let (file_system, sandbox) = self.file_system_for(sandbox)?;
let (file_system, sandbox) = self.file_system_for_writes(sandbox)?;
file_system
.copy(source_path, destination_path, options, sandbox)
.await
@@ -1173,7 +1192,9 @@ fn file_metadata(metadata: std::fs::Metadata, is_symlink: bool) -> FileMetadata
}
fn reject_platform_sandbox_context(sandbox: Option<&FileSystemSandboxContext>) -> io::Result<()> {
if sandbox.is_some_and(FileSystemSandboxContext::should_run_in_sandbox) {
if sandbox.is_some_and(|context| {
context.should_read_from_sandbox() || context.should_write_into_sandbox()
}) {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
"sandboxed filesystem operations require configured runtime paths",
codex-rs/exec-server/src/remote_file_system.rs
+9 -75
View File
@@ -68,7 +68,7 @@ impl RemoteFileSystem {
let response = client
.fs_canonicalize(FsCanonicalizeParams {
path: path.clone(),
sandbox: remote_sandbox_context(sandbox),
sandbox: sandbox.cloned(),
})
.await
.map_err(map_remote_error)?;
@@ -87,7 +87,7 @@ impl RemoteFileSystem {
.fs_read_file(FsReadFileParams {
path: path.clone(),
follow_symlinks: (!options.follow_symlinks).then_some(false),
sandbox: remote_sandbox_context(sandbox),
sandbox: sandbox.cloned(),
})
.await
.map_err(map_remote_error)?;
@@ -123,7 +123,7 @@ impl RemoteFileSystem {
path: path.clone(),
data_base64: STANDARD.encode(contents),
follow_symlinks: (!options.follow_symlinks).then_some(false),
sandbox: remote_sandbox_context(sandbox),
sandbox: sandbox.cloned(),
})
.await;
self.metadata_requests.lock().await.clear();
@@ -144,7 +144,7 @@ impl RemoteFileSystem {
path: path.clone(),
recursive: Some(options.recursive),
follow_symlinks: (!options.follow_symlinks).then_some(false),
sandbox: remote_sandbox_context(sandbox),
sandbox: sandbox.cloned(),
})
.await;
self.metadata_requests.lock().await.clear();
@@ -204,7 +204,7 @@ impl RemoteFileSystem {
.fs_get_metadata(FsGetMetadataParams {
path: path.clone(),
follow_symlinks: (!options.follow_symlinks).then_some(false),
sandbox: remote_sandbox_context(sandbox),
sandbox: sandbox.cloned(),
})
.await
.map_err(map_remote_error)?;
@@ -228,7 +228,7 @@ impl RemoteFileSystem {
let response = client
.fs_read_directory(FsReadDirectoryParams {
path: path.clone(),
sandbox: remote_sandbox_context(sandbox),
sandbox: sandbox.cloned(),
})
.await
.map_err(map_remote_error)?;
@@ -255,7 +255,7 @@ impl RemoteFileSystem {
.fs_walk(FsWalkParams {
path: path.clone(),
options,
sandbox: remote_sandbox_context(sandbox),
sandbox: sandbox.cloned(),
})
.await
.map_err(map_remote_error)
@@ -275,7 +275,7 @@ impl RemoteFileSystem {
recursive: Some(options.recursive),
force: Some(options.force),
follow_symlinks: (!options.follow_symlinks).then_some(false),
sandbox: remote_sandbox_context(sandbox),
sandbox: sandbox.cloned(),
})
.await;
self.metadata_requests.lock().await.clear();
@@ -297,7 +297,7 @@ impl RemoteFileSystem {
source_path: source_path.clone(),
destination_path: destination_path.clone(),
recursive: options.recursive,
sandbox: remote_sandbox_context(sandbox),
sandbox: sandbox.cloned(),
})
.await;
self.metadata_requests.lock().await.clear();
@@ -407,12 +407,6 @@ impl ExecutorFileSystem for RemoteFileSystem {
}
}
fn remote_sandbox_context(
sandbox: Option<&FileSystemSandboxContext>,
) -> Option<FileSystemSandboxContext> {
sandbox.cloned()
}
fn map_remote_error(error: ExecServerError) -> io::Error {
match error {
ExecServerError::Server { code, message } if code == NOT_FOUND_ERROR_CODE => {
@@ -435,61 +429,10 @@ mod path_uri_tests;
#[cfg(test)]
mod tests {
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use pretty_assertions::assert_eq;
use super::*;
#[test]
fn remote_sandbox_context_preserves_cwd_for_absolute_permissions() {
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: absolute_test_path("remote-root").into(),
},
access: FileSystemAccessMode::Read,
missing_path_behavior: None,
}]);
let permissions =
PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted);
let cwd = path_uri("executor-checkout");
let sandbox_context =
FileSystemSandboxContext::from_permission_profile(permissions, cwd.clone());
let remote_context =
remote_sandbox_context(Some(&sandbox_context)).expect("remote sandbox context");
assert_eq!(remote_context.cwd, cwd);
}
#[test]
fn remote_sandbox_context_preserves_required_cwd() {
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
}]);
let permissions =
PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted);
let cwd = path_uri("host-checkout");
let sandbox_context =
FileSystemSandboxContext::from_permission_profile(permissions, cwd.clone());
let remote_context =
remote_sandbox_context(Some(&sandbox_context)).expect("remote sandbox context");
assert_eq!(remote_context.cwd, cwd);
}
#[test]
fn transport_errors_map_to_broken_pipe() {
let errors = [
@@ -519,13 +462,4 @@ mod tests {
]
);
}
fn absolute_test_path(name: &str) -> AbsolutePathBuf {
let path = std::env::temp_dir().join(name);
AbsolutePathBuf::from_absolute_path(&path).expect("absolute path")
}
fn path_uri(name: &str) -> PathUri {
PathUri::from_abs_path(&absolute_test_path(name))
}
}
codex-rs/exec-server/src/sandboxed_file_system.rs
+1 -1
View File
@@ -444,7 +444,7 @@ fn require_platform_sandbox(
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<&FileSystemSandboxContext> {
sandbox
.filter(|sandbox| sandbox.should_run_in_sandbox())
.filter(|sandbox| sandbox.should_read_from_sandbox() || sandbox.should_write_into_sandbox())
.ok_or_else(|| {
io::Error::new(
io::ErrorKind::InvalidInput,
codex-rs/exec-server/src/sandboxed_file_system_path_uri_tests.rs
+116
View File
@@ -11,6 +11,122 @@ use tokio::io;
use super::*;
/// Reads and writes follow their own policies, and temporary rules follow the executor's OS.
#[test]
fn sandbox_routing_uses_independent_read_write_policies_and_executor_temp_rules() {
use FileSystemAccessMode::Deny;
use FileSystemAccessMode::Read;
use FileSystemAccessMode::Write;
use FileSystemSpecialPath::Minimal;
use FileSystemSpecialPath::Root;
use FileSystemSpecialPath::SlashTmp;
use FileSystemSpecialPath::Tmpdir;
for (cwd, slash_tmp_applies, foreign_path) in [
("file:///workspace", true, "file:///C:/private"),
("file:///C:/workspace", false, "file:///private"),
] {
let cwd = PathUri::parse(cwd).expect("valid executor cwd");
for (entries, expected) in [
(vec![(Root, Read)], (false, true)),
(vec![(Root, Write)], (false, false)),
(vec![(Root, Read), (Minimal, Deny)], (true, true)),
(vec![(Root, Write), (Minimal, Deny)], (true, false)),
(
vec![(Root, Read), (SlashTmp, Deny)],
(slash_tmp_applies, true),
),
(
vec![(Root, Write), (SlashTmp, Deny)],
(slash_tmp_applies, slash_tmp_applies),
),
(vec![(Root, Read), (Tmpdir, Deny)], (true, true)),
(vec![(Root, Write), (Tmpdir, Deny)], (true, true)),
] {
let policy = FileSystemSandboxPolicy::restricted(
entries
.into_iter()
.map(|(value, access)| {
FileSystemSandboxEntry::new(FileSystemPath::Special { value }, access)
})
.collect(),
);
let context = FileSystemSandboxContext::from_permission_profile(
PermissionProfile::from_runtime_permissions(
&policy,
NetworkSandboxPolicy::Restricted,
),
cwd.clone(),
);
assert_eq!(
(
context.should_read_from_sandbox(),
context.should_write_into_sandbox()
),
expected,
"cwd={cwd}, policy={policy:?}",
);
}
let foreign = PathUri::parse(foreign_path).expect("valid foreign path");
for (entries, expected) in [
(
vec![FileSystemSandboxEntry::new(cwd.clone().into(), Read)],
(true, true),
),
(
vec![
FileSystemSandboxEntry::new(FileSystemPath::Special { value: Root }, Write),
FileSystemSandboxEntry::new(foreign.clone().into(), Deny),
],
(true, true),
),
(
vec![
FileSystemSandboxEntry::new(FileSystemPath::Special { value: Root }, Read),
FileSystemSandboxEntry::new(foreign.clone().into(), Write),
],
(false, true),
),
(
vec![
FileSystemSandboxEntry::new(FileSystemPath::Special { value: Root }, Write),
FileSystemSandboxEntry::new(foreign.into(), Write),
],
(false, false),
),
] {
let policy = FileSystemSandboxPolicy::restricted(entries);
let context = FileSystemSandboxContext::from_permission_profile(
PermissionProfile::from_runtime_permissions(
&policy,
NetworkSandboxPolicy::Restricted,
),
cwd.clone(),
);
assert_eq!(
(
context.should_read_from_sandbox(),
context.should_write_into_sandbox()
),
expected,
"cwd={cwd}, policy={policy:?}",
);
}
}
let opaque = PathUri::parse("file:///%00/bad/path/YQ").expect("valid opaque path");
let context =
FileSystemSandboxContext::from_permission_profile(PermissionProfile::read_only(), opaque);
assert_eq!(
(
context.should_read_from_sandbox(),
context.should_write_into_sandbox()
),
(false, true),
);
}
#[tokio::test]
async fn sandboxed_file_system_rejects_non_native_uri_as_invalid_input() {
let runtime_paths = ExecServerRuntimePaths::new(
codex-rs/exec-server/src/server/file_system_handler.rs
+1 -1
View File
@@ -82,7 +82,7 @@ impl FileSystemHandler {
sandbox
.validate_file_system_paths_for_current_host()
.is_ok()
&& sandbox.should_run_in_sandbox()
&& sandbox.should_read_from_sandbox()
&& (!cfg!(target_os = "windows") || sandbox.windows_sandbox_is_requested())
&& params
.roots
codex-rs/exec-server/tests/environment.rs
+140
View File
@@ -12,11 +12,19 @@ use codex_exec_server::ExecutorCapabilityDiscoveryCache;
use codex_exec_server::REMOTE_ENVIRONMENT_ID;
use codex_exec_server::SelectedCapabilityRootsStatus;
use codex_exec_server_protocol::CAPABILITY_ROOTS_DISCOVER_METHOD;
use codex_exec_server_protocol::CapabilityRootDiscoverRequest;
use codex_exec_server_protocol::CapabilityRootsDiscoverParams;
use codex_file_system::FileSystemSandboxContext;
use codex_http_client::HttpClientFactory;
use codex_http_client::OutboundProxyPolicy;
use codex_http_client::cache_system_proxy_route_for_test;
use codex_protocol::capabilities::CapabilityRootLocation;
use codex_protocol::capabilities::SelectedCapabilityRoot;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_utils_path_uri::PathUri;
use common::exec_server::exec_server;
use futures::SinkExt;
@@ -107,6 +115,138 @@ async fn prepared_remote_environment_uses_configured_system_proxy() -> anyhow::R
Ok(())
}
/// Older discovery executors receive direct-read contexts unchanged but never receive restricted reads.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn capability_discovery_without_sandbox_support_only_sends_full_disk_reads()
-> anyhow::Result<()> {
let server = exec_server().await?;
let listener = TcpListener::bind("127.0.0.1:0").await?;
let proxy_url = format!("ws://{}", listener.local_addr()?);
let upstream_url = server.websocket_url().to_string();
let (requests_tx, mut requests_rx) = tokio::sync::mpsc::unbounded_channel();
let _proxy = AbortOnDropHandle::new(tokio::spawn(async move {
let (stream, _) = listener.accept().await?;
let mut downstream = accept_async(stream).await?;
let (mut upstream, _) = connect_async(&upstream_url).await?;
loop {
tokio::select! {
message = downstream.next() => {
let Some(message) = message.transpose()? else { break };
if let Message::Text(text) = &message {
let request: serde_json::Value = serde_json::from_str(text.as_ref())?;
if request.get("method").and_then(serde_json::Value::as_str)
== Some(CAPABILITY_ROOTS_DISCOVER_METHOD)
{
requests_tx.send(request)?;
}
}
upstream.send(message).await?;
}
message = upstream.next() => {
let Some(mut message) = message.transpose()? else { break };
if let Message::Text(text) = &message {
let mut response: serde_json::Value = serde_json::from_str(text.as_ref())?;
for pointer in [
"/result/environmentInfo/capabilities/capabilityDiscoverySandbox",
"/result/capabilities/capabilityDiscoverySandbox",
] {
if let Some(capability) = response.pointer_mut(pointer) {
*capability = false.into();
}
}
message = Message::Text(response.to_string().into());
}
downstream.send(message).await?;
}
}
}
Ok::<(), anyhow::Error>(())
}));
let manager =
EnvironmentManager::create_for_tests(Some(proxy_url), /*local_runtime_paths*/ None).await;
let environment = manager
.default_environment()
.context("remote environment")?;
assert!(
!environment
.info()
.await?
.capabilities
.capability_discovery_sandbox
);
let workspace = tempfile::tempdir()?;
let cwd = PathUri::from_host_native_path(workspace.path())?;
let full_read = FileSystemSandboxContext::from_permission_profile(
PermissionProfile::read_only(),
cwd.clone(),
);
let request = CapabilityRootDiscoverRequest {
id: "root".to_string(),
path: cwd.clone(),
sandbox: Some(full_read),
};
let unrestricted = CapabilityRootDiscoverRequest {
id: "unrestricted".to_string(),
path: cwd.clone(),
sandbox: Some(FileSystemSandboxContext::from_permission_profile(
PermissionProfile::Disabled,
cwd.clone(),
)),
};
let response = environment
.discover_capability_roots(CapabilityRootsDiscoverParams {
roots: vec![request.clone(), unrestricted.clone()],
})
.await?;
assert_eq!(
response
.roots
.iter()
.map(|root| (root.id.as_str(), &root.error))
.collect::<Vec<_>>(),
vec![("root", &None), ("unrestricted", &None)]
);
let sent = timeout(Duration::from_secs(5), requests_rx.recv())
.await?
.context("discovery request")?;
let sent: CapabilityRootsDiscoverParams = serde_json::from_value(sent["params"].clone())?;
assert_eq!(sent.roots, vec![request.clone(), unrestricted]);
let restricted = FileSystemSandboxContext::from_permission_profile(
PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new(
cwd.clone().into(),
FileSystemAccessMode::Read,
)]),
NetworkSandboxPolicy::Restricted,
),
cwd,
);
let error = environment
.discover_capability_roots(CapabilityRootsDiscoverParams {
roots: vec![
request.clone(),
CapabilityRootDiscoverRequest {
sandbox: Some(restricted),
..request
},
],
})
.await
.expect_err("restricted reads must not fall back to old discovery");
assert!(
error
.to_string()
.contains("exec-server does not support sandboxed capability discovery")
);
assert_eq!(
requests_rx.try_recv(),
Err(tokio::sync::mpsc::error::TryRecvError::Empty)
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[serial_test::serial(remote_exec_server)]
async fn selected_capability_inspection_tracks_connection_recovery() -> anyhow::Result<()> {
codex-rs/exec-server/tests/file_system/shared.rs
+182 -2
View File
@@ -4,11 +4,10 @@ use codex_exec_server::CopyOptions;
use codex_exec_server::CreateDirectoryOptions;
#[cfg(unix)]
use codex_exec_server::ExecServerRuntimePaths;
#[cfg(unix)]
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::FILE_READ_CHUNK_SIZE;
use codex_exec_server::FileMetadata;
#[cfg(unix)]
use codex_exec_server::FileSystemSandboxContext;
use codex_exec_server::LocalFileSystem;
use codex_exec_server::ReadDirectoryEntry;
use codex_exec_server::RemoveOptions;
@@ -23,6 +22,12 @@ use codex_file_system::MAX_WALK_ENTRIES;
use codex_protocol::models::AdditionalPermissionProfile;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy;
use codex_sandboxing::policy_transforms::effective_network_sandbox_policy;
use codex_utils_path_uri::PathUri;
@@ -903,6 +908,181 @@ async fn file_system_sandboxed_metadata_and_read_allow_readable_root(
Ok(())
}
/// Full-disk read permission allows all read APIs without a helper; mutations still require one.
#[tokio::test]
async fn file_system_full_disk_read_skips_sandbox_only_for_reads() -> Result<()> {
let file_system = LocalFileSystem::unsandboxed();
let tmp = TempDir::new()?;
let file = tmp.path().join("note.txt");
std::fs::write(&file, b"note")?;
let file_uri = PathUri::from_host_native_path(&file)?;
let root_uri = PathUri::from_host_native_path(tmp.path())?;
let destination = PathUri::from_host_native_path(tmp.path().join("destination"))?;
let sandbox = FileSystemSandboxContext::from_permission_profile(
PermissionProfile::read_only(),
root_uri.clone(),
);
assert_eq!(
file_system.canonicalize(&file_uri, Some(&sandbox)).await?,
PathUri::from_host_native_path(std::fs::canonicalize(&file)?)?
);
assert_eq!(
file_system
.read_file(&file_uri, Default::default(), Some(&sandbox))
.await?,
b"note"
);
assert_eq!(
file_system
.read_file_stream(&file_uri, Some(&sandbox))
.await?
.try_collect::<Vec<_>>()
.await?
.concat(),
b"note"
);
let metadata = file_system
.get_metadata(&file_uri, Default::default(), Some(&sandbox))
.await?;
assert_eq!(
metadata,
FileMetadata {
is_directory: false,
is_file: true,
is_symlink: false,
size: 4,
created_at_ms: metadata.created_at_ms,
modified_at_ms: metadata.modified_at_ms,
}
);
assert_eq!(
file_system
.read_directory(&root_uri, Some(&sandbox))
.await?,
vec![ReadDirectoryEntry {
file_name: "note.txt".to_string(),
is_directory: false,
is_file: true,
}]
);
assert_eq!(
file_system
.walk(
&root_uri,
WalkOptions {
max_depth: 1,
max_directories: 2,
max_entries: 2,
follow_directory_symlinks: false,
prune_hidden_directories: false,
},
Some(&sandbox),
)
.await?,
WalkOutcome {
entries: vec![WalkEntry {
path: file_uri.clone(),
kind: WalkEntryKind::File,
}],
errors: Vec::new(),
truncated: false,
}
);
let mutations = [
file_system
.write_file(
&file_uri,
b"changed".to_vec(),
Default::default(),
Some(&sandbox),
)
.await,
file_system
.create_directory(
&destination,
CreateDirectoryOptions {
recursive: false,
follow_symlinks: true,
},
Some(&sandbox),
)
.await,
file_system
.remove(
&file_uri,
RemoveOptions {
recursive: false,
force: false,
follow_symlinks: true,
},
Some(&sandbox),
)
.await,
file_system
.copy(
&file_uri,
&destination,
CopyOptions { recursive: false },
Some(&sandbox),
)
.await,
];
for mutation in mutations {
let error = mutation.expect_err("restricted writes must require the sandbox");
assert_eq!(
error.to_string(),
"sandboxed filesystem operations require configured runtime paths"
);
}
assert_eq!(std::fs::read(&file)?, b"note");
assert!(!tmp.path().join("destination").exists());
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<()> {
let file_system = LocalFileSystem::unsandboxed();
let tmp = TempDir::new()?;
let file = tmp.path().join("note.txt");
std::fs::write(&file, b"note")?;
let file_uri = PathUri::from_host_native_path(&file)?;
let sandbox = FileSystemSandboxContext::from_permission_profile(
PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry::new(
FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
FileSystemAccessMode::Read,
),
FileSystemSandboxEntry::new(absolute_path(file).into(), FileSystemAccessMode::Deny),
]),
NetworkSandboxPolicy::Restricted,
),
PathUri::from_host_native_path(tmp.path())?,
);
let read = file_system
.read_file(&file_uri, Default::default(), Some(&sandbox))
.await
.map(|_| ());
let stream = file_system
.read_file_stream(&file_uri, Some(&sandbox))
.await
.map(|_| ());
for result in [read, stream] {
let error = result.expect_err("restricted reads must require the sandbox");
assert_eq!(
error.to_string(),
"sandboxed filesystem operations require configured runtime paths"
);
}
Ok(())
}
#[cfg(unix)]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn sandboxed_file_operations_cannot_read_helper_siblings() -> Result<()> {
codex-rs/exec-server/tests/file_system_windows.rs
+52 -37
View File
@@ -236,10 +236,20 @@ async fn file_system_operations_can_reject_junctions_in_any_path_component(
.file_system
.read_file(&uri(&existing)?, no_follow_read, Some(&sandbox))
.await;
if is_unsupported_restricted_token_host(&read_result) {
assert_eq!(read_result?, b"unchanged");
let write_result = context
.file_system
.write_file(
&uri(&existing)?,
b"unchanged".to_vec(),
no_follow_write,
Some(&sandbox),
)
.await;
if is_unsupported_restricted_token_host(&write_result) {
return Ok(());
}
assert_eq!(read_result?, b"unchanged");
write_result?;
assert!(
context
.file_system
@@ -380,6 +390,17 @@ async fn file_system_remote_fs_helper_respects_windows_sandbox_write_policy(
}
}
let readable_file = readonly_dir.join("readable.txt");
std::fs::write(&readable_file, b"readable")?;
let read_result = file_system
.read_file(
&PathUri::from_host_native_path(&readable_file)?,
ReadFileOptions::default(),
Some(&sandbox),
)
.await;
assert_eq!(read_result?, b"readable");
let blocked_file = readonly_dir.join("blocked.txt");
if sandbox_type == SandboxType::WindowsMxc && !codex_sandboxing::windows_mxc_available() {
let error = file_system
@@ -402,34 +423,23 @@ async fn file_system_remote_fs_helper_respects_windows_sandbox_write_policy(
return Ok(());
}
let readable_file = readonly_dir.join("readable.txt");
std::fs::write(&readable_file, b"readable")?;
let read_result = file_system
.read_file(
&PathUri::from_host_native_path(&readable_file)?,
ReadFileOptions::default(),
Some(&sandbox),
)
.await;
// Some local Windows hosts cannot create restricted tokens. Reaching that
// error still proves the remote fs helper went through the Windows sandbox
// launcher; before the wrapper fix this read would have run unsandboxed.
if sandbox_type == SandboxType::WindowsRestrictedToken
&& is_unsupported_restricted_token_host(&read_result)
{
return Ok(());
}
assert_eq!(read_result?, b"readable");
let error = file_system
let write_result = file_system
.write_file(
&PathUri::from_host_native_path(&blocked_file)?,
b"blocked".to_vec(),
WriteFileOptions::default(),
Some(&sandbox),
)
.await
.expect_err("write outside the sandbox should fail");
.await;
// Some local Windows hosts cannot create restricted tokens. Reaching that
// error still proves the write went through the Windows sandbox launcher.
if sandbox_type == SandboxType::WindowsRestrictedToken
&& is_unsupported_restricted_token_host(&write_result)
{
assert!(!blocked_file.exists());
return Ok(());
}
let error = write_result.expect_err("write outside the sandbox should fail");
assert!(
!blocked_file.exists(),
"sandboxed fs helper must not create blocked file after error: {error}"
@@ -562,14 +572,19 @@ async fn file_system_private_desktop_survives_helper_exits_and_separates_permiss
let mut sandbox = workspace_write_sandbox(tmp.path().to_path_buf());
sandbox.windows_sandbox_private_desktop = true;
let before = process_private_desktops()?;
let read = file_system
.read_file(&uri, ReadFileOptions::default(), Some(&sandbox))
let write = file_system
.write_file(
&uri,
b"initial".to_vec(),
WriteFileOptions::default(),
Some(&sandbox),
)
.await;
if is_unsupported_restricted_token_host(&read) {
if is_unsupported_restricted_token_host(&write) {
eprintln!("Skipping private desktop reuse: this host cannot create restricted tokens");
return Ok(());
}
assert_eq!(read?, b"initial");
write?;
// Ownership must outlive each helper; a desktop held only by the helper disappears here.
let warmed = process_private_desktops()?;
@@ -610,15 +625,6 @@ async fn file_system_private_desktop_survives_helper_exits_and_separates_permiss
let mut readonly = read_only_sandbox_for_cwd(tmp.path().to_path_buf())?;
readonly.windows_sandbox_selection = WindowsSandboxSelection::RestrictedToken;
readonly.windows_sandbox_private_desktop = true;
assert_eq!(
file_system
.read_file(&uri, ReadFileOptions::default(), Some(&readonly))
.await?,
b"updated again"
);
let separated = process_private_desktops()?;
assert!(warmed.is_subset(&separated));
assert_eq!(separated.difference(&warmed).count(), 1);
file_system
.write_file(
&uri,
@@ -629,6 +635,15 @@ async fn file_system_private_desktop_survives_helper_exits_and_separates_permiss
.await
.expect_err("read-only filesystem requests must reject writes");
assert_eq!(std::fs::read(&path)?, b"updated again");
let separated = process_private_desktops()?;
assert!(warmed.is_subset(&separated));
assert_eq!(separated.difference(&warmed).count(), 1);
assert_eq!(
file_system
.read_file(&uri, ReadFileOptions::default(), Some(&readonly))
.await?,
b"updated again"
);
assert_eq!(process_private_desktops()?, separated);
Ok(())
}
codex-rs/ext/skills/src/provider/executor.rs
+1 -1
View File
@@ -333,7 +333,7 @@ async fn read_bounded_text(
"failed to read executor skill resource {resource}: {err}"
))
};
if sandbox.is_some_and(FileSystemSandboxContext::should_run_in_sandbox)
if sandbox.is_some_and(FileSystemSandboxContext::should_read_from_sandbox)
&& path.infer_path_convention() == Some(PathConvention::Windows)
&& sandbox.is_some_and(|context| !context.windows_sandbox_is_requested())
{
codex-rs/file-system/src/lib.rs
+14 -9
View File
@@ -11,7 +11,6 @@ use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxEntryMissingPathBehavior;
use codex_protocol::permissions::FileSystemSandboxKind;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSandboxPolicyContext;
use codex_protocol::permissions::FileSystemSpecialPath;
@@ -401,14 +400,20 @@ impl FileSystemSandboxContext {
}
}
pub fn should_run_in_sandbox(&self) -> bool {
if self.validate_file_system_paths_for_current_host().is_err() {
// A sandbox context for another host must not select the unsandboxed filesystem.
return true;
}
let file_system_policy = self.permissions.file_system_sandbox_policy();
matches!(file_system_policy.kind, FileSystemSandboxKind::Restricted)
&& !file_system_policy.has_full_disk_write_access()
/// Whether filesystem reads need a platform sandbox on the selected executor.
pub fn should_read_from_sandbox(&self) -> bool {
!self
.permissions
.file_system_sandbox_policy()
.has_full_disk_read_access_for_convention(self.cwd.infer_path_convention())
}
/// Whether filesystem writes need a platform sandbox on the selected executor.
pub fn should_write_into_sandbox(&self) -> bool {
!self
.permissions
.file_system_sandbox_policy()
.has_full_disk_write_access_for_convention(self.cwd.infer_path_convention())
}
/// Whether this context selects either supported Windows sandbox implementation.
codex-rs/protocol/src/permissions.rs
+24 -4
View File
@@ -674,6 +674,14 @@ impl FileSystemSandboxPolicy {
}
pub fn has_denied_read_restrictions(&self) -> bool {
// TODO(anp) Migrate callers to select the executor's path convention explicitly.
self.has_denied_read_restrictions_for_convention(Some(PathConvention::native()))
}
fn has_denied_read_restrictions_for_convention(
&self,
convention: Option<PathConvention>,
) -> bool {
matches!(self.kind, FileSystemSandboxKind::Restricted)
&& self.entries.iter().any(|entry| {
entry.access == FileSystemAccessMode::Deny
@@ -681,7 +689,7 @@ impl FileSystemSandboxPolicy {
&entry.path,
FileSystemPath::Special {
value: FileSystemSpecialPath::SlashTmp,
} if !cfg!(unix)
} if convention == Some(PathConvention::Windows)
)
})
}
@@ -888,19 +896,30 @@ impl FileSystemSandboxPolicy {
file_system_policy
}
/// Returns true when filesystem reads are unrestricted.
/// Returns true when filesystem reads are unrestricted on this host.
pub fn has_full_disk_read_access(&self) -> bool {
// TODO(anp) Migrate callers to select the executor's path convention explicitly.
self.has_full_disk_read_access_for_convention(Some(PathConvention::native()))
}
/// Returns true when filesystem reads are unrestricted on the selected executor.
/// If the convention is unknown, a `:slash_tmp` denial is still treated as a restriction.
pub fn has_full_disk_read_access_for_convention(
&self,
convention: Option<PathConvention>,
) -> bool {
match self.kind {
FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => true,
FileSystemSandboxKind::Restricted => {
self.has_root_access(FileSystemAccessMode::can_read)
&& !self.has_denied_read_restrictions()
&& !self.has_denied_read_restrictions_for_convention(convention)
}
}
}
/// Returns true when filesystem writes are unrestricted on this host.
pub fn has_full_disk_write_access(&self) -> bool {
// TODO(anp) Migrate callers to select the executor's path convention explicitly.
self.has_full_disk_write_access_for_convention(Some(PathConvention::native()))
}
@@ -912,7 +931,8 @@ impl FileSystemSandboxPolicy {
self.has_full_disk_write_access_for_convention(context.cwd.infer_path_convention())
}
fn has_full_disk_write_access_for_convention(
/// Returns true when filesystem writes are unrestricted for the selected path convention.
pub fn has_full_disk_write_access_for_convention(
&self,
convention: Option<PathConvention>,
) -> bool {
codex-rs/protocol/src/permissions/target_tests.rs
+43
View File
@@ -231,6 +231,49 @@ fn slash_tmp_denials_follow_the_executor_convention() {
}
}
/// Full-disk read policy shares the native check and ignores `:slash_tmp` only on Windows.
#[test]
fn full_disk_read_policy_uses_the_executor_convention() {
use FileSystemAccessMode::Read;
use FileSystemAccessMode::Write;
use FileSystemSpecialPath::Minimal;
use FileSystemSpecialPath::Root;
use FileSystemSpecialPath::SlashTmp;
use FileSystemSpecialPath::Tmpdir;
for (root_access, denial, posix, windows, unknown) in [
(Read, None, true, true, true),
(Read, Some(SlashTmp), false, true, false),
(Read, Some(Tmpdir), false, false, false),
(Write, Some(Minimal), false, false, false),
(Write, Some(Root), false, false, false),
] {
let mut entries = vec![FileSystemSandboxEntry::new(
FileSystemPath::Special { value: Root },
root_access,
)];
if let Some(value) = denial {
entries.push(deny(FileSystemPath::Special { value }));
}
let policy = FileSystemSandboxPolicy::restricted(entries);
for (convention, expected) in [
(Some(PathConvention::Posix), posix),
(Some(PathConvention::Windows), windows),
(None, unknown),
] {
assert_eq!(
policy.has_full_disk_read_access_for_convention(convention),
expected,
"policy={policy:?}, convention={convention:?}",
);
}
assert_eq!(
policy.has_full_disk_read_access(),
policy.has_full_disk_read_access_for_convention(Some(PathConvention::native())),
);
}
}
#[test]
fn remote_workspace_write_keeps_metadata_protected() {
let cwd = uri("file:///C:/repo");