mirror of
https://github.com/openai/codex.git
synced 2026-11-10 08:43:33 +00:00
Support configured credential providers across shell snapshots (#44072)
## Why Credential brokerage must retain trusted destination hints even when shell environment policy hides them from child processes. Snapshot replay also needs to preserve credential aliases when tokens are short or captured dummy values change. ## What changed - Add `features.network_proxy.credentials` and preserve provider settings when toggling the proxy. Prevent project configuration from overriding providers or their credential and destination environment variables. - Use provider metadata throughout snapshot capture and replay, and pass hidden destination context to the broker without exposing it in the child environment. - Rewrite aliases containing short credentials or previously captured dummies, respecting environment filters and explicit overrides. - Preserve case-distinct `shell_environment_policy.set` keys so Windows credential ambiguity checks survive configuration merging and permission profile changes. - Restore independent MITM settings when credential brokerage is disabled. ## Testing Add regression coverage for custom providers, hidden destination hints, Bash and Zsh alias replay, configuration edits and merging, Windows ambiguity handling, and credential protection during approved execution. GitOrigin-RevId: b2bd935d2d8555be56baf8feb1a8e5b3a0c666bc
This commit is contained in:
1 parent
129fd21687
commit
38cbebaf3f
24 files changed
+1482
-415
No files matched your search
@@ -1926,6 +1926,85 @@ enable_socks5 = false
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn credential_broker_ambiguity_guard_survives_permission_profile_selection()
|
||||
-> anyhow::Result<()> {
|
||||
for (key, other_key, other_value, ambiguous) in [
|
||||
("GH_HOST", "gh_host", "second.example", true),
|
||||
("VENDOR_HOST", "vendor_host", "second.example", true),
|
||||
("VENDOR_PASSWORD", "vendor_password", "second.example", true),
|
||||
("VENDOR_HOST", "vendor_host", "first.example", false),
|
||||
("FOO", "foo", "second.example", false),
|
||||
] {
|
||||
let home = tempdir()?;
|
||||
tokio::fs::write(
|
||||
home.path().join(CONFIG_TOML_FILE),
|
||||
format!(
|
||||
r#"
|
||||
default_permissions = "first"
|
||||
[features.network_proxy]
|
||||
enabled = true
|
||||
credential_broker = true
|
||||
[features.network_proxy.credentials.vendor]
|
||||
env = ["VENDOR_PASSWORD"]
|
||||
patterns = ["pin_[a-z]{{8}}"]
|
||||
url_prefixes = ["https://api.vendor.example"]
|
||||
url_prefix_from_env = "VENDOR_HOST"
|
||||
auth = ["bearer"]
|
||||
[permissions.first]
|
||||
extends = ":workspace"
|
||||
[permissions.first.network]
|
||||
enabled = true
|
||||
[permissions.second]
|
||||
extends = "first"
|
||||
[shell_environment_policy.set]
|
||||
{key} = "first.example"
|
||||
{other_key} = "{other_value}"
|
||||
"#
|
||||
),
|
||||
)
|
||||
.await?;
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(home.path().to_path_buf())
|
||||
.fallback_cwd(Some(home.path().to_path_buf()))
|
||||
.loader_overrides(LoaderOverrides::without_managed_config_for_tests())
|
||||
.build()
|
||||
.await?;
|
||||
let expected_enabled = !(cfg!(windows) && ambiguous);
|
||||
assert_eq!(
|
||||
config.permissions.shell_environment_policy.r#set,
|
||||
HashMap::from([
|
||||
(key.to_string(), "first.example".to_string()),
|
||||
(other_key.to_string(), other_value.to_string()),
|
||||
])
|
||||
);
|
||||
assert_eq!(
|
||||
config
|
||||
.permissions
|
||||
.network
|
||||
.as_ref()
|
||||
.unwrap()
|
||||
.credential_broker_enabled(),
|
||||
expected_enabled,
|
||||
"{key}"
|
||||
);
|
||||
let mut selected = config.permissions.active_permission_profile().unwrap();
|
||||
selected.id = "second".to_string();
|
||||
let rebuilt = config
|
||||
.network_proxy_spec_for_active_permission_profile(
|
||||
&selected,
|
||||
config.permissions.permission_profile(),
|
||||
)?
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
rebuilt.credential_broker_enabled(),
|
||||
expected_enabled,
|
||||
"{key}"
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resolve_permission_profile_inherits_across_configured_and_managed_profiles()
|
||||
-> anyhow::Result<()> {
|
||||
|
||||
@@ -349,7 +349,10 @@ impl ConfigDocument {
|
||||
item.as_table_like()?.get(segment)
|
||||
})
|
||||
.and_then(TomlItem::as_table_like)
|
||||
.is_some_and(|feature| feature.contains_key("credential_broker"));
|
||||
.is_some_and(|feature| {
|
||||
feature.contains_key("credential_broker")
|
||||
|| feature.contains_key("credentials")
|
||||
});
|
||||
if preserves_broker_settings {
|
||||
let mut enabled_segments = segments.clone();
|
||||
enabled_segments.push("enabled".to_string());
|
||||
|
||||
@@ -179,6 +179,11 @@ fn network_proxy_feature_toggle_preserves_credential_broker_configuration() {
|
||||
"[features.network_proxy]\nenabled = false\ncredential_broker = true\ndomains = { \"github.com\" = \"allow\" }\n",
|
||||
"[features.network_proxy]\nenabled = true\ncredential_broker = true\ndomains = { \"github.com\" = \"allow\" }\n",
|
||||
),
|
||||
(
|
||||
"[features.network_proxy]\nenabled = true\ncredentials = { vendor = { env = [\"VENDOR_TOKEN\"] } }\n",
|
||||
"[features.network_proxy]\nenabled = false\ncredentials = { vendor = { env = [\"VENDOR_TOKEN\"] } }\n",
|
||||
"[features.network_proxy]\nenabled = true\ncredentials = { vendor = { env = [\"VENDOR_TOKEN\"] } }\n",
|
||||
),
|
||||
] {
|
||||
std::fs::write(&config_path, initial).expect("write config");
|
||||
for (enabled, expected) in [(false, disabled), (true, enabled_again)] {
|
||||
|
||||
@@ -563,10 +563,12 @@ fn profile_allows_configured_network_proxy(permission_profile: &PermissionProfil
|
||||
}
|
||||
|
||||
fn build_network_proxy_spec(
|
||||
configured_network_proxy_config: NetworkProxyConfig,
|
||||
mut configured_network_proxy_config: NetworkProxyConfig,
|
||||
network_requirements: Option<Sourced<codex_config::NetworkConstraints>>,
|
||||
permission_profile: &PermissionProfile,
|
||||
environment_overrides: &HashMap<String, String>,
|
||||
) -> std::io::Result<Option<NetworkProxySpec>> {
|
||||
configured_network_proxy_config.configure_credential_broker_environment(environment_overrides);
|
||||
let (network_requirements, network_requirements_source) = match network_requirements {
|
||||
Some(Sourced { value, source }) => (Some(value), Some(source)),
|
||||
None => (None, None),
|
||||
@@ -3750,7 +3752,7 @@ impl Config {
|
||||
})?
|
||||
.clone();
|
||||
|
||||
let shell_environment_policy = cfg.shell_environment_policy.into();
|
||||
let shell_environment_policy = ShellEnvironmentPolicy::from(cfg.shell_environment_policy);
|
||||
let allow_login_shell = cfg.allow_login_shell.unwrap_or(true);
|
||||
|
||||
let history = cfg.history.unwrap_or_default();
|
||||
@@ -4058,6 +4060,7 @@ impl Config {
|
||||
configured_network_proxy_config,
|
||||
network_requirements,
|
||||
&network_permission_profile,
|
||||
&shell_environment_policy.r#set,
|
||||
)?;
|
||||
let mut helper_readable_roots = get_readable_roots_required_for_codex_runtime(
|
||||
&codex_home,
|
||||
@@ -4581,6 +4584,7 @@ impl Config {
|
||||
configured_network_proxy_config,
|
||||
self.config_layer_stack.requirements().network.clone(),
|
||||
permission_profile,
|
||||
&self.permissions.shell_environment_policy.r#set,
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -192,6 +192,9 @@ pub(crate) fn apply_network_proxy_feature_config(
|
||||
if let Some(credential_broker) = feature_config.credential_broker {
|
||||
config.set_credential_broker_enabled(credential_broker);
|
||||
}
|
||||
if let Some(credentials) = feature_config.credentials.as_ref() {
|
||||
config.credential_providers.clone_from(credentials);
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolves a named permission profile and its inherited configuration.
|
||||
|
||||
Reference in new issue
Block a user