openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-14 16:38:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

Protect ripgrep lookup during Linux sandbox construction (#51407)

Browse Source 
## Why

Deny-read glob expansion runs before sandbox confinement. Resolving `rg` directly from `PATH` could execute a writable workspace binary at that stage.

## What changed

Use the existing protected executable lookup to select a canonical `rg` path under the filesystem policy. Fall back to the native glob walker when no suitable executable is found, preserving masks for matching denied paths.

## Testing

Extend denied-file integration tests to verify that a writable workspace `rg` never runs, a protected `rg` is used when available, and denied files remain blocked with the native fallback.

GitOrigin-RevId: f06674f3535c8a72a4656871762e45fa13c732f2
This commit is contained in:
viyatb-oai
2026-10-06 16:30:34 +00:00
committed by copyberry
parent 551bd409eb
commit ccde2fc8b7
2 changed files with 102 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/linux-sandbox/src/bwrap.rs
+13 -3
View File
@@ -37,6 +37,7 @@ use codex_protocol::protocol::FileSystemPath;
use codex_protocol::protocol::FileSystemSandboxPolicy;
use codex_protocol::protocol::FileSystemSpecialPath;
use codex_protocol::protocol::WritableRoot;
use codex_sandboxing::find_pre_sandbox_executable_in_path;
use codex_utils_absolute_path::AbsolutePathBuf;
use globset::GlobBuilder;
use globset::GlobSet;
@@ -498,6 +499,7 @@ fn create_filesystem_args(
expand_unreadable_globs_with_ripgrep(
&unreadable_globs,
cwd,
file_system_sandbox_policy,
options
.glob_scan_max_depth
.or(file_system_sandbox_policy.glob_scan_max_depth),
@@ -889,12 +891,15 @@ fn append_daemon_socket_masks(
fn expand_unreadable_globs_with_ripgrep(
patterns: &[String],
cwd: &Path,
file_system_policy: &FileSystemSandboxPolicy,
max_depth: Option<usize>,
) -> Result<Vec<AbsolutePathBuf>> {
if patterns.is_empty() || max_depth == Some(0) {
return Ok(Vec::new());
}
let rg_path = find_pre_sandbox_executable_in_path("rg", file_system_policy, cwd);
// Group each pattern by the static path prefix before its first glob
// metacharacter. That keeps scans narrow, avoids searching from `/`, and
// lets one `rg --files` call handle all patterns under the same root.
@@ -918,7 +923,11 @@ fn expand_unreadable_globs_with_ripgrep(
// bypassing an unreadable glob match.
let mut expanded_paths = BTreeSet::new();
for (search_root, globs) in patterns_by_search_root {
for path in ripgrep_files(search_root.as_path(), &globs, max_depth)? {
let paths = match &rg_path {
Some(rg_path) => ripgrep_files(rg_path, search_root.as_path(), &globs, max_depth)?,
None => glob_files(search_root.as_path(), &globs, max_depth)?,
};
for path in paths {
if let Some(target) = canonical_target_if_symlinked_path(path.as_path()) {
expanded_paths.insert(AbsolutePathBuf::from_absolute_path_checked(target)?);
}
@@ -1000,6 +1009,7 @@ fn escape_unclosed_glob_classes(glob: &str) -> String {
}
fn ripgrep_files(
rg_path: &Path,
search_root: &Path,
globs: &[String],
max_depth: Option<usize>,
@@ -1007,7 +1017,7 @@ fn ripgrep_files(
// Use `rg --files` rather than shell expansion so dotfiles and ignored files
// are still considered. A status 1 with no stderr is ripgrep's "no matches"
// case, not a sandbox construction error.
let mut command = Command::new("rg");
let mut command = Command::new(rg_path);
command
.arg("--files")
.arg("--hidden")
@@ -1025,7 +1035,7 @@ fn ripgrep_files(
* Prefer ripgrep for unreadable glob expansion because it is fast and
* already implements the file-walking semantics we want here: include
* dotfiles, ignore ignore files, and do not recurse through symlinked
* directories. If `rg` is not installed in the runtime environment, fall
* directories. If the selected `rg` is unavailable at execution time, fall
* back to the internal globset walker so sandbox construction still masks
* matching paths. Other ripgrep failures stay fatal so deny-read does not
* silently weaken.
codex-rs/linux-sandbox/tests/suite/denied_files_tests.rs
+89 -5
View File
@@ -13,15 +13,20 @@ use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_cargo_bin::write_executable;
use pretty_assertions::assert_eq;
enum DeniedFileRules {
ExactPaths,
Globs,
GlobsWithProtectedRg,
GlobsWithNativeFallback,
}
#[test_case::test_case(DeniedFileRules::ExactPaths; "exact_paths")]
#[test_case::test_case(DeniedFileRules::Globs; "globs")]
#[test_case::test_case(DeniedFileRules::GlobsWithProtectedRg; "globs_with_protected_rg")]
#[test_case::test_case(DeniedFileRules::GlobsWithNativeFallback; "globs_with_native_fallback")]
#[tokio::test]
async fn sandbox_starts_with_multiple_denied_files(rules: DeniedFileRules) {
if should_skip_bwrap_tests().await {
@@ -29,7 +34,18 @@ async fn sandbox_starts_with_multiple_denied_files(rules: DeniedFileRules) {
return;
}
let temp = tempfile::tempdir().expect("tempdir");
let use_controlled_rg_path = matches!(
rules,
DeniedFileRules::GlobsWithProtectedRg | DeniedFileRules::GlobsWithNativeFallback
);
let temp = if use_controlled_rg_path {
// Bazel can mount the default temp directory noexec.
tempfile::tempdir_in(std::env::current_dir().expect("current directory"))
.or_else(|_| tempfile::tempdir())
} else {
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")
@@ -56,13 +72,71 @@ async fn sandbox_starts_with_multiple_denied_files(rules: DeniedFileRules) {
FileSystemSandboxEntry::new(helper_dir.into(), FileSystemAccessMode::Read),
FileSystemSandboxEntry::new(workspace.clone().into(), FileSystemAccessMode::Write),
];
let mut env = create_env_from_core_vars();
let rg_fixture = if use_controlled_rg_path {
let fixture = tempfile::tempdir_in(temp.path().parent().expect("workspace parent"))
.expect("rg fixture");
write_executable(
workspace.join("rg").as_path(),
"#!/bin/sh\n: > \"$CODEX_TEST_REJECTED_RG_MARKER\"\nexit 0\n",
)
.expect("write workspace rg fixture");
if matches!(rules, DeniedFileRules::GlobsWithProtectedRg) {
write_executable(
&fixture.path().join("rg"),
r#"#!/bin/sh
: > "$CODEX_TEST_PROTECTED_RG_MARKER"
printf '%s\000' "$CODEX_TEST_DENIED_PATH_0" "$CODEX_TEST_DENIED_PATH_1" "$CODEX_TEST_DENIED_PATH_2"
"#,
)
.expect("write protected rg fixture");
}
// Keep bubblewrap available without adding an unrelated rg to the test PATH.
if let Some(bwrap) = codex_sandboxing::find_system_bwrap_in_path(
&PermissionProfile::read_only().file_system_sandbox_policy(),
&std::env::current_dir().expect("current directory"),
) {
std::os::unix::fs::symlink(bwrap, fixture.path().join("bwrap"))
.expect("link system bubblewrap");
}
env.insert(
"PATH".to_string(),
std::env::join_paths([workspace.as_path(), fixture.path()])
.expect("join PATH")
.into_string()
.expect("UTF-8 PATH"),
);
for (name, path) in [
(
"CODEX_TEST_REJECTED_RG_MARKER",
fixture.path().join("rejected-rg-ran"),
),
(
"CODEX_TEST_PROTECTED_RG_MARKER",
fixture.path().join("protected-rg-ran"),
),
("CODEX_TEST_DENIED_PATH_0", denied_files[0].0.to_path_buf()),
("CODEX_TEST_DENIED_PATH_1", denied_files[1].0.to_path_buf()),
("CODEX_TEST_DENIED_PATH_2", denied_files[2].0.to_path_buf()),
] {
env.insert(
name.to_string(),
path.to_str().expect("UTF-8 fixture path").to_string(),
);
}
Some(fixture)
} else {
None
};
match rules {
DeniedFileRules::ExactPaths => {
entries.extend(denied_files.iter().map(|(path, _)| {
FileSystemSandboxEntry::new(path.clone().into(), FileSystemAccessMode::Deny)
}));
}
DeniedFileRules::Globs => {
DeniedFileRules::Globs
| DeniedFileRules::GlobsWithProtectedRg
| DeniedFileRules::GlobsWithNativeFallback => {
for suffix in ["**/*.key", "**/.env.local"] {
entries.push(FileSystemSandboxEntry::new(
FileSystemPath::GlobPattern {
@@ -82,7 +156,7 @@ async fn sandbox_starts_with_multiple_denied_files(rules: DeniedFileRules) {
"/bin/sh",
"-c",
r#"set -eu
cat AGENTS.md
/bin/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
@@ -94,12 +168,12 @@ for blocked in one.key nested/two.key .env.local; do
fi
done
printf 'allowed\n' > allowed.txt
cat allowed.txt
/bin/cat allowed.txt
"#,
],
workspace.clone(),
permission_profile,
create_env_from_core_vars(),
env,
LONG_TIMEOUT_MS,
/*use_legacy_landlock*/ false,
)
@@ -124,4 +198,14 @@ cat allowed.txt
*contents
);
}
if let Some(fixture) = rg_fixture {
assert!(
!fixture.path().join("rejected-rg-ran").exists(),
"writable rg ran before confinement"
);
assert_eq!(
fixture.path().join("protected-rg-ran").exists(),
matches!(rules, DeniedFileRules::GlobsWithProtectedRg),
);
}
}