mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
Remove obsolete protected-path monitoring from the Linux sandbox (#54445)
## Why Missing protected metadata paths already receive read-only synthetic mounts, as established in https://github.com/openai/codex/pull/39629. The fallback creation monitor no longer receives any targets because those paths are included in the read-only mount list. ## What changed Remove `ProtectedCreateTarget` tracking and its inotify monitor, deletion fallback, registry markers, startup pipe, and policy-violation exit override. Keep synthetic mount cleanup and propagate the bubblewrap child's exit status directly. GitOrigin-RevId: ac747860cad845ebb89ada5a9c1009e56835c4fb
This commit is contained in:
@@ -128,7 +128,6 @@ pub(crate) struct BwrapArgs {
|
||||
pub args: Vec<String>,
|
||||
pub preserved_files: Vec<File>,
|
||||
pub synthetic_mount_targets: Vec<SyntheticMountTarget>,
|
||||
pub protected_create_targets: Vec<ProtectedCreateTarget>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
@@ -161,23 +160,6 @@ pub(crate) struct SyntheticMountTarget {
|
||||
pre_existing_path: Option<FileIdentity>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub(crate) struct ProtectedCreateTarget {
|
||||
path: PathBuf,
|
||||
}
|
||||
|
||||
impl ProtectedCreateTarget {
|
||||
pub(crate) fn missing(path: &Path) -> Self {
|
||||
Self {
|
||||
path: path.to_path_buf(),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn path(&self) -> &Path {
|
||||
&self.path
|
||||
}
|
||||
}
|
||||
|
||||
impl SyntheticMountTarget {
|
||||
pub(crate) fn missing(path: &Path) -> Self {
|
||||
Self {
|
||||
@@ -268,7 +250,6 @@ pub(crate) fn create_bwrap_command_args(
|
||||
args: command,
|
||||
preserved_files: Vec::new(),
|
||||
synthetic_mount_targets: Vec::new(),
|
||||
protected_create_targets: Vec::new(),
|
||||
})
|
||||
} else {
|
||||
Ok(create_bwrap_flags_full_filesystem(command, options))
|
||||
@@ -321,7 +302,6 @@ fn create_bwrap_flags_full_filesystem(command: Vec<String>, options: BwrapOption
|
||||
args,
|
||||
preserved_files: Vec::new(),
|
||||
synthetic_mount_targets: Vec::new(),
|
||||
protected_create_targets: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -354,7 +334,6 @@ fn create_bwrap_flags(
|
||||
args: filesystem_args,
|
||||
preserved_files,
|
||||
synthetic_mount_targets,
|
||||
protected_create_targets,
|
||||
} = create_filesystem_args(file_system_sandbox_policy, sandbox_policy_cwd, options)?;
|
||||
let normalized_command_cwd = normalize_command_cwd_for_bwrap(command_cwd);
|
||||
let mut args = Vec::new();
|
||||
@@ -407,7 +386,6 @@ fn create_bwrap_flags(
|
||||
args,
|
||||
preserved_files,
|
||||
synthetic_mount_targets,
|
||||
protected_create_targets,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -608,7 +586,6 @@ fn create_filesystem_args(
|
||||
args,
|
||||
preserved_files: Vec::new(),
|
||||
synthetic_mount_targets: Vec::new(),
|
||||
protected_create_targets: Vec::new(),
|
||||
};
|
||||
let mut allowed_write_paths = Vec::with_capacity(writable_roots.len());
|
||||
for writable_root in &writable_roots {
|
||||
@@ -719,13 +696,6 @@ fn create_filesystem_args(
|
||||
deferred_read_only_subpaths =
|
||||
remap_paths_for_symlink_target(deferred_read_only_subpaths, root, target);
|
||||
}
|
||||
append_protected_create_targets_for_writable_root(
|
||||
&mut bwrap_args,
|
||||
protected_metadata_names,
|
||||
root,
|
||||
symlink_target.as_deref(),
|
||||
&read_only_subpaths,
|
||||
);
|
||||
read_only_subpaths.sort_by_key(|path| path_depth(path));
|
||||
for subpath in read_only_subpaths {
|
||||
if let Some(symlink) =
|
||||
@@ -828,29 +798,6 @@ fn create_filesystem_args(
|
||||
Ok(bwrap_args)
|
||||
}
|
||||
|
||||
fn append_protected_create_targets_for_writable_root(
|
||||
bwrap_args: &mut BwrapArgs,
|
||||
protected_metadata_names: &[String],
|
||||
root: &Path,
|
||||
symlink_target: Option<&Path>,
|
||||
read_only_subpaths: &[PathBuf],
|
||||
) {
|
||||
for name in protected_metadata_names {
|
||||
let mut path = root.join(name);
|
||||
if let Some(target) = symlink_target
|
||||
&& let Ok(relative_path) = path.strip_prefix(root)
|
||||
{
|
||||
path = target.join(relative_path);
|
||||
}
|
||||
if read_only_subpaths.iter().any(|subpath| subpath == &path) || path.exists() {
|
||||
continue;
|
||||
}
|
||||
bwrap_args
|
||||
.protected_create_targets
|
||||
.push(ProtectedCreateTarget::missing(&path));
|
||||
}
|
||||
}
|
||||
|
||||
fn append_metadata_path_masks_for_writable_root(
|
||||
read_only_subpaths: &mut Vec<PathBuf>,
|
||||
root: &Path,
|
||||
@@ -2197,10 +2144,6 @@ mod tests {
|
||||
workspace.join(".aws"),
|
||||
],
|
||||
);
|
||||
assert!(
|
||||
protected_create_target_paths(&args).is_empty(),
|
||||
"missing child .git should be mount protected before command execution",
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
@@ -2239,10 +2182,6 @@ mod tests {
|
||||
workspace.join(".aws"),
|
||||
],
|
||||
);
|
||||
assert!(
|
||||
protected_create_target_paths(&args).is_empty(),
|
||||
"symlinked missing child .git should be mount protected before command execution",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -2333,11 +2272,6 @@ mod tests {
|
||||
assert!(synthetic_targets.contains(&PathBuf::from(&dot_git)));
|
||||
assert!(synthetic_targets.contains(&PathBuf::from(&dot_agents)));
|
||||
assert!(synthetic_targets.contains(&PathBuf::from(&dot_codex)));
|
||||
assert_eq!(
|
||||
protected_create_target_paths(&args),
|
||||
Vec::<PathBuf>::new(),
|
||||
"missing protected metadata paths should fail at creation time through read-only mounts",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -3140,11 +3074,4 @@ mod tests {
|
||||
.map(|target| target.path().to_path_buf())
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn protected_create_target_paths(args: &BwrapArgs) -> Vec<PathBuf> {
|
||||
args.protected_create_targets
|
||||
.iter()
|
||||
.map(|target| target.path().to_path_buf())
|
||||
.collect()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,17 +8,11 @@ use std::io::Read;
|
||||
use std::os::fd::AsRawFd;
|
||||
use std::os::fd::FromRawFd;
|
||||
use std::os::unix::ffi::OsStrExt;
|
||||
use std::os::unix::fs::OpenOptionsExt;
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::sync::OnceLock;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::AtomicI32;
|
||||
use std::sync::atomic::Ordering;
|
||||
use std::thread;
|
||||
use std::time::Duration;
|
||||
|
||||
use crate::bwrap::BwrapNetworkMode;
|
||||
use crate::bwrap::BwrapOptions;
|
||||
@@ -50,7 +44,6 @@ const FORWARDED_SIGNALS: &[libc::c_int] =
|
||||
const LINUX_CAPABILITY_VERSION_3: u32 = 0x2008_0522;
|
||||
const SYNTHETIC_MOUNT_MARKER_SYNTHETIC: &[u8] = b"synthetic\n";
|
||||
const SYNTHETIC_MOUNT_MARKER_EXISTING: &[u8] = b"existing\n";
|
||||
const PROTECTED_CREATE_MARKER: &[u8] = b"protected-create\n";
|
||||
|
||||
#[derive(Debug)]
|
||||
struct SyntheticMountTargetRegistration {
|
||||
@@ -59,30 +52,6 @@ struct SyntheticMountTargetRegistration {
|
||||
marker_dir: PathBuf,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct ProtectedCreateTargetRegistration {
|
||||
target: crate::bwrap::ProtectedCreateTarget,
|
||||
marker_file: PathBuf,
|
||||
marker_dir: PathBuf,
|
||||
}
|
||||
|
||||
struct ProtectedCreateMonitor {
|
||||
stop: Arc<AtomicBool>,
|
||||
violation: Arc<AtomicBool>,
|
||||
handle: thread::JoinHandle<()>,
|
||||
}
|
||||
|
||||
struct ProtectedCreateWatcher {
|
||||
fd: libc::c_int,
|
||||
_watches: Vec<libc::c_int>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
enum ProtectedCreateRemoval {
|
||||
Directory,
|
||||
Other,
|
||||
}
|
||||
|
||||
#[derive(Debug, Parser)]
|
||||
/// CLI surface for the Linux sandbox helper.
|
||||
///
|
||||
@@ -486,7 +455,6 @@ fn build_bwrap_argv(
|
||||
args: argv,
|
||||
preserved_files: bwrap_args.preserved_files,
|
||||
synthetic_mount_targets: bwrap_args.synthetic_mount_targets,
|
||||
protected_create_targets: bwrap_args.protected_create_targets,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -574,9 +542,7 @@ fn resolve_true_command() -> String {
|
||||
}
|
||||
|
||||
fn run_or_exec_bwrap(bwrap_args: crate::bwrap::BwrapArgs) -> ! {
|
||||
if bwrap_args.synthetic_mount_targets.is_empty()
|
||||
&& bwrap_args.protected_create_targets.is_empty()
|
||||
{
|
||||
if bwrap_args.synthetic_mount_targets.is_empty() {
|
||||
exec_bwrap(bwrap_args.args, bwrap_args.preserved_files);
|
||||
}
|
||||
run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args);
|
||||
@@ -587,13 +553,9 @@ fn run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args: crate::bwrap::Bwr
|
||||
args,
|
||||
preserved_files,
|
||||
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);
|
||||
let exec_start_pipe = create_exec_start_pipe(!protected_create_targets.is_empty());
|
||||
let parent_pid = unsafe { libc::getpid() };
|
||||
let pid = unsafe { libc::fork() };
|
||||
if pid < 0 {
|
||||
@@ -610,219 +572,19 @@ fn run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args: crate::bwrap::Bwr
|
||||
panic!("failed to place bubblewrap child in its own process group: {err}");
|
||||
}
|
||||
terminate_with_parent(parent_pid);
|
||||
wait_for_parent_exec_start(exec_start_pipe[0], exec_start_pipe[1]);
|
||||
exec_bwrap(args, preserved_files);
|
||||
}
|
||||
|
||||
drop(preserved_files);
|
||||
close_child_exec_start_read(exec_start_pipe[0]);
|
||||
let protected_create_monitor = ProtectedCreateMonitor::start(&protected_create_targets);
|
||||
let signal_forwarders = install_bwrap_signal_forwarders(pid);
|
||||
release_child_exec_start(exec_start_pipe[1]);
|
||||
setup_signal_mask.restore();
|
||||
let status = wait_for_bwrap_child(pid);
|
||||
let cleanup_signal_mask = ForwardedSignalMask::block();
|
||||
BWRAP_CHILD_PID.store(0, Ordering::SeqCst);
|
||||
let protected_create_monitor_violation = protected_create_monitor
|
||||
.map(ProtectedCreateMonitor::stop)
|
||||
.unwrap_or(false);
|
||||
cleanup_synthetic_mount_targets(&synthetic_mount_registrations);
|
||||
let protected_create_violation = protected_create_monitor_violation
|
||||
|| cleanup_protected_create_targets(&protected_create_registrations);
|
||||
signal_forwarders.restore();
|
||||
cleanup_signal_mask.restore();
|
||||
exit_with_wait_status_or_policy_violation(status, protected_create_violation);
|
||||
}
|
||||
|
||||
impl ProtectedCreateMonitor {
|
||||
fn start(targets: &[crate::bwrap::ProtectedCreateTarget]) -> Option<Self> {
|
||||
if targets.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let targets = targets.to_vec();
|
||||
let stop = Arc::new(AtomicBool::new(false));
|
||||
let violation = Arc::new(AtomicBool::new(false));
|
||||
let monitor_stop = Arc::clone(&stop);
|
||||
let monitor_violation = Arc::clone(&violation);
|
||||
let handle = thread::spawn(move || {
|
||||
let watcher = ProtectedCreateWatcher::new(&targets);
|
||||
while !monitor_stop.load(Ordering::SeqCst) {
|
||||
for target in &targets {
|
||||
if remove_protected_create_target_best_effort(target).is_some() {
|
||||
monitor_violation.store(true, Ordering::SeqCst);
|
||||
}
|
||||
}
|
||||
if let Some(watcher) = &watcher {
|
||||
watcher.wait_for_create_event(&monitor_stop);
|
||||
} else {
|
||||
thread::sleep(Duration::from_millis(1));
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
Some(Self {
|
||||
stop,
|
||||
violation,
|
||||
handle,
|
||||
})
|
||||
}
|
||||
|
||||
fn stop(self) -> bool {
|
||||
self.stop.store(true, Ordering::SeqCst);
|
||||
self.handle
|
||||
.join()
|
||||
.unwrap_or_else(|_| panic!("protected create monitor thread panicked"));
|
||||
self.violation.load(Ordering::SeqCst)
|
||||
}
|
||||
}
|
||||
|
||||
impl ProtectedCreateWatcher {
|
||||
fn new(targets: &[crate::bwrap::ProtectedCreateTarget]) -> Option<Self> {
|
||||
let fd = unsafe { libc::inotify_init1(libc::IN_NONBLOCK | libc::IN_CLOEXEC) };
|
||||
if fd < 0 {
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut watched_parents = Vec::<PathBuf>::new();
|
||||
let mut watches = Vec::new();
|
||||
for target in targets {
|
||||
let Some(parent) = target.path().parent() else {
|
||||
continue;
|
||||
};
|
||||
if watched_parents.iter().any(|watched| watched == parent) {
|
||||
continue;
|
||||
}
|
||||
watched_parents.push(parent.to_path_buf());
|
||||
let Ok(parent_cstr) = CString::new(parent.as_os_str().as_bytes()) else {
|
||||
continue;
|
||||
};
|
||||
let mask =
|
||||
libc::IN_CREATE | libc::IN_MOVED_TO | libc::IN_DELETE_SELF | libc::IN_MOVE_SELF;
|
||||
let watch = unsafe { libc::inotify_add_watch(fd, parent_cstr.as_ptr(), mask) };
|
||||
if watch >= 0 {
|
||||
watches.push(watch);
|
||||
}
|
||||
}
|
||||
|
||||
if watches.is_empty() {
|
||||
unsafe {
|
||||
libc::close(fd);
|
||||
}
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(Self {
|
||||
fd,
|
||||
_watches: watches,
|
||||
})
|
||||
}
|
||||
|
||||
fn wait_for_create_event(&self, stop: &AtomicBool) {
|
||||
let mut poll_fd = libc::pollfd {
|
||||
fd: self.fd,
|
||||
events: libc::POLLIN,
|
||||
revents: 0,
|
||||
};
|
||||
while !stop.load(Ordering::SeqCst) {
|
||||
let res = unsafe { libc::poll(&mut poll_fd, 1, 10) };
|
||||
if res > 0 {
|
||||
self.drain_events();
|
||||
return;
|
||||
}
|
||||
if res == 0 {
|
||||
return;
|
||||
}
|
||||
let err = std::io::Error::last_os_error();
|
||||
if err.kind() == std::io::ErrorKind::Interrupted {
|
||||
continue;
|
||||
}
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
fn drain_events(&self) {
|
||||
let mut buf = [0_u8; 4096];
|
||||
loop {
|
||||
let read = unsafe { libc::read(self.fd, buf.as_mut_ptr().cast(), buf.len()) };
|
||||
if read > 0 {
|
||||
continue;
|
||||
}
|
||||
if read == 0 {
|
||||
return;
|
||||
}
|
||||
let err = std::io::Error::last_os_error();
|
||||
if err.kind() == std::io::ErrorKind::Interrupted {
|
||||
continue;
|
||||
}
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for ProtectedCreateWatcher {
|
||||
fn drop(&mut self) {
|
||||
unsafe {
|
||||
libc::close(self.fd);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn create_exec_start_pipe(enabled: bool) -> [libc::c_int; 2] {
|
||||
if !enabled {
|
||||
return [-1, -1];
|
||||
}
|
||||
let mut pipe = [-1, -1];
|
||||
if unsafe { libc::pipe2(pipe.as_mut_ptr(), libc::O_CLOEXEC) } < 0 {
|
||||
let err = std::io::Error::last_os_error();
|
||||
panic!("failed to create bubblewrap exec start pipe: {err}");
|
||||
}
|
||||
pipe
|
||||
}
|
||||
|
||||
fn wait_for_parent_exec_start(read_fd: libc::c_int, write_fd: libc::c_int) {
|
||||
if write_fd >= 0 {
|
||||
unsafe {
|
||||
libc::close(write_fd);
|
||||
}
|
||||
}
|
||||
if read_fd < 0 {
|
||||
return;
|
||||
}
|
||||
|
||||
let mut byte = [0_u8; 1];
|
||||
loop {
|
||||
let read = unsafe { libc::read(read_fd, byte.as_mut_ptr().cast(), byte.len()) };
|
||||
if read >= 0 {
|
||||
break;
|
||||
}
|
||||
let err = std::io::Error::last_os_error();
|
||||
if err.kind() != std::io::ErrorKind::Interrupted {
|
||||
break;
|
||||
}
|
||||
}
|
||||
unsafe {
|
||||
libc::close(read_fd);
|
||||
}
|
||||
}
|
||||
|
||||
fn close_child_exec_start_read(read_fd: libc::c_int) {
|
||||
if read_fd >= 0 {
|
||||
unsafe {
|
||||
libc::close(read_fd);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn release_child_exec_start(write_fd: libc::c_int) {
|
||||
if write_fd < 0 {
|
||||
return;
|
||||
}
|
||||
let byte = [0_u8; 1];
|
||||
unsafe {
|
||||
libc::write(write_fd, byte.as_ptr().cast(), byte.len());
|
||||
libc::close(write_fd);
|
||||
}
|
||||
exit_with_wait_status(status);
|
||||
}
|
||||
|
||||
struct ForwardedSignalMask {
|
||||
@@ -1009,37 +771,6 @@ fn register_synthetic_mount_targets(
|
||||
})
|
||||
}
|
||||
|
||||
fn register_protected_create_targets(
|
||||
targets: &[crate::bwrap::ProtectedCreateTarget],
|
||||
) -> Vec<ProtectedCreateTargetRegistration> {
|
||||
with_synthetic_mount_registry_lock(|| {
|
||||
targets
|
||||
.iter()
|
||||
.map(|target| {
|
||||
let marker_dir = synthetic_mount_marker_dir(target.path());
|
||||
fs::create_dir_all(&marker_dir).unwrap_or_else(|err| {
|
||||
panic!(
|
||||
"failed to create protected create marker directory {}: {err}",
|
||||
marker_dir.display()
|
||||
)
|
||||
});
|
||||
let marker_file = marker_dir.join(std::process::id().to_string());
|
||||
fs::write(&marker_file, PROTECTED_CREATE_MARKER).unwrap_or_else(|err| {
|
||||
panic!(
|
||||
"failed to register protected create target {}: {err}",
|
||||
target.path().display()
|
||||
)
|
||||
});
|
||||
ProtectedCreateTargetRegistration {
|
||||
target: target.clone(),
|
||||
marker_file,
|
||||
marker_dir,
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
})
|
||||
}
|
||||
|
||||
fn synthetic_mount_marker_contents(target: &crate::bwrap::SyntheticMountTarget) -> &'static [u8] {
|
||||
if target.preserves_pre_existing_path() {
|
||||
SYNTHETIC_MOUNT_MARKER_EXISTING
|
||||
@@ -1142,129 +873,6 @@ fn cleanup_synthetic_mount_targets(targets: &[SyntheticMountTargetRegistration])
|
||||
});
|
||||
}
|
||||
|
||||
fn cleanup_protected_create_targets(targets: &[ProtectedCreateTargetRegistration]) -> bool {
|
||||
with_synthetic_mount_registry_lock(|| {
|
||||
for target in targets.iter().rev() {
|
||||
match fs::remove_file(&target.marker_file) {
|
||||
Ok(()) => {}
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
|
||||
Err(err) => panic!(
|
||||
"failed to unregister protected create target {}: {err}",
|
||||
target.target.path().display()
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
let mut violation = false;
|
||||
for target in targets.iter().rev() {
|
||||
violation |= remove_protected_create_target(&target.target);
|
||||
match fs::remove_dir(&target.marker_dir) {
|
||||
Ok(()) => {}
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
|
||||
Err(err) if err.kind() == std::io::ErrorKind::DirectoryNotEmpty => {}
|
||||
Err(err) => panic!(
|
||||
"failed to remove protected create marker directory {}: {err}",
|
||||
target.marker_dir.display()
|
||||
),
|
||||
}
|
||||
}
|
||||
violation
|
||||
})
|
||||
}
|
||||
|
||||
fn remove_protected_create_target(target: &crate::bwrap::ProtectedCreateTarget) -> bool {
|
||||
for attempt in 0..100 {
|
||||
match try_remove_protected_create_target(target) {
|
||||
Ok(removal) => return removal.is_some(),
|
||||
Err(err) if err.kind() == std::io::ErrorKind::DirectoryNotEmpty && attempt < 99 => {
|
||||
thread::sleep(Duration::from_millis(1));
|
||||
}
|
||||
Err(err) => {
|
||||
panic!(
|
||||
"failed to remove protected create target {}: {err}",
|
||||
target.path().display()
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
unreachable!("protected create removal retry loop should return or panic")
|
||||
}
|
||||
|
||||
fn remove_protected_create_target_best_effort(
|
||||
target: &crate::bwrap::ProtectedCreateTarget,
|
||||
) -> Option<ProtectedCreateRemoval> {
|
||||
for _ in 0..100 {
|
||||
match try_remove_protected_create_target(target) {
|
||||
Ok(removal) => return removal,
|
||||
Err(err) if err.kind() == std::io::ErrorKind::DirectoryNotEmpty => {
|
||||
thread::sleep(Duration::from_millis(1));
|
||||
}
|
||||
Err(_) => return Some(ProtectedCreateRemoval::Other),
|
||||
}
|
||||
}
|
||||
Some(ProtectedCreateRemoval::Other)
|
||||
}
|
||||
|
||||
fn try_remove_protected_create_target(
|
||||
target: &crate::bwrap::ProtectedCreateTarget,
|
||||
) -> std::io::Result<Option<ProtectedCreateRemoval>> {
|
||||
let path = target.path();
|
||||
let metadata = match fs::symlink_metadata(path) {
|
||||
Ok(metadata) => metadata,
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None),
|
||||
Err(err) => return Err(err),
|
||||
};
|
||||
|
||||
let removal = if metadata.is_dir() {
|
||||
ProtectedCreateRemoval::Directory
|
||||
} else {
|
||||
ProtectedCreateRemoval::Other
|
||||
};
|
||||
let result = if removal == ProtectedCreateRemoval::Directory {
|
||||
match fs::remove_dir_all(path) {
|
||||
Err(err) if err.kind() == std::io::ErrorKind::PermissionDenied => {
|
||||
make_directory_tree_writable(path)?;
|
||||
fs::remove_dir_all(path)
|
||||
}
|
||||
result => result,
|
||||
}
|
||||
} else {
|
||||
fs::remove_file(path)
|
||||
};
|
||||
match result {
|
||||
Ok(()) => {}
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None),
|
||||
Err(err) => return Err(err),
|
||||
}
|
||||
eprintln!(
|
||||
"sandbox blocked creation of protected workspace metadata path {}",
|
||||
path.display()
|
||||
);
|
||||
Ok(Some(removal))
|
||||
}
|
||||
|
||||
fn make_directory_tree_writable(path: &Path) -> std::io::Result<()> {
|
||||
let metadata = match fs::symlink_metadata(path) {
|
||||
Ok(metadata) => metadata,
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(()),
|
||||
Err(err) => return Err(err),
|
||||
};
|
||||
if !metadata.is_dir() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let directory = OpenOptions::new()
|
||||
.read(true)
|
||||
.custom_flags(libc::O_PATH | libc::O_DIRECTORY | libc::O_NOFOLLOW)
|
||||
.open(path)?;
|
||||
let directory_path = PathBuf::from(format!("/proc/self/fd/{}", directory.as_raw_fd()));
|
||||
fs::set_permissions(&directory_path, fs::Permissions::from_mode(0o700))?;
|
||||
for entry in fs::read_dir(directory_path)? {
|
||||
make_directory_tree_writable(&entry?.path())?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn remove_synthetic_mount_target(target: &crate::bwrap::SyntheticMountTarget) {
|
||||
let path = target.path();
|
||||
let metadata = match fs::symlink_metadata(path) {
|
||||
@@ -1405,17 +1013,6 @@ fn exit_with_wait_status(status: libc::c_int) -> ! {
|
||||
std::process::exit(1);
|
||||
}
|
||||
|
||||
fn exit_with_wait_status_or_policy_violation(
|
||||
status: libc::c_int,
|
||||
protected_create_violation: bool,
|
||||
) -> ! {
|
||||
if protected_create_violation && libc::WIFEXITED(status) && libc::WEXITSTATUS(status) == 0 {
|
||||
std::process::exit(1);
|
||||
}
|
||||
|
||||
exit_with_wait_status(status);
|
||||
}
|
||||
|
||||
/// Run a short-lived bubblewrap preflight in a child process and capture stderr.
|
||||
///
|
||||
/// Strategy:
|
||||
@@ -1433,12 +1030,9 @@ fn run_bwrap_in_child_capture_stderr(bwrap_args: crate::bwrap::BwrapArgs) -> Str
|
||||
args,
|
||||
preserved_files,
|
||||
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);
|
||||
|
||||
let mut pipe_fds = [0; 2];
|
||||
let pipe_res = unsafe { libc::pipe2(pipe_fds.as_mut_ptr(), libc::O_CLOEXEC) };
|
||||
@@ -1488,7 +1082,6 @@ fn run_bwrap_in_child_capture_stderr(bwrap_args: crate::bwrap::BwrapArgs) -> Str
|
||||
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);
|
||||
signal_forwarders.restore();
|
||||
cleanup_signal_mask.restore();
|
||||
if libc::WIFSIGNALED(status) {
|
||||
|
||||
@@ -16,8 +16,6 @@ use codex_protocol::protocol::NetworkSandboxPolicy;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
#[cfg(test)]
|
||||
use pretty_assertions::assert_eq;
|
||||
#[cfg(test)]
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
fn read_only_permission_profile() -> PermissionProfile {
|
||||
PermissionProfile::read_only()
|
||||
@@ -458,68 +456,6 @@ fn cleanup_synthetic_mount_targets_preserves_real_pre_existing_empty_file() {
|
||||
assert!(empty_file.exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cleanup_protected_create_targets_removes_created_path_and_reports_violation() {
|
||||
let temp_dir = tempfile::TempDir::new().expect("tempdir");
|
||||
let dot_git = temp_dir.path().join(".git");
|
||||
let target = crate::bwrap::ProtectedCreateTarget::missing(&dot_git);
|
||||
|
||||
let registrations = register_protected_create_targets(&[target]);
|
||||
std::fs::create_dir(&dot_git).expect("create protected path");
|
||||
let violation = cleanup_protected_create_targets(®istrations);
|
||||
|
||||
assert!(violation);
|
||||
assert!(!dot_git.exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cleanup_protected_create_targets_removes_path_despite_active_marker() {
|
||||
let temp_dir = tempfile::TempDir::new().expect("tempdir");
|
||||
let dot_git = temp_dir.path().join(".git");
|
||||
let target = crate::bwrap::ProtectedCreateTarget::missing(&dot_git);
|
||||
|
||||
let registrations = register_protected_create_targets(std::slice::from_ref(&target));
|
||||
let active_marker = registrations[0].marker_dir.join("1");
|
||||
std::fs::write(&active_marker, PROTECTED_CREATE_MARKER).expect("write active marker");
|
||||
std::fs::write(&dot_git, "").expect("create protected path");
|
||||
|
||||
let violation = cleanup_protected_create_targets(®istrations);
|
||||
assert!(violation);
|
||||
assert!(!dot_git.exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cleanup_protected_create_targets_removes_read_only_directory_and_reports_violation() {
|
||||
let temp_dir = tempfile::TempDir::new().expect("tempdir");
|
||||
let dot_git = temp_dir.path().join(".git");
|
||||
let outside = temp_dir.path().join("outside");
|
||||
let target = crate::bwrap::ProtectedCreateTarget::missing(&dot_git);
|
||||
|
||||
let registrations = register_protected_create_targets(&[target]);
|
||||
std::fs::create_dir(&outside).expect("create outside directory");
|
||||
std::fs::set_permissions(&outside, std::fs::Permissions::from_mode(0o755))
|
||||
.expect("set outside directory permissions");
|
||||
std::fs::create_dir(&dot_git).expect("create protected path");
|
||||
std::fs::write(dot_git.join("config"), "[core]\n").expect("write protected child");
|
||||
std::os::unix::fs::symlink(&outside, dot_git.join("outside-link"))
|
||||
.expect("link outside directory");
|
||||
std::fs::set_permissions(&dot_git, std::fs::Permissions::from_mode(0o000))
|
||||
.expect("make protected path read-only");
|
||||
|
||||
let violation = cleanup_protected_create_targets(®istrations);
|
||||
|
||||
assert!(violation);
|
||||
assert!(!dot_git.exists());
|
||||
assert_eq!(
|
||||
std::fs::metadata(&outside)
|
||||
.expect("outside directory remains")
|
||||
.permissions()
|
||||
.mode()
|
||||
& 0o777,
|
||||
0o755
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bwrap_signal_forwarder_terminates_child_and_keeps_parent_alive() {
|
||||
let supervisor_pid = unsafe { libc::fork() };
|
||||
|
||||
Reference in New Issue
Block a user