From 701275a8e1ca5b19848b0adbfebdc7075422bd48 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Mon, 15 Jun 2026 23:25:06 +0000 Subject: [PATCH 1/4] Enforce managed shell environment policy --- codex-rs/config/src/config_requirements.rs | 72 +++++++++++++ codex-rs/config/src/lib.rs | 1 + .../config/src/requirements_layers/stack.rs | 2 + .../src/requirements_layers/stack_tests.rs | 52 +++++++++ codex-rs/core/src/config/config_tests.rs | 59 +++++++++++ codex-rs/core/src/config/mod.rs | 1 + codex-rs/core/src/config/requirements.rs | 100 ++++++++++++++++++ 7 files changed, 287 insertions(+) diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index 43c075bea346..3bfc96213457 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -1,5 +1,7 @@ use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::config_types::SandboxMode; +use codex_protocol::config_types::ShellEnvironmentPolicyInherit; +use codex_protocol::config_types::ShellEnvironmentPolicyRule; use codex_protocol::config_types::WebSearchMode; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; @@ -10,6 +12,7 @@ use serde::de::Error as _; use serde::de::value::Error as ValueDeserializerError; use serde::de::value::StrDeserializer; use std::collections::BTreeMap; +use std::collections::HashMap; use std::fmt; use wildmatch::WildMatchPattern; @@ -155,6 +158,7 @@ pub struct ConfigRequirements { pub check_for_update_on_startup: Option>, pub otel: Option>, pub allow_login_shell: Option>, + pub shell_environment_policy: Option>, pub feedback: Option>, pub approval_policy: ConstrainedWithSource, pub approvals_reviewer: ConstrainedWithSource, @@ -193,6 +197,7 @@ impl Default for ConfigRequirements { check_for_update_on_startup: None, otel: None, allow_login_shell: None, + shell_environment_policy: None, feedback: None, approval_policy: ConstrainedWithSource::new( Constrained::allow_any_from_default(), @@ -859,6 +864,7 @@ pub struct ConfigRequirementsToml { pub check_for_update_on_startup: Option, pub otel: Option, pub allow_login_shell: Option, + pub shell_environment_policy: Option, pub feedback: Option, pub allowed_approval_policies: Option>, pub allowed_approvals_reviewers: Option>, @@ -886,6 +892,22 @@ pub struct ConfigRequirementsToml { pub guardian_policy_config: Option, } +/// Managed shell environment policy fields accepted by `requirements.toml`. +/// +/// Unlike ordinary `config.toml`, requirements intentionally accept only the +/// keyed `rules` form. This makes include/exclude actions compose by pattern, +/// while config arrays remain a migration-only compatibility path that can be +/// deprecated independently later. +#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +pub struct ShellEnvironmentPolicyRequirementsToml { + pub inherit: Option, + pub ignore_default_excludes: Option, + pub r#set: Option>, + pub rules: Option>, + pub experimental_use_profile: Option, +} + #[derive(Deserialize, Debug, Clone, PartialEq)] pub struct RemoteSandboxConfigToml { pub hostname_patterns: Vec, @@ -926,6 +948,7 @@ pub struct ConfigRequirementsWithSources { pub check_for_update_on_startup: Option>, pub otel: Option>, pub allow_login_shell: Option>, + pub shell_environment_policy: Option>, pub feedback: Option>, pub allowed_approval_policies: Option>>, pub allowed_approvals_reviewers: Option>>, @@ -979,6 +1002,7 @@ impl ConfigRequirementsWithSources { check_for_update_on_startup: _, otel: _, allow_login_shell: _, + shell_environment_policy: _, feedback: _, allowed_approval_policies: _, allowed_approvals_reviewers: _, @@ -1027,6 +1051,7 @@ impl ConfigRequirementsWithSources { check_for_update_on_startup, otel, allow_login_shell, + shell_environment_policy, feedback, allowed_approval_policies, allowed_approvals_reviewers, @@ -1072,6 +1097,7 @@ impl ConfigRequirementsWithSources { check_for_update_on_startup, otel, allow_login_shell, + shell_environment_policy, feedback, allowed_approval_policies, allowed_approvals_reviewers, @@ -1106,6 +1132,7 @@ impl ConfigRequirementsWithSources { check_for_update_on_startup: check_for_update_on_startup.map(|sourced| sourced.value), otel: otel.map(|sourced| sourced.value), allow_login_shell: allow_login_shell.map(|sourced| sourced.value), + shell_environment_policy: shell_environment_policy.map(|sourced| sourced.value), feedback: feedback.map(|sourced| sourced.value), allowed_approval_policies: allowed_approval_policies.map(|sourced| sourced.value), allowed_approvals_reviewers: allowed_approvals_reviewers.map(|sourced| sourced.value), @@ -1210,6 +1237,10 @@ impl ConfigRequirementsToml { .as_ref() .is_none_or(|otel| otel == &OtelConfigToml::default()) && self.allow_login_shell.is_none() + && self + .shell_environment_policy + .as_ref() + .is_none_or(|policy| policy == &ShellEnvironmentPolicyRequirementsToml::default()) && self .feedback .as_ref() @@ -1278,6 +1309,7 @@ impl TryFrom for ConfigRequirements { check_for_update_on_startup, otel, allow_login_shell, + shell_environment_policy, feedback, allowed_approval_policies, allowed_approvals_reviewers, @@ -1618,6 +1650,7 @@ impl TryFrom for ConfigRequirements { check_for_update_on_startup, otel, allow_login_shell, + shell_environment_policy, feedback, approval_policy, approvals_reviewer, @@ -1720,6 +1753,7 @@ mod tests { check_for_update_on_startup, otel, allow_login_shell, + shell_environment_policy, feedback, allowed_approval_policies, allowed_approvals_reviewers, @@ -1762,6 +1796,8 @@ mod tests { otel: otel.map(|value| Sourced::new(value, RequirementSource::Unknown)), allow_login_shell: allow_login_shell .map(|value| Sourced::new(value, RequirementSource::Unknown)), + shell_environment_policy: shell_environment_policy + .map(|value| Sourced::new(value, RequirementSource::Unknown)), feedback: feedback.map(|value| Sourced::new(value, RequirementSource::Unknown)), allowed_approval_policies: allowed_approval_policies .map(|value| Sourced::new(value, RequirementSource::Unknown)), @@ -3779,4 +3815,40 @@ command = "python3 /enterprise/hooks/pre.py" ); Ok(()) } + + #[test] + fn shell_environment_policy_requires_rules_for_list_fields() -> Result<()> { + let requirements: ConfigRequirementsToml = from_str( + r#" +[shell_environment_policy.rules] +"HOME" = "include" +"SECRET_*" = "exclude" +"#, + )?; + + assert_eq!( + requirements.shell_environment_policy, + Some(ShellEnvironmentPolicyRequirementsToml { + rules: Some(BTreeMap::from([ + ("HOME".to_string(), ShellEnvironmentPolicyRule::Include,), + ("SECRET_*".to_string(), ShellEnvironmentPolicyRule::Exclude,), + ])), + ..Default::default() + }) + ); + + for field in ["exclude", "include_only"] { + let error = from_str::(&format!( + "[shell_environment_policy]\n{field} = [\"LEGACY_*\"]\n" + )) + .expect_err("legacy requirements.toml arrays should be rejected"); + assert!(error.to_string().contains(field)); + } + let error = from_str::( + "[shell_environment_policy.rules]\n\"PATH\" = \"keep\"\n", + ) + .expect_err("unknown rule actions should be rejected"); + assert!(error.to_string().contains("keep")); + Ok(()) + } } diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index ed26757715f8..c2ef5ad785b3 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -74,6 +74,7 @@ pub use config_requirements::RemoteSandboxConfigToml; pub use config_requirements::RequirementSource; pub use config_requirements::ResidencyRequirement; pub use config_requirements::SandboxModeRequirement; +pub use config_requirements::ShellEnvironmentPolicyRequirementsToml; pub use config_requirements::Sourced; pub use config_requirements::WebSearchModeRequirement; pub use config_requirements::WindowsRequirementsToml; diff --git a/codex-rs/config/src/requirements_layers/stack.rs b/codex-rs/config/src/requirements_layers/stack.rs index 11dfb3af5e38..24e5e86d016e 100644 --- a/codex-rs/config/src/requirements_layers/stack.rs +++ b/codex-rs/config/src/requirements_layers/stack.rs @@ -184,6 +184,7 @@ fn populate_merged_regular_fields_with_sources( check_for_update_on_startup, otel: _, allow_login_shell, + shell_environment_policy, feedback, allowed_approval_policies, allowed_approvals_reviewers, @@ -221,6 +222,7 @@ fn populate_merged_regular_fields_with_sources( &["check_for_update_on_startup"] ); set_sourced!(allow_login_shell, &["allow_login_shell"]); + set_sourced!(shell_environment_policy, &["shell_environment_policy"]); set_sourced!(feedback, &["feedback"]); set_sourced!(allowed_approval_policies, &["allowed_approval_policies"]); set_sourced!( diff --git a/codex-rs/config/src/requirements_layers/stack_tests.rs b/codex-rs/config/src/requirements_layers/stack_tests.rs index f9b50412c432..497c64b2938a 100644 --- a/codex-rs/config/src/requirements_layers/stack_tests.rs +++ b/codex-rs/config/src/requirements_layers/stack_tests.rs @@ -6,7 +6,9 @@ use super::compose_requirements_for_hostname_and_hook_directory; use crate::ConfigRequirementsToml; use crate::ConfigRequirementsWithSources; use crate::RequirementSource; +use crate::ShellEnvironmentPolicyRequirementsToml; use crate::Sourced; +use codex_protocol::config_types::ShellEnvironmentPolicyRule; use codex_protocol::protocol::AskForApproval; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; @@ -1074,3 +1076,53 @@ shared = "high" )) ); } + +#[test] +fn shell_environment_rules_compose_by_pattern() { + let high_source = RequirementSource::EnterpriseManaged { + id: "req_high".to_string(), + name: "High".to_string(), + }; + let low_source = RequirementSource::EnterpriseManaged { + id: "req_low".to_string(), + name: "Low".to_string(), + }; + let composed = compose_requirements_for_hostname( + vec![ + RequirementsLayerEntry::from_toml( + low_source.clone(), + r#" +[shell_environment_policy.rules] +"FLIP_*" = "exclude" +"KEEP_*" = "include" +"#, + ), + RequirementsLayerEntry::from_toml( + high_source.clone(), + r#" +[shell_environment_policy.rules] +"ADD_*" = "exclude" +"FLIP_*" = "include" +"#, + ), + ], + /*hostname*/ None, + ) + .expect("compose requirements") + .expect("requirements present"); + + assert_eq!( + composed.shell_environment_policy, + Some(Sourced::new( + ShellEnvironmentPolicyRequirementsToml { + rules: Some(BTreeMap::from([ + ("ADD_*".to_string(), ShellEnvironmentPolicyRule::Exclude,), + ("FLIP_*".to_string(), ShellEnvironmentPolicyRule::Include,), + ("KEEP_*".to_string(), ShellEnvironmentPolicyRule::Include,), + ])), + ..Default::default() + }, + RequirementSource::composite([high_source, low_source]), + )) + ); +} diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index d502c56c3d65..18057838622e 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -8340,6 +8340,7 @@ async fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset() check_for_update_on_startup: None, otel: None, allow_login_shell: None, + shell_environment_policy: None, feedback: None, allowed_approval_policies: None, allowed_approvals_reviewers: None, @@ -11170,3 +11171,61 @@ zz_overflow = {configured_overflow:?} })); Ok(()) } + +#[tokio::test] +async fn managed_shell_environment_policy_overrides_matching_patterns() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#" +[shell_environment_policy] +ignore_default_excludes = true +exclude = ["USER_*", "FLIP_TO_INCLUDE"] +include_only = ["HOME", "FLIP_TO_EXCLUDE"] + +[shell_environment_policy.set] +CONFIGURED = "kept" +MANAGED = "old" +"#, + )?; + let config = load_with_enterprise_requirement( + &codex_home, + r#" +[shell_environment_policy] +ignore_default_excludes = false +experimental_use_profile = true + +[shell_environment_policy.rules] +"FLIP_TO_EXCLUDE" = "exclude" +"FLIP_TO_INCLUDE" = "include" +"MANAGED_*" = "exclude" + +[shell_environment_policy.set] +MANAGED = "required" +"#, + ) + .await?; + + let expected: codex_protocol::config_types::ShellEnvironmentPolicy = + codex_config::types::ShellEnvironmentPolicyToml { + ignore_default_excludes: Some(false), + exclude: Some(vec![ + "USER_*".to_string(), + "FLIP_TO_EXCLUDE".to_string(), + "MANAGED_*".to_string(), + ]), + include_only: Some(vec!["HOME".to_string(), "FLIP_TO_INCLUDE".to_string()]), + experimental_use_profile: Some(true), + r#set: Some(HashMap::from([ + ("CONFIGURED".to_string(), "kept".to_string()), + ("MANAGED".to_string(), "required".to_string()), + ])), + ..Default::default() + } + .into(); + assert_eq!(config.permissions.shell_environment_policy, expected); + assert!(config.startup_warnings.iter().any(|warning| { + warning.contains("Configured values under `shell_environment_policy` are overridden") + })); + Ok(()) +} diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 737c1ae7fadb..e9f6c4bba46f 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -2716,6 +2716,7 @@ impl Config { check_for_update_on_startup: _, otel: _, allow_login_shell: _, + shell_environment_policy: _, feedback: _, approval_policy: mut constrained_approval_policy, approvals_reviewer: mut constrained_approvals_reviewer, diff --git a/codex-rs/core/src/config/requirements.rs b/codex-rs/core/src/config/requirements.rs index 5ff571d7e7ff..b48b80bc6e14 100644 --- a/codex-rs/core/src/config/requirements.rs +++ b/codex-rs/core/src/config/requirements.rs @@ -1,10 +1,14 @@ use codex_config::ConfigRequirements; use codex_config::RequirementSource; +use codex_config::ShellEnvironmentPolicyRequirementsToml; use codex_config::Sourced; use codex_config::config_toml::ConfigToml; use codex_config::types::AuthCredentialsStoreMode; use codex_config::types::FeedbackConfigToml; +use codex_config::types::ShellEnvironmentPolicyToml; use codex_protocol::config_types::ForcedLoginMethod; +use codex_protocol::config_types::ShellEnvironmentPolicyRule; +use std::collections::BTreeMap; use super::otel; @@ -42,6 +46,11 @@ pub(super) fn apply_to_config( apply_exact!(model_catalog_json); apply_exact!(check_for_update_on_startup); apply_exact!(allow_login_shell); + apply_shell_environment_policy_requirement( + &mut config.shell_environment_policy, + requirements.shell_environment_policy.as_ref(), + startup_warnings, + ); if let Some(requirement) = requirements.otel.as_ref() { otel::apply_requirement(&mut config.otel, requirement, startup_warnings)?; } @@ -122,6 +131,97 @@ pub(super) fn replace_required_leaf( conflict } +fn apply_shell_environment_policy_requirement( + configured: &mut ShellEnvironmentPolicyToml, + requirement: Option<&Sourced>, + startup_warnings: &mut Vec, +) { + let Some(Sourced { value, source }) = requirement else { + return; + }; + let ShellEnvironmentPolicyRequirementsToml { + inherit, + ignore_default_excludes, + r#set: required_set, + rules, + experimental_use_profile, + } = value; + let mut conflict = false; + + conflict |= replace_required_leaf(&mut configured.inherit, inherit); + conflict |= replace_required_leaf( + &mut configured.ignore_default_excludes, + ignore_default_excludes, + ); + conflict |= apply_required_pattern_rules(configured, rules); + conflict |= replace_required_leaf( + &mut configured.experimental_use_profile, + experimental_use_profile, + ); + + if let Some(required) = required_set.as_ref() { + let configured_set = configured.r#set.get_or_insert_default(); + conflict |= required.iter().any(|(key, value)| { + configured_set + .get(key) + .is_some_and(|current| current != value) + }); + configured_set.extend(required.clone()); + } + + push_structured_requirement_override_warning( + "shell_environment_policy", + conflict, + source, + startup_warnings, + ); +} + +fn apply_required_pattern_rules( + configured: &mut ShellEnvironmentPolicyToml, + required: &Option>, +) -> bool { + let Some(required) = required else { + return false; + }; + let conflict = required.iter().any(|(pattern, required_action)| { + let configured_action = configured + .rules + .as_ref() + .and_then(|rules| rules.get(pattern).copied()) + .or_else(|| { + configured + .exclude + .as_ref() + .is_some_and(|patterns| patterns.contains(pattern)) + .then_some(ShellEnvironmentPolicyRule::Exclude) + }) + .or_else(|| { + configured + .include_only + .as_ref() + .is_some_and(|patterns| patterns.contains(pattern)) + .then_some(ShellEnvironmentPolicyRule::Include) + }); + configured_action.is_some_and(|action| action != *required_action) + }); + + for pattern in required.keys() { + if let Some(exclude) = configured.exclude.as_mut() { + exclude.retain(|candidate| candidate != pattern); + } + if let Some(include_only) = configured.include_only.as_mut() { + include_only.retain(|candidate| candidate != pattern); + } + } + configured + .rules + .get_or_insert_default() + .extend(required.clone()); + + conflict +} + fn apply_feedback_requirement( configured: &mut Option, requirement: Option<&Sourced>, From 3fe937007a82fe96fa737e6ad4ae5ec74a85966d Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Mon, 15 Jun 2026 23:26:00 +0000 Subject: [PATCH 2/4] Clarify managed requirements application semantics --- codex-rs/core/src/config/mod.rs | 10 +++++----- codex-rs/core/src/config/requirements.rs | 10 ++++++++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index e9f6c4bba46f..8e435249690c 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -2706,6 +2706,11 @@ impl Config { // Destructure every field to ensure ConfigRequirements additions are // either applied above or handled while constructing the final Config. let ConfigRequirements { + approval_policy: mut constrained_approval_policy, + approvals_reviewer: mut constrained_approvals_reviewer, + permission_profile: mut constrained_permission_profile, + windows_sandbox_mode: mut constrained_windows_sandbox_mode, + web_search_mode: mut constrained_web_search_mode, allowed_login_methods: _, allowed_chatgpt_workspaces: _, cli_auth_credentials_store: _, @@ -2718,12 +2723,7 @@ impl Config { allow_login_shell: _, shell_environment_policy: _, feedback: _, - approval_policy: mut constrained_approval_policy, - approvals_reviewer: mut constrained_approvals_reviewer, - permission_profile: mut constrained_permission_profile, - windows_sandbox_mode: mut constrained_windows_sandbox_mode, windows_sandbox_private_desktop: _, - web_search_mode: mut constrained_web_search_mode, allow_managed_hooks_only: _, allow_appshots: _, allow_remote_control: _, diff --git a/codex-rs/core/src/config/requirements.rs b/codex-rs/core/src/config/requirements.rs index b48b80bc6e14..c583f1017020 100644 --- a/codex-rs/core/src/config/requirements.rs +++ b/codex-rs/core/src/config/requirements.rs @@ -13,6 +13,10 @@ use std::collections::BTreeMap; use super::otel; /// Runtime values that cannot be applied by mutating [`ConfigToml`] alone. +/// +/// Other managed settings are written back into `ConfigToml`. These values need +/// additional credential-store compatibility handling or intersection with the +/// configured authentication restrictions before constructing the final config. pub(super) struct AppliedConfigRequirements { pub cli_auth_credentials_store_mode: AuthCredentialsStoreMode, pub forced_login_method: Option, @@ -21,8 +25,10 @@ pub(super) struct AppliedConfigRequirements { /// Applies managed requirements to regular config before final config construction. /// -/// Managed values replace their configured counterparts, and conflicts produce -/// source-aware startup warnings. +/// Managed values replace or merge with their configured counterparts, and +/// conflicts produce source-aware startup warnings. Effective authentication +/// values that need additional resolution are returned separately. Invalid or +/// conflicting requirements return an error. pub(super) fn apply_to_config( config: &mut ConfigToml, requirements: &ConfigRequirements, From 9799062d4c25a9a2a0208f8710cfa6a7c9052605 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Tue, 16 Jun 2026 07:02:22 +0000 Subject: [PATCH 3/4] Close managed shell environment policy gaps --- codex-rs/config/src/config_requirements.rs | 22 +++-- codex-rs/config/src/merge.rs | 44 ++++++++- codex-rs/config/src/merge_tests.rs | 52 +++++++++++ .../src/requirements_layers/stack_tests.rs | 16 ++-- codex-rs/config/src/types.rs | 7 -- codex-rs/core/config.schema.json | 6 +- codex-rs/core/src/config/config_tests.rs | 14 +-- codex-rs/core/src/config/requirements.rs | 93 ++++++++++++++----- .../src/tools/handlers/multi_agents_tests.rs | 2 +- codex-rs/exec-server/src/local_process.rs | 1 - codex-rs/protocol/src/config_types.rs | 4 - .../rmcp-client/src/stdio_server_launcher.rs | 1 - 12 files changed, 192 insertions(+), 70 deletions(-) diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index 3bfc96213457..9fe3ad0f7803 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -1,7 +1,7 @@ use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::config_types::SandboxMode; +use codex_protocol::config_types::ShellEnvironmentPolicyFilter; use codex_protocol::config_types::ShellEnvironmentPolicyInherit; -use codex_protocol::config_types::ShellEnvironmentPolicyRule; use codex_protocol::config_types::WebSearchMode; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; @@ -895,7 +895,7 @@ pub struct ConfigRequirementsToml { /// Managed shell environment policy fields accepted by `requirements.toml`. /// /// Unlike ordinary `config.toml`, requirements intentionally accept only the -/// keyed `rules` form. This makes include/exclude actions compose by pattern, +/// keyed `filters` form. This makes include/exclude actions compose by pattern, /// while config arrays remain a migration-only compatibility path that can be /// deprecated independently later. #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq)] @@ -904,8 +904,7 @@ pub struct ShellEnvironmentPolicyRequirementsToml { pub inherit: Option, pub ignore_default_excludes: Option, pub r#set: Option>, - pub rules: Option>, - pub experimental_use_profile: Option, + pub filters: Option>, } #[derive(Deserialize, Debug, Clone, PartialEq)] @@ -3817,10 +3816,10 @@ command = "python3 /enterprise/hooks/pre.py" } #[test] - fn shell_environment_policy_requires_rules_for_list_fields() -> Result<()> { + fn shell_environment_policy_requires_filters_for_list_fields() -> Result<()> { let requirements: ConfigRequirementsToml = from_str( r#" -[shell_environment_policy.rules] +[shell_environment_policy.filters] "HOME" = "include" "SECRET_*" = "exclude" "#, @@ -3829,9 +3828,12 @@ command = "python3 /enterprise/hooks/pre.py" assert_eq!( requirements.shell_environment_policy, Some(ShellEnvironmentPolicyRequirementsToml { - rules: Some(BTreeMap::from([ - ("HOME".to_string(), ShellEnvironmentPolicyRule::Include,), - ("SECRET_*".to_string(), ShellEnvironmentPolicyRule::Exclude,), + filters: Some(BTreeMap::from([ + ("HOME".to_string(), ShellEnvironmentPolicyFilter::Include,), + ( + "SECRET_*".to_string(), + ShellEnvironmentPolicyFilter::Exclude, + ), ])), ..Default::default() }) @@ -3845,7 +3847,7 @@ command = "python3 /enterprise/hooks/pre.py" assert!(error.to_string().contains(field)); } let error = from_str::( - "[shell_environment_policy.rules]\n\"PATH\" = \"keep\"\n", + "[shell_environment_policy.filters]\n\"PATH\" = \"keep\"\n", ) .expect_err("unknown rule actions should be rejected"); assert!(error.to_string().contains("keep")); diff --git a/codex-rs/config/src/merge.rs b/codex-rs/config/src/merge.rs index 71cbc310a2ff..373f2c3b7c58 100644 --- a/codex-rs/config/src/merge.rs +++ b/codex-rs/config/src/merge.rs @@ -21,7 +21,9 @@ fn merge_toml_values_at_path(base: &mut TomlValue, overlay: &TomlValue, path: &m normalize_network_domain_keys(base_table); normalize_network_domain_keys(&mut overlay_table); } - if is_shell_environment_filters_path(path) { + if is_shell_environment_filters_path(path) + || cfg!(target_os = "windows") && is_shell_environment_set_path(path) + { normalize_case_insensitive_keys(base_table); normalize_case_insensitive_keys(&mut overlay_table); } @@ -31,12 +33,12 @@ fn merge_toml_values_at_path(base: &mut TomlValue, overlay: &TomlValue, path: &m if let Some(existing) = base_table.get_mut(&key) { merge_toml_values_at_path(existing, &value, path); } else { - base_table.insert(key, normalized_with_key_aliases(&value, path)); + base_table.insert(key, normalized_for_merge(&value, path)); } path.pop(); } } else { - *base = normalized_with_key_aliases(overlay, path); + *base = normalized_for_merge(overlay, path); } } @@ -176,6 +178,13 @@ fn is_shell_environment_filters_path(path: &[String]) -> bool { ) } +fn is_shell_environment_set_path(path: &[String]) -> bool { + matches!( + path, + [policy, set] if policy == "shell_environment_policy" && set == "set" + ) +} + fn is_permission_network_domains_path(path: &[String]) -> bool { matches!( path, @@ -198,6 +207,35 @@ fn normalize_case_insensitive_keys(table: &mut toml::map::Map } } +fn normalized_for_merge(value: &TomlValue, path: &[String]) -> TomlValue { + let mut normalized = normalized_with_key_aliases(value, path); + normalize_nested_case_insensitive_keys(&mut normalized, &mut path.to_vec()); + normalized +} + +fn normalize_nested_case_insensitive_keys(value: &mut TomlValue, path: &mut Vec) { + match value { + TomlValue::Table(table) => { + if is_shell_environment_filters_path(path) + || cfg!(target_os = "windows") && is_shell_environment_set_path(path) + { + normalize_case_insensitive_keys(table); + } + for (key, value) in table { + path.push(key.clone()); + normalize_nested_case_insensitive_keys(value, path); + path.pop(); + } + } + TomlValue::Array(items) => { + for item in items { + normalize_nested_case_insensitive_keys(item, path); + } + } + _ => {} + } +} + #[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 81636b15338c..fb43337ecc7a 100644 --- a/codex-rs/config/src/merge_tests.rs +++ b/codex-rs/config/src/merge_tests.rs @@ -178,6 +178,58 @@ fn shell_environment_policy_filters_overlay_merges_by_key_case_insensitively() { ); } +#[test] +fn shell_environment_policy_filters_are_normalized_in_the_first_layer() { + let mut base = parse_toml(""); + let overlay = parse_toml( + r#" +[shell_environment_policy.filters] +"UPPER_*" = "exclude" +"#, + ); + + merge_toml_values(&mut base, &overlay); + + assert_eq!( + base, + parse_toml( + r#" +[shell_environment_policy.filters] +"upper_*" = "exclude" +"#, + ) + ); +} + +#[cfg(target_os = "windows")] +#[test] +fn shell_environment_policy_set_keys_merge_case_insensitively_on_windows() { + let mut base = parse_toml( + r#" +[shell_environment_policy.set] +Path = "low" +"#, + ); + let overlay = parse_toml( + r#" +[shell_environment_policy.set] +PATH = "high" +"#, + ); + + merge_toml_values(&mut base, &overlay); + + assert_eq!( + base, + parse_toml( + r#" +[shell_environment_policy.set] +path = "high" +"#, + ) + ); +} + #[test] fn shell_environment_policy_filters_override_lower_legacy_arrays_by_pattern() { let mut base = parse_toml( diff --git a/codex-rs/config/src/requirements_layers/stack_tests.rs b/codex-rs/config/src/requirements_layers/stack_tests.rs index 497c64b2938a..4cdb1f75d446 100644 --- a/codex-rs/config/src/requirements_layers/stack_tests.rs +++ b/codex-rs/config/src/requirements_layers/stack_tests.rs @@ -8,7 +8,7 @@ use crate::ConfigRequirementsWithSources; use crate::RequirementSource; use crate::ShellEnvironmentPolicyRequirementsToml; use crate::Sourced; -use codex_protocol::config_types::ShellEnvironmentPolicyRule; +use codex_protocol::config_types::ShellEnvironmentPolicyFilter; use codex_protocol::protocol::AskForApproval; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; @@ -1078,7 +1078,7 @@ shared = "high" } #[test] -fn shell_environment_rules_compose_by_pattern() { +fn shell_environment_filters_compose_by_pattern() { let high_source = RequirementSource::EnterpriseManaged { id: "req_high".to_string(), name: "High".to_string(), @@ -1092,7 +1092,7 @@ fn shell_environment_rules_compose_by_pattern() { RequirementsLayerEntry::from_toml( low_source.clone(), r#" -[shell_environment_policy.rules] +[shell_environment_policy.filters] "FLIP_*" = "exclude" "KEEP_*" = "include" "#, @@ -1100,7 +1100,7 @@ fn shell_environment_rules_compose_by_pattern() { RequirementsLayerEntry::from_toml( high_source.clone(), r#" -[shell_environment_policy.rules] +[shell_environment_policy.filters] "ADD_*" = "exclude" "FLIP_*" = "include" "#, @@ -1115,10 +1115,10 @@ fn shell_environment_rules_compose_by_pattern() { composed.shell_environment_policy, Some(Sourced::new( ShellEnvironmentPolicyRequirementsToml { - rules: Some(BTreeMap::from([ - ("ADD_*".to_string(), ShellEnvironmentPolicyRule::Exclude,), - ("FLIP_*".to_string(), ShellEnvironmentPolicyRule::Include,), - ("KEEP_*".to_string(), ShellEnvironmentPolicyRule::Include,), + filters: Some(BTreeMap::from([ + ("add_*".to_string(), ShellEnvironmentPolicyFilter::Exclude,), + ("flip_*".to_string(), ShellEnvironmentPolicyFilter::Include,), + ("keep_*".to_string(), ShellEnvironmentPolicyFilter::Include,), ])), ..Default::default() }, diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index f5f935dd64c6..70f553eb6527 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -936,8 +936,6 @@ pub struct ShellEnvironmentPolicyToml { /// migration. Requirements accept only this keyed form, keeping array /// compatibility isolated so the legacy fields can be deprecated later. pub filters: Option>, - - pub experimental_use_profile: Option, } #[derive(Deserialize)] @@ -949,7 +947,6 @@ struct ShellEnvironmentPolicyTomlRaw { r#set: Option>, include_only: Option>, filters: Option>, - experimental_use_profile: Option, } impl<'de> Deserialize<'de> for ShellEnvironmentPolicyToml { @@ -970,7 +967,6 @@ impl<'de> Deserialize<'de> for ShellEnvironmentPolicyToml { r#set: raw.r#set, include_only: raw.include_only, filters: raw.filters, - experimental_use_profile: raw.experimental_use_profile, }) } } @@ -999,15 +995,12 @@ impl From for ShellEnvironmentPolicy { .into_iter() .map(|s| EnvironmentVariablePattern::new_case_insensitive(&s)) .collect(); - let use_profile = toml.experimental_use_profile.unwrap_or(false); - Self { inherit, ignore_default_excludes, exclude, r#set, include_only, - use_profile, } } } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index c2d365df22aa..0c7a5769970a 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -2639,9 +2639,6 @@ }, "type": "array" }, - "experimental_use_profile": { - "type": "boolean" - }, "filters": { "additionalProperties": { "$ref": "#/definitions/ShellEnvironmentPolicyFilter" @@ -5213,7 +5210,6 @@ ], "default": { "exclude": null, - "experimental_use_profile": null, "filters": null, "ignore_default_excludes": null, "include_only": null, @@ -5295,4 +5291,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 18057838622e..c49c3793ee71 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -11185,7 +11185,9 @@ include_only = ["HOME", "FLIP_TO_EXCLUDE"] [shell_environment_policy.set] CONFIGURED = "kept" +CONFIGURED_SECRET = "removed" MANAGED = "old" +MANAGED_BLOCKED = "removed" "#, )?; let config = load_with_enterprise_requirement( @@ -11193,15 +11195,15 @@ MANAGED = "old" r#" [shell_environment_policy] ignore_default_excludes = false -experimental_use_profile = true -[shell_environment_policy.rules] +[shell_environment_policy.filters] "FLIP_TO_EXCLUDE" = "exclude" "FLIP_TO_INCLUDE" = "include" "MANAGED_*" = "exclude" [shell_environment_policy.set] MANAGED = "required" +MANAGED_SECRET_ALLOWED = "required" "#, ) .await?; @@ -11211,14 +11213,14 @@ MANAGED = "required" ignore_default_excludes: Some(false), exclude: Some(vec![ "USER_*".to_string(), - "FLIP_TO_EXCLUDE".to_string(), - "MANAGED_*".to_string(), + "flip_to_exclude".to_string(), + "managed_*".to_string(), ]), - include_only: Some(vec!["HOME".to_string(), "FLIP_TO_INCLUDE".to_string()]), - experimental_use_profile: Some(true), + include_only: Some(vec!["HOME".to_string(), "flip_to_include".to_string()]), r#set: Some(HashMap::from([ ("CONFIGURED".to_string(), "kept".to_string()), ("MANAGED".to_string(), "required".to_string()), + ("MANAGED_SECRET_ALLOWED".to_string(), "required".to_string()), ])), ..Default::default() } diff --git a/codex-rs/core/src/config/requirements.rs b/codex-rs/core/src/config/requirements.rs index c583f1017020..a014d0f2c91f 100644 --- a/codex-rs/core/src/config/requirements.rs +++ b/codex-rs/core/src/config/requirements.rs @@ -6,8 +6,9 @@ use codex_config::config_toml::ConfigToml; use codex_config::types::AuthCredentialsStoreMode; use codex_config::types::FeedbackConfigToml; use codex_config::types::ShellEnvironmentPolicyToml; +use codex_protocol::config_types::EnvironmentVariablePattern; use codex_protocol::config_types::ForcedLoginMethod; -use codex_protocol::config_types::ShellEnvironmentPolicyRule; +use codex_protocol::config_types::ShellEnvironmentPolicyFilter; use std::collections::BTreeMap; use super::otel; @@ -149,8 +150,7 @@ fn apply_shell_environment_policy_requirement( inherit, ignore_default_excludes, r#set: required_set, - rules, - experimental_use_profile, + filters, } = value; let mut conflict = false; @@ -159,20 +159,40 @@ fn apply_shell_environment_policy_requirement( &mut configured.ignore_default_excludes, ignore_default_excludes, ); - conflict |= apply_required_pattern_rules(configured, rules); - conflict |= replace_required_leaf( - &mut configured.experimental_use_profile, - experimental_use_profile, - ); + conflict |= apply_required_pattern_filters(configured, filters); + + if let Some(configured_set) = configured.r#set.as_mut() { + let mut required_excludes = filters + .iter() + .flatten() + .filter(|(_, action)| **action == ShellEnvironmentPolicyFilter::Exclude) + .map(|(pattern, _)| EnvironmentVariablePattern::new_case_insensitive(pattern)) + .collect::>(); + if ignore_default_excludes == &Some(false) { + required_excludes.extend( + ["*KEY*", "*SECRET*", "*TOKEN*"] + .map(EnvironmentVariablePattern::new_case_insensitive), + ); + } + let previous_len = configured_set.len(); + configured_set + .retain(|key, _| !required_excludes.iter().any(|pattern| pattern.matches(key))); + conflict |= configured_set.len() != previous_len; + } if let Some(required) = required_set.as_ref() { let configured_set = configured.r#set.get_or_insert_default(); conflict |= required.iter().any(|(key, value)| { configured_set - .get(key) + .iter() + .find(|(configured_key, _)| environment_keys_equal(configured_key, key)) + .map(|(_, current)| current) .is_some_and(|current| current != value) }); - configured_set.extend(required.clone()); + for (key, value) in required { + configured_set.retain(|configured_key, _| !environment_keys_equal(configured_key, key)); + configured_set.insert(key.clone(), value.clone()); + } } push_structured_requirement_override_warning( @@ -183,51 +203,76 @@ fn apply_shell_environment_policy_requirement( ); } -fn apply_required_pattern_rules( +fn apply_required_pattern_filters( configured: &mut ShellEnvironmentPolicyToml, - required: &Option>, + required: &Option>, ) -> bool { let Some(required) = required else { return false; }; let conflict = required.iter().any(|(pattern, required_action)| { let configured_action = configured - .rules + .filters .as_ref() - .and_then(|rules| rules.get(pattern).copied()) + .and_then(|filters| { + filters + .iter() + .find(|(configured_pattern, _)| { + configured_pattern.eq_ignore_ascii_case(pattern) + }) + .map(|(_, action)| *action) + }) .or_else(|| { configured .exclude .as_ref() - .is_some_and(|patterns| patterns.contains(pattern)) - .then_some(ShellEnvironmentPolicyRule::Exclude) + .is_some_and(|patterns| { + patterns + .iter() + .any(|candidate| candidate.eq_ignore_ascii_case(pattern)) + }) + .then_some(ShellEnvironmentPolicyFilter::Exclude) }) .or_else(|| { configured .include_only .as_ref() - .is_some_and(|patterns| patterns.contains(pattern)) - .then_some(ShellEnvironmentPolicyRule::Include) + .is_some_and(|patterns| { + patterns + .iter() + .any(|candidate| candidate.eq_ignore_ascii_case(pattern)) + }) + .then_some(ShellEnvironmentPolicyFilter::Include) }); configured_action.is_some_and(|action| action != *required_action) }); for pattern in required.keys() { if let Some(exclude) = configured.exclude.as_mut() { - exclude.retain(|candidate| candidate != pattern); + exclude.retain(|candidate| !candidate.eq_ignore_ascii_case(pattern)); } if let Some(include_only) = configured.include_only.as_mut() { - include_only.retain(|candidate| candidate != pattern); + include_only.retain(|candidate| !candidate.eq_ignore_ascii_case(pattern)); } } - configured - .rules - .get_or_insert_default() - .extend(required.clone()); + let configured_filters = configured.filters.get_or_insert_default(); + for (pattern, action) in required { + configured_filters + .retain(|configured_pattern, _| !configured_pattern.eq_ignore_ascii_case(pattern)); + configured_filters.insert(pattern.clone(), *action); + } conflict } +fn environment_keys_equal(left: &str, right: &str) -> bool { + if cfg!(target_os = "windows") { + left.eq_ignore_ascii_case(right) + } else { + left == right + } +} + fn apply_feedback_requirement( configured: &mut Option, requirement: Option<&Sourced>, diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index 8c3ea14325ac..0550c596c542 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -4465,7 +4465,7 @@ async fn build_agent_spawn_config_uses_turn_context_values() { turn.developer_instructions = Some("dev".to_string()); turn.compact_prompt = Some("compact".to_string()); turn.shell_environment_policy = ShellEnvironmentPolicy { - use_profile: true, + ignore_default_excludes: false, ..ShellEnvironmentPolicy::default() }; let temp_dir = tempfile::tempdir().expect("temp dir"); diff --git a/codex-rs/exec-server/src/local_process.rs b/codex-rs/exec-server/src/local_process.rs index 78a53897ee48..398229cf6c7a 100644 --- a/codex-rs/exec-server/src/local_process.rs +++ b/codex-rs/exec-server/src/local_process.rs @@ -463,7 +463,6 @@ fn shell_environment_policy(env_policy: &ExecEnvPolicy) -> ShellEnvironmentPolic .iter() .map(|pattern| EnvironmentVariablePattern::new_case_insensitive(pattern)) .collect(), - use_profile: false, } } diff --git a/codex-rs/protocol/src/config_types.rs b/codex-rs/protocol/src/config_types.rs index 051fd2e68252..c45bb3f3aa63 100644 --- a/codex-rs/protocol/src/config_types.rs +++ b/codex-rs/protocol/src/config_types.rs @@ -252,9 +252,6 @@ pub struct ShellEnvironmentPolicy { /// Environment variable names to retain in the environment. pub include_only: Vec, - - /// If true, the shell profile will be used to run the command. - pub use_profile: bool, } impl Default for ShellEnvironmentPolicy { @@ -265,7 +262,6 @@ impl Default for ShellEnvironmentPolicy { exclude: Vec::new(), r#set: HashMap::new(), include_only: Vec::new(), - use_profile: false, } } } diff --git a/codex-rs/rmcp-client/src/stdio_server_launcher.rs b/codex-rs/rmcp-client/src/stdio_server_launcher.rs index 8ada806ff754..eb4f20f207d5 100644 --- a/codex-rs/rmcp-client/src/stdio_server_launcher.rs +++ b/codex-rs/rmcp-client/src/stdio_server_launcher.rs @@ -628,7 +628,6 @@ mod tests { .iter() .map(|pattern| EnvironmentVariablePattern::new_case_insensitive(pattern)) .collect(), - use_profile: false, }; let env = shell_environment::create_env_from_vars( From 96602d7990cb35d94ec34fd23c3f82377cd1c2b1 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Tue, 16 Jun 2026 08:00:03 +0000 Subject: [PATCH 4/4] Fix managed shell policy test on Windows --- codex-rs/core/src/config/config_tests.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index c49c3793ee71..67858be94149 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -11208,6 +11208,16 @@ MANAGED_SECRET_ALLOWED = "required" ) .await?; + let expected_set = HashMap::from([ + ("CONFIGURED".to_string(), "kept".to_string()), + ("MANAGED".to_string(), "required".to_string()), + ("MANAGED_SECRET_ALLOWED".to_string(), "required".to_string()), + ]); + #[cfg(target_os = "windows")] + let expected_set = expected_set + .into_iter() + .map(|(key, value)| (key.to_ascii_lowercase(), value)) + .collect(); let expected: codex_protocol::config_types::ShellEnvironmentPolicy = codex_config::types::ShellEnvironmentPolicyToml { ignore_default_excludes: Some(false), @@ -11217,11 +11227,7 @@ MANAGED_SECRET_ALLOWED = "required" "managed_*".to_string(), ]), include_only: Some(vec!["HOME".to_string(), "flip_to_include".to_string()]), - r#set: Some(HashMap::from([ - ("CONFIGURED".to_string(), "kept".to_string()), - ("MANAGED".to_string(), "required".to_string()), - ("MANAGED_SECRET_ALLOWED".to_string(), "required".to_string()), - ])), + r#set: Some(expected_set), ..Default::default() } .into();