mirror of
https://github.com/openai/codex.git
synced 2026-10-27 18:48:37 +00:00
Fail closed on unreadable glob search root metadata errors (#57186)
## Why Linux sandbox deny-glob expansion silently skipped search roots when metadata lookup failed, potentially discarding a deny rule. ## What changed Return a fatal error with the search root and underlying error when metadata lookup fails, except for missing or non-directory roots, which still have no matches. Continue following directory symlinks. ## Testing Add regression tests for symlink-loop metadata errors stopping expansion before scanning, absent and non-directory roots being skipped, and symlink roots expanding to both logical and canonical paths without ripgrep. GitOrigin-RevId: bd97804ec5b599376c525addaacce6b1f29f9895
This commit is contained in:
@@ -899,11 +899,27 @@ fn expand_unreadable_globs_with_ripgrep(
|
||||
"unreadable glob `{pattern}` cannot be safely expanded; use a pattern with a non-root directory prefix"
|
||||
)));
|
||||
};
|
||||
if search_root.as_path().is_dir() {
|
||||
patterns_by_search_root
|
||||
.entry(search_root)
|
||||
.or_default()
|
||||
.push(glob);
|
||||
// Follow symlink roots, but never discard a deny because inspecting its
|
||||
// search root failed. Only absent or non-directory roots have no matches.
|
||||
match fs::metadata(search_root.as_path()) {
|
||||
Ok(metadata) if metadata.is_dir() => {
|
||||
patterns_by_search_root
|
||||
.entry(search_root)
|
||||
.or_default()
|
||||
.push(glob);
|
||||
}
|
||||
Ok(_) => {}
|
||||
Err(err)
|
||||
if matches!(
|
||||
err.kind(),
|
||||
io::ErrorKind::NotFound | io::ErrorKind::NotADirectory
|
||||
) => {}
|
||||
Err(err) => {
|
||||
return Err(CodexErr::Fatal(format!(
|
||||
"failed to inspect unreadable glob search root {}: {err}",
|
||||
search_root.display()
|
||||
)));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1460,6 +1460,72 @@ fn unreadable_globs_add_canonical_targets_for_symlink_matches() {
|
||||
assert_file_masked(&args.args, &real_secret);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unreadable_glob_root_metadata_errors_fail_before_scanning() {
|
||||
let temp_dir = TempDir::new().expect("temp dir");
|
||||
let root = temp_dir.path().join("loop");
|
||||
std::os::unix::fs::symlink(&root, &root).expect("create symlink loop");
|
||||
let error = expand_unreadable_globs_with_ripgrep(
|
||||
&[format!("{}/**/.env", root.display())],
|
||||
temp_dir.path(),
|
||||
|| Some(temp_dir.path().join("scanner-must-not-run")),
|
||||
/*max_depth*/ None,
|
||||
|_| panic!("scanner must not run after a root metadata error"),
|
||||
)
|
||||
.expect_err("metadata errors must reject deny-glob expansion")
|
||||
.to_string();
|
||||
|
||||
assert!(error.contains(&root.display().to_string()));
|
||||
assert!(error.contains(&io::Error::from_raw_os_error(libc::ELOOP).to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unreadable_globs_skip_absent_and_non_directory_roots() {
|
||||
let temp_dir = TempDir::new().expect("temp dir");
|
||||
let missing = temp_dir.path().join("missing");
|
||||
let file = temp_dir.path().join("file");
|
||||
fs::write(&file, "ordinary file").expect("write file");
|
||||
|
||||
for root in [missing, file.join("child"), file] {
|
||||
let paths = expand_unreadable_globs_with_ripgrep(
|
||||
&[format!("{}/**/.env", root.display())],
|
||||
temp_dir.path(),
|
||||
|| None,
|
||||
/*max_depth*/ None,
|
||||
|_| {},
|
||||
)
|
||||
.expect("roots without possible matches should be skipped");
|
||||
|
||||
assert_eq!(paths, Vec::<AbsolutePathBuf>::new());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unreadable_globs_follow_symlink_roots_without_ripgrep() {
|
||||
let temp_dir = TempDir::new().expect("temp dir");
|
||||
let root = temp_dir.path().join("data");
|
||||
fs::create_dir(&root).expect("create directory");
|
||||
let secret = root.join(".env");
|
||||
fs::write(&secret, "secret").expect("write secret");
|
||||
let link = temp_dir.path().join("link");
|
||||
std::os::unix::fs::symlink(&root, &link).expect("create directory symlink");
|
||||
|
||||
let paths = expand_unreadable_globs_with_ripgrep(
|
||||
&[format!("{}/**/.env", link.display())],
|
||||
temp_dir.path(),
|
||||
|| None,
|
||||
/*max_depth*/ None,
|
||||
|_| panic!("fallback scanner must not launch ripgrep"),
|
||||
)
|
||||
.expect("expand deny glob");
|
||||
let expected = [secret, link.join(".env")]
|
||||
.into_iter()
|
||||
.map(|path| AbsolutePathBuf::from_absolute_path_checked(path).expect("absolute path"))
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
assert_eq!(paths, expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn root_prefix_unreadable_globs_fail_closed_on_linux() {
|
||||
let policy = default_policy_with_unreadable_glob("/**/*.env".to_string());
|
||||
|
||||
Reference in New Issue
Block a user