From 731d28ee224bafd766c8fa8628a4a6b1737d1706 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Tue, 4 Aug 2026 11:33:56 -0400 Subject: [PATCH 1/3] fix: fail closed generated enforcement hooks Generate explicit per-event forwarding policies so permission-bearing pre-operation hooks reject delivery failures while observational hooks remain fail open. Preserve legacy installation migration and update packaged hooks, trust handling, tests, and documentation. Signed-off-by: Will Killian --- crates/cli/src/agents/claude/launch.rs | 6 +- crates/cli/src/agents/codex/host.rs | 73 +++++--- crates/cli/src/agents/codex/launch.rs | 6 +- crates/cli/src/agents/hermes/config.rs | 45 ++--- crates/cli/src/agents/hermes/integration.rs | 84 ++++++--- crates/cli/src/agents/hermes/trust.rs | 24 ++- crates/cli/src/agents/mod.rs | 4 +- crates/cli/src/commands/hook_forward.rs | 13 +- crates/cli/src/hooks/delivery.rs | 3 +- crates/cli/src/hooks/encoding.rs | 128 ++++++++++++-- crates/cli/src/hooks/mod.rs | 11 +- crates/cli/src/hooks/types.rs | 19 +- crates/cli/tests/cli_tests.rs | 70 +++++++- .../cli/tests/coverage/agents/hermes_tests.rs | 165 +++++++++++------- .../coverage/agents/plugin_host_tests.rs | 62 ++++--- .../coverage/agents/plugin_install_tests.rs | 9 +- .../tests/coverage/shared/installer_tests.rs | 108 ++++++++---- docs/about-nemo-relay/release-notes/index.mdx | 4 + docs/nemo-relay-cli/basic-usage.mdx | 19 +- integrations/coding-agents/README.md | 20 ++- .../claude-code/hooks/hooks.json | 28 +-- .../coding-agents/codex/hooks/hooks.json | 20 +-- 22 files changed, 642 insertions(+), 279 deletions(-) diff --git a/crates/cli/src/agents/claude/launch.rs b/crates/cli/src/agents/claude/launch.rs index 4147c7ccb..89ab79e32 100644 --- a/crates/cli/src/agents/claude/launch.rs +++ b/crates/cli/src/agents/claude/launch.rs @@ -7,7 +7,7 @@ use serde_json::{Value, json}; use crate::agents::CodingAgent; use crate::error::CliError; -use crate::hooks::{generated_hooks, transparent_hook_forward_command}; +use crate::hooks::{generated_policy_hooks, transparent_hook_forward_commands}; use crate::process::{PreparedAgentLaunch, insert_after_host}; pub(crate) fn prepare( @@ -61,7 +61,7 @@ pub(crate) fn prepare( })) .map_err(|error| CliError::Launch(error.to_string()))?, )?; - let hook_command = transparent_hook_forward_command( + let hook_commands = transparent_hook_forward_commands( &transparent_hook_executable(), CodingAgent::ClaudeCode, gateway_url, @@ -69,7 +69,7 @@ pub(crate) fn prepare( .map_err(CliError::Launch)?; write_hooks( &root.join("hooks/hooks.json"), - generated_hooks(CodingAgent::ClaudeCode, &hook_command), + generated_policy_hooks(CodingAgent::ClaudeCode, &hook_commands), )?; let settings_path = root.join("settings.json"); let settings = settings_overlay(&launch.argv, launch.host_index, gateway_url)?; diff --git a/crates/cli/src/agents/codex/host.rs b/crates/cli/src/agents/codex/host.rs index 185121ffb..7871ac2c1 100644 --- a/crates/cli/src/agents/codex/host.rs +++ b/crates/cli/src/agents/codex/host.rs @@ -14,6 +14,7 @@ use toml_edit::{DocumentMut, InlineTable, Item, Table, Value as TomlValue, value use crate::agents::CodingAgent; use crate::configuration::{BOOTSTRAP_CLIENT_TOKEN_HEADER, BootstrapChallengeKey, RELAY_PLUGIN_ID}; +#[cfg(test)] use crate::hooks::generated_hooks; #[cfg(test)] use crate::hooks::merge_hooks; @@ -109,11 +110,11 @@ pub(crate) fn install_codex_with_generation( pub(crate) fn install_codex_with_trust( gateway_url: &str, - expected_command: &str, + expected_commands: &crate::hooks::GeneratedHookCommands, trust_hooks: F, ) -> Result where - F: FnOnce(&Path, &Path, &str) -> Result<(), String>, + F: FnOnce(&Path, &Path, &crate::hooks::GeneratedHookCommands) -> Result<(), String>, { let home = home_dir()?; let codex_dir = codex_home_dir()?; @@ -125,7 +126,7 @@ where let snapshots = codex_install_snapshots(&config_path, &hooks_path)?; let install_result = remove_legacy_codex_hooks(&hooks_path) .and_then(|()| install_codex_config(&config_path, gateway_url)) - .and_then(|()| trust_hooks(&home, &config_path, expected_command)); + .and_then(|()| trust_hooks(&home, &config_path, expected_commands)); if let Err(error) = install_result { return match restore_codex_install_snapshots(&snapshots) { Ok(()) => Err(error), @@ -257,9 +258,9 @@ pub(crate) fn codex_hook_trust_report_with_generation( pub(crate) fn codex_hook_trust_report_with_client( client: &mut dyn CodexHooksClient, cwd: &Path, - expected_command: &str, + expected_commands: &crate::hooks::GeneratedHookCommands, ) -> Result { - let hooks = relay_codex_hooks(client, cwd, expected_command)?; + let hooks = relay_codex_hooks(client, cwd, expected_commands)?; Ok(codex_hook_trust_report_for(&hooks)) } @@ -267,9 +268,9 @@ pub(crate) fn auto_trust_codex_hooks( client: &mut dyn CodexHooksClient, cwd: &Path, config_path: &Path, - expected_command: &str, + expected_commands: &crate::hooks::GeneratedHookCommands, ) -> Result<(), String> { - let hooks = relay_codex_hooks(client, cwd, expected_command)?; + let hooks = relay_codex_hooks(client, cwd, expected_commands)?; let before = codex_hook_trust_report_for(&hooks); if !before.missing_required.is_empty() || !before.duplicate_required.is_empty() { return Err(format!( @@ -280,7 +281,7 @@ pub(crate) fn auto_trust_codex_hooks( } let state = snapshot_hook_trust_state(config_path, &hooks)?; let trust_result = client.trust_hooks(&hooks).and_then(|()| { - let verified_hooks = relay_codex_hooks(client, cwd, expected_command)?; + let verified_hooks = relay_codex_hooks(client, cwd, expected_commands)?; let verified = codex_hook_trust_report_for(&verified_hooks); let unverified_targets = hooks .iter() @@ -308,7 +309,7 @@ pub(crate) fn auto_trust_codex_hooks( return restore_hook_trust_after_failure( client, cwd, - expected_command, + expected_commands, &hooks, &state, error, @@ -320,21 +321,24 @@ pub(crate) fn auto_trust_codex_hooks( fn relay_codex_hooks( client: &mut dyn CodexHooksClient, cwd: &Path, - expected_command: &str, + expected_commands: &crate::hooks::GeneratedHookCommands, ) -> Result, String> { let hooks = relay_codex_plugin_hooks(client, cwd)? .into_iter() - .filter(|hook| hook.command.as_deref() == Some(expected_command)) + .filter(|hook| { + hook.command.as_deref() + == expected_codex_hook_command(expected_commands, &hook.event_name) + }) .collect::>(); - validate_loaded_hook_sources(&hooks, expected_command)?; + validate_loaded_hook_sources(&hooks, expected_commands)?; Ok(hooks) } fn validate_loaded_hook_sources( hooks: &[CodexHookMetadata], - expected_command: &str, + expected_commands: &crate::hooks::GeneratedHookCommands, ) -> Result<(), String> { - let expected = generated_hooks(CodingAgent::Codex, expected_command); + let expected = crate::hooks::generated_policy_hooks(CodingAgent::Codex, expected_commands); let sources = hooks .iter() .map(|hook| hook.source_path.as_str()) @@ -538,7 +542,7 @@ fn verify_restored_hook_trust( fn restore_hook_trust_after_failure( client: &mut dyn CodexHooksClient, cwd: &Path, - expected_command: &str, + expected_commands: &crate::hooks::GeneratedHookCommands, before: &[CodexHookMetadata], state: &[(String, Option)], original_error: String, @@ -548,7 +552,7 @@ fn restore_hook_trust_after_failure( "{original_error}; additionally failed to restore Codex hook trust: {rollback_error}" )); } - let restored = relay_codex_hooks(client, cwd, expected_command).map_err(|rollback_error| { + let restored = relay_codex_hooks(client, cwd, expected_commands).map_err(|rollback_error| { format!( "{original_error}; additionally failed to verify restored Codex hook trust: {rollback_error}" ) @@ -569,14 +573,16 @@ fn restore_hook_trust_after_failure( } #[cfg(test)] -pub(crate) fn expected_plugin_hook_command(plugin_hooks_path: &Path) -> Result { +pub(crate) fn expected_plugin_hook_command( + plugin_hooks_path: &Path, +) -> Result { expected_plugin_hook_command_with_token(plugin_hooks_path, None) } fn expected_plugin_hook_command_with_token( plugin_hooks_path: &Path, generation_token: Option<&str>, -) -> Result { +) -> Result { let relay = current_exe()?; let relay = relay.canonicalize().unwrap_or(relay); let relay = portable_executable_path(relay); @@ -614,9 +620,12 @@ fn plugin_generation_file(plugin_hooks_path: &Path) -> Result { } } -fn validate_plugin_hooks(path: &Path, expected_command: &str) -> Result<(), String> { +fn validate_plugin_hooks( + path: &Path, + expected_commands: &crate::hooks::GeneratedHookCommands, +) -> Result<(), String> { let actual = read_json_object(path)?; - let expected = generated_hooks(CodingAgent::Codex, expected_command); + let expected = crate::hooks::generated_policy_hooks(CodingAgent::Codex, expected_commands); if actual == expected { Ok(()) } else { @@ -674,7 +683,7 @@ fn is_generated_codex_hook_event(event: &str) -> bool { .any(|expected| normalize_hook_event(expected) == normalized) } -fn normalize_hook_event(event: &str) -> String { +pub(crate) fn normalize_hook_event(event: &str) -> String { event .chars() .filter(|character| character.is_ascii_alphanumeric()) @@ -682,6 +691,18 @@ fn normalize_hook_event(event: &str) -> String { .collect() } +pub(crate) fn expected_codex_hook_command<'a>( + commands: &'a crate::hooks::GeneratedHookCommands, + event: &str, +) -> Option<&'a str> { + let normalized = normalize_hook_event(event); + CodingAgent::Codex + .hook_events() + .iter() + .find(|expected| normalize_hook_event(expected) == normalized) + .map(|event| commands.for_event(event)) +} + fn codex_install_snapshots( config_path: &Path, hooks_path: &Path, @@ -1635,7 +1656,7 @@ pub(crate) fn codex_hooks_installed_with_generation( generation_token: Option<&str>, ) -> Result { let value = read_json_object(path)?; - let generated = generated_hooks( + let generated = crate::hooks::generated_policy_hooks( CodingAgent::Codex, &expected_plugin_hook_command_with_token(path, generation_token)?, ); @@ -1660,8 +1681,8 @@ pub(crate) fn codex_plugin_hook_command( relay: &Path, generation: &Path, generation_token: &str, -) -> Result { - crate::hooks::persistent_hook_forward_command( +) -> Result { + crate::hooks::persistent_hook_forward_commands( relay, CodingAgent::Codex, generation, @@ -1675,8 +1696,8 @@ pub(crate) fn codex_plugin_hook_command_for_platform( generation: &Path, generation_token: &str, windows: bool, -) -> String { - crate::hooks::persistent_hook_forward_command_for_platform( +) -> crate::hooks::GeneratedHookCommands { + crate::hooks::persistent_hook_forward_commands_for_platform( relay, CodingAgent::Codex, generation, diff --git a/crates/cli/src/agents/codex/launch.rs b/crates/cli/src/agents/codex/launch.rs index ad20c5cc0..41cfeafe6 100644 --- a/crates/cli/src/agents/codex/launch.rs +++ b/crates/cli/src/agents/codex/launch.rs @@ -8,7 +8,7 @@ use serde_json::Value; use crate::agents::CodingAgent; use crate::configuration::{RELAY_PLUGIN_ID, RELAY_SOURCE_PLUGIN_ID}; use crate::error::CliError; -use crate::hooks::{generated_hooks, transparent_hook_forward_command}; +use crate::hooks::{generated_policy_hooks, transparent_hook_forward_commands}; use crate::process::{PreparedAgentLaunch, insert_after_host}; pub(crate) fn prepare(launch: &mut PreparedAgentLaunch, gateway_url: &str) -> Result<(), CliError> { @@ -26,13 +26,13 @@ pub(crate) fn prepare(launch: &mut PreparedAgentLaunch, gateway_url: &str) -> Re or pass `--openai-base-url` to an upstream that needs no key." ); } - let hook_command = transparent_hook_forward_command( + let hook_commands = transparent_hook_forward_commands( &transparent_hook_executable(), CodingAgent::Codex, gateway_url, ) .map_err(CliError::Launch)?; - let hook_groups = generated_hooks(CodingAgent::Codex, &hook_command); + let hook_groups = generated_policy_hooks(CodingAgent::Codex, &hook_commands); let mut args = vec![ "--config".to_string(), "features.hooks=true".to_string(), diff --git a/crates/cli/src/agents/hermes/config.rs b/crates/cli/src/agents/hermes/config.rs index 95d305fb3..06c3d413b 100644 --- a/crates/cli/src/agents/hermes/config.rs +++ b/crates/cli/src/agents/hermes/config.rs @@ -8,7 +8,7 @@ use std::path::{Path, PathBuf}; use serde_json::{Map, Value, json}; use crate::error::CliError; -use crate::hooks::{generated_hooks, merge_hooks}; +use crate::hooks::{GeneratedHookCommands, generated_policy_hooks, merge_hooks}; pub(super) use crate::mcp::SERVER_NAME as MCP_SERVER_NAME; @@ -32,9 +32,9 @@ pub(crate) fn transparent_config( ) -> Result { let mut root = parse_yaml_object(Some(existing), "Hermes config")?; let owned = owned_install_command(&root, relay, None)?; - strip_owned_hooks(&mut root, owned.as_deref())?; + strip_owned_hooks(&mut root, owned.as_ref())?; remove_owned_mcp(&mut root, owned.is_some())?; - let command = crate::hooks::transparent_hook_forward_command( + let commands = crate::hooks::transparent_hook_forward_commands( relay, crate::agents::CodingAgent::Hermes, gateway_url, @@ -42,7 +42,7 @@ pub(crate) fn transparent_config( .map_err(CliError::Install)?; let mut root = merge_hooks( root, - generated_hooks(crate::agents::CodingAgent::Hermes, &command), + generated_policy_hooks(crate::agents::CodingAgent::Hermes, &commands), )?; let object = root .as_object_mut() @@ -75,12 +75,12 @@ pub(crate) fn transparent_config( serde_yaml::to_string(&root).map_err(|error| CliError::Install(error.to_string())) } -pub(crate) fn persistent_hook_command( +pub(crate) fn persistent_hook_commands( relay: &Path, generation: &Path, generation_token: &str, -) -> Result { - crate::hooks::persistent_hook_forward_command( +) -> Result { + crate::hooks::persistent_hook_forward_commands( relay, crate::agents::CodingAgent::Hermes, generation, @@ -89,13 +89,13 @@ pub(crate) fn persistent_hook_command( } #[cfg(test)] -pub(super) fn persistent_hook_command_for_platform( +pub(super) fn persistent_hook_commands_for_platform( relay: &Path, generation: &Path, generation_token: &str, windows: bool, -) -> String { - crate::hooks::persistent_hook_forward_command_for_platform( +) -> GeneratedHookCommands { + crate::hooks::persistent_hook_forward_commands_for_platform( relay, crate::agents::CodingAgent::Hermes, generation, @@ -107,7 +107,7 @@ pub(super) fn persistent_hook_command_for_platform( pub(super) fn persistent_config( existing: Option<&str>, relay: &Path, - command: &str, + commands: &GeneratedHookCommands, generation: &Path, generation_token: &str, environment: &[String], @@ -123,10 +123,10 @@ pub(super) fn persistent_config( "Hermes MCP server `{MCP_SERVER_NAME}` already exists and is not managed by Relay; rename or remove it before installing the Relay integration" ))); } - strip_owned_hooks(&mut root, owned.as_deref())?; + strip_owned_hooks(&mut root, owned.as_ref())?; root = merge_hooks( root, - generated_hooks(crate::agents::CodingAgent::Hermes, command), + generated_policy_hooks(crate::agents::CodingAgent::Hermes, commands), )?; let servers = object_field_mut(&mut root, "mcp_servers", "mcp_servers")?; servers.insert( @@ -162,7 +162,7 @@ pub(super) fn forwarded_environment_names( pub(super) fn strip_owned_hooks( root: &mut Value, - owned_command: Option<&str>, + owned_commands: Option<&GeneratedHookCommands>, ) -> Result<(), CliError> { let Some(hooks) = root.get_mut("hooks") else { return Ok(()); @@ -180,7 +180,9 @@ pub(super) fn strip_owned_hooks( group .get("command") .and_then(Value::as_str) - .is_none_or(|command| Some(command) != owned_command) + .is_none_or(|command| { + owned_commands.is_none_or(|commands| !commands.contains(command)) + }) }); if groups.is_empty() { empty.push(event.clone()); @@ -221,7 +223,7 @@ pub(super) fn owned_install_command( root: &Value, relay: &Path, expected_generation: Option<&Path>, -) -> Result, CliError> { +) -> Result, CliError> { let Some(server) = root.pointer(&format!("/mcp_servers/{MCP_SERVER_NAME}")) else { return Ok(None); }; @@ -243,15 +245,18 @@ pub(super) fn owned_install_command( && !token.is_empty() && expected_generation.is_none_or(|expected| Path::new(generation) == expected) { - let command = persistent_hook_command(relay, Path::new(generation), token) + let commands = persistent_hook_commands(relay, Path::new(generation), token) .map_err(CliError::Install)?; - return Ok(Some(command)); + return Ok(Some(commands)); } } legacy_owned_command(root, relay) } -fn legacy_owned_command(root: &Value, relay: &Path) -> Result, CliError> { +fn legacy_owned_command( + root: &Value, + relay: &Path, +) -> Result, CliError> { let server = &root["mcp_servers"][MCP_SERVER_NAME]; if server.get("args") != Some(&json!(["mcp", "--agent", "hermes"])) { return Ok(None); @@ -274,7 +279,7 @@ fn legacy_owned_command(root: &Value, relay: &Path) -> Result, Cl } common = Some(commands[0]); } - Ok(common.map(str::to_owned)) + Ok(common.map(GeneratedHookCommands::uniform)) } fn legacy_command_uses_relay(command: &str, relay: &Path) -> bool { diff --git a/crates/cli/src/agents/hermes/integration.rs b/crates/cli/src/agents/hermes/integration.rs index f14457a3f..115c04ca0 100644 --- a/crates/cli/src/agents/hermes/integration.rs +++ b/crates/cli/src/agents/hermes/integration.rs @@ -11,13 +11,13 @@ use std::time::SystemTime; use serde_json::{Map, Value, json}; #[cfg(test)] -use super::config::persistent_hook_command_for_platform; +use super::config::persistent_hook_commands_for_platform; use super::config::{ MCP_SERVER_NAME, expected_mcp_server, forwarded_environment_names, owned_install_command, parse_yaml_object, persistent_config, relay_is_executable, remove_owned_mcp, strip_owned_hooks, user_config_path_with_override, yaml_bytes, }; -pub(crate) use super::config::{persistent_hook_command, transparent_config}; +pub(crate) use super::config::{persistent_hook_commands, transparent_config}; use super::files::{ FileSnapshot, INSTALL_LOCK_TIMEOUT, PersistentPaths, acquire_allowlist_lock, acquire_install_lock, read_optional_utf8, remove_optional_file, replace_optional_file, @@ -27,6 +27,7 @@ use crate::agents::CodingAgent; use crate::bootstrap::DEFAULT_BIND; use crate::error::CliError; use crate::filesystem::atomic_write; +use crate::hooks::GeneratedHookCommands; #[cfg(test)] use crate::installation::generation::GENERATION_FILE_NAME; use crate::installation::generation::{ @@ -166,7 +167,10 @@ fn config_has_managed_state(config: &Value) -> bool { owned_command_from_config(config, None).is_some() } -fn allowlist_has_owned_command(allowlist: &Value, command: Option<&str>) -> bool { +fn allowlist_has_owned_command( + allowlist: &Value, + commands: Option<&GeneratedHookCommands>, +) -> bool { allowlist .get("approvals") .and_then(Value::as_array) @@ -174,16 +178,23 @@ fn allowlist_has_owned_command(allowlist: &Value, command: Option<&str>) -> bool .flatten() .filter_map(|entry| entry.get("command").and_then(Value::as_str)) .any(|candidate| { - command == Some(candidate) - || (command.is_none() && is_persistent_relay_hook_command(candidate)) + commands.is_some_and(|commands| commands.contains(candidate)) + || (commands.is_none() && is_persistent_relay_hook_command(candidate)) }) } fn is_persistent_relay_hook_command(command: &str) -> bool { #[cfg(any(windows, test))] if let Some(arguments) = crate::hooks::decode_windows_hook_command(command) { + let arguments = arguments.as_slice(); + let base = match arguments { + [base @ .., policy] if matches!(policy.as_str(), "--fail-open" | "--fail-closed") => { + base + } + base => base, + }; return matches!( - arguments.as_slice(), + base, [ _, hook_forward, @@ -211,7 +222,10 @@ fn is_persistent_relay_hook_command(command: &str) -> bool { && command.contains("--generation-token") } -fn owned_command_from_config(config: &Value, generation: Option<&Path>) -> Option { +fn owned_command_from_config( + config: &Value, + generation: Option<&Path>, +) -> Option { let relay = config .pointer(&format!("/mcp_servers/{MCP_SERVER_NAME}/command")) .and_then(Value::as_str) @@ -235,9 +249,9 @@ pub(crate) fn diagnose_persistent(config_path: &Path) -> Result )); } let generation = InstallGeneration::capture(paths.generation.clone())?; - let command = persistent_hook_command(&relay, &paths.generation, generation.token())?; - verify_hook_definitions(&config, &command)?; - verify_trust(&paths.allowlist, &command)?; + let commands = persistent_hook_commands(&relay, &paths.generation, generation.token())?; + verify_hook_definitions(&config, &commands)?; + verify_trust(&paths.allowlist, &commands)?; let mcp_env = config["mcp_servers"][MCP_SERVER_NAME] .get("env") @@ -374,20 +388,20 @@ where }; let environment = forwarded_environment_names(environment, plugin_config); let token = uuid::Uuid::now_v7().to_string(); - let command = - persistent_hook_command(relay, &paths.generation, &token).map_err(CliError::Install)?; + let commands = + persistent_hook_commands(relay, &paths.generation, &token).map_err(CliError::Install)?; let config = persistent_config( existing_config.as_deref(), relay, - &command, + &commands, &paths.generation, &token, &environment, )?; let allowlist = trusted_hooks( existing_allowlist.as_deref(), - previous_command.as_deref(), - &command, + previous_command.as_ref(), + &commands, relay, now, )?; @@ -404,7 +418,7 @@ where verify_install( &paths, relay, - &command, + &commands, &environment, &token, generation_transaction, @@ -437,7 +451,7 @@ where .map(|raw| { let mut root = parse_yaml_object(Some(&raw), "Hermes config")?; let owned = owned_command_from_config(&root, Some(&paths.generation)); - strip_owned_hooks(&mut root, owned.as_deref())?; + strip_owned_hooks(&mut root, owned.as_ref())?; remove_owned_mcp(&mut root, owned.is_some())?; if root.as_object().is_some_and(Map::is_empty) { Ok(None) @@ -466,7 +480,11 @@ where entry .get("command") .and_then(Value::as_str) - .is_none_or(|command| Some(command) != owned.as_deref()) + .is_none_or(|command| { + owned + .as_ref() + .is_none_or(|commands| !commands.contains(command)) + }) }); if approvals.is_empty() { object.remove("approvals"); @@ -485,7 +503,7 @@ where remove_optional_file(&paths.generation)?; replace_optional_file(&paths.allowlist, allowlist.as_deref(), &mut write)?; replace_optional_file(&paths.config, config.as_deref(), &mut write)?; - verify_uninstall(&paths, owned.as_deref()) + verify_uninstall(&paths, owned.as_ref()) })(); if let Err(error) = result { return rollback_error("uninstall", error, &snapshots, &mut write); @@ -520,7 +538,7 @@ where fn verify_install( paths: &PersistentPaths, relay: &Path, - command: &str, + commands: &GeneratedHookCommands, environment: &[String], token: &str, generation_transaction: Option<&GenerationRetirement>, @@ -532,8 +550,8 @@ fn verify_install( if config.pointer("/mcp_servers/nemo-relay") != Some(&expected) { return Err("Hermes MCP server did not persist exactly".into()); } - verify_hook_definitions(&config, command)?; - verify_trust(&paths.allowlist, command)?; + verify_hook_definitions(&config, commands)?; + verify_trust(&paths.allowlist, commands)?; let actual_token = match generation_transaction { Some(transaction) => transaction.active_visible_token()?, @@ -547,7 +565,7 @@ fn verify_install( Ok(()) } -fn verify_hook_definitions(config: &Value, command: &str) -> Result<(), String> { +fn verify_hook_definitions(config: &Value, commands: &GeneratedHookCommands) -> Result<(), String> { for event in CodingAgent::Hermes.hook_events() { let groups = config .pointer(&format!("/hooks/{event}")) @@ -555,7 +573,9 @@ fn verify_hook_definitions(config: &Value, command: &str) -> Result<(), String> .ok_or_else(|| format!("Hermes hook {event} is missing"))?; let matching = groups .iter() - .filter(|group| group.get("command").and_then(Value::as_str) == Some(command)) + .filter(|group| { + group.get("command").and_then(Value::as_str) == Some(commands.for_event(event)) + }) .count(); if matching != 1 { return Err(format!( @@ -573,9 +593,12 @@ fn verify_hook_definitions(config: &Value, command: &str) -> Result<(), String> .as_array() .ok_or_else(|| format!("Hermes {event} hooks must be an array"))?; if !CodingAgent::Hermes.hook_events().contains(&event.as_str()) - && groups - .iter() - .any(|group| group.get("command").and_then(Value::as_str) == Some(command)) + && groups.iter().any(|group| { + group + .get("command") + .and_then(Value::as_str) + .is_some_and(|command| commands.contains(command)) + }) { return Err("Hermes config contains an unexpected Relay hook handler".into()); } @@ -583,7 +606,10 @@ fn verify_hook_definitions(config: &Value, command: &str) -> Result<(), String> Ok(()) } -fn verify_uninstall(paths: &PersistentPaths, owned_command: Option<&str>) -> Result<(), String> { +fn verify_uninstall( + paths: &PersistentPaths, + owned_commands: Option<&GeneratedHookCommands>, +) -> Result<(), String> { if paths.generation.exists() { return Err("Hermes MCP generation fence still exists".into()); } @@ -596,7 +622,7 @@ fn verify_uninstall(paths: &PersistentPaths, owned_command: Option<&str>) -> Res if let Some(raw) = read_optional_utf8(&paths.allowlist).map_err(|error| error.to_string())? { let allowlist = parse_json_object(Some(&raw), "Hermes shell-hook allowlist") .map_err(|e| e.to_string())?; - if allowlist_has_owned_command(&allowlist, owned_command) { + if allowlist_has_owned_command(&allowlist, owned_commands) { return Err("managed Hermes Relay trust approval still exists".into()); } } diff --git a/crates/cli/src/agents/hermes/trust.rs b/crates/cli/src/agents/hermes/trust.rs index 494c26c53..f6ae969ad 100644 --- a/crates/cli/src/agents/hermes/trust.rs +++ b/crates/cli/src/agents/hermes/trust.rs @@ -12,11 +12,12 @@ use serde_json::{Value, json}; use crate::agents::CodingAgent; use crate::error::CliError; +use crate::hooks::GeneratedHookCommands; pub(super) fn trusted_hooks( existing: Option<&str>, - previous_command: Option<&str>, - command: &str, + previous_commands: Option<&GeneratedHookCommands>, + commands: &GeneratedHookCommands, relay: &Path, now: SystemTime, ) -> Result { @@ -34,7 +35,9 @@ pub(super) fn trusted_hooks( entry .get("command") .and_then(Value::as_str) - .is_none_or(|candidate| Some(candidate) != previous_command) + .is_none_or(|candidate| { + previous_commands.is_none_or(|commands| !commands.contains(candidate)) + }) }); let approved_at = timestamp(now); let script_mtime_at_approval = fs::metadata(relay) @@ -44,7 +47,7 @@ pub(super) fn trusted_hooks( approvals.extend(CodingAgent::Hermes.hook_events().iter().map(|event| { json!({ "event": event, - "command": command, + "command": commands.for_event(event), "approved_at": approved_at, "script_mtime_at_approval": script_mtime_at_approval, }) @@ -56,7 +59,10 @@ fn timestamp(time: SystemTime) -> String { DateTime::::from(time).to_rfc3339_opts(SecondsFormat::Micros, true) } -pub(super) fn verify_trust(allowlist_path: &Path, command: &str) -> Result<(), String> { +pub(super) fn verify_trust( + allowlist_path: &Path, + commands: &GeneratedHookCommands, +) -> Result<(), String> { let raw = fs::read_to_string(allowlist_path) .map_err(|error| format!("failed to read {}: {error}", allowlist_path.display()))?; let allowlist = @@ -70,7 +76,8 @@ pub(super) fn verify_trust(allowlist_path: &Path, command: &str) -> Result<(), S .iter() .filter(|entry| { entry.get("event").and_then(Value::as_str) == Some(event) - && entry.get("command").and_then(Value::as_str) == Some(command) + && entry.get("command").and_then(Value::as_str) + == Some(commands.for_event(event)) }) .count(); if matching != 1 { @@ -80,7 +87,10 @@ pub(super) fn verify_trust(allowlist_path: &Path, command: &str) -> Result<(), S } } for entry in approvals { - if entry.get("command").and_then(Value::as_str) != Some(command) { + let Some(command) = entry.get("command").and_then(Value::as_str) else { + continue; + }; + if !commands.contains(command) { continue; } let event = entry diff --git a/crates/cli/src/agents/mod.rs b/crates/cli/src/agents/mod.rs index 41cd5f72a..376387797 100644 --- a/crates/cli/src/agents/mod.rs +++ b/crates/cli/src/agents/mod.rs @@ -200,13 +200,13 @@ impl crate::installation::marketplace::MarketplaceHost for CodingAgent { generation_fence: &std::path::Path, generation_token: &str, ) -> Result { - let command = crate::hooks::persistent_hook_forward_command( + let commands = crate::hooks::persistent_hook_forward_commands( relay, self, generation_fence, generation_token, )?; - Ok(crate::hooks::generated_hooks(self, &command)) + Ok(crate::hooks::generated_policy_hooks(self, &commands)) } fn plugin_registration_args(self, plugin_id: &str) -> Vec { diff --git a/crates/cli/src/commands/hook_forward.rs b/crates/cli/src/commands/hook_forward.rs index 9b767570f..14fdf5770 100644 --- a/crates/cli/src/commands/hook_forward.rs +++ b/crates/cli/src/commands/hook_forward.rs @@ -45,8 +45,11 @@ pub(crate) struct HookForwardCommand { #[arg(long, value_enum)] pub(crate) gateway_mode: Option, /// Return a failure when the payload cannot be delivered or Relay rejects it. - #[arg(long)] + #[arg(long, conflicts_with = "fail_open")] pub(crate) fail_closed: bool, + /// Allow the coding agent to continue when the payload cannot be delivered. + #[arg(long, conflicts_with = "fail_closed")] + pub(crate) fail_open: bool, } impl HookForwardCommand { @@ -61,7 +64,13 @@ impl HookForwardCommand { profile: self.profile, session_metadata: self.session_metadata, gateway_mode: self.gateway_mode.map(Into::into), - fail_closed: self.fail_closed, + failure_policy: if self.fail_closed { + crate::hooks::HookFailurePolicy::FailClosed + } else if self.fail_open { + crate::hooks::HookFailurePolicy::FailOpen + } else { + crate::hooks::HookFailurePolicy::Default + }, } } } diff --git a/crates/cli/src/hooks/delivery.rs b/crates/cli/src/hooks/delivery.rs index b8db65620..cb9cf566f 100644 --- a/crates/cli/src/hooks/delivery.rs +++ b/crates/cli/src/hooks/delivery.rs @@ -29,8 +29,7 @@ pub(crate) async fn hook_forward(command: HookForwardRequest) -> Result<(), CliE return Ok(()); } validate_optional_json("session metadata", command.session_metadata.as_deref())?; - let fail_closed = - command.fail_closed || std::env::var("NEMO_RELAY_FAIL_CLOSED").ok().as_deref() == Some("1"); + let fail_closed = command.failure_policy.fail_closed(); let destination = hook_destination(&command); let persistent = match persistent_gateway(&destination) { Ok(persistent) => persistent, diff --git a/crates/cli/src/hooks/encoding.rs b/crates/cli/src/hooks/encoding.rs index 1ac762c65..aee4d51bc 100644 --- a/crates/cli/src/hooks/encoding.rs +++ b/crates/cli/src/hooks/encoding.rs @@ -12,22 +12,71 @@ use crate::agents::CodingAgent; #[cfg(any(windows, test))] use base64::Engine; +#[cfg(test)] pub(crate) fn generated_hooks(agent: CodingAgent, command: &str) -> Value { + generated_policy_hooks(agent, &GeneratedHookCommands::uniform(command)) +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct GeneratedHookCommands { + fail_open: String, + fail_closed: String, + legacy: Option, +} + +impl GeneratedHookCommands { + pub(crate) fn new(fail_open: impl Into, fail_closed: impl Into) -> Self { + Self { + fail_open: fail_open.into(), + fail_closed: fail_closed.into(), + legacy: None, + } + } + + pub(crate) fn uniform(command: impl Into) -> Self { + let command = command.into(); + Self::new(command.clone(), command) + } + + pub(crate) fn for_event(&self, event: &str) -> &str { + if event_requires_fail_closed(event) { + &self.fail_closed + } else { + &self.fail_open + } + } + + pub(crate) fn contains(&self, command: &str) -> bool { + command == self.fail_open + || command == self.fail_closed + || self.legacy.as_deref() == Some(command) + } + + #[cfg(test)] + pub(crate) fn legacy(&self) -> Option<&str> { + self.legacy.as_deref() + } +} + +pub(crate) fn generated_policy_hooks( + agent: CodingAgent, + commands: &GeneratedHookCommands, +) -> Value { if agent.uses_direct_hook_entries() { - direct_hooks(agent.hook_events(), command) + direct_hooks(agent.hook_events(), commands) } else { - grouped_hooks(agent.hook_events(), command) + grouped_hooks(agent.hook_events(), commands) } } /// Canonical persistent hook command used by every supported host. -pub(crate) fn persistent_hook_forward_command( +pub(crate) fn persistent_hook_forward_commands( relay: &Path, agent: CodingAgent, generation_file: &Path, generation_token: &str, -) -> Result { - hook_command( +) -> Result { + hook_commands( relay, &persistent_hook_arguments(agent, generation_file, generation_token), ) @@ -35,22 +84,22 @@ pub(crate) fn persistent_hook_forward_command( /// Canonical transparent hook command. It embeds the process-private dynamic gateway so hook hosts /// that filter inherited environment variables cannot redirect delivery to the fixed endpoint. -pub(crate) fn transparent_hook_forward_command( +pub(crate) fn transparent_hook_forward_commands( relay: &Path, agent: CodingAgent, gateway_url: &str, -) -> Result { - hook_command(relay, &transparent_hook_arguments(agent, gateway_url)) +) -> Result { + hook_commands(relay, &transparent_hook_arguments(agent, gateway_url)) } #[cfg(test)] -pub(crate) fn transparent_hook_forward_command_for_platform( +pub(crate) fn transparent_hook_forward_commands_for_platform( relay: &Path, agent: CodingAgent, gateway_url: &str, windows: bool, -) -> String { - hook_command_for_platform( +) -> GeneratedHookCommands { + hook_commands_for_platform( relay, &transparent_hook_arguments(agent, gateway_url), windows, @@ -58,14 +107,14 @@ pub(crate) fn transparent_hook_forward_command_for_platform( } #[cfg(test)] -pub(crate) fn persistent_hook_forward_command_for_platform( +pub(crate) fn persistent_hook_forward_commands_for_platform( relay: &Path, agent: CodingAgent, generation_file: &Path, generation_token: &str, windows: bool, -) -> String { - hook_command_for_platform( +) -> GeneratedHookCommands { + hook_commands_for_platform( relay, &persistent_hook_arguments(agent, generation_file, generation_token), windows, @@ -99,6 +148,45 @@ pub(super) fn persistent_hook_arguments( ] } +fn hook_commands(relay: &Path, arguments: &[String]) -> Result { + let mut commands = GeneratedHookCommands::new( + hook_command(relay, &with_failure_policy(arguments, "--fail-open"))?, + hook_command(relay, &with_failure_policy(arguments, "--fail-closed"))?, + ); + commands.legacy = Some(hook_command(relay, arguments)?); + Ok(commands) +} + +#[cfg(test)] +fn hook_commands_for_platform( + relay: &Path, + arguments: &[String], + windows: bool, +) -> GeneratedHookCommands { + let mut commands = GeneratedHookCommands::new( + hook_command_for_platform( + relay, + &with_failure_policy(arguments, "--fail-open"), + windows, + ), + hook_command_for_platform( + relay, + &with_failure_policy(arguments, "--fail-closed"), + windows, + ), + ); + commands.legacy = Some(hook_command_for_platform(relay, arguments, windows)); + commands +} + +fn with_failure_policy(arguments: &[String], policy: &str) -> Vec { + arguments + .iter() + .cloned() + .chain(std::iter::once(policy.to_string())) + .collect() +} + pub(super) fn hook_command(relay: &Path, arguments: &[String]) -> Result { #[cfg(windows)] { @@ -302,14 +390,14 @@ pub(super) fn parse_powershell_single_quoted_arguments(mut raw: &str) -> Option< (!arguments.is_empty()).then_some(arguments) } -pub(super) fn direct_hooks(events: &[&str], command: &str) -> Value { +fn direct_hooks(events: &[&str], commands: &GeneratedHookCommands) -> Value { let hooks: serde_json::Map = events .iter() .map(|event| { ( (*event).to_string(), json!([{ - "command": command, + "command": commands.for_event(event), "timeout": 30 }]), ) @@ -321,7 +409,7 @@ pub(super) fn direct_hooks(events: &[&str], command: &str) -> Value { // Generates hook groups for Claude/Codex events and adds a wildcard matcher to tool events when // the target agent requires matcher-scoped tool hooks. Non-tool events omit matchers so they fire // for the full lifecycle. -pub(super) fn grouped_hooks(events: &[&str], command: &str) -> Value { +fn grouped_hooks(events: &[&str], commands: &GeneratedHookCommands) -> Value { let hooks: serde_json::Map = events .iter() .map(|event| { @@ -333,7 +421,7 @@ pub(super) fn grouped_hooks(events: &[&str], command: &str) -> Value { "hooks".into(), json!([{ "type": "command", - "command": command, + "command": commands.for_event(event), "timeout": 30 }]), ); @@ -354,3 +442,7 @@ pub(crate) fn event_matches_tools(event: &str) -> bool { "PreToolUse" | "PostToolUse" | "PostToolUseFailure" | "PermissionRequest" ) } + +pub(crate) fn event_requires_fail_closed(event: &str) -> bool { + matches!(event, "PreToolUse" | "PermissionRequest" | "pre_tool_call") +} diff --git a/crates/cli/src/hooks/mod.rs b/crates/cli/src/hooks/mod.rs index 6389f3c2e..d21efe041 100644 --- a/crates/cli/src/hooks/mod.rs +++ b/crates/cli/src/hooks/mod.rs @@ -23,18 +23,19 @@ pub(crate) use destination::{ pub(crate) use encoding::decode_windows_hook_command; #[cfg(all(test, windows))] pub(crate) use encoding::windows_powershell_path; -#[cfg(test)] pub(crate) use encoding::{ - encoded_windows_hook_command, event_matches_tools, - persistent_hook_forward_command_for_platform, transparent_hook_forward_command_for_platform, + GeneratedHookCommands, generated_policy_hooks, persistent_hook_forward_commands, + transparent_hook_forward_commands, }; +#[cfg(test)] pub(crate) use encoding::{ - generated_hooks, persistent_hook_forward_command, transparent_hook_forward_command, + encoded_windows_hook_command, event_matches_tools, event_requires_fail_closed, generated_hooks, + persistent_hook_forward_commands_for_platform, transparent_hook_forward_commands_for_platform, }; pub(crate) use merging::merge_hooks; #[cfg(test)] pub(crate) use response::{handle_hook_forward_status, handle_verified_hook_forward_response}; -pub(crate) use types::{GatewayMode, HookForwardRequest}; +pub(crate) use types::{GatewayMode, HookFailurePolicy, HookForwardRequest}; #[cfg(test)] use serde_json::json; diff --git a/crates/cli/src/hooks/types.rs b/crates/cli/src/hooks/types.rs index 9959b309d..b11713112 100644 --- a/crates/cli/src/hooks/types.rs +++ b/crates/cli/src/hooks/types.rs @@ -16,7 +16,24 @@ pub(crate) struct HookForwardRequest { pub(crate) profile: Option, pub(crate) session_metadata: Option, pub(crate) gateway_mode: Option, - pub(crate) fail_closed: bool, + pub(crate) failure_policy: HookFailurePolicy, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum HookFailurePolicy { + Default, + FailOpen, + FailClosed, +} + +impl HookFailurePolicy { + pub(crate) fn fail_closed(self) -> bool { + match self { + Self::Default => std::env::var("NEMO_RELAY_FAIL_CLOSED").ok().as_deref() == Some("1"), + Self::FailOpen => false, + Self::FailClosed => true, + } + } } #[derive(Debug, Clone, Copy, PartialEq, Eq)] diff --git a/crates/cli/tests/cli_tests.rs b/crates/cli/tests/cli_tests.rs index 4fce1eea1..8c6aa6ec3 100644 --- a/crates/cli/tests/cli_tests.rs +++ b/crates/cli/tests/cli_tests.rs @@ -786,17 +786,23 @@ fn assert_hermes_install_config(config_path: &std::path::Path, hermes_home: &std .unwrap() .contains("not-written-to-config") ); - let command = config["hooks"]["on_session_start"][0]["command"] - .as_str() - .unwrap(); - assert!(command.contains("hook-forward hermes")); let approvals: serde_json::Value = serde_json::from_str( &std::fs::read_to_string(hermes_home.join("shell-hooks-allowlist.json")).unwrap(), ) .unwrap(); let approvals = approvals["approvals"].as_array().unwrap(); assert_eq!(approvals.len(), 13); - assert!(approvals.iter().all(|entry| entry["command"] == command)); + for approval in approvals { + let event = approval["event"].as_str().unwrap(); + let command = approval["command"].as_str().unwrap(); + assert!(command.contains("hook-forward hermes")); + assert_eq!(approval["command"], config["hooks"][event][0]["command"]); + if event == "pre_tool_call" { + assert!(command.ends_with(" --fail-closed")); + } else { + assert!(command.ends_with(" --fail-open")); + } + } } #[cfg(unix)] @@ -1314,6 +1320,60 @@ fn cli_codex_hook_launch_resolution_error_respects_forwarding_policy() { } } +#[test] +fn cli_hook_forward_explicit_policy_overrides_the_environment() { + let temp = tempfile::tempdir().unwrap(); + let generation = write_active_generation(temp.path()); + for (policy, environment, succeeds) in [ + ("--fail-open", Some("1"), true), + ("--fail-closed", None, false), + ] { + let mut command = Command::new(gateway_bin()); + command + .args([ + "hook-forward", + "codex", + "--gateway-url", + "http://127.0.0.1:1", + "--generation-file", + ]) + .arg(&generation) + .arg("--generation-token") + .arg(ACTIVE_GENERATION_TOKEN) + .arg(policy) + .env("HOME", temp.path()) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("XDG_RUNTIME_DIR", temp.path().join("runtime")) + .env("TMPDIR", temp.path()) + .env("NEMO_RELAY_PLUGIN_IDLE_TIMEOUT_SECS", "not-a-number") + .env_remove("NEMO_RELAY_FAIL_CLOSED") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + if let Some(value) = environment { + command.env("NEMO_RELAY_FAIL_CLOSED", value); + } + let mut child = command.spawn().unwrap(); + child.stdin.take().unwrap().write_all(b"{}").unwrap(); + let output = wait_child_with_output(child); + assert_eq!(output.status.success(), succeeds, "{policy}"); + assert!( + String::from_utf8_lossy(&output.stderr).contains("NEMO_RELAY_PLUGIN_IDLE_TIMEOUT_SECS") + ); + } +} + +#[test] +fn cli_hook_forward_rejects_conflicting_failure_policies() { + let output = Command::new(gateway_bin()) + .args(["hook-forward", "codex", "--fail-open", "--fail-closed"]) + .output() + .unwrap(); + + assert!(!output.status.success()); + assert!(String::from_utf8_lossy(&output.stderr).contains("cannot be used with")); +} + #[test] fn cli_codex_hook_launch_resolution_error_retains_default_payload_cap() { const DEFAULT_HOOK_PAYLOAD_BYTES: usize = 20 * 1024 * 1024; diff --git a/crates/cli/tests/coverage/agents/hermes_tests.rs b/crates/cli/tests/coverage/agents/hermes_tests.rs index eae5ee906..a9e7579ea 100644 --- a/crates/cli/tests/coverage/agents/hermes_tests.rs +++ b/crates/cli/tests/coverage/agents/hermes_tests.rs @@ -118,16 +118,20 @@ fn hook_command_round_trips_paths_and_platform_metacharacters() { let relay = Path::new("/tmp/NeMo $Relay`test'/bin/nemo-relay"); let generation = Path::new("/tmp/generation"); assert_eq!( - persistent_hook_command_for_platform(relay, generation, TEST_GENERATION_TOKEN, false), - "'/tmp/NeMo $Relay`test'\\''/bin/nemo-relay' hook-forward hermes --gateway-url http://127.0.0.1:47632 --generation-file /tmp/generation --generation-token test-generation" + persistent_hook_commands_for_platform(relay, generation, TEST_GENERATION_TOKEN, false) + .for_event("on_session_start"), + "'/tmp/NeMo $Relay`test'\\''/bin/nemo-relay' hook-forward hermes --gateway-url http://127.0.0.1:47632 --generation-file /tmp/generation --generation-token test-generation --fail-open" ); assert_eq!( - crate::hooks::decode_windows_hook_command(&persistent_hook_command_for_platform( - Path::new(r"C:\Program Files\NeMo 100%\bin\nemo-relay.exe"), - Path::new(r"C:\Temp\generation"), - TEST_GENERATION_TOKEN, - true, - )) + crate::hooks::decode_windows_hook_command( + persistent_hook_commands_for_platform( + Path::new(r"C:\Program Files\NeMo 100%\bin\nemo-relay.exe"), + Path::new(r"C:\Temp\generation"), + TEST_GENERATION_TOKEN, + true, + ) + .for_event("pre_tool_call") + ) .unwrap(), vec![ r"C:\Program Files\NeMo 100%\bin\nemo-relay.exe", @@ -139,25 +143,29 @@ fn hook_command_round_trips_paths_and_platform_metacharacters() { r"C:\Temp\generation", "--generation-token", TEST_GENERATION_TOKEN, + "--fail-closed", ] ); assert_eq!( - crate::hooks::transparent_hook_forward_command_for_platform( + crate::hooks::transparent_hook_forward_commands_for_platform( relay, CodingAgent::Hermes, "http://127.0.0.1:1234", false, - ), - "'/tmp/NeMo $Relay`test'\\''/bin/nemo-relay' hook-forward hermes --gateway-url http://127.0.0.1:1234 --transparent-run" + ) + .for_event("on_session_start"), + "'/tmp/NeMo $Relay`test'\\''/bin/nemo-relay' hook-forward hermes --gateway-url http://127.0.0.1:1234 --transparent-run --fail-open" ); - let encoded = persistent_hook_command_for_platform( + let encoded = persistent_hook_commands_for_platform( Path::new(r"C:\Program Files\NeMo 100%\bin\nemo-relay.exe"), Path::new(r"C:\Temp\generation"), TEST_GENERATION_TOKEN, true, ); - assert!(is_persistent_relay_hook_command(&encoded)); - let encoded_codex = crate::hooks::persistent_hook_forward_command_for_platform( + assert!(is_persistent_relay_hook_command( + encoded.for_event("pre_tool_call") + )); + let encoded_codex = crate::hooks::persistent_hook_forward_commands_for_platform( Path::new(r"C:\Program Files\NeMo 100%\bin\nemo-relay.exe"), CodingAgent::Codex, Path::new(r"C:\Temp\generation"), @@ -165,7 +173,9 @@ fn hook_command_round_trips_paths_and_platform_metacharacters() { true, ); assert_ne!(encoded, encoded_codex); - assert!(!is_persistent_relay_hook_command(&encoded_codex)); + assert!(!is_persistent_relay_hook_command( + encoded_codex.for_event("PreToolUse") + )); } #[test] @@ -200,7 +210,7 @@ fn persistent_config_migrates_owned_state_and_preserves_unrelated_config() { let temp = tempfile::tempdir().unwrap(); let relay = relay_binary(temp.path()); let generation = temp.path().join(GENERATION_FILE_NAME); - let command = persistent_hook_command(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); + let command = persistent_hook_commands(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); let legacy_command = format!("{} hook-forward hermes", relay.display()); let mut legacy_hooks = serde_json::Map::new(); for event in CodingAgent::Hermes.hook_events() { @@ -264,7 +274,7 @@ fn persistent_config_migrates_owned_state_and_preserves_unrelated_config() { ); assert_eq!( merged["hooks"]["on_session_start"][1]["command"], - json!(command) + json!(command.for_event("on_session_start")) ); assert_eq!( merged["hooks"]["custom_event"][0]["command"], @@ -275,7 +285,7 @@ fn persistent_config_migrates_owned_state_and_preserves_unrelated_config() { assert_eq!( groups .iter() - .filter(|group| group["command"] == json!(command)) + .filter(|group| group["command"] == json!(command.for_event(event))) .count(), 1, "event {event}" @@ -288,7 +298,7 @@ fn persistent_config_rejects_a_foreign_server_with_the_reserved_name() { let temp = tempfile::tempdir().unwrap(); let relay = relay_binary(temp.path()); let generation = temp.path().join(GENERATION_FILE_NAME); - let command = persistent_hook_command(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); + let command = persistent_hook_commands(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); let existing = r#" model: keep-me mcp_servers: @@ -317,7 +327,7 @@ fn manual_same_named_mcp_and_hooks_are_never_claimed() { let temp = tempfile::tempdir().unwrap(); let relay = relay_binary(temp.path()); let generation = temp.path().join(GENERATION_FILE_NAME); - let command = persistent_hook_command(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); + let command = persistent_hook_commands(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); let manual = serde_yaml::to_string(&json!({ "mcp_servers": { MCP_SERVER_NAME: {"command": relay, "args": ["mcp"], "env": {"CUSTOM": "keep"}} @@ -373,21 +383,15 @@ fn modern_mcp_generation_proves_ownership_independently_of_hook_completeness() { let mut root = persistent_config( None, &relay, - &persistent_hook_command(&relay, &generation, "hook-token").unwrap(), + &persistent_hook_commands(&relay, &generation, "hook-token").unwrap(), &generation, "mcp-token", &[], ) .unwrap(); assert_eq!( - owned_install_command(&root, &relay, Some(&generation)) - .unwrap() - .as_deref(), - Some( - persistent_hook_command(&relay, &generation, "mcp-token") - .unwrap() - .as_str() - ) + owned_install_command(&root, &relay, Some(&generation)).unwrap(), + Some(persistent_hook_commands(&relay, &generation, "mcp-token").unwrap()) ); root["mcp_servers"][MCP_SERVER_NAME]["command"] = json!(temp.path().join("other/nemo-relay")); @@ -398,6 +402,43 @@ fn modern_mcp_generation_proves_ownership_independently_of_hook_completeness() { ); } +#[test] +fn persistent_config_migrates_modern_single_command_hooks_to_explicit_policies() { + let temp = tempfile::tempdir().unwrap(); + let relay = relay_binary(temp.path()); + let generation = temp.path().join(GENERATION_FILE_NAME); + let commands = persistent_hook_commands(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); + let mut installed = persistent_config( + None, + &relay, + &commands, + &generation, + TEST_GENERATION_TOKEN, + &[], + ) + .unwrap(); + let legacy = commands.legacy().unwrap(); + for event in CodingAgent::Hermes.hook_events() { + installed["hooks"][event][0]["command"] = json!(legacy); + } + + let migrated = persistent_config( + Some(&serde_yaml::to_string(&installed).unwrap()), + &relay, + &commands, + &generation, + TEST_GENERATION_TOKEN, + &[], + ) + .unwrap(); + + for event in CodingAgent::Hermes.hook_events() { + let hooks = migrated["hooks"][event].as_array().unwrap(); + assert_eq!(hooks.len(), 1, "event {event}"); + assert_eq!(hooks[0]["command"], json!(commands.for_event(event))); + } +} + #[test] fn foreign_reserved_server_aborts_install_before_any_file_changes() { let temp = tempfile::tempdir().unwrap(); @@ -425,7 +466,7 @@ fn trusted_hooks_migrates_only_relay_approvals_and_records_every_event() { let temp = tempfile::tempdir().unwrap(); let relay = relay_binary(temp.path()); let generation = temp.path().join(GENERATION_FILE_NAME); - let command = persistent_hook_command(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); + let command = persistent_hook_commands(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); let existing = json!({ "schema": 7, "approvals": [ @@ -435,9 +476,10 @@ fn trusted_hooks_migrates_only_relay_approvals_and_records_every_event() { ] }); let now = UNIX_EPOCH + Duration::from_secs(1_700_000_000); + let legacy = crate::hooks::GeneratedHookCommands::uniform("nemo-relay hook-forward hermes"); let merged = trusted_hooks( Some(&serde_json::to_string(&existing).unwrap()), - Some("nemo-relay hook-forward hermes"), + Some(&legacy), &command, &relay, now, @@ -455,7 +497,10 @@ fn trusted_hooks_migrates_only_relay_approvals_and_records_every_event() { for event in CodingAgent::Hermes.hook_events() { let entries = approvals .iter() - .filter(|entry| entry["event"] == json!(event) && entry["command"] == json!(command)) + .filter(|entry| { + entry["event"] == json!(event) + && entry["command"] == json!(command.for_event(event)) + }) .collect::>(); assert_eq!(entries.len(), 1, "event {event}"); assert_eq!( @@ -471,7 +516,7 @@ fn verification_rejects_relay_handlers_and_approvals_on_unexpected_events() { let temp = tempfile::tempdir().unwrap(); let relay = relay_binary(temp.path()); let generation = temp.path().join(GENERATION_FILE_NAME); - let command = persistent_hook_command(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); + let command = persistent_hook_commands(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); let mut config = persistent_config( None, &relay, @@ -481,7 +526,8 @@ fn verification_rejects_relay_handlers_and_approvals_on_unexpected_events() { &[], ) .unwrap(); - config["hooks"]["unexpected_event"] = json!([{"command": command, "timeout": 30}]); + config["hooks"]["unexpected_event"] = + json!([{"command": command.for_event("on_session_start"), "timeout": 30}]); let error = verify_hook_definitions(&config, &command).unwrap_err(); assert!(error.contains("unexpected Relay hook")); let mut malformed = persistent_config( @@ -493,14 +539,15 @@ fn verification_rejects_relay_handlers_and_approvals_on_unexpected_events() { &[], ) .unwrap(); - malformed["hooks"]["unexpected_event"] = json!({"command": command}); + malformed["hooks"]["unexpected_event"] = + json!({"command": command.for_event("on_session_start")}); let error = verify_hook_definitions(&malformed, &command).unwrap_err(); assert!(error.contains("must be an array")); let mut allowlist = trusted_hooks(None, None, &command, &relay, UNIX_EPOCH).unwrap(); allowlist["approvals"].as_array_mut().unwrap().push(json!({ "event": "unexpected_event", - "command": command, + "command": command.for_event("on_session_start"), "approved_at": "1970-01-01T00:00:00.000000Z" })); let path = temp.path().join("shell-hooks-allowlist.json"); @@ -513,7 +560,7 @@ fn verification_rejects_relay_handlers_and_approvals_on_unexpected_events() { .as_array_mut() .unwrap() .push(json!({ - "command": command, + "command": command.for_event("on_session_start"), "approved_at": "1970-01-01T00:00:00.000000Z" })); std::fs::write(&path, serde_json::to_vec(&missing_event).unwrap()).unwrap(); @@ -526,7 +573,7 @@ fn hermes_structure_and_trust_validation_cover_exact_failure_shapes() { let temp = tempfile::tempdir().unwrap(); let relay = relay_binary(temp.path()); let generation = temp.path().join(GENERATION_FILE_NAME); - let command = persistent_hook_command(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); + let command = persistent_hook_commands(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); let error = trusted_hooks( Some(r#"{"approvals": {}}"#), @@ -617,13 +664,15 @@ fn install_is_verified_idempotent_and_rotates_the_generation() { let config = yaml(&paths.config); let second_command = - persistent_hook_command(&relay, &paths.generation, &second_generation).unwrap(); + persistent_hook_commands(&relay, &paths.generation, &second_generation).unwrap(); assert_eq!( config["hooks"]["on_session_start"] .as_array() .unwrap() .iter() - .filter(|group| group["command"] == json!(second_command)) + .filter(|group| { + group["command"] == json!(second_command.for_event("on_session_start")) + }) .count(), 1 ); @@ -1084,11 +1133,11 @@ fn persistent_state_detection_recognizes_each_relay_owned_surface() { &roots[2].config, serde_yaml::to_string(&json!({ "hooks": { - "on_session_start": [{"command": persistent_hook_command( + "on_session_start": [{"command": persistent_hook_commands( &relay, &roots[2].generation, TEST_GENERATION_TOKEN - ).unwrap()}] + ).unwrap().for_event("on_session_start")}] } })) .unwrap(), @@ -1099,11 +1148,11 @@ fn persistent_state_detection_recognizes_each_relay_owned_surface() { serde_json::to_vec(&json!({ "approvals": [{ "event": "on_session_start", - "command": persistent_hook_command( + "command": persistent_hook_commands( &relay, &roots[3].generation, TEST_GENERATION_TOKEN - ).unwrap() + ).unwrap().for_event("on_session_start") }] })) .unwrap(), @@ -1124,7 +1173,7 @@ fn persistent_state_detection_recognizes_each_relay_owned_surface() { fn transparent_config_suppresses_only_the_managed_mcp_and_uses_one_relay_hook() { let temp = tempfile::tempdir().unwrap(); let relay = relay_binary(temp.path()); - let command = crate::hooks::transparent_hook_forward_command( + let command = crate::hooks::transparent_hook_forward_commands( &relay, CodingAgent::Hermes, "http://127.0.0.1:1234", @@ -1132,7 +1181,7 @@ fn transparent_config_suppresses_only_the_managed_mcp_and_uses_one_relay_hook() .unwrap(); let generation = temp.path().join(GENERATION_FILE_NAME); let persistent_command = - persistent_hook_command(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); + persistent_hook_commands(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); let mut existing = persistent_config( None, &relay, @@ -1172,7 +1221,7 @@ fn transparent_config_suppresses_only_the_managed_mcp_and_uses_one_relay_hook() groups .iter() .filter_map(|group| group.get("command").and_then(Value::as_str)) - .filter(|candidate| **candidate == command) + .filter(|candidate| *candidate == command.for_event(event)) .count(), 1, "event {event}" @@ -1180,7 +1229,7 @@ fn transparent_config_suppresses_only_the_managed_mcp_and_uses_one_relay_hook() assert!( groups .iter() - .any(|group| group["command"] == json!(command)) + .any(|group| group["command"] == json!(command.for_event(event))) ); } assert!( @@ -1345,21 +1394,18 @@ fn hermes_uninstall_and_verification_reject_malformed_or_residual_state() { .unwrap(); let config = yaml(&hermes_paths.config); - let command = config["hooks"]["on_session_start"][0]["command"] - .as_str() - .unwrap() - .to_string(); let token = InstallGeneration::capture(hermes_paths.generation.clone()) .unwrap() .token() .to_owned(); + let command = persistent_hook_commands(&relay, &hermes_paths.generation, &token).unwrap(); let expected_environment = forwarded_environment_names(&[], None); let mut duplicate_hook = config.clone(); duplicate_hook["hooks"]["on_session_start"] .as_array_mut() .unwrap() - .push(json!({"command": command})); + .push(json!({"command": command.for_event("on_session_start")})); let error = verify_hook_definitions(&duplicate_hook, &command).unwrap_err(); assert!( error.contains("exactly one trusted Relay handler"), @@ -1406,15 +1452,12 @@ fn hermes_uninstall_and_verification_reject_malformed_or_residual_state() { atomic_write, ) .unwrap(); - let config = yaml(&hermes_paths.config); - let command = config["hooks"]["on_session_start"][0]["command"] - .as_str() - .unwrap() - .to_string(); let expected_token = InstallGeneration::capture(hermes_paths.generation.clone()) .unwrap() .token() .to_owned(); + let command = + persistent_hook_commands(&relay, &hermes_paths.generation, &expected_token).unwrap(); crate::installation::generation::write_new_generation(&hermes_paths.generation).unwrap(); let error = verify_install( &hermes_paths, @@ -1447,18 +1490,18 @@ fn hermes_uninstall_verifier_identifies_each_residual_owned_surface() { install_persistent_with(paths.clone(), &relay, &[], None, UNIX_EPOCH, atomic_write).unwrap(); let command = owned_command_from_config(&yaml(&paths.config), Some(&paths.generation)); - let error = verify_uninstall(&paths, command.as_deref()).unwrap_err(); + let error = verify_uninstall(&paths, command.as_ref()).unwrap_err(); assert!(error.contains("generation fence still exists"), "{error}"); std::fs::remove_file(&paths.generation).unwrap(); - let error = verify_uninstall(&paths, command.as_deref()).unwrap_err(); + let error = verify_uninstall(&paths, command.as_ref()).unwrap_err(); assert!( error.contains("managed Hermes Relay config still exists"), "{error}" ); std::fs::remove_file(&paths.config).unwrap(); - let error = verify_uninstall(&paths, command.as_deref()).unwrap_err(); + let error = verify_uninstall(&paths, command.as_ref()).unwrap_err(); assert!( error.contains("managed Hermes Relay trust approval still exists"), "{error}" diff --git a/crates/cli/tests/coverage/agents/plugin_host_tests.rs b/crates/cli/tests/coverage/agents/plugin_host_tests.rs index c44f13e99..f231c8f27 100644 --- a/crates/cli/tests/coverage/agents/plugin_host_tests.rs +++ b/crates/cli/tests/coverage/agents/plugin_host_tests.rs @@ -86,7 +86,7 @@ impl CodexHooksClient for FakeCodexHooksClient { } } -fn expected_plugin_command() -> String { +fn expected_plugin_command() -> crate::hooks::GeneratedHookCommands { let relay = current_exe().unwrap(); let relay = relay.canonicalize().unwrap_or(relay); let relay = portable_executable_path(relay); @@ -108,7 +108,7 @@ fn write_plugin_generation_for_hooks(path: &Path) { .unwrap(); } -fn expected_plugin_command_for_hooks(path: &Path) -> String { +fn expected_plugin_command_for_hooks(path: &Path, event: &str) -> String { fs::read_to_string(path) .ok() .and_then(|raw| serde_json::from_str::(&raw).ok()) @@ -116,8 +116,9 @@ fn expected_plugin_command_for_hooks(path: &Path) -> String { value .get("hooks")? .as_object()? - .values() - .next()? + .iter() + .find(|(name, _)| normalize_hook_event(name) == normalize_hook_event(event))? + .1 .as_array()? .first()? .get("hooks")? @@ -127,7 +128,7 @@ fn expected_plugin_command_for_hooks(path: &Path) -> String { .as_str() .map(str::to_owned) }) - .unwrap_or_else(expected_plugin_command) + .unwrap_or_else(|| expected_plugin_command().for_event(event).to_owned()) } fn empty_codex_hooks_client() -> FakeCodexHooksClient { @@ -143,7 +144,7 @@ fn write_plugin_hooks(plugin_root: &Path) -> PathBuf { write_plugin_generation_for_hooks(&path); fs::write( &path, - serde_json::to_vec_pretty(&generated_hooks( + serde_json::to_vec_pretty(&crate::hooks::generated_policy_hooks( CodingAgent::Codex, &expected_plugin_hook_command(&path).unwrap(), )) @@ -170,9 +171,9 @@ fn codex_hook_metadata( fs::create_dir_all(hooks_path.parent().unwrap()).unwrap(); fs::write( &hooks_path, - serde_json::to_vec_pretty(&generated_hooks( + serde_json::to_vec_pretty(&crate::hooks::generated_policy_hooks( CodingAgent::Codex, - &expected_plugin_command_for_hooks(&hooks_path), + &expected_plugin_command(), )) .unwrap(), ) @@ -182,7 +183,7 @@ fn codex_hook_metadata( key: key.into(), event_name: event_name.into(), handler_type: "command".into(), - command: Some(expected_plugin_command_for_hooks(&hooks_path)), + command: Some(expected_plugin_command_for_hooks(&hooks_path, event_name)), source_path: hooks_path.display().to_string(), source: "plugin".into(), plugin_id: Some(CODEX_PLUGIN_ID.into()), @@ -949,8 +950,13 @@ fn codex_auto_trust_rejects_modified_loaded_plugin_hook_file() { let config_path = dir.path().join("config.toml"); fs::write(&config_path, "").unwrap(); let mut hooks = required_codex_hook_metadata(&reported_hooks_path, "untrusted", true); + let expected = expected_plugin_command(); for hook in &mut hooks { - hook.command = Some(expected_plugin_command()); + hook.command = Some( + expected_codex_hook_command(&expected, &hook.event_name) + .unwrap() + .to_owned(), + ); } let mut client = FakeCodexHooksClient { hook_lists: VecDeque::from([Ok(hooks)]), @@ -1896,7 +1902,11 @@ fn codex_setup_can_validate_hooks_while_installer_holds_the_generation_lock() { fs::create_dir_all(hooks_path.parent().unwrap()).unwrap(); fs::write( &hooks_path, - serde_json::to_vec_pretty(&generated_hooks(CodingAgent::Codex, &command)).unwrap(), + serde_json::to_vec_pretty(&crate::hooks::generated_policy_hooks( + CodingAgent::Codex, + &command, + )) + .unwrap(), ) .unwrap(); let _transaction = @@ -2000,7 +2010,7 @@ fn codex_setup_uses_plugin_hooks_without_writing_user_hooks() { DEFAULT_URL, &expected_plugin_command(), |_home, _config, command| { - assert_eq!(command, expected_plugin_command()); + assert_eq!(command, &expected_plugin_command()); Ok(()) }, ) @@ -2876,12 +2886,10 @@ fn windows_shell_argument_quoting_and_hook_encoding_preserve_paths() { r#""C:\Program Files\NeMo 100%%cd:~,%\bin\nemo-relay.exe""# ); assert_eq!( - crate::hooks::decode_windows_hook_command(&codex_plugin_hook_command_for_platform( - &relay, - &generation, - "test-generation", - true, - )) + crate::hooks::decode_windows_hook_command( + codex_plugin_hook_command_for_platform(&relay, &generation, "test-generation", true,) + .for_event("PreToolUse") + ) .unwrap(), vec![ relay.display().to_string(), @@ -2893,6 +2901,7 @@ fn windows_shell_argument_quoting_and_hook_encoding_preserve_paths() { generation.display().to_string(), "--generation-token".into(), "test-generation".into(), + "--fail-closed".into(), ] ); assert_eq!( @@ -2913,7 +2922,10 @@ fn generated_windows_hook_command_executes_exact_arguments() { let marker = temp.path().join("hook-ran.txt"); let input_marker = temp.path().join("hook-input.txt"); let generation = temp.path().join("Generation & %USERPROFILE%"); - let command = codex_plugin_hook_command(&relay, &generation, "test-generation").unwrap(); + let command = codex_plugin_hook_command(&relay, &generation, "test-generation") + .unwrap() + .for_event("PreToolUse") + .to_owned(); let mut child = std::process::Command::new("cmd.exe") .arg("/C") .arg(&command) @@ -2950,7 +2962,10 @@ fn generated_windows_hook_command_propagates_the_relay_exit_code() { let relay = temp.path().join("relay failure.exe"); compile_windows_hook_test_relay(&relay); let generation = temp.path().join("generation"); - let command = codex_plugin_hook_command(&relay, &generation, "test-generation").unwrap(); + let command = codex_plugin_hook_command(&relay, &generation, "test-generation") + .unwrap() + .for_event("PreToolUse") + .to_owned(); let status = std::process::Command::new("cmd.exe") .arg("/C") @@ -2990,8 +3005,9 @@ fn posix_shell_argument_quoting_and_hook_encoding_preserve_paths() { "'/tmp/NeMo $Relay`test'\\''/bin/nemo-relay'" ); assert_eq!( - codex_plugin_hook_command_for_platform(&relay, &generation, "test-generation", false), - "'/tmp/NeMo $Relay`test'\\''/bin/nemo-relay' hook-forward codex --gateway-url http://127.0.0.1:47632 --generation-file '/tmp/NeMo $Relay`test'\\''/plugin/.nemo-relay-generation' --generation-token test-generation" + codex_plugin_hook_command_for_platform(&relay, &generation, "test-generation", false) + .for_event("SessionStart"), + "'/tmp/NeMo $Relay`test'\\''/bin/nemo-relay' hook-forward codex --gateway-url http://127.0.0.1:47632 --generation-file '/tmp/NeMo $Relay`test'\\''/plugin/.nemo-relay-generation' --generation-token test-generation --fail-open" ); assert_eq!(shell_quote_arg_for_platform("", false), "''"); assert_eq!( @@ -3780,7 +3796,7 @@ fn plugin_host_entrypoints_reject_unsupported_agents_and_report_json() { "SessionStart": [{ "hooks": [{ "type": "command", - "command": expected_plugin_command(), + "command": expected_plugin_command().for_event("SessionStart"), "timeout": 30 }] }] diff --git a/crates/cli/tests/coverage/agents/plugin_install_tests.rs b/crates/cli/tests/coverage/agents/plugin_install_tests.rs index 8af743d26..d20daa95b 100644 --- a/crates/cli/tests/coverage/agents/plugin_install_tests.rs +++ b/crates/cli/tests/coverage/agents/plugin_install_tests.rs @@ -1420,13 +1420,14 @@ fn plugin_manifests_and_hooks_use_path_based_relay_command() { ) .unwrap()["hooks"]["SessionStart"][0]["hooks"][0]["command"], json!( - crate::hooks::persistent_hook_forward_command( + crate::hooks::persistent_hook_forward_commands( Path::new("/bin/nemo-relay"), CodingAgent::Codex, &generation_fence, TEST_GENERATION_TOKEN, ) .unwrap() + .for_event("SessionStart") ) ); assert_eq!( @@ -1438,13 +1439,14 @@ fn plugin_manifests_and_hooks_use_path_based_relay_command() { ) .unwrap()["hooks"]["SessionStart"][0]["hooks"][0]["command"], json!( - crate::hooks::persistent_hook_forward_command( + crate::hooks::persistent_hook_forward_commands( Path::new("/bin/nemo-relay"), CodingAgent::ClaudeCode, &generation_fence, TEST_GENERATION_TOKEN, ) .unwrap() + .for_event("SessionStart") ) ); } @@ -1471,13 +1473,14 @@ fn relay_identity_prefers_the_path_resolved_executable() { ) .unwrap()["hooks"]["SessionStart"][0]["hooks"][0]["command"], json!( - crate::hooks::persistent_hook_forward_command( + crate::hooks::persistent_hook_forward_commands( &relay, CodingAgent::Codex, &generation, TEST_GENERATION_TOKEN, ) .unwrap() + .for_event("SessionStart") ) ); assert_eq!( diff --git a/crates/cli/tests/coverage/shared/installer_tests.rs b/crates/cli/tests/coverage/shared/installer_tests.rs index f77f7d74d..40d82f12d 100644 --- a/crates/cli/tests/coverage/shared/installer_tests.rs +++ b/crates/cli/tests/coverage/shared/installer_tests.rs @@ -92,7 +92,7 @@ async fn transparent_hook_delivery_authenticates_the_wrapper_gateway() { profile: None, session_metadata: None, gateway_mode: None, - fail_closed: true, + failure_policy: HookFailurePolicy::FailClosed, }; let gateway = transparent_gateway_spec(&gateway_url).unwrap(); @@ -329,42 +329,39 @@ fn helper_formatting_and_headers_cover_optional_paths() { #[test] fn generated_hook_dispatch_covers_all_agents() { - for agent in [ - CodingAgent::ClaudeCode, - CodingAgent::Codex, - CodingAgent::Hermes, - ] { - assert!(generated_hooks(agent, "cmd")["hooks"].is_object()); - } + assert_generated_hook_policies(); assert_eq!( - transparent_hook_forward_command_for_platform( + transparent_hook_forward_commands_for_platform( Path::new("nemo-relay"), CodingAgent::Hermes, "http://127.0.0.1:1234", false, - ), - "nemo-relay hook-forward hermes --gateway-url http://127.0.0.1:1234 --transparent-run" + ) + .for_event("on_session_start"), + "nemo-relay hook-forward hermes --gateway-url http://127.0.0.1:1234 --transparent-run --fail-open" ); assert_eq!( - transparent_hook_forward_command_for_platform( + transparent_hook_forward_commands_for_platform( Path::new("/abs/path/to/nemo-relay"), CodingAgent::Codex, "http://127.0.0.1:1234", false, - ), - "/abs/path/to/nemo-relay hook-forward codex --gateway-url http://127.0.0.1:1234 --transparent-run" + ) + .for_event("PreToolUse"), + "/abs/path/to/nemo-relay hook-forward codex --gateway-url http://127.0.0.1:1234 --transparent-run --fail-closed" ); let relay = Path::new("/opt/NeMo Relay's & tools/nemo-relay"); assert_eq!( - transparent_hook_forward_command_for_platform( + transparent_hook_forward_commands_for_platform( relay, CodingAgent::Codex, "http://127.0.0.1:1234", false - ), - r#"'/opt/NeMo Relay'\''s & tools/nemo-relay' hook-forward codex --gateway-url http://127.0.0.1:1234 --transparent-run"# + ) + .for_event("SessionStart"), + r#"'/opt/NeMo Relay'\''s & tools/nemo-relay' hook-forward codex --gateway-url http://127.0.0.1:1234 --transparent-run --fail-open"# ); - let native = transparent_hook_forward_command( + let native = transparent_hook_forward_commands( Path::new("nemo-relay"), CodingAgent::Hermes, "http://127.0.0.1:1234", @@ -372,7 +369,7 @@ fn generated_hook_dispatch_covers_all_agents() { .unwrap(); if cfg!(windows) { assert_eq!( - decode_windows_hook_command(&native).unwrap(), + decode_windows_hook_command(native.for_event("on_session_start")).unwrap(), vec![ String::from("nemo-relay"), String::from("hook-forward"), @@ -380,12 +377,13 @@ fn generated_hook_dispatch_covers_all_agents() { String::from("--gateway-url"), String::from("http://127.0.0.1:1234"), String::from("--transparent-run"), + String::from("--fail-open"), ] ); } else { assert_eq!( native, - transparent_hook_forward_command_for_platform( + transparent_hook_forward_commands_for_platform( Path::new("nemo-relay"), CodingAgent::Hermes, "http://127.0.0.1:1234", @@ -393,12 +391,13 @@ fn generated_hook_dispatch_covers_all_agents() { ) ); } - let windows = transparent_hook_forward_command_for_platform( + let windows = transparent_hook_forward_commands_for_platform( relay, CodingAgent::ClaudeCode, "http://127.0.0.1:1234", true, ); + let windows = windows.for_event("PreToolUse"); let (launcher, encoded) = windows.rsplit_once(' ').unwrap(); assert_eq!( launcher, @@ -412,7 +411,7 @@ fn generated_hook_dispatch_covers_all_agents() { || matches!(character, '+' | '/' | '=')) ); assert_eq!( - decode_windows_hook_command(&windows).unwrap(), + decode_windows_hook_command(windows).unwrap(), vec![ relay.display().to_string(), "hook-forward".into(), @@ -420,6 +419,7 @@ fn generated_hook_dispatch_covers_all_agents() { "--gateway-url".into(), "http://127.0.0.1:1234".into(), "--transparent-run".into(), + "--fail-closed".into(), ] ); assert!(decode_windows_hook_command("powershell.exe -EncodedCommand invalid").is_none()); @@ -446,6 +446,38 @@ fn generated_hook_dispatch_covers_all_agents() { assert!(error.contains("shorten the Relay or plugin installation path")); } +fn assert_generated_hook_policies() { + for agent in [ + CodingAgent::ClaudeCode, + CodingAgent::Codex, + CodingAgent::Hermes, + ] { + assert!(generated_hooks(agent, "cmd")["hooks"].is_object()); + let commands = GeneratedHookCommands::new("cmd --fail-open", "cmd --fail-closed"); + let generated = generated_policy_hooks(agent, &commands); + for event in agent.hook_events() { + let command = if agent.uses_direct_hook_entries() { + generated["hooks"][event][0]["command"].as_str() + } else { + generated["hooks"][event][0]["hooks"][0]["command"].as_str() + } + .unwrap(); + assert_eq!( + command, + commands.for_event(event), + "unexpected policy for {} {event}", + agent.label() + ); + assert_eq!( + command.ends_with("--fail-closed"), + event_requires_fail_closed(event), + "unexpected enforcement classification for {} {event}", + agent.label() + ); + } + } +} + #[test] fn codex_generation_uses_exactly_the_supported_hook_schema() { let generated = generated_hooks(CodingAgent::Codex, "cmd"); @@ -521,34 +553,46 @@ fn packaged_plugin_hooks_use_expected_forwarding_commands() { assert_eq!( claude["hooks"]["SessionStart"][0]["hooks"][0]["command"], json!(format!( - "nemo-relay hook-forward claude --gateway-url {} --forward-only", + "nemo-relay hook-forward claude --gateway-url {} --forward-only --fail-open", crate::bootstrap::DEFAULT_URL )) ); assert_eq!( codex["hooks"]["SessionStart"][0]["hooks"][0]["command"], json!(format!( - "nemo-relay hook-forward codex --gateway-url {} --forward-only", + "nemo-relay hook-forward codex --gateway-url {} --forward-only --fail-open", crate::bootstrap::DEFAULT_URL )) ); assert_eq!( claude["hooks"], - generated_hooks( + generated_policy_hooks( CodingAgent::ClaudeCode, - &format!( - "nemo-relay hook-forward claude --gateway-url {} --forward-only", - crate::bootstrap::DEFAULT_URL + &GeneratedHookCommands::new( + format!( + "nemo-relay hook-forward claude --gateway-url {} --forward-only --fail-open", + crate::bootstrap::DEFAULT_URL + ), + format!( + "nemo-relay hook-forward claude --gateway-url {} --forward-only --fail-closed", + crate::bootstrap::DEFAULT_URL + ), ), )["hooks"] ); assert_eq!( codex["hooks"], - generated_hooks( + generated_policy_hooks( CodingAgent::Codex, - &format!( - "nemo-relay hook-forward codex --gateway-url {} --forward-only", - crate::bootstrap::DEFAULT_URL + &GeneratedHookCommands::new( + format!( + "nemo-relay hook-forward codex --gateway-url {} --forward-only --fail-open", + crate::bootstrap::DEFAULT_URL + ), + format!( + "nemo-relay hook-forward codex --gateway-url {} --forward-only --fail-closed", + crate::bootstrap::DEFAULT_URL + ), ), )["hooks"] ); diff --git a/docs/about-nemo-relay/release-notes/index.mdx b/docs/about-nemo-relay/release-notes/index.mdx index 408d20151..a839cb127 100644 --- a/docs/about-nemo-relay/release-notes/index.mdx +++ b/docs/about-nemo-relay/release-notes/index.mdx @@ -32,6 +32,10 @@ compatibility information applies to the current 0.7 prerelease. ### Highlights +- Generated coding-agent enforcement hooks now fail closed when Relay cannot + start, authenticate, evaluate, or deliver a response. Lifecycle and + after-the-fact hooks explicitly fail open. Reinstall each coding-agent + integration with `nemo-relay install --force` after upgrading. - LLM observability sanitizers now receive the active request or response codec for each managed call. Sanitizers can normalize built-in, runtime-registered, and opaque codec payloads without changing the diff --git a/docs/nemo-relay-cli/basic-usage.mdx b/docs/nemo-relay-cli/basic-usage.mdx index 13417c964..c8907eaaa 100644 --- a/docs/nemo-relay-cli/basic-usage.mdx +++ b/docs/nemo-relay-cli/basic-usage.mdx @@ -516,9 +516,17 @@ stores the canonical absolute command and trusts only its exact event pairs; it does not enable global hook auto-acceptance. `hook-forward` reads the canonical hook payload from standard input, sends it -to the matching endpoint, and prints the endpoint response. It fails open by -default so observability outages do not block the coding agent. Add -`--fail-closed` only when policy requires hook delivery to block the agent. +to the matching endpoint, and prints the endpoint response. Generated +`PreToolUse`, `PermissionRequest`, and Hermes `pre_tool_call` hooks use +`--fail-closed`; generated lifecycle and after-the-fact hooks use +`--fail-open`. This blocks permission-bearing operations when Relay cannot +evaluate them without making observability-only hooks a runtime dependency. +Rerun `nemo-relay install --force` after upgrading to replace legacy +generated hooks. + +Manually authored commands fail open when neither policy flag is present. +`NEMO_RELAY_FAIL_CLOSED=1` changes that default for compatibility, while an +explicit flag takes precedence over the environment. These flags control delivery and metadata: @@ -530,9 +538,10 @@ These flags control delivery and metadata: - `--session-metadata` sets `x-nemo-relay-session-metadata`. - `--profile` sets `x-nemo-relay-config-profile`. - `--gateway-mode` sets `x-nemo-relay-gateway-mode`. +- `--fail-open` allows the agent to continue after a delivery failure, even + when `NEMO_RELAY_FAIL_CLOSED=1` is set. - `--fail-closed` returns a failure when delivery fails or Relay rejects the - hook. Without it, forwarding fails open so an observability outage does not - block the coding agent. + hook. ## Agent Guides diff --git a/integrations/coding-agents/README.md b/integrations/coding-agents/README.md index 20ab6d57f..0ef4d79e9 100644 --- a/integrations/coding-agents/README.md +++ b/integrations/coding-agents/README.md @@ -262,14 +262,16 @@ URL. For Codex, the installed plugin file is the sole persistent Relay hook source; installation does not add Relay groups to `~/.codex/hooks.json`. -Since hook forwarding fails open by default, gateway or sidecar outages do not -block the coding agent. The hook command exits successfully after logging the -forwarding problem, so the host agent can continue even though that hook -payload can be missing from telemetry. For wrapper-generated `hook-forward` -commands, add `--fail-closed` when policy requires hook delivery to block the -agent. For generated persistent hooks, set `NEMO_RELAY_FAIL_CLOSED=1` in the hook -execution environment. In that mode, forwarding failures return a non-zero -hook command status to the host. +Generated hooks select an explicit failure policy by event. `PreToolUse`, +`PermissionRequest`, and Hermes `pre_tool_call` hooks use `--fail-closed`, so +Relay startup, authentication, delivery, and response failures block the +permission-bearing operation. Lifecycle and after-the-fact hooks use +`--fail-open`, so observability outages do not block unrelated agent work. + +After upgrading, rerun `nemo-relay install --force` to replace legacy +generated hooks that did not carry an explicit policy. Manually authored +`hook-forward` commands still fail open by default; set +`NEMO_RELAY_FAIL_CLOSED=1` or add `--fail-closed` when they enforce policy. These `hook-forward` options control delivery and metadata: @@ -284,6 +286,8 @@ These `hook-forward` options control delivery and metadata: - `--profile ` records a configuration profile in session metadata. - `--gateway-mode hook-only|passthrough|required` records the expected gateway behavior in session metadata. +- `--fail-open` allows the coding agent to continue after a delivery failure, + even when `NEMO_RELAY_FAIL_CLOSED=1` is set. - `--fail-closed` returns a failure when delivery fails or Relay rejects the hook instead of allowing the coding agent to continue. diff --git a/integrations/coding-agents/claude-code/hooks/hooks.json b/integrations/coding-agents/claude-code/hooks/hooks.json index 73cb06ef4..7524fa193 100644 --- a/integrations/coding-agents/claude-code/hooks/hooks.json +++ b/integrations/coding-agents/claude-code/hooks/hooks.json @@ -5,7 +5,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -16,7 +16,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -27,7 +27,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -39,7 +39,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-closed", "timeout": 30 } ] @@ -51,7 +51,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -63,7 +63,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -75,7 +75,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-closed", "timeout": 30 } ] @@ -86,7 +86,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -97,7 +97,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -108,7 +108,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -119,7 +119,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -130,7 +130,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -141,7 +141,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -152,7 +152,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] diff --git a/integrations/coding-agents/codex/hooks/hooks.json b/integrations/coding-agents/codex/hooks/hooks.json index 550a462bc..a20626ad0 100644 --- a/integrations/coding-agents/codex/hooks/hooks.json +++ b/integrations/coding-agents/codex/hooks/hooks.json @@ -6,7 +6,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -17,7 +17,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -29,7 +29,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-closed", "timeout": 30 } ] @@ -41,7 +41,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -53,7 +53,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-closed", "timeout": 30 } ] @@ -64,7 +64,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -75,7 +75,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -86,7 +86,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -97,7 +97,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -108,7 +108,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] From eae8b35c8a2d6c721d586176a36e7c914e499df8 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Tue, 4 Aug 2026 12:17:46 -0400 Subject: [PATCH 2/3] test: update Windows hook fixture policy Signed-off-by: Will Killian --- crates/cli/tests/fixtures/windows_hook_relay.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/cli/tests/fixtures/windows_hook_relay.rs b/crates/cli/tests/fixtures/windows_hook_relay.rs index e8a03b5d0..cedd3c885 100644 --- a/crates/cli/tests/fixtures/windows_hook_relay.rs +++ b/crates/cli/tests/fixtures/windows_hook_relay.rs @@ -16,6 +16,7 @@ fn main() { generation, OsString::from("--generation-token"), OsString::from("test-generation"), + OsString::from("--fail-closed"), ]; let actual = std::env::args_os().skip(1).collect::>(); if actual != expected { From ec6fdc6b4db495ba663431300a80ac955691cf64 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Tue, 4 Aug 2026 15:12:37 -0400 Subject: [PATCH 3/3] fix: address hook policy review feedback Signed-off-by: Will Killian --- crates/cli/src/agents/codex/host.rs | 8 +++- crates/cli/src/agents/hermes/integration.rs | 7 +-- crates/cli/src/agents/hermes/trust.rs | 4 +- crates/cli/src/hooks/encoding.rs | 1 - .../cli/tests/coverage/agents/hermes_tests.rs | 48 +++++++++++++++++++ .../coverage/agents/plugin_host_tests.rs | 27 +++++++++++ docs/nemo-relay-cli/basic-usage.mdx | 3 +- 7 files changed, 90 insertions(+), 8 deletions(-) diff --git a/crates/cli/src/agents/codex/host.rs b/crates/cli/src/agents/codex/host.rs index 7871ac2c1..ffaeb4b51 100644 --- a/crates/cli/src/agents/codex/host.rs +++ b/crates/cli/src/agents/codex/host.rs @@ -326,8 +326,12 @@ fn relay_codex_hooks( let hooks = relay_codex_plugin_hooks(client, cwd)? .into_iter() .filter(|hook| { - hook.command.as_deref() - == expected_codex_hook_command(expected_commands, &hook.event_name) + expected_codex_hook_command(expected_commands, &hook.event_name).is_some_and( + |expected| { + hook.command.as_deref() == Some(expected) + || hook.command.as_deref() == expected_commands.legacy() + }, + ) }) .collect::>(); validate_loaded_hook_sources(&hooks, expected_commands)?; diff --git a/crates/cli/src/agents/hermes/integration.rs b/crates/cli/src/agents/hermes/integration.rs index 115c04ca0..e1ea8c2b7 100644 --- a/crates/cli/src/agents/hermes/integration.rs +++ b/crates/cli/src/agents/hermes/integration.rs @@ -481,9 +481,10 @@ where .get("command") .and_then(Value::as_str) .is_none_or(|command| { - owned - .as_ref() - .is_none_or(|commands| !commands.contains(command)) + owned.as_ref().map_or_else( + || !is_persistent_relay_hook_command(command), + |commands| !commands.contains(command), + ) }) }); if approvals.is_empty() { diff --git a/crates/cli/src/agents/hermes/trust.rs b/crates/cli/src/agents/hermes/trust.rs index f6ae969ad..4dd529dff 100644 --- a/crates/cli/src/agents/hermes/trust.rs +++ b/crates/cli/src/agents/hermes/trust.rs @@ -97,7 +97,9 @@ pub(super) fn verify_trust( .get("event") .and_then(Value::as_str) .ok_or_else(|| "Hermes Relay hook approval is missing its event".to_string())?; - if !CodingAgent::Hermes.hook_events().contains(&event) { + if !CodingAgent::Hermes.hook_events().contains(&event) + || command != commands.for_event(event) + { return Err("Hermes allowlist contains an unexpected Relay hook approval".into()); } } diff --git a/crates/cli/src/hooks/encoding.rs b/crates/cli/src/hooks/encoding.rs index aee4d51bc..f9eaf5113 100644 --- a/crates/cli/src/hooks/encoding.rs +++ b/crates/cli/src/hooks/encoding.rs @@ -52,7 +52,6 @@ impl GeneratedHookCommands { || self.legacy.as_deref() == Some(command) } - #[cfg(test)] pub(crate) fn legacy(&self) -> Option<&str> { self.legacy.as_deref() } diff --git a/crates/cli/tests/coverage/agents/hermes_tests.rs b/crates/cli/tests/coverage/agents/hermes_tests.rs index a9e7579ea..ede96d7fc 100644 --- a/crates/cli/tests/coverage/agents/hermes_tests.rs +++ b/crates/cli/tests/coverage/agents/hermes_tests.rs @@ -566,6 +566,19 @@ fn verification_rejects_relay_handlers_and_approvals_on_unexpected_events() { std::fs::write(&path, serde_json::to_vec(&missing_event).unwrap()).unwrap(); let error = verify_trust(&path, &command).unwrap_err(); assert!(error.contains("missing its event")); + + let mut wrong_policy = trusted_hooks(None, None, &command, &relay, UNIX_EPOCH).unwrap(); + wrong_policy["approvals"] + .as_array_mut() + .unwrap() + .push(json!({ + "event": "pre_tool_call", + "command": command.for_event("on_session_start"), + "approved_at": "1970-01-01T00:00:00.000000Z" + })); + std::fs::write(&path, serde_json::to_vec(&wrong_policy).unwrap()).unwrap(); + let error = verify_trust(&path, &command).unwrap_err(); + assert!(error.contains("unexpected Relay hook approval")); } #[test] @@ -1003,6 +1016,41 @@ fn uninstall_removes_only_relay_owned_hermes_state() { assert_eq!(allowlist["approvals"][0]["command"], json!("custom-hook")); } +#[test] +fn uninstall_removes_orphaned_generated_approval_without_config() { + let temp = tempfile::tempdir().unwrap(); + let relay = relay_binary(temp.path()); + let paths = paths(&temp.path().join("hermes")); + let commands = + persistent_hook_commands(&relay, &paths.generation, TEST_GENERATION_TOKEN).unwrap(); + std::fs::create_dir_all(paths.allowlist.parent().unwrap()).unwrap(); + std::fs::write( + &paths.allowlist, + serde_json::to_vec(&json!({ + "approvals": [ + { + "event": "pre_tool_call", + "command": commands.for_event("pre_tool_call") + }, + { + "event": "custom_event", + "command": "custom-hook" + } + ] + })) + .unwrap(), + ) + .unwrap(); + + let removed = uninstall_persistent_with(paths.clone(), atomic_write).unwrap(); + + assert_eq!(removed, vec![paths.allowlist.clone()]); + assert_eq!( + json_file(&paths.allowlist)["approvals"], + json!([{"event": "custom_event", "command": "custom-hook"}]) + ); +} + #[test] fn uninstall_rolls_back_every_file_when_commit_fails() { let temp = tempfile::tempdir().unwrap(); diff --git a/crates/cli/tests/coverage/agents/plugin_host_tests.rs b/crates/cli/tests/coverage/agents/plugin_host_tests.rs index f231c8f27..9d6b15447 100644 --- a/crates/cli/tests/coverage/agents/plugin_host_tests.rs +++ b/crates/cli/tests/coverage/agents/plugin_host_tests.rs @@ -1024,6 +1024,33 @@ fn codex_hook_trust_report_distinguishes_modified_disabled_and_missing_hooks() { ); } +#[test] +fn codex_hook_trust_report_recognizes_legacy_generated_hooks_as_stale() { + let dir = tempdir().unwrap(); + let hooks_path = dir.path().join(".codex").join("hooks.json"); + let expected = expected_plugin_command(); + let legacy = expected.legacy().unwrap().to_owned(); + let mut hooks = required_codex_hook_metadata(&hooks_path, "trusted", true); + for hook in &mut hooks { + hook.command = Some(legacy.clone()); + } + fs::write( + &hooks_path, + serde_json::to_vec_pretty(&generated_hooks(CodingAgent::Codex, &legacy)).unwrap(), + ) + .unwrap(); + let mut client = FakeCodexHooksClient { + hook_lists: VecDeque::from([Ok(hooks)]), + ..FakeCodexHooksClient::default() + }; + + let error = codex_hook_trust_report_with_client(&mut client, dir.path(), &expected) + .expect_err("legacy generated hooks must require reinstall"); + + assert!(error.contains("loaded modified Relay hooks"), "{error}"); + assert!(error.contains("install codex --force"), "{error}"); +} + #[test] fn codex_hook_state_key_path_quotes_arbitrary_hook_identity() { assert_eq!( diff --git a/docs/nemo-relay-cli/basic-usage.mdx b/docs/nemo-relay-cli/basic-usage.mdx index c8907eaaa..55d9b4e8a 100644 --- a/docs/nemo-relay-cli/basic-usage.mdx +++ b/docs/nemo-relay-cli/basic-usage.mdx @@ -539,7 +539,8 @@ These flags control delivery and metadata: - `--profile` sets `x-nemo-relay-config-profile`. - `--gateway-mode` sets `x-nemo-relay-gateway-mode`. - `--fail-open` allows the agent to continue after a delivery failure, even - when `NEMO_RELAY_FAIL_CLOSED=1` is set. + when `NEMO_RELAY_FAIL_CLOSED=1` is set. Structured guardrail rejections still + return a failure. - `--fail-closed` returns a failure when delivery fails or Relay rejects the hook.