diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index 00af972336..3c14d29c11 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -359,6 +359,7 @@ pub struct NetworkRequirementsToml { pub allow_upstream_proxy: Option, pub dangerously_allow_non_loopback_proxy: Option, pub dangerously_allow_all_unix_sockets: Option, + pub dangerously_allow_plaintext_credential_injection: Option, pub domains: Option, /// When true, only managed `allowed_domains` are respected while managed /// network enforcement is active. User allowlist entries are ignored. @@ -375,6 +376,7 @@ struct RawNetworkRequirementsToml { allow_upstream_proxy: Option, dangerously_allow_non_loopback_proxy: Option, dangerously_allow_all_unix_sockets: Option, + dangerously_allow_plaintext_credential_injection: Option, domains: Option, #[serde(default)] allowed_domains: Option>, @@ -402,6 +404,7 @@ impl<'de> Deserialize<'de> for NetworkRequirementsToml { allow_upstream_proxy, dangerously_allow_non_loopback_proxy, dangerously_allow_all_unix_sockets, + dangerously_allow_plaintext_credential_injection, domains, allowed_domains, managed_allowed_domains_only, @@ -430,6 +433,7 @@ impl<'de> Deserialize<'de> for NetworkRequirementsToml { allow_upstream_proxy, dangerously_allow_non_loopback_proxy, dangerously_allow_all_unix_sockets, + dangerously_allow_plaintext_credential_injection, domains: domains .or_else(|| legacy_domain_permissions_from_lists(allowed_domains, denied_domains)), managed_allowed_domains_only, @@ -481,6 +485,7 @@ pub struct NetworkConstraints { pub allow_upstream_proxy: Option, pub dangerously_allow_non_loopback_proxy: Option, pub dangerously_allow_all_unix_sockets: Option, + pub dangerously_allow_plaintext_credential_injection: Option, pub domains: Option, /// When true, only managed `allowed_domains` are respected while managed /// network enforcement is active. User allowlist entries are ignored. @@ -508,6 +513,7 @@ impl From for NetworkConstraints { allow_upstream_proxy, dangerously_allow_non_loopback_proxy, dangerously_allow_all_unix_sockets, + dangerously_allow_plaintext_credential_injection, domains, managed_allowed_domains_only, unix_sockets, @@ -520,6 +526,7 @@ impl From for NetworkConstraints { allow_upstream_proxy, dangerously_allow_non_loopback_proxy, dangerously_allow_all_unix_sockets, + dangerously_allow_plaintext_credential_injection, domains, managed_allowed_domains_only, unix_sockets, @@ -3324,6 +3331,7 @@ command = "python3 /enterprise/hooks/pre.py" enabled = true allow_upstream_proxy = false dangerously_allow_all_unix_sockets = true + dangerously_allow_plaintext_credential_injection = false managed_allowed_domains_only = true allow_local_binding = false @@ -3353,6 +3361,12 @@ command = "python3 /enterprise/hooks/pre.py" sourced_network.value.dangerously_allow_all_unix_sockets, Some(true) ); + assert_eq!( + sourced_network + .value + .dangerously_allow_plaintext_credential_injection, + Some(false) + ); assert_eq!( sourced_network.value.domains.as_ref(), Some(&NetworkDomainPermissionsToml { diff --git a/codex-rs/core/src/config/network_proxy_spec.rs b/codex-rs/core/src/config/network_proxy_spec.rs index 653e6987f3..07b45b5000 100644 --- a/codex-rs/core/src/config/network_proxy_spec.rs +++ b/codex-rs/core/src/config/network_proxy_spec.rs @@ -249,6 +249,16 @@ impl NetworkProxySpec { constraints.dangerously_allow_all_unix_sockets = Some(dangerously_allow_all_unix_sockets); } + if let Some(dangerously_allow_plaintext_credential_injection) = + requirements.dangerously_allow_plaintext_credential_injection + { + config + .network + .dangerously_allow_plaintext_credential_injection = + dangerously_allow_plaintext_credential_injection; + constraints.dangerously_allow_plaintext_credential_injection = + Some(dangerously_allow_plaintext_credential_injection); + } let managed_allowed_domains = if hard_deny_allowlist_misses { Some( requirements diff --git a/codex-rs/core/src/config/network_proxy_spec_tests.rs b/codex-rs/core/src/config/network_proxy_spec_tests.rs index 6dfb1e3a25..a12f573f23 100644 --- a/codex-rs/core/src/config/network_proxy_spec_tests.rs +++ b/codex-rs/core/src/config/network_proxy_spec_tests.rs @@ -40,6 +40,48 @@ fn build_state_with_audit_metadata_threads_metadata_to_state() { assert_eq!(state.audit_metadata(), &metadata); } +#[test] +fn managed_requirement_disables_plaintext_credential_injection() { + let mut config = NetworkProxyConfig::default(); + config + .network + .dangerously_allow_plaintext_credential_injection = true; + let requirements = NetworkConstraints { + dangerously_allow_plaintext_credential_injection: Some(false), + ..Default::default() + }; + + let spec = NetworkProxySpec::from_config_and_constraints( + config, + Some(requirements), + &PermissionProfile::workspace_write(), + ) + .expect("managed requirement should override the user setting"); + + assert!( + !spec + .config + .network + .dangerously_allow_plaintext_credential_injection + ); + assert_eq!( + spec.constraints + .dangerously_allow_plaintext_credential_injection, + Some(false) + ); + + let mut candidate = spec.config.clone(); + candidate + .network + .dangerously_allow_plaintext_credential_injection = true; + let err = validate_policy_against_constraints(&candidate, &spec.constraints) + .expect_err("managed constraint should reject re-enabling plaintext injection"); + assert_eq!( + err.to_string(), + "invalid value for network.dangerously_allow_plaintext_credential_injection: true (allowed false (disabled by managed config))" + ); +} + #[test] fn requirements_allowed_domains_are_a_baseline_for_user_allowlist() { let mut config = NetworkProxyConfig::default(); diff --git a/codex-rs/core/src/network_proxy_loader.rs b/codex-rs/core/src/network_proxy_loader.rs index 912073b1b4..ef6992da09 100644 --- a/codex-rs/core/src/network_proxy_loader.rs +++ b/codex-rs/core/src/network_proxy_loader.rs @@ -159,6 +159,14 @@ fn apply_network_constraints(network: NetworkToml, constraints: &mut NetworkProx if let Some(dangerously_allow_all_unix_sockets) = network.dangerously_allow_all_unix_sockets { constraints.dangerously_allow_all_unix_sockets = Some(dangerously_allow_all_unix_sockets); } + if let Some(dangerously_allow_plaintext_credential_injection) = network + .mitm + .as_ref() + .and_then(|mitm| mitm.dangerously_allow_plaintext_credential_injection) + { + constraints.dangerously_allow_plaintext_credential_injection = + Some(dangerously_allow_plaintext_credential_injection); + } if let Some(domains) = network.domains.as_ref() { let mut config = NetworkProxyConfig::default(); if let Some(allowed_domains) = constraints.allowed_domains.take() { diff --git a/codex-rs/core/src/network_proxy_loader_tests.rs b/codex-rs/core/src/network_proxy_loader_tests.rs index 879d826920..4b9d865447 100644 --- a/codex-rs/core/src/network_proxy_loader_tests.rs +++ b/codex-rs/core/src/network_proxy_loader_tests.rs @@ -325,6 +325,32 @@ dangerously_allow_all_unix_sockets = true assert_eq!(constraints.dangerously_allow_all_unix_sockets, Some(true)); } +#[test] +fn apply_network_constraints_includes_plaintext_credential_injection_flag() { + let config: toml::Value = toml::from_str( + r#" +default_permissions = "dev" + +[permissions.dev.network.mitm] +dangerously_allow_plaintext_credential_injection = false +"#, + ) + .expect("permissions profile should parse"); + let network = selected_network_from_tables( + network_tables_from_toml(&config).expect("permissions profile should deserialize"), + ) + .expect("permissions profile should select a network table") + .expect("network table should be present"); + + let mut constraints = NetworkProxyConstraints::default(); + apply_network_constraints(network, &mut constraints); + + assert_eq!( + constraints.dangerously_allow_plaintext_credential_injection, + Some(false) + ); +} + #[test] fn selected_network_from_tables_ignores_builtin_profile_without_permissions_table() { let config: toml::Value = toml::from_str( diff --git a/codex-rs/core/tests/suite/credential_broker.rs b/codex-rs/core/tests/suite/credential_broker.rs new file mode 100644 index 0000000000..3830545dc1 --- /dev/null +++ b/codex-rs/core/tests/suite/credential_broker.rs @@ -0,0 +1,145 @@ +use anyhow::Result; +use codex_features::Feature; +use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::Op; +use codex_protocol::user_input::UserInput; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_function_call; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::mount_sse_sequence; +use core_test_support::responses::sse; +use core_test_support::skip_if_sandbox; +use core_test_support::skip_if_target_windows; +use core_test_support::test_codex::TestCodexHarness; +use core_test_support::test_codex::local_selections; +use core_test_support::test_codex::test_codex; +use core_test_support::test_codex::turn_permission_fields; +use core_test_support::wait_for_event; +use core_test_support::wait_for_event_match; +use pretty_assertions::assert_eq; +use serde_json::json; +use std::collections::HashMap; +use std::fs; +use std::sync::Arc; +use tempfile::TempDir; + +const REAL_OPENAI_API_KEY: &str = + "sk-proj-abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_"; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn configured_credential_broker_virtualizes_child_environment() -> Result<()> { + skip_if_target_windows!(Ok(()), "uses a POSIX shell command"); + skip_if_sandbox!(Ok(())); + + let home = Arc::new(TempDir::new()?); + fs::write( + home.path().join("config.toml"), + r#" +default_permissions = "workspace" + +[features.network_proxy] +credential_broker = true + +[permissions.workspace.filesystem] +":minimal" = "read" + +[permissions.workspace.network] +enabled = true +mode = "full" +"#, + )?; + let builder = test_codex().with_home(home).with_config(|config| { + config.use_experimental_unified_exec_tool = true; + config + .features + .enable(Feature::UnifiedExec) + .expect("test config should allow feature update"); + config.permissions.shell_environment_policy.r#set = HashMap::from([( + "OPENAI_API_KEY".to_string(), + REAL_OPENAI_API_KEY.to_string(), + )]); + }); + let harness = TestCodexHarness::with_auto_env_builder(builder).await?; + let test = harness.test(); + + assert!(test.config.features.enabled(Feature::NetworkProxy)); + assert!( + test.config.permissions.network.is_some(), + "credential broker config should start the managed network proxy" + ); + + let call_id = "credential-broker-child-env"; + let args = json!({ + "cmd": "printf '%s\\n%s' \"$OPENAI_API_KEY\" \"$CODEX_NETWORK_PROXY_CREDENTIAL_BROKER_ACTIVE\"", + "yield_time_ms": 1_000, + }); + let responses = vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call(call_id, "exec_command", &serde_json::to_string(&args)?), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ]; + mount_sse_sequence(harness.server(), responses).await; + + let session_model = test.session_configured.model.clone(); + let cwd = test.config.cwd.clone(); + let (sandbox_policy, permission_profile) = turn_permission_fields( + test.config.permissions.permission_profile().clone(), + cwd.as_path(), + ); + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "show the brokered child credential".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: codex_protocol::protocol::ThreadSettingsOverrides { + environments: Some(local_selections(cwd)), + approval_policy: Some(AskForApproval::Never), + sandbox_policy: Some(sandbox_policy), + permission_profile, + collaboration_mode: Some(codex_protocol::config_types::CollaborationMode { + mode: codex_protocol::config_types::ModeKind::Default, + settings: codex_protocol::config_types::Settings { + model: session_model, + reasoning_effort: None, + developer_instructions: None, + }, + }), + ..Default::default() + }, + }) + .await?; + + let end = wait_for_event_match(&test.codex, |event| match event { + EventMsg::ExecCommandEnd(event) if event.call_id == call_id => Some(event.clone()), + _ => None, + }) + .await; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + assert_eq!(end.exit_code, 0); + let output = end.aggregated_output.trim().lines().collect::>(); + assert_eq!(output.len(), 2, "unexpected command output: {output:?}"); + let brokered_openai_api_key = output[0]; + assert_ne!(brokered_openai_api_key, REAL_OPENAI_API_KEY); + assert!(brokered_openai_api_key.starts_with("sk-proj-")); + assert_eq!(brokered_openai_api_key.len(), REAL_OPENAI_API_KEY.len()); + assert_eq!(output[1], "1"); + + Ok(()) +} diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 3e27463370..518883f44b 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -48,6 +48,7 @@ mod compact; mod compact_remote; mod compact_remote_parity; mod compact_resume_fork; +mod credential_broker; mod current_time_reminder; mod deprecation_notice; mod exec; diff --git a/codex-rs/network-proxy/src/runtime.rs b/codex-rs/network-proxy/src/runtime.rs index a51f1d8f16..13a2e7cf25 100644 --- a/codex-rs/network-proxy/src/runtime.rs +++ b/codex-rs/network-proxy/src/runtime.rs @@ -1883,6 +1883,44 @@ mod tests { assert!(validate_policy_against_constraints(&config, &constraints).is_err()); } + #[test] + fn validate_policy_against_constraints_disallows_plaintext_credential_injection_without_managed_opt_in() + { + let constraints = NetworkProxyConstraints { + dangerously_allow_plaintext_credential_injection: Some(false), + ..NetworkProxyConstraints::default() + }; + + let config = NetworkProxyConfig { + network: NetworkProxySettings { + enabled: true, + dangerously_allow_plaintext_credential_injection: true, + ..NetworkProxySettings::default() + }, + }; + + assert!(validate_policy_against_constraints(&config, &constraints).is_err()); + } + + #[test] + fn validate_policy_against_constraints_allows_plaintext_credential_injection_with_managed_opt_in() + { + let constraints = NetworkProxyConstraints { + dangerously_allow_plaintext_credential_injection: Some(true), + ..NetworkProxyConstraints::default() + }; + + let config = NetworkProxyConfig { + network: NetworkProxySettings { + enabled: true, + dangerously_allow_plaintext_credential_injection: true, + ..NetworkProxySettings::default() + }, + }; + + assert!(validate_policy_against_constraints(&config, &constraints).is_ok()); + } + #[test] fn validate_policy_against_constraints_disallows_allow_all_unix_sockets_when_allowlist_is_managed() { diff --git a/codex-rs/network-proxy/src/state.rs b/codex-rs/network-proxy/src/state.rs index 9a5287dac4..0a95e07a09 100644 --- a/codex-rs/network-proxy/src/state.rs +++ b/codex-rs/network-proxy/src/state.rs @@ -30,6 +30,7 @@ pub struct NetworkProxyConstraints { pub allow_upstream_proxy: Option, pub dangerously_allow_non_loopback_proxy: Option, pub dangerously_allow_all_unix_sockets: Option, + pub dangerously_allow_plaintext_credential_injection: Option, pub allowed_domains: Option>, pub allowlist_expansion_enabled: Option, pub denied_domains: Option>, @@ -216,6 +217,28 @@ pub fn validate_policy_against_constraints( }, )?; + let allow_plaintext_credential_injection = + constraints.dangerously_allow_plaintext_credential_injection; + validate( + config + .network + .dangerously_allow_plaintext_credential_injection, + move |candidate| match allow_plaintext_credential_injection { + Some(true) | None => Ok(()), + Some(false) => { + if *candidate { + Err(invalid_value( + "network.dangerously_allow_plaintext_credential_injection", + "true", + "false (disabled by managed config)", + )) + } else { + Ok(()) + } + } + }, + )?; + if let Some(allow_local_binding) = constraints.allow_local_binding { validate(config.network.allow_local_binding, move |candidate| { if *candidate && !allow_local_binding { diff --git a/codex-rs/tui/src/debug_config.rs b/codex-rs/tui/src/debug_config.rs index c02d4c8261..acb33e9c53 100644 --- a/codex-rs/tui/src/debug_config.rs +++ b/codex-rs/tui/src/debug_config.rs @@ -468,6 +468,7 @@ fn format_network_constraints(network: &NetworkConstraints) -> String { allow_upstream_proxy, dangerously_allow_non_loopback_proxy, dangerously_allow_all_unix_sockets, + dangerously_allow_plaintext_credential_injection, domains, managed_allowed_domains_only, unix_sockets, @@ -496,6 +497,13 @@ fn format_network_constraints(network: &NetworkConstraints) -> String { "dangerously_allow_all_unix_sockets={dangerously_allow_all_unix_sockets}" )); } + if let Some(dangerously_allow_plaintext_credential_injection) = + dangerously_allow_plaintext_credential_injection + { + parts.push(format!( + "dangerously_allow_plaintext_credential_injection={dangerously_allow_plaintext_credential_injection}" + )); + } if let Some(domains) = domains { parts.push(format!( "domains={}",