mirror of
https://github.com/openai/codex.git
synced 2026-10-23 18:08:45 +00:00
Support configured credential providers in remote execution (#56049)
## Why Remote credential masking previously fell back to unmasked execution whenever custom credential providers were configured. ## What changed - Transport configured provider rules to the executor-local network proxy, where credential values and destination context are resolved. - Advertise `credentialMaskingCustomProviders` and negotiate masking support. Keep the warning and unmasked fallback for executors that support only built-in providers. - Discard provider rules when masking is disabled and omit their contents from debug output. ## Testing Extend coverage for capability compatibility, provider configuration round trips, and fallback on older executors. Exercise configured-provider masking in remote request history and verify that masked placeholders are restored to real credentials in upstream request headers. GitOrigin-RevId: b036f56900b343ddccc17b5293c82bc1f3426237
This commit is contained in:
@@ -107,6 +107,7 @@ pub use proxy::is_managed_proxy_env_var;
|
||||
pub use proxy::proxy_url_env_value;
|
||||
pub use proxy::strip_managed_proxy_env;
|
||||
pub use remote_config::RemoteCredentialBrokerCache;
|
||||
pub use remote_config::RemoteCredentialMasking;
|
||||
pub use remote_config::RemoteNetworkProxyConfig;
|
||||
pub use remote_config::RemoteNetworkProxyLaunchConfig;
|
||||
pub use request_cancellation::NetworkRequestCancellation;
|
||||
|
||||
@@ -12,6 +12,7 @@ use crate::http_proxy;
|
||||
use crate::network_policy::NetworkDecision;
|
||||
use crate::network_policy::NetworkDecisionSource;
|
||||
use crate::network_policy::NetworkPolicyDecider;
|
||||
use crate::remote_config::RemoteCredentialMasking;
|
||||
use crate::runtime::BlockedRequestObserver;
|
||||
use crate::runtime::ConfigState;
|
||||
use crate::runtime::HostAuthorization;
|
||||
@@ -949,15 +950,17 @@ impl NetworkProxy {
|
||||
pub async fn remote_launch_config(
|
||||
&self,
|
||||
local_binding_policy: crate::LocalBindingPolicy,
|
||||
credential_masking: bool,
|
||||
credential_masking: RemoteCredentialMasking,
|
||||
) -> Result<crate::RemoteNetworkProxyLaunchConfig> {
|
||||
let (mut config, brokerage_created_default_allowlist, _) =
|
||||
self.state.current_cfg_with_brokerage_provenance().await?;
|
||||
// Configured providers are not transported yet, so these commands run unmasked.
|
||||
let credential_masking = credential_masking
|
||||
&& config.enabled
|
||||
let credential_masking = config.enabled
|
||||
&& config.credential_broker
|
||||
&& config.credential_providers.is_empty();
|
||||
&& match credential_masking {
|
||||
RemoteCredentialMasking::Disabled => false,
|
||||
RemoteCredentialMasking::BuiltInOnly => config.credential_providers.is_empty(),
|
||||
RemoteCredentialMasking::WithConfiguredProviders => true,
|
||||
};
|
||||
anyhow::ensure!(
|
||||
!credential_masking
|
||||
|| (!config.dangerously_allow_plaintext_credential_injection
|
||||
@@ -1021,7 +1024,9 @@ impl NetworkProxy {
|
||||
"MXC cannot enforce allow_local_binding=false: native host-loopback access is bidirectional; use true or a different sandbox backend"
|
||||
);
|
||||
let mut proxy = crate::RemoteNetworkProxyConfig::from_effective_config(&config)?;
|
||||
proxy.credential_masking = credential_masking;
|
||||
if credential_masking {
|
||||
proxy.enable_credential_masking(config.credential_providers);
|
||||
}
|
||||
tracing::debug!(network_config = ?proxy, "resolved remote execution network configuration");
|
||||
let (environment_id, execution_id) = self
|
||||
.execution_scope
|
||||
@@ -1970,7 +1975,7 @@ mod tests {
|
||||
assert_eq!(proxy.current_cfg().await?, config);
|
||||
|
||||
let remote = proxy
|
||||
.remote_launch_config(DefaultFalse, /*credential_masking*/ false)
|
||||
.remote_launch_config(DefaultFalse, RemoteCredentialMasking::Disabled)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
(
|
||||
@@ -2068,7 +2073,7 @@ mod tests {
|
||||
owner_config.set_denied_domains(vec!["denied.example".to_string()]);
|
||||
assert_eq!(
|
||||
scoped
|
||||
.remote_launch_config(DefaultFalse, /*credential_masking*/ false)
|
||||
.remote_launch_config(DefaultFalse, RemoteCredentialMasking::Disabled)
|
||||
.await?
|
||||
.proxy
|
||||
.domains,
|
||||
@@ -2507,7 +2512,7 @@ mod tests {
|
||||
/*fallback_policy_decider*/ None,
|
||||
)?;
|
||||
let launch = scoped
|
||||
.remote_launch_config(RequireTrue, /*credential_masking*/ false)
|
||||
.remote_launch_config(RequireTrue, RemoteCredentialMasking::Disabled)
|
||||
.await?;
|
||||
let prepared = scoped.prepare_for_optional_environment(
|
||||
HashMap::from([(
|
||||
@@ -2522,7 +2527,7 @@ mod tests {
|
||||
assert!(!launch.proxy.enabled);
|
||||
assert!(
|
||||
scoped
|
||||
.remote_launch_config(DefaultFalse, /*credential_masking*/ true)
|
||||
.remote_launch_config(DefaultFalse, RemoteCredentialMasking::BuiltInOnly)
|
||||
.await
|
||||
.is_err()
|
||||
);
|
||||
@@ -2538,16 +2543,51 @@ mod tests {
|
||||
},
|
||||
);
|
||||
let custom_provider_proxy = NetworkProxy::builder()
|
||||
.state(Arc::new(network_proxy_state_for_policy(config)))
|
||||
.state(Arc::new(network_proxy_state_for_policy(config.clone())))
|
||||
.build()
|
||||
.await?;
|
||||
assert_eq!(
|
||||
custom_provider_proxy
|
||||
.remote_launch_config(DefaultFalse, /*credential_masking*/ true)
|
||||
.remote_launch_config(DefaultFalse, RemoteCredentialMasking::BuiltInOnly)
|
||||
.await?
|
||||
.proxy,
|
||||
launch.proxy
|
||||
);
|
||||
assert!(
|
||||
custom_provider_proxy
|
||||
.remote_launch_config(
|
||||
DefaultFalse,
|
||||
RemoteCredentialMasking::WithConfiguredProviders
|
||||
)
|
||||
.await
|
||||
.is_err()
|
||||
);
|
||||
config.dangerously_allow_plaintext_credential_injection = false;
|
||||
let custom_provider_proxy = NetworkProxy::builder()
|
||||
.state(Arc::new(network_proxy_state_for_policy(config.clone())))
|
||||
.build()
|
||||
.await?;
|
||||
assert_eq!(
|
||||
custom_provider_proxy
|
||||
.remote_launch_config(DefaultFalse, RemoteCredentialMasking::Disabled)
|
||||
.await?
|
||||
.proxy,
|
||||
launch.proxy
|
||||
);
|
||||
let custom_launch = custom_provider_proxy
|
||||
.remote_launch_config(
|
||||
DefaultFalse,
|
||||
RemoteCredentialMasking::WithConfiguredProviders,
|
||||
)
|
||||
.await?;
|
||||
assert!(custom_launch.proxy.credential_masking_enabled());
|
||||
assert_eq!(
|
||||
custom_launch
|
||||
.proxy
|
||||
.into_network_proxy_config()
|
||||
.credential_providers,
|
||||
config.credential_providers,
|
||||
);
|
||||
|
||||
let mut owner_config = NetworkProxyConfig::default();
|
||||
owner_config.set_allowed_domains(vec!["owner.example".to_string()]);
|
||||
@@ -2563,7 +2603,7 @@ mod tests {
|
||||
)?;
|
||||
assert!(
|
||||
owner_scoped
|
||||
.remote_launch_config(DefaultFalse, /*credential_masking*/ false)
|
||||
.remote_launch_config(DefaultFalse, RemoteCredentialMasking::Disabled)
|
||||
.await
|
||||
.is_err()
|
||||
);
|
||||
@@ -2595,13 +2635,16 @@ mod tests {
|
||||
.state(Arc::new(network_proxy_state_for_policy(config)))
|
||||
.build()
|
||||
.await?;
|
||||
for credential_masking in [false, true] {
|
||||
for (credential_masking, expected_masking) in [
|
||||
(RemoteCredentialMasking::Disabled, false),
|
||||
(RemoteCredentialMasking::BuiltInOnly, true),
|
||||
] {
|
||||
let launch = proxy
|
||||
.remote_launch_config(DefaultFalse, credential_masking)
|
||||
.await?;
|
||||
assert!(launch.proxy.enabled);
|
||||
assert_eq!(launch.proxy.mode, mode);
|
||||
assert_eq!(launch.proxy.credential_masking, credential_masking);
|
||||
assert_eq!(launch.proxy.credential_masking_enabled(), expected_masking);
|
||||
}
|
||||
}
|
||||
assert_eq!(
|
||||
|
||||
@@ -2,8 +2,11 @@ use anyhow::Result;
|
||||
use anyhow::ensure;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use std::collections::BTreeMap;
|
||||
use std::fmt;
|
||||
use std::sync::Mutex;
|
||||
|
||||
use crate::CredentialProviderConfig;
|
||||
use crate::NetworkDomainPermissions;
|
||||
use crate::NetworkMode;
|
||||
use crate::NetworkProxyAuditMetadata;
|
||||
@@ -69,6 +72,54 @@ impl RemoteCredentialBrokerCache {
|
||||
}
|
||||
}
|
||||
|
||||
/// Credential masking supported by an executor or requested for a remote launch.
|
||||
#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
|
||||
pub enum RemoteCredentialMasking {
|
||||
#[default]
|
||||
Disabled,
|
||||
/// Mask only when the configuration contains no custom providers.
|
||||
BuiltInOnly,
|
||||
WithConfiguredProviders,
|
||||
}
|
||||
|
||||
/// Masking fields shared with independently deployed executors.
|
||||
#[derive(Clone, Default, Deserialize, Eq, PartialEq, Serialize)]
|
||||
#[serde(default)]
|
||||
struct CredentialMaskingConfig {
|
||||
#[serde(
|
||||
rename = "credentialMasking",
|
||||
skip_serializing_if = "std::ops::Not::not"
|
||||
)]
|
||||
enabled: bool,
|
||||
#[serde(
|
||||
rename = "credentialProviders",
|
||||
skip_serializing_if = "BTreeMap::is_empty"
|
||||
)]
|
||||
providers: BTreeMap<String, CredentialProviderConfig>,
|
||||
}
|
||||
|
||||
impl CredentialMaskingConfig {
|
||||
fn deserialize_normalized<'de, D: serde::Deserializer<'de>>(
|
||||
deserializer: D,
|
||||
) -> Result<Self, D::Error> {
|
||||
let mut config = Self::deserialize(deserializer)?;
|
||||
// Discard provider rules sent with masking off, even if we forward this configuration.
|
||||
if !config.enabled {
|
||||
config.providers.clear();
|
||||
}
|
||||
Ok(config)
|
||||
}
|
||||
}
|
||||
|
||||
impl fmt::Debug for CredentialMaskingConfig {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
f.debug_struct("CredentialMaskingConfig")
|
||||
.field("enabled", &self.enabled)
|
||||
.field("credential_provider_count", &self.providers.len())
|
||||
.finish()
|
||||
}
|
||||
}
|
||||
|
||||
/// Executor-local proxy launch inputs transported with one process start.
|
||||
///
|
||||
/// Unlike [`crate::ManagedNetworkSandboxContext`], this describes how the executor should create
|
||||
@@ -115,15 +166,18 @@ impl RemoteNetworkProxyLaunchConfig {
|
||||
/// Effective network proxy settings that are safe to send to a remote executor.
|
||||
///
|
||||
/// Listener addresses are deliberately omitted because the executor chooses its own loopback
|
||||
/// ports. Masking carries only built-in broker activation; credentials, destination context, custom
|
||||
/// providers, CA configuration, and MITM hooks stay on their execution host.
|
||||
/// ports. Masking carries provider rules; credential values and destination context are resolved
|
||||
/// on the executor. CA configuration and MITM hooks stay on their execution host.
|
||||
#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[non_exhaustive]
|
||||
pub struct RemoteNetworkProxyConfig {
|
||||
pub enabled: bool,
|
||||
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
|
||||
pub credential_masking: bool,
|
||||
#[serde(
|
||||
flatten,
|
||||
deserialize_with = "CredentialMaskingConfig::deserialize_normalized"
|
||||
)]
|
||||
credential_masking: CredentialMaskingConfig,
|
||||
pub enable_socks5: bool,
|
||||
pub enable_socks5_udp: bool,
|
||||
pub allow_upstream_proxy: bool,
|
||||
@@ -147,7 +201,7 @@ impl RemoteNetworkProxyConfig {
|
||||
);
|
||||
Ok(Self {
|
||||
enabled: config.enabled,
|
||||
credential_masking: false,
|
||||
credential_masking: CredentialMaskingConfig::default(),
|
||||
enable_socks5: config.enable_socks5,
|
||||
enable_socks5_udp: config.enable_socks5_udp,
|
||||
allow_upstream_proxy: config.allow_upstream_proxy,
|
||||
@@ -161,6 +215,22 @@ impl RemoteNetworkProxyConfig {
|
||||
})
|
||||
}
|
||||
|
||||
/// Enable masking with the given provider rules, or just the built-ins if empty.
|
||||
pub fn enable_credential_masking(
|
||||
&mut self,
|
||||
providers: BTreeMap<String, CredentialProviderConfig>,
|
||||
) {
|
||||
self.credential_masking = CredentialMaskingConfig {
|
||||
enabled: true,
|
||||
providers,
|
||||
};
|
||||
}
|
||||
|
||||
/// Whether the executor should mask credentials for this launch.
|
||||
pub fn credential_masking_enabled(&self) -> bool {
|
||||
self.credential_masking.enabled
|
||||
}
|
||||
|
||||
pub(crate) fn into_network_proxy_config(self) -> NetworkProxyConfig {
|
||||
let mut config = NetworkProxyConfig {
|
||||
enabled: self.enabled,
|
||||
@@ -174,7 +244,10 @@ impl RemoteNetworkProxyConfig {
|
||||
allow_local_binding: Some(self.allow_local_binding),
|
||||
..NetworkProxyConfig::default()
|
||||
};
|
||||
config.set_credential_broker_enabled(self.credential_masking);
|
||||
if self.credential_masking.enabled {
|
||||
config.set_credential_broker_enabled(/*enabled*/ true);
|
||||
config.credential_providers = self.credential_masking.providers;
|
||||
}
|
||||
config
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,6 +12,7 @@ use crate::NetworkProxyAuditMetadata;
|
||||
use crate::NetworkProxyConfig;
|
||||
use crate::NetworkProxyState;
|
||||
use crate::Platform;
|
||||
use crate::RemoteCredentialMasking;
|
||||
use std::sync::Arc;
|
||||
|
||||
#[test]
|
||||
@@ -47,13 +48,14 @@ fn optional_socket_policy_preserves_input_and_resolves_at_remote_boundary() {
|
||||
|
||||
#[tokio::test]
|
||||
async fn round_trip_preserves_supported_effective_settings() {
|
||||
for (executor_os, socket, credential_masking) in [
|
||||
(Platform::Linux, "/var/run/example.sock", false),
|
||||
(Platform::Linux, "/var/run/example.sock", true),
|
||||
(Platform::Macos, "/var/run/example.sock", false),
|
||||
(Platform::Macos, "/var/run/example.sock", true),
|
||||
(Platform::Windows, r"C:\example.sock", false),
|
||||
(Platform::Unknown, r"C:\example.sock", false),
|
||||
for (executor_os, socket, credential_masking, configured_provider) in [
|
||||
(Platform::Linux, "/var/run/example.sock", false, false),
|
||||
(Platform::Linux, "/var/run/example.sock", true, false),
|
||||
(Platform::Linux, "/var/run/example.sock", true, true),
|
||||
(Platform::Macos, "/var/run/example.sock", false, false),
|
||||
(Platform::Macos, "/var/run/example.sock", true, true),
|
||||
(Platform::Windows, r"C:\example.sock", false, false),
|
||||
(Platform::Unknown, r"C:\example.sock", false, false),
|
||||
] {
|
||||
let mut config = NetworkProxyConfig {
|
||||
enabled: true,
|
||||
@@ -65,14 +67,67 @@ async fn round_trip_preserves_supported_effective_settings() {
|
||||
allow_local_binding: Some(true),
|
||||
..NetworkProxyConfig::default()
|
||||
};
|
||||
config.set_allowed_domains(vec!["example.com".into()]);
|
||||
// Domain permissions serialize as a sorted map.
|
||||
config.set_denied_domains(vec!["blocked.example.com".into()]);
|
||||
config.set_allowed_domains(vec!["example.com".into()]);
|
||||
config.set_allow_unix_sockets(vec![socket.into()]);
|
||||
|
||||
let mut remote = RemoteNetworkProxyConfig::from_effective_config(&config)
|
||||
.expect("supported remote config");
|
||||
remote.credential_masking = credential_masking;
|
||||
config.set_credential_broker_enabled(credential_masking);
|
||||
if configured_provider {
|
||||
config.credential_providers.insert(
|
||||
"vendor".to_string(),
|
||||
crate::CredentialProviderConfig {
|
||||
env: vec!["VENDOR_TOKEN".to_string()],
|
||||
patterns: vec!["vendor_[a-z]{8}".to_string()],
|
||||
url_prefix_from_env: Some("VENDOR_BASE_URL".to_string()),
|
||||
auth: vec![crate::CredentialAuthMethod::Bearer],
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
}
|
||||
if credential_masking {
|
||||
remote.enable_credential_masking(config.credential_providers.clone());
|
||||
}
|
||||
let wire = serde_json::to_value(&remote).expect("serialize remote config");
|
||||
assert_eq!(
|
||||
wire.get("credentialMasking"),
|
||||
credential_masking.then_some(&serde_json::Value::Bool(true))
|
||||
);
|
||||
assert_eq!(
|
||||
wire.get("credentialProviders"),
|
||||
configured_provider
|
||||
.then(|| serde_json::to_value(&config.credential_providers).unwrap())
|
||||
.as_ref()
|
||||
);
|
||||
assert_eq!(
|
||||
serde_json::from_value::<RemoteNetworkProxyConfig>(wire.clone())
|
||||
.expect("deserialize remote config"),
|
||||
remote
|
||||
);
|
||||
let mut disabled_wire = wire;
|
||||
disabled_wire
|
||||
.as_object_mut()
|
||||
.unwrap()
|
||||
.remove("credentialMasking");
|
||||
let mut expected_wire = disabled_wire.clone();
|
||||
expected_wire
|
||||
.as_object_mut()
|
||||
.unwrap()
|
||||
.remove("credentialProviders");
|
||||
// Ignore provider rules when the peer omits or explicitly disables masking.
|
||||
let mut explicitly_disabled_wire = disabled_wire.clone();
|
||||
explicitly_disabled_wire["credentialMasking"] = false.into();
|
||||
for wire in [disabled_wire, explicitly_disabled_wire] {
|
||||
let disabled: RemoteNetworkProxyConfig =
|
||||
serde_json::from_value(wire).expect("disabled masking ignores provider rules");
|
||||
assert!(!disabled.credential_masking_enabled());
|
||||
assert_eq!(serde_json::to_value(disabled).unwrap(), expected_wire);
|
||||
}
|
||||
let diagnostic = format!("{remote:?}");
|
||||
assert!(!diagnostic.contains("vendor_[a-z]{8}"));
|
||||
assert!(!diagnostic.contains("VENDOR_BASE_URL"));
|
||||
let round_trip = remote.clone().into_network_proxy_config();
|
||||
|
||||
assert_eq!(round_trip, config);
|
||||
@@ -102,7 +157,14 @@ async fn round_trip_preserves_supported_effective_settings() {
|
||||
);
|
||||
assert_eq!(
|
||||
proxy
|
||||
.remote_launch_config(DefaultFalse, credential_masking)
|
||||
.remote_launch_config(
|
||||
DefaultFalse,
|
||||
if credential_masking {
|
||||
RemoteCredentialMasking::WithConfiguredProviders
|
||||
} else {
|
||||
RemoteCredentialMasking::Disabled
|
||||
},
|
||||
)
|
||||
.await
|
||||
.unwrap()
|
||||
.proxy
|
||||
@@ -242,7 +304,7 @@ async fn remote_credential_mappings_follow_the_environment_and_broker_config() {
|
||||
..NetworkProxyConfig::default()
|
||||
})
|
||||
.unwrap();
|
||||
proxy.credential_masking = true;
|
||||
proxy.enable_credential_masking(Default::default());
|
||||
NetworkProxyState::from_remote_launch_config(
|
||||
RemoteNetworkProxyLaunchConfig::new(proxy)
|
||||
.for_execution(environment_id.to_string(), "execution".to_string())
|
||||
@@ -371,7 +433,7 @@ async fn local_binding_is_resolved_for_each_executor() -> anyhow::Result<()> {
|
||||
.await?;
|
||||
assert_eq!(
|
||||
proxy
|
||||
.remote_launch_config(policy, /*credential_masking*/ false)
|
||||
.remote_launch_config(policy, RemoteCredentialMasking::Disabled)
|
||||
.await
|
||||
.ok()
|
||||
.map(|launch| launch.proxy.allow_local_binding),
|
||||
|
||||
Reference in New Issue
Block a user