diff --git a/codex-rs/core/src/config/service_tests.rs b/codex-rs/core/src/config/service_tests.rs index bc3006a9a9..a3cfb87f86 100644 --- a/codex-rs/core/src/config/service_tests.rs +++ b/codex-rs/core/src/config/service_tests.rs @@ -7,8 +7,18 @@ use codex_app_server_protocol::AskForApproval; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::collections::BTreeMap; +use std::path::PathBuf; use tempfile::tempdir; +fn test_loader_overrides(managed_config_path: Option) -> LoaderOverrides { + LoaderOverrides { + managed_config_path, + #[cfg(target_os = "macos")] + managed_preferences_base64: Some(String::new()), + macos_managed_config_requirements_base64: Some(String::new()), + } +} + #[test] fn toml_value_to_item_handles_nested_config_tables() { let config = r#" @@ -178,12 +188,7 @@ async fn read_includes_origins_and_layers() { let service = ConfigService::new( tmp.path().to_path_buf(), vec![], - LoaderOverrides { - managed_config_path: Some(managed_path.clone()), - #[cfg(target_os = "macos")] - managed_preferences_base64: None, - macos_managed_config_requirements_base64: None, - }, + test_loader_overrides(Some(managed_path.clone())), CloudRequirementsLoader::default(), ); @@ -253,12 +258,7 @@ async fn write_value_reports_override() { let service = ConfigService::new( tmp.path().to_path_buf(), vec![], - LoaderOverrides { - managed_config_path: Some(managed_path.clone()), - #[cfg(target_os = "macos")] - managed_preferences_base64: None, - macos_managed_config_requirements_base64: None, - }, + test_loader_overrides(Some(managed_path.clone())), CloudRequirementsLoader::default(), ); @@ -357,12 +357,7 @@ async fn invalid_user_value_rejected_even_if_overridden_by_managed() { let service = ConfigService::new( tmp.path().to_path_buf(), vec![], - LoaderOverrides { - managed_config_path: Some(managed_path.clone()), - #[cfg(target_os = "macos")] - managed_preferences_base64: None, - macos_managed_config_requirements_base64: None, - }, + test_loader_overrides(Some(managed_path.clone())), CloudRequirementsLoader::default(), ); @@ -422,12 +417,7 @@ async fn write_value_rejects_feature_requirement_conflict() { let service = ConfigService::new( tmp.path().to_path_buf(), vec![], - LoaderOverrides { - managed_config_path: None, - #[cfg(target_os = "macos")] - managed_preferences_base64: None, - macos_managed_config_requirements_base64: None, - }, + test_loader_overrides(None), CloudRequirementsLoader::new(async { Ok(Some(ConfigRequirementsToml { feature_requirements: Some(crate::config_loader::FeatureRequirementsToml { @@ -473,12 +463,7 @@ async fn write_value_rejects_profile_feature_requirement_conflict() { let service = ConfigService::new( tmp.path().to_path_buf(), vec![], - LoaderOverrides { - managed_config_path: None, - #[cfg(target_os = "macos")] - managed_preferences_base64: None, - macos_managed_config_requirements_base64: None, - }, + test_loader_overrides(None), CloudRequirementsLoader::new(async { Ok(Some(ConfigRequirementsToml { feature_requirements: Some(crate::config_loader::FeatureRequirementsToml { @@ -535,12 +520,7 @@ async fn read_reports_managed_overrides_user_and_session_flags() { let service = ConfigService::new( tmp.path().to_path_buf(), cli_overrides, - LoaderOverrides { - managed_config_path: Some(managed_path.clone()), - #[cfg(target_os = "macos")] - managed_preferences_base64: None, - macos_managed_config_requirements_base64: None, - }, + test_loader_overrides(Some(managed_path.clone())), CloudRequirementsLoader::default(), ); @@ -593,12 +573,7 @@ async fn write_value_reports_managed_override() { let service = ConfigService::new( tmp.path().to_path_buf(), vec![], - LoaderOverrides { - managed_config_path: Some(managed_path.clone()), - #[cfg(target_os = "macos")] - managed_preferences_base64: None, - macos_managed_config_requirements_base64: None, - }, + test_loader_overrides(Some(managed_path.clone())), CloudRequirementsLoader::default(), ); diff --git a/codex-rs/core/src/config_loader/macos.rs b/codex-rs/core/src/config_loader/macos.rs index 87f6ddd3c7..5570e97c54 100644 --- a/codex-rs/core/src/config_loader/macos.rs +++ b/codex-rs/core/src/config_loader/macos.rs @@ -74,9 +74,10 @@ pub(crate) async fn load_managed_admin_requirements_toml( return Ok(()); } - if let Some(requirements) = parse_managed_requirements_base64(trimmed)? { - target.merge_unset_fields(managed_preferences_requirements_source(), requirements); - } + target.merge_unset_fields( + managed_preferences_requirements_source(), + parse_managed_requirements_base64(trimmed)?, + ); return Ok(()); } @@ -104,7 +105,6 @@ fn load_managed_admin_requirements() -> io::Result io::Result> { @@ -182,55 +182,21 @@ fn parse_managed_config_base64(encoded: &str) -> io::Result io::Result> { - let raw_toml = match decode_managed_preferences_base64(encoded) { - Ok(raw_toml) => raw_toml, - Err(err) => { - tracing::warn!( - error = %err, - "Ignoring invalid MDM managed requirements payload", - ); - return Ok(None); - } - }; - let parsed = match toml::from_str::(&raw_toml) { - Ok(TomlValue::Table(parsed)) => TomlValue::Table(parsed), - Ok(other) => { - tracing::warn!( - managed_value = ?other, - "Ignoring invalid MDM managed requirements payload: root must be a table", - ); - return Ok(None); - } - Err(err) => { - tracing::warn!( - error = %err, - "Ignoring invalid MDM managed requirements payload", - ); - return Ok(None); - } - }; - let sanitized = match sanitize_toml_value::(parsed) { - Ok(sanitized) => sanitized, - Err(err) => { - tracing::warn!( - error = %err, - "Ignoring invalid MDM managed requirements payload", - ); - return Ok(None); - } - }; - for dropped_entry in &sanitized.dropped_entries { - tracing::warn!( - dropped_entry = %dropped_entry, - "Ignoring invalid MDM managed requirements entry", - ); - } - let requirements = sanitized - .value - .try_into() - .map_err(|err: toml::de::Error| io::Error::new(io::ErrorKind::InvalidData, err))?; - Ok(Some(requirements)) +fn parse_managed_requirements_base64(encoded: &str) -> io::Result { + let source = managed_preferences_requirements_source(); + let raw_toml = decode_managed_preferences_base64(encoded).map_err(|err| { + io::Error::new( + err.kind(), + format!("Error parsing managed requirements from {source}: {err}"), + ) + })?; + + toml::from_str::(&raw_toml).map_err(|err| { + io::Error::new( + io::ErrorKind::InvalidData, + format!("Error parsing managed requirements from {source}: {err}"), + ) + }) } fn decode_managed_preferences_base64(encoded: &str) -> io::Result { diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs index f2d9a0807a..e441786bd8 100644 --- a/codex-rs/core/src/config_loader/tests.rs +++ b/codex-rs/core/src/config_loader/tests.rs @@ -492,28 +492,46 @@ async fn managed_preferences_ignore_invalid_payload() -> anyhow::Result<()> { #[cfg(target_os = "macos")] #[tokio::test] -async fn managed_preferences_ignore_invalid_requirements_payload() -> anyhow::Result<()> { +async fn managed_preferences_invalid_requirements_fail_closed() -> anyhow::Result<()> { use base64::Engine; - let tmp = tempdir()?; + for (payload, expected_fragment) in [ + ( + "allowed_approval_policies = [\"bogus\"]", + "allowed_approval_policies", + ), + ( + "allowed_sandbox_modes = [\"bogus\"]", + "allowed_sandbox_modes", + ), + ( + "allowed_web_search_modes = [\"bogus\"]", + "allowed_web_search_modes", + ), + ] { + let tmp = tempdir()?; + let err = load_config_layers_state( + tmp.path(), + Some(AbsolutePathBuf::try_from(tmp.path())?), + &[] as &[(String, TomlValue)], + LoaderOverrides { + managed_config_path: Some(tmp.path().join("managed_config.toml")), + managed_preferences_base64: Some(String::new()), + macos_managed_config_requirements_base64: Some( + base64::prelude::BASE64_STANDARD.encode(payload.as_bytes()), + ), + }, + CloudRequirementsLoader::default(), + ) + .await + .expect_err("invalid managed requirements should fail closed"); - let state = load_config_layers_state( - tmp.path(), - Some(AbsolutePathBuf::try_from(tmp.path())?), - &[] as &[(String, TomlValue)], - LoaderOverrides { - managed_config_path: Some(tmp.path().join("managed_config.toml")), - managed_preferences_base64: Some(String::new()), - macos_managed_config_requirements_base64: Some( - base64::prelude::BASE64_STANDARD - .encode("allowed_sandbox_modes = [\"bogus\"]".as_bytes()), - ), - }, - CloudRequirementsLoader::default(), - ) - .await?; - - assert_eq!(state.requirements_toml().allowed_sandbox_modes, None); + assert_eq!(err.kind(), std::io::ErrorKind::InvalidData); + let message = err.to_string(); + assert!(message.contains("Error parsing managed requirements from MDM")); + assert!(message.contains(expected_fragment), "{message}"); + assert!(message.contains("bogus"), "{message}"); + } Ok(()) }