mirror of
https://github.com/openai/codex.git
synced 2026-11-13 22:47:20 +00:00
Add keyed shell environment policy filters (#34590)
## What changed - Add `shell_environment_policy.filters`, mapping environment-variable patterns to `include` or `exclude`, while continuing to accept the legacy `exclude` and `include_only` arrays. - Merge filter keys case-insensitively across config layers so higher-precedence entries can override individual patterns. Switching representations replaces the other representation instead of combining them. - Reject mixed, duplicate, or malformed filter definitions in enabled layers, config writes, and reloads, while retaining the previous session configuration when a reload is invalid. - Make config-manager reads and writes representation-aware and preserve existing formatting when updating individual policy values. ## Testing Add coverage for parsing, schema constraints, layered merging, validation and diagnostics, config writes, override metadata, and invalid session reloads. GitOrigin-RevId: e15464bca53e6e7ef31fcc97537fda60ed5d670a
This commit is contained in:
1 parent
65f8bf6853
commit
7442f5f932
29 files changed
+1662
-115
No files matched your search
@@ -25,12 +25,14 @@ use codex_config::ThreadConfigSource;
|
||||
use codex_config::compose_requirements;
|
||||
use codex_config::config_error_from_ignored_toml_fields;
|
||||
use codex_config::config_error_from_toml;
|
||||
use codex_config::config_error_from_typed_toml;
|
||||
use codex_config::config_toml::ConfigToml;
|
||||
use codex_config::config_toml::ProjectConfig;
|
||||
use codex_config::loader::load_config_layers_state;
|
||||
use codex_config::loader::load_requirements_toml;
|
||||
use codex_config::test_support::CloudConfigBundleFixture;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_protocol::config_types::EnvironmentVariablePattern;
|
||||
use codex_protocol::config_types::TrustLevel;
|
||||
use codex_protocol::config_types::WebSearchMode;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS;
|
||||
@@ -381,6 +383,236 @@ unknown_key = true"#;
|
||||
assert_eq!(config_error, &expected_config_error);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn non_strict_config_rejects_mixed_shell_environment_policy_before_higher_layer_merge() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let contents = r#"
|
||||
[shell_environment_policy]
|
||||
exclude = ["LEGACY_*"]
|
||||
|
||||
[shell_environment_policy.filters]
|
||||
"CANONICAL_*" = "include"
|
||||
"#;
|
||||
let config_path = tmp.path().join(CONFIG_TOML_FILE);
|
||||
std::fs::write(&config_path, contents).expect("write config");
|
||||
|
||||
let err = ConfigBuilder::default()
|
||||
.codex_home(tmp.path().to_path_buf())
|
||||
.fallback_cwd(Some(tmp.path().to_path_buf()))
|
||||
.loader_overrides(LoaderOverrides::without_managed_config_for_tests())
|
||||
.cli_overrides(vec![(
|
||||
"shell_environment_policy.filters.HIGHER_*".to_string(),
|
||||
TomlValue::String("exclude".to_string()),
|
||||
)])
|
||||
.strict_config(/*strict_config*/ false)
|
||||
.build()
|
||||
.await
|
||||
.expect_err("one config layer must not mix legacy lists and filters");
|
||||
|
||||
assert_eq!(
|
||||
config_error_from_io(&err),
|
||||
&config_error_from_typed_toml::<ConfigToml>(&config_path, contents)
|
||||
.expect("mixed shell policy should produce a typed config error")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shell_environment_policy_unknown_fields_follow_strict_config() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let contents = r#"
|
||||
[shell_environment_policy]
|
||||
future_field = true
|
||||
"#;
|
||||
let config_path = tmp.path().join(CONFIG_TOML_FILE);
|
||||
std::fs::write(&config_path, contents).expect("write config");
|
||||
|
||||
ConfigBuilder::default()
|
||||
.codex_home(tmp.path().to_path_buf())
|
||||
.fallback_cwd(Some(tmp.path().to_path_buf()))
|
||||
.loader_overrides(LoaderOverrides::without_managed_config_for_tests())
|
||||
.strict_config(/*strict_config*/ false)
|
||||
.build()
|
||||
.await
|
||||
.expect("non-strict config should ignore unknown fields");
|
||||
|
||||
let err = ConfigBuilder::default()
|
||||
.codex_home(tmp.path().to_path_buf())
|
||||
.fallback_cwd(Some(tmp.path().to_path_buf()))
|
||||
.loader_overrides(LoaderOverrides::without_managed_config_for_tests())
|
||||
.strict_config(/*strict_config*/ true)
|
||||
.build()
|
||||
.await
|
||||
.expect_err("strict config should reject unknown fields");
|
||||
|
||||
assert_eq!(
|
||||
config_error_from_io(&err).message,
|
||||
"unknown configuration field `shell_environment_policy.future_field`"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn non_strict_config_merges_shell_filter_case_variants_across_layers() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let contents = r#"
|
||||
[shell_environment_policy.filters]
|
||||
"SECRET_TOKEN" = "exclude"
|
||||
"#;
|
||||
std::fs::write(tmp.path().join(CONFIG_TOML_FILE), contents).expect("write config");
|
||||
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(tmp.path().to_path_buf())
|
||||
.fallback_cwd(Some(tmp.path().to_path_buf()))
|
||||
.loader_overrides(LoaderOverrides::without_managed_config_for_tests())
|
||||
.cli_overrides(vec![(
|
||||
"shell_environment_policy.filters.secret_token".to_string(),
|
||||
TomlValue::String("include".to_string()),
|
||||
)])
|
||||
.strict_config(/*strict_config*/ false)
|
||||
.build()
|
||||
.await
|
||||
.expect("higher-priority case-variant filter should override the lower layer");
|
||||
|
||||
assert!(
|
||||
config
|
||||
.permissions
|
||||
.shell_environment_policy
|
||||
.exclude
|
||||
.is_empty()
|
||||
);
|
||||
assert_eq!(
|
||||
config.permissions.shell_environment_policy.include_only,
|
||||
vec![EnvironmentVariablePattern::new_case_insensitive(
|
||||
"secret_token"
|
||||
)]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn non_strict_config_rejects_malformed_shell_policy_before_representation_conversion() {
|
||||
let cases = [
|
||||
(
|
||||
r#"
|
||||
[shell_environment_policy]
|
||||
exclude = ["SECRET_*", 17]
|
||||
"#,
|
||||
vec![(
|
||||
"shell_environment_policy.filters.PATH".to_string(),
|
||||
TomlValue::String("include".to_string()),
|
||||
)],
|
||||
),
|
||||
(
|
||||
r#"
|
||||
[shell_environment_policy.filters]
|
||||
"SECRET_*" = "keep"
|
||||
"#,
|
||||
vec![(
|
||||
"shell_environment_policy.exclude".to_string(),
|
||||
TomlValue::Array(vec![TomlValue::String("PATH".to_string())]),
|
||||
)],
|
||||
),
|
||||
];
|
||||
|
||||
for (contents, cli_overrides) in cases {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
std::fs::write(tmp.path().join(CONFIG_TOML_FILE), contents).expect("write config");
|
||||
|
||||
ConfigBuilder::default()
|
||||
.codex_home(tmp.path().to_path_buf())
|
||||
.fallback_cwd(Some(tmp.path().to_path_buf()))
|
||||
.loader_overrides(LoaderOverrides::without_managed_config_for_tests())
|
||||
.cli_overrides(cli_overrides)
|
||||
.strict_config(/*strict_config*/ false)
|
||||
.build()
|
||||
.await
|
||||
.expect_err("malformed shell policy should be rejected before conversion");
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn non_strict_config_allows_replaced_shell_policy_fields_outside_filter_representation() {
|
||||
let cases = [
|
||||
(
|
||||
r#"
|
||||
[shell_environment_policy]
|
||||
inherit = "invalid"
|
||||
set = ["invalid"]
|
||||
"#,
|
||||
vec![
|
||||
(
|
||||
"shell_environment_policy.inherit".to_string(),
|
||||
TomlValue::String("core".to_string()),
|
||||
),
|
||||
(
|
||||
"shell_environment_policy.set.PATH".to_string(),
|
||||
TomlValue::String("/bin".to_string()),
|
||||
),
|
||||
],
|
||||
),
|
||||
(
|
||||
r#"shell_environment_policy = 17"#,
|
||||
vec![(
|
||||
"shell_environment_policy.inherit".to_string(),
|
||||
TomlValue::String("core".to_string()),
|
||||
)],
|
||||
),
|
||||
];
|
||||
|
||||
for (contents, cli_overrides) in cases {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
std::fs::write(tmp.path().join(CONFIG_TOML_FILE), contents).expect("write config");
|
||||
|
||||
ConfigBuilder::default()
|
||||
.codex_home(tmp.path().to_path_buf())
|
||||
.fallback_cwd(Some(tmp.path().to_path_buf()))
|
||||
.loader_overrides(LoaderOverrides::without_managed_config_for_tests())
|
||||
.cli_overrides(cli_overrides)
|
||||
.strict_config(/*strict_config*/ false)
|
||||
.build()
|
||||
.await
|
||||
.expect("replaced shell policy fields should preserve normal overlay behavior");
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn malformed_higher_shell_filter_reports_its_layer_when_lower_fields_are_replaced() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let managed_path = tmp.path().join("managed_config.toml");
|
||||
std::fs::write(
|
||||
tmp.path().join(CONFIG_TOML_FILE),
|
||||
r#"[shell_environment_policy]
|
||||
inherit = "invalid"
|
||||
set = ["invalid"]
|
||||
"#,
|
||||
)
|
||||
.expect("write user config");
|
||||
std::fs::write(
|
||||
&managed_path,
|
||||
r#"[shell_environment_policy]
|
||||
inherit = "core"
|
||||
set = { PATH = "/bin" }
|
||||
|
||||
[shell_environment_policy.filters]
|
||||
"SECRET_*" = "keep"
|
||||
"#,
|
||||
)
|
||||
.expect("write managed config");
|
||||
|
||||
let err = ConfigBuilder::default()
|
||||
.codex_home(tmp.path().to_path_buf())
|
||||
.fallback_cwd(Some(tmp.path().to_path_buf()))
|
||||
.loader_overrides(LoaderOverrides::with_managed_config_path_for_tests(
|
||||
managed_path.clone(),
|
||||
))
|
||||
.strict_config(/*strict_config*/ false)
|
||||
.build()
|
||||
.await
|
||||
.expect_err("malformed shell filter should be rejected");
|
||||
|
||||
let config_error = config_error_from_io(&err);
|
||||
assert_eq!(config_error.path, managed_path);
|
||||
assert!(config_error.message.contains("unknown variant `keep`"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn strict_config_rejects_unknown_cli_override_key() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
|
||||
@@ -73,3 +73,31 @@ fn config_schema_hides_unsupported_inline_mcp_bearer_token() {
|
||||
(false, true),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shell_environment_policy_schema_rejects_mixed_filter_representations() {
|
||||
let schema_json = config_schema_json().expect("serialize config schema");
|
||||
let schema_value: serde_json::Value =
|
||||
serde_json::from_slice(&schema_json).expect("decode schema json");
|
||||
let constraints = schema_value
|
||||
.pointer("/definitions/ShellEnvironmentPolicyToml/allOf")
|
||||
.and_then(serde_json::Value::as_array)
|
||||
.expect("shell environment policy constraints should be an array");
|
||||
let required_pairs = constraints
|
||||
.iter()
|
||||
.map(|constraint| {
|
||||
constraint
|
||||
.pointer("/not/required")
|
||||
.expect("constraint should prohibit a required-field pair")
|
||||
.clone()
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
assert_eq!(
|
||||
required_pairs,
|
||||
vec![
|
||||
serde_json::json!(["exclude", "filters"]),
|
||||
serde_json::json!(["filters", "include_only"]),
|
||||
]
|
||||
);
|
||||
}
|
||||
@@ -1709,9 +1709,17 @@ impl Session {
|
||||
let state = self.state.lock().await;
|
||||
let mut config = (*state.session_configuration.original_config_do_not_use).clone();
|
||||
for (config_toml_path, user_config) in reloaded_user_configs {
|
||||
config.config_layer_stack = config
|
||||
let config_layer_stack = match config
|
||||
.config_layer_stack
|
||||
.with_user_config(&config_toml_path, user_config);
|
||||
.with_user_config(&config_toml_path, user_config)
|
||||
{
|
||||
Ok(config_layer_stack) => config_layer_stack,
|
||||
Err(err) => {
|
||||
warn!("failed to validate user config while reloading layer: {err}");
|
||||
return;
|
||||
}
|
||||
};
|
||||
config.config_layer_stack = config_layer_stack;
|
||||
}
|
||||
config.tool_suggest =
|
||||
resolve_tool_suggest_config_from_layer_stack(&config.config_layer_stack);
|
||||
|
||||
@@ -1333,6 +1333,45 @@ async fn reload_user_config_layer_updates_effective_apps_config() {
|
||||
assert_eq!(app.destructive_enabled, Some(false));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reload_user_config_layer_keeps_previous_config_for_malformed_shell_policy() {
|
||||
let (session, _turn_context) = make_session_and_context().await;
|
||||
let codex_home = session.codex_home().await;
|
||||
std::fs::create_dir_all(&codex_home).expect("create codex home");
|
||||
let config_toml_path = codex_home.join(CONFIG_TOML_FILE);
|
||||
std::fs::write(&config_toml_path, "[apps.calendar]\nenabled = false\n")
|
||||
.expect("write valid user config");
|
||||
session.reload_user_config_layer().await;
|
||||
let previous_config = session
|
||||
.get_config()
|
||||
.await
|
||||
.config_layer_stack
|
||||
.effective_user_config()
|
||||
.expect("previous user config");
|
||||
|
||||
std::fs::write(
|
||||
&config_toml_path,
|
||||
r#"
|
||||
[apps.calendar]
|
||||
enabled = true
|
||||
|
||||
[shell_environment_policy]
|
||||
exclude = ["SECRET_*", 17]
|
||||
"#,
|
||||
)
|
||||
.expect("write malformed user config");
|
||||
|
||||
session.reload_user_config_layer().await;
|
||||
|
||||
let current_config = session
|
||||
.get_config()
|
||||
.await
|
||||
.config_layer_stack
|
||||
.effective_user_config()
|
||||
.expect("current user config");
|
||||
assert_eq!(current_config, previous_config);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reload_user_config_layer_updates_base_and_selected_profile_layers() {
|
||||
let (session, _turn_context) = make_session_and_context().await;
|
||||
@@ -1438,7 +1477,8 @@ async fn reload_user_config_layer_refreshes_hooks() -> anyhow::Result<()> {
|
||||
config_layer_stack: Some(
|
||||
config
|
||||
.config_layer_stack
|
||||
.with_user_config(&config_toml_path, user_config.clone()),
|
||||
.with_user_config(&config_toml_path, user_config.clone())
|
||||
.expect("hook user config should be valid"),
|
||||
),
|
||||
..codex_hooks::HooksConfig::default()
|
||||
});
|
||||
|
||||
@@ -528,7 +528,7 @@ async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Resul
|
||||
},
|
||||
}))
|
||||
.context("build trusted hook state")?,
|
||||
);
|
||||
)?;
|
||||
|
||||
let mut hook_shell_argv = session
|
||||
.user_shell()
|
||||
|
||||
Reference in new issue
Block a user