mirror of
https://github.com/openai/codex.git
synced 2026-10-24 18:18:41 +00:00
Support remote credential masking with Zsh shell snapshots (#54432)
## Why Remote credential masking previously supported only Bash commands without login startup or shell snapshots, leaving Zsh commands with unmasked credentials. ## What changed - Enable masking for Zsh login commands using shell snapshots on Linux and macOS. - Capture trusted startup profiles per launch, bind the credential broker to the resulting environment, and replace credentials and supported aliases with placeholders before snapshot replay. - Preserve explicit environment overrides, credential unsets, and launcher-managed proxy settings during replay. - Warn and fall back to unmasked startup when a snapshot cannot safely replay credentials; stop execution if protected snapshot restoration fails. ## Testing Extend remote execution tests for inherited and profile-defined credentials, overrides, destination changes, and unsupported-profile fallback. Add a Zsh scenario snapshot showing a credential placeholder in command output. GitOrigin-RevId: 83193d6cbc05f3deb0d6d0b9ae80974b353ce900
This commit is contained in:
@@ -927,6 +927,18 @@ impl NetworkProxy {
|
||||
self.state.current_cfg().await
|
||||
}
|
||||
|
||||
/// Selects cached credentials for the startup environment without changing listeners or policy.
|
||||
/// Restore any existing placeholders before binding, then prepare the environment for execution.
|
||||
pub async fn bind_credential_broker_environment(
|
||||
&self,
|
||||
credential_brokers: &crate::RemoteCredentialBrokerCache,
|
||||
environment: &HashMap<String, String>,
|
||||
) -> Result<()> {
|
||||
self.state
|
||||
.bind_credential_broker_environment(credential_brokers, environment)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Revision of credential configuration, excluding ordinary network policy changes.
|
||||
pub fn credential_broker_config_revision(&self) -> u64 {
|
||||
self.state.credential_broker_config_revision()
|
||||
@@ -1253,7 +1265,8 @@ impl NetworkProxy {
|
||||
let environment_id = self
|
||||
.execution_scope
|
||||
.as_ref()
|
||||
.map(|scope| scope.environment_id.as_str());
|
||||
.map(|scope| scope.environment_id.as_str())
|
||||
.or(self.state.environment_id());
|
||||
self.state
|
||||
.virtualize_snapshot_credentials(env, environment_id);
|
||||
}
|
||||
@@ -1646,6 +1659,7 @@ impl NetworkProxy {
|
||||
*active_route = Some(Arc::clone(&route));
|
||||
drop(active_route);
|
||||
return Ok(NetworkProxyHandle::windows_shared(
|
||||
self.clone(),
|
||||
route,
|
||||
Arc::clone(&windows_runtime.active_route),
|
||||
Arc::clone(&self.environment_proxies),
|
||||
@@ -1720,6 +1734,7 @@ impl NetworkProxy {
|
||||
|
||||
Ok(NetworkProxyHandle {
|
||||
runtime: Some(ProxyRuntime::Listeners(listeners)),
|
||||
proxy: Some(self.clone()),
|
||||
environment_proxies: Some(Arc::clone(&self.environment_proxies)),
|
||||
#[cfg(target_os = "windows")]
|
||||
windows_active_route: None,
|
||||
@@ -1729,6 +1744,7 @@ impl NetworkProxy {
|
||||
|
||||
pub struct NetworkProxyHandle {
|
||||
runtime: Option<ProxyRuntime>,
|
||||
proxy: Option<NetworkProxy>,
|
||||
environment_proxies: Option<Arc<Mutex<HashMap<String, EnvironmentProxy>>>>,
|
||||
#[cfg(target_os = "windows")]
|
||||
windows_active_route: Option<Arc<Mutex<Option<Arc<WindowsProxyRoute>>>>>,
|
||||
@@ -1738,6 +1754,7 @@ impl NetworkProxyHandle {
|
||||
fn noop() -> Self {
|
||||
Self {
|
||||
runtime: None,
|
||||
proxy: None,
|
||||
environment_proxies: None,
|
||||
#[cfg(target_os = "windows")]
|
||||
windows_active_route: None,
|
||||
@@ -1746,17 +1763,24 @@ impl NetworkProxyHandle {
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
fn windows_shared(
|
||||
proxy: NetworkProxy,
|
||||
route: Arc<WindowsProxyRoute>,
|
||||
active_route: Arc<Mutex<Option<Arc<WindowsProxyRoute>>>>,
|
||||
environment_proxies: Arc<Mutex<HashMap<String, EnvironmentProxy>>>,
|
||||
) -> Self {
|
||||
Self {
|
||||
runtime: Some(ProxyRuntime::SharedIngress { route }),
|
||||
proxy: Some(proxy),
|
||||
environment_proxies: Some(environment_proxies),
|
||||
windows_active_route: Some(active_route),
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns the running proxy. Disabled proxy handles return `None`.
|
||||
pub fn proxy(&self) -> Option<&NetworkProxy> {
|
||||
self.proxy.as_ref()
|
||||
}
|
||||
|
||||
pub async fn wait(mut self) -> Result<()> {
|
||||
let result = match self.runtime.as_mut() {
|
||||
Some(runtime) => runtime.wait().await,
|
||||
@@ -1857,6 +1881,7 @@ mod tests {
|
||||
});
|
||||
let handle = NetworkProxyHandle {
|
||||
runtime: Some(ProxyRuntime::Listeners(listeners)),
|
||||
proxy: None,
|
||||
environment_proxies: Some(Arc::new(Mutex::new(HashMap::new()))),
|
||||
#[cfg(target_os = "windows")]
|
||||
windows_active_route: None,
|
||||
|
||||
@@ -226,8 +226,8 @@ fn launch_config_materializes_audit_and_execution_attribution() {
|
||||
assert_eq!(state.execution_id().as_deref(), Some("execution-1"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remote_credential_mappings_follow_the_environment_and_broker_config() {
|
||||
#[tokio::test]
|
||||
async fn remote_credential_mappings_follow_the_environment_and_broker_config() {
|
||||
const TOKEN: &str = "ghp_abcdefghijklmnopqrstuvwxyz0123456789";
|
||||
let brokers = crate::RemoteCredentialBrokerCache::default();
|
||||
let launch = |environment_id: &str,
|
||||
@@ -283,13 +283,20 @@ fn remote_credential_mappings_follow_the_environment_and_broker_config() {
|
||||
"thread",
|
||||
NetworkMode::Full,
|
||||
/*allow_local_binding*/ false,
|
||||
&next_env,
|
||||
&env,
|
||||
);
|
||||
let listener = next.clone();
|
||||
next.bind_credential_broker_environment(&brokers, &next_env)
|
||||
.await
|
||||
.expect("bind startup credential context");
|
||||
next.virtualize_child_credentials(&mut next_env);
|
||||
let mut headers = rama_http::HeaderMap::from_iter([(
|
||||
rama_http::header::AUTHORIZATION,
|
||||
rama_http::HeaderValue::from_str(&format!("Bearer {placeholder}")).unwrap(),
|
||||
)]);
|
||||
let mut listener_headers = headers.clone();
|
||||
listener.inject_request_credentials("github.enterprise.example", &mut listener_headers);
|
||||
assert_eq!(listener_headers, headers);
|
||||
first.inject_request_credentials("github.enterprise.example", &mut headers);
|
||||
assert_eq!(
|
||||
headers[rama_http::header::AUTHORIZATION].to_str().unwrap(),
|
||||
|
||||
@@ -3,6 +3,7 @@ use crate::config::NetworkMode;
|
||||
use crate::config::NetworkProxyConfig;
|
||||
use crate::config::ValidatedUnixSocketPath;
|
||||
use crate::credential_broker::CredentialBroker;
|
||||
use crate::credential_broker::env_value;
|
||||
use crate::domain_matcher::DomainPatternSet;
|
||||
use crate::mitm::MitmState;
|
||||
use crate::mitm_hook::HookEvaluation;
|
||||
@@ -274,7 +275,8 @@ pub struct NetworkProxyState {
|
||||
pub(crate) policy_audit_observer: Option<NetworkPolicyAuditObserver>,
|
||||
pub(crate) launch_span_context: Option<SpanContext>,
|
||||
pub(crate) process_log_metadata: crate::NetworkProxyProcessLogMetadata,
|
||||
credential_broker: CredentialBroker,
|
||||
/// Shared by this proxy's listeners; rebinding leaves other proxies' cached brokers intact.
|
||||
credential_broker: Arc<std::sync::RwLock<CredentialBroker>>,
|
||||
audit_metadata: NetworkProxyAuditMetadata,
|
||||
execution_attributions: Arc<Mutex<HashMap<String, ExecutionAttribution>>>,
|
||||
environment_id: Option<Arc<str>>,
|
||||
@@ -367,7 +369,7 @@ impl NetworkProxyState {
|
||||
)
|
||||
};
|
||||
if let Some(broker) = cached_broker {
|
||||
proxy_state.credential_broker = broker;
|
||||
proxy_state.credential_broker = Arc::new(std::sync::RwLock::new(broker));
|
||||
}
|
||||
Ok(proxy_state)
|
||||
}
|
||||
@@ -420,7 +422,7 @@ impl NetworkProxyState {
|
||||
let credential_broker = CredentialBroker::new(state.config.credential_broker);
|
||||
credential_broker.configure(&local_binding_policy.resolved_config(&state.config));
|
||||
Self {
|
||||
credential_broker,
|
||||
credential_broker: Arc::new(std::sync::RwLock::new(credential_broker)),
|
||||
state: Arc::new(RwLock::new(state)),
|
||||
local_binding_policy,
|
||||
proxy_private_ips_via_upstream: false,
|
||||
@@ -519,6 +521,45 @@ impl NetworkProxyState {
|
||||
&self.audit_metadata
|
||||
}
|
||||
|
||||
fn credential_broker(&self) -> CredentialBroker {
|
||||
self.credential_broker
|
||||
.read()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.clone()
|
||||
}
|
||||
|
||||
pub(crate) async fn bind_credential_broker_environment(
|
||||
&self,
|
||||
credential_brokers: &crate::RemoteCredentialBrokerCache,
|
||||
environment: &HashMap<String, String>,
|
||||
) -> Result<()> {
|
||||
self.reload_if_needed().await?;
|
||||
let mut state = self.state.write().await;
|
||||
let mut config = self.local_binding_policy.resolved_config(&state.config);
|
||||
config.configure_credential_broker_environment(environment);
|
||||
// Captures are complete environments: absent hints are removals, not launch fallbacks.
|
||||
config.credential_broker_context = self
|
||||
.credential_broker_environment(environment)
|
||||
.provider_context_keys
|
||||
.into_iter()
|
||||
.filter_map(|key| env_value(environment, &key).map(|value| (key, value.to_string())))
|
||||
.collect::<HashMap<_, _>>()
|
||||
.into();
|
||||
let broker = credential_brokers
|
||||
.broker_if_enabled(
|
||||
self.environment_id(),
|
||||
self.audit_metadata.conversation_id.as_deref(),
|
||||
&config,
|
||||
)
|
||||
.context("credential brokerage is disabled for the startup environment")?;
|
||||
state.config.credential_broker_context = config.credential_broker_context;
|
||||
*self
|
||||
.credential_broker
|
||||
.write()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner) = broker;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn virtualize_child_credentials(&self, env: &mut HashMap<String, String>) {
|
||||
self.virtualize_child_credentials_for_environment(env, self.environment_id());
|
||||
}
|
||||
@@ -531,10 +572,13 @@ impl NetworkProxyState {
|
||||
let parent_env = std::env::vars_os()
|
||||
.filter_map(|(key, value)| Some((key.into_string().ok()?, value.into_string().ok()?)))
|
||||
.collect();
|
||||
self.credential_broker
|
||||
.discover_parent_credentials_for_environment(&parent_env, env, environment_id);
|
||||
self.credential_broker
|
||||
.virtualize_child_env_for_environment(env, environment_id);
|
||||
let credential_broker = self.credential_broker();
|
||||
credential_broker.discover_parent_credentials_for_environment(
|
||||
&parent_env,
|
||||
env,
|
||||
environment_id,
|
||||
);
|
||||
credential_broker.virtualize_child_env_for_environment(env, environment_id);
|
||||
}
|
||||
|
||||
pub(crate) fn restore_child_credentials(
|
||||
@@ -542,7 +586,7 @@ impl NetworkProxyState {
|
||||
env: &mut HashMap<String, String>,
|
||||
command: &mut [String],
|
||||
) {
|
||||
self.credential_broker.restore_child_env(env, command);
|
||||
self.credential_broker().restore_child_env(env, command);
|
||||
}
|
||||
|
||||
pub(crate) fn virtualize_snapshot_credentials(
|
||||
@@ -550,7 +594,7 @@ impl NetworkProxyState {
|
||||
env: &mut HashMap<String, String>,
|
||||
environment_id: Option<&str>,
|
||||
) {
|
||||
self.credential_broker
|
||||
self.credential_broker()
|
||||
.virtualize_snapshot_env(env, environment_id);
|
||||
}
|
||||
|
||||
@@ -561,7 +605,7 @@ impl NetworkProxyState {
|
||||
snapshot_value: &str,
|
||||
environment_id: Option<&str>,
|
||||
) -> bool {
|
||||
self.credential_broker
|
||||
self.credential_broker()
|
||||
.child_alias_matches(key, value, snapshot_value, environment_id)
|
||||
}
|
||||
|
||||
@@ -570,7 +614,7 @@ impl NetworkProxyState {
|
||||
env: &mut HashMap<String, String>,
|
||||
command: &mut [String],
|
||||
) {
|
||||
self.credential_broker
|
||||
self.credential_broker()
|
||||
.restore_and_disable_child_env(env, command);
|
||||
}
|
||||
|
||||
@@ -579,7 +623,7 @@ impl NetworkProxyState {
|
||||
text: &mut String,
|
||||
env: &HashMap<String, String>,
|
||||
) -> bool {
|
||||
self.credential_broker.virtualize_text(text, env)
|
||||
self.credential_broker().virtualize_text(text, env)
|
||||
}
|
||||
|
||||
pub(crate) fn handle_unmasked_credentials(
|
||||
@@ -587,7 +631,7 @@ impl NetworkProxyState {
|
||||
env: &mut HashMap<String, String>,
|
||||
mode: crate::CredentialMaskingFailureMode,
|
||||
) -> anyhow::Result<()> {
|
||||
self.credential_broker
|
||||
self.credential_broker()
|
||||
.handle_unmasked_credentials(env, mode)
|
||||
}
|
||||
|
||||
@@ -595,7 +639,7 @@ impl NetworkProxyState {
|
||||
&self,
|
||||
env: &HashMap<String, String>,
|
||||
) -> crate::CredentialBrokerEnvironment {
|
||||
self.credential_broker.environment(env)
|
||||
self.credential_broker().environment(env)
|
||||
}
|
||||
|
||||
pub(crate) fn credential_broker_environment_for_text(
|
||||
@@ -603,7 +647,7 @@ impl NetworkProxyState {
|
||||
text: &str,
|
||||
env: &HashMap<String, String>,
|
||||
) -> crate::CredentialBrokerEnvironment {
|
||||
self.credential_broker.environment_for_text(text, env)
|
||||
self.credential_broker().environment_for_text(text, env)
|
||||
}
|
||||
|
||||
pub(crate) fn credential_broker_source_matches_text(
|
||||
@@ -612,7 +656,7 @@ impl NetworkProxyState {
|
||||
source_value: &str,
|
||||
text: &str,
|
||||
) -> bool {
|
||||
self.credential_broker
|
||||
self.credential_broker()
|
||||
.source_matches_text(source, source_value, text)
|
||||
}
|
||||
|
||||
@@ -623,16 +667,20 @@ impl NetworkProxyState {
|
||||
source_env: &HashMap<String, String>,
|
||||
is_allowed: impl Fn(&str) -> bool,
|
||||
) -> bool {
|
||||
self.credential_broker
|
||||
.provider_sources_allowed(value, virtualized, source_env, is_allowed)
|
||||
self.credential_broker().provider_sources_allowed(
|
||||
value,
|
||||
virtualized,
|
||||
source_env,
|
||||
is_allowed,
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn restore_brokered_text(&self, text: &mut String) -> bool {
|
||||
self.credential_broker.restore_text(text)
|
||||
self.credential_broker().restore_text(text)
|
||||
}
|
||||
|
||||
pub fn inject_request_credentials(&self, host: &str, headers: &mut rama_http::HeaderMap) {
|
||||
self.credential_broker
|
||||
self.credential_broker()
|
||||
.inject_request_headers_for_environment(host, headers, self.environment_id());
|
||||
}
|
||||
|
||||
@@ -651,7 +699,7 @@ impl NetworkProxyState {
|
||||
}
|
||||
|
||||
pub(crate) fn credential_broker_config_revision(&self) -> u64 {
|
||||
self.credential_broker.config_revision()
|
||||
self.credential_broker().config_revision()
|
||||
}
|
||||
|
||||
pub(crate) async fn current_cfg_with_brokerage_provenance(
|
||||
@@ -688,7 +736,7 @@ impl NetworkProxyState {
|
||||
Ok(mut new_state) => {
|
||||
{
|
||||
let mut guard = self.state.write().await;
|
||||
self.credential_broker
|
||||
self.credential_broker()
|
||||
.configure(&self.local_binding_policy.resolved_config(&new_state.config));
|
||||
// Log policy diffs without dumping potentially sensitive config values.
|
||||
log_policy_changes(&guard.config, &new_state.config);
|
||||
@@ -710,7 +758,7 @@ impl NetworkProxyState {
|
||||
pub async fn replace_config_state(&self, mut new_state: ConfigState) -> Result<()> {
|
||||
self.reload_if_needed().await?;
|
||||
let mut guard = self.state.write().await;
|
||||
self.credential_broker
|
||||
self.credential_broker()
|
||||
.configure(&self.local_binding_policy.resolved_config(&new_state.config));
|
||||
log_policy_changes(&guard.config, &new_state.config);
|
||||
new_state.blocked = guard.blocked.clone();
|
||||
@@ -1016,8 +1064,9 @@ impl NetworkProxyState {
|
||||
}
|
||||
};
|
||||
|
||||
let credential_broker = self.credential_broker();
|
||||
if credential_destinations.iter().any(|destination| {
|
||||
self.credential_broker.request_matches_hooked_host_alias(
|
||||
credential_broker.request_matches_hooked_host_alias(
|
||||
destination,
|
||||
req.headers(),
|
||||
&hooked_hosts,
|
||||
@@ -1041,7 +1090,7 @@ impl NetworkProxyState {
|
||||
let guard = self.state.read().await;
|
||||
guard.mitm_hooks.contains_key(&normalized_host)
|
||||
};
|
||||
let protocols = self.credential_broker.host_protocols_for_environment(
|
||||
let protocols = self.credential_broker().host_protocols_for_environment(
|
||||
&normalized_host,
|
||||
port,
|
||||
self.environment_id(),
|
||||
@@ -1134,7 +1183,7 @@ impl NetworkProxyState {
|
||||
Some(mut new_state) => {
|
||||
{
|
||||
let mut guard = self.state.write().await;
|
||||
self.credential_broker
|
||||
self.credential_broker()
|
||||
.configure(&self.local_binding_policy.resolved_config(&new_state.config));
|
||||
log_policy_changes(&guard.config, &new_state.config);
|
||||
new_state.blocked = guard.blocked.clone();
|
||||
|
||||
Reference in New Issue
Block a user