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

fix(core): simplify exact deny-read enforcement

Browse Source 
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
viyatb-oai
2026-03-27 07:20:18 -07:00
co-authored by Codex
parent 57df260e4c
commit 75e76eb178
7 changed files with 62 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/codex.rs
+8 -7
View File
@@ -1158,12 +1158,6 @@ impl SessionConfiguration {
let mut sandbox_policy_changed = false;
if let Some(sandbox_policy) = updates.sandbox_policy.clone() {
next_configuration.sandbox_policy.set(sandbox_policy)?;
next_configuration.file_system_sandbox_policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy_preserving_read_denies(
next_configuration.sandbox_policy.get(),
&next_configuration.cwd,
&next_configuration.file_system_sandbox_policy,
);
next_configuration.network_sandbox_policy =
NetworkSandboxPolicy::from(next_configuration.sandbox_policy.get());
sandbox_policy_changed = true;
@@ -1188,7 +1182,14 @@ impl SessionConfiguration {
let cwd_changed = absolute_cwd.as_path() != self.cwd.as_path();
next_configuration.cwd = absolute_cwd;
if sandbox_policy_changed || (cwd_changed && file_system_policy_matches_legacy) {
if sandbox_policy_changed {
next_configuration.file_system_sandbox_policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy_preserving_read_denies(
next_configuration.sandbox_policy.get(),
&next_configuration.cwd,
&next_configuration.file_system_sandbox_policy,
);
} else if cwd_changed && file_system_policy_matches_legacy {
// Preserve richer split policies across cwd-only updates; only
// rederive when the session is already using the legacy bridge.
next_configuration.file_system_sandbox_policy =
codex-rs/core/src/filesystem_deny_read.rs
+39 -63
View File
@@ -9,12 +9,47 @@ use crate::function_tool::FunctionCallError;
const DENY_READ_POLICY_MESSAGE: &str =
"access denied: reading this path is blocked by filesystem deny_read policy";
pub(crate) struct ReadDenyMatcher {
denied_candidates: Vec<Vec<PathBuf>>,
}
impl ReadDenyMatcher {
pub(crate) fn new(
file_system_sandbox_policy: &FileSystemSandboxPolicy,
cwd: &Path,
) -> Option<Self> {
if !file_system_sandbox_policy.has_denied_read_restrictions() {
return None;
}
let denied_candidates = file_system_sandbox_policy
.get_unreadable_roots_with_cwd(cwd)
.into_iter()
.map(|path| normalized_and_canonical_candidates(path.as_path()))
.collect();
Some(Self { denied_candidates })
}
pub(crate) fn is_read_denied(&self, path: &Path) -> bool {
let path_candidates = normalized_and_canonical_candidates(path);
self.denied_candidates.iter().any(|denied_candidates| {
path_candidates.iter().any(|candidate| {
denied_candidates.iter().any(|denied_candidate| {
candidate == denied_candidate || candidate.starts_with(denied_candidate)
})
})
})
}
}
pub(crate) fn ensure_read_allowed(
path: &Path,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
cwd: &Path,
) -> Result<(), FunctionCallError> {
if is_read_denied(path, file_system_sandbox_policy, cwd) {
if ReadDenyMatcher::new(file_system_sandbox_policy, cwd)
.is_some_and(|matcher| matcher.is_read_denied(path))
{
return Err(FunctionCallError::RespondToModel(format!(
"{DENY_READ_POLICY_MESSAGE}: `{}`",
path.display()
@@ -23,52 +58,14 @@ pub(crate) fn ensure_read_allowed(
Ok(())
}
pub(crate) fn ensure_search_root_does_not_overlap_deny_read(
search_root: &Path,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
cwd: &Path,
) -> Result<(), FunctionCallError> {
if overlaps_deny_read(search_root, file_system_sandbox_policy, cwd) {
return Err(FunctionCallError::RespondToModel(format!(
"access denied: grep_files path `{}` overlaps a filesystem deny_read path; narrow the search path",
search_root.display()
)));
}
Ok(())
}
#[cfg(test)]
pub(crate) fn is_read_denied(
path: &Path,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
cwd: &Path,
) -> bool {
let denied_paths = file_system_sandbox_policy.get_unreadable_roots_with_cwd(cwd);
let path_candidates = normalized_and_canonical_candidates(path);
denied_paths.iter().any(|denied| {
let denied_candidates = normalized_and_canonical_candidates(denied.as_path());
path_candidates.iter().any(|candidate| {
denied_candidates.iter().any(|denied_candidate| {
candidate == denied_candidate || candidate.starts_with(denied_candidate)
})
})
})
}
pub(crate) fn overlaps_deny_read(
path: &Path,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
cwd: &Path,
) -> bool {
let denied_paths = file_system_sandbox_policy.get_unreadable_roots_with_cwd(cwd);
let path_candidates = normalized_and_canonical_candidates(path);
denied_paths.iter().any(|denied| {
let denied_candidates = normalized_and_canonical_candidates(denied.as_path());
path_candidates.iter().any(|candidate| {
denied_candidates.iter().any(|denied_candidate| {
candidate.starts_with(denied_candidate) || denied_candidate.starts_with(candidate)
})
})
})
ReadDenyMatcher::new(file_system_sandbox_policy, cwd)
.is_some_and(|matcher| matcher.is_read_denied(path))
}
fn normalized_and_canonical_candidates(path: &Path) -> Vec<PathBuf> {
@@ -104,7 +101,6 @@ mod tests {
use tempfile::tempdir;
use super::is_read_denied;
use super::overlaps_deny_read;
use super::*;
fn deny_policy(path: &std::path::Path) -> FileSystemSandboxPolicy {
@@ -151,24 +147,4 @@ mod tests {
let policy = deny_policy(&real_dir);
assert_eq!(is_read_denied(&alias_secret, &policy, temp.path()), true);
}
#[test]
fn overlap_detects_parent_and_child_relationships() {
let temp = tempdir().expect("temp dir");
let denied = temp.path().join("private");
let search_root = temp.path();
let nested_search_root = denied.join("nested");
std::fs::create_dir_all(&nested_search_root).expect("create nested");
let policy = deny_policy(&denied);
assert_eq!(overlaps_deny_read(search_root, &policy, temp.path()), true);
assert_eq!(
overlaps_deny_read(&nested_search_root, &policy, temp.path()),
true
);
assert_eq!(
overlaps_deny_read(&temp.path().join("public"), &policy, temp.path()),
false
);
}
}
codex-rs/core/src/tools/handlers/list_dir.rs
+10 -14
View File
@@ -9,8 +9,8 @@ use codex_utils_string::take_bytes_at_char_boundary;
use serde::Deserialize;
use tokio::fs;
use crate::filesystem_deny_read::ReadDenyMatcher;
use crate::filesystem_deny_read::ensure_read_allowed;
use crate::filesystem_deny_read::is_read_denied;
use crate::function_tool::FunctionCallError;
use crate::tools::context::FunctionToolOutput;
use crate::tools::context::ToolInvocation;
@@ -101,15 +101,11 @@ impl ToolHandler for ListDirHandler {
));
}
ensure_read_allowed(&path, &turn.file_system_sandbox_policy, &turn.cwd)?;
let read_deny_matcher = ReadDenyMatcher::new(&turn.file_system_sandbox_policy, &turn.cwd);
let entries = list_dir_slice_with_policy(
&path,
offset,
limit,
depth,
Some((&turn.file_system_sandbox_policy, &turn.cwd)),
)
.await?;
let entries =
list_dir_slice_with_policy(&path, offset, limit, depth, read_deny_matcher.as_ref())
.await?;
let mut output = Vec::with_capacity(entries.len() + 1);
output.push(format!("Absolute path: {}", path.display()));
output.extend(entries);
@@ -132,10 +128,10 @@ async fn list_dir_slice_with_policy(
offset: usize,
limit: usize,
depth: usize,
sandbox_policy: Option<(&codex_protocol::permissions::FileSystemSandboxPolicy, &Path)>,
read_deny_matcher: Option<&ReadDenyMatcher>,
) -> Result<Vec<String>, FunctionCallError> {
let mut entries = Vec::new();
collect_entries(path, Path::new(""), depth, sandbox_policy, &mut entries).await?;
collect_entries(path, Path::new(""), depth, read_deny_matcher, &mut entries).await?;
if entries.is_empty() {
return Ok(Vec::new());
@@ -171,7 +167,7 @@ async fn collect_entries(
dir_path: &Path,
relative_prefix: &Path,
depth: usize,
sandbox_policy: Option<(&codex_protocol::permissions::FileSystemSandboxPolicy, &Path)>,
read_deny_matcher: Option<&ReadDenyMatcher>,
entries: &mut Vec<DirEntry>,
) -> Result<(), FunctionCallError> {
let mut queue = VecDeque::new();
@@ -188,8 +184,8 @@ async fn collect_entries(
FunctionCallError::RespondToModel(format!("failed to read directory: {err}"))
})? {
let entry_path = entry.path();
if let Some((policy, cwd)) = sandbox_policy
&& is_read_denied(&entry_path, policy, cwd)
if let Some(read_deny_matcher) = read_deny_matcher
&& read_deny_matcher.is_read_denied(&entry_path)
{
continue;
}
codex-rs/core/src/tools/handlers/list_dir_tests.rs
+3 -1
View File
@@ -1,4 +1,5 @@
use super::*;
use crate::filesystem_deny_read::ReadDenyMatcher;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
@@ -284,7 +285,8 @@ async fn hides_denied_entries_and_prunes_denied_subtrees() {
},
]);
let entries = list_dir_slice_with_policy(dir_path, 1, 20, 3, Some((&policy, dir_path)))
let read_deny_matcher = ReadDenyMatcher::new(&policy, dir_path);
let entries = list_dir_slice_with_policy(dir_path, 1, 20, 3, read_deny_matcher.as_ref())
.await
.expect("list directory");
codex-rs/core/src/tools/handlers/mod.rs
+2 -1
View File
@@ -35,6 +35,7 @@ pub(crate) use crate::tools::code_mode::CodeModeExecuteHandler;
pub(crate) use crate::tools::code_mode::CodeModeWaitHandler;
pub use apply_patch::ApplyPatchHandler;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::protocol::AskForApproval;
pub use dynamic::DynamicToolHandler;
pub use js_repl::JsReplHandler;
@@ -182,7 +183,7 @@ pub(super) fn reject_explicit_escalation_if_deny_read_present(
&& file_system_sandbox_policy.has_denied_read_restrictions()
{
return Err(FunctionCallError::RespondToModel(
"filesystem deny_read policy is enforced; reject command — you cannot ask for escalated permissions because managed read restrictions must remain sandboxed".to_string(),
"filesystem deny_read policy is enforced; reject command — you cannot ask for escalated permissions because deny_read restrictions must remain sandboxed".to_string(),
));
}
codex-rs/linux-sandbox/src/landlock.rs
-9
View File
@@ -8,7 +8,6 @@ use std::path::Path;
use codex_core::error::CodexErr;
use codex_core::error::Result;
use codex_core::error::SandboxErr;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::protocol::NetworkSandboxPolicy;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
@@ -42,7 +41,6 @@ use seccompiler::apply_filter;
/// Filesystem restrictions are intentionally handled by bubblewrap.
pub(crate) fn apply_sandbox_policy_to_current_thread(
sandbox_policy: &SandboxPolicy,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
network_sandbox_policy: NetworkSandboxPolicy,
cwd: &Path,
apply_landlock_fs: bool,
@@ -69,13 +67,6 @@ pub(crate) fn apply_sandbox_policy_to_current_thread(
install_network_seccomp_filter_on_current_thread(mode)?;
}
if apply_landlock_fs && file_system_sandbox_policy.has_denied_read_restrictions() {
return Err(CodexErr::UnsupportedOperation(
"Filesystem deny_read restrictions are not supported by the legacy Linux Landlock filesystem backend."
.to_string(),
));
}
if apply_landlock_fs && !sandbox_policy.has_full_disk_write_access() {
if !sandbox_policy.has_full_disk_read_access() {
return Err(CodexErr::UnsupportedOperation(
codex-rs/linux-sandbox/src/linux_run_main.rs
-3
View File
@@ -149,7 +149,6 @@ pub fn run_main() -> ! {
let proxy_routing_active = allow_network_for_proxy;
if let Err(e) = apply_sandbox_policy_to_current_thread(
&sandbox_policy,
&file_system_sandbox_policy,
network_sandbox_policy,
&sandbox_policy_cwd,
/*apply_landlock_fs*/ false,
@@ -164,7 +163,6 @@ pub fn run_main() -> ! {
if file_system_sandbox_policy.has_full_disk_write_access() && !allow_network_for_proxy {
if let Err(e) = apply_sandbox_policy_to_current_thread(
&sandbox_policy,
&file_system_sandbox_policy,
network_sandbox_policy,
&sandbox_policy_cwd,
/*apply_landlock_fs*/ false,
@@ -212,7 +210,6 @@ pub fn run_main() -> ! {
// Legacy path: Landlock enforcement only, when bwrap sandboxing is not enabled.
if let Err(e) = apply_sandbox_policy_to_current_thread(
&sandbox_policy,
&file_system_sandbox_policy,
network_sandbox_policy,
&sandbox_policy_cwd,
/*apply_landlock_fs*/ true,