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 e16d84cadd28..5e5269bfb24b 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 @@ -7325,6 +7325,100 @@ "title": "ConfigWriteResponse", "type": "object" }, + "ConfiguredHookHandler": { + "oneOf": [ + { + "properties": { + "async": { + "type": "boolean" + }, + "command": { + "type": "string" + }, + "statusMessage": { + "type": [ + "string", + "null" + ] + }, + "timeoutSec": { + "format": "uint64", + "minimum": 0.0, + "type": [ + "integer", + "null" + ] + }, + "type": { + "enum": [ + "command" + ], + "title": "CommandConfiguredHookHandlerType", + "type": "string" + } + }, + "required": [ + "async", + "command", + "type" + ], + "title": "CommandConfiguredHookHandler", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "prompt" + ], + "title": "PromptConfiguredHookHandlerType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "PromptConfiguredHookHandler", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "agent" + ], + "title": "AgentConfiguredHookHandlerType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "AgentConfiguredHookHandler", + "type": "object" + } + ] + }, + "ConfiguredHookMatcherGroup": { + "properties": { + "hooks": { + "items": { + "$ref": "#/definitions/v2/ConfiguredHookHandler" + }, + "type": "array" + }, + "matcher": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "hooks" + ], + "type": "object" + }, "ContentItem": { "oneOf": [ { @@ -10018,6 +10112,67 @@ "title": "LogoutAccountResponse", "type": "object" }, + "ManagedHooksRequirements": { + "properties": { + "PermissionRequest": { + "items": { + "$ref": "#/definitions/v2/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "PostToolUse": { + "items": { + "$ref": "#/definitions/v2/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "PreToolUse": { + "items": { + "$ref": "#/definitions/v2/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "SessionStart": { + "items": { + "$ref": "#/definitions/v2/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "Stop": { + "items": { + "$ref": "#/definitions/v2/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "UserPromptSubmit": { + "items": { + "$ref": "#/definitions/v2/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "managedDir": { + "type": [ + "string", + "null" + ] + }, + "windowsManagedDir": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "PermissionRequest", + "PostToolUse", + "PreToolUse", + "SessionStart", + "Stop", + "UserPromptSubmit" + ], + "type": "object" + }, "MarketplaceAddParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { 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 20e0f8f6e33c..aaf08566270e 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 @@ -3905,6 +3905,100 @@ "title": "ConfigWriteResponse", "type": "object" }, + "ConfiguredHookHandler": { + "oneOf": [ + { + "properties": { + "async": { + "type": "boolean" + }, + "command": { + "type": "string" + }, + "statusMessage": { + "type": [ + "string", + "null" + ] + }, + "timeoutSec": { + "format": "uint64", + "minimum": 0.0, + "type": [ + "integer", + "null" + ] + }, + "type": { + "enum": [ + "command" + ], + "title": "CommandConfiguredHookHandlerType", + "type": "string" + } + }, + "required": [ + "async", + "command", + "type" + ], + "title": "CommandConfiguredHookHandler", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "prompt" + ], + "title": "PromptConfiguredHookHandlerType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "PromptConfiguredHookHandler", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "agent" + ], + "title": "AgentConfiguredHookHandlerType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "AgentConfiguredHookHandler", + "type": "object" + } + ] + }, + "ConfiguredHookMatcherGroup": { + "properties": { + "hooks": { + "items": { + "$ref": "#/definitions/ConfiguredHookHandler" + }, + "type": "array" + }, + "matcher": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "hooks" + ], + "type": "object" + }, "ContentItem": { "oneOf": [ { @@ -6753,6 +6847,67 @@ "title": "LogoutAccountResponse", "type": "object" }, + "ManagedHooksRequirements": { + "properties": { + "PermissionRequest": { + "items": { + "$ref": "#/definitions/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "PostToolUse": { + "items": { + "$ref": "#/definitions/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "PreToolUse": { + "items": { + "$ref": "#/definitions/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "SessionStart": { + "items": { + "$ref": "#/definitions/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "Stop": { + "items": { + "$ref": "#/definitions/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "UserPromptSubmit": { + "items": { + "$ref": "#/definitions/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "managedDir": { + "type": [ + "string", + "null" + ] + }, + "windowsManagedDir": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "PermissionRequest", + "PostToolUse", + "PreToolUse", + "SessionStart", + "Stop", + "UserPromptSubmit" + ], + "type": "object" + }, "MarketplaceAddParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { 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 f75833af1ab7..545d8dc9b406 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json @@ -111,6 +111,161 @@ }, "type": "object" }, + "ConfiguredHookHandler": { + "oneOf": [ + { + "properties": { + "async": { + "type": "boolean" + }, + "command": { + "type": "string" + }, + "statusMessage": { + "type": [ + "string", + "null" + ] + }, + "timeoutSec": { + "format": "uint64", + "minimum": 0.0, + "type": [ + "integer", + "null" + ] + }, + "type": { + "enum": [ + "command" + ], + "title": "CommandConfiguredHookHandlerType", + "type": "string" + } + }, + "required": [ + "async", + "command", + "type" + ], + "title": "CommandConfiguredHookHandler", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "prompt" + ], + "title": "PromptConfiguredHookHandlerType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "PromptConfiguredHookHandler", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "agent" + ], + "title": "AgentConfiguredHookHandlerType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "AgentConfiguredHookHandler", + "type": "object" + } + ] + }, + "ConfiguredHookMatcherGroup": { + "properties": { + "hooks": { + "items": { + "$ref": "#/definitions/ConfiguredHookHandler" + }, + "type": "array" + }, + "matcher": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "hooks" + ], + "type": "object" + }, + "ManagedHooksRequirements": { + "properties": { + "PermissionRequest": { + "items": { + "$ref": "#/definitions/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "PostToolUse": { + "items": { + "$ref": "#/definitions/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "PreToolUse": { + "items": { + "$ref": "#/definitions/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "SessionStart": { + "items": { + "$ref": "#/definitions/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "Stop": { + "items": { + "$ref": "#/definitions/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "UserPromptSubmit": { + "items": { + "$ref": "#/definitions/ConfiguredHookMatcherGroup" + }, + "type": "array" + }, + "managedDir": { + "type": [ + "string", + "null" + ] + }, + "windowsManagedDir": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "PermissionRequest", + "PostToolUse", + "PreToolUse", + "SessionStart", + "Stop", + "UserPromptSubmit" + ], + "type": "object" + }, "NetworkDomainPermission": { "enum": [ "allow", diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ConfiguredHookHandler.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ConfiguredHookHandler.ts new file mode 100644 index 000000000000..a81ce61f6e97 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ConfiguredHookHandler.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type ConfiguredHookHandler = { "type": "command", command: string, timeoutSec: bigint | null, async: boolean, statusMessage: string | null, } | { "type": "prompt", } | { "type": "agent", }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ConfiguredHookMatcherGroup.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ConfiguredHookMatcherGroup.ts new file mode 100644 index 000000000000..2c00fc166c2d --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ConfiguredHookMatcherGroup.ts @@ -0,0 +1,6 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { ConfiguredHookHandler } from "./ConfiguredHookHandler"; + +export type ConfiguredHookMatcherGroup = { matcher: string | null, hooks: Array, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ManagedHooksRequirements.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ManagedHooksRequirements.ts new file mode 100644 index 000000000000..3386d16ec325 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ManagedHooksRequirements.ts @@ -0,0 +1,6 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { ConfiguredHookMatcherGroup } from "./ConfiguredHookMatcherGroup"; + +export type ManagedHooksRequirements = { managedDir: string | null, windowsManagedDir: string | null, PreToolUse: Array, PermissionRequest: Array, PostToolUse: Array, SessionStart: Array, UserPromptSubmit: Array, Stop: Array, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts index aa0ed36d256b..72a7a0d63ba4 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -71,6 +71,8 @@ export type { ConfigRequirementsReadResponse } from "./ConfigRequirementsReadRes export type { ConfigValueWriteParams } from "./ConfigValueWriteParams"; export type { ConfigWarningNotification } from "./ConfigWarningNotification"; export type { ConfigWriteResponse } from "./ConfigWriteResponse"; +export type { ConfiguredHookHandler } from "./ConfiguredHookHandler"; +export type { ConfiguredHookMatcherGroup } from "./ConfiguredHookMatcherGroup"; export type { ContextCompactedNotification } from "./ContextCompactedNotification"; export type { CreditsSnapshot } from "./CreditsSnapshot"; export type { DeprecationNoticeNotification } from "./DeprecationNoticeNotification"; @@ -169,6 +171,7 @@ export type { ListMcpServerStatusResponse } from "./ListMcpServerStatusResponse" export type { LoginAccountParams } from "./LoginAccountParams"; export type { LoginAccountResponse } from "./LoginAccountResponse"; export type { LogoutAccountResponse } from "./LogoutAccountResponse"; +export type { ManagedHooksRequirements } from "./ManagedHooksRequirements"; export type { MarketplaceAddParams } from "./MarketplaceAddParams"; export type { MarketplaceAddResponse } from "./MarketplaceAddResponse"; export type { MarketplaceInterface } from "./MarketplaceInterface"; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 1f23c7b53648..9f1801c64f31 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -941,11 +941,71 @@ pub struct ConfigRequirements { pub allowed_sandbox_modes: Option>, pub allowed_web_search_modes: Option>, pub feature_requirements: Option>, + #[experimental("configRequirements/read.hooks")] + pub hooks: Option, pub enforce_residency: Option, #[experimental("configRequirements/read.network")] pub network: Option, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ManagedHooksRequirements { + pub managed_dir: Option, + pub windows_managed_dir: Option, + #[serde(rename = "PreToolUse")] + #[ts(rename = "PreToolUse")] + pub pre_tool_use: Vec, + #[serde(rename = "PermissionRequest")] + #[ts(rename = "PermissionRequest")] + pub permission_request: Vec, + #[serde(rename = "PostToolUse")] + #[ts(rename = "PostToolUse")] + pub post_tool_use: Vec, + #[serde(rename = "SessionStart")] + #[ts(rename = "SessionStart")] + pub session_start: Vec, + #[serde(rename = "UserPromptSubmit")] + #[ts(rename = "UserPromptSubmit")] + pub user_prompt_submit: Vec, + #[serde(rename = "Stop")] + #[ts(rename = "Stop")] + pub stop: Vec, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ConfiguredHookMatcherGroup { + pub matcher: Option, + pub hooks: Vec, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(tag = "type")] +#[ts(tag = "type", export_to = "v2/")] +pub enum ConfiguredHookHandler { + #[serde(rename = "command")] + #[ts(rename = "command")] + Command { + command: String, + #[serde(rename = "timeoutSec")] + #[ts(rename = "timeoutSec")] + timeout_sec: Option, + r#async: bool, + #[serde(rename = "statusMessage")] + #[ts(rename = "statusMessage")] + status_message: Option, + }, + #[serde(rename = "prompt")] + #[ts(rename = "prompt")] + Prompt {}, + #[serde(rename = "agent")] + #[ts(rename = "agent")] + Agent {}, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -8837,6 +8897,7 @@ mod tests { allowed_sandbox_modes: None, allowed_web_search_modes: None, feature_requirements: None, + hooks: None, enforce_residency: None, network: None, }); diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 15b7f9a5be87..8a6ce3b728ed 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -209,7 +209,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 `details` returned by detect. When a request includes plugin imports, the server emits `externalAgentConfig/import/completed` after the full import finishes (immediately after the response when everything completed synchronously, or after background remote 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`), `enforceResidency`, and `network` constraints such as canonical domain/socket permissions plus `managedAllowedDomainsOnly`. +- `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`. ### Example: Start or resume a thread diff --git a/codex-rs/app-server/src/config_api.rs b/codex-rs/app-server/src/config_api.rs index a1df875c28ca..ce0ea340697b 100644 --- a/codex-rs/app-server/src/config_api.rs +++ b/codex-rs/app-server/src/config_api.rs @@ -12,9 +12,12 @@ use codex_app_server_protocol::ConfigRequirementsReadResponse; use codex_app_server_protocol::ConfigValueWriteParams; use codex_app_server_protocol::ConfigWriteErrorCode; use codex_app_server_protocol::ConfigWriteResponse; +use codex_app_server_protocol::ConfiguredHookHandler; +use codex_app_server_protocol::ConfiguredHookMatcherGroup; use codex_app_server_protocol::ExperimentalFeatureEnablementSetParams; use codex_app_server_protocol::ExperimentalFeatureEnablementSetResponse; use codex_app_server_protocol::JSONRPCErrorError; +use codex_app_server_protocol::ManagedHooksRequirements; use codex_app_server_protocol::NetworkDomainPermission; use codex_app_server_protocol::NetworkRequirements; use codex_app_server_protocol::NetworkUnixSocketPermission; @@ -22,6 +25,10 @@ use codex_app_server_protocol::SandboxMode; use codex_core::ThreadManager; use codex_core::config::Config; use codex_core::config_loader::ConfigRequirementsToml; +use codex_core::config_loader::HookEventsToml; +use codex_core::config_loader::HookHandlerConfig as CoreHookHandlerConfig; +use codex_core::config_loader::ManagedHooksRequirementsToml; +use codex_core::config_loader::MatcherGroup as CoreMatcherGroup; use codex_core::config_loader::ResidencyRequirement as CoreResidencyRequirement; use codex_core::config_loader::SandboxModeRequirement as CoreSandboxModeRequirement; use codex_core::plugins::PluginId; @@ -290,6 +297,7 @@ fn map_requirements_toml_to_api(requirements: ConfigRequirementsToml) -> ConfigR feature_requirements: requirements .feature_requirements .map(|requirements| requirements.entries), + hooks: requirements.hooks.map(map_hooks_requirements_to_api), enforce_residency: requirements .enforce_residency .map(map_residency_requirement_to_api), @@ -297,6 +305,71 @@ fn map_requirements_toml_to_api(requirements: ConfigRequirementsToml) -> ConfigR } } +fn map_hooks_requirements_to_api(hooks: ManagedHooksRequirementsToml) -> ManagedHooksRequirements { + let ManagedHooksRequirementsToml { + managed_dir, + windows_managed_dir, + hooks, + } = hooks; + let HookEventsToml { + pre_tool_use, + permission_request, + post_tool_use, + session_start, + user_prompt_submit, + stop, + } = hooks; + + ManagedHooksRequirements { + managed_dir, + windows_managed_dir, + pre_tool_use: map_hook_matcher_groups_to_api(pre_tool_use), + permission_request: map_hook_matcher_groups_to_api(permission_request), + post_tool_use: map_hook_matcher_groups_to_api(post_tool_use), + session_start: map_hook_matcher_groups_to_api(session_start), + user_prompt_submit: map_hook_matcher_groups_to_api(user_prompt_submit), + stop: map_hook_matcher_groups_to_api(stop), + } +} + +fn map_hook_matcher_groups_to_api( + groups: Vec, +) -> Vec { + groups + .into_iter() + .map(map_hook_matcher_group_to_api) + .collect() +} + +fn map_hook_matcher_group_to_api(group: CoreMatcherGroup) -> ConfiguredHookMatcherGroup { + ConfiguredHookMatcherGroup { + matcher: group.matcher, + hooks: group + .hooks + .into_iter() + .map(map_hook_handler_to_api) + .collect(), + } +} + +fn map_hook_handler_to_api(handler: CoreHookHandlerConfig) -> ConfiguredHookHandler { + match handler { + CoreHookHandlerConfig::Command { + command, + timeout_sec, + r#async, + status_message, + } => ConfiguredHookHandler::Command { + command, + timeout_sec, + r#async, + status_message, + }, + CoreHookHandlerConfig::Prompt {} => ConfiguredHookHandler::Prompt {}, + CoreHookHandlerConfig::Agent {} => ConfiguredHookHandler::Agent {}, + } +} + fn map_sandbox_mode_requirement_to_api(mode: CoreSandboxModeRequirement) -> Option { match mode { CoreSandboxModeRequirement::ReadOnly => Some(SandboxMode::ReadOnly), @@ -476,6 +549,22 @@ mod tests { ("personality".to_string(), true), ]), }), + hooks: Some(ManagedHooksRequirementsToml { + managed_dir: Some(PathBuf::from("/enterprise/hooks")), + windows_managed_dir: Some(PathBuf::from(r"C:\enterprise\hooks")), + hooks: HookEventsToml { + pre_tool_use: vec![CoreMatcherGroup { + matcher: Some("^Bash$".to_string()), + hooks: vec![CoreHookHandlerConfig::Command { + command: "python3 /enterprise/hooks/pre.py".to_string(), + timeout_sec: Some(10), + r#async: false, + status_message: Some("checking".to_string()), + }], + }], + ..Default::default() + }, + }), mcp_servers: None, apps: None, rules: None, @@ -542,6 +631,27 @@ mod tests { ("personality".to_string(), true), ])), ); + assert_eq!( + mapped.hooks, + Some(ManagedHooksRequirements { + managed_dir: Some(PathBuf::from("/enterprise/hooks")), + windows_managed_dir: Some(PathBuf::from(r"C:\enterprise\hooks")), + pre_tool_use: vec![ConfiguredHookMatcherGroup { + matcher: Some("^Bash$".to_string()), + hooks: vec![ConfiguredHookHandler::Command { + command: "python3 /enterprise/hooks/pre.py".to_string(), + timeout_sec: Some(10), + r#async: false, + status_message: Some("checking".to_string()), + }], + }], + permission_request: Vec::new(), + post_tool_use: Vec::new(), + session_start: Vec::new(), + user_prompt_submit: Vec::new(), + stop: Vec::new(), + }), + ); assert_eq!( mapped.enforce_residency, Some(codex_app_server_protocol::ResidencyRequirement::Us), @@ -582,6 +692,7 @@ mod tests { allowed_web_search_modes: None, guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -641,6 +752,7 @@ mod tests { allowed_web_search_modes: Some(Vec::new()), guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, diff --git a/codex-rs/app-server/src/message_processor/tracing_tests.rs b/codex-rs/app-server/src/message_processor/tracing_tests.rs index e0a1ed4bc436..84fddf5e8524 100644 --- a/codex-rs/app-server/src/message_processor/tracing_tests.rs +++ b/codex-rs/app-server/src/message_processor/tracing_tests.rs @@ -48,6 +48,7 @@ use opentelemetry_sdk::trace::SpanData; use pretty_assertions::assert_eq; use serial_test::serial; use std::collections::BTreeMap; +use std::future::Future; use std::path::Path; use std::sync::Arc; use std::sync::OnceLock; @@ -291,6 +292,28 @@ fn build_test_processor( (processor, outgoing_rx) } +fn run_current_thread_test_with_stack(name: &str, future: F) -> Result<()> +where + F: Future> + Send + 'static, +{ + const TEST_STACK_SIZE_BYTES: usize = 4 * 1024 * 1024; + + let handle = std::thread::Builder::new() + .name(name.to_string()) + .stack_size(TEST_STACK_SIZE_BYTES) + .spawn(move || -> Result<()> { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build()?; + runtime.block_on(Box::pin(future)) + })?; + + match handle.join() { + Ok(result) => result, + Err(_) => Err(anyhow::anyhow!("{name} thread panicked")), + } +} + fn span_attr<'a>(span: &'a SpanData, key: &str) -> Option<&'a str> { span.attributes .iter() @@ -568,83 +591,96 @@ where spans.into_iter().skip(baseline_len).collect() } -#[tokio::test(flavor = "current_thread")] +#[test] #[serial(app_server_tracing)] -async fn thread_start_jsonrpc_span_exports_server_span_and_parents_children() -> Result<()> { - let mut harness = TracingHarness::new().await?; - - let RemoteTrace { - trace_id: remote_trace_id, - parent_span_id: remote_parent_span_id, - context: remote_trace, - .. - } = RemoteTrace::new("00000000000000000000000000000011", "0000000000000022"); - - let _: ThreadStartResponse = harness - .start_thread(/*request_id*/ 20_002, /*trace*/ None) - .await; - let untraced_spans = wait_for_exported_spans(harness.tracing, |spans| { - spans.iter().any(|span| { - span.span_kind == SpanKind::Server - && span_attr(span, "rpc.method") == Some("thread/start") - }) - }) - .await; - let untraced_server_span = find_rpc_span_with_trace( - &untraced_spans, - SpanKind::Server, - "thread/start", - untraced_spans - .iter() - .rev() - .find(|span| { - span.span_kind == SpanKind::Server - && span_attr(span, "rpc.system") == Some("jsonrpc") - && span_attr(span, "rpc.method") == Some("thread/start") +fn thread_start_jsonrpc_span_exports_server_span_and_parents_children() -> Result<()> { + run_current_thread_test_with_stack( + "thread_start_jsonrpc_span_exports_server_span_and_parents_children", + async { + let mut harness = TracingHarness::new().await?; + + let RemoteTrace { + trace_id: remote_trace_id, + parent_span_id: remote_parent_span_id, + context: remote_trace, + .. + } = RemoteTrace::new("00000000000000000000000000000011", "0000000000000022"); + + let _: ThreadStartResponse = harness + .start_thread(/*request_id*/ 20_002, /*trace*/ None) + .await; + let untraced_spans = wait_for_exported_spans(harness.tracing, |spans| { + spans.iter().any(|span| { + span.span_kind == SpanKind::Server + && span_attr(span, "rpc.method") == Some("thread/start") + }) }) - .unwrap_or_else(|| { - panic!( - "missing latest thread/start server span; exported spans:\n{}", - format_spans(&untraced_spans) - ) + .await; + let untraced_server_span = find_rpc_span_with_trace( + &untraced_spans, + SpanKind::Server, + "thread/start", + untraced_spans + .iter() + .rev() + .find(|span| { + span.span_kind == SpanKind::Server + && span_attr(span, "rpc.system") == Some("jsonrpc") + && span_attr(span, "rpc.method") == Some("thread/start") + }) + .unwrap_or_else(|| { + panic!( + "missing latest thread/start server span; exported spans:\n{}", + format_spans(&untraced_spans) + ) + }) + .span_context + .trace_id(), + ); + assert_has_internal_descendant_at_min_depth( + &untraced_spans, + untraced_server_span, + /*min_depth*/ 1, + ); + + let baseline_len = untraced_spans.len(); + let _: ThreadStartResponse = harness + .start_thread(/*request_id*/ 20_003, Some(remote_trace)) + .await; + let spans = wait_for_new_exported_spans(harness.tracing, baseline_len, |spans| { + spans.iter().any(|span| { + span.span_kind == SpanKind::Server + && span_attr(span, "rpc.method") == Some("thread/start") + && span.span_context.trace_id() == remote_trace_id + }) && spans.iter().any(|span| { + span.name.as_ref() == "app_server.thread_start.notify_started" + && span.span_context.trace_id() == remote_trace_id + }) }) - .span_context - .trace_id(), - ); - assert_has_internal_descendant_at_min_depth( - &untraced_spans, - untraced_server_span, - /*min_depth*/ 1, - ); - - let baseline_len = untraced_spans.len(); - let _: ThreadStartResponse = harness - .start_thread(/*request_id*/ 20_003, Some(remote_trace)) - .await; - let spans = wait_for_new_exported_spans(harness.tracing, baseline_len, |spans| { - spans.iter().any(|span| { - span.span_kind == SpanKind::Server - && span_attr(span, "rpc.method") == Some("thread/start") - && span.span_context.trace_id() == remote_trace_id - }) && spans.iter().any(|span| { - span.name.as_ref() == "app_server.thread_start.notify_started" - && span.span_context.trace_id() == remote_trace_id - }) - }) - .await; - - let server_request_span = - find_rpc_span_with_trace(&spans, SpanKind::Server, "thread/start", remote_trace_id); - assert_eq!(server_request_span.name.as_ref(), "thread/start"); - assert_eq!(server_request_span.parent_span_id, remote_parent_span_id); - assert!(server_request_span.parent_span_is_remote); - assert_eq!(server_request_span.span_context.trace_id(), remote_trace_id); - assert_ne!(server_request_span.span_context.span_id(), SpanId::INVALID); - assert_has_internal_descendant_at_min_depth(&spans, server_request_span, /*min_depth*/ 1); - assert_has_internal_descendant_at_min_depth(&spans, server_request_span, /*min_depth*/ 2); - harness.shutdown().await; + .await; - Ok(()) + let server_request_span = + find_rpc_span_with_trace(&spans, SpanKind::Server, "thread/start", remote_trace_id); + assert_eq!(server_request_span.name.as_ref(), "thread/start"); + assert_eq!(server_request_span.parent_span_id, remote_parent_span_id); + assert!(server_request_span.parent_span_is_remote); + assert_eq!(server_request_span.span_context.trace_id(), remote_trace_id); + assert_ne!(server_request_span.span_context.span_id(), SpanId::INVALID); + assert_has_internal_descendant_at_min_depth( + &spans, + server_request_span, + /*min_depth*/ 1, + ); + assert_has_internal_descendant_at_min_depth( + &spans, + server_request_span, + /*min_depth*/ 2, + ); + harness.shutdown().await; + + Ok(()) + }, + ) } #[tokio::test(flavor = "current_thread")] diff --git a/codex-rs/cloud-requirements/src/lib.rs b/codex-rs/cloud-requirements/src/lib.rs index 0b50aa834fa7..aeed90e8b09b 100644 --- a/codex-rs/cloud-requirements/src/lib.rs +++ b/codex-rs/cloud-requirements/src/lib.rs @@ -1172,6 +1172,7 @@ mod tests { allowed_web_search_modes: None, guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -1203,6 +1204,7 @@ mod tests { allowed_web_search_modes: None, guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -1234,6 +1236,7 @@ mod tests { allowed_web_search_modes: None, guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -1282,6 +1285,7 @@ mod tests { allowed_web_search_modes: None, guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -1366,6 +1370,7 @@ enabled = false allowed_web_search_modes: None, guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -1441,6 +1446,7 @@ enabled = false allowed_web_search_modes: None, guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -1514,6 +1520,7 @@ enabled = false allowed_web_search_modes: None, guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -1714,6 +1721,7 @@ enabled = false allowed_web_search_modes: None, guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -1751,6 +1759,7 @@ enabled = false allowed_web_search_modes: None, guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -1808,6 +1817,7 @@ enabled = false allowed_web_search_modes: None, guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -1860,6 +1870,7 @@ enabled = false allowed_web_search_modes: None, guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -1916,6 +1927,7 @@ enabled = false allowed_web_search_modes: None, guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -1973,6 +1985,7 @@ enabled = false allowed_web_search_modes: None, guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -2030,6 +2043,7 @@ enabled = false allowed_web_search_modes: None, guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -2120,6 +2134,7 @@ enabled = false allowed_web_search_modes: None, guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -2149,6 +2164,7 @@ enabled = false allowed_web_search_modes: None, guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index 1d3ea49cf3dd..56ff26f907dc 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -17,6 +17,7 @@ use super::requirements_exec_policy::RequirementsExecPolicy; use super::requirements_exec_policy::RequirementsExecPolicyToml; use crate::Constrained; use crate::ConstraintError; +use crate::ManagedHooksRequirementsToml; #[derive(Debug, Clone, PartialEq, Eq)] pub enum RequirementSource { @@ -86,6 +87,7 @@ pub struct ConfigRequirements { pub sandbox_policy: ConstrainedWithSource, pub web_search_mode: ConstrainedWithSource, pub feature_requirements: Option>, + pub managed_hooks: Option>, pub mcp_servers: Option>>, pub exec_policy: Option>, pub enforce_residency: ConstrainedWithSource>, @@ -117,6 +119,7 @@ impl Default for ConfigRequirements { /*source*/ None, ), feature_requirements: None, + managed_hooks: None, mcp_servers: None, exec_policy: None, enforce_residency: ConstrainedWithSource::new( @@ -628,6 +631,7 @@ pub struct ConfigRequirementsToml { pub allowed_web_search_modes: Option>, #[serde(rename = "features", alias = "feature_requirements")] pub feature_requirements: Option, + pub hooks: Option, pub mcp_servers: Option>, pub apps: Option, pub rules: Option, @@ -673,6 +677,7 @@ pub struct ConfigRequirementsWithSources { pub allowed_sandbox_modes: Option>>, pub allowed_web_search_modes: Option>>, pub feature_requirements: Option>, + pub hooks: Option>, pub mcp_servers: Option>>, pub apps: Option>, pub rules: Option>, @@ -707,6 +712,7 @@ impl ConfigRequirementsWithSources { remote_sandbox_config: _, allowed_web_search_modes: _, feature_requirements: _, + hooks: _, mcp_servers: _, apps: _, rules: _, @@ -734,6 +740,7 @@ impl ConfigRequirementsWithSources { allowed_sandbox_modes, allowed_web_search_modes, feature_requirements, + hooks, mcp_servers, rules, enforce_residency, @@ -759,6 +766,7 @@ impl ConfigRequirementsWithSources { allowed_sandbox_modes, allowed_web_search_modes, feature_requirements, + hooks, mcp_servers, apps, rules, @@ -774,6 +782,7 @@ impl ConfigRequirementsWithSources { remote_sandbox_config: None, allowed_web_search_modes: allowed_web_search_modes.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), apps: apps.map(|sourced| sourced.value), rules: rules.map(|sourced| sourced.value), @@ -858,6 +867,10 @@ impl ConfigRequirementsToml { .feature_requirements .as_ref() .is_none_or(FeatureRequirementsToml::is_empty) + && self + .hooks + .as_ref() + .is_none_or(ManagedHooksRequirementsToml::is_empty) && self.mcp_servers.is_none() && self .apps @@ -884,6 +897,7 @@ impl TryFrom for ConfigRequirements { allowed_sandbox_modes, allowed_web_search_modes, feature_requirements, + hooks, mcp_servers, apps: _apps, rules, @@ -1064,6 +1078,34 @@ impl TryFrom for ConfigRequirements { }; let feature_requirements = feature_requirements.filter(|requirements| !requirements.value.is_empty()); + let managed_hooks = hooks + .filter(|managed_hooks| managed_hooks.value.handler_count() > 0) + .map(|sourced_hooks| { + let Sourced { + value, + source: requirement_source, + } = sourced_hooks; + let allowed = value; + let allowed_for_error = format!("{allowed:?}"); + let requirement_source_for_error = requirement_source.clone(); + let constrained = Constrained::new(allowed.clone(), move |candidate| { + if candidate == &allowed { + Ok(()) + } else { + Err(ConstraintError::InvalidValue { + field_name: "hooks", + candidate: format!("{candidate:?}"), + allowed: allowed_for_error.clone(), + requirement_source: requirement_source_for_error.clone(), + }) + } + })?; + Ok(ConstrainedWithSource::new( + constrained, + Some(requirement_source), + )) + }) + .transpose()?; let enforce_residency = match enforce_residency { Some(Sourced { @@ -1106,6 +1148,7 @@ impl TryFrom for ConfigRequirements { sandbox_policy, web_search_mode, feature_requirements, + managed_hooks, mcp_servers, exec_policy, enforce_residency, @@ -1119,6 +1162,7 @@ impl TryFrom for ConfigRequirements { #[cfg(test)] mod tests { use super::*; + use crate::HookEventsToml; use anyhow::Result; use codex_execpolicy::Decision; use codex_execpolicy::Evaluation; @@ -1147,6 +1191,7 @@ mod tests { remote_sandbox_config: _, allowed_web_search_modes, feature_requirements, + hooks, mcp_servers, apps, rules, @@ -1166,6 +1211,7 @@ mod tests { .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)), mcp_servers: mcp_servers.map(|value| Sourced::new(value, RequirementSource::Unknown)), apps: apps.map(|value| Sourced::new(value, RequirementSource::Unknown)), rules: rules.map(|value| Sourced::new(value, RequirementSource::Unknown)), @@ -1210,6 +1256,7 @@ mod tests { remote_sandbox_config: None, allowed_web_search_modes: Some(allowed_web_search_modes.clone()), feature_requirements: Some(feature_requirements.clone()), + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -1241,6 +1288,7 @@ mod tests { feature_requirements, enforce_source.clone(), )), + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -1278,6 +1326,7 @@ mod tests { allowed_sandbox_modes: None, allowed_web_search_modes: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -1323,6 +1372,7 @@ mod tests { allowed_sandbox_modes: None, allowed_web_search_modes: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -2232,6 +2282,124 @@ allowed_approvals_reviewers = ["user"] Ok(()) } + #[test] + fn deserialize_managed_hooks_requirements() -> Result<()> { + let toml_str = r#" +managed_dir = "/enterprise/hooks" +windows_managed_dir = 'C:\enterprise\hooks' + +[[PreToolUse]] +matcher = "^Bash$" + +[[PreToolUse.hooks]] +type = "command" +command = "python3 /enterprise/hooks/pre.py" +timeout = 10 +statusMessage = "checking" + "#; + let hooks: ManagedHooksRequirementsToml = from_str(toml_str)?; + + assert_eq!( + hooks.managed_dir.as_deref(), + Some(std::path::Path::new("/enterprise/hooks")) + ); + assert_eq!(hooks.handler_count(), 1); + assert_eq!(hooks.hooks.pre_tool_use.len(), 1); + Ok(()) + } + + #[test] + fn merge_unset_fields_does_not_overwrite_existing_hooks() -> Result<()> { + let mut target = ConfigRequirementsWithSources::default(); + target.merge_unset_fields( + RequirementSource::CloudRequirements, + from_str::( + r#" +[hooks] +managed_dir = "/cloud/hooks" + +[[hooks.PreToolUse]] +matcher = "^Bash$" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "python3 /cloud/hooks/pre.py" + "#, + )?, + ); + target.merge_unset_fields( + RequirementSource::SystemRequirementsToml { + file: system_requirements_toml_file_for_test()?, + }, + from_str::( + r#" +[hooks] +managed_dir = "/system/hooks" + +[[hooks.PreToolUse]] +matcher = "^Bash$" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "python3 /system/hooks/pre.py" + "#, + )?, + ); + + assert_eq!( + target + .hooks + .as_ref() + .and_then(|hooks| hooks.value.managed_dir.as_ref()) + .map(std::path::PathBuf::as_path), + Some(std::path::Path::new("/cloud/hooks")) + ); + assert_eq!( + target.hooks.as_ref().map(|hooks| hooks.source.clone()), + Some(RequirementSource::CloudRequirements) + ); + Ok(()) + } + + #[test] + fn managed_hooks_constraint_rejects_drift() -> Result<()> { + let config: ConfigRequirementsToml = from_str( + r#" +[hooks] +managed_dir = "/enterprise/hooks" + +[[hooks.PreToolUse]] +matcher = "^Bash$" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "python3 /enterprise/hooks/pre.py" + "#, + )?; + let requirements: ConfigRequirements = with_unknown_source(config).try_into()?; + let mut managed_hooks = requirements + .managed_hooks + .expect("expected managed hooks requirements"); + + let err = managed_hooks + .set(ManagedHooksRequirementsToml { + managed_dir: Some(std::path::PathBuf::from("/other/hooks")), + windows_managed_dir: None, + hooks: HookEventsToml::default(), + }) + .expect_err("managed hooks should reject drift"); + + assert!(matches!( + err, + ConstraintError::InvalidValue { + field_name: "hooks", + requirement_source: RequirementSource::Unknown, + .. + } + )); + Ok(()) + } + #[test] fn network_requirements_are_preserved_as_constraints_with_source() -> Result<()> { let toml_str = r#" diff --git a/codex-rs/config/src/config_toml.rs b/codex-rs/config/src/config_toml.rs index 40f384f41248..543cb59ef3bb 100644 --- a/codex-rs/config/src/config_toml.rs +++ b/codex-rs/config/src/config_toml.rs @@ -4,6 +4,7 @@ use std::collections::BTreeMap; use std::collections::HashMap; use std::path::Path; +use crate::HookEventsToml; use crate::permissions_toml::PermissionsToml; use crate::profile_toml::ConfigProfile; use crate::types::AnalyticsConfigToml; @@ -332,6 +333,9 @@ pub struct ConfigToml { /// User-level skill config entries keyed by SKILL.md path. pub skills: Option, + /// Lifecycle hooks configured inline in TOML. + pub hooks: Option, + /// User-level plugin config entries keyed by plugin name. #[serde(default)] pub plugins: HashMap, diff --git a/codex-rs/config/src/hook_config.rs b/codex-rs/config/src/hook_config.rs new file mode 100644 index 000000000000..8a5c73d6b9ba --- /dev/null +++ b/codex-rs/config/src/hook_config.rs @@ -0,0 +1,148 @@ +use std::path::Path; +use std::path::PathBuf; + +use codex_protocol::protocol::HookEventName; +use schemars::JsonSchema; +use serde::Deserialize; +use serde::Serialize; + +#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +pub struct HooksFile { + #[serde(default)] + pub hooks: HookEventsToml, +} + +#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +pub struct HookEventsToml { + #[serde(rename = "PreToolUse", default)] + pub pre_tool_use: Vec, + #[serde(rename = "PermissionRequest", default)] + pub permission_request: Vec, + #[serde(rename = "PostToolUse", default)] + pub post_tool_use: Vec, + #[serde(rename = "SessionStart", default)] + pub session_start: Vec, + #[serde(rename = "UserPromptSubmit", default)] + pub user_prompt_submit: Vec, + #[serde(rename = "Stop", default)] + pub stop: Vec, +} + +impl HookEventsToml { + pub fn is_empty(&self) -> bool { + let Self { + pre_tool_use, + permission_request, + post_tool_use, + session_start, + user_prompt_submit, + stop, + } = self; + pre_tool_use.is_empty() + && permission_request.is_empty() + && post_tool_use.is_empty() + && session_start.is_empty() + && user_prompt_submit.is_empty() + && stop.is_empty() + } + + pub fn handler_count(&self) -> usize { + let Self { + pre_tool_use, + permission_request, + post_tool_use, + session_start, + user_prompt_submit, + stop, + } = self; + [ + pre_tool_use, + permission_request, + post_tool_use, + session_start, + user_prompt_submit, + stop, + ] + .into_iter() + .flatten() + .map(|group| group.hooks.len()) + .sum() + } + + pub fn into_matcher_groups(self) -> [(HookEventName, Vec); 6] { + [ + (HookEventName::PreToolUse, self.pre_tool_use), + (HookEventName::PermissionRequest, self.permission_request), + (HookEventName::PostToolUse, self.post_tool_use), + (HookEventName::SessionStart, self.session_start), + (HookEventName::UserPromptSubmit, self.user_prompt_submit), + (HookEventName::Stop, self.stop), + ] + } +} + +#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +pub struct MatcherGroup { + #[serde(default)] + pub matcher: Option, + #[serde(default)] + pub hooks: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(tag = "type")] +pub enum HookHandlerConfig { + #[serde(rename = "command")] + Command { + command: String, + #[serde(default, rename = "timeout")] + timeout_sec: Option, + #[serde(default)] + r#async: bool, + #[serde(default, rename = "statusMessage")] + status_message: Option, + }, + #[serde(rename = "prompt")] + Prompt {}, + #[serde(rename = "agent")] + Agent {}, +} + +#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ManagedHooksRequirementsToml { + pub managed_dir: Option, + pub windows_managed_dir: Option, + #[serde(flatten)] + pub hooks: HookEventsToml, +} + +impl ManagedHooksRequirementsToml { + pub fn is_empty(&self) -> bool { + let Self { + managed_dir, + windows_managed_dir, + hooks, + } = self; + managed_dir.is_none() && windows_managed_dir.is_none() && hooks.is_empty() + } + + pub fn handler_count(&self) -> usize { + self.hooks.handler_count() + } + + pub fn managed_dir_for_current_platform(&self) -> Option<&Path> { + #[cfg(windows)] + { + self.windows_managed_dir.as_deref() + } + + #[cfg(not(windows))] + { + self.managed_dir.as_deref() + } + } +} + +#[cfg(test)] +#[path = "hooks_tests.rs"] +mod tests; diff --git a/codex-rs/config/src/hooks_tests.rs b/codex-rs/config/src/hooks_tests.rs new file mode 100644 index 000000000000..5e3f1df67475 --- /dev/null +++ b/codex-rs/config/src/hooks_tests.rs @@ -0,0 +1,119 @@ +use pretty_assertions::assert_eq; + +use super::HookEventsToml; +use super::HookHandlerConfig; +use super::HooksFile; +use super::ManagedHooksRequirementsToml; +use super::MatcherGroup; + +#[test] +fn hooks_file_deserializes_existing_json_shape() { + let parsed: HooksFile = serde_json::from_str( + r#"{ + "hooks": { + "PreToolUse": [ + { + "matcher": "^Bash$", + "hooks": [ + { + "type": "command", + "command": "python3 /tmp/pre.py", + "timeout": 10, + "statusMessage": "checking" + } + ] + } + ] + } +}"#, + ) + .expect("hooks.json should deserialize"); + + assert_eq!( + parsed, + HooksFile { + hooks: HookEventsToml { + pre_tool_use: vec![MatcherGroup { + matcher: Some("^Bash$".to_string()), + hooks: vec![HookHandlerConfig::Command { + command: "python3 /tmp/pre.py".to_string(), + timeout_sec: Some(10), + r#async: false, + status_message: Some("checking".to_string()), + }], + }], + ..Default::default() + }, + } + ); +} + +#[test] +fn hook_events_deserialize_from_toml_arrays_of_tables() { + let parsed: HookEventsToml = toml::from_str( + r#" +[[PreToolUse]] +matcher = "^Bash$" + +[[PreToolUse.hooks]] +type = "command" +command = "python3 /tmp/pre.py" +timeout = 10 +statusMessage = "checking" +"#, + ) + .expect("hook events TOML should deserialize"); + + assert_eq!( + parsed, + HookEventsToml { + pre_tool_use: vec![MatcherGroup { + matcher: Some("^Bash$".to_string()), + hooks: vec![HookHandlerConfig::Command { + command: "python3 /tmp/pre.py".to_string(), + timeout_sec: Some(10), + r#async: false, + status_message: Some("checking".to_string()), + }], + }], + ..Default::default() + } + ); +} + +#[test] +fn managed_hooks_requirements_flatten_hook_events() { + let parsed: ManagedHooksRequirementsToml = toml::from_str( + r#" +managed_dir = "/enterprise/place" + +[[PreToolUse]] +matcher = "^Bash$" + +[[PreToolUse.hooks]] +type = "command" +command = "python3 /enterprise/place/pre.py" +"#, + ) + .expect("requirements hooks TOML should deserialize"); + + assert_eq!( + parsed, + ManagedHooksRequirementsToml { + managed_dir: Some(std::path::PathBuf::from("/enterprise/place")), + windows_managed_dir: None, + hooks: HookEventsToml { + pre_tool_use: vec![MatcherGroup { + matcher: Some("^Bash$".to_string()), + hooks: vec![HookHandlerConfig::Command { + command: "python3 /enterprise/place/pre.py".to_string(), + timeout_sec: None, + r#async: false, + status_message: None, + }], + }], + ..Default::default() + }, + } + ); +} diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index e70550f00960..e90d09c09da1 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -4,6 +4,7 @@ pub mod config_toml; mod constraint; mod diagnostics; mod fingerprint; +mod hook_config; mod host_name; mod key_aliases; mod marketplace_edit; @@ -27,6 +28,8 @@ pub const CONFIG_TOML_FILE: &str = "config.toml"; pub use cloud_requirements::CloudRequirementsLoadError; pub use cloud_requirements::CloudRequirementsLoadErrorCode; pub use cloud_requirements::CloudRequirementsLoader; +pub use codex_app_server_protocol::ConfigLayerSource; +pub use codex_utils_absolute_path::AbsolutePathBuf; pub use config_requirements::AppRequirementToml; pub use config_requirements::AppsRequirementsToml; pub use config_requirements::ConfigRequirements; @@ -65,6 +68,11 @@ pub use diagnostics::format_config_error; pub use diagnostics::format_config_error_with_source; pub use diagnostics::io_error_from_config_error; pub use fingerprint::version_for_toml; +pub use hook_config::HookEventsToml; +pub use hook_config::HookHandlerConfig; +pub use hook_config::HooksFile; +pub use hook_config::ManagedHooksRequirementsToml; +pub use hook_config::MatcherGroup; pub use host_name::host_name; pub use marketplace_edit::MarketplaceConfigUpdate; pub use marketplace_edit::RemoveMarketplaceConfigOutcome; @@ -106,5 +114,4 @@ pub use thread_config::ThreadConfigLoadErrorCode; pub use thread_config::ThreadConfigLoader; pub use thread_config::ThreadConfigSource; pub use thread_config::UserThreadConfig; - -pub use codex_app_server_protocol::ConfigLayerSource; +pub use toml::Value as TomlValue; diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 245a55e1bd25..a3cf84af4451 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -848,6 +848,117 @@ } ] }, + "HookEventsToml": { + "properties": { + "PermissionRequest": { + "default": [], + "items": { + "$ref": "#/definitions/MatcherGroup" + }, + "type": "array" + }, + "PostToolUse": { + "default": [], + "items": { + "$ref": "#/definitions/MatcherGroup" + }, + "type": "array" + }, + "PreToolUse": { + "default": [], + "items": { + "$ref": "#/definitions/MatcherGroup" + }, + "type": "array" + }, + "SessionStart": { + "default": [], + "items": { + "$ref": "#/definitions/MatcherGroup" + }, + "type": "array" + }, + "Stop": { + "default": [], + "items": { + "$ref": "#/definitions/MatcherGroup" + }, + "type": "array" + }, + "UserPromptSubmit": { + "default": [], + "items": { + "$ref": "#/definitions/MatcherGroup" + }, + "type": "array" + } + }, + "type": "object" + }, + "HookHandlerConfig": { + "oneOf": [ + { + "properties": { + "async": { + "default": false, + "type": "boolean" + }, + "command": { + "type": "string" + }, + "statusMessage": { + "default": null, + "type": "string" + }, + "timeout": { + "default": null, + "format": "uint64", + "minimum": 0.0, + "type": "integer" + }, + "type": { + "enum": [ + "command" + ], + "type": "string" + } + }, + "required": [ + "command", + "type" + ], + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "prompt" + ], + "type": "string" + } + }, + "required": [ + "type" + ], + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "agent" + ], + "type": "string" + } + }, + "required": [ + "type" + ], + "type": "object" + } + ] + }, "MarketplaceConfig": { "additionalProperties": false, "properties": { @@ -898,6 +1009,22 @@ ], "type": "string" }, + "MatcherGroup": { + "properties": { + "hooks": { + "default": [], + "items": { + "$ref": "#/definitions/HookHandlerConfig" + }, + "type": "array" + }, + "matcher": { + "default": null, + "type": "string" + } + }, + "type": "object" + }, "McpServerEnvVar": { "anyOf": [ { @@ -2634,6 +2761,14 @@ "default": null, "description": "Settings that govern if and what will be written to `~/.codex/history.jsonl`." }, + "hooks": { + "allOf": [ + { + "$ref": "#/definitions/HookEventsToml" + } + ], + "description": "Lifecycle hooks configured inline in TOML." + }, "include_apps_instructions": { "description": "Whether to inject the `` developer block.", "type": "boolean" diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 500483f6594e..deaf4b2db8b3 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -5732,6 +5732,7 @@ async fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset() crate::config_loader::WebSearchModeRequirement::Cached, ]), feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -6407,6 +6408,7 @@ async fn explicit_sandbox_mode_falls_back_when_disallowed_by_requirements() -> s remote_sandbox_config: None, allowed_web_search_modes: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index e89cb7ca0e5f..771c1a48694f 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1577,6 +1577,7 @@ impl Config { sandbox_policy: mut constrained_sandbox_policy, web_search_mode: mut constrained_web_search_mode, feature_requirements, + managed_hooks: _, mcp_servers, exec_policy: _, enforce_residency, diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index 95dd76f622f4..4681aa0753d3 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -45,7 +45,11 @@ pub use codex_config::ConstrainedWithSource; pub use codex_config::FeatureRequirementsToml; pub use codex_config::FilesystemConstraints; pub use codex_config::FilesystemDenyReadPattern; +pub use codex_config::HookEventsToml; +pub use codex_config::HookHandlerConfig; pub use codex_config::LoaderOverrides; +pub use codex_config::ManagedHooksRequirementsToml; +pub use codex_config::MatcherGroup; pub use codex_config::McpServerIdentity; pub use codex_config::McpServerRequirement; pub use codex_config::NetworkConstraints; diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs index 337960f1b960..7f6195269073 100644 --- a/codex-rs/core/src/config_loader/tests.rs +++ b/codex-rs/core/src/config_loader/tests.rs @@ -777,6 +777,7 @@ allowed_approval_policies = ["on-request"] remote_sandbox_config: None, allowed_web_search_modes: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -833,6 +834,7 @@ allowed_approval_policies = ["on-request"] remote_sandbox_config: None, allowed_web_search_modes: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -1041,6 +1043,7 @@ async fn load_config_layers_includes_cloud_requirements() -> anyhow::Result<()> remote_sandbox_config: None, allowed_web_search_modes: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -1084,6 +1087,62 @@ async fn load_config_layers_includes_cloud_requirements() -> anyhow::Result<()> Ok(()) } +#[tokio::test] +async fn load_config_layers_includes_cloud_hook_requirements() -> anyhow::Result<()> { + let tmp = tempdir()?; + let codex_home = tmp.path().join("home"); + tokio::fs::create_dir_all(&codex_home).await?; + let managed_dir = tmp.path().join("managed-hooks"); + tokio::fs::create_dir_all(&managed_dir).await?; + let cwd = AbsolutePathBuf::from_absolute_path(tmp.path())?; + + let requirements = ConfigRequirementsToml { + hooks: Some(codex_config::ManagedHooksRequirementsToml { + managed_dir: Some(managed_dir.clone()), + windows_managed_dir: None, + hooks: codex_config::HookEventsToml { + pre_tool_use: vec![codex_config::MatcherGroup { + matcher: Some("^Bash$".to_string()), + hooks: vec![codex_config::HookHandlerConfig::Command { + command: format!("python3 {}/pre.py", managed_dir.display()), + timeout_sec: Some(10), + r#async: false, + status_message: Some("checking".to_string()), + }], + }], + ..Default::default() + }, + }), + ..ConfigRequirementsToml::default() + }; + let expected = requirements.clone(); + let cloud_requirements = CloudRequirementsLoader::new(async move { Ok(Some(requirements)) }); + + let layers = load_config_layers_state( + LOCAL_FS.as_ref(), + &codex_home, + Some(cwd), + &[] as &[(String, TomlValue)], + LoaderOverrides::default(), + cloud_requirements, + &codex_config::NoopThreadConfigLoader, + /*host_name*/ None, + ) + .await?; + + assert_eq!(layers.requirements_toml().hooks, expected.hooks); + assert_eq!( + layers + .requirements() + .managed_hooks + .as_ref() + .map(|hooks| hooks.source.clone()), + Some(Some(RequirementSource::CloudRequirements)) + ); + + Ok(()) +} + #[tokio::test] async fn load_config_layers_applies_matching_remote_sandbox_config() -> anyhow::Result<()> { let tmp = tempdir()?; diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 74b05f6e1774..b108be15f84e 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -482,12 +482,12 @@ impl TestCodexBuilder { ..built_in_model_providers(/*openai_base_url*/ None)["openai"].clone() }; let cwd = Arc::new(TempDir::new()?); - let mut config = load_default_config_for_test(home).await; - config.cwd = cwd_override; - config.model_provider = model_provider; for hook in self.pre_build_hooks.drain(..) { hook(home.path()); } + let mut config = load_default_config_for_test(home).await; + config.cwd = cwd_override; + config.model_provider = model_provider; if let Ok(path) = codex_utils_cargo_bin::cargo_bin("codex") { config.codex_self_exe = Some(path); } else if let Ok(path) = codex_utils_cargo_bin::cargo_bin("codex-exec") { diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index 022c528753df..3a0669c00857 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -254,6 +254,74 @@ elif mode == "exit_2": Ok(()) } +fn write_pre_tool_use_hook_toml( + home: &Path, + script_name: &str, + log_name: &str, + matcher: Option<&str>, + mode: &str, + reason: &str, +) -> Result<()> { + let script_path = home.join(script_name); + let log_path = home.join(log_name); + let mode_json = serde_json::to_string(mode).context("serialize pre tool use mode")?; + let reason_json = serde_json::to_string(reason).context("serialize pre tool use reason")?; + let script = format!( + r#"import json +from pathlib import Path +import sys + +log_path = Path(r"{log_path}") +mode = {mode_json} +reason = {reason_json} + +payload = json.load(sys.stdin) + +with log_path.open("a", encoding="utf-8") as handle: + handle.write(json.dumps(payload) + "\n") + +if mode == "json_deny": + print(json.dumps({{ + "hookSpecificOutput": {{ + "hookEventName": "PreToolUse", + "permissionDecision": "deny", + "permissionDecisionReason": reason + }} + }})) +elif mode == "exit_2": + sys.stderr.write(reason + "\n") + raise SystemExit(2) +"#, + log_path = log_path.display(), + mode_json = mode_json, + reason_json = reason_json, + ); + let matcher_line = matcher + .map(|matcher| format!("matcher = '{matcher}'\n")) + .unwrap_or_default(); + let config_toml = format!( + r#"[features] +codex_hooks = true + +[hooks] + +[[hooks.PreToolUse]] +{matcher_line} + +[[hooks.PreToolUse.hooks]] +type = "command" +command = 'python3 {script_path}' +statusMessage = "running pre tool use hook" +"#, + matcher_line = matcher_line, + script_path = script_path.display(), + ); + + fs::write(&script_path, script).context("write TOML pre tool use hook script")?; + fs::write(home.join("config.toml"), config_toml).context("write config.toml hooks")?; + Ok(()) +} + fn write_permission_request_hook( home: &Path, matcher: Option<&str>, @@ -546,12 +614,7 @@ fn read_stop_hook_inputs(home: &Path) -> Result> { } fn read_pre_tool_use_hook_inputs(home: &Path) -> Result> { - fs::read_to_string(home.join("pre_tool_use_hook_log.jsonl")) - .context("read pre tool use hook log")? - .lines() - .filter(|line| !line.trim().is_empty()) - .map(|line| serde_json::from_str(line).context("parse pre tool use hook log line")) - .collect() + read_hook_inputs_from_log(home.join("pre_tool_use_hook_log.jsonl").as_path()) } fn read_permission_request_hook_inputs(home: &Path) -> Result> { @@ -605,11 +668,15 @@ fn assert_single_permission_request_hook_input_for_tool( } fn read_post_tool_use_hook_inputs(home: &Path) -> Result> { - fs::read_to_string(home.join("post_tool_use_hook_log.jsonl")) - .context("read post tool use hook log")? + read_hook_inputs_from_log(home.join("post_tool_use_hook_log.jsonl").as_path()) +} + +fn read_hook_inputs_from_log(log_path: &Path) -> Result> { + fs::read_to_string(log_path) + .with_context(|| format!("read hook log {}", log_path.display()))? .lines() .filter(|line| !line.trim().is_empty()) - .map(|line| serde_json::from_str(line).context("parse post tool use hook log line")) + .map(|line| serde_json::from_str(line).context("parse hook log line")) .collect() } @@ -1785,6 +1852,190 @@ async fn pre_tool_use_blocks_shell_command_before_execution() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn pre_tool_use_blocks_shell_when_defined_in_config_toml() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let call_id = "pretooluse-config-toml"; + let marker = std::env::temp_dir().join("pretooluse-config-toml-marker"); + let command = format!("printf blocked > {}", marker.display()); + let args = serde_json::json!({ "command": command }); + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + core_test_support::responses::ev_function_call( + call_id, + "shell_command", + &serde_json::to_string(&args)?, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "config.toml hook blocked it"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + let mut builder = test_codex().with_pre_build_hook(|home| { + if let Err(error) = write_pre_tool_use_hook_toml( + home, + "pre_tool_use_config_hook.py", + "pre_tool_use_config_hook_log.jsonl", + Some("^Bash$"), + "json_deny", + "blocked by config toml hook", + ) { + panic!("failed to write config.toml hook test fixture: {error}"); + } + }); + let test = builder.build(&server).await?; + + if marker.exists() { + fs::remove_file(&marker).context("remove leftover config.toml marker")?; + } + + test.submit_turn_with_policy( + "run the blocked shell command from config toml", + codex_protocol::protocol::SandboxPolicy::DangerFullAccess, + ) + .await?; + + let requests = responses.requests(); + assert_eq!(requests.len(), 2); + let output_item = requests[1].function_call_output(call_id); + let output = output_item + .get("output") + .and_then(Value::as_str) + .expect("shell command output string"); + assert!( + output.contains("Command blocked by PreToolUse hook: blocked by config toml hook"), + "blocked tool output should surface the config.toml hook reason", + ); + assert!( + !marker.exists(), + "config.toml hook should block command execution" + ); + + let hook_inputs = read_hook_inputs_from_log( + test.codex_home_path() + .join("pre_tool_use_config_hook_log.jsonl") + .as_path(), + )?; + assert_eq!(hook_inputs.len(), 1); + assert_eq!(hook_inputs[0]["hook_event_name"], "PreToolUse"); + assert_eq!(hook_inputs[0]["tool_use_id"], call_id); + assert_eq!(hook_inputs[0]["tool_input"]["command"], command); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn pre_tool_use_merges_hooks_json_and_config_toml() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let call_id = "pretooluse-merged-sources"; + let command = "printf merged-hooks".to_string(); + let args = serde_json::json!({ "command": command }); + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + core_test_support::responses::ev_function_call( + call_id, + "shell_command", + &serde_json::to_string(&args)?, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "merged hook context observed"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + let mut builder = test_codex().with_pre_build_hook(|home| { + if let Err(error) = write_pre_tool_use_hook(home, Some("^Bash$"), "allow", "unused") { + panic!("failed to write hooks.json hook fixture: {error}"); + } + if let Err(error) = write_pre_tool_use_hook_toml( + home, + "pre_tool_use_toml_hook.py", + "pre_tool_use_toml_hook_log.jsonl", + Some("^Bash$"), + "allow", + "unused", + ) { + panic!("failed to write config.toml hook fixture: {error}"); + } + }); + let test = builder.build(&server).await?; + + test.submit_turn("run the shell command with merged hook sources") + .await?; + + let requests = responses.requests(); + assert_eq!(requests.len(), 2); + let output_item = requests[1].function_call_output(call_id); + let output = output_item + .get("output") + .and_then(Value::as_str) + .expect("shell command output string"); + assert!( + output.contains("merged-hooks"), + "shell command output should still reach the model", + ); + + let json_hook_inputs = read_pre_tool_use_hook_inputs(test.codex_home_path())? + .into_iter() + .map(|hook_input| { + serde_json::json!({ + "hook_event_name": hook_input["hook_event_name"], + "tool_name": hook_input["tool_name"], + "tool_use_id": hook_input["tool_use_id"], + "tool_input": hook_input["tool_input"], + }) + }) + .collect::>(); + let toml_hook_inputs = read_hook_inputs_from_log( + test.codex_home_path() + .join("pre_tool_use_toml_hook_log.jsonl") + .as_path(), + )? + .into_iter() + .map(|hook_input| { + serde_json::json!({ + "hook_event_name": hook_input["hook_event_name"], + "tool_name": hook_input["tool_name"], + "tool_use_id": hook_input["tool_use_id"], + "tool_input": hook_input["tool_input"], + }) + }) + .collect::>(); + let expected_hook_inputs = vec![serde_json::json!({ + "hook_event_name": "PreToolUse", + "tool_name": "Bash", + "tool_use_id": call_id, + "tool_input": { + "command": command, + }, + })]; + assert_eq!(expected_hook_inputs, json_hook_inputs); + assert_eq!(expected_hook_inputs, toml_hook_inputs); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn pre_tool_use_blocks_local_shell_before_execution() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/hooks/src/engine/config.rs b/codex-rs/hooks/src/engine/config.rs deleted file mode 100644 index f70a6fceb0b9..000000000000 --- a/codex-rs/hooks/src/engine/config.rs +++ /dev/null @@ -1,50 +0,0 @@ -use serde::Deserialize; - -#[derive(Debug, Default, Deserialize)] -pub(crate) struct HooksFile { - #[serde(default)] - pub hooks: HookEvents, -} - -#[derive(Debug, Default, Deserialize)] -pub(crate) struct HookEvents { - #[serde(rename = "PreToolUse", default)] - pub pre_tool_use: Vec, - #[serde(rename = "PermissionRequest", default)] - pub permission_request: Vec, - #[serde(rename = "PostToolUse", default)] - pub post_tool_use: Vec, - #[serde(rename = "SessionStart", default)] - pub session_start: Vec, - #[serde(rename = "UserPromptSubmit", default)] - pub user_prompt_submit: Vec, - #[serde(rename = "Stop", default)] - pub stop: Vec, -} - -#[derive(Debug, Default, Deserialize)] -pub(crate) struct MatcherGroup { - #[serde(default)] - pub matcher: Option, - #[serde(default)] - pub hooks: Vec, -} - -#[derive(Debug, Clone, Deserialize)] -#[serde(tag = "type")] -pub(crate) enum HookHandlerConfig { - #[serde(rename = "command")] - Command { - command: String, - #[serde(default, rename = "timeout", alias = "timeoutSec")] - timeout_sec: Option, - #[serde(default)] - r#async: bool, - #[serde(default, rename = "statusMessage")] - status_message: Option, - }, - #[serde(rename = "prompt")] - Prompt {}, - #[serde(rename = "agent")] - Agent {}, -} diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index 3b6354e8291e..4e704e0a0358 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -1,15 +1,23 @@ +use std::fs; +use std::path::Path; + +use codex_config::CONFIG_TOML_FILE; +use codex_config::ConfigLayerEntry; +use codex_config::ConfigLayerSource; use codex_config::ConfigLayerStack; use codex_config::ConfigLayerStackOrdering; +use codex_config::HookEventsToml; +use codex_config::HookHandlerConfig; +use codex_config::HooksFile; +use codex_config::ManagedHooksRequirementsToml; +use codex_config::MatcherGroup; +use codex_config::RequirementSource; use codex_utils_absolute_path::AbsolutePathBuf; -use std::fs; +use serde::Deserialize; use super::ConfiguredHandler; -use super::config::HookHandlerConfig; -use super::config::HooksFile; -use super::config::MatcherGroup; use crate::events::common::matcher_pattern_for_event; use crate::events::common::validate_matcher_pattern; -use codex_config::ConfigLayerSource; use codex_protocol::protocol::HookSource; pub(crate) struct DiscoveryResult { @@ -17,6 +25,13 @@ pub(crate) struct DiscoveryResult { pub warnings: Vec, } +#[derive(Clone, Copy)] +struct HookHandlerSource<'a> { + path: &'a AbsolutePathBuf, + is_managed: bool, + source: HookSource, +} + pub(crate) fn discover_handlers(config_layer_stack: Option<&ConfigLayerStack>) -> DiscoveryResult { let Some(config_layer_stack) = config_layer_stack else { return DiscoveryResult { @@ -29,80 +44,58 @@ pub(crate) fn discover_handlers(config_layer_stack: Option<&ConfigLayerStack>) - let mut warnings = Vec::new(); let mut display_order = 0_i64; + append_managed_requirement_handlers( + &mut handlers, + &mut warnings, + &mut display_order, + config_layer_stack, + ); + for layer in config_layer_stack.get_layers( ConfigLayerStackOrdering::LowestPrecedenceFirst, /*include_disabled*/ false, ) { - let Some(folder) = layer.config_folder() else { - continue; - }; - let source_path = folder.join("hooks.json"); - if !source_path.as_path().is_file() { - continue; + let hook_source = hook_source_for_config_layer_source(&layer.name); + let json_hooks = load_hooks_json(layer.config_folder().as_deref(), &mut warnings); + let toml_hooks = load_toml_hooks_from_layer(layer, &mut warnings); + + if let (Some((json_source_path, json_events)), Some((toml_source_path, toml_events))) = + (&json_hooks, &toml_hooks) + && !json_events.is_empty() + && !toml_events.is_empty() + { + warnings.push(format!( + "loading hooks from both {} and {}; prefer a single representation for this layer", + json_source_path.display(), + toml_source_path.display() + )); } - let contents = match fs::read_to_string(source_path.as_path()) { - Ok(contents) => contents, - Err(err) => { - warnings.push(format!( - "failed to read hooks config {}: {err}", - source_path.display() - )); - continue; - } - }; - - let parsed: HooksFile = match serde_json::from_str(&contents) { - Ok(parsed) => parsed, - Err(err) => { - warnings.push(format!( - "failed to parse hooks config {}: {err}", - source_path.display() - )); - continue; - } - }; + if let Some((source_path, hook_events)) = json_hooks { + append_hook_events( + &mut handlers, + &mut warnings, + &mut display_order, + HookHandlerSource { + path: &source_path, + is_managed: false, + source: hook_source, + }, + hook_events, + ); + } - let super::config::HookEvents { - pre_tool_use, - permission_request, - post_tool_use, - session_start, - user_prompt_submit, - stop, - } = parsed.hooks; - - for (event_name, groups) in [ - ( - codex_protocol::protocol::HookEventName::PreToolUse, - pre_tool_use, - ), - ( - codex_protocol::protocol::HookEventName::PermissionRequest, - permission_request, - ), - ( - codex_protocol::protocol::HookEventName::PostToolUse, - post_tool_use, - ), - ( - codex_protocol::protocol::HookEventName::SessionStart, - session_start, - ), - ( - codex_protocol::protocol::HookEventName::UserPromptSubmit, - user_prompt_submit, - ), - (codex_protocol::protocol::HookEventName::Stop, stop), - ] { - append_matcher_groups( + if let Some((source_path, hook_events)) = toml_hooks { + append_hook_events( &mut handlers, &mut warnings, &mut display_order, - &source_path, - hook_source_for_config_layer_source(&layer.name), - event_name, - groups, + HookHandlerSource { + path: &source_path, + is_managed: false, + source: hook_source, + }, + hook_events, ); } } @@ -110,71 +103,272 @@ pub(crate) fn discover_handlers(config_layer_stack: Option<&ConfigLayerStack>) - DiscoveryResult { handlers, warnings } } -fn append_matcher_groups( +fn append_managed_requirement_handlers( handlers: &mut Vec, warnings: &mut Vec, display_order: &mut i64, - source_path: &AbsolutePathBuf, - source: HookSource, - event_name: codex_protocol::protocol::HookEventName, - groups: Vec, + config_layer_stack: &ConfigLayerStack, ) { - for group in groups { - let matcher = matcher_pattern_for_event(event_name, group.matcher.as_deref()); - if let Some(matcher) = matcher - && let Err(err) = validate_matcher_pattern(matcher) - { + 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; + }; + append_hook_events( + handlers, + warnings, + display_order, + HookHandlerSource { + path: &source_path, + is_managed: true, + source: hook_source_for_requirement_source(managed_hooks.source.as_ref()), + }, + managed_hooks.get().hooks.clone(), + ); +} + +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; + }; + + 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() + } +} + +fn load_hooks_json( + config_folder: Option<&Path>, + warnings: &mut Vec, +) -> Option<(AbsolutePathBuf, HookEventsToml)> { + let source_path = config_folder?.join("hooks.json"); + if !source_path.as_path().is_file() { + return None; + } + + let contents = match fs::read_to_string(source_path.as_path()) { + Ok(contents) => contents, + Err(err) => { + warnings.push(format!( + "failed to read hooks config {}: {err}", + source_path.display() + )); + return None; + } + }; + + let parsed: HooksFile = match serde_json::from_str(&contents) { + Ok(parsed) => parsed, + Err(err) => { + warnings.push(format!( + "failed to parse hooks config {}: {err}", + source_path.display() + )); + return None; + } + }; + + let source_path = AbsolutePathBuf::from_absolute_path(&source_path) + .inspect_err(|err| { warnings.push(format!( - "invalid matcher {matcher:?} in {}: {err}", + "failed to normalize hooks config path {}: {err}", source_path.display() )); - continue; + }) + .ok()?; + + (!parsed.hooks.is_empty()).then_some((source_path, parsed.hooks)) +} + +fn load_toml_hooks_from_layer( + layer: &ConfigLayerEntry, + warnings: &mut Vec, +) -> Option<(AbsolutePathBuf, HookEventsToml)> { + let source_path = config_toml_source_path(layer); + let hook_value = layer.config.get("hooks")?.clone(); + let parsed = match HookEventsToml::deserialize(hook_value) { + Ok(parsed) => parsed, + Err(err) => { + warnings.push(format!( + "failed to parse TOML hooks in {}: {err}", + source_path.display() + )); + return None; } + }; + + (!parsed.is_empty()).then_some((source_path, parsed)) +} + +fn config_toml_source_path(layer: &ConfigLayerEntry) -> AbsolutePathBuf { + match &layer.name { + ConfigLayerSource::System { file } + | ConfigLayerSource::User { file } + | ConfigLayerSource::LegacyManagedConfigTomlFromFile { file } => file.clone(), + ConfigLayerSource::Project { dot_codex_folder } => dot_codex_folder.join(CONFIG_TOML_FILE), + ConfigLayerSource::Mdm { domain, key } => { + synthetic_layer_path(&format!("/{CONFIG_TOML_FILE}")) + } + ConfigLayerSource::LegacyManagedConfigTomlFromMdm => { + synthetic_layer_path("/managed_config.toml") + } + ConfigLayerSource::SessionFlags => synthetic_layer_path("/config.toml"), + } +} + +fn synthetic_layer_path(path: &str) -> AbsolutePathBuf { + #[cfg(windows)] + { + AbsolutePathBuf::resolve_path_against_base(path, r"C:\") + } - for handler in group.hooks { - match handler { - HookHandlerConfig::Command { + #[cfg(not(windows))] + { + AbsolutePathBuf::resolve_path_against_base(path, "/") + } +} + +fn append_hook_events( + handlers: &mut Vec, + warnings: &mut Vec, + display_order: &mut i64, + source: HookHandlerSource<'_>, + hook_events: HookEventsToml, +) { + for (event_name, groups) in hook_events.into_matcher_groups() { + append_matcher_groups( + handlers, + warnings, + display_order, + source, + event_name, + groups, + ); + } +} + +fn append_matcher_groups( + handlers: &mut Vec, + warnings: &mut Vec, + display_order: &mut i64, + source: HookHandlerSource<'_>, + event_name: codex_protocol::protocol::HookEventName, + groups: Vec, +) { + for group in groups { + append_group_handlers( + handlers, + warnings, + display_order, + source, + event_name, + matcher_pattern_for_event(event_name, group.matcher.as_deref()), + group.hooks, + ); + } +} + +fn append_group_handlers( + handlers: &mut Vec, + warnings: &mut Vec, + display_order: &mut i64, + source: HookHandlerSource<'_>, + event_name: codex_protocol::protocol::HookEventName, + matcher: Option<&str>, + group_handlers: Vec, +) { + if let Some(matcher) = matcher + && let Err(err) = validate_matcher_pattern(matcher) + { + warnings.push(format!( + "invalid matcher {matcher:?} in {}: {err}", + source.path.display() + )); + return; + } + + for handler in group_handlers { + match handler { + HookHandlerConfig::Command { + command, + timeout_sec, + r#async, + status_message, + } => { + if r#async { + warnings.push(format!( + "skipping async hook in {}: async hooks are not supported yet", + source.path.display() + )); + continue; + } + if command.trim().is_empty() { + warnings.push(format!( + "skipping empty hook command in {}", + source.path.display() + )); + continue; + } + let timeout_sec = timeout_sec.unwrap_or(600).max(1); + handlers.push(ConfiguredHandler { + event_name, + is_managed: source.is_managed, + matcher: matcher.map(ToOwned::to_owned), command, timeout_sec, - r#async, status_message, - } => { - if r#async { - warnings.push(format!( - "skipping async hook in {}: async hooks are not supported yet", - source_path.display() - )); - continue; - } - if command.trim().is_empty() { - warnings.push(format!( - "skipping empty hook command in {}", - source_path.display() - )); - continue; - } - let timeout_sec = timeout_sec.unwrap_or(600).max(1); - handlers.push(ConfiguredHandler { - event_name, - matcher: matcher.map(ToOwned::to_owned), - command, - timeout_sec, - status_message, - source_path: source_path.clone(), - source, - display_order: *display_order, - }); - *display_order += 1; - } - HookHandlerConfig::Prompt {} => warnings.push(format!( - "skipping prompt hook in {}: prompt hooks are not supported yet", - source_path.display() - )), - HookHandlerConfig::Agent {} => warnings.push(format!( - "skipping agent hook in {}: agent hooks are not supported yet", - source_path.display() - )), + source_path: source.path.clone(), + source: source.source, + display_order: *display_order, + }); + *display_order += 1; } + HookHandlerConfig::Prompt {} => warnings.push(format!( + "skipping prompt hook in {}: prompt hooks are not supported yet", + source.path.display() + )), + HookHandlerConfig::Agent {} => warnings.push(format!( + "skipping agent hook in {}: agent hooks are not supported yet", + source.path.display() + )), } } } @@ -193,6 +387,22 @@ fn hook_source_for_config_layer_source(source: &ConfigLayerSource) -> HookSource } } +fn hook_source_for_requirement_source(source: Option<&RequirementSource>) -> HookSource { + match source { + Some(RequirementSource::MdmManagedPreferences { .. }) => HookSource::Mdm, + Some(RequirementSource::SystemRequirementsToml { .. }) => HookSource::System, + Some(RequirementSource::LegacyManagedConfigTomlFromFile { .. }) => { + HookSource::LegacyManagedConfigFile + } + Some(RequirementSource::LegacyManagedConfigTomlFromMdm) => { + HookSource::LegacyManagedConfigMdm + } + Some(RequirementSource::CloudRequirements | RequirementSource::Unknown) | None => { + HookSource::Unknown + } + } +} + #[cfg(test)] mod tests { use codex_config::ConfigLayerSource; @@ -204,9 +414,9 @@ mod tests { use pretty_assertions::assert_eq; use super::ConfiguredHandler; - use super::HookHandlerConfig; - use super::MatcherGroup; use super::append_matcher_groups; + use codex_config::HookHandlerConfig; + use codex_config::MatcherGroup; fn source_path() -> AbsolutePathBuf { test_path_buf("/tmp/hooks.json").abs() @@ -216,6 +426,14 @@ mod tests { HookSource::User } + fn hook_handler_source(path: &AbsolutePathBuf) -> super::HookHandlerSource<'_> { + super::HookHandlerSource { + path, + is_managed: false, + source: hook_source(), + } + } + fn command_group(matcher: Option<&str>) -> MatcherGroup { MatcherGroup { matcher: matcher.map(str::to_string), @@ -233,13 +451,13 @@ mod tests { let mut handlers = Vec::new(); let mut warnings = Vec::new(); let mut display_order = 0; + let source_path = source_path(); append_matcher_groups( &mut handlers, &mut warnings, &mut display_order, - &source_path(), - hook_source(), + hook_handler_source(&source_path), HookEventName::UserPromptSubmit, vec![command_group(Some("["))], ); @@ -249,11 +467,12 @@ mod tests { handlers, vec![ConfiguredHandler { event_name: HookEventName::UserPromptSubmit, + is_managed: false, matcher: None, command: "echo hello".to_string(), timeout_sec: 600, status_message: None, - source_path: source_path(), + source_path: source_path.clone(), source: hook_source(), display_order: 0, }] @@ -265,13 +484,13 @@ mod tests { let mut handlers = Vec::new(); let mut warnings = Vec::new(); let mut display_order = 0; + let source_path = source_path(); append_matcher_groups( &mut handlers, &mut warnings, &mut display_order, - &source_path(), - hook_source(), + hook_handler_source(&source_path), HookEventName::PreToolUse, vec![command_group(Some("^Bash$"))], ); @@ -281,11 +500,12 @@ mod tests { handlers, vec![ConfiguredHandler { event_name: HookEventName::PreToolUse, + is_managed: false, matcher: Some("^Bash$".to_string()), command: "echo hello".to_string(), timeout_sec: 600, status_message: None, - source_path: source_path(), + source_path: source_path.clone(), source: hook_source(), display_order: 0, }] @@ -297,13 +517,13 @@ mod tests { let mut handlers = Vec::new(); let mut warnings = Vec::new(); let mut display_order = 0; + let source_path = source_path(); append_matcher_groups( &mut handlers, &mut warnings, &mut display_order, - &source_path(), - hook_source(), + hook_handler_source(&source_path), HookEventName::PreToolUse, vec![command_group(Some("*"))], ); @@ -318,13 +538,13 @@ mod tests { let mut handlers = Vec::new(); let mut warnings = Vec::new(); let mut display_order = 0; + let source_path = source_path(); append_matcher_groups( &mut handlers, &mut warnings, &mut display_order, - &source_path(), - hook_source(), + hook_handler_source(&source_path), HookEventName::PostToolUse, vec![command_group(Some("Edit|Write"))], ); diff --git a/codex-rs/hooks/src/engine/dispatcher.rs b/codex-rs/hooks/src/engine/dispatcher.rs index d2c342897d65..d1cda96541ab 100644 --- a/codex-rs/hooks/src/engine/dispatcher.rs +++ b/codex-rs/hooks/src/engine/dispatcher.rs @@ -156,6 +156,7 @@ mod tests { ) -> ConfiguredHandler { ConfiguredHandler { event_name, + is_managed: false, matcher: matcher.map(str::to_owned), command: command.to_string(), timeout_sec: 5, diff --git a/codex-rs/hooks/src/engine/mod.rs b/codex-rs/hooks/src/engine/mod.rs index b4d59fc6ccea..3bfb17f6d6f0 100644 --- a/codex-rs/hooks/src/engine/mod.rs +++ b/codex-rs/hooks/src/engine/mod.rs @@ -1,5 +1,4 @@ pub(crate) mod command_runner; -pub(crate) mod config; pub(crate) mod discovery; pub(crate) mod dispatcher; pub(crate) mod output_parser; @@ -32,6 +31,7 @@ pub(crate) struct CommandShell { #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct ConfiguredHandler { pub event_name: codex_protocol::protocol::HookEventName, + pub is_managed: bool, pub matcher: Option, pub command: String, pub timeout_sec: u64, @@ -170,3 +170,7 @@ impl ClaudeHooksEngine { crate::events::stop::run(&self.handlers, &self.shell, request).await } } + +#[cfg(test)] +#[path = "mod_tests.rs"] +mod tests; diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs new file mode 100644 index 000000000000..527574b500a7 --- /dev/null +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -0,0 +1,327 @@ +use std::fs; +use std::path::Path; + +use codex_config::AbsolutePathBuf; +use codex_config::ConfigLayerEntry; +use codex_config::ConfigLayerSource; +use codex_config::ConfigLayerStack; +use codex_config::ConfigRequirements; +use codex_config::ConfigRequirementsToml; +use codex_config::Constrained; +use codex_config::ConstrainedWithSource; +use codex_config::HookEventsToml; +use codex_config::HookHandlerConfig; +use codex_config::ManagedHooksRequirementsToml; +use codex_config::MatcherGroup; +use codex_config::RequirementSource; +use codex_config::TomlValue; +use codex_protocol::ThreadId; +use pretty_assertions::assert_eq; +use tempfile::tempdir; + +use super::ClaudeHooksEngine; +use super::CommandShell; +use crate::events::pre_tool_use::PreToolUseRequest; + +fn cwd() -> AbsolutePathBuf { + AbsolutePathBuf::current_dir().expect("current dir") +} + +fn managed_hooks_for_current_platform( + managed_dir: impl AsRef, + hooks: HookEventsToml, +) -> ManagedHooksRequirementsToml { + let managed_dir = managed_dir.as_ref().to_path_buf(); + ManagedHooksRequirementsToml { + managed_dir: if cfg!(windows) { + None + } else { + Some(managed_dir.clone()) + }, + windows_managed_dir: if cfg!(windows) { + Some(managed_dir) + } else { + None + }, + hooks, + } +} + +#[tokio::test] +async fn requirements_managed_hooks_execute_from_managed_dir() { + 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 script_path = managed_dir.join("pre_tool_use.py"); + let log_path = managed_dir.join("pre_tool_use_log.jsonl"); + fs::write( + script_path.as_path(), + format!( + r#"import json +from pathlib import Path +import sys + +payload = json.load(sys.stdin) +with Path(r"{log_path}").open("a", encoding="utf-8") as handle: + handle.write(json.dumps(payload) + "\n") +"#, + log_path = log_path.display(), + ), + ) + .expect("write managed hook script"); + + let managed_hooks = managed_hooks_for_current_platform( + managed_dir.clone(), + HookEventsToml { + pre_tool_use: vec![MatcherGroup { + matcher: Some("^Bash$".to_string()), + hooks: vec![HookHandlerConfig::Command { + command: format!("python3 {}", script_path.display()), + timeout_sec: Some(10), + r#async: false, + status_message: Some("checking".to_string()), + }], + }], + ..Default::default() + }, + ); + let config_layer_stack = ConfigLayerStack::new( + Vec::new(), + ConfigRequirements { + managed_hooks: Some(ConstrainedWithSource::new( + Constrained::allow_any(managed_hooks.clone()), + Some(RequirementSource::CloudRequirements), + )), + ..ConfigRequirements::default() + }, + ConfigRequirementsToml { + hooks: Some(managed_hooks), + ..ConfigRequirementsToml::default() + }, + ) + .expect("config layer stack"); + + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + Some(&config_layer_stack), + CommandShell { + program: String::new(), + args: Vec::new(), + }, + ); + + assert!(engine.warnings().is_empty()); + assert_eq!(engine.handlers.len(), 1); + assert!(engine.handlers[0].is_managed); + let cwd = cwd(); + let preview = engine.preview_pre_tool_use(&PreToolUseRequest { + session_id: ThreadId::new(), + turn_id: "turn-1".to_string(), + cwd: cwd.clone(), + 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(), + command: "echo hello".to_string(), + }); + assert_eq!(preview.len(), 1); + assert_eq!(preview[0].source_path, managed_dir); + + let outcome = engine + .run_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(), + command: "echo hello".to_string(), + }) + .await; + + assert!(!outcome.should_block); + let log_contents = fs::read_to_string(log_path).expect("read managed hook log"); + assert!(log_contents.contains("\"hook_event_name\": \"PreToolUse\"")); +} + +#[test] +fn requirements_managed_hooks_warn_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( + missing_dir.clone(), + HookEventsToml { + pre_tool_use: vec![MatcherGroup { + matcher: Some("^Bash$".to_string()), + hooks: vec![HookHandlerConfig::Command { + command: format!("python3 {}", missing_dir.join("pre.py").display()), + timeout_sec: Some(10), + r#async: false, + status_message: Some("checking".to_string()), + }], + }], + ..Default::default() + }, + ); + let config_layer_stack = ConfigLayerStack::new( + Vec::new(), + ConfigRequirements { + managed_hooks: Some(ConstrainedWithSource::new( + Constrained::allow_any(managed_hooks.clone()), + Some(RequirementSource::CloudRequirements), + )), + ..ConfigRequirements::default() + }, + ConfigRequirementsToml { + hooks: Some(managed_hooks), + ..ConfigRequirementsToml::default() + }, + ) + .expect("config layer stack"); + + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + Some(&config_layer_stack), + CommandShell { + program: String::new(), + args: Vec::new(), + }, + ); + + assert!(engine.warnings().iter().any(|warning| { + warning.contains("managed hook directory") + && warning.contains("does not exist") + && warning.contains(&missing_dir.display().to_string()) + })); + 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(), + command: "echo hello".to_string(), + }) + .is_empty() + ); +} + +#[test] +fn discovers_hooks_from_json_and_toml_in_the_same_layer() { + 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 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()), + ); + pre_tool_use_group_entries.insert( + "hooks".to_string(), + TomlValue::Array(vec![TomlValue::Table(Default::default())]), + ); + let Some(TomlValue::Array(hooks_array)) = pre_tool_use_group_entries.get_mut("hooks") else { + unreachable!("PreToolUse hooks should be an array"); + }; + let Some(TomlValue::Table(handler_entries)) = hooks_array.first_mut() 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("python3 /tmp/toml-hook.py".to_string()), + ); + hooks_entries.insert( + "PreToolUse".to_string(), + TomlValue::Array(vec![pre_tool_use_group]), + ); + config_table.insert("hooks".to_string(), hooks_table); + let config_layer_stack = ConfigLayerStack::new( + vec![ConfigLayerEntry::new( + ConfigLayerSource::User { + file: config_path.clone(), + }, + config_toml, + )], + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("config layer stack"); + + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + Some(&config_layer_stack), + CommandShell { + program: String::new(), + args: Vec::new(), + }, + ); + + assert!(engine.warnings().iter().any(|warning| { + warning.contains("loading hooks from both") + && warning.contains(&hooks_json_path.display().to_string()) + && warning.contains(&config_path.display().to_string()) + })); + + let cwd = cwd(); + 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(), + command: "echo hello".to_string(), + }); + assert_eq!(preview.len(), 2); + assert!(engine.handlers.iter().all(|handler| !handler.is_managed)); + assert_eq!(preview[0].source_path, hooks_json_path); + assert_eq!(preview[1].source_path, config_path); +} diff --git a/codex-rs/hooks/src/events/post_tool_use.rs b/codex-rs/hooks/src/events/post_tool_use.rs index 294b08628e21..265572069b78 100644 --- a/codex-rs/hooks/src/events/post_tool_use.rs +++ b/codex-rs/hooks/src/events/post_tool_use.rs @@ -545,6 +545,7 @@ mod tests { fn handler() -> ConfiguredHandler { ConfiguredHandler { event_name: HookEventName::PostToolUse, + is_managed: false, matcher: Some("^Bash$".to_string()), command: "python3 post_tool_use_hook.py".to_string(), timeout_sec: 5, diff --git a/codex-rs/hooks/src/events/pre_tool_use.rs b/codex-rs/hooks/src/events/pre_tool_use.rs index ba4ebe167e43..926fa9a603b3 100644 --- a/codex-rs/hooks/src/events/pre_tool_use.rs +++ b/codex-rs/hooks/src/events/pre_tool_use.rs @@ -534,6 +534,7 @@ mod tests { fn handler() -> ConfiguredHandler { ConfiguredHandler { event_name: HookEventName::PreToolUse, + is_managed: false, matcher: Some("^Bash$".to_string()), command: "echo hook".to_string(), timeout_sec: 5, diff --git a/codex-rs/hooks/src/events/session_start.rs b/codex-rs/hooks/src/events/session_start.rs index 3c51426fa414..b1ccdd440a37 100644 --- a/codex-rs/hooks/src/events/session_start.rs +++ b/codex-rs/hooks/src/events/session_start.rs @@ -356,6 +356,7 @@ mod tests { fn handler() -> ConfiguredHandler { ConfiguredHandler { event_name: HookEventName::SessionStart, + is_managed: false, matcher: None, command: "echo hook".to_string(), timeout_sec: 600, diff --git a/codex-rs/hooks/src/events/stop.rs b/codex-rs/hooks/src/events/stop.rs index 5fe55568a478..f376dccd2c07 100644 --- a/codex-rs/hooks/src/events/stop.rs +++ b/codex-rs/hooks/src/events/stop.rs @@ -523,6 +523,7 @@ mod tests { fn handler() -> ConfiguredHandler { ConfiguredHandler { event_name: HookEventName::Stop, + is_managed: false, matcher: None, command: "echo hook".to_string(), timeout_sec: 600, diff --git a/codex-rs/hooks/src/events/user_prompt_submit.rs b/codex-rs/hooks/src/events/user_prompt_submit.rs index 9aaff9a7d420..2acd4808b8b4 100644 --- a/codex-rs/hooks/src/events/user_prompt_submit.rs +++ b/codex-rs/hooks/src/events/user_prompt_submit.rs @@ -414,6 +414,7 @@ mod tests { fn handler() -> ConfiguredHandler { ConfiguredHandler { event_name: HookEventName::UserPromptSubmit, + is_managed: false, matcher: None, command: "echo hook".to_string(), timeout_sec: 5, diff --git a/codex-rs/tui/src/chatwidget/tests/permissions.rs b/codex-rs/tui/src/chatwidget/tests/permissions.rs index f9fcd3179785..6a3421fe33a8 100644 --- a/codex-rs/tui/src/chatwidget/tests/permissions.rs +++ b/codex-rs/tui/src/chatwidget/tests/permissions.rs @@ -189,6 +189,7 @@ async fn approvals_popup_shows_disabled_presets() { #[tokio::test] async fn approvals_popup_navigation_skips_disabled() { let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false); chat.config.permissions.approval_policy = Constrained::new(AskForApproval::OnRequest, |candidate| match candidate { @@ -198,14 +199,23 @@ async fn approvals_popup_navigation_skips_disabled() { .expect("construct constrained approval policy"); chat.open_approvals_popup(); - // The approvals popup is the active bottom-pane view; drive navigation via chat handle_key_event. - // Start selected at idx 0 (enabled), move down twice; the disabled option should be skipped - // and selection should wrap back to idx 0 (also enabled). + // Keep the popup layout stable across platforms and feature defaults so + // the hidden numeric shortcut below still targets the disabled Full Access row. + let disabled_shortcut = if cfg!(target_os = "windows") { + '3' + } else { + '2' + }; + + // The approvals popup is the active bottom-pane view; drive navigation via + // chat.handle_key_event. Move through the menu so selection stays on an + // enabled preset even when disabled rows are skipped. chat.handle_key_event(KeyEvent::from(KeyCode::Down)); chat.handle_key_event(KeyEvent::from(KeyCode::Down)); - // Press numeric shortcut for the disabled row (3 => idx 2); should not close or accept. - chat.handle_key_event(KeyEvent::from(KeyCode::Char('3'))); + // Press the hidden numeric shortcut for the disabled Full Access row; it + // should not close the popup or accept the preset. + chat.handle_key_event(KeyEvent::from(KeyCode::Char(disabled_shortcut))); // Ensure the popup remains open and no selection actions were sent. let width = 80; diff --git a/codex-rs/tui/src/debug_config.rs b/codex-rs/tui/src/debug_config.rs index c3383497fba0..1502db16f75c 100644 --- a/codex-rs/tui/src/debug_config.rs +++ b/codex-rs/tui/src/debug_config.rs @@ -4,6 +4,7 @@ use codex_app_server_protocol::ConfigLayerSource; use codex_config::ConfigLayerEntry; use codex_config::ConfigLayerStack; use codex_config::ConfigLayerStackOrdering; +use codex_config::ManagedHooksRequirementsToml; use codex_config::NetworkConstraints; use codex_config::NetworkDomainPermissionToml; use codex_config::NetworkUnixSocketPermissionToml; @@ -168,6 +169,17 @@ fn render_debug_config_lines(stack: &ConfigLayerStack) -> Vec> { )); } + if let Some(hooks) = requirements_toml.hooks.as_ref() { + requirement_lines.push(requirement_line( + "hooks", + format_managed_hooks_requirements(hooks), + requirements + .managed_hooks + .as_ref() + .and_then(|managed_hooks| managed_hooks.source.as_ref()), + )); + } + if let Some(servers) = requirements_toml.mcp_servers.as_ref() { let value = join_or_empty(servers.keys().cloned().collect::>()); requirement_lines.push(requirement_line( @@ -257,6 +269,23 @@ fn render_session_flag_details(config: &TomlValue) -> Vec> { .collect() } +fn format_managed_hooks_requirements(hooks: &ManagedHooksRequirementsToml) -> String { + let mut parts = Vec::new(); + + if let Some(managed_dir) = hooks.managed_dir.as_ref() { + parts.push(format!("managed_dir={}", managed_dir.display())); + } + if let Some(windows_managed_dir) = hooks.windows_managed_dir.as_ref() { + parts.push(format!( + "windows_managed_dir={}", + windows_managed_dir.display() + )); + } + parts.push(format!("handlers={}", hooks.handler_count())); + + join_or_empty(parts) +} + fn render_mdm_layer_details(layer: &ConfigLayerEntry) -> Vec> { let value = layer .raw_toml() @@ -484,6 +513,10 @@ mod tests { use codex_config::ConstrainedWithSource; use codex_config::FeatureRequirementsToml; use codex_config::FilesystemConstraints; + use codex_config::HookEventsToml; + use codex_config::HookHandlerConfig; + use codex_config::ManagedHooksRequirementsToml; + use codex_config::MatcherGroup; use codex_config::McpServerIdentity; use codex_config::McpServerRequirement; use codex_config::NetworkConstraints; @@ -655,6 +688,7 @@ mod tests { feature_requirements: Some(FeatureRequirementsToml { entries: BTreeMap::from([("guardian_approval".to_string(), true)]), }), + hooks: None, mcp_servers: Some(BTreeMap::from([( "docs".to_string(), McpServerRequirement { @@ -860,6 +894,7 @@ approval_policy = "never" allowed_web_search_modes: Some(Vec::new()), guardian_policy_config: None, feature_requirements: None, + hooks: None, mcp_servers: None, apps: None, rules: None, @@ -877,6 +912,50 @@ approval_policy = "never" ); } + #[test] + fn debug_config_output_lists_managed_hooks_requirement() { + let requirements = ConfigRequirements { + managed_hooks: Some(ConstrainedWithSource::new( + Constrained::allow_any(ManagedHooksRequirementsToml { + managed_dir: Some(if cfg!(windows) { + std::path::PathBuf::from(r"C:\enterprise\hooks") + } else { + std::path::PathBuf::from("/enterprise/hooks") + }), + windows_managed_dir: Some(std::path::PathBuf::from(r"C:\enterprise\hooks")), + hooks: HookEventsToml { + pre_tool_use: vec![MatcherGroup { + matcher: Some("^Bash$".to_string()), + hooks: vec![HookHandlerConfig::Command { + command: "python3 /enterprise/hooks/pre.py".to_string(), + timeout_sec: Some(10), + r#async: false, + status_message: Some("checking".to_string()), + }], + }], + ..Default::default() + }, + }), + Some(RequirementSource::CloudRequirements), + )), + ..ConfigRequirements::default() + }; + let requirements_toml = ConfigRequirementsToml { + hooks: requirements + .managed_hooks + .as_ref() + .map(|hooks| hooks.get().clone()), + ..ConfigRequirementsToml::default() + }; + let stack = ConfigLayerStack::new(Vec::new(), requirements, requirements_toml) + .expect("config layer stack"); + + let rendered = render_to_text(&render_debug_config_lines(&stack)); + assert!(rendered.contains("hooks:")); + assert!(rendered.contains("handlers=1")); + assert!(rendered.contains("(source: cloud requirements)")); + } + #[test] fn session_all_proxy_url_uses_socks_when_enabled() { assert_eq!(