Fix missing git sandbox mount

This commit is contained in:
Eva Wong
2026-04-27 08:41:48 -07:00
parent f7a0cc04dd
commit a71e463924
7 changed files with 180 additions and 161 deletions
+73 -2
View File
@@ -35,6 +35,10 @@ use crate::exit_status::handle_exit_status;
#[cfg(target_os = "macos")]
use seatbelt::DenialLogger;
#[cfg(target_os = "linux")]
const LINUX_SANDBOX_FORWARDED_SIGNALS: &[libc::c_int] =
&[libc::SIGHUP, libc::SIGINT, libc::SIGQUIT, libc::SIGTERM];
#[cfg(target_os = "macos")]
pub async fn run_command_under_seatbelt(
command: SeatbeltCommand,
@@ -268,6 +272,9 @@ async fn run_command_under_sandbox(
denial_logger.on_child_spawn(&child);
}
#[cfg(target_os = "linux")]
let status = wait_for_debug_sandbox_child(&mut child).await?;
#[cfg(not(target_os = "linux"))]
let status = child.wait().await?;
#[cfg(target_os = "macos")]
@@ -449,10 +456,12 @@ async fn spawn_debug_sandbox_child(
{
let parent_pid = unsafe { libc::getpid() };
// SAFETY: `pre_exec` runs in the child immediately before exec. The
// closure only installs a parent-death signal and checks the inherited
// parent pid to close the fork/exec race.
// closure only adjusts the child signal mask, installs a parent-death
// signal, and checks the inherited parent pid to close the fork/exec
// race.
unsafe {
cmd.pre_exec(move || {
block_linux_sandbox_forwarded_signals()?;
if libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGTERM) == -1 {
return Err(std::io::Error::last_os_error());
}
@@ -471,6 +480,68 @@ async fn spawn_debug_sandbox_child(
.spawn()
}
#[cfg(target_os = "linux")]
async fn wait_for_debug_sandbox_child(
child: &mut Child,
) -> std::io::Result<std::process::ExitStatus> {
let child_pid = child.id().map(|pid| pid as libc::pid_t);
tokio::select! {
status = child.wait() => status,
signal = recv_linux_sandbox_forwarded_signal() => {
let signal = signal?;
if let Some(child_pid) = child_pid {
signal_debug_sandbox_child(child_pid, signal)?;
}
child.wait().await
}
}
}
#[cfg(target_os = "linux")]
async fn recv_linux_sandbox_forwarded_signal() -> std::io::Result<libc::c_int> {
use tokio::signal::unix::SignalKind;
use tokio::signal::unix::signal;
let mut sighup = signal(SignalKind::hangup())?;
let mut sigint = signal(SignalKind::interrupt())?;
let mut sigquit = signal(SignalKind::quit())?;
let mut sigterm = signal(SignalKind::terminate())?;
let signal = tokio::select! {
_ = sighup.recv() => libc::SIGHUP,
_ = sigint.recv() => libc::SIGINT,
_ = sigquit.recv() => libc::SIGQUIT,
_ = sigterm.recv() => libc::SIGTERM,
};
Ok(signal)
}
#[cfg(target_os = "linux")]
fn signal_debug_sandbox_child(pid: libc::pid_t, signal: libc::c_int) -> std::io::Result<()> {
if unsafe { libc::kill(pid, signal) } < 0 {
let err = std::io::Error::last_os_error();
if err.raw_os_error() != Some(libc::ESRCH) {
return Err(err);
}
}
Ok(())
}
#[cfg(target_os = "linux")]
fn block_linux_sandbox_forwarded_signals() -> std::io::Result<()> {
let mut blocked: libc::sigset_t = unsafe { std::mem::zeroed() };
unsafe {
libc::sigemptyset(&mut blocked);
for signal in LINUX_SANDBOX_FORWARDED_SIGNALS {
libc::sigaddset(&mut blocked, *signal);
}
if libc::sigprocmask(libc::SIG_BLOCK, &blocked, std::ptr::null_mut()) < 0 {
return Err(std::io::Error::last_os_error());
}
}
Ok(())
}
#[cfg(target_os = "windows")]
mod windows_stdio_bridge {
use std::io::Read;
+8 -14
View File
@@ -26,7 +26,6 @@ use std::process::Command;
use codex_protocol::error::CodexErr;
use codex_protocol::error::Result;
use codex_protocol::permissions::is_preserved_directory_path_name;
use codex_protocol::permissions::is_preserved_path_name;
use codex_protocol::protocol::FileSystemSandboxPolicy;
use codex_protocol::protocol::WritableRoot;
@@ -1004,10 +1003,7 @@ fn append_empty_directory_args(bwrap_args: &mut BwrapArgs, path: &Path) {
}
fn append_missing_read_only_subpath_args(bwrap_args: &mut BwrapArgs, path: &Path) -> Result<()> {
if path
.file_name()
.is_some_and(is_preserved_directory_path_name)
{
if path.file_name().is_some_and(is_preserved_path_name) {
append_empty_directory_args(bwrap_args, path);
bwrap_args
.synthetic_mount_targets
@@ -1632,6 +1628,7 @@ mod tests {
.expect("filesystem args");
assert_empty_file_bound_without_perms(&args.args, &blocked);
assert_empty_directory_mounted_read_only(&args.args, &workspace.join(".git"));
assert_empty_directory_mounted_read_only(&args.args, &workspace.join(".agents"));
assert_empty_directory_mounted_read_only(&args.args, &workspace.join(".codex"));
assert_eq!(args.preserved_files.len(), 1);
@@ -1698,7 +1695,7 @@ mod tests {
}
#[test]
fn missing_child_git_under_parent_repo_stays_absent() {
fn missing_child_git_under_parent_repo_uses_read_only_empty_directory() {
let temp_dir = TempDir::new().expect("temp dir");
let repo = temp_dir.path().join("repo");
let workspace = repo.join("workspace");
@@ -1717,18 +1714,15 @@ mod tests {
let args = create_filesystem_args(&policy, &workspace, NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH)
.expect("filesystem args");
assert_empty_directory_mounted_read_only(&args.args, &dot_git);
assert!(
!args.args.iter().any(|arg| arg == &path_to_string(&dot_git)),
"missing child .git under an existing parent repo must not be materialized",
);
assert!(
!synthetic_mount_target_paths(&args).contains(&dot_git),
"missing child .git should not be materialized as a synthetic mount target",
synthetic_mount_target_paths(&args).contains(&dot_git),
"missing child .git should be a transient mount target",
);
assert_eq!(
protected_create_target_paths(&args),
vec![dot_git],
"missing child .git is enforced by the preserved-name sandbox policy",
Vec::<PathBuf>::new(),
"missing child .git should fail at creation time through the read-only mount",
);
}
+76 -2
View File
@@ -27,6 +27,7 @@ use codex_protocol::protocol::SandboxPolicy;
use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0;
static BWRAP_CHILD_PID: AtomicI32 = AtomicI32::new(0);
static PENDING_FORWARDED_SIGNAL: AtomicI32 = AtomicI32::new(0);
const FORWARDED_SIGNALS: &[libc::c_int] =
&[libc::SIGHUP, libc::SIGINT, libc::SIGQUIT, libc::SIGTERM];
@@ -614,6 +615,7 @@ fn run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args: crate::bwrap::Bwr
synthetic_mount_targets,
protected_create_targets,
} = bwrap_args;
let setup_signal_mask = ForwardedSignalMask::block();
let synthetic_mount_registrations = register_synthetic_mount_targets(&synthetic_mount_targets);
let protected_create_registrations =
register_protected_create_targets(&protected_create_targets);
@@ -625,6 +627,8 @@ fn run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args: crate::bwrap::Bwr
}
if pid == 0 {
reset_forwarded_signal_handlers_to_default();
setup_signal_mask.restore();
let setpgid_res = unsafe { libc::setpgid(0, 0) };
if setpgid_res < 0 {
let err = std::io::Error::last_os_error();
@@ -635,14 +639,52 @@ fn run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args: crate::bwrap::Bwr
}
install_bwrap_signal_forwarders(pid);
setup_signal_mask.restore();
let status = wait_for_bwrap_child(pid);
let cleanup_signal_mask = ForwardedSignalMask::block();
BWRAP_CHILD_PID.store(0, Ordering::SeqCst);
cleanup_synthetic_mount_targets(&synthetic_mount_registrations);
let protected_create_violation =
cleanup_protected_create_targets(&protected_create_registrations);
cleanup_signal_mask.restore();
exit_with_wait_status_or_policy_violation(status, protected_create_violation);
}
struct ForwardedSignalMask {
previous: libc::sigset_t,
}
impl ForwardedSignalMask {
fn block() -> Self {
let mut blocked: libc::sigset_t = unsafe { std::mem::zeroed() };
let mut previous: libc::sigset_t = unsafe { std::mem::zeroed() };
unsafe {
libc::sigemptyset(&mut blocked);
for signal in FORWARDED_SIGNALS {
libc::sigaddset(&mut blocked, *signal);
}
if libc::sigprocmask(libc::SIG_BLOCK, &blocked, &mut previous) < 0 {
let err = std::io::Error::last_os_error();
panic!("failed to block bubblewrap forwarded signals: {err}");
}
}
Self { previous }
}
fn restore(&self) {
let mut restored = self.previous;
unsafe {
for signal in FORWARDED_SIGNALS {
libc::sigdelset(&mut restored, *signal);
}
if libc::sigprocmask(libc::SIG_SETMASK, &restored, std::ptr::null_mut()) < 0 {
let err = std::io::Error::last_os_error();
panic!("failed to restore bubblewrap forwarded signals: {err}");
}
}
}
}
fn terminate_with_parent(parent_pid: libc::pid_t) {
let res = unsafe { libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGTERM) };
if res < 0 {
@@ -669,14 +711,38 @@ fn install_bwrap_signal_forwarders(pid: libc::pid_t) {
}
}
}
replay_pending_forwarded_signal(pid);
}
extern "C" fn forward_signal_to_bwrap_child(signal: libc::c_int) {
PENDING_FORWARDED_SIGNAL.store(signal, Ordering::SeqCst);
let pid = BWRAP_CHILD_PID.load(Ordering::SeqCst);
if pid > 0 {
send_signal_to_bwrap_child(pid, signal);
}
}
fn replay_pending_forwarded_signal(pid: libc::pid_t) {
let signal = PENDING_FORWARDED_SIGNAL.swap(0, Ordering::SeqCst);
if signal > 0 {
send_signal_to_bwrap_child(pid, signal);
}
}
fn send_signal_to_bwrap_child(pid: libc::pid_t, signal: libc::c_int) {
unsafe {
libc::kill(-pid, signal);
libc::kill(pid, signal);
}
}
fn reset_forwarded_signal_handlers_to_default() {
for signal in FORWARDED_SIGNALS {
unsafe {
libc::kill(-pid, signal);
libc::kill(pid, signal);
if libc::signal(*signal, libc::SIG_DFL) == libc::SIG_ERR {
let err = std::io::Error::last_os_error();
panic!("failed to reset bubblewrap signal handler for {signal}: {err}");
}
}
}
}
@@ -1093,6 +1159,7 @@ fn run_bwrap_in_child_capture_stderr(bwrap_args: crate::bwrap::BwrapArgs) -> Str
synthetic_mount_targets,
protected_create_targets,
} = bwrap_args;
let setup_signal_mask = ForwardedSignalMask::block();
let synthetic_mount_registrations = register_synthetic_mount_targets(&synthetic_mount_targets);
let protected_create_registrations =
register_protected_create_targets(&protected_create_targets);
@@ -1113,6 +1180,8 @@ fn run_bwrap_in_child_capture_stderr(bwrap_args: crate::bwrap::BwrapArgs) -> Str
}
if pid == 0 {
reset_forwarded_signal_handlers_to_default();
setup_signal_mask.restore();
// Child: redirect stderr to the pipe, then run bubblewrap.
unsafe {
close_fd_or_panic(read_fd, "close read end in bubblewrap child");
@@ -1126,6 +1195,8 @@ fn run_bwrap_in_child_capture_stderr(bwrap_args: crate::bwrap::BwrapArgs) -> Str
exec_bwrap(args, preserved_files);
}
install_bwrap_signal_forwarders(pid);
setup_signal_mask.restore();
// Parent: close the write end and read stderr while the child runs.
close_fd_or_panic(write_fd, "close write end in bubblewrap parent");
@@ -1138,8 +1209,11 @@ fn run_bwrap_in_child_capture_stderr(bwrap_args: crate::bwrap::BwrapArgs) -> Str
}
wait_for_bwrap_child(pid);
let cleanup_signal_mask = ForwardedSignalMask::block();
BWRAP_CHILD_PID.store(0, Ordering::SeqCst);
cleanup_synthetic_mount_targets(&synthetic_mount_registrations);
cleanup_protected_create_targets(&protected_create_registrations);
cleanup_signal_mask.restore();
String::from_utf8_lossy(&stderr_bytes).into_owned()
}
+18 -43
View File
@@ -1377,9 +1377,8 @@ pub(crate) fn default_read_only_subpaths_for_writable_root(
// writable root itself.
let top_level_git_is_file = top_level_git.as_path().is_file();
let top_level_git_is_dir = top_level_git.as_path().is_dir();
let should_protect_top_level_git = top_level_git_is_dir
|| top_level_git_is_file
|| (protect_missing_preserved_paths && !has_ancestor_git_metadata(writable_root.as_path()));
let should_protect_top_level_git =
top_level_git_is_dir || top_level_git_is_file || protect_missing_preserved_paths;
if should_protect_top_level_git {
if top_level_git_is_file
&& is_git_pointer_file(&top_level_git)
@@ -1558,34 +1557,6 @@ fn has_explicit_write_entry_for_preserved_path(
})
}
fn has_ancestor_git_metadata(path: &Path) -> bool {
let Some(parent) = path.parent() else {
return false;
};
parent
.ancestors()
.any(|ancestor| git_metadata_dir(&ancestor.join(PRESERVED_GIT_PATH_NAME)).is_some())
}
fn git_metadata_dir(path: &Path) -> Option<AbsolutePathBuf> {
let Ok(metadata) = std::fs::symlink_metadata(path) else {
return None;
};
if metadata.is_dir() {
return AbsolutePathBuf::from_absolute_path(path).ok();
}
if !metadata.is_file() {
return None;
}
let Ok(dot_git) = AbsolutePathBuf::from_absolute_path(path) else {
return None;
};
if !is_git_pointer_file(&dot_git) {
return None;
}
resolve_gitdir_from_file(&dot_git)
}
fn is_git_pointer_file(path: &AbsolutePathBuf) -> bool {
path.as_path().is_file()
&& path.as_path().file_name() == Some(OsStr::new(PRESERVED_GIT_PATH_NAME))
@@ -1788,7 +1759,7 @@ mod tests {
#[cfg(unix)]
#[test]
fn writable_roots_keep_missing_git_absent_under_parent_git_repo() {
fn writable_roots_protect_missing_git_under_parent_git_repo() {
let repo = TempDir::new().expect("tempdir");
fs::create_dir(repo.path().join(".git")).expect("create parent .git");
let cwd = repo.path().join("sub");
@@ -1811,10 +1782,10 @@ mod tests {
assert_eq!(writable_roots.len(), 1);
assert_eq!(writable_roots[0].root, expected_root);
assert!(
!writable_roots[0]
writable_roots[0]
.read_only_subpaths
.contains(&expected_dot_git),
"missing child .git under an existing parent repo stays absent so Git discovery still works"
"missing child .git under an existing parent repo should be read-only so git init fails"
);
assert!(
writable_roots[0]
@@ -1975,10 +1946,6 @@ mod tests {
let extra_root = AbsolutePathBuf::from_absolute_path(extra.path()).expect("absolute extra");
let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![extra_root],
read_only_access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: vec![],
},
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
@@ -2019,12 +1986,20 @@ mod tests {
let file_system_policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&policy, relative_cwd);
let mut expected_entries = vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
let mut expected_entries = vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
access: FileSystemAccessMode::Write,
}];
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
},
access: FileSystemAccessMode::Write,
},
];
expected_entries.extend(
default_read_only_subpaths_for_writable_root(
&expected_root,
+5 -51
View File
@@ -4,7 +4,6 @@
//! between user and agent.
use std::collections::HashMap;
use std::collections::HashSet;
use std::fmt;
use std::ops::Mul;
use std::path::Path;
@@ -4436,22 +4435,20 @@ mod tests {
}
#[test]
fn workspace_write_keeps_missing_git_absent_under_parent_repo() {
fn workspace_write_protects_missing_git_under_parent_repo() {
let repo = TempDir::new().expect("tempdir");
std::fs::create_dir(repo.path().join(".git")).expect("create parent .git");
let cwd = repo.path().join("sub");
std::fs::create_dir(&cwd).expect("create subdir");
let expected_root = AbsolutePathBuf::from_absolute_path(&cwd).expect("absolute cwd");
let expected_dot_git =
AbsolutePathBuf::from_absolute_path(cwd.join(".git")).expect("canonical .git");
let expected_dot_codex =
AbsolutePathBuf::from_absolute_path(cwd.join(".codex")).expect("canonical .codex");
let expected_dot_agents =
AbsolutePathBuf::from_absolute_path(cwd.join(".agents")).expect("canonical .agents");
let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
read_only_access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: vec![],
},
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
@@ -4463,7 +4460,8 @@ mod tests {
expected_root.to_path_buf(),
vec![
expected_dot_agents.to_path_buf(),
expected_dot_codex.to_path_buf()
expected_dot_codex.to_path_buf(),
expected_dot_git.to_path_buf()
],
)]
);
@@ -4481,10 +4479,6 @@ mod tests {
AbsolutePathBuf::from_absolute_path(&extra).expect("absolute extra root");
let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![expected_extra.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,
@@ -4513,46 +4507,6 @@ mod tests {
);
}
#[test]
fn legacy_workspace_write_nested_readable_root_stays_writable() {
let cwd = TempDir::new().expect("tempdir");
let docs = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path());
let canonical_cwd = codex_utils_absolute_path::canonicalize_preserving_symlinks(cwd.path())
.expect("canonicalize cwd");
let expected_dot_codex = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".codex"))
.expect("canonical .codex");
let expected_dot_git = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".git"))
.expect("canonical .git");
let expected_dot_agents =
AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".agents"))
.expect("canonical .agents");
let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
read_only_access: ReadOnlyAccess::Restricted {
include_platform_defaults: true,
readable_roots: vec![docs],
},
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
};
assert_eq!(
sorted_writable_roots(
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&policy, cwd.path())
.get_writable_roots_with_cwd(cwd.path())
),
vec![(
canonical_cwd,
vec![
expected_dot_agents.to_path_buf(),
expected_dot_codex.to_path_buf(),
expected_dot_git.to_path_buf()
]
)]
);
}
#[test]
fn file_system_policy_rejects_legacy_bridge_for_non_workspace_writes() {
let cwd = if cfg!(windows) {
-44
View File
@@ -27,7 +27,6 @@ use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::permissions::PRESERVED_PATH_NAMES;
use codex_protocol::protocol::ReadOnlyAccess;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
@@ -373,49 +372,6 @@ fn seatbelt_args_without_extension_profile_keep_legacy_preferences_read_access()
assert!(!policy.contains("(allow user-preference-write)"));
}
#[test]
fn seatbelt_legacy_workspace_write_nested_readable_root_stays_writable() {
let tmp = TempDir::new().expect("tempdir");
let cwd = tmp.path().join("workspace");
fs::create_dir_all(cwd.join("docs")).expect("create docs");
let docs = AbsolutePathBuf::from_absolute_path(cwd.join("docs")).expect("absolute docs");
let args = create_seatbelt_command_args_for_legacy_policy(
vec!["/bin/true".to_string()],
&SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
read_only_access: ReadOnlyAccess::Restricted {
include_platform_defaults: true,
readable_roots: vec![docs.clone()],
},
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
},
cwd.as_path(),
/*enforce_managed_network*/ false,
/*network*/ None,
);
assert!(
!args
.iter()
.any(|arg| arg.ends_with(&format!("={}", docs.as_path().display()))),
"legacy workspace-write readable roots under cwd should not become seatbelt carveouts:\n{args:#?}",
);
assert!(
args.iter()
.any(|arg| arg.starts_with("-DWRITABLE_ROOT_0_EXCLUDED_")
&& arg.ends_with("/workspace/.agents")),
"expected proactive .agents carveout for cwd root: {args:#?}",
);
assert!(
args.iter()
.any(|arg| arg.starts_with("-DWRITABLE_ROOT_0_EXCLUDED_")
&& arg.ends_with("/workspace/.codex")),
"expected proactive .codex carveout for cwd root: {args:#?}",
);
}
#[test]
fn create_seatbelt_args_allows_local_binding_when_explicitly_enabled() {
let policy = dynamic_network_policy(
@@ -32,7 +32,6 @@ mod tests {
use std::path::PathBuf;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::protocol::ReadOnlyAccess;
use codex_protocol::protocol::SandboxPolicy;
use pretty_assertions::assert_eq;
@@ -67,10 +66,6 @@ mod tests {
fn legacy_workspace_write_policy() -> FileSystemSandboxPolicy {
let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
read_only_access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: vec![],
},
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,