Kill analyzer process groups on deadline

cmd/room-semgrep/adapter.go

@@ -47,7 +47,7 @@ func newAdapter(semgrepCore, config, repositoryRoot string, covered []string) (*
 	}
 	configData, err := readRegularFile(config)
 	if err != nil {
-		return nil, errors.New("Semgrep config must be a regular file")
+		return nil, fmt.Errorf("Semgrep config must be a regular file: %w", err)
 	}
 	if err := validateRuleCoverage(configData, coveredSet); err != nil {
 		return nil, err

cmd/room-semgrep/main_test.go

@@ -2,14 +2,18 @@ package main
 
 import (
 	"bytes"
+	"context"
 	"crypto/sha256"
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
+	"syscall"
 	"testing"
+	"time"
 )
 
 const sqlSignal = "SIGNAL_KIND_DYNAMIC_SQL_WITH_UNTRUSTED_INPUT"
@@ -313,6 +317,54 @@ func TestAdapterRejectsSymlinkTargets(t *testing.T) {
 	}
 }
 
+func TestAdapterKillsSemgrepProcessGroup(t *testing.T) {
+	_, repository := workspace(t)
+	if err := os.WriteFile(filepath.Join(repository, "main.go"), []byte("package main\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	pidFile := filepath.Join(t.TempDir(), "child.pid")
+	semgrep := writeExecutable(t, "semgrep", "#!/bin/sh\nsleep 60 &\necho $! > '"+pidFile+"'\nwait\n")
+	config := writeFile(t, "rules.yml", testRules)
+	adapter, err := newAdapter(semgrep, config, repository, []string{sqlSignal})
+	if err != nil {
+		t.Fatal(err)
+	}
+	ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond)
+	defer cancel()
+	diff := []byte("diff --git a/main.go b/main.go\n--- /dev/null\n+++ b/main.go\n@@ -0,0 +1 @@\n+package main\n")
+	response := adapter.analyze(ctx, requestFor(repository, config, semgrep, diff))
+	if response.Status != failedStatus || response.FailureCode != "semgrep_failed" {
+		t.Fatalf("response = %+v", response)
+	}
+	assertProcessDies(t, pidFile)
+}
+
+func assertProcessDies(t *testing.T, pidFile string) {
+	t.Helper()
+	deadline := time.Now().Add(2 * time.Second)
+	var pid int
+	for time.Now().Before(deadline) {
+		data, err := os.ReadFile(pidFile)
+		if err == nil {
+			pid, err = strconv.Atoi(strings.TrimSpace(string(data)))
+			if err == nil && pid > 0 {
+				break
+			}
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	if pid <= 0 {
+		t.Fatal("child pid was not recorded")
+	}
+	for time.Now().Before(deadline) {
+		if err := syscall.Kill(pid, 0); err == syscall.ESRCH {
+			return
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	t.Fatalf("process %d survived the group kill", pid)
+}
+
 func TestAdapterRejectsSymlinkedConfig(t *testing.T) {
 	_, repository := workspace(t)
 	target := writeFile(t, "real-rules.yml", testRules)

cmd/room-semgrep/semgrep.go

@@ -10,6 +10,7 @@ import (
 	"os"
 	"os/exec"
 	"sort"
+	"syscall"
 )
 
 type semgrepReport struct {
@@ -43,6 +44,13 @@ type semgrepResult struct {
 func (a *adapter) scan(ctx context.Context, snapshot snapshot) (semgrepReport, string, string) {
 	args := []string{"-json_nodots", "-strict", "-rules", snapshot.config, "-targets", snapshot.targetsFile, "-j", "1", "-timeout", "0", "-timeout_threshold", "0", "-max_memory", "0"}
 	command := exec.CommandContext(ctx, a.semgrepCore, args...)
+	command.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+	command.Cancel = func() error {
+		if err := syscall.Kill(-command.Process.Pid, syscall.SIGKILL); err != nil && err != syscall.ESRCH {
+			return err
+		}
+		return nil
+	}
 	command.Dir = snapshot.directory
 	var stdout, stderr limitedBuffer
 	stdout.limit, stderr.limit = maxOutputBytes, 1<<20

internal/analyzer/analyzer.go

@@ -208,6 +208,7 @@ func (a *externalAnalyzer) Analyze(ctx context.Context, input Input) *roomv1.Ana
 		return a.failure(report, roomv1.AnalysisStatus_ANALYSIS_STATUS_FAILED, "request_encoding_failed", digest[:])
 	}
 	command := exec.CommandContext(ctx, a.config.Executable, a.config.Args...)
+	arrangeGroupKill(command)
 	command.Stdin = bytes.NewReader(requestJSON)
 	stdout := boundedDrainWriter{limit: a.maxOutputBytes}
 	command.Stdout = &stdout

internal/analyzer/procattr_other.go

@@ -0,0 +1,9 @@
+//go:build !unix
+
+package analyzer
+
+import "os/exec"
+
+// arrangeGroupKill falls back to the default direct-process kill on platforms
+// without process groups.
+func arrangeGroupKill(command *exec.Cmd) {}

internal/analyzer/procattr_unix.go

@@ -0,0 +1,21 @@
+//go:build unix
+
+package analyzer
+
+import (
+	"os/exec"
+	"syscall"
+)
+
+// arrangeGroupKill runs the provider in its own process group and kills the
+// whole group when the context ends, so scanners spawned by the provider
+// cannot outlive the analyzer deadline.
+func arrangeGroupKill(command *exec.Cmd) {
+	command.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+	command.Cancel = func() error {
+		if err := syscall.Kill(-command.Process.Pid, syscall.SIGKILL); err != nil && err != syscall.ESRCH {
+			return err
+		}
+		return nil
+	}
+}

internal/analyzer/procattr_unix_test.go

@@ -0,0 +1,60 @@
+//go:build unix
+
+package analyzer
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"syscall"
+	"testing"
+	"time"
+
+	roomv1 "github.com/haasonsaas/room/gen/go/room/v1"
+)
+
+func TestExternalAnalyzerKillsProviderProcessGroup(t *testing.T) {
+	pidFile := filepath.Join(t.TempDir(), "child.pid")
+	executable := filepath.Join(t.TempDir(), "group-provider")
+	script := "#!/bin/sh\nsleep 60 &\necho $! > '" + pidFile + "'\nwait\n"
+	if err := os.WriteFile(executable, []byte(script), 0o700); err != nil {
+		t.Fatal(err)
+	}
+	a, err := NewExternal(Config{ID: "group", Version: "1", Executable: executable, Timeout: 100 * time.Millisecond, CoveredSignals: []roomv1.SignalKind{secretSignal}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	report := a.Analyze(context.Background(), Input{Phase: roomv1.AnalysisPhase_ANALYSIS_PHASE_PLAN})
+	if report.GetStatus() != roomv1.AnalysisStatus_ANALYSIS_STATUS_FAILED || report.GetReceipts()[0].GetFailureCode() != "provider_failed" {
+		t.Fatalf("report = %+v", report)
+	}
+	assertProcessGroupDies(t, pidFile)
+}
+
+func assertProcessGroupDies(t *testing.T, pidFile string) {
+	t.Helper()
+	deadline := time.Now().Add(2 * time.Second)
+	var pid int
+	for time.Now().Before(deadline) {
+		data, err := os.ReadFile(pidFile)
+		if err == nil {
+			pid, err = strconv.Atoi(strings.TrimSpace(string(data)))
+			if err == nil && pid > 0 {
+				break
+			}
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	if pid <= 0 {
+		t.Fatal("child pid was not recorded")
+	}
+	for time.Now().Before(deadline) {
+		if err := syscall.Kill(pid, 0); err == syscall.ESRCH {
+			return
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	t.Fatalf("process %d survived the group kill", pid)
+}