From e53f8b0ef1bc775155195970104d17f1742ed8c9 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Fri, 28 Aug 2026 12:20:18 -0400 Subject: [PATCH 01/16] fix: use private files for generated hook commands Signed-off-by: Will Killian --- crates/cli/src/agents/claude/launch.rs | 10 +- crates/cli/src/agents/codex/host.rs | 8 +- crates/cli/src/agents/codex/launch.rs | 18 +- crates/cli/src/agents/mod.rs | 14 + crates/cli/src/commands/hook_forward.rs | 17 ++ crates/cli/src/hooks/config.rs | 135 +++++++++ crates/cli/src/hooks/delivery.rs | 19 +- crates/cli/src/hooks/encoding.rs | 259 +++--------------- crates/cli/src/hooks/mod.rs | 10 +- crates/cli/src/hooks/types.rs | 17 +- .../src/installation/marketplace/assets.rs | 13 +- .../cli/src/installation/marketplace/mod.rs | 2 + .../cli/src/installation/marketplace/spec.rs | 6 + .../cli/src/installation/marketplace/state.rs | 3 + .../tests/coverage/shared/installer_tests.rs | 124 ++++----- docs/nemo-relay-cli/plugin-installation.mdx | 20 +- 16 files changed, 342 insertions(+), 333 deletions(-) create mode 100644 crates/cli/src/hooks/config.rs diff --git a/crates/cli/src/agents/claude/launch.rs b/crates/cli/src/agents/claude/launch.rs index 4c67e89cc..b9ec83aca 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_policy_hooks, transparent_hook_forward_commands}; +use crate::hooks::{generated_policy_hooks, transparent_hook_forward_commands_with_config}; use crate::process::{PreparedAgentLaunch, insert_after_host}; pub(crate) fn prepare( @@ -64,10 +64,14 @@ pub(crate) fn prepare( })) .map_err(|error| CliError::Launch(error.to_string()))?, )?; - let hook_commands = transparent_hook_forward_commands( + let hook_config = root.join(".nemo-relay-hook-config.json"); + crate::hooks::HookCommandConfig::transparent(CodingAgent::ClaudeCode, gateway_url) + .write(&hook_config) + .map_err(CliError::Launch)?; + let hook_commands = transparent_hook_forward_commands_with_config( &transparent_hook_executable(), CodingAgent::ClaudeCode, - gateway_url, + &hook_config, ) .map_err(CliError::Launch)?; write_hooks( diff --git a/crates/cli/src/agents/codex/host.rs b/crates/cli/src/agents/codex/host.rs index d634b5b3e..0c66f7725 100644 --- a/crates/cli/src/agents/codex/host.rs +++ b/crates/cli/src/agents/codex/host.rs @@ -1800,13 +1800,13 @@ pub(crate) fn codex_hook_command(gateway_url: &str) -> String { pub(crate) fn codex_plugin_hook_command( relay: &Path, generation: &Path, - generation_token: &str, + _generation_token: &str, ) -> Result { crate::hooks::persistent_hook_forward_commands( relay, CodingAgent::Codex, generation, - generation_token, + _generation_token, ) } @@ -1814,14 +1814,14 @@ pub(crate) fn codex_plugin_hook_command( pub(crate) fn codex_plugin_hook_command_for_platform( relay: &Path, generation: &Path, - generation_token: &str, + _generation_token: &str, windows: bool, ) -> crate::hooks::GeneratedHookCommands { crate::hooks::persistent_hook_forward_commands_for_platform( relay, CodingAgent::Codex, generation, - generation_token, + _generation_token, windows, ) } diff --git a/crates/cli/src/agents/codex/launch.rs b/crates/cli/src/agents/codex/launch.rs index 76c8d4195..b59f485bd 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_policy_hooks, transparent_hook_forward_commands}; +use crate::hooks::{generated_policy_hooks, transparent_hook_forward_commands_with_config}; use crate::process::PreparedAgentLaunch; pub(crate) fn prepare(launch: &mut PreparedAgentLaunch, gateway_url: &str) -> Result<(), CliError> { @@ -26,10 +26,16 @@ 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_commands = transparent_hook_forward_commands( + let hook_root = temp_dir("nemo-relay-codex-hooks")?; + let hook_config = hook_root.join(".nemo-relay-hook-config.json"); + crate::hooks::HookCommandConfig::transparent(CodingAgent::Codex, gateway_url) + .write(&hook_config) + .map_err(CliError::Launch)?; + launch.temp_dirs.push(hook_root); + let hook_commands = transparent_hook_forward_commands_with_config( &transparent_hook_executable(), CodingAgent::Codex, - gateway_url, + &hook_config, ) .map_err(CliError::Launch)?; let hook_groups = generated_policy_hooks(CodingAgent::Codex, &hook_commands); @@ -324,3 +330,9 @@ fn transparent_hook_executable() -> PathBuf { .map(crate::agents::portable_executable_path) .unwrap_or_else(|_| PathBuf::from("nemo-relay")) } + +fn temp_dir(prefix: &str) -> Result { + let path = std::env::temp_dir().join(format!("{prefix}-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&path)?; + Ok(path) +} diff --git a/crates/cli/src/agents/mod.rs b/crates/cli/src/agents/mod.rs index b38559b53..58e1750aa 100644 --- a/crates/cli/src/agents/mod.rs +++ b/crates/cli/src/agents/mod.rs @@ -259,11 +259,25 @@ impl crate::installation::marketplace::MarketplaceHost for CodingAgent { plugin_mcp_config(self, server) } + fn persistent_hook_config( + self, + generation_fence: &std::path::Path, + generation_token: &str, + ) -> crate::hooks::HookCommandConfig { + crate::hooks::HookCommandConfig::persistent( + self, + crate::bootstrap::DEFAULT_URL, + generation_fence.to_owned(), + generation_token, + ) + } + fn plugin_hooks( self, relay: &std::path::Path, generation_fence: &std::path::Path, generation_token: &str, + _hook_config: &std::path::Path, ) -> Result { let commands = crate::hooks::persistent_hook_forward_commands( relay, diff --git a/crates/cli/src/commands/hook_forward.rs b/crates/cli/src/commands/hook_forward.rs index 14fdf5770..2deb5e455 100644 --- a/crates/cli/src/commands/hook_forward.rs +++ b/crates/cli/src/commands/hook_forward.rs @@ -13,6 +13,22 @@ pub(crate) struct HookForwardCommand { /// Coding agent whose canonical lifecycle payload is read from standard input. #[arg(value_enum)] pub(crate) agent: AgentArg, + /// Private Relay-owned configuration used by generated coding-agent hooks. + #[arg( + long, + hide = true, + conflicts_with_all = [ + "gateway_url", + "generation_file", + "generation_token", + "forward_only", + "transparent_run", + "profile", + "session_metadata", + "gateway_mode" + ] + )] + pub(crate) hook_config: Option, /// Base URL of the Relay gateway that receives the lifecycle payload. #[arg(long)] pub(crate) gateway_url: Option, @@ -56,6 +72,7 @@ impl HookForwardCommand { fn into_runtime(self) -> crate::hooks::HookForwardRequest { crate::hooks::HookForwardRequest { agent: self.agent.into(), + hook_config: self.hook_config, gateway_url: self.gateway_url, generation_file: self.generation_file, generation_token: self.generation_token, diff --git a/crates/cli/src/hooks/config.rs b/crates/cli/src/hooks/config.rs new file mode 100644 index 000000000..a8ebee3b6 --- /dev/null +++ b/crates/cli/src/hooks/config.rs @@ -0,0 +1,135 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Private, installer-owned configuration for generated coding-agent hooks. + +use std::path::{Path, PathBuf}; + +use serde::{Deserialize, Serialize}; + +use crate::agents::CodingAgent; + +use super::{GatewayMode, HookForwardRequest}; + +const HOOK_CONFIG_VERSION: u32 = 1; + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub(crate) struct HookCommandConfig { + version: u32, + agent: String, + gateway_url: String, + generation_file: Option, + generation_token: Option, + forward_only: bool, + transparent_run: bool, + profile: Option, + session_metadata: Option, + gateway_mode: Option, +} + +impl HookCommandConfig { + pub(crate) fn persistent( + agent: CodingAgent, + gateway_url: impl Into, + generation_file: PathBuf, + generation_token: impl Into, + ) -> Self { + Self { + version: HOOK_CONFIG_VERSION, + agent: agent.as_arg().into(), + gateway_url: gateway_url.into(), + generation_file: Some(generation_file), + generation_token: Some(generation_token.into()), + forward_only: false, + transparent_run: false, + profile: None, + session_metadata: None, + gateway_mode: None, + } + } + + pub(crate) fn transparent(agent: CodingAgent, gateway_url: impl Into) -> Self { + Self { + version: HOOK_CONFIG_VERSION, + agent: agent.as_arg().into(), + gateway_url: gateway_url.into(), + generation_file: None, + generation_token: None, + forward_only: false, + transparent_run: true, + profile: None, + session_metadata: None, + gateway_mode: None, + } + } + + pub(crate) fn write(&self, path: &Path) -> Result<(), String> { + let bytes = serde_json::to_vec_pretty(self) + .map_err(|error| format!("failed to serialize hook configuration: {error}"))?; + crate::filesystem::atomic_write_private(path, &bytes) + } + + pub(crate) fn load(path: &Path) -> Result { + let bytes = std::fs::read(path).map_err(|error| { + format!( + "failed to read hook configuration {}: {error}", + path.display() + ) + })?; + let config = serde_json::from_slice::(&bytes).map_err(|error| { + format!( + "failed to parse hook configuration {}: {error}", + path.display() + ) + })?; + config.validate()?; + Ok(config) + } + + pub(crate) fn apply(self, request: &mut HookForwardRequest) -> Result<(), String> { + if self.agent != request.agent.as_arg() { + return Err(format!( + "hook configuration is for {} but the command requested {}", + self.agent, + request.agent.as_arg() + )); + } + if request.has_inline_configuration() { + return Err( + "--hook-config cannot be combined with inline hook configuration options".into(), + ); + } + request.gateway_url = Some(self.gateway_url); + request.generation_file = self.generation_file; + request.generation_token = self.generation_token; + request.forward_only = self.forward_only; + request.transparent_run = self.transparent_run; + request.profile = self.profile; + request.session_metadata = self.session_metadata; + request.gateway_mode = self.gateway_mode; + Ok(()) + } + + fn validate(&self) -> Result<(), String> { + if self.version != HOOK_CONFIG_VERSION { + return Err(format!( + "unsupported hook configuration version {}; expected {HOOK_CONFIG_VERSION}", + self.version + )); + } + if self.agent.trim().is_empty() || self.gateway_url.trim().is_empty() { + return Err("hook configuration requires an agent and gateway URL".into()); + } + if self.generation_file.is_some() != self.generation_token.is_some() { + return Err("hook configuration must include both generation file and token".into()); + } + if self.forward_only && (self.generation_file.is_some() || self.transparent_run) { + return Err("forward-only hook configuration cannot include a generation fence or transparent mode".into()); + } + if self.transparent_run && self.generation_file.is_some() { + return Err("transparent hook configuration cannot include a generation fence".into()); + } + Ok(()) + } +} diff --git a/crates/cli/src/hooks/delivery.rs b/crates/cli/src/hooks/delivery.rs index 232c362c8..db1578fb3 100644 --- a/crates/cli/src/hooks/delivery.rs +++ b/crates/cli/src/hooks/delivery.rs @@ -20,16 +20,25 @@ use super::{GatewayMode, HookForwardRequest}; const HOOK_FORWARD_TIMEOUT: Duration = Duration::from_secs(2); -pub(crate) async fn hook_forward(command: HookForwardRequest) -> Result<(), CliError> { +pub(crate) async fn hook_forward(mut command: HookForwardRequest) -> Result<(), CliError> { + let fail_closed = command.failure_policy.fail_closed(); + if let Some(path) = command.hook_config.clone() + && let Err(error) = + super::HookCommandConfig::load(&path).and_then(|config| config.apply(&mut command)) + { + return handle_hook_error(CliError::Launch(error), fail_closed); + } + if let Err(error) = + validate_optional_json("session metadata", command.session_metadata.as_deref()) + { + return handle_hook_error(error, fail_closed); + } // A transparent wrapper can coexist with any installed Relay plugin. Its process marker makes // persistent plugin hooks inert, while only the wrapper-owned command carries - // `--transparent-run` and forwards to the process-private gateway. This avoids rewriting host - // plugin settings and works for both installer and source-marketplace plugin identities. + // `--transparent-run` and forwards to the process-private gateway. if transparent_run_active() && !command.transparent_run { return Ok(()); } - validate_optional_json("session metadata", command.session_metadata.as_deref())?; - 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 a704f2677..bb9a3a010 100644 --- a/crates/cli/src/hooks/encoding.rs +++ b/crates/cli/src/hooks/encoding.rs @@ -9,9 +9,6 @@ use serde_json::{Value, json}; 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::new(command, command)) @@ -53,27 +50,33 @@ pub(crate) fn generated_policy_hooks( grouped_hooks(agent.hook_events(), commands) } -/// Canonical persistent hook command used by every supported host. pub(crate) fn persistent_hook_forward_commands( relay: &Path, agent: CodingAgent, generation_file: &Path, - generation_token: &str, + _generation_token: &str, ) -> Result { hook_commands( relay, - &persistent_hook_arguments(agent, generation_file, generation_token), + &hook_config_arguments(agent, &config_path(generation_file)), ) } -/// 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. +#[cfg(test)] pub(crate) fn transparent_hook_forward_commands( relay: &Path, agent: CodingAgent, gateway_url: &str, ) -> Result { - hook_commands(relay, &transparent_hook_arguments(agent, gateway_url)) + hook_commands(relay, &hook_config_arguments(agent, Path::new(gateway_url))) +} + +pub(crate) fn transparent_hook_forward_commands_with_config( + relay: &Path, + agent: CodingAgent, + hook_config: &Path, +) -> Result { + hook_commands(relay, &hook_config_arguments(agent, hook_config)) } #[cfg(test)] @@ -85,7 +88,7 @@ pub(crate) fn transparent_hook_forward_commands_for_platform( ) -> GeneratedHookCommands { hook_commands_for_platform( relay, - &transparent_hook_arguments(agent, gateway_url), + &hook_config_arguments(agent, Path::new(gateway_url)), windows, ) } @@ -95,40 +98,31 @@ pub(crate) fn persistent_hook_forward_commands_for_platform( relay: &Path, agent: CodingAgent, generation_file: &Path, - generation_token: &str, + _generation_token: &str, windows: bool, ) -> GeneratedHookCommands { hook_commands_for_platform( relay, - &persistent_hook_arguments(agent, generation_file, generation_token), + &hook_config_arguments(agent, &config_path(generation_file)), windows, ) } -pub(super) fn transparent_hook_arguments(agent: CodingAgent, gateway_url: &str) -> Vec { - vec![ - "hook-forward".into(), - agent.as_arg().into(), - "--gateway-url".into(), - gateway_url.into(), - "--transparent-run".into(), - ] +fn config_path(generation_file: &Path) -> std::path::PathBuf { + generation_file.with_file_name(".nemo-relay-hook-config.json") } -pub(super) fn persistent_hook_arguments( - agent: CodingAgent, - generation_file: &Path, - generation_token: &str, -) -> Vec { +#[cfg(test)] +pub(crate) fn decode_windows_hook_command(_command: &str) -> Option> { + None +} + +pub(super) fn hook_config_arguments(agent: CodingAgent, hook_config: &Path) -> Vec { vec![ "hook-forward".into(), agent.as_arg().into(), - "--gateway-url".into(), - crate::bootstrap::DEFAULT_URL.into(), - "--generation-file".into(), - generation_file.display().to_string(), - "--generation-token".into(), - generation_token.into(), + "--hook-config".into(), + hook_config.display().to_string(), ] } @@ -172,14 +166,15 @@ fn with_failure_policy(arguments: &[String], policy: &str) -> Vec { } pub(super) fn hook_command(relay: &Path, arguments: &[String]) -> Result { + let command = render_hook_command(relay, arguments, cfg!(windows)); #[cfg(windows)] - { - return encoded_windows_hook_command(&windows_powershell_launcher()?, relay, arguments); - } - #[cfg(not(windows))] - { - Ok(posix_hook_command(relay, arguments)) + if command.encode_utf16().count() > MAX_WINDOWS_HOOK_COMMAND_UTF16_UNITS { + return Err(format!( + "generated Windows coding-agent hook command is {} characters and exceeds the {MAX_WINDOWS_HOOK_COMMAND_UTF16_UNITS}-character safety limit; shorten the Relay or hook configuration path", + command.encode_utf16().count() + )); } + Ok(command) } #[cfg(test)] @@ -188,193 +183,21 @@ pub(super) fn hook_command_for_platform( arguments: &[String], windows: bool, ) -> String { - if windows { - return encoded_windows_hook_command( - "C:/Windows/System32/WindowsPowerShell/v1.0/powershell.exe", - relay, - arguments, - ) - .expect("test hook command must fit within the Windows command-line limit"); - } - posix_hook_command(relay, arguments) + render_hook_command(relay, arguments, windows) } -#[cfg(any(not(windows), test))] -pub(super) fn posix_hook_command(relay: &Path, arguments: &[String]) -> String { +fn render_hook_command(relay: &Path, arguments: &[String], windows: bool) -> String { std::iter::once(relay.display().to_string()) .chain(arguments.iter().cloned()) - .map(|argument| crate::agents::shell_quote_arg_for_platform(&argument, false)) + .map(|argument| crate::agents::shell_quote_arg_for_platform(&argument, windows)) .collect::>() .join(" ") } -// `cmd.exe` accepts at most 8,191 characters. Leave room for `/C` and the executable path added -// by the hook host instead of generating a command that will be truncated at runtime. -#[cfg(any(windows, test))] -const MAX_WINDOWS_HOOK_COMMAND_UTF16_UNITS: usize = 8_000; - -/// Encode a native Relay invocation so Windows hook hosts can pass it through `cmd.exe /C` as one -/// argument without corrupting quotes in canonical paths. Windows PowerShell is part of the -/// supported Windows platform; it only launches the Rust binary and preserves its standard I/O. -#[cfg(any(windows, test))] -pub(crate) fn encoded_windows_hook_command( - powershell: &str, - relay: &Path, - arguments: &[String], -) -> Result { - const PREFIX: &str = "$ErrorActionPreference='Stop'; & "; - const SUFFIX: &str = "; if ($null -eq $LASTEXITCODE) { exit 1 }; exit $LASTEXITCODE"; - - let invocation = std::iter::once(relay.display().to_string()) - .chain(arguments.iter().cloned()) - .map(|argument| format!("'{}'", argument.replace('\'', "''"))) - .collect::>() - .join(" "); - let script = format!("{PREFIX}{invocation}{SUFFIX}"); - let bytes = script - .encode_utf16() - .flat_map(u16::to_le_bytes) - .collect::>(); - let encoded = base64::engine::general_purpose::STANDARD.encode(bytes); - let command = - format!("{powershell} -NoLogo -NoProfile -NonInteractive -EncodedCommand {encoded}"); - if command.encode_utf16().count() > MAX_WINDOWS_HOOK_COMMAND_UTF16_UNITS { - return Err(format!( - "generated Windows coding-agent hook command exceeds the {MAX_WINDOWS_HOOK_COMMAND_UTF16_UNITS}-character safety limit; shorten the Relay or plugin installation path" - )); - } - Ok(command) -} - +// `cmd.exe` accepts at most 8,191 characters. Leave room for `/C` and host-added text. #[cfg(windows)] -pub(super) fn windows_powershell_launcher() -> Result { - let powershell = windows_powershell_path()?; - if !Path::new(&powershell).is_file() { - return Err(format!( - "trusted Windows PowerShell launcher is missing at {powershell}; install Windows PowerShell before configuring coding-agent hooks" - )); - } - Ok(powershell) -} - -#[cfg(windows)] -pub(crate) fn windows_powershell_path() -> Result { - use std::os::windows::ffi::OsStringExt; - use windows_sys::Win32::System::SystemInformation::GetSystemDirectoryW; - - let mut buffer = vec![0_u16; 260]; - let length = loop { - // SAFETY: `buffer` is writable for its declared length and remains live for the call. - let length = unsafe { GetSystemDirectoryW(buffer.as_mut_ptr(), buffer.len() as u32) }; - if length == 0 { - return Err(format!( - "failed to resolve the trusted Windows system directory: {}", - std::io::Error::last_os_error() - )); - } - if (length as usize) < buffer.len() { - break length as usize; - } - buffer.resize(length as usize + 1, 0); - }; - let system = std::path::PathBuf::from(std::ffi::OsString::from_wide(&buffer[..length])); - let powershell = system.join("WindowsPowerShell/v1.0/powershell.exe"); - let powershell = powershell - .into_os_string() - .into_string() - .map_err(|_| "trusted Windows PowerShell path is not valid Unicode".to_string())? - .replace('\\', "/"); - if !safe_windows_launcher_token(&powershell) { - return Err(format!( - "trusted Windows PowerShell path {powershell} contains characters that cannot be represented safely in coding-agent hook commands" - )); - } - Ok(powershell) -} - -#[cfg(any(windows, test))] -pub(super) fn safe_windows_launcher_token(launcher: &str) -> bool { - !launcher.is_empty() - && launcher.chars().all(|character| { - character.is_ascii_alphanumeric() || matches!(character, '/' | ':' | '.' | '_' | '-') - }) - && launcher - .to_ascii_lowercase() - .ends_with("/system32/windowspowershell/v1.0/powershell.exe") -} - -/// Decode only the exact PowerShell envelope emitted by [`encoded_windows_hook_command`]. -#[cfg(test)] -pub(crate) fn decode_windows_hook_command(command: &str) -> Option> { - const COMMAND_SEPARATOR: &str = " -NoLogo -NoProfile -NonInteractive -EncodedCommand "; - const SCRIPT_PREFIX: &str = "$ErrorActionPreference='Stop'; & "; - const SCRIPT_SUFFIX: &str = "; if ($null -eq $LASTEXITCODE) { exit 1 }; exit $LASTEXITCODE"; - - if command.encode_utf16().count() > MAX_WINDOWS_HOOK_COMMAND_UTF16_UNITS { - return None; - } - let (launcher, encoded) = command.split_once(COMMAND_SEPARATOR)?; - if !safe_windows_launcher_token(launcher) { - return None; - } - #[cfg(windows)] - if !launcher.eq_ignore_ascii_case(&windows_powershell_path().ok()?) { - return None; - } - if encoded.is_empty() || encoded.chars().any(char::is_whitespace) { - return None; - } - let bytes = base64::engine::general_purpose::STANDARD - .decode(encoded) - .ok()?; - let pairs = bytes.chunks_exact(2); - if !pairs.remainder().is_empty() { - return None; - } - let script = String::from_utf16( - &pairs - .map(|pair| u16::from_le_bytes([pair[0], pair[1]])) - .collect::>(), - ) - .ok()?; - let invocation = script - .strip_prefix(SCRIPT_PREFIX)? - .strip_suffix(SCRIPT_SUFFIX)?; - parse_powershell_single_quoted_arguments(invocation) -} - -#[cfg(test)] -pub(super) fn parse_powershell_single_quoted_arguments(mut raw: &str) -> Option> { - let mut arguments = Vec::new(); - while !raw.is_empty() { - raw = raw.strip_prefix('\'')?; - let mut argument = String::new(); - loop { - let quote = raw.find('\'')?; - argument.push_str(&raw[..quote]); - raw = &raw[quote + 1..]; - if let Some(rest) = raw.strip_prefix('\'') { - argument.push('\''); - raw = rest; - } else { - break; - } - } - arguments.push(argument); - if raw.is_empty() { - break; - } - raw = raw.strip_prefix(' ')?; - if raw.is_empty() { - return None; - } - } - (!arguments.is_empty()).then_some(arguments) -} +const MAX_WINDOWS_HOOK_COMMAND_UTF16_UNITS: usize = 8_000; -// 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. fn grouped_hooks(events: &[&str], commands: &GeneratedHookCommands) -> Value { let hooks: serde_json::Map = events .iter() @@ -385,11 +208,7 @@ fn grouped_hooks(events: &[&str], commands: &GeneratedHookCommands) -> Value { } group.insert( "hooks".into(), - json!([{ - "type": "command", - "command": commands.for_event(event), - "timeout": 30 - }]), + json!([{"type": "command", "command": commands.for_event(event), "timeout": 30}]), ); ( (*event).to_string(), @@ -400,8 +219,6 @@ fn grouped_hooks(events: &[&str], commands: &GeneratedHookCommands) -> Value { json!({ "hooks": Value::Object(hooks) }) } -// Identifies hook events that should receive wildcard tool matchers. The list includes current -// Claude/Codex spellings. pub(crate) fn event_matches_tools(event: &str) -> bool { matches!( event, diff --git a/crates/cli/src/hooks/mod.rs b/crates/cli/src/hooks/mod.rs index 5aceb355f..b05c86522 100644 --- a/crates/cli/src/hooks/mod.rs +++ b/crates/cli/src/hooks/mod.rs @@ -3,6 +3,7 @@ //! Hook delivery, command encoding, generated definitions, and configuration merging. +mod config; mod delivery; mod destination; mod encoding; @@ -11,6 +12,7 @@ mod merging; mod response; mod types; +pub(crate) use config::HookCommandConfig; pub(crate) use delivery::hook_forward; #[cfg(test)] pub(crate) use delivery::send_verified_hook_forward_request; @@ -21,16 +23,14 @@ pub(crate) use destination::{ HookGatewayLifecycle, resolve_hook_destination, transparent_gateway_spec, }; #[cfg(test)] -pub(crate) use encoding::decode_windows_hook_command; -#[cfg(all(test, windows))] -pub(crate) use encoding::windows_powershell_path; +pub(crate) use encoding::transparent_hook_forward_commands; pub(crate) use encoding::{ GeneratedHookCommands, generated_policy_hooks, persistent_hook_forward_commands, - transparent_hook_forward_commands, + transparent_hook_forward_commands_with_config, }; #[cfg(test)] pub(crate) use encoding::{ - encoded_windows_hook_command, event_matches_tools, event_requires_fail_closed, generated_hooks, + decode_windows_hook_command, event_matches_tools, event_requires_fail_closed, generated_hooks, persistent_hook_forward_commands_for_platform, transparent_hook_forward_commands_for_platform, }; #[cfg(test)] diff --git a/crates/cli/src/hooks/types.rs b/crates/cli/src/hooks/types.rs index b11713112..45b28e730 100644 --- a/crates/cli/src/hooks/types.rs +++ b/crates/cli/src/hooks/types.rs @@ -8,6 +8,7 @@ use crate::agents::CodingAgent; #[derive(Debug, Clone)] pub(crate) struct HookForwardRequest { pub(crate) agent: CodingAgent, + pub(crate) hook_config: Option, pub(crate) gateway_url: Option, pub(crate) generation_file: Option, pub(crate) generation_token: Option, @@ -19,6 +20,19 @@ pub(crate) struct HookForwardRequest { pub(crate) failure_policy: HookFailurePolicy, } +impl HookForwardRequest { + pub(crate) fn has_inline_configuration(&self) -> bool { + self.gateway_url.is_some() + || self.generation_file.is_some() + || self.generation_token.is_some() + || self.forward_only + || self.transparent_run + || self.profile.is_some() + || self.session_metadata.is_some() + || self.gateway_mode.is_some() + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum HookFailurePolicy { Default, @@ -36,7 +50,8 @@ impl HookFailurePolicy { } } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "kebab-case")] pub(crate) enum GatewayMode { HookOnly, Passthrough, diff --git a/crates/cli/src/installation/marketplace/assets.rs b/crates/cli/src/installation/marketplace/assets.rs index 9de9b2ba4..63c6334ab 100644 --- a/crates/cli/src/installation/marketplace/assets.rs +++ b/crates/cli/src/installation/marketplace/assets.rs @@ -47,6 +47,7 @@ pub(super) fn write_plugin_marketplace_for_generation( println!("write {}", layout.plugin_manifest.display()); println!("write {}", layout.mcp_config.display()); println!("write {}", layout.generation_fence.display()); + println!("write {}", layout.hook_config.display()); println!("write {}", layout.hooks_path.display()); return Ok(()); } @@ -67,13 +68,17 @@ pub(super) fn write_plugin_marketplace_for_generation( } else { write_staged_generation_with_token(&layout.generation_fence, active_generation_lock) }?; + let generation_fence = absolute_or_self(active_generation_fence)?; + let hook_config = absolute_or_self(&layout.hook_config)?; + host.persistent_hook_config(&generation_fence, &generation_token) + .write(&hook_config)?; write_json( &layout.mcp_config, - &plugin_mcp_config(host, relay, active_generation_fence, &generation_token)?, + &plugin_mcp_config(host, relay, &generation_fence, &generation_token)?, )?; write_json( &layout.hooks_path, - &plugin_hooks(host, relay, active_generation_fence, &generation_token)?, + &plugin_hooks(host, relay, &generation_fence, &generation_token)?, )?; Ok(()) } @@ -113,6 +118,6 @@ pub(super) fn plugin_hooks( generation_fence: &Path, generation_token: &str, ) -> Result { - let generation_fence = absolute_or_self(generation_fence)?; - host.plugin_hooks(relay, &generation_fence, generation_token) + let hook_config = generation_fence.with_file_name(".nemo-relay-hook-config.json"); + host.plugin_hooks(relay, generation_fence, generation_token, &hook_config) } diff --git a/crates/cli/src/installation/marketplace/mod.rs b/crates/cli/src/installation/marketplace/mod.rs index cf8bab041..24d036244 100644 --- a/crates/cli/src/installation/marketplace/mod.rs +++ b/crates/cli/src/installation/marketplace/mod.rs @@ -1570,9 +1570,11 @@ fn collect_host_plugin_readiness( if let Some(plugin) = readiness.plugin.as_ref() { let generation_fence = plugin.join(crate::installation::generation::GENERATION_FILE_NAME); + let hook_config = plugin.join(".nemo-relay-hook-config.json"); readiness.push( "Generated hooks", InstallGeneration::capture(generation_fence.clone()).and_then(|generation| { + crate::hooks::HookCommandConfig::load(&hook_config)?; let expected = plugin_hooks(host, &relay, &generation_fence, generation.token())?; generated_manifest_check( diff --git a/crates/cli/src/installation/marketplace/spec.rs b/crates/cli/src/installation/marketplace/spec.rs index 8fca35a7e..d1acd02c6 100644 --- a/crates/cli/src/installation/marketplace/spec.rs +++ b/crates/cli/src/installation/marketplace/spec.rs @@ -41,11 +41,17 @@ pub(crate) trait MarketplaceHost: Copy { fn marketplace_manifest(self, marketplace: &str, plugin: &str) -> Value; fn plugin_manifest(self, plugin: &str) -> Value; fn plugin_mcp_config(self, server: Value) -> Result; + fn persistent_hook_config( + self, + generation_fence: &Path, + generation_token: &str, + ) -> crate::hooks::HookCommandConfig; fn plugin_hooks( self, relay: &Path, generation_fence: &Path, generation_token: &str, + hook_config: &Path, ) -> Result; fn plugin_registration_args(self, plugin_id: &str) -> Vec; fn plugin_removal_args(self, plugin_name: &str, plugin_id: &str) -> Vec; diff --git a/crates/cli/src/installation/marketplace/state.rs b/crates/cli/src/installation/marketplace/state.rs index 8e019cab7..62bb55f0b 100644 --- a/crates/cli/src/installation/marketplace/state.rs +++ b/crates/cli/src/installation/marketplace/state.rs @@ -48,6 +48,7 @@ pub(super) struct PluginLayout { pub(super) mcp_config: PathBuf, pub(super) generation_fence: PathBuf, pub(super) generation_lock: PathBuf, + pub(super) hook_config: PathBuf, pub(super) hooks_path: PathBuf, pub(super) state_path: PathBuf, } @@ -73,6 +74,7 @@ impl PluginLayout { host.install_arg() )); let hooks_path = plugin_root.join("hooks").join("hooks.json"); + let hook_config = plugin_root.join(".nemo-relay-hook-config.json"); let state_path = state_path(host, install_dir); Self { host_arg: host.install_arg(), @@ -84,6 +86,7 @@ impl PluginLayout { mcp_config, generation_fence, generation_lock, + hook_config, hooks_path, state_path, } diff --git a/crates/cli/tests/coverage/shared/installer_tests.rs b/crates/cli/tests/coverage/shared/installer_tests.rs index f7ae88d74..af0d3ed3e 100644 --- a/crates/cli/tests/coverage/shared/installer_tests.rs +++ b/crates/cli/tests/coverage/shared/installer_tests.rs @@ -11,6 +11,36 @@ use serde_json::Value; use crate::agents::CodingAgent; +#[test] +fn private_hook_config_round_trips_and_rejects_agent_mismatch() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("hook.json"); + HookCommandConfig::transparent(CodingAgent::Codex, "http://127.0.0.1:1234") + .write(&path) + .unwrap(); + + let config = HookCommandConfig::load(&path).unwrap(); + let mut request = HookForwardRequest { + agent: CodingAgent::ClaudeCode, + hook_config: Some(path), + gateway_url: None, + generation_file: None, + generation_token: None, + forward_only: false, + transparent_run: false, + profile: None, + session_metadata: None, + gateway_mode: None, + failure_policy: HookFailurePolicy::Default, + }; + assert!( + config + .apply(&mut request) + .unwrap_err() + .contains("requested claude") + ); +} + struct BootstrapConfigHome { _guard: std::sync::MutexGuard<'static, ()>, previous: Option, @@ -115,6 +145,7 @@ async fn transparent_hook_delivery_authenticates_the_wrapper_gateway() { .expect("wrapper gateway did not become healthy"); let command = HookForwardRequest { agent: CodingAgent::Codex, + hook_config: None, gateway_url: Some(gateway_url.clone()), generation_file: None, generation_token: None, @@ -361,53 +392,33 @@ fn helper_formatting_and_headers_cover_optional_paths() { #[test] fn generated_hook_dispatch_covers_all_agents() { assert_generated_hook_policies(); + let config = "/private/nemo-relay-hook.json"; assert_eq!( transparent_hook_forward_commands_for_platform( Path::new("/abs/path/to/nemo-relay"), CodingAgent::Codex, - "http://127.0.0.1:1234", + config, false, ) .for_event("PreToolUse"), - "/abs/path/to/nemo-relay hook-forward codex --gateway-url http://127.0.0.1:1234 --transparent-run --fail-closed" + "/abs/path/to/nemo-relay hook-forward codex --hook-config /private/nemo-relay-hook.json --fail-closed" ); let relay = Path::new("/opt/NeMo Relay's & tools/nemo-relay"); assert_eq!( - transparent_hook_forward_commands_for_platform( - relay, - CodingAgent::Codex, - "http://127.0.0.1:1234", - false - ) - .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"# + transparent_hook_forward_commands_for_platform(relay, CodingAgent::Codex, config, false) + .for_event("SessionStart"), + r#"'/opt/NeMo Relay'\''s & tools/nemo-relay' hook-forward codex --hook-config /private/nemo-relay-hook.json --fail-open"# ); - let native = transparent_hook_forward_commands( - Path::new("nemo-relay"), - CodingAgent::Codex, - "http://127.0.0.1:1234", - ) - .unwrap(); - if cfg!(windows) { - assert_eq!( - decode_windows_hook_command(native.for_event("on_session_start")).unwrap(), - vec![ - String::from("nemo-relay"), - String::from("hook-forward"), - String::from("codex"), - String::from("--gateway-url"), - String::from("http://127.0.0.1:1234"), - String::from("--transparent-run"), - String::from("--fail-open"), - ] - ); - } else { + let native = + transparent_hook_forward_commands(Path::new("nemo-relay"), CodingAgent::Codex, config) + .unwrap(); + if !cfg!(windows) { assert_eq!( native, transparent_hook_forward_commands_for_platform( Path::new("nemo-relay"), CodingAgent::Codex, - "http://127.0.0.1:1234", + config, false, ) ); @@ -415,56 +426,13 @@ fn generated_hook_dispatch_covers_all_agents() { let windows = transparent_hook_forward_commands_for_platform( relay, CodingAgent::ClaudeCode, - "http://127.0.0.1:1234", + config, true, ); let windows = windows.for_event("PreToolUse"); - let (launcher, encoded) = windows.rsplit_once(' ').unwrap(); - assert_eq!( - launcher, - "C:/Windows/System32/WindowsPowerShell/v1.0/powershell.exe -NoLogo -NoProfile -NonInteractive -EncodedCommand" - ); - assert!( - !encoded.is_empty() - && encoded - .chars() - .all(|character| character.is_ascii_alphanumeric() - || matches!(character, '+' | '/' | '=')) - ); - assert_eq!( - decode_windows_hook_command(windows).unwrap(), - vec![ - relay.display().to_string(), - "hook-forward".into(), - "claude".into(), - "--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()); - assert!( - decode_windows_hook_command( - "C:/Windows/System32/WindowsPowerShell/v1.0/powershell.exe -NoLogo -NoProfile -NonInteractive -EncodedCommand invalid payload" - ) - .is_none() - ); - let oversized = format!( - "C:/Windows/System32/WindowsPowerShell/v1.0/powershell.exe -NoLogo -NoProfile -NonInteractive -EncodedCommand {}", - "A".repeat(8_000) - ); - assert!(decode_windows_hook_command(&oversized).is_none()); - - let oversized_path = format!("C:/{}nemo-relay.exe", "long/".repeat(2_000)); - let error = encoded_windows_hook_command( - "C:/Windows/System32/WindowsPowerShell/v1.0/powershell.exe", - Path::new(&oversized_path), - &["hook-forward".into(), "codex".into()], - ) - .unwrap_err(); - assert!(error.contains("exceeds the 8000-character safety limit")); - assert!(error.contains("shorten the Relay or plugin installation path")); + assert!(windows.contains("--hook-config")); + assert!(!windows.contains("PowerShell")); + assert!(!windows.contains("EncodedCommand")); } fn assert_generated_hook_policies() { diff --git a/docs/nemo-relay-cli/plugin-installation.mdx b/docs/nemo-relay-cli/plugin-installation.mdx index daa2a6e97..35a6323ff 100644 --- a/docs/nemo-relay-cli/plugin-installation.mdx +++ b/docs/nemo-relay-cli/plugin-installation.mdx @@ -136,13 +136,15 @@ undiscoverable `PostToolUseFailure`, `Notification`, or `SessionEnd` handlers. Upgrade removes legacy Relay groups from `~/.codex/hooks.json` while preserving unrelated hooks. -On Windows, generated hooks use the built-in Windows PowerShell encoded-command -format. This avoids quoting and metacharacter differences between the Codex and -Claude Code command runners. The encoded payload contains only the -canonical `nemo-relay.exe` path and `hook-forward` arguments. PowerShell starts -that Rust binary directly and preserves its standard input, standard output, -standard error, and exit code. The MCP client and gateway remain Rust-native, -and both install and doctor verify the generated command and event ownership. +After upgrading to a release with file-backed hook configuration, uninstall and +reinstall the Relay plugin so the coding-agent host refreshes its stored hook +commands and trust hashes. + +Generated hooks invoke the native Relay binary with a short private +`--hook-config` path. The private Relay-owned file contains the gateway URL, +generation fence, and hook lifecycle settings, so host-managed hook +configuration does not expose those values or exceed Windows command-length +limits. Both install and doctor verify the generated command and event ownership. Start a new Codex CLI process after installation. Restart the Codex desktop app if it was already running so it reloads the provider and hook configuration. @@ -406,8 +408,8 @@ custom automation, use these supported replacements: | `nemo-relay plugin-shim provider claude status` | `nemo-relay doctor --plugin claude-code` | | `nemo-relay plugin-shim doctor ` | `nemo-relay doctor --plugin ` | -Persistent generated hook commands include the fixed gateway URL. Transparent -wrapper hooks embed their dynamic gateway URL, while the process environment +Persistent and transparent generated hook commands reference a private Relay +hook configuration instead of embedding gateway details. The process environment lets an installed plugin MCP authenticate, borrow, and monitor that exact gateway. Transparent hook delivery authenticates the wrapper gateway before writing its lifecycle payload. When a transparent run uses a recognizable From 8b5a72038922c8fc2243b05d06d7cfc887b53769 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Fri, 28 Aug 2026 12:42:54 -0400 Subject: [PATCH 02/16] fix: build generated hooks on Windows Signed-off-by: Will Killian --- crates/cli/src/agents/mod.rs | 2 +- crates/cli/src/hooks/encoding.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/cli/src/agents/mod.rs b/crates/cli/src/agents/mod.rs index 58e1750aa..63c497863 100644 --- a/crates/cli/src/agents/mod.rs +++ b/crates/cli/src/agents/mod.rs @@ -834,7 +834,7 @@ fn failed_integration_readiness( } pub(crate) use crate::process::portable_executable_path; -#[cfg(any(not(windows), test))] +#[cfg(test)] pub(crate) use crate::process::shell_quote_arg_for_platform; #[cfg(test)] pub(crate) use crate::process::strip_windows_verbatim_prefix; diff --git a/crates/cli/src/hooks/encoding.rs b/crates/cli/src/hooks/encoding.rs index bb9a3a010..69ff5dc1f 100644 --- a/crates/cli/src/hooks/encoding.rs +++ b/crates/cli/src/hooks/encoding.rs @@ -189,7 +189,7 @@ pub(super) fn hook_command_for_platform( fn render_hook_command(relay: &Path, arguments: &[String], windows: bool) -> String { std::iter::once(relay.display().to_string()) .chain(arguments.iter().cloned()) - .map(|argument| crate::agents::shell_quote_arg_for_platform(&argument, windows)) + .map(|argument| crate::process::shell_quote_arg_for_platform(&argument, windows)) .collect::>() .join(" ") } From 97b5838462dd0befbbcba06020d510ff76fe7274 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Fri, 28 Aug 2026 13:17:56 -0400 Subject: [PATCH 03/16] test: update hook command coverage Signed-off-by: Will Killian --- crates/cli/src/hooks/encoding.rs | 5 ---- crates/cli/src/hooks/mod.rs | 2 +- .../tests/coverage/agents/launcher_tests.rs | 8 +++--- .../coverage/agents/plugin_host_tests.rs | 25 +++++++++++-------- .../coverage/agents/plugin_install_tests.rs | 22 +++++++++------- .../tests/coverage/shared/hook_assertions.rs | 13 ++++++++-- .../tests/coverage/shared/installer_tests.rs | 22 ---------------- .../cli/tests/fixtures/windows_hook_relay.rs | 12 +++------ 8 files changed, 49 insertions(+), 60 deletions(-) diff --git a/crates/cli/src/hooks/encoding.rs b/crates/cli/src/hooks/encoding.rs index 69ff5dc1f..d21fac6de 100644 --- a/crates/cli/src/hooks/encoding.rs +++ b/crates/cli/src/hooks/encoding.rs @@ -112,11 +112,6 @@ fn config_path(generation_file: &Path) -> std::path::PathBuf { generation_file.with_file_name(".nemo-relay-hook-config.json") } -#[cfg(test)] -pub(crate) fn decode_windows_hook_command(_command: &str) -> Option> { - None -} - pub(super) fn hook_config_arguments(agent: CodingAgent, hook_config: &Path) -> Vec { vec![ "hook-forward".into(), diff --git a/crates/cli/src/hooks/mod.rs b/crates/cli/src/hooks/mod.rs index b05c86522..407eae257 100644 --- a/crates/cli/src/hooks/mod.rs +++ b/crates/cli/src/hooks/mod.rs @@ -30,7 +30,7 @@ pub(crate) use encoding::{ }; #[cfg(test)] pub(crate) use encoding::{ - decode_windows_hook_command, event_matches_tools, event_requires_fail_closed, generated_hooks, + event_matches_tools, event_requires_fail_closed, generated_hooks, persistent_hook_forward_commands_for_platform, transparent_hook_forward_commands_for_platform, }; #[cfg(test)] diff --git a/crates/cli/tests/coverage/agents/launcher_tests.rs b/crates/cli/tests/coverage/agents/launcher_tests.rs index f292b9cb9..6aa13d979 100644 --- a/crates/cli/tests/coverage/agents/launcher_tests.rs +++ b/crates/cli/tests/coverage/agents/launcher_tests.rs @@ -1234,9 +1234,11 @@ fn prepares_claude_temp_plugin() { &[ "hook-forward", "claude", - "--gateway-url", - "http://127.0.0.1:1234", - "--transparent-run", + "--hook-config", + plugin_dir + .join(".nemo-relay-hook-config.json") + .to_str() + .unwrap(), ], )); assert!( diff --git a/crates/cli/tests/coverage/agents/plugin_host_tests.rs b/crates/cli/tests/coverage/agents/plugin_host_tests.rs index 0b2dafaef..24fb0a526 100644 --- a/crates/cli/tests/coverage/agents/plugin_host_tests.rs +++ b/crates/cli/tests/coverage/agents/plugin_host_tests.rs @@ -3211,7 +3211,7 @@ 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( + crate::hook_assertions::decode_windows_hook_command( codex_plugin_hook_command_for_platform(&relay, &generation, "test-generation", true,) .for_event("PreToolUse") ) @@ -3220,12 +3220,11 @@ fn windows_shell_argument_quoting_and_hook_encoding_preserve_paths() { relay.display().to_string(), "hook-forward".into(), "codex".into(), - "--gateway-url".into(), - DEFAULT_URL.into(), - "--generation-file".into(), - generation.display().to_string(), - "--generation-token".into(), - "test-generation".into(), + "--hook-config".into(), + generation + .with_file_name(".nemo-relay-hook-config.json") + .display() + .to_string(), "--fail-closed".into(), ] ); @@ -3259,7 +3258,10 @@ fn generated_windows_hook_command_executes_exact_arguments() { .stderr(std::process::Stdio::piped()) .env("NEMO_RELAY_HOOK_MARKER", &marker) .env("NEMO_RELAY_HOOK_INPUT_MARKER", &input_marker) - .env("NEMO_RELAY_HOOK_GENERATION", &generation) + .env( + "NEMO_RELAY_HOOK_CONFIG", + generation.with_file_name(".nemo-relay-hook-config.json"), + ) .env("NEMO_RELAY_HOOK_EMIT_OUTPUT", "1") .spawn() .unwrap(); @@ -3295,7 +3297,10 @@ fn generated_windows_hook_command_propagates_the_relay_exit_code() { let status = std::process::Command::new("cmd.exe") .arg("/C") .arg(&command) - .env("NEMO_RELAY_HOOK_GENERATION", &generation) + .env( + "NEMO_RELAY_HOOK_CONFIG", + generation.with_file_name(".nemo-relay-hook-config.json"), + ) .env("NEMO_RELAY_HOOK_EXIT_CODE", "23") .status() .unwrap(); @@ -3332,7 +3337,7 @@ fn posix_shell_argument_quoting_and_hook_encoding_preserve_paths() { assert_eq!( 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" + "'/tmp/NeMo $Relay`test'\\''/bin/nemo-relay' hook-forward codex --hook-config '/tmp/NeMo $Relay`test'\\''/plugin/.nemo-relay-hook-config.json' --fail-open" ); assert_eq!(shell_quote_arg_for_platform("", false), "''"); assert_eq!( diff --git a/crates/cli/tests/coverage/agents/plugin_install_tests.rs b/crates/cli/tests/coverage/agents/plugin_install_tests.rs index fe235a25c..4ed0592c5 100644 --- a/crates/cli/tests/coverage/agents/plugin_install_tests.rs +++ b/crates/cli/tests/coverage/agents/plugin_install_tests.rs @@ -2635,10 +2635,8 @@ fn force_install_retires_previous_mcp_generation() { let cached_mcp = serde_json::from_str::(&std::fs::read_to_string(&layout.mcp_config).unwrap()) .unwrap(); - let cached_hooks = serde_json::from_str::( - &std::fs::read_to_string(&layout.hooks_path).unwrap(), - ) - .unwrap(); + let cached_hook_config: Value = + serde_json::from_str(&std::fs::read_to_string(&layout.hook_config).unwrap()).unwrap(); install_host(CodingAgent::Codex, &options, &runner, &setup_runner).unwrap(); @@ -2660,18 +2658,24 @@ fn force_install_retires_previous_mcp_generation() { cached_mcp["nemo-relay"]["env"]["NEMO_RELAY_MCP_GENERATION"], json!(previous_token) ); - assert!(crate::hook_assertions::value_has_command_arguments( - &cached_hooks, - &["--generation-token", &previous_token] - )); + assert_eq!( + cached_hook_config["generation_token"], + json!(previous_token) + ); let current_hooks = serde_json::from_str::( &std::fs::read_to_string(&layout.hooks_path).unwrap(), ) .unwrap(); assert!(crate::hook_assertions::value_has_command_arguments( ¤t_hooks, - &["--generation-token", current.token()] + &["--hook-config", layout.hook_config.to_str().unwrap()] )); + let current_hook_config: Value = + serde_json::from_str(&std::fs::read_to_string(&layout.hook_config).unwrap()).unwrap(); + assert_eq!( + current_hook_config["generation_token"], + json!(current.token()) + ); assert!(layout.generation_lock.exists()); } diff --git a/crates/cli/tests/coverage/shared/hook_assertions.rs b/crates/cli/tests/coverage/shared/hook_assertions.rs index 10a97825e..3b50b6b67 100644 --- a/crates/cli/tests/coverage/shared/hook_assertions.rs +++ b/crates/cli/tests/coverage/shared/hook_assertions.rs @@ -3,9 +3,18 @@ use serde_json::Value; +pub(crate) fn decode_windows_hook_command(command: &str) -> Option> { + shell_words::split(command).ok().map(|arguments| { + arguments + .into_iter() + .map(|argument| argument.replace("%%cd:~,%", "%")) + .collect() + }) +} + pub(crate) fn command_has_arguments(command: &str, expected: &[&str]) -> bool { - let arguments = crate::hooks::decode_windows_hook_command(command) - .or_else(|| shell_words::split(command).ok()); + let arguments = + decode_windows_hook_command(command).or_else(|| shell_words::split(command).ok()); arguments.is_some_and(|arguments| { arguments.windows(expected.len()).any(|window| { window diff --git a/crates/cli/tests/coverage/shared/installer_tests.rs b/crates/cli/tests/coverage/shared/installer_tests.rs index af0d3ed3e..eb5e0a73d 100644 --- a/crates/cli/tests/coverage/shared/installer_tests.rs +++ b/crates/cli/tests/coverage/shared/installer_tests.rs @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 use super::*; -use base64::Engine; use std::path::Path; use std::time::Duration; @@ -308,27 +307,6 @@ fn hook_response_statuses_preserve_guardrail_rejections_and_fail_closed_errors() assert!(error.contains("HTTP 502"), "{error}"); } -#[test] -fn windows_hook_decoder_rejects_unsafe_odd_and_trailing_argument_envelopes() { - const SEPARATOR: &str = " -NoLogo -NoProfile -NonInteractive -EncodedCommand "; - #[cfg(windows)] - let launcher = windows_powershell_path().unwrap(); - #[cfg(not(windows))] - let launcher = "C:/Windows/System32/WindowsPowerShell/v1.0/powershell.exe".to_string(); - - assert!(decode_windows_hook_command(&format!("powershell.exe{SEPARATOR}QQ==")).is_none()); - assert!(decode_windows_hook_command(&format!("{launcher}{SEPARATOR}QQ==")).is_none()); - - let script = "$ErrorActionPreference='Stop'; & 'relay' ; if ($null -eq $LASTEXITCODE) { exit 1 }; exit $LASTEXITCODE"; - let encoded = base64::engine::general_purpose::STANDARD.encode( - script - .encode_utf16() - .flat_map(u16::to_le_bytes) - .collect::>(), - ); - assert!(decode_windows_hook_command(&format!("{launcher}{SEPARATOR}{encoded}")).is_none()); -} - #[test] fn merge_hooks_is_idempotent_and_preserves_existing_entries() { let existing = json!({ diff --git a/crates/cli/tests/fixtures/windows_hook_relay.rs b/crates/cli/tests/fixtures/windows_hook_relay.rs index cedd3c885..b01d383a2 100644 --- a/crates/cli/tests/fixtures/windows_hook_relay.rs +++ b/crates/cli/tests/fixtures/windows_hook_relay.rs @@ -5,17 +5,13 @@ use std::ffi::OsString; use std::io::Read; fn main() { - let generation = std::env::var_os("NEMO_RELAY_HOOK_GENERATION") - .expect("NEMO_RELAY_HOOK_GENERATION is required"); + let hook_config = std::env::var_os("NEMO_RELAY_HOOK_CONFIG") + .expect("NEMO_RELAY_HOOK_CONFIG is required"); let expected = vec![ OsString::from("hook-forward"), OsString::from("codex"), - OsString::from("--gateway-url"), - OsString::from("http://127.0.0.1:47632"), - OsString::from("--generation-file"), - generation, - OsString::from("--generation-token"), - OsString::from("test-generation"), + OsString::from("--hook-config"), + hook_config, OsString::from("--fail-closed"), ]; let actual = std::env::args_os().skip(1).collect::>(); From 1a89ae3da203b3e5465250327dc3a3d57349e856 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Fri, 28 Aug 2026 13:31:32 -0400 Subject: [PATCH 04/16] fix: quote native Windows hook commands Signed-off-by: Will Killian --- crates/cli/src/hooks/encoding.rs | 9 +++++++-- crates/cli/src/process/mod.rs | 2 +- crates/cli/tests/coverage/agents/plugin_host_tests.rs | 10 ++++++++-- crates/cli/tests/coverage/shared/hook_assertions.rs | 6 +++++- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/crates/cli/src/hooks/encoding.rs b/crates/cli/src/hooks/encoding.rs index d21fac6de..2b32c3999 100644 --- a/crates/cli/src/hooks/encoding.rs +++ b/crates/cli/src/hooks/encoding.rs @@ -182,11 +182,16 @@ pub(super) fn hook_command_for_platform( } fn render_hook_command(relay: &Path, arguments: &[String], windows: bool) -> String { - std::iter::once(relay.display().to_string()) + let command = std::iter::once(relay.display().to_string()) .chain(arguments.iter().cloned()) .map(|argument| crate::process::shell_quote_arg_for_platform(&argument, windows)) .collect::>() - .join(" ") + .join(" "); + if windows { + format!("\"{command}\"") + } else { + command + } } // `cmd.exe` accepts at most 8,191 characters. Leave room for `/C` and host-added text. diff --git a/crates/cli/src/process/mod.rs b/crates/cli/src/process/mod.rs index 4fb6f7d7c..7126b8a5b 100644 --- a/crates/cli/src/process/mod.rs +++ b/crates/cli/src/process/mod.rs @@ -51,7 +51,7 @@ fn cmd_quote_arg(raw: &str) -> String { let mut escaped = String::new(); for ch in raw.chars() { match ch { - '%' => escaped.push_str("%%cd:~,%"), + '%' => escaped.push_str("^%"), '"' => escaped.push_str("\"\""), _ => escaped.push(ch), } diff --git a/crates/cli/tests/coverage/agents/plugin_host_tests.rs b/crates/cli/tests/coverage/agents/plugin_host_tests.rs index 24fb0a526..232897e65 100644 --- a/crates/cli/tests/coverage/agents/plugin_host_tests.rs +++ b/crates/cli/tests/coverage/agents/plugin_host_tests.rs @@ -3208,7 +3208,7 @@ fn windows_shell_argument_quoting_and_hook_encoding_preserve_paths() { std::path::PathBuf::from(r"C:\Program Files\NeMo 100%\plugin\.nemo-relay-generation"); assert_eq!( shell_quote_arg_for_platform(relay.to_str().unwrap(), true), - r#""C:\Program Files\NeMo 100%%cd:~,%\bin\nemo-relay.exe""# + r#""C:\Program Files\NeMo 100^%\bin\nemo-relay.exe""# ); assert_eq!( crate::hook_assertions::decode_windows_hook_command( @@ -3334,10 +3334,16 @@ fn posix_shell_argument_quoting_and_hook_encoding_preserve_paths() { shell_quote_arg_for_platform(relay.to_str().unwrap(), false), "'/tmp/NeMo $Relay`test'\\''/bin/nemo-relay'" ); + let hook_config = generation.with_file_name(".nemo-relay-hook-config.json"); + let expected = format!( + "{} hook-forward codex --hook-config {} --fail-open", + shell_quote_arg_for_platform(relay.to_str().unwrap(), false), + shell_quote_arg_for_platform(hook_config.to_str().unwrap(), false), + ); assert_eq!( codex_plugin_hook_command_for_platform(&relay, &generation, "test-generation", false) .for_event("SessionStart"), - "'/tmp/NeMo $Relay`test'\\''/bin/nemo-relay' hook-forward codex --hook-config '/tmp/NeMo $Relay`test'\\''/plugin/.nemo-relay-hook-config.json' --fail-open" + expected ); assert_eq!(shell_quote_arg_for_platform("", false), "''"); assert_eq!( diff --git a/crates/cli/tests/coverage/shared/hook_assertions.rs b/crates/cli/tests/coverage/shared/hook_assertions.rs index 3b50b6b67..0376fc7d6 100644 --- a/crates/cli/tests/coverage/shared/hook_assertions.rs +++ b/crates/cli/tests/coverage/shared/hook_assertions.rs @@ -4,10 +4,14 @@ use serde_json::Value; pub(crate) fn decode_windows_hook_command(command: &str) -> Option> { + let command = command + .strip_prefix('"') + .and_then(|command| command.strip_suffix('"')) + .unwrap_or(command); shell_words::split(command).ok().map(|arguments| { arguments .into_iter() - .map(|argument| argument.replace("%%cd:~,%", "%")) + .map(|argument| argument.replace("^%", "%")) .collect() }) } From 85bccd6b60619e66530a7d51e0c3eba908f3336a Mon Sep 17 00:00:00 2001 From: Will Killian Date: Fri, 28 Aug 2026 13:41:23 -0400 Subject: [PATCH 05/16] test: preserve rendered Windows hook commands Signed-off-by: Will Killian --- crates/cli/tests/coverage/agents/plugin_host_tests.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/cli/tests/coverage/agents/plugin_host_tests.rs b/crates/cli/tests/coverage/agents/plugin_host_tests.rs index 232897e65..66e9c9953 100644 --- a/crates/cli/tests/coverage/agents/plugin_host_tests.rs +++ b/crates/cli/tests/coverage/agents/plugin_host_tests.rs @@ -3238,6 +3238,8 @@ fn windows_shell_argument_quoting_and_hook_encoding_preserve_paths() { #[cfg(windows)] #[test] fn generated_windows_hook_command_executes_exact_arguments() { + use std::os::windows::process::CommandExt; + let temp = tempfile::tempdir().unwrap(); let bin = temp.path().join("Relay & %USERPROFILE% !^ Tools"); std::fs::create_dir(&bin).unwrap(); @@ -3252,7 +3254,7 @@ fn generated_windows_hook_command_executes_exact_arguments() { .to_owned(); let mut child = std::process::Command::new("cmd.exe") .arg("/C") - .arg(&command) + .raw_arg(&command) .stdin(std::process::Stdio::piped()) .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()) @@ -3285,6 +3287,8 @@ fn generated_windows_hook_command_executes_exact_arguments() { #[cfg(windows)] #[test] fn generated_windows_hook_command_propagates_the_relay_exit_code() { + use std::os::windows::process::CommandExt; + let temp = tempfile::tempdir().unwrap(); let relay = temp.path().join("relay failure.exe"); compile_windows_hook_test_relay(&relay); @@ -3296,7 +3300,7 @@ fn generated_windows_hook_command_propagates_the_relay_exit_code() { let status = std::process::Command::new("cmd.exe") .arg("/C") - .arg(&command) + .raw_arg(&command) .env( "NEMO_RELAY_HOOK_CONFIG", generation.with_file_name(".nemo-relay-hook-config.json"), From 7295f1f8580b36596c097909a4eee123da1042f5 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Fri, 28 Aug 2026 13:49:48 -0400 Subject: [PATCH 06/16] test: delimit native Windows hook commands Signed-off-by: Will Killian --- crates/cli/tests/coverage/agents/plugin_host_tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/cli/tests/coverage/agents/plugin_host_tests.rs b/crates/cli/tests/coverage/agents/plugin_host_tests.rs index 66e9c9953..88a3332be 100644 --- a/crates/cli/tests/coverage/agents/plugin_host_tests.rs +++ b/crates/cli/tests/coverage/agents/plugin_host_tests.rs @@ -3254,7 +3254,7 @@ fn generated_windows_hook_command_executes_exact_arguments() { .to_owned(); let mut child = std::process::Command::new("cmd.exe") .arg("/C") - .raw_arg(&command) + .raw_arg(format!(" {command}")) .stdin(std::process::Stdio::piped()) .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()) @@ -3300,7 +3300,7 @@ fn generated_windows_hook_command_propagates_the_relay_exit_code() { let status = std::process::Command::new("cmd.exe") .arg("/C") - .raw_arg(&command) + .raw_arg(format!(" {command}")) .env( "NEMO_RELAY_HOOK_CONFIG", generation.with_file_name(".nemo-relay-hook-config.json"), From e74344f6aca9354f6174a8ba9b0088e61a1e0501 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Fri, 28 Aug 2026 13:57:44 -0400 Subject: [PATCH 07/16] test: report Windows hook command failures Signed-off-by: Will Killian --- crates/cli/tests/coverage/agents/plugin_host_tests.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/cli/tests/coverage/agents/plugin_host_tests.rs b/crates/cli/tests/coverage/agents/plugin_host_tests.rs index 88a3332be..dbe13ae6a 100644 --- a/crates/cli/tests/coverage/agents/plugin_host_tests.rs +++ b/crates/cli/tests/coverage/agents/plugin_host_tests.rs @@ -3271,7 +3271,13 @@ fn generated_windows_hook_command_executes_exact_arguments() { child.stdin.take().unwrap().write_all(b"ping\n").unwrap(); let output = child.wait_with_output().unwrap(); - assert!(output.status.success(), "{command}"); + assert!( + output.status.success(), + "command: {command}\nstatus: {:?}\nstdout: {}\nstderr: {}", + output.status, + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); assert_eq!(std::fs::read_to_string(marker).unwrap().trim(), "ok"); assert_eq!(std::fs::read(input_marker).unwrap(), b"ping\n"); assert_eq!( From cd5cf81a17c4163fc4da10903ffb3fd08041f897 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Fri, 28 Aug 2026 14:01:36 -0400 Subject: [PATCH 08/16] fix: preserve carets in Windows hook commands Signed-off-by: Will Killian --- crates/cli/src/process/mod.rs | 1 + crates/cli/tests/coverage/agents/plugin_host_tests.rs | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/crates/cli/src/process/mod.rs b/crates/cli/src/process/mod.rs index 7126b8a5b..748830000 100644 --- a/crates/cli/src/process/mod.rs +++ b/crates/cli/src/process/mod.rs @@ -52,6 +52,7 @@ fn cmd_quote_arg(raw: &str) -> String { for ch in raw.chars() { match ch { '%' => escaped.push_str("^%"), + '^' => escaped.push_str("^^"), '"' => escaped.push_str("\"\""), _ => escaped.push(ch), } diff --git a/crates/cli/tests/coverage/agents/plugin_host_tests.rs b/crates/cli/tests/coverage/agents/plugin_host_tests.rs index dbe13ae6a..646221ece 100644 --- a/crates/cli/tests/coverage/agents/plugin_host_tests.rs +++ b/crates/cli/tests/coverage/agents/plugin_host_tests.rs @@ -3232,6 +3232,10 @@ fn windows_shell_argument_quoting_and_hook_encoding_preserve_paths() { shell_quote_arg_for_platform("foo&bar", true), r#""foo&bar""# ); + assert_eq!( + shell_quote_arg_for_platform("foo^bar", true), + r#""foo^^bar""# + ); assert_eq!(shell_quote_arg_for_platform("", true), r#""""#); } From 6be276ae72001acea4dd6f9b89eb92e518afd4a7 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Fri, 28 Aug 2026 14:13:48 -0400 Subject: [PATCH 09/16] fix: shorten Windows hook executable paths Signed-off-by: Will Killian --- crates/cli/src/hooks/encoding.rs | 15 ++++++++++++++ crates/cli/src/process/mod.rs | 35 ++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/crates/cli/src/hooks/encoding.rs b/crates/cli/src/hooks/encoding.rs index 2b32c3999..8dbc5ea88 100644 --- a/crates/cli/src/hooks/encoding.rs +++ b/crates/cli/src/hooks/encoding.rs @@ -182,6 +182,7 @@ pub(super) fn hook_command_for_platform( } fn render_hook_command(relay: &Path, arguments: &[String], windows: bool) -> String { + let relay = relay_for_command(relay, windows); let command = std::iter::once(relay.display().to_string()) .chain(arguments.iter().cloned()) .map(|argument| crate::process::shell_quote_arg_for_platform(&argument, windows)) @@ -194,6 +195,20 @@ fn render_hook_command(relay: &Path, arguments: &[String], windows: bool) -> Str } } +#[cfg(windows)] +fn relay_for_command(relay: &Path, windows: bool) -> std::path::PathBuf { + if windows { + crate::process::short_windows_path(relay).unwrap_or_else(|| relay.to_path_buf()) + } else { + relay.to_path_buf() + } +} + +#[cfg(not(windows))] +fn relay_for_command(relay: &Path, _windows: bool) -> std::path::PathBuf { + relay.to_path_buf() +} + // `cmd.exe` accepts at most 8,191 characters. Leave room for `/C` and host-added text. #[cfg(windows)] const MAX_WINDOWS_HOOK_COMMAND_UTF16_UNITS: usize = 8_000; diff --git a/crates/cli/src/process/mod.rs b/crates/cli/src/process/mod.rs index 748830000..a341a582d 100644 --- a/crates/cli/src/process/mod.rs +++ b/crates/cli/src/process/mod.rs @@ -72,6 +72,41 @@ pub(crate) fn portable_executable_path(path: PathBuf) -> PathBuf { .unwrap_or(path) } +#[cfg(windows)] +pub(crate) fn short_windows_path(path: &Path) -> Option { + use std::os::windows::ffi::{OsStrExt, OsStringExt}; + + let source = path + .as_os_str() + .encode_wide() + .chain(std::iter::once(0)) + .collect::>(); + // SAFETY: `source` is NUL-terminated and the null output buffer is explicitly supported + // for querying the required output length. + let required = unsafe { + windows_sys::Win32::Storage::FileSystem::GetShortPathNameW( + source.as_ptr(), + std::ptr::null_mut(), + 0, + ) + }; + if required == 0 { + return None; + } + let mut destination = vec![0_u16; required as usize + 1]; + // SAFETY: both buffers are valid for the supplied lengths and `destination` has room for + // the documented terminating NUL. + let written = unsafe { + windows_sys::Win32::Storage::FileSystem::GetShortPathNameW( + source.as_ptr(), + destination.as_mut_ptr(), + destination.len() as u32, + ) + }; + (written > 0 && written <= required) + .then(|| PathBuf::from(OsString::from_wide(&destination[..written as usize]))) +} + #[cfg(not(windows))] pub(crate) fn portable_executable_path(path: PathBuf) -> PathBuf { path From deee198c8b39fb06499613bdd26c3e2fabe604cf Mon Sep 17 00:00:00 2001 From: Will Killian Date: Tue, 8 Sep 2026 14:45:56 -0400 Subject: [PATCH 10/16] ci: scope package metadata to build target Signed-off-by: Will Killian --- .github/workflows/ci_rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci_rust.yml b/.github/workflows/ci_rust.yml index ab86eec31..ff972d29f 100644 --- a/.github/workflows/ci_rust.yml +++ b/.github/workflows/ci_rust.yml @@ -279,7 +279,7 @@ jobs: fi just set-cargo-version "$version" # Refresh workspace package versions without changing resolved dependencies. - cargo metadata --offline --format-version 1 > /dev/null + cargo metadata --offline --filter-platform "${{ matrix.target }}" --format-version 1 > /dev/null printf 'NEMO_RELAY_CLI_PACKAGE_VERSION=%s\n' "$version" >> "$GITHUB_ENV" - name: Install musl tools From 9545269b4bda06a6ac1ea0cef4b36dff71a4fbd4 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Tue, 8 Sep 2026 15:06:58 -0400 Subject: [PATCH 11/16] ci: align Go coverage target with baseline Signed-off-by: Will Killian --- codecov.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codecov.yml b/codecov.yml index c52334c14..a4f16fdd6 100644 --- a/codecov.yml +++ b/codecov.yml @@ -80,7 +80,7 @@ component_management: - "crates/ffi/src" statuses: - type: project - target: 95% + target: 94% threshold: 0.5% base: auto if_ci_failed: error From 9eace4b6e668948d04b34633a76809df899a2713 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Tue, 8 Sep 2026 15:13:40 -0400 Subject: [PATCH 12/16] test: cover file-backed hook configuration validation Signed-off-by: Will Killian --- .../tests/coverage/shared/installer_tests.rs | 92 ++++++++++++++++--- 1 file changed, 77 insertions(+), 15 deletions(-) diff --git a/crates/cli/tests/coverage/shared/installer_tests.rs b/crates/cli/tests/coverage/shared/installer_tests.rs index eb5e0a73d..9b82650e1 100644 --- a/crates/cli/tests/coverage/shared/installer_tests.rs +++ b/crates/cli/tests/coverage/shared/installer_tests.rs @@ -10,18 +10,10 @@ use serde_json::Value; use crate::agents::CodingAgent; -#[test] -fn private_hook_config_round_trips_and_rejects_agent_mismatch() { - let directory = tempfile::tempdir().unwrap(); - let path = directory.path().join("hook.json"); - HookCommandConfig::transparent(CodingAgent::Codex, "http://127.0.0.1:1234") - .write(&path) - .unwrap(); - - let config = HookCommandConfig::load(&path).unwrap(); - let mut request = HookForwardRequest { - agent: CodingAgent::ClaudeCode, - hook_config: Some(path), +fn hook_request(agent: CodingAgent) -> HookForwardRequest { + HookForwardRequest { + agent, + hook_config: None, gateway_url: None, generation_file: None, generation_token: None, @@ -31,13 +23,83 @@ fn private_hook_config_round_trips_and_rejects_agent_mismatch() { session_metadata: None, gateway_mode: None, failure_policy: HookFailurePolicy::Default, - }; + } +} + +#[test] +fn private_hook_config_round_trips_and_hydrates_a_hook_request() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("hook.json"); + HookCommandConfig::transparent(CodingAgent::Codex, "http://127.0.0.1:1234") + .write(&path) + .unwrap(); + + let config = HookCommandConfig::load(&path).unwrap(); + let mut request = hook_request(CodingAgent::Codex); + config.apply(&mut request).unwrap(); + assert_eq!( + request.gateway_url.as_deref(), + Some("http://127.0.0.1:1234") + ); + assert!(request.transparent_run); + assert!(request.generation_file.is_none()); + assert!(request.generation_token.is_none()); +} + +#[test] +fn private_hook_config_rejects_agent_mismatch_and_inline_configuration() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("hook.json"); + HookCommandConfig::transparent(CodingAgent::Codex, "http://127.0.0.1:1234") + .write(&path) + .unwrap(); + + let mut agent_mismatch = hook_request(CodingAgent::ClaudeCode); assert!( - config - .apply(&mut request) + HookCommandConfig::load(&path) + .unwrap() + .apply(&mut agent_mismatch) .unwrap_err() .contains("requested claude") ); + + let mut inline_configuration = hook_request(CodingAgent::Codex); + inline_configuration.gateway_url = Some("http://127.0.0.1:5678".into()); + assert!( + HookCommandConfig::load(&path) + .unwrap() + .apply(&mut inline_configuration) + .unwrap_err() + .contains("cannot be combined") + ); +} + +#[test] +fn private_hook_config_rejects_unknown_and_incomplete_values() { + let directory = tempfile::tempdir().unwrap(); + let unknown = directory.path().join("unknown.json"); + std::fs::write( + &unknown, + r#"{"version":1,"agent":"codex","gateway_url":"http://127.0.0.1:1234","forward_only":false,"transparent_run":true,"unexpected":true}"#, + ) + .unwrap(); + assert!( + HookCommandConfig::load(&unknown) + .unwrap_err() + .contains("failed to parse") + ); + + let incomplete = directory.path().join("incomplete.json"); + std::fs::write( + &incomplete, + r#"{"version":1,"agent":"codex","gateway_url":"http://127.0.0.1:1234","generation_file":"generation","generation_token":null,"forward_only":false,"transparent_run":false}"#, + ) + .unwrap(); + assert!( + HookCommandConfig::load(&incomplete) + .unwrap_err() + .contains("both generation file and token") + ); } struct BootstrapConfigHome { From fcd7ee525d5c5add02c78531579ccd8a5da1d02d Mon Sep 17 00:00:00 2001 From: Will Killian Date: Tue, 8 Sep 2026 16:21:24 -0400 Subject: [PATCH 13/16] fix: harden generated hook configuration Signed-off-by: Will Killian --- crates/cli/src/commands/hook_forward.rs | 1 - crates/cli/src/hooks/config.rs | 81 ++++++++++++++++++- crates/cli/src/hooks/delivery.rs | 16 ++-- crates/cli/src/hooks/encoding.rs | 31 ++++--- crates/cli/src/hooks/mod.rs | 8 +- crates/cli/src/hooks/types.rs | 1 - .../src/installation/marketplace/assets.rs | 2 +- .../cli/src/installation/marketplace/mod.rs | 2 +- .../cli/src/installation/marketplace/state.rs | 2 +- .../tests/coverage/shared/installer_tests.rs | 77 ++++++++++++++++-- docs/nemo-relay-cli/plugin-installation.mdx | 23 +++--- 11 files changed, 200 insertions(+), 44 deletions(-) diff --git a/crates/cli/src/commands/hook_forward.rs b/crates/cli/src/commands/hook_forward.rs index 2deb5e455..a313b9fd8 100644 --- a/crates/cli/src/commands/hook_forward.rs +++ b/crates/cli/src/commands/hook_forward.rs @@ -22,7 +22,6 @@ pub(crate) struct HookForwardCommand { "generation_file", "generation_token", "forward_only", - "transparent_run", "profile", "session_metadata", "gateway_mode" diff --git a/crates/cli/src/hooks/config.rs b/crates/cli/src/hooks/config.rs index a8ebee3b6..60e3944b0 100644 --- a/crates/cli/src/hooks/config.rs +++ b/crates/cli/src/hooks/config.rs @@ -3,6 +3,7 @@ //! Private, installer-owned configuration for generated coding-agent hooks. +use std::io::Read; use std::path::{Path, PathBuf}; use serde::{Deserialize, Serialize}; @@ -71,7 +72,7 @@ impl HookCommandConfig { } pub(crate) fn load(path: &Path) -> Result { - let bytes = std::fs::read(path).map_err(|error| { + let bytes = read_private_hook_config(path).map_err(|error| { format!( "failed to read hook configuration {}: {error}", path.display() @@ -95,7 +96,7 @@ impl HookCommandConfig { request.agent.as_arg() )); } - if request.has_inline_configuration() { + if request.has_inline_configuration() || request.transparent_run != self.transparent_run { return Err( "--hook-config cannot be combined with inline hook configuration options".into(), ); @@ -133,3 +134,79 @@ impl HookCommandConfig { Ok(()) } } + +/// Reads a Relay-owned hook config without following a replacement symlink. +/// +/// The configuration carries generation credentials or a process-private gateway URL. On Unix, +/// require an owner-only file and a current-user-owned, non-group/world-writable parent before +/// opening it with `O_NOFOLLOW`. The post-open metadata check closes the replacement race between +/// inspecting the path and reading its bytes. +#[cfg(unix)] +fn read_private_hook_config(path: &Path) -> Result, std::io::Error> { + use std::fs::{self, OpenOptions}; + use std::os::unix::fs::{MetadataExt, OpenOptionsExt}; + + let expected_uid = unsafe { libc::geteuid() }; + let metadata = fs::symlink_metadata(path)?; + if !metadata.file_type().is_file() + || metadata.uid() != expected_uid + || metadata.mode() & 0o077 != 0 + { + return Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "hook configuration must be a current-user-owned owner-only regular file", + )); + } + let parent = path.parent().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "hook configuration has no parent", + ) + })?; + let parent_metadata = fs::symlink_metadata(parent)?; + if !parent_metadata.file_type().is_dir() + || parent_metadata.uid() != expected_uid + || parent_metadata.mode() & 0o022 != 0 + { + return Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "hook configuration parent must be current-user-owned and non-group/world-writable", + )); + } + + let mut file = OpenOptions::new() + .read(true) + .custom_flags(libc::O_NOFOLLOW) + .open(path)?; + let opened = file.metadata()?; + if !opened.is_file() + || opened.uid() != expected_uid + || opened.mode() & 0o077 != 0 + || opened.dev() != metadata.dev() + || opened.ino() != metadata.ino() + { + return Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "hook configuration changed while it was opened", + )); + } + let mut bytes = Vec::new(); + file.read_to_end(&mut bytes)?; + Ok(bytes) +} + +#[cfg(windows)] +fn read_private_hook_config(path: &Path) -> Result, std::io::Error> { + if !crate::filesystem::windows_path_is_private(path)? { + return Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "hook configuration must have the Relay private owner/System access control list", + )); + } + std::fs::read(path) +} + +#[cfg(not(any(unix, windows)))] +fn read_private_hook_config(path: &Path) -> Result, std::io::Error> { + std::fs::read(path) +} diff --git a/crates/cli/src/hooks/delivery.rs b/crates/cli/src/hooks/delivery.rs index db1578fb3..550df2c73 100644 --- a/crates/cli/src/hooks/delivery.rs +++ b/crates/cli/src/hooks/delivery.rs @@ -22,6 +22,12 @@ const HOOK_FORWARD_TIMEOUT: Duration = Duration::from_secs(2); pub(crate) async fn hook_forward(mut command: HookForwardRequest) -> Result<(), CliError> { let fail_closed = command.failure_policy.fail_closed(); + // Persistent hooks do not carry this marker, so they can become inert without reading a + // stale configuration file while a process-private transparent gateway is active. Generated + // transparent wrappers do carry it and still load their private configuration fail-closed. + if transparent_hook_is_inert(&command) { + return Ok(()); + } if let Some(path) = command.hook_config.clone() && let Err(error) = super::HookCommandConfig::load(&path).and_then(|config| config.apply(&mut command)) @@ -33,12 +39,6 @@ pub(crate) async fn hook_forward(mut command: HookForwardRequest) -> Result<(), { return handle_hook_error(error, fail_closed); } - // A transparent wrapper can coexist with any installed Relay plugin. Its process marker makes - // persistent plugin hooks inert, while only the wrapper-owned command carries - // `--transparent-run` and forwards to the process-private gateway. - if transparent_run_active() && !command.transparent_run { - return Ok(()); - } let destination = hook_destination(&command); let persistent = match persistent_gateway(&destination) { Ok(persistent) => persistent, @@ -106,6 +106,10 @@ pub(crate) async fn hook_forward(mut command: HookForwardRequest) -> Result<(), handle_hook_forward_response(response, fail_closed).await } +pub(crate) fn transparent_hook_is_inert(command: &HookForwardRequest) -> bool { + transparent_run_active() && !command.transparent_run +} + fn persistent_gateway( destination: &super::destination::HookDestination, ) -> Result, CliError> { diff --git a/crates/cli/src/hooks/encoding.rs b/crates/cli/src/hooks/encoding.rs index 8dbc5ea88..fdd267e12 100644 --- a/crates/cli/src/hooks/encoding.rs +++ b/crates/cli/src/hooks/encoding.rs @@ -3,7 +3,7 @@ //! Hook definition and portable command encoding. -use std::path::Path; +use std::path::{Path, PathBuf}; use serde_json::{Value, json}; @@ -58,7 +58,7 @@ pub(crate) fn persistent_hook_forward_commands( ) -> Result { hook_commands( relay, - &hook_config_arguments(agent, &config_path(generation_file)), + &hook_config_arguments(agent, &persistent_hook_config_path(generation_file), false), ) } @@ -68,7 +68,10 @@ pub(crate) fn transparent_hook_forward_commands( agent: CodingAgent, gateway_url: &str, ) -> Result { - hook_commands(relay, &hook_config_arguments(agent, Path::new(gateway_url))) + hook_commands( + relay, + &hook_config_arguments(agent, Path::new(gateway_url), true), + ) } pub(crate) fn transparent_hook_forward_commands_with_config( @@ -76,7 +79,7 @@ pub(crate) fn transparent_hook_forward_commands_with_config( agent: CodingAgent, hook_config: &Path, ) -> Result { - hook_commands(relay, &hook_config_arguments(agent, hook_config)) + hook_commands(relay, &hook_config_arguments(agent, hook_config, true)) } #[cfg(test)] @@ -88,7 +91,7 @@ pub(crate) fn transparent_hook_forward_commands_for_platform( ) -> GeneratedHookCommands { hook_commands_for_platform( relay, - &hook_config_arguments(agent, Path::new(gateway_url)), + &hook_config_arguments(agent, Path::new(gateway_url), true), windows, ) } @@ -103,22 +106,30 @@ pub(crate) fn persistent_hook_forward_commands_for_platform( ) -> GeneratedHookCommands { hook_commands_for_platform( relay, - &hook_config_arguments(agent, &config_path(generation_file)), + &hook_config_arguments(agent, &persistent_hook_config_path(generation_file), false), windows, ) } -fn config_path(generation_file: &Path) -> std::path::PathBuf { +pub(crate) fn persistent_hook_config_path(generation_file: &Path) -> PathBuf { generation_file.with_file_name(".nemo-relay-hook-config.json") } -pub(super) fn hook_config_arguments(agent: CodingAgent, hook_config: &Path) -> Vec { - vec![ +pub(super) fn hook_config_arguments( + agent: CodingAgent, + hook_config: &Path, + transparent_run: bool, +) -> Vec { + let mut arguments = vec![ "hook-forward".into(), agent.as_arg().into(), "--hook-config".into(), hook_config.display().to_string(), - ] + ]; + if transparent_run { + arguments.push("--transparent-run".into()); + } + arguments } fn hook_commands(relay: &Path, arguments: &[String]) -> Result { diff --git a/crates/cli/src/hooks/mod.rs b/crates/cli/src/hooks/mod.rs index 407eae257..8a1e9e98b 100644 --- a/crates/cli/src/hooks/mod.rs +++ b/crates/cli/src/hooks/mod.rs @@ -17,7 +17,9 @@ pub(crate) use delivery::hook_forward; #[cfg(test)] pub(crate) use delivery::send_verified_hook_forward_request; #[cfg(test)] -pub(crate) use delivery::{gateway_headers, insert_header, read_hook_payload_from}; +pub(crate) use delivery::{ + gateway_headers, insert_header, read_hook_payload_from, transparent_hook_is_inert, +}; #[cfg(test)] pub(crate) use destination::{ HookGatewayLifecycle, resolve_hook_destination, transparent_gateway_spec, @@ -25,8 +27,8 @@ pub(crate) use destination::{ #[cfg(test)] pub(crate) use encoding::transparent_hook_forward_commands; pub(crate) use encoding::{ - GeneratedHookCommands, generated_policy_hooks, persistent_hook_forward_commands, - transparent_hook_forward_commands_with_config, + GeneratedHookCommands, generated_policy_hooks, persistent_hook_config_path, + persistent_hook_forward_commands, transparent_hook_forward_commands_with_config, }; #[cfg(test)] pub(crate) use encoding::{ diff --git a/crates/cli/src/hooks/types.rs b/crates/cli/src/hooks/types.rs index 45b28e730..018346dff 100644 --- a/crates/cli/src/hooks/types.rs +++ b/crates/cli/src/hooks/types.rs @@ -26,7 +26,6 @@ impl HookForwardRequest { || self.generation_file.is_some() || self.generation_token.is_some() || self.forward_only - || self.transparent_run || self.profile.is_some() || self.session_metadata.is_some() || self.gateway_mode.is_some() diff --git a/crates/cli/src/installation/marketplace/assets.rs b/crates/cli/src/installation/marketplace/assets.rs index 63c6334ab..3212cf57b 100644 --- a/crates/cli/src/installation/marketplace/assets.rs +++ b/crates/cli/src/installation/marketplace/assets.rs @@ -118,6 +118,6 @@ pub(super) fn plugin_hooks( generation_fence: &Path, generation_token: &str, ) -> Result { - let hook_config = generation_fence.with_file_name(".nemo-relay-hook-config.json"); + let hook_config = crate::hooks::persistent_hook_config_path(generation_fence); host.plugin_hooks(relay, generation_fence, generation_token, &hook_config) } diff --git a/crates/cli/src/installation/marketplace/mod.rs b/crates/cli/src/installation/marketplace/mod.rs index 24d036244..92910acb8 100644 --- a/crates/cli/src/installation/marketplace/mod.rs +++ b/crates/cli/src/installation/marketplace/mod.rs @@ -1570,7 +1570,7 @@ fn collect_host_plugin_readiness( if let Some(plugin) = readiness.plugin.as_ref() { let generation_fence = plugin.join(crate::installation::generation::GENERATION_FILE_NAME); - let hook_config = plugin.join(".nemo-relay-hook-config.json"); + let hook_config = crate::hooks::persistent_hook_config_path(&generation_fence); readiness.push( "Generated hooks", InstallGeneration::capture(generation_fence.clone()).and_then(|generation| { diff --git a/crates/cli/src/installation/marketplace/state.rs b/crates/cli/src/installation/marketplace/state.rs index 62bb55f0b..d3d6b905d 100644 --- a/crates/cli/src/installation/marketplace/state.rs +++ b/crates/cli/src/installation/marketplace/state.rs @@ -74,7 +74,7 @@ impl PluginLayout { host.install_arg() )); let hooks_path = plugin_root.join("hooks").join("hooks.json"); - let hook_config = plugin_root.join(".nemo-relay-hook-config.json"); + let hook_config = crate::hooks::persistent_hook_config_path(&generation_fence); let state_path = state_path(host, install_dir); Self { host_arg: host.install_arg(), diff --git a/crates/cli/tests/coverage/shared/installer_tests.rs b/crates/cli/tests/coverage/shared/installer_tests.rs index 9b82650e1..e3dd94227 100644 --- a/crates/cli/tests/coverage/shared/installer_tests.rs +++ b/crates/cli/tests/coverage/shared/installer_tests.rs @@ -26,6 +26,18 @@ fn hook_request(agent: CodingAgent) -> HookForwardRequest { } } +fn write_private_hook_config(path: &std::path::Path, contents: &str) { + std::fs::write(path, contents).unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600)).unwrap(); + } + #[cfg(windows)] + crate::filesystem::protect_private_windows_path(path).unwrap(); +} + #[test] fn private_hook_config_round_trips_and_hydrates_a_hook_request() { let directory = tempfile::tempdir().unwrap(); @@ -36,6 +48,7 @@ fn private_hook_config_round_trips_and_hydrates_a_hook_request() { let config = HookCommandConfig::load(&path).unwrap(); let mut request = hook_request(CodingAgent::Codex); + request.transparent_run = true; config.apply(&mut request).unwrap(); assert_eq!( request.gateway_url.as_deref(), @@ -78,11 +91,10 @@ fn private_hook_config_rejects_agent_mismatch_and_inline_configuration() { fn private_hook_config_rejects_unknown_and_incomplete_values() { let directory = tempfile::tempdir().unwrap(); let unknown = directory.path().join("unknown.json"); - std::fs::write( + write_private_hook_config( &unknown, r#"{"version":1,"agent":"codex","gateway_url":"http://127.0.0.1:1234","forward_only":false,"transparent_run":true,"unexpected":true}"#, - ) - .unwrap(); + ); assert!( HookCommandConfig::load(&unknown) .unwrap_err() @@ -90,11 +102,10 @@ fn private_hook_config_rejects_unknown_and_incomplete_values() { ); let incomplete = directory.path().join("incomplete.json"); - std::fs::write( + write_private_hook_config( &incomplete, r#"{"version":1,"agent":"codex","gateway_url":"http://127.0.0.1:1234","generation_file":"generation","generation_token":null,"forward_only":false,"transparent_run":false}"#, - ) - .unwrap(); + ); assert!( HookCommandConfig::load(&incomplete) .unwrap_err() @@ -102,6 +113,56 @@ fn private_hook_config_rejects_unknown_and_incomplete_values() { ); } +#[cfg(unix)] +#[test] +fn private_hook_config_rejects_symlinks_and_broad_permissions() { + use std::os::unix::fs::{PermissionsExt, symlink}; + + let directory = tempfile::tempdir().unwrap(); + let config = directory.path().join("hook.json"); + HookCommandConfig::transparent(CodingAgent::Codex, "http://127.0.0.1:1234") + .write(&config) + .unwrap(); + let link = directory.path().join("hook-link.json"); + symlink(&config, &link).unwrap(); + assert!( + HookCommandConfig::load(&link) + .unwrap_err() + .contains("owner-only regular file") + ); + + std::fs::set_permissions(&config, std::fs::Permissions::from_mode(0o644)).unwrap(); + assert!( + HookCommandConfig::load(&config) + .unwrap_err() + .contains("owner-only regular file") + ); +} + +#[test] +fn transparent_run_skips_stale_persistent_hook_config_before_loading_it() { + let _guard = crate::test_support::ENV_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let previous = std::env::var_os(crate::configuration::TRANSPARENT_RUN_ENV); + // SAFETY: The process-wide environment lock is held for this test. + unsafe { std::env::set_var(crate::configuration::TRANSPARENT_RUN_ENV, "1") }; + let mut request = hook_request(CodingAgent::Codex); + request.hook_config = Some(std::path::PathBuf::from( + "missing-persistent-hook-config.json", + )); + request.failure_policy = HookFailurePolicy::FailClosed; + let result = crate::hooks::transparent_hook_is_inert(&request); + // SAFETY: The process-wide environment lock is still held for this test. + unsafe { + match previous { + Some(value) => std::env::set_var(crate::configuration::TRANSPARENT_RUN_ENV, value), + None => std::env::remove_var(crate::configuration::TRANSPARENT_RUN_ENV), + } + } + assert!(result); +} + struct BootstrapConfigHome { _guard: std::sync::MutexGuard<'static, ()>, previous: Option, @@ -441,13 +502,13 @@ fn generated_hook_dispatch_covers_all_agents() { false, ) .for_event("PreToolUse"), - "/abs/path/to/nemo-relay hook-forward codex --hook-config /private/nemo-relay-hook.json --fail-closed" + "/abs/path/to/nemo-relay hook-forward codex --hook-config /private/nemo-relay-hook.json --transparent-run --fail-closed" ); let relay = Path::new("/opt/NeMo Relay's & tools/nemo-relay"); assert_eq!( transparent_hook_forward_commands_for_platform(relay, CodingAgent::Codex, config, false) .for_event("SessionStart"), - r#"'/opt/NeMo Relay'\''s & tools/nemo-relay' hook-forward codex --hook-config /private/nemo-relay-hook.json --fail-open"# + r#"'/opt/NeMo Relay'\''s & tools/nemo-relay' hook-forward codex --hook-config /private/nemo-relay-hook.json --transparent-run --fail-open"# ); let native = transparent_hook_forward_commands(Path::new("nemo-relay"), CodingAgent::Codex, config) diff --git a/docs/nemo-relay-cli/plugin-installation.mdx b/docs/nemo-relay-cli/plugin-installation.mdx index 35a6323ff..38b97e8b0 100644 --- a/docs/nemo-relay-cli/plugin-installation.mdx +++ b/docs/nemo-relay-cli/plugin-installation.mdx @@ -233,16 +233,19 @@ always terminates the gateway process tree. ### Hook Delivery and Upgrade Safety Before contacting the gateway, Relay rejects a fixed-endpoint `hook-forward` -command that lacks a valid installer-owned generation fence. It uses the -configured hook failure policy. This prevents a legacy hook retained by a host -process from reviving a retired installation. If install or uninstall reports a -missing or invalid generation marker, follow its cleanup instructions: close -the host and standalone `nemo-relay mcp` processes, remove the stale -registration and state it identifies, and then run the requested `--force` -command. - -Transparent hooks embed their process-private gateway URL and never recover a -persistent gateway, so they do not need this fence. Source plugins and custom +command that lacks a valid installer-owned generation fence. Generated +persistent hook commands reference a private Relay-owned configuration file; +that file contains the generation-file path and immutable generation identity. +Relay uses the configured hook failure policy. This prevents a legacy hook +retained by a host process from reviving a retired installation. If install or +uninstall reports a missing or invalid generation marker, follow its cleanup +instructions: close the host and standalone `nemo-relay mcp` processes, remove +the stale registration and state it identifies, and then run the requested +`--force` command. + +Transparent hooks reference a temporary private configuration file containing +their process-private gateway URL and never recover a persistent gateway, so +they do not need this fence. Source plugins and custom automation can use `--forward-only` to contact an existing gateway without a fence. That mode waits for an authenticated, configuration-compatible Relay gateway and rejects foreign or incompatible listeners before it sends the From 8ea8f8c15b4635e182d8cc13b5761864e98d82c7 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Tue, 8 Sep 2026 17:48:04 -0400 Subject: [PATCH 14/16] fix: compile private hook config on Windows Signed-off-by: Will Killian --- crates/cli/src/hooks/config.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/cli/src/hooks/config.rs b/crates/cli/src/hooks/config.rs index 60e3944b0..2518e9eb9 100644 --- a/crates/cli/src/hooks/config.rs +++ b/crates/cli/src/hooks/config.rs @@ -3,9 +3,11 @@ //! Private, installer-owned configuration for generated coding-agent hooks. -use std::io::Read; use std::path::{Path, PathBuf}; +#[cfg(unix)] +use std::io::Read; + use serde::{Deserialize, Serialize}; use crate::agents::CodingAgent; From ab34637b2dd0c37ced05d2ad8be30291ee072b4a Mon Sep 17 00:00:00 2001 From: Will Killian Date: Tue, 8 Sep 2026 19:33:49 -0400 Subject: [PATCH 15/16] fix: address hook configuration review feedback Signed-off-by: Will Killian --- crates/cli/src/filesystem/atomic.rs | 135 +++++++++++++++++- crates/cli/src/filesystem/mod.rs | 4 +- crates/cli/src/hooks/config.rs | 13 +- crates/cli/src/hooks/mod.rs | 4 +- .../cli/src/installation/marketplace/mod.rs | 5 +- .../tests/coverage/shared/installer_tests.rs | 22 ++- 6 files changed, 159 insertions(+), 24 deletions(-) diff --git a/crates/cli/src/filesystem/atomic.rs b/crates/cli/src/filesystem/atomic.rs index b866e2e4c..c63353c3a 100644 --- a/crates/cli/src/filesystem/atomic.rs +++ b/crates/cli/src/filesystem/atomic.rs @@ -199,7 +199,7 @@ fn create_private_windows_file(path: &Path) -> io::Result { pub(crate) fn open_private_windows_file(path: &Path) -> io::Result { use windows_sys::Win32::Foundation::{GENERIC_READ, GENERIC_WRITE}; use windows_sys::Win32::Storage::FileSystem::{ - FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, OPEN_ALWAYS, + FILE_ATTRIBUTE_NORMAL, FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, OPEN_ALWAYS, }; let file = with_private_windows_descriptor(|descriptor| { @@ -209,12 +209,53 @@ pub(crate) fn open_private_windows_file(path: &Path) -> io::Result { GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, OPEN_ALWAYS, + FILE_ATTRIBUTE_NORMAL, ) })?; protect_private_windows_path(path)?; Ok(file) } +/// Opens an existing private file without following a reparse point. +#[cfg(windows)] +pub(crate) fn open_private_windows_file_for_read(path: &Path) -> io::Result { + use std::os::windows::io::AsRawHandle; + use windows_sys::Win32::Foundation::GENERIC_READ; + use windows_sys::Win32::Storage::FileSystem::{ + BY_HANDLE_FILE_INFORMATION, FILE_ATTRIBUTE_NORMAL, FILE_ATTRIBUTE_REPARSE_POINT, + FILE_FLAG_OPEN_REPARSE_POINT, FILE_SHARE_READ, GetFileInformationByHandle, OPEN_EXISTING, + }; + + let file = with_private_windows_descriptor(|descriptor| { + open_windows_file( + path, + descriptor, + GENERIC_READ, + FILE_SHARE_READ, + OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL | FILE_FLAG_OPEN_REPARSE_POINT, + ) + })?; + let mut information = BY_HANDLE_FILE_INFORMATION::default(); + // SAFETY: `file` owns a valid handle and `information` points to writable storage. + if unsafe { GetFileInformationByHandle(file.as_raw_handle().cast(), &mut information) } == 0 { + return Err(io::Error::last_os_error()); + } + if information.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT != 0 { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "private file must not be a reparse point", + )); + } + if !windows_handle_is_private(file.as_raw_handle().cast())? { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "private file must have the Relay private owner/System access control list", + )); + } + Ok(file) +} + /// Applies and verifies the protected owner/System DACL used for secret-bearing Windows paths. #[cfg(windows)] pub(crate) fn protect_private_windows_path(path: &Path) -> io::Result<()> { @@ -361,6 +402,82 @@ pub(crate) fn windows_path_is_private(path: &Path) -> io::Result { with_private_windows_descriptor(|expected| Ok(actual == windows_dacl_sddl(expected)?)) } +#[cfg(windows)] +fn windows_handle_is_private(handle: windows_sys::Win32::Foundation::HANDLE) -> io::Result { + use windows_sys::Win32::Foundation::{ERROR_SUCCESS, LocalFree}; + use windows_sys::Win32::Security::Authorization::{GetSecurityInfo, SE_FILE_OBJECT}; + use windows_sys::Win32::Security::{ + DACL_SECURITY_INFORMATION, EqualSid, GetTokenInformation, OWNER_SECURITY_INFORMATION, + PSECURITY_DESCRIPTOR, TOKEN_QUERY, TOKEN_USER, TokenUser, + }; + use windows_sys::Win32::System::Threading::{GetCurrentProcess, OpenProcessToken}; + + let mut owner = std::ptr::null_mut(); + let mut descriptor: PSECURITY_DESCRIPTOR = std::ptr::null_mut(); + // SAFETY: `handle` is valid and the output pointers remain writable for the call. + let status = unsafe { + GetSecurityInfo( + handle, + SE_FILE_OBJECT, + OWNER_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION, + &mut owner, + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::null_mut(), + &mut descriptor, + ) + }; + if status != ERROR_SUCCESS || descriptor.is_null() || owner.is_null() { + return Err(io::Error::from_raw_os_error(status as i32)); + } + let result = (|| { + let mut token = std::ptr::null_mut(); + // SAFETY: GetCurrentProcess returns a valid pseudo-handle and `token` is writable. + if unsafe { OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut token) } == 0 { + return Err(io::Error::last_os_error()); + } + let owner_matches = (|| { + let mut required = 0; + // SAFETY: This sizing call intentionally supplies a null output buffer. + unsafe { + GetTokenInformation(token, TokenUser, std::ptr::null_mut(), 0, &mut required) + }; + if required == 0 { + return Err(io::Error::last_os_error()); + } + let word = std::mem::size_of::(); + let mut buffer = vec![0_usize; (required as usize).div_ceil(word)]; + // SAFETY: The aligned buffer has at least `required` writable bytes. + if unsafe { + GetTokenInformation( + token, + TokenUser, + buffer.as_mut_ptr().cast(), + required, + &mut required, + ) + } == 0 + { + return Err(io::Error::last_os_error()); + } + // SAFETY: GetTokenInformation initialized a TOKEN_USER at the aligned buffer address. + let user = unsafe { &*buffer.as_ptr().cast::() }; + // SAFETY: Both SIDs remain valid while their backing storage is alive. + Ok(unsafe { EqualSid(owner, user.User.Sid) != 0 }) + })(); + // SAFETY: `token` is an owned handle returned by OpenProcessToken. + unsafe { windows_sys::Win32::Foundation::CloseHandle(token) }; + if !owner_matches? { + return Ok(false); + } + let actual = windows_dacl_sddl(descriptor)?; + with_private_windows_descriptor(|expected| Ok(actual == windows_dacl_sddl(expected)?)) + })(); + // SAFETY: GetSecurityInfo allocated `descriptor` for the caller. + unsafe { LocalFree(descriptor.cast()) }; + result +} + #[cfg(windows)] fn windows_dacl_sddl( descriptor: windows_sys::Win32::Security::PSECURITY_DESCRIPTOR, @@ -450,9 +567,16 @@ fn create_windows_file( descriptor: windows_sys::Win32::Security::PSECURITY_DESCRIPTOR, ) -> io::Result { use windows_sys::Win32::Foundation::GENERIC_WRITE; - use windows_sys::Win32::Storage::FileSystem::CREATE_NEW; + use windows_sys::Win32::Storage::FileSystem::{CREATE_NEW, FILE_ATTRIBUTE_NORMAL}; - open_windows_file(path, descriptor, GENERIC_WRITE, 0, CREATE_NEW) + open_windows_file( + path, + descriptor, + GENERIC_WRITE, + 0, + CREATE_NEW, + FILE_ATTRIBUTE_NORMAL, + ) } #[cfg(windows)] @@ -462,11 +586,12 @@ fn open_windows_file( desired_access: u32, share_mode: u32, creation_disposition: u32, + flags_and_attributes: u32, ) -> io::Result { use std::os::windows::io::FromRawHandle; use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE; use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; - use windows_sys::Win32::Storage::FileSystem::{CreateFileW, FILE_ATTRIBUTE_NORMAL}; + use windows_sys::Win32::Storage::FileSystem::CreateFileW; let path = windows_wide(path.as_os_str()); let attributes = SECURITY_ATTRIBUTES { @@ -483,7 +608,7 @@ fn open_windows_file( share_mode, &attributes, creation_disposition, - FILE_ATTRIBUTE_NORMAL, + flags_and_attributes, std::ptr::null_mut(), ) }; diff --git a/crates/cli/src/filesystem/mod.rs b/crates/cli/src/filesystem/mod.rs index fc41d681e..19cfcd3b9 100644 --- a/crates/cli/src/filesystem/mod.rs +++ b/crates/cli/src/filesystem/mod.rs @@ -15,8 +15,8 @@ pub(crate) use atomic::windows_wide; pub(crate) use atomic::{atomic_write, atomic_write_private, atomic_write_system_readable}; #[cfg(windows)] pub(crate) use atomic::{ - atomic_write_with_windows_dacl, open_private_windows_file, protect_private_windows_path, - read_windows_dacl, windows_path_is_private, + atomic_write_with_windows_dacl, open_private_windows_file, open_private_windows_file_for_read, + protect_private_windows_path, read_windows_dacl, windows_path_is_private, }; #[cfg(all(test, windows))] pub(crate) use locks::normalize_lock_attempt; diff --git a/crates/cli/src/hooks/config.rs b/crates/cli/src/hooks/config.rs index 2518e9eb9..6759d0499 100644 --- a/crates/cli/src/hooks/config.rs +++ b/crates/cli/src/hooks/config.rs @@ -5,7 +5,7 @@ use std::path::{Path, PathBuf}; -#[cfg(unix)] +#[cfg(any(unix, windows))] use std::io::Read; use serde::{Deserialize, Serialize}; @@ -199,13 +199,10 @@ fn read_private_hook_config(path: &Path) -> Result, std::io::Error> { #[cfg(windows)] fn read_private_hook_config(path: &Path) -> Result, std::io::Error> { - if !crate::filesystem::windows_path_is_private(path)? { - return Err(std::io::Error::new( - std::io::ErrorKind::PermissionDenied, - "hook configuration must have the Relay private owner/System access control list", - )); - } - std::fs::read(path) + let mut file = crate::filesystem::open_private_windows_file_for_read(path)?; + let mut bytes = Vec::new(); + file.read_to_end(&mut bytes)?; + Ok(bytes) } #[cfg(not(any(unix, windows)))] diff --git a/crates/cli/src/hooks/mod.rs b/crates/cli/src/hooks/mod.rs index 8a1e9e98b..cd0aab243 100644 --- a/crates/cli/src/hooks/mod.rs +++ b/crates/cli/src/hooks/mod.rs @@ -17,9 +17,7 @@ pub(crate) use delivery::hook_forward; #[cfg(test)] pub(crate) use delivery::send_verified_hook_forward_request; #[cfg(test)] -pub(crate) use delivery::{ - gateway_headers, insert_header, read_hook_payload_from, transparent_hook_is_inert, -}; +pub(crate) use delivery::{gateway_headers, insert_header, read_hook_payload_from}; #[cfg(test)] pub(crate) use destination::{ HookGatewayLifecycle, resolve_hook_destination, transparent_gateway_spec, diff --git a/crates/cli/src/installation/marketplace/mod.rs b/crates/cli/src/installation/marketplace/mod.rs index 92910acb8..28e4bf3fb 100644 --- a/crates/cli/src/installation/marketplace/mod.rs +++ b/crates/cli/src/installation/marketplace/mod.rs @@ -1568,8 +1568,9 @@ fn collect_host_plugin_readiness( .map(|_| "hook-forward is supported".into()), ); if let Some(plugin) = readiness.plugin.as_ref() { - let generation_fence = - plugin.join(crate::installation::generation::GENERATION_FILE_NAME); + let generation_fence = plugin + .join(crate::installation::generation::GENERATION_FILE_NAME) + .canonicalize_or_self(); let hook_config = crate::hooks::persistent_hook_config_path(&generation_fence); readiness.push( "Generated hooks", diff --git a/crates/cli/tests/coverage/shared/installer_tests.rs b/crates/cli/tests/coverage/shared/installer_tests.rs index e3dd94227..d03c763f6 100644 --- a/crates/cli/tests/coverage/shared/installer_tests.rs +++ b/crates/cli/tests/coverage/shared/installer_tests.rs @@ -137,10 +137,24 @@ fn private_hook_config_rejects_symlinks_and_broad_permissions() { .unwrap_err() .contains("owner-only regular file") ); + + let unsafe_parent = directory.path().join("unsafe-parent"); + std::fs::create_dir(&unsafe_parent).unwrap(); + let private_config = unsafe_parent.join("hook.json"); + HookCommandConfig::transparent(CodingAgent::Codex, "http://127.0.0.1:1234") + .write(&private_config) + .unwrap(); + std::fs::set_permissions(&unsafe_parent, std::fs::Permissions::from_mode(0o777)).unwrap(); + assert!( + HookCommandConfig::load(&private_config) + .unwrap_err() + .contains("parent must be current-user-owned and non-group/world-writable") + ); } -#[test] -fn transparent_run_skips_stale_persistent_hook_config_before_loading_it() { +#[tokio::test] +#[allow(clippy::await_holding_lock)] // The process-wide environment lock must cover the hook call. +async fn transparent_run_skips_stale_persistent_hook_config_before_loading_it() { let _guard = crate::test_support::ENV_TEST_LOCK .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); @@ -152,7 +166,7 @@ fn transparent_run_skips_stale_persistent_hook_config_before_loading_it() { "missing-persistent-hook-config.json", )); request.failure_policy = HookFailurePolicy::FailClosed; - let result = crate::hooks::transparent_hook_is_inert(&request); + let result = crate::hooks::hook_forward(request).await; // SAFETY: The process-wide environment lock is still held for this test. unsafe { match previous { @@ -160,7 +174,7 @@ fn transparent_run_skips_stale_persistent_hook_config_before_loading_it() { None => std::env::remove_var(crate::configuration::TRANSPARENT_RUN_ENV), } } - assert!(result); + assert!(result.is_ok()); } struct BootstrapConfigHome { From c365d48b021eaa3854db2dfeebe9e5e4125bf978 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Tue, 8 Sep 2026 19:47:16 -0400 Subject: [PATCH 16/16] fix: preserve generated hook paths in readiness Signed-off-by: Will Killian --- .../src/installation/marketplace/assets.rs | 2 +- .../cli/src/installation/marketplace/mod.rs | 29 ++++++++++--------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/crates/cli/src/installation/marketplace/assets.rs b/crates/cli/src/installation/marketplace/assets.rs index 3212cf57b..932346ab1 100644 --- a/crates/cli/src/installation/marketplace/assets.rs +++ b/crates/cli/src/installation/marketplace/assets.rs @@ -103,7 +103,7 @@ pub(super) fn plugin_mcp_config( host.plugin_mcp_config(server) } -fn absolute_or_self(path: &Path) -> Result { +pub(super) fn absolute_or_self(path: &Path) -> Result { if path.is_absolute() { return Ok(path.to_owned()); } diff --git a/crates/cli/src/installation/marketplace/mod.rs b/crates/cli/src/installation/marketplace/mod.rs index 28e4bf3fb..e7daaeaee 100644 --- a/crates/cli/src/installation/marketplace/mod.rs +++ b/crates/cli/src/installation/marketplace/mod.rs @@ -28,7 +28,7 @@ use crate::installation::{InstallRequest, UninstallRequest}; use crate::installation::operation_lock::{DEFAULT_OPERATION_LOCK_TIMEOUT, PluginOperationLock}; use assets::{ - marketplace_manifest, plugin_hooks, plugin_manifest, plugin_mcp_config, + absolute_or_self, marketplace_manifest, plugin_hooks, plugin_manifest, plugin_mcp_config, write_plugin_marketplace, write_plugin_marketplace_for_generation, }; use host::{ @@ -1568,21 +1568,22 @@ fn collect_host_plugin_readiness( .map(|_| "hook-forward is supported".into()), ); if let Some(plugin) = readiness.plugin.as_ref() { - let generation_fence = plugin - .join(crate::installation::generation::GENERATION_FILE_NAME) - .canonicalize_or_self(); - let hook_config = crate::hooks::persistent_hook_config_path(&generation_fence); + let generation_fence = + plugin.join(crate::installation::generation::GENERATION_FILE_NAME); readiness.push( "Generated hooks", - InstallGeneration::capture(generation_fence.clone()).and_then(|generation| { - crate::hooks::HookCommandConfig::load(&hook_config)?; - let expected = - plugin_hooks(host, &relay, &generation_fence, generation.token())?; - generated_manifest_check( - &plugin.join("hooks").join("hooks.json"), - &expected, - "hooks", - ) + absolute_or_self(&generation_fence).and_then(|generation_fence| { + let hook_config = crate::hooks::persistent_hook_config_path(&generation_fence); + InstallGeneration::capture(generation_fence.clone()).and_then(|generation| { + crate::hooks::HookCommandConfig::load(&hook_config)?; + let expected = + plugin_hooks(host, &relay, &generation_fence, generation.token())?; + generated_manifest_check( + &plugin.join("hooks").join("hooks.json"), + &expected, + "hooks", + ) + }) }), ); }