From 31e8c46554d9f27f693afc6c7cc3f9afe6618b93 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Sat, 9 May 2026 22:40:41 -0400 Subject: [PATCH] Keep hook visibility out of trust hashes --- codex-rs/config/src/hook_config.rs | 6 ++- codex-rs/hooks/src/engine/discovery.rs | 2 +- codex-rs/hooks/src/engine/mod_tests.rs | 54 ++++++++++++++++++++++++++ codex-rs/protocol/src/protocol.rs | 6 +++ 4 files changed, 66 insertions(+), 2 deletions(-) diff --git a/codex-rs/config/src/hook_config.rs b/codex-rs/config/src/hook_config.rs index 77bad55832..2e6eecd1ef 100644 --- a/codex-rs/config/src/hook_config.rs +++ b/codex-rs/config/src/hook_config.rs @@ -133,7 +133,11 @@ pub enum HookHandlerConfig { r#async: bool, #[serde(default, rename = "statusMessage")] status_message: Option, - #[serde(default, rename = "visibilityHint")] + #[serde( + default, + rename = "visibilityHint", + skip_serializing_if = "HookVisibilityHint::is_default" + )] visibility_hint: HookVisibilityHint, }, #[serde(rename = "prompt")] diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index a5a0625609..75778a1e41 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -413,7 +413,7 @@ fn append_matcher_groups( timeout_sec: Some(timeout_sec), r#async, status_message: status_message.clone(), - visibility_hint, + visibility_hint: Default::default(), }; let current_hash = command_hook_hash(event_name, matcher, &group, normalized_handler); diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index e2069fe688..b8ed200356 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -483,6 +483,60 @@ fn config_with_pre_tool_use_hook(command: &str) -> TomlValue { .expect("config TOML should deserialize") } +#[test] +fn presentation_only_visibility_changes_preserve_existing_hook_trust() { + let temp = tempdir().expect("create temp dir"); + let config_path = + AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute path"); + let hook_key = format!("{}:pre_tool_use:0:0", config_path.display()); + let legacy_identity: TomlValue = serde_json::from_value(serde_json::json!({ + "event_name": "pre_tool_use", + "hooks": [{ + "type": "command", + "command": "python3 /tmp/user.py", + "timeout": 600, + "async": false, + }], + })) + .expect("legacy identity should deserialize"); + let trusted_hash = codex_config::version_for_toml(&legacy_identity); + let config_layer_stack = ConfigLayerStack::new( + vec![ConfigLayerEntry::new( + ConfigLayerSource::User { file: config_path }, + serde_json::from_value(serde_json::json!({ + "hooks": { + "state": { + (hook_key): { + "trusted_hash": trusted_hash, + }, + }, + "PreToolUse": [{ + "hooks": [{ + "type": "command", + "command": "python3 /tmp/user.py", + "visibilityHint": "hidden", + }], + }], + }, + })) + .expect("config TOML should deserialize"), + )], + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("config layer stack"); + + let discovered = + super::discovery::discover_handlers(Some(&config_layer_stack), Vec::new(), Vec::new()); + + assert_eq!(discovered.hook_entries.len(), 1); + assert_eq!(discovered.hook_entries[0].current_hash, trusted_hash); + assert_eq!( + discovered.hook_entries[0].trust_status, + HookTrustStatus::Trusted + ); +} + fn trusted_plugin_hook_stack( config_path: AbsolutePathBuf, plugin_hook_sources: &[PluginHookSource], diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 444848db63..22b2099f6b 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1526,6 +1526,12 @@ pub enum HookVisibilityHint { Hidden, } +impl HookVisibilityHint { + pub fn is_default(&self) -> bool { + *self == Self::Default + } +} + #[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "snake_case")] pub enum HookOutputEntryKind {