agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

exec-server: expose arg0 alias root to fs sandbox (#19016)

Browse Source 
## Why

The post-merge `rust-ci-full` run for #18999 still failed the Ubuntu
remote `suite::remote_env` sandboxed filesystem tests. That run checked
out merge commit `ddde50c611e4800cb805f243ed3c50bbafe7d011`, so the arg0
guard lifetime fix was present.

The Docker-backed failure had two remaining pieces:

- The sandboxed filesystem helper needs to execute Codex through the
`codex-linux-sandbox` arg0 alias path. The helper sandbox was only
granting read access to the real Codex executable parent, so the alias
parent also has to be visible inside the helper sandbox.
- The remote-env tests were building sandbox contexts with
`FileSystemSandboxContext::new()`, which captures the local test runner
cwd. In the Docker remote exec-server, that host checkout path does not
exist, so spawning the filesystem helper failed with `No such file or
directory` before the helper could process the request.

## What Changed

- Track all helper runtime read roots instead of a single root.
- Add both the real Codex executable parent and the
`codex-linux-sandbox` alias parent to sandbox readable roots.
- Avoid sending an unused local cwd in remote filesystem sandbox
contexts when the permission profile has no cwd-dependent entries.
- Build the Docker remote-env test sandbox contexts with a cwd path that
exists inside the container.
- Add unit coverage for the alias-parent root and remote sandbox cwd
handling.

## Verification

- `cargo test -p codex-exec-server`
- `cargo test -p codex-core
remote_test_env_sandboxed_read_allows_readable_root`
- `just fix -p codex-exec-server`
- `just fix -p codex-core`
This commit is contained in:
Michael Bolin
2026-04-22 14:34:22 -07:00
committed by GitHub
Unverified
parent 16eeeb534a
commit d3dd0d759b
5 changed files with 194 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/tests/suite/remote_env.rs
+27 -15
View File
@@ -59,26 +59,38 @@ fn absolute_path(path: PathBuf) -> AbsolutePathBuf {
}
fn read_only_sandbox(readable_root: PathBuf) -> FileSystemSandboxContext {
FileSystemSandboxContext::new(SandboxPolicy::ReadOnly {
access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: vec![absolute_path(readable_root)],
let readable_root = absolute_path(readable_root);
// The policy is evaluated in the remote container, so use a container path
// for cwd instead of capturing the local test runner cwd.
FileSystemSandboxContext::from_legacy_sandbox_policy(
SandboxPolicy::ReadOnly {
access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: vec![readable_root.clone()],
},
network_access: false,
},
network_access: false,
})
readable_root,
)
}
fn workspace_write_sandbox(writable_root: PathBuf) -> FileSystemSandboxContext {
FileSystemSandboxContext::new(SandboxPolicy::WorkspaceWrite {
writable_roots: vec![absolute_path(writable_root)],
read_only_access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: vec![],
let writable_root = absolute_path(writable_root);
// The policy is evaluated in the remote container, so use a container path
// for cwd instead of capturing the local test runner cwd.
FileSystemSandboxContext::from_legacy_sandbox_policy(
SandboxPolicy::WorkspaceWrite {
writable_roots: vec![writable_root.clone()],
read_only_access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: vec![],
},
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
},
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
})
writable_root,
)
}
fn assert_normalized_path_rejected(error: &std::io::Error) {
codex-rs/exec-server/src/file_system.rs
+28
View File
@@ -1,11 +1,14 @@
use async_trait::async_trait;
use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxKind;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::path::Path;
use tokio::io;
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
@@ -103,6 +106,31 @@ impl FileSystemSandboxContext {
matches!(file_system_policy.kind, FileSystemSandboxKind::Restricted)
&& !file_system_policy.has_full_disk_write_access()
}
pub(crate) fn drop_cwd_if_unused(mut self) -> Self {
let file_system_policy = self.permissions.file_system_sandbox_policy();
if !file_system_policy_has_cwd_dependent_entries(&file_system_policy) {
self.cwd = None;
}
self
}
}
pub(crate) fn file_system_policy_has_cwd_dependent_entries(
file_system_policy: &FileSystemSandboxPolicy,
) -> bool {
file_system_policy
.entries
.iter()
.any(|entry| match &entry.path {
FileSystemPath::GlobPattern { pattern } => !Path::new(pattern).is_absolute(),
FileSystemPath::Special {
value:
FileSystemSpecialPath::CurrentWorkingDirectory
| FileSystemSpecialPath::ProjectRoots { .. },
} => true,
FileSystemPath::Path { .. } | FileSystemPath::Special { .. } => false,
})
}
pub type FileSystemResult<T> = io::Result<T>;
codex-rs/exec-server/src/fs_sandbox.rs
+69 -44
View File
@@ -21,6 +21,7 @@ use tokio::process::Command;
use crate::ExecServerRuntimePaths;
use crate::FileSystemSandboxContext;
use crate::file_system::file_system_policy_has_cwd_dependent_entries;
use crate::fs_helper::CODEX_FS_HELPER_ARG1;
use crate::fs_helper::FsHelperPayload;
use crate::fs_helper::FsHelperRequest;
@@ -52,12 +53,12 @@ impl FileSystemSandboxRunner {
) -> Result<FsHelperPayload, JSONRPCErrorError> {
let cwd = sandbox_cwd(sandbox)?;
let mut file_system_policy = sandbox.permissions.file_system_sandbox_policy();
let helper_read_root = if sandbox.use_legacy_landlock {
None
let helper_read_roots = if sandbox.use_legacy_landlock {
Vec::new()
} else {
helper_read_root(&self.runtime_paths)
helper_read_roots(&self.runtime_paths)
};
add_helper_runtime_permissions(&mut file_system_policy, helper_read_root, cwd.as_path());
add_helper_runtime_permissions(&mut file_system_policy, &helper_read_roots, cwd.as_path());
normalize_file_system_policy_root_aliases(&mut file_system_policy);
let network_policy = NetworkSandboxPolicy::Restricted;
let sandbox_policy =
@@ -133,33 +134,24 @@ fn sandbox_cwd(sandbox: &FileSystemSandboxContext) -> Result<AbsolutePathBuf, JS
.map_err(|err| invalid_request(format!("current directory is not absolute: {err}")))
}
fn file_system_policy_has_cwd_dependent_entries(
file_system_policy: &FileSystemSandboxPolicy,
) -> bool {
file_system_policy
.entries
.iter()
.any(|entry| match &entry.path {
FileSystemPath::GlobPattern { pattern } => !std::path::Path::new(pattern).is_absolute(),
FileSystemPath::Special {
value:
FileSystemSpecialPath::CurrentWorkingDirectory
| FileSystemSpecialPath::ProjectRoots { .. },
} => true,
FileSystemPath::Path { .. } | FileSystemPath::Special { .. } => false,
})
}
fn helper_read_root(runtime_paths: &ExecServerRuntimePaths) -> Option<AbsolutePathBuf> {
runtime_paths
.codex_self_exe
.parent()
.and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok())
fn helper_read_roots(runtime_paths: &ExecServerRuntimePaths) -> Vec<AbsolutePathBuf> {
let mut roots = Vec::new();
for path in std::iter::once(runtime_paths.codex_self_exe.as_path())
.chain(runtime_paths.codex_linux_sandbox_exe.as_deref().into_iter())
{
if let Some(parent) = path.parent()
&& let Ok(root) = AbsolutePathBuf::from_absolute_path(parent)
&& !roots.contains(&root)
{
roots.push(root);
}
}
roots
}
fn add_helper_runtime_permissions(
file_system_policy: &mut FileSystemSandboxPolicy,
helper_read_root: Option<AbsolutePathBuf>,
helper_read_roots: &[AbsolutePathBuf],
cwd: &std::path::Path,
) {
if !file_system_policy.has_full_disk_read_access() {
@@ -174,19 +166,18 @@ fn add_helper_runtime_permissions(
}
}
let Some(helper_read_root) = helper_read_root else {
return;
};
if file_system_policy.can_read_path_with_cwd(helper_read_root.as_path(), cwd) {
return;
}
for helper_read_root in helper_read_roots {
if file_system_policy.can_read_path_with_cwd(helper_read_root.as_path(), cwd) {
continue;
}
file_system_policy.entries.push(FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: helper_read_root,
},
access: FileSystemAccessMode::Read,
});
file_system_policy.entries.push(FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: helper_read_root.clone(),
},
access: FileSystemAccessMode::Read,
});
}
}
fn compatibility_sandbox_policy(
@@ -371,7 +362,7 @@ mod tests {
use super::helper_env;
use super::helper_env_from_vars;
use super::helper_env_key_is_allowed;
use super::helper_read_root;
use super::helper_read_roots;
use super::sandbox_cwd;
#[test]
@@ -388,7 +379,7 @@ mod tests {
let mut policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, cwd.as_path());
add_helper_runtime_permissions(&mut policy, /*helper_read_root*/ None, cwd.as_path());
add_helper_runtime_permissions(&mut policy, /*helper_read_roots*/ &[], cwd.as_path());
assert!(policy.include_platform_defaults());
}
@@ -410,7 +401,7 @@ mod tests {
let mut policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, cwd.as_path());
add_helper_runtime_permissions(&mut policy, /*helper_read_root*/ None, cwd.as_path());
add_helper_runtime_permissions(&mut policy, /*helper_read_roots*/ &[], cwd.as_path());
assert!(policy.include_platform_defaults());
}
@@ -449,7 +440,7 @@ mod tests {
add_helper_runtime_permissions(
&mut policy,
helper_read_root(&runtime_paths),
&helper_read_roots(&runtime_paths),
cwd.as_path(),
);
@@ -611,10 +602,44 @@ mod tests {
add_helper_runtime_permissions(
&mut policy,
helper_read_root(&runtime_paths),
&helper_read_roots(&runtime_paths),
cwd.as_path(),
);
assert!(policy.can_read_path_with_cwd(readable.as_path(), cwd.as_path()));
}
#[test]
fn helper_permissions_include_linux_sandbox_alias_parent() {
let root = tempfile::tempdir().expect("temp dir");
let codex_self_exe = root.path().join("bin").join("codex");
let codex_linux_sandbox_exe = root.path().join("aliases").join("codex-linux-sandbox");
let runtime_paths =
ExecServerRuntimePaths::new(codex_self_exe, Some(codex_linux_sandbox_exe))
.expect("runtime paths");
let cwd = AbsolutePathBuf::from_absolute_path(std::env::temp_dir().as_path())
.expect("absolute cwd");
let sandbox_policy = SandboxPolicy::ReadOnly {
access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: Vec::new(),
},
network_access: false,
};
let mut policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, cwd.as_path());
let codex_parent = AbsolutePathBuf::from_absolute_path(root.path().join("bin"))
.expect("absolute codex parent");
let alias_parent = AbsolutePathBuf::from_absolute_path(root.path().join("aliases"))
.expect("absolute alias parent");
add_helper_runtime_permissions(
&mut policy,
&helper_read_roots(&runtime_paths),
cwd.as_path(),
);
assert!(policy.can_read_path_with_cwd(codex_parent.as_path(), cwd.as_path()));
assert!(policy.can_read_path_with_cwd(alias_parent.as_path(), cwd.as_path()));
}
}
codex-rs/exec-server/src/remote_file_system.rs
+68 -7
View File
@@ -50,7 +50,7 @@ impl ExecutorFileSystem for RemoteFileSystem {
let response = client
.fs_read_file(FsReadFileParams {
path: path.clone(),
sandbox: sandbox.cloned(),
sandbox: remote_sandbox_context(sandbox),
})
.await
.map_err(map_remote_error)?;
@@ -74,7 +74,7 @@ impl ExecutorFileSystem for RemoteFileSystem {
.fs_write_file(FsWriteFileParams {
path: path.clone(),
data_base64: STANDARD.encode(contents),
sandbox: sandbox.cloned(),
sandbox: remote_sandbox_context(sandbox),
})
.await
.map_err(map_remote_error)?;
@@ -93,7 +93,7 @@ impl ExecutorFileSystem for RemoteFileSystem {
.fs_create_directory(FsCreateDirectoryParams {
path: path.clone(),
recursive: Some(options.recursive),
sandbox: sandbox.cloned(),
sandbox: remote_sandbox_context(sandbox),
})
.await
.map_err(map_remote_error)?;
@@ -110,7 +110,7 @@ impl ExecutorFileSystem for RemoteFileSystem {
let response = client
.fs_get_metadata(FsGetMetadataParams {
path: path.clone(),
sandbox: sandbox.cloned(),
sandbox: remote_sandbox_context(sandbox),
})
.await
.map_err(map_remote_error)?;
@@ -133,7 +133,7 @@ impl ExecutorFileSystem for RemoteFileSystem {
let response = client
.fs_read_directory(FsReadDirectoryParams {
path: path.clone(),
sandbox: sandbox.cloned(),
sandbox: remote_sandbox_context(sandbox),
})
.await
.map_err(map_remote_error)?;
@@ -161,7 +161,7 @@ impl ExecutorFileSystem for RemoteFileSystem {
path: path.clone(),
recursive: Some(options.recursive),
force: Some(options.force),
sandbox: sandbox.cloned(),
sandbox: remote_sandbox_context(sandbox),
})
.await
.map_err(map_remote_error)?;
@@ -182,7 +182,7 @@ impl ExecutorFileSystem for RemoteFileSystem {
source_path: source_path.clone(),
destination_path: destination_path.clone(),
recursive: options.recursive,
sandbox: sandbox.cloned(),
sandbox: remote_sandbox_context(sandbox),
})
.await
.map_err(map_remote_error)?;
@@ -190,6 +190,14 @@ impl ExecutorFileSystem for RemoteFileSystem {
}
}
fn remote_sandbox_context(
sandbox: Option<&FileSystemSandboxContext>,
) -> Option<FileSystemSandboxContext> {
sandbox
.cloned()
.map(FileSystemSandboxContext::drop_cwd_if_unused)
}
fn map_remote_error(error: ExecServerError) -> io::Error {
match error {
ExecServerError::Server { code, message } if code == NOT_FOUND_ERROR_CODE => {
@@ -208,10 +216,58 @@ fn map_remote_error(error: ExecServerError) -> io::Error {
#[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 pretty_assertions::assert_eq;
use super::*;
#[test]
fn remote_sandbox_context_drops_unused_cwd() {
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: absolute_test_path("remote-root"),
},
access: FileSystemAccessMode::Read,
}]);
let permissions =
PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted);
let sandbox_context = FileSystemSandboxContext::from_permission_profile_with_cwd(
permissions,
absolute_test_path("host-checkout"),
);
let remote_context =
remote_sandbox_context(Some(&sandbox_context)).expect("remote sandbox context");
assert_eq!(remote_context.cwd, None);
}
#[test]
fn remote_sandbox_context_preserves_required_cwd() {
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
},
access: FileSystemAccessMode::Write,
}]);
let permissions =
PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted);
let cwd = absolute_test_path("host-checkout");
let sandbox_context =
FileSystemSandboxContext::from_permission_profile_with_cwd(permissions, cwd.clone());
let remote_context =
remote_sandbox_context(Some(&sandbox_context)).expect("remote sandbox context");
assert_eq!(remote_context.cwd, Some(cwd));
}
#[test]
fn transport_errors_map_to_broken_pipe() {
let errors = [
@@ -241,4 +297,9 @@ 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")
}
}
codex-rs/exec-server/src/server/handler/tests.rs
+2 -2
View File
@@ -317,7 +317,7 @@ async fn read_process_until_closed(
handler: &ExecServerHandler,
process_id: ProcessId,
) -> (String, Option<i32>) {
let deadline = tokio::time::Instant::now() + Duration::from_secs(2);
let deadline = tokio::time::Instant::now() + Duration::from_secs(5);
let mut output = String::new();
let mut exit_code = None;
let mut after_seq = None;
@@ -346,7 +346,7 @@ async fn read_process_until_closed(
after_seq = response.next_seq.checked_sub(1).or(after_seq);
assert!(
tokio::time::Instant::now() < deadline,
"process should close within 2s"
"process should close within 5s"
);
}
}