From a5d06c23e6d22d533ed77ccc0369571e57805513 Mon Sep 17 00:00:00 2001
From: Janne Sinivirta <janne.sinivirta@gmail.com>
Date: Tue, 11 Aug 2026 16:04:45 +0300
Subject: [PATCH] Enforce --timeout when a command leaves a child running
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Stdout and Stderr are buffers rather than files, so os/exec pipes them and
Wait blocks until the pipe reaches EOF. CommandContext kills only the direct
child, so a grandchild holding the write end keeps the pipe open and the
deadline never applies.

A version command that daemonizes — a service wrapper, an agent, a docker
shim — therefore hung the check indefinitely. Measured with a script that
backgrounds a sleep before exiting:

    preflight cmd ./slowchild --timeout 2s     31s elapsed

In a container entrypoint that is a permanent stall rather than a failed
check, which is the worse outcome: a failure is reported, a hang is not.

Setting cmd.WaitDelay bounds Wait after cancellation. Same command now
returns in 1s, and commands that exit normally are unaffected.
---
 pkg/cmdcheck/runner.go           |  8 +++++
 pkg/cmdcheck/runner_unix_test.go | 50 ++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)
 create mode 100644 pkg/cmdcheck/runner_unix_test.go

diff --git a/pkg/cmdcheck/runner.go b/pkg/cmdcheck/runner.go
index 34d4183..58cc534 100644
--- a/pkg/cmdcheck/runner.go
+++ b/pkg/cmdcheck/runner.go
@@ -23,11 +23,19 @@ func (r *RealCmdRunner) LookPath(file string) (string, error) {
 	return exec.LookPath(file)
 }
 
+// waitDelay bounds how long Wait blocks after the context is cancelled. Because
+// Stdout and Stderr are buffers rather than files, os/exec pipes them and waits
+// for EOF, but CommandContext kills only the direct child — a grandchild holding
+// the write end keeps the pipe open, so without this the timeout is
+// unenforceable and a version command that daemonizes hangs the check forever.
+const waitDelay = time.Second
+
 func (r *RealCmdRunner) RunCommandContext(ctx context.Context, name string, args ...string) (stdout, stderr string, err error) {
 	cmd := exec.CommandContext(ctx, name, args...) //nolint:gosec // intentional: executing user-specified version check commands
 	var outBuf, errBuf bytes.Buffer
 	cmd.Stdout = &outBuf
 	cmd.Stderr = &errBuf
+	cmd.WaitDelay = waitDelay
 	err = cmd.Run()
 	return outBuf.String(), errBuf.String(), err
 }
diff --git a/pkg/cmdcheck/runner_unix_test.go b/pkg/cmdcheck/runner_unix_test.go
new file mode 100644
index 0000000..e5a4a13
--- /dev/null
+++ b/pkg/cmdcheck/runner_unix_test.go
@@ -0,0 +1,50 @@
+//go:build unix
+
+package cmdcheck
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+)
+
+// A command that exits while a background child still holds the stdout pipe.
+// os/exec waits for the pipe to reach EOF, and CommandContext kills only the
+// direct child, so without WaitDelay the context deadline is unenforceable and
+// RunCommandContext blocks for as long as the grandchild lives.
+func TestRunCommandContext_ReturnsWhenGrandchildHoldsThePipe(t *testing.T) {
+	script := filepath.Join(t.TempDir(), "daemonize.sh")
+	require.NoError(t, os.WriteFile(script, []byte("#!/bin/sh\nsleep 60 &\necho v1.0.0\n"), 0o700)) //nolint:gosec // the script must be executable to run
+
+	ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond)
+	defer cancel()
+
+	done := make(chan struct{})
+	go func() {
+		defer close(done)
+		r := &RealCmdRunner{}
+		_, _, _ = r.RunCommandContext(ctx, script)
+	}()
+
+	select {
+	case <-done:
+	case <-time.After(15 * time.Second):
+		t.Fatal("RunCommandContext did not return; the 200ms timeout was not enforced")
+	}
+}
+
+// The ordinary path must be unaffected: a command that finishes normally still
+// returns its output rather than being cut off by WaitDelay.
+func TestRunCommandContext_NormalCommandStillReturnsOutput(t *testing.T) {
+	script := filepath.Join(t.TempDir(), "version.sh")
+	require.NoError(t, os.WriteFile(script, []byte("#!/bin/sh\necho v1.2.3\n"), 0o700)) //nolint:gosec // the script must be executable to run
+
+	r := &RealCmdRunner{}
+	stdout, _, err := r.RunCommandContext(context.Background(), script)
+	require.NoError(t, err)
+	require.Contains(t, stdout, "v1.2.3")
+}