mirror of
https://github.com/openai/codex.git
synced 2026-11-06 20:20:34 +00:00
Add credential brokering to network proxy feature config (#40466)
## What changed - Add `features.network_proxy.credential_broker` and preserve structured network proxy settings when toggling the feature through config APIs. - Protect broker enablement and provider context variables from project config, while carrying trusted context into filtered shell environments. - Propagate brokered credentials through shell snapshots and restore real credentials for unsandboxed escalated commands. - Normalize broker context environment keys case-insensitively on Windows and avoid conflicting remote managed-network enforcement when no proxy launch configuration is available. ## Testing - Cover config layering, editing, project sanitization, Windows environment handling, shell snapshots, escalated execution, and end-to-end GitHub credential brokering in the zsh-fork runtime. GitOrigin-RevId: 500e51e804e1988022052ccfe00b8fac5e495f4f
This commit is contained in:
@@ -39,6 +39,8 @@ pub(crate) fn exec_env_for_sandbox_permissions(
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
) -> HashMap<String, String> {
|
||||
let mut env = env.clone();
|
||||
// Escalated commands intentionally use the original, unbrokered environment. This branch is
|
||||
// defensive cleanup for a caller that passes an environment already prepared by Codex.
|
||||
if sandbox_permissions.requires_escalated_permissions()
|
||||
&& env.contains_key(PROXY_ACTIVE_ENV_KEY)
|
||||
{
|
||||
@@ -253,7 +255,7 @@ pub(crate) fn maybe_wrap_shell_lc_with_snapshot(
|
||||
PLUGIN_METRICS_OUTPUT_ENV_VAR,
|
||||
],
|
||||
);
|
||||
let (proxy_captures, proxy_exports) = build_proxy_env_exports();
|
||||
let (proxy_captures, proxy_exports) = build_proxy_env_exports(env);
|
||||
let runtime_path_prepend_exports =
|
||||
runtime_path_prepends.shell_exports_after_snapshot(explicit_env_overrides);
|
||||
let override_captures = join_shell_blocks([override_captures, proxy_captures]);
|
||||
@@ -292,10 +294,11 @@ fn build_override_exports(
|
||||
build_override_exports_for_keys("__CODEX_SNAPSHOT_OVERRIDE", &keys)
|
||||
}
|
||||
|
||||
fn build_proxy_env_exports() -> (String, String) {
|
||||
fn build_proxy_env_exports(env: &HashMap<String, String>) -> (String, String) {
|
||||
let mut keys = PROXY_ENV_KEYS
|
||||
.iter()
|
||||
.copied()
|
||||
.chain(codex_network_proxy::brokered_credential_env_keys(env))
|
||||
.chain(CUSTOM_CA_ENV_KEYS)
|
||||
.filter(|key| is_valid_shell_variable_name(key))
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
@@ -63,10 +63,20 @@ fn shell_with_snapshot(
|
||||
}
|
||||
|
||||
async fn test_network_proxy() -> anyhow::Result<NetworkProxy> {
|
||||
let state = codex_network_proxy::build_config_state(
|
||||
NetworkProxyConfig::default(),
|
||||
NetworkProxyConstraints::default(),
|
||||
)?;
|
||||
test_network_proxy_with_config(NetworkProxyConfig::default()).await
|
||||
}
|
||||
|
||||
pub(super) async fn test_credential_broker_network_proxy() -> anyhow::Result<NetworkProxy> {
|
||||
let mut config = NetworkProxyConfig::default();
|
||||
config.set_credential_broker_enabled(/*enabled*/ true);
|
||||
test_network_proxy_with_config(config).await
|
||||
}
|
||||
|
||||
async fn test_network_proxy_with_config(
|
||||
config: NetworkProxyConfig,
|
||||
) -> anyhow::Result<NetworkProxy> {
|
||||
let state =
|
||||
codex_network_proxy::build_config_state(config, NetworkProxyConstraints::default())?;
|
||||
NetworkProxy::builder()
|
||||
.state(Arc::new(NetworkProxyState::with_reloader(
|
||||
state,
|
||||
@@ -737,6 +747,54 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_proxy_env_from_process_env() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn snapshot_wrapper_replays_dummy_and_preserves_unbrokered_credentials() -> anyhow::Result<()>
|
||||
{
|
||||
let proxy = test_credential_broker_network_proxy().await?;
|
||||
let dir = tempdir()?;
|
||||
let snapshot = dir.path().join("snapshot.sh");
|
||||
std::fs::write(
|
||||
&snapshot,
|
||||
"# Snapshot file\nexport OPENAI_API_KEY='stale'\nexport GITHUB_TOKEN='ghp_snapshot_only'\nexport GH_HOST='github.example.com'\n",
|
||||
)?;
|
||||
let (shell, snapshot) = shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot.abs());
|
||||
let mut env = HashMap::from([
|
||||
(
|
||||
"OPENAI_API_KEY".to_string(),
|
||||
"sk-proj-abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_".to_string(),
|
||||
),
|
||||
("GITHUB_TOKEN".to_string(), String::new()),
|
||||
]);
|
||||
proxy.apply_to_env(&mut env);
|
||||
let dummy = env["OPENAI_API_KEY"].clone();
|
||||
let command = vec![
|
||||
"/bin/bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"printf '%s\\n%s\\n%s' \"$OPENAI_API_KEY\" \"${GITHUB_TOKEN-unset}\" \"$GH_HOST\""
|
||||
.to_string(),
|
||||
];
|
||||
let rewritten = maybe_wrap_shell_lc_with_snapshot(
|
||||
&command,
|
||||
&shell,
|
||||
Some(&snapshot),
|
||||
&HashMap::new(),
|
||||
&env,
|
||||
&RuntimePathPrepends::default(),
|
||||
);
|
||||
let output = Command::new(&rewritten[0])
|
||||
.args(&rewritten[1..])
|
||||
.env_clear()
|
||||
.envs(env)
|
||||
.output()?;
|
||||
|
||||
assert!(output.status.success(), "command failed: {output:?}");
|
||||
assert_eq!(
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
format!("{dummy}\nghp_snapshot_only\ngithub.example.com")
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
#[test]
|
||||
fn maybe_wrap_shell_lc_with_snapshot_refreshes_codex_proxy_git_ssh_command() {
|
||||
|
||||
@@ -672,7 +672,7 @@ impl CoreShellCommandExecutor {
|
||||
program: &AbsolutePathBuf,
|
||||
argv: &[String],
|
||||
workdir: &AbsolutePathBuf,
|
||||
env: HashMap<String, String>,
|
||||
mut env: HashMap<String, String>,
|
||||
execution: EscalationExecution,
|
||||
) -> anyhow::Result<PreparedExec> {
|
||||
let command = join_program_and_argv(program, argv);
|
||||
@@ -683,12 +683,20 @@ impl CoreShellCommandExecutor {
|
||||
};
|
||||
|
||||
let prepared = match execution {
|
||||
EscalationExecution::Unsandboxed => PreparedExec {
|
||||
command,
|
||||
cwd: workdir.to_path_buf(),
|
||||
env: exec_env_for_sandbox_permissions(&env, SandboxPermissions::RequireEscalated),
|
||||
arg0: Some(first_arg.clone()),
|
||||
},
|
||||
EscalationExecution::Unsandboxed => {
|
||||
if let Some(network) = self.network.as_ref() {
|
||||
network.restore_brokered_credentials(&mut env, &mut []);
|
||||
}
|
||||
PreparedExec {
|
||||
command,
|
||||
cwd: workdir.to_path_buf(),
|
||||
env: exec_env_for_sandbox_permissions(
|
||||
&env,
|
||||
SandboxPermissions::RequireEscalated,
|
||||
),
|
||||
arg0: Some(first_arg.clone()),
|
||||
}
|
||||
}
|
||||
EscalationExecution::TurnDefault => {
|
||||
self.prepare_sandboxed_exec(PrepareSandboxedExecParams {
|
||||
command,
|
||||
|
||||
@@ -10,6 +10,7 @@ use crate::config::Constrained;
|
||||
use crate::guardian::GuardianReviewContext;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::session::tests::make_session_and_context;
|
||||
use crate::tools::runtimes::tests::test_credential_broker_network_proxy;
|
||||
use anyhow::Context;
|
||||
use codex_execpolicy::Decision;
|
||||
use codex_execpolicy::Evaluation;
|
||||
@@ -355,13 +356,14 @@ fn shell_request_escalation_execution_is_explicit() {
|
||||
#[tokio::test]
|
||||
async fn unsandboxed_intercepted_exec_strips_managed_network_env() -> anyhow::Result<()> {
|
||||
let workdir = test_sandbox_cwd();
|
||||
let network = test_credential_broker_network_proxy().await?;
|
||||
let executor = CoreShellCommandExecutor {
|
||||
command: Vec::new(),
|
||||
cwd: workdir.clone(),
|
||||
permission_profile: PermissionProfile::workspace_write(),
|
||||
sandbox: SandboxType::None,
|
||||
env: HashMap::new(),
|
||||
network: None,
|
||||
network: Some(network.clone()),
|
||||
network_environment_id: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
arg0: None,
|
||||
@@ -371,15 +373,24 @@ async fn unsandboxed_intercepted_exec_strips_managed_network_env() -> anyhow::Re
|
||||
use_legacy_landlock: false,
|
||||
};
|
||||
let mut env = HashMap::new();
|
||||
env.insert("GH_TOKEN".to_string(), "ghp-real".to_string());
|
||||
env.insert(PROXY_ACTIVE_ENV_KEY.to_string(), "1".to_string());
|
||||
for key in PROXY_ENV_KEYS {
|
||||
env.insert((*key).to_string(), format!("proxy-{key}"));
|
||||
}
|
||||
network.apply_to_env(&mut env);
|
||||
assert_ne!(env.get("GH_TOKEN").map(String::as_str), Some("ghp-real"));
|
||||
let dummy = env.get("GH_TOKEN").expect("brokered token").clone();
|
||||
|
||||
let prepared = executor
|
||||
.prepare_escalated_exec(
|
||||
&AbsolutePathBuf::from_absolute_path("/usr/bin/curl")?,
|
||||
&["curl".to_string(), "example.com".to_string()],
|
||||
&[
|
||||
"curl".to_string(),
|
||||
"-H".to_string(),
|
||||
format!("Authorization: Bearer {dummy}"),
|
||||
"example.com".to_string(),
|
||||
],
|
||||
&workdir,
|
||||
env,
|
||||
EscalationExecution::Unsandboxed,
|
||||
@@ -396,6 +407,19 @@ async fn unsandboxed_intercepted_exec_strips_managed_network_env() -> anyhow::Re
|
||||
"unsandboxed intercepted exec should strip managed-network proxy env var {key}"
|
||||
);
|
||||
}
|
||||
assert_eq!(
|
||||
prepared.env.get("GH_TOKEN").map(String::as_str),
|
||||
Some("ghp-real")
|
||||
);
|
||||
assert_eq!(
|
||||
prepared.command,
|
||||
vec![
|
||||
"/usr/bin/curl".to_string(),
|
||||
"-H".to_string(),
|
||||
format!("Authorization: Bearer {dummy}"),
|
||||
"example.com".to_string(),
|
||||
]
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user