diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 16a4e7baa..fc75cd846 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -7540,6 +7540,12 @@ }, "ConfigRequirements": { "properties": { + "allowManagedHooksOnly": { + "type": [ + "boolean", + "null" + ] + }, "allowedApprovalPolicies": { "items": { "$ref": "#/definitions/v2/AskForApproval" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 06b19a0af..4242bc6e3 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -3929,6 +3929,12 @@ }, "ConfigRequirements": { "properties": { + "allowManagedHooksOnly": { + "type": [ + "boolean", + "null" + ] + }, "allowedApprovalPolicies": { "items": { "$ref": "#/definitions/AskForApproval" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json index b229d8ef9..41bd337de 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json @@ -62,6 +62,12 @@ }, "ConfigRequirements": { "properties": { + "allowManagedHooksOnly": { + "type": [ + "boolean", + "null" + ] + }, "allowedApprovalPolicies": { "items": { "$ref": "#/definitions/AskForApproval" diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ConfigRequirements.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ConfigRequirements.ts index 47a99453f..1b0625e5e 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ConfigRequirements.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ConfigRequirements.ts @@ -6,4 +6,4 @@ import type { AskForApproval } from "./AskForApproval"; import type { ResidencyRequirement } from "./ResidencyRequirement"; import type { SandboxMode } from "./SandboxMode"; -export type ConfigRequirements = {allowedApprovalPolicies: Array | null, allowedSandboxModes: Array | null, allowedWebSearchModes: Array | null, featureRequirements: { [key in string]?: boolean } | null, enforceResidency: ResidencyRequirement | null}; +export type ConfigRequirements = {allowedApprovalPolicies: Array | null, allowedSandboxModes: Array | null, allowedWebSearchModes: Array | null, allowManagedHooksOnly: boolean | null, featureRequirements: { [key in string]?: boolean } | null, enforceResidency: ResidencyRequirement | null}; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/config.rs b/codex-rs/app-server-protocol/src/protocol/v2/config.rs index 35e487e45..2c0e2df8e 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/config.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/config.rs @@ -357,6 +357,7 @@ pub struct ConfigRequirements { pub allowed_approvals_reviewers: Option>, pub allowed_sandbox_modes: Option>, pub allowed_web_search_modes: Option>, + pub allow_managed_hooks_only: Option, pub feature_requirements: Option>, #[experimental("configRequirements/read.hooks")] pub hooks: Option, diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index 26cd2b005..6e93fffd2 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -1688,6 +1688,7 @@ fn config_requirements_granular_allowed_approval_policy_is_marked_experimental() allowed_approvals_reviewers: None, allowed_sandbox_modes: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, feature_requirements: None, hooks: None, enforce_residency: None, diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 2be4694bf..57a133dbd 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -219,7 +219,7 @@ Example with notification opt-out: - `externalAgentConfig/import` — apply selected external-agent migration items by passing explicit `migrationItems` with `cwd` (`null` for home) and any plugin/session `details` returned by detect. When a request includes migration items, the server emits `externalAgentConfig/import/completed` once after the full import finishes (immediately after the response when everything completed synchronously, or after background imports finish). - `config/value/write` — write a single config key/value to the user's config.toml on disk. - `config/batchWrite` — apply multiple config edits atomically to the user's config.toml on disk, with optional `reloadUserConfig: true` to hot-reload loaded threads. -- `configRequirements/read` — fetch loaded requirements constraints from `requirements.toml` and/or MDM (or `null` if none are configured), including allow-lists (`allowedApprovalPolicies`, `allowedSandboxModes`, `allowedWebSearchModes`), pinned feature values (`featureRequirements`), managed lifecycle hooks (`hooks`), `enforceResidency`, and `network` constraints such as canonical domain/socket permissions plus `managedAllowedDomainsOnly` and `dangerFullAccessDenylistOnly`. +- `configRequirements/read` — fetch loaded requirements constraints from `requirements.toml` and/or MDM (or `null` if none are configured), including allow-lists (`allowedApprovalPolicies`, `allowedSandboxModes`, `allowedWebSearchModes`), lifecycle hook lockdown (`allowManagedHooksOnly`), pinned feature values (`featureRequirements`), managed lifecycle hooks (`hooks`), `enforceResidency`, and `network` constraints such as canonical domain/socket permissions plus `managedAllowedDomainsOnly` and `dangerFullAccessDenylistOnly`. ### Example: Start or resume a thread diff --git a/codex-rs/app-server/src/request_processors/config_processor.rs b/codex-rs/app-server/src/request_processors/config_processor.rs index b2721d996..b7f973b07 100644 --- a/codex-rs/app-server/src/request_processors/config_processor.rs +++ b/codex-rs/app-server/src/request_processors/config_processor.rs @@ -429,6 +429,7 @@ fn map_requirements_toml_to_api(requirements: ConfigRequirementsToml) -> ConfigR } normalized }), + allow_managed_hooks_only: requirements.allow_managed_hooks_only, feature_requirements: requirements .feature_requirements .map(|requirements| requirements.entries), @@ -611,3 +612,21 @@ fn config_write_error(code: ConfigWriteErrorCode, message: impl Into) -> })); error } + +#[cfg(test)] +mod tests { + use super::map_requirements_toml_to_api; + use codex_config::ConfigRequirementsToml; + use pretty_assertions::assert_eq; + + #[test] + fn requirements_api_includes_allow_managed_hooks_only() { + let mapped = map_requirements_toml_to_api(ConfigRequirementsToml { + allow_managed_hooks_only: Some(true), + ..ConfigRequirementsToml::default() + }); + + assert_eq!(mapped.allow_managed_hooks_only, Some(true)); + assert_eq!(mapped.hooks, None); + } +} diff --git a/codex-rs/cloud-requirements/src/lib.rs b/codex-rs/cloud-requirements/src/lib.rs index 360e99a21..8812762cb 100644 --- a/codex-rs/cloud-requirements/src/lib.rs +++ b/codex-rs/cloud-requirements/src/lib.rs @@ -1204,6 +1204,7 @@ mod tests { allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, guardian_policy_config: None, feature_requirements: None, hooks: None, @@ -1286,6 +1287,7 @@ mod tests { allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, guardian_policy_config: None, feature_requirements: None, hooks: None, @@ -1319,6 +1321,7 @@ mod tests { allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, guardian_policy_config: None, feature_requirements: None, hooks: None, @@ -1369,6 +1372,7 @@ mod tests { allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, guardian_policy_config: None, feature_requirements: None, hooks: None, @@ -1519,6 +1523,7 @@ command = "sample-mcp" allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, guardian_policy_config: None, feature_requirements: None, hooks: None, @@ -1599,6 +1604,7 @@ command = "sample-mcp" allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, guardian_policy_config: None, feature_requirements: None, hooks: None, @@ -1677,6 +1683,7 @@ command = "sample-mcp" allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, guardian_policy_config: None, feature_requirements: None, hooks: None, @@ -1883,6 +1890,7 @@ command = "sample-mcp" allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, guardian_policy_config: None, feature_requirements: None, hooks: None, @@ -1923,6 +1931,7 @@ command = "sample-mcp" allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, guardian_policy_config: None, feature_requirements: None, hooks: None, @@ -1983,6 +1992,7 @@ command = "sample-mcp" allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, guardian_policy_config: None, feature_requirements: None, hooks: None, @@ -2039,6 +2049,7 @@ command = "sample-mcp" allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, guardian_policy_config: None, feature_requirements: None, hooks: None, @@ -2097,6 +2108,7 @@ command = "sample-mcp" allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, guardian_policy_config: None, feature_requirements: None, hooks: None, @@ -2156,6 +2168,7 @@ command = "sample-mcp" allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, guardian_policy_config: None, feature_requirements: None, hooks: None, @@ -2215,6 +2228,7 @@ command = "sample-mcp" allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, guardian_policy_config: None, feature_requirements: None, hooks: None, @@ -2304,6 +2318,7 @@ command = "sample-mcp" allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, guardian_policy_config: None, feature_requirements: None, hooks: None, @@ -2335,6 +2350,7 @@ command = "sample-mcp" allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, guardian_policy_config: None, feature_requirements: None, hooks: None, diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index 8bf3148ba..9faa84613 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -87,6 +87,7 @@ pub struct ConfigRequirements { pub approvals_reviewer: ConstrainedWithSource, pub permission_profile: ConstrainedWithSource, pub web_search_mode: ConstrainedWithSource, + pub allow_managed_hooks_only: Option>, pub feature_requirements: Option>, pub managed_hooks: Option>, pub mcp_servers: Option>>, @@ -120,6 +121,7 @@ impl Default for ConfigRequirements { Constrained::allow_any(WebSearchMode::Cached), /*source*/ None, ), + allow_managed_hooks_only: None, feature_requirements: None, managed_hooks: None, mcp_servers: None, @@ -688,6 +690,7 @@ pub struct ConfigRequirementsToml { pub allowed_sandbox_modes: Option>, pub remote_sandbox_config: Option>, pub allowed_web_search_modes: Option>, + pub allow_managed_hooks_only: Option, #[serde(rename = "features", alias = "feature_requirements")] pub feature_requirements: Option, pub hooks: Option, @@ -736,6 +739,7 @@ pub struct ConfigRequirementsWithSources { pub allowed_approvals_reviewers: Option>>, pub allowed_sandbox_modes: Option>>, pub allowed_web_search_modes: Option>>, + pub allow_managed_hooks_only: Option>, pub feature_requirements: Option>, pub hooks: Option>, pub mcp_servers: Option>>, @@ -772,6 +776,7 @@ impl ConfigRequirementsWithSources { allowed_sandbox_modes: _, remote_sandbox_config: _, allowed_web_search_modes: _, + allow_managed_hooks_only: _, feature_requirements: _, hooks: _, mcp_servers: _, @@ -801,6 +806,7 @@ impl ConfigRequirementsWithSources { allowed_approvals_reviewers, allowed_sandbox_modes, allowed_web_search_modes, + allow_managed_hooks_only, feature_requirements, hooks, mcp_servers, @@ -828,6 +834,7 @@ impl ConfigRequirementsWithSources { allowed_approvals_reviewers, allowed_sandbox_modes, allowed_web_search_modes, + allow_managed_hooks_only, feature_requirements, hooks, mcp_servers, @@ -845,6 +852,7 @@ impl ConfigRequirementsWithSources { allowed_sandbox_modes: allowed_sandbox_modes.map(|sourced| sourced.value), remote_sandbox_config: None, allowed_web_search_modes: allowed_web_search_modes.map(|sourced| sourced.value), + allow_managed_hooks_only: allow_managed_hooks_only.map(|sourced| sourced.value), feature_requirements: feature_requirements.map(|sourced| sourced.value), hooks: hooks.map(|sourced| sourced.value), mcp_servers: mcp_servers.map(|sourced| sourced.value), @@ -928,6 +936,7 @@ impl ConfigRequirementsToml { && self.allowed_sandbox_modes.is_none() && self.remote_sandbox_config.is_none() && self.allowed_web_search_modes.is_none() + && self.allow_managed_hooks_only.is_none() && self .feature_requirements .as_ref() @@ -965,6 +974,7 @@ impl TryFrom for ConfigRequirements { allowed_approvals_reviewers, allowed_sandbox_modes, allowed_web_search_modes, + allow_managed_hooks_only, feature_requirements, hooks, mcp_servers, @@ -1200,6 +1210,7 @@ impl TryFrom for ConfigRequirements { approvals_reviewer, permission_profile, web_search_mode, + allow_managed_hooks_only, feature_requirements, managed_hooks, mcp_servers, @@ -1272,6 +1283,7 @@ mod tests { allowed_sandbox_modes, remote_sandbox_config: _, allowed_web_search_modes, + allow_managed_hooks_only, feature_requirements, hooks, mcp_servers, @@ -1292,6 +1304,8 @@ mod tests { .map(|value| Sourced::new(value, RequirementSource::Unknown)), allowed_web_search_modes: allowed_web_search_modes .map(|value| Sourced::new(value, RequirementSource::Unknown)), + allow_managed_hooks_only: allow_managed_hooks_only + .map(|value| Sourced::new(value, RequirementSource::Unknown)), feature_requirements: feature_requirements .map(|value| Sourced::new(value, RequirementSource::Unknown)), hooks: hooks.map(|value| Sourced::new(value, RequirementSource::Unknown)), @@ -1308,6 +1322,32 @@ mod tests { } } + #[test] + fn deserialize_allow_managed_hooks_only() -> Result<()> { + let requirements: ConfigRequirementsToml = from_str( + r#" + allow_managed_hooks_only = true + "#, + )?; + + assert_eq!(requirements.allow_managed_hooks_only, Some(true)); + assert!(!requirements.is_empty()); + Ok(()) + } + + #[test] + fn allow_managed_hooks_only_false_is_still_configured() -> Result<()> { + let requirements: ConfigRequirementsToml = from_str( + r#" + allow_managed_hooks_only = false + "#, + )?; + + assert_eq!(requirements.allow_managed_hooks_only, Some(false)); + assert!(!requirements.is_empty()); + Ok(()) + } + #[test] fn merge_unset_fields_copies_every_field_and_sets_sources() { let mut target = ConfigRequirementsWithSources::default(); @@ -1339,6 +1379,7 @@ mod tests { allowed_sandbox_modes: Some(allowed_sandbox_modes.clone()), remote_sandbox_config: None, allowed_web_search_modes: Some(allowed_web_search_modes.clone()), + allow_managed_hooks_only: Some(true), feature_requirements: Some(feature_requirements.clone()), hooks: None, mcp_servers: None, @@ -1369,6 +1410,10 @@ mod tests { allowed_web_search_modes, enforce_source.clone(), )), + allow_managed_hooks_only: Some(Sourced::new( + /*value*/ true, + enforce_source.clone(), + )), feature_requirements: Some(Sourced::new( feature_requirements, enforce_source.clone(), @@ -1411,6 +1456,7 @@ mod tests { allowed_approvals_reviewers: None, allowed_sandbox_modes: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, feature_requirements: None, hooks: None, mcp_servers: None, @@ -1458,6 +1504,7 @@ mod tests { allowed_approvals_reviewers: None, allowed_sandbox_modes: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, feature_requirements: None, hooks: None, mcp_servers: None, diff --git a/codex-rs/core/src/config/config_loader_tests.rs b/codex-rs/core/src/config/config_loader_tests.rs index 6296d1388..7d96ef26d 100644 --- a/codex-rs/core/src/config/config_loader_tests.rs +++ b/codex-rs/core/src/config/config_loader_tests.rs @@ -228,6 +228,76 @@ async fn returns_config_error_for_schema_error_in_user_config() { assert_eq!(config_error, &expected_config_error); } +#[tokio::test] +async fn top_level_allow_managed_hooks_only_in_user_config_does_not_enable_requirements_policy() +-> std::io::Result<()> { + let tmp = tempdir().expect("tempdir"); + std::fs::write( + tmp.path().join(CONFIG_TOML_FILE), + "allow_managed_hooks_only = true", + ) + .expect("write config"); + + let cwd = AbsolutePathBuf::try_from(tmp.path()).expect("cwd"); + let layers = load_config_layers_state( + LOCAL_FS.as_ref(), + tmp.path(), + Some(cwd), + &[] as &[(String, TomlValue)], + LoaderOverrides::default(), + CloudRequirementsLoader::default(), + &codex_config::NoopThreadConfigLoader, + ) + .await?; + + assert_eq!(layers.requirements_toml().allow_managed_hooks_only, None); + assert!(layers.requirements().allow_managed_hooks_only.is_none()); + + Ok(()) +} + +#[tokio::test] +async fn hooks_allow_managed_hooks_only_in_user_config_does_not_enable_requirements_policy() +-> std::io::Result<()> { + let tmp = tempdir().expect("tempdir"); + let contents = r#" +[hooks] +allow_managed_hooks_only = true + +[[hooks.PreToolUse]] +matcher = "^Bash$" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "python3 /tmp/user-hook.py" +"#; + std::fs::write(tmp.path().join(CONFIG_TOML_FILE), contents).expect("write config"); + + let cwd = AbsolutePathBuf::try_from(tmp.path()).expect("cwd"); + let layers = load_config_layers_state( + LOCAL_FS.as_ref(), + tmp.path(), + Some(cwd), + &[] as &[(String, TomlValue)], + LoaderOverrides::default(), + CloudRequirementsLoader::default(), + &codex_config::NoopThreadConfigLoader, + ) + .await?; + + assert!( + layers + .get_user_layer() + .and_then(|layer| layer.config.get("hooks")) + .is_some(), + "hooks should still deserialize from config.toml" + ); + assert_eq!(layers.requirements_toml().allow_managed_hooks_only, None); + assert!(layers.requirements().allow_managed_hooks_only.is_none()); + + Ok(()) +} + #[test] fn schema_error_points_to_feature_value() { let tmp = tempdir().expect("tempdir"); @@ -777,6 +847,7 @@ allowed_approval_policies = ["on-request"] allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, feature_requirements: None, hooks: None, mcp_servers: None, @@ -834,6 +905,7 @@ allowed_approval_policies = ["on-request"] allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, feature_requirements: None, hooks: None, mcp_servers: None, @@ -1042,6 +1114,7 @@ async fn load_config_layers_includes_cloud_requirements() -> anyhow::Result<()> allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, feature_requirements: None, hooks: None, mcp_servers: None, diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 664040e2b..76520d23e 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -8167,6 +8167,7 @@ async fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset() allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: Some(vec![codex_config::WebSearchModeRequirement::Cached]), + allow_managed_hooks_only: None, feature_requirements: None, hooks: None, mcp_servers: None, @@ -8879,6 +8880,7 @@ async fn explicit_sandbox_mode_falls_back_when_disallowed_by_requirements() -> s allowed_sandbox_modes: Some(vec![codex_config::SandboxModeRequirement::ReadOnly]), remote_sandbox_config: None, allowed_web_search_modes: None, + allow_managed_hooks_only: None, feature_requirements: None, hooks: None, mcp_servers: None, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 21bad4ee5..d1145c2e5 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -2125,6 +2125,7 @@ impl Config { approvals_reviewer: mut constrained_approvals_reviewer, permission_profile: mut constrained_permission_profile, web_search_mode: mut constrained_web_search_mode, + allow_managed_hooks_only: _, feature_requirements, managed_hooks: _, mcp_servers, diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index 8dac67e30..e4bb3c884 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::fs; use std::path::Path; @@ -19,7 +20,6 @@ use codex_plugin::PluginHookSource; use codex_utils_absolute_path::AbsolutePathBuf; use serde::Deserialize; use serde::Serialize; -use std::collections::HashMap; use super::ConfiguredHandler; use super::HookListEntry; @@ -46,6 +46,17 @@ struct HookHandlerSource<'a> { plugin_id: Option, } +#[derive(Clone, Copy)] +struct HookDiscoveryPolicy { + allow_managed_hooks_only: bool, +} + +impl HookDiscoveryPolicy { + fn allows(self, source: &HookHandlerSource<'_>) -> bool { + !self.allow_managed_hooks_only || source.is_managed + } +} + pub(crate) fn discover_handlers( config_layer_stack: Option<&ConfigLayerStack>, plugin_hook_sources: Vec, @@ -56,6 +67,15 @@ pub(crate) fn discover_handlers( let mut warnings = plugin_hook_load_warnings; let mut display_order = 0_i64; let hook_states = hook_states_from_stack(config_layer_stack); + let policy = HookDiscoveryPolicy { + allow_managed_hooks_only: config_layer_stack.is_some_and(|config_layer_stack| { + config_layer_stack + .requirements() + .allow_managed_hooks_only + .as_ref() + .is_some_and(|requirement| requirement.value) + }), + }; if let Some(config_layer_stack) = config_layer_stack { append_managed_requirement_handlers( @@ -65,6 +85,7 @@ pub(crate) fn discover_handlers( &mut display_order, config_layer_stack, &hook_states, + policy, ); for layer in config_layer_stack.get_layers( @@ -72,6 +93,19 @@ pub(crate) fn discover_handlers( /*include_disabled*/ false, ) { let (hook_source, is_managed) = hook_metadata_for_config_layer_source(&layer.name); + let policy_path = config_toml_source_path(layer); + let policy_source = HookHandlerSource { + path: &policy_path, + key_source: policy_path.display().to_string(), + source: hook_source, + is_managed, + hook_states: &hook_states, + env: HashMap::new(), + plugin_id: None, + }; + if !policy.allows(&policy_source) { + continue; + } let json_hooks = load_hooks_json(layer.config_folder().as_deref(), &mut warnings); let toml_hooks = load_toml_hooks_from_layer(layer, &mut warnings); @@ -103,6 +137,7 @@ pub(crate) fn discover_handlers( plugin_id: None, }, hook_events, + policy, ); } } @@ -115,6 +150,7 @@ pub(crate) fn discover_handlers( &mut display_order, plugin_hook_sources, &hook_states, + policy, ); DiscoveryResult { @@ -131,15 +167,12 @@ fn append_managed_requirement_handlers( display_order: &mut i64, config_layer_stack: &ConfigLayerStack, hook_states: &HashMap, + policy: HookDiscoveryPolicy, ) { let Some(managed_hooks) = config_layer_stack.requirements().managed_hooks.as_ref() else { return; }; - let Some(source_path) = - managed_hooks_source_path(managed_hooks.get(), managed_hooks.source.as_ref(), warnings) - else { - return; - }; + let source_path = managed_hooks_source_path(managed_hooks.get(), managed_hooks.source.as_ref()); append_hook_events( handlers, hook_entries, @@ -155,6 +188,7 @@ fn append_managed_requirement_handlers( plugin_id: None, }, managed_hooks.get().hooks.clone(), + policy, ); } @@ -165,6 +199,7 @@ fn append_plugin_hook_sources( display_order: &mut i64, plugin_hook_sources: Vec, hook_states: &HashMap, + policy: HookDiscoveryPolicy, ) { for source in plugin_hook_sources { let PluginHookSource { @@ -203,6 +238,7 @@ fn append_plugin_hook_sources( plugin_id: Some(plugin_id), }, hooks, + policy, ); } } @@ -210,45 +246,35 @@ fn append_plugin_hook_sources( fn managed_hooks_source_path( managed_hooks: &ManagedHooksRequirementsToml, requirement_source: Option<&RequirementSource>, - warnings: &mut Vec, -) -> Option { - let source = requirement_source - .map(ToString::to_string) - .unwrap_or_else(|| "managed requirements".to_string()); - let Some(source_path) = managed_hooks.managed_dir_for_current_platform() else { - warnings.push(format!( - "skipping managed hooks from {source}: no managed hook directory is configured for this platform" - )); - return None; - }; +) -> AbsolutePathBuf { + if let Some(source_path) = managed_hooks.managed_dir_for_current_platform() + && source_path.is_absolute() + && let Ok(source_path) = AbsolutePathBuf::from_absolute_path(source_path) + { + return source_path; + } - if !source_path.is_absolute() { - warnings.push(format!( - "skipping managed hooks from {source}: managed hook directory {} is not absolute", - source_path.display() - )); - None - } else if !source_path.exists() { - warnings.push(format!( - "skipping managed hooks from {source}: managed hook directory {} does not exist", - source_path.display() - )); - None - } else if !source_path.is_dir() { - warnings.push(format!( - "skipping managed hooks from {source}: managed hook directory {} is not a directory", - source_path.display() - )); - None - } else { - AbsolutePathBuf::from_absolute_path(source_path) - .inspect_err(|err| { - warnings.push(format!( - "skipping managed hooks from {source}: could not normalize managed hook directory {}: {err}", - source_path.display() - )); - }) - .ok() + fallback_managed_hooks_source_path(requirement_source) +} + +fn fallback_managed_hooks_source_path( + requirement_source: Option<&RequirementSource>, +) -> AbsolutePathBuf { + match requirement_source { + Some(RequirementSource::SystemRequirementsToml { file }) + | Some(RequirementSource::LegacyManagedConfigTomlFromFile { file }) => file.clone(), + Some(RequirementSource::MdmManagedPreferences { domain, key }) => { + synthetic_layer_path(&format!("/requirements.toml")) + } + Some(RequirementSource::CloudRequirements) => { + synthetic_layer_path("/requirements.toml") + } + Some(RequirementSource::LegacyManagedConfigTomlFromMdm) => { + synthetic_layer_path("/managed_config.toml") + } + Some(RequirementSource::Unknown) | None => { + synthetic_layer_path("/requirements.toml") + } } } @@ -350,7 +376,12 @@ fn append_hook_events( display_order: &mut i64, source: HookHandlerSource<'_>, hook_events: HookEventsToml, + policy: HookDiscoveryPolicy, ) { + if !policy.allows(&source) { + return; + } + for (event_name, groups) in hook_events.into_matcher_groups() { append_matcher_groups( handlers, diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index cb5682690..af13687a0 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -15,6 +15,7 @@ use codex_config::HookHandlerConfig; use codex_config::ManagedHooksRequirementsToml; use codex_config::MatcherGroup; use codex_config::RequirementSource; +use codex_config::Sourced; use codex_config::TomlValue; use codex_plugin::PluginHookSource; use codex_plugin::PluginId; @@ -55,6 +56,88 @@ fn managed_hooks_for_current_platform( } } +fn pre_tool_use_hook_events(command: impl Into) -> HookEventsToml { + HookEventsToml { + pre_tool_use: vec![MatcherGroup { + matcher: Some("^Bash$".to_string()), + hooks: vec![HookHandlerConfig::Command { + command: command.into(), + command_windows: None, + timeout_sec: Some(10), + r#async: false, + status_message: Some("checking".to_string()), + }], + }], + ..Default::default() + } +} + +fn config_toml_with_pre_tool_use(command: &str) -> TomlValue { + let mut config_toml = TomlValue::Table(Default::default()); + let TomlValue::Table(config_table) = &mut config_toml else { + unreachable!("config TOML root should be a table"); + }; + let mut hooks_table = TomlValue::Table(Default::default()); + let TomlValue::Table(hooks_entries) = &mut hooks_table else { + unreachable!("hooks entry should be a table"); + }; + let mut pre_tool_use_group = TomlValue::Table(Default::default()); + let TomlValue::Table(pre_tool_use_group_entries) = &mut pre_tool_use_group else { + unreachable!("PreToolUse group should be a table"); + }; + pre_tool_use_group_entries.insert( + "matcher".to_string(), + TomlValue::String("^Bash$".to_string()), + ); + let mut handler = TomlValue::Table(Default::default()); + let TomlValue::Table(handler_entries) = &mut handler else { + unreachable!("PreToolUse handler should be a table"); + }; + handler_entries.insert("type".to_string(), TomlValue::String("command".to_string())); + handler_entries.insert( + "command".to_string(), + TomlValue::String(command.to_string()), + ); + handler_entries.insert("timeout".to_string(), TomlValue::Integer(10)); + handler_entries.insert( + "statusMessage".to_string(), + TomlValue::String("checking".to_string()), + ); + pre_tool_use_group_entries.insert("hooks".to_string(), TomlValue::Array(vec![handler])); + hooks_entries.insert( + "PreToolUse".to_string(), + TomlValue::Array(vec![pre_tool_use_group]), + ); + config_table.insert("hooks".to_string(), hooks_table); + config_toml +} + +fn requirements_with_managed_hooks_only( + allow_managed_hooks_only: bool, + managed_hooks: Option, +) -> (ConfigRequirements, ConfigRequirementsToml) { + ( + ConfigRequirements { + allow_managed_hooks_only: Some(Sourced::new( + allow_managed_hooks_only, + RequirementSource::CloudRequirements, + )), + managed_hooks: managed_hooks.clone().map(|hooks| { + ConstrainedWithSource::new( + Constrained::allow_any(hooks), + Some(RequirementSource::CloudRequirements), + ) + }), + ..ConfigRequirements::default() + }, + ConfigRequirementsToml { + allow_managed_hooks_only: Some(allow_managed_hooks_only), + hooks: managed_hooks, + ..ConfigRequirementsToml::default() + }, + ) +} + #[tokio::test] async fn requirements_managed_hooks_execute_from_managed_dir() { let temp = tempdir().expect("create temp dir"); @@ -535,7 +618,7 @@ fn trusted_plugin_hook_stack( } #[test] -fn requirements_managed_hooks_warn_when_managed_dir_is_missing() { +fn requirements_managed_hooks_load_when_managed_dir_is_missing() { let temp = tempdir().expect("create temp dir"); let missing_dir = temp.path().join("missing-managed-hooks"); let managed_hooks = managed_hooks_for_current_platform( @@ -544,7 +627,7 @@ fn requirements_managed_hooks_warn_when_managed_dir_is_missing() { pre_tool_use: vec![MatcherGroup { matcher: Some("^Bash$".to_string()), hooks: vec![HookHandlerConfig::Command { - command: format!("python3 {}", missing_dir.join("pre.py").display()), + command: "echo hi".to_string(), command_windows: None, timeout_sec: Some(10), r#async: false, @@ -581,30 +664,294 @@ fn requirements_managed_hooks_warn_when_managed_dir_is_missing() { }, ); - assert!(engine.warnings().iter().any(|warning| { - warning.contains("managed hook directory") - && warning.contains("does not exist") - && warning.contains(&missing_dir.display().to_string()) - })); + assert!(engine.warnings().is_empty()); let cwd = cwd(); - assert!( - engine - .preview_pre_tool_use(&PreToolUseRequest { - session_id: ThreadId::new(), - turn_id: "turn-1".to_string(), - cwd, - transcript_path: None, - model: "gpt-test".to_string(), - permission_mode: "default".to_string(), - tool_name: "Bash".to_string(), - matcher_aliases: Vec::new(), - tool_use_id: "tool-1".to_string(), - tool_input: serde_json::json!({ "command": "echo hello" }), - }) - .is_empty() + let preview = engine.preview_pre_tool_use(&PreToolUseRequest { + session_id: ThreadId::new(), + turn_id: "turn-1".to_string(), + cwd, + transcript_path: None, + model: "gpt-test".to_string(), + permission_mode: "default".to_string(), + tool_name: "Bash".to_string(), + matcher_aliases: Vec::new(), + tool_use_id: "tool-1".to_string(), + tool_input: serde_json::json!({ "command": "echo hello" }), + }); + assert_eq!(preview.len(), 1); + assert_eq!(engine.handlers[0].command, "echo hi"); + assert_eq!( + engine.handlers[0].source_path, + AbsolutePathBuf::try_from(missing_dir).expect("absolute missing dir") ); } +#[test] +fn allow_managed_hooks_only_false_keeps_unmanaged_hooks() { + let temp = tempdir().expect("create temp dir"); + let config_path = + AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute config path"); + let (requirements, requirements_toml) = requirements_with_managed_hooks_only( + /*allow_managed_hooks_only*/ false, /*managed_hooks*/ None, + ); + let config_layer_stack = ConfigLayerStack::new( + vec![ConfigLayerEntry::new( + ConfigLayerSource::User { file: config_path }, + config_toml_with_pre_tool_use("python3 /tmp/user-hook.py"), + )], + requirements, + requirements_toml, + ) + .expect("config layer stack"); + + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + Some(&config_layer_stack), + Vec::new(), + Vec::new(), + CommandShell { + program: String::new(), + args: Vec::new(), + }, + ); + + assert!(engine.warnings().is_empty()); + assert!(engine.handlers.is_empty()); + let discovered = + super::discovery::discover_handlers(Some(&config_layer_stack), Vec::new(), Vec::new()); + assert_eq!(discovered.hook_entries.len(), 1); + assert!(!discovered.hook_entries[0].is_managed); + assert_eq!( + discovered.hook_entries[0].command.as_deref(), + Some("python3 /tmp/user-hook.py") + ); +} + +#[test] +fn allow_managed_hooks_only_in_config_toml_does_not_enable_policy() { + let temp = tempdir().expect("create temp dir"); + let config_path = + AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute config path"); + let mut config_toml = config_toml_with_pre_tool_use("python3 /tmp/user-hook.py"); + let TomlValue::Table(config_table) = &mut config_toml else { + unreachable!("config TOML root should be a table"); + }; + config_table.insert( + "allow_managed_hooks_only".to_string(), + TomlValue::Boolean(true), + ); + let config_layer_stack = ConfigLayerStack::new( + vec![ConfigLayerEntry::new( + ConfigLayerSource::User { file: config_path }, + config_toml, + )], + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("config layer stack"); + + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + Some(&config_layer_stack), + Vec::new(), + Vec::new(), + CommandShell { + program: String::new(), + args: Vec::new(), + }, + ); + + assert!(engine.warnings().is_empty()); + assert!(engine.handlers.is_empty()); + let discovered = + super::discovery::discover_handlers(Some(&config_layer_stack), Vec::new(), Vec::new()); + assert_eq!(discovered.hook_entries.len(), 1); + assert!(!discovered.hook_entries[0].is_managed); + assert_eq!( + discovered.hook_entries[0].command.as_deref(), + Some("python3 /tmp/user-hook.py") + ); +} + +#[test] +fn allow_managed_hooks_only_skips_unmanaged_json_and_toml_hooks() { + let temp = tempdir().expect("create temp dir"); + let config_path = + AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute config path"); + let hooks_json_path = + AbsolutePathBuf::try_from(temp.path().join("hooks.json")).expect("absolute hooks path"); + fs::write( + hooks_json_path.as_path(), + r#"{ + "hooks": { + "PreToolUse": [ + { + "matcher": "^Bash$", + "hooks": [ + { + "type": "command", + "command": "python3 /tmp/json-hook.py" + } + ] + } + ] + } + }"#, + ) + .expect("write hooks.json"); + let (requirements, requirements_toml) = requirements_with_managed_hooks_only( + /*allow_managed_hooks_only*/ true, /*managed_hooks*/ None, + ); + let config_layer_stack = ConfigLayerStack::new( + vec![ConfigLayerEntry::new( + ConfigLayerSource::User { file: config_path }, + config_toml_with_pre_tool_use("python3 /tmp/toml-hook.py"), + )], + requirements, + requirements_toml, + ) + .expect("config layer stack"); + + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + Some(&config_layer_stack), + Vec::new(), + Vec::new(), + CommandShell { + program: String::new(), + args: Vec::new(), + }, + ); + + assert!(engine.handlers.is_empty()); + assert!(engine.warnings().is_empty()); +} + +#[test] +fn allow_managed_hooks_only_skips_unmanaged_plugin_hooks() { + let temp = tempdir().expect("create temp dir"); + let plugin_root = + AbsolutePathBuf::try_from(temp.path().join("demo-plugin")).expect("plugin root"); + let plugin_data_root = + AbsolutePathBuf::try_from(temp.path().join("plugin-data")).expect("plugin data root"); + let source_path = plugin_root.join("hooks/hooks.json"); + let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id"); + let plugin_hook_sources = vec![PluginHookSource { + plugin_id, + plugin_root, + plugin_data_root, + source_path, + source_relative_path: "hooks/hooks.json".to_string(), + hooks: pre_tool_use_hook_events("python3 /tmp/plugin-hook.py"), + }]; + let (requirements, requirements_toml) = requirements_with_managed_hooks_only( + /*allow_managed_hooks_only*/ true, /*managed_hooks*/ None, + ); + let config_layer_stack = ConfigLayerStack::new(Vec::new(), requirements, requirements_toml) + .expect("config layer stack"); + + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + Some(&config_layer_stack), + plugin_hook_sources, + Vec::new(), + CommandShell { + program: String::new(), + args: Vec::new(), + }, + ); + + assert!(engine.handlers.is_empty()); + assert!(engine.warnings().is_empty()); +} + +#[test] +fn allow_managed_hooks_only_keeps_managed_requirement_and_config_layer_hooks() { + let temp = tempdir().expect("create temp dir"); + let managed_dir = + AbsolutePathBuf::try_from(temp.path().join("managed-hooks")).expect("absolute path"); + fs::create_dir_all(managed_dir.as_path()).expect("create managed hooks dir"); + let system_config_path = + AbsolutePathBuf::try_from(temp.path().join("system").join("config.toml")) + .expect("absolute system config path"); + let system_parent = system_config_path + .as_path() + .parent() + .expect("system config parent"); + fs::create_dir_all(system_parent).expect("create system config dir"); + let legacy_config_path = AbsolutePathBuf::try_from(temp.path().join("managed_config.toml")) + .expect("absolute legacy config path"); + + let managed_hooks = managed_hooks_for_current_platform( + managed_dir, + pre_tool_use_hook_events("python3 /tmp/requirements-hook.py"), + ); + let (requirements, requirements_toml) = requirements_with_managed_hooks_only( + /*allow_managed_hooks_only*/ true, + Some(managed_hooks), + ); + let config_layer_stack = ConfigLayerStack::new( + vec![ + ConfigLayerEntry::new( + ConfigLayerSource::Mdm { + domain: "com.openai.codex".to_string(), + key: "config".to_string(), + }, + config_toml_with_pre_tool_use("python3 /tmp/mdm-hook.py"), + ), + ConfigLayerEntry::new( + ConfigLayerSource::System { + file: system_config_path, + }, + config_toml_with_pre_tool_use("python3 /tmp/system-hook.py"), + ), + ConfigLayerEntry::new( + ConfigLayerSource::LegacyManagedConfigTomlFromFile { + file: legacy_config_path, + }, + config_toml_with_pre_tool_use("python3 /tmp/legacy-file-hook.py"), + ), + ConfigLayerEntry::new( + ConfigLayerSource::LegacyManagedConfigTomlFromMdm, + config_toml_with_pre_tool_use("python3 /tmp/legacy-mdm-hook.py"), + ), + ], + requirements, + requirements_toml, + ) + .expect("config layer stack"); + + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + Some(&config_layer_stack), + Vec::new(), + Vec::new(), + CommandShell { + program: String::new(), + args: Vec::new(), + }, + ); + + assert!(engine.warnings().is_empty()); + assert_eq!( + engine + .handlers + .iter() + .map(|handler| handler.command.as_str()) + .collect::>(), + vec![ + "python3 /tmp/requirements-hook.py", + "python3 /tmp/mdm-hook.py", + "python3 /tmp/system-hook.py", + "python3 /tmp/legacy-file-hook.py", + "python3 /tmp/legacy-mdm-hook.py", + ] + ); + let discovered = + super::discovery::discover_handlers(Some(&config_layer_stack), Vec::new(), Vec::new()); + assert!(discovered.hook_entries.iter().all(|entry| entry.is_managed)); +} + #[test] fn discovers_hooks_from_json_and_toml_in_the_same_layer() { let temp = tempdir().expect("create temp dir"); diff --git a/codex-rs/tui/src/debug_config.rs b/codex-rs/tui/src/debug_config.rs index 3f5b53266..3dd2d8235 100644 --- a/codex-rs/tui/src/debug_config.rs +++ b/codex-rs/tui/src/debug_config.rs @@ -145,6 +145,17 @@ fn render_debug_config_lines(stack: &ConfigLayerStack) -> Vec> { )); } + if let Some(allow_managed_hooks_only) = requirements_toml.allow_managed_hooks_only { + requirement_lines.push(requirement_line( + "allow_managed_hooks_only", + allow_managed_hooks_only.to_string(), + requirements + .allow_managed_hooks_only + .as_ref() + .map(|sourced| &sourced.source), + )); + } + if requirements_toml.guardian_policy_config.is_some() { requirement_lines.push(requirement_line( "guardian_policy_config", @@ -647,6 +658,10 @@ mod tests { Constrained::allow_any(WebSearchMode::Cached), Some(RequirementSource::CloudRequirements), ), + allow_managed_hooks_only: Some(Sourced::new( + /*value*/ true, + RequirementSource::CloudRequirements, + )), feature_requirements: Some(Sourced::new( FeatureRequirementsToml { entries: BTreeMap::from([("guardian_approval".to_string(), true)]), @@ -684,6 +699,7 @@ mod tests { allowed_sandbox_modes: Some(vec![SandboxModeRequirement::ReadOnly]), remote_sandbox_config: None, allowed_web_search_modes: Some(vec![WebSearchModeRequirement::Cached]), + allow_managed_hooks_only: Some(true), guardian_policy_config: Some("Use the managed guardian policy.".to_string()), feature_requirements: Some(FeatureRequirementsToml { entries: BTreeMap::from([("guardian_approval".to_string(), true)]), @@ -741,6 +757,7 @@ mod tests { "allowed_web_search_modes: cached, disabled (source: cloud requirements)" ) ); + assert!(rendered.contains("allow_managed_hooks_only: true (source: cloud requirements)")); assert!( rendered.contains("guardian_policy_config: configured (source: cloud requirements)") ); @@ -893,6 +910,7 @@ approval_policy = "never" allowed_sandbox_modes: None, remote_sandbox_config: None, allowed_web_search_modes: Some(Vec::new()), + allow_managed_hooks_only: None, guardian_policy_config: None, feature_requirements: None, hooks: None, diff --git a/docs/config.md b/docs/config.md index 950a631fc..d35b0a87e 100644 --- a/docs/config.md +++ b/docs/config.md @@ -5,3 +5,11 @@ For basic configuration instructions, see [this documentation](https://developer For advanced configuration instructions, see [this documentation](https://developers.openai.com/codex/config-advanced). For a full configuration reference, see [this documentation](https://developers.openai.com/codex/config-reference). + +## Lifecycle hooks + +Admins can set top-level `allow_managed_hooks_only = true` in +`requirements.toml` to ignore user, project, and session hook configs while +still allowing managed hooks from requirements and managed config layers. This +setting is only supported in `requirements.toml`; putting it in `config.toml` +does not enable managed-hooks-only mode.