fix(process): reject a pid that would signal a process group

internal/controller/api/node/process/process_get.go

@@ -41,6 +41,10 @@ func (s *Process) GetNodeProcessByPid(
 		return gen.GetNodeProcessByPid400JSONResponse{Error: &errMsg}, nil
 	}
 
+	if errMsg, ok := validatePid(request.Pid); !ok {
+		return gen.GetNodeProcessByPid400JSONResponse{Error: &errMsg}, nil
+	}
+
 	hostname := request.Hostname
 	pid := request.Pid
 

internal/controller/api/node/process/process_get_public_test.go

@@ -398,6 +398,28 @@ func (s *ProcessGetPublicTestSuite) TestGetNodeProcessByPidValidationHTTP() {
 				s.Contains(rec.Body.String(), `"results"`)
 			},
 		},
+		{
+			name: "when the pid is zero returns 400",
+			path: "/api/node/server1/process/0",
+			setupJobMock: func() *jobmocks.MockJobClient {
+				return jobmocks.NewMockJobClient(s.mockCtrl)
+			},
+			validateFunc: func(rec *httptest.ResponseRecorder) {
+				s.Equal(http.StatusBadRequest, rec.Code)
+				s.Contains(rec.Body.String(), "required")
+			},
+		},
+		{
+			name: "when the pid is minus one returns 400",
+			path: "/api/node/server1/process/-1",
+			setupJobMock: func() *jobmocks.MockJobClient {
+				return jobmocks.NewMockJobClient(s.mockCtrl)
+			},
+			validateFunc: func(rec *httptest.ResponseRecorder) {
+				s.Equal(http.StatusBadRequest, rec.Code)
+				s.Contains(rec.Body.String(), "min")
+			},
+		},
 		{
 			name: "when target agent not found",
 			path: "/api/node/nonexistent/process/1234",

internal/controller/api/node/process/process_signal_post.go

@@ -42,6 +42,10 @@ func (s *Process) PostNodeProcessSignal(
 		return gen.PostNodeProcessSignal400JSONResponse{Error: &errMsg}, nil
 	}
 
+	if errMsg, ok := validatePid(request.Pid); !ok {
+		return gen.PostNodeProcessSignal400JSONResponse{Error: &errMsg}, nil
+	}
+
 	if errMsg, ok := validation.Struct(request.Body); !ok {
 		return gen.PostNodeProcessSignal400JSONResponse{Error: &errMsg}, nil
 	}

internal/controller/api/node/process/process_signal_post_public_test.go

@@ -483,6 +483,42 @@ func (s *ProcessSignalPublicTestSuite) TestPostNodeProcessSignalValidationHTTP()
 				s.Contains(rec.Body.String(), "oneof")
 			},
 		},
+		{
+			name: "when the pid is zero returns 400",
+			path: "/api/node/server1/process/0/signal",
+			body: `{"signal":"KILL"}`,
+			setupJobMock: func() *jobmocks.MockJobClient {
+				return jobmocks.NewMockJobClient(s.mockCtrl)
+			},
+			validateFunc: func(rec *httptest.ResponseRecorder) {
+				s.Equal(http.StatusBadRequest, rec.Code)
+				s.Contains(rec.Body.String(), "required")
+			},
+		},
+		{
+			name: "when the pid is minus one returns 400",
+			path: "/api/node/server1/process/-1/signal",
+			body: `{"signal":"KILL"}`,
+			setupJobMock: func() *jobmocks.MockJobClient {
+				return jobmocks.NewMockJobClient(s.mockCtrl)
+			},
+			validateFunc: func(rec *httptest.ResponseRecorder) {
+				s.Equal(http.StatusBadRequest, rec.Code)
+				s.Contains(rec.Body.String(), "min")
+			},
+		},
+		{
+			name: "when the pid is a process group returns 400",
+			path: "/api/node/server1/process/-42/signal",
+			body: `{"signal":"KILL"}`,
+			setupJobMock: func() *jobmocks.MockJobClient {
+				return jobmocks.NewMockJobClient(s.mockCtrl)
+			},
+			validateFunc: func(rec *httptest.ResponseRecorder) {
+				s.Equal(http.StatusBadRequest, rec.Code)
+				s.Contains(rec.Body.String(), "min")
+			},
+		},
 		{
 			name: "when target agent not found",
 			path: "/api/node/nonexistent/process/1234/signal",

internal/controller/api/node/process/validate.go

@@ -20,7 +20,11 @@
 
 package process
 
-import "github.com/osapi-io/osapi/internal/validation"
+import (
+	"fmt"
+
+	"github.com/osapi-io/osapi/internal/validation"
+)
 
 // validateHostname validates a hostname path parameter using the shared
 // validator. Returns the error message and false if invalid.
@@ -32,3 +36,30 @@ func validateHostname(
 ) (string, bool) {
 	return validation.Var(hostname, "required,min=1,valid_target")
 }
+
+// validatePid validates a pid path parameter against the bound the
+// specification declares for it. Same upstream limitation as
+// validateHostname.
+//
+// min=1 is the rule that matters, not required. A pid reaches kill(2), where a
+// value below 1 does not mean "that process": 0 is every process in the
+// caller's process group, -1 is every process the caller may signal, and
+// anything lower is a whole process group. required only rejects 0, so without
+// min=1 a negative pid turns one signal into a fleet-wide one.
+func validatePid(
+	pid int,
+) (string, bool) {
+	errMsg, ok := validation.Var(pid, "required,min=1")
+	if ok {
+		return "", true
+	}
+
+	// validation.Var has no field name to report and min carries no hint, so
+	// the bare message is "Field validation for '' failed on the 'min' tag",
+	// which tells a caller nothing. Name the value and the bound, and keep the
+	// tag's own message so a test can still assert which rule rejected it.
+	return fmt.Sprintf(
+		"pid %d must be greater than zero: %s",
+		pid, errMsg,
+	), false
+}

internal/provider/node/process/debian.go

@@ -33,6 +33,27 @@ import (
 )
 
 // allowedSignals maps signal names to their syscall equivalents.
+// validatePid rejects a pid that kill(2) would not read as one process.
+//
+// kill(2) overloads the sign: 0 means every process in the caller's process
+// group, -1 means every process the caller may signal, and anything below -1
+// means a whole process group. The agent usually runs as root, so a value
+// below 1 turns one signal into a host-wide one. The API validates this too,
+// and this is the copy that matters, because this is the frame that calls
+// kill(2).
+func validatePid(
+	pid int,
+) error {
+	if pid < 1 {
+		return fmt.Errorf(
+			"process: pid %d must be greater than zero, a lower value signals a process group rather than a process",
+			pid,
+		)
+	}
+
+	return nil
+}
+
 var allowedSignals = map[string]syscall.Signal{
 	"TERM": syscall.SIGTERM,
 	"KILL": syscall.SIGKILL,
@@ -156,6 +177,10 @@ func (d *Debian) Get(
 	_ context.Context,
 	pid int,
 ) (*Info, error) {
+	if err := validatePid(pid); err != nil {
+		return nil, err
+	}
+
 	p, err := d.lister.NewProcess(int32(pid))
 	if err != nil {
 		return nil, fmt.Errorf("process: get: %w", err)
@@ -175,6 +200,10 @@ func (d *Debian) Signal(
 	pid int,
 	signal string,
 ) (*SignalResult, error) {
+	if err := validatePid(pid); err != nil {
+		return nil, err
+	}
+
 	sig, ok := allowedSignals[signal]
 	if !ok {
 		return nil, fmt.Errorf("process: signal: invalid signal %q", signal)

internal/provider/node/process/debian_public_test.go

@@ -245,6 +245,24 @@ func (suite *DebianPublicTestSuite) TestGet() {
 		wantErrMsg   string
 		validateFunc func(result *process.Info)
 	}{
+		// setupMock sets no expectation, so the controller fails the test if
+		// NewProcess is called at all. kill(2) is not reached from Get, but the
+		// guard is here because the API and the provider validate the same
+		// bound, and a pid below 1 is not a process either way.
+		{
+			name:       "when pid is zero it never reaches the lister",
+			pid:        0,
+			setupMock:  func() {},
+			wantErr:    true,
+			wantErrMsg: "pid 0 must be greater than zero",
+		},
+		{
+			name:       "when pid is negative it never reaches the lister",
+			pid:        -1,
+			setupMock:  func() {},
+			wantErr:    true,
+			wantErrMsg: "pid -1 must be greater than zero",
+		},
 		{
 			name: "when successful returns process info",
 			pid:  42,
@@ -394,6 +412,42 @@ func (suite *DebianPublicTestSuite) TestSignal() {
 			wantErr:    true,
 			wantErrMsg: "process: signal: unexpected error",
 		},
+		// Each of these would reach kill(2) as a process-group signal rather
+		// than a process signal. setupMock sets no expectation, so the mock
+		// controller fails the test if Kill is called at all, which is the
+		// assertion that matters: the signal is never sent.
+		{
+			name:       "when pid is zero it never reaches kill",
+			pid:        0,
+			signal:     "KILL",
+			setupMock:  func() {},
+			wantErr:    true,
+			wantErrMsg: "pid 0 must be greater than zero",
+		},
+		{
+			name:       "when pid is minus one it never reaches kill",
+			pid:        -1,
+			signal:     "KILL",
+			setupMock:  func() {},
+			wantErr:    true,
+			wantErrMsg: "pid -1 must be greater than zero",
+		},
+		{
+			name:       "when pid is a process group it never reaches kill",
+			pid:        -42,
+			signal:     "TERM",
+			setupMock:  func() {},
+			wantErr:    true,
+			wantErrMsg: "pid -42 must be greater than zero",
+		},
+		{
+			name:       "the pid is checked before the signal name",
+			pid:        -1,
+			signal:     "NOT-A-SIGNAL",
+			setupMock:  func() {},
+			wantErr:    true,
+			wantErrMsg: "pid -1 must be greater than zero",
+		},
 	}
 
 	for _, tc := range tests {