From 6300a59bb31e03aa187a8c6cd44558721f78ac94 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Wed, 29 Apr 2026 21:41:47 -0700 Subject: [PATCH] Add hook trust metadata --- .../codex_app_server_protocol.schemas.json | 25 +++- .../codex_app_server_protocol.v2.schemas.json | 25 +++- .../schema/json/v2/HooksListResponse.json | 25 +++- .../schema/typescript/v2/HookMetadata.ts | 3 +- .../schema/typescript/v2/HookTrustStatus.ts | 5 + .../schema/typescript/v2/index.ts | 1 + .../app-server-protocol/src/protocol/v2.rs | 10 ++ codex-rs/app-server/README.md | 7 +- .../app-server/src/codex_message_processor.rs | 3 + .../app-server/tests/suite/v2/hooks_list.rs | 71 ++++++++++ codex-rs/config/src/hook_config.rs | 2 + codex-rs/config/src/hooks_tests.rs | 2 + codex-rs/core/config.schema.json | 3 + codex-rs/hooks/src/config_rules.rs | 133 ++++++++++++++---- codex-rs/hooks/src/engine/discovery.rs | 125 ++++++++++++---- codex-rs/hooks/src/engine/mod.rs | 4 + codex-rs/hooks/src/engine/mod_tests.rs | 113 +++++++++++++++ codex-rs/protocol/src/protocol.rs | 9 ++ 18 files changed, 505 insertions(+), 61 deletions(-) create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/HookTrustStatus.ts 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 f856b43d66..93fb0f7c4a 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 @@ -9768,6 +9768,9 @@ "null" ] }, + "currentHash": { + "type": "string" + }, "displayOrder": { "format": "int64", "type": "integer" @@ -9815,9 +9818,19 @@ "format": "uint64", "minimum": 0.0, "type": "integer" + }, + "trustStatus": { + "$ref": "#/definitions/v2/HookTrustStatus" + }, + "trustedHash": { + "type": [ + "string", + "null" + ] } }, "required": [ + "currentHash", "displayOrder", "enabled", "eventName", @@ -9826,7 +9839,8 @@ "key", "source", "sourcePath", - "timeoutSec" + "timeoutSec", + "trustStatus" ], "type": "object" }, @@ -10016,6 +10030,15 @@ "title": "HookStartedNotification", "type": "object" }, + "HookTrustStatus": { + "enum": [ + "managed", + "untrusted", + "trusted", + "changed" + ], + "type": "string" + }, "HooksListEntry": { "properties": { "cwd": { 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 c17efe7a45..842f5ba7b0 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 @@ -6377,6 +6377,9 @@ "null" ] }, + "currentHash": { + "type": "string" + }, "displayOrder": { "format": "int64", "type": "integer" @@ -6424,9 +6427,19 @@ "format": "uint64", "minimum": 0.0, "type": "integer" + }, + "trustStatus": { + "$ref": "#/definitions/HookTrustStatus" + }, + "trustedHash": { + "type": [ + "string", + "null" + ] } }, "required": [ + "currentHash", "displayOrder", "enabled", "eventName", @@ -6435,7 +6448,8 @@ "key", "source", "sourcePath", - "timeoutSec" + "timeoutSec", + "trustStatus" ], "type": "object" }, @@ -6625,6 +6639,15 @@ "title": "HookStartedNotification", "type": "object" }, + "HookTrustStatus": { + "enum": [ + "managed", + "untrusted", + "trusted", + "changed" + ], + "type": "string" + }, "HooksListEntry": { "properties": { "cwd": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json b/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json index 5190b22711..690330a0d1 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json @@ -47,6 +47,9 @@ "null" ] }, + "currentHash": { + "type": "string" + }, "displayOrder": { "format": "int64", "type": "integer" @@ -94,9 +97,19 @@ "format": "uint64", "minimum": 0.0, "type": "integer" + }, + "trustStatus": { + "$ref": "#/definitions/HookTrustStatus" + }, + "trustedHash": { + "type": [ + "string", + "null" + ] } }, "required": [ + "currentHash", "displayOrder", "enabled", "eventName", @@ -105,7 +118,8 @@ "key", "source", "sourcePath", - "timeoutSec" + "timeoutSec", + "trustStatus" ], "type": "object" }, @@ -124,6 +138,15 @@ ], "type": "string" }, + "HookTrustStatus": { + "enum": [ + "managed", + "untrusted", + "trusted", + "changed" + ], + "type": "string" + }, "HooksListEntry": { "properties": { "cwd": { diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/HookMetadata.ts b/codex-rs/app-server-protocol/schema/typescript/v2/HookMetadata.ts index 8ccd2b1825..da4f068a2f 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/HookMetadata.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/HookMetadata.ts @@ -5,5 +5,6 @@ import type { AbsolutePathBuf } from "../AbsolutePathBuf"; import type { HookEventName } from "./HookEventName"; import type { HookHandlerType } from "./HookHandlerType"; import type { HookSource } from "./HookSource"; +import type { HookTrustStatus } from "./HookTrustStatus"; -export type HookMetadata = { key: string, eventName: HookEventName, handlerType: HookHandlerType, matcher: string | null, command: string | null, timeoutSec: bigint, statusMessage: string | null, sourcePath: AbsolutePathBuf, source: HookSource, pluginId: string | null, displayOrder: bigint, enabled: boolean, isManaged: boolean, }; +export type HookMetadata = { key: string, eventName: HookEventName, handlerType: HookHandlerType, matcher: string | null, command: string | null, timeoutSec: bigint, statusMessage: string | null, sourcePath: AbsolutePathBuf, source: HookSource, pluginId: string | null, displayOrder: bigint, enabled: boolean, isManaged: boolean, currentHash: string, trustedHash: string | null, trustStatus: HookTrustStatus, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/HookTrustStatus.ts b/codex-rs/app-server-protocol/schema/typescript/v2/HookTrustStatus.ts new file mode 100644 index 0000000000..ae9da91b6c --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/HookTrustStatus.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 HookTrustStatus = "managed" | "untrusted" | "trusted" | "changed"; 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 d369ba3423..9d54a11575 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -168,6 +168,7 @@ export type { HookRunSummary } from "./HookRunSummary"; export type { HookScope } from "./HookScope"; export type { HookSource } from "./HookSource"; export type { HookStartedNotification } from "./HookStartedNotification"; +export type { HookTrustStatus } from "./HookTrustStatus"; export type { HooksListEntry } from "./HooksListEntry"; export type { HooksListParams } from "./HooksListParams"; export type { HooksListResponse } from "./HooksListResponse"; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 963ac69000..091839a60d 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -78,6 +78,7 @@ use codex_protocol::protocol::HookRunStatus as CoreHookRunStatus; use codex_protocol::protocol::HookRunSummary as CoreHookRunSummary; use codex_protocol::protocol::HookScope as CoreHookScope; use codex_protocol::protocol::HookSource as CoreHookSource; +use codex_protocol::protocol::HookTrustStatus as CoreHookTrustStatus; use codex_protocol::protocol::ModelRerouteReason as CoreModelRerouteReason; use codex_protocol::protocol::ModelVerification as CoreModelVerification; use codex_protocol::protocol::NetworkAccess as CoreNetworkAccess; @@ -480,6 +481,12 @@ v2_enum_from_core!( } ); +v2_enum_from_core!( + pub enum HookTrustStatus from CoreHookTrustStatus { + Managed, Untrusted, Trusted, Changed + } +); + fn default_hook_source() -> HookSource { HookSource::Unknown } @@ -4799,6 +4806,9 @@ pub struct HookMetadata { pub display_order: i64, pub enabled: bool, pub is_managed: bool, + pub current_hash: String, + pub trusted_hash: Option, + pub trust_status: HookTrustStatus, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index dab47ec3a2..91cee0a6c9 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1457,7 +1457,7 @@ To enable or disable a skill by name: } ``` -Use `hooks/list` to fetch the discovered hooks for one or more `cwds`. Each entry is evaluated using that `cwd`'s effective config, so feature gating and discovered config layers can differ across entries in the same request. Disabled hooks are still returned with `"enabled": false` so clients can render and re-enable them. Hook state is stored under `hooks.state`; clients should treat hooks from managed sources as non-configurable, and user config entries for those keys are ignored during loading. Hook keys combine the source identity with a trailing event/group/handler selector that is currently positional. +Use `hooks/list` to fetch the discovered hooks for one or more `cwds`. Each entry is evaluated using that `cwd`'s effective config, so feature gating and discovered config layers can differ across entries in the same request. Disabled hooks are still returned with `"enabled": false` so clients can render and re-enable them. Hook state is stored under `hooks.state`; clients should treat hooks from managed sources as non-configurable, and user config entries for those keys are ignored during loading. Unmanaged hooks expose a `currentHash`, persisted `trustedHash`, and derived `trustStatus` so clients can distinguish first-seen hooks from hooks whose approved contents have changed. Hook keys combine the source identity with a trailing event/group/handler selector that is currently positional. ```json { @@ -1488,7 +1488,10 @@ Use `hooks/list` to fetch the discovered hooks for one or more `cwds`. Each entr "source": "user", "pluginId": null, "displayOrder": 0, - "enabled": true + "enabled": true, + "currentHash": "sha256:...", + "trustedHash": null, + "trustStatus": "untrusted" }], "warnings": [], "errors": [] diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index edc142c840..e34263c8c5 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -8851,6 +8851,9 @@ fn hooks_to_info(hooks: &[codex_hooks::HookListEntry]) -> Vec { display_order: hook.display_order, enabled: hook.enabled, is_managed: hook.is_managed, + current_hash: hook.current_hash.clone(), + trusted_hash: hook.trusted_hash.clone(), + trust_status: hook.trust_status.into(), }) .collect() } diff --git a/codex-rs/app-server/tests/suite/v2/hooks_list.rs b/codex-rs/app-server/tests/suite/v2/hooks_list.rs index f80d59d96d..e76a610d3e 100644 --- a/codex-rs/app-server/tests/suite/v2/hooks_list.rs +++ b/codex-rs/app-server/tests/suite/v2/hooks_list.rs @@ -11,6 +11,7 @@ use codex_app_server_protocol::HookEventName; use codex_app_server_protocol::HookHandlerType; use codex_app_server_protocol::HookMetadata; use codex_app_server_protocol::HookSource; +use codex_app_server_protocol::HookTrustStatus; use codex_app_server_protocol::HooksListEntry; use codex_app_server_protocol::HooksListParams; use codex_app_server_protocol::HooksListResponse; @@ -31,6 +32,49 @@ use tokio::time::timeout; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(30); +fn command_hook_hash( + event_name: &str, + matcher: Option<&str>, + command: &str, + timeout_sec: u64, + status_message: Option<&str>, +) -> String { + let codex_config::TomlValue::Table(mut table) = + codex_config::TomlValue::Table(Default::default()) + else { + unreachable!("TOML table construction should stay a table"); + }; + table.insert( + "event_name".to_string(), + codex_config::TomlValue::String(event_name.to_string()), + ); + if let Some(matcher) = matcher { + table.insert( + "matcher".to_string(), + codex_config::TomlValue::String(matcher.to_string()), + ); + } + table.insert( + "handler_type".to_string(), + codex_config::TomlValue::String("command".to_string()), + ); + table.insert( + "command".to_string(), + codex_config::TomlValue::String(command.to_string()), + ); + table.insert( + "timeout_sec".to_string(), + codex_config::TomlValue::Integer(i64::try_from(timeout_sec).unwrap_or(i64::MAX)), + ); + if let Some(status_message) = status_message { + table.insert( + "status_message".to_string(), + codex_config::TomlValue::String(status_message.to_string()), + ); + } + codex_config::version_for_toml(&codex_config::TomlValue::Table(table)) +} + fn write_user_hook_config(codex_home: &std::path::Path) -> Result<()> { std::fs::write( codex_home.join("config.toml"), @@ -113,6 +157,15 @@ async fn hooks_list_shows_discovered_hook() -> Result<()> { display_order: 0, enabled: true, is_managed: false, + current_hash: command_hook_hash( + "pre_tool_use", + Some("Bash"), + "python3 /tmp/listed-hook.py", + 5, + Some("running listed hook"), + ), + trusted_hash: None, + trust_status: HookTrustStatus::Untrusted, }], warnings: Vec::new(), errors: Vec::new(), @@ -183,6 +236,15 @@ async fn hooks_list_shows_discovered_plugin_hook() -> Result<()> { display_order: 0, enabled: true, is_managed: false, + current_hash: command_hook_hash( + "pre_tool_use", + Some("Bash"), + "echo plugin hook", + 7, + Some("running plugin hook"), + ), + trusted_hash: None, + trust_status: HookTrustStatus::Untrusted, }], warnings: Vec::new(), errors: Vec::new(), @@ -300,6 +362,15 @@ timeout = 5 display_order: 0, enabled: true, is_managed: false, + current_hash: command_hook_hash( + "pre_tool_use", + Some("Bash"), + "echo project hook", + 5, + None, + ), + trusted_hash: None, + trust_status: HookTrustStatus::Untrusted, }], warnings: Vec::new(), errors: Vec::new(), diff --git a/codex-rs/config/src/hook_config.rs b/codex-rs/config/src/hook_config.rs index d947ebb867..27cca781c7 100644 --- a/codex-rs/config/src/hook_config.rs +++ b/codex-rs/config/src/hook_config.rs @@ -25,6 +25,8 @@ pub struct HooksToml { pub struct HookStateToml { #[serde(default, skip_serializing_if = "Option::is_none")] pub enabled: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub trusted_hash: Option, } #[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] diff --git a/codex-rs/config/src/hooks_tests.rs b/codex-rs/config/src/hooks_tests.rs index 93541ee7f8..69fcd3fe95 100644 --- a/codex-rs/config/src/hooks_tests.rs +++ b/codex-rs/config/src/hooks_tests.rs @@ -90,6 +90,7 @@ fn hooks_toml_deserializes_inline_events_and_state_map() { r#" [state."/tmp/hooks.json:pre_tool_use:0:0"] enabled = false +trusted_hash = "sha256:abc123" [[PreToolUse]] matcher = "^Bash$" @@ -120,6 +121,7 @@ command = "python3 /tmp/pre.py" "/tmp/hooks.json:pre_tool_use:0:0".to_string(), super::HookStateToml { enabled: Some(false), + trusted_hash: Some("sha256:abc123".to_string()), }, )]), } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index a30a3ed925..009da93232 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -996,6 +996,9 @@ "properties": { "enabled": { "type": "boolean" + }, + "trusted_hash": { + "type": "string" } }, "type": "object" diff --git a/codex-rs/hooks/src/config_rules.rs b/codex-rs/hooks/src/config_rules.rs index b9fa871504..359c068ee2 100644 --- a/codex-rs/hooks/src/config_rules.rs +++ b/codex-rs/hooks/src/config_rules.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::collections::HashMap; use codex_config::ConfigLayerSource; use codex_config::ConfigLayerStack; @@ -6,21 +6,21 @@ use codex_config::ConfigLayerStackOrdering; use codex_config::HookStateToml; use codex_config::TomlValue; -/// Build hook enablement rules from config layers that are allowed to override +/// Build effective hook state from config layers that are allowed to override /// user preferences. /// /// This intentionally reads only user and session flag layers, including /// disabled layers, to match the skills config behavior. Project, managed, and -/// plugin layers can discover hooks, but they do not get to write user -/// enablement state. -pub(crate) fn disabled_hook_keys_from_stack( +/// plugin layers can discover hooks, but they do not get to write user hook +/// state. +pub(crate) fn hook_states_from_stack( config_layer_stack: Option<&ConfigLayerStack>, -) -> HashSet { +) -> HashMap { let Some(config_layer_stack) = config_layer_stack else { - return HashSet::new(); + return HashMap::new(); }; - let mut disabled_keys = HashSet::new(); + let mut states: HashMap = HashMap::new(); for layer in config_layer_stack.get_layers( ConfigLayerStackOrdering::LowestPrecedenceFirst, /*include_disabled*/ true, @@ -54,21 +54,19 @@ pub(crate) fn disabled_hook_keys_from_stack( if key.is_empty() { continue; } - // Later layers win. Hooks without an explicit enabled override can - // still carry future per-hook state without changing enablement. - match state.enabled { - Some(false) => { - disabled_keys.insert(key.to_string()); - } - Some(true) => { - disabled_keys.remove(key); - } - None => {} + // Later layers win field-by-field so a future per-hook state write + // does not accidentally erase an existing enablement override. + let effective_state = states.entry(key.to_string()).or_default(); + if let Some(enabled) = state.enabled { + effective_state.enabled = Some(enabled); + } + if let Some(trusted_hash) = state.trusted_hash { + effective_state.trusted_hash = Some(trusted_hash); } } } - disabled_keys + states } #[cfg(test)] @@ -82,7 +80,7 @@ mod tests { use super::*; #[test] - fn disabled_hook_keys_from_stack_respects_layer_precedence() { + fn hook_states_from_stack_respects_layer_precedence() { let key = "file:/tmp/hooks.json:pre_tool_use:0:0"; let stack = ConfigLayerStack::new( vec![ @@ -102,11 +100,65 @@ mod tests { ) .expect("config layer stack"); - assert_eq!(disabled_hook_keys_from_stack(Some(&stack)), HashSet::new()); + assert_eq!( + hook_states_from_stack(Some(&stack)), + HashMap::from([( + key.to_string(), + HookStateToml { + enabled: Some(true), + trusted_hash: None, + }, + )]) + ); } #[test] - fn disabled_hook_keys_from_stack_ignores_malformed_hook_events() { + fn hook_states_from_stack_merges_fields_across_layers() { + let key = "file:/tmp/hooks.json:pre_tool_use:0:0"; + let stack = ConfigLayerStack::new( + vec![ + ConfigLayerEntry::new( + ConfigLayerSource::User { + file: test_path_buf("/tmp/config.toml").abs(), + }, + config_with_hook_state( + key, + HookStateToml { + enabled: Some(/*enabled*/ false), + trusted_hash: None, + }, + ), + ), + ConfigLayerEntry::new( + ConfigLayerSource::SessionFlags, + config_with_hook_state( + key, + HookStateToml { + enabled: None, + trusted_hash: Some("sha256:trusted".to_string()), + }, + ), + ), + ], + Default::default(), + Default::default(), + ) + .expect("config layer stack"); + + assert_eq!( + hook_states_from_stack(Some(&stack)), + HashMap::from([( + key.to_string(), + HookStateToml { + enabled: Some(false), + trusted_hash: Some("sha256:trusted".to_string()), + }, + )]) + ); + } + + #[test] + fn hook_states_from_stack_ignores_malformed_hook_events() { let key = "file:/tmp/hooks.json:pre_tool_use:0:0"; let config: TomlValue = serde_json::from_value(serde_json::json!({ "hooks": { @@ -132,13 +184,19 @@ mod tests { .expect("config layer stack"); assert_eq!( - disabled_hook_keys_from_stack(Some(&stack)), - HashSet::from([key.to_string()]) + hook_states_from_stack(Some(&stack)), + HashMap::from([( + key.to_string(), + HookStateToml { + enabled: Some(false), + trusted_hash: None, + }, + )]) ); } #[test] - fn disabled_hook_keys_from_stack_ignores_malformed_state_entries() { + fn hook_states_from_stack_ignores_malformed_state_entries() { let key = "file:/tmp/hooks.json:pre_tool_use:0:0"; let config: TomlValue = serde_json::from_value(serde_json::json!({ "hooks": { @@ -166,16 +224,29 @@ mod tests { .expect("config layer stack"); assert_eq!( - disabled_hook_keys_from_stack(Some(&stack)), - HashSet::from([key.to_string()]) + hook_states_from_stack(Some(&stack)), + HashMap::from([( + key.to_string(), + HookStateToml { + enabled: Some(false), + trusted_hash: None, + }, + )]) ); } fn config_with_hook_override(key: &str, enabled: Option) -> TomlValue { - let hook_state = match enabled { - Some(enabled) => serde_json::json!({ "enabled": enabled }), - None => serde_json::json!({}), - }; + config_with_hook_state( + key, + HookStateToml { + enabled, + trusted_hash: None, + }, + ) + } + + fn config_with_hook_state(key: &str, state: HookStateToml) -> TomlValue { + let hook_state = serde_json::to_value(state).expect("hook state should serialize"); serde_json::from_value(serde_json::json!({ "hooks": { "state": { diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index 8c520b7497..4d441c7bb1 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -8,23 +8,26 @@ use codex_config::ConfigLayerStack; use codex_config::ConfigLayerStackOrdering; use codex_config::HookEventsToml; use codex_config::HookHandlerConfig; +use codex_config::HookStateToml; use codex_config::HooksFile; use codex_config::ManagedHooksRequirementsToml; use codex_config::MatcherGroup; use codex_config::RequirementSource; +use codex_config::TomlValue; +use codex_config::version_for_toml; use codex_plugin::PluginHookSource; use codex_utils_absolute_path::AbsolutePathBuf; use serde::Deserialize; use std::collections::HashMap; -use std::collections::HashSet; use super::ConfiguredHandler; use super::HookListEntry; -use crate::config_rules::disabled_hook_keys_from_stack; +use crate::config_rules::hook_states_from_stack; use crate::events::common::matcher_pattern_for_event; use crate::events::common::validate_matcher_pattern; use codex_protocol::protocol::HookHandlerType; use codex_protocol::protocol::HookSource; +use codex_protocol::protocol::HookTrustStatus; pub(crate) struct DiscoveryResult { pub handlers: Vec, @@ -36,7 +39,7 @@ struct HookHandlerSource<'a> { path: &'a AbsolutePathBuf, key_source: String, source: HookSource, - disabled_hook_keys: &'a HashSet, + hook_states: &'a HashMap, env: HashMap, plugin_id: Option, } @@ -50,7 +53,7 @@ pub(crate) fn discover_handlers( let mut hook_entries = Vec::new(); let mut warnings = plugin_hook_load_warnings; let mut display_order = 0_i64; - let disabled_hook_keys = disabled_hook_keys_from_stack(config_layer_stack); + let hook_states = hook_states_from_stack(config_layer_stack); if let Some(config_layer_stack) = config_layer_stack { append_managed_requirement_handlers( @@ -59,7 +62,7 @@ pub(crate) fn discover_handlers( &mut warnings, &mut display_order, config_layer_stack, - &disabled_hook_keys, + &hook_states, ); for layer in config_layer_stack.get_layers( @@ -92,7 +95,7 @@ pub(crate) fn discover_handlers( path: &source_path, key_source: source_path.display().to_string(), source: hook_source, - disabled_hook_keys: &disabled_hook_keys, + hook_states: &hook_states, env: HashMap::new(), plugin_id: None, }, @@ -108,7 +111,7 @@ pub(crate) fn discover_handlers( &mut warnings, &mut display_order, plugin_hook_sources, - &disabled_hook_keys, + &hook_states, ); DiscoveryResult { @@ -124,7 +127,7 @@ fn append_managed_requirement_handlers( warnings: &mut Vec, display_order: &mut i64, config_layer_stack: &ConfigLayerStack, - disabled_hook_keys: &HashSet, + hook_states: &HashMap, ) { let Some(managed_hooks) = config_layer_stack.requirements().managed_hooks.as_ref() else { return; @@ -143,7 +146,7 @@ fn append_managed_requirement_handlers( path: &source_path, key_source: source_path.display().to_string(), source: hook_source_for_requirement_source(managed_hooks.source.as_ref()), - disabled_hook_keys, + hook_states, env: HashMap::new(), plugin_id: None, }, @@ -157,7 +160,7 @@ fn append_plugin_hook_sources( warnings: &mut Vec, display_order: &mut i64, plugin_hook_sources: Vec, - disabled_hook_keys: &HashSet, + hook_states: &HashMap, ) { // TODO(abhinav): check enabled/trusted state here before plugin hooks become runnable. for source in plugin_hook_sources { @@ -188,7 +191,7 @@ fn append_plugin_hook_sources( path: &source_path, key_source: format!("{plugin_id}:{source_relative_path}"), source: HookSource::Plugin, - disabled_hook_keys, + hook_states, env, plugin_id: Some(plugin_id), }, @@ -396,10 +399,17 @@ fn append_matcher_groups( )); continue; } + let timeout_sec = timeout_sec.unwrap_or(600).max(1); + let current_hash = command_hook_hash( + event_name, + matcher, + &command, + timeout_sec, + status_message.as_deref(), + ); let command = source.env.iter().fold(command, |command, (key, value)| { command.replace(&format!("${{{key}}}"), value) }); - let timeout_sec = timeout_sec.unwrap_or(600).max(1); // TODO(abhinav): replace this positional suffix with a durable hook id. let key = format!( "{}:{}:{}:{}", @@ -408,8 +418,14 @@ fn append_matcher_groups( group_index, handler_index ); - let enabled = - source.source.is_managed() || !source.disabled_hook_keys.contains(&key); + let state = source.hook_states.get(&key); + let enabled = source.source.is_managed() + || state.and_then(|state| state.enabled) != Some(false); + let trusted_hash = (!source.source.is_managed()) + .then(|| state.and_then(|state| state.trusted_hash.clone())) + .flatten(); + let trust_status = + hook_trust_status(source.source, ¤t_hash, trusted_hash.as_deref()); hook_entries.push(HookListEntry { key, event_name, @@ -424,6 +440,9 @@ fn append_matcher_groups( display_order: *display_order, enabled, is_managed: source.source.is_managed(), + current_hash, + trusted_hash, + trust_status, }); if enabled { handlers.push(ConfiguredHandler { @@ -453,6 +472,63 @@ fn append_matcher_groups( } } +fn command_hook_hash( + event_name: codex_protocol::protocol::HookEventName, + matcher: Option<&str>, + command: &str, + timeout_sec: u64, + status_message: Option<&str>, +) -> String { + let TomlValue::Table(mut table) = TomlValue::Table(Default::default()) else { + unreachable!("TOML table construction should stay a table"); + }; + table.insert( + "event_name".to_string(), + TomlValue::String(hook_event_key_label(event_name).to_string()), + ); + if let Some(matcher) = matcher { + table.insert( + "matcher".to_string(), + TomlValue::String(matcher.to_string()), + ); + } + table.insert( + "handler_type".to_string(), + TomlValue::String("command".to_string()), + ); + table.insert( + "command".to_string(), + TomlValue::String(command.to_string()), + ); + table.insert( + "timeout_sec".to_string(), + TomlValue::Integer(i64::try_from(timeout_sec).unwrap_or(i64::MAX)), + ); + if let Some(status_message) = status_message { + table.insert( + "status_message".to_string(), + TomlValue::String(status_message.to_string()), + ); + } + version_for_toml(&TomlValue::Table(table)) +} + +fn hook_trust_status( + source: HookSource, + current_hash: &str, + trusted_hash: Option<&str>, +) -> HookTrustStatus { + if source.is_managed() { + return HookTrustStatus::Managed; + } + + match trusted_hash { + Some(trusted_hash) if trusted_hash == current_hash => HookTrustStatus::Trusted, + Some(_) => HookTrustStatus::Changed, + None => HookTrustStatus::Untrusted, + } +} + fn hook_event_key_label(event_name: codex_protocol::protocol::HookEventName) -> &'static str { match event_name { codex_protocol::protocol::HookEventName::PreToolUse => "pre_tool_use", @@ -508,6 +584,7 @@ mod tests { use super::ConfiguredHandler; use super::append_matcher_groups; use codex_config::HookHandlerConfig; + use codex_config::HookStateToml; use codex_config::MatcherGroup; use codex_config::TomlValue; @@ -521,13 +598,13 @@ mod tests { fn hook_handler_source<'a>( path: &'a AbsolutePathBuf, - disabled_hook_keys: &'a std::collections::HashSet, + hook_states: &'a std::collections::HashMap, ) -> super::HookHandlerSource<'a> { super::HookHandlerSource { path, key_source: path.display().to_string(), source: hook_source(), - disabled_hook_keys, + hook_states, env: std::collections::HashMap::new(), plugin_id: None, } @@ -551,14 +628,14 @@ mod tests { let mut warnings = Vec::new(); let mut display_order = 0; let source_path = source_path(); - let disabled_hook_keys = std::collections::HashSet::new(); + let hook_states = std::collections::HashMap::new(); append_matcher_groups( &mut handlers, &mut Vec::new(), &mut warnings, &mut display_order, - &hook_handler_source(&source_path, &disabled_hook_keys), + &hook_handler_source(&source_path, &hook_states), HookEventName::UserPromptSubmit, vec![command_group(Some("["))], ); @@ -586,14 +663,14 @@ mod tests { let mut warnings = Vec::new(); let mut display_order = 0; let source_path = source_path(); - let disabled_hook_keys = std::collections::HashSet::new(); + let hook_states = std::collections::HashMap::new(); append_matcher_groups( &mut handlers, &mut Vec::new(), &mut warnings, &mut display_order, - &hook_handler_source(&source_path, &disabled_hook_keys), + &hook_handler_source(&source_path, &hook_states), HookEventName::PreToolUse, vec![command_group(Some("^Bash$"))], ); @@ -621,14 +698,14 @@ mod tests { let mut warnings = Vec::new(); let mut display_order = 0; let source_path = source_path(); - let disabled_hook_keys = std::collections::HashSet::new(); + let hook_states = std::collections::HashMap::new(); append_matcher_groups( &mut handlers, &mut Vec::new(), &mut warnings, &mut display_order, - &hook_handler_source(&source_path, &disabled_hook_keys), + &hook_handler_source(&source_path, &hook_states), HookEventName::PreToolUse, vec![command_group(Some("*"))], ); @@ -644,14 +721,14 @@ mod tests { let mut warnings = Vec::new(); let mut display_order = 0; let source_path = source_path(); - let disabled_hook_keys = std::collections::HashSet::new(); + let hook_states = std::collections::HashMap::new(); append_matcher_groups( &mut handlers, &mut Vec::new(), &mut warnings, &mut display_order, - &hook_handler_source(&source_path, &disabled_hook_keys), + &hook_handler_source(&source_path, &hook_states), HookEventName::PostToolUse, vec![command_group(Some("Edit|Write"))], ); diff --git a/codex-rs/hooks/src/engine/mod.rs b/codex-rs/hooks/src/engine/mod.rs index c06c9fabd7..63bf4b371b 100644 --- a/codex-rs/hooks/src/engine/mod.rs +++ b/codex-rs/hooks/src/engine/mod.rs @@ -12,6 +12,7 @@ use codex_protocol::protocol::HookEventName; use codex_protocol::protocol::HookHandlerType; use codex_protocol::protocol::HookRunSummary; use codex_protocol::protocol::HookSource; +use codex_protocol::protocol::HookTrustStatus; use codex_utils_absolute_path::AbsolutePathBuf; use crate::events::permission_request::PermissionRequestOutcome; @@ -83,6 +84,9 @@ pub struct HookListEntry { pub display_order: i64, pub enabled: bool, pub is_managed: bool, + pub current_hash: String, + pub trusted_hash: Option, + pub trust_status: HookTrustStatus, } #[derive(Clone)] diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index c37539bb1d..aa88c9a281 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -12,6 +12,7 @@ use codex_config::Constrained; use codex_config::ConstrainedWithSource; use codex_config::HookEventsToml; use codex_config::HookHandlerConfig; +use codex_config::HookStateToml; use codex_config::ManagedHooksRequirementsToml; use codex_config::MatcherGroup; use codex_config::RequirementSource; @@ -22,6 +23,7 @@ use codex_protocol::ThreadId; use codex_protocol::protocol::HookOutputEntryKind; use codex_protocol::protocol::HookRunStatus; use codex_protocol::protocol::HookSource; +use codex_protocol::protocol::HookTrustStatus; use pretty_assertions::assert_eq; use tempfile::tempdir; @@ -235,6 +237,10 @@ fn user_disablement_filters_non_managed_hooks_but_not_managed_hooks() { assert_eq!(discovered.hook_entries[0].key, managed_disabled_key); assert_eq!(discovered.hook_entries[0].enabled, true); assert!(discovered.hook_entries[0].is_managed); + assert_eq!( + discovered.hook_entries[0].trust_status, + HookTrustStatus::Managed + ); assert_eq!(discovered.hook_entries[1].key, user_disabled_key); assert_eq!(discovered.hook_entries[1].enabled, false); assert!(!discovered.hook_entries[1].is_managed); @@ -288,6 +294,92 @@ fn user_disablement_does_not_filter_managed_layer_hooks() { assert_eq!(discovered.hook_entries[0].key, managed_key); assert_eq!(discovered.hook_entries[0].enabled, true); assert!(discovered.hook_entries[0].is_managed); + assert_eq!( + discovered.hook_entries[0].trust_status, + HookTrustStatus::Managed + ); +} + +#[test] +fn unmanaged_hook_trust_status_tracks_stored_hash() { + let temp = tempdir().expect("create temp dir"); + let config_path = + AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute path"); + let key = format!("{}:pre_tool_use:0:0", config_path.display()); + + let untrusted_stack = ConfigLayerStack::new( + vec![ConfigLayerEntry::new( + ConfigLayerSource::User { + file: config_path.clone(), + }, + config_with_pre_tool_use_hook("python3 /tmp/user.py"), + )], + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("config layer stack"); + let untrusted = + super::discovery::discover_handlers(Some(&untrusted_stack), Vec::new(), Vec::new()); + assert_eq!(untrusted.hook_entries.len(), 1); + assert_eq!( + untrusted.hook_entries[0].trust_status, + HookTrustStatus::Untrusted + ); + assert_eq!(untrusted.hook_entries[0].trusted_hash, None); + + let current_hash = untrusted.hook_entries[0].current_hash.clone(); + let trusted_stack = ConfigLayerStack::new( + vec![ConfigLayerEntry::new( + ConfigLayerSource::User { file: config_path }, + config_with_pre_tool_use_hook_and_state( + "python3 /tmp/user.py", + &key, + HookStateToml { + enabled: None, + trusted_hash: Some(current_hash.clone()), + }, + ), + )], + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("config layer stack"); + let trusted = super::discovery::discover_handlers(Some(&trusted_stack), Vec::new(), Vec::new()); + assert_eq!(trusted.hook_entries.len(), 1); + assert_eq!( + trusted.hook_entries[0].trust_status, + HookTrustStatus::Trusted + ); + assert_eq!(trusted.hook_entries[0].trusted_hash, Some(current_hash)); + + let changed_stack = ConfigLayerStack::new( + vec![ConfigLayerEntry::new( + ConfigLayerSource::User { + file: trusted.hook_entries[0].source_path.clone(), + }, + config_with_pre_tool_use_hook_and_state( + "python3 /tmp/user.py", + &key, + HookStateToml { + enabled: None, + trusted_hash: Some("sha256:old".to_string()), + }, + ), + )], + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("config layer stack"); + let changed = super::discovery::discover_handlers(Some(&changed_stack), Vec::new(), Vec::new()); + assert_eq!(changed.hook_entries.len(), 1); + assert_eq!( + changed.hook_entries[0].trust_status, + HookTrustStatus::Changed + ); + assert_eq!( + changed.hook_entries[0].trusted_hash.as_deref(), + Some("sha256:old") + ); } fn config_with_hook_state(key: &str, enabled: bool) -> TomlValue { @@ -303,6 +395,27 @@ fn config_with_hook_state(key: &str, enabled: bool) -> TomlValue { .expect("config TOML should deserialize") } +fn config_with_pre_tool_use_hook_and_state( + command: &str, + key: &str, + state: HookStateToml, +) -> TomlValue { + serde_json::from_value(serde_json::json!({ + "hooks": { + "state": { + (key): serde_json::to_value(state).expect("hook state should serialize"), + }, + "PreToolUse": [{ + "hooks": [{ + "type": "command", + "command": command, + }], + }], + }, + })) + .expect("config TOML should deserialize") +} + fn config_with_pre_tool_use_hook_and_states( command: &str, disabled_keys: [&str; N], diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 60137fa8b0..2e78c04d70 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1574,6 +1574,15 @@ impl HookSource { } } +#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +pub enum HookTrustStatus { + Managed, + Untrusted, + Trusted, + Changed, +} + #[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "snake_case")] pub enum HookRunStatus {