Skip to content

Fix Linux sandbox startup with multiple denied files #50059

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
copyberry merged 1 commit into from
Oct 1, 2026
Merged

Fix Linux sandbox startup with multiple denied files #50059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Fix Linux sandbox startup with multiple denied files (#50059) 
## Why

Bubblewrap consumes and closes the file descriptor for each `--ro-bind-data` mount. Reusing one descriptor across multiple file masks prevents the sandbox from starting.

## What changed

Open and preserve a separate `/dev/null` descriptor for each empty-file mask.

## Testing

Extend unit tests to verify distinct descriptors for multiple denied files and missing-path masks. Add integration coverage for exact-path and glob deny rules, checking that the sandbox starts, denied files remain unreadable and unwritable, and allowed files remain accessible.

GitOrigin-RevId: 85158c83699afd36bb7aca453b2652d777cf65e4
  • Loading branch information
@yuzhu-oai copyberry
yuzhu-oai authored and copyberry committed Oct 1, 2026
commit ae7aad586ea0769f329e214e9827c541823fd191
69 changes: 54 additions & 15 deletions codex-rs/linux-sandbox/src/bwrap.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1253,10 +1253,10 @@ fn append_read_only_subpath_args(
}

fn append_empty_file_bind_data_args(bwrap_args: &mut BwrapArgs, path: &Path) -> Result<()> {
if bwrap_args.preserved_files.is_empty() {
bwrap_args.preserved_files.push(File::open("/dev/null")?);
}
let null_fd = bwrap_args.preserved_files[0].as_raw_fd().to_string();
// Bubblewrap consumes and closes the descriptor for each bind-data mount.
let null_file = File::open("/dev/null")?;
let null_fd = null_file.as_raw_fd().to_string();
bwrap_args.preserved_files.push(null_file);
bwrap_args.args.push("--ro-bind-data".to_string());
bwrap_args.args.push(null_fd);
bwrap_args.args.push(path_to_string(path));
Expand Down Expand Up @@ -2005,6 +2005,7 @@ mod tests {
let temp_dir = TempDir::new().expect("temp dir");
let workspace = temp_dir.path().join("workspace");
let blocked = workspace.join("blocked");
let second_blocked = workspace.join("second-blocked");
std::fs::create_dir_all(&workspace).expect("create workspace");

let workspace_root =
Expand All @@ -2021,28 +2022,38 @@ mod tests {
access: FileSystemAccessMode::Read,
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: AbsolutePathBuf::from_absolute_path(&second_blocked)
.expect("absolute second blocked")
.into(),
access: FileSystemAccessMode::Read,
missing_path_behavior: None,
},
]);

let args = create_filesystem_args(&policy, temp_dir.path(), BwrapOptions::default())
.expect("filesystem args");

assert_empty_file_bound_without_perms(&args.args, &blocked);
assert_empty_file_bound_without_perms(&args.args, &second_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);
assert_eq!(args.preserved_files.len(), 2);
assert_bind_data_uses_distinct_preserved_fds(&args);
assert_eq!(
synthetic_mount_target_paths(&args),
vec![
blocked.clone(),
second_blocked.clone(),
workspace.join(".git"),
workspace.join(".agents"),
workspace.join(".codex"),
workspace.join(".aws"),
]
);
assert!(
!blocked.exists(),
!blocked.exists() && !second_blocked.exists(),
"missing path mask should not materialize host-side metadata paths at arg construction time",
);
}
Expand Down Expand Up @@ -2848,7 +2859,9 @@ mod tests {
fn split_policy_masks_root_read_file_carveouts() {
let temp_dir = TempDir::new().expect("temp dir");
let blocked_file = temp_dir.path().join("blocked.txt");
let second_blocked_file = temp_dir.path().join("second-blocked.txt");
std::fs::write(&blocked_file, "secret").expect("create blocked file");
std::fs::write(&second_blocked_file, "dummy secret").expect("create second blocked file");
let blocked_file =
AbsolutePathBuf::from_absolute_path(&blocked_file).expect("absolute blocked file");
let policy = FileSystemSandboxPolicy::restricted(vec![
Expand All @@ -2864,20 +2877,46 @@ mod tests {
access: FileSystemAccessMode::Deny,
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: AbsolutePathBuf::from_absolute_path(&second_blocked_file)
.expect("absolute second blocked file")
.into(),
access: FileSystemAccessMode::Deny,
missing_path_behavior: None,
},
]);

let args = create_filesystem_args(&policy, temp_dir.path(), BwrapOptions::default())
.expect("filesystem args");
let blocked_file_str = path_to_string(blocked_file.as_path());

assert_eq!(args.preserved_files.len(), 1);
assert_eq!(args.preserved_files.len(), 2);
assert_bind_data_uses_distinct_preserved_fds(&args);
assert!(args.synthetic_mount_targets.is_empty());
assert!(args.args.windows(5).any(|window| {
window[0] == "--perms"
&& window[1] == "000"
&& window[2] == "--ro-bind-data"
&& window[4] == blocked_file_str
}));
for blocked_file in [blocked_file.as_path(), second_blocked_file.as_path()] {
assert!(args.args.windows(5).any(|window| {
window[0] == "--perms"
&& window[1] == "000"
&& window[2] == "--ro-bind-data"
&& window[4] == path_to_string(blocked_file)
}));
}
}

fn assert_bind_data_uses_distinct_preserved_fds(args: &BwrapArgs) {
let mount_fds = args
.args
.windows(3)
.filter(|window| window[0] == "--ro-bind-data")
.map(|window| window[1].parse::<i32>().expect("bind-data fd"))
.collect::<Vec<_>>();
let distinct_fds = mount_fds.iter().copied().collect::<HashSet<_>>();
assert_eq!(mount_fds.len(), distinct_fds.len());
assert_eq!(
distinct_fds,
args.preserved_files
.iter()
.map(AsRawFd::as_raw_fd)
.collect()
);
}

#[test]
Expand Down
127 changes: 127 additions & 0 deletions codex-rs/linux-sandbox/tests/suite/denied_files_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
//! Multiple denied files must not prevent sandbox startup or expose protected contents.

use super::LONG_TIMEOUT_MS;
use super::codex_linux_sandbox_exe;
use super::create_env_from_core_vars;
use super::run_cmd_result_with_permission_profile_for_cwd;
use super::should_skip_bwrap_tests;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;

enum DeniedFileRules {
ExactPaths,
Globs,
}

#[test_case::test_case(DeniedFileRules::ExactPaths; "exact_paths")]
#[test_case::test_case(DeniedFileRules::Globs; "globs")]
#[tokio::test]
async fn sandbox_starts_with_multiple_denied_files(rules: DeniedFileRules) {
if should_skip_bwrap_tests().await {
eprintln!("skipping bwrap test: bwrap sandbox prerequisites are unavailable");
return;
}

let temp = tempfile::tempdir().expect("tempdir");
let workspace = AbsolutePathBuf::try_from(temp.path()).expect("absolute workspace");
std::fs::create_dir(workspace.join("nested")).expect("create nested directory");
std::fs::write(workspace.join("AGENTS.md"), "project instructions\n")
.expect("write allowed instructions");
let denied_files = [
(workspace.join("one.key"), "dummy first key"),
(workspace.join("nested/two.key"), "dummy second key"),
(workspace.join(".env.local"), "dummy environment"),
];
for (path, contents) in &denied_files {
std::fs::write(path, contents).expect("write denied dummy file");
}

let sandbox_helper = codex_linux_sandbox_exe();
let helper_dir = AbsolutePathBuf::try_from(sandbox_helper.parent().expect("helper parent"))
.expect("absolute helper directory");
let mut entries = vec![
FileSystemSandboxEntry::new(
FileSystemPath::Special {
value: FileSystemSpecialPath::Minimal,
},
FileSystemAccessMode::Read,
),
FileSystemSandboxEntry::new(helper_dir.into(), FileSystemAccessMode::Read),
FileSystemSandboxEntry::new(workspace.clone().into(), FileSystemAccessMode::Write),
];
match rules {
DeniedFileRules::ExactPaths => {
entries.extend(denied_files.iter().map(|(path, _)| {
FileSystemSandboxEntry::new(path.clone().into(), FileSystemAccessMode::Deny)
}));
}
DeniedFileRules::Globs => {
for suffix in ["**/*.key", "**/.env.local"] {
entries.push(FileSystemSandboxEntry::new(
FileSystemPath::GlobPattern {
pattern: format!("{}/{suffix}", workspace.display()),
},
FileSystemAccessMode::Deny,
));
}
}
}
let permission_profile = PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::restricted(entries),
NetworkSandboxPolicy::Enabled,
);
let output = run_cmd_result_with_permission_profile_for_cwd(
&[
"/bin/sh",
"-c",
r#"set -eu
cat AGENTS.md
for blocked in one.key nested/two.key .env.local; do
if (: < "$blocked") 2>/dev/null; then
printf 'read unexpectedly allowed: %s\n' "$blocked" >&2
exit 10
fi
if (printf changed > "$blocked") 2>/dev/null; then
printf 'write unexpectedly allowed: %s\n' "$blocked" >&2
exit 11
fi
done
printf 'allowed\n' > allowed.txt
cat allowed.txt
"#,
],
workspace.clone(),
permission_profile,
create_env_from_core_vars(),
LONG_TIMEOUT_MS,
/*use_legacy_landlock*/ false,
)
.await
.expect("sandbox should start with multiple denied files");

assert_eq!(
(output.exit_code, output.stdout.text, output.stderr.text),
(
0,
"project instructions\nallowed\n".to_string(),
String::new()
)
);
assert_eq!(
std::fs::read_to_string(workspace.join("allowed.txt")).expect("read allowed file"),
"allowed\n"
);
for (path, contents) in &denied_files {
assert_eq!(
std::fs::read_to_string(path).expect("read host dummy file"),
*contents
);
}
}
3 changes: 3 additions & 0 deletions codex-rs/linux-sandbox/tests/suite/landlock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ mod nested_metadata_tests;
#[path = "root_metadata_tests.rs"]
mod root_metadata_tests;

#[path = "denied_files_tests.rs"]
mod denied_files_tests;

// At least on GitHub CI, the arm64 tests appear to need longer timeouts.

#[cfg(not(target_arch = "aarch64"))]
Expand Down
Loading