mirror of
https://github.com/openai/codex.git
synced 2026-10-21 17:48:44 +00:00
Enable credential masking for remote exec-server commands (#54309)
## What changed Negotiate executor support for `credential_masking` and enable built-in credential brokerage in executor-local network proxies. Supported non-login Bash commands on Linux and macOS receive credential placeholders, with real credentials injected by the proxy for matching destinations. Retain credential mappings across process launches within the same executor session, scoped by environment, conversation, and broker configuration. Keep credentials and destination context on the executor. Fall back to unmasked execution for unsupported shells or shell startup, and for configured credential providers. Emit warnings when masking is unavailable, preserving shell profiles and snapshots. ## Testing Add coverage for capability negotiation, fallback behavior, mapping reuse and isolation, proxy credential injection, and keeping executor credentials out of model-visible request history. Test warning, removal, and rejection modes for credentials left unmasked during preparation. GitOrigin-RevId: f64e916a5de8350fa0d289d3440ec29761accd75
This commit is contained in:
@@ -161,6 +161,7 @@ struct Inner {
|
||||
notifications: std::sync::RwLock<Option<RpcNotificationSender>>,
|
||||
requests: Arc<std::sync::RwLock<Option<RpcServerRequestSender>>>,
|
||||
processes: Mutex<HashMap<ProcessId, ProcessEntry>>,
|
||||
credential_brokers: codex_network_proxy::RemoteCredentialBrokerCache,
|
||||
#[cfg(unix)]
|
||||
shell_snapshots: crate::shell_snapshot::ShellSnapshotCache,
|
||||
telemetry: ExecServerTelemetry,
|
||||
@@ -220,6 +221,7 @@ impl LocalProcess {
|
||||
notifications: std::sync::RwLock::new(Some(notifications)),
|
||||
requests: Arc::new(std::sync::RwLock::new(Some(requests))),
|
||||
processes: Mutex::new(HashMap::new()),
|
||||
credential_brokers: codex_network_proxy::RemoteCredentialBrokerCache::default(),
|
||||
#[cfg(unix)]
|
||||
shell_snapshots: crate::shell_snapshot::ShellSnapshotCache::default(),
|
||||
telemetry,
|
||||
@@ -368,6 +370,7 @@ impl LocalProcess {
|
||||
self.runtime_paths.as_ref(),
|
||||
network_policy_decider,
|
||||
network_policy_audit_observer,
|
||||
&self.inner.credential_brokers,
|
||||
&telemetry,
|
||||
)
|
||||
.await?;
|
||||
@@ -838,6 +841,7 @@ impl ExecBackend for LocalProcess {
|
||||
self.runtime_paths.as_ref(),
|
||||
/*network_policy_decider*/ None,
|
||||
/*network_policy_audit_observer*/ None,
|
||||
&self.inner.credential_brokers,
|
||||
&ProcessTelemetry::default(),
|
||||
)
|
||||
.await
|
||||
@@ -1928,6 +1932,8 @@ mod tests {
|
||||
let state = NetworkProxyState::from_remote_launch_config(
|
||||
RemoteNetworkProxyLaunchConfig::new(proxy_config),
|
||||
codex_utils_path_uri::Platform::native(),
|
||||
&codex_network_proxy::RemoteCredentialBrokerCache::default(),
|
||||
&HashMap::new(),
|
||||
)
|
||||
.expect("build network proxy state");
|
||||
let proxy = NetworkProxy::builder()
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
use std::collections::HashMap;
|
||||
#[cfg(windows)]
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
|
||||
@@ -7,6 +6,7 @@ use crate::process_telemetry::ProcessTelemetry;
|
||||
use codex_exec_server_protocol::JSONRPCErrorError;
|
||||
use codex_file_system::WindowsSandboxSelection;
|
||||
use codex_network_proxy::CUSTOM_CA_ENV_KEYS;
|
||||
use codex_network_proxy::CredentialMaskingFailureMode;
|
||||
use codex_network_proxy::ManagedNetworkSandboxContext;
|
||||
use codex_network_proxy::ManagedProxyRouting;
|
||||
use codex_network_proxy::NetworkPolicyAuditObserver;
|
||||
@@ -14,7 +14,7 @@ use codex_network_proxy::NetworkPolicyDecider;
|
||||
use codex_network_proxy::NetworkProxy;
|
||||
use codex_network_proxy::NetworkProxyHandle;
|
||||
use codex_network_proxy::NetworkProxyState;
|
||||
use codex_network_proxy::RemoteNetworkProxyLaunchConfig;
|
||||
use codex_network_proxy::RemoteCredentialBrokerCache;
|
||||
use codex_network_proxy::is_managed_mitm_ca_trust_bundle_path;
|
||||
#[cfg(target_os = "windows")]
|
||||
use codex_network_proxy::strip_managed_proxy_env;
|
||||
@@ -46,6 +46,9 @@ use crate::rpc::internal_error;
|
||||
use crate::rpc::invalid_params;
|
||||
use crate::sandbox_selection::select_sandbox;
|
||||
|
||||
const CREDENTIAL_MASKING_FAILURE_MODE: CredentialMaskingFailureMode =
|
||||
CredentialMaskingFailureMode::Warn;
|
||||
|
||||
pub(crate) struct PreparedExecRequest {
|
||||
pub(crate) command: Vec<String>,
|
||||
pub(crate) cwd: AbsolutePathBuf,
|
||||
@@ -93,8 +96,10 @@ pub(crate) async fn prepare_exec_request_with_telemetry(
|
||||
runtime_paths: Option<&ExecServerRuntimeOptions>,
|
||||
network_policy_decider: Option<Arc<dyn NetworkPolicyDecider>>,
|
||||
network_policy_audit_observer: Option<NetworkPolicyAuditObserver>,
|
||||
credential_brokers: &RemoteCredentialBrokerCache,
|
||||
telemetry: &ProcessTelemetry,
|
||||
) -> Result<PreparedExecRequest, JSONRPCErrorError> {
|
||||
let network_proxy = params.network_proxy.clone();
|
||||
if let Some(sandbox) = params.sandbox.as_ref()
|
||||
&& sandbox.windows_sandbox_selection == WindowsSandboxSelection::Mxc
|
||||
{
|
||||
@@ -120,10 +125,19 @@ pub(crate) async fn prepare_exec_request_with_telemetry(
|
||||
}
|
||||
None
|
||||
} else {
|
||||
params.network_proxy.as_ref()
|
||||
network_proxy
|
||||
};
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
let network_proxy = params.network_proxy.as_ref();
|
||||
let network_proxy = network_proxy
|
||||
.map(|launch| {
|
||||
NetworkProxyState::from_remote_launch_config(
|
||||
launch,
|
||||
codex_utils_path_uri::Platform::native(),
|
||||
credential_brokers,
|
||||
&env,
|
||||
)
|
||||
})
|
||||
.transpose()
|
||||
.map_err(|err| invalid_params(format!("invalid network proxy config: {err}")))?;
|
||||
|
||||
let (env, managed_network, network_proxy_handle, network_proxy_restricting_sid) =
|
||||
prepare_managed_network(
|
||||
@@ -348,7 +362,7 @@ pub(crate) async fn prepare_exec_request_with_telemetry(
|
||||
|
||||
async fn prepare_managed_network(
|
||||
params: &ExecParams,
|
||||
network_proxy: Option<&RemoteNetworkProxyLaunchConfig>,
|
||||
network_proxy: Option<NetworkProxyState>,
|
||||
proxy_private_ips_via_upstream: bool,
|
||||
env: HashMap<String, String>,
|
||||
network_policy_decider: Option<Arc<dyn NetworkPolicyDecider>>,
|
||||
@@ -363,9 +377,18 @@ async fn prepare_managed_network(
|
||||
),
|
||||
JSONRPCErrorError,
|
||||
> {
|
||||
let Some(network_proxy) = network_proxy.cloned() else {
|
||||
let Some(mut state) = network_proxy else {
|
||||
return Ok((env, params.managed_network.clone(), None, None));
|
||||
};
|
||||
let credential_masking = state
|
||||
.current_cfg()
|
||||
.await
|
||||
.map_err(|err| {
|
||||
internal_error(format!(
|
||||
"failed to read executor network proxy config: {err}"
|
||||
))
|
||||
})?
|
||||
.credential_broker;
|
||||
let routing =
|
||||
if params.sandbox.as_ref().is_some_and(|sandbox| {
|
||||
sandbox.windows_sandbox_selection == WindowsSandboxSelection::Mxc
|
||||
@@ -374,11 +397,6 @@ async fn prepare_managed_network(
|
||||
} else {
|
||||
ManagedProxyRouting::SharedIngress
|
||||
};
|
||||
let mut state = NetworkProxyState::from_remote_launch_config(
|
||||
network_proxy,
|
||||
codex_utils_path_uri::Platform::native(),
|
||||
)
|
||||
.map_err(|err| invalid_params(format!("invalid network proxy config: {err}")))?;
|
||||
state.set_proxy_private_ips_via_upstream(proxy_private_ips_via_upstream);
|
||||
if let Some(observer) = network_policy_audit_observer {
|
||||
state.set_policy_audit_observer(observer);
|
||||
@@ -427,11 +445,34 @@ async fn prepare_managed_network(
|
||||
};
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
let network_proxy_restricting_sid = None;
|
||||
let prepared = proxy
|
||||
let supported_shell = matches!(params.argv.as_slice(), [program, flag, _]
|
||||
if Path::new(program).file_name().is_some_and(|name| name == "bash") && flag == "-c")
|
||||
&& !["BASH_ENV", "ENV", "SSH_CLIENT", "SSH2_CLIENT"]
|
||||
.iter()
|
||||
.any(|key| env.contains_key(*key));
|
||||
let mut prepared = proxy
|
||||
.prepare_for_optional_environment(env, /*environment_id*/ None)
|
||||
.map_err(|err| {
|
||||
internal_error(format!("failed to prepare executor network proxy: {err}"))
|
||||
})?;
|
||||
if credential_masking {
|
||||
if cfg!(any(target_os = "linux", target_os = "macos"))
|
||||
&& supported_shell
|
||||
&& params.arg0.is_none()
|
||||
&& params.shell_snapshot.is_none()
|
||||
{
|
||||
proxy
|
||||
.handle_unmasked_credentials(&mut prepared.env, CREDENTIAL_MASKING_FAILURE_MODE)
|
||||
.map_err(|err| invalid_params(err.to_string()))?;
|
||||
} else {
|
||||
tracing::warn!(
|
||||
"Credential masking is unavailable for this shell startup; preserving shell profiles and snapshots and running with unmasked credentials."
|
||||
);
|
||||
// Restore this child's environment without disabling translation of placeholders
|
||||
// issued to earlier commands in the same environment.
|
||||
proxy.restore_and_disable_brokered_credentials(&mut prepared.env, &mut []);
|
||||
}
|
||||
}
|
||||
Ok((
|
||||
prepared.env,
|
||||
Some(prepared.sandbox_context),
|
||||
|
||||
@@ -29,7 +29,6 @@ use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
#[cfg(unix)]
|
||||
use tempfile::tempdir;
|
||||
#[cfg(windows)]
|
||||
use test_case::test_case;
|
||||
use tokio::io::AsyncReadExt;
|
||||
use tokio::io::AsyncWriteExt;
|
||||
@@ -59,6 +58,7 @@ async fn prepare_exec_request(
|
||||
runtime_paths,
|
||||
network_policy_decider,
|
||||
network_policy_audit_observer,
|
||||
&codex_network_proxy::RemoteCredentialBrokerCache::default(),
|
||||
&ProcessTelemetry::default(),
|
||||
)
|
||||
.await
|
||||
@@ -502,8 +502,16 @@ async fn native_request_preserves_native_launch_fields() {
|
||||
assert_eq!(prepared.arg0, params.arg0);
|
||||
}
|
||||
|
||||
#[test_case(false, None, &["-c", "true"]; "unmasked")]
|
||||
#[test_case(true, Some("BASH_ENV"), &["-c", "true"]; "masking_with_startup_hook")]
|
||||
#[test_case(true, Some("SSH_CLIENT"), &["-c", "true"]; "masking_with_remote_shell_startup")]
|
||||
#[test_case(true, None, &["-c", "-l", "true"]; "masking_with_extra_login_option")]
|
||||
#[tokio::test]
|
||||
async fn native_request_handles_remote_proxy_config_for_platform() {
|
||||
async fn native_request_handles_remote_proxy_config_for_platform(
|
||||
credential_masking: bool,
|
||||
startup_env: Option<&str>,
|
||||
shell_args: &[&str],
|
||||
) {
|
||||
let cwd: AbsolutePathBuf = std::env::current_dir()
|
||||
.expect("current directory")
|
||||
.try_into()
|
||||
@@ -513,12 +521,16 @@ async fn native_request_handles_remote_proxy_config_for_platform() {
|
||||
..NetworkProxyConfig::default()
|
||||
};
|
||||
config.set_allowed_domains(vec!["allowed.example".to_string()]);
|
||||
let proxy_config = RemoteNetworkProxyConfig::from_effective_config(&config)
|
||||
let mut proxy_config = RemoteNetworkProxyConfig::from_effective_config(&config)
|
||||
.expect("supported remote proxy config");
|
||||
proxy_config.credential_masking = credential_masking;
|
||||
let params = ExecParams {
|
||||
metadata: Default::default(),
|
||||
process_id: ProcessId::from("process-remote-proxy"),
|
||||
argv: vec!["echo".to_string(), "hello".to_string()],
|
||||
argv: std::iter::once("/bin/bash")
|
||||
.chain(shell_args.iter().copied())
|
||||
.map(str::to_string)
|
||||
.collect(),
|
||||
cwd: PathUri::from_abs_path(&cwd),
|
||||
shell_snapshot: None,
|
||||
env_policy: None,
|
||||
@@ -535,22 +547,38 @@ async fn native_request_handles_remote_proxy_config_for_platform() {
|
||||
),
|
||||
};
|
||||
let stale_proxy = "http://127.0.0.1:9".to_string();
|
||||
let env = HashMap::from([
|
||||
let mut env = HashMap::from([
|
||||
("HTTP_PROXY".to_string(), stale_proxy.clone()),
|
||||
("TEST_ENV".to_string(), "value".to_string()),
|
||||
("PATH".to_string(), "/devbox/bin:/usr/bin:/bin".to_string()),
|
||||
(
|
||||
"OPENAI_API_KEY".to_string(),
|
||||
"sk-proj-executor-only-0000000000000000000000000000000000".to_string(),
|
||||
),
|
||||
(
|
||||
PROXY_ATTRIBUTION_TOKEN_ENV_KEY.to_string(),
|
||||
"foreign-token".to_string(),
|
||||
),
|
||||
]);
|
||||
if let Some(key) = startup_env {
|
||||
env.insert(key.to_string(), "startup-hook".to_string());
|
||||
}
|
||||
|
||||
let prepared = prepare_exec_request(
|
||||
¶ms, env, /*runtime_paths*/ None, /*network_policy_decider*/ None,
|
||||
¶ms,
|
||||
env.clone(),
|
||||
/*runtime_paths*/ None,
|
||||
/*network_policy_decider*/ None,
|
||||
/*network_policy_audit_observer*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("prepare request with executor-local proxy");
|
||||
|
||||
assert_eq!(prepared.command, params.argv);
|
||||
for key in ["PATH", "OPENAI_API_KEY"].into_iter().chain(startup_env) {
|
||||
assert_eq!(prepared.env.get(key), env.get(key), "{key}");
|
||||
}
|
||||
|
||||
if cfg!(target_os = "windows") {
|
||||
assert_eq!(prepared.env.get("HTTP_PROXY"), None);
|
||||
assert_eq!(prepared.env.get("TEST_ENV"), Some(&"value".to_string()));
|
||||
@@ -596,13 +624,18 @@ async fn disabled_remote_proxy_config_is_rejected_before_exporting_ports() {
|
||||
.expect("current directory")
|
||||
.try_into()
|
||||
.expect("absolute cwd");
|
||||
let proxy_config =
|
||||
let mut proxy_config =
|
||||
RemoteNetworkProxyConfig::from_effective_config(&NetworkProxyConfig::default())
|
||||
.expect("serializable disabled proxy config");
|
||||
proxy_config.credential_masking = true;
|
||||
let params = ExecParams {
|
||||
metadata: Default::default(),
|
||||
process_id: ProcessId::from("process-disabled-remote-proxy"),
|
||||
argv: vec!["echo".to_string(), "hello".to_string()],
|
||||
argv: vec![
|
||||
"/bin/bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"true".to_string(),
|
||||
],
|
||||
cwd: PathUri::from_abs_path(&cwd),
|
||||
shell_snapshot: None,
|
||||
env_policy: None,
|
||||
|
||||
@@ -101,6 +101,7 @@ async fn snapshot_failure_retries_are_bounded_and_single_flight(
|
||||
/*runtime_paths*/ None,
|
||||
/*network_policy_decider*/ None,
|
||||
/*network_policy_audit_observer*/ None,
|
||||
&codex_network_proxy::RemoteCredentialBrokerCache::default(),
|
||||
&ProcessTelemetry::default(),
|
||||
)
|
||||
.await
|
||||
@@ -111,6 +112,7 @@ async fn snapshot_failure_retries_are_bounded_and_single_flight(
|
||||
/*runtime_paths*/ None,
|
||||
/*network_policy_decider*/ None,
|
||||
/*network_policy_audit_observer*/ None,
|
||||
&codex_network_proxy::RemoteCredentialBrokerCache::default(),
|
||||
&ProcessTelemetry::default(),
|
||||
)
|
||||
.await
|
||||
|
||||
Reference in New Issue
Block a user