From fe7c069fe65b8148950c4d98c194e6283f9731a2 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 20 May 2026 13:12:07 -0700 Subject: [PATCH] feat(permissions): resolve permission profile inheritance (#22270) ## Stack This is the foundation PR for the permission-profile inheritance stack. - This PR adds config-level `extends` resolution and merge semantics. - Follow-up: #23705 applies resolved profiles at runtime and updates the active-profile protocol surfaces. ## Why Permission profiles are starting to carry enough policy that copy-pasting near-identical definitions becomes hard to review and easy to drift. Before the runtime can consume inherited profiles, the config layer needs one explicit resolver that can merge parent chains and reject unsafe or invalid inheritance shapes. ## What changed - Add `extends` to permission-profile TOML and resolve parent chains in inheritance order. - Merge inherited profile TOML with the existing config merge behavior while preserving the permission-specific normalization needed for network domain keys. - Keep parent descriptions out of resolved child profiles and record inherited profile names separately for downstream consumers. - Reject undefined parents, unsupported built-in parents, and inheritance cycles with targeted errors. - Cover resolver behavior with TOML fixture tests and refresh the generated config schema. ## Validation - `cargo test -p codex-config` - `cargo test -p codex-core permissions_profiles_` --- .../schema/json/ServerNotification.json | 2 +- .../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 +- .../v2/ThreadSettingsUpdatedNotification.json | 2 +- .../schema/json/v2/ThreadStartResponse.json | 2 +- .../typescript/v2/ActivePermissionProfile.ts | 4 +- .../src/protocol/v2/permissions.rs | 4 +- codex-rs/config/src/merge.rs | 20 + codex-rs/config/src/merge_tests.rs | 26 ++ codex-rs/config/src/permissions_toml.rs | 176 ++++++++ codex-rs/core/config.schema.json | 5 +- codex-rs/core/src/config/config_tests.rs | 320 +++++++++++++-- codex-rs/core/src/config/mod.rs | 12 +- codex-rs/core/src/config/permissions.rs | 96 +++-- codex-rs/core/src/config/permissions_tests.rs | 154 +++++++ codex-rs/core/src/network_proxy_loader.rs | 26 +- .../core/src/network_proxy_loader_tests.rs | 384 ++++++++++++++++-- codex-rs/protocol/src/models.rs | 4 +- 20 files changed, 1131 insertions(+), 114 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 2a36d66c1d..9270af85e7 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/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index ee48d1e259..b0158d0a19 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 871cd81777..7e0053dcf2 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 3d66152a60..cccc7e5b8d 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 f379455b59..f57257f94c 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/ThreadSettingsUpdatedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadSettingsUpdatedNotification.json index d016e28311..42a76ae364 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" 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 7d31c52f1c..02d156d7a6 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 73f9efcab5..ee9026b543 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 99924ca1e5..dfc1942276 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/config/src/merge.rs b/codex-rs/config/src/merge.rs index 020adb0313..94d6701546 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 0bb92c0015..f9da6e7ebc 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 4d533776ad..60ec9e128d 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,190 @@ 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(); + + // 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; + + 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 0e811e9017..636b1fcf3e 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" }, @@ -4873,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 1eb55c4562..19fd762da9 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -733,44 +733,45 @@ 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 mode = "full" -[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), @@ -831,12 +832,63 @@ 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, + 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()), + 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, @@ -986,12 +1038,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, @@ -1138,12 +1191,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, @@ -1242,12 +1296,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, @@ -1292,12 +1347,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, @@ -1340,12 +1396,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, @@ -1436,7 +1493,66 @@ 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(()) +} + +#[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(()) } @@ -1645,12 +1761,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, @@ -1706,12 +1823,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), @@ -1789,9 +1907,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, @@ -1920,6 +2039,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(), @@ -2040,6 +2160,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()?; @@ -2373,12 +2605,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, @@ -2403,7 +2636,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"), @@ -2435,12 +2668,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, @@ -2484,9 +2718,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() }, @@ -2503,6 +2737,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, @@ -2548,6 +2783,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, @@ -2586,6 +2822,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, @@ -2604,7 +2841,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 @@ -2616,6 +2853,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, @@ -2631,7 +2869,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 @@ -2647,12 +2885,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, @@ -2695,12 +2934,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/mod.rs b/codex-rs/core/src/config/mod.rs index 51afdf0122..434969e2c5 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 4235cd0f65..7c757d386a 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/config/permissions_tests.rs b/codex-rs/core/src/config/permissions_tests.rs index 5e1fcf3525..09aa7606aa 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, diff --git a/codex-rs/core/src/network_proxy_loader.rs b/codex-rs/core/src/network_proxy_loader.rs index 1c49be2daf..dc4b5b6b16 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 b5adb935ec..e2f3e7b98b 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 2927cb1cd1..28a7d6522c 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,