mirror of
https://github.com/openai/codex.git
synced 2026-10-23 18:08:45 +00:00
exec-server: expose arg0 alias root to fs sandbox
## 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 remaining gap is that the remote exec-server can pass an arg0 alias path such as `codex-linux-sandbox` as a runtime helper, but the sandboxed filesystem helper only added the real Codex binary parent as a readable runtime root. When bubblewrap re-enters Codex through the alias path, the alias directory also has to be visible inside the sandbox. ## 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. - Add unit coverage for the alias-parent root. ## Verification - `cargo test -p codex-exec-server` - `just argument-comment-lint -p codex-exec-server` - `just fix -p codex-exec-server`
This commit is contained in:
@@ -52,12 +52,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 =
|
||||
@@ -150,16 +150,24 @@ fn file_system_policy_has_cwd_dependent_entries(
|
||||
})
|
||||
}
|
||||
|
||||
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 +182,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 +378,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 +395,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 +417,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 +456,7 @@ mod tests {
|
||||
|
||||
add_helper_runtime_permissions(
|
||||
&mut policy,
|
||||
helper_read_root(&runtime_paths),
|
||||
&helper_read_roots(&runtime_paths),
|
||||
cwd.as_path(),
|
||||
);
|
||||
|
||||
@@ -611,10 +618,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()));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user