feat(permissions): expand deny_read globs at config load

This commit is contained in:
viyatb-oai
2026-02-26 16:34:48 -08:00
parent d5517ce674
commit e868666a59
7 changed files with 436 additions and 9 deletions
+127 -4
View File
@@ -5,6 +5,9 @@ use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use serde::Deserialize;
use serde::Serialize;
use serde::de::Error as SerdeError;
use serde::de::value::Error as ValueDeserializerError;
use serde::de::value::StrDeserializer;
use std::collections::BTreeMap;
use std::fmt;
@@ -194,7 +197,7 @@ impl From<NetworkRequirementsToml> for NetworkConstraints {
#[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)]
pub struct FilesystemRequirementsToml {
pub deny_read: Option<Vec<AbsolutePathBuf>>,
pub deny_read: Option<Vec<FilesystemDenyReadPattern>>,
}
#[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)]
@@ -205,7 +208,7 @@ pub struct PermissionsRequirementsToml {
/// Normalized filesystem constraints derived from requirements TOML.
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
pub struct FilesystemConstraints {
pub deny_read: Vec<AbsolutePathBuf>,
pub deny_read: Vec<FilesystemDenyReadPattern>,
}
impl From<PermissionsRequirementsToml> for FilesystemConstraints {
@@ -218,6 +221,97 @@ impl From<PermissionsRequirementsToml> for FilesystemConstraints {
}
}
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize)]
#[serde(transparent)]
pub struct FilesystemDenyReadPattern(String);
impl FilesystemDenyReadPattern {
pub fn as_str(&self) -> &str {
&self.0
}
pub fn contains_glob(&self) -> bool {
self.0.contains('*')
}
pub fn from_input(input: &str) -> Result<Self, String> {
if !input.contains('*') {
let path = deserialize_absolute_path(input)?;
return Ok(Self(path.to_string_lossy().into_owned()));
}
let (directory_prefix, suffix) = split_glob_pattern(input);
let normalized_prefix = if directory_prefix.is_empty() {
deserialize_absolute_path(".")?
} else {
deserialize_absolute_path(directory_prefix)?
};
let normalized_prefix = normalized_prefix.to_string_lossy();
let normalized = if suffix.is_empty() {
normalized_prefix.into_owned()
} else if normalized_prefix == "/" {
format!("/{suffix}")
} else {
format!("{normalized_prefix}/{suffix}")
};
Ok(Self(normalized))
}
}
impl From<AbsolutePathBuf> for FilesystemDenyReadPattern {
fn from(value: AbsolutePathBuf) -> Self {
Self(value.to_string_lossy().into_owned())
}
}
impl<'de> Deserialize<'de> for FilesystemDenyReadPattern {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
let input = String::deserialize(deserializer)?;
Self::from_input(&input).map_err(SerdeError::custom)
}
}
fn deserialize_absolute_path(input: &str) -> Result<AbsolutePathBuf, String> {
AbsolutePathBuf::deserialize(StrDeserializer::<ValueDeserializerError>::new(input))
.map_err(|err| err.to_string())
}
fn split_glob_pattern(input: &str) -> (&str, &str) {
let Some(first_glob) = input.find('*') else {
return ("", input);
};
let separator_index = input[..first_glob]
.char_indices()
.rev()
.find(|(_, ch)| is_path_separator(*ch))
.map(|(index, _)| index);
match separator_index {
Some(0) => ("/", &input[1..]),
Some(index)
if cfg!(windows)
&& index == 2
&& input.as_bytes().get(1) == Some(&b':')
&& input.as_bytes().get(2).is_some() =>
{
(&input[..=index], &input[index + 1..])
}
Some(index) => (&input[..index], &input[index + 1..]),
None => ("", input),
}
}
fn is_path_separator(ch: char) -> bool {
if cfg!(windows) {
ch == '/' || ch == '\\'
} else {
ch == '/'
}
}
#[derive(Deserialize, Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[serde(rename_all = "lowercase")]
pub enum WebSearchModeRequirement {
@@ -611,6 +705,7 @@ mod tests {
use codex_execpolicy::RuleMatch;
use codex_protocol::protocol::NetworkAccess;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::AbsolutePathBufGuard;
use pretty_assertions::assert_eq;
use toml::from_str;
@@ -1054,8 +1149,36 @@ mod tests {
Some(Sourced::new(
FilesystemConstraints {
deny_read: vec![
AbsolutePathBuf::from_absolute_path(deny_read_0)?,
AbsolutePathBuf::from_absolute_path(deny_read_1)?,
AbsolutePathBuf::from_absolute_path(deny_read_0)?.into(),
AbsolutePathBuf::from_absolute_path(deny_read_1)?.into(),
],
},
RequirementSource::Unknown,
))
);
Ok(())
}
#[test]
fn deserialize_filesystem_deny_read_glob_requirements() -> Result<()> {
let temp_dir = std::env::temp_dir();
let _guard = AbsolutePathBufGuard::new(&temp_dir);
let config: ConfigRequirementsToml = from_str(
r#"
[permissions.filesystem]
deny_read = ["./private/**/*.txt"]
"#,
)?;
let requirements: ConfigRequirements = with_unknown_source(config).try_into()?;
assert_eq!(
requirements.filesystem,
Some(Sourced::new(
FilesystemConstraints {
deny_read: vec![
FilesystemDenyReadPattern::from_input("./private/**/*.txt")
.expect("normalize glob pattern")
],
},
RequirementSource::Unknown,
+1
View File
@@ -13,6 +13,7 @@ pub use config_requirements::ConfigRequirementsToml;
pub use config_requirements::ConfigRequirementsWithSources;
pub use config_requirements::ConstrainedWithSource;
pub use config_requirements::FilesystemConstraints;
pub use config_requirements::FilesystemDenyReadPattern;
pub use config_requirements::McpServerIdentity;
pub use config_requirements::McpServerRequirement;
pub use config_requirements::NetworkConstraints;
+45 -1
View File
@@ -37,6 +37,7 @@ use crate::features::Feature;
use crate::features::FeatureOverrides;
use crate::features::Features;
use crate::features::FeaturesToml;
use crate::filesystem_deny_read::expand_deny_read_patterns;
use crate::git_info::resolve_root_git_project_for_trust;
use crate::model_provider_info::LEGACY_OLLAMA_CHAT_PROVIDER_ID;
use crate::model_provider_info::LMSTUDIO_OSS_PROVIDER_ID;
@@ -1908,7 +1909,7 @@ impl Config {
}) = filesystem_requirements
&& !filesystem_requirements.deny_read.is_empty()
{
let deny_read_paths = filesystem_requirements.deny_read;
let deny_read_paths = expand_deny_read_patterns(&filesystem_requirements.deny_read);
constrained_sandbox_policy
.value
.add_normalizer(move |mut policy| {
@@ -5627,6 +5628,49 @@ mcp_oauth_callback_url = "https://example.com/callback"
Ok(())
}
#[tokio::test]
async fn requirements_filesystem_deny_read_glob_expands_to_matching_paths()
-> std::io::Result<()> {
let codex_home = TempDir::new()?;
let sensitive_dir = codex_home.path().join("sensitive");
let nested_dir = sensitive_dir.join("nested");
std::fs::create_dir_all(&nested_dir)?;
let top_match = sensitive_dir.join("top.txt");
let nested_match = nested_dir.join("deep.txt");
let ignored = sensitive_dir.join("top.log");
std::fs::write(&top_match, "top")?;
std::fs::write(&nested_match, "deep")?;
std::fs::write(&ignored, "ignored")?;
let glob_pattern = format!("{}/sensitive/**/*.txt", codex_home.path().display());
let top_match = AbsolutePathBuf::try_from(top_match).expect("absolute path");
let nested_match = AbsolutePathBuf::try_from(nested_match).expect("absolute path");
let config = ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.fallback_cwd(Some(codex_home.path().to_path_buf()))
.cloud_requirements(CloudRequirementsLoader::new(async move {
Some(
toml::from_str::<crate::config_loader::ConfigRequirementsToml>(&format!(
r#"
[permissions.filesystem]
deny_read = [{glob_pattern:?}]
"#
))
.expect("parse requirements toml"),
)
}))
.build()
.await?;
assert_eq!(
config.permissions.sandbox_policy.get().denied_read_paths(),
vec![top_match, nested_match]
);
Ok(())
}
#[tokio::test]
async fn requirements_filesystem_deny_read_persists_when_sandbox_mode_changes()
-> std::io::Result<()> {
+1
View File
@@ -34,6 +34,7 @@ pub use codex_config::ConfigRequirements;
pub use codex_config::ConfigRequirementsToml;
pub use codex_config::ConstrainedWithSource;
pub use codex_config::FilesystemConstraints;
pub use codex_config::FilesystemDenyReadPattern;
pub use codex_config::LoaderOverrides;
pub use codex_config::McpServerIdentity;
pub use codex_config::McpServerRequirement;
+57 -2
View File
@@ -12,6 +12,7 @@ use crate::config_loader::ConfigLoadError;
use crate::config_loader::ConfigRequirements;
use crate::config_loader::ConfigRequirementsToml;
use crate::config_loader::ConfigRequirementsWithSources;
use crate::config_loader::FilesystemDenyReadPattern;
use crate::config_loader::RequirementSource;
use crate::config_loader::load_requirements_toml;
use crate::config_loader::version_for_toml;
@@ -688,8 +689,62 @@ deny_read = ["./sensitive", "../shared/secret.txt"]
assert_eq!(
deny_read,
vec![
AbsolutePathBuf::try_from(requirements_dir.join("sensitive"))?,
AbsolutePathBuf::try_from(tmp.path().join("shared").join("secret.txt"))?,
FilesystemDenyReadPattern::from(AbsolutePathBuf::try_from(
requirements_dir.join("sensitive")
)?,),
FilesystemDenyReadPattern::from(AbsolutePathBuf::try_from(
tmp.path().join("shared").join("secret.txt"),
)?),
]
);
assert_eq!(
permissions.source,
RequirementSource::SystemRequirementsToml {
file: AbsolutePathBuf::try_from(requirements_file)?,
}
);
Ok(())
}
#[tokio::test(flavor = "current_thread")]
async fn load_requirements_toml_resolves_filesystem_deny_read_glob_against_requirements_parent()
-> anyhow::Result<()> {
let tmp = tempdir()?;
let requirements_dir = tmp.path().join("managed");
tokio::fs::create_dir_all(&requirements_dir).await?;
let requirements_file = requirements_dir.join("requirements.toml");
tokio::fs::write(
&requirements_file,
r#"
[permissions.filesystem]
deny_read = ["./sensitive/**/*.txt"]
"#,
)
.await?;
let mut config_requirements_toml = ConfigRequirementsWithSources::default();
load_requirements_toml(&mut config_requirements_toml, &requirements_file).await?;
let permissions = config_requirements_toml
.permissions
.expect("permissions requirements should load");
let filesystem = permissions
.value
.filesystem
.expect("filesystem requirements should load");
let deny_read = filesystem
.deny_read
.expect("deny_read patterns should load");
assert_eq!(
deny_read,
vec![
FilesystemDenyReadPattern::from_input(&format!(
"{}/sensitive/**/*.txt",
requirements_dir.display()
))
.expect("normalize glob pattern")
]
);
assert_eq!(
+203
View File
@@ -1,6 +1,7 @@
use std::path::Path;
use std::path::PathBuf;
use codex_config::FilesystemDenyReadPattern;
use codex_utils_absolute_path::AbsolutePathBuf;
use crate::function_tool::FunctionCallError;
@@ -35,6 +36,20 @@ pub(crate) fn ensure_search_root_does_not_overlap_deny_read(
Ok(())
}
pub(crate) fn expand_deny_read_patterns(
patterns: &[FilesystemDenyReadPattern],
) -> Vec<AbsolutePathBuf> {
let mut expanded = Vec::new();
for pattern in patterns {
if pattern.contains_glob() {
expand_glob_pattern(pattern, &mut expanded);
} else if let Ok(path) = AbsolutePathBuf::try_from(pattern.as_str()) {
push_unique_absolute(&mut expanded, path);
}
}
expanded
}
pub(crate) fn is_read_denied(path: &Path, sandbox_policy: &SandboxPolicy) -> bool {
let denied_paths = sandbox_policy.denied_read_paths();
if denied_paths.is_empty() {
@@ -93,12 +108,165 @@ fn push_unique(candidates: &mut Vec<PathBuf>, candidate: PathBuf) {
}
}
fn push_unique_absolute(candidates: &mut Vec<AbsolutePathBuf>, candidate: AbsolutePathBuf) {
if !candidates
.iter()
.any(|existing| existing.as_path() == candidate.as_path())
{
candidates.push(candidate);
}
}
fn expand_glob_pattern(pattern: &FilesystemDenyReadPattern, expanded: &mut Vec<AbsolutePathBuf>) {
let (root, components) = split_glob_pattern(pattern.as_str());
let Ok(root) = AbsolutePathBuf::try_from(root) else {
return;
};
expand_glob_components(root.as_path(), &components, expanded);
}
fn split_glob_pattern(pattern: &str) -> (&str, Vec<&str>) {
let Some(first_glob) = pattern.find('*') else {
return (pattern, Vec::new());
};
let separator_index = pattern[..first_glob]
.char_indices()
.rev()
.find(|(_, ch)| is_path_separator(*ch))
.map(|(index, _)| index);
let (root, suffix) = match separator_index {
Some(0) => ("/", &pattern[1..]),
Some(index)
if cfg!(windows)
&& index == 2
&& pattern.as_bytes().get(1) == Some(&b':')
&& pattern.as_bytes().get(2).is_some() =>
{
(&pattern[..=index], &pattern[index + 1..])
}
Some(index) => (&pattern[..index], &pattern[index + 1..]),
None => ("", pattern),
};
let components = suffix
.split(is_path_separator)
.filter(|component| !component.is_empty())
.collect();
(root, components)
}
fn expand_glob_components(current: &Path, remaining: &[&str], expanded: &mut Vec<AbsolutePathBuf>) {
if remaining.is_empty() {
if let Ok(path) = AbsolutePathBuf::try_from(current) {
push_unique_absolute(expanded, path);
}
return;
}
let component = remaining[0];
let rest = &remaining[1..];
if component == "**" {
expand_glob_components(current, rest, expanded);
let Ok(entries) = std::fs::read_dir(current) else {
return;
};
for entry in entries.flatten() {
let path = entry.path();
if rest.is_empty()
&& let Ok(path) = AbsolutePathBuf::try_from(path.as_path())
{
push_unique_absolute(expanded, path);
}
if path.is_dir() {
expand_glob_components(&path, remaining, expanded);
}
}
return;
}
let Ok(entries) = std::fs::read_dir(current) else {
return;
};
for entry in entries.flatten() {
let path = entry.path();
let Some(name) = path.file_name().and_then(|name| name.to_str()) else {
continue;
};
if !segment_matches(component, name) {
continue;
}
if rest.is_empty() {
if let Ok(path) = AbsolutePathBuf::try_from(path.as_path()) {
push_unique_absolute(expanded, path);
}
} else if path.is_dir() {
expand_glob_components(&path, rest, expanded);
}
}
}
fn segment_matches(pattern: &str, candidate: &str) -> bool {
if !pattern.contains('*') {
return pattern == candidate;
}
let pattern_chars: Vec<char> = pattern.chars().collect();
let candidate_chars: Vec<char> = candidate.chars().collect();
let mut pattern_index = 0;
let mut candidate_index = 0;
let mut star_index = None;
let mut star_candidate_index = 0;
while candidate_index < candidate_chars.len() {
if pattern_index < pattern_chars.len()
&& pattern_chars[pattern_index] == candidate_chars[candidate_index]
{
pattern_index += 1;
candidate_index += 1;
continue;
}
if pattern_index < pattern_chars.len() && pattern_chars[pattern_index] == '*' {
star_index = Some(pattern_index);
pattern_index += 1;
star_candidate_index = candidate_index;
continue;
}
if let Some(star_index) = star_index {
pattern_index = star_index + 1;
star_candidate_index += 1;
candidate_index = star_candidate_index;
continue;
}
return false;
}
while pattern_index < pattern_chars.len() && pattern_chars[pattern_index] == '*' {
pattern_index += 1;
}
pattern_index == pattern_chars.len()
}
fn is_path_separator(ch: char) -> bool {
if cfg!(windows) {
ch == '/' || ch == '\\'
} else {
ch == '/'
}
}
#[cfg(test)]
mod tests {
use super::expand_deny_read_patterns;
use super::is_read_denied;
use super::overlaps_deny_read;
use crate::protocol::ReadOnlyAccess;
use crate::protocol::SandboxPolicy;
use codex_config::FilesystemDenyReadPattern;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use tempfile::tempdir;
@@ -162,4 +330,39 @@ mod tests {
false
);
}
#[test]
fn expand_deny_read_patterns_supports_star_and_globstar() {
let temp = tempdir().expect("temp dir");
let secrets = temp.path().join("secrets");
let nested = secrets.join("nested");
std::fs::create_dir_all(&nested).expect("create nested");
let top = secrets.join("top.txt");
let deep = nested.join("deep.txt");
let ignored = secrets.join("top.log");
std::fs::write(&top, "top").expect("write top");
std::fs::write(&deep, "deep").expect("write deep");
std::fs::write(&ignored, "ignored").expect("write ignored");
let top_pattern = FilesystemDenyReadPattern::from_input(&format!(
"{}/secrets/*.txt",
temp.path().display()
))
.expect("normalize pattern");
let deep_pattern = FilesystemDenyReadPattern::from_input(&format!(
"{}/secrets/**/*.txt",
temp.path().display()
))
.expect("normalize pattern");
let expanded = expand_deny_read_patterns(&[top_pattern, deep_pattern]);
assert_eq!(
expanded,
vec![
AbsolutePathBuf::try_from(top).expect("absolute path"),
AbsolutePathBuf::try_from(deep).expect("absolute path"),
]
);
}
}
+2 -2
View File
@@ -170,7 +170,7 @@ fn render_debug_config_lines(stack: &ConfigLayerStack) -> Vec<Line<'static>> {
.value
.deny_read
.iter()
.map(|path| path.as_path().display().to_string())
.map(|pattern| pattern.as_str().to_string())
.collect::<Vec<_>>(),
);
requirement_lines.push(requirement_line(
@@ -544,7 +544,7 @@ mod tests {
)),
filesystem: Some(Sourced::new(
FilesystemConstraints {
deny_read: vec![denied_path.clone()],
deny_read: vec![denied_path.clone().into()],
},
RequirementSource::SystemRequirementsToml {
file: requirements_file.clone(),