github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-19 16:17:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Bind plugin hook approval to plugin contents (#54619)

Browse Source 
## Why

A plugin's scripts can change without changing its hook declaration. Declaration-only approval therefore lets changed plugin code run under an existing hook approval.

## What changed

- Fingerprint plugin bundle contents and include the content hash in the approval value returned by `hooks/list`. Persist it as `trusted_content_hash`, keeping `trusted_hash` compatible with older clients.
- Verify contents before each command or MCP hook execution, within the hook's timeout, and require renewed approval when approved contents change.
- Migrate legacy declaration-only approvals to a shared, immutable content baseline per plugin and config home.
- Exclude generated Python bytecode, `.venv`, and `.egg-info` entries from fingerprints. Run app-server hook listing on blocking tasks.

## Testing

Extend integration coverage for plugin upgrades and rollbacks requiring approval, same-turn script changes preventing further hook execution, shared legacy approval baselines, and generated Python artifacts preserving trust.

GitOrigin-RevId: faaf7658210d8adb2534ddd614c72ec993920840
This commit is contained in:
jif
2026-10-16 14:38:19 +00:00
committed by copyberry
parent 4eb4a65407
commit 2bf2bdf82c
31 changed files with 652 additions and 115 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/Cargo.lock
Generated
+1
View File
@@ -3712,6 +3712,7 @@ dependencies = [
"schemars 0.8.22",
"serde",
"serde_json",
"sha2 0.10.9",
"tempfile",
"tokio",
"tracing",
codex-rs/app-server/src/config_manager_service.rs
+33
View File
@@ -24,6 +24,7 @@ use codex_config::merge_toml_values;
use codex_config::shell_environment_filter_entry;
use codex_config::validate_shell_environment_policy_filter_config;
use codex_core::config::deserialize_config_toml_with_base;
use codex_core::config::edit::ConfigEdit;
use codex_core::config::edit::ConfigEditsBuilder;
use codex_core::config::validate_feature_requirements_for_config_toml;
use codex_core::path_utils;
@@ -360,6 +361,38 @@ impl ConfigManager {
}
config_edits.extend(credential_provider_edits.remapping_edits(&user_config)?);
// hooks/list returns an opaque declaration/content approval value. Split it
// on persistence so older clients still recognize the declaration approval.
if let Some(states) = user_config
.get_mut("hooks")
.and_then(|hooks| hooks.get_mut("state"))
.and_then(TomlValue::as_table_mut)
{
for (key, state) in states {
let Some(state) = state.as_table_mut() else {
continue;
};
let Some((declaration, content)) = state
.get("trusted_hash")
.and_then(TomlValue::as_str)
.and_then(|hash| hash.split_once('/'))
else {
continue;
};
let hashes = [
("trusted_hash", declaration.to_owned()),
("trusted_content_hash", content.to_owned()),
];
for (field, hash) in hashes {
state.insert(field.to_string(), TomlValue::String(hash.clone()));
config_edits.push(ConfigEdit::SetPath {
segments: vec!["hooks".into(), "state".into(), key.clone(), field.into()],
value: toml_edit::value(hash),
});
}
}
}
validate_config(&user_config).map_err(|err| {
ConfigManagerError::write(
ConfigWriteErrorCode::ConfigValidationError,
codex-rs/app-server/src/effective_plugin_change.rs
+5 -2
View File
@@ -119,14 +119,17 @@ pub(crate) async fn trust_materialized_plugin_hooks(
.plugins_manager()
.plugins_for_config(&config.plugins_config_input())
.await;
let hooks = codex_hooks::list_hooks(codex_hooks::HooksConfig {
let hooks_config = codex_hooks::HooksConfig {
feature_enabled: true,
bypass_hook_trust: config.bypass_hook_trust,
config_layer_stack: Some(config.config_layer_stack),
plugin_hook_sources: plugin_outcome.effective_plugin_hook_sources(),
plugin_hook_load_warnings: plugin_outcome.effective_plugin_hook_warnings(),
..Default::default()
});
};
let hooks = tokio::task::spawn_blocking(move || codex_hooks::list_hooks(hooks_config))
.await
.map_err(|err| format!("hook discovery task failed: {err}"))?;
if !hooks.warnings.is_empty() {
warn!(
warnings = ?hooks.warnings,
codex-rs/app-server/src/request_processors/catalog_processor.rs
+5 -2
View File
@@ -671,14 +671,17 @@ impl CatalogRequestProcessor {
} else {
codex_core_plugins::PluginHookLoadOutcome::default()
};
let hooks = codex_hooks::list_hooks(codex_hooks::HooksConfig {
let hooks_config = codex_hooks::HooksConfig {
feature_enabled: hooks_enabled,
bypass_hook_trust: config.bypass_hook_trust,
config_layer_stack: Some(config.config_layer_stack),
plugin_hook_sources: plugin_hooks.hook_sources,
plugin_hook_load_warnings: plugin_hooks.hook_load_warnings,
..Default::default()
});
};
let hooks = tokio::task::spawn_blocking(move || codex_hooks::list_hooks(hooks_config))
.await
.map_err(|err| internal_error(format!("hook discovery task failed: {err}")))?;
data.push(codex_app_server_protocol::HooksListEntry {
cwd,
hooks: hooks_to_info(&hooks.hooks),
codex-rs/app-server/tests/suite/v2/hooks_list.rs
+79 -37
View File
@@ -289,6 +289,7 @@ async fn hooks_list_shows_discovered_plugin_hook() -> Result<()> {
.path()
.join("plugins/cache/test/demo/local/hooks/hooks.json"),
)?)?;
let current_hash = data[0].hooks[0].current_hash.clone();
assert_eq!(
data,
vec![HooksListEntry {
@@ -310,15 +311,7 @@ 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",
/*timeout_sec*/ 7,
/*async*/ false,
Some("running plugin hook"),
/*additional_context_limit*/ None,
),
current_hash,
trust_status: HookTrustStatus::Untrusted,
}],
warnings: Vec::new(),
@@ -419,6 +412,11 @@ async fn plugin_upgrade_refreshes_hook_runtime_for_loaded_session() -> Result<()
let thread_start_id = mcp
.send_thread_start_request_with_auto_env(ThreadStartParams {
model: Some("mock-model".to_string()),
// Keep this refresh/disablement test independent of approval changes.
config: Some(std::collections::HashMap::from([(
"bypass_hook_trust".to_string(),
serde_json::json!(true),
)])),
..Default::default()
})
.await?;
@@ -559,7 +557,9 @@ async fn loaded_session_refreshes_externally_updated_plugin_hooks() -> Result<()
let server = create_mock_responses_server_sequence_unchecked(vec![
create_final_assistant_message_sse_response("Version 1")?,
create_final_assistant_message_sse_response("Version 2 needs approval")?,
create_final_assistant_message_sse_response("Version 2")?,
create_final_assistant_message_sse_response("Rollback needs approval")?,
create_final_assistant_message_sse_response("Rollback")?,
])
.await;
@@ -622,7 +622,13 @@ async fn loaded_session_refreshes_externally_updated_plugin_hooks() -> Result<()
timeout(DEFAULT_TIMEOUT, mcp.read_response(start_id)).await??;
let mut expected_versions = String::new();
for version in ["1.0.0", "2.0.0", "0.9.0"] {
for (version, expected_trust) in [
("1.0.0", HookTrustStatus::Trusted),
("2.0.0", HookTrustStatus::Modified),
("2.0.0", HookTrustStatus::Trusted),
("0.9.0", HookTrustStatus::Modified),
("0.9.0", HookTrustStatus::Trusted),
] {
// This test process updates the shared store without notifying app-server.
write_versioned_plugin_hook(source.path(), version, &hook_log_path)?;
store.install(AbsolutePathBuf::try_from(source.path())?, plugin_id.clone())?;
@@ -642,8 +648,66 @@ async fn loaded_session_refreshes_externally_updated_plugin_hooks() -> Result<()
mcp.read_stream_until_notification_message("turn/completed"),
)
.await??;
expected_versions.push_str(&format!("{version}\n"));
if expected_trust == HookTrustStatus::Trusted {
expected_versions.push_str(&format!("{version}\n"));
}
assert_eq!(std::fs::read_to_string(&hook_log_path)?, expected_versions);
let list_id = mcp
.send_hooks_list_request(HooksListParams {
cwds: vec![codex_home.path().to_path_buf()],
})
.await?;
let HooksListResponse { data } =
timeout(DEFAULT_TIMEOUT, mcp.read_response(list_id)).await??;
let hook = data[0]
.hooks
.iter()
.find(|hook| hook.event_name == HookEventName::UserPromptSubmit)
.expect("plugin should register a user-prompt hook");
assert_eq!(hook.trust_status, expected_trust);
if expected_trust == HookTrustStatus::Trusted {
let saved: toml::Value = toml::from_str(&std::fs::read_to_string(
codex_home.path().join("config.toml"),
)?)?;
// Older clients compare only the declaration hash; keep that approval valid.
let declaration_hash = command_hook_hash(
"user_prompt_submit",
/*matcher*/ None,
"python3 ${PLUGIN_ROOT}/hooks/log_version.py",
hook.timeout_sec,
/*async*/ false,
/*status_message*/ None,
/*additional_context_limit*/ None,
);
assert_eq!(
saved["hooks"]["state"][&hook.key]
.clone()
.try_into::<codex_config::HookStateToml>()?,
codex_config::HookStateToml {
enabled: None,
trusted_hash: Some(declaration_hash),
trusted_content_hash: Some(
hook.current_hash.split_once('/').unwrap().1.to_string()
),
},
);
}
if expected_trust == HookTrustStatus::Modified {
let trust_id = mcp
.send_config_batch_write_request(ConfigBatchWriteParams {
edits: vec![ConfigEdit {
key_path: format!("hooks.state.\"{}\".trusted_hash", hook.key),
value: serde_json::json!(hook.current_hash),
merge_strategy: MergeStrategy::Upsert,
}],
file_path: None,
expected_version: None,
reload_user_config: true,
})
.await?;
let _: codex_app_server_protocol::ConfigWriteResponse =
timeout(DEFAULT_TIMEOUT, mcp.read_response(trust_id)).await??;
}
}
Ok(())
}
@@ -861,22 +925,7 @@ async fn hooks_list_shows_discovered_plugin_mcp_tool_hook() -> Result<()> {
.path()
.join("plugins/cache/test/demo/local/hooks/hooks.json"),
)?)?;
let identity = NormalizedHookIdentity {
event_name: "pre_tool_use",
group: codex_config::MatcherGroup {
matcher: Some("Bash".to_string()),
hooks: vec![codex_config::HookHandlerConfig::McpTool {
server: "security".to_string(),
tool: "inspect".to_string(),
input: serde_json::from_value(serde_json::json!({
"path": "${tool_input.path}",
}))?,
timeout_sec: Some(9),
status_message: Some("checking security policy".to_string()),
}],
},
};
let identity = codex_config::TomlValue::try_from(identity)?;
let current_hash = data[0].hooks[0].current_hash.clone();
assert_eq!(
data,
@@ -899,7 +948,7 @@ async fn hooks_list_shows_discovered_plugin_mcp_tool_hook() -> Result<()> {
display_order: 0,
enabled: true,
is_managed: false,
current_hash: codex_config::version_for_toml(&identity),
current_hash,
trust_status: HookTrustStatus::Untrusted,
}],
warnings: Vec::new(),
@@ -969,6 +1018,7 @@ enabled = false
let source_path = AbsolutePathBuf::from_absolute_path(std::fs::canonicalize(
plugin_root.join("hooks/hooks.json"),
)?)?;
let current_hash = data[0].hooks[0].current_hash.clone();
assert_eq!(
data,
vec![HooksListEntry {
@@ -990,15 +1040,7 @@ enabled = false
display_order: 1,
enabled: false,
is_managed: false,
current_hash: command_hook_hash(
"stop",
/*matcher*/ None,
ordinary_command,
/*timeout_sec*/ 5,
/*async*/ false,
/*status_message*/ None,
/*additional_context_limit*/ None,
),
current_hash,
trust_status: HookTrustStatus::Untrusted,
}],
warnings: Vec::new(),
codex-rs/app-server/tests/suite/v2/plugin_list.rs
+9 -2
View File
@@ -3915,9 +3915,16 @@ trusted_hash = "sha256:unrelated"
)?)?;
let hook_states = config["hooks"]["state"].as_table().expect("hook states");
for hook in &hooks {
let (declaration_hash, content_hash) = hook
.current_hash
.split_once('/')
.expect("declaration and content hashes");
assert_eq!(
hook_states[hook.key.as_str()]["trusted_hash"].as_str(),
Some(hook.current_hash.as_str())
(
hook_states[hook.key.as_str()]["trusted_hash"].as_str(),
hook_states[hook.key.as_str()]["trusted_content_hash"].as_str(),
),
(Some(declaration_hash), Some(content_hash))
);
}
assert!(
codex-rs/config/src/hook_config.rs
+3
View File
@@ -30,6 +30,9 @@ pub struct HookStateToml {
pub enabled: Option<bool>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub trusted_hash: Option<String>,
/// Approved plugin contents; kept separate for declaration-only clients.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub trusted_content_hash: Option<String>,
}
#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
codex-rs/config/src/hooks_tests.rs
+1
View File
@@ -229,6 +229,7 @@ command = "python3 /tmp/pre.py"
super::HookStateToml {
enabled: Some(false),
trusted_hash: Some("sha256:abc123".to_string()),
trusted_content_hash: None,
},
)]),
}
codex-rs/core/config.schema.json
+4
View File
@@ -2262,6 +2262,10 @@
"enabled": {
"type": "boolean"
},
"trusted_content_hash": {
"description": "Approved plugin contents; kept separate for declaration-only clients.",
"type": "string"
},
"trusted_hash": {
"type": "string"
}
codex-rs/core/tests/suite/hooks.rs
+81 -54
View File
@@ -11,7 +11,6 @@ use codex_core::StartThreadOptions;
use codex_core::TurnInput;
use codex_core::TurnInputRequest;
use codex_core::TurnStartOptions;
use codex_core::config::Config;
use codex_core::config::Constrained;
use codex_core::config::ThreadStoreConfig;
use codex_features::Feature;
@@ -20,8 +19,6 @@ use codex_history::RolloutItem;
use codex_model_provider_info::ModelProviderInfo;
use codex_model_provider_info::built_in_model_providers;
use codex_models_manager::bundled_models_response;
use codex_plugin::PluginHookSource;
use codex_plugin::PluginId;
use codex_protocol::items::parse_hook_prompt_fragment;
use codex_protocol::mcp::ClientMcpExtensions;
use codex_protocol::models::ContentItem;
@@ -47,11 +44,9 @@ use codex_protocol::request_user_input::RequestUserInputAnswer;
use codex_protocol::request_user_input::RequestUserInputResponse;
use codex_protocol::user_input::UserInput;
use codex_thread_store::InMemoryThreadStore;
use codex_utils_absolute_path::AbsolutePathBuf;
use core_test_support::TestTargetOs;
use core_test_support::fs_wait;
use core_test_support::hooks::trust_discovered_hooks;
use core_test_support::hooks::trust_hooks;
use core_test_support::managed_network_requirements_loader;
use core_test_support::responses;
use core_test_support::responses::ev_apply_patch_custom_tool_call;
@@ -170,24 +165,6 @@ fn non_openai_model_provider(server: &wiremock::MockServer) -> ModelProviderInfo
provider
}
fn trust_plugin_hooks(config: &mut Config, plugin_hook_sources: Vec<PluginHookSource>) {
config
.features
.enable(Feature::CodexHooks)
.expect("test config should allow feature update");
let listed = codex_hooks::list_hooks(codex_hooks::HooksConfig {
feature_enabled: true,
config_layer_stack: Some(config.config_layer_stack.clone()),
plugin_hook_sources,
..codex_hooks::HooksConfig::default()
});
assert!(
!listed.hooks.is_empty(),
"trusted plugin hook fixture should discover at least one hook"
);
trust_hooks(config, listed.hooks);
}
fn write_stop_hook(home: &Path, block_prompts: &[&str]) -> Result<()> {
let script_path = home.join("stop_hook.py");
let log_path = home.join("stop_hook_log.jsonl");
@@ -4569,6 +4546,7 @@ async fn local_bundled_cleanup_hook_runs_without_saved_trust(
HookStateToml {
enabled: Some(false),
trusted_hash: None,
trusted_content_hash: None,
},
)]);
if !hooks_enabled {
@@ -4584,6 +4562,7 @@ async fn local_bundled_cleanup_hook_runs_without_saved_trust(
HookStateToml {
enabled: None,
trusted_hash: Some(hook.current_hash),
trusted_content_hash: None,
},
);
}
@@ -4651,8 +4630,11 @@ async fn plugin_pre_tool_use_blocks_exec_command_before_execution() -> Result<()
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let home = Arc::new(TempDir::new()?);
let call_id = "plugin-pretooluse-exec-command";
let marker = std::env::temp_dir().join("plugin-pretooluse-exec-command-marker");
let same_turn_call_id = "plugin-pretooluse-exec-command-same-turn";
let changed_call_id = "plugin-pretooluse-changed-script";
let marker = home.path().join("plugin-pretooluse-exec-command-marker");
let command = format!("printf blocked > {}", marker.display());
let args = serde_json::json!({ "cmd": command });
let responses = mount_sse_sequence(
@@ -4667,6 +4649,24 @@ async fn plugin_pre_tool_use_blocks_exec_command_before_execution() -> Result<()
),
ev_completed("resp-1"),
]),
sse(vec![
ev_response_created("resp-same-turn"),
ev_function_call(
same_turn_call_id,
"exec_command",
&serde_json::to_string(&args)?,
),
ev_completed("resp-same-turn"),
]),
sse(vec![
ev_response_created("resp-changed"),
ev_function_call(
changed_call_id,
"exec_command",
&serde_json::to_string(&args)?,
),
ev_completed("resp-changed"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-1", "plugin hook blocked it"),
@@ -4676,7 +4676,6 @@ async fn plugin_pre_tool_use_blocks_exec_command_before_execution() -> Result<()
)
.await;
let home = Arc::new(TempDir::new()?);
let plugin_root = home.path().join("plugins/cache/test/sample/local");
let hooks_dir = plugin_root.join("hooks");
fs::create_dir_all(plugin_root.join(".codex-plugin"))
@@ -4691,18 +4690,40 @@ async fn plugin_pre_tool_use_blocks_exec_command_before_execution() -> Result<()
home.path().join("config.toml"),
r#"[plugins."sample@test"]
enabled = true
# Declaration-only approvals from before content fingerprinting.
[hooks.state."sample@test:hooks/hooks.json:pre_tool_use:0:0"]
trusted_hash = "sha256:c9ee1aca1b5d8c72b99df6571c87d61350b09f7f4594ae30e7250a2027e9d76b"
[hooks.state."sample@test:hooks/hooks.json:post_tool_use:0:0"]
trusted_hash = "sha256:9eef43ad6f26b1f9b94f318a50445f8bb62c363da08152a2966b73d308bf42ba"
"#,
)
.context("write plugin config")?;
let script_path = hooks_dir.join("pre_tool_use_hook.py");
let log_path = hooks_dir.join("pre_tool_use_hook_log.jsonl");
let log_path = home.path().join("pre_tool_use_hook_log.jsonl");
fs::write(
hooks_dir.join("hook_support.py"),
"DENIAL_REASON = 'blocked by plugin hook'\n",
)
.context("write bundled hook module")?;
fs::write(
&script_path,
format!(
r#"import json
from pathlib import Path
import sys
sys.path.insert(0, str(Path(__file__).parent))
from hook_support import DENIAL_REASON
root = Path(__file__).parents[1]
for relative in (".venv/pyvenv.cfg", "sample.egg-info/PKG-INFO"):
artifact = root / relative
artifact.parent.mkdir(exist_ok=True)
artifact.write_text("generated")
python_link = root / ".venv/python"
if sys.platform != "win32" and not python_link.exists():
python_link.symlink_to(sys.executable)
payload = json.load(sys.stdin)
with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
@@ -4712,9 +4733,12 @@ print(json.dumps({{
"hookSpecificOutput": {{
"hookEventName": "PreToolUse",
"permissionDecision": "deny",
"permissionDecisionReason": "blocked by plugin hook"
"permissionDecisionReason": DENIAL_REASON
}}
}}))
if len(Path(r"{log_path}").read_text().splitlines()) == 2:
with Path(__file__).open("a", encoding="utf-8") as script:
script.write("\n# updated plugin bundle\n")
"#,
log_path = log_path.display(),
),
@@ -4726,30 +4750,20 @@ print(json.dumps({{
"matcher": "^Bash$",
"hooks": [{
"type": "command",
"command": "python3 ${PLUGIN_ROOT}/hooks/pre_tool_use_hook.py"
"command": "python3 -I -S ${PLUGIN_ROOT}/hooks/pre_tool_use_hook.py"
}]
}],
"PostToolUse": [{
"matcher": "^Bash$",
"hooks": [{
"type": "command",
"command": "python3 -I -S ${PLUGIN_ROOT}/hooks/pre_tool_use_hook.py"
}]
}]
}
}"#;
let plugin_hooks_path = hooks_dir.join("hooks.json");
fs::write(&plugin_hooks_path, plugin_hooks_json).context("write plugin hooks config")?;
let plugin_root_abs =
AbsolutePathBuf::try_from(plugin_root.clone()).context("absolute plugin root")?;
let plugin_hooks_path_abs =
AbsolutePathBuf::try_from(plugin_hooks_path).context("absolute plugin hooks path")?;
let plugin_data_root =
AbsolutePathBuf::try_from(plugin_root.join("data")).context("absolute plugin data root")?;
let plugin_hook_sources = vec![PluginHookSource {
plugin_id: PluginId::parse("sample@test").context("plugin id")?,
plugin_root: plugin_root_abs,
plugin_data_root,
source_path: plugin_hooks_path_abs,
source_relative_path: "hooks/hooks.json".to_string(),
hooks: serde_json::from_str::<codex_config::HooksFile>(plugin_hooks_json)
.context("parse plugin hooks")?
.hooks,
}];
let mut builder = test_codex()
.with_home(Arc::clone(&home))
.with_config(move |config| {
@@ -4757,13 +4771,11 @@ print(json.dumps({{
.features
.enable(Feature::Plugins)
.expect("test config should allow feature update");
trust_plugin_hooks(config, plugin_hook_sources);
config.features.enable(Feature::CodexHooks).unwrap();
});
let test = builder.build(&server).await?;
if marker.exists() {
fs::remove_file(&marker).context("remove leftover plugin pre tool use marker")?;
}
assert!(!home.path().join(".hook-trust").exists());
let legacy_config = fs::read_to_string(home.path().join("config.toml"))?;
test.submit_turn_with_policy(
"run the shell command blocked by a plugin hook",
@@ -4772,7 +4784,7 @@ print(json.dumps({{
.await?;
let requests = responses.requests();
assert_eq!(requests.len(), 2);
assert_eq!(requests.len(), 4);
let output_item = requests[1].function_call_output(call_id);
let output = output_item
.get("output")
@@ -4782,17 +4794,32 @@ print(json.dumps({{
output.contains("Command blocked by PreToolUse hook: blocked by plugin hook"),
"blocked tool output should surface the plugin hook reason",
);
assert!(marker.exists(), "the modified hook must not execute again");
assert!(
!marker.exists(),
"plugin hook should block shell command execution"
requests[2].function_call_output(same_turn_call_id)["output"]
.as_str()
.unwrap()
.contains("Command blocked by PreToolUse hook")
);
let hook_inputs = read_hook_inputs_from_log(&log_path)?;
assert_eq!(hook_inputs.len(), 1);
// A legacy hook first used after a bundle change must share the earlier baseline.
assert_eq!(hook_inputs.len(), 2);
assert_eq!(hook_inputs[0]["hook_event_name"], "PreToolUse");
assert_eq!(hook_inputs[0]["tool_name"], "Bash");
assert_eq!(hook_inputs[0]["tool_use_id"], call_id);
assert_eq!(hook_inputs[0]["tool_input"]["command"], command);
assert!(
hooks_dir.join("__pycache__").exists(),
"isolated Python should create bytecode without invalidating hook trust"
);
assert_eq!(hook_inputs[1]["tool_use_id"], same_turn_call_id);
// Migration keeps declaration-only approvals readable by older clients.
assert_eq!(
fs::read_to_string(home.path().join("config.toml"))?,
legacy_config
);
assert_eq!(fs::read_dir(home.path().join(".hook-trust"))?.count(), 1);
Ok(())
}
codex-rs/hooks/Cargo.toml
+2 -1
View File
@@ -28,6 +28,8 @@ regex = { workspace = true }
schemars = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
sha2 = { workspace = true }
tempfile = { workspace = true }
tokio = { workspace = true, features = ["fs", "io-util", "process", "sync", "time"] }
tracing = { workspace = true }
uuid = { workspace = true, features = ["v4"] }
@@ -35,7 +37,6 @@ uuid = { workspace = true, features = ["v4"] }
[dev-dependencies]
libc = { workspace = true }
pretty_assertions = { workspace = true }
tempfile = { workspace = true }
tokio = { workspace = true, features = ["macros", "rt-multi-thread", "time"] }
[target.'cfg(any(unix, windows))'.dependencies]
codex-rs/hooks/src/config_rules.rs
+10
View File
@@ -59,6 +59,9 @@ pub fn hook_states_from_stack(
if let Some(trusted_hash) = state.trusted_hash {
effective_state.trusted_hash = Some(trusted_hash);
}
if let Some(trusted_content_hash) = state.trusted_content_hash {
effective_state.trusted_content_hash = Some(trusted_content_hash);
}
}
}
@@ -104,6 +107,7 @@ mod tests {
HookStateToml {
enabled: Some(true),
trusted_hash: None,
trusted_content_hash: None,
},
)])
);
@@ -124,6 +128,7 @@ mod tests {
HookStateToml {
enabled: Some(/*enabled*/ false),
trusted_hash: None,
trusted_content_hash: None,
},
),
),
@@ -134,6 +139,7 @@ mod tests {
HookStateToml {
enabled: None,
trusted_hash: Some("sha256:trusted".to_string()),
trusted_content_hash: None,
},
),
),
@@ -150,6 +156,7 @@ mod tests {
HookStateToml {
enabled: Some(false),
trusted_hash: Some("sha256:trusted".to_string()),
trusted_content_hash: None,
},
)])
);
@@ -189,6 +196,7 @@ mod tests {
HookStateToml {
enabled: Some(false),
trusted_hash: None,
trusted_content_hash: None,
},
)])
);
@@ -230,6 +238,7 @@ mod tests {
HookStateToml {
enabled: Some(false),
trusted_hash: None,
trusted_content_hash: None,
},
)])
);
@@ -241,6 +250,7 @@ mod tests {
HookStateToml {
enabled,
trusted_hash: None,
trusted_content_hash: None,
},
)
}
codex-rs/hooks/src/engine/command_runner.rs
+28 -2
View File
@@ -216,6 +216,28 @@ pub(crate) async fn run_command(
) -> HandlerRunResult {
let started_at = chrono::Utc::now().timestamp();
let started = Instant::now();
let timeout_started = tokio::time::Instant::now();
let duration = Duration::from_secs(handler.timeout_sec);
if let Some(trust) = &handler.plugin_trust {
let error = match timeout(duration, trust.verify()).await {
Ok(Ok(())) => None,
Ok(Err(err)) => Some(err.to_string()),
Err(_) => Some(format!("hook timed out after {}s", handler.timeout_sec)),
};
if let Some(error) = error {
return finish_command_run(
started_at,
started,
CommandRunCompletion {
exit_code: None,
stdout: String::new(),
stderr: String::new(),
error: Some(error),
outcome: "trust_error",
},
);
}
}
let mut command = build_command(&runtime.shell, command_line, &runtime.environment, env);
command.current_dir(cwd);
@@ -268,10 +290,14 @@ pub(crate) async fn run_command(
.map_err(|err| ("wait_error", err.to_string()))
};
let timeout_duration = Duration::from_secs(handler.timeout_sec);
// Drain output while sending input so neither pipe can block the other, and
// include stdin writes in the deadline even when the hook never reads them.
match timeout(timeout_duration, try_join(write_stdin, wait_with_output)).await {
match timeout(
duration.saturating_sub(timeout_started.elapsed()),
try_join(write_stdin, wait_with_output),
)
.await
{
Ok(Ok(((), output))) => {
// Successfully completed hooks may intentionally leave detached helpers running.
#[cfg(windows)]
codex-rs/hooks/src/engine/command_runner_tests.rs
+5
View File
@@ -112,6 +112,7 @@ async fn hook_shell_startup_does_not_stop_on_controlling_terminal() {
)]);
let handler = ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name: HookEventName::SessionStart,
matcher: None,
timeout_sec: 2,
@@ -216,6 +217,7 @@ async fn cmd_shell_runs_quoted_hook_command_path() {
let env = HashMap::new();
let handler = ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name: HookEventName::SessionStart,
matcher: None,
timeout_sec: 10,
@@ -266,6 +268,7 @@ async fn fast_exiting_hook_preserves_stdout_when_stdin_is_not_consumed() {
let env = HashMap::new();
let handler = ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name: HookEventName::SessionStart,
matcher: None,
timeout_sec: 10,
@@ -354,6 +357,7 @@ async fn command_hook_does_not_expose_configured_noise_auth_token() {
]);
let handler = ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name: HookEventName::SessionStart,
matcher: None,
timeout_sec: 10,
@@ -492,6 +496,7 @@ fn write_handler(temp: &TempDir, source: &str) -> ConfiguredHandler {
std::fs::write(&script_path, source).expect("write async test hook");
ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name: HookEventName::UserPromptSubmit,
matcher: None,
timeout_sec: 10,
codex-rs/hooks/src/engine/discovery.rs
+118 -12
View File
@@ -28,6 +28,7 @@ use super::ConfiguredHandlerKind;
use super::HookListEntry;
use super::HookListEntryHandler;
use super::dispatcher::hook_event_name_label;
use super::plugin_trust::PluginHookTrust;
use crate::config_rules::hook_states_from_stack;
use crate::engine::HookMatcher;
use crate::events::common::matcher_pattern_for_event;
@@ -55,6 +56,9 @@ struct HookHandlerSource<'a> {
hook_states: &'a HashMap<String, HookStateToml>,
env: HashMap<String, String>,
plugin_id: Option<String>,
plugin_content_hash: Option<&'a std::io::Result<String>>,
config_layer_stack: Option<&'a ConfigLayerStack>,
plugin_root: Option<&'a AbsolutePathBuf>,
}
enum HookRequirement<'a> {
@@ -79,13 +83,21 @@ struct NormalizedHandler {
additional_context_limit: Option<usize>,
}
#[derive(Clone, Copy)]
struct HookDiscoveryPolicy {
allow_managed_hooks_only: bool,
bypass_hook_trust: bool,
#[derive(Clone, Copy, PartialEq, Eq)]
pub(crate) enum DiscoveryMode {
Runtime,
Listing,
}
impl HookDiscoveryPolicy {
#[derive(Clone, Copy)]
struct HookDiscoveryPolicy<'a> {
config_layer_stack: Option<&'a ConfigLayerStack>,
allow_managed_hooks_only: bool,
bypass_hook_trust: bool,
mode: DiscoveryMode,
}
impl HookDiscoveryPolicy<'_> {
fn allows(self, source: &HookHandlerSource<'_>) -> bool {
!self.allow_managed_hooks_only || source.is_managed
}
@@ -96,6 +108,7 @@ pub(crate) fn discover_handlers(
plugin_hook_sources: Vec<PluginHookSource>,
plugin_hook_load_warnings: Vec<String>,
bypass_hook_trust: bool,
mode: DiscoveryMode,
) -> DiscoveryResult {
let mut handlers = Vec::new();
let mut hook_entries = Vec::new();
@@ -105,6 +118,7 @@ pub(crate) fn discover_handlers(
let mut visited_json_hook_folders = HashSet::new();
let hook_states = hook_states_from_stack(config_layer_stack);
let policy = HookDiscoveryPolicy {
config_layer_stack,
allow_managed_hooks_only: config_layer_stack.is_some_and(|config_layer_stack| {
config_layer_stack
.requirements()
@@ -113,6 +127,7 @@ pub(crate) fn discover_handlers(
.is_some_and(|requirement| requirement.value)
}),
bypass_hook_trust,
mode,
};
if let Some(config_layer_stack) = config_layer_stack {
@@ -139,6 +154,9 @@ pub(crate) fn discover_handlers(
hook_states: &hook_states,
env: HashMap::new(),
plugin_id: None,
plugin_content_hash: None,
config_layer_stack: None,
plugin_root: None,
};
if !policy.allows(&policy_source) {
continue;
@@ -179,6 +197,9 @@ pub(crate) fn discover_handlers(
hook_states: &hook_states,
env: HashMap::new(),
plugin_id: None,
plugin_content_hash: None,
config_layer_stack: None,
plugin_root: None,
},
hook_events,
policy,
@@ -212,7 +233,7 @@ fn append_managed_requirement_handlers(
display_order: &mut i64,
config_layer_stack: &ConfigLayerStack,
hook_states: &HashMap<String, HookStateToml>,
policy: HookDiscoveryPolicy,
policy: HookDiscoveryPolicy<'_>,
) -> Vec<String> {
let Some(managed_hooks) = config_layer_stack.requirements().managed_hooks.as_ref() else {
return Vec::new();
@@ -234,6 +255,9 @@ fn append_managed_requirement_handlers(
hook_states,
env: HashMap::new(),
plugin_id: None,
plugin_content_hash: None,
config_layer_stack: None,
plugin_root: None,
},
managed_hooks.get().hooks.clone(),
policy,
@@ -248,8 +272,13 @@ fn append_plugin_hook_sources(
display_order: &mut i64,
plugin_hook_sources: Vec<PluginHookSource>,
hook_states: &HashMap<String, HookStateToml>,
policy: HookDiscoveryPolicy,
policy: HookDiscoveryPolicy<'_>,
) {
if policy.allow_managed_hooks_only {
return;
}
// Listing inspects current contents; runtime discovery defers hashing until execution.
let mut content_hashes = HashMap::new();
for source in plugin_hook_sources {
let PluginHookSource {
plugin_root,
@@ -259,6 +288,14 @@ fn append_plugin_hook_sources(
source_relative_path,
hooks,
} = source;
let plugin_id = plugin_id.as_key();
let content_hash = (policy.mode == DiscoveryMode::Listing).then(|| {
content_hashes
.entry(plugin_root.clone())
.or_insert_with(|| {
super::plugin_fingerprint::plugin_content_hash(&plugin_root, &|| false)
})
});
let mut env = HashMap::new();
let plugin_root_value = plugin_root.display().to_string();
let plugin_data_root_value = plugin_data_root.display().to_string();
@@ -268,7 +305,6 @@ fn append_plugin_hook_sources(
env.insert("PLUGIN_DATA".to_string(), plugin_data_root_value.clone());
// For OOTB compat with existing plugins that use this env var.
env.insert("CLAUDE_PLUGIN_DATA".to_string(), plugin_data_root_value);
let plugin_id = plugin_id.as_key();
append_hook_events(
handlers,
hook_entries,
@@ -287,6 +323,9 @@ fn append_plugin_hook_sources(
hook_states,
env,
plugin_id: Some(plugin_id),
plugin_content_hash: content_hash.as_deref(),
config_layer_stack: policy.config_layer_stack,
plugin_root: Some(&plugin_root),
},
hooks,
policy,
@@ -456,7 +495,7 @@ fn append_hook_events(
display_order: &mut i64,
mut source: HookHandlerSource<'_>,
hook_events: HookEventsToml,
policy: HookDiscoveryPolicy,
policy: HookDiscoveryPolicy<'_>,
) {
if !policy.allows(&source) {
return;
@@ -663,7 +702,6 @@ fn append_matcher_groups(
status_message,
additional_context_limit,
} = normalized;
let current_hash = hook_hash(event_name, matcher, &group, &config);
let key = crate::hook_key(&source.key_source, event_name, group_index, handler_index);
let state = source.hook_states.get(&key);
let builtin = source.plugin_id.as_deref().is_some_and(|plugin_id| {
@@ -675,10 +713,67 @@ fn append_matcher_groups(
/*app_connector_id*/ None,
)
});
let plugin_content_hash = match source.plugin_content_hash {
Some(Ok(hash)) => Some(hash.as_str()),
Some(Err(err)) if !builtin && !source.bypass_hook_trust => {
source.record_load_failure(
format!(
"skipping plugin hook in {}: failed to fingerprint plugin contents: {err}",
source.path.display()
),
warnings,
);
continue;
}
Some(Err(_)) | None => None,
};
let declaration_hash = hook_hash(event_name, matcher, &group, &config);
// The config writer persists these two approval hashes separately.
let current_hash = plugin_content_hash.map_or_else(
|| declaration_hash.clone(),
|hash| format!("{declaration_hash}/{hash}"),
);
let enabled = hook_enabled(source.is_managed, builtin, state);
let trusted_hash = hook_trusted_hash(source.is_managed, state);
let trust_status =
hook_trust_status(source.is_managed, builtin, &current_hash, trusted_hash);
let plugin_trust = if !builtin
&& (!source.bypass_hook_trust || source.plugin_content_hash.is_some())
&& let Some(root) = source.plugin_root
&& let Some(plugin_id) = source.plugin_id.as_deref()
&& let Some(trusted_hash) = trusted_hash
{
Some(PluginHookTrust {
root: root.clone(),
legacy_path: source
.config_layer_stack
.and_then(|stack| super::plugin_trust::legacy_trust_path(stack, plugin_id)),
declaration_hash,
trusted_hash: trusted_hash.to_owned(),
trusted_content_hash: state
.and_then(|state| state.trusted_content_hash.clone()),
})
} else {
None
};
let trust_status = if let Some(content_hash) = plugin_content_hash
&& let Some(trust) = plugin_trust.as_ref()
{
match trust.matches_hash(content_hash) {
Ok(true) => HookTrustStatus::Trusted,
Ok(false) => HookTrustStatus::Modified,
Err(err) => {
source.record_load_failure(
format!(
"skipping plugin hook in {}: failed to migrate hook trust: {err}",
source.path.display()
),
warnings,
);
continue;
}
}
} else {
hook_trust_status(source.is_managed, builtin, &current_hash, trusted_hash)
};
let handler = match &kind {
ConfiguredHandlerKind::Command {
command, r#async, ..
@@ -714,6 +809,7 @@ fn append_matcher_groups(
});
if enabled
&& (source.bypass_hook_trust
|| (source.plugin_content_hash.is_none() && plugin_trust.is_some())
|| matches!(
trust_status,
HookTrustStatus::Managed | HookTrustStatus::Trusted
@@ -721,6 +817,7 @@ fn append_matcher_groups(
{
handlers.push(ConfiguredHandler {
builtin,
plugin_trust,
event_name,
matcher: compiled_matcher.clone(),
timeout_sec,
@@ -913,6 +1010,9 @@ mod tests {
hook_states,
env: std::collections::HashMap::new(),
plugin_id: None,
plugin_content_hash: None,
config_layer_stack: None,
plugin_root: None,
}
}
@@ -931,6 +1031,9 @@ mod tests {
hook_states,
env: std::collections::HashMap::new(),
plugin_id: None,
plugin_content_hash: None,
config_layer_stack: None,
plugin_root: None,
}
}
@@ -1258,6 +1361,7 @@ mod tests {
handlers,
vec![ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name: HookEventName::UserPromptSubmit,
matcher: None,
timeout_sec: 600,
@@ -1298,6 +1402,7 @@ mod tests {
handlers,
vec![ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name: HookEventName::PreToolUse,
matcher: Some(HookMatcher::new("^Bash$").expect("valid matcher")),
timeout_sec: 600,
@@ -1517,6 +1622,7 @@ mod tests {
HookStateToml {
enabled: Some(false),
trusted_hash: None,
trusted_content_hash: None,
},
)]);
codex-rs/hooks/src/engine/dispatcher.rs
+1
View File
@@ -352,6 +352,7 @@ mod tests {
) -> ConfiguredHandler {
ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name,
matcher: matcher
.map(|pattern| crate::engine::HookMatcher::new(pattern).expect("valid matcher")),
codex-rs/hooks/src/engine/mcp_runner.rs
+12 -1
View File
@@ -37,7 +37,14 @@ pub(crate) async fn run_mcp_tool(
) -> HandlerRunResult {
let started_at = chrono::Utc::now().timestamp();
let started = Instant::now();
let timeout_started = tokio::time::Instant::now();
let duration = Duration::from_secs(handler.timeout_sec);
let result = async {
if let Some(trust) = &handler.plugin_trust {
tokio::time::timeout(duration, trust.verify())
.await
.context("hook trust verification timed out")??;
}
let hook_event: Value =
serde_json::from_str(hook_event_json).context("failed to parse hook event input")?;
let input = expand_mcp_argument_template(argument_template, &hook_event)?;
@@ -70,7 +77,11 @@ pub(crate) async fn run_mcp_tool(
environment_id,
metadata: call_metadata,
input,
timeout: Duration::from_secs(handler.timeout_sec),
timeout: if handler.plugin_trust.is_some() {
duration.saturating_sub(timeout_started.elapsed())
} else {
Duration::from_secs(handler.timeout_sec)
},
})
.await
}
codex-rs/hooks/src/engine/mcp_runner_tests.rs
+1
View File
@@ -96,6 +96,7 @@ async fn mcp_tool_results_use_command_hook_output_contract() {
.expect("object input");
let handler = ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name: HookEventName::PostToolUse,
matcher: None,
timeout_sec: 30,
codex-rs/hooks/src/engine/mod.rs
+5
View File
@@ -4,6 +4,8 @@ pub(crate) mod dispatcher;
mod matcher;
pub(crate) mod mcp_runner;
pub(crate) mod output_parser;
mod plugin_fingerprint;
mod plugin_trust;
pub(crate) mod schema_loader;
pub(crate) use matcher::HookMatcher;
@@ -62,6 +64,7 @@ pub(crate) struct CommandShell {
pub(crate) struct ConfiguredHandler {
/// Internally admitted cleanup hook, enabled independently of per-hook state.
pub builtin: bool,
pub plugin_trust: Option<plugin_trust::PluginHookTrust>,
pub event_name: codex_protocol::protocol::HookEventName,
pub matcher: Option<HookMatcher>,
pub timeout_sec: u64,
@@ -256,6 +259,7 @@ impl ClaudeHooksEngine {
plugin_hook_sources,
plugin_hook_load_warnings,
bypass_hook_trust,
discovery::DiscoveryMode::Runtime,
);
if !enabled {
discovered.handlers.retain(|handler| handler.builtin);
@@ -335,6 +339,7 @@ impl ClaudeHooksEngine {
}
handlers.push(ConfiguredHandler {
builtin: true,
plugin_trust: None,
event_name,
matcher: None,
timeout_sec: timeout_sec.unwrap_or(5).max(1),
codex-rs/hooks/src/engine/mod_tests.rs
+34 -2
View File
@@ -45,6 +45,7 @@ use super::ConfiguredHandler;
use super::ConfiguredHandlerKind;
use super::HandlerSourcePath;
use super::HookListEntryHandler;
use super::discovery::DiscoveryMode;
use crate::events::interrupt::InterruptRequest;
use crate::events::pre_tool_use::PreToolUseRequest;
use crate::events::stop::StopHookTarget;
@@ -92,6 +93,7 @@ fn permission_request_timeout_only_counts_synchronous_handlers() {
let command = "echo synchronous permission hook";
let synchronous_handler = ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name: HookEventName::PermissionRequest,
matcher: None,
timeout_sec: 5,
@@ -815,6 +817,7 @@ fn unknown_requirement_source_hooks_stay_managed() {
Vec::new(),
Vec::new(),
/*bypass_hook_trust*/ false,
DiscoveryMode::Listing,
);
assert_eq!(discovered.hook_entries.len(), 1);
assert_eq!(discovered.hook_entries[0].source, HookSource::Unknown);
@@ -902,6 +905,7 @@ fn user_disablement_filters_non_managed_hooks_but_not_managed_hooks() {
Vec::new(),
Vec::new(),
/*bypass_hook_trust*/ false,
DiscoveryMode::Listing,
);
assert_eq!(discovered.hook_entries.len(), 2);
assert_eq!(discovered.hook_entries[0].key, managed_disabled_key);
@@ -969,6 +973,7 @@ fn user_disablement_does_not_filter_managed_layer_hooks() {
Vec::new(),
Vec::new(),
/*bypass_hook_trust*/ false,
DiscoveryMode::Listing,
);
assert_eq!(discovered.hook_entries.len(), 1);
assert_eq!(discovered.hook_entries[0].key, managed_key);
@@ -1038,15 +1043,18 @@ fn trusted_plugin_hook_stack(
plugin_hook_sources.to_vec(),
Vec::new(),
/*bypass_hook_trust*/ false,
DiscoveryMode::Listing,
);
let state = discovered
.hook_entries
.into_iter()
.map(|entry| {
let (declaration, content) = entry.current_hash.split_once('/').unwrap();
(
entry.key,
serde_json::json!({
"trusted_hash": entry.current_hash,
"trusted_hash": declaration,
"trusted_content_hash": content,
}),
)
})
@@ -1195,6 +1203,7 @@ fn allow_managed_hooks_only_false_keeps_unmanaged_hooks() {
Vec::new(),
Vec::new(),
/*bypass_hook_trust*/ false,
DiscoveryMode::Listing,
);
assert_eq!(discovered.hook_entries.len(), 1);
assert!(!discovered.hook_entries[0].is_managed);
@@ -1253,6 +1262,7 @@ fn allow_managed_hooks_only_in_config_toml_does_not_enable_policy() {
Vec::new(),
Vec::new(),
/*bypass_hook_trust*/ false,
DiscoveryMode::Listing,
);
assert_eq!(discovered.hook_entries.len(), 1);
assert!(!discovered.hook_entries[0].is_managed);
@@ -1456,6 +1466,7 @@ fn allow_managed_hooks_only_keeps_managed_requirement_and_config_layer_hooks() {
Vec::new(),
Vec::new(),
/*bypass_hook_trust*/ false,
DiscoveryMode::Listing,
);
assert!(discovered.hook_entries.iter().all(|entry| entry.is_managed));
}
@@ -1765,6 +1776,7 @@ fn bundled_cleanup_hooks_are_trusted_without_saved_hashes() {
vec![bundled_cleanup_source(plugin_id, server, event)],
Vec::new(),
/*bypass_hook_trust*/ false,
DiscoveryMode::Listing,
);
assert_eq!(discovered.handlers.len(), 1, "{plugin_id} {event}");
assert_eq!(
@@ -1789,7 +1801,9 @@ fn bundled_cleanup_hooks_are_trusted_without_saved_hashes() {
#[test]
fn bundled_cleanup_trust_does_not_extend_to_other_handlers() {
let temp = tempdir().expect("create plugin directory");
let mut source = bundled_cleanup_source("browser@openai-bundled", "node_repl", "Stop");
source.plugin_root = AbsolutePathBuf::try_from(temp.path()).expect("plugin root");
let cleanup = source.hooks.stop[0].hooks[0].clone();
source.hooks.stop[0].hooks.extend([
HookHandlerConfig::McpTool {
@@ -1824,6 +1838,7 @@ fn bundled_cleanup_trust_does_not_extend_to_other_handlers() {
vec![source],
Vec::new(),
/*bypass_hook_trust*/ false,
DiscoveryMode::Listing,
);
assert_eq!(discovered.handlers.len(), 1);
assert_eq!(
@@ -1844,6 +1859,7 @@ fn bundled_cleanup_trust_does_not_extend_to_other_handlers() {
#[test]
fn bundled_cleanup_trust_requires_matching_plugin_server_and_event() {
let temp = tempdir().expect("create plugin directory");
for (plugin_id, server, event) in [
("other@openai-bundled", "node_repl", "Stop"),
("browser@other", "node_repl", "Stop"),
@@ -1858,11 +1874,14 @@ fn bundled_cleanup_trust_requires_matching_plugin_server_and_event() {
"PreToolUse",
),
] {
let mut source = bundled_cleanup_source(plugin_id, server, event);
source.plugin_root = AbsolutePathBuf::try_from(temp.path()).expect("plugin root");
let discovered = super::discovery::discover_handlers(
/*config_layer_stack*/ None,
vec![bundled_cleanup_source(plugin_id, server, event)],
vec![source],
Vec::new(),
/*bypass_hook_trust*/ false,
DiscoveryMode::Listing,
);
assert!(discovered.handlers.is_empty(), "{plugin_id} {event}");
assert_eq!(discovered.hook_entries.len(), 1);
@@ -1875,7 +1894,9 @@ fn bundled_cleanup_trust_requires_matching_plugin_server_and_event() {
#[test]
fn local_hosted_app_cleanup_hooks_require_saved_trust() {
let temp = tempdir().expect("create plugin directory");
let mut source = bundled_cleanup_source("browser@openai-curated-remote", "codex_apps", "Stop");
source.plugin_root = AbsolutePathBuf::try_from(temp.path()).expect("plugin root");
source.hooks.stop[0].hooks[0] = HookHandlerConfig::McpTool {
server: "codex_apps".to_string(),
tool: "browser.turn_ended".to_string(),
@@ -1888,6 +1909,7 @@ fn local_hosted_app_cleanup_hooks_require_saved_trust() {
vec![source],
Vec::new(),
/*bypass_hook_trust*/ false,
DiscoveryMode::Listing,
);
assert!(discovered.handlers.is_empty());
assert_eq!(
@@ -1908,6 +1930,7 @@ fn builtin_cleanup_ignores_disablement_but_preserves_managed_only_policy() {
vec![source.clone()],
Vec::new(),
/*bypass_hook_trust*/ false,
DiscoveryMode::Listing,
);
let expected_entry = discovered.hook_entries[0].clone();
let disabled_stack = ConfigLayerStack::new(
@@ -1927,6 +1950,7 @@ fn builtin_cleanup_ignores_disablement_but_preserves_managed_only_policy() {
vec![source.clone()],
Vec::new(),
/*bypass_hook_trust*/ false,
DiscoveryMode::Listing,
);
assert_eq!(disabled.handlers, discovered.handlers);
assert_eq!(disabled.hook_entries, vec![expected_entry]);
@@ -1941,6 +1965,7 @@ fn builtin_cleanup_ignores_disablement_but_preserves_managed_only_policy() {
vec![source.clone()],
Vec::new(),
/*bypass_hook_trust*/ false,
DiscoveryMode::Listing,
);
assert!(managed_only.handlers.is_empty());
assert!(managed_only.hook_entries.is_empty());
@@ -1967,7 +1992,9 @@ fn builtin_cleanup_ignores_disablement_but_preserves_managed_only_policy() {
#[test]
fn disabled_hooks_feature_keeps_builtin_cleanup_but_not_trusted_plugin_hooks() {
let temp = tempdir().expect("create plugin directory");
let mut source = bundled_cleanup_source("browser@openai-bundled", "node_repl", "Stop");
source.plugin_root = AbsolutePathBuf::try_from(temp.path()).expect("plugin root");
source.hooks.stop[0].hooks.push(HookHandlerConfig::Command {
command: "echo ordinary hook".to_string(),
command_windows: None,
@@ -1982,6 +2009,7 @@ fn disabled_hooks_feature_keeps_builtin_cleanup_but_not_trusted_plugin_hooks() {
vec![source.clone()],
Vec::new(),
/*bypass_hook_trust*/ false,
DiscoveryMode::Runtime,
);
assert_eq!(discovered.handlers.len(), 2);
@@ -2144,6 +2172,7 @@ fn plugin_hook_sources_expand_plugin_placeholders() {
AbsolutePathBuf::try_from(temp.path().join("demo-plugin")).expect("plugin root");
let plugin_data_root =
AbsolutePathBuf::try_from(temp.path().join("plugin-data")).expect("plugin data root");
fs::create_dir_all(&plugin_root).expect("create plugin directory");
let source_path = plugin_root.join("hooks/hooks.json");
let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id");
let plugin_hook_sources = vec![PluginHookSource {
@@ -2320,6 +2349,7 @@ fn executor_stop_hook_fixture() -> (
engine.handlers.as_slice(),
&[ConfiguredHandler {
builtin: true,
plugin_trust: None,
event_name: HookEventName::Stop,
matcher: None,
timeout_sec: 5,
@@ -2409,6 +2439,7 @@ async fn executor_stop_hooks_run_unless_regular_hooks_block_without_stopping() {
Arc::make_mut(&mut engine.handlers).push(ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name: HookEventName::Stop,
matcher: None,
timeout_sec: 30,
@@ -2444,6 +2475,7 @@ async fn executor_stop_hooks_run_unless_regular_hooks_block_without_stopping() {
Arc::make_mut(&mut engine.handlers).push(ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name: HookEventName::Stop,
matcher: None,
timeout_sec: 30,
codex-rs/hooks/src/engine/plugin_fingerprint.rs
+91
View File
@@ -0,0 +1,91 @@
//! Best-effort source identity for installed plugins used by hook trust.
//!
//! Hash bundle files, excluding generated Python bytecode, virtual environments,
//! and package metadata. This detects source changes, not tampering inside those
//! runtime artifacts. Cancellation is checked between entries and bounded reads.
use std::fs;
use std::io;
use std::io::Read;
use codex_utils_absolute_path::AbsolutePathBuf;
use sha2::Digest;
use sha2::Sha256;
pub(super) fn plugin_content_hash(
root: &AbsolutePathBuf,
cancelled: &dyn Fn() -> bool,
) -> io::Result<String> {
let check_cancelled = || {
if cancelled() {
Err(io::Error::from(io::ErrorKind::Interrupted))
} else {
Ok(())
}
};
check_cancelled()?;
if !fs::symlink_metadata(root)?.is_dir() {
return Err(io::Error::other("plugin root is not a directory"));
}
let mut pending = vec![root.to_path_buf()];
let mut hash = Sha256::new();
hash.update(b"codex-plugin-bundle-v1");
let mut buffer = [0_u8; 64 * 1024];
while let Some(path) = pending.pop() {
check_cancelled()?;
let relative_path = path.strip_prefix(root).map_err(io::Error::other)?;
if relative_path
.file_name()
.and_then(std::ffi::OsStr::to_str)
.is_some_and(|name| {
matches!(name, "__pycache__" | ".venv")
|| name.ends_with(".pyc")
|| name.ends_with(".egg-info")
})
{
continue;
}
let name = relative_path.as_os_str().as_encoded_bytes();
hash.update((name.len() as u64).to_le_bytes());
hash.update(name);
let metadata = fs::symlink_metadata(&path)?;
if metadata.is_dir() {
hash.update(b"directory");
let mut entries = Vec::new();
for entry in fs::read_dir(&path)? {
check_cancelled()?;
entries.push(entry?);
}
entries.sort_by_key(fs::DirEntry::file_name);
pending.extend(entries.iter().rev().map(fs::DirEntry::path));
} else if metadata.is_file() {
hash.update(b"file");
let length = metadata.len();
hash.update(length.to_le_bytes());
// Bound each read to the observed size, detecting growth or truncation
// instead of trusting a partial fingerprint of a changing file.
check_cancelled()?;
let mut file = fs::File::open(&path)?.take(length.saturating_add(1));
let mut bytes_read = 0_u64;
loop {
check_cancelled()?;
let count = file.read(&mut buffer)?;
if count == 0 {
break;
}
bytes_read += count as u64;
hash.update(&buffer[..count]);
}
if bytes_read != length {
return Err(io::Error::other("plugin file changed while fingerprinting"));
}
} else {
return Err(io::Error::other(format!(
"unsupported plugin entry type: {}",
path.display()
)));
}
}
check_cancelled()?;
Ok(format!("sha256:{:x}", hash.finalize()))
}
codex-rs/hooks/src/engine/plugin_trust.rs
+115
View File
@@ -0,0 +1,115 @@
//! Verifies plugin contents at execution, including one-time legacy approval migration.
//!
//! Runtime discovery stores declarations and approvals without scanning bundles. Each
//! execution verifies fresh contents; dropping its future cancels the scan. Legacy
//! approvals share one immutable bundle baseline per plugin and config home, across
//! hooks, profiles, versions, and sessions.
use codex_utils_absolute_path::AbsolutePathBuf;
use std::fs;
use std::io;
use std::io::Write;
use codex_config::ConfigLayerSource;
use codex_config::ConfigLayerStack;
use sha2::Digest;
use sha2::Sha256;
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct PluginHookTrust {
pub(super) root: AbsolutePathBuf,
pub(super) declaration_hash: String,
pub(super) trusted_hash: String,
pub(super) trusted_content_hash: Option<String>,
pub(super) legacy_path: Option<AbsolutePathBuf>,
}
impl PluginHookTrust {
pub(super) async fn verify(&self) -> io::Result<()> {
let trust = self.clone();
// Receiver lifetime propagates cancellation even to a queued blocking job.
let (cancelled, _cancel_on_drop) = tokio::sync::oneshot::channel::<()>();
tokio::task::spawn_blocking(move || {
let content_hash =
super::plugin_fingerprint::plugin_content_hash(&trust.root, &|| {
cancelled.is_closed()
})?;
if cancelled.is_closed() {
return Err(io::ErrorKind::Interrupted.into());
}
if !trust.matches_hash(&content_hash)? {
return Err(io::Error::other(
"plugin contents changed; hook approval required",
));
}
Ok(())
})
.await
.map_err(io::Error::other)?
}
pub(super) fn matches_hash(&self, content_hash: &str) -> io::Result<bool> {
if self.trusted_hash != self.declaration_hash {
return Ok(false);
}
if let Some(expected) = &self.trusted_content_hash {
if expected != content_hash {
return Ok(false);
}
if let Some(path) = &self.legacy_path {
// A modern approval also pins the baseline for legacy siblings.
migrate_legacy_trust(path, content_hash)?;
}
return Ok(true);
}
if let Some(path) = &self.legacy_path {
return Ok(migrate_legacy_trust(path, content_hash)? == content_hash);
}
Ok(false)
}
}
pub(super) fn legacy_trust_path(
stack: &ConfigLayerStack,
plugin_id: &str,
) -> Option<AbsolutePathBuf> {
let config_file = stack.all_layers_low_to_high().find_map(|layer| {
if let ConfigLayerSource::User { file, .. } = &layer.name {
Some(file)
} else {
None
}
});
let config_file = config_file?;
let parent = config_file.parent()?;
let mut identity = Sha256::new();
for part in [
config_file.as_os_str().as_encoded_bytes(),
plugin_id.as_bytes(),
] {
identity.update((part.len() as u64).to_le_bytes());
identity.update(part);
}
let directory = parent.join(".hook-trust");
Some(directory.join(format!("{:x}", identity.finalize())))
}
fn migrate_legacy_trust(path: &AbsolutePathBuf, current_hash: &str) -> io::Result<String> {
match fs::read_to_string(path) {
Ok(baseline) => return Ok(baseline),
Err(err) if err.kind() == io::ErrorKind::NotFound => {}
Err(err) => return Err(err),
}
let directory = path
.parent()
.ok_or_else(|| io::Error::other("hook trust baseline has no parent"))?;
fs::create_dir_all(directory.as_path())?;
let mut temporary = tempfile::NamedTempFile::new_in(directory)?;
temporary.write_all(current_hash.as_bytes())?;
temporary.as_file().sync_all()?;
match temporary.persist_noclobber(path) {
Ok(_) => Ok(current_hash.to_owned()),
Err(err) if err.error.kind() == io::ErrorKind::AlreadyExists => fs::read_to_string(path),
Err(err) => Err(err.error),
}
}
codex-rs/hooks/src/events/compact.rs
+1
View File
@@ -524,6 +524,7 @@ mod tests {
fn handler(event_name: HookEventName) -> ConfiguredHandler {
ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name,
matcher: None,
timeout_sec: 5,
codex-rs/hooks/src/events/interrupt_tests.rs
+1
View File
@@ -99,6 +99,7 @@ fn error(text: &str) -> HookOutputEntry {
fn handler() -> ConfiguredHandler {
ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name: HookEventName::Interrupt,
matcher: None,
timeout_sec: 600,
codex-rs/hooks/src/events/post_tool_use.rs
+1
View File
@@ -593,6 +593,7 @@ mod tests {
fn handler_with_async(r#async: bool) -> ConfiguredHandler {
ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name: HookEventName::PostToolUse,
matcher: Some(crate::engine::HookMatcher::new("^Bash$").expect("valid matcher")),
timeout_sec: 5,
codex-rs/hooks/src/events/pre_tool_use.rs
+1
View File
@@ -775,6 +775,7 @@ mod tests {
fn handler_with_async(r#async: bool) -> ConfiguredHandler {
ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name: HookEventName::PreToolUse,
matcher: Some(crate::engine::HookMatcher::new("^Bash$").expect("valid matcher")),
timeout_sec: 5,
codex-rs/hooks/src/events/session_end_tests.rs
+1
View File
@@ -62,6 +62,7 @@ fn session_end_ignores_successful_output() {
fn handler(matcher: Option<&str>) -> ConfiguredHandler {
ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name: HookEventName::SessionEnd,
matcher: matcher
.map(|pattern| crate::engine::HookMatcher::new(pattern).expect("valid matcher")),
codex-rs/hooks/src/events/session_start.rs
+1
View File
@@ -545,6 +545,7 @@ mod tests {
fn handler_for(event_name: HookEventName) -> ConfiguredHandler {
ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name,
matcher: None,
timeout_sec: 600,
codex-rs/hooks/src/events/stop.rs
+1
View File
@@ -696,6 +696,7 @@ mod tests {
fn handler_with_async(r#async: bool) -> ConfiguredHandler {
ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name: HookEventName::Stop,
matcher: None,
timeout_sec: 600,
codex-rs/hooks/src/events/user_prompt_submit.rs
+1
View File
@@ -464,6 +464,7 @@ mod tests {
fn handler_with_async(r#async: bool) -> ConfiguredHandler {
ConfiguredHandler {
builtin: false,
plugin_trust: None,
event_name: HookEventName::UserPromptSubmit,
matcher: None,
timeout_sec: 5,
codex-rs/hooks/src/registry.rs
+1
View File
@@ -311,6 +311,7 @@ pub fn list_hooks(config: HooksConfig) -> HookListOutcome {
config.plugin_hook_sources,
config.plugin_hook_load_warnings,
config.bypass_hook_trust,
crate::engine::discovery::DiscoveryMode::Listing,
);
HookListOutcome {
hooks: discovered.hook_entries,