@sigitcodereview / sigit
Code Commits Issues Pulls Cloud Agent CI/CD
claude/sigit-cloud-agent-expansion-reox0a
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 510 lines 19 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 // Never the agent's stdin: in ACP mode that is the JSON-RPC pipe from
202 // the editor, and a hook reading it swallows the client's next request.
203 .stdin(Stdio::null())
204 .stdout(Stdio::piped())
205 .stderr(Stdio::piped());
206 // Put the hook in its own process group so a timeout kill can take out the
207 // whole tree. Killing just the shell is not enough: `sh -c` may fork
208 // rather than exec (dash does), and the surviving grandchild would both
209 // keep running and hold the pipe write ends open, blocking the drains
210 // below until it exits on its own.
211 #[cfg(unix)]
212 std::os::unix::process::CommandExt::process_group(&mut command, 0);
213 let mut child = match command.spawn() {
214 Ok(child) => child,
215 Err(err) => {
216 log::warn!("Failed to run hook: {err}");
217 return;
218 }
219 };
220
221 #[cfg(unix)]
222 fn kill_hook(child: &mut std::process::Child) {
223 // SIGKILL the process group (negative pid); the child is its leader.
224 // Fall back to killing the child alone if that fails.
225 if unsafe { libc::kill(-(child.id() as i32), libc::SIGKILL) } != 0 {
226 let _ = child.kill();
227 }
228 let _ = child.wait();
229 }
230 #[cfg(windows)]
231 fn kill_hook(child: &mut std::process::Child) {
232 let _ = child.kill();
233 let _ = child.wait();
234 }
235
236 // Drain stdout/stderr on their own threads so a hook that writes more than
237 // the pipe buffer holds (a build, say) keeps making progress instead of
238 // blocking on a full pipe and looking like a hang. The threads report back
239 // over a channel rather than being joined: a receive can be bounded, while
240 // a join could block forever on a pipe a detached grandchild still holds.
241 let (out_tx, out_rx) = std::sync::mpsc::channel::<(u8, Vec<u8>)>();
242 let drain = |tag: u8, pipe: Option<Box<dyn Read + Send>>| {
243 let tx = out_tx.clone();
244 std::thread::spawn(move || {
245 let mut buf = Vec::new();
246 if let Some(mut p) = pipe {
247 let _ = p.read_to_end(&mut buf);
248 }
249 let _ = tx.send((tag, buf));
250 });
251 };
252 drain(0, child.stdout.take().map(|p| Box::new(p) as _));
253 drain(1, child.stderr.take().map(|p| Box::new(p) as _));
254 drop(out_tx);
255
256 let deadline = Instant::now() + timeout;
257 let status = loop {
258 match child.try_wait() {
259 Ok(Some(status)) => break Some(status),
260 Ok(None) => {
261 if Instant::now() >= deadline {
262 kill_hook(&mut child);
263 break None;
264 }
265 std::thread::sleep(Duration::from_millis(50));
266 }
267 Err(err) => {
268 log::warn!("Failed to wait on hook: {err}");
269 kill_hook(&mut child);
270 break None;
271 }
272 }
273 };
274
275 // Collect what the drains read, bounded: if something in the hook's tree
276 // outlived the kill (or daemonized past the process group) and still holds
277 // a pipe open, give up on its output after a short grace instead of
278 // hanging the caller on it.
279 let mut stdout = Vec::new();
280 let mut stderr = Vec::new();
281 let grace = Duration::from_secs(1);
282 for _ in 0..2 {
283 match out_rx.recv_timeout(grace) {
284 Ok((0, buf)) => stdout = buf,
285 Ok((_, buf)) => stderr = buf,
286 Err(_) => {
287 log::debug!("Hook output pipe still open after exit; skipping its output");
288 break;
289 }
290 }
291 }
292
293 match status {
294 None => log::warn!(
295 "Hook timed out after {}s and was killed: {substituted}",
296 timeout.as_secs()
297 ),
298 Some(status) if !status.success() => log::warn!(
299 "Hook failed with status {:?}: {}",
300 status.code(),
301 String::from_utf8_lossy(&stderr).trim()
302 ),
303 Some(_) => {
304 let out = String::from_utf8_lossy(&stdout);
305 if !out.trim().is_empty() {
306 log::debug!("Hook output: {}", out.trim());
307 }
308 }
309 }
310 }
311
312 /// Run all session start hooks.
313 pub fn run_session_start_hooks(hooks: &HookSettings, cwd: &Path, session_id: &str) {
314 if hooks.session_start.is_empty() {
315 return;
316 }
317 log::info!("Running {} session_start hooks", hooks.session_start.len());
318 let context = HookContext {
319 cwd: Some(cwd.display().to_string()),
320 session_id: Some(session_id.to_string()),
321 ..HookContext::new()
322 };
323 for cmd in &hooks.session_start {
324 run_hook(cmd, &context, cwd, hooks.timeout());
325 }
326 }
327
328 /// Run all pre-tool-use hooks.
329 pub fn run_pre_tool_use_hooks(hooks: &HookSettings, tool_name: &str, tool_args: &str, cwd: &Path) {
330 if hooks.pre_tool_use.is_empty() {
331 return;
332 }
333 log::debug!("Running {} pre_tool_use hooks", hooks.pre_tool_use.len());
334 let context = HookContext {
335 cwd: Some(cwd.display().to_string()),
336 tool_name: Some(tool_name.to_string()),
337 tool_args_len: Some(tool_args.len()),
338 ..HookContext::new()
339 };
340 for cmd in &hooks.pre_tool_use {
341 run_hook(cmd, &context, cwd, hooks.timeout());
342 }
343 }
344
345 /// Run all post-tool-use hooks.
346 pub fn run_post_tool_use_hooks(
347 hooks: &HookSettings,
348 tool_name: &str,
349 tool_result: &str,
350 cwd: &Path,
351 ) {
352 if hooks.post_tool_use.is_empty() {
353 return;
354 }
355 log::debug!("Running {} post_tool_use hooks", hooks.post_tool_use.len());
356 let context = HookContext {
357 cwd: Some(cwd.display().to_string()),
358 tool_name: Some(tool_name.to_string()),
359 tool_result_len: Some(tool_result.len()),
360 ..HookContext::new()
361 };
362 for cmd in &hooks.post_tool_use {
363 run_hook(cmd, &context, cwd, hooks.timeout());
364 }
365 }
366
367 #[cfg(test)]
368 mod tests {
369 use super::*;
370
371 #[cfg(unix)]
372 #[test]
373 fn hook_that_hangs_is_killed_at_the_timeout() {
374 // A hook that would sleep far past the deadline must be killed near
375 // it, not block the caller for the full sleep. The two-command form
376 // forces the shell to fork `sleep` rather than exec it (what dash does
377 // even for a single command), so this also proves the kill takes out
378 // the whole process group: a surviving grandchild would hold the
379 // output pipes open and stall the caller until its own exit.
380 let context = HookContext::new();
381 let start = Instant::now();
382 run_hook(
383 "sleep 30; echo done",
384 &context,
385 Path::new("."),
386 Duration::from_millis(300),
387 );
388 assert!(
389 start.elapsed() < Duration::from_secs(5),
390 "hook should have been killed shortly after its 300ms timeout, took {:?}",
391 start.elapsed()
392 );
393 }
394
395 #[test]
396 fn timeout_defaults_when_unset_and_honors_override() {
397 let default = HookSettings::default();
398 assert_eq!(
399 default.timeout(),
400 Duration::from_secs(DEFAULT_HOOK_TIMEOUT_SECS)
401 );
402 let custom = HookSettings {
403 timeout_secs: Some(5),
404 ..HookSettings::default()
405 };
406 assert_eq!(custom.timeout(), Duration::from_secs(5));
407 }
408
409 #[test]
410 fn test_hook_context_substitution() {
411 let context = HookContext {
412 cwd: Some("/home/user/project".to_string()),
413 session_id: Some("session-123".to_string()),
414 tool_name: Some("read_file".to_string()),
415 tool_args_len: Some(42),
416 tool_result_len: Some(1000),
417 };
418 assert_eq!(
419 context.substitute("cd {cwd} && echo {session_id}", posix_quote),
420 "cd '/home/user/project' && echo 'session-123'"
421 );
422 assert_eq!(
423 context.substitute(
424 "Tool: {tool_name}, args={tool_args_len}, result={tool_result_len}",
425 posix_quote
426 ),
427 "Tool: 'read_file', args=42, result=1000"
428 );
429 }
430
431 #[test]
432 fn test_posix_quote_neutralizes_shell_metacharacters() {
433 // A directory or MCP tool name containing shell syntax must come
434 // through as inert literal text, not execute.
435 let context = HookContext {
436 cwd: Some("/tmp/$(rm -rf ~); echo pwned".to_string()),
437 ..HookContext::new()
438 };
439 let substituted = context.substitute("echo {cwd}", posix_quote);
440 assert_eq!(
441 substituted, "echo '/tmp/$(rm -rf ~); echo pwned'",
442 "the payload must stay inside single quotes, not break out of them"
443 );
444
445 // Embedded single quotes are the classic escape attempt; confirm the
446 // standard '\'' technique closes and reopens the quoted string
447 // safely instead of terminating it early.
448 let context = HookContext {
449 tool_name: Some("evil'; rm -rf ~; echo '".to_string()),
450 ..HookContext::new()
451 };
452 let substituted = context.substitute("echo {tool_name}", posix_quote);
453 assert_eq!(substituted, "echo 'evil'\\''; rm -rf ~; echo '\\'''");
454 }
455
456 #[test]
457 fn test_windows_quote_neutralizes_embedded_quotes() {
458 // aws-lc-sys can't cross-compile to the Windows target from this
459 // host, so this is exercised as a plain unit test rather than a
460 // Windows-target integration run.
461 assert_eq!(
462 windows_quote(r#"evil" & del /f /q *"#),
463 r#""evil"" & del /f /q *""#
464 );
465 assert_eq!(
466 windows_quote("/home/user/project"),
467 "\"/home/user/project\""
468 );
469 }
470
471 #[test]
472 fn test_pre_tool_use_hook_does_not_execute_injected_shell_syntax() {
473 // End-to-end: run a real `sh -c` (not just the string-substitution
474 // layer) with a tool_name crafted to look like a shell command
475 // injection, and confirm the injected `touch` never actually runs.
476 let dir = std::env::temp_dir().join(format!(
477 "sigit-hooks-test-{:?}-{}",
478 std::thread::current().id(),
479 std::process::id()
480 ));
481 std::fs::create_dir_all(&dir).unwrap();
482 let marker = dir.join("pwned");
483 assert!(!marker.exists());
484
485 let malicious_tool_name = format!("read_file'; touch {}; echo '", marker.display());
486 let hooks = HookSettings {
487 pre_tool_use: vec!["echo hook-ran {tool_name}".to_string()],
488 ..HookSettings::default()
489 };
490 run_pre_tool_use_hooks(&hooks, &malicious_tool_name, "{}", &dir);
491
492 assert!(
493 !marker.exists(),
494 "injected `touch` executed — {{tool_name}} substitution is not shell-safe"
495 );
496 std::fs::remove_dir_all(&dir).ok();
497 }
498
499 #[test]
500 fn test_hook_settings_has_hooks() {
501 let empty = HookSettings::default();
502 assert!(!empty.has_hooks());
503
504 let with_hooks = HookSettings {
505 session_start: vec!["echo start".to_string()],
506 ..HookSettings::default()
507 };
508 assert!(with_hooks.has_hooks());
509 }
510 }