mirror of
https://github.com/openai/codex.git
synced 2026-10-23 17:48:28 +00:00
feat(core): integrate experimental credential broker
Co-authored-by: Codex noreply@openai.com
This commit is contained in:
committed by
Winston Howes
parent
358575465c
commit
fd3858655b
@@ -355,6 +355,7 @@ pub struct NetworkToml {
|
||||
#[derive(Serialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)]
|
||||
#[schemars(deny_unknown_fields)]
|
||||
pub struct NetworkMitmToml {
|
||||
pub dangerously_allow_plaintext_credential_injection: Option<bool>,
|
||||
#[schemars(with = "Option<BTreeMap<String, NetworkMitmHookToml>>")]
|
||||
pub hooks: Option<IndexMap<String, NetworkMitmHookToml>>,
|
||||
#[schemars(with = "Option<BTreeMap<String, NetworkMitmActionToml>>")]
|
||||
@@ -364,6 +365,7 @@ pub struct NetworkMitmToml {
|
||||
#[derive(Deserialize)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
struct NetworkMitmTomlUnchecked {
|
||||
pub dangerously_allow_plaintext_credential_injection: Option<bool>,
|
||||
pub hooks: Option<IndexMap<String, NetworkMitmHookToml>>,
|
||||
pub actions: Option<IndexMap<String, NetworkMitmActionToml>>,
|
||||
}
|
||||
@@ -417,6 +419,8 @@ impl<'de> Deserialize<'de> for NetworkMitmToml {
|
||||
{
|
||||
let unchecked = NetworkMitmTomlUnchecked::deserialize(deserializer)?;
|
||||
let mitm = Self {
|
||||
dangerously_allow_plaintext_credential_injection: unchecked
|
||||
.dangerously_allow_plaintext_credential_injection,
|
||||
hooks: unchecked.hooks,
|
||||
actions: unchecked.actions,
|
||||
};
|
||||
@@ -551,6 +555,14 @@ impl NetworkToml {
|
||||
config.network.allow_local_binding = allow_local_binding;
|
||||
}
|
||||
if let Some(mitm) = self.mitm.as_ref() {
|
||||
if let Some(dangerously_allow_plaintext_credential_injection) =
|
||||
mitm.dangerously_allow_plaintext_credential_injection
|
||||
{
|
||||
config
|
||||
.network
|
||||
.dangerously_allow_plaintext_credential_injection =
|
||||
dangerously_allow_plaintext_credential_injection;
|
||||
}
|
||||
config.network.mitm_hooks = mitm.to_runtime_hooks(mitm.actions.as_ref());
|
||||
}
|
||||
config.network.mitm =
|
||||
|
||||
@@ -1895,6 +1895,9 @@
|
||||
},
|
||||
"type": "object"
|
||||
},
|
||||
"dangerously_allow_plaintext_credential_injection": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"hooks": {
|
||||
"additionalProperties": {
|
||||
"$ref": "#/definitions/NetworkMitmHookToml"
|
||||
@@ -1920,6 +1923,9 @@
|
||||
"allow_upstream_proxy": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"credential_broker": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"dangerously_allow_all_unix_sockets": {
|
||||
"type": "boolean"
|
||||
},
|
||||
|
||||
@@ -1078,6 +1078,7 @@ strip_request_headers = ["authorization"]
|
||||
unix_sockets: None,
|
||||
allow_local_binding: None,
|
||||
mitm: Some(NetworkMitmToml {
|
||||
dangerously_allow_plaintext_credential_injection: None,
|
||||
hooks: Some(IndexMap::from([(
|
||||
"github_write".to_string(),
|
||||
NetworkMitmHookToml {
|
||||
@@ -1159,6 +1160,7 @@ fn permissions_profile_network_to_proxy_config_preserves_mitm_hooks() {
|
||||
let network = NetworkToml {
|
||||
mode: Some(NetworkMode::Full),
|
||||
mitm: Some(NetworkMitmToml {
|
||||
dangerously_allow_plaintext_credential_injection: None,
|
||||
hooks: Some(IndexMap::from([(
|
||||
"github_write".to_string(),
|
||||
NetworkMitmHookToml {
|
||||
|
||||
@@ -189,6 +189,9 @@ pub(crate) fn apply_network_proxy_feature_config(
|
||||
mitm: None,
|
||||
}
|
||||
.apply_to_network_proxy_config(config);
|
||||
if let Some(credential_broker) = feature_config.credential_broker {
|
||||
config.set_credential_broker_enabled(credential_broker);
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn resolve_permission_profile(
|
||||
|
||||
@@ -238,6 +238,14 @@ impl NetworkConfigAccumulator {
|
||||
network.apply_to_network_proxy_config(&mut self.config);
|
||||
|
||||
if let Some(mitm) = mitm {
|
||||
if let Some(dangerously_allow_plaintext_credential_injection) =
|
||||
mitm.dangerously_allow_plaintext_credential_injection
|
||||
{
|
||||
self.config
|
||||
.network
|
||||
.dangerously_allow_plaintext_credential_injection =
|
||||
dangerously_allow_plaintext_credential_injection;
|
||||
}
|
||||
if let Some(actions) = mitm.actions {
|
||||
self.mitm_actions.extend(actions);
|
||||
}
|
||||
@@ -251,6 +259,7 @@ impl NetworkConfigAccumulator {
|
||||
if !self.mitm_hooks.is_empty() {
|
||||
let actions = self.mitm_actions;
|
||||
let mitm = NetworkMitmToml {
|
||||
dangerously_allow_plaintext_credential_injection: None,
|
||||
hooks: Some(self.mitm_hooks),
|
||||
actions: Some(actions.clone()),
|
||||
};
|
||||
|
||||
@@ -187,6 +187,33 @@ strip_request_headers = ["x-api-key"]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn profile_network_mitm_applies_plaintext_credential_injection_flag() {
|
||||
let config: toml::Value = toml::from_str(
|
||||
r#"
|
||||
default_permissions = "workspace"
|
||||
|
||||
[permissions.workspace.network.mitm]
|
||||
dangerously_allow_plaintext_credential_injection = true
|
||||
"#,
|
||||
)
|
||||
.expect("permissions profile should parse");
|
||||
let mut accumulator = NetworkConfigAccumulator::default();
|
||||
accumulator
|
||||
.apply_network_tables(
|
||||
network_tables_from_toml(&config).expect("permissions profile should deserialize"),
|
||||
)
|
||||
.expect("permissions profile should apply");
|
||||
|
||||
let config = accumulator.finish().expect("merged config should build");
|
||||
|
||||
assert!(
|
||||
config
|
||||
.network
|
||||
.dangerously_allow_plaintext_credential_injection
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn execpolicy_network_rules_overlay_network_lists() {
|
||||
let mut config = NetworkProxyConfig::default();
|
||||
|
||||
@@ -88,7 +88,13 @@ pub(crate) fn is_managed_proxy_env_var(key: &str, value: &str) -> bool {
|
||||
}
|
||||
|
||||
pub(crate) fn strip_managed_proxy_env(env: &mut HashMap<String, String>) {
|
||||
env.retain(|key, value| !is_managed_proxy_env_var(key, value));
|
||||
// Escalated commands bypass the credential broker. Remove only values that still match this
|
||||
// child's broker-generated dummies, while preserving credentials replaced by the user.
|
||||
let brokered_credential_dummy_env_keys =
|
||||
codex_network_proxy::brokered_credential_dummy_env_keys(env);
|
||||
env.retain(|key, value| {
|
||||
!brokered_credential_dummy_env_keys.contains(key) && !is_managed_proxy_env_var(key, value)
|
||||
});
|
||||
}
|
||||
|
||||
/// Prepends `path_entry` to `PATH`, removing duplicate and empty existing
|
||||
@@ -296,7 +302,7 @@ pub(crate) fn maybe_wrap_shell_lc_with_snapshot(
|
||||
// Do not let a snapshot resurrect a stale profile when no named profile is active.
|
||||
let (override_captures, override_exports) =
|
||||
build_override_exports(&override_env, &[CODEX_PERMISSION_PROFILE_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]);
|
||||
@@ -334,10 +340,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<_>>();
|
||||
|
||||
@@ -7,6 +7,7 @@ use crate::tools::sandboxing::SandboxAttempt;
|
||||
use crate::tools::sandboxing::managed_network_for_sandbox_permissions;
|
||||
#[cfg(target_os = "macos")]
|
||||
use codex_network_proxy::CODEX_PROXY_GIT_SSH_COMMAND_MARKER;
|
||||
use codex_network_proxy::CREDENTIAL_BROKER_ACTIVE_ENV_KEY;
|
||||
use codex_network_proxy::CUSTOM_CA_ENV_KEYS;
|
||||
use codex_network_proxy::ConfigReloader;
|
||||
use codex_network_proxy::ConfigReloaderFuture;
|
||||
@@ -63,10 +64,10 @@ fn shell_with_snapshot(
|
||||
}
|
||||
|
||||
async fn test_network_proxy() -> anyhow::Result<NetworkProxy> {
|
||||
let state = codex_network_proxy::build_config_state(
|
||||
NetworkProxyConfig::default(),
|
||||
NetworkProxyConstraints::default(),
|
||||
)?;
|
||||
let mut config = NetworkProxyConfig::default();
|
||||
config.set_credential_broker_enabled(true);
|
||||
let state =
|
||||
codex_network_proxy::build_config_state(config, NetworkProxyConstraints::default())?;
|
||||
NetworkProxy::builder()
|
||||
.state(Arc::new(NetworkProxyState::with_reloader(
|
||||
state,
|
||||
@@ -85,8 +86,20 @@ async fn explicit_escalation_prepares_exec_without_managed_network() -> anyhow::
|
||||
let dir = tempdir().expect("create temp dir");
|
||||
let command_cwd = dir.path().join("command").abs();
|
||||
let native_sandbox_policy_cwd = dir.path().join("sandbox-policy").abs();
|
||||
let mut env = HashMap::from([("CUSTOM_ENV".to_string(), "kept".to_string())]);
|
||||
let github_token = "github_pat_11AA0bbCC_abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGH";
|
||||
let openai_api_key = "sk-proj-abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_";
|
||||
let mut env = HashMap::from([
|
||||
("CUSTOM_ENV".to_string(), "kept".to_string()),
|
||||
("GH_TOKEN".to_string(), github_token.to_string()),
|
||||
("OPENAI_API_KEY".to_string(), openai_api_key.to_string()),
|
||||
]);
|
||||
proxy.apply_to_env(&mut env);
|
||||
assert_ne!(env.get("GH_TOKEN").map(String::as_str), Some(github_token));
|
||||
assert_ne!(
|
||||
env.get("OPENAI_API_KEY").map(String::as_str),
|
||||
Some(openai_api_key)
|
||||
);
|
||||
env.insert("OPENAI_API_KEY".to_string(), "sk-user-override".to_string());
|
||||
|
||||
let command = vec!["/bin/echo".to_string(), "ok".to_string()];
|
||||
let command = build_sandbox_command(
|
||||
@@ -147,6 +160,11 @@ async fn explicit_escalation_prepares_exec_without_managed_network() -> anyhow::
|
||||
}
|
||||
#[cfg(target_os = "macos")]
|
||||
assert_eq!(exec_request.env.get(PROXY_GIT_SSH_COMMAND_ENV_KEY), None);
|
||||
assert_eq!(exec_request.env.get("GH_TOKEN"), None);
|
||||
assert_eq!(
|
||||
exec_request.env.get("OPENAI_API_KEY"),
|
||||
Some(&"sk-user-override".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
exec_request.env.get("CUSTOM_ENV"),
|
||||
Some(&"kept".to_string())
|
||||
@@ -617,6 +635,7 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_proxy_env_from_process_env() {
|
||||
export PIP_PROXY='http://127.0.0.1:8080'\n\
|
||||
export HTTP_PROXY='http://127.0.0.1:8080'\n\
|
||||
export http_proxy='http://127.0.0.1:8080'\n\
|
||||
export OPENAI_API_KEY='sk-real'\n\
|
||||
export GIT_SSH_COMMAND='ssh -o ProxyCommand=stale'\n",
|
||||
)
|
||||
.expect("write snapshot");
|
||||
@@ -625,7 +644,7 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_proxy_env_from_process_env() {
|
||||
let command = vec![
|
||||
"/bin/bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"printf '%s\\n%s\\n%s\\n%s' \"$PIP_PROXY\" \"$HTTP_PROXY\" \"$http_proxy\" \"$GIT_SSH_COMMAND\""
|
||||
"printf '%s\\n%s\\n%s\\n%s\\n%s' \"$PIP_PROXY\" \"$HTTP_PROXY\" \"$http_proxy\" \"$GIT_SSH_COMMAND\" \"$OPENAI_API_KEY\""
|
||||
.to_string(),
|
||||
];
|
||||
let rewritten = maybe_wrap_shell_lc_with_snapshot(
|
||||
@@ -633,16 +652,18 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_proxy_env_from_process_env() {
|
||||
&session_shell,
|
||||
Some(&shell_snapshot),
|
||||
&HashMap::new(),
|
||||
&HashMap::new(),
|
||||
&HashMap::from([(CREDENTIAL_BROKER_ACTIVE_ENV_KEY.into(), "1".into())]),
|
||||
&RuntimePathPrepends::default(),
|
||||
);
|
||||
let output = Command::new(&rewritten[0])
|
||||
.args(&rewritten[1..])
|
||||
.env(PROXY_ACTIVE_ENV_KEY, "1")
|
||||
.env(CREDENTIAL_BROKER_ACTIVE_ENV_KEY, "1")
|
||||
.env("PIP_PROXY", "http://127.0.0.1:4321")
|
||||
.env("HTTP_PROXY", "http://127.0.0.1:4321")
|
||||
.env("http_proxy", "http://127.0.0.1:4321")
|
||||
.env("GIT_SSH_COMMAND", "ssh -o ProxyCommand=fresh")
|
||||
.env("OPENAI_API_KEY", "sk-brokered")
|
||||
.output()
|
||||
.expect("run rewritten command");
|
||||
|
||||
@@ -652,10 +673,17 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_proxy_env_from_process_env() {
|
||||
"http://127.0.0.1:4321\n\
|
||||
http://127.0.0.1:4321\n\
|
||||
http://127.0.0.1:4321\n\
|
||||
ssh -o ProxyCommand=stale"
|
||||
ssh -o ProxyCommand=stale\n\
|
||||
sk-brokered"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn broker_inactive_snapshot_exports_omit_credentials() {
|
||||
let (captures, restores) = build_proxy_env_exports(&HashMap::new());
|
||||
assert!(!format!("{captures}{restores}").contains("OPENAI_API_KEY"));
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
#[test]
|
||||
fn maybe_wrap_shell_lc_with_snapshot_refreshes_codex_proxy_git_ssh_command() {
|
||||
|
||||
@@ -216,11 +216,14 @@ pub struct NetworkProxyConfigToml {
|
||||
pub unix_sockets: Option<BTreeMap<String, NetworkProxyUnixSocketPermissionToml>>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub allow_local_binding: Option<bool>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub credential_broker: Option<bool>,
|
||||
}
|
||||
|
||||
impl FeatureConfig for NetworkProxyConfigToml {
|
||||
fn enabled(&self) -> Option<bool> {
|
||||
self.enabled
|
||||
.or(self.credential_broker.filter(|enabled| *enabled))
|
||||
}
|
||||
|
||||
fn set_enabled(&mut self, enabled: bool) {
|
||||
|
||||
Reference in New Issue
Block a user