diff --git a/codex-rs/config/src/loader/local.rs b/codex-rs/config/src/loader/local.rs index c235166cc2..f45c204746 100644 --- a/codex-rs/config/src/loader/local.rs +++ b/codex-rs/config/src/loader/local.rs @@ -1,4 +1,3 @@ -use super::CredentialBrokerProjectState; use super::apply_credential_broker_requirements; use super::credential_broker_trusted_config; use super::discover_project_layers; @@ -150,23 +149,30 @@ pub(super) async fn load_local_config_layers_with_overrides( &user_file, ) .await?; - if trust_context.credential_broker != CredentialBrokerProjectState::Unconfigured { - let broker_requirements = requirements.clone().project(&[ - vec!["features".to_string(), "network_proxy".to_string()], - vec![ - "feature_requirements".to_string(), - "network_proxy".to_string(), - ], - vec!["experimental_network".to_string(), "enabled".to_string()], - ]); - let effective_requirements = - compose_requirements(broker_requirements.layers.into_iter().map(|layer| { - RequirementsLayerEntry::from_toml_value(layer.source, layer.toml) - .with_base_dir(layer.base_dir) - }))? - .unwrap_or_default(); - apply_credential_broker_requirements(&mut trust_context, &effective_requirements); - } + let broker_requirements = requirements.clone().project(&[ + vec!["features".to_string(), "network_proxy".to_string()], + vec!["features".to_string(), "credential_masking".to_string()], + vec![ + "feature_requirements".to_string(), + "network_proxy".to_string(), + ], + vec![ + "feature_requirements".to_string(), + "credential_masking".to_string(), + ], + vec!["experimental_network".to_string(), "enabled".to_string()], + ]); + let effective_requirements = + compose_requirements(broker_requirements.layers.into_iter().map(|layer| { + RequirementsLayerEntry::from_toml_value(layer.source, layer.toml) + .with_base_dir(layer.base_dir) + }))? + .unwrap_or_default(); + apply_credential_broker_requirements( + &mut trust_context, + &effective_requirements, + &trusted_broker_config, + ); let project_layers = discover_project_layers( fs, cwd, diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index e524076d7b..46dbb8b055 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -399,7 +399,11 @@ pub async fn load_config_layers_state( return Err(err); } }; - apply_credential_broker_requirements(&mut project_trust_context, &config_requirements_toml); + apply_credential_broker_requirements( + &mut project_trust_context, + &config_requirements_toml, + &trusted_broker_config, + ); let project_layers = load_project_layers( fs, &cwd, @@ -991,16 +995,32 @@ enum CredentialBrokerProjectState { fn apply_credential_broker_requirements( context: &mut ProjectTrustContext, requirements: &crate::ConfigRequirementsWithSources, + trusted_broker_config: &TomlValue, ) { - if context.credential_broker == CredentialBrokerProjectState::Unconfigured { - return; - } - let enabled = requirements + let credential_masking_enabled = requirements .feature_requirements .as_ref() - .and_then(|requirements| requirements.entries.get("network_proxy")) + .and_then(|requirements| requirements.entries.get("credential_masking")) .copied() - .unwrap_or(context.credential_broker == CredentialBrokerProjectState::Enabled); + .or_else(|| { + trusted_broker_config + .get("features") + .and_then(|features| features.get("credential_masking")) + .and_then(TomlValue::as_bool) + }) + .unwrap_or(false); + if !credential_masking_enabled + && context.credential_broker == CredentialBrokerProjectState::Unconfigured + { + return; + } + let enabled = credential_masking_enabled + || requirements + .feature_requirements + .as_ref() + .and_then(|requirements| requirements.entries.get("network_proxy")) + .copied() + .unwrap_or(context.credential_broker == CredentialBrokerProjectState::Enabled); context.credential_broker = if enabled && requirements .network @@ -1171,7 +1191,11 @@ fn sanitize_project_config( { ignored_keys.push("features.multi_agent_v2.message_board_remote".to_string()); } - for key in ["respect_system_proxy", "system_proxy_fallback"] { + for key in [ + "respect_system_proxy", + "system_proxy_fallback", + "credential_masking", + ] { if features.remove(key).is_some() { ignored_keys.push(format!("features.{key}")); } diff --git a/codex-rs/config/src/loader/tests.rs b/codex-rs/config/src/loader/tests.rs index e1ae693f30..e38074b812 100644 --- a/codex-rs/config/src/loader/tests.rs +++ b/codex-rs/config/src/loader/tests.rs @@ -185,6 +185,140 @@ fn disabled_credential_broker_preserves_project_shell_settings() { ); } +#[tokio::test] +async fn credential_masking_preserves_project_protections_in_both_loaders() { + use CredentialBrokerProjectState::Disabled; + use CredentialBrokerProjectState::Enabled; + use CredentialBrokerProjectState::Unconfigured; + + for (features, requirements, broker_state) in [ + ( + "[features]\ncredential_masking=true\nnetwork_proxy=true", + "", + Enabled, + ), + ( + "[features]\ncredential_masking=true\n[features.network_proxy]\nenabled=true", + "", + Enabled, + ), + ( + "[features]\ncredential_masking=true", + "[experimental_network]\nenabled=true", + Enabled, + ), + ( + "[features]\ncredential_masking=true\nnetwork_proxy=false", + "", + Enabled, + ), + ( + "[features]\ncredential_masking=false\nnetwork_proxy=true", + "[features]\ncredential_masking=true", + Enabled, + ), + ( + "[features]\ncredential_masking=true\nnetwork_proxy=true", + "[features]\ncredential_masking=false", + Unconfigured, + ), + ( + "[features]\ncredential_masking=false\n[features.network_proxy]\nenabled=true\ncredential_broker=true", + "[features]\ncredential_masking=false", + Enabled, + ), + ( + "[features]\ncredential_masking=true\nnetwork_proxy=true", + "[experimental_network]\nenabled=false", + Disabled, + ), + ( + "[features]\ncredential_masking=false\nnetwork_proxy=true", + "[features]\ncredential_masking=true\n[experimental_network]\nenabled=false", + Disabled, + ), + ( + "[features]\ncredential_masking=false\n[features.network_proxy]\nenabled=true\ncredential_broker=true", + "[experimental_network]\nenabled=false", + Disabled, + ), + ( + "[features]\ncredential_masking=true\nnetwork_proxy=true", + "[features]\nnetwork_proxy=false\n[experimental_network]\nenabled=true", + Enabled, + ), + ] { + let tmp = tempdir().expect("tempdir"); + let codex_home = tmp.path().join("codex-home"); + let project = tmp.path().join("project"); + let dot_codex = project.join(".codex"); + std::fs::create_dir_all(&codex_home).expect("create Codex home"); + std::fs::create_dir_all(&dot_codex).expect("create project config directory"); + std::fs::write(project.join(".project-root"), "").expect("write project marker"); + let project_key = TomlValue::String(project_trust_key(&project)).to_string(); + std::fs::write( + codex_home.join(CONFIG_TOML_FILE), + format!("project_root_markers=['.project-root']\n[projects.{project_key}]\ntrust_level='trusted'\n{features}"), + ) + .expect("write trusted config"); + let project_config = "[features]\ncredential_masking=false\nnetwork_proxy=false\nshell_snapshot=false\n\ + [shell_environment_policy]\nexperimental_use_profile=true\n\ + [shell_environment_policy.set]\nGH_HOST='untrusted.example'\nGH_TOKEN='project-value'\n\ + ZDOTDIR='/project-startup'\nBASH_ENV='/project-startup'\nUNCHANGED='allowed'"; + std::fs::write(dot_codex.join(CONFIG_TOML_FILE), project_config) + .expect("write project config"); + let requirements_path = tmp.path().join("requirements.toml"); + std::fs::write(&requirements_path, requirements).expect("write requirements"); + let mut overrides = LoaderOverrides::without_managed_config_for_tests(); + overrides.system_config_path = Some(tmp.path().join("missing-system.toml")); + overrides.system_requirements_path = Some(requirements_path); + let cwd = AbsolutePathBuf::from_absolute_path(&project).expect("absolute cwd"); + let stack = load_config_layers_state( + &TestFileSystem, + &codex_home, + Some(cwd.clone()), + &[], + overrides.clone(), + &crate::NoopThreadConfigLoader, + ) + .await + .expect("load config stack"); + let local = local::load_local_config_layers_with_overrides( + &TestFileSystem, + &codex_home, + &cwd, + &overrides, + ) + .await + .expect("load local layers"); + let expected = match broker_state { + Enabled => { + "[features]\n[shell_environment_policy]\n[shell_environment_policy.set]\nUNCHANGED='allowed'".to_string() + } + Disabled => project_config + .replace("credential_masking=false\n", "") + .replace("network_proxy=false\n", ""), + Unconfigured => project_config.replace("credential_masking=false\n", ""), + }; + let expected: TomlValue = toml::from_str(&expected).expect("expected project config"); + let project_layer = stack + .all_layers_low_to_high() + .find(|layer| matches!(layer.name, ConfigLayerSource::Project { .. })) + .expect("project config layer"); + assert_eq!(project_layer.config, expected, "{features}; {requirements}"); + let local_project_layer = local + .config + .layers + .iter() + .find(|layer| matches!(layer.source, ConfigLayerSource::Project { .. })) + .expect("local project config layer"); + assert_eq!( + local_project_layer.toml, expected, + "{features}; {requirements}" + ); + } +} + #[test] fn project_environment_filters_preserve_child_policy() { for project_config in [ @@ -1058,8 +1192,12 @@ async fn local_layers_keep_raw_paths_order_and_legacy_requirements() { } #[test] -fn project_config_cannot_change_system_proxy_routing() { - for key in ["respect_system_proxy", "system_proxy_fallback"] { +fn project_config_cannot_change_system_proxy_routing_or_credential_masking() { + for key in [ + "respect_system_proxy", + "system_proxy_fallback", + "credential_masking", + ] { for enabled in [false, true] { let mut config: TomlValue = toml::from_str(&format!("[features]\n{key} = {enabled}\nplugins = true")) diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 432b31c63a..d9594bb0e7 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -854,6 +854,9 @@ "context_management": { "$ref": "#/definitions/FeatureToml_for_ContextManagementConfigToml" }, + "credential_masking": { + "type": "boolean" + }, "current_time_reminder": { "$ref": "#/definitions/FeatureToml_for_CurrentTimeReminderConfigToml" }, @@ -7125,6 +7128,9 @@ "context_management": { "$ref": "#/definitions/FeatureToml_for_ContextManagementConfigToml" }, + "credential_masking": { + "type": "boolean" + }, "current_time_reminder": { "$ref": "#/definitions/FeatureToml_for_CurrentTimeReminderConfigToml" }, diff --git a/codex-rs/core/src/config/config_loader_tests.rs b/codex-rs/core/src/config/config_loader_tests.rs index 39df96b292..45fbf4930a 100644 --- a/codex-rs/core/src/config/config_loader_tests.rs +++ b/codex-rs/core/src/config/config_loader_tests.rs @@ -37,13 +37,17 @@ use codex_features::Feature; use codex_protocol::config_types::EnvironmentVariablePattern; use codex_protocol::config_types::TrustLevel; use codex_protocol::config_types::WebSearchMode; +use codex_protocol::models::ActivePermissionProfile; use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS; use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE; +use codex_protocol::models::ManagedFileSystemPermissions; use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathConvention; use codex_utils_path_uri::PathUri; +use codex_utils_path_uri::Platform; use pretty_assertions::assert_eq; use std::collections::BTreeMap; use std::collections::HashMap; @@ -1930,15 +1934,140 @@ enable_socks5 = false Ok(()) } +#[tokio::test] +async fn credential_masking_respects_config_precedence() -> std::io::Result<()> { + for (features, requirements, expected_proxy, expected_broker) in [ + ( + "credential_masking = true\n[features.network_proxy]\nenabled = true\ncredential_broker = false", + "", + true, + true, + ), + ( + "credential_masking = false\n[features.network_proxy]\nenabled = true\ncredential_broker = true", + "", + true, + true, + ), + ( + r#"credential_masking = false +[features.network_proxy] +enabled = false +[features.network_proxy.credentials.vendor] +env = ["VENDOR_TOKEN"] +patterns = ["vendor_[a-z]{8}"] +url_prefixes = ["https://api.vendor.example"] +auth = ["bearer"]"#, + "[features]\ncredential_masking = true\n[experimental_network]\nenabled = true", + true, + true, + ), + ( + "credential_masking = true\nnetwork_proxy = true", + "[features]\ncredential_masking = false", + true, + false, + ), + ( + "credential_masking = true\nnetwork_proxy = true", + "[experimental_network]\nenabled = false", + false, + false, + ), + ( + "credential_masking = true", + "[experimental_network]\nallow_local_binding = true", + false, + false, + ), + ] { + let home = tempdir()?; + let source = + format!("openai_base_url = 'https://api.example.test/v1'\n[features]\n{features}\n"); + let configured: ConfigToml = toml::from_str(&source).expect("valid config"); + tokio::fs::write(home.path().join(CONFIG_TOML_FILE), source).await?; + let config = ConfigBuilder::without_managed_config_for_tests() + .codex_home(home.path().to_path_buf()) + .harness_overrides(ConfigOverrides { + cwd: Some(home.path().to_path_buf()), + permission_profile: Some(PermissionProfile::Managed { + file_system: ManagedFileSystemPermissions::Unrestricted, + network: NetworkSandboxPolicy::Enabled, + }), + ..Default::default() + }) + .cloud_config_bundle( + CloudConfigBundleFixture::loader_with_enterprise_requirement(requirements), + ) + .build() + .await?; + let network = config.permissions.network.as_ref().expect("proxy settings"); + assert_eq!( + (network.enabled(), network.credential_broker_enabled()), + (expected_proxy, expected_broker), + "{features}; {requirements}" + ); + let proxy = network + .build_config_state_for_spec(Platform::native())? + .config; + assert_eq!( + (proxy.enabled, proxy.credential_broker, proxy.mitm), + (expected_proxy, expected_broker, expected_broker), + "{features}; {requirements}" + ); + if expected_broker { + let expected_providers = super::network_proxy_toml_config(configured.features.as_ref()) + .and_then(|network_proxy| network_proxy.credentials.clone()) + .unwrap_or_default(); + assert_eq!(proxy.credential_providers, expected_providers); + assert_eq!( + proxy.credential_broker_openai_host.as_deref(), + Some("api.example.test") + ); + } + + let active_profile = ActivePermissionProfile { + id: BUILT_IN_PERMISSION_PROFILE_WORKSPACE.to_string(), + extends: None, + }; + let reloaded = config.network_proxy_spec_for_active_permission_profile( + &active_profile, + &config.permissions.effective_permission_profile(), + )?; + assert_eq!( + reloaded, config.permissions.network, + "{features}; {requirements}" + ); + if requirements.is_empty() { + assert_eq!( + config.network_proxy_spec_for_active_permission_profile( + &active_profile, + &PermissionProfile::read_only(), + )?, + None, + "profile change should leave masking inactive when the proxy is disabled" + ); + } + } + Ok(()) +} + #[tokio::test] async fn credential_broker_ambiguity_guard_survives_permission_profile_selection() -> anyhow::Result<()> { - for (key, other_key, other_value, ambiguous) in [ - ("GH_HOST", "gh_host", "second.example", true), - ("VENDOR_HOST", "vendor_host", "second.example", true), - ("VENDOR_PASSWORD", "vendor_password", "second.example", true), - ("VENDOR_HOST", "vendor_host", "first.example", false), - ("FOO", "foo", "second.example", false), + for (key, other_key, other_value, ambiguous, credential_masking) in [ + ("GH_HOST", "gh_host", "second.example", true, false), + ("VENDOR_HOST", "vendor_host", "second.example", true, false), + ( + "VENDOR_PASSWORD", + "vendor_password", + "second.example", + true, + false, + ), + ("VENDOR_HOST", "vendor_host", "first.example", false, false), + ("FOO", "foo", "second.example", false, false), + ("GH_HOST", "gh_host", "second.example", true, true), ] { let home = tempdir()?; tokio::fs::write( @@ -1946,6 +2075,8 @@ async fn credential_broker_ambiguity_guard_survives_permission_profile_selection format!( r#" default_permissions = "first" +[features] +credential_masking = {credential_masking} [features.network_proxy] enabled = true credential_broker = true @@ -1975,6 +2106,11 @@ extends = "first" .build() .await?; let expected_enabled = !(cfg!(windows) && ambiguous); + if credential_masking && !expected_enabled { + insta::assert_snapshot!(config.startup_warnings.join("\n"), @r"Credential masking is inactive because shell environment overrides contain conflicting case-insensitive provider keys. Commands will receive unmasked credentials. Remove the conflicting entries from `shell_environment_policy.set`."); + } else { + assert_eq!(config.startup_warnings, Vec::::new()); + } assert_eq!( config.permissions.shell_environment_policy.r#set, HashMap::from([ diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 696872522d..f10ea126f9 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -152,7 +152,6 @@ use std::time::Duration; use crate::config::permissions::BUILT_IN_READ_ONLY_PROFILE; use crate::config::permissions::BUILT_IN_WORKSPACE_PROFILE; -use crate::config::permissions::apply_network_proxy_feature_config; use crate::config::permissions::default_builtin_permission_profile_name; use crate::config::permissions::network_proxy_config_for_profile_selection; use crate::config::permissions::validate_user_permission_profile_names; @@ -587,7 +586,19 @@ fn build_network_proxy_spec( network_requirements: Option>, permission_profile: &PermissionProfile, environment_overrides: &HashMap, + features: &ManagedFeatures, + feature_settings: Option<&FeaturesToml>, ) -> std::io::Result> { + if features.enabled(Feature::CredentialMasking) { + configured_network_proxy_config.set_credential_broker_enabled(/*enabled*/ true); + if let Some(credentials) = network_proxy_toml_config(feature_settings) + .and_then(|network_proxy| network_proxy.credentials.as_ref()) + { + configured_network_proxy_config + .credential_providers + .clone_from(credentials); + } + } configured_network_proxy_config.configure_credential_broker_environment(environment_overrides); PreparedNetworkConfig { configured_proxy: configured_network_proxy_config, @@ -4206,6 +4217,8 @@ impl Config { network_requirements, &network_permission_profile, &shell_environment_policy.r#set, + &features, + cfg.features.as_ref(), )?; let mut helper_readable_roots: Vec<_> = std::env::var_os("PATH") .as_deref() @@ -4318,7 +4331,7 @@ impl Config { ) .map_err(std::io::Error::from)?; let otel = otel::resolve_config(cfg.otel.unwrap_or_default(), &mut startup_warnings); - let config = Self { + let mut config = Self { plugins: cfg.plugins, prefer_mxc, model, @@ -4617,6 +4630,10 @@ impl Config { .unwrap_or_default(), otel, }; + if let Some(message) = config.credential_masking_warning(config.permissions.permission_profile()) { + tracing::warn!("{message}"); + config.startup_warnings.push(message.to_string()); + } Ok(config) }) .await @@ -4726,53 +4743,86 @@ impl Config { ) -> std::io::Result> { let profile_allows_network_proxy = profile_allows_configured_network_proxy(permission_profile); + let cfg: ConfigToml = self + .config_layer_stack + .effective_config() + .try_into() + .map_err(|err| { + std::io::Error::new( + ErrorKind::InvalidInput, + format!( + "failed to read effective config for selected permission profile: {err}" + ), + ) + })?; let configured_network_proxy_config = if profile_allows_network_proxy { - let cfg: ConfigToml = self - .config_layer_stack - .effective_config() - .try_into() - .map_err(|err| { - std::io::Error::new( - ErrorKind::InvalidInput, - format!( - "failed to read effective config for selected permission profile: {err}" - ), - ) - })?; let permissions = merge_managed_permission_profiles( cfg.permissions.as_ref(), self.config_layer_stack.requirements_toml(), )?; - let mut configured_network_proxy_config = network_proxy_config_for_profile_selection( + network_proxy_config_for_profile_selection( permissions.as_ref(), active_permission_profile.id.as_str(), - )?; - if self.features.enabled(Feature::NetworkProxy) - && permission_profile.network_sandbox_policy().is_enabled() - { - if let Some(network_proxy) = network_proxy_toml_config(cfg.features.as_ref()) { - apply_network_proxy_feature_config( - &mut configured_network_proxy_config, - network_proxy, - ); - } - configured_network_proxy_config - .set_credential_broker_openai_base_url(cfg.openai_base_url.as_deref()); - configured_network_proxy_config.enabled = true; - } - configured_network_proxy_config + )? } else { NetworkProxyConfig::default() }; - + let prepared_network = PreparedNetworkConfig::from_inputs(NetworkConfigInputs { + configured_proxy: configured_network_proxy_config, + feature_enabled: profile_allows_network_proxy + && self.features.enabled(Feature::NetworkProxy), + features: cfg.features.as_ref(), + candidate_permission_profile: permission_profile, + credential_broker_base_url: cfg.openai_base_url.as_deref(), + }); build_network_proxy_spec( - configured_network_proxy_config, + prepared_network.configured_proxy, self.config_layer_stack.requirements().network.clone(), permission_profile, &self.permissions.shell_environment_policy.r#set, + &self.features, + cfg.features.as_ref(), ) } + pub(crate) fn credential_masking_warning( + &self, + permission_profile: &PermissionProfile, + ) -> Option<&'static str> { + if !self.features.enabled(Feature::CredentialMasking) { + return None; + } + // The session bypasses the configured proxy for unsandboxed execution, + // even when managed requirements keep the proxy spec enabled. + if matches!(permission_profile, PermissionProfile::Disabled) { + return Some( + "Credential masking is inactive because sandboxing is disabled. Commands will receive unmasked credentials. Select a sandboxed permission profile to use credential masking.", + ); + } + if self + .permissions + .network + .as_ref() + .is_some_and(NetworkProxySpec::credential_broker_enabled) + { + return None; + } + if self + .permissions + .network + .as_ref() + .is_some_and(NetworkProxySpec::enabled) + { + Some( + "Credential masking is inactive because shell environment overrides contain conflicting case-insensitive provider keys. Commands will receive unmasked credentials. Remove the conflicting entries from `shell_environment_policy.set`.", + ) + } else { + Some( + "Credential masking is inactive because the network proxy is disabled. Commands will receive unmasked credentials. Enable `features.network_proxy` or configure managed network requirements to use credential masking.", + ) + } + } + pub fn bundled_skills_enabled(&self) -> bool { codex_config::bundled_skills_enabled_from_stack(&self.config_layer_stack) } diff --git a/codex-rs/core/src/config/network_config.rs b/codex-rs/core/src/config/network_config.rs index 6de02ee17f..505d896abe 100644 --- a/codex-rs/core/src/config/network_config.rs +++ b/codex-rs/core/src/config/network_config.rs @@ -105,9 +105,9 @@ impl PreparedNetworkConfig { { apply_network_proxy_feature_config(&mut configured_proxy, config); } - configured_proxy.set_credential_broker_openai_base_url(credential_broker_base_url); configured_proxy.enabled = true; } + configured_proxy.set_credential_broker_openai_base_url(credential_broker_base_url); Self { configured_proxy } } diff --git a/codex-rs/core/src/config/network_proxy_spec.rs b/codex-rs/core/src/config/network_proxy_spec.rs index 53a61ddd10..ace90ef9c2 100644 --- a/codex-rs/core/src/config/network_proxy_spec.rs +++ b/codex-rs/core/src/config/network_proxy_spec.rs @@ -94,7 +94,7 @@ impl NetworkProxySpec { } pub(crate) fn credential_broker_enabled(&self) -> bool { - self.config.credential_broker && self.constraints.enabled != Some(false) + self.enabled() && self.config.credential_broker && self.constraints.enabled != Some(false) } pub fn proxy_host_and_port(&self) -> String { @@ -378,9 +378,14 @@ impl NetworkProxySpec { &self, executor_os: Platform, ) -> std::io::Result { - build_config_state(self.config.clone(), self.constraints.clone(), executor_os).map_err( - |err| std::io::Error::other(format!("failed to build network proxy state: {err}")), - ) + let mut config = self.config.clone(); + if !self.enabled() { + // Retained managed constraints must not let brokerage activate a disabled proxy. + config.set_credential_broker_enabled(/*enabled*/ false); + } + build_config_state(config, self.constraints.clone(), executor_os).map_err(|err| { + std::io::Error::other(format!("failed to build network proxy state: {err}")) + }) } fn apply_requirements( diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 240877c2ba..b9e96e3c8a 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -1946,7 +1946,14 @@ impl Session { ) -> ConstraintResult> { let notify_config_contributors = !self.services.extensions.config_contributors().is_empty(); let windows_sandbox_changed; - let (commit, previous_config, new_config, permission_profile_changed, mcp_inputs_changed) = { + let ( + commit, + previous_config, + new_config, + permission_profile_changed, + mcp_inputs_changed, + credential_masking_warning, + ) = { let mut state = self.state.lock().await; let updated = match self.apply_session_settings(&state.session_configuration, &updates) { @@ -1961,6 +1968,18 @@ impl Session { return Ok(None); } + let credential_masking_warning = updated + .original_config_do_not_use + .credential_masking_warning(&updated.permission_profile()) + .filter(|warning| { + Some(*warning) + != state + .session_configuration + .original_config_do_not_use + .credential_masking_warning( + &state.session_configuration.permission_profile(), + ) + }); let previous_config = notify_config_contributors .then(|| self.build_effective_session_config(&state.session_configuration)); let previous_permission_profile = state.session_configuration.permission_profile(); @@ -2003,6 +2022,7 @@ impl Session { new_config, permission_profile_changed, mcp_inputs_changed, + credential_masking_warning, ) }; if windows_sandbox_changed { @@ -2017,6 +2037,16 @@ impl Session { self.refresh_managed_network_proxy_for_current_permission_profile() .await; } + if let Some(message) = credential_masking_warning { + warn!("{message}"); + self.send_event_raw(Event { + id: INITIAL_SUBMIT_ID.to_owned(), + msg: EventMsg::Warning(WarningEvent { + message: message.to_string(), + }), + }) + .await; + } if mcp_inputs_changed { self.schedule_mcp_prewarm(); } diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 08743217ed..928c976edb 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -1399,7 +1399,8 @@ impl Session { .user_shell_override .clone() .unwrap_or_else(shell::default_user_shell); - let credential_broker_available = config.features.enabled(Feature::NetworkProxy) + let credential_broker_available = (config.features.enabled(Feature::NetworkProxy) + || config.features.enabled(Feature::CredentialMasking)) && config .config_layer_stack .requirements() @@ -1407,12 +1408,13 @@ impl Session { .as_ref() .is_none_or(|network| network.value.enabled != Some(false)); let credential_broker_configured = credential_broker_available - && effective_config - .get("features") - .and_then(|features| features.get("network_proxy")) - .and_then(|network_proxy| network_proxy.get("credential_broker")) - .and_then(TomlValue::as_bool) - .unwrap_or(false); + && (config.features.enabled(Feature::CredentialMasking) + || effective_config + .get("features") + .and_then(|features| features.get("network_proxy")) + .and_then(|network_proxy| network_proxy.get("credential_broker")) + .and_then(TomlValue::as_bool) + .unwrap_or(false)); let credential_broker_active = credential_broker_configured && config .permissions diff --git a/codex-rs/core/src/shell_snapshot_tests.rs b/codex-rs/core/src/shell_snapshot_tests.rs index 44241ae045..fc191bf590 100644 --- a/codex-rs/core/src/shell_snapshot_tests.rs +++ b/codex-rs/core/src/shell_snapshot_tests.rs @@ -669,8 +669,12 @@ async fn inactive_profiles_keep_snapshots_but_active_brokers_require_sandbox() - #[cfg(unix)] async fn credential_snapshot_proxy() -> Result { - let mut network_config = NetworkProxyConfig::default(); + let mut network_config = NetworkProxyConfig { + enabled: true, + ..NetworkProxyConfig::default() + }; network_config.set_credential_broker_enabled(/*enabled*/ true); + network_config.set_allowed_domains(vec!["*".to_string()]); network_config.allow_local_binding = Some(true); network_config.credential_providers.insert( "local".to_string(), diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index 074adbbd03..cd26e963c7 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -2,6 +2,8 @@ use anyhow::Context; use anyhow::Result; +#[cfg(unix)] +use codex_config::test_support::CloudConfigBundleFixture; use codex_config::types::ApprovalsReviewer; use codex_core::CodexThread; use codex_core::TurnInputRequest; @@ -17,6 +19,8 @@ use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::EnvironmentVariablePattern; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; +#[cfg(unix)] +use codex_protocol::models::ActivePermissionProfile; use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE; use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemAccessMode; @@ -3115,12 +3119,23 @@ ZDOTDIR = "{}" #[test_case("failed")] #[test_case("heredoc")] #[test_case("enabled")] +#[test_case("flat_feature")] +#[test_case("flat_profile_transition")] +#[test_case("flat_yolo")] +#[test_case("flat_yolo_profile_transition")] +#[test_case("flat_yolo_unnamed_transition")] +#[test_case("flat_yolo_legacy_transition")] +#[test_case("flat_managed")] +#[test_case("flat_no_proxy")] +#[test_case("flat_disabled_proxy")] +#[test_case("flat_policy_without_proxy")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn brokered_shell_snapshot_fallback(mode: &'static str) -> Result<()> { skip_if_no_network!(Ok(())); const REAL_GITHUB_TOKEN: &str = "ghp_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + const REAL_CUSTOM_TOKEN: &str = "custom_abcdefghijklmnopqrstuvwx"; let Some(bash) = codex_core::shell::get_shell(codex_core::shell::ShellType::Bash) else { return Ok(()); @@ -3139,17 +3154,58 @@ async fn brokered_shell_snapshot_fallback(mode: &'static str) -> Result<()> { EOF\n}\nexport -f brokered_heredoc\n" .to_string() } else { - "shopt -s extglob expand_aliases\nexport CORP_REGION=west\ncorp_auth() { case \"$CORP_REGION\" in @(west|east)) printf '%s\\n' \"$CORP_REGION\" \"$GH_TOKEN\" ;; esac; }\nalias corp_login=corp_auth\n".to_string() + let mut startup = "shopt -s extglob expand_aliases\nexport CORP_REGION=west\ncorp_auth() { case \"$CORP_REGION\" in @(west|east)) printf '%s\\n' \"$CORP_REGION\" \"$GH_TOKEN\" ;; esac; }\nalias corp_login=corp_auth\n".to_string(); + if mode == "flat_managed" { + startup.push_str(&format!("export CUSTOM_API_KEY='{REAL_CUSTOM_TOKEN}'\n")); + } + startup }, )?; + let features = match mode { + "flat_feature" + | "flat_profile_transition" + | "flat_disabled_proxy" + | "flat_yolo" + | "flat_yolo_profile_transition" + | "flat_yolo_unnamed_transition" + | "flat_yolo_legacy_transition" => "credential_masking = true, network_proxy = true", + "flat_managed" => concat!( + "credential_masking = true, network_proxy = { enabled = false, ", + "credentials = { custom = { env = [\"CUSTOM_API_KEY\"], ", + "patterns = [\"custom_[a-z]{24}\"], ", + "url_prefixes = [\"https://custom.example.test\"] } } }", + ), + "flat_no_proxy" | "flat_policy_without_proxy" => "credential_masking = true", + _ => "network_proxy = { enabled = true, credential_broker = true }", + }; + let network_enabled = !matches!(mode, "flat_no_proxy" | "flat_policy_without_proxy"); + let cloud_config_bundle = match mode { + "flat_no_proxy" | "flat_profile_transition" => { + CloudConfigBundleFixture::loader_with_enterprise_requirement("") + } + "flat_disabled_proxy" => CloudConfigBundleFixture::loader_with_enterprise_requirement( + "[experimental_network]\nenabled = false", + ), + "flat_policy_without_proxy" => { + CloudConfigBundleFixture::loader_with_enterprise_requirement( + "[experimental_network]\nallow_local_binding = true", + ) + } + _ => managed_network_requirements_loader(), + }; let home = Arc::new(TempDir::new()?); + let default_permissions = if mode == "flat_yolo" { + ":danger-full-access" + } else { + "brokered" + }; fs::write( home.path().join("config.toml"), format!( - r#"default_permissions = "brokered" -features = {{ network_proxy = {{ enabled = true, credential_broker = true }} }} -permissions = {{ brokered = {{ extends = ":workspace", network = {{ enabled = true, allow_local_binding = true }} }} }} + r#"default_permissions = "{default_permissions}" +features = {{ {features} }} +permissions = {{ brokered = {{ extends = ":workspace", network = {{ enabled = {network_enabled}, allow_local_binding = true }} }} }} [shell_environment_policy.set] BASH_ENV = "{}" @@ -3179,9 +3235,15 @@ GH_TOKEN = "{REAL_GITHUB_TOKEN}" let mut builder = test_codex() .with_home(home) .with_user_shell(bash) - .with_cloud_config_bundle(managed_network_requirements_loader()) + .with_cloud_config_bundle(cloud_config_bundle) .with_config(move |config| { config.permissions.approval_policy = Constrained::allow_any(approval_policy); + if mode == "flat_managed" { + config + .features + .enable(Feature::ShellSnapshotV2) + .expect("test config should allow executor-owned shell snapshots"); + } if mode == "disabled" { config.features.disable(Feature::ShellSnapshot) } else { @@ -3190,6 +3252,80 @@ GH_TOKEN = "{REAL_GITHUB_TOKEN}" .expect("test config should allow ShellSnapshot override"); }); let test = builder.build(&server).await?; + if mode.starts_with("flat_yolo") { + assert!( + test.config.permissions.network.is_some(), + "managed requirements retain the configured proxy in YOLO mode" + ); + assert_eq!( + test.config + .startup_warnings + .iter() + .any(|warning| warning.starts_with("Credential masking is inactive")), + mode == "flat_yolo", + ); + if mode != "flat_yolo" { + test.codex + .update_thread_settings(ThreadSettingsOverrides { + permission_profile: (mode != "flat_yolo_legacy_transition") + .then_some(PermissionProfile::Disabled), + active_permission_profile: (mode == "flat_yolo_profile_transition").then_some( + ActivePermissionProfile { + id: ":danger-full-access".to_string(), + extends: None, + }, + ), + sandbox_policy: (mode == "flat_yolo_legacy_transition") + .then_some(SandboxPolicy::DangerFullAccess), + ..Default::default() + }) + .await?; + } + let warning = wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::Warning(warning) if warning.message.starts_with("Credential masking is inactive")) + }) + .await; + let EventMsg::Warning(warning) = warning else { + panic!("expected credential masking warning"); + }; + insta::allow_duplicates! { + insta::assert_snapshot!(warning.message, @r"Credential masking is inactive because sandboxing is disabled. Commands will receive unmasked credentials. Select a sandboxed permission profile to use credential masking."); + } + } + if mode == "flat_profile_transition" { + assert!( + !test + .config + .startup_warnings + .iter() + .any(|warning| warning.starts_with("Credential masking is inactive")) + ); + test.codex + .update_thread_settings(ThreadSettingsOverrides { + permission_profile: Some(PermissionProfile::read_only()), + active_permission_profile: Some(ActivePermissionProfile::read_only()), + ..Default::default() + }) + .await?; + } + if matches!( + mode, + "flat_no_proxy" + | "flat_disabled_proxy" + | "flat_policy_without_proxy" + | "flat_profile_transition" + ) { + let warning = wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::Warning(warning) if warning.message.starts_with("Credential masking is inactive")) + }) + .await; + let EventMsg::Warning(warning) = warning else { + panic!("expected credential masking warning"); + }; + insta::allow_duplicates! { + insta::assert_snapshot!(warning.message, @r"Credential masking is inactive because the network proxy is disabled. Commands will receive unmasked credentials. Enable `features.network_proxy` or configure managed network requirements to use credential masking."); + } + } let call_id = "escalated-brokered-shell-startup"; let command = if mode == "heredoc" { @@ -3198,7 +3334,9 @@ GH_TOKEN = "{REAL_GITHUB_TOKEN}" format!( "cat {snapshot_dir}/*.sh > captured-snapshot && brokered_heredoc && printf '%s\\n' \"$GH_TOKEN\"" ) - } else if mode == "enabled" { + } else if mode == "flat_managed" { + "corp_login && printf '%s\\n' \"$CUSTOM_API_KEY\"".to_string() + } else if matches!(mode, "enabled" | "flat_feature") { "corp_login".to_string() } else { "printenv GH_TOKEN".to_string() @@ -3327,13 +3465,18 @@ GH_TOKEN = "{REAL_GITHUB_TOKEN}" assert!(snapshot.contains("Authorization: ${GH_TOKEN}")); return Ok(()); } - if mode == "enabled" { + if matches!(mode, "enabled" | "flat_feature" | "flat_managed") { assert_eq!(result.exit_code, Some(0), "command failed: {result:?}"); let values = result.stdout.lines().collect::>(); - assert_eq!(values.len(), 2); + assert_eq!(values.len(), if mode == "flat_managed" { 3 } else { 2 }); assert_eq!(values[0], "west"); assert!(values[1].starts_with("ghp_")); assert_ne!(values[1], REAL_GITHUB_TOKEN); + if mode == "flat_managed" { + assert!(values[2].starts_with("custom_")); + assert_eq!(values[2].len(), REAL_CUSTOM_TOKEN.len()); + assert_ne!(values[2], REAL_CUSTOM_TOKEN); + } return Ok(()); } assert_eq!( diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index 4e6a85d206..89c19fc6f8 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -209,6 +209,8 @@ pub enum Feature { UnboundedConnectionRetries, /// Start the managed network proxy for sandboxed sessions. NetworkProxy, + /// Mask credentials and inject them through an already enabled network proxy. + CredentialMasking, /// Enable managed worktree creation and repository-aware sessions. Worktrees, /// Respect host system proxy settings for Codex-owned network clients. @@ -1414,6 +1416,12 @@ pub const FEATURES: &[FeatureSpec] = &[ }, default_enabled: false, }, + FeatureSpec { + id: Feature::CredentialMasking, + key: "credential_masking", + stage: Stage::UnderDevelopment, + default_enabled: false, + }, FeatureSpec { id: Feature::Worktrees, key: "worktrees",