mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat(config): support managed deny-read requirements (#17740)
## Summary
- adds managed requirements support for deny-read filesystem entries
- constrains config layers so managed deny-read requirements cannot be
widened by user-controlled config
- surfaces managed deny-read requirements through debug/config plumbing
This PR lets managed requirements inject deny-read filesystem
constraints into the effective filesystem sandbox policy.
User-controlled config can still choose the surrounding permission
profile, but it cannot remove or weaken the managed deny-read entries.
## Managed deny-read shape
A managed requirements file can declare exact paths and glob patterns
under `[permissions.filesystem]`:
```toml
# /etc/codex/requirements.toml
[permissions.filesystem]
deny_read = [
"/Users/alice/.gitconfig",
"/Users/alice/.ssh",
"./managed-private/**/*.env",
]
```
Those entries are compiled into the effective filesystem policy as
`access = none` rules, equivalent in shape to filesystem permission
entries like:
```toml
[permissions.workspace.filesystem]
"/Users/alice/.gitconfig" = "none"
"/Users/alice/.ssh" = "none"
"/absolute/path/to/managed-private/**/*.env" = "none"
```
The important difference is that the managed entries come from
requirements, so lower-precedence user config cannot remove them or make
those paths readable again.
Relative managed `deny_read` entries are resolved relative to the
directory containing the managed requirements file. Glob entries keep
their glob suffix after the non-glob prefix is normalized.
## Runtime behavior
- Managed `deny_read` entries are appended to the effective
`FileSystemSandboxPolicy` after the selected permission profile is
resolved.
- Exact paths become `FileSystemPath::Path { access: None }`; glob
patterns become `FileSystemPath::GlobPattern { access: None }`.
- When managed deny-read entries are present, `sandbox_mode` is
constrained to `read-only` or `workspace-write`; `danger-full-access`
and `external-sandbox` cannot silently bypass the managed read-deny
policy.
- On Windows, the managed deny-read policy is enforced for direct file
tools, but shell subprocess reads are not sandboxed yet, so startup
emits a warning for that platform.
- `/debug-config` shows the effective managed requirement as
`permissions.filesystem.deny_read` with its source.
## Stack
1. #15979 - glob deny-read policy/config/direct-tool support
2. #18096 - macOS and Linux sandbox enforcement
3. This PR - managed deny-read requirements
---------
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
2dd6734dd3
commit
dae0608c06
@@ -605,6 +605,7 @@ mod tests {
|
||||
}),
|
||||
allow_local_binding: Some(true),
|
||||
}),
|
||||
permissions: None,
|
||||
};
|
||||
|
||||
let mapped = map_requirements_toml_to_api(requirements);
|
||||
@@ -698,6 +699,7 @@ mod tests {
|
||||
}),
|
||||
allow_local_binding: None,
|
||||
}),
|
||||
permissions: None,
|
||||
};
|
||||
|
||||
let mapped = map_requirements_toml_to_api(requirements);
|
||||
@@ -739,6 +741,7 @@ mod tests {
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
};
|
||||
|
||||
let mapped = map_requirements_toml_to_api(requirements);
|
||||
|
||||
@@ -1172,6 +1172,7 @@ mod tests {
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
}))
|
||||
);
|
||||
}
|
||||
@@ -1201,6 +1202,7 @@ mod tests {
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
}))
|
||||
);
|
||||
}
|
||||
@@ -1230,6 +1232,7 @@ mod tests {
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
}))
|
||||
);
|
||||
}
|
||||
@@ -1276,6 +1279,7 @@ mod tests {
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
})
|
||||
);
|
||||
}
|
||||
@@ -1358,6 +1362,7 @@ enabled = false
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
}))
|
||||
);
|
||||
assert_eq!(fetcher.request_count.load(Ordering::SeqCst), 2);
|
||||
@@ -1430,6 +1435,7 @@ enabled = false
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
}))
|
||||
);
|
||||
assert_eq!(fetcher.request_count.load(Ordering::SeqCst), 2);
|
||||
@@ -1500,6 +1506,7 @@ enabled = false
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
}))
|
||||
);
|
||||
|
||||
@@ -1697,6 +1704,7 @@ enabled = false
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
}))
|
||||
);
|
||||
assert_eq!(fetcher.request_count.load(Ordering::SeqCst), 0);
|
||||
@@ -1732,6 +1740,7 @@ enabled = false
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
}))
|
||||
);
|
||||
|
||||
@@ -1787,6 +1796,7 @@ enabled = false
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
}))
|
||||
);
|
||||
assert_eq!(fetcher.request_count.load(Ordering::SeqCst), 1);
|
||||
@@ -1837,6 +1847,7 @@ enabled = false
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
}))
|
||||
);
|
||||
assert_eq!(fetcher.request_count.load(Ordering::SeqCst), 1);
|
||||
@@ -1891,6 +1902,7 @@ enabled = false
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
}))
|
||||
);
|
||||
assert_eq!(fetcher.request_count.load(Ordering::SeqCst), 1);
|
||||
@@ -1946,6 +1958,7 @@ enabled = false
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
}))
|
||||
);
|
||||
assert_eq!(fetcher.request_count.load(Ordering::SeqCst), 1);
|
||||
@@ -2001,6 +2014,7 @@ enabled = false
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
})
|
||||
);
|
||||
let payload_bytes = cache_payload_bytes(&cache_file.signed_payload).expect("payload bytes");
|
||||
@@ -2089,6 +2103,7 @@ enabled = false
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
}))
|
||||
);
|
||||
|
||||
@@ -2116,6 +2131,7 @@ enabled = false
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
@@ -7,6 +7,8 @@ use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde::de::Error as _;
|
||||
use serde::de::value::Error as ValueDeserializerError;
|
||||
use serde::de::value::StrDeserializer;
|
||||
use std::collections::BTreeMap;
|
||||
use std::fmt;
|
||||
|
||||
@@ -88,6 +90,8 @@ pub struct ConfigRequirements {
|
||||
pub enforce_residency: ConstrainedWithSource<Option<ResidencyRequirement>>,
|
||||
/// Managed network constraints derived from requirements.
|
||||
pub network: Option<Sourced<NetworkConstraints>>,
|
||||
/// Managed filesystem constraints derived from requirements.
|
||||
pub filesystem: Option<Sourced<FilesystemConstraints>>,
|
||||
}
|
||||
|
||||
impl Default for ConfigRequirements {
|
||||
@@ -117,6 +121,7 @@ impl Default for ConfigRequirements {
|
||||
/*source*/ None,
|
||||
),
|
||||
network: None,
|
||||
filesystem: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -402,6 +407,126 @@ impl From<NetworkRequirementsToml> for NetworkConstraints {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)]
|
||||
pub struct FilesystemRequirementsToml {
|
||||
pub deny_read: Option<Vec<FilesystemDenyReadPattern>>,
|
||||
}
|
||||
|
||||
#[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)]
|
||||
pub struct PermissionsRequirementsToml {
|
||||
pub filesystem: Option<FilesystemRequirementsToml>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct FilesystemConstraints {
|
||||
pub deny_read: Vec<FilesystemDenyReadPattern>,
|
||||
}
|
||||
|
||||
impl From<PermissionsRequirementsToml> for FilesystemConstraints {
|
||||
fn from(value: PermissionsRequirementsToml) -> Self {
|
||||
let deny_read = value
|
||||
.filesystem
|
||||
.and_then(|filesystem| filesystem.deny_read)
|
||||
.unwrap_or_default();
|
||||
Self { deny_read }
|
||||
}
|
||||
}
|
||||
|
||||
#[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.chars().any(is_glob_metacharacter)
|
||||
}
|
||||
|
||||
pub fn from_input(input: &str) -> Result<Self, String> {
|
||||
if !input.chars().any(is_glob_metacharacter) {
|
||||
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(D::Error::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(is_glob_metacharacter) 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 == '/'
|
||||
}
|
||||
}
|
||||
|
||||
fn is_glob_metacharacter(ch: char) -> bool {
|
||||
matches!(ch, '*' | '?' | '[')
|
||||
}
|
||||
|
||||
#[derive(Deserialize, Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
|
||||
#[serde(rename_all = "lowercase")]
|
||||
pub enum WebSearchModeRequirement {
|
||||
@@ -504,6 +629,7 @@ pub struct ConfigRequirementsToml {
|
||||
pub enforce_residency: Option<ResidencyRequirement>,
|
||||
#[serde(rename = "experimental_network")]
|
||||
pub network: Option<NetworkRequirementsToml>,
|
||||
pub permissions: Option<PermissionsRequirementsToml>,
|
||||
pub guardian_policy_config: Option<String>,
|
||||
}
|
||||
|
||||
@@ -541,6 +667,7 @@ pub struct ConfigRequirementsWithSources {
|
||||
pub rules: Option<Sourced<RequirementsExecPolicyToml>>,
|
||||
pub enforce_residency: Option<Sourced<ResidencyRequirement>>,
|
||||
pub network: Option<Sourced<NetworkRequirementsToml>>,
|
||||
pub permissions: Option<Sourced<PermissionsRequirementsToml>>,
|
||||
pub guardian_policy_config: Option<Sourced<String>>,
|
||||
}
|
||||
|
||||
@@ -573,6 +700,7 @@ impl ConfigRequirementsWithSources {
|
||||
rules: _,
|
||||
enforce_residency: _,
|
||||
network: _,
|
||||
permissions: _,
|
||||
guardian_policy_config: _,
|
||||
} = &other;
|
||||
|
||||
@@ -598,6 +726,7 @@ impl ConfigRequirementsWithSources {
|
||||
rules,
|
||||
enforce_residency,
|
||||
network,
|
||||
permissions,
|
||||
guardian_policy_config,
|
||||
}
|
||||
);
|
||||
@@ -623,6 +752,7 @@ impl ConfigRequirementsWithSources {
|
||||
rules,
|
||||
enforce_residency,
|
||||
network,
|
||||
permissions,
|
||||
guardian_policy_config,
|
||||
} = self;
|
||||
ConfigRequirementsToml {
|
||||
@@ -636,6 +766,7 @@ impl ConfigRequirementsWithSources {
|
||||
rules: rules.map(|sourced| sourced.value),
|
||||
enforce_residency: enforce_residency.map(|sourced| sourced.value),
|
||||
network: network.map(|sourced| sourced.value),
|
||||
permissions: permissions.map(|sourced| sourced.value),
|
||||
guardian_policy_config: guardian_policy_config.map(|sourced| sourced.value),
|
||||
}
|
||||
}
|
||||
@@ -692,6 +823,7 @@ impl ConfigRequirementsToml {
|
||||
&& self.rules.is_none()
|
||||
&& self.enforce_residency.is_none()
|
||||
&& self.network.is_none()
|
||||
&& self.permissions.is_none()
|
||||
&& self
|
||||
.guardian_policy_config
|
||||
.as_deref()
|
||||
@@ -714,6 +846,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
rules,
|
||||
enforce_residency,
|
||||
network,
|
||||
permissions,
|
||||
guardian_policy_config: _guardian_policy_config,
|
||||
} = toml;
|
||||
|
||||
@@ -919,6 +1052,10 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
let Sourced { value, source } = sourced_network;
|
||||
Sourced::new(NetworkConstraints::from(value), source)
|
||||
});
|
||||
let filesystem = permissions.map(|sourced_permissions| {
|
||||
let Sourced { value, source } = sourced_permissions;
|
||||
Sourced::new(FilesystemConstraints::from(value), source)
|
||||
});
|
||||
Ok(ConfigRequirements {
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
@@ -929,6 +1066,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
exec_policy,
|
||||
enforce_residency,
|
||||
network,
|
||||
filesystem,
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -942,6 +1080,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;
|
||||
|
||||
@@ -967,6 +1106,7 @@ mod tests {
|
||||
rules,
|
||||
enforce_residency,
|
||||
network,
|
||||
permissions,
|
||||
guardian_policy_config,
|
||||
} = toml;
|
||||
ConfigRequirementsWithSources {
|
||||
@@ -986,6 +1126,7 @@ mod tests {
|
||||
enforce_residency: enforce_residency
|
||||
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
network: network.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
permissions: permissions.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
guardian_policy_config: guardian_policy_config
|
||||
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
}
|
||||
@@ -1027,6 +1168,7 @@ mod tests {
|
||||
rules: None,
|
||||
enforce_residency: Some(enforce_residency),
|
||||
network: None,
|
||||
permissions: None,
|
||||
guardian_policy_config: Some(guardian_policy_config.clone()),
|
||||
};
|
||||
|
||||
@@ -1057,6 +1199,7 @@ mod tests {
|
||||
rules: None,
|
||||
enforce_residency: Some(Sourced::new(enforce_residency, enforce_source)),
|
||||
network: None,
|
||||
permissions: None,
|
||||
guardian_policy_config: Some(Sourced::new(guardian_policy_config, source)),
|
||||
}
|
||||
);
|
||||
@@ -1093,6 +1236,7 @@ mod tests {
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
guardian_policy_config: None,
|
||||
}
|
||||
);
|
||||
@@ -1137,6 +1281,7 @@ mod tests {
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
guardian_policy_config: None,
|
||||
}
|
||||
);
|
||||
@@ -1219,6 +1364,71 @@ allowed_approvals_reviewers = ["user"]
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserialize_filesystem_deny_read_requirements() -> Result<()> {
|
||||
let deny_read_0 = if cfg!(windows) {
|
||||
r"C:\Users\alice\.gitconfig"
|
||||
} else {
|
||||
"/home/alice/.gitconfig"
|
||||
};
|
||||
let deny_read_1 = if cfg!(windows) {
|
||||
r"C:\Users\alice\.ssh"
|
||||
} else {
|
||||
"/home/alice/.ssh"
|
||||
};
|
||||
let toml_str = format!(
|
||||
r#"
|
||||
[permissions.filesystem]
|
||||
deny_read = [{deny_read_0:?}, {deny_read_1:?}]
|
||||
"#
|
||||
);
|
||||
|
||||
let config: ConfigRequirementsToml = from_str(&toml_str)?;
|
||||
let requirements: ConfigRequirements = with_unknown_source(config).try_into()?;
|
||||
|
||||
assert_eq!(
|
||||
requirements.filesystem,
|
||||
Some(Sourced::new(
|
||||
FilesystemConstraints {
|
||||
deny_read: vec![
|
||||
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,
|
||||
))
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserialize_apps_requirements() -> Result<()> {
|
||||
let toml_str = r#"
|
||||
|
||||
@@ -138,6 +138,27 @@ impl<T: Send + Sync> Constrained<T> {
|
||||
(self.validator)(candidate)
|
||||
}
|
||||
|
||||
/// Composes an additional validator onto the current constraint.
|
||||
///
|
||||
/// The existing value must satisfy the combined validator before it is installed.
|
||||
pub fn add_validator(
|
||||
&mut self,
|
||||
validator: impl Fn(&T) -> ConstraintResult<()> + Send + Sync + 'static,
|
||||
) -> ConstraintResult<()>
|
||||
where
|
||||
T: 'static,
|
||||
{
|
||||
let existing_validator = self.validator.clone();
|
||||
let combined_validator: Arc<ConstraintValidator<T>> = Arc::new(move |candidate| {
|
||||
existing_validator(candidate)?;
|
||||
validator(candidate)
|
||||
});
|
||||
|
||||
combined_validator(&self.value)?;
|
||||
self.validator = combined_validator;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn set(&mut self, value: T) -> ConstraintResult<()> {
|
||||
let value = if let Some(normalizer) = &self.normalizer {
|
||||
normalizer(value)
|
||||
@@ -227,6 +248,36 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn constrained_add_validator_composes_with_existing_validator() -> anyhow::Result<()> {
|
||||
let mut constrained = Constrained::new(/*initial_value*/ 5, |value: &i32| {
|
||||
if *value >= 0 {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(ConstraintError::empty_field("value"))
|
||||
}
|
||||
})?;
|
||||
constrained.add_validator(|value| {
|
||||
if *value <= 10 {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(ConstraintError::empty_field("value"))
|
||||
}
|
||||
})?;
|
||||
|
||||
assert_eq!(constrained.can_set(&7), Ok(()));
|
||||
assert_eq!(
|
||||
constrained.can_set(&11),
|
||||
Err(ConstraintError::empty_field("value"))
|
||||
);
|
||||
assert_eq!(
|
||||
constrained.can_set(&-1),
|
||||
Err(ConstraintError::empty_field("value"))
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn constrained_new_rejects_invalid_initial_value() {
|
||||
let result = Constrained::new(/*initial_value*/ 0, |value| {
|
||||
|
||||
@@ -31,6 +31,8 @@ pub use config_requirements::ConfigRequirementsToml;
|
||||
pub use config_requirements::ConfigRequirementsWithSources;
|
||||
pub use config_requirements::ConstrainedWithSource;
|
||||
pub use config_requirements::FeatureRequirementsToml;
|
||||
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;
|
||||
|
||||
@@ -5306,6 +5306,7 @@ async fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset()
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
guardian_policy_config: None,
|
||||
};
|
||||
let requirement_source = crate::config_loader::RequirementSource::Unknown;
|
||||
@@ -5949,6 +5950,7 @@ async fn explicit_sandbox_mode_falls_back_when_disallowed_by_requirements() -> s
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
guardian_policy_config: None,
|
||||
};
|
||||
|
||||
|
||||
@@ -1250,6 +1250,37 @@ fn resolve_permission_config_syntax(
|
||||
})
|
||||
}
|
||||
|
||||
fn apply_managed_filesystem_constraints(
|
||||
file_system_sandbox_policy: &mut FileSystemSandboxPolicy,
|
||||
filesystem_constraints: &crate::config_loader::FilesystemConstraints,
|
||||
) {
|
||||
for deny_read in &filesystem_constraints.deny_read {
|
||||
let deny_entry = if deny_read.contains_glob() {
|
||||
codex_protocol::permissions::FileSystemSandboxEntry {
|
||||
path: codex_protocol::permissions::FileSystemPath::GlobPattern {
|
||||
pattern: deny_read.as_str().to_string(),
|
||||
},
|
||||
access: codex_protocol::permissions::FileSystemAccessMode::None,
|
||||
}
|
||||
} else {
|
||||
let Ok(path) = AbsolutePathBuf::try_from(deny_read.as_str()) else {
|
||||
continue;
|
||||
};
|
||||
codex_protocol::permissions::FileSystemSandboxEntry {
|
||||
path: codex_protocol::permissions::FileSystemPath::Path { path },
|
||||
access: codex_protocol::permissions::FileSystemAccessMode::None,
|
||||
}
|
||||
};
|
||||
if !file_system_sandbox_policy
|
||||
.entries
|
||||
.iter()
|
||||
.any(|existing| existing == &deny_entry)
|
||||
{
|
||||
file_system_sandbox_policy.entries.push(deny_entry);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Optional overrides for user configuration (e.g., from CLI flags).
|
||||
#[derive(Default, Debug, Clone)]
|
||||
pub struct ConfigOverrides {
|
||||
@@ -1461,6 +1492,7 @@ impl Config {
|
||||
exec_policy: _,
|
||||
enforce_residency,
|
||||
network: network_requirements,
|
||||
filesystem: filesystem_requirements,
|
||||
} = config_layer_stack.requirements().clone();
|
||||
|
||||
let user_instructions = AgentsMdManager::load_global_instructions(Some(&codex_home))
|
||||
@@ -1970,6 +2002,34 @@ impl Config {
|
||||
&mut constrained_approval_policy,
|
||||
&mut startup_warnings,
|
||||
)?;
|
||||
if let Some(Sourced {
|
||||
value: filesystem_requirements,
|
||||
source: filesystem_requirements_source,
|
||||
}) = filesystem_requirements.as_ref()
|
||||
&& !filesystem_requirements.deny_read.is_empty()
|
||||
{
|
||||
let requirement_source = filesystem_requirements_source.clone();
|
||||
constrained_sandbox_policy
|
||||
.value
|
||||
.add_validator(move |policy| match policy {
|
||||
SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } => Ok(()),
|
||||
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => {
|
||||
Err(ConstraintError::InvalidValue {
|
||||
field_name: "sandbox_mode",
|
||||
candidate: policy.to_string(),
|
||||
allowed: "[read-only, workspace-write]".to_string(),
|
||||
requirement_source: requirement_source.clone(),
|
||||
})
|
||||
}
|
||||
})
|
||||
.map_err(std::io::Error::from)?;
|
||||
|
||||
if cfg!(target_os = "windows") {
|
||||
startup_warnings.push(format!(
|
||||
"managed filesystem deny_read from {filesystem_requirements_source} is only enforced for direct file tools on Windows; shell subprocess reads are not sandboxed"
|
||||
));
|
||||
}
|
||||
}
|
||||
apply_requirement_constrained_value(
|
||||
"approvals_reviewer",
|
||||
approvals_reviewer,
|
||||
@@ -2023,7 +2083,7 @@ impl Config {
|
||||
main_execve_wrapper_exe.as_ref(),
|
||||
);
|
||||
let effective_sandbox_policy = constrained_sandbox_policy.value.get().clone();
|
||||
let effective_file_system_sandbox_policy =
|
||||
let mut effective_file_system_sandbox_policy =
|
||||
if effective_sandbox_policy == original_sandbox_policy {
|
||||
file_system_sandbox_policy
|
||||
} else {
|
||||
@@ -2033,6 +2093,16 @@ impl Config {
|
||||
&file_system_sandbox_policy,
|
||||
)
|
||||
};
|
||||
if let Some(Sourced {
|
||||
value: filesystem_requirements,
|
||||
..
|
||||
}) = filesystem_requirements.as_ref()
|
||||
{
|
||||
apply_managed_filesystem_constraints(
|
||||
&mut effective_file_system_sandbox_policy,
|
||||
filesystem_requirements,
|
||||
);
|
||||
}
|
||||
let effective_file_system_sandbox_policy = effective_file_system_sandbox_policy
|
||||
.with_additional_readable_roots(resolved_cwd.as_path(), &helper_readable_roots);
|
||||
let effective_network_sandbox_policy =
|
||||
|
||||
@@ -41,6 +41,8 @@ pub use codex_config::ConfigRequirements;
|
||||
pub use codex_config::ConfigRequirementsToml;
|
||||
pub use codex_config::ConstrainedWithSource;
|
||||
pub use codex_config::FeatureRequirementsToml;
|
||||
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;
|
||||
@@ -378,6 +380,16 @@ async fn load_requirements_toml(
|
||||
.await
|
||||
{
|
||||
Ok(contents) => {
|
||||
let requirements_parent = requirements_toml_file.parent().ok_or_else(|| {
|
||||
io::Error::new(
|
||||
io::ErrorKind::InvalidData,
|
||||
format!(
|
||||
"Requirements file {} has no parent directory",
|
||||
requirements_toml_file.as_ref().display()
|
||||
),
|
||||
)
|
||||
})?;
|
||||
let _guard = AbsolutePathBufGuard::new(requirements_parent.as_path());
|
||||
let requirements_config: ConfigRequirementsToml =
|
||||
toml::from_str(&contents).map_err(|e| {
|
||||
io::Error::new(
|
||||
|
||||
@@ -10,6 +10,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;
|
||||
@@ -651,6 +652,7 @@ allowed_approval_policies = ["on-request"]
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
guardian_policy_config: None,
|
||||
}))
|
||||
}),
|
||||
@@ -703,6 +705,7 @@ allowed_approval_policies = ["on-request"]
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
guardian_policy_config: None,
|
||||
},
|
||||
);
|
||||
@@ -731,6 +734,103 @@ allowed_approval_policies = ["on-request"]
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn load_requirements_toml_resolves_deny_read_against_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", "../shared/secret.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 paths should load");
|
||||
|
||||
assert_eq!(
|
||||
deny_read,
|
||||
vec![
|
||||
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_deny_read_glob_against_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!(
|
||||
permissions.source,
|
||||
RequirementSource::SystemRequirementsToml {
|
||||
file: AbsolutePathBuf::try_from(requirements_file)?,
|
||||
}
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn load_config_layers_includes_cloud_requirements() -> anyhow::Result<()> {
|
||||
let tmp = tempdir()?;
|
||||
@@ -749,6 +849,7 @@ async fn load_config_layers_includes_cloud_requirements() -> anyhow::Result<()>
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
guardian_policy_config: None,
|
||||
};
|
||||
let expected = requirements.clone();
|
||||
|
||||
@@ -197,6 +197,22 @@ fn render_debug_config_lines(stack: &ConfigLayerStack) -> Vec<Line<'static>> {
|
||||
));
|
||||
}
|
||||
|
||||
if let Some(filesystem) = requirements.filesystem.as_ref() {
|
||||
let deny_read = join_or_empty(
|
||||
filesystem
|
||||
.value
|
||||
.deny_read
|
||||
.iter()
|
||||
.map(|pattern| pattern.as_str().to_string())
|
||||
.collect::<Vec<_>>(),
|
||||
);
|
||||
requirement_lines.push(requirement_line(
|
||||
"permissions.filesystem.deny_read",
|
||||
deny_read,
|
||||
Some(&filesystem.source),
|
||||
));
|
||||
}
|
||||
|
||||
if requirement_lines.is_empty() {
|
||||
lines.push(" <none>".dim().into());
|
||||
} else {
|
||||
@@ -458,6 +474,7 @@ mod tests {
|
||||
use crate::legacy_core::config_loader::ConfigRequirementsToml;
|
||||
use crate::legacy_core::config_loader::ConstrainedWithSource;
|
||||
use crate::legacy_core::config_loader::FeatureRequirementsToml;
|
||||
use crate::legacy_core::config_loader::FilesystemConstraints;
|
||||
use crate::legacy_core::config_loader::McpServerIdentity;
|
||||
use crate::legacy_core::config_loader::McpServerRequirement;
|
||||
use crate::legacy_core::config_loader::NetworkConstraints;
|
||||
@@ -549,6 +566,11 @@ mod tests {
|
||||
} else {
|
||||
absolute_path("/etc/codex/requirements.toml")
|
||||
};
|
||||
let denied_path = if cfg!(windows) {
|
||||
absolute_path("C:\\Users\\alice\\.gitconfig")
|
||||
} else {
|
||||
absolute_path("/home/alice/.gitconfig")
|
||||
};
|
||||
|
||||
let requirements = ConfigRequirements {
|
||||
approval_policy: ConstrainedWithSource::new(
|
||||
@@ -603,6 +625,14 @@ mod tests {
|
||||
},
|
||||
RequirementSource::CloudRequirements,
|
||||
)),
|
||||
filesystem: Some(Sourced::new(
|
||||
FilesystemConstraints {
|
||||
deny_read: vec![denied_path.clone().into()],
|
||||
},
|
||||
RequirementSource::SystemRequirementsToml {
|
||||
file: requirements_file.clone(),
|
||||
},
|
||||
)),
|
||||
..ConfigRequirements::default()
|
||||
};
|
||||
|
||||
@@ -627,6 +657,7 @@ mod tests {
|
||||
rules: None,
|
||||
enforce_residency: Some(ResidencyRequirement::Us),
|
||||
network: None,
|
||||
permissions: None,
|
||||
};
|
||||
|
||||
let user_file = if cfg!(windows) {
|
||||
@@ -671,6 +702,15 @@ mod tests {
|
||||
assert!(rendered.contains(
|
||||
"experimental_network: enabled=true, domains={example.com=allow} (source: cloud requirements)"
|
||||
));
|
||||
assert!(
|
||||
rendered.contains(
|
||||
format!(
|
||||
"permissions.filesystem.deny_read: {}",
|
||||
denied_path.as_path().display()
|
||||
)
|
||||
.as_str()
|
||||
)
|
||||
);
|
||||
assert!(!rendered.contains(" - rules:"));
|
||||
}
|
||||
|
||||
@@ -811,6 +851,7 @@ approval_policy = "never"
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
permissions: None,
|
||||
};
|
||||
|
||||
let stack = ConfigLayerStack::new(Vec::new(), requirements, requirements_toml)
|
||||
|
||||
Reference in New Issue
Block a user