diff --git a/internal/sandbox-sidecar/command/command.go b/internal/sandbox-sidecar/command/command.go index 01067cad..e52487d8 100644 --- a/internal/sandbox-sidecar/command/command.go +++ b/internal/sandbox-sidecar/command/command.go @@ -95,9 +95,24 @@ func (b *ChrootCommandBuilder) Build(ctx context.Context, pid int, req sidecarap Chroot: fmt.Sprintf("/proc/%d/root", pid), Credential: &syscall.Credential{Uid: 0, Gid: 0}, } + setupProcessGroup(cmd) return cmd, nil } +func setupProcessGroup(cmd *exec.Cmd) { + if cmd.SysProcAttr == nil { + cmd.SysProcAttr = &syscall.SysProcAttr{} + } + cmd.SysProcAttr.Setpgid = true + cmd.Cancel = func() error { + err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) + if errors.Is(err, syscall.ESRCH) { + return os.ErrProcessDone + } + return err + } +} + // --- Input/Output types --- type CreateCommandInput struct { diff --git a/internal/sandbox-sidecar/command/command_test.go b/internal/sandbox-sidecar/command/command_test.go index 70e17aca..872b3477 100644 --- a/internal/sandbox-sidecar/command/command_test.go +++ b/internal/sandbox-sidecar/command/command_test.go @@ -24,7 +24,9 @@ import ( "os" "path/filepath" "sort" + "strconv" "strings" + "syscall" "time" "github.com/danielgtaylor/huma/v2" @@ -59,6 +61,10 @@ func extractSSEData(body string) string { return result.String() } +func processAlive(pid int) bool { + return syscall.Kill(pid, 0) == nil +} + var _ = Describe("Command Handlers", func() { var ( commandAPI humatest.TestAPI @@ -913,6 +919,78 @@ var _ = Describe("Command Handlers", func() { }) }) + Describe("process group termination", func() { + startWithChild := func(extra map[string]any) (string, int) { + f, err := os.CreateTemp("", "sidecar-child-pid-*") + Expect(err).NotTo(HaveOccurred()) + pidFile := f.Name() + Expect(f.Close()).To(Succeed()) + DeferCleanup(func() { _ = os.Remove(pidFile) }) + + req := map[string]any{ + "args": []string{"/bin/sh", "-c", `sleep 300 & echo $! > "$CHILD_PID_FILE"; wait`}, + "env": map[string]string{"CHILD_PID_FILE": pidFile}, + } + for k, v := range extra { + req[k] = v + } + body, err := json.Marshal(req) + Expect(err).NotTo(HaveOccurred()) + + resp := commandAPI.Post("/v1/commands", "Content-Type: application/json", strings.NewReader(string(body))) + Expect(resp.Code).To(Equal(http.StatusAccepted)) + var result sidecarapi.CreateCommandResponse + Expect(json.NewDecoder(resp.Body).Decode(&result)).To(Succeed()) + + var childPID int + Eventually(func() bool { + data, readErr := os.ReadFile(pidFile) //nolint:gosec // test-created temp file path + if readErr != nil { + return false + } + pid, convErr := strconv.Atoi(strings.TrimSpace(string(data))) + if convErr != nil { + return false + } + childPID = pid + return true + }, "3s", "20ms").Should(BeTrue()) + + DeferCleanup(func() { _ = syscall.Kill(childPID, syscall.SIGKILL) }) + return result.ID, childPID + } + + waitExited := func(id string) { + Eventually(func() *int { + resp := commandAPI.Get(fmt.Sprintf("/v1/commands/%s/status", id)) + var status sidecarapi.CommandStatusResponse + Expect(json.NewDecoder(resp.Body).Decode(&status)).To(Succeed()) + return status.ExitCode + }, "5s").ShouldNot(BeNil()) + } + + It("kills the whole process tree on DELETE", func() { + id, childPID := startWithChild(nil) + Expect(processAlive(childPID)).To(BeTrue(), "child should be running before kill") + + resp := commandAPI.Delete(fmt.Sprintf("/v1/commands/%s", id)) + Expect(resp.Code).To(Equal(http.StatusNoContent)) + + waitExited(id) + Eventually(func() bool { return processAlive(childPID) }, "3s", "20ms"). + Should(BeFalse(), "forked child must not survive after the command is killed") + }) + + It("kills the whole process tree on timeout", func() { + id, childPID := startWithChild(map[string]any{"timeoutSeconds": 1}) + Expect(processAlive(childPID)).To(BeTrue(), "child should be running before timeout") + + waitExited(id) + Eventually(func() bool { return processAlive(childPID) }, "3s", "20ms"). + Should(BeFalse(), "forked child must not survive after the command times out") + }) + }) + Describe("working directory", func() { It("runs command in specified cwd", func() { code, result := postCommand(`{"args": ["/bin/sh", "-c", "pwd"], "cwd": "/tmp"}`) diff --git a/internal/sandbox-sidecar/command/suite_test.go b/internal/sandbox-sidecar/command/suite_test.go index 4ed61075..a6627ad6 100644 --- a/internal/sandbox-sidecar/command/suite_test.go +++ b/internal/sandbox-sidecar/command/suite_test.go @@ -45,6 +45,7 @@ func (b *DirectCommandBuilder) Build(ctx context.Context, _ int, req sidecarapi. cmd.Env = env cmd.Dir = req.Cwd cmd.WaitDelay = waitDelayGracePeriod + setupProcessGroup(cmd) return cmd, nil }