Fix stale heartbeat process PIDs cleanup issue (#902)

* dev/tools/resourcectl: fix persistent PID landmine and prevent arbitrary process termination

#### What this PR does / why we need it:
Fixes a logic vulnerability in `resourcectl cleanup` where stale heartbeat process PIDs remained permanently trapped in the local state file (`~/.local/resourcectl/state.json`) if the heartbeat process exited prematurely. When the OS eventually recycled that PID and assigned it to a new innocent process, the subsequent execution of `resourcectl cleanup` would violently terminate that unrelated process via `SIGKILL`.

To fix this, we:
- Safely verify the heartbeat process identity on Linux by reading and parsing `/proc/<pid>/cmdline` to ensure it contains the `"heartbeat"` and `"--name" <resource>` arguments before attempting to kill it.
- Guarantee that stale PIDs are zeroed in `state.json` even if the kill operation fails.
- Prune fully released resources completely from the state file rather than leaving empty structs, preventing unbounded file growth.
- Ignore `ESRCH` errors if a heartbeat process is already dead.
- Added comprehensive unit tests in `main_test.go` to reproduce the conditions and verify safety and cleanup behavior.

#### Which issue(s) this PR is related to:
Fixes b/511322601

#### Release Note
```release-note
Fixed a logic issue in `resourcectl` where stale heartbeat PIDs could lead to arbitrary process termination due to PID recycling.
```

* address feedback

* pr feedback

Author: aditya-shantanu

dev/tools/resourcectl/main.go

@@ -27,6 +27,7 @@ import (
 	"os/exec"
 	"os/signal"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"syscall"
 	"time"
@@ -300,6 +301,8 @@ func runCleanup(ctx context.Context) error {
 		return err
 	}
 	var errs []error
+	var remainingResources []BoskosResource
+
 	for i := range state.BoskosResources {
 		r := &state.BoskosResources[i]
 		if err := killHeartbeatProcess(ctx, r); err != nil {
@@ -310,12 +313,12 @@ func runCleanup(ctx context.Context) error {
 
 		if err := r.ReleaseFromBoskos(ctx); err != nil {
 			errs = append(errs, err)
-		} else {
-			r.Name = ""
-			r.Type = ""
+			remainingResources = append(remainingResources, *r)
 		}
 	}
 
+	state.BoskosResources = remainingResources
+
 	if err := writeState(state); err != nil {
 		return err
 	}
@@ -329,20 +332,61 @@ func killHeartbeatProcess(ctx context.Context, r *BoskosResource) error {
 		return nil
 	}
 
+	log := klog.FromContext(ctx)
+
+	// Verify the process is actually the heartbeat process if we are on Linux.
+	// On non-Linux platforms (e.g., macOS), we currently skip this verification
+	// because /proc is not available. This leaves a small window for PID-recycling
+	// issues on those platforms.
+	if runtime.GOOS == "linux" {
+		if !isHeartbeatProcess(r.HeartbeatPID, r.Name) {
+			return nil
+		}
+	} else {
+		log.V(4).Info("skipping heartbeat process verification on non-linux platform", "goos", runtime.GOOS, "pid", r.HeartbeatPID)
+	}
+
 	process, err := os.FindProcess(r.HeartbeatPID)
 	if err != nil {
 		return fmt.Errorf("error finding heartbeat process: %w", err)
 	}
 
-	// TODO: Verify the process is actually the heartbeat process
-
 	if err := process.Kill(); err != nil {
+		if errors.Is(err, syscall.ESRCH) {
+			return nil
+		}
 		return fmt.Errorf("error killing heartbeat process: %w", err)
 	}
 
 	return nil
 }
 
+// isHeartbeatProcess verifies if the process with the given PID is actually our heartbeat process.
+func isHeartbeatProcess(pid int, resourceName string) bool {
+	cmdlinePath := fmt.Sprintf("/proc/%d/cmdline", pid)
+	data, err := os.ReadFile(cmdlinePath)
+	if err != nil {
+		return false
+	}
+
+	// The /proc/<pid>/cmdline file contains arguments separated by null bytes (\x00).
+	args := strings.Split(string(data), "\x00")
+
+	// Check if the arguments contain "heartbeat" and "--name" <resourceName>
+	hasHeartbeat := false
+	hasName := false
+	for i := 0; i < len(args); i++ {
+		if args[i] == "heartbeat" {
+			hasHeartbeat = true
+		}
+		if args[i] == "--name" && i+1 < len(args) && args[i+1] == resourceName {
+			hasName = true
+		}
+	}
+
+	return hasHeartbeat && hasName
+}
+
 // runHeartbeat sends periodic heartbeats to Boskos to keep the resource alive.
 // This is run as a child process of runGet.
 func runHeartbeat(ctx context.Context, name, owner string) error {

dev/tools/resourcectl/main_test.go

@@ -0,0 +1,227 @@
+// Copyright 2026 The Kubernetes Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"os/exec"
+	"runtime"
+	"syscall"
+	"testing"
+	"time"
+)
+
+func TestHelperProcess(t *testing.T) {
+	if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" {
+		return
+	}
+	// Just sleep forever (or until killed)
+	select {}
+}
+
+func TestIsHeartbeatProcess(t *testing.T) {
+	if runtime.GOOS != "linux" {
+		t.Skip("skipping /proc based test on non-linux platform")
+	}
+
+	cmd := exec.Command(os.Args[0], "-test.run=TestHelperProcess", "--", "heartbeat", "--name", "test-resource")
+	cmd.Env = append(os.Environ(), "GO_WANT_HELPER_PROCESS=1")
+	if err := cmd.Start(); err != nil {
+		t.Fatalf("failed to start helper process: %v", err)
+	}
+	defer func() {
+		_ = cmd.Process.Kill()
+		_ = cmd.Wait()
+	}()
+	// Let the OS populate /proc/<pid>/cmdline
+	time.Sleep(50 * time.Millisecond)
+
+	// Verify isHeartbeatProcess returns true for our helper process
+	if !isHeartbeatProcess(cmd.Process.Pid, "test-resource") {
+		t.Errorf("expected isHeartbeatProcess to return true, but got false")
+	}
+
+	// Verify isHeartbeatProcess returns false for a different resource name
+	if isHeartbeatProcess(cmd.Process.Pid, "other-resource") {
+		t.Errorf("expected isHeartbeatProcess to return false for different resource name, but got true")
+	}
+
+	// Spawn a standard sleep command which does not have "heartbeat" arguments
+	dummy := exec.Command("sleep", "10")
+	if err := dummy.Start(); err != nil {
+		t.Fatalf("failed to start dummy process: %v", err)
+	}
+	defer func() {
+		_ = dummy.Process.Kill()
+		_ = dummy.Wait()
+	}()
+
+	if isHeartbeatProcess(dummy.Process.Pid, "test-resource") {
+		t.Errorf("expected isHeartbeatProcess to return false for generic process, but got true")
+	}
+}
+
+func TestRunCleanup(t *testing.T) {
+	// Create temporary home directory to sandbox state file
+	tmpHome, err := os.MkdirTemp("", "resourcectl-test-home-*")
+	if err != nil {
+		t.Fatalf("failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpHome)
+
+	// Override HOME
+	origHome := os.Getenv("HOME")
+	os.Setenv("HOME", tmpHome)
+	defer os.Setenv("HOME", origHome)
+
+	// Start a mock Boskos server
+	mockBoskos := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Just return 200 OK
+		w.WriteHeader(http.StatusOK)
+		fmt.Fprint(w, `{"status": "ok"}`)
+	}))
+	defer mockBoskos.Close()
+
+	// Override BOSKOS_HOST
+	origBoskosHost := os.Getenv("BOSKOS_HOST")
+	os.Setenv("BOSKOS_HOST", mockBoskos.URL)
+	defer os.Setenv("BOSKOS_HOST", origBoskosHost)
+
+	// 1. Spawn a matching heartbeat process that we want to kill.
+	cmd := exec.Command(os.Args[0], "-test.run=TestHelperProcess", "--", "heartbeat", "--name", "matching-resource")
+	cmd.Env = append(os.Environ(), "GO_WANT_HELPER_PROCESS=1")
+	if err := cmd.Start(); err != nil {
+		t.Fatalf("failed to start matching helper process: %v", err)
+	}
+	var cmdWaited bool
+	defer func() {
+		_ = cmd.Process.Kill()
+		if !cmdWaited {
+			_ = cmd.Wait()
+		}
+	}()
+
+	// 2. Spawn a recycled/innocent process that should NOT be killed.
+	// We'll use a simple sleep command as the innocent process.
+	innocentCmd := exec.Command("sleep", "30")
+	if err := innocentCmd.Start(); err != nil {
+		t.Fatalf("failed to start innocent process: %v", err)
+	}
+	defer func() {
+		_ = innocentCmd.Process.Kill()
+		_ = innocentCmd.Wait()
+	}()
+
+	// Let the OS populate /proc/<pid>/cmdline for both processes
+	time.Sleep(50 * time.Millisecond)
+
+	// 3. Generate a dead PID (start and exit)
+	deadCmd := exec.Command("true")
+	if err := deadCmd.Run(); err != nil {
+		t.Fatalf("failed to run dead process: %v", err)
+	}
+	deadPid := deadCmd.Process.Pid
+
+	// Write initial state.json
+	stateFile, err := stateFilePath()
+	if err != nil {
+		t.Fatalf("failed to get state file path: %v", err)
+	}
+
+	initialState := State{
+		BoskosResources: []BoskosResource{
+			{
+				Name:         "matching-resource",
+				Type:         "gce-project",
+				HeartbeatPID: cmd.Process.Pid,
+			},
+			{
+				Name:         "innocent-resource",
+				Type:         "gce-project",
+				HeartbeatPID: innocentCmd.Process.Pid,
+			},
+			{
+				Name:         "dead-resource",
+				Type:         "gce-project",
+				HeartbeatPID: deadPid,
+			},
+		},
+	}
+
+	data, err := json.MarshalIndent(initialState, "", "  ")
+	if err != nil {
+		t.Fatalf("failed to marshal initial state: %v", err)
+	}
+	if err := os.WriteFile(stateFile, data, 0600); err != nil {
+		t.Fatalf("failed to write initial state file: %v", err)
+	}
+
+	// Run cleanup!
+	ctx := context.Background()
+	err = runCleanup(ctx)
+	if err != nil {
+		t.Fatalf("runCleanup failed: %v", err)
+	}
+
+	// 4. Verify results:
+	// - The matching-resource heartbeat process should be killed.
+	// - The innocent-resource process should STILL be running (not killed).
+	// - The state file should be completely empty of resources (since all released successfully).
+
+	// Verify matching heartbeat is killed.
+	// Since the child process was killed by SIGKILL, wait should return an error indicating it was signaled.
+	err = cmd.Wait()
+	cmdWaited = true
+	if err == nil {
+		t.Errorf("expected matching heartbeat process to be killed, but it exited cleanly")
+	} else {
+		var exitErr *exec.ExitError
+		if errors.As(err, &exitErr) {
+			status := exitErr.Sys().(syscall.WaitStatus)
+			if !status.Signaled() || status.Signal() != syscall.SIGKILL {
+				t.Errorf("expected process to be killed by SIGKILL, but got exit status: %v", err)
+			}
+		} else {
+			t.Errorf("unexpected error waiting for process: %v", err)
+		}
+	}
+
+	// Verify innocent process is NOT killed (on Linux only, where verification is active).
+	if runtime.GOOS == "linux" {
+		process, err := os.FindProcess(innocentCmd.Process.Pid)
+		if err == nil {
+			err = process.Signal(syscall.Signal(0))
+			if err != nil {
+				t.Errorf("expected innocent process to still be running, but got error: %v", err)
+			}
+		}
+	}
+
+	// Verify state file is empty.
+	state, err := readState()
+	if err != nil {
+		t.Fatalf("failed to read state file: %v", err)
+	}
+
+	if len(state.BoskosResources) != 0 {
+		t.Errorf("expected 0 resources left in state.json, but got %d: %+v", len(state.BoskosResources), state.BoskosResources)
+	}
+}