From ed9434683ea778c2b4efba42d5c95db50acb6a78 Mon Sep 17 00:00:00 2001
From: Umar Sabirin <umarsabirin369@gmail.com>
Date: Sun, 13 Sep 2026 20:49:31 +0700
Subject: [PATCH] fix(hooks): let a hook that ignores its stdin succeed

A hook that exits without reading its stdin was reported as failed. The
state is written with write_all and the error propagated, so when the
child closed the read end first the write returned EPIPE and the hook
was refused even though it had exited 0.

That is a race against the pipe buffer. A small payload usually lands
before the child goes away and the hook passes; a large one, or a busy
machine, does not. It surfaced as a_hook_that_succeeds_is_not_an_error
failing once in CI on a change that touched no Rust at all, and passing
on a re-run with nothing altered.

The runtime-spec writes the state to the hook's stdin but makes the exit
code the verdict. A hook is entitled to ignore what it is handed, and
most real ones do - they run a command and never read anything. So a
broken pipe on that write is not a failure, and the exit status decides
as it always did. Any other write error still fails, and now kills the
child rather than leaving it behind.

The new test does not depend on the race: it hands /bin/true a megabyte,
which cannot fit in the pipe buffer, so the write always outlives the
child. It fails deterministically without this change.
---
 src/container/hooks.rs | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/src/container/hooks.rs b/src/container/hooks.rs
index 1c22044..fb662aa 100644
--- a/src/container/hooks.rs
+++ b/src/container/hooks.rs
@@ -1,4 +1,4 @@
-use std::io::Write;
+use std::io::{ErrorKind, Write};
 use std::os::unix::process::CommandExt;
 use std::process::{Command, Stdio};
 use std::time::{Duration, Instant};
@@ -106,12 +106,20 @@ fn one(hook: &Hook, payload: &[u8]) -> Result<()> {
         .spawn()
         .ctx(format!("spawn hook {}", hook.path().display()))?;
 
-    child
+    let mut stdin = child
         .stdin
         .take()
-        .ok_or_else(|| Error::Invalid("hook stdin was not captured".to_string()))?
-        .write_all(payload)
-        .ctx("write the container state to the hook's stdin")?;
+        .ok_or_else(|| Error::Invalid("hook stdin was not captured".to_string()))?;
+    match stdin.write_all(payload) {
+        Ok(()) => {}
+        Err(error) if error.kind() == ErrorKind::BrokenPipe => {}
+        Err(error) => {
+            let _ = child.kill();
+            let _ = child.wait();
+            return Err(error).ctx("write the container state to the hook's stdin");
+        }
+    }
+    drop(stdin);
 
     let deadline = hook
         .timeout()
@@ -175,6 +183,18 @@ mod tests {
         run(Some(&hooks), Phase::CreateRuntime, &state()).unwrap();
     }
 
+    #[test]
+    fn a_hook_that_never_reads_its_stdin_is_not_an_error() {
+        let hook = HookBuilder::default()
+            .path(PathBuf::from("/bin/true"))
+            .build()
+            .unwrap();
+
+        let payload = vec![b'x'; 1 << 20];
+
+        one(&hook, &payload).unwrap();
+    }
+
     #[test]
     fn a_failing_create_runtime_hook_aborts_the_operation() {
         let hooks = HooksBuilder::default()