Merge branch 'main' into issue-3876

Author: amirejaz

.github/workflows/security-scan.yml

@@ -26,6 +26,7 @@ jobs:
       - name: Run Trivy vulnerability scanner in repo mode
         uses: aquasecurity/trivy-action@e368e328979b113139d6f9068e03accaed98a518 # 0.34.1
         with:
+          version: 'v0.69.3'
           scan-type: 'fs'
           scan-ref: '.'
           format: 'sarif'
@@ -47,6 +48,7 @@ jobs:
       - name: Run Trivy configuration scanner
         uses: aquasecurity/trivy-action@e368e328979b113139d6f9068e03accaed98a518 # 0.34.1
         with:
+          version: 'v0.69.3'
           scan-type: 'config'
           scan-ref: './deploy/charts'
           format: 'sarif'

go.mod

@@ -41,6 +41,7 @@ require (
 	github.com/pressly/goose/v3 v3.27.0
 	github.com/prometheus/client_golang v1.23.2
 	github.com/redis/go-redis/v9 v9.18.0
+	github.com/shirou/gopsutil/v4 v4.25.6
 	github.com/spf13/viper v1.21.0
 	github.com/stacklok/toolhive-catalog v0.20260223.0
 	github.com/stacklok/toolhive-core v0.0.8
@@ -237,7 +238,6 @@ require (
 	github.com/sergi/go-diff v1.4.0 // indirect
 	github.com/sethvargo/go-retry v0.3.0 // indirect
 	github.com/shibumi/go-pathspec v1.3.0 // indirect
-	github.com/shirou/gopsutil/v4 v4.25.6 // indirect
 	github.com/sigstore/protobuf-specs v0.5.0 // indirect
 	github.com/sigstore/rekor v1.5.0 // indirect
 	github.com/sigstore/rekor-tiles/v2 v2.0.1 // indirect

pkg/networking/port.go

@@ -11,6 +11,8 @@ import (
 	"log/slog"
 	"math/big"
 	"net"
+
+	gopsutilnet "github.com/shirou/gopsutil/v4/net"
 )
 
 const (
@@ -177,3 +179,25 @@ func ValidateCallbackPort(callbackPort int, clientID string) error {
 func IsPreRegisteredClient(clientID string) bool {
 	return clientID != ""
 }
+
+// GetProcessOnPort returns the PID of the process listening on the given TCP port.
+// Returns 0 if the port is free or if the holder cannot be determined.
+// Uses gopsutil which provides cross-platform support (Linux: /proc, Windows: GetExtendedTcpTable,
+// Darwin/FreeBSD: lsof).
+func GetProcessOnPort(port int) (int, error) {
+	if port <= 0 || port > MaxPort {
+		return 0, fmt.Errorf("invalid port %d", port)
+	}
+
+	conns, err := gopsutilnet.Connections("tcp")
+	if err != nil {
+		return 0, fmt.Errorf("failed to get TCP connections: %w", err)
+	}
+
+	for _, c := range conns {
+		if c.Laddr.Port == uint32(port) && c.Status == "LISTEN" && c.Pid > 0 { //nolint:gosec // G115 - port validated in [1, 65535]
+			return int(c.Pid), nil
+		}
+	}
+	return 0, nil
+}

pkg/networking/port_test.go

@@ -4,8 +4,12 @@
 package networking_test
 
 import (
+	"net"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/stacklok/toolhive/pkg/networking"
 )
 
@@ -80,3 +84,53 @@ func TestValidateCallbackPort(t *testing.T) {
 		})
 	}
 }
+
+func TestGetProcessOnPort_InvalidPort(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		port int
+	}{
+		{"zero port", 0},
+		{"negative port", -1},
+		{"port too large", 65536},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			pid, err := networking.GetProcessOnPort(tt.port)
+			require.Error(t, err)
+			assert.Equal(t, 0, pid)
+		})
+	}
+}
+
+func TestGetProcessOnPort_FreePort(t *testing.T) {
+	t.Parallel()
+
+	// Use a port that FindAvailable guarantees is free
+	port := networking.FindAvailable()
+	require.NotZero(t, port, "FindAvailable should find a free port")
+
+	pid, err := networking.GetProcessOnPort(port)
+	require.NoError(t, err)
+	assert.Equal(t, 0, pid)
+}
+
+func TestGetProcessOnPort_PortInUse(t *testing.T) {
+	t.Parallel()
+
+	// Bind to a port, then verify GetProcessOnPort returns our process
+	listener, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	defer listener.Close()
+
+	tcpAddr, ok := listener.Addr().(*net.TCPAddr)
+	require.True(t, ok)
+	port := tcpAddr.Port
+
+	pid, err := networking.GetProcessOnPort(port)
+	require.NoError(t, err)
+	assert.NotZero(t, pid, "port is in use, GetProcessOnPort should return the process PID")
+}

pkg/process/toolhive_proxy.go

@@ -0,0 +1,116 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package process
+
+import (
+	"strings"
+
+	gopsutilprocess "github.com/shirou/gopsutil/v4/process"
+)
+
+const (
+	// toolHiveBinaryName is the binary name used to identify ToolHive processes.
+	// Used when TOOLHIVE_DETACHED cannot be read (e.g. platform restrictions).
+	toolHiveBinaryName = "thv"
+)
+
+// containsThvBinary returns true if s appears to reference the thv binary
+// (e.g. /usr/bin/thv, thv start, thv.exe), avoiding false positives like "toolhive".
+func containsThvBinary(s string) bool {
+	lower := strings.ToLower(s)
+	return strings.Contains(lower, toolHiveBinaryName+".exe") ||
+		strings.HasSuffix(lower, toolHiveBinaryName) ||
+		strings.Contains(lower, "/"+toolHiveBinaryName) ||
+		strings.Contains(lower, `\`+toolHiveBinaryName) ||
+		strings.HasPrefix(lower, toolHiveBinaryName+" ") ||
+		strings.Contains(lower, " "+toolHiveBinaryName+" ") ||
+		strings.HasSuffix(lower, " "+toolHiveBinaryName)
+}
+
+// IsToolHiveProxyForWorkload returns true if the given PID belongs to the ToolHive
+// proxy for the specified workload, so it is safe to kill when freeing a port.
+// Returns false if the process is not that workload's proxy or if identity cannot
+// be verified (fail-safe: do not kill).
+//
+// When workloadName is empty, only verifies it is a ToolHive process.
+// When workloadName is non-empty, also verifies the process cmdline contains
+// " start <workloadName> " (the detached proxy runs "thv start <name> --foreground").
+//
+// Verification checks, in order:
+//  1. TOOLHIVE_DETACHED=1 in process environment (most reliable)
+//  2. "thv" in executable path or command line (fallback when env unavailable)
+//  3. workloadName in cmdline (when provided, avoids killing another workload's proxy)
+func IsToolHiveProxyForWorkload(pid int, workloadName string) (bool, error) {
+	if pid <= 0 {
+		return false, nil
+	}
+
+	// PID fits in int32 on all supported platforms
+	p, err := gopsutilprocess.NewProcess(int32(pid)) //nolint:gosec // G115
+	if err != nil {
+		return false, err
+	}
+
+	if !isToolHiveProcess(p) {
+		return false, nil
+	}
+
+	if workloadName != "" {
+		cmdline, err := p.Cmdline()
+		if err != nil || !cmdlineContainsWorkload(cmdline, workloadName) {
+			return false, nil
+		}
+	}
+
+	return true, nil
+}
+
+// isToolHiveProcess returns true if p is a ToolHive process (TOOLHIVE_DETACHED
+// or thv binary in exe/cmdline).
+func isToolHiveProcess(p *gopsutilprocess.Process) bool {
+	if hasToolHiveDetachedEnv(p) {
+		return true
+	}
+	if exe, err := p.Exe(); err == nil && containsThvBinary(exe) {
+		return true
+	}
+	if cmdline, err := p.Cmdline(); err == nil && containsThvBinary(cmdline) {
+		return true
+	}
+	return false
+}
+
+func hasToolHiveDetachedEnv(p *gopsutilprocess.Process) bool {
+	env, err := p.Environ()
+	if err != nil {
+		return false
+	}
+	target := ToolHiveDetachedEnv + "=" + ToolHiveDetachedValue
+	for _, e := range env {
+		if e == target {
+			return true
+		}
+	}
+	return false
+}
+
+// cmdlineContainsWorkload returns true if the cmdline indicates a "thv start <name> ..." process.
+// Uses word boundaries to avoid partial matches (e.g. "g" matching "github").
+func cmdlineContainsWorkload(cmdline, workloadName string) bool {
+	if workloadName == "" {
+		return false
+	}
+	pattern := " start " + workloadName
+	if !strings.Contains(cmdline, pattern) {
+		return false
+	}
+	// Ensure workloadName is not a prefix of a longer word: next char must be space, -, or end
+	idx := strings.Index(cmdline, pattern)
+	end := idx + len(pattern)
+	if end >= len(cmdline) {
+		return true
+	}
+	next := cmdline[end]
+	return next == ' ' || next == '-'
+}

pkg/process/toolhive_proxy_test.go

@@ -0,0 +1,88 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package process
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestIsToolHiveProxyForWorkload_InvalidPID(t *testing.T) {
+	t.Parallel()
+	isToolHive, err := IsToolHiveProxyForWorkload(0, "")
+	require.NoError(t, err)
+	assert.False(t, isToolHive)
+
+	isToolHive, err = IsToolHiveProxyForWorkload(-1, "")
+	require.NoError(t, err)
+	assert.False(t, isToolHive)
+}
+
+func TestIsToolHiveProxyForWorkload_NonToolHiveProcess(t *testing.T) {
+	t.Parallel()
+	// Use a very high PID that almost certainly doesn't exist.
+	// IsToolHiveProxyForWorkload should return false (fail-safe: don't kill unknown processes).
+	isToolHive, err := IsToolHiveProxyForWorkload(999999999, "")
+	if err != nil {
+		// Process may not exist; either way we must not report it as ToolHive
+		assert.False(t, isToolHive)
+		return
+	}
+	require.NoError(t, err)
+	assert.False(t, isToolHive)
+}
+
+func TestContainsThvBinary(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name     string
+		input    string
+		expected bool
+	}{
+		{"thv exe", "thv.exe", true},
+		{"path with thv", "/usr/local/bin/thv", true},
+		{"path with thv windows", `C:\tools\thv.exe`, true},
+		{"cmd thv start", "thv start myworkload --foreground", true},
+		{"cmd with thv in middle", "/path/to/thv start x", true},
+		{"toolhive not thv", "toolhive", false},
+		{"toolhive.test", "/tmp/toolhive.test", false},
+		{"unrelated", "node server.js", false},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got := containsThvBinary(tt.input)
+			assert.Equal(t, tt.expected, got, "containsThvBinary(%q)", tt.input)
+		})
+	}
+}
+
+func TestCmdlineContainsWorkload(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name     string
+		cmdline  string
+		workload string
+		expected bool
+	}{
+		{"matches github", "/usr/bin/thv start github --foreground", "github", true},
+		{"matches with debug", "thv start slack --foreground --debug", "slack", true},
+		{"matches at end", "thv start myworkload", "myworkload", true},
+		{"rejects different workload", "/usr/bin/thv start slack --foreground", "github", false},
+		{"rejects partial match", "thv start github --foreground", "git", false},
+		{"rejects suffix match", "thv start github --foreground", "hub", false},
+		{"empty workload", "thv start github --foreground", "", false},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got := cmdlineContainsWorkload(tt.cmdline, tt.workload)
+			assert.Equal(t, tt.expected, got, "cmdlineContainsWorkload(%q, %q)", tt.cmdline, tt.workload)
+		})
+	}
+}

pkg/process/wait.go

@@ -0,0 +1,35 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package process
+
+import (
+	"context"
+	"time"
+)
+
+// WaitForExit waits for the process with the given PID to exit.
+// It polls FindProcess every 50ms until the process is no longer running
+// or the context is cancelled. Callers should use context.WithTimeout to
+// impose a deadline.
+// Returns nil when the process has exited, or an error on context cancellation.
+func WaitForExit(ctx context.Context, pid int) error {
+	ticker := time.NewTicker(50 * time.Millisecond)
+	defer ticker.Stop()
+
+	for {
+		alive, err := FindProcess(pid)
+		if err != nil {
+			return err
+		}
+		if !alive {
+			return nil
+		}
+
+		select {
+		case <-ctx.Done():
+			return ctx.Err()
+		case <-ticker.C:
+		}
+	}
+}