openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-06 15:29:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

Harden project config when credential brokering is active (#40490)

Browse Source 
## Why

Project configuration must not influence credential-provider environment variables or shell startup behavior while credentials are being brokered.

## What changed

- Track credential brokering as unconfigured, disabled, or enabled, and apply effective network proxy requirements before sanitizing project layers.
- When brokering is enabled, ignore project settings for shell snapshots, profile loading, `ZDOTDIR`, `BASH_ENV`, and credential-provider environment variables.
- Preserve those project shell settings when requirements disable credential brokering.

## Testing

Added coverage for active and disabled broker states, protected credential environment variables, and shell startup settings.

GitOrigin-RevId: 6967cbffdfbb60424a2d2324bad5b91bd2ccfcf6
This commit is contained in:
Winston Howes
2026-08-24 22:37:05 +00:00
committed by copyberry
parent 1e9e916923
commit fd1bf50410
3 changed files with 169 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
codex-rs/config/src/loader/local.rs
View File

@@ -1,3 +1,5 @@
use super::CredentialBrokerProjectState;
use super::apply_credential_broker_requirements;
use super::credential_broker_trusted_config;
use super::discover_project_layers;
use super::layer_io;
@@ -15,6 +17,7 @@ use crate::ConfigLayerSource;
use crate::LoaderOverrides;
use crate::RequirementSource;
use crate::RequirementsLayerEntry;
use crate::compose_requirements;
use crate::default_project_root_markers;
use crate::merge_toml_values;
use codex_file_system::ExecutorFileSystem;
@@ -132,7 +135,10 @@ pub(super) async fn load_local_config_layers_with_overrides(
)?;
let project_root_markers = project_root_markers_from_config(&discovery_config)?
.unwrap_or_else(default_project_root_markers);
let trust_context = project_trust_context(
let requirements =
local_requirements_layers(fs, codex_home.as_path(), overrides, loaded_managed.clone())
.await?;
let mut trust_context = project_trust_context(
fs,
&discovery_config,
&credential_broker_trusted_config(&discovery_config, &[], &loaded_managed),
@@ -142,6 +148,23 @@ pub(super) async fn load_local_config_layers_with_overrides(
&user_file,
)
.await?;
if trust_context.credential_broker != CredentialBrokerProjectState::Unconfigured {
let broker_requirements = requirements.clone().project(&[
vec!["features".to_string(), "network_proxy".to_string()],
vec![
"feature_requirements".to_string(),
"network_proxy".to_string(),
],
vec!["experimental_network".to_string(), "enabled".to_string()],
]);
let effective_requirements =
compose_requirements(broker_requirements.layers.into_iter().map(|layer| {
RequirementsLayerEntry::from_toml_value(layer.source, layer.toml)
.with_base_dir(layer.base_dir)
}))?
.unwrap_or_default();
apply_credential_broker_requirements(&mut trust_context, &effective_requirements);
}
let project_layers = discover_project_layers(
fs,
cwd,
@@ -169,9 +192,6 @@ pub(super) async fn load_local_config_layers_with_overrides(
];
append_project_layers(fs, &mut config_layers, project_layers.layers).await?;
let requirements =
local_requirements_layers(fs, codex_home.as_path(), overrides, loaded_managed.clone())
.await?;
append_legacy_config_layers(&mut config_layers, loaded_managed, &codex_home)?;
Ok(LocalConfigLayers {

88
codex-rs/config/src/loader/mod.rs
View File

@@ -39,6 +39,7 @@ use crate::thread_config::ThreadConfigLoader;
use codex_file_system::ExecutorFileSystem;
use codex_git_utils::resolve_root_git_project_for_trust;
use codex_network_proxy::credential_broker_provider_context_env_keys;
use codex_network_proxy::is_credential_broker_provider_env_key;
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::config_types::EnvironmentVariablePattern;
use codex_protocol::config_types::SandboxMode;
@@ -389,7 +390,7 @@ pub async fn load_config_layers_state(
return Err(err);
}
};
let project_trust_context = match project_trust_context(
let mut project_trust_context = match project_trust_context(
fs,
&merged_so_far,
&credential_broker_trusted_config(
@@ -420,6 +421,7 @@ pub async fn load_config_layers_state(
return Err(err);
}
};
apply_credential_broker_requirements(&mut project_trust_context, &config_requirements_toml);
let project_layers = load_project_layers(
fs,
&cwd,
@@ -994,6 +996,38 @@ fn toml_value_from_serializable<T: serde::Serialize>(value: T) -> io::Result<Tom
TomlValue::try_from(value).map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))
}
#[derive(Clone, Copy, PartialEq, Eq)]
enum CredentialBrokerProjectState {
Unconfigured,
Disabled,
Enabled,
}
fn apply_credential_broker_requirements(
context: &mut ProjectTrustContext,
requirements: &crate::ConfigRequirementsWithSources,
) {
if context.credential_broker == CredentialBrokerProjectState::Unconfigured {
return;
}
let enabled = requirements
.feature_requirements
.as_ref()
.and_then(|requirements| requirements.entries.get("network_proxy"))
.copied()
.unwrap_or(context.credential_broker == CredentialBrokerProjectState::Enabled);
context.credential_broker = if enabled
&& requirements
.network
.as_ref()
.is_none_or(|network| network.enabled != Some(false))
{
CredentialBrokerProjectState::Enabled
} else {
CredentialBrokerProjectState::Disabled
};
}
struct ProjectTrustContext {
project_root: AbsolutePathBuf,
project_root_key: String,
@@ -1004,7 +1038,7 @@ struct ProjectTrustContext {
repo_root_lookup_keys: Option<Vec<String>>,
projects_trust: std::collections::HashMap<String, TrustLevel>,
user_config_file: AbsolutePathBuf,
credential_broker_configured: bool,
credential_broker: CredentialBrokerProjectState,
credential_broker_binding_env: HashMap<String, String>,
}
@@ -1123,13 +1157,15 @@ fn project_layer_entry(
fn sanitize_project_config(
config: &mut TomlValue,
credential_broker_configured: bool,
credential_broker: CredentialBrokerProjectState,
trusted_binding_env: &HashMap<String, String>,
) -> Vec<String> {
let Some(table) = config.as_table_mut() else {
return Vec::new();
};
let credential_broker_configured =
credential_broker != CredentialBrokerProjectState::Unconfigured;
let mut ignored_keys = Vec::new();
for key in PROJECT_LOCAL_CONFIG_DENYLIST {
if table.remove(*key).is_some() {
@@ -1137,6 +1173,11 @@ fn sanitize_project_config(
}
}
if let Some(features) = table.get_mut("features").and_then(TomlValue::as_table_mut) {
if credential_broker == CredentialBrokerProjectState::Enabled
&& features.remove("shell_snapshot").is_some()
{
ignored_keys.push("features.shell_snapshot".to_string());
}
if features.remove("respect_system_proxy").is_some() {
ignored_keys.push("features.respect_system_proxy".to_string());
}
@@ -1159,17 +1200,23 @@ fn sanitize_project_config(
}
}
}
if credential_broker_configured
if credential_broker == CredentialBrokerProjectState::Enabled
&& let Some(policy) = table
.get_mut("shell_environment_policy")
.and_then(TomlValue::as_table_mut)
{
if policy.remove("experimental_use_profile").is_some() {
ignored_keys.push("shell_environment_policy.experimental_use_profile".to_string());
}
let binding_keys = credential_broker_provider_context_env_keys().collect::<Vec<_>>();
if let Some(overrides) = policy.get_mut("set").and_then(TomlValue::as_table_mut) {
overrides.retain(|key, _| {
if binding_keys
.iter()
.any(|binding_key| key.eq_ignore_ascii_case(binding_key))
if key.eq_ignore_ascii_case("ZDOTDIR")
|| key.eq_ignore_ascii_case("BASH_ENV")
|| is_credential_broker_provider_env_key(key)
|| binding_keys
.iter()
.any(|binding_key| key.eq_ignore_ascii_case(binding_key))
{
ignored_keys.push(format!("shell_environment_policy.set.{key}"));
false
@@ -1302,13 +1349,26 @@ async fn project_trust_context(
.into_iter()
.filter_map(|(key, project)| project.trust_level.map(|trust_level| (key, trust_level)))
.collect();
let credential_broker_configured = trusted_broker_config
let network_proxy = trusted_broker_config
.get("features")
.and_then(|features| features.get("network_proxy"))
.and_then(TomlValue::as_table)
.is_some_and(|network_proxy| {
network_proxy.get("credential_broker") == Some(&TomlValue::Boolean(true))
});
.and_then(TomlValue::as_table);
let credential_broker_configured = network_proxy.is_some_and(|network_proxy| {
network_proxy.get("credential_broker") == Some(&TomlValue::Boolean(true))
});
let credential_broker = if credential_broker_configured {
if network_proxy
.and_then(|network_proxy| network_proxy.get("enabled"))
.and_then(TomlValue::as_bool)
.unwrap_or(false)
{
CredentialBrokerProjectState::Enabled
} else {
CredentialBrokerProjectState::Disabled
}
} else {
CredentialBrokerProjectState::Unconfigured
};
let credential_broker_binding_env = if credential_broker_configured {
let trusted_policy_overrides = trusted_broker_config
.get("shell_environment_policy")
@@ -1352,7 +1412,7 @@ async fn project_trust_context(
repo_root_lookup_keys,
projects_trust,
user_config_file: user_config_file.clone(),
credential_broker_configured,
credential_broker,
credential_broker_binding_env,
})
}
@@ -1721,7 +1781,7 @@ async fn discover_project_layers(
}
let ignored_project_config_keys = sanitize_project_config(
&mut config,
trust_context.credential_broker_configured,
trust_context.credential_broker,
&trust_context.credential_broker_binding_env,
);
if disabled_reason.is_none() && !ignored_project_config_keys.is_empty() {

81
codex-rs/config/src/loader/tests.rs
View File

@@ -30,7 +30,7 @@ fn project_config_cannot_override_configured_credential_broker_hosts() {
let ignored = sanitize_project_config(
&mut config,
/*credential_broker_configured*/ true,
CredentialBrokerProjectState::Enabled,
&HashMap::new(),
);
@@ -55,12 +55,17 @@ fn project_config_cannot_change_configured_credential_broker_state() {
"[features]\nnetwork_proxy = false",
"[features.network_proxy]\nenabled = true",
"[features.network_proxy]\nenabled = false",
"[features]\nshell_snapshot = true",
"[features]\nshell_snapshot = false",
"[shell_environment_policy]\nexperimental_use_profile = true",
"[shell_environment_policy.set]\nGH_TOKEN = ''",
"[shell_environment_policy.set]\nOPENAI_API_KEY = ''",
] {
let mut config: TomlValue = toml::from_str(project_config).expect("valid project config");
let ignored = sanitize_project_config(
&mut config,
/*credential_broker_configured*/ true,
CredentialBrokerProjectState::Enabled,
&HashMap::new(),
);
@@ -75,9 +80,54 @@ fn project_config_cannot_change_configured_credential_broker_state() {
.is_some_and(|network_proxy| !network_proxy.contains_key("enabled"))
})
);
assert!(
config
.get("features")
.and_then(|features| features.get("shell_snapshot"))
.is_none()
);
assert!(
config
.get("shell_environment_policy")
.and_then(|policy| policy.get("experimental_use_profile"))
.is_none()
);
}
}
#[test]
fn disabled_credential_broker_preserves_project_shell_settings() {
let mut config: TomlValue = toml::from_str(
"[features]\nnetwork_proxy = true\nshell_snapshot = false\n\
[shell_environment_policy]\nexperimental_use_profile = true\n\
[shell_environment_policy.set]\n\
GH_HOST = 'attacker.example'\n\
OPENAI_BASE_URL = 'https://project.example/v1'\n\
ZDOTDIR = '/project-startup'\nBASH_ENV = '/project-startup'",
)
.expect("valid project config");
let ignored = sanitize_project_config(
&mut config,
CredentialBrokerProjectState::Disabled,
&HashMap::new(),
);
assert_eq!(ignored, vec!["features.network_proxy".to_string()]);
assert_eq!(
config,
toml::from_str::<TomlValue>(
"[features]\nshell_snapshot = false\n\
[shell_environment_policy]\nexperimental_use_profile = true\n\
[shell_environment_policy.set]\n\
GH_HOST = 'attacker.example'\n\
OPENAI_BASE_URL = 'https://project.example/v1'\n\
ZDOTDIR = '/project-startup'\nBASH_ENV = '/project-startup'"
)
.expect("valid expected config")
);
}
#[test]
fn project_environment_filters_preserve_credential_host_bindings() {
for (project_config, expected_policy) in [
@@ -107,7 +157,7 @@ fn project_environment_filters_preserve_credential_host_bindings() {
assert!(
sanitize_project_config(
&mut config,
/*credential_broker_configured*/ true,
CredentialBrokerProjectState::Enabled,
&HashMap::new(),
)
.is_empty()
@@ -132,12 +182,16 @@ fn project_environment_filters_preserve_only_trusted_credential_host_bindings()
"filters = { '*' = 'exclude' }",
"include_only = ['GH_ENTERPRISE_TOKEN']",
] {
let mut project: TomlValue =
toml::from_str(&format!("[shell_environment_policy]\n{project_policy}"))
.expect("valid project config");
let mut project: TomlValue = toml::from_str(&format!(
"[shell_environment_policy]\n{project_policy}\n\
[shell_environment_policy.set]\n\
ZDOTDIR = '/untrusted-project-startup'\n\
BASH_ENV = '/untrusted-project-startup'"
))
.expect("valid project config");
sanitize_project_config(
&mut project,
/*credential_broker_configured*/ true,
CredentialBrokerProjectState::Enabled,
&trusted_binding_env,
);
@@ -182,7 +236,7 @@ fn project_config_cannot_bind_permission_shortcuts() {
assert_eq!(
sanitize_project_config(
&mut config,
/*credential_broker_configured*/ false,
CredentialBrokerProjectState::Unconfigured,
&HashMap::new(),
),
[format!("tui.keymap.chat.{key}")]
@@ -829,7 +883,14 @@ async fn local_layers_keep_raw_paths_order_and_legacy_requirements() {
)
};
let user_file = codex_home.join(CONFIG_TOML_FILE);
std::fs::write(&user_file, user_config("trusted")).expect("write user config");
std::fs::write(
&user_file,
format!(
"{}\n[features.network_proxy]\nenabled=true\ncredential_broker=true\n",
user_config("trusted")
),
)
.expect("write user config");
let system_file = system_dir.join(CONFIG_TOML_FILE);
std::fs::write(&system_file, "model_instructions_file = \"./system.md\"")
.expect("write system config");
@@ -847,7 +908,7 @@ async fn local_layers_keep_raw_paths_order_and_legacy_requirements() {
let requirements_file = managed_dir.join("requirements.toml");
std::fs::write(
&requirements_file,
"allowed_sandbox_modes = [\"read-only\"]\nlog_dir = \"./logs\"",
"allowed_sandbox_modes = [\"future-mode\"]\nlog_dir = \"./logs\"",
)
.expect("write system requirements");