Merge pull request #34 from swarit-stepsecurity/swarit/fix/33

fix: add nil check before attempting to kill

Author: migue

pipe/command_nil_panic_test.go

@@ -0,0 +1,47 @@
+//go:build !windows
+// +build !windows
+
+package pipe
+
+import (
+	"context"
+	"os/exec"
+	"testing"
+)
+
+func TestKillWithNilProcess(t *testing.T) {
+	cmd := exec.Command("sleep", "100")
+	stage := &commandStage{
+		name: "test-command",
+		cmd:  cmd,
+		done: make(chan struct{}),
+	}
+
+	defer func() {
+		if r := recover(); r != nil {
+			t.Fatalf("PANIC OCCURRED (bug not fixed): %v", r)
+		}
+	}()
+
+	stage.Kill(context.Canceled)
+
+	t.Log("SUCCESS: Kill() handled nil Process gracefully without panicking")
+}
+
+func TestKillWithFailedStart(t *testing.T) {
+	ctx := context.Background()
+
+	stage := Command("/this/path/does/not/exist/invalid_command_12345")
+
+	_, err := stage.Start(ctx, Env{}, nil)
+	if err == nil {
+		t.Fatal("Expected start to fail, but it succeeded")
+	}
+
+	// At this point, if someone calls Kill (perhaps from a memory monitor
+	// or other external component), it could panic if Process is nil
+
+	// Note: In the current implementation, Kill won't be called from the
+	// context goroutine because Start failed. But external callers like
+	// MemoryLimit could potentially call Kill on a failed stage.
+}

pipe/command_unix.go

@@ -30,6 +30,11 @@ func (s *commandStage) Kill(err error) {
 	// use internal synchronization. But those methods only kill the
 	// process, not the process group, so they are not suitable here.
 
+	// Check if the process was started successfully before attempting to kill
+	if s.cmd.Process == nil {
+		return
+	}
+
 	// We started the process with PGID == PID:
 	pid := s.cmd.Process.Pid
 	select {

pipe/command_windows.go

@@ -9,6 +9,11 @@ func (s *commandStage) runInOwnProcessGroup() {}
 // kill is called to kill the process if the context expires. `err` is
 // the corresponding value of `Context.Err()`.
 func (s *commandStage) Kill(err error) {
+	// Check if the process was started successfully before attempting to kill
+	if s.cmd.Process == nil {
+		return
+	}
+
 	select {
 	case <-s.done:
 		// Process has ended; no need to kill it again.