Harden shell environment policy loading

codex-rs/config/src/schema.rs

@@ -117,12 +117,43 @@ pub fn mcp_servers_schema(schema_gen: &mut SchemaGenerator) -> Schema {
 
 /// Build the config schema for `config.toml`.
 pub fn config_schema() -> RootSchema {
-    SchemaSettings::draft07()
+    let mut schema = SchemaSettings::draft07()
         .with(|settings| {
             settings.option_add_null_type = false;
         })
         .into_generator()
-        .into_root_schema_for::<ConfigToml>()
+        .into_root_schema_for::<ConfigToml>();
+    add_shell_environment_policy_constraints(&mut schema);
+    schema
+}
+
+// Mirror the custom deserializer's mixed-form rejection in the generated schema.
+fn add_shell_environment_policy_constraints(schema: &mut RootSchema) {
+    let Some(Schema::Object(policy)) = schema.definitions.get_mut("ShellEnvironmentPolicyToml")
+    else {
+        return;
+    };
+    let all_of = policy
+        .subschemas
+        .get_or_insert_default()
+        .all_of
+        .get_or_insert_default();
+    for fields in [["exclude", "filters"], ["filters", "include_only"]] {
+        let forbidden_fields = Schema::Object(SchemaObject {
+            object: Some(Box::new(ObjectValidation {
+                required: fields.into_iter().map(str::to_string).collect(),
+                ..Default::default()
+            })),
+            ..Default::default()
+        });
+        all_of.push(Schema::Object(SchemaObject {
+            subschemas: Some(Box::new(SubschemaValidation {
+                not: Some(Box::new(forbidden_fields)),
+                ..Default::default()
+            })),
+            ..Default::default()
+        }));
+    }
 }
 
 /// Canonicalize a JSON value by sorting its keys.

codex-rs/config/src/state.rs

@@ -9,6 +9,7 @@ use super::key_aliases::normalized_with_key_aliases;
 use super::merge::merge_toml_values;
 use crate::CloudConfigBundleLoader;
 use crate::ProfileV2Name;
+use crate::types::ShellEnvironmentPolicyToml;
 use codex_app_server_protocol::ConfigLayer;
 use codex_app_server_protocol::ConfigLayerMetadata;
 use codex_app_server_protocol::ConfigLayerSource;
@@ -277,23 +278,20 @@ impl ConfigLayerStack {
         requirements: ConfigRequirements,
         requirements_toml: ConfigRequirementsToml,
     ) -> std::io::Result<Self> {
-        if let Some(layer) = layers.iter().find(|layer| {
-            layer
-                .config
-                .get("shell_environment_policy")
-                .and_then(TomlValue::as_table)
-                .is_some_and(|policy| {
-                    policy.contains_key("filters")
-                        && (policy.contains_key("exclude") || policy.contains_key("include_only"))
-                })
-        }) {
-            return Err(std::io::Error::new(
-                std::io::ErrorKind::InvalidData,
-                format!(
-                    "cannot mix `filters` with legacy `exclude` or `include_only` in {}",
-                    format_config_layer_source(&layer.name, CONFIG_TOML_FILE)
-                ),
-            ));
+        // Validate enabled layers before merging to reject mixed forms and malformed entries
+        for layer in layers.iter().filter(|layer| !layer.is_disabled()) {
+            let Some(policy) = layer.config.get("shell_environment_policy") else {
+                continue;
+            };
+            let _: ShellEnvironmentPolicyToml = policy.clone().try_into().map_err(|error| {
+                std::io::Error::new(
+                    std::io::ErrorKind::InvalidData,
+                    format!(
+                        "invalid shell environment policy in {}: {error}",
+                        format_config_layer_source(&layer.name, CONFIG_TOML_FILE)
+                    ),
+                )
+            })?;
         }
         let user_layer_index = verify_layer_ordering(&layers)?;
         Ok(Self {

codex-rs/config/src/state_tests.rs

@@ -39,6 +39,34 @@ no_memories_if_mcp_or_web_search = true
     );
 }
 
+#[test]
+fn disabled_layers_do_not_validate_shell_environment_policy() {
+    let layer = ConfigLayerEntry::new_disabled(
+        ConfigLayerSource::Project {
+            dot_codex_folder: AbsolutePathBuf::from_absolute_path("/untrusted/.codex")
+                .expect("project path should be absolute"),
+        },
+        toml::from_str(
+            r#"
+[shell_environment_policy]
+exclude = ["LEGACY_*"]
+
+[shell_environment_policy.filters]
+"CANONICAL_*" = "include"
+"#,
+        )
+        .expect("project config"),
+        "project is untrusted",
+    );
+
+    ConfigLayerStack::new(
+        vec![layer],
+        ConfigRequirements::default(),
+        ConfigRequirementsToml::default(),
+    )
+    .expect("disabled layers should not be validated");
+}
+
 #[test]
 fn active_user_layer_is_highest_precedence_user_layer() {
     let temp_dir = TempDir::new().expect("tempdir");

codex-rs/config/src/types.rs

@@ -955,7 +955,6 @@ pub struct ShellEnvironmentPolicyToml {
 }
 
 #[derive(Deserialize)]
-#[serde(deny_unknown_fields)]
 struct ShellEnvironmentPolicyTomlRaw {
     inherit: Option<ShellEnvironmentPolicyInherit>,
     ignore_default_excludes: Option<bool>,
@@ -977,6 +976,16 @@ impl<'de> Deserialize<'de> for ShellEnvironmentPolicyToml {
                 "cannot mix `filters` with legacy `exclude` or `include_only`",
             ));
         }
+        if let Some(filters) = raw.filters.as_ref() {
+            let mut patterns = std::collections::HashSet::new();
+            for pattern in filters.keys() {
+                if !patterns.insert(pattern.to_ascii_lowercase()) {
+                    return Err(serde::de::Error::custom(format!(
+                        "duplicate shell environment filter `{pattern}` ignoring ASCII case"
+                    )));
+                }
+            }
+        }
         Ok(Self {
             inherit: raw.inherit,
             ignore_default_excludes: raw.ignore_default_excludes,

codex-rs/core/config.schema.json

@@ -2630,6 +2630,24 @@
     },
     "ShellEnvironmentPolicyToml": {
       "additionalProperties": false,
+      "allOf": [
+        {
+          "not": {
+            "required": [
+              "exclude",
+              "filters"
+            ]
+          }
+        },
+        {
+          "not": {
+            "required": [
+              "filters",
+              "include_only"
+            ]
+          }
+        }
+      ],
       "description": "Policy for building the `env` when spawning a process via shell-like tools.",
       "properties": {
         "exclude": {

codex-rs/core/src/config/config_loader_tests.rs

@@ -410,6 +410,110 @@ exclude = ["LEGACY_*"]
     );
 }
 
+#[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_rejects_duplicate_shell_environment_policy_filters_before_merging() {
+    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");
+
+    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.PATH".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");
+
+    assert!(
+        err.to_string()
+            .contains("duplicate shell environment filter")
+    );
+}
+
+#[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_*" = "exlude"
+"#,
+            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 strict_config_rejects_unknown_cli_override_key() {
     let tmp = tempdir().expect("tempdir");

codex-rs/core/src/config/schema_tests.rs

@@ -73,3 +73,32 @@ 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")
+        .expect("shell environment policy constraints should exist")
+        .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"]),
+        ]
+    );
+}