From dfcb5b59942a0c1139162a74b6d3a80fa4ca66e5 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Thu, 29 Oct 2026 22:25:14 +0000 Subject: [PATCH] Support managed network requirements under `permissions.network` (#58182) ## What changed Allow shared network constraints in `[permissions.network]` in `requirements.toml`, alongside `[permissions.filesystem]`. Apply them throughout configuration loading, network approvals, Windows sandbox policy validation, and `configRequirements/read`, and show their source in `/debug-config`. Retain `[network_proxy]` and `[experimental_network]`, but reject mixing either with `[permissions.network]`, even across requirements layers. Preserve independent network and filesystem precedence and source attribution, and report unknown nested network fields. Reserve `network` as a managed requirements table name and reject empty or profile-shaped tables with migration guidance. Profiles named `network` remain valid in `config.toml`. Document that policy publishers must keep using `[experimental_network]` until all readers support the chosen replacement. ## Testing Add coverage for layered constraints and source attribution, conflicting spellings, profile-name migration errors, unknown-field diagnostics, and cloud validation. Extend API, project discovery, network approval, and Windows policy tests to cover `[permissions.network]`, and verify managed proxy settings remain independent of disabled feature configuration. GitOrigin-RevId: 994dfca9abf5a7aea914276c0ded8b40d7ed0372 --- codex-rs/app-server/README.md | 26 ++- .../request_processors/config_processor.rs | 8 +- .../v2/config_requirements_application.rs | 6 +- codex-rs/cloud-config/src/validation_tests.rs | 27 +++ codex-rs/config/src/config_requirements.rs | 158 ++++++++++++++---- codex-rs/config/src/loader/local.rs | 11 +- .../loader/managed_project_discovery_tests.rs | 58 ++++++- codex-rs/config/src/loader/mod.rs | 21 +-- .../src/requirements_layers/permissions.rs | 7 +- .../config/src/requirements_layers/stack.rs | 38 ++++- .../src/requirements_layers/stack_tests.rs | 95 +++++++++++ codex-rs/config/src/state.rs | 6 +- codex-rs/config/src/strict_config.rs | 20 ++- codex-rs/config/src/strict_config_tests.rs | 31 ++++ .../core/src/config/config_loader_tests.rs | 6 +- codex-rs/core/src/config/config_tests.rs | 111 ++++++++++++ codex-rs/core/src/config/mod.rs | 6 +- codex-rs/core/src/session/session.rs | 7 +- codex-rs/core/tests/suite/network_approval.rs | 20 ++- codex-rs/tui/src/debug_config.rs | 34 +++- ...g_config_network_requirement_surfaces.snap | 30 ++++ .../src/machine_policy.rs | 17 +- .../src/machine_policy_tests.rs | 76 ++++++--- 23 files changed, 708 insertions(+), 111 deletions(-) create mode 100644 codex-rs/tui/src/snapshots/codex_tui__debug_config__tests__debug_config_network_requirement_surfaces.snap diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index aade06a992..061c0839a6 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -339,19 +339,33 @@ fallback for unrecognized structured error variants. ## Managed network requirements -Use `[network_proxy]` in `requirements.toml` for shared managed network constraints. -`[experimental_network]` remains a compatibility alias. A single requirements layer cannot -contain both names; across layers, they merge with normal precedence. The -`configRequirements/read.network` response exposes the resulting constraints. +In `requirements.toml`, put shared network constraints directly in `[permissions.network]`, +alongside `[permissions.filesystem]`: + +```toml +[permissions.network] +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`. In `config.toml`, `[features.network_proxy]` configures the proxy, while `[permissions..network]` configures a profile's network settings. Managed requirements remain separate and override conflicting profile settings. They can start the proxy without enabling the feature; feature settings apply only when the feature is enabled. +In `requirements.toml`, `network` is reserved, like `filesystem`. Rename any managed profile +named `network` and update its references before upgrading. An empty `[permissions.network]` +is rejected because it is indistinguishable from an old empty profile. Omit the table when +there are no shared network settings. A profile named `network` is still allowed in `config.toml`. + Policy publishers must continue using `[experimental_network]` until every client and policy -validation layer supports `[network_proxy]`. Older clients or intermediaries may ignore or strip -the new spelling, or skip host-local validation. This reader change does not migrate or +validation layer supports the chosen new spelling. Older clients or intermediaries may ignore or +strip constraints in `[network_proxy]` or `[permissions.network]`; intermediaries may also skip +host-local validation. This reader change does not migrate or version-gate published policy bundles. # Amazon Bedrock authentication diff --git a/codex-rs/app-server/src/request_processors/config_processor.rs b/codex-rs/app-server/src/request_processors/config_processor.rs index b31f910fe2..5a47e3e0f2 100644 --- a/codex-rs/app-server/src/request_processors/config_processor.rs +++ b/codex-rs/app-server/src/request_processors/config_processor.rs @@ -453,6 +453,12 @@ pub(crate) fn map_requirements_to_api( } None => ConfigRequirementsToml::default(), }; + let network = requirements + .permissions + .as_ref() + .and_then(|permissions| permissions.network.as_deref()) + .cloned() + .or_else(|| requirements.network.clone()); Some(ConfigRequirements { model_provider: requirements.model_provider, @@ -578,7 +584,7 @@ pub(crate) fn map_requirements_to_api( enforce_residency: requirements .enforce_residency .map(map_residency_requirement_to_api), - network: requirements.network.map(map_network_requirements_to_api), + network: network.map(map_network_requirements_to_api), auto_review: requirements .auto_review .map(|auto_review| AutoReviewRequirements { diff --git a/codex-rs/app-server/tests/suite/v2/config_requirements_application.rs b/codex-rs/app-server/tests/suite/v2/config_requirements_application.rs index f79f99d5e5..84311bcd24 100644 --- a/codex-rs/app-server/tests/suite/v2/config_requirements_application.rs +++ b/codex-rs/app-server/tests/suite/v2/config_requirements_application.rs @@ -67,7 +67,11 @@ async fn application_network_defaults_and_null_base_url_are_exposed() -> Result< #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn application_destinations_stay_separate_from_agent_network() -> Result<()> { - for table in ["network_proxy", "experimental_network"] { + for table in [ + "network_proxy", + "experimental_network", + "permissions.network", + ] { let home = TempDir::new()?; std::fs::write( home.path().join("requirements.toml"), diff --git a/codex-rs/cloud-config/src/validation_tests.rs b/codex-rs/cloud-config/src/validation_tests.rs index 7f011f50ba..ad5503f9ea 100644 --- a/codex-rs/cloud-config/src/validation_tests.rs +++ b/codex-rs/cloud-config/src/validation_tests.rs @@ -3,6 +3,33 @@ use codex_config::CloudRequirementsFragment; use pretty_assertions::assert_eq; use tempfile::tempdir; +#[test] +fn cloud_validation_rejects_mixed_network_requirements() { + 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], + ] { + bundle.requirements_toml.enterprise_managed = contents + .into_iter() + .enumerate() + .map(|(index, contents)| CloudRequirementsFragment { + id: index.to_string(), + name: index.to_string(), + contents, + }) + .collect(); + let err = validate_bundle(&bundle, &base_dir).expect_err("mixed policy spellings"); + assert_eq!(err.code(), CloudConfigBundleLoadErrorCode::InvalidBundle); + } + } +} + #[test] fn cloud_fragments_combine_before_provider_validation() { let home = tempdir().expect("tempdir"); diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index e90c6ac020..523f5299fd 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -694,12 +694,57 @@ impl<'de> Deserialize<'de> for FilesystemRequirementsToml { #[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)] pub struct PermissionsRequirementsToml { pub filesystem: Option, - // For legacy reasons, `filesystem` stays reserved for requirements-level - // filesystem constraints and cannot name a profile. + /// Shared network constraints. Policy publishers must use `experimental_network` + /// until all clients that can read the policy support the chosen replacement. + // Keep the optional raw payload out of config-loading futures' inline storage. + #[serde(default, deserialize_with = "deserialize_permissions_network")] + pub network: Option>, + // `filesystem` and `network` are reserved for requirements-level + // constraints and cannot name profiles. #[serde(default, flatten)] pub profiles: BTreeMap, } +impl PermissionsRequirementsToml { + /// Split out the independent network policy without leaving an empty profile table behind. + pub(crate) fn take_network(table: &mut Option) -> Option> { + let permissions = table.as_mut()?; + let network = permissions.network.take()?; + if permissions.filesystem.is_none() && permissions.profiles.is_empty() { + *table = None; + } + Some(network) + } +} + +fn deserialize_permissions_network<'de, D>( + deserializer: D, +) -> Result>, D::Error> +where + D: serde::Deserializer<'de>, +{ + let table = toml::Table::deserialize(deserializer)?; + if table.is_empty() + || [ + "description", + "extends", + "workspace_roots", + "filesystem", + "network", + ] + .iter() + .any(|key| table.contains_key(*key)) + { + return Err(D::Error::custom( + "`permissions.network` in requirements.toml is reserved for shared network constraints; rename the managed profile (for example, to `network-access`) and update `default_permissions`, `allowed_permission_profiles`, and `extends`; check saved session profile references. Profiles named `network` in config.toml remain valid", + )); + } + toml::Value::Table(table) + .try_into() + .map(Some) + .map_err(D::Error::custom) +} + #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct FilesystemConstraints { pub deny_read: Vec, @@ -1254,6 +1299,8 @@ pub struct ConfigRequirementsWithSources { pub network: Option>, pub application: Option>, pub permissions: Option>, + /// Kept separate from profiles and filesystem so each retains its own precedence and source. + pub permissions_network: Option>>, pub auto_review: Option>, pub models: Option>, pub additional_developer_instructions: Option>, @@ -1262,6 +1309,22 @@ 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) { + 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(), + requirement_source: nested.source.clone(), + }); + } + Ok(()) + } + pub fn merge_unset_fields(&mut self, source: RequirementSource, other: ConfigRequirementsToml) { // For every field in `other` that is `Some`, if the corresponding field // in `self` is `None`, copy the value from `other` into `self`. @@ -1329,6 +1392,11 @@ impl ConfigRequirementsWithSources { } = &other; let mut other = other; + if let Some(network) = PermissionsRequirementsToml::take_network(&mut other.permissions) + && self.permissions_network.is_none() + { + self.permissions_network = Some(Box::new(Sourced::new(*network, source.clone()))); + } if other .guardian_policy_config .as_deref() @@ -1482,12 +1550,18 @@ impl ConfigRequirementsWithSources { network, application, permissions, + permissions_network, auto_review, models, additional_developer_instructions, guardian_policy_config, guardian_extra_policy, } = self; + let mut permissions = permissions.map(|sourced| sourced.value); + if let Some(network) = permissions_network { + permissions.get_or_insert_with(Default::default).network = + Some(Box::new(network.value)); + } ConfigRequirementsToml { config_server: config_server.map(|sourced| sourced.value), allowed_login_methods: allowed_login_methods.map(|sourced| sourced.value), @@ -1531,7 +1605,7 @@ impl ConfigRequirementsWithSources { enforce_residency: enforce_residency.map(|sourced| sourced.value), network: network.map(|sourced| sourced.value), application: application.map(|sourced| sourced.value), - permissions: permissions.map(|sourced| sourced.value), + permissions, auto_review: auto_review.map(|sourced| sourced.value), models: models.map(|sourced| sourced.value), additional_developer_instructions: additional_developer_instructions @@ -2025,6 +2099,7 @@ impl TryFrom for ConfigRequirements { type Error = ConstraintError; fn try_from(toml: ConfigRequirementsWithSources) -> Result { + toml.validate_network_spelling()?; // Profile catalog selection remains on ConfigRequirementsToml for // config loading and requirements API projection. Managed new-thread // defaults also remain there because they are initialization values; @@ -2070,6 +2145,7 @@ impl TryFrom for ConfigRequirements { network, application, permissions, + permissions_network, auto_review, models: _, additional_developer_instructions, @@ -2412,14 +2488,15 @@ impl TryFrom for ConfigRequirements { /*source*/ None, ), }; - let network = network.map(|sourced_network| { - 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) - }); + let network = permissions_network + .map(|network| *network) + .or(network) + .map(|Sourced { value, source }| Sourced::new(NetworkConstraints::from(value), source)); + let filesystem = permissions + .filter(|permissions| permissions.value.filesystem.is_some()) + .map(|Sourced { value, source }| { + Sourced::new(FilesystemConstraints::from(value), source) + }); let guardian_policy_config_source = guardian_policy_config.map(|sourced| sourced.source); let guardian_extra_policy_source = guardian_extra_policy.map(|sourced| sourced.source); Ok(ConfigRequirements { @@ -2726,6 +2803,9 @@ tracestate = { managed = { tenant = "enterprise", region = "us" }, other = { kee guardian_policy_config, guardian_extra_policy, } = toml; + let mut permissions = permissions; + let permissions_network = PermissionsRequirementsToml::take_network(&mut permissions) + .map(|network| Box::new(Sourced::new(*network, RequirementSource::Unknown))); ConfigRequirementsWithSources { config_server: config_server .map(|value| Sourced::new(value, RequirementSource::Unknown)), @@ -2791,6 +2871,7 @@ tracestate = { managed = { tenant = "enterprise", region = "us" }, other = { kee network: network.map(|value| Sourced::new(value, RequirementSource::Unknown)), application: application.map(|value| Sourced::new(value, RequirementSource::Unknown)), permissions: permissions.map(|value| Sourced::new(value, RequirementSource::Unknown)), + permissions_network, auto_review: auto_review.map(|value| Sourced::new(value, RequirementSource::Unknown)), models: models.map(|value| Sourced::new(value, RequirementSource::Unknown)), additional_developer_instructions: additional_developer_instructions @@ -2904,6 +2985,30 @@ tracestate = { managed = { tenant = "enterprise", region = "us" }, other = { kee ); } + #[test] + fn network_requirements_table_cannot_define_a_permission_profile() { + for profile_field in [ + "", + "description = 'network profile'", + "extends = ':workspace'", + "workspace_roots = {}", + "filesystem = {}", + "network = {}", + ] { + let err = from_str::(&format!( + "[permissions.network]\n{profile_field}" + )) + .expect_err("network requirements cannot define a permission profile"); + + assert!( + err.to_string().contains( + "`permissions.network` in requirements.toml is reserved for shared network constraints; rename the managed profile (for example, to `network-access`) and update `default_permissions`, `allowed_permission_profiles`, and `extends`; check saved session profile references. Profiles named `network` in config.toml remain valid" + ), + "{profile_field}: unexpected error: {err:#}" + ); + } + } + #[test] fn allow_appshots_false_is_still_configured() -> Result<()> { let requirements: ConfigRequirementsToml = from_str( @@ -3410,6 +3515,7 @@ tracestate = { managed = { tenant = "enterprise", region = "us" }, other = { kee network: None, application: None, permissions: None, + permissions_network: None, auto_review: Some(Sourced::new(auto_review, source.clone())), models: Some(Sourced::new(models, source.clone())), additional_developer_instructions: Some(Sourced::new( @@ -4853,7 +4959,6 @@ command = "python3 /enterprise/hooks/pre.py" )) }; assert_eq!(parse("network_proxy")?, parse("experimental_network")?); - for values in ["", "enabled = true"] { let both = format!("[network_proxy]\n{values}\n[experimental_network]\n{values}"); assert!(from_str::(&both).is_err()); @@ -4862,45 +4967,31 @@ command = "python3 /enterprise/hooks/pre.py" } #[test] - fn network_proxy_does_not_reserve_the_managed_network_profile() -> Result<()> { - let requirements: ConfigRequirementsToml = from_str( - "[network_proxy]\nenabled = true\n[permissions.network]\nextends = ':workspace'", - )?; - assert!( - requirements - .permissions - .expect("managed permissions") - .profiles - .contains_key("network") - ); - Ok(()) - } - - #[test] - fn network_requirements_are_preserved_as_constraints_with_source() -> Result<()> { + fn permissions_network_requirements_are_preserved_as_constraints_with_source() -> Result<()> { let toml_str = r#" - [experimental_network] + [permissions.network] enabled = true allow_upstream_proxy = false dangerously_allow_all_unix_sockets = true managed_allowed_domains_only = true allow_local_binding = false + future_setting = "ignored" - [experimental_network.domains] + [permissions.network.domains] "api.example.com" = "allow" "*.openai.com" = "allow" "blocked.example.com" = "deny" - [experimental_network.unix_sockets] + [permissions.network.unix_sockets] "/tmp/example.sock" = "allow" "/tmp/blocked.sock" = "deny" - [[experimental_network.header_injections]] + [[permissions.network.header_injections]] host = "api.example.com" methods = ["POST"] path_prefixes = ["/console/v1"] - [experimental_network.header_injections.headers] + [permissions.network.header_injections.headers] "x-statsig-change-source" = "codex" "#; @@ -4983,6 +5074,7 @@ command = "python3 /enterprise/hooks/pre.py" let toml_str = r#" [experimental_network] enabled = true + description = "ignored legacy metadata" allow_upstream_proxy = false dangerously_allow_all_unix_sockets = true allowed_domains = ["api.example.com", "*.openai.com"] diff --git a/codex-rs/config/src/loader/local.rs b/codex-rs/config/src/loader/local.rs index 86eca4d277..5a42cb4775 100644 --- a/codex-rs/config/src/loader/local.rs +++ b/codex-rs/config/src/loader/local.rs @@ -13,6 +13,7 @@ use super::system_config_toml_file_with_overrides; use super::system_requirements_toml_file_with_overrides; use crate::CONFIG_TOML_FILE; use crate::ConfigLayerSource; +use crate::ConfigRequirements; use crate::LoaderOverrides; use crate::RequirementSource; use crate::RequirementsLayerEntry; @@ -160,8 +161,10 @@ pub(super) async fn load_local_config_layers_with_overrides( "feature_requirements".to_string(), "credential_masking".to_string(), ], - vec!["experimental_network".to_string(), "enabled".to_string()], - vec!["network_proxy".to_string(), "enabled".to_string()], + // Keep the tables intact so discovery validates the same declarations as startup. + vec!["experimental_network".to_string()], + vec!["network_proxy".to_string()], + vec!["permissions".to_string(), "network".to_string()], ]); let effective_requirements = compose_requirements(broker_requirements.layers.into_iter().map(|layer| { @@ -171,7 +174,7 @@ pub(super) async fn load_local_config_layers_with_overrides( .unwrap_or_default(); apply_credential_broker_requirements( &mut trust_context, - &effective_requirements, + &ConfigRequirements::try_from(effective_requirements)?, &trusted_broker_config, ); let project_layers = discover_project_layers( @@ -219,7 +222,7 @@ pub(super) async fn load_local_config_layers_with_overrides( .as_table() .into_iter() .flatten() - .filter(|(name, _)| name.as_str() != "filesystem") + .filter(|(name, _)| !matches!(name.as_str(), "filesystem" | "network")) .map(|(name, profile)| { ( name.clone(), diff --git a/codex-rs/config/src/loader/managed_project_discovery_tests.rs b/codex-rs/config/src/loader/managed_project_discovery_tests.rs index b3aa423a3b..1148a048b5 100644 --- a/codex-rs/config/src/loader/managed_project_discovery_tests.rs +++ b/codex-rs/config/src/loader/managed_project_discovery_tests.rs @@ -161,7 +161,11 @@ async fn managed_network_requirements_control_broker_project_discovery() -> anyh let requirements_path = fixture._temp.path().join("requirements.toml"); fixture.overrides.system_requirements_path = Some(requirements_path.clone()); - for table in ["network_proxy", "experimental_network"] { + for table in [ + "network_proxy", + "experimental_network", + "permissions.network", + ] { for enabled in [false, true] { std::fs::write( &requirements_path, @@ -192,6 +196,47 @@ async fn managed_network_requirements_control_broker_project_discovery() -> anyh Ok(()) } +#[tokio::test] +async fn project_discovery_rejects_invalid_network_requirements() -> anyhow::Result<()> { + let mut fixture = Fixture::new()?; + let path = fixture._temp.path().join("requirements.toml"); + 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"), + ("", "rename the managed profile"), + ( + "extends = ':workspace'\nenabled = true", + "rename the managed profile", + ), + ] { + std::fs::write(&path, format!("[{table}]\n[permissions.network]\n{nested}"))?; + let local = load_local_config_layers_with_overrides( + &TestFileSystem, + &fixture.home, + &fixture.cwd, + &fixture.overrides, + ) + .await + .expect_err("executor-local discovery must reject mixed network requirements"); + let canonical = load_config_layers_state( + &TestFileSystem, + &fixture.home, + Some(fixture.cwd.clone()), + &[], + fixture.overrides.clone(), + &NoopThreadConfigLoader, + ) + .await + .expect_err("startup must reject mixed network requirements"); + assert_eq!(local.to_string(), canonical.to_string()); + assert!(local.to_string().contains(message), "{local}"); + } + } + Ok(()) +} + #[tokio::test] async fn managed_project_discovery_uses_file_markers_without_changing_precedence() -> anyhow::Result<()> { @@ -316,6 +361,8 @@ async fn project_cannot_define_or_rebind_credentials_via_permission_profiles() - let fixture = Fixture::new()?; let trusted = r#" [permissions.bridge] +[permissions.network] +extends = "bridge" [permissions.vault.network.mitm] version = 2 [[permissions.vault.network.mitm.actions.token.inject_request_headers]] @@ -333,8 +380,10 @@ action = ["token"] format!("{}{trusted}", fixture.trust("trusted")), )?; let managed = r#" +[permissions.network] +enabled = true [permissions.documents] -extends = "bridge" +extends = "network" [permissions.documents.network.mitm] version = 2 [permissions.documents.network.mitm.hooks.public] @@ -363,6 +412,11 @@ action = ["token"]"#, extends = "vault""#, "inheritance of MITM profile bridge", ), + ( + r#"[permissions.network] +extends = "vault""#, + "inheritance of MITM profile network", + ), ] { std::fs::write(&project_config, project)?; let canonical = load_config_layers_state( diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index a0ce5fcc48..cd57399506 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -21,6 +21,7 @@ use self::layer_io::LoadedConfigLayers; use crate::CONFIG_TOML_FILE; use crate::CloudConfigBundleLayers; use crate::ConfigLayerSource; +use crate::ConfigRequirements; use crate::ProfileV2Name; use crate::RequirementsLayerEntry; use crate::config_requirements::RequirementSource; @@ -339,6 +340,7 @@ pub async fn load_config_layers_state( ); } + let requirements = ConfigRequirements::try_from(config_requirements_toml.clone())?; let mut is_projectless = false; if !ignore_project_config && let Some(cwd) = cwd { let mut merged_so_far = TomlValue::Table(toml::map::Map::new()); @@ -403,7 +405,7 @@ pub async fn load_config_layers_state( }; apply_credential_broker_requirements( &mut project_trust_context, - &config_requirements_toml, + &requirements, &trusted_broker_config, ); let project_layers = load_project_layers( @@ -496,14 +498,13 @@ pub async fn load_config_layers_state( return Err(err); } - let mut config_layer_stack = ConfigLayerStack::new( - layers, - config_requirements_toml.clone().try_into()?, - config_requirements_toml.into_toml(), - )? - .with_cloud_config_binding(cloud_config_binding) - .with_applied_config_server(applied_config_server) - .with_user_and_project_exec_policy_rules_ignored(ignore_user_and_project_exec_policy_rules); + let mut config_layer_stack = + ConfigLayerStack::new(layers, requirements, config_requirements_toml.into_toml())? + .with_cloud_config_binding(cloud_config_binding) + .with_applied_config_server(applied_config_server) + .with_user_and_project_exec_policy_rules_ignored( + ignore_user_and_project_exec_policy_rules, + ); config_layer_stack.is_projectless = is_projectless; startup_warnings.extend(ignored_config_warning( &config_layer_stack, @@ -1007,7 +1008,7 @@ enum CredentialBrokerProjectState { fn apply_credential_broker_requirements( context: &mut ProjectTrustContext, - requirements: &crate::ConfigRequirementsWithSources, + requirements: &ConfigRequirements, trusted_broker_config: &TomlValue, ) { let credential_masking_enabled = requirements diff --git a/codex-rs/config/src/requirements_layers/permissions.rs b/codex-rs/config/src/requirements_layers/permissions.rs index e4dd2f0ef2..341c968c62 100644 --- a/codex-rs/config/src/requirements_layers/permissions.rs +++ b/codex-rs/config/src/requirements_layers/permissions.rs @@ -50,6 +50,7 @@ impl DenyReadMergeState { filesystem: Some(FilesystemRequirementsToml { deny_read: Some(self.deny_read), }), + network: None, profiles: Default::default(), }, source, @@ -67,9 +68,9 @@ impl DenyReadMergeState { deny_read.push(pattern); } } - if existing.source != source { - existing.source = RequirementSource::composite([existing.source.clone(), source]); - } + // The normalized filesystem constraint must name the layers that supplied denials, + // not layers that only supplied sibling profiles. + existing.source = source; } fn merge_source(&mut self, source: &RequirementSource) { diff --git a/codex-rs/config/src/requirements_layers/stack.rs b/codex-rs/config/src/requirements_layers/stack.rs index 10f31ae569..1323bb8cd2 100644 --- a/codex-rs/config/src/requirements_layers/stack.rs +++ b/codex-rs/config/src/requirements_layers/stack.rs @@ -20,6 +20,7 @@ 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; @@ -172,6 +173,11 @@ 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(), + } + })?; // Merge OTEL requirements so higher-priority exact values replace // lower-priority values while exporters stay atomic. let mut otel = None; @@ -277,7 +283,7 @@ fn populate_merged_regular_fields_with_sources( enforce_residency, network, application, - permissions, + mut permissions, auto_review, models, additional_developer_instructions, @@ -337,7 +343,35 @@ fn populate_merged_regular_fields_with_sources( set_sourced!(enforce_residency, &["enforce_residency"]); set_sourced!(network, &["network_proxy"]); set_sourced!(application, &["application"]); - set_sourced!(permissions, &["permissions"]); + let permission_sources = |contributes: fn(&toml::Table) -> bool| { + RequirementSource::composite( + layers + .iter() + .rev() + .filter(|layer| { + layer + .regular_toml + .get("permissions") + .and_then(TomlValue::as_table) + .is_some_and(contributes) + }) + .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")) + } else { + permission_sources(|table| table.keys().any(|key| key != "network")) + }; + Sourced::new(value, source) + }); set_sourced!(models, &["models"]); set_sourced!( additional_developer_instructions, diff --git a/codex-rs/config/src/requirements_layers/stack_tests.rs b/codex-rs/config/src/requirements_layers/stack_tests.rs index e402ccafab..4a638b26c7 100644 --- a/codex-rs/config/src/requirements_layers/stack_tests.rs +++ b/codex-rs/config/src/requirements_layers/stack_tests.rs @@ -870,6 +870,32 @@ fn network_aliases_merge_with_layer_precedence_and_sources() { } } +#[test] +fn permissions_network_conflicts_with_top_level_network_across_layers() { + for table in ["network_proxy", "experimental_network"] { + for (root_first, root_values) in [(true, "enabled = true"), (false, "")] { + let root = layer("root", "Root", &format!("[{table}]\n{root_values}")); + let permissions = layer( + "permissions", + "Permissions", + "[permissions.network]\nenabled = true", + ); + let root_source = root.source.clone(); + let permissions_source = permissions.source.clone(); + let layers = if root_first { + vec![root, permissions] + } else { + vec![permissions, root] + }; + let err = compose_requirements_for_hostname(layers, /*hostname*/ None) + .expect_err("network requirement surfaces must not coexist") + .to_string(); + assert!(err.contains(&root_source.to_string()), "{err}"); + assert!(err.contains(&permissions_source.to_string()), "{err}"); + } + } +} + #[test] fn browser_and_computer_use_requirements_use_regular_toml_merge() { let composed = compose(vec![ @@ -1542,6 +1568,75 @@ extends = ":workspace" ); } +#[test] +fn permissions_network_merges_with_higher_precedence_filesystem_requirements() { + let denied_path = if cfg!(windows) { + "C:\\secret" + } else { + "/secret" + }; + let low = layer( + "req_low", + "Low", + "[permissions.network]\nenabled = true\n[permissions.managed]\nextends = ':workspace'", + ); + let high = layer( + "req_high", + "High", + &format!( + r#" +[permissions.filesystem] +deny_read = [{denied_path:?}] + +[permissions.network.domains] +'example.com' = 'allow' +"# + ), + ); + let network_source = RequirementSource::composite([high.source.clone(), low.source.clone()]); + let filesystem_source = high.source.clone(); + let composed = compose_requirements_for_hostname( + [ + low, + high, + layer( + "profile", + "Profile", + "[permissions.other]\nextends = ':workspace'", + ), + ], + /*hostname*/ None, + ) + .expect("compose requirements") + .expect("requirements present"); + let effective = crate::ConfigRequirements::try_from(composed.clone()).expect("requirements"); + assert_eq!(effective.network.expect("network").source, network_source); + assert_eq!( + effective.filesystem.expect("filesystem").source, + filesystem_source + ); + assert_eq!( + composed.into_toml(), + expected_requirements(format!( + r#" +[permissions.filesystem] +deny_read = [{denied_path:?}] + +[permissions.network] +enabled = true +[permissions.network.domains] +'example.com' = 'allow' + +[permissions.other] +extends = ":workspace" + +[permissions.managed] +extends = ":workspace" +"# + )) + ); +} + #[test] fn deny_read_only_layers_do_not_leave_empty_permissions_tables() { let path = if cfg!(windows) { diff --git a/codex-rs/config/src/state.rs b/codex-rs/config/src/state.rs index 39bf96a852..2d679410d7 100644 --- a/codex-rs/config/src/state.rs +++ b/codex-rs/config/src/state.rs @@ -265,8 +265,8 @@ pub struct ConfigLayerStack { /// Raw requirements data as loaded from requirements.toml/MDM/legacy /// sources. This preserves the original allow-lists so they can be - /// surfaced via APIs. - requirements_toml: ConfigRequirementsToml, + /// surfaced via APIs. Keep it off the stack of futures that carry a config. + requirements_toml: Box, /// Whether execpolicy should skip `.rules` files from user and project config-layer folders. ignore_user_and_project_exec_policy_rules: bool, @@ -323,7 +323,7 @@ impl ConfigLayerStack { model_provider_requirements, layers, requirements, - requirements_toml, + requirements_toml: Box::new(requirements_toml), ignore_user_and_project_exec_policy_rules: false, startup_warnings: None, is_projectless: false, diff --git a/codex-rs/config/src/strict_config.rs b/codex-rs/config/src/strict_config.rs index 81e0d7e08c..5da8e8ab4c 100644 --- a/codex-rs/config/src/strict_config.rs +++ b/codex-rs/config/src/strict_config.rs @@ -3,6 +3,7 @@ use crate::CONFIG_TOML_FILE; use crate::ConfigLayerStack; use crate::ConfigRequirementsToml; +use crate::NetworkRequirementsToml; use crate::RequirementsLayerEntry; use crate::config_toml::ConfigToml; use crate::diagnostics::ConfigDiagnosticSource; @@ -277,11 +278,28 @@ pub(crate) fn ignored_config_warning( .as_ref() .map(|base_dir| AbsolutePathBufGuard::new(base_dir.as_path())); strip_cloud_auth_requirements(&source, &mut value); + // The flattened permissions deserializer consumes unknown network keys. + // Inspect that table separately to retain the usual typo diagnostics. + let permissions_network = value + .get("permissions") + .and_then(|permissions| permissions.get("network")) + .cloned(); fields.extend( ignored_toml_value_fields::(value) .into_iter() .map(|path| (format!("requirements ({source})"), path)), ); + if let Some(network) = permissions_network { + fields.extend( + ignored_toml_value_fields::(network) + .into_iter() + .map(|path| { + let mut full_path = vec!["permissions".to_string(), "network".to_string()]; + full_path.extend(path); + (format!("requirements ({source})"), full_path) + }), + ); + } } if fields.is_empty() { @@ -305,7 +323,7 @@ pub(crate) fn ignored_config_warning( " Remove windows.sandbox_private_desktop; legacy Windows sandboxes always use a private desktop." } ["network_proxy"] => { - " Use [permissions..network] for network settings, or [network_proxy] in requirements.toml for enforced network policy." + " Use [features.network_proxy] or [permissions..network] for settings in config.toml, or [network_proxy], [permissions.network], or [experimental_network] for enforced policy in requirements.toml." } ["allowed_permissions"] => { " Use [allowed_permission_profiles] and default_permissions; the old allowlist is ignored even when both forms are present." diff --git a/codex-rs/config/src/strict_config_tests.rs b/codex-rs/config/src/strict_config_tests.rs index 22a10b48ef..fb93006306 100644 --- a/codex-rs/config/src/strict_config_tests.rs +++ b/codex-rs/config/src/strict_config_tests.rs @@ -295,3 +295,34 @@ fn removed_private_desktop_setting_has_migration_hint() { "Codex is ignoring 1 unrecognized configuration setting. Check for typos or deprecated settings.\n session-flags: `windows.sandbox_private_desktop` is ignored. Remove windows.sandbox_private_desktop; legacy Windows sandboxes always use a private desktop." ); } + +#[test] +fn managed_network_unknown_fields_are_reported_for_each_spelling() { + let config = ConfigLayerStack::new( + Vec::new(), + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .unwrap(); + for table in [ + "network_proxy", + "experimental_network", + "permissions.network", + ] { + let requirements = RequirementsLayerEntry::from_toml( + crate::RequirementSource::Unknown, + format!( + "[{table}]\nenabld = true\n[{table}.domains]\n'example.com' = 'allow'\n\ + [[{table}.header_injections]]\nhost = 'example.com'\nmethods = ['GET']\n\ + path_prefixes = ['/']\nheaders = {{}}\nunknown = true\n" + ), + ); + assert_eq!( + ignored_config_warning(&config, &[requirements]), + Some(format!( + "Codex is ignoring 2 unrecognized configuration settings. Check for typos or deprecated settings.\n requirements (): `{table}.enabld` is ignored.\n requirements (): `{table}.header_injections.0.unknown` is ignored." + )), + "{table}" + ); + } +} diff --git a/codex-rs/core/src/config/config_loader_tests.rs b/codex-rs/core/src/config/config_loader_tests.rs index 3552bf3981..e0e85a4d79 100644 --- a/codex-rs/core/src/config/config_loader_tests.rs +++ b/codex-rs/core/src/config/config_loader_tests.rs @@ -1943,7 +1943,11 @@ async fn network_requirements_start_proxy_for_legacy_and_permission_configs() -> "default_permissions = 'dev'\n[permissions.dev]\nextends = ':workspace'\n[permissions.dev.network]\nenabled = true\n", "default_permissions = 'network'\n[permissions.network]\nextends = ':workspace'\n[permissions.network.network]\nenabled = true\n", ] { - for table in ["network_proxy", "experimental_network"] { + for table in [ + "network_proxy", + "experimental_network", + "permissions.network", + ] { let requirements_toml = format!( "[{table}]\nenabled = true\n[{table}.domains]\n'api.example.com' = 'allow'\n" ); diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 70cc395671..8b2548f7ea 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -2433,6 +2433,117 @@ enabled = true Ok(()) } +#[tokio::test] +async fn managed_network_requirements_ignore_disabled_proxy_feature_config() -> std::io::Result<()> +{ + let feature_config = r#" +[features.network_proxy] +enabled = true +dangerously_allow_all_unix_sockets = true +allow_local_binding = true +domains = { "unmanaged.example" = "allow" } +unix_sockets = { "/tmp/unmanaged.sock" = "allow" } +"#; + let requirements = r#" +default_permissions = "managed-primary" + +[features] +network_proxy = false + +[allowed_permission_profiles] +managed-primary = true +managed-secondary = true + +[permissions.network] +enabled = true +allow_upstream_proxy = false +domains = { "shared.example" = "allow", "blocked.example" = "deny" } + +[permissions.managed-primary] +extends = ":workspace" + +[permissions.managed-primary.network] +enabled = true +proxy_url = "http://127.0.0.1:43128" +dangerously_allow_all_unix_sockets = false +domains = { "primary.example" = "allow" } +unix_sockets = { "/tmp/primary.sock" = "allow" } + +[permissions.managed-secondary] +extends = "managed-primary" + +[permissions.managed-secondary.network] +proxy_url = "http://127.0.0.1:43129" +"#; + let mut baseline: Option<(NetworkProxySpec, NetworkProxySpec)> = None; + + for source in ["baseline", "user", "trusted project"] { + let codex_home = TempDir::new()?; + let workspace = TempDir::new()?; + let workspace_key = TomlValue::String(project_trust_key(workspace.path())); + let user_features = if source == "user" { feature_config } else { "" }; + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + format!( + r#" +[projects.{workspace_key}] +trust_level = "trusted" +{user_features} +"# + ), + )?; + if source == "trusted project" { + let project_config_dir = workspace.path().join(".codex"); + std::fs::create_dir_all(&project_config_dir)?; + std::fs::write(project_config_dir.join(CONFIG_TOML_FILE), feature_config)?; + } + + let config = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.path().to_path_buf()) + .harness_overrides(ConfigOverrides { + cwd: Some(workspace.path().to_path_buf()), + ..Default::default() + }) + .cloud_config_bundle( + CloudConfigBundleFixture::loader_with_enterprise_requirement(requirements), + ) + .build() + .await?; + + assert!(!config.features.enabled(Feature::NetworkProxy), "{source}"); + assert!(config.managed_network_requirements_enabled(), "{source}"); + let active_profile = config + .permissions + .active_permission_profile() + .expect("managed profile should be active"); + let initial = config + .permissions + .network + .as_ref() + .expect("shared requirements should enable the proxy"); + let mut selected_profile = active_profile; + selected_profile.id = "managed-secondary".to_string(); + let reselected = config + .network_proxy_spec_for_active_permission_profile( + &selected_profile, + config.permissions.permission_profile(), + )? + .expect("reselected managed profile should retain the proxy"); + + if let Some((baseline_initial, baseline_reselected)) = &baseline { + assert_eq!(initial, baseline_initial, "{source} initial"); + assert_eq!(&reselected, baseline_reselected, "{source} reselected"); + } else { + assert!(initial.enabled()); + assert!(reselected.enabled()); + assert_eq!(initial.proxy_host_and_port(), "127.0.0.1:43128"); + assert_eq!(reselected.proxy_host_and_port(), "127.0.0.1:43129"); + baseline = Some((initial.clone(), reselected)); + } + } + Ok(()) +} + #[tokio::test] async fn network_proxy_feature_uses_profile_network_proxy_settings() -> std::io::Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index eb949b8a46..1e16519c99 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -4916,11 +4916,7 @@ impl Config { !matches!( self.permissions.permission_profile(), PermissionProfile::Disabled - ) && self - .config_layer_stack - .requirements_toml() - .network - .is_some() + ) && self.config_layer_stack.requirements().network.is_some() } /// Resolves a named permission profile from effective config and managed requirements. diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 86b6cd600f..b3e6fd82cf 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -1591,11 +1591,8 @@ impl Session { state.reasoning_effort_pin = pin.as_ref().clone(); } state.active_disabled_plugin_ids = session_configuration.disabled_plugin_ids.clone(); - let managed_network_requirements_configured = config - .config_layer_stack - .requirements_toml() - .network - .is_some(); + let managed_network_requirements_configured = + config.config_layer_stack.requirements().network.is_some(); let managed_network_requirements_enabled = config.managed_network_requirements_enabled(); let network_approval = Arc::new(NetworkApprovalService::default()); diff --git a/codex-rs/core/tests/suite/network_approval.rs b/codex-rs/core/tests/suite/network_approval.rs index eb947c3688..f3b40af714 100644 --- a/codex-rs/core/tests/suite/network_approval.rs +++ b/codex-rs/core/tests/suite/network_approval.rs @@ -118,6 +118,7 @@ const NETWORK_TEST_TARGET: &str = "http://codex-network-test.invalid:80"; #[test_case("experimental_network"; "legacy_requirements")] #[test_case("network_proxy"; "proxy_requirements")] +#[test_case("permissions.network"; "canonical_requirements")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[cfg_attr( not(target_os = "linux"), @@ -1139,19 +1140,34 @@ async fn background_network_approval_uses_current_review_settings_and_original_e Ok(()) } +#[test_case("experimental_network"; "legacy_requirements")] +#[test_case("network_proxy"; "proxy_requirements")] +#[test_case("permissions.network"; "canonical_requirements")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[cfg_attr( not(target_os = "linux"), ignore = "requires the trusted Linux proxy bridge" )] -async fn user_network_approval_once_session_and_denial_semantics() -> Result<()> { +async fn user_network_approval_once_session_and_denial_semantics( + requirements_table: &str, +) -> Result<()> { skip_if_target_windows!(Ok(()), "uses the POSIX/Python network fixture"); skip_if_host_windows!(Ok(())); skip_if_no_network!(Ok(())); skip_if_sandbox!(Ok(())); let server = start_mock_server().await; - let test = managed_network_unified_exec_test(&server).await?; + let test = managed_network_unified_exec_builder(&[])? + .with_cloud_config_bundle( + CloudConfigBundleFixture::loader_with_enterprise_requirement(format!( + "[{requirements_table}]\nenabled = true\nallow_local_binding = true\n" + )), + ) + .build_with_remote_and_local_env(&server) + .await?; + assert!(test.config.managed_network_requirements_enabled()); + assert!(test.config.permissions.network.is_some()); + assert!(test.session_configured.network_proxy.is_some()); let environments = vec![local(test.config.cwd.clone())]; mount_exec_network_turn( diff --git a/codex-rs/tui/src/debug_config.rs b/codex-rs/tui/src/debug_config.rs index e7eb4d814a..73927c5545 100644 --- a/codex-rs/tui/src/debug_config.rs +++ b/codex-rs/tui/src/debug_config.rs @@ -358,8 +358,18 @@ fn render_debug_config_lines( } if let Some(network) = requirements.network.as_ref() { + let key = if requirements_toml + .permissions + .as_ref() + .and_then(|permissions| permissions.network.as_ref()) + .is_some() + { + "permissions.network" + } else { + "network_proxy" + }; requirement_lines.push(requirement_line( - "network_proxy", + key, format_network_constraints(&network.value), Some(&network.source), )); @@ -670,6 +680,7 @@ 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; @@ -1232,6 +1243,27 @@ interrupt_message = false ))); } + #[test] + fn debug_config_network_requirement_surfaces() { + let mut outputs = Vec::new(); + for table in [ + "network_proxy", + "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"); + outputs.push(format!("{table}:\n{}", render_stack_to_text(&stack))); + } + + insta::assert_snapshot!(outputs.join("\n\n")); + } + #[test] fn debug_config_output_lists_session_flag_key_value_pairs() { let session_flags = toml::from_str::( diff --git a/codex-rs/tui/src/snapshots/codex_tui__debug_config__tests__debug_config_network_requirement_surfaces.snap b/codex-rs/tui/src/snapshots/codex_tui__debug_config__tests__debug_config_network_requirement_surfaces.snap new file mode 100644 index 0000000000..4dc4214def --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__debug_config__tests__debug_config_network_requirement_surfaces.snap @@ -0,0 +1,30 @@ +--- +source: tui/src/debug_config.rs +expression: "outputs.join(\"\\n\\n\")" +--- +network_proxy: +/debug-config + +Config layer stack (lowest precedence first): + + +Requirements: + - network_proxy: enabled=true (source: ) + +experimental_network: +/debug-config + +Config layer stack (lowest precedence first): + + +Requirements: + - network_proxy: enabled=true (source: ) + +permissions.network: +/debug-config + +Config layer stack (lowest precedence first): + + +Requirements: + - permissions.network: enabled=true (source: ) diff --git a/codex-rs/windows-sandbox-service/src/machine_policy.rs b/codex-rs/windows-sandbox-service/src/machine_policy.rs index 971be1d3c6..4acac15b86 100644 --- a/codex-rs/windows-sandbox-service/src/machine_policy.rs +++ b/codex-rs/windows-sandbox-service/src/machine_policy.rs @@ -6,8 +6,8 @@ use anyhow::Result; use anyhow::bail; use codex_cloud_config::chatgpt_config_bundle_loader_for_storage_without_cache; use codex_config::ConfigLoadOptions; -use codex_config::ConfigRequirementsToml; -use codex_config::WindowsSandboxImplementationToml; +use codex_config::ConfigRequirements; +use codex_config::types::WindowsSandboxModeToml; use codex_core::config::bootstrap_auth_config; use codex_core::config::load_config_toml_with_layer_stack; use codex_windows_sandbox::WindowsSandboxProvisioningSettings; @@ -94,7 +94,7 @@ pub(crate) fn validate_provisioning_request( .await .context("load managed configuration")?; - Ok::<_, anyhow::Error>(config.config_layer_stack.requirements_toml().clone()) + Ok::<_, anyhow::Error>(config.config_layer_stack.requirements().clone()) } => result, } })?; @@ -106,18 +106,17 @@ pub(crate) fn validate_provisioning_request( fn validate_requirements( settings: &WindowsSandboxProvisioningSettings, listeners: &WindowsSandboxProxyListeners, - requirements: &ConfigRequirementsToml, + requirements: &ConfigRequirements, ) -> Result<()> { if requirements - .windows - .as_ref() - .and_then(|windows| windows.allowed_sandbox_implementations.as_ref()) - .is_some_and(|allowed| !allowed.contains(&WindowsSandboxImplementationToml::Elevated)) + .windows_sandbox_mode + .can_set(&Some(WindowsSandboxModeToml::Elevated)) + .is_err() { bail!("managed policy does not permit the elevated Windows sandbox"); } - let Some(network) = requirements.network.as_ref() else { + let Some(network) = requirements.network.as_ref().map(|network| &network.value) else { return Ok(()); }; if network.enabled == Some(false) diff --git a/codex-rs/windows-sandbox-service/src/machine_policy_tests.rs b/codex-rs/windows-sandbox-service/src/machine_policy_tests.rs index 520aa5a6ee..db194e757f 100644 --- a/codex-rs/windows-sandbox-service/src/machine_policy_tests.rs +++ b/codex-rs/windows-sandbox-service/src/machine_policy_tests.rs @@ -1,17 +1,28 @@ use anyhow::Result; -use codex_config::ConfigRequirementsToml; +use codex_config::ConfigRequirements; +use codex_config::ConfigRequirementsWithSources; +use codex_config::RequirementSource; use codex_windows_sandbox::WindowsSandboxProvisioningSettings; use codex_windows_sandbox::WindowsSandboxProxyListeners; use pretty_assertions::assert_eq; use crate::ipc::ProvisioningRequest; +const NETWORK_TABLES: [&str; 3] = [ + "network_proxy", + "experimental_network", + "permissions.network", +]; + fn validate_requirements( settings: &WindowsSandboxProvisioningSettings, listeners: WindowsSandboxProxyListeners, contents: &str, ) -> Result<()> { - let requirements: ConfigRequirementsToml = toml::from_str(contents)?; + let mut requirements_with_sources = ConfigRequirementsWithSources::default(); + requirements_with_sources + .merge_unset_fields(RequirementSource::Unknown, toml::from_str(contents)?); + let requirements = ConfigRequirements::try_from(requirements_with_sources)?; super::validate_requirements(settings, &listeners, &requirements) } @@ -57,7 +68,7 @@ fn unmanaged_policy_allows_default_and_requested_network_settings() -> Result<() http_ports: vec![3128], socks_ports: vec![8080], }, - "allowed_sandbox_modes = [\"workspace-write\"]", + "allowed_sandbox_modes = [\"read-only\", \"workspace-write\"]", ) } @@ -91,14 +102,14 @@ fn forbidden_local_binding_is_rejected() { proxy_ports: Vec::new(), allow_local_binding: true, }; - for table in ["network_proxy", "experimental_network"] { - assert!( - validate_requirements( - &settings, - WindowsSandboxProxyListeners::default(), - &format!("[{table}]\nallow_local_binding = false") - ) - .is_err(), + for table in NETWORK_TABLES { + let policy = format!("[{table}]\nallow_local_binding = false"); + let error = + validate_requirements(&settings, WindowsSandboxProxyListeners::default(), &policy) + .expect_err("local binding must be rejected"); + assert_eq!( + error.to_string(), + "managed policy does not permit local network binding", "{table}" ); } @@ -106,7 +117,7 @@ fn forbidden_local_binding_is_rejected() { #[test] fn disabled_network_rejects_proxy_ports_and_local_binding() -> Result<()> { - for table in ["network_proxy", "experimental_network"] { + for table in NETWORK_TABLES { let policy = format!("[{table}]\nenabled = false"); validate_requirements( &WindowsSandboxProvisioningSettings::default(), @@ -167,7 +178,7 @@ fn managed_proxy_ports_allow_only_configured_ports() -> Result<()> { proxy_ports: vec![1080, 3128, 8080], allow_local_binding: false, }; - for table in ["network_proxy", "experimental_network"] { + for table in NETWORK_TABLES { let policy = format!("[{table}]\nhttp_port = 3128\nsocks_port = 1080"); for listeners in [ WindowsSandboxProxyListeners { @@ -183,13 +194,20 @@ fn managed_proxy_ports_allow_only_configured_ports() -> Result<()> { http_ports: vec![3128, 8080], socks_ports: vec![1080], }, + WindowsSandboxProxyListeners { + http_ports: vec![3128], + socks_ports: vec![1080, 8080], + }, WindowsSandboxProxyListeners { http_ports: vec![3128], socks_ports: vec![1080], }, WindowsSandboxProxyListeners::default(), ] { - assert!(validate_requirements(&prohibited, listeners, &policy).is_err()); + assert!( + validate_requirements(&prohibited, listeners, &policy).is_err(), + "{table}" + ); } validate_requirements( &WindowsSandboxProvisioningSettings::default(), @@ -283,14 +301,28 @@ fn managed_socks_port_does_not_require_a_disabled_socks_listener() -> Result<()> #[test] fn malformed_machine_policy_fails_closed() { - assert!( - validate_requirements( - &WindowsSandboxProvisioningSettings::default(), - WindowsSandboxProxyListeners::default(), - "[experimental_network]\nhttp_port = \"invalid\"" - ) - .is_err() - ); + for table in NETWORK_TABLES { + let policy = format!("[{table}]\nhttp_port = \"invalid\""); + assert!( + validate_requirements( + &WindowsSandboxProvisioningSettings::default(), + WindowsSandboxProxyListeners::default(), + &policy, + ) + .is_err(), + "{table}" + ); + } + for table in ["network_proxy", "experimental_network"] { + assert!( + validate_requirements( + &WindowsSandboxProvisioningSettings::default(), + WindowsSandboxProxyListeners::default(), + &format!("[{table}]\nenabled = true\n[permissions.network]\nenabled = true") + ) + .is_err() + ); + } assert!( validate_requirements( &WindowsSandboxProvisioningSettings::default(),