From 904d4d3ea73613f9665f3538c5c0ddf168850818 Mon Sep 17 00:00:00 2001 From: Alex Hancock Date: Tue, 19 May 2026 14:13:15 -0400 Subject: [PATCH] feat(hooks): PreToolUse denial (#9304) Signed-off-by: Alex Hancock Co-authored-by: Douwe Osinga --- crates/goose/src/agents/agent.rs | 20 ++++- crates/goose/src/hooks/mod.rs | 127 ++++++++++++++++++++++++++----- 2 files changed, 127 insertions(+), 20 deletions(-) diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index cf785571bf..57dcec5d6b 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -919,9 +919,23 @@ impl Agent { .map(|a| serde_json::Value::Object(a.clone())), ) .with_working_dir(session.working_dir.to_string_lossy().to_string()); - self.hook_manager - .emit(crate::hooks::HookEvent::PreToolUse, ctx) - .await; + if let crate::hooks::HookDecision::Deny { reason, plugin } = self + .hook_manager + .emit_blocking(crate::hooks::HookEvent::PreToolUse, ctx) + .await + { + return ( + request_id, + Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!( + "Tool call denied by policy hook `{plugin}`: {reason}. \ + Do not retry; this is a policy denial, not a transient failure." + ), + None, + )), + ); + } } let tool_input_for_extended = tool_call diff --git a/crates/goose/src/hooks/mod.rs b/crates/goose/src/hooks/mod.rs index ea7f226682..2578694a4d 100644 --- a/crates/goose/src/hooks/mod.rs +++ b/crates/goose/src/hooks/mod.rs @@ -208,6 +208,12 @@ impl HookContext { } } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum HookDecision { + Allow, + Deny { reason: String, plugin: String }, +} + /// Loads and executes plugin hooks. #[derive(Debug, Default, Clone)] pub struct HookManager { @@ -297,9 +303,20 @@ impl HookManager { command = %command, "Running plugin hook", ); - if let Err(err) = - run_command_hook(command, &rule.plugin_root, &payload, *timeout).await - { + let res = run_command_hook(command, &rule.plugin_root, &payload, *timeout) + .await + .and_then(|o| { + if o.status.success() { + Ok(()) + } else { + anyhow::bail!( + "hook `{command}` exited with {:?}: {}", + o.status.code(), + String::from_utf8_lossy(&o.stderr).trim() + ) + } + }); + if let Err(err) = res { warn!( plugin = %rule.plugin_name, event = %event, @@ -311,6 +328,93 @@ impl HookManager { } } } + + /// Like [`Self::emit`], but stops at the first rule that denies the event + /// and returns the denial. A hook denies by exiting with status code 2 + /// (reason on stderr) or by printing `{"decision":"block","reason":"..."}` + /// to stdout. All other failures (spawn, timeout, other non-zero exits) + /// are logged and treated as Allow — a misbehaving hook MUST NOT block. + pub async fn emit_blocking(&self, event: HookEvent, ctx: HookContext) -> HookDecision { + let Some(rules) = self.rules.get(&event) else { + return HookDecision::Allow; + }; + + let payload = match serde_json::to_string(&ctx) { + Ok(s) => s, + Err(err) => { + warn!(event = %event, error = %err, "Failed to serialize hook context"); + return HookDecision::Allow; + } + }; + + for rule in rules { + if let Some(matcher) = &rule.matcher { + let target = ctx.matcher_context.as_deref().unwrap_or(""); + if !matcher.is_match(target) { + continue; + } + } + + for action in &rule.actions { + let LoadedAction::Command { command, timeout } = action; + let output = + match run_command_hook(command, &rule.plugin_root, &payload, *timeout).await { + Ok(o) => o, + Err(err) => { + warn!( + plugin = %rule.plugin_name, + event = %event, + command = %command, + error = %err, + "Plugin hook failed", + ); + continue; + } + }; + + if let Some(reason) = deny_reason(&output) { + info!( + plugin = %rule.plugin_name, + event = %event, + command = %command, + reason = %reason, + "Plugin hook denied tool call", + ); + return HookDecision::Deny { + reason, + plugin: rule.plugin_name.clone(), + }; + } + } + } + + HookDecision::Allow + } +} + +fn deny_reason(output: &std::process::Output) -> Option { + const DEFAULT: &str = "denied by plugin hook"; + let non_empty = |s: String| if s.is_empty() { DEFAULT.into() } else { s }; + + if output.status.code() == Some(2) { + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + return Some(non_empty(stderr)); + } + + #[derive(Deserialize)] + struct Resp { + decision: Option, + reason: Option, + } + + let stdout = String::from_utf8_lossy(&output.stdout); + let trimmed = stdout.trim(); + if !trimmed.starts_with('{') { + return None; + } + let parsed: Resp = serde_json::from_str(trimmed).ok()?; + (parsed.decision.as_deref() == Some("block")) + .then(|| non_empty(parsed.reason.unwrap_or_default())) } fn load_hooks_file( @@ -392,7 +496,7 @@ async fn run_command_hook( plugin_root: &Path, payload: &str, timeout: Duration, -) -> Result<()> { +) -> Result { let command = expand_plugin_root(raw_command, plugin_root); let mut child = Command::new("sh") .arg("-c") @@ -410,21 +514,10 @@ async fn run_command_hook( let _ = stdin.shutdown().await; } - let output = match tokio::time::timeout(timeout, child.wait_with_output()).await { - Ok(res) => res.with_context(|| format!("waiting on hook `{command}`"))?, + match tokio::time::timeout(timeout, child.wait_with_output()).await { + Ok(res) => res.with_context(|| format!("waiting on hook `{command}`")), Err(_) => anyhow::bail!("hook `{command}` timed out after {:?}", timeout), - }; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - anyhow::bail!( - "hook `{command}` exited with {:?}: {}", - output.status.code(), - stderr.trim() - ); } - - Ok(()) } fn expand_plugin_root(command: &str, plugin_root: &Path) -> String {