Merge 1506cb173c into sapling-pr-archive-bolinfest

This commit is contained in:
Michael Bolin
2026-04-22 11:33:22 -07:00
committed by GitHub
2 changed files with 147 additions and 42 deletions

View File

@@ -185,22 +185,39 @@ where
// Regular invocation create a Tokio runtime and execute the provided
// async entry-point.
let runtime = build_runtime()?;
runtime.block_on(async move {
let current_exe = std::env::current_exe().ok();
let paths = Arg0DispatchPaths {
codex_self_exe: current_exe.clone(),
codex_linux_sandbox_exe: if cfg!(target_os = "linux") {
linux_sandbox_exe_path(path_entry_guard.as_ref(), current_exe)
} else {
None
},
main_execve_wrapper_exe: path_entry_guard
.as_ref()
.and_then(|path_entry| path_entry.paths().main_execve_wrapper_exe.clone()),
};
runtime.block_on(run_main_with_arg0_guard(
path_entry_guard,
std::env::current_exe().ok(),
main_fn,
))
}
main_fn(paths).await
})
async fn run_main_with_arg0_guard<F, Fut>(
path_entry_guard: Option<Arg0PathEntryGuard>,
current_exe: Option<PathBuf>,
main_fn: F,
) -> anyhow::Result<()>
where
F: FnOnce(Arg0DispatchPaths) -> Fut,
Fut: Future<Output = anyhow::Result<()>>,
{
let paths = Arg0DispatchPaths {
codex_self_exe: current_exe.clone(),
codex_linux_sandbox_exe: if cfg!(target_os = "linux") {
linux_sandbox_exe_path(path_entry_guard.as_ref(), current_exe)
} else {
None
},
main_execve_wrapper_exe: path_entry_guard
.as_ref()
.and_then(|path_entry| path_entry.paths().main_execve_wrapper_exe.clone()),
};
let result = main_fn(paths).await;
// Keep the arg0 tempdir guard alive until the async entry point finishes;
// runtime paths above can point at aliases inside that directory.
drop(path_entry_guard);
result
}
fn linux_sandbox_exe_path(
@@ -442,6 +459,10 @@ mod tests {
use super::LOCK_FILENAME;
use super::janitor_cleanup;
use super::linux_sandbox_exe_path;
#[cfg(unix)]
use super::run_main_with_arg0_guard;
#[cfg(unix)]
use anyhow::ensure;
use std::fs;
use std::fs::File;
use std::path::Path;
@@ -480,6 +501,49 @@ mod tests {
Ok(())
}
#[cfg(unix)]
#[test]
fn run_main_with_arg0_guard_keeps_aliases_alive_until_main_returns() -> anyhow::Result<()> {
let temp_dir = TempDir::new()?;
let alias_path = temp_dir.path().join("codex-helper-alias");
fs::write(&alias_path, b"")?;
let lock_file = create_lock(temp_dir.path())?;
let path_entry = Arg0PathEntryGuard::new(
temp_dir,
lock_file,
Arg0DispatchPaths {
codex_self_exe: Some(PathBuf::from("/usr/bin/codex")),
codex_linux_sandbox_exe: Some(alias_path.clone()),
main_execve_wrapper_exe: Some(alias_path),
},
);
super::build_runtime()?.block_on(run_main_with_arg0_guard(
/*path_entry_guard*/ Some(path_entry),
Some(PathBuf::from("/usr/bin/codex")),
|paths| async move {
let alias_path = paths
.codex_linux_sandbox_exe
.or(paths.main_execve_wrapper_exe)
.expect("unix dispatch should create at least one alias path");
ensure!(
alias_path.exists(),
"alias path disappeared before main future was polled: {}",
alias_path.display()
);
tokio::task::yield_now().await;
ensure!(
alias_path.exists(),
"alias path disappeared while main future was running: {}",
alias_path.display()
);
Ok(())
},
))
}
#[test]
fn janitor_skips_dirs_without_lock_file() -> std::io::Result<()> {
let root = tempfile::tempdir()?;

View File

@@ -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()));
}
}