diff --git a/server/cmd/api/api/process.go b/server/cmd/api/api/process.go index 38a458f4..124878ef 100644 --- a/server/cmd/api/api/process.go +++ b/server/cmd/api/api/process.go @@ -179,6 +179,14 @@ func (s *ApiService) ProcessExec(ctx context.Context, request oapi.ProcessExecRe var stdoutBuf, stderrBuf bytes.Buffer cmd.Stdout = &stdoutBuf cmd.Stderr = &stderrBuf + // Run the command in its own process group so a timeout stops every + // process still in that group. Its children inherit stdout and stderr, and + // cmd.Wait waits for them to close, so killing only the direct child does + // not end the call. + if cmd.SysProcAttr == nil { + cmd.SysProcAttr = &syscall.SysProcAttr{} + } + cmd.SysProcAttr.Setpgid = true // Handle timeout if provided start := time.Now() @@ -199,7 +207,7 @@ func (s *ApiService) ProcessExec(ctx context.Context, request oapi.ProcessExecRe go func() { done <- cmd.Wait() }() select { case <-ctx.Done(): - _ = cmd.Process.Kill() + _ = signalChildProcessGroup(cmd, killSignal) <-done // ensure wait returns return oapi.ProcessExec500JSONResponse{InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: "process timed out"}}, nil case err := <-done: diff --git a/server/cmd/api/api/process_test.go b/server/cmd/api/api/process_test.go index 741734d7..0ce18b37 100644 --- a/server/cmd/api/api/process_test.go +++ b/server/cmd/api/api/process_test.go @@ -6,7 +6,10 @@ import ( "encoding/base64" "encoding/json" "io" + "os" "os/user" + "path/filepath" + "runtime" "strconv" "strings" "testing" @@ -41,6 +44,37 @@ func TestProcessExec(t *testing.T) { require.Equal(t, "err", string(errB), "stderr mismatch") } +func TestProcessExec_TimeoutKillsDescendants(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("process-group lifecycle is only enforced on Linux") + } + t.Parallel() + ctx := context.Background() + svc := &ApiService{procs: make(map[string]*processHandle)} + + // The shell waits on a background child that inherited its stdout and + // stderr, so the call can only end once that child is gone too. + pidFile := filepath.Join(t.TempDir(), "pid") + args := []string{"-c", `sleep 30 & echo $! > "$1"; wait`, "sh", pidFile} + timeout := 1 + body := &oapi.ProcessExecRequest{Command: "sh", Args: &args, TimeoutSec: &timeout} + start := time.Now() + resp, err := svc.ProcessExec(ctx, oapi.ProcessExecRequestObject{Body: body}) + elapsed := time.Since(start) + require.NoError(t, err, "ProcessExec error") + r500, ok := resp.(oapi.ProcessExec500JSONResponse) + require.True(t, ok, "unexpected resp type: %T", resp) + require.Equal(t, "process timed out", r500.Message) + require.Less(t, elapsed, 10*time.Second, "exec with timeout_sec=1 returned after %v", elapsed) + + raw, err := os.ReadFile(pidFile) + require.NoError(t, err, "read background pid") + pid, err := strconv.Atoi(strings.TrimSpace(string(raw))) + require.NoError(t, err, "parse background pid %q", raw) + require.Eventually(t, func() bool { return !processAlive(pid) }, 3*time.Second, 20*time.Millisecond, + "timeout must kill the command's background child") +} + func TestProcessSpawnStatusAndStream(t *testing.T) { t.Parallel() ctx := context.Background()