@sigitcodereview / sigit
Code Commits Issues Pulls Cloud Agent CI/CD
claude/github-issue-40-0au5ua
claude/github-issue-40-0au5ua claude/linked-issue-acp-tui-u4vvpa claude/minor-release-cut-vkugow claude/sigit-acp-local-chat-cx6380 claude/sigit-cloud-agent-expansion-reox0a claude/tool-permission-system claude/zen-feynman-0u78dk development feature/acp-cancel-request feature/acp-session-list-cursor feature/acp-v1-conformance feature/agent-tools-multiedit-glob-todos-remember feature/cargo-workspace feature/hooks feature/load-local-model-explicitly feature/sigit-agent-profile feature/sigit-code-cloud feature/xcode-mcp-server fix/acp-replay-loaded-session-history fix/acp-replay-reasoning-and-attachments fix/dsml-doubled-bar-markers main release/v1.5.10 release/v1.5.6 release/v1.6.1
rs 507 lines 18.8 KB
Raw
1 //! Extensibility hooks for agent lifecycle and tool execution.
2 //!
3 //! Hooks allow users to run custom logic at key moments:
4 //! - `SessionStart`: when a session begins (new/load/fork)
5 //! - `PreToolUse`: before a tool is executed
6 //! - `PostToolUse`: after a tool is executed
7 //!
8 //! Hooks are configured in `settings.toml` under `[hooks]`:
9 //! ```toml
10 //! [hooks]
11 //! session_start = ["echo 'Starting siGit session'"]
12 //! pre_tool_use = ["echo 'About to run {tool_name}'"]
13 //! post_tool_use = ["echo 'Finished {tool_name}: {tool_result_len} chars'"]
14 //! ```
15 //!
16 //! Hooks support substitution for context variables:
17 //! - SessionStart: `{cwd}`, `{session_id}`
18 //! - PreToolUse: `{tool_name}`, `{tool_args_len}`
19 //! - PostToolUse: `{tool_name}`, `{tool_result_len}`
20 //!
21 //! Every value used in `{var}` substitution is quote-escaped for the platform
22 //! shell before being spliced into the command line (`posix_quote` /
23 //! `windows_quote`), so a project directory or MCP-server-supplied tool name
24 //! containing shell metacharacters can never execute as shell syntax. The
25 //! same values are also exported as `SIGIT_HOOK_*` env vars, which hook
26 //! scripts should prefer when they need to consume a value programmatically
27 //! rather than embed it in a `{var}` placeholder.
28 //!
29 //! Hooks run in the session cwd and inherit the environment. Shell hook
30 //! failures are logged but do not interrupt the session. `PreToolUse` /
31 //! `PostToolUse` hooks fire for every call to [`crate::tools::execute_tool`],
32 //! including tool calls made by a `task` subagent — there is no separate
33 //! "top-level only" mode today.
34
35 use serde::{Deserialize, Serialize};
36 use std::io::Read;
37 use std::path::Path;
38 use std::process::{Command, Stdio};
39 use std::time::{Duration, Instant};
40
41 /// How long a single hook command may run before it is killed, unless
42 /// `[hooks] timeout_secs` overrides it. Without a bound, a hook that never
43 /// exits (waits on input, a stalled network mount, a deadlock) blocks the
44 /// tool call or session start that fired it forever.
45 const DEFAULT_HOOK_TIMEOUT_SECS: u64 = 30;
46
47 /// Hook configuration: lists of shell commands to run at key moments.
48 #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
49 pub struct HookSettings {
50 /// Commands to run when a session starts (new/load/fork).
51 #[serde(default)]
52 pub session_start: Vec<String>,
53 /// Commands to run before a tool is executed.
54 #[serde(default)]
55 pub pre_tool_use: Vec<String>,
56 /// Commands to run after a tool is executed.
57 #[serde(default)]
58 pub post_tool_use: Vec<String>,
59 /// Per-hook wall-clock limit in seconds. `None` uses
60 /// [`DEFAULT_HOOK_TIMEOUT_SECS`]. Raise it for slow hooks (a build, a
61 /// linter); a hook that exceeds it is killed and logged.
62 #[serde(default)]
63 pub timeout_secs: Option<u64>,
64 }
65
66 impl HookSettings {
67 /// Whether any hooks are configured.
68 pub fn has_hooks(&self) -> bool {
69 !self.session_start.is_empty()
70 || !self.pre_tool_use.is_empty()
71 || !self.post_tool_use.is_empty()
72 }
73
74 /// The wall-clock limit applied to each hook command.
75 fn timeout(&self) -> Duration {
76 Duration::from_secs(self.timeout_secs.unwrap_or(DEFAULT_HOOK_TIMEOUT_SECS))
77 }
78 }
79
80 /// Context passed to hook substitution.
81 pub struct HookContext {
82 pub cwd: Option<String>,
83 pub session_id: Option<String>,
84 pub tool_name: Option<String>,
85 pub tool_args_len: Option<usize>,
86 pub tool_result_len: Option<usize>,
87 }
88
89 impl HookContext {
90 /// Create a new empty context.
91 pub fn new() -> Self {
92 Self {
93 cwd: None,
94 session_id: None,
95 tool_name: None,
96 tool_args_len: None,
97 tool_result_len: None,
98 }
99 }
100
101 /// Substitute `{var}` placeholders in the command string, passing each
102 /// value through `quote` first so it can never be interpreted as shell
103 /// syntax regardless of its contents.
104 fn substitute(&self, cmd: &str, quote: fn(&str) -> String) -> String {
105 let mut result = cmd.to_string();
106 if let Some(cwd) = &self.cwd {
107 result = result.replace("{cwd}", &quote(cwd));
108 }
109 if let Some(session_id) = &self.session_id {
110 result = result.replace("{session_id}", &quote(session_id));
111 }
112 if let Some(tool_name) = &self.tool_name {
113 result = result.replace("{tool_name}", &quote(tool_name));
114 }
115 if let Some(tool_args_len) = self.tool_args_len {
116 // Numeric — always a safe literal, no quoting needed.
117 result = result.replace("{tool_args_len}", &tool_args_len.to_string());
118 }
119 if let Some(tool_result_len) = self.tool_result_len {
120 result = result.replace("{tool_result_len}", &tool_result_len.to_string());
121 }
122 result
123 }
124
125 /// The same context values as `SIGIT_HOOK_*` environment variables, so a
126 /// hook script can read them without any quoting or substitution at all.
127 fn env_vars(&self) -> Vec<(&'static str, String)> {
128 let mut vars = Vec::new();
129 if let Some(cwd) = &self.cwd {
130 vars.push(("SIGIT_HOOK_CWD", cwd.clone()));
131 }
132 if let Some(session_id) = &self.session_id {
133 vars.push(("SIGIT_HOOK_SESSION_ID", session_id.clone()));
134 }
135 if let Some(tool_name) = &self.tool_name {
136 vars.push(("SIGIT_HOOK_TOOL_NAME", tool_name.clone()));
137 }
138 if let Some(tool_args_len) = self.tool_args_len {
139 vars.push(("SIGIT_HOOK_TOOL_ARGS_LEN", tool_args_len.to_string()));
140 }
141 if let Some(tool_result_len) = self.tool_result_len {
142 vars.push(("SIGIT_HOOK_TOOL_RESULT_LEN", tool_result_len.to_string()));
143 }
144 vars
145 }
146 }
147
148 impl Default for HookContext {
149 fn default() -> Self {
150 Self::new()
151 }
152 }
153
154 /// Wrap `value` as a single POSIX shell word, so it is always treated as
155 /// literal data by `sh -c` even if it contains spaces, quotes, or shell
156 /// metacharacters (e.g. a project directory or an MCP-server-supplied tool
157 /// name that happens to contain `$(...)` or backticks).
158 ///
159 /// Only reached from production code on Unix (`run_hook`'s `#[cfg(unix)]`
160 /// arm); always exercised via tests below regardless of host platform.
161 #[cfg_attr(not(unix), allow(dead_code))]
162 fn posix_quote(value: &str) -> String {
163 format!("'{}'", value.replace('\'', r"'\''"))
164 }
165
166 /// Best-effort quoting for `cmd.exe`: wrap in double quotes and escape
167 /// embedded double quotes. `cmd` has no fully safe quoting story (see
168 /// `spawn_shell`'s platform split in `tools.rs`), so this covers common
169 /// cases but callers on Windows should prefer the `SIGIT_HOOK_*` env vars
170 /// (set on every hook invocation, see [`run_hook`]) over `{cwd}`-style
171 /// substitution when values may contain special characters.
172 ///
173 /// Only reached from production code on Windows (`run_hook`'s `#[cfg(windows)]`
174 /// arm); exercised on every platform via `test_windows_quote_neutralizes_embedded_quotes`
175 /// below, since `aws-lc-sys` can't cross-compile to the Windows target from this host.
176 #[cfg_attr(not(windows), allow(dead_code))]
177 fn windows_quote(value: &str) -> String {
178 format!("\"{}\"", value.replace('"', "\"\""))
179 }
180
181 /// Run a single hook command in the given context. Substituted `{var}`
182 /// placeholders are shell-quoted so a value like a directory name or an
183 /// MCP tool name can never break out of its position and run as shell
184 /// syntax. The same values are also exported as `SIGIT_HOOK_*` env vars
185 /// (`SIGIT_HOOK_CWD`, `SIGIT_HOOK_SESSION_ID`, `SIGIT_HOOK_TOOL_NAME`,
186 /// `SIGIT_HOOK_TOOL_ARGS_LEN`, `SIGIT_HOOK_TOOL_RESULT_LEN`) so hook scripts
187 /// can read them without any quoting concerns at all.
188 fn run_hook(cmd: &str, context: &HookContext, cwd: &Path, timeout: Duration) {
189 #[cfg(unix)]
190 let (shell, flag, substituted) = ("sh", "-c", context.substitute(cmd, posix_quote));
191 #[cfg(windows)]
192 let (shell, flag, substituted) = ("cmd", "/C", context.substitute(cmd, windows_quote));
193
194 log::debug!("Running hook: {substituted}");
195 let mut command = Command::new(shell);
196 command
197 .arg(flag)
198 .arg(&substituted)
199 .current_dir(cwd)
200 .envs(context.env_vars())
201 .stdout(Stdio::piped())
202 .stderr(Stdio::piped());
203 // Put the hook in its own process group so a timeout kill can take out the
204 // whole tree. Killing just the shell is not enough: `sh -c` may fork
205 // rather than exec (dash does), and the surviving grandchild would both
206 // keep running and hold the pipe write ends open, blocking the drains
207 // below until it exits on its own.
208 #[cfg(unix)]
209 std::os::unix::process::CommandExt::process_group(&mut command, 0);
210 let mut child = match command.spawn() {
211 Ok(child) => child,
212 Err(err) => {
213 log::warn!("Failed to run hook: {err}");
214 return;
215 }
216 };
217
218 #[cfg(unix)]
219 fn kill_hook(child: &mut std::process::Child) {
220 // SIGKILL the process group (negative pid); the child is its leader.
221 // Fall back to killing the child alone if that fails.
222 if unsafe { libc::kill(-(child.id() as i32), libc::SIGKILL) } != 0 {
223 let _ = child.kill();
224 }
225 let _ = child.wait();
226 }
227 #[cfg(windows)]
228 fn kill_hook(child: &mut std::process::Child) {
229 let _ = child.kill();
230 let _ = child.wait();
231 }
232
233 // Drain stdout/stderr on their own threads so a hook that writes more than
234 // the pipe buffer holds (a build, say) keeps making progress instead of
235 // blocking on a full pipe and looking like a hang. The threads report back
236 // over a channel rather than being joined: a receive can be bounded, while
237 // a join could block forever on a pipe a detached grandchild still holds.
238 let (out_tx, out_rx) = std::sync::mpsc::channel::<(u8, Vec<u8>)>();
239 let drain = |tag: u8, pipe: Option<Box<dyn Read + Send>>| {
240 let tx = out_tx.clone();
241 std::thread::spawn(move || {
242 let mut buf = Vec::new();
243 if let Some(mut p) = pipe {
244 let _ = p.read_to_end(&mut buf);
245 }
246 let _ = tx.send((tag, buf));
247 });
248 };
249 drain(0, child.stdout.take().map(|p| Box::new(p) as _));
250 drain(1, child.stderr.take().map(|p| Box::new(p) as _));
251 drop(out_tx);
252
253 let deadline = Instant::now() + timeout;
254 let status = loop {
255 match child.try_wait() {
256 Ok(Some(status)) => break Some(status),
257 Ok(None) => {
258 if Instant::now() >= deadline {
259 kill_hook(&mut child);
260 break None;
261 }
262 std::thread::sleep(Duration::from_millis(50));
263 }
264 Err(err) => {
265 log::warn!("Failed to wait on hook: {err}");
266 kill_hook(&mut child);
267 break None;
268 }
269 }
270 };
271
272 // Collect what the drains read, bounded: if something in the hook's tree
273 // outlived the kill (or daemonized past the process group) and still holds
274 // a pipe open, give up on its output after a short grace instead of
275 // hanging the caller on it.
276 let mut stdout = Vec::new();
277 let mut stderr = Vec::new();
278 let grace = Duration::from_secs(1);
279 for _ in 0..2 {
280 match out_rx.recv_timeout(grace) {
281 Ok((0, buf)) => stdout = buf,
282 Ok((_, buf)) => stderr = buf,
283 Err(_) => {
284 log::debug!("Hook output pipe still open after exit; skipping its output");
285 break;
286 }
287 }
288 }
289
290 match status {
291 None => log::warn!(
292 "Hook timed out after {}s and was killed: {substituted}",
293 timeout.as_secs()
294 ),
295 Some(status) if !status.success() => log::warn!(
296 "Hook failed with status {:?}: {}",
297 status.code(),
298 String::from_utf8_lossy(&stderr).trim()
299 ),
300 Some(_) => {
301 let out = String::from_utf8_lossy(&stdout);
302 if !out.trim().is_empty() {
303 log::debug!("Hook output: {}", out.trim());
304 }
305 }
306 }
307 }
308
309 /// Run all session start hooks.
310 pub fn run_session_start_hooks(hooks: &HookSettings, cwd: &Path, session_id: &str) {
311 if hooks.session_start.is_empty() {
312 return;
313 }
314 log::info!("Running {} session_start hooks", hooks.session_start.len());
315 let context = HookContext {
316 cwd: Some(cwd.display().to_string()),
317 session_id: Some(session_id.to_string()),
318 ..HookContext::new()
319 };
320 for cmd in &hooks.session_start {
321 run_hook(cmd, &context, cwd, hooks.timeout());
322 }
323 }
324
325 /// Run all pre-tool-use hooks.
326 pub fn run_pre_tool_use_hooks(hooks: &HookSettings, tool_name: &str, tool_args: &str, cwd: &Path) {
327 if hooks.pre_tool_use.is_empty() {
328 return;
329 }
330 log::debug!("Running {} pre_tool_use hooks", hooks.pre_tool_use.len());
331 let context = HookContext {
332 cwd: Some(cwd.display().to_string()),
333 tool_name: Some(tool_name.to_string()),
334 tool_args_len: Some(tool_args.len()),
335 ..HookContext::new()
336 };
337 for cmd in &hooks.pre_tool_use {
338 run_hook(cmd, &context, cwd, hooks.timeout());
339 }
340 }
341
342 /// Run all post-tool-use hooks.
343 pub fn run_post_tool_use_hooks(
344 hooks: &HookSettings,
345 tool_name: &str,
346 tool_result: &str,
347 cwd: &Path,
348 ) {
349 if hooks.post_tool_use.is_empty() {
350 return;
351 }
352 log::debug!("Running {} post_tool_use hooks", hooks.post_tool_use.len());
353 let context = HookContext {
354 cwd: Some(cwd.display().to_string()),
355 tool_name: Some(tool_name.to_string()),
356 tool_result_len: Some(tool_result.len()),
357 ..HookContext::new()
358 };
359 for cmd in &hooks.post_tool_use {
360 run_hook(cmd, &context, cwd, hooks.timeout());
361 }
362 }
363
364 #[cfg(test)]
365 mod tests {
366 use super::*;
367
368 #[cfg(unix)]
369 #[test]
370 fn hook_that_hangs_is_killed_at_the_timeout() {
371 // A hook that would sleep far past the deadline must be killed near
372 // it, not block the caller for the full sleep. The two-command form
373 // forces the shell to fork `sleep` rather than exec it (what dash does
374 // even for a single command), so this also proves the kill takes out
375 // the whole process group: a surviving grandchild would hold the
376 // output pipes open and stall the caller until its own exit.
377 let context = HookContext::new();
378 let start = Instant::now();
379 run_hook(
380 "sleep 30; echo done",
381 &context,
382 Path::new("."),
383 Duration::from_millis(300),
384 );
385 assert!(
386 start.elapsed() < Duration::from_secs(5),
387 "hook should have been killed shortly after its 300ms timeout, took {:?}",
388 start.elapsed()
389 );
390 }
391
392 #[test]
393 fn timeout_defaults_when_unset_and_honors_override() {
394 let default = HookSettings::default();
395 assert_eq!(
396 default.timeout(),
397 Duration::from_secs(DEFAULT_HOOK_TIMEOUT_SECS)
398 );
399 let custom = HookSettings {
400 timeout_secs: Some(5),
401 ..HookSettings::default()
402 };
403 assert_eq!(custom.timeout(), Duration::from_secs(5));
404 }
405
406 #[test]
407 fn test_hook_context_substitution() {
408 let context = HookContext {
409 cwd: Some("/home/user/project".to_string()),
410 session_id: Some("session-123".to_string()),
411 tool_name: Some("read_file".to_string()),
412 tool_args_len: Some(42),
413 tool_result_len: Some(1000),
414 };
415 assert_eq!(
416 context.substitute("cd {cwd} && echo {session_id}", posix_quote),
417 "cd '/home/user/project' && echo 'session-123'"
418 );
419 assert_eq!(
420 context.substitute(
421 "Tool: {tool_name}, args={tool_args_len}, result={tool_result_len}",
422 posix_quote
423 ),
424 "Tool: 'read_file', args=42, result=1000"
425 );
426 }
427
428 #[test]
429 fn test_posix_quote_neutralizes_shell_metacharacters() {
430 // A directory or MCP tool name containing shell syntax must come
431 // through as inert literal text, not execute.
432 let context = HookContext {
433 cwd: Some("/tmp/$(rm -rf ~); echo pwned".to_string()),
434 ..HookContext::new()
435 };
436 let substituted = context.substitute("echo {cwd}", posix_quote);
437 assert_eq!(
438 substituted, "echo '/tmp/$(rm -rf ~); echo pwned'",
439 "the payload must stay inside single quotes, not break out of them"
440 );
441
442 // Embedded single quotes are the classic escape attempt; confirm the
443 // standard '\'' technique closes and reopens the quoted string
444 // safely instead of terminating it early.
445 let context = HookContext {
446 tool_name: Some("evil'; rm -rf ~; echo '".to_string()),
447 ..HookContext::new()
448 };
449 let substituted = context.substitute("echo {tool_name}", posix_quote);
450 assert_eq!(substituted, "echo 'evil'\\''; rm -rf ~; echo '\\'''");
451 }
452
453 #[test]
454 fn test_windows_quote_neutralizes_embedded_quotes() {
455 // aws-lc-sys can't cross-compile to the Windows target from this
456 // host, so this is exercised as a plain unit test rather than a
457 // Windows-target integration run.
458 assert_eq!(
459 windows_quote(r#"evil" & del /f /q *"#),
460 r#""evil"" & del /f /q *""#
461 );
462 assert_eq!(
463 windows_quote("/home/user/project"),
464 "\"/home/user/project\""
465 );
466 }
467
468 #[test]
469 fn test_pre_tool_use_hook_does_not_execute_injected_shell_syntax() {
470 // End-to-end: run a real `sh -c` (not just the string-substitution
471 // layer) with a tool_name crafted to look like a shell command
472 // injection, and confirm the injected `touch` never actually runs.
473 let dir = std::env::temp_dir().join(format!(
474 "sigit-hooks-test-{:?}-{}",
475 std::thread::current().id(),
476 std::process::id()
477 ));
478 std::fs::create_dir_all(&dir).unwrap();
479 let marker = dir.join("pwned");
480 assert!(!marker.exists());
481
482 let malicious_tool_name = format!("read_file'; touch {}; echo '", marker.display());
483 let hooks = HookSettings {
484 pre_tool_use: vec!["echo hook-ran {tool_name}".to_string()],
485 ..HookSettings::default()
486 };
487 run_pre_tool_use_hooks(&hooks, &malicious_tool_name, "{}", &dir);
488
489 assert!(
490 !marker.exists(),
491 "injected `touch` executed — {{tool_name}} substitution is not shell-safe"
492 );
493 std::fs::remove_dir_all(&dir).ok();
494 }
495
496 #[test]
497 fn test_hook_settings_has_hooks() {
498 let empty = HookSettings::default();
499 assert!(!empty.has_hooks());
500
501 let with_hooks = HookSettings {
502 session_start: vec!["echo start".to_string()],
503 ..HookSettings::default()
504 };
505 assert!(with_hooks.has_hooks());
506 }
507 }