From f45115a13746bca56274551136cb9bc6dbcbf203 Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Wed, 9 Sep 2026 05:34:51 +0000 Subject: [PATCH] Preserve credential broker destinations across environment filtering (#44068) ## Why Filtering destination variables out of a child environment can undo credential registrations. Rotating a provider token and destination must also leave older credential aliases bound to their original destinations. ## What changed - Add `CredentialBrokerContext` to retain local destination hints for built-in and configured providers without adding them to child environments or serialized configuration. Explicit environment values, including empty values, override these fallbacks. - Preserve registered destinations when hints are absent, reconcile fallback changes, and keep credential aliases scoped to their source identities and environments during rotation and inheritance. - Expose provider context and source matching for trusted captured text, including configured provider context keys. - Disable brokerage on Windows when provider environment overrides contain conflicting case-insensitive keys. ## Testing Add regression coverage for filtered destinations, token rotation, inherited aliases, fallback updates and clearing, context redaction, trusted text matching, and non-Unicode environment values. GitOrigin-RevId: 411790ea6339e1d8fa568049cf8160a67b134666 --- codex-rs/network-proxy/src/config.rs | 82 ++++ .../network-proxy/src/credential_broker.rs | 354 ++++++++++++++--- .../src/credential_broker/configured_tests.rs | 373 +++++++++++++++++- .../src/credential_broker/environment.rs | 53 +++ .../src/credential_broker/matching.rs | 3 +- .../src/credential_broker/registry.rs | 76 +++- .../src/credential_broker_tests.rs | 269 +++++++++++++ codex-rs/network-proxy/src/lib.rs | 1 + codex-rs/network-proxy/src/proxy.rs | 25 ++ codex-rs/network-proxy/src/runtime.rs | 18 + 10 files changed, 1188 insertions(+), 66 deletions(-) diff --git a/codex-rs/network-proxy/src/config.rs b/codex-rs/network-proxy/src/config.rs index a3c48b63eb..4c203eb826 100644 --- a/codex-rs/network-proxy/src/config.rs +++ b/codex-rs/network-proxy/src/config.rs @@ -7,6 +7,7 @@ use serde::Deserializer; use serde::Serialize; use serde::Serializer; use std::collections::BTreeMap; +use std::collections::HashMap; use std::net::IpAddr; use std::net::SocketAddr; use std::path::Path; @@ -143,6 +144,9 @@ pub struct NetworkProxyConfig { /// Trusted OpenAI endpoint derived from local configuration, never sent to remote executors. #[serde(skip)] pub credential_broker_openai_host: Option, + /// Trusted local destination context, never sent to remote executors or child environments. + #[serde(skip)] + pub credential_broker_context: crate::CredentialBrokerContext, #[serde(default)] pub dangerously_allow_plaintext_credential_injection: bool, #[serde(default)] @@ -168,6 +172,7 @@ impl Default for NetworkProxyConfig { credential_broker: false, credential_providers: BTreeMap::new(), credential_broker_openai_host: None, + credential_broker_context: crate::CredentialBrokerContext::default(), dangerously_allow_plaintext_credential_injection: false, mitm_hooks: Vec::new(), } @@ -184,6 +189,52 @@ impl NetworkProxyConfig { self.credential_broker_openai_host = base_url.and_then(trusted_credential_broker_host); } + /// Retains trusted destination context without changing child environment policy. Conflicting + /// case-insensitive provider overrides disable brokerage on Windows. + pub fn configure_credential_broker_environment( + &mut self, + environment: &HashMap, + ) { + if cfg!(windows) + && self.credential_broker + && self.has_ambiguous_windows_credential_environment(environment) + { + warn!( + "credential brokerage disabled because shell environment overrides contain \ + conflicting case-insensitive provider keys" + ); + self.set_credential_broker_enabled(/*enabled*/ false); + } + self.credential_broker_context = if self.credential_broker { + crate::CredentialBrokerContext::capture(self, environment) + } else { + crate::CredentialBrokerContext::default() + }; + } + + fn has_ambiguous_windows_credential_environment( + &self, + environment: &HashMap, + ) -> bool { + environment.iter().any(|(key, value)| { + let is_provider_key = + crate::credential_broker::is_credential_broker_provider_env_key(key) + || self.credential_providers.values().any(|provider| { + provider + .env + .iter() + .chain(provider.url_prefix_from_env.iter()) + .any(|candidate| key.eq_ignore_ascii_case(candidate)) + }); + is_provider_key + && environment.iter().any(|(candidate, candidate_value)| { + key != candidate + && key.eq_ignore_ascii_case(candidate) + && value != candidate_value + }) + }) + } + pub fn allowed_domains(&self) -> Option> { self.domain_entries(NetworkDomainPermission::Allow) } @@ -642,12 +693,43 @@ mod tests { credential_broker: false, credential_providers: BTreeMap::new(), credential_broker_openai_host: None, + credential_broker_context: crate::CredentialBrokerContext::default(), dangerously_allow_plaintext_credential_injection: false, mitm_hooks: Vec::new(), } ); } + #[test] + #[cfg(unix)] + fn credential_broker_context_accepts_non_unicode_environment() { + use std::ffi::OsString; + use std::os::unix::ffi::OsStringExt; + use std::process::Command; + + const CHILD_ENV: &str = "CODEX_TEST_NON_UNICODE_BROKER_CONTEXT"; + if std::env::var_os(CHILD_ENV).is_none() { + let output = Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "config::tests::credential_broker_context_accepts_non_unicode_environment", + "--nocapture", + ]) + .env(CHILD_ENV, OsString::from_vec(vec![0xff])) + .env(OsString::from_vec(vec![0xfe]), "unrelated") + .env("GH_HOST", OsString::from_vec(vec![0xff])) + .output() + .unwrap(); + assert!(output.status.success(), "{output:?}"); + return; + } + + let mut config = NetworkProxyConfig::default(); + config.set_credential_broker_enabled(/*enabled*/ true); + config.configure_credential_broker_environment(&HashMap::new()); + assert!(config.credential_broker); + } + #[test] fn credential_broker_only_accepts_trusted_https_openai_endpoints() { let mut config = NetworkProxyConfig::default(); diff --git a/codex-rs/network-proxy/src/credential_broker.rs b/codex-rs/network-proxy/src/credential_broker.rs index 4ed60bdf55..5b598c34d8 100644 --- a/codex-rs/network-proxy/src/credential_broker.rs +++ b/codex-rs/network-proxy/src/credential_broker.rs @@ -16,14 +16,17 @@ use registry::BrokeredCredentialProvider; use registry::active_credential_sources; use registry::is_builtin_shaped_credential; use registry::prioritized_credentials; +use registry::registered_credential_source; use registry::select_credentials; use replacement::Replacements; use std::collections::BTreeMap; use std::collections::HashMap; +use std::collections::HashSet; use std::sync::Arc; use std::sync::RwLock; use url::Url; +pub use environment::CredentialBrokerContext; pub use environment::CredentialBrokerEnvironment; pub use environment::brokered_credential_binding_env_keys; pub use environment::brokered_credential_dummy_env_keys; @@ -53,6 +56,7 @@ struct CredentialBrokerState { enabled: bool, allow_local_binding: bool, openai_api_host: Option, + context: CredentialBrokerContext, configured_provider_configs: BTreeMap, configured_providers: Vec>, credentials: Vec, @@ -70,16 +74,45 @@ struct CredentialRecord { provider: BrokeredCredentialProvider, host_binding: providers::CredentialHostBinding, additional_host_bindings: Vec, + fallback_host_bindings: Vec, environment_id: Option, real_value: String, dummy_value: String, + // Canonical identities accompanying discovery, distinct from an alias's own credential. + source_values: Vec, generated_aliases: Arc>>, } impl CredentialRecord { - fn should_reconcile_host_binding(&self, env: &HashMap) -> bool { + fn has_carried_identity( + &self, + env: &HashMap, + credentials: &[CredentialRecord], + sources: &[ActiveCredentialSource], + ) -> bool { + let mut source_present = false; + let source_matches = sources + .iter() + .filter(|source| source_accepts_credential(source, self)) + .flat_map(|source| &source.env_vars) + .filter_map(|key| env_value(env, key)) + .any(|value| { + source_present = true; + let value = match &self.provider { + BrokeredCredentialProvider::Builtin(_) => value.trim(), + BrokeredCredentialProvider::Configured(_) => value, + }; + let value = credentials + .iter() + .find(|source| source.dummy_value == value) + .map_or(value, |source| source.real_value.as_str()); + value == self.real_value + || value == self.dummy_value + || self.source_values.iter().any(|source| source == value) + }); + let source_changed = source_present && !source_matches; env_contains_credential_value(env, &self.dummy_value) - || self.invalidates_host_binding(env) + || (source_changed || self.invalidates_host_binding(env)) && env.iter().any(|(key, value)| { !env_key_matches(key, CREDENTIAL_BROKER_ACTIVE_ENV_KEY) && !env_key_matches(key, BROKERED_CREDENTIALS_ENV_KEY) @@ -118,6 +151,7 @@ impl CredentialRecord { ) { if self.invalidates_host_binding(env) { self.additional_host_bindings.clear(); + self.fallback_host_bindings.clear(); self.host_binding = host_binding; return; } @@ -160,6 +194,10 @@ fn source_tracks_credential( if !source_accepts_credential(source, credential) { return false; } + // Aliases may hold a different credential from the canonical source. + if source.host_binding == credential.host_binding { + return true; + } let mut source_present = false; let contains_credential = source .env_vars @@ -171,7 +209,12 @@ fn source_tracks_credential( BrokeredCredentialProvider::Builtin(_) => value.trim(), BrokeredCredentialProvider::Configured(_) => value, }; - value == credential.dummy_value || value == credential.real_value + value == credential.dummy_value + || value == credential.real_value + || credential + .source_values + .iter() + .any(|source| source == value) }); !source_present || contains_credential } @@ -240,13 +283,17 @@ impl CredentialBroker { pub(crate) fn configure(&self, config: &NetworkProxyConfig) { let mut state = self.write_state(); + let previous_context_sources = (state.context != config.credential_broker_context) + .then(|| active_credential_sources(&state, &HashMap::new())); if state.enabled != config.credential_broker || state.openai_api_host != config.credential_broker_openai_host || state.allow_local_binding != config.allow_local_binding || state.configured_provider_configs != config.credential_providers + || previous_context_sources.is_some() { state.config_revision += 1; } + state.context.clone_from(&config.credential_broker_context); state.allow_local_binding = config.allow_local_binding; if state.enabled != config.credential_broker { state.enabled = config.credential_broker; @@ -330,6 +377,82 @@ impl CredentialBroker { .configured_provider_configs .clone_from(&config.credential_providers); } + if let Some(previous_sources) = previous_context_sources { + // Update registrations that used the old fallback, leaving distinct captured bindings intact. + let context = state.context.with_fallbacks(&HashMap::new()).into_owned(); + let sources = active_credential_sources(&state, &context); + state.credentials.retain_mut(|credential| { + let Some(previous_source) = previous_sources.iter().find(|source| { + source_accepts_credential(source, credential) + && credential + .host_bindings() + .any(|binding| binding == &source.host_binding) + }) else { + return true; + }; + let source = sources + .iter() + .find(|source| source_accepts_credential(source, credential)); + if source.is_some_and(|source| source.host_binding == previous_source.host_binding) + { + return true; + } + if !credential + .fallback_host_bindings + .contains(&previous_source.host_binding) + { + credential + .fallback_host_bindings + .push(previous_source.host_binding.clone()); + } + if source.is_none() || credential.invalidates_host_binding(&context) { + // Clearing a fallback revokes its history, not independently captured hosts. + credential + .additional_host_bindings + .retain(|binding| !credential.fallback_host_bindings.contains(binding)); + if credential + .fallback_host_bindings + .contains(&credential.host_binding) + { + let replacement = source + .map(|source| source.host_binding.clone()) + .or_else(|| credential.additional_host_bindings.pop()); + let Some(replacement) = replacement else { + return false; + }; + credential.host_binding = replacement; + } + credential.fallback_host_bindings.clear(); + } + if credential.host_binding == previous_source.host_binding { + let Some(source) = source else { + return false; + }; + credential.observe_host_binding(source.host_binding.clone(), &context); + } else { + // A captured primary destination must stay primary when its older fallback changes. + if let Some(source) = source + && !credential + .host_bindings() + .any(|binding| binding == &source.host_binding) + { + credential + .additional_host_bindings + .push(source.host_binding.clone()); + } + } + if let Some(source) = source + && !credential + .fallback_host_bindings + .contains(&source.host_binding) + { + credential + .fallback_host_bindings + .push(source.host_binding.clone()); + } + true + }); + } } pub(crate) fn config_revision(&self) -> u64 { @@ -359,15 +482,30 @@ impl CredentialBroker { } state.observe_credential_owners(parent_env); - for source in active_credential_sources(&state, child_env) { - for env_var in source.env_vars { + let active_sources = active_credential_sources(&state, child_env); + for source in &active_sources { + for env_var in &source.env_vars { let Some(real_value) = - brokerable_credential_value(parent_env, &state, &env_var, &source.provider) + brokerable_credential_value(parent_env, &state, env_var, &source.provider) .map(str::to_string) else { continue; }; - if env_value(child_env, &env_var) != Some(real_value.as_str()) + // Reconcile carried identities in virtualize_env, preserving their registered + // destinations instead of binding the parent's value to a rotated child source. + if state.credentials.iter().any(|credential| { + env_key_matches(&credential.env_var, env_var) + && credential.provider.same_provider(&source.provider) + && credential.real_value == real_value + && credential.has_carried_identity( + child_env, + &state.credentials, + &active_sources, + ) + }) { + continue; + } + if env_value(child_env, env_var) != Some(real_value.as_str()) && child_env.values().any(|value| { value == &real_value || is_builtin_shaped_credential(&real_value) @@ -376,7 +514,7 @@ impl CredentialBroker { }) { let _ = state.register( - &env_var, + env_var, source.provider.clone(), source.host_binding.clone(), environment_id, @@ -425,15 +563,25 @@ impl CredentialBroker { state.observe_credential_owners(env); let active_sources = active_credential_sources(&state, env); + let carried_dummies = state + .credentials + .iter() + .filter(|credential| { + credential.has_carried_identity(env, &state.credentials, &active_sources) + }) + .map(|credential| credential.dummy_value.clone()) + .collect::>(); for credential in state.credentials.iter_mut().filter(|credential| { credential.belongs_to_environment(environment_id) - && credential.should_reconcile_host_binding(env) + && carried_dummies.contains(&credential.dummy_value) }) { - if let Some(source) = active_sources - .iter() - .find(|source| source_tracks_credential(source, credential, env)) + if let Some(source) = registered_credential_source(credential, &active_sources, env) + .filter(|source| { + credential.invalidates_host_binding(env) + || source_tracks_credential(source, credential, env) + }) { - credential.observe_host_binding(source.host_binding.clone(), env); + credential.observe_host_binding(source.host_binding, env); } } let stale_credentials = state @@ -441,10 +589,9 @@ impl CredentialBroker { .iter() .filter(|credential| { credential.belongs_to_environment(environment_id) - && credential.should_reconcile_host_binding(env) - && !active_sources - .iter() - .any(|source| source_tracks_credential(source, credential, env)) + && carried_dummies.contains(&credential.dummy_value) + && !registered_credential_source(credential, &active_sources, env) + .is_some_and(|source| source_tracks_credential(&source, credential, env)) }) .map(|credential| { ( @@ -461,15 +608,23 @@ impl CredentialBroker { .any(|(dummy_value, _)| credential.dummy_value == *dummy_value) }); + let mut owned_dummies = state + .credentials + .iter() + .filter(|credential| { + credential.belongs_to_environment(environment_id) + && carried_dummies.contains(&credential.dummy_value) + }) + .map(|credential| credential.dummy_value.clone()) + .collect::>(); let unbound_inherited_credentials = state .credentials .iter() .filter(|credential| { !credential.belongs_to_environment(environment_id) + && !owned_dummies.contains(&credential.dummy_value) && env_contains_credential_value(env, &credential.dummy_value) - && !active_sources - .iter() - .any(|source| source_accepts_credential(source, credential)) + && registered_credential_source(credential, &active_sources, env).is_none() }) .map(|credential| { ( @@ -480,32 +635,38 @@ impl CredentialBroker { .collect::>(); state.replace_child_env_dummies(env, &unbound_inherited_credentials); - let inherited_credentials = active_sources + let inherited_credentials = state + .credentials .iter() - .flat_map(|source| { - state - .credentials - .iter() - .filter(|&credential| { - !credential.belongs_to_environment(environment_id) - && source_accepts_credential(source, credential) - && env_contains_credential_value(env, &credential.dummy_value) - }) - .map(|credential| { - ( - credential.env_var.clone(), - source.provider.clone(), - source.host_binding.clone(), - credential.real_value.clone(), - credential.dummy_value.clone(), - Arc::clone(&credential.generated_aliases), - ) - }) + .filter(|credential| { + !credential.belongs_to_environment(environment_id) + && !owned_dummies.contains(&credential.dummy_value) + && carried_dummies.contains(&credential.dummy_value) + }) + .filter_map(|credential| { + registered_credential_source(credential, &active_sources, env).map(|source| { + ( + credential.env_var.clone(), + source.provider, + source.host_binding, + credential.real_value.clone(), + credential.dummy_value.clone(), + credential.source_values.clone(), + Arc::clone(&credential.generated_aliases), + ) + }) }) .collect::>(); let mut rebound_dummies = Vec::new(); - for (env_var, provider, host_binding, real_value, inherited_dummy, generated_aliases) in - inherited_credentials + for ( + env_var, + provider, + host_binding, + real_value, + inherited_dummy, + source_values, + generated_aliases, + ) in inherited_credentials { let current_dummy = if let Some(credential) = state.credentials.iter_mut().find(|credential| { @@ -522,18 +683,34 @@ impl CredentialBroker { provider, host_binding, additional_host_bindings: Vec::new(), + fallback_host_bindings: Vec::new(), environment_id: environment_id.map(str::to_string), real_value, dummy_value: inherited_dummy.clone(), + source_values, generated_aliases, }); inherited_dummy.clone() }; if current_dummy != inherited_dummy { - rebound_dummies.push((inherited_dummy, current_dummy)); + rebound_dummies.push((inherited_dummy, current_dummy.clone())); } + owned_dummies.insert(current_dummy); } state.replace_child_env_dummies(env, &rebound_dummies); + for credential in state + .credentials + .iter_mut() + .filter(|credential| credential.belongs_to_environment(environment_id)) + { + for source in &mut credential.source_values { + if let Some((_, rebound)) = + rebound_dummies.iter().find(|(dummy, _)| dummy == source) + { + source.clone_from(rebound); + } + } + } for source in &active_sources { for env_var in &source.env_vars { @@ -549,6 +726,7 @@ impl CredentialBroker { } let provider_context_keys = state.environment(env).provider_context_keys; let mut known_registrations = Vec::new(); + let binding_env = state.context.with_fallbacks(env); let discoverable_values = env .iter() .filter(|(key, value)| { @@ -563,14 +741,11 @@ impl CredentialBroker { let known = matching::known_credential_matches(&state, value, env); for matched in &known { if matched.value == matched.real_value - && env_value(env, matched.env_var).is_none_or(|value| { - !state.is_dummy_value(value) - || state.credentials.iter().any(|credential| { - credential.dummy_value == value - && credential.real_value == matched.real_value - && credential.provider.same_provider(&matched.provider) - && env_key_matches(&credential.env_var, matched.env_var) - }) + && !state.credentials.iter().any(|credential| { + owned_dummies.contains(&credential.dummy_value) + && credential.real_value == matched.real_value + && credential.provider.same_provider(&matched.provider) + && env_key_matches(&credential.env_var, matched.env_var) }) && let Some(source) = active_sources.iter().find(|source| { source.provider.same_provider(&matched.provider) @@ -618,7 +793,7 @@ impl CredentialBroker { for provider in providers::credential_providers() { let brokered_provider = BrokeredCredentialProvider::Builtin(provider); let Some((source, host_binding)) = provider.sources().iter().rev().find_map(|source| { - (source.host_binding)(env, state.openai_api_host.as_deref()) + (source.host_binding)(&binding_env, state.openai_api_host.as_deref()) .map(|binding| (source, binding)) }) else { continue; @@ -698,7 +873,7 @@ impl CredentialBroker { .iter() .any(|key| env_value(env, key) == Some(credential)) && (candidate.host_binding)( - env, + &binding_env, state.openai_api_host.as_deref(), ) .is_none() @@ -719,7 +894,7 @@ impl CredentialBroker { } } for (provider, value, matched) in &configured_discoveries { - let Some(host_binding) = provider.host_binding(env) else { + let Some(host_binding) = provider.host_binding(&binding_env) else { continue; }; let Some(env_var) = provider.config.env.first() else { @@ -768,10 +943,11 @@ impl CredentialBroker { .into_iter() .filter(|credential| { credential.belongs_to_environment(environment_id) - && active_sources.iter().any(|source| { - credential.provider.same_provider(&source.provider) - && credential.host_binding == source.host_binding - }) + && (owned_dummies.contains(&credential.dummy_value) + || active_sources.iter().any(|source| { + credential.provider.same_provider(&source.provider) + && credential.host_binding == source.host_binding + })) }) .collect::>(); let mut credential_aliases = Vec::new(); @@ -955,6 +1131,42 @@ impl CredentialBroker { self.read_state().environment(env) } + pub(crate) fn environment_for_text( + &self, + text: &str, + env: &HashMap, + ) -> CredentialBrokerEnvironment { + let state = self.read_state(); + let mut scoped_env = env.clone(); + for credential in &state.credentials { + remove_env_value(&mut scoped_env, &credential.env_var); + } + for credential in &state.credentials { + if text == credential.dummy_value + || credential.contains_embedded_value(text, &credential.dummy_value) + { + set_env_value( + &mut scoped_env, + &credential.env_var, + credential.dummy_value.clone(), + ); + } + } + update_brokered_credentials_marker(&state, &mut scoped_env); + state.environment(&scoped_env) + } + + pub(crate) fn source_matches_text(&self, source: &str, source_value: &str, text: &str) -> bool { + let state = self.read_state(); + state.enabled + && state.credentials.iter().any(|credential| { + env_key_matches(&credential.env_var, source) + && credential.real_value == source_value + && (text == credential.real_value + || credential.contains_embedded_value(text, &credential.real_value)) + }) + } + pub(crate) fn provider_sources_allowed( &self, value: &str, @@ -1306,7 +1518,10 @@ impl CredentialBrokerState { && credential.belongs_to_environment(environment_id) && credential.real_value == real_value }) { - existing.observe_host_binding(host_binding, existing_env); + // Existing dummies were reconciled before fresh-value discovery. + if !env_contains_credential_value(existing_env, &existing.dummy_value) { + existing.observe_host_binding(host_binding, existing_env); + } return Some(existing.dummy_value.clone()); } if self.credentials.iter().any(|credential| { @@ -1347,14 +1562,35 @@ impl CredentialBrokerState { ); return None; }; + let source_values = env_value(existing_env, env_var) + .map(|value| { + let value = match &provider { + BrokeredCredentialProvider::Builtin(_) => value.trim(), + BrokeredCredentialProvider::Configured(_) => value, + }; + if value == real_value { + vec![real_value.to_string(), dummy_value.clone()] + } else if let Some(source) = self + .credentials + .iter() + .find(|source| source.dummy_value == value) + { + vec![source.real_value.clone(), source.dummy_value.clone()] + } else { + vec![value.to_string()] + } + }) + .unwrap_or_default(); self.credentials.push(CredentialRecord { env_var: env_var.to_string(), provider, host_binding, additional_host_bindings: Vec::new(), + fallback_host_bindings: Vec::new(), environment_id: environment_id.map(str::to_string), real_value: real_value.to_string(), dummy_value: dummy_value.clone(), + source_values, generated_aliases: Arc::default(), }); Some(dummy_value) diff --git a/codex-rs/network-proxy/src/credential_broker/configured_tests.rs b/codex-rs/network-proxy/src/credential_broker/configured_tests.rs index 863cf4ec9f..9831fd4b71 100644 --- a/codex-rs/network-proxy/src/credential_broker/configured_tests.rs +++ b/codex-rs/network-proxy/src/credential_broker/configured_tests.rs @@ -558,6 +558,91 @@ fn configured_provider_discovers_credentials_without_canonical_variables() { assert!(!env.contains_key("STRIPE_API_KEY")); } +#[test] +fn configured_provider_uses_private_destination_context() { + let token = "pin_abcdefgh"; + let destination = "https://api.vendor.example/v2"; + let mut config = NetworkProxyConfig { + credential_broker: true, + credential_providers: BTreeMap::from([( + "vendor".to_string(), + CredentialProviderConfig { + env: vec!["VENDOR_PASSWORD".to_string()], + patterns: vec!["pin_[a-z]{8}".to_string()], + url_prefix_from_env: Some("VENDOR_HOST".to_string()), + ..CredentialProviderConfig::default() + }, + )]), + ..NetworkProxyConfig::default() + }; + config.configure_credential_broker_environment(&HashMap::from([ + ("VENDOR_HOST".to_string(), destination.to_string()), + ("UNRELATED_SECRET".to_string(), token.to_string()), + ])); + assert!(!format!("{config:?}").contains(destination)); + assert!( + !serde_json::to_string(&config) + .unwrap() + .contains(destination) + ); + let broker = CredentialBroker::new(/*enabled*/ true); + broker.configure(&config); + let mut env = HashMap::from([ + ("VENDOR_PASSWORD".to_string(), token.to_string()), + ("AUTH_HEADER".to_string(), format!("Bearer {token}")), + ]); + + for environment_id in [None, Some("child")] { + broker.virtualize_child_env_for_environment(&mut env, environment_id); + assert!(!env.contains_key("VENDOR_HOST")); + assert!(!env.contains_key("UNRELATED_SECRET")); + assert_ne!(env["AUTH_HEADER"], format!("Bearer {token}")); + let dummy_header = env["AUTH_HEADER"].clone(); + env.remove("VENDOR_PASSWORD"); + broker.virtualize_child_env_for_environment(&mut env, environment_id); + assert_eq!(env["AUTH_HEADER"], dummy_header); + let mut snapshot = format!("export AUTH_HEADER='Bearer {token}'"); + assert!(broker.virtualize_text(&mut snapshot, &env)); + assert_eq!(snapshot, format!("export AUTH_HEADER='{dummy_header}'")); + for host in [destination, "https://other.example/v2"] { + let mut headers = HeaderMap::new(); + headers.insert(AUTHORIZATION, HeaderValue::from_str(&dummy_header).unwrap()); + broker.inject_request_headers_for_environment(host, &mut headers, environment_id); + assert_eq!( + headers[AUTHORIZATION], + if host == destination { + format!("Bearer {token}") + } else { + dummy_header.clone() + } + ); + } + } + + for host in ["https://override.example/v3", "", "not a valid destination"] { + let mut env = HashMap::from([ + ("VENDOR_PASSWORD".to_string(), token.to_string()), + ("VENDOR_HOST".to_string(), host.to_string()), + ]); + broker.virtualize_child_env(&mut env); + assert_eq!(env["VENDOR_HOST"], host); + assert_eq!( + env["VENDOR_PASSWORD"] == token, + !host.starts_with("https://") + ); + let dummy_header = format!("Bearer {}", env["VENDOR_PASSWORD"]); + let mut headers = HeaderMap::new(); + headers.insert(AUTHORIZATION, HeaderValue::from_str(&dummy_header).unwrap()); + broker.inject_request_headers(destination, &mut headers); + assert_eq!(headers[AUTHORIZATION], format!("Bearer {token}")); + if host.starts_with("https://") { + headers.insert(AUTHORIZATION, HeaderValue::from_str(&dummy_header).unwrap()); + broker.inject_request_headers(host, &mut headers); + assert_eq!(headers[AUTHORIZATION], format!("Bearer {token}")); + } + } +} + #[test] fn configured_provider_preserves_credentials_without_a_resolved_destination() { let token = "stripe_live_abcdefghijklmnopqrstuvwx"; @@ -793,6 +878,17 @@ fn configured_owner_keeps_prefixed_builtin_shaped_aliases_scoped() { .contains(&key.to_string()) ); assert_eq!(snapshot_alias, env[key]); + assert!( + broker + .environment_for_text(&snapshot_alias, &env) + .credential_keys + .contains(&"VENDOR_KEY".to_string()) + ); + assert!(broker.source_matches_text( + "VENDOR_KEY", + &token, + &original_env[key] + )); assert!(broker.child_alias_matches( key, &env[key], @@ -1154,6 +1250,227 @@ fn discovered_credential_aliases_rebind_when_the_destination_changes() { } } +#[test] +fn credential_rotation_preserves_alias_destination_ownership() { + for (key, rotated_key, host_key, prefix, static_host) in [ + ( + "PROVIDER_TOKEN", + "PROVIDER_TOKEN", + "PROVIDER_URL", + "provider_", + None, + ), + ( + "PROVIDER_TOKEN", + "PROVIDER_TOKEN_FALLBACK", + "PROVIDER_URL", + "provider_", + None, + ), + ( + "PROVIDER_TOKEN", + "PROVIDER_TOKEN", + "PROVIDER_URL", + "provider_", + Some("static.example"), + ), + ( + "GH_ENTERPRISE_TOKEN", + "GH_ENTERPRISE_TOKEN", + "GH_HOST", + "ghp_", + None, + ), + ( + "GH_ENTERPRISE_TOKEN", + "GITHUB_ENTERPRISE_TOKEN", + "GH_HOST", + "ghp_", + None, + ), + ( + "OPENAI_API_KEY", + "OPENAI_API_KEY", + "OPENAI_BASE_URL", + "sk-proj-", + Some("api.openai.com"), + ), + ] { + for (environment, real_aliases, discover_parent) in [ + ("parent", false, false), + ("child", false, false), + ("parent", true, false), + ("child", true, false), + ("parent", true, true), + ("child", true, true), + ] { + let broker = broker_for(CredentialProviderConfig { + env: vec![ + "PROVIDER_TOKEN".to_string(), + "PROVIDER_TOKEN_FALLBACK".to_string(), + ], + patterns: vec!["^provider_[a-z]{36}$".to_string()], + url_prefixes: static_host + .map(|host| format!("https://{host}")) + .into_iter() + .collect(), + url_prefix_from_env: Some("PROVIDER_URL".to_string()), + ..CredentialProviderConfig::default() + }); + let suffix_len = if key == "OPENAI_API_KEY" { 64 } else { 36 }; + let first = format!("{prefix}{}", "a".repeat(suffix_len)); + let second = format!("{prefix}{}", "b".repeat(suffix_len)); + let auxiliary = format!("{prefix}{}", "c".repeat(suffix_len)); + let endpoint = |host: &str| { + if host_key == "GH_HOST" { + host.to_string() + } else { + format!("https://{host}/v1") + } + }; + let mut env = HashMap::from([ + (key.to_string(), first.clone()), + (host_key.to_string(), endpoint("first.example")), + ("AUTH_HEADER".to_string(), format!("Bearer {first}")), + ( + "SECOND_AUTH_HEADER".to_string(), + format!("Bearer {auxiliary}"), + ), + ]); + let parent_env = env.clone(); + broker.virtualize_child_env_for_environment(&mut env, Some("parent")); + let old_dummy = env[key].clone(); + let alias_dummy = env["SECOND_AUTH_HEADER"] + .strip_prefix("Bearer ") + .unwrap() + .to_string(); + assert_ne!(old_dummy, first); + assert_ne!(alias_dummy, auxiliary); + env.remove(key); + env.insert(rotated_key.to_string(), second.clone()); + env.insert(host_key.to_string(), endpoint("second.example")); + if real_aliases { + env.insert("AUTH_HEADER".to_string(), format!("Bearer {first}")); + env.insert( + "SECOND_AUTH_HEADER".to_string(), + format!("Bearer {auxiliary}"), + ); + } + if discover_parent { + broker.discover_parent_credentials_for_environment( + &parent_env, + &env, + Some(environment), + ); + } + broker.virtualize_child_env_for_environment(&mut env, Some(environment)); + let new_dummy = env[rotated_key].clone(); + assert_eq!( + env["AUTH_HEADER"], + format!("Bearer {old_dummy}"), + "{key}, {environment}, real_aliases={real_aliases}, discover_parent={discover_parent}" + ); + assert_eq!(env["SECOND_AUTH_HEADER"], format!("Bearer {alias_dummy}")); + assert!(!env.values().any(|value| { + [&first, &second, &auxiliary] + .iter() + .any(|real| value.contains(real.as_str())) + })); + for (dummy, real, host, allowed) in [ + (&old_dummy, &first, "second.example", false), + (&old_dummy, &first, "first.example", true), + (&alias_dummy, &auxiliary, "second.example", false), + (&alias_dummy, &auxiliary, "first.example", true), + (&new_dummy, &second, "first.example", false), + (&new_dummy, &second, "second.example", true), + ] { + let mut headers = HeaderMap::from_iter([( + AUTHORIZATION, + HeaderValue::from_str(&format!("Bearer {dummy}")).unwrap(), + )]); + broker.inject_request_headers_for_environment( + &format!("https://{host}/v1"), + &mut headers, + Some(environment), + ); + assert_eq!( + headers[AUTHORIZATION], + format!("Bearer {}", if allowed { real } else { dummy }), + "{key}, {environment}, {host}, real_aliases={real_aliases}, discover_parent={discover_parent}" + ); + } + + env.insert(host_key.to_string(), String::new()); + if real_aliases { + env.insert("AUTH_HEADER".to_string(), format!("Bearer {first}")); + env.insert( + "SECOND_AUTH_HEADER".to_string(), + format!("Bearer {auxiliary}"), + ); + } + if discover_parent { + broker.discover_parent_credentials_for_environment( + &parent_env, + &env, + Some(environment), + ); + } + broker.virtualize_child_env_for_environment(&mut env, Some(environment)); + for host in ["first.example", "second.example"] + .into_iter() + .chain(static_host) + { + for (dummy, real) in [ + (&old_dummy, &first), + (&alias_dummy, &auxiliary), + (&new_dummy, &second), + ] { + let mut headers = HeaderMap::from_iter([( + AUTHORIZATION, + HeaderValue::from_str(&format!("Bearer {dummy}")).unwrap(), + )]); + broker.inject_request_headers_for_environment( + &format!("https://{host}/v1"), + &mut headers, + Some(environment), + ); + assert_eq!( + headers[AUTHORIZATION], + format!( + "Bearer {}", + if Some(host) == static_host { + real + } else { + dummy + } + ) + ); + } + } + let mut parent_headers = HeaderMap::from_iter([( + AUTHORIZATION, + HeaderValue::from_str(&format!("Bearer {old_dummy}")).unwrap(), + )]); + broker.inject_request_headers_for_environment( + "https://first.example/v1", + &mut parent_headers, + Some("parent"), + ); + assert_eq!( + parent_headers[AUTHORIZATION], + format!( + "Bearer {}", + if environment == "child" { + &first + } else { + &old_dummy + } + ) + ); + } + } +} + #[test] fn configured_provider_accepts_anchored_alternatives_and_ascii_character_classes() { for (pattern, token) in [ @@ -2315,8 +2632,9 @@ fn configured_provider_destination_history_is_scoped_to_the_environment() { } #[test] -fn configured_provider_restores_credentials_when_the_dynamic_destination_disappears() { +fn configured_provider_preserves_filtered_destinations_but_honors_explicit_overrides() { let token = "stripe_live_abcdefghijklmnopqrstuvwx"; + let second_token = "stripe_live_zyxwvutsrqponmlkjihgfedc"; let broker = broker_for(CredentialProviderConfig { env: vec!["STRIPE_API_KEY".to_string()], patterns: vec!["^stripe_live_[a-z]{24}$".to_string()], @@ -2327,15 +2645,64 @@ fn configured_provider_restores_credentials_when_the_dynamic_destination_disappe ("STRIPE_API_KEY".to_string(), token.to_string()), ("STRIPE_HOST".to_string(), "first.example".to_string()), ("AUTH_HEADER".to_string(), format!("Bearer {token}")), + ( + "SECOND_AUTH_HEADER".to_string(), + format!("Bearer {second_token}"), + ), ]); broker.virtualize_child_env_for_environment(&mut env, Some("environment")); let dummy = env["STRIPE_API_KEY"].clone(); + let mut filtered_child = env.clone(); + filtered_child.remove("STRIPE_HOST"); + let expected = filtered_child.clone(); + broker.virtualize_child_env_for_environment(&mut filtered_child, Some("filtered-child")); + assert_eq!(filtered_child, expected); + for (alias, real) in [("AUTH_HEADER", token), ("SECOND_AUTH_HEADER", second_token)] { + let mut headers = HeaderMap::from_iter([( + AUTHORIZATION, + HeaderValue::from_str(&filtered_child[alias]).unwrap(), + )]); + broker.inject_request_headers_for_environment( + "https://first.example/", + &mut headers, + Some("filtered-child"), + ); + assert_eq!(headers[AUTHORIZATION], format!("Bearer {real}")); + } + + let mut child_env = env.clone(); + child_env.insert("STRIPE_HOST".to_string(), "child.example".to_string()); + broker.virtualize_child_env_for_environment(&mut child_env, Some("child")); + child_env.remove("STRIPE_HOST"); env.remove("STRIPE_HOST"); + for (environment, destination, current_env) in [ + ("environment", "first.example", &mut env), + ("child", "child.example", &mut child_env), + ] { + let expected = current_env.clone(); + broker.virtualize_child_env_for_environment(current_env, Some(environment)); + assert_eq!(*current_env, expected); + for (alias, real) in [("AUTH_HEADER", token), ("SECOND_AUTH_HEADER", second_token)] { + let mut headers = HeaderMap::from_iter([( + AUTHORIZATION, + HeaderValue::from_str(¤t_env[alias]).unwrap(), + )]); + broker.inject_request_headers_for_environment( + &format!("https://{destination}/"), + &mut headers, + Some(environment), + ); + assert_eq!(headers[AUTHORIZATION], format!("Bearer {real}")); + } + } + + env.insert("STRIPE_HOST".to_string(), String::new()); broker.virtualize_child_env_for_environment(&mut env, Some("environment")); assert_eq!(env["STRIPE_API_KEY"], token); assert_eq!(env["AUTH_HEADER"], format!("Bearer {token}")); + assert_eq!(env["SECOND_AUTH_HEADER"], format!("Bearer {second_token}")); let mut headers = HeaderMap::from_iter([( AUTHORIZATION, HeaderValue::from_str(&format!("Bearer {dummy}")).expect("valid dummy authentication"), @@ -2642,6 +3009,10 @@ fn configured_provider_accepts_hostname_or_https_url_from_one_environment_key() broker.environment(&env).binding_keys, vec!["PROVIDER_HOST".to_string()] ); + assert_eq!( + broker.environment(&env).configured_provider_context_keys, + vec!["PROVIDER_HOST".to_string()] + ); } None => { assert_eq!(env["PROVIDER_TOKEN"], token); diff --git a/codex-rs/network-proxy/src/credential_broker/environment.rs b/codex-rs/network-proxy/src/credential_broker/environment.rs index 4b5e08c5af..55314fb839 100644 --- a/codex-rs/network-proxy/src/credential_broker/environment.rs +++ b/codex-rs/network-proxy/src/credential_broker/environment.rs @@ -12,8 +12,59 @@ use super::providers; use super::registry::BrokeredCredentialProvider; use super::remove_env_value; use super::set_env_value; +use crate::NetworkProxyConfig; +use std::borrow::Cow; use std::collections::HashMap; +/// Local destination hints retained by the broker, not added to child environments. +#[derive(Clone, Default, Eq, PartialEq)] +pub struct CredentialBrokerContext(HashMap); + +impl std::fmt::Debug for CredentialBrokerContext { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter.write_str("CredentialBrokerContext()") + } +} + +impl CredentialBrokerContext { + pub(crate) fn capture( + config: &NetworkProxyConfig, + overrides: &HashMap, + ) -> Self { + Self( + credential_broker_provider_context_env_keys() + .map(str::to_string) + .chain( + config + .credential_providers + .values() + .filter_map(|provider| provider.url_prefix_from_env.clone()), + ) + .filter_map(|key| { + env_value(overrides, &key) + .map(str::to_string) + .or_else(|| std::env::var(&key).ok()) + .map(|value| (key, value)) + }) + .collect(), + ) + } + + pub(super) fn with_fallbacks<'a>( + &self, + env: &'a HashMap, + ) -> Cow<'a, HashMap> { + let mut context = Cow::Borrowed(env); + for (key, value) in &self.0 { + // A present value, including an empty one, overrides the trusted fallback. + if env_value(&context, key).is_none() { + set_env_value(context.to_mut(), key, value.clone()); + } + } + context + } +} + /// Trusted provider metadata and active credential bindings for one child environment. #[derive(Clone, Debug, Default, Eq, PartialEq)] pub struct CredentialBrokerEnvironment { @@ -22,6 +73,7 @@ pub struct CredentialBrokerEnvironment { pub context_keys: Vec, pub provider_keys: Vec, pub provider_context_keys: Vec, + pub configured_provider_context_keys: Vec, } impl CredentialBrokerState { @@ -42,6 +94,7 @@ impl CredentialBrokerState { if let Some(key) = provider.config.url_prefix_from_env.as_deref() { push_unique_key(&mut metadata.provider_keys, key); push_unique_key(&mut metadata.provider_context_keys, key); + push_unique_key(&mut metadata.configured_provider_context_keys, key); } } diff --git a/codex-rs/network-proxy/src/credential_broker/matching.rs b/codex-rs/network-proxy/src/credential_broker/matching.rs index f716dd4939..7e20501024 100644 --- a/codex-rs/network-proxy/src/credential_broker/matching.rs +++ b/codex-rs/network-proxy/src/credential_broker/matching.rs @@ -170,6 +170,7 @@ pub(super) fn virtualize_text( let source_env = HashMap::new(); let known = known_credential_matches(state, text, &source_env); let mut replacements = Replacements::default(); + let binding_env = state.context.with_fallbacks(env); let mut allowed = true; for credential in &credentials { let ranges = known @@ -321,7 +322,7 @@ pub(super) fn virtualize_text( } for provider in &state.configured_providers { - if provider.host_binding(env).is_none() { + if provider.host_binding(&binding_env).is_none() { continue; } let mut matches = provider diff --git a/codex-rs/network-proxy/src/credential_broker/registry.rs b/codex-rs/network-proxy/src/credential_broker/registry.rs index d7abbfaf95..d9e1555f03 100644 --- a/codex-rs/network-proxy/src/credential_broker/registry.rs +++ b/codex-rs/network-proxy/src/credential_broker/registry.rs @@ -3,10 +3,13 @@ use super::CredentialRecord; use super::MIN_EMBEDDED_CREDENTIAL_LENGTH; use super::configured::ConfiguredCredentialProvider; use super::env_entry; +use super::env_key_matches; use super::env_value; use super::matching; use super::matching::is_operational_path_match; use super::providers; +use super::source_accepts_credential; +use super::source_tracks_credential; use base64::Engine; use rama_http::HeaderMap; use rama_http::HeaderName; @@ -22,6 +25,7 @@ pub(super) enum BrokeredCredentialProvider { Configured(Arc), } +#[derive(Clone)] pub(super) struct ActiveCredentialSource { pub(super) provider: BrokeredCredentialProvider, pub(super) host_binding: providers::CredentialHostBinding, @@ -310,10 +314,12 @@ pub(super) fn active_credential_sources( state: &CredentialBrokerState, env: &HashMap, ) -> Vec { + let binding_env = state.context.with_fallbacks(env); let mut sources = Vec::new(); for provider in providers::credential_providers() { for source in provider.sources() { - if let Some(host_binding) = (source.host_binding)(env, state.openai_api_host.as_deref()) + if let Some(host_binding) = + (source.host_binding)(&binding_env, state.openai_api_host.as_deref()) { sources.push(ActiveCredentialSource { provider: BrokeredCredentialProvider::Builtin(provider), @@ -328,7 +334,7 @@ pub(super) fn active_credential_sources( } } for provider in &state.configured_providers { - if let Some(host_binding) = provider.host_binding(env) { + if let Some(host_binding) = provider.host_binding(&binding_env) { sources.push(ActiveCredentialSource { provider: BrokeredCredentialProvider::Configured(provider.clone()), host_binding, @@ -339,10 +345,70 @@ pub(super) fn active_credential_sources( sources } +pub(super) fn registered_credential_source( + credential: &CredentialRecord, + sources: &[ActiveCredentialSource], + env: &HashMap, +) -> Option { + let retained_env_vars = match &credential.provider { + BrokeredCredentialProvider::Builtin(provider) => provider + .sources() + .iter() + .find(|source| { + source + .env_vars + .iter() + .any(|key| env_key_matches(key, &credential.env_var)) + && !source.binding_env_vars.is_empty() + && source + .binding_env_vars + .iter() + .all(|key| env_value(env, key).is_none()) + }) + .map(|source| { + source + .env_vars + .iter() + .map(|key| (*key).to_string()) + .collect() + }), + BrokeredCredentialProvider::Configured(provider) => provider + .config + .url_prefix_from_env + .as_deref() + .filter(|key| env_value(env, key).is_none()) + .map(|_| provider.config.env.clone()), + }; + // Filtering a destination hint out of the child must not undo an existing + // registration or rebind it to the broker's ambient fallback. + if let Some(env_vars) = retained_env_vars { + Some(ActiveCredentialSource { + provider: credential.provider.clone(), + host_binding: credential.host_binding.clone(), + env_vars, + }) + } else { + sources + .iter() + .find(|source| source_accepts_credential(source, credential)) + .cloned() + .map(|mut source| { + // A new canonical token does not authorize an older alias at its destination. + if !credential.invalidates_host_binding(env) + && !source_tracks_credential(&source, credential, env) + { + source.host_binding = credential.host_binding.clone(); + } + source + }) + } +} + pub(super) fn prioritized_credentials<'a>( state: &'a CredentialBrokerState, env: &HashMap, ) -> Vec<&'a CredentialRecord> { + let binding_env = state.context.with_fallbacks(env); let mut credentials = state.credentials.iter().collect::>(); credentials.sort_unstable_by_key(|credential| { let active = env_entry(env, &credential.env_var) @@ -351,7 +417,7 @@ pub(super) fn prioritized_credentials<'a>( BrokeredCredentialProvider::Builtin(provider) => { provider.sources().iter().any(|source| { !source.binding_env_vars.is_empty() - && (source.host_binding)(env, state.openai_api_host.as_deref()) + && (source.host_binding)(&binding_env, state.openai_api_host.as_deref()) .is_some_and(|binding| binding == credential.host_binding) }) } @@ -360,9 +426,9 @@ pub(super) fn prioritized_credentials<'a>( .url_prefix_from_env .as_deref() .is_some_and(|key| { - env_value(env, key).is_some() + env_value(&binding_env, key).is_some() && provider - .host_binding(env) + .host_binding(&binding_env) .is_some_and(|binding| binding == credential.host_binding) }), }; diff --git a/codex-rs/network-proxy/src/credential_broker_tests.rs b/codex-rs/network-proxy/src/credential_broker_tests.rs index 122cd64b8f..14e748efd6 100644 --- a/codex-rs/network-proxy/src/credential_broker_tests.rs +++ b/codex-rs/network-proxy/src/credential_broker_tests.rs @@ -1137,6 +1137,275 @@ fn concurrent_commands_preserve_discovered_credential_destinations() { } } +#[test] +fn builtin_credentials_use_private_destination_context() { + let broker = CredentialBroker::new(/*enabled*/ true); + let mut config = NetworkProxyConfig::default(); + config.set_credential_broker_enabled(/*enabled*/ true); + config.configure_credential_broker_environment(&env_map([ + ("GH_HOST", "github.enterprise.example"), + ("OPENAI_BASE_URL", "https://gateway.example/v1"), + ])); + broker.configure(&config); + for (key, context_key, token, host) in [ + ( + "GH_ENTERPRISE_TOKEN", + "GH_HOST", + "ghp-real", + "github.enterprise.example", + ), + ( + "OPENAI_API_KEY", + "OPENAI_BASE_URL", + "sk-real", + "gateway.example", + ), + ] { + let mut env = env_map([(key, token)]); + broker.virtualize_child_env(&mut env); + assert_ne!(env[key], token); + assert!(!env.contains_key("GH_HOST")); + assert!(!env.contains_key("OPENAI_BASE_URL")); + let mut headers = headers_with_bearer(&env[key]); + broker.inject_request_headers(host, &mut headers); + assert_eq!( + authorization(&headers), + Some(format!("Bearer {token}").as_str()) + ); + + let snapshot_destination = if context_key == "GH_HOST" { + "snapshot.example" + } else { + "https://snapshot.example/v1" + }; + env.insert(context_key.to_string(), snapshot_destination.to_string()); + broker.virtualize_child_env(&mut env); + env.remove(context_key); + env.insert( + "NEW_AUTH_HEADER".to_string(), + format!("Bearer {}", env[key]), + ); + env.insert("REAL_COPY".to_string(), token.to_string()); + let dummy = env[key].clone(); + broker.virtualize_child_env(&mut env); + assert_eq!((&env[key], &env["REAL_COPY"]), (&dummy, &dummy)); + assert!(!env.contains_key(context_key)); + for destination in ["snapshot.example", host] { + let mut headers = headers_with_bearer(&dummy); + broker.inject_request_headers(destination, &mut headers); + assert_eq!( + authorization(&headers), + Some(format!("Bearer {token}").as_str()) + ); + } + let mut inherited_env = env.clone(); + broker.virtualize_child_env_for_environment(&mut inherited_env, Some("child")); + assert_eq!(inherited_env[key], dummy); + for (destination, injected) in [("snapshot.example", true), (host, false)] { + let mut headers = headers_with_bearer(&dummy); + broker.inject_request_headers_for_environment(destination, &mut headers, Some("child")); + assert_eq!( + headers, + headers_with_bearer(if injected { token } else { &dummy }), + ); + } + broker.restore_child_env(&mut env, &mut []); + assert_eq!(env["NEW_AUTH_HEADER"], format!("Bearer {token}")); + } +} + +#[test] +fn private_destination_updates_reconcile_registered_fallbacks() { + for ((key, context_key, token), previously_used_fallback) in [ + ("GH_ENTERPRISE_TOKEN", "GH_HOST", "ghp-real"), + ("OPENAI_API_KEY", "OPENAI_BASE_URL", "sk-real"), + ( + "VENDOR_TOKEN", + "VENDOR_HOST", + "vendor_abcdefghijklmnopqrstuvwx", + ), + ] + .into_iter() + .flat_map(|source| [(source, false), (source, true)]) + { + let destination = |host: &str| { + if context_key == "GH_HOST" { + host.to_string() + } else { + format!("https://{host}") + } + }; + let broker = CredentialBroker::new(/*enabled*/ true); + let mut config = NetworkProxyConfig { + credential_broker: true, + credential_providers: BTreeMap::from([( + "vendor".to_string(), + CredentialProviderConfig { + env: vec!["VENDOR_TOKEN".to_string()], + patterns: vec!["^vendor_[a-z]{24}$".to_string()], + url_prefix_from_env: Some("VENDOR_HOST".to_string()), + ..CredentialProviderConfig::default() + }, + )]), + ..NetworkProxyConfig::default() + }; + config.configure_credential_broker_environment(&env_map([( + context_key, + &destination("first.example"), + )])); + broker.configure(&config); + let mut env = env_map([(key, token)]); + broker.virtualize_child_env(&mut env); + let dummy = env[key].clone(); + assert_ne!(dummy, token); + + let mut explicit_env = env_map([(key, token)]); + if previously_used_fallback { + broker.virtualize_child_env_for_environment(&mut explicit_env, Some("explicit")); + } + explicit_env.insert( + context_key.to_string(), + destination("other-explicit.example"), + ); + broker.virtualize_child_env_for_environment(&mut explicit_env, Some("explicit")); + explicit_env.insert(context_key.to_string(), destination("explicit.example")); + broker.virtualize_child_env_for_environment(&mut explicit_env, Some("explicit")); + explicit_env.remove(context_key); + let explicit_dummy = explicit_env[key].clone(); + let assert_inherited_destination = |child: &str| { + let mut inherited_env = explicit_env.clone(); + inherited_env.insert("CREDENTIAL_COPY".to_string(), token.to_string()); + broker.virtualize_child_env_for_environment(&mut inherited_env, Some(child)); + assert_eq!( + inherited_env["CREDENTIAL_COPY"], explicit_dummy, + "{key}: {child}" + ); + for host in ["explicit.example", "first.example", "second.example"] { + let mut headers = headers_with_bearer(&explicit_dummy); + broker.inject_request_headers_for_environment( + &format!("https://{host}/v1"), + &mut headers, + Some(child), + ); + assert_eq!( + headers, + headers_with_bearer(if host == "explicit.example" { + token + } else { + &explicit_dummy + }), + "{key}: {child}, {host}" + ); + } + }; + let unrelated_key = if context_key == "GH_HOST" { + "OPENAI_BASE_URL" + } else { + "GH_HOST" + }; + config.configure_credential_broker_environment(&env_map([ + (context_key, &destination("first.example")), + (unrelated_key, "https://unrelated.example"), + ])); + broker.configure(&config); + assert_inherited_destination("unchanged-fallback-child"); + + config.configure_credential_broker_environment(&env_map([( + context_key, + &destination("second.example"), + )])); + let revision = broker.config_revision(); + broker.configure(&config); + assert_eq!(broker.config_revision(), revision + 1); + assert_inherited_destination("updated-fallback-child"); + broker.virtualize_child_env(&mut env); + broker.virtualize_child_env_for_environment(&mut explicit_env, Some("explicit")); + assert_eq!(env[key], dummy); + for (destination, environment_id, value, injected) in [ + ("first.example", None, &dummy, true), + ("second.example", None, &dummy, true), + ("explicit.example", Some("explicit"), &explicit_dummy, true), + ( + "second.example", + Some("explicit"), + &explicit_dummy, + previously_used_fallback, + ), + ] { + let mut headers = headers_with_bearer(value); + broker.inject_request_headers_for_environment( + &format!("https://{destination}/v1"), + &mut headers, + environment_id, + ); + assert_eq!( + headers, + headers_with_bearer(if injected { token } else { value }), + "{key}: {destination}, {environment_id:?}" + ); + } + + config.configure_credential_broker_environment(&env_map([(context_key, "")])); + broker.configure(&config); + for destination in ["first.example", "second.example"] { + for (environment, value) in [(None, &dummy), (Some("explicit"), &explicit_dummy)] { + let mut headers = headers_with_bearer(value); + broker.inject_request_headers_for_environment( + &format!("https://{destination}/v1"), + &mut headers, + environment, + ); + assert_eq!(headers, headers_with_bearer(value)); + } + } + let mut headers = headers_with_bearer(&explicit_dummy); + broker.inject_request_headers_for_environment( + "https://explicit.example/v1", + &mut headers, + Some("explicit"), + ); + assert_eq!(headers, headers_with_bearer(token)); + let mut headers = headers_with_bearer(&explicit_dummy); + broker.inject_request_headers_for_environment( + "https://other-explicit.example/v1", + &mut headers, + Some("explicit"), + ); + assert_eq!(headers, headers_with_bearer(token)); + + // A fallback can become primary again without erasing captured destinations. + config.configure_credential_broker_environment(&env_map([( + context_key, + &destination("third.example"), + )])); + broker.configure(&config); + explicit_env.insert(context_key.to_string(), destination("third.example")); + broker.virtualize_child_env_for_environment(&mut explicit_env, Some("explicit")); + config.configure_credential_broker_environment(&env_map([(context_key, "")])); + broker.configure(&config); + for host in [ + "explicit.example", + "other-explicit.example", + "third.example", + ] { + let mut headers = headers_with_bearer(&explicit_dummy); + broker.inject_request_headers_for_environment( + &format!("https://{host}/v1"), + &mut headers, + Some("explicit"), + ); + assert_eq!( + headers, + headers_with_bearer(if host == "third.example" { + &explicit_dummy + } else { + token + }) + ); + } + } +} + #[test] fn openai_credentials_bind_only_to_default_and_configured_trusted_hosts() { let broker = CredentialBroker::new(/*enabled*/ true); diff --git a/codex-rs/network-proxy/src/lib.rs b/codex-rs/network-proxy/src/lib.rs index f53e497edc..565c99e70c 100644 --- a/codex-rs/network-proxy/src/lib.rs +++ b/codex-rs/network-proxy/src/lib.rs @@ -45,6 +45,7 @@ pub use config::host_and_port_from_network_addr; pub use config::managed_proxy_ports; pub use credential_broker::CREDENTIAL_BROKER_ACTIVE_ENV_KEY; pub use credential_broker::CredentialAuthMethod; +pub use credential_broker::CredentialBrokerContext; pub use credential_broker::CredentialBrokerEnvironment; pub use credential_broker::CredentialProviderConfig; pub use credential_broker::brokered_credential_binding_env_keys; diff --git a/codex-rs/network-proxy/src/proxy.rs b/codex-rs/network-proxy/src/proxy.rs index 26d29d806c..eb5ec10410 100644 --- a/codex-rs/network-proxy/src/proxy.rs +++ b/codex-rs/network-proxy/src/proxy.rs @@ -896,6 +896,7 @@ impl NetworkProxy { enabled: config.enabled, credential_providers: config.credential_providers.clone(), credential_broker_openai_host: config.credential_broker_openai_host.clone(), + credential_broker_context: config.credential_broker_context.clone(), dangerously_allow_plaintext_credential_injection: config .dangerously_allow_plaintext_credential_injection, ..config::NetworkProxyConfig::default() @@ -1202,6 +1203,26 @@ impl NetworkProxy { self.state.credential_broker_environment(env) } + /// Returns provider context for the dummy credentials contained in trusted text. + pub fn credential_broker_environment_for_text( + &self, + text: &str, + env: &HashMap, + ) -> crate::CredentialBrokerEnvironment { + self.state.credential_broker_environment_for_text(text, env) + } + + /// Checks whether a registered provider source occurs in trusted captured text. + pub fn credential_broker_source_matches_text( + &self, + source: &str, + source_value: &str, + text: &str, + ) -> bool { + self.state + .credential_broker_source_matches_text(source, source_value, text) + } + /// Checks whether recognized credentials originate from permitted provider variables. pub fn credential_broker_sources_allowed( &self, @@ -2145,6 +2166,10 @@ mod tests { ..NetworkProxyConfig::default() }; config.set_credential_broker_enabled(/*enabled*/ true); + config.configure_credential_broker_environment(&HashMap::from([( + "GH_HOST".to_string(), + "local-github.example".to_string(), + )])); let state = Arc::new(network_proxy_state_for_policy(config)); let proxy = match NetworkProxy::builder().state(state).build().await { Ok(proxy) => proxy, diff --git a/codex-rs/network-proxy/src/runtime.rs b/codex-rs/network-proxy/src/runtime.rs index 6fd8bb9e62..0f417e97ca 100644 --- a/codex-rs/network-proxy/src/runtime.rs +++ b/codex-rs/network-proxy/src/runtime.rs @@ -522,6 +522,24 @@ impl NetworkProxyState { self.credential_broker.environment(env) } + pub(crate) fn credential_broker_environment_for_text( + &self, + text: &str, + env: &HashMap, + ) -> crate::CredentialBrokerEnvironment { + self.credential_broker.environment_for_text(text, env) + } + + pub(crate) fn credential_broker_source_matches_text( + &self, + source: &str, + source_value: &str, + text: &str, + ) -> bool { + self.credential_broker + .source_matches_text(source, source_value, text) + } + pub(crate) fn credential_broker_sources_allowed( &self, value: &str,