From c3d7d1e214627c936e0288a2249f6f74c3db5d5e Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 20 May 2026 08:14:38 -0700 Subject: [PATCH 1/4] feat(permissions): resolve profile inheritance Co-authored-by: Codex noreply@openai.com --- codex-rs/config/src/permissions_toml.rs | 174 ++++++++++++++++++ codex-rs/core/config.schema.json | 3 + codex-rs/core/src/config/config_tests.rs | 99 ++++++---- codex-rs/core/src/config/permissions_tests.rs | 154 ++++++++++++++++ 4 files changed, 390 insertions(+), 40 deletions(-) diff --git a/codex-rs/config/src/permissions_toml.rs b/codex-rs/config/src/permissions_toml.rs index 4d533776ad0..151d9e5ff2e 100644 --- a/codex-rs/config/src/permissions_toml.rs +++ b/codex-rs/config/src/permissions_toml.rs @@ -1,5 +1,6 @@ use std::collections::BTreeMap; +use crate::merge::merge_toml_values; use codex_network_proxy::NetworkDomainPermission as ProxyNetworkDomainPermission; use codex_network_proxy::NetworkMode; use codex_network_proxy::NetworkProxyConfig; @@ -9,6 +10,8 @@ use codex_protocol::permissions::FileSystemAccessMode; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; +use thiserror::Error; +use toml::Value as TomlValue; #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] pub struct PermissionsToml { @@ -20,17 +23,188 @@ impl PermissionsToml { pub fn is_empty(&self) -> bool { self.entries.is_empty() } + + pub fn resolve_profile( + &self, + profile_name: &str, + mut parent_profile: F, + ) -> Result + where + F: FnMut(&str) -> Option, + { + let mut profile_names = Vec::new(); + let mut profiles = Vec::new(); + let mut next_profile_name = profile_name.to_string(); + let mut referenced_by: Option = None; + + loop { + if let Some(cycle_start) = profile_names + .iter() + .position(|name| name == &next_profile_name) + { + let cycle = profile_names[cycle_start..] + .iter() + .cloned() + .chain(std::iter::once(next_profile_name)) + .collect::>(); + return Err(PermissionProfileResolutionError::Cycle { cycle }); + } + + let profile = self + .entries + .get(&next_profile_name) + .cloned() + .or_else(|| parent_profile(&next_profile_name)) + .ok_or_else(|| { + referenced_by.as_deref().map_or_else( + || PermissionProfileResolutionError::UndefinedProfile { + profile_name: next_profile_name.clone(), + }, + |referenced_by| { + if next_profile_name.starts_with(':') { + PermissionProfileResolutionError::UnsupportedBuiltInParent { + profile_name: referenced_by.to_string(), + parent_profile_name: next_profile_name.clone(), + } + } else { + PermissionProfileResolutionError::UndefinedParent { + profile_name: referenced_by.to_string(), + parent_profile_name: next_profile_name.clone(), + } + } + }, + ) + })?; + let parent_profile_name = profile.extends.clone(); + + profile_names.push(next_profile_name.clone()); + + if let Some(parent_profile_name) = parent_profile_name { + profiles.push(profile); + referenced_by = Some(next_profile_name); + next_profile_name = parent_profile_name; + continue; + } + + let profile = profiles + .into_iter() + .rev() + .try_fold(profile, merge_permission_profiles)?; + return Ok(ResolvedPermissionProfileToml { + profile, + inherited_profile_names: profile_names.into_iter().skip(1).collect(), + }); + } + } } #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] #[schemars(deny_unknown_fields)] pub struct PermissionProfileToml { pub description: Option, + pub extends: Option, pub workspace_roots: Option, pub filesystem: Option, pub network: Option, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ResolvedPermissionProfileToml { + pub profile: PermissionProfileToml, + /// Names of profiles inherited while resolving `profile`, ordered from the + /// selected profile's direct parent to the farthest ancestor. + /// + /// Callers use this to preserve which built-in baseline contributed the + /// resolved permissions after the parent profiles have been merged away. + pub inherited_profile_names: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq, Error)] +pub enum PermissionProfileResolutionError { + #[error("default_permissions refers to undefined profile `{profile_name}`")] + UndefinedProfile { profile_name: String }, + #[error( + "permissions profile `{profile_name}` extends undefined profile `{parent_profile_name}`" + )] + UndefinedParent { + profile_name: String, + parent_profile_name: String, + }, + #[error( + "permissions profile `{profile_name}` cannot extend unsupported built-in profile `{parent_profile_name}`" + )] + UnsupportedBuiltInParent { + profile_name: String, + parent_profile_name: String, + }, + #[error( + "permissions profile inheritance cycle detected: {}", + cycle.join(" -> ") + )] + Cycle { cycle: Vec }, + #[error("failed to serialize permissions profile while resolving inheritance: {source}")] + SerializeProfileToml { + #[source] + source: toml::ser::Error, + }, + #[error( + "failed to deserialize merged permissions profile while resolving inheritance: {source}" + )] + DeserializeProfileToml { + #[source] + source: toml::de::Error, + }, +} + +fn merge_permission_profiles( + mut parent: PermissionProfileToml, + mut child: PermissionProfileToml, +) -> Result { + let merges_network_domains = parent + .network + .as_ref() + .and_then(|network| network.domains.as_ref()) + .is_some() + && child + .network + .as_ref() + .and_then(|network| network.domains.as_ref()) + .is_some(); + + parent.description = None; + parent.extends = None; + + if merges_network_domains { + normalize_profile_network_domains(&mut parent); + normalize_profile_network_domains(&mut child); + } + + let mut merged = TomlValue::try_from(parent) + .map_err(|source| PermissionProfileResolutionError::SerializeProfileToml { source })?; + let child = TomlValue::try_from(child) + .map_err(|source| PermissionProfileResolutionError::SerializeProfileToml { source })?; + merge_toml_values(&mut merged, &child); + merged + .try_into() + .map_err(|source| PermissionProfileResolutionError::DeserializeProfileToml { source }) +} + +fn normalize_profile_network_domains(profile: &mut PermissionProfileToml) { + let Some(domains) = profile + .network + .as_mut() + .and_then(|network| network.domains.as_mut()) + else { + return; + }; + + let entries = std::mem::take(&mut domains.entries); + domains.entries = entries + .into_iter() + .map(|(pattern, permission)| (normalize_host(&pattern), permission)) + .collect(); +} + #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] pub struct WorkspaceRootsToml { #[serde(flatten)] diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 0e811e9017e..f97286475fb 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -2005,6 +2005,9 @@ "description": { "type": "string" }, + "extends": { + "type": "string" + }, "filesystem": { "$ref": "#/definitions/FilesystemPermissionsToml" }, diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index f968677483f..1e4d0f16b96 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -732,43 +732,44 @@ async fn runtime_config_uses_tui_raw_output_mode() { #[test] fn config_toml_deserializes_permission_profiles() { let toml = r#" -default_permissions = "workspace" +default_permissions = "dev" -[permissions.workspace] +[permissions.dev] description = "Day-to-day workspace access." -[permissions.workspace.workspace_roots] +[permissions.dev.workspace_roots] "~/code/openai" = true "~/code/ignored" = false -[permissions.workspace.filesystem] +[permissions.dev.filesystem] ":minimal" = "read" "/tmp/secret.env" = "deny" -[permissions.workspace.filesystem.":workspace_roots"] +[permissions.dev.filesystem.":workspace_roots"] "." = "write" "docs" = "read" -[permissions.workspace.network] +[permissions.dev.network] enabled = true proxy_url = "http://127.0.0.1:43128" enable_socks5 = false allow_upstream_proxy = false -[permissions.workspace.network.domains] +[permissions.dev.network.domains] "openai.com" = "allow" "#; let cfg: ConfigToml = toml::from_str(toml).expect("TOML deserialization should succeed for permissions profiles"); - assert_eq!(cfg.default_permissions.as_deref(), Some("workspace")); + assert_eq!(cfg.default_permissions.as_deref(), Some("dev")); assert_eq!( cfg.permissions.expect("[permissions] should deserialize"), PermissionsToml { entries: BTreeMap::from([( - "workspace".to_string(), + "dev".to_string(), PermissionProfileToml { description: Some("Day-to-day workspace access.".to_string()), + extends: None, workspace_roots: Some(WorkspaceRootsToml { entries: BTreeMap::from([ ("~/code/ignored".to_string(), false), @@ -829,12 +830,13 @@ async fn permissions_profiles_proxy_policy_does_not_start_managed_network_proxy_ let config = Config::load_from_base_config_with_overrides( ConfigToml { - default_permissions: Some("workspace".to_string()), + default_permissions: Some("dev".to_string()), permissions: Some(PermissionsToml { entries: BTreeMap::from([( - "workspace".to_string(), + "dev".to_string(), PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: None, @@ -984,12 +986,13 @@ async fn network_proxy_feature_matrix_preserves_sandbox_network_semantics() -> s .then(|| toml::from_str("network_proxy = true").expect("valid features")); let base_config = match case.surface { Surface::PermissionProfile => ConfigToml { - default_permissions: Some("workspace".to_string()), + default_permissions: Some("dev".to_string()), permissions: Some(PermissionsToml { entries: BTreeMap::from([( - "workspace".to_string(), + "dev".to_string(), PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: None, @@ -1136,12 +1139,13 @@ async fn network_proxy_feature_uses_profile_network_proxy_settings() -> std::io: let config = Config::load_from_base_config_with_overrides( ConfigToml { features: Some(toml::from_str("network_proxy = true").expect("valid features")), - default_permissions: Some("workspace".to_string()), + default_permissions: Some("dev".to_string()), permissions: Some(PermissionsToml { entries: BTreeMap::from([( - "workspace".to_string(), + "dev".to_string(), PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: None, @@ -1240,12 +1244,13 @@ enabled = false ) .expect("valid features"), ), - default_permissions: Some("workspace".to_string()), + default_permissions: Some("dev".to_string()), permissions: Some(PermissionsToml { entries: BTreeMap::from([( - "workspace".to_string(), + "dev".to_string(), PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: None, @@ -1290,12 +1295,13 @@ async fn permissions_profiles_network_disabled_by_default_does_not_start_proxy() let config = Config::load_from_base_config_with_overrides( ConfigToml { - default_permissions: Some("workspace".to_string()), + default_permissions: Some("dev".to_string()), permissions: Some(PermissionsToml { entries: BTreeMap::from([( - "workspace".to_string(), + "dev".to_string(), PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: None, @@ -1338,12 +1344,13 @@ async fn default_permissions_profile_populates_runtime_sandbox_policy() -> std:: std::fs::write(cwd.path().join(".git"), "gitdir: nowhere")?; let cfg = ConfigToml { - default_permissions: Some("workspace".to_string()), + default_permissions: Some("dev".to_string()), permissions: Some(PermissionsToml { entries: BTreeMap::from([( - "workspace".to_string(), + "dev".to_string(), PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: None, @@ -1434,7 +1441,7 @@ async fn default_permissions_profile_populates_runtime_sandbox_policy() -> std:: .active_permission_profile() .as_ref() .map(|active| active.id.as_str()), - Some("workspace") + Some("dev") ); Ok(()) } @@ -1643,12 +1650,13 @@ async fn permission_profile_override_preserves_configured_network_policy_without let config = Config::load_from_base_config_with_overrides( ConfigToml { - default_permissions: Some("workspace".to_string()), + default_permissions: Some("dev".to_string()), permissions: Some(PermissionsToml { entries: BTreeMap::from([( - "workspace".to_string(), + "dev".to_string(), PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: None, @@ -1704,12 +1712,13 @@ async fn workspace_root_glob_none_compiles_to_filesystem_pattern_entry() -> std: let config = Config::load_from_base_config_with_overrides( ConfigToml { - default_permissions: Some("workspace".to_string()), + default_permissions: Some("dev".to_string()), permissions: Some(PermissionsToml { entries: BTreeMap::from([( - "workspace".to_string(), + "dev".to_string(), PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: Some(2), @@ -1787,9 +1796,10 @@ async fn permissions_profiles_require_default_permissions() -> std::io::Result<( ConfigToml { permissions: Some(PermissionsToml { entries: BTreeMap::from([( - "workspace".to_string(), + "dev".to_string(), PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: None, @@ -1918,6 +1928,7 @@ async fn workspace_profile_applies_rules_to_runtime_and_profile_workspace_roots( "dev".to_string(), PermissionProfileToml { description: None, + extends: None, workspace_roots: Some(WorkspaceRootsToml { entries: BTreeMap::from([( profile_root.to_string_lossy().into_owned(), @@ -2371,12 +2382,13 @@ async fn permissions_profiles_allow_direct_write_roots_outside_workspace_root() let config = Config::load_from_base_config_with_overrides( ConfigToml { - default_permissions: Some("workspace".to_string()), + default_permissions: Some("dev".to_string()), permissions: Some(PermissionsToml { entries: BTreeMap::from([( - "workspace".to_string(), + "dev".to_string(), PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: None, @@ -2401,7 +2413,7 @@ async fn permissions_profiles_allow_direct_write_roots_outside_workspace_root() assert_eq!( config.custom_permission_profile_ids, - vec!["workspace".to_string()] + vec!["dev".to_string()] ); let memories_root = AbsolutePathBuf::from_absolute_path(std::fs::canonicalize( codex_home.path().join("memories"), @@ -2433,12 +2445,13 @@ async fn permissions_profiles_reject_nested_entries_for_non_workspace_roots() -> let err = Config::load_from_base_config_with_overrides( ConfigToml { - default_permissions: Some("workspace".to_string()), + default_permissions: Some("dev".to_string()), permissions: Some(PermissionsToml { entries: BTreeMap::from([( - "workspace".to_string(), + "dev".to_string(), PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: None, @@ -2482,9 +2495,9 @@ async fn load_workspace_permission_profile( Config::load_from_base_config_with_overrides( ConfigToml { - default_permissions: Some("workspace".to_string()), + default_permissions: Some("dev".to_string()), permissions: Some(PermissionsToml { - entries: BTreeMap::from([("workspace".to_string(), profile)]), + entries: BTreeMap::from([("dev".to_string(), profile)]), }), ..Default::default() }, @@ -2501,6 +2514,7 @@ async fn load_workspace_permission_profile( async fn permissions_profiles_allow_unknown_special_paths() -> std::io::Result<()> { let config = load_workspace_permission_profile(PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: None, @@ -2546,6 +2560,7 @@ async fn permissions_profiles_allow_unknown_special_paths_with_nested_entries() -> std::io::Result<()> { let config = load_workspace_permission_profile(PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: None, @@ -2584,6 +2599,7 @@ async fn permissions_profiles_allow_unknown_special_paths_with_nested_entries() async fn permissions_profiles_allow_missing_filesystem_with_warning() -> std::io::Result<()> { let config = load_workspace_permission_profile(PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: None, network: None, @@ -2602,7 +2618,7 @@ async fn permissions_profiles_allow_missing_filesystem_with_warning() -> std::io ); assert!( config.startup_warnings.iter().any(|warning| warning.contains( - "Permissions profile `workspace` does not define any recognized filesystem entries for this version of Codex." + "Permissions profile `dev` does not define any recognized filesystem entries for this version of Codex." )), "{:?}", config.startup_warnings @@ -2614,6 +2630,7 @@ async fn permissions_profiles_allow_missing_filesystem_with_warning() -> std::io async fn permissions_profiles_allow_empty_filesystem_with_warning() -> std::io::Result<()> { let config = load_workspace_permission_profile(PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: None, @@ -2629,7 +2646,7 @@ async fn permissions_profiles_allow_empty_filesystem_with_warning() -> std::io:: ); assert!( config.startup_warnings.iter().any(|warning| warning.contains( - "Permissions profile `workspace` does not define any recognized filesystem entries for this version of Codex." + "Permissions profile `dev` does not define any recognized filesystem entries for this version of Codex." )), "{:?}", config.startup_warnings @@ -2645,12 +2662,13 @@ async fn permissions_profiles_reject_workspace_root_parent_traversal() -> std::i let err = Config::load_from_base_config_with_overrides( ConfigToml { - default_permissions: Some("workspace".to_string()), + default_permissions: Some("dev".to_string()), permissions: Some(PermissionsToml { entries: BTreeMap::from([( - "workspace".to_string(), + "dev".to_string(), PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: None, @@ -2693,12 +2711,13 @@ async fn permissions_profiles_allow_network_enablement() -> std::io::Result<()> let config = Config::load_from_base_config_with_overrides( ConfigToml { - default_permissions: Some("workspace".to_string()), + default_permissions: Some("dev".to_string()), permissions: Some(PermissionsToml { entries: BTreeMap::from([( - "workspace".to_string(), + "dev".to_string(), PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: None, diff --git a/codex-rs/core/src/config/permissions_tests.rs b/codex-rs/core/src/config/permissions_tests.rs index 5e1fcf3525b..09aa7606aaa 100644 --- a/codex-rs/core/src/config/permissions_tests.rs +++ b/codex-rs/core/src/config/permissions_tests.rs @@ -68,6 +68,7 @@ async fn restricted_read_implicitly_allows_helper_executables() -> std::io::Resu "workspace".to_string(), PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: None, @@ -239,6 +240,157 @@ fn network_toml_overlays_unix_socket_permissions_by_path() { ); } +#[test] +fn permissions_profiles_resolve_extends_parent_first_with_child_overrides() { + let permissions = toml::from_str::( + r#" +[base] +description = "Base profile" + +[base.filesystem] +glob_scan_max_depth = 1 +"/tmp/base" = "read" +"/tmp/shared" = "read" + +[base.filesystem.":project_roots"] +"**/*.env" = "deny" +docs = "read" + +[base.network] +enabled = true + +[base.network.domains] +"base.example.com" = "allow" +"SHARED.EXAMPLE.COM." = "deny" + +[base.network.unix_sockets] +"/tmp/base.sock" = "allow" + +[child] +extends = "base" + +[child.filesystem] +glob_scan_max_depth = 3 +"/tmp/shared" = "write" + +[child.filesystem.":project_roots"] +docs = "write" +src = "read" + +[child.network] +enabled = false +allow_local_binding = true + +[child.network.domains] +"child.example.com" = "allow" +"shared.example.com" = "allow" + +[child.network.unix_sockets] +"/tmp/child.sock" = "allow" +"#, + ) + .expect("permissions should deserialize"); + + let resolved = permissions + .resolve_profile("child", |_| None) + .expect("child profile should resolve"); + let expected_profile = toml::from_str::( + r#" +extends = "base" + +[filesystem] +glob_scan_max_depth = 3 +"/tmp/base" = "read" +"/tmp/shared" = "write" + +[filesystem.":project_roots"] +"**/*.env" = "deny" +docs = "write" +src = "read" + +[network] +enabled = false +allow_local_binding = true + +[network.domains] +"base.example.com" = "allow" +"child.example.com" = "allow" +"shared.example.com" = "allow" + +[network.unix_sockets] +"/tmp/base.sock" = "allow" +"/tmp/child.sock" = "allow" +"#, + ) + .expect("expected profile should deserialize"); + + assert_eq!(resolved.profile, expected_profile); + assert_eq!(resolved.inherited_profile_names, vec!["base".to_string()]); +} + +#[test] +fn permissions_profiles_reject_undefined_extends_parent() { + let permissions = toml::from_str::( + r#" +[child] +extends = "base" +"#, + ) + .expect("permissions should deserialize"); + + let err = permissions + .resolve_profile("child", |_| None) + .expect_err("missing parent should be rejected"); + + assert_eq!( + err.to_string(), + "permissions profile `child` extends undefined profile `base`" + ); +} + +#[test] +fn permissions_profiles_reject_unsupported_builtin_extends_parent() { + let permissions = toml::from_str::( + r#" +[child] +extends = ":danger-full-access" +"#, + ) + .expect("permissions should deserialize"); + + let err = permissions + .resolve_profile("child", |_| None) + .expect_err("unsupported built-in parent should be rejected"); + + assert_eq!( + err.to_string(), + "permissions profile `child` cannot extend unsupported built-in profile `:danger-full-access`" + ); +} + +#[test] +fn permissions_profiles_reject_extends_cycles() { + let permissions = toml::from_str::( + r#" +[alpha] +extends = "beta" + +[beta] +extends = "alpha" +"#, + ) + .expect("permissions should deserialize"); + + let err = permissions + .resolve_profile("alpha", |_| None) + .expect_err("cycle should be rejected"); + + assert_eq!( + err.to_string(), + "permissions profile inheritance cycle detected: alpha -> beta -> alpha" + ); +} + #[test] fn profile_network_proxy_config_keeps_proxy_disabled_for_bare_network_access() { let config = network_proxy_config_from_profile_network(Some(&NetworkToml { @@ -287,6 +439,7 @@ fn compile_permission_profile_workspace_roots_resolves_enabled_entries() -> std: "workspace".to_string(), PermissionProfileToml { description: None, + extends: None, workspace_roots: Some(WorkspaceRootsToml { entries: BTreeMap::from([ ("backend".to_string(), true), @@ -397,6 +550,7 @@ fn read_write_trailing_glob_suffix_compiles_as_subpath() -> std::io::Result<()> "workspace".to_string(), PermissionProfileToml { description: None, + extends: None, workspace_roots: None, filesystem: Some(FilesystemPermissionsToml { glob_scan_max_depth: None, From 05e0dc1a22b00c92239b69ed1f2ff24c5d5a4c57 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 20 May 2026 10:38:57 -0700 Subject: [PATCH 2/4] fix(permissions): preserve normalized domain precedence Co-authored-by: Codex noreply@openai.com --- codex-rs/config/src/merge.rs | 20 +++++++++++++++++++ codex-rs/config/src/merge_tests.rs | 26 +++++++++++++++++++++++++ codex-rs/config/src/permissions_toml.rs | 2 ++ 3 files changed, 48 insertions(+) diff --git a/codex-rs/config/src/merge.rs b/codex-rs/config/src/merge.rs index 020adb0313a..94d67015466 100644 --- a/codex-rs/config/src/merge.rs +++ b/codex-rs/config/src/merge.rs @@ -1,5 +1,6 @@ use crate::key_aliases::normalize_key_aliases; use crate::key_aliases::normalized_with_key_aliases; +use codex_network_proxy::normalize_host; use toml::Value as TomlValue; /// Merge config `overlay` into `base`, giving `overlay` precedence. @@ -14,6 +15,10 @@ fn merge_toml_values_at_path(base: &mut TomlValue, overlay: &TomlValue, path: &m normalize_key_aliases(path, base_table); let mut overlay_table = overlay_table.clone(); normalize_key_aliases(path, &mut overlay_table); + if is_permission_network_domains_path(path) { + normalize_network_domain_keys(base_table); + normalize_network_domain_keys(&mut overlay_table); + } for (key, value) in overlay_table { path.push(key.clone()); @@ -29,6 +34,21 @@ fn merge_toml_values_at_path(base: &mut TomlValue, overlay: &TomlValue, path: &m } } +fn is_permission_network_domains_path(path: &[String]) -> bool { + matches!( + path, + [permissions, _, network, domains] + if permissions == "permissions" && network == "network" && domains == "domains" + ) +} + +fn normalize_network_domain_keys(table: &mut toml::map::Map) { + let entries = std::mem::take(table); + for (pattern, value) in entries { + table.insert(normalize_host(&pattern), value); + } +} + #[cfg(test)] #[path = "merge_tests.rs"] mod tests; diff --git a/codex-rs/config/src/merge_tests.rs b/codex-rs/config/src/merge_tests.rs index 0bb92c00155..f9da6e7ebc1 100644 --- a/codex-rs/config/src/merge_tests.rs +++ b/codex-rs/config/src/merge_tests.rs @@ -98,3 +98,29 @@ disable_on_external_context = true ); assert_eq!(base, expected); } + +#[test] +fn merge_toml_values_normalizes_permission_network_domains_before_overlaying() { + let mut base = parse_toml( + r#" +[permissions.dev.network.domains] +"example.com" = "deny" +"#, + ); + let overlay = parse_toml( + r#" +[permissions.dev.network.domains] +"EXAMPLE.COM" = "allow" +"#, + ); + + merge_toml_values(&mut base, &overlay); + + let expected = parse_toml( + r#" +[permissions.dev.network.domains] +"example.com" = "allow" +"#, + ); + assert_eq!(base, expected); +} diff --git a/codex-rs/config/src/permissions_toml.rs b/codex-rs/config/src/permissions_toml.rs index 151d9e5ff2e..60ec9e128da 100644 --- a/codex-rs/config/src/permissions_toml.rs +++ b/codex-rs/config/src/permissions_toml.rs @@ -171,6 +171,8 @@ fn merge_permission_profiles( .and_then(|network| network.domains.as_ref()) .is_some(); + // Description and inheritance metadata belong to the selected profile + // declaration, so an inherited profile must not fill those gaps. parent.description = None; parent.extends = None; From 4be6e4dcf8455d543c6e148015a859d01e82d9bd Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 20 May 2026 12:20:22 -0700 Subject: [PATCH 3/4] feat(permissions): apply inherited profiles at runtime (#23705) ## Stack This PR is stacked on #22270. Review #22270 first for the config-level `extends` resolver and merge semantics; this follow-up contains the runtime and protocol wiring on top of that foundation. ## Why The resolver in #22270 makes inherited permission profiles well-defined in config, but selected profiles still need to flow through runtime permission compilation, network-proxy selection, and the active-profile metadata exposed to clients. This PR keeps that integration separate from the inheritance rules themselves so each diff has a narrower review surface. ## What changed - Resolve selected custom permission profiles before compiling runtime sandbox and network policy, including supported built-in baselines such as `:read-only` and `:workspace`. - Preserve the selected profile's `extends` metadata on the active permission profile while the resolved permissions drive runtime behavior. - Update network proxy profile loading so built-in defaults and inherited custom profiles select the effective network policy consistently. - Add runtime coverage for built-in extension baselines, active-profile metadata, and network-proxy selection. - Refresh app-server protocol comments and generated schema/TypeScript fixtures for the now-active `extends` field. ## Validation Run on the full inheritance stack before the PR split: - `cargo test -p codex-config` - `cargo test -p codex-core permissions_profiles_` - `cargo test -p codex-core selected_network_from_tables_` - `cargo test -p codex-core default_permissions_profile_can_extend_builtin_workspace` --- .../codex_app_server_protocol.schemas.json | 2 +- .../codex_app_server_protocol.v2.schemas.json | 2 +- .../schema/json/v2/ThreadForkResponse.json | 2 +- .../schema/json/v2/ThreadResumeResponse.json | 2 +- .../schema/json/v2/ThreadStartResponse.json | 2 +- .../typescript/v2/ActivePermissionProfile.ts | 4 +- .../src/protocol/v2/permissions.rs | 4 +- codex-rs/core/config.schema.json | 2 +- codex-rs/core/src/config/config_tests.rs | 221 ++++++++++ codex-rs/core/src/config/mod.rs | 12 +- codex-rs/core/src/config/permissions.rs | 96 +++-- codex-rs/core/src/network_proxy_loader.rs | 26 +- .../core/src/network_proxy_loader_tests.rs | 384 ++++++++++++++++-- codex-rs/protocol/src/models.rs | 4 +- 14 files changed, 691 insertions(+), 72 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index ee48d1e2597..b0158d0a19b 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -5666,7 +5666,7 @@ "properties": { "extends": { "default": null, - "description": "Parent profile identifier once permissions profiles support inheritance. This is currently always `null`.", + "description": "Parent profile identifier from the selected permissions profile's `extends` setting, when present.", "type": [ "string", "null" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 871cd81777c..7e0053dcf2c 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -134,7 +134,7 @@ "properties": { "extends": { "default": null, - "description": "Parent profile identifier once permissions profiles support inheritance. This is currently always `null`.", + "description": "Parent profile identifier from the selected permissions profile's `extends` setting, when present.", "type": [ "string", "null" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json index 3d66152a600..cccc7e5b8d8 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json @@ -9,7 +9,7 @@ "properties": { "extends": { "default": null, - "description": "Parent profile identifier once permissions profiles support inheritance. This is currently always `null`.", + "description": "Parent profile identifier from the selected permissions profile's `extends` setting, when present.", "type": [ "string", "null" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json index f379455b593..f57257f94cb 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json @@ -9,7 +9,7 @@ "properties": { "extends": { "default": null, - "description": "Parent profile identifier once permissions profiles support inheritance. This is currently always `null`.", + "description": "Parent profile identifier from the selected permissions profile's `extends` setting, when present.", "type": [ "string", "null" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json index 7d31c52f1cc..02d156d7a62 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json @@ -9,7 +9,7 @@ "properties": { "extends": { "default": null, - "description": "Parent profile identifier once permissions profiles support inheritance. This is currently always `null`.", + "description": "Parent profile identifier from the selected permissions profile's `extends` setting, when present.", "type": [ "string", "null" diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ActivePermissionProfile.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ActivePermissionProfile.ts index 73f9efcab57..ee9026b5430 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ActivePermissionProfile.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ActivePermissionProfile.ts @@ -9,7 +9,7 @@ export type ActivePermissionProfile = { */ id: string, /** - * Parent profile identifier once permissions profiles support - * inheritance. This is currently always `null`. + * Parent profile identifier from the selected permissions profile's + * `extends` setting, when present. */ extends: string | null, }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs b/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs index 99924ca1e5e..dfc1942276e 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs @@ -329,8 +329,8 @@ pub struct ActivePermissionProfile { /// Identifier from `default_permissions` or the implicit built-in default, /// such as `:workspace` or a user-defined `[permissions.]` profile. pub id: String, - /// Parent profile identifier once permissions profiles support - /// inheritance. This is currently always `null`. + /// Parent profile identifier from the selected permissions profile's + /// `extends` setting, when present. #[serde(default)] pub extends: Option, } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index f97286475fb..636b1fcf3ec 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -4876,4 +4876,4 @@ }, "title": "ConfigToml", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 1e4d0f16b96..068da519c68 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -828,6 +828,56 @@ async fn permissions_profiles_proxy_policy_does_not_start_managed_network_proxy_ let cwd = TempDir::new()?; std::fs::write(cwd.path().join(".git"), "gitdir: nowhere")?; + let config = Config::load_from_base_config_with_overrides( + ConfigToml { + default_permissions: Some("dev".to_string()), + permissions: Some(PermissionsToml { + entries: BTreeMap::from([( + "dev".to_string(), + PermissionProfileToml { + description: None, + extends: None, + workspace_roots: None, + filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, + entries: BTreeMap::from([( + ":minimal".to_string(), + FilesystemPermissionToml::Access(FileSystemAccessMode::Read), + )]), + }), + network: Some(NetworkToml { + enabled: Some(true), + ..Default::default() + }), + }, + )]), + }), + ..Default::default() + }, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + ..Default::default() + }, + codex_home.abs(), + ) + .await?; + assert_eq!( + config.permissions.network_sandbox_policy(), + NetworkSandboxPolicy::Enabled + ); + assert!( + config.permissions.network.is_none(), + "bare profile network.enabled should not start the managed network proxy" + ); + Ok(()) +} + +#[tokio::test] +async fn permissions_profiles_proxy_policy_starts_managed_network_proxy() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + std::fs::write(cwd.path().join(".git"), "gitdir: nowhere")?; + let config = Config::load_from_base_config_with_overrides( ConfigToml { default_permissions: Some("dev".to_string()), @@ -1446,6 +1496,65 @@ async fn default_permissions_profile_populates_runtime_sandbox_policy() -> std:: Ok(()) } +#[tokio::test] +async fn default_permissions_extended_profile_preserves_parent_metadata() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + std::fs::write(cwd.path().join(".git"), "gitdir: nowhere")?; + + let config = Config::load_from_base_config_with_overrides( + ConfigToml { + default_permissions: Some("dev".to_string()), + permissions: Some(PermissionsToml { + entries: BTreeMap::from([ + ( + "base".to_string(), + PermissionProfileToml { + description: None, + extends: None, + workspace_roots: None, + filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, + entries: BTreeMap::from([( + ":minimal".to_string(), + FilesystemPermissionToml::Access(FileSystemAccessMode::Read), + )]), + }), + network: None, + }, + ), + ( + "dev".to_string(), + PermissionProfileToml { + description: None, + extends: Some("base".to_string()), + workspace_roots: None, + filesystem: None, + network: None, + }, + ), + ]), + }), + ..Default::default() + }, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + ..Default::default() + }, + codex_home.abs(), + ) + .await?; + + assert_eq!( + config.permissions.active_permission_profile(), + Some(ActivePermissionProfile { + id: "dev".to_string(), + extends: Some("base".to_string()), + }) + ); + Ok(()) +} + #[tokio::test] async fn permission_profile_override_populates_runtime_permissions() -> std::io::Result<()> { let codex_home = TempDir::new()?; @@ -2049,6 +2158,118 @@ async fn explicit_builtin_workspace_profile_ignores_legacy_workspace_write_setti Ok(()) } +#[tokio::test] +async fn default_permissions_profile_can_extend_builtin_workspace() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + + let config = Config::load_from_base_config_with_overrides( + ConfigToml { + default_permissions: Some("workspace-with-network".to_string()), + permissions: Some(PermissionsToml { + entries: BTreeMap::from([( + "workspace-with-network".to_string(), + PermissionProfileToml { + description: None, + extends: Some(BUILT_IN_PERMISSION_PROFILE_WORKSPACE.to_string()), + workspace_roots: None, + filesystem: None, + network: Some(NetworkToml { + enabled: Some(true), + ..Default::default() + }), + }, + )]), + }), + ..Default::default() + }, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + ..Default::default() + }, + codex_home.abs(), + ) + .await?; + + let policy = config.permissions.file_system_sandbox_policy(); + assert!( + policy.can_write_path_with_cwd(cwd.path(), cwd.path()), + "expected profile extending :workspace to keep project-root writes, policy: {policy:?}" + ); + assert!( + !policy.can_write_path_with_cwd(&cwd.path().join(".git"), cwd.path()), + "expected profile extending :workspace to keep metadata carveouts, policy: {policy:?}" + ); + assert_eq!( + config.permissions.network_sandbox_policy(), + NetworkSandboxPolicy::Enabled + ); + assert_eq!( + config.permissions.active_permission_profile(), + Some(ActivePermissionProfile { + id: "workspace-with-network".to_string(), + extends: Some(BUILT_IN_PERMISSION_PROFILE_WORKSPACE.to_string()), + }) + ); + Ok(()) +} + +#[tokio::test] +async fn default_permissions_profile_can_extend_builtin_read_only() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + + let config = Config::load_from_base_config_with_overrides( + ConfigToml { + default_permissions: Some("read-only-with-network".to_string()), + permissions: Some(PermissionsToml { + entries: BTreeMap::from([( + "read-only-with-network".to_string(), + PermissionProfileToml { + description: None, + extends: Some(BUILT_IN_PERMISSION_PROFILE_READ_ONLY.to_string()), + workspace_roots: None, + filesystem: None, + network: Some(NetworkToml { + enabled: Some(true), + ..Default::default() + }), + }, + )]), + }), + ..Default::default() + }, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + ..Default::default() + }, + codex_home.abs(), + ) + .await?; + + let policy = config.permissions.file_system_sandbox_policy(); + assert!( + policy.can_read_path_with_cwd(cwd.path(), cwd.path()), + "expected profile extending :read-only to keep read access, policy: {policy:?}" + ); + assert!( + !policy.can_write_path_with_cwd(cwd.path(), cwd.path()), + "expected profile extending :read-only to stay non-writable, policy: {policy:?}" + ); + assert_eq!( + config.permissions.network_sandbox_policy(), + NetworkSandboxPolicy::Enabled + ); + assert_eq!( + config.permissions.active_permission_profile(), + Some(ActivePermissionProfile { + id: "read-only-with-network".to_string(), + extends: Some(BUILT_IN_PERMISSION_PROFILE_READ_ONLY.to_string()), + }) + ); + Ok(()) +} + #[tokio::test] async fn empty_config_defaults_to_builtin_profile_for_trusted_project() -> 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 51afdf01222..434969e2c5f 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -150,6 +150,8 @@ pub use codex_sandboxing::system_bwrap_warning; pub use managed_features::ManagedFeatures; pub use network_proxy_spec::NetworkProxySpec; pub use network_proxy_spec::StartedNetworkProxy; +pub(crate) use permissions::is_builtin_permission_profile_name; +pub(crate) use permissions::reject_unknown_builtin_permission_profile; pub(crate) use permissions::resolve_permission_profile; pub use resolved_permission_profile::PermissionProfileSnapshot; pub(crate) use resolved_permission_profile::PermissionProfileState; @@ -2807,7 +2809,15 @@ impl Config { // when doing so would lose roots, network, or tmp settings. None } else { - Some(ActivePermissionProfile::new(default_permissions)) + let selected_profile_extends = cfg + .permissions + .as_ref() + .and_then(|permissions| permissions.entries.get(default_permissions)) + .and_then(|profile| profile.extends.clone()); + Some(ActivePermissionProfile { + id: default_permissions.to_string(), + extends: selected_profile_extends, + }) }; ( configured_network_proxy_config, diff --git a/codex-rs/core/src/config/permissions.rs b/codex-rs/core/src/config/permissions.rs index 4235cd0f65e..7c757d386a6 100644 --- a/codex-rs/core/src/config/permissions.rs +++ b/codex-rs/core/src/config/permissions.rs @@ -13,6 +13,7 @@ use codex_config::permissions_toml::NetworkUnixSocketPermissionToml; use codex_config::permissions_toml::NetworkUnixSocketPermissionsToml; use codex_config::permissions_toml::PermissionProfileToml; use codex_config::permissions_toml::PermissionsToml; +use codex_config::permissions_toml::ResolvedPermissionProfileToml; use codex_config::permissions_toml::WorkspaceRootsToml; use codex_config::types::SandboxWorkspaceWrite; use codex_features::NetworkProxyConfigToml; @@ -189,15 +190,29 @@ pub(crate) fn apply_network_proxy_feature_config( .apply_to_network_proxy_config(config); } -pub(crate) fn resolve_permission_profile<'a>( - permissions: &'a PermissionsToml, +pub(crate) fn resolve_permission_profile( + permissions: &PermissionsToml, profile_name: &str, -) -> io::Result<&'a PermissionProfileToml> { - permissions.entries.get(profile_name).ok_or_else(|| { - io::Error::new( - io::ErrorKind::InvalidInput, - format!("default_permissions refers to undefined profile `{profile_name}`"), - ) +) -> io::Result { + permissions + .resolve_profile(profile_name, extensible_builtin_parent_profile_marker) + .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err.to_string())) +} + +/// Built-in parents provide their runtime permissions below. Resolution only +/// needs an empty profile marker so inheritance can terminate while preserving +/// the built-in parent id in `inherited_profile_names`. +fn extensible_builtin_parent_profile_marker(profile_name: &str) -> Option { + matches!( + profile_name, + BUILT_IN_READ_ONLY_PROFILE | BUILT_IN_WORKSPACE_PROFILE + ) + .then_some(PermissionProfileToml { + description: None, + extends: None, + workspace_roots: None, + filesystem: None, + network: None, }) } @@ -216,7 +231,7 @@ pub(crate) fn network_proxy_config_for_profile_selection( "default_permissions requires a `[permissions]` table", ) })?; - let profile = resolve_permission_profile(permissions, profile_name)?; + let profile = resolve_permission_profile(permissions, profile_name)?.profile; Ok(network_proxy_config_from_profile_network( profile.network.as_ref(), )) @@ -228,11 +243,27 @@ pub(crate) fn compile_permission_profile( policy_cwd: &Path, startup_warnings: &mut Vec, ) -> io::Result<(FileSystemSandboxPolicy, NetworkSandboxPolicy)> { - let profile = resolve_permission_profile(permissions, profile_name)?; - - let mut entries = Vec::new(); + let ResolvedPermissionProfileToml { + profile, + inherited_profile_names, + } = resolve_permission_profile(permissions, profile_name)?; + let base_permissions = inherited_profile_names.iter().find_map(|name| { + match name.as_str() { + BUILT_IN_READ_ONLY_PROFILE => Some(PermissionProfile::read_only()), + BUILT_IN_WORKSPACE_PROFILE => Some(PermissionProfile::workspace_write()), + _ => None, + } + .map(|profile| profile.to_runtime_permissions()) + }); + let (mut file_system_sandbox_policy, base_network_sandbox_policy) = base_permissions + .unwrap_or_else(|| { + ( + FileSystemSandboxPolicy::restricted(Vec::new()), + NetworkSandboxPolicy::Restricted, + ) + }); if let Some(filesystem) = profile.filesystem.as_ref() { - if filesystem.is_empty() { + if filesystem.is_empty() && file_system_sandbox_policy.entries.is_empty() { push_warning( startup_warnings, missing_filesystem_entries_warning(profile_name), @@ -257,15 +288,17 @@ pub(crate) fn compile_permission_profile( } } for (path, permission) in &filesystem.entries { - entries.extend(compile_filesystem_permission( - path, - permission, - policy_cwd, - startup_warnings, - )?); + file_system_sandbox_policy + .entries + .extend(compile_filesystem_permission( + path, + permission, + policy_cwd, + startup_warnings, + )?); } } - } else { + } else if file_system_sandbox_policy.entries.is_empty() { push_warning( startup_warnings, missing_filesystem_entries_warning(profile_name), @@ -277,10 +310,11 @@ pub(crate) fn compile_permission_profile( .as_ref() .and_then(|filesystem| filesystem.glob_scan_max_depth), )?; - - let network_sandbox_policy = compile_network_sandbox_policy(profile.network.as_ref()); - let mut file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(entries); - file_system_sandbox_policy.glob_scan_max_depth = glob_scan_max_depth; + if let Some(glob_scan_max_depth) = glob_scan_max_depth { + file_system_sandbox_policy.glob_scan_max_depth = Some(glob_scan_max_depth); + } + let network_sandbox_policy = + compile_network_sandbox_policy(profile.network.as_ref(), base_network_sandbox_policy); Ok((file_system_sandbox_policy, network_sandbox_policy)) } @@ -323,7 +357,7 @@ pub(crate) fn compile_permission_profile_workspace_roots( })?; let profile = resolve_permission_profile(permissions, profile_name)?; Ok(compile_workspace_roots( - profile.workspace_roots.as_ref(), + profile.profile.workspace_roots.as_ref(), policy_cwd, )) } @@ -340,7 +374,7 @@ fn compile_workspace_roots( }) } -fn reject_unknown_builtin_permission_profile(profile_name: &str) -> io::Result<()> { +pub(crate) fn reject_unknown_builtin_permission_profile(profile_name: &str) -> io::Result<()> { if profile_name.starts_with(':') { return Err(io::Error::new( io::ErrorKind::InvalidInput, @@ -382,14 +416,18 @@ pub(crate) fn get_readable_roots_required_for_codex_runtime( readable_roots } -fn compile_network_sandbox_policy(network: Option<&NetworkToml>) -> NetworkSandboxPolicy { +fn compile_network_sandbox_policy( + network: Option<&NetworkToml>, + base_network_sandbox_policy: NetworkSandboxPolicy, +) -> NetworkSandboxPolicy { let Some(network) = network else { - return NetworkSandboxPolicy::Restricted; + return base_network_sandbox_policy; }; match network.enabled { Some(true) => NetworkSandboxPolicy::Enabled, - _ => NetworkSandboxPolicy::Restricted, + Some(false) => NetworkSandboxPolicy::Restricted, + None => base_network_sandbox_policy, } } diff --git a/codex-rs/core/src/network_proxy_loader.rs b/codex-rs/core/src/network_proxy_loader.rs index 1c49be2daf7..dc4b5b6b165 100644 --- a/codex-rs/core/src/network_proxy_loader.rs +++ b/codex-rs/core/src/network_proxy_loader.rs @@ -1,4 +1,6 @@ use crate::config::find_codex_home; +use crate::config::is_builtin_permission_profile_name; +use crate::config::reject_unknown_builtin_permission_profile; use crate::config::resolve_permission_profile; use crate::exec_policy::ExecPolicyError; use crate::exec_policy::format_exec_policy_error_with_source; @@ -13,6 +15,7 @@ use codex_config::ConfigLayerStack; use codex_config::ConfigLayerStackOrdering; use codex_config::LoaderOverrides; use codex_config::loader::load_config_layers_state; +use codex_config::merge_toml_values; use codex_config::permissions_toml::NetworkToml; use codex_config::permissions_toml::PermissionsToml; use codex_config::permissions_toml::overlay_network_domain_permissions; @@ -117,6 +120,7 @@ fn network_constraints_from_trusted_layers( layers: &ConfigLayerStack, ) -> Result { let mut constraints = NetworkProxyConstraints::default(); + let mut merged = toml::Value::Table(toml::map::Map::new()); for layer in layers.get_layers( ConfigLayerStackOrdering::LowestPrecedenceFirst, /*include_disabled*/ false, @@ -125,10 +129,12 @@ fn network_constraints_from_trusted_layers( continue; } - let parsed = network_tables_from_toml(&layer.config)?; - if let Some(network) = selected_network_from_tables(parsed)? { - apply_network_constraints(network, &mut constraints); - } + merge_toml_values(&mut merged, &layer.config); + } + + let parsed = network_tables_from_toml(&merged)?; + if let Some(network) = selected_network_from_tables(parsed)? { + apply_network_constraints(network, &mut constraints); } Ok(constraints) } @@ -189,13 +195,17 @@ fn selected_network_from_tables(parsed: NetworkTablesToml) -> Result Result<()> { @@ -210,13 +220,15 @@ fn config_from_layers( exec_policy: &codex_execpolicy::Policy, ) -> Result { let mut config = NetworkProxyConfig::default(); + let mut merged = toml::Value::Table(toml::map::Map::new()); for layer in layers.get_layers( ConfigLayerStackOrdering::LowestPrecedenceFirst, /*include_disabled*/ false, ) { - let parsed = network_tables_from_toml(&layer.config)?; - apply_network_tables(&mut config, parsed)?; + merge_toml_values(&mut merged, &layer.config); } + let parsed = network_tables_from_toml(&merged)?; + apply_network_tables(&mut config, parsed)?; apply_exec_policy_network_rules(&mut config, exec_policy); Ok(config) } diff --git a/codex-rs/core/src/network_proxy_loader_tests.rs b/codex-rs/core/src/network_proxy_loader_tests.rs index b5adb935ec7..e2f3e7b98b3 100644 --- a/codex-rs/core/src/network_proxy_loader_tests.rs +++ b/codex-rs/core/src/network_proxy_loader_tests.rs @@ -1,19 +1,27 @@ use super::*; +use codex_app_server_protocol::ConfigLayerSource; +use codex_config::ConfigLayerEntry; +use codex_config::ConfigLayerStack; +use codex_config::ConfigRequirements; +use codex_config::ConfigRequirementsToml; +use codex_config::permissions_toml::NetworkDomainPermissionToml; +use codex_config::permissions_toml::NetworkDomainPermissionsToml; use codex_execpolicy::Decision; use codex_execpolicy::NetworkRuleProtocol; use codex_execpolicy::Policy; use pretty_assertions::assert_eq; +use std::collections::BTreeMap; #[test] fn higher_precedence_profile_network_overlays_domain_entries() { let lower_network: toml::Value = toml::from_str( r#" -default_permissions = "workspace" +default_permissions = "dev" -[permissions.workspace.network] +[permissions.dev.network] -[permissions.workspace.network.domains] +[permissions.dev.network.domains] "lower.example.com" = "allow" "blocked.example.com" = "deny" "#, @@ -21,11 +29,11 @@ default_permissions = "workspace" .expect("lower layer should parse"); let higher_network: toml::Value = toml::from_str( r#" -default_permissions = "workspace" +default_permissions = "dev" -[permissions.workspace.network] +[permissions.dev.network] -[permissions.workspace.network.domains] +[permissions.dev.network.domains] "higher.example.com" = "allow" "#, ) @@ -60,11 +68,11 @@ default_permissions = "workspace" fn higher_precedence_profile_network_overrides_matching_domain_entries() { let lower_network: toml::Value = toml::from_str( r#" -default_permissions = "workspace" +default_permissions = "dev" -[permissions.workspace.network] +[permissions.dev.network] -[permissions.workspace.network.domains] +[permissions.dev.network.domains] "shared.example.com" = "deny" "other.example.com" = "allow" "#, @@ -72,11 +80,11 @@ default_permissions = "workspace" .expect("lower layer should parse"); let higher_network: toml::Value = toml::from_str( r#" -default_permissions = "workspace" +default_permissions = "dev" -[permissions.workspace.network] +[permissions.dev.network] -[permissions.workspace.network.domains] +[permissions.dev.network.domains] "shared.example.com" = "allow" "#, ) @@ -151,9 +159,9 @@ fn execpolicy_network_rules_overlay_network_lists() { fn apply_network_constraints_includes_allow_all_unix_sockets_flag() { let config: toml::Value = toml::from_str( r#" -default_permissions = "workspace" +default_permissions = "dev" -[permissions.workspace.network] +[permissions.dev.network] dangerously_allow_all_unix_sockets = true "#, ) @@ -170,15 +178,345 @@ dangerously_allow_all_unix_sockets = true assert_eq!(constraints.dangerously_allow_all_unix_sockets, Some(true)); } +#[test] +fn selected_network_from_tables_ignores_builtin_profile_without_permissions_table() { + let config: toml::Value = toml::from_str( + r#" +default_permissions = ":workspace" +"#, + ) + .expect("built-in profile config should parse"); + + let network = selected_network_from_tables( + network_tables_from_toml(&config).expect("built-in profile config should deserialize"), + ) + .expect("built-in profile selection should not require permissions tables"); + + assert_eq!(network, None); +} + +#[test] +fn selected_network_from_tables_rejects_unknown_builtin_profile_without_permissions_table() { + let config: toml::Value = toml::from_str( + r#" +default_permissions = ":unknown" +"#, + ) + .expect("unknown built-in config should parse"); + + let err = selected_network_from_tables( + network_tables_from_toml(&config).expect("unknown built-in config should deserialize"), + ) + .expect_err("unknown built-in profile should be rejected"); + + assert_eq!( + err.to_string(), + "default_permissions refers to unknown built-in profile `:unknown`" + ); +} + +#[test] +fn selected_network_from_tables_resolves_builtin_workspace_parent() { + let config: toml::Value = toml::from_str( + r#" +default_permissions = "dev" + +[permissions.dev] +extends = ":workspace" + +[permissions.dev.network] +enabled = true + +[permissions.dev.network.domains] +"child.example.com" = "allow" +"#, + ) + .expect("dev extension config should parse"); + + let network = selected_network_from_tables( + network_tables_from_toml(&config).expect("dev extension config should deserialize"), + ) + .expect("dev extension should resolve") + .expect("dev extension should expose child network config"); + + assert_eq!( + network, + NetworkToml { + enabled: Some(true), + domains: Some(NetworkDomainPermissionsToml { + entries: BTreeMap::from([( + "child.example.com".to_string(), + NetworkDomainPermissionToml::Allow, + )]), + }), + ..Default::default() + } + ); +} + +#[test] +fn selected_network_from_tables_resolves_permission_profile_inheritance() { + let config: toml::Value = toml::from_str( + r#" +default_permissions = "dev" + +[permissions.base.network] +enabled = true +dangerously_allow_all_unix_sockets = true + +[permissions.base.network.domains] +"base.example.com" = "allow" +"shared.example.com" = "deny" + +[permissions.dev] +extends = "base" + +[permissions.dev.network] +allow_local_binding = true + +[permissions.dev.network.domains] +"child.example.com" = "allow" +"shared.example.com" = "allow" +"#, + ) + .expect("permissions profiles should parse"); + + let network = selected_network_from_tables( + network_tables_from_toml(&config).expect("permissions profiles should deserialize"), + ) + .expect("permissions profiles should select a network table") + .expect("network table should be present"); + + assert_eq!( + network, + NetworkToml { + enabled: Some(true), + dangerously_allow_all_unix_sockets: Some(true), + allow_local_binding: Some(true), + domains: Some(NetworkDomainPermissionsToml { + entries: BTreeMap::from([ + ( + "base.example.com".to_string(), + NetworkDomainPermissionToml::Allow, + ), + ( + "child.example.com".to_string(), + NetworkDomainPermissionToml::Allow, + ), + ( + "shared.example.com".to_string(), + NetworkDomainPermissionToml::Allow, + ), + ]), + }), + ..Default::default() + } + ); +} + +#[test] +fn config_from_layers_resolves_inherited_profiles_across_layers() { + let lower_layer = ConfigLayerEntry::new( + ConfigLayerSource::SessionFlags, + toml::toml! { + [permissions.base.network.domains] + "base.example.com" = "allow" + } + .into(), + ); + let higher_layer = ConfigLayerEntry::new( + ConfigLayerSource::SessionFlags, + toml::toml! { + default_permissions = "dev" + + [permissions.dev] + extends = "base" + + [permissions.dev.network.domains] + "child.example.com" = "allow" + } + .into(), + ); + let layers = ConfigLayerStack::new( + vec![lower_layer, higher_layer], + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("layer stack should be valid"); + + let config = + config_from_layers(&layers, &Policy::empty()).expect("inherited profiles should load"); + + assert_eq!( + config.network.allowed_domains(), + Some(vec![ + "base.example.com".to_string(), + "child.example.com".to_string(), + ]) + ); +} + +#[test] +fn config_from_layers_normalizes_profile_network_domains_before_merging_layers() { + let lower_layer = ConfigLayerEntry::new( + ConfigLayerSource::SessionFlags, + toml::toml! { + default_permissions = "dev" + + [permissions.dev.network.domains] + "example.com" = "deny" + } + .into(), + ); + let higher_layer = ConfigLayerEntry::new( + ConfigLayerSource::SessionFlags, + toml::toml! { + [permissions.dev.network.domains] + "EXAMPLE.COM" = "allow" + } + .into(), + ); + let layers = ConfigLayerStack::new( + vec![lower_layer, higher_layer], + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("layer stack should be valid"); + + let config = config_from_layers(&layers, &Policy::empty()) + .expect("network domain layer precedence should load"); + + assert_eq!( + config.network.allowed_domains(), + Some(vec!["example.com".to_string()]) + ); + assert_eq!(config.network.denied_domains(), None); +} + +#[test] +fn config_from_layers_uses_only_the_final_selected_profile_network() { + let lower_layer = ConfigLayerEntry::new( + ConfigLayerSource::SessionFlags, + toml::toml! { + default_permissions = "dev" + + [permissions.dev.network.domains] + "lower.example.com" = "allow" + } + .into(), + ); + let higher_layer = ConfigLayerEntry::new( + ConfigLayerSource::SessionFlags, + toml::toml! { + default_permissions = ":workspace" + } + .into(), + ); + let layers = ConfigLayerStack::new( + vec![lower_layer, higher_layer], + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("layer stack should be valid"); + + let config = config_from_layers(&layers, &Policy::empty()) + .expect("final built-in profile selection should load"); + + assert_eq!(config.network.allowed_domains(), None); + assert_eq!(config.network.denied_domains(), None); +} + +#[test] +fn trusted_constraints_use_only_the_final_selected_profile_network() { + let lower_layer = ConfigLayerEntry::new( + ConfigLayerSource::System { + file: AbsolutePathBuf::try_from(std::path::PathBuf::from("/tmp/system.toml")) + .expect("system config path should be absolute"), + }, + toml::toml! { + default_permissions = "dev" + + [permissions.dev.network.domains] + "managed.example.com" = "allow" + } + .into(), + ); + let higher_layer = ConfigLayerEntry::new( + ConfigLayerSource::LegacyManagedConfigTomlFromFile { + file: AbsolutePathBuf::try_from(std::path::PathBuf::from("/tmp/managed.toml")) + .expect("managed config path should be absolute"), + }, + toml::toml! { + default_permissions = ":workspace" + } + .into(), + ); + let layers = ConfigLayerStack::new( + vec![lower_layer, higher_layer], + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("layer stack should be valid"); + + let constraints = network_constraints_from_trusted_layers(&layers) + .expect("final built-in trusted selection should load"); + + assert_eq!(constraints.allowed_domains, None); + assert_eq!(constraints.denied_domains, None); +} + +#[test] +fn trusted_constraints_normalize_profile_network_domains_before_merging_layers() { + let lower_layer = ConfigLayerEntry::new( + ConfigLayerSource::System { + file: AbsolutePathBuf::try_from(std::path::PathBuf::from("/tmp/system.toml")) + .expect("system config path should be absolute"), + }, + toml::toml! { + default_permissions = "dev" + + [permissions.dev.network.domains] + "example.com" = "deny" + } + .into(), + ); + let higher_layer = ConfigLayerEntry::new( + ConfigLayerSource::LegacyManagedConfigTomlFromFile { + file: AbsolutePathBuf::try_from(std::path::PathBuf::from("/tmp/managed.toml")) + .expect("managed config path should be absolute"), + }, + toml::toml! { + [permissions.dev.network.domains] + "EXAMPLE.COM" = "allow" + } + .into(), + ); + let layers = ConfigLayerStack::new( + vec![lower_layer, higher_layer], + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("layer stack should be valid"); + + let constraints = network_constraints_from_trusted_layers(&layers) + .expect("trusted network domain layer precedence should load"); + + assert_eq!( + constraints.allowed_domains, + Some(vec!["example.com".to_string()]) + ); + assert_eq!(constraints.denied_domains, None); +} + #[test] fn apply_network_constraints_skips_empty_domain_sides() { let config: toml::Value = toml::from_str( r#" -default_permissions = "workspace" +default_permissions = "dev" -[permissions.workspace.network] +[permissions.dev.network] -[permissions.workspace.network.domains] +[permissions.dev.network.domains] "managed.example.com" = "allow" "#, ) @@ -203,22 +541,22 @@ default_permissions = "workspace" fn apply_network_constraints_overlay_domain_entries() { let lower_network: toml::Value = toml::from_str( r#" -default_permissions = "workspace" +default_permissions = "dev" -[permissions.workspace.network] +[permissions.dev.network] -[permissions.workspace.network.domains] +[permissions.dev.network.domains] "blocked.example.com" = "deny" "#, ) .expect("lower layer should parse"); let higher_network: toml::Value = toml::from_str( r#" -default_permissions = "workspace" +default_permissions = "dev" -[permissions.workspace.network] +[permissions.dev.network] -[permissions.workspace.network.domains] +[permissions.dev.network.domains] "api.example.com" = "allow" "#, ) diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 2927cb1cd13..28a7d6522c8 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -339,8 +339,8 @@ pub struct ActivePermissionProfile { /// profile. pub id: String, - /// Optional parent profile identifier once permissions profiles support - /// inheritance. This is always `None` until that config feature exists. + /// Optional parent profile identifier from the selected permissions + /// profile's `extends` setting. #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] pub extends: Option, From 7dc178c55e08304f7959b1c72560a8636fa0638c Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 20 May 2026 12:56:26 -0700 Subject: [PATCH 4/4] fix(app-server-protocol): refresh permission profile schema Co-authored-by: Codex noreply@openai.com --- .../app-server-protocol/schema/json/ServerNotification.json | 2 +- .../schema/json/v2/ThreadSettingsUpdatedNotification.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 2a36d66c1d8..9270af85e70 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -68,7 +68,7 @@ "properties": { "extends": { "default": null, - "description": "Parent profile identifier once permissions profiles support inheritance. This is currently always `null`.", + "description": "Parent profile identifier from the selected permissions profile's `extends` setting, when present.", "type": [ "string", "null" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadSettingsUpdatedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadSettingsUpdatedNotification.json index d016e283114..42a76ae3643 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadSettingsUpdatedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadSettingsUpdatedNotification.json @@ -9,7 +9,7 @@ "properties": { "extends": { "default": null, - "description": "Parent profile identifier once permissions profiles support inheritance. This is currently always `null`.", + "description": "Parent profile identifier from the selected permissions profile's `extends` setting, when present.", "type": [ "string", "null"