fix: bound hook execution with a timeout
run_hook called std::process::Command::output() with no timeout, straight from the async execute_tool path and from the ACP session start handlers. A hook that never returns (waits on input, a stalled network mount, a deadlocked build) blocked every tool call or session creation forever, with no error and no way to cancel short of killing the process. Each hook now runs under a wall-clock limit (30s by default, override with [hooks] timeout_secs) and is killed if it exceeds it. stdout and stderr are drained on their own threads so a hook that writes a lot, like a build, keeps making progress instead of blocking on a full pipe and looking like a hang.
paydii committed
Jul 11, 2026 at 15:10 UTC
ebfb3ad7bdeacfe526d07679e5120bb131f94f30
1 file changed
+122
-21
src/hooks.rs
+122
-21
@@ -33,8 +33,16 @@
33
//! "top-level only" mode today.
34
35
use serde::{Deserialize, Serialize};
36
+use std::io::Read;
37
use std::path::Path;
37
-use std::process::Command;
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)]
@@ -48,6 +56,11 @@ pub struct HookSettings {
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 {
@@ -57,6 +70,11 @@ impl HookSettings {
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.
@@ -167,37 +185,86 @@ fn windows_quote(value: &str) -> String {
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.
170
-fn run_hook(cmd: &str, context: &HookContext, cwd: &Path) {
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}");
177
- match Command::new(shell)
195
+ let mut child = match Command::new(shell)
196
.arg(flag)
197
.arg(&substituted)
198
.current_dir(cwd)
199
.envs(context.env_vars())
182
- .output()
200
+ .stdout(Stdio::piped())
201
+ .stderr(Stdio::piped())
202
+ .spawn()
203
{
184
- Ok(output) => {
185
- if !output.status.success() {
186
- let stderr = String::from_utf8_lossy(&output.stderr);
187
- log::warn!(
188
- "Hook failed with status {:?}: {}",
189
- output.status.code(),
190
- stderr.trim()
191
- );
192
- } else {
193
- let stdout = String::from_utf8_lossy(&output.stdout);
194
- if !stdout.is_empty() {
195
- log::debug!("Hook output: {}", stdout.trim());
204
+ Ok(child) => child,
205
+ Err(err) => {
206
+ log::warn!("Failed to run hook: {err}");
207
+ return;
208
+ }
209
+ };
210
+
211
+ // Drain stdout/stderr on their own threads so a hook that writes more than
212
+ // the pipe buffer holds (a build, say) keeps making progress instead of
213
+ // blocking on a full pipe and looking like a hang. The main thread polls
214
+ // for exit and enforces the deadline.
215
+ fn drain<R: Read + Send + 'static>(pipe: Option<R>) -> std::thread::JoinHandle<Vec<u8>> {
216
+ std::thread::spawn(move || {
217
+ let mut buf = Vec::new();
218
+ if let Some(mut p) = pipe {
219
+ let _ = p.read_to_end(&mut buf);
220
+ }
221
+ buf
222
+ })
223
+ }
224
+ let stdout_handle = drain(child.stdout.take());
225
+ let stderr_handle = drain(child.stderr.take());
226
+
227
+ let deadline = Instant::now() + timeout;
228
+ let status = loop {
229
+ match child.try_wait() {
230
+ Ok(Some(status)) => break Some(status),
231
+ Ok(None) => {
232
+ if Instant::now() >= deadline {
233
+ let _ = child.kill();
234
+ let _ = child.wait();
235
+ break None;
236
}
237
+ std::thread::sleep(Duration::from_millis(50));
238
+ }
239
+ Err(err) => {
240
+ log::warn!("Failed to wait on hook: {err}");
241
+ let _ = child.kill();
242
+ let _ = child.wait();
243
+ break None;
244
}
245
}
199
- Err(err) => {
200
- log::warn!("Failed to run hook: {err}");
246
+ };
247
+
248
+ // The child has exited (or been killed), so both pipes are closed and the
249
+ // drain threads finish promptly.
250
+ let stdout = stdout_handle.join().unwrap_or_default();
251
+ let stderr = stderr_handle.join().unwrap_or_default();
252
+
253
+ match status {
254
+ None => log::warn!(
255
+ "Hook timed out after {}s and was killed: {substituted}",
256
+ timeout.as_secs()
257
+ ),
258
+ Some(status) if !status.success() => log::warn!(
259
+ "Hook failed with status {:?}: {}",
260
+ status.code(),
261
+ String::from_utf8_lossy(&stderr).trim()
262
+ ),
263
+ Some(_) => {
264
+ let out = String::from_utf8_lossy(&stdout);
265
+ if !out.trim().is_empty() {
266
+ log::debug!("Hook output: {}", out.trim());
267
+ }
268
}
269
}
270
}
@@ -214,7 +281,7 @@ pub fn run_session_start_hooks(hooks: &HookSettings, cwd: &Path, session_id: &st
281
..HookContext::new()
282
};
283
for cmd in &hooks.session_start {
217
- run_hook(cmd, &context, cwd);
284
+ run_hook(cmd, &context, cwd, hooks.timeout());
285
}
286
}
287
@@ -231,7 +298,7 @@ pub fn run_pre_tool_use_hooks(hooks: &HookSettings, tool_name: &str, tool_args:
298
..HookContext::new()
299
};
300
for cmd in &hooks.pre_tool_use {
234
- run_hook(cmd, &context, cwd);
301
+ run_hook(cmd, &context, cwd, hooks.timeout());
302
}
303
}
304
@@ -253,7 +320,7 @@ pub fn run_post_tool_use_hooks(
320
..HookContext::new()
321
};
322
for cmd in &hooks.post_tool_use {
256
- run_hook(cmd, &context, cwd);
323
+ run_hook(cmd, &context, cwd, hooks.timeout());
324
}
325
}
326
@@ -261,6 +328,40 @@ pub fn run_post_tool_use_hooks(
328
mod tests {
329
use super::*;
330
331
+ #[cfg(unix)]
332
+ #[test]
333
+ fn hook_that_hangs_is_killed_at_the_timeout() {
334
+ // A hook that would sleep far past the deadline must be killed near it,
335
+ // not block the caller for the full sleep.
336
+ let context = HookContext::new();
337
+ let start = Instant::now();
338
+ run_hook(
339
+ "sleep 30",
340
+ &context,
341
+ Path::new("."),
342
+ Duration::from_millis(300),
343
+ );
344
+ assert!(
345
+ start.elapsed() < Duration::from_secs(5),
346
+ "hook should have been killed shortly after its 300ms timeout, took {:?}",
347
+ start.elapsed()
348
+ );
349
+ }
350
+
351
+ #[test]
352
+ fn timeout_defaults_when_unset_and_honors_override() {
353
+ let default = HookSettings::default();
354
+ assert_eq!(
355
+ default.timeout(),
356
+ Duration::from_secs(DEFAULT_HOOK_TIMEOUT_SECS)
357
+ );
358
+ let custom = HookSettings {
359
+ timeout_secs: Some(5),
360
+ ..HookSettings::default()
361
+ };
362
+ assert_eq!(custom.timeout(), Duration::from_secs(5));
363
+ }
364
+
365
#[test]
366
fn test_hook_context_substitution() {
367
let context = HookContext {