openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-08 15:50:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: hide preserved path masks from git status

Browse Source 
Represent missing .agents and .codex preserved path masks as read-only empty directories instead of empty files so Git ignores them while cleanup still removes the host mount targets after bwrap exits.

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
viyatb-oai
2026-04-23 14:18:18 -07:00
committed by Eva Wong
parent 87266432a0
commit eb7a3bfa0c
4 changed files with 198 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

185
codex-rs/linux-sandbox/src/bwrap.rs
View File

@@ -26,6 +26,7 @@ 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;
@@ -122,44 +123,82 @@ impl FileIdentity {
}
}
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(crate) enum SyntheticMountTargetKind {
EmptyFile,
EmptyDirectory,
}
#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) struct SyntheticMountTarget {
path: PathBuf,
kind: SyntheticMountTargetKind,
// If an empty preserved path was already present, remember its inode so
// cleanup does not delete a real pre-existing file.
pre_existing_file: Option<FileIdentity>,
// cleanup does not delete a real pre-existing file or directory.
pre_existing_path: Option<FileIdentity>,
}
impl SyntheticMountTarget {
pub(crate) fn missing(path: &Path) -> Self {
Self {
path: path.to_path_buf(),
pre_existing_file: None,
kind: SyntheticMountTargetKind::EmptyFile,
pre_existing_path: None,
}
}
pub(crate) fn missing_empty_directory(path: &Path) -> Self {
Self {
path: path.to_path_buf(),
kind: SyntheticMountTargetKind::EmptyDirectory,
pre_existing_path: None,
}
}
pub(crate) fn existing_empty_file(path: &Path, metadata: &Metadata) -> Self {
Self {
path: path.to_path_buf(),
pre_existing_file: Some(FileIdentity::from_metadata(metadata)),
kind: SyntheticMountTargetKind::EmptyFile,
pre_existing_path: Some(FileIdentity::from_metadata(metadata)),
}
}
pub(crate) fn preserves_pre_existing_file(&self) -> bool {
self.pre_existing_file.is_some()
fn existing_empty_directory(path: &Path, metadata: &Metadata) -> Self {
Self {
path: path.to_path_buf(),
kind: SyntheticMountTargetKind::EmptyDirectory,
pre_existing_path: Some(FileIdentity::from_metadata(metadata)),
}
}
pub(crate) fn preserves_pre_existing_path(&self) -> bool {
self.pre_existing_path.is_some()
}
pub(crate) fn path(&self) -> &Path {
&self.path
}
pub(crate) fn kind(&self) -> SyntheticMountTargetKind {
self.kind
}
pub(crate) fn should_remove_after_bwrap(&self, metadata: &Metadata) -> bool {
if !metadata.file_type().is_file() || metadata.len() != 0 {
return false;
match self.kind {
SyntheticMountTargetKind::EmptyFile => {
if !metadata.file_type().is_file() || metadata.len() != 0 {
return false;
}
}
SyntheticMountTargetKind::EmptyDirectory => {
if !metadata.file_type().is_dir() {
return false;
}
}
}
match self.pre_existing_file {
Some(pre_existing_file) => pre_existing_file != FileIdentity::from_metadata(metadata),
match self.pre_existing_path {
Some(pre_existing_path) => pre_existing_path != FileIdentity::from_metadata(metadata),
None => true,
}
}
@@ -856,13 +895,20 @@ fn append_read_only_subpath_args(
)));
}
if let Some(metadata) = transient_empty_preserved_file_metadata(subpath)
if let Some(metadata) = transient_empty_preserved_path_metadata(subpath)
&& is_within_allowed_write_paths(subpath, allowed_write_paths)
{
// Another concurrent bwrap setup can leave a zero-byte mount target at
// Another concurrent bwrap setup can leave an empty mount target at
// a missing preserved path. Treat it like the missing case instead of
// binding that transient host path as the stable source.
append_existing_empty_file_bind_data_args(bwrap_args, subpath, &metadata)?;
match metadata {
EmptyPreservedPathMetadata::File(metadata) => {
append_existing_empty_file_bind_data_args(bwrap_args, subpath, &metadata)?;
}
EmptyPreservedPathMetadata::Directory(metadata) => {
append_existing_empty_directory_args(bwrap_args, subpath, &metadata);
}
}
return Ok(());
}
@@ -870,7 +916,7 @@ fn append_read_only_subpath_args(
if let Some(first_missing_component) = find_first_non_existent_component(subpath)
&& is_within_allowed_write_paths(&first_missing_component, allowed_write_paths)
{
append_missing_empty_file_bind_data_args(bwrap_args, &first_missing_component)?;
append_missing_read_only_subpath_args(bwrap_args, &first_missing_component)?;
}
return Ok(());
}
@@ -894,6 +940,30 @@ fn append_empty_file_bind_data_args(bwrap_args: &mut BwrapArgs, path: &Path) ->
Ok(())
}
fn append_empty_directory_args(bwrap_args: &mut BwrapArgs, path: &Path) {
bwrap_args.args.push("--perms".to_string());
bwrap_args.args.push("555".to_string());
bwrap_args.args.push("--tmpfs".to_string());
bwrap_args.args.push(path_to_string(path));
bwrap_args.args.push("--remount-ro".to_string());
bwrap_args.args.push(path_to_string(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)
{
append_empty_directory_args(bwrap_args, path);
bwrap_args
.synthetic_mount_targets
.push(SyntheticMountTarget::missing_empty_directory(path));
return Ok(());
}
append_missing_empty_file_bind_data_args(bwrap_args, path)
}
fn append_missing_empty_file_bind_data_args(bwrap_args: &mut BwrapArgs, path: &Path) -> Result<()> {
append_empty_file_bind_data_args(bwrap_args, path)?;
bwrap_args
@@ -914,6 +984,19 @@ fn append_existing_empty_file_bind_data_args(
Ok(())
}
fn append_existing_empty_directory_args(
bwrap_args: &mut BwrapArgs,
path: &Path,
metadata: &Metadata,
) {
append_empty_directory_args(bwrap_args, path);
bwrap_args
.synthetic_mount_targets
.push(SyntheticMountTarget::existing_empty_directory(
path, metadata,
));
}
fn append_unreadable_root_args(
bwrap_args: &mut BwrapArgs,
unreadable_root: &Path,
@@ -999,17 +1082,33 @@ fn is_within_allowed_write_paths(path: &Path, allowed_write_paths: &[PathBuf]) -
.any(|root| path.starts_with(root))
}
fn transient_empty_preserved_file_metadata(path: &Path) -> Option<Metadata> {
enum EmptyPreservedPathMetadata {
File(Metadata),
Directory(Metadata),
}
fn transient_empty_preserved_path_metadata(path: &Path) -> Option<EmptyPreservedPathMetadata> {
if !path.file_name().is_some_and(is_preserved_path_name) {
return None;
}
let metadata = fs::symlink_metadata(path).ok()?;
if metadata.file_type().is_file() && metadata.len() == 0 {
Some(metadata)
} else {
None
return Some(EmptyPreservedPathMetadata::File(metadata));
}
if metadata.file_type().is_dir() && directory_is_empty(path) {
return Some(EmptyPreservedPathMetadata::Directory(metadata));
}
None
}
fn directory_is_empty(path: &Path) -> bool {
let Ok(mut entries) = fs::read_dir(path) else {
return false;
};
entries.next().is_none()
}
fn first_writable_symlink_component_in_path(
@@ -1479,6 +1578,8 @@ mod tests {
.expect("filesystem args");
assert_empty_file_bound_without_perms(&args.args, &blocked);
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);
assert_eq!(
synthetic_mount_target_paths(&args),
@@ -1518,6 +1619,8 @@ mod tests {
let dot_git_str = path_to_string(&dot_git);
assert_empty_file_bound_without_perms(&args.args, &dot_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!(
synthetic_mount_target_paths(&args),
vec![
@@ -1656,11 +1759,17 @@ mod tests {
"--ro-bind-data".to_string(),
null_fd.clone(),
"/.git".to_string(),
"--ro-bind-data".to_string(),
null_fd.clone(),
"--perms".to_string(),
"555".to_string(),
"--tmpfs".to_string(),
"/.agents".to_string(),
"--ro-bind-data".to_string(),
null_fd.clone(),
"--remount-ro".to_string(),
"/.agents".to_string(),
"--perms".to_string(),
"555".to_string(),
"--tmpfs".to_string(),
"/.codex".to_string(),
"--remount-ro".to_string(),
"/.codex".to_string(),
// Rebind /dev after the root bind so device nodes remain
// writable/usable inside the writable root.
@@ -1668,13 +1777,19 @@ mod tests {
"/dev".to_string(),
"/dev".to_string(),
"--ro-bind-data".to_string(),
null_fd.clone(),
"/dev/.git".to_string(),
"--ro-bind-data".to_string(),
null_fd.clone(),
"/dev/.agents".to_string(),
"--ro-bind-data".to_string(),
null_fd,
"/dev/.git".to_string(),
"--perms".to_string(),
"555".to_string(),
"--tmpfs".to_string(),
"/dev/.agents".to_string(),
"--remount-ro".to_string(),
"/dev/.agents".to_string(),
"--perms".to_string(),
"555".to_string(),
"--tmpfs".to_string(),
"/dev/.codex".to_string(),
"--remount-ro".to_string(),
"/dev/.codex".to_string(),
]
);
@@ -2264,6 +2379,20 @@ mod tests {
);
}
fn assert_empty_directory_mounted_read_only(args: &[String], path: &Path) {
let path = path_to_string(path);
assert!(
args.windows(4)
.any(|window| window == ["--perms", "555", "--tmpfs", path.as_str()]),
"expected empty directory mount for {path}: {args:#?}"
);
assert!(
args.windows(2)
.any(|window| window == ["--remount-ro", path.as_str()]),
"expected read-only remount for {path}: {args:#?}"
);
}
fn synthetic_mount_target_paths(args: &BwrapArgs) -> Vec<PathBuf> {
args.synthetic_mount_targets
.iter()

40
codex-rs/linux-sandbox/src/linux_run_main.rs
View File

@@ -694,10 +694,19 @@ fn register_synthetic_mount_targets(
marker_dir.display()
)
});
let target = if target.preserves_pre_existing_file()
let target = if target.preserves_pre_existing_path()
&& synthetic_mount_marker_dir_has_active_synthetic_owner(&marker_dir)
{
crate::bwrap::SyntheticMountTarget::missing(target.path())
match target.kind() {
crate::bwrap::SyntheticMountTargetKind::EmptyFile => {
crate::bwrap::SyntheticMountTarget::missing(target.path())
}
crate::bwrap::SyntheticMountTargetKind::EmptyDirectory => {
crate::bwrap::SyntheticMountTarget::missing_empty_directory(
target.path(),
)
}
}
} else {
target.clone()
};
@@ -721,7 +730,7 @@ fn register_synthetic_mount_targets(
}
fn synthetic_mount_marker_contents(target: &crate::bwrap::SyntheticMountTarget) -> &'static [u8] {
if target.preserves_pre_existing_file() {
if target.preserves_pre_existing_path() {
SYNTHETIC_MOUNT_MARKER_EXISTING
} else {
SYNTHETIC_MOUNT_MARKER_SYNTHETIC
@@ -835,13 +844,24 @@ fn remove_synthetic_mount_target(target: &crate::bwrap::SyntheticMountTarget) {
if !target.should_remove_after_bwrap(&metadata) {
return;
}
match fs::remove_file(path) {
Ok(()) => {}
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
Err(err) => panic!(
"failed to remove synthetic bubblewrap mount target {}: {err}",
path.display()
),
match target.kind() {
crate::bwrap::SyntheticMountTargetKind::EmptyFile => match fs::remove_file(path) {
Ok(()) => {}
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
Err(err) => panic!(
"failed to remove synthetic bubblewrap mount target {}: {err}",
path.display()
),
},
crate::bwrap::SyntheticMountTargetKind::EmptyDirectory => match fs::remove_dir(path) {
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 synthetic bubblewrap mount target {}: {err}",
path.display()
),
},
}
}

10
codex-rs/linux-sandbox/src/linux_run_main_tests.rs
View File

@@ -260,22 +260,26 @@ fn managed_proxy_preflight_argv_is_wrapped_for_full_access_policy() {
}
#[test]
fn cleanup_synthetic_mount_targets_removes_only_empty_files() {
fn cleanup_synthetic_mount_targets_removes_only_empty_mount_targets() {
let temp_dir = tempfile::TempDir::new().expect("tempdir");
let empty_file = temp_dir.path().join(".agents");
let non_empty_file = temp_dir.path().join(".codex");
let empty_file = temp_dir.path().join(".git");
let empty_dir = temp_dir.path().join(".agents");
let non_empty_file = temp_dir.path().join("non-empty");
let missing_file = temp_dir.path().join(".missing");
std::fs::write(&empty_file, "").expect("write empty file");
std::fs::create_dir(&empty_dir).expect("create empty dir");
std::fs::write(&non_empty_file, "keep").expect("write nonempty file");
let registrations = register_synthetic_mount_targets(&[
crate::bwrap::SyntheticMountTarget::missing(&empty_file),
crate::bwrap::SyntheticMountTarget::missing_empty_directory(&empty_dir),
crate::bwrap::SyntheticMountTarget::missing(&non_empty_file),
crate::bwrap::SyntheticMountTarget::missing(&missing_file),
]);
cleanup_synthetic_mount_targets(&registrations);
assert!(!empty_file.exists());
assert!(!empty_dir.exists());
assert_eq!(
std::fs::read_to_string(&non_empty_file).expect("read nonempty file"),
"keep"

4
codex-rs/protocol/src/permissions.rs
View File

@@ -37,6 +37,10 @@ pub fn is_preserved_path_name(name: &OsStr) -> bool {
.any(|preserved| name == OsStr::new(preserved))
}
pub fn is_preserved_directory_path_name(name: &OsStr) -> bool {
name == OsStr::new(PRESERVED_AGENTS_PATH_NAME) || name == OsStr::new(PRESERVED_CODEX_PATH_NAME)
}
/// Returns the preserved workspace metadata name when an agent write to `path`
/// should be blocked before execution.
pub fn forbidden_agent_preserved_path_write(