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

Make shell filter merges case-sensitive

Browse Source
This commit is contained in:
Abhinav Vedmala
2026-06-17 00:17:25 +00:00
parent 8cec5f1a63
commit ce0ebcff28
6 changed files with 38 additions and 135 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
codex-rs/app-server/src/config_manager_service.rs
View File

@@ -616,26 +616,6 @@ fn value_at_path<'a>(root: &'a TomlValue, segments: &[String]) -> Option<&'a Tom
Some(current)
}
/// Looks up a path according to its config semantics.
///
/// Shell filter patterns compare case-insensitively and are normalized in the
/// merged effective config, while raw layers preserve their original spelling.
/// Match that path segment case-insensitively so equivalent raw and effective
/// paths compare without changing ordinary TOML path semantics.
fn value_at_semantic_path<'a>(root: &'a TomlValue, segments: &[String]) -> Option<&'a TomlValue> {
let [policy, filters, pattern] = segments else {
return value_at_path(root, segments);
};
if policy != "shell_environment_policy" || filters != "filters" {
return value_at_path(root, segments);
}
let filters = value_at_path(root, &segments[..2])?.as_table()?;
filters
.iter()
.find_map(|(candidate, value)| candidate.eq_ignore_ascii_case(pattern).then_some(value))
}
fn override_message(layer: &ConfigLayerSource) -> String {
match layer {
ConfigLayerSource::Mdm { domain, key: _ } => {
@@ -673,10 +653,10 @@ fn compute_override_metadata(
segments: &[String],
) -> Option<OverriddenMetadata> {
let user_value = match layers.get_active_user_layer() {
Some(user_layer) => value_at_semantic_path(&user_layer.config, segments),
Some(user_layer) => value_at_path(&user_layer.config, segments),
None => return None,
};
let effective_value = value_at_semantic_path(effective, segments);
let effective_value = value_at_path(effective, segments);
if user_value.is_some() && user_value == effective_value {
return None;
@@ -716,9 +696,7 @@ fn find_effective_layer(
segments: &[String],
) -> Option<ConfigLayerMetadata> {
for layer in layers.layers_high_to_low() {
if let Some(meta) =
value_at_semantic_path(&layer.config, segments).map(|_| layer.metadata())
{
if let Some(meta) = value_at_path(&layer.config, segments).map(|_| layer.metadata()) {
return Some(meta);
}
}

62
codex-rs/app-server/src/config_manager_service_tests.rs
View File

@@ -841,68 +841,6 @@ async fn write_value_reports_managed_override() {
assert_eq!(overridden.effective_value, serde_json::json!("never"));
}
#[tokio::test]
async fn write_value_matches_shell_filter_paths_case_insensitively() -> Result<()> {
let tmp = tempdir()?;
let config_path = tmp.path().join(CONFIG_TOML_FILE);
std::fs::write(&config_path, "")?;
let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf());
let result = service
.write_value(ConfigValueWriteParams {
file_path: Some(config_path.display().to_string()),
key_path: "shell_environment_policy.filters.PATH".to_string(),
value: serde_json::json!("include"),
merge_strategy: MergeStrategy::Replace,
expected_version: None,
})
.await?;
assert_eq!(result.status, WriteStatus::Ok);
assert_eq!(result.overridden_metadata, None);
assert!(std::fs::read_to_string(config_path)?.contains("PATH = \"include\""));
Ok(())
}
#[tokio::test]
async fn write_value_reports_case_insensitive_shell_filter_override() -> Result<()> {
let tmp = tempdir()?;
let config_path = tmp.path().join(CONFIG_TOML_FILE);
std::fs::write(&config_path, "")?;
let managed_path = tmp.path().join("managed_config.toml");
std::fs::write(
&managed_path,
"[shell_environment_policy.filters]\npath = \"exclude\"\n",
)?;
let managed_file = AbsolutePathBuf::try_from(managed_path.clone())?;
let service = ConfigManager::new_for_tests(
tmp.path().to_path_buf(),
vec![],
LoaderOverrides::with_managed_config_path_for_tests(managed_path),
CloudConfigBundleLoader::default(),
);
let result = service
.write_value(ConfigValueWriteParams {
file_path: Some(config_path.display().to_string()),
key_path: "shell_environment_policy.filters.PATH".to_string(),
value: serde_json::json!("include"),
merge_strategy: MergeStrategy::Replace,
expected_version: None,
})
.await?;
assert_eq!(result.status, WriteStatus::OkOverridden);
let overridden = result.overridden_metadata.expect("overridden metadata");
assert_eq!(
overridden.overriding_layer.name,
ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file }
);
assert_eq!(overridden.effective_value, serde_json::json!("exclude"));
Ok(())
}
#[tokio::test]
async fn upsert_merges_tables_replace_overwrites() -> Result<()> {
let tmp = tempdir().expect("tempdir");

55
codex-rs/config/src/merge.rs
View File

@@ -17,8 +17,10 @@ fn merge_toml_values_at_path(base: &mut TomlValue, overlay: &TomlValue, path: &m
normalize_key_aliases(path, base_table);
let mut overlay_table = overlay_table.clone();
normalize_key_aliases(path, &mut overlay_table);
normalize_merge_table_keys(path, base_table);
normalize_merge_table_keys(path, &mut overlay_table);
if is_permission_network_domains_path(path) {
normalize_network_domain_keys(base_table);
normalize_network_domain_keys(&mut overlay_table);
}
for (key, value) in overlay_table {
path.push(key.clone());
@@ -87,7 +89,6 @@ fn convert_legacy_to_filters(table: &mut toml::map::Map<String, TomlValue>) {
None => toml::map::Map::new(),
};
table.remove("filters");
normalize_table_keys(&mut filters, str::to_ascii_lowercase);
for (field, action) in [("exclude", "exclude"), ("include_only", "include")] {
let Some(TomlValue::Array(patterns)) = table.get(field).cloned() else {
continue;
@@ -96,7 +97,7 @@ fn convert_legacy_to_filters(table: &mut toml::map::Map<String, TomlValue>) {
for pattern in patterns {
if let TomlValue::String(pattern) = pattern {
filters
.entry(pattern.to_ascii_lowercase())
.entry(pattern)
.or_insert_with(|| TomlValue::String(action.to_string()));
}
}
@@ -107,11 +108,10 @@ fn convert_legacy_to_filters(table: &mut toml::map::Map<String, TomlValue>) {
}
fn convert_filters_to_legacy(table: &mut toml::map::Map<String, TomlValue>) {
let Some(TomlValue::Table(mut filters)) = table.get("filters").cloned() else {
let Some(TomlValue::Table(filters)) = table.get("filters").cloned() else {
return;
};
table.remove("filters");
normalize_table_keys(&mut filters, str::to_ascii_lowercase);
for (pattern, action) in filters {
match action.as_str() {
Some("exclude") => push_legacy_pattern(table, "exclude", "include_only", pattern),
@@ -134,10 +134,10 @@ fn push_legacy_pattern(
let Some(items) = items.as_array_mut() else {
return;
};
if !items.iter().any(|item| {
item.as_str()
.is_some_and(|candidate| candidate.eq_ignore_ascii_case(&pattern))
}) {
if !items
.iter()
.any(|item| item.as_str().is_some_and(|candidate| candidate == pattern))
{
items.push(TomlValue::String(pattern));
}
}
@@ -154,38 +154,23 @@ fn remove_patterns_from_legacy_array(
return;
}
items.retain(|item| {
item.as_str().is_none_or(|candidate| {
!patterns
.iter()
.any(|pattern| candidate.eq_ignore_ascii_case(pattern))
})
item.as_str()
.is_none_or(|candidate| !patterns.iter().any(|pattern| candidate == *pattern))
});
}
/// Canonicalizes keys for maps whose keys compare independently of TOML's
/// case-sensitive key semantics, so equivalent entries collide before merging.
fn normalize_merge_table_keys(path: &[String], table: &mut toml::map::Map<String, TomlValue>) {
match path {
fn is_permission_network_domains_path(path: &[String]) -> bool {
matches!(
path,
[permissions, _, network, domains]
if permissions == "permissions" && network == "network" && domains == "domains" =>
{
normalize_table_keys(table, normalize_host);
}
[policy, filters] if policy == "shell_environment_policy" && filters == "filters" => {
// Environment-variable patterns compare case-insensitively.
normalize_table_keys(table, str::to_ascii_lowercase);
}
_ => {}
}
if permissions == "permissions" && network == "network" && domains == "domains"
)
}
fn normalize_table_keys(
table: &mut toml::map::Map<String, TomlValue>,
normalize_key: impl Fn(&str) -> String,
) {
fn normalize_network_domain_keys(table: &mut toml::map::Map<String, TomlValue>) {
let entries = std::mem::take(table);
for (key, value) in entries {
table.insert(normalize_key(&key), value);
for (pattern, value) in entries {
table.insert(normalize_host(&pattern), value);
}
}

19
codex-rs/config/src/merge_tests.rs
View File

@@ -146,7 +146,7 @@ exclude = ["HIGH_*"]
}
#[test]
fn shell_environment_policy_filters_overlay_merges_by_key_case_insensitively() {
fn shell_environment_policy_filters_overlay_merges_by_exact_key() {
let mut base = parse_toml(
r#"
[shell_environment_policy.filters]
@@ -169,9 +169,10 @@ fn shell_environment_policy_filters_overlay_merges_by_key_case_insensitively() {
parse_toml(
r#"
[shell_environment_policy.filters]
"add_*" = "exclude"
"ADD_*" = "exclude"
"FLIP_*" = "exclude"
"KEEP_*" = "include"
"flip_*" = "include"
"keep_*" = "include"
"#,
)
);
@@ -202,11 +203,11 @@ include_only = ["FLIP_TO_EXCLUDE", "KEEP_INCLUDED"]
parse_toml(
r#"
[shell_environment_policy.filters]
"add_included" = "include"
"flip_to_exclude" = "exclude"
"flip_to_include" = "include"
"keep_excluded" = "exclude"
"keep_included" = "include"
"ADD_INCLUDED" = "include"
"FLIP_TO_EXCLUDE" = "exclude"
"FLIP_TO_INCLUDE" = "include"
"KEEP_EXCLUDED" = "exclude"
"KEEP_INCLUDED" = "include"
"#,
)
);
@@ -237,7 +238,7 @@ exclude = ["FLIP_TO_EXCLUDE", "HIGH_EXCLUDED"]
r#"
[shell_environment_policy]
exclude = ["FLIP_TO_EXCLUDE", "HIGH_EXCLUDED"]
include_only = ["keep_included"]
include_only = ["KEEP_INCLUDED"]
"#,
)
);

2
codex-rs/config/src/types.rs
View File

@@ -949,6 +949,8 @@ pub struct ShellEnvironmentPolicyToml {
/// Ordinary config keeps accepting the legacy arrays above during the
/// migration. Requirements will accept only this keyed form, keeping array
/// compatibility isolated so the legacy fields can be deprecated later.
/// Pattern keys merge case-sensitively across config layers even though the
/// resulting patterns match environment variable names case-insensitively.
pub filters: Option<BTreeMap<String, ShellEnvironmentPolicyFilter>>,
pub experimental_use_profile: Option<bool>,

7
codex-rs/core/src/config/config_loader_tests.rs
View File

@@ -445,12 +445,11 @@ future_field = true
}
#[tokio::test]
async fn non_strict_config_rejects_duplicate_shell_environment_policy_filters_before_merging() {
async fn non_strict_config_rejects_shell_filter_case_variants_across_layers() {
let tmp = tempdir().expect("tempdir");
let contents = r#"
[shell_environment_policy.filters]
"SECRET_TOKEN" = "exclude"
"secret_token" = "include"
"#;
std::fs::write(tmp.path().join(CONFIG_TOML_FILE), contents).expect("write config");
@@ -459,13 +458,13 @@ async fn non_strict_config_rejects_duplicate_shell_environment_policy_filters_be
.fallback_cwd(Some(tmp.path().to_path_buf()))
.loader_overrides(LoaderOverrides::without_managed_config_for_tests())
.cli_overrides(vec![(
"shell_environment_policy.filters.PATH".to_string(),
"shell_environment_policy.filters.secret_token".to_string(),
TomlValue::String("include".to_string()),
)])
.strict_config(/*strict_config*/ false)
.build()
.await
.expect_err("case-insensitive duplicate filters should be rejected per layer");
.expect_err("case variants across layers should be rejected after merging");
assert!(
err.to_string()