mirror of
https://github.com/openai/codex.git
synced 2026-11-14 21:40:39 +00:00
Normalize managed network requirements before merging (#61391)
## Why Managed network requirements support both legacy tables and `permissions.network`. Equivalent declarations in one source should be accepted, while older runtime clients still need the top-level network field. ## What changed - Normalize `network_proxy` and `experimental_network` into `permissions.network` before merging requirements, preserving source attribution and explicit empty allowlist replacements. - Accept legacy and nested declarations in one source when they describe the same policy, comparing domain and review selectors by their canonical identities. Continue rejecting mixed surfaces across sources. - Populate the legacy output field for compatibility and preserve legacy selector text when both forms are supplied. - Include contributing requirement sources in cloud bundle errors without exposing submitted TOML. ## Testing Add coverage for equivalent and conflicting declarations, empty allowlist overrides, loader compatibility, and debug output. Add a proxy regression test verifying that a saved grant cannot bypass managed loopback restrictions. GitOrigin-RevId: e0d2a600f5c7c6c07a665ccc0e18a72ff172fd6d
This commit is contained in:
@@ -379,10 +379,11 @@ alongside `[permissions.filesystem]`:
|
||||
enabled = true
|
||||
```
|
||||
|
||||
`[network_proxy]` and its compatibility alias `[experimental_network]` still work.
|
||||
Do not mix either with `[permissions.network]`, even across requirements layers. A single
|
||||
layer cannot use both top-level names; across layers the top-level names merge with normal
|
||||
precedence. `configRequirements/read` returns the result in `requirements.network`.
|
||||
Codex reads `[network_proxy]` and its alias `[experimental_network]` as
|
||||
`[permissions.network]` within each source before merging managed requirements.
|
||||
Across sources, use the same surface (legacy or nested). Use at most one top-level spelling per layer.
|
||||
If a layer also sets `[permissions.network]`, both tables must describe the same policy.
|
||||
`configRequirements/read` returns the result in `requirements.network`.
|
||||
|
||||
In `config.toml`, `[features.network_proxy]` configures the proxy, while
|
||||
`[permissions.<profile>.network]` configures a profile's network settings. Managed requirements
|
||||
|
||||
@@ -23,13 +23,7 @@ pub(crate) fn validate_bundle(
|
||||
enterprise_managed_requirements,
|
||||
} = bundle_layers;
|
||||
|
||||
let requirements = compose_requirements(enterprise_managed_requirements).map_err(|_| {
|
||||
CloudConfigBundleLoadError::new(
|
||||
CloudConfigBundleLoadErrorCode::InvalidBundle,
|
||||
/*status_code*/ None,
|
||||
"invalid cloud config bundle: invalid requirements",
|
||||
)
|
||||
})?;
|
||||
let requirements = compose_requirements(enterprise_managed_requirements)?;
|
||||
if let Some(providers) = requirements.and_then(|requirements| requirements.model_providers) {
|
||||
validate_model_providers(&providers).map_err(|_| {
|
||||
CloudConfigBundleLoadError::new(
|
||||
|
||||
@@ -4,28 +4,56 @@ use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
|
||||
#[test]
|
||||
fn cloud_validation_rejects_mixed_network_requirements() {
|
||||
fn cloud_validation_checks_network_spellings_per_fragment() {
|
||||
let home = tempdir().expect("tempdir");
|
||||
let base_dir = AbsolutePathBuf::from_absolute_path(home.path()).expect("absolute path");
|
||||
let mut bundle = CloudConfigBundle::default();
|
||||
for alias in ["experimental_network", "network_proxy"] {
|
||||
let nested = "[permissions.network]\nenabled = false";
|
||||
let top_level = format!("[{alias}]\nenabled = false");
|
||||
for contents in [
|
||||
vec![format!("{nested}\n{top_level}")],
|
||||
vec![nested.to_owned(), top_level],
|
||||
let conflicting = format!("[{alias}]\nenabled = true");
|
||||
for (contents, expected) in [
|
||||
(vec![format!("{nested}\n{top_level}")], Ok(())),
|
||||
(
|
||||
vec![nested.to_owned(), conflicting.clone()],
|
||||
Err(CloudConfigBundleLoadErrorCode::InvalidBundle),
|
||||
),
|
||||
(
|
||||
vec![conflicting.clone(), nested.to_owned()],
|
||||
Err(CloudConfigBundleLoadErrorCode::InvalidBundle),
|
||||
),
|
||||
(
|
||||
vec![
|
||||
"[permissions.network]\ndenied_domains = ['shared.test']".to_string(),
|
||||
format!("[{alias}]\nallowed_domains = ['shared.test']"),
|
||||
],
|
||||
Err(CloudConfigBundleLoadErrorCode::InvalidBundle),
|
||||
),
|
||||
(
|
||||
vec![format!(
|
||||
"{nested}\n{conflicting}\nreview_domains = ['private-policy.example']"
|
||||
)],
|
||||
Err(CloudConfigBundleLoadErrorCode::InvalidBundle),
|
||||
),
|
||||
] {
|
||||
bundle.requirements_toml.enterprise_managed = contents
|
||||
.into_iter()
|
||||
.enumerate()
|
||||
.map(|(index, contents)| CloudRequirementsFragment {
|
||||
id: index.to_string(),
|
||||
id: format!("fragment-{index}"),
|
||||
name: index.to_string(),
|
||||
contents,
|
||||
})
|
||||
.collect();
|
||||
let err = validate_bundle(&bundle, &base_dir).expect_err("mixed policy spellings");
|
||||
assert_eq!(err.code(), CloudConfigBundleLoadErrorCode::InvalidBundle);
|
||||
let result = validate_bundle(&bundle, &base_dir);
|
||||
if let Err(error) = &result {
|
||||
let message = error.to_string();
|
||||
for fragment in &bundle.requirements_toml.enterprise_managed {
|
||||
assert!(message.contains(&fragment.id), "{message}");
|
||||
}
|
||||
assert!(!message.contains("private-policy.example"), "{message}");
|
||||
}
|
||||
assert_eq!(result.map_err(|error| error.code()), expected);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -390,6 +390,18 @@ pub enum NetworkDomainPermissionToml {
|
||||
Deny,
|
||||
}
|
||||
|
||||
impl NetworkDomainPermissionToml {
|
||||
/// Unsupported patterns retain their exact spelling. They can never compare equal
|
||||
/// to a valid selector and disappear when two published declarations are combined.
|
||||
pub(crate) fn pattern_identity(self, pattern: &str) -> Result<String, String> {
|
||||
match self {
|
||||
Self::Allow => codex_network_proxy::canonical_allow_pattern(pattern),
|
||||
Self::Deny => codex_network_proxy::canonical_deny_pattern(pattern),
|
||||
}
|
||||
.map_err(|_| pattern.to_owned())
|
||||
}
|
||||
}
|
||||
|
||||
impl std::fmt::Display for NetworkDomainPermissionToml {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
let permission = match self {
|
||||
@@ -456,6 +468,45 @@ pub struct NetworkRequirementsToml {
|
||||
pub header_injections: Option<Vec<NetworkHeaderInjectionToml>>,
|
||||
}
|
||||
|
||||
impl NetworkRequirementsToml {
|
||||
/// Compare the selector identities used by the proxy without modifying the published
|
||||
/// representation. Keep allow and deny contributions distinct, including shadowed grants.
|
||||
pub(crate) fn describes_same_policy(&self, other: &Self) -> bool {
|
||||
if self == other {
|
||||
return true;
|
||||
}
|
||||
let domain_rules = |policy: &Self| {
|
||||
policy.domains.as_ref().map(|domains| {
|
||||
domains
|
||||
.entries
|
||||
.iter()
|
||||
.map(|(pattern, &rule)| (rule, rule.pattern_identity(pattern)))
|
||||
.collect::<BTreeSet<_>>()
|
||||
})
|
||||
};
|
||||
let review_patterns = |policy: &Self| {
|
||||
policy.review_domains.as_ref().map(|patterns| {
|
||||
patterns
|
||||
.iter()
|
||||
.map(|pattern| {
|
||||
codex_network_proxy::canonical_review_pattern(pattern)
|
||||
.map_err(|_| pattern.to_owned())
|
||||
})
|
||||
.collect::<BTreeSet<_>>()
|
||||
})
|
||||
};
|
||||
if domain_rules(self) != domain_rules(other)
|
||||
|| review_patterns(self) != review_patterns(other)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
let mut comparable = other.clone();
|
||||
comparable.domains.clone_from(&self.domains);
|
||||
comparable.review_domains.clone_from(&self.review_domains);
|
||||
self == &comparable
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Clone, PartialEq, Eq)]
|
||||
pub struct NetworkHeaderInjectionToml {
|
||||
pub host: String,
|
||||
@@ -1344,15 +1395,16 @@ pub struct ConfigRequirementsWithSources {
|
||||
|
||||
impl ConfigRequirementsWithSources {
|
||||
pub(crate) fn validate_network_spelling(&self) -> Result<(), ConstraintError> {
|
||||
if let (Some(nested), Some(top_level)) = (&self.permissions_network, &self.network) {
|
||||
if let (Some(nested), Some(top_level)) = (&self.permissions_network, &self.network)
|
||||
&& !nested.value.describes_same_policy(&top_level.value)
|
||||
{
|
||||
return Err(ConstraintError::InvalidValue {
|
||||
field_name: "permissions.network",
|
||||
candidate: format!(
|
||||
"configured together with top-level network requirements from {}",
|
||||
top_level.source
|
||||
),
|
||||
allowed: "specify managed network requirements through exactly one surface"
|
||||
.to_string(),
|
||||
allowed: "legacy and permissions.network requirements must describe the same network policy".to_string(),
|
||||
requirement_source: nested.source.clone(),
|
||||
});
|
||||
}
|
||||
@@ -1542,6 +1594,8 @@ impl ConfigRequirementsWithSources {
|
||||
}
|
||||
}
|
||||
|
||||
/// Public loader view. Older runtime clients still read the top-level
|
||||
/// network field when selecting an executor policy, including on resume.
|
||||
pub fn into_toml(self) -> ConfigRequirementsToml {
|
||||
let ConfigRequirementsWithSources {
|
||||
config_server,
|
||||
@@ -1596,6 +1650,12 @@ impl ConfigRequirementsWithSources {
|
||||
permissions.get_or_insert_with(Default::default).network =
|
||||
Some(Box::new(network.value));
|
||||
}
|
||||
let network = network.map(|sourced| sourced.value).or_else(|| {
|
||||
permissions
|
||||
.as_ref()
|
||||
.and_then(|permissions| permissions.network.as_deref())
|
||||
.cloned()
|
||||
});
|
||||
ConfigRequirementsToml {
|
||||
config_server: config_server.map(|sourced| sourced.value),
|
||||
allowed_login_methods: allowed_login_methods.map(|sourced| sourced.value),
|
||||
@@ -1637,7 +1697,7 @@ impl ConfigRequirementsWithSources {
|
||||
apps: apps.map(|sourced| sourced.value),
|
||||
rules: rules.map(|sourced| sourced.value),
|
||||
enforce_residency: enforce_residency.map(|sourced| sourced.value),
|
||||
network: network.map(|sourced| sourced.value),
|
||||
network,
|
||||
application: application.map(|sourced| sourced.value),
|
||||
permissions,
|
||||
auto_review: auto_review.map(|sourced| sourced.value),
|
||||
@@ -2522,10 +2582,13 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
/*source*/ None,
|
||||
),
|
||||
};
|
||||
let network = permissions_network
|
||||
.map(|network| *network)
|
||||
.or(network)
|
||||
.map(|Sourced { value, source }| Sourced::new(NetworkConstraints::from(value), source));
|
||||
let network = match (permissions_network, network) {
|
||||
// A dual-published source keeps its original selector text on older executors.
|
||||
(Some(nested), Some(legacy)) if nested.source == legacy.source => Some(legacy),
|
||||
(Some(nested), _) => Some(*nested),
|
||||
(None, legacy) => legacy,
|
||||
}
|
||||
.map(|Sourced { value, source }| Sourced::new(NetworkConstraints::from(value), source));
|
||||
let filesystem = permissions
|
||||
.filter(|permissions| permissions.value.filesystem.is_some())
|
||||
.map(|Sourced { value, source }| {
|
||||
@@ -4995,6 +5058,15 @@ command = "python3 /enterprise/hooks/pre.py"
|
||||
))
|
||||
};
|
||||
assert_eq!(parse("network_proxy")?, parse("experimental_network")?);
|
||||
for table in ["network_proxy", "experimental_network"] {
|
||||
let dual: ConfigRequirementsToml = from_str(&format!(
|
||||
"[{table}]\ndenied_domains = ['2001:db8::1']\n\
|
||||
[permissions.network]\ndenied_domains = ['2001:0db8:0:0:0:0:0:1']"
|
||||
))?;
|
||||
let expected = dual.network.clone().map(NetworkConstraints::from);
|
||||
let runtime = ConfigRequirements::try_from(with_unknown_source(dual))?;
|
||||
assert_eq!(runtime.network.map(|network| network.value), expected);
|
||||
}
|
||||
for values in ["", "enabled = true"] {
|
||||
let both = format!("[network_proxy]\n{values}\n[experimental_network]\n{values}");
|
||||
assert!(from_str::<ConfigRequirementsToml>(&both).is_err());
|
||||
|
||||
@@ -172,6 +172,13 @@ async fn managed_network_requirements_control_broker_project_discovery() -> anyh
|
||||
format!("[{table}]\nenabled = {enabled}\n"),
|
||||
)?;
|
||||
let (canonical, local) = fixture.load().await?;
|
||||
let policy_only = crate::loader::load_managed_requirements_state(
|
||||
&TestFileSystem,
|
||||
&fixture.home,
|
||||
fixture.overrides.clone(),
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(&policy_only, canonical.requirements_toml());
|
||||
let mut local_effective = TomlValue::Table(toml::map::Map::new());
|
||||
for layer in local.config.layers {
|
||||
merge_toml_values(&mut local_effective, &layer.toml);
|
||||
@@ -203,8 +210,9 @@ async fn project_discovery_rejects_invalid_network_requirements() -> anyhow::Res
|
||||
fixture.overrides.system_requirements_path = Some(path.clone());
|
||||
for table in ["network_proxy", "experimental_network"] {
|
||||
for (nested, message) in [
|
||||
("http_port = 8090", "exactly one surface"),
|
||||
("enabled = false", "exactly one surface"),
|
||||
("http_port = 8090", "same network policy"),
|
||||
("enabled = false", "same network policy"),
|
||||
("review_domains = []", "same network policy"),
|
||||
("", "rename the managed profile"),
|
||||
(
|
||||
"extends = ':workspace'\nenabled = true",
|
||||
|
||||
@@ -154,10 +154,17 @@ impl ComposableRequirementsLayer {
|
||||
{
|
||||
table.insert("features".to_string(), features);
|
||||
}
|
||||
if let Some(table) = regular_toml.as_table_mut()
|
||||
&& let Some(network) = table.remove("experimental_network")
|
||||
if let Some(legacy) = &requirements.network
|
||||
&& let Some(nested) = requirements
|
||||
.permissions
|
||||
.as_ref()
|
||||
.and_then(|p| p.network.as_deref())
|
||||
&& !legacy.describes_same_policy(nested)
|
||||
{
|
||||
table.insert("network_proxy".to_string(), network);
|
||||
return Err(RequirementsCompositionError::Parse {
|
||||
layer_source: source,
|
||||
message: "legacy and permissions.network requirements must describe the same network policy".into(),
|
||||
});
|
||||
}
|
||||
|
||||
// Hostname lookup is configuration-driven and may block on DNS, so only
|
||||
@@ -301,11 +308,14 @@ pub(crate) fn strip_cloud_auth_requirements(source: &RequirementSource, value: &
|
||||
}
|
||||
}
|
||||
|
||||
fn remove_top_level_field(value: &mut TomlValue, key: &str) -> Option<TomlValue> {
|
||||
pub(super) fn remove_top_level_field(value: &mut TomlValue, key: &str) -> Option<TomlValue> {
|
||||
value.as_table_mut()?.remove(key)
|
||||
}
|
||||
|
||||
fn remove_nested_field_and_prune_empty(value: &mut TomlValue, path: &[&str]) -> Option<TomlValue> {
|
||||
pub(super) fn remove_nested_field_and_prune_empty(
|
||||
value: &mut TomlValue,
|
||||
path: &[&str],
|
||||
) -> Option<TomlValue> {
|
||||
let (key, remaining) = path.split_first()?;
|
||||
let table = value.as_table_mut()?;
|
||||
if remaining.is_empty() {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
mod hooks;
|
||||
mod layer;
|
||||
mod models;
|
||||
mod network;
|
||||
mod permissions;
|
||||
mod rules;
|
||||
mod stack;
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
//! Read compatible spellings of the shared network policy and preserve source attribution.
|
||||
|
||||
use crate::NetworkRequirementsToml;
|
||||
use crate::RequirementSource;
|
||||
use crate::Sourced;
|
||||
use crate::merge::merge_toml_values;
|
||||
use toml::Value;
|
||||
|
||||
use super::layer::ComposableRequirementsLayer;
|
||||
use super::layer::remove_nested_field_and_prune_empty;
|
||||
use super::layer::remove_top_level_field;
|
||||
use super::stack::RequirementsCompositionError;
|
||||
|
||||
/// Fold each validated source's network table before the regular merge. The
|
||||
/// submitted TOML distinguishes empty replacement lists from omitted lists.
|
||||
pub(super) fn compose(
|
||||
layers: &mut [ComposableRequirementsLayer],
|
||||
) -> Result<Option<Box<Sourced<NetworkRequirementsToml>>>, RequirementsCompositionError> {
|
||||
let mut merged = Value::Table(toml::Table::new());
|
||||
let mut sources = Vec::new();
|
||||
let mut has_legacy = false;
|
||||
let mut has_nested = false;
|
||||
for layer in layers {
|
||||
let regular_toml = &mut layer.regular_toml;
|
||||
let legacy = remove_top_level_field(regular_toml, "network_proxy")
|
||||
.or_else(|| remove_top_level_field(regular_toml, "experimental_network"));
|
||||
let nested = remove_nested_field_and_prune_empty(regular_toml, &["permissions", "network"]);
|
||||
has_legacy |= legacy.is_some();
|
||||
has_nested |= nested.is_some();
|
||||
let value = match (legacy, nested) {
|
||||
(Some(Value::Table(legacy)), Some(Value::Table(nested))) => {
|
||||
// The effective rules already agree. A deny can shadow an allow,
|
||||
// but an explicit empty allowlist clears inherited allows.
|
||||
if let (Some(left), Some(right)) = (
|
||||
legacy.get("allowed_domains").and_then(Value::as_array),
|
||||
nested.get("allowed_domains").and_then(Value::as_array),
|
||||
) && left.is_empty() != right.is_empty()
|
||||
{
|
||||
return Err(RequirementsCompositionError::Parse {
|
||||
layer_source: layer.source.clone(),
|
||||
message: "legacy and permissions.network requirements specify different allowed_domains replacements".into(),
|
||||
});
|
||||
}
|
||||
// Keep the legacy spelling for the executor.
|
||||
Value::Table(legacy)
|
||||
}
|
||||
(Some(value), None) | (None, Some(value)) => value,
|
||||
(None, None) => continue,
|
||||
(Some(_), Some(_)) => {
|
||||
return Err(RequirementsCompositionError::Parse {
|
||||
layer_source: layer.source.clone(),
|
||||
message: "shared network requirements must be tables".into(),
|
||||
});
|
||||
}
|
||||
};
|
||||
sources.push(layer.source.clone());
|
||||
merge_toml_values(&mut merged, &value);
|
||||
}
|
||||
if sources.is_empty() {
|
||||
return Ok(None);
|
||||
}
|
||||
// Until list composition can preserve competing replacements, require
|
||||
// multiple contributing sources to use the same surface.
|
||||
let mixed_sources = sources.len() > 1 && has_legacy && has_nested;
|
||||
let source = RequirementSource::composite(sources.into_iter().rev());
|
||||
if mixed_sources {
|
||||
return Err(RequirementsCompositionError::Parse {
|
||||
layer_source: source,
|
||||
message: "mixed legacy and permissions.network requirements".to_string(),
|
||||
});
|
||||
}
|
||||
let value = merged.try_into().map_err(|err: toml::de::Error| {
|
||||
RequirementsCompositionError::ComposedParse {
|
||||
message: err.to_string(),
|
||||
}
|
||||
})?;
|
||||
Ok(Some(Box::new(Sourced::new(value, source))))
|
||||
}
|
||||
@@ -20,7 +20,6 @@ use crate::ConfigRequirementsToml;
|
||||
use crate::ConfigRequirementsWithSources;
|
||||
use crate::RequirementSource;
|
||||
use crate::Sourced;
|
||||
use crate::config_requirements::PermissionsRequirementsToml;
|
||||
use crate::config_requirements::merge_otel_requirements;
|
||||
use crate::merge::merge_toml_values;
|
||||
use std::cell::OnceCell;
|
||||
@@ -55,6 +54,30 @@ pub enum RequirementsCompositionError {
|
||||
},
|
||||
}
|
||||
|
||||
impl From<RequirementsCompositionError> for crate::CloudConfigBundleLoadError {
|
||||
fn from(error: RequirementsCompositionError) -> Self {
|
||||
// Parser messages can contain the submitted TOML, including credentials.
|
||||
let reason = match error {
|
||||
RequirementsCompositionError::Parse { layer_source, .. } => {
|
||||
format!("invalid requirements in {layer_source}")
|
||||
}
|
||||
RequirementsCompositionError::Conflict {
|
||||
existing_source,
|
||||
incoming_source,
|
||||
..
|
||||
} => {
|
||||
format!("conflicting requirements from {existing_source} and {incoming_source}")
|
||||
}
|
||||
RequirementsCompositionError::ComposedParse { .. } => "invalid requirements".to_owned(),
|
||||
};
|
||||
Self::new(
|
||||
crate::CloudConfigBundleLoadErrorCode::InvalidBundle,
|
||||
/*status_code*/ None,
|
||||
format!("invalid cloud config bundle: {reason}"),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
impl From<RequirementsCompositionError> for io::Error {
|
||||
fn from(error: RequirementsCompositionError) -> Self {
|
||||
io::Error::new(io::ErrorKind::InvalidData, error)
|
||||
@@ -150,10 +173,11 @@ impl RequirementsLayerStack {
|
||||
self,
|
||||
) -> Result<Option<ConfigRequirementsWithSources>, RequirementsCompositionError> {
|
||||
let Self {
|
||||
layers,
|
||||
mut layers,
|
||||
hook_directory_field,
|
||||
} = self;
|
||||
|
||||
let network = super::network::compose(&mut layers)?;
|
||||
let mut merged_toml = TomlValue::Table(toml::map::Map::new());
|
||||
for layer in &layers {
|
||||
// An endpoint and its credentials must come from the same requirements layer.
|
||||
@@ -173,11 +197,8 @@ impl RequirementsLayerStack {
|
||||
})?;
|
||||
let mut output = ConfigRequirementsWithSources::default();
|
||||
populate_merged_regular_fields_with_sources(&mut output, requirements, &layers);
|
||||
output.validate_network_spelling().map_err(|err| {
|
||||
RequirementsCompositionError::ComposedParse {
|
||||
message: err.to_string(),
|
||||
}
|
||||
})?;
|
||||
output.permissions_network = network;
|
||||
|
||||
// Merge OTEL requirements so higher-priority exact values replace
|
||||
// lower-priority values while exporters stay atomic.
|
||||
let mut otel = None;
|
||||
@@ -281,9 +302,9 @@ fn populate_merged_regular_fields_with_sources(
|
||||
apps,
|
||||
rules: _,
|
||||
enforce_residency,
|
||||
network,
|
||||
network: _,
|
||||
application,
|
||||
mut permissions,
|
||||
permissions,
|
||||
auto_review,
|
||||
models,
|
||||
additional_developer_instructions,
|
||||
@@ -341,7 +362,6 @@ fn populate_merged_regular_fields_with_sources(
|
||||
set_sourced!(marketplaces, &["marketplaces"]);
|
||||
set_sourced!(apps, &["apps"]);
|
||||
set_sourced!(enforce_residency, &["enforce_residency"]);
|
||||
set_sourced!(network, &["network_proxy"]);
|
||||
set_sourced!(application, &["application"]);
|
||||
let permission_sources = |contributes: fn(&toml::Table) -> bool| {
|
||||
RequirementSource::composite(
|
||||
@@ -358,12 +378,6 @@ fn populate_merged_regular_fields_with_sources(
|
||||
.map(|layer| layer.source.clone()),
|
||||
)
|
||||
};
|
||||
if let Some(network) = PermissionsRequirementsToml::take_network(&mut permissions) {
|
||||
output.permissions_network = Some(Box::new(Sourced::new(
|
||||
*network,
|
||||
permission_sources(|table| table.contains_key("network")),
|
||||
)));
|
||||
}
|
||||
output.permissions = permissions.map(|value| {
|
||||
let source = if value.filesystem.is_some() {
|
||||
permission_sources(|table| table.contains_key("filesystem"))
|
||||
|
||||
@@ -48,7 +48,16 @@ fn compose_with_hook_directory_field(
|
||||
}
|
||||
|
||||
fn expected_requirements(contents: impl AsRef<str>) -> ConfigRequirementsToml {
|
||||
toml::from_str(contents.as_ref()).expect("parse expected requirements TOML")
|
||||
let mut expected: ConfigRequirementsToml =
|
||||
toml::from_str(contents.as_ref()).expect("parse expected requirements TOML");
|
||||
if let Some(shared) = expected
|
||||
.permissions
|
||||
.as_ref()
|
||||
.and_then(|p| p.network.as_deref())
|
||||
{
|
||||
expected.network = Some(shared.clone());
|
||||
}
|
||||
expected
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -813,13 +822,13 @@ fn network_maps_use_regular_toml_merge() {
|
||||
composed,
|
||||
expected_requirements(
|
||||
r#"
|
||||
[experimental_network.domains]
|
||||
[permissions.network.domains]
|
||||
"example.com" = "allow"
|
||||
"high.example.com" = "allow"
|
||||
"internal.example.com" = "deny"
|
||||
"low.example.com" = "deny"
|
||||
|
||||
[experimental_network.unix_sockets]
|
||||
[permissions.network.unix_sockets]
|
||||
"/tmp/admin.sock" = "deny"
|
||||
"/tmp/high.sock" = "allow"
|
||||
"/tmp/low.sock" = "allow"
|
||||
@@ -856,17 +865,26 @@ fn network_aliases_merge_with_layer_precedence_and_sources() {
|
||||
let composed = compose_requirements_for_hostname(vec![low, high], /*hostname*/ None)
|
||||
.expect("compose network aliases")
|
||||
.expect("requirements present");
|
||||
let expected: crate::NetworkRequirementsToml = toml::from_str(
|
||||
"enabled = false\nallow_local_binding = false\nallow_upstream_proxy = true\n[domains]\n'low.example.com' = 'allow'\n'high.example.com' = 'allow'\n'shared.example.com' = 'deny'",
|
||||
).expect("expected network");
|
||||
assert_eq!(
|
||||
composed.network,
|
||||
Some(Sourced::new(
|
||||
expected_requirements(
|
||||
"[network_proxy]\nenabled = false\nallow_local_binding = false\nallow_upstream_proxy = true\n\
|
||||
[network_proxy.domains]\n'low.example.com' = 'allow'\n'high.example.com' = 'allow'\n'shared.example.com' = 'deny'"
|
||||
)
|
||||
.network
|
||||
.expect("network requirements"),
|
||||
expected_source,
|
||||
))
|
||||
composed,
|
||||
ConfigRequirementsWithSources {
|
||||
permissions_network: Some(Box::new(Sourced::new(
|
||||
expected.clone(),
|
||||
expected_source
|
||||
))),
|
||||
..Default::default()
|
||||
}
|
||||
);
|
||||
let public_view = composed.into_toml();
|
||||
assert_eq!(public_view.network.as_ref(), Some(&expected));
|
||||
assert_eq!(
|
||||
public_view
|
||||
.permissions
|
||||
.and_then(|permissions| permissions.network),
|
||||
Some(Box::new(expected))
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -897,6 +915,151 @@ fn permissions_network_conflicts_with_top_level_network_across_layers() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shadowed_allows_agree_across_network_spellings() {
|
||||
let legacy = "[experimental_network]\nallowed_domains = ['a.test', 'b.test']\ndenied_domains = ['a.test', 'b.test']";
|
||||
let expected = expected_requirements(
|
||||
"[permissions.network.domains]\n'a.test' = 'deny'\n'b.test' = 'deny'",
|
||||
);
|
||||
for nested in [
|
||||
"[permissions.network]\nallowed_domains = ['a.test']\ndenied_domains = ['a.test', 'b.test']",
|
||||
"[permissions.network.domains]\n'a.test' = 'deny'\n'b.test' = 'deny'",
|
||||
] {
|
||||
let dual = format!("{legacy}\n{nested}");
|
||||
assert_eq!(
|
||||
compose(vec![layer("same", "Same", &dual)]).expect("both hosts remain denied"),
|
||||
Some(expected.clone()),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_network_allowlist_clears_inherited_allows() {
|
||||
for table in ["experimental_network", "permissions.network"] {
|
||||
let low = format!(
|
||||
"[{table}]\nallowed_domains = ['inherited.test']\ndenied_domains = ['blocked.test']"
|
||||
);
|
||||
let clear = format!("[{table}]\nallowed_domains = []");
|
||||
assert_eq!(
|
||||
compose(vec![
|
||||
layer("low", "Low", &low),
|
||||
layer("high", "High", &clear)
|
||||
])
|
||||
.expect("clear inherited allowlist"),
|
||||
Some(expected_requirements(
|
||||
"[permissions.network.domains]\n'blocked.test' = 'deny'"
|
||||
))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn network_surfaces_in_one_layer_must_agree() {
|
||||
for table in ["network_proxy", "experimental_network"] {
|
||||
for fields in ["", "allowed_domains = []", "description = 'device policy'"] {
|
||||
let entry = layer("device", "Device", &format!("[{table}]\n{fields}"));
|
||||
let expected = Sourced::new(
|
||||
crate::NetworkRequirementsToml::default(),
|
||||
entry.source.clone(),
|
||||
);
|
||||
assert_eq!(
|
||||
compose_requirements_for_hostname([entry], /*hostname*/ None)
|
||||
.expect("valid legacy input"),
|
||||
Some(ConfigRequirementsWithSources {
|
||||
permissions_network: Some(Box::new(expected)),
|
||||
..Default::default()
|
||||
}),
|
||||
);
|
||||
}
|
||||
let legacy_lists = "allowed_domains = ['MANAGED.TEST.']\ndenied_domains = ['2001:db8::1']";
|
||||
let legacy_map = "domains = { 'MANAGED.TEST.' = 'allow', '2001:db8::1' = 'deny' }";
|
||||
let nested_lists =
|
||||
"allowed_domains = ['managed.test']\ndenied_domains = ['2001:0db8:0:0:0:0:0:1']";
|
||||
let nested_map = "domains = { 'managed.test' = 'allow', '2001:0db8:0:0:0:0:0:1' = 'deny' }";
|
||||
let expected = expected_requirements(format!(
|
||||
"[permissions.network]\n{legacy_lists}\nreview_domains = ['*.First.Test.', 'second.test', '2001:db8::2']"
|
||||
));
|
||||
for legacy in [legacy_lists, legacy_map] {
|
||||
for nested in [nested_lists, nested_map] {
|
||||
let dual = format!(
|
||||
"[{table}]\n{legacy}\nreview_domains = ['*.First.Test.', 'second.test', '2001:db8::2']\n\
|
||||
[permissions.network]\n{nested}\nreview_domains = ['SECOND.TEST.', '?*.first.test', '*.first.test', '2001:0db8:0:0:0:0:0:2']"
|
||||
);
|
||||
assert_eq!(
|
||||
compose(vec![layer("same", "Same", &dual)]).expect("equivalent copies"),
|
||||
Some(expected.clone())
|
||||
);
|
||||
}
|
||||
}
|
||||
// An exact allow/deny pair stays a single denial even when only the nested
|
||||
// copy supplies the shadowed allow. The published denial keeps the legacy text.
|
||||
for (left, right, published) in [
|
||||
(
|
||||
"allowed_domains = ['ONE.TEST.']\ndenied_domains = ['ONE.TEST.']",
|
||||
"denied_domains = ['one.test']",
|
||||
"ONE.TEST.",
|
||||
),
|
||||
(
|
||||
"denied_domains = ['one.test']",
|
||||
"allowed_domains = ['ONE.TEST.']\ndenied_domains = ['ONE.TEST.']",
|
||||
"one.test",
|
||||
),
|
||||
] {
|
||||
let dual = format!("[{table}]\n{left}\n[permissions.network]\n{right}");
|
||||
assert_eq!(
|
||||
compose(vec![layer("same", "Same", &dual)]).expect("equivalent list operations"),
|
||||
Some(expected_requirements(format!(
|
||||
"[permissions.network]\ndenied_domains = ['{published}']"
|
||||
))),
|
||||
);
|
||||
}
|
||||
for (legacy, nested) in [
|
||||
(
|
||||
"allowed_domains = ['127.0.0.1:*']",
|
||||
"allowed_domains = ['127.0.0.1']",
|
||||
),
|
||||
(
|
||||
"allowed_domains = ['one.test']",
|
||||
"allowed_domains = ['one.test:443/path']",
|
||||
),
|
||||
(
|
||||
"review_domains = ['*.one.test']",
|
||||
"review_domains = ['**.ONE.TEST.']",
|
||||
),
|
||||
(
|
||||
"allowed_domains = ['ONE.TEST.']\ndenied_domains = ['one.test']",
|
||||
"denied_domains = ['one.test']",
|
||||
),
|
||||
] {
|
||||
let conflicting = format!("[{table}]\n{legacy}\n[permissions.network]\n{nested}");
|
||||
assert!(matches!(
|
||||
compose(vec![layer("conflict", "Conflict", &conflicting)]),
|
||||
Err(RequirementsCompositionError::Parse { .. })
|
||||
));
|
||||
}
|
||||
let shadowed = format!(
|
||||
"[{table}]\nallowed_domains = ['host.test']\ndenied_domains = ['host.test']\n[permissions.network]\nallowed_domains = []\ndenied_domains = ['host.test']"
|
||||
);
|
||||
assert!(matches!(
|
||||
compose(vec![layer("dual", "Dual", &shadowed)]),
|
||||
Err(RequirementsCompositionError::Parse { message, .. }) if message.contains("different allowed_domains replacements")
|
||||
));
|
||||
let entry = layer(
|
||||
"invalid",
|
||||
"Invalid",
|
||||
&format!("[{table}]\n[permissions.network]\nenabled = false"),
|
||||
);
|
||||
let source = entry.source.clone();
|
||||
let err = compose_requirements_for_hostname(vec![entry], /*hostname*/ None)
|
||||
.expect_err("conflicting network requirement surfaces must fail within one layer");
|
||||
assert!(
|
||||
matches!(&err, RequirementsCompositionError::Parse { layer_source, .. } if *layer_source == source),
|
||||
"{err}"
|
||||
);
|
||||
assert!(err.to_string().contains("same network policy"), "{err}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn browser_and_computer_use_requirements_use_regular_toml_merge() {
|
||||
let composed = compose(vec![
|
||||
@@ -1603,7 +1766,7 @@ deny_read = [{denied_path:?}]
|
||||
layer(
|
||||
"profile",
|
||||
"Profile",
|
||||
"[permissions.other]\nextends = ':workspace'",
|
||||
"[permissions.other]\nextends = ':workspace'\n[permissions.other.network]\nenabled = false",
|
||||
),
|
||||
],
|
||||
/*hostname*/ None,
|
||||
@@ -1630,6 +1793,8 @@ enabled = true
|
||||
|
||||
[permissions.other]
|
||||
extends = ":workspace"
|
||||
[permissions.other.network]
|
||||
enabled = false
|
||||
|
||||
[permissions.managed]
|
||||
extends = ":workspace"
|
||||
|
||||
@@ -146,7 +146,7 @@ action = ['redact']
|
||||
"[unix_sockets]\n'relative.sock' = 'allow'",
|
||||
"[unix_sockets]\n'~/allowed.sock' = 'allow'",
|
||||
"[unix_sockets]\n\"/tmp/\\u0000allowed.sock\" = 'allow'",
|
||||
"[domains]\n'[' = 'allow'",
|
||||
"[domains]\n'api[12].example' = 'allow'",
|
||||
] {
|
||||
let network: NetworkToml = toml::from_str(raw).unwrap();
|
||||
let projected = project_environment_profile_network(Some(network.clone()))
|
||||
@@ -154,6 +154,14 @@ action = ['redact']
|
||||
.unwrap();
|
||||
assert_eq!(projected, network);
|
||||
let configured_proxy = projected.to_network_proxy_config();
|
||||
assert!(
|
||||
PreparedNetworkConfig {
|
||||
configured_proxy: configured_proxy.clone()
|
||||
}
|
||||
.build(/*requirements*/ None, &PermissionProfile::read_only())
|
||||
.expect("a disabled unmanaged profile can load unused selectors")
|
||||
.is_none()
|
||||
);
|
||||
let policy = EnvironmentNetworkPolicy::from_config(
|
||||
&configured_proxy,
|
||||
/*managed_allowed_domains_only*/ false,
|
||||
|
||||
@@ -27,6 +27,72 @@ fn domain_permissions(
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn saved_grant_cannot_open_loopback_under_fixed_allowlist() -> anyhow::Result<()> {
|
||||
let profile = PermissionProfile::workspace_write();
|
||||
let spec = NetworkProxySpec::from_config_and_constraints(
|
||||
NetworkProxyConfig {
|
||||
proxy_url: "http://127.0.0.1:0".into(),
|
||||
enable_socks5: false,
|
||||
allow_upstream_proxy: false,
|
||||
..Default::default()
|
||||
},
|
||||
Some(NetworkConstraints {
|
||||
enabled: Some(true),
|
||||
managed_allowed_domains_only: Some(true),
|
||||
allow_local_binding: Some(false),
|
||||
domains: Some(domain_permissions([(
|
||||
"127.0.0.1:*",
|
||||
NetworkDomainPermissionToml::Allow,
|
||||
)])),
|
||||
..Default::default()
|
||||
}),
|
||||
&profile,
|
||||
)?;
|
||||
let mut saved = Policy::empty();
|
||||
saved.add_network_rule(
|
||||
"127.0.0.1",
|
||||
codex_execpolicy::NetworkRuleProtocol::Http,
|
||||
Allow,
|
||||
/*justification*/ None,
|
||||
)?;
|
||||
let spec = spec.with_exec_policy_network_rules(&saved)?;
|
||||
assert_eq!(
|
||||
spec.config.allowed_domains(),
|
||||
Some(vec!["127.0.0.1:*".to_owned()])
|
||||
);
|
||||
|
||||
core_test_support::skip_if_no_network!(Ok(()));
|
||||
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await?;
|
||||
let proxy = spec
|
||||
.start_proxy(
|
||||
&profile,
|
||||
ManagedProxyRouting::DedicatedListeners,
|
||||
DefaultFalse,
|
||||
/*policy_decider*/ None,
|
||||
/*blocked_request_observer*/ None,
|
||||
/*enable_network_approval_flow*/ false,
|
||||
NetworkProxyAuditMetadata::default(),
|
||||
)
|
||||
.await?;
|
||||
let request = codex_http_client::HttpClientBuilder::new()
|
||||
.build_direct()?
|
||||
.get(format!("http://{}/", proxy.proxy().http_addr()))
|
||||
.header(http::header::HOST, listener.local_addr()?.to_string())
|
||||
.timeout(std::time::Duration::from_secs(5))
|
||||
.send();
|
||||
tokio::select! {
|
||||
biased;
|
||||
connection = listener.accept() => anyhow::bail!("loopback destination reached: {:?}", connection?.1),
|
||||
response = request => {
|
||||
let response = response?;
|
||||
assert_eq!(response.status(), http::StatusCode::FORBIDDEN);
|
||||
assert_eq!(response.json::<serde_json::Value>().await?["reason"], "not_allowed_local");
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn attachment_socket_grants_respect_configured_restrictions_at_remote_launch()
|
||||
-> anyhow::Result<()> {
|
||||
|
||||
@@ -668,7 +668,6 @@ mod tests {
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::ConfigRequirements;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use codex_config::ConfigRequirementsWithSources;
|
||||
use codex_config::Constrained;
|
||||
use codex_config::ConstrainedWithSource;
|
||||
use codex_config::ConstraintError;
|
||||
@@ -686,11 +685,13 @@ mod tests {
|
||||
use codex_config::NetworkUnixSocketPermissionToml;
|
||||
use codex_config::NetworkUnixSocketPermissionsToml;
|
||||
use codex_config::RequirementSource;
|
||||
use codex_config::RequirementsLayerEntry;
|
||||
use codex_config::ResidencyRequirement;
|
||||
use codex_config::SandboxModeRequirement;
|
||||
use codex_config::Sourced;
|
||||
use codex_config::WebSearchModeRequirement;
|
||||
use codex_config::WindowsRequirementsToml;
|
||||
use codex_config::compose_requirements;
|
||||
use codex_config::sandbox_mode_requirement_for_permission_profile;
|
||||
use codex_config::types::FeedbackConfigToml;
|
||||
use codex_protocol::config_types::ApprovalsReviewer;
|
||||
@@ -1220,13 +1221,20 @@ interrupt_message = false
|
||||
"experimental_network",
|
||||
"permissions.network",
|
||||
] {
|
||||
let toml: ConfigRequirementsToml =
|
||||
toml::from_str(&format!("[{table}]\nenabled = true")).expect("requirements");
|
||||
let mut sourced = ConfigRequirementsWithSources::default();
|
||||
sourced.merge_unset_fields(RequirementSource::Unknown, toml.clone());
|
||||
let requirements = ConfigRequirements::try_from(sourced).expect("valid requirements");
|
||||
let stack =
|
||||
ConfigLayerStack::new(Vec::new(), requirements, toml).expect("config layer stack");
|
||||
let layer = RequirementsLayerEntry::from_toml(
|
||||
RequirementSource::EnterpriseManaged {
|
||||
id: "network".to_string(),
|
||||
name: "Network policy".to_string(),
|
||||
},
|
||||
format!("[{table}]\nenabled = true"),
|
||||
);
|
||||
let sourced = compose_requirements([layer])
|
||||
.expect("compose requirements")
|
||||
.expect("managed network requirements");
|
||||
let requirements =
|
||||
ConfigRequirements::try_from(sourced.clone()).expect("valid requirements");
|
||||
let stack = ConfigLayerStack::new(Vec::new(), requirements, sourced.into_toml())
|
||||
.expect("config layer stack");
|
||||
outputs.push(format!("{table}:\n{}", render_stack_to_text(&stack)));
|
||||
}
|
||||
|
||||
|
||||
+3
-3
@@ -9,7 +9,7 @@ Config layer stack (lowest precedence first):
|
||||
<none>
|
||||
|
||||
Requirements:
|
||||
- network_proxy: enabled=true (source: <unspecified>)
|
||||
- permissions.network: enabled=true (source: enterprise-managed requirements Network policy (network))
|
||||
|
||||
experimental_network:
|
||||
/debug-config
|
||||
@@ -18,7 +18,7 @@ Config layer stack (lowest precedence first):
|
||||
<none>
|
||||
|
||||
Requirements:
|
||||
- network_proxy: enabled=true (source: <unspecified>)
|
||||
- permissions.network: enabled=true (source: enterprise-managed requirements Network policy (network))
|
||||
|
||||
permissions.network:
|
||||
/debug-config
|
||||
@@ -27,4 +27,4 @@ Config layer stack (lowest precedence first):
|
||||
<none>
|
||||
|
||||
Requirements:
|
||||
- permissions.network: enabled=true (source: <unspecified>)
|
||||
- permissions.network: enabled=true (source: enterprise-managed requirements Network policy (network))
|
||||
|
||||
@@ -318,7 +318,7 @@ fn malformed_machine_policy_fails_closed() {
|
||||
validate_requirements(
|
||||
&WindowsSandboxProvisioningSettings::default(),
|
||||
WindowsSandboxProxyListeners::default(),
|
||||
&format!("[{table}]\nenabled = true\n[permissions.network]\nenabled = true")
|
||||
&format!("[{table}]\nenabled = true\n[permissions.network]\nenabled = false")
|
||||
)
|
||||
.is_err()
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user