openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-23 18:08:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

Gate runnable hooks on trust

Browse Source
This commit is contained in:
Abhinav Vedmala
2026-05-01 14:01:27 -07:00
parent 6300a59bb3
commit dafa805c3c
4 changed files with 94 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/README.md
+5 -1
View File
@@ -1457,7 +1457,11 @@ 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. 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.
Use `hooks/list` to fetch discovered hooks for one or more `cwds`. Each result is evaluated with that `cwd`'s effective config, so feature gates and discovered config layers can differ within a single response.
Hooks are returned even when disabled so clients can render and re-enable them. User-controlled state lives under `hooks.state`. Managed hooks are non-configurable, and user entries for managed hook keys are ignored during loading.
For unmanaged hooks, `currentHash`, `trustedHash`, and `trustStatus` describe whether the current definition is first-seen, approved, or changed since approval. Only trusted unmanaged hooks become runnable. Hook keys combine the source identity with a trailing event/group/handler selector that is currently positional.
```json
{
codex-rs/app-server/tests/suite/v2/hooks_list.rs
+27 -4
View File
@@ -161,7 +161,7 @@ async fn hooks_list_shows_discovered_hook() -> Result<()> {
"pre_tool_use",
Some("Bash"),
"python3 /tmp/listed-hook.py",
5,
/*timeout_sec*/ 5,
Some("running listed hook"),
),
trusted_hash: None,
@@ -240,7 +240,7 @@ async fn hooks_list_shows_discovered_plugin_hook() -> Result<()> {
"pre_tool_use",
Some("Bash"),
"echo plugin hook",
7,
/*timeout_sec*/ 7,
Some("running plugin hook"),
),
trusted_hash: None,
@@ -366,8 +366,8 @@ timeout = 5
"pre_tool_use",
Some("Bash"),
"echo project hook",
5,
None,
/*timeout_sec*/ 5,
/*status_message*/ None,
),
trusted_hash: None,
trust_status: HookTrustStatus::Untrusted,
@@ -553,6 +553,29 @@ command = "python3 {hook_script_path}"
let hook = &data[0].hooks[0];
assert_eq!(hook.enabled, true);
let write_id = mcp
.send_config_batch_write_request(ConfigBatchWriteParams {
edits: vec![ConfigEdit {
key_path: "hooks.state".to_string(),
value: serde_json::json!({
hook.key.clone(): {
"trusted_hash": hook.current_hash.clone()
}
}),
merge_strategy: MergeStrategy::Upsert,
}],
file_path: None,
expected_version: None,
reload_user_config: true,
})
.await?;
let response: JSONRPCResponse = timeout(
DEFAULT_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(write_id)),
)
.await??;
let _: codex_app_server_protocol::ConfigWriteResponse = to_response(response)?;
let thread_start_id = mcp
.send_thread_start_request(ThreadStartParams {
model: Some("mock-model".to_string()),
codex-rs/hooks/src/engine/discovery.rs
+7 -3
View File
@@ -162,7 +162,6 @@ fn append_plugin_hook_sources(
plugin_hook_sources: Vec<PluginHookSource>,
hook_states: &HashMap<String, HookStateToml>,
) {
// TODO(abhinav): check enabled/trusted state here before plugin hooks become runnable.
for source in plugin_hook_sources {
let PluginHookSource {
plugin_root,
@@ -444,7 +443,12 @@ fn append_matcher_groups(
trusted_hash,
trust_status,
});
if enabled {
if enabled
&& matches!(
trust_status,
HookTrustStatus::Managed | HookTrustStatus::Trusted
)
{
handlers.push(ConfiguredHandler {
event_name,
matcher: matcher.map(ToOwned::to_owned),
@@ -593,7 +597,7 @@ mod tests {
}
fn hook_source() -> HookSource {
HookSource::User
HookSource::System
}
fn hook_handler_source<'a>(
codex-rs/hooks/src/engine/mod_tests.rs
+55 -5
View File
@@ -326,6 +326,7 @@ fn unmanaged_hook_trust_status_tracks_stored_hash() {
HookTrustStatus::Untrusted
);
assert_eq!(untrusted.hook_entries[0].trusted_hash, None);
assert_eq!(untrusted.handlers, Vec::new());
let current_hash = untrusted.hook_entries[0].current_hash.clone();
let trusted_stack = ConfigLayerStack::new(
@@ -351,6 +352,7 @@ fn unmanaged_hook_trust_status_tracks_stored_hash() {
HookTrustStatus::Trusted
);
assert_eq!(trusted.hook_entries[0].trusted_hash, Some(current_hash));
assert_eq!(trusted.handlers.len(), 1);
let changed_stack = ConfigLayerStack::new(
vec![ConfigLayerEntry::new(
@@ -380,6 +382,7 @@ fn unmanaged_hook_trust_status_tracks_stored_hash() {
changed.hook_entries[0].trusted_hash.as_deref(),
Some("sha256:old")
);
assert_eq!(changed.handlers, Vec::new());
}
fn config_with_hook_state(key: &str, enabled: bool) -> TomlValue {
@@ -452,6 +455,45 @@ fn config_with_pre_tool_use_hook(command: &str) -> TomlValue {
.expect("config TOML should deserialize")
}
fn trusted_plugin_hook_stack(
config_path: AbsolutePathBuf,
plugin_hook_sources: &[PluginHookSource],
) -> ConfigLayerStack {
let discovered = super::discovery::discover_handlers(
/*config_layer_stack*/ None,
plugin_hook_sources.to_vec(),
Vec::new(),
);
let state = discovered
.hook_entries
.into_iter()
.map(|entry| {
(
entry.key,
serde_json::json!({
"trusted_hash": entry.current_hash,
}),
)
})
.collect::<serde_json::Map<_, _>>();
let config = serde_json::from_value(serde_json::json!({
"hooks": {
"state": state,
},
}))
.expect("config TOML should deserialize");
ConfigLayerStack::new(
vec![ConfigLayerEntry::new(
ConfigLayerSource::User { file: config_path },
config,
)],
ConfigRequirements::default(),
ConfigRequirementsToml::default(),
)
.expect("config layer stack")
}
#[test]
fn requirements_managed_hooks_warn_when_managed_dir_is_missing() {
let temp = tempdir().expect("create temp dir");
@@ -586,7 +628,7 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() {
config_table.insert("hooks".to_string(), hooks_table);
let config_layer_stack = ConfigLayerStack::new(
vec![ConfigLayerEntry::new(
ConfigLayerSource::User {
ConfigLayerSource::System {
file: config_path.clone(),
},
config_toml,
@@ -631,7 +673,7 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() {
engine
.handlers
.iter()
.all(|handler| !handler.source.is_managed())
.all(|handler| handler.source.is_managed())
);
assert_eq!(preview[0].source_path, hooks_json_path);
assert_eq!(preview[1].source_path, config_path);
@@ -680,10 +722,14 @@ print(json.dumps({
..Default::default()
},
}];
let config_layer_stack = trusted_plugin_hook_stack(
AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute config path"),
&plugin_hook_sources,
);
let engine = ClaudeHooksEngine::new(
/*enabled*/ true,
/*config_layer_stack*/ None,
plugin_hook_sources.clone(),
Some(&config_layer_stack),
plugin_hook_sources,
Vec::new(),
CommandShell {
program: String::new(),
@@ -784,9 +830,13 @@ fn plugin_hook_sources_expand_plugin_placeholders() {
..Default::default()
},
}];
let config_layer_stack = trusted_plugin_hook_stack(
AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute config path"),
&plugin_hook_sources,
);
let engine = ClaudeHooksEngine::new(
/*enabled*/ true,
/*config_layer_stack*/ None,
Some(&config_layer_stack),
plugin_hook_sources,
Vec::new(),
CommandShell {