diff --git a/cmd/room-semgrep/adapter.go b/cmd/room-semgrep/adapter.go index 9710d10..50464c4 100644 --- a/cmd/room-semgrep/adapter.go +++ b/cmd/room-semgrep/adapter.go @@ -21,6 +21,7 @@ type adapter struct { repositoryRoot string covered []string coveredSet map[string]bool + tool toolBinding } func newAdapter(semgrepCore, config, repositoryRoot string, covered []string) (*adapter, error) { @@ -51,8 +52,26 @@ func newAdapter(semgrepCore, config, repositoryRoot string, covered []string) (* if err := validateRuleCoverage(configData, coveredSet); err != nil { return nil, err } + resolvedCore, err := filepath.EvalSymlinks(semgrepCore) + if err != nil { + return nil, errors.New("semgrep-core executable cannot be resolved") + } + tool, err := hashToolBinary(resolvedCore) + if err != nil { + return nil, fmt.Errorf("semgrep-core executable must resolve to a regular file: %w", err) + } sort.Strings(covered) - return &adapter{semgrepCore: semgrepCore, config: config, repositoryRoot: repositoryRoot, covered: covered, coveredSet: coveredSet}, nil + return &adapter{semgrepCore: semgrepCore, config: config, repositoryRoot: repositoryRoot, covered: covered, coveredSet: coveredSet, tool: tool}, nil +} + +// toolMatches reports whether the binary the semgrep-core path currently +// resolves to is still the startup-pinned binary with the expected digest. +func (a *adapter) toolMatches(expected string) bool { + resolved, err := filepath.EvalSymlinks(a.semgrepCore) + if err != nil { + return false + } + return a.tool.matches(resolved, expected) } func validateRuleCoverage(config []byte, covered map[string]bool) error { @@ -134,6 +153,10 @@ func (a *adapter) analyze(ctx context.Context, request analyzerRequest) analyzer response.FailureCode = "config_digest_mismatch" return response } + if !a.toolMatches(request.ToolSHA256) { + response.FailureCode = "tool_digest_mismatch" + return response + } snapshot, err := a.createSnapshot(request, artifact) if err != nil { response.FailureCode = "snapshot_invalid" diff --git a/cmd/room-semgrep/adapter_integration_test.go b/cmd/room-semgrep/adapter_integration_test.go index 2cf1f9b..b87fda7 100644 --- a/cmd/room-semgrep/adapter_integration_test.go +++ b/cmd/room-semgrep/adapter_integration_test.go @@ -30,7 +30,7 @@ func handler(db *sql.DB, r *http.Request) { t.Fatal(err) } diff := []byte("diff --git a/query.go b/query.go\n--- a/query.go\n+++ b/query.go\n@@ -6,0 +7 @@\n+\tdb.Query(second)\n") - response := adapter.analyze(t.Context(), requestFor(repository, config, diff)) + response := adapter.analyze(t.Context(), requestFor(repository, config, core, diff)) if response.Status != completeStatus || len(response.Signals) != 1 { t.Fatalf("response = %+v", response) } @@ -116,7 +116,7 @@ func TestSemgrepCoreIntegrationIncludesAddedSources(t *testing.T) { if err != nil { t.Fatal(err) } - response := adapter.analyze(t.Context(), requestFor(repository, config, diff)) + response := adapter.analyze(t.Context(), requestFor(repository, config, core, diff)) if response.Status != completeStatus || len(response.Signals) != 1 || response.Signals[0].Kind != test.signal || response.Signals[0].Location.StartLine != int32(test.resultLine) { t.Fatalf("response = %+v", response) } @@ -149,7 +149,7 @@ func TestSemgrepCoreIntegrationRejectsInvalidRule(t *testing.T) { if err != nil { t.Fatal(err) } - response := adapter.analyze(t.Context(), requestFor(repository, config, newFileDiff("main.go", source))) + response := adapter.analyze(t.Context(), requestFor(repository, config, core, newFileDiff("main.go", source))) if response.Status != failedStatus || response.FailureCode != "semgrep_report_invalid" { t.Fatalf("response = %+v", response) } @@ -166,7 +166,7 @@ func TestSemgrepCoreIntegrationFailsClosedForUnsupportedLanguage(t *testing.T) { if err != nil { t.Fatal(err) } - response := adapter.analyze(t.Context(), requestFor(repository, config, newFileDiff("credentials.txt", source))) + response := adapter.analyze(t.Context(), requestFor(repository, config, core, newFileDiff("credentials.txt", source))) if response.Status != failedStatus || response.FailureCode != "semgrep_targets_incomplete" { t.Fatalf("response = %+v", response) } diff --git a/cmd/room-semgrep/main.go b/cmd/room-semgrep/main.go index 589b479..23bd76d 100644 --- a/cmd/room-semgrep/main.go +++ b/cmd/room-semgrep/main.go @@ -34,6 +34,7 @@ type analyzerRequest struct { ChangedFiles []string `json:"changed_files,omitempty"` WorkingDirectory string `json:"working_directory,omitempty"` ConfigSHA256 string `json:"config_sha256"` + ToolSHA256 string `json:"tool_sha256,omitempty"` InputSHA256 string `json:"input_sha256"` } diff --git a/cmd/room-semgrep/main_test.go b/cmd/room-semgrep/main_test.go index 0d0eeec..5e5436b 100644 --- a/cmd/room-semgrep/main_test.go +++ b/cmd/room-semgrep/main_test.go @@ -43,7 +43,7 @@ func TestAdapterMapsSemgrepMetadataAndFiltersToAddedLines(t *testing.T) { t.Fatal(err) } diff := []byte("diff --git a/handler.go b/handler.go\n--- a/handler.go\n+++ b/handler.go\n@@ -7,0 +8 @@\n+db.Query(query)\n") - response := adapter.analyze(t.Context(), requestFor(repository, config, diff)) + response := adapter.analyze(t.Context(), requestFor(repository, config, semgrep, diff)) if response.Status != completeStatus || fmt.Sprint(response.CoveredSignals) != "["+sqlSignal+"]" { t.Fatalf("response = %+v", response) @@ -77,19 +77,20 @@ func TestAdapterFiltersAgainstSemgrepDataflowTrace(t *testing.T) { }}] }` config := writeFile(t, "rules.yml", testRules) - adapter, err := newAdapter(fakeSemgrep(t, report, 0), config, repository, []string{sqlSignal}) + semgrep := fakeSemgrep(t, report, 0) + adapter, err := newAdapter(semgrep, config, repository, []string{sqlSignal}) if err != nil { t.Fatal(err) } diff := []byte("diff --git a/handler.go b/handler.go\n--- a/handler.go\n+++ b/handler.go\n@@ -1,0 +2 @@\n+\tlet query = input();\n") - response := adapter.analyze(t.Context(), requestFor(repository, config, diff)) + response := adapter.analyze(t.Context(), requestFor(repository, config, semgrep, diff)) if response.Status != completeStatus || len(response.Signals) != 1 || response.Signals[0].Location.StartLine != 3 { t.Fatalf("source-intersection response = %+v", response) } diff = []byte("diff --git a/handler.go b/handler.go\n--- a/handler.go\n+++ b/handler.go\n@@ -0,0 +1 @@\n+fn handler() {\n") - response = adapter.analyze(t.Context(), requestFor(repository, config, diff)) + response = adapter.analyze(t.Context(), requestFor(repository, config, semgrep, diff)) if response.Status != completeStatus || len(response.Signals) != 0 { t.Fatalf("non-intersection response = %+v", response) } @@ -122,12 +123,13 @@ func TestAdapterRejectsInvalidSemgrepRanges(t *testing.T) { } }}]}`, test.resultEnd, test.tracePath, test.traceLine, test.traceLine) config := writeFile(t, "rules.yml", testRules) - adapter, err := newAdapter(fakeSemgrep(t, report, 0), config, repository, []string{sqlSignal}) + semgrep := fakeSemgrep(t, report, 0) + adapter, err := newAdapter(semgrep, config, repository, []string{sqlSignal}) if err != nil { t.Fatal(err) } diff := []byte("diff --git a/handler.go b/handler.go\n--- a/handler.go\n+++ b/handler.go\n@@ -1,0 +2 @@\n+\tlet query = input();\n") - response := adapter.analyze(t.Context(), requestFor(repository, config, diff)) + response := adapter.analyze(t.Context(), requestFor(repository, config, semgrep, diff)) if response.Status != failedStatus || response.FailureCode != "semgrep_result_invalid" { t.Fatalf("response = %+v", response) } @@ -138,7 +140,7 @@ func TestAdapterRejectsInvalidSemgrepRanges(t *testing.T) { func TestAdapterReturnsPartialForPlansWithoutRunningSemgrep(t *testing.T) { root, repository := workspace(t) config := writeFile(t, "rules.yml", testRules) - adapter, err := newAdapter("/missing/semgrep", config, root, []string{sqlSignal}) + adapter, err := newAdapter(fakeTool(t), config, root, []string{sqlSignal}) if err != nil { t.Fatal(err) } @@ -157,11 +159,12 @@ func TestAdapterRejectsWorkspaceOutsideConfiguredRoot(t *testing.T) { if err := os.WriteFile(filepath.Join(outside, "main.go"), []byte("package main\n"), 0o600); err != nil { t.Fatal(err) } - adapter, err := newAdapter("/missing/semgrep", config, repository, []string{sqlSignal}) + tool := fakeTool(t) + adapter, err := newAdapter(tool, config, repository, []string{sqlSignal}) if err != nil { t.Fatal(err) } - response := adapter.analyze(t.Context(), requestFor(outside, config, diff)) + response := adapter.analyze(t.Context(), requestFor(outside, config, tool, diff)) if response.Status != failedStatus || response.FailureCode != "snapshot_invalid" { t.Fatalf("response = %+v", response) } @@ -190,7 +193,7 @@ func TestAdapterRejectsMalformedSemgrepResult(t *testing.T) { t.Fatal(err) } diff := []byte("diff --git a/main.go b/main.go\n--- a/main.go\n+++ b/main.go\n@@ -0,0 +1 @@\n+package main\n") - response := adapter.analyze(t.Context(), requestFor(repository, config, diff)) + response := adapter.analyze(t.Context(), requestFor(repository, config, semgrep, diff)) if response.Status != failedStatus || response.FailureCode != "semgrep_result_invalid" { t.Fatalf("response = %+v", response) } @@ -212,12 +215,13 @@ func TestAdapterFailsClosedForIncompleteOrSkippedScans(t *testing.T) { t.Fatal(err) } config := writeFile(t, "rules.yml", testRules) - adapter, err := newAdapter(fakeSemgrep(t, tt.report, 0), config, repository, []string{sqlSignal}) + semgrep := fakeSemgrep(t, tt.report, 0) + adapter, err := newAdapter(semgrep, config, repository, []string{sqlSignal}) if err != nil { t.Fatal(err) } 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(t.Context(), requestFor(repository, config, diff)) + response := adapter.analyze(t.Context(), requestFor(repository, config, semgrep, diff)) if response.Status != failedStatus || response.FailureCode != tt.code { t.Fatalf("response = %+v", response) } @@ -231,18 +235,19 @@ func TestAdapterBindsConfigAndSourcePostimage(t *testing.T) { t.Fatal(err) } config := writeFile(t, "rules.yml", testRules) - adapter, err := newAdapter("/missing/semgrep", config, repository, []string{sqlSignal}) + tool := fakeTool(t) + adapter, err := newAdapter(tool, config, repository, []string{sqlSignal}) if err != nil { t.Fatal(err) } 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(t.Context(), requestFor(repository, config, diff)) + response := adapter.analyze(t.Context(), requestFor(repository, config, tool, diff)) if response.FailureCode != "snapshot_invalid" { t.Fatalf("source mismatch response = %+v", response) } deletion := []byte("diff --git a/old.go b/old.go\n--- a/old.go\n+++ /dev/null\n@@ -1 +0,0 @@\n-package old\n") - request := requestFor(repository, config, deletion) + request := requestFor(repository, config, tool, deletion) if err := os.WriteFile(config, []byte(testRules+"# changed\n"), 0o600); err != nil { t.Fatal(err) } @@ -252,6 +257,41 @@ func TestAdapterBindsConfigAndSourcePostimage(t *testing.T) { } } +func TestAdapterBindsToolDigest(t *testing.T) { + _, repository := workspace(t) + if err := os.WriteFile(filepath.Join(repository, "handler.go"), []byte(strings.Repeat("\n", 7)+"db.Query(query)\n"), 0o600); err != nil { + t.Fatal(err) + } + report := `{ + "version":"1.139.0", + "errors":[], + "paths":{"scanned":["handler.go"],"skipped":[]}, + "skipped_rules":[], + "results":[ + {"check_id":"old","path":"handler.go","start":{"line":3},"end":{"line":3},"extra":{"metadata":{"room_signal":"SIGNAL_KIND_DYNAMIC_SQL_WITH_UNTRUSTED_INPUT","room_confidence_basis_points":9000}}}, + {"check_id":"new","path":"handler.go","start":{"line":8},"end":{"line":8},"extra":{"metadata":{"room_signal":"SIGNAL_KIND_DYNAMIC_SQL_WITH_UNTRUSTED_INPUT","room_confidence_basis_points":9000}}} + ] +}` + semgrep := fakeSemgrep(t, report, 0) + config := writeFile(t, "rules.yml", testRules) + adapter, err := newAdapter(semgrep, config, repository, []string{sqlSignal}) + if err != nil { + t.Fatal(err) + } + diff := []byte("diff --git a/handler.go b/handler.go\n--- a/handler.go\n+++ b/handler.go\n@@ -7,0 +8 @@\n+db.Query(query)\n") + request := requestFor(repository, config, semgrep, diff) + + request.ToolSHA256 = "" + if response := adapter.analyze(t.Context(), request); response.Status != failedStatus || response.FailureCode != "tool_digest_mismatch" { + t.Fatalf("empty tool digest response = %+v", response) + } + other := sha256.Sum256([]byte("other-binary")) + request.ToolSHA256 = hex.EncodeToString(other[:]) + if response := adapter.analyze(t.Context(), request); response.Status != failedStatus || response.FailureCode != "tool_digest_mismatch" { + t.Fatalf("wrong tool digest response = %+v", response) + } +} + func TestAdapterRejectsSymlinkTargets(t *testing.T) { _, repository := workspace(t) realFile := filepath.Join(repository, "real.go") @@ -262,12 +302,13 @@ func TestAdapterRejectsSymlinkTargets(t *testing.T) { t.Fatal(err) } config := writeFile(t, "rules.yml", testRules) - adapter, err := newAdapter("/missing/semgrep", config, repository, []string{sqlSignal}) + tool := fakeTool(t) + adapter, err := newAdapter(tool, config, repository, []string{sqlSignal}) if err != nil { t.Fatal(err) } 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") - if response := adapter.analyze(t.Context(), requestFor(repository, config, diff)); response.FailureCode != "snapshot_invalid" { + if response := adapter.analyze(t.Context(), requestFor(repository, config, tool, diff)); response.FailureCode != "snapshot_invalid" { t.Fatalf("response = %+v", response) } } @@ -279,22 +320,42 @@ func TestAdapterRejectsSymlinkedConfig(t *testing.T) { if err := os.Symlink(target, linked); err != nil { t.Fatal(err) } - if _, err := newAdapter("/missing/semgrep", linked, repository, []string{sqlSignal}); err == nil { + if _, err := newAdapter(fakeTool(t), linked, repository, []string{sqlSignal}); err == nil { t.Fatal("expected symlinked config to be rejected") } } +func TestAdapterBindsSymlinkedToolToRegularTarget(t *testing.T) { + _, repository := workspace(t) + config := writeFile(t, "rules.yml", testRules) + linked := filepath.Join(t.TempDir(), "semgrep-core") + if err := os.Symlink(fakeTool(t), linked); err != nil { + t.Fatal(err) + } + if _, err := newAdapter(linked, config, repository, []string{sqlSignal}); err != nil { + t.Fatalf("symlink to a regular tool must be accepted: %v", err) + } + dangling := filepath.Join(t.TempDir(), "dangling-core") + if err := os.Symlink(filepath.Join(t.TempDir(), "missing"), dangling); err != nil { + t.Fatal(err) + } + if _, err := newAdapter(dangling, config, repository, []string{sqlSignal}); err == nil { + t.Fatal("expected dangling symlink to be rejected") + } +} + func TestAdapterFailsClosedWhenConfigBecomesSymlink(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) } config := writeFile(t, "rules.yml", testRules) - adapter, err := newAdapter("/missing/semgrep", config, repository, []string{sqlSignal}) + tool := fakeTool(t) + adapter, err := newAdapter(tool, config, repository, []string{sqlSignal}) if err != nil { t.Fatal(err) } - request := requestFor(repository, config, []byte("diff --git a/main.go b/main.go\n--- /dev/null\n+++ b/main.go\n@@ -0,0 +1 @@\n+package main\n")) + request := requestFor(repository, config, tool, []byte("diff --git a/main.go b/main.go\n--- /dev/null\n+++ b/main.go\n@@ -0,0 +1 @@\n+package main\n")) target := writeFile(t, "same-rules.yml", testRules) if err := os.Remove(config); err != nil { t.Fatal(err) @@ -307,10 +368,30 @@ func TestAdapterFailsClosedWhenConfigBecomesSymlink(t *testing.T) { } } +func TestAdapterFailsClosedWhenToolChanges(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) + } + tool := fakeTool(t) + config := writeFile(t, "rules.yml", testRules) + adapter, err := newAdapter(tool, config, repository, []string{sqlSignal}) + if err != nil { + t.Fatal(err) + } + request := requestFor(repository, config, tool, []byte("diff --git a/main.go b/main.go\n--- /dev/null\n+++ b/main.go\n@@ -0,0 +1 @@\n+package main\n")) + if err := os.WriteFile(tool, []byte("#!/bin/sh\nexit 2\n# changed\n"), 0o700); err != nil { + t.Fatal(err) + } + if response := adapter.analyze(t.Context(), request); response.Status != failedStatus || response.FailureCode != "tool_digest_mismatch" { + t.Fatalf("response = %+v", response) + } +} + func TestAdapterRejectsUnimplementedCoverageAndTraversalDiff(t *testing.T) { _, repository := workspace(t) emptyConfig := writeFile(t, "empty.yml", "rules: []\n") - if _, err := newAdapter("/missing/semgrep", emptyConfig, repository, []string{sqlSignal}); err == nil { + if _, err := newAdapter(fakeTool(t), emptyConfig, repository, []string{sqlSignal}); err == nil { t.Fatal("expected empty ruleset to be rejected") } diff := []byte("diff --git a/x b/x\n--- a/../../x\n+++ /dev/null\n@@ -1 +0,0 @@\n-secret\n") @@ -363,11 +444,14 @@ func workspace(t *testing.T) (string, string) { return root, repository } -func requestFor(repository, config string, content []byte) analyzerRequest { +func requestFor(repository, config, semgrepCore string, content []byte) analyzerRequest { request := requestForPhase(repository, content, "ANALYSIS_PHASE_DIFF") configData, _ := os.ReadFile(config) configDigest := sha256.Sum256(configData) request.ConfigSHA256 = hex.EncodeToString(configDigest[:]) + toolData, _ := os.ReadFile(semgrepCore) + toolDigest := sha256.Sum256(toolData) + request.ToolSHA256 = hex.EncodeToString(toolDigest[:]) return request } @@ -381,6 +465,11 @@ func fakeSemgrep(t *testing.T, report string, exitCode int) string { return writeExecutable(t, "semgrep", fmt.Sprintf("#!/bin/sh\nprintf '%%s' '%s'\nexit %d\n", report, exitCode)) } +func fakeTool(t *testing.T) string { + t.Helper() + return writeExecutable(t, "semgrep-core", "#!/bin/sh\nexit 1\n") +} + func writeExecutable(t *testing.T, name, content string) string { t.Helper() path := filepath.Join(t.TempDir(), name) @@ -406,7 +495,7 @@ func TestRunEmitsStrictProviderJSON(t *testing.T) { encoded, _ := json.Marshal(request) var stdout, stderr bytes.Buffer config := writeFile(t, "rules.yml", testRules) - args := []string{"--semgrep-core", "/missing/semgrep-core", "--config", config, "--repository-root", repository, "--covered-signal", sqlSignal} + args := []string{"--semgrep-core", fakeTool(t), "--config", config, "--repository-root", repository, "--covered-signal", sqlSignal} if code := run(args, bytes.NewReader(encoded), &stdout, &stderr); code != 0 { t.Fatalf("run exit %d: %s", code, stderr.String()) } diff --git a/cmd/room-semgrep/rules_integration_test.go b/cmd/room-semgrep/rules_integration_test.go index f316afd..d05d896 100644 --- a/cmd/room-semgrep/rules_integration_test.go +++ b/cmd/room-semgrep/rules_integration_test.go @@ -281,7 +281,7 @@ fn load() { if err != nil { t.Fatal(err) } - response := adapter.analyze(t.Context(), requestFor(repository, config, newFileDiff(test.path, test.source))) + response := adapter.analyze(t.Context(), requestFor(repository, config, core, newFileDiff(test.path, test.source))) if response.Status != completeStatus { t.Fatalf("response = %+v", response) } diff --git a/cmd/room-semgrep/rules_negative_integration_test.go b/cmd/room-semgrep/rules_negative_integration_test.go index 8400602..9448874 100644 --- a/cmd/room-semgrep/rules_negative_integration_test.go +++ b/cmd/room-semgrep/rules_negative_integration_test.go @@ -179,7 +179,7 @@ fn handler(request: Request) { if err != nil { t.Fatal(err) } - response := adapter.analyze(t.Context(), requestFor(repository, config, newFileDiff(test.path, test.source))) + response := adapter.analyze(t.Context(), requestFor(repository, config, core, newFileDiff(test.path, test.source))) if response.Status != completeStatus || len(response.Signals) != 0 { t.Fatalf("response = %+v", response) } diff --git a/cmd/room-semgrep/snapshot.go b/cmd/room-semgrep/snapshot.go index a39d69a..32a1aeb 100644 --- a/cmd/room-semgrep/snapshot.go +++ b/cmd/room-semgrep/snapshot.go @@ -131,6 +131,48 @@ func (a *adapter) createSnapshot(request analyzerRequest, artifact diffArtifact) return snapshot{directory: repository, config: configPath, targetsFile: targetsPath, targets: targets, lineCounts: lineCounts}, nil } +// toolBinding pins one scanner binary by digest and filesystem metadata. The +// caller resolves symlinks before hashing and again on every request, so +// pipx-style symlinked tool paths are supported while a swapped or upgraded +// binary still fails closed until the adapter restarts. +type toolBinding struct { + digest string + stat unix.Stat_t +} + +func hashToolBinary(path string) (toolBinding, error) { + fileFD, err := unix.Open(path, unix.O_RDONLY|unix.O_CLOEXEC|unix.O_NOFOLLOW|unix.O_NONBLOCK, 0) + if err != nil { + return toolBinding{}, err + } + file := os.NewFile(uintptr(fileFD), path) + defer file.Close() + var before, after unix.Stat_t + if err := unix.Fstat(fileFD, &before); err != nil || before.Mode&unix.S_IFMT != unix.S_IFREG { + return toolBinding{}, errors.New("must be a regular file") + } + digest := sha256.New() + if _, err := io.Copy(digest, file); err != nil { + return toolBinding{}, err + } + if err := unix.Fstat(fileFD, &after); err != nil || before.Dev != after.Dev || before.Ino != after.Ino || before.Size != after.Size || before.Mtim != after.Mtim || before.Ctim != after.Ctim { + return toolBinding{}, errors.New("changed while being read") + } + return toolBinding{digest: hex.EncodeToString(digest.Sum(nil)), stat: before}, nil +} + +func (binding toolBinding) matches(path, expected string) bool { + var current unix.Stat_t + if err := unix.Lstat(path, ¤t); err != nil { + return false + } + before := binding.stat + if current.Dev != before.Dev || current.Ino != before.Ino || current.Size != before.Size || current.Mtim != before.Mtim || current.Ctim != before.Ctim { + return false + } + return expected != "" && strings.EqualFold(expected, binding.digest) +} + func readRegularBeneath(rootFD int, path string) ([]byte, error) { fileFD, err := unix.Openat2(rootFD, filepath.FromSlash(path), &unix.OpenHow{ Flags: unix.O_RDONLY | unix.O_CLOEXEC | unix.O_NOFOLLOW, diff --git a/cmd/roomd/main.go b/cmd/roomd/main.go index 0110019..147da11 100644 --- a/cmd/roomd/main.go +++ b/cmd/roomd/main.go @@ -44,10 +44,18 @@ func main() { log.Fatalf("read analyzer config: %v", err) } } + var analyzerTool []byte + if cfg.AnalyzerToolFile != "" { + analyzerTool, err = os.ReadFile(cfg.AnalyzerToolFile) + if err != nil { + log.Fatalf("read analyzer tool: %v", err) + } + } provider, buildErr := analyzer.NewExternal(analyzer.Config{ ID: cfg.AnalyzerID, Version: cfg.AnalyzerVersion, Executable: cfg.AnalyzerExecutable, - Args: cfg.AnalyzerArgs, Config: analyzerConfig, CoveredSignals: coveredSignals, Timeout: cfg.AnalyzerTimeout, + Args: cfg.AnalyzerArgs, Config: analyzerConfig, Tool: analyzerTool, CoveredSignals: coveredSignals, Timeout: cfg.AnalyzerTimeout, }) + analyzerTool = nil // the digest is bound; the binary bytes stay out of resident memory if buildErr != nil { log.Fatalf("configure analyzer: %v", buildErr) } diff --git a/docs/analyzer.md b/docs/analyzer.md index 7f42323..301a846 100644 --- a/docs/analyzer.md +++ b/docs/analyzer.md @@ -2,7 +2,9 @@ Room launches one explicitly configured absolute executable without a shell. The request is strict JSON on stdin and contains the analysis phase, base64 JSON -content bytes, changed files, working directory, and SHA-256 input digest. The +content bytes, changed files, working directory, and SHA-256 digests for the +input, the analyzer configuration, and—when `ROOM_ANALYZER_TOOL_FILE` is +set—the tool binary. The executable must return exactly one JSON object on stdout; unknown or trailing fields are rejected. The working directory is caller-supplied and must be restricted by analyzers that read files. @@ -15,7 +17,8 @@ policy uses them only when the report contains a valid receipt from the configur analyzer. Agent-supplied classification cannot narrow a rule's scope. Every signal has a stable fingerprint, a confidence from 0–10000, and optional typed location/evidence hashes. Room—not the provider—stamps the configured analyzer -ID, version, and configuration digest onto accepted receipts. +ID, version, configuration digest, and tool-binary digest onto accepted +receipts. For `COMPLETE`, all signals configured for the analyzer must be present in `covered_signals`, even when no finding exists. Process failure, digest mismatch, @@ -30,6 +33,7 @@ ROOM_ANALYZER_EXECUTABLE=/absolute/path/to/analyzer ROOM_ANALYZER_ARGS='["--format","room-v1"]' ROOM_ANALYZER_COVERED_SIGNALS='["SIGNAL_KIND_RUST_UNSAFE_WITHOUT_SAFETY_CONTRACT","SIGNAL_KIND_RUST_PANIC_IN_REQUEST_PATH"]' ROOM_ANALYZER_CONFIG_FILE=/path/to/analyzer-config +ROOM_ANALYZER_TOOL_FILE=/path/to/scanner-binary # optional; SHA-256 bound into receipts ROOM_ANALYZER_ID=company.security-analyzer ROOM_ANALYZER_VERSION=1 ROOM_ANALYZER_TIMEOUT=30s @@ -115,12 +119,20 @@ go build -o ~/.local/bin/room-semgrep ./cmd/room-semgrep ROOM_ANALYZER_EXECUTABLE="$HOME/.local/bin/room-semgrep" ROOM_ANALYZER_ARGS='["--semgrep-core","/absolute/path/to/semgrep-core","--config","/absolute/path/to/room/analyzers/semgrep/room.yml","--repository-root","/srv/repos/my-repository","--covered-signal","SIGNAL_KIND_SECRET_LITERAL","--covered-signal","SIGNAL_KIND_DYNAMIC_SQL_WITH_UNTRUSTED_INPUT","--covered-signal","SIGNAL_KIND_UNTRUSTED_OUTBOUND_DESTINATION","--covered-signal","SIGNAL_KIND_RUST_PANIC_IN_REQUEST_PATH","--covered-signal","SIGNAL_KIND_RUST_COMMAND_WITH_UNTRUSTED_ARGUMENT","--covered-signal","SIGNAL_KIND_RUST_WEAK_RNG_FOR_SECRET","--covered-signal","SIGNAL_KIND_RUST_UNTRUSTED_PATH","--covered-signal","SIGNAL_KIND_RUST_BLOCKING_LOCK_ACROSS_AWAIT"]' ROOM_ANALYZER_CONFIG_FILE=/absolute/path/to/room/analyzers/semgrep/room.yml +ROOM_ANALYZER_TOOL_FILE=/absolute/path/to/semgrep-core ROOM_ANALYZER_COVERED_SIGNALS='["SIGNAL_KIND_SECRET_LITERAL","SIGNAL_KIND_DYNAMIC_SQL_WITH_UNTRUSTED_INPUT","SIGNAL_KIND_UNTRUSTED_OUTBOUND_DESTINATION","SIGNAL_KIND_RUST_PANIC_IN_REQUEST_PATH","SIGNAL_KIND_RUST_COMMAND_WITH_UNTRUSTED_ARGUMENT","SIGNAL_KIND_RUST_WEAK_RNG_FOR_SECRET","SIGNAL_KIND_RUST_UNTRUSTED_PATH","SIGNAL_KIND_RUST_BLOCKING_LOCK_ACROSS_AWAIT"]' ROOM_ANALYZER_ID=room.semgrep ROOM_ANALYZER_VERSION=1 ``` `ROOM_ANALYZER_CONFIG_FILE` binds the rules file contents to Room's analyzer -identity. Update `ROOM_ANALYZER_VERSION` when the `semgrep-core` binary changes. +identity, and `ROOM_ANALYZER_TOOL_FILE` binds the `semgrep-core` binary the +same way. The setting is optional in the analyzer contract, but the stock +`room-semgrep` adapter requires it: without a digest, every diff request fails +with `tool_digest_mismatch`. The adapter resolves the tool path at +startup—pipx-style symlinked installs work—hashes the resolved binary once, +and re-resolves it on every diff request; a binary that changed since startup +also fails with `tool_digest_mismatch`. Restart the adapter after upgrading +`semgrep-core`. The adapter returns `PARTIAL` for plan analysis and does not claim signal coverage for plans. diff --git a/gen/go/room/v1/rules.pb.go b/gen/go/room/v1/rules.pb.go index 5d4dd88..6009e5c 100644 --- a/gen/go/room/v1/rules.pb.go +++ b/gen/go/room/v1/rules.pb.go @@ -1892,6 +1892,7 @@ type AnalyzerIdentity struct { Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"` Version string `protobuf:"bytes,2,opt,name=version,proto3" json:"version,omitempty"` ConfigSha256 []byte `protobuf:"bytes,3,opt,name=config_sha256,json=configSha256,proto3" json:"config_sha256,omitempty"` + ToolSha256 []byte `protobuf:"bytes,4,opt,name=tool_sha256,json=toolSha256,proto3" json:"tool_sha256,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -1947,6 +1948,13 @@ func (x *AnalyzerIdentity) GetConfigSha256() []byte { return nil } +func (x *AnalyzerIdentity) GetToolSha256() []byte { + if x != nil { + return x.ToolSha256 + } + return nil +} + type SecuritySignal struct { state protoimpl.MessageState `protogen:"open.v1"` Kind SignalKind `protobuf:"varint,1,opt,name=kind,proto3,enum=room.v1.SignalKind" json:"kind,omitempty"` @@ -7924,11 +7932,13 @@ const file_room_v1_rules_proto_rawDesc = "" + "\tlanguages\x18\x04 \x03(\tR\tlanguages\x12\x1e\n" + "\n" + "frameworks\x18\x05 \x03(\tR\n" + - "frameworks\"a\n" + + "frameworks\"\x82\x01\n" + "\x10AnalyzerIdentity\x12\x0e\n" + "\x02id\x18\x01 \x01(\tR\x02id\x12\x18\n" + "\aversion\x18\x02 \x01(\tR\aversion\x12#\n" + - "\rconfig_sha256\x18\x03 \x01(\fR\fconfigSha256\"\xa8\x02\n" + + "\rconfig_sha256\x18\x03 \x01(\fR\fconfigSha256\x12\x1f\n" + + "\vtool_sha256\x18\x04 \x01(\fR\n" + + "toolSha256\"\xa8\x02\n" + "\x0eSecuritySignal\x12'\n" + "\x04kind\x18\x01 \x01(\x0e2\x13.room.v1.SignalKindR\x04kind\x12 \n" + "\vfingerprint\x18\x02 \x01(\tR\vfingerprint\x125\n" + diff --git a/internal/analyzer/analyzer.go b/internal/analyzer/analyzer.go index 7c60776..52ee0fd 100644 --- a/internal/analyzer/analyzer.go +++ b/internal/analyzer/analyzer.go @@ -34,13 +34,16 @@ type Input struct { } // Config identifies and launches one analyzer. Args are passed directly to the -// executable; they are never interpreted by a shell. +// executable; they are never interpreted by a shell. Tool optionally holds the +// bytes of the analyzer's underlying scanner binary; they are hashed into the +// analyzer identity and then discarded, never retained. type Config struct { ID string Version string Executable string Args []string Config []byte + Tool []byte CoveredSignals []roomv1.SignalKind MaxOutputBytes int64 Timeout time.Duration @@ -98,12 +101,18 @@ func NewExternal(config Config) (Analyzer, error) { coverage[signal] = struct{}{} } configDigest := sha256.Sum256(config.Config) + var toolDigest []byte + if len(config.Tool) > 0 { + digest := sha256.Sum256(config.Tool) + toolDigest = digest[:] + } config.Args = append([]string(nil), config.Args...) config.Config = append([]byte(nil), config.Config...) + config.Tool = nil config.CoveredSignals = append([]roomv1.SignalKind(nil), config.CoveredSignals...) return &externalAnalyzer{ config: config, - identity: &roomv1.AnalyzerIdentity{Id: config.ID, Version: config.Version, ConfigSha256: configDigest[:]}, + identity: &roomv1.AnalyzerIdentity{Id: config.ID, Version: config.Version, ConfigSha256: configDigest[:], ToolSha256: toolDigest}, coverage: coverage, maxOutputBytes: maxOutputBytes, timeout: timeout, @@ -144,6 +153,7 @@ type providerRequest struct { ChangedFiles []string `json:"changed_files,omitempty"` WorkingDirectory string `json:"working_directory,omitempty"` ConfigSHA256 string `json:"config_sha256"` + ToolSHA256 string `json:"tool_sha256,omitempty"` InputSHA256 string `json:"input_sha256"` } @@ -190,6 +200,9 @@ func (a *externalAnalyzer) Analyze(ctx context.Context, input Input) *roomv1.Ana ChangedFiles: append([]string(nil), input.ChangedFiles...), WorkingDirectory: input.WorkingDirectory, ConfigSHA256: hex.EncodeToString(a.identity.GetConfigSha256()), InputSHA256: hex.EncodeToString(digest[:]), } + if tool := a.identity.GetToolSha256(); len(tool) > 0 { + request.ToolSHA256 = hex.EncodeToString(tool) + } requestJSON, err := json.Marshal(request) if err != nil { return a.failure(report, roomv1.AnalysisStatus_ANALYSIS_STATUS_FAILED, "request_encoding_failed", digest[:]) diff --git a/internal/analyzer/analyzer_test.go b/internal/analyzer/analyzer_test.go index d879fb5..9549f4f 100644 --- a/internal/analyzer/analyzer_test.go +++ b/internal/analyzer/analyzer_test.go @@ -38,7 +38,7 @@ func TestExternalAnalyzerStampsTrustedIdentityAndArtifact(t *testing.T) { analyzer, err := NewExternal(Config{ ID: "semgrep", Version: "1.2.3", Executable: executable, - Args: []string{"--mode", "room"}, Config: []byte("rules-v4"), CoveredSignals: []roomv1.SignalKind{secretSignal}, + Args: []string{"--mode", "room"}, Config: []byte("rules-v4"), Tool: []byte("semgrep-core-1.139.0"), CoveredSignals: []roomv1.SignalKind{secretSignal}, }) if err != nil { t.Fatal(err) @@ -62,18 +62,25 @@ func TestExternalAnalyzerStampsTrustedIdentityAndArtifact(t *testing.T) { } receipt := report.GetReceipts()[0] wantConfigDigest := sha256.Sum256([]byte("rules-v4")) + wantToolDigest := sha256.Sum256([]byte("semgrep-core-1.139.0")) if receipt.GetAnalyzer().GetId() != "semgrep" || receipt.GetAnalyzer().GetVersion() != "1.2.3" { t.Fatalf("identity = %+v", receipt.GetAnalyzer()) } if string(receipt.GetAnalyzer().GetConfigSha256()) != string(wantConfigDigest[:]) { t.Fatalf("config digest = %x", receipt.GetAnalyzer().GetConfigSha256()) } + if string(receipt.GetAnalyzer().GetToolSha256()) != string(wantToolDigest[:]) { + t.Fatalf("tool digest = %x", receipt.GetAnalyzer().GetToolSha256()) + } if string(receipt.GetInputSha256()) != string(digest[:]) { t.Fatalf("input digest = %x", receipt.GetInputSha256()) } - if got := receipt.GetSignals()[0].GetAnalyzer(); got.GetId() != "semgrep" || string(got.GetConfigSha256()) != string(wantConfigDigest[:]) { + if got := receipt.GetSignals()[0].GetAnalyzer(); got.GetId() != "semgrep" || string(got.GetConfigSha256()) != string(wantConfigDigest[:]) || string(got.GetToolSha256()) != string(wantToolDigest[:]) { t.Fatalf("signal identity = %+v", got) } + if got := analyzer.(*externalAnalyzer).config.Tool; len(got) != 0 { + t.Fatalf("tool bytes retained: %q", got) + } } func TestExternalAnalyzerRejectsInvalidProviderReceipts(t *testing.T) { @@ -205,7 +212,9 @@ func TestExternalAnalyzerUsesJSONStdinAndLiteralArguments(t *testing.T) { } configBytes := []byte("rules-v4") wantConfigDigest := sha256.Sum256(configBytes) - a, err := NewExternal(Config{ID: "boundary", Version: "1", Executable: executable, Args: []string{literalArgument}, Config: configBytes, CoveredSignals: []roomv1.SignalKind{secretSignal}}) + toolBytes := []byte("tool-bytes") + wantToolDigest := sha256.Sum256(toolBytes) + a, err := NewExternal(Config{ID: "boundary", Version: "1", Executable: executable, Args: []string{literalArgument}, Config: configBytes, Tool: toolBytes, CoveredSignals: []roomv1.SignalKind{secretSignal}}) if err != nil { t.Fatal(err) } @@ -235,6 +244,9 @@ func TestExternalAnalyzerUsesJSONStdinAndLiteralArguments(t *testing.T) { if request.ConfigSHA256 != hex.EncodeToString(wantConfigDigest[:]) { t.Fatalf("config digest = %q", request.ConfigSHA256) } + if request.ToolSHA256 != hex.EncodeToString(wantToolDigest[:]) { + t.Fatalf("tool digest = %q", request.ToolSHA256) + } } func TestNewExternalRejectsUnsafeOrIncompleteConfiguration(t *testing.T) { diff --git a/internal/config/config.go b/internal/config/config.go index 04f8a79..90139b1 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -29,6 +29,7 @@ type Config struct { AnalyzerSignals []string AnalyzerSignalsValid bool AnalyzerConfigFile string + AnalyzerToolFile string AnalyzerTimeout time.Duration AnalyzerTimeoutValid bool AuditOnly bool @@ -74,6 +75,7 @@ func Load() Config { AnalyzerSignals: analyzerSignals, AnalyzerSignalsValid: analyzerSignalsValid, AnalyzerConfigFile: strings.TrimSpace(os.Getenv("ROOM_ANALYZER_CONFIG_FILE")), + AnalyzerToolFile: strings.TrimSpace(os.Getenv("ROOM_ANALYZER_TOOL_FILE")), AnalyzerTimeout: analyzerTimeout, AnalyzerTimeoutValid: analyzerTimeoutValid, AuditOnly: auditOnly, diff --git a/internal/review/canonical.go b/internal/review/canonical.go index 98a4723..83c8f54 100644 --- a/internal/review/canonical.go +++ b/internal/review/canonical.go @@ -283,6 +283,11 @@ func canonicalAnalyzer(value *roomv1.AnalyzerIdentity) (*roomv1.AnalyzerIdentity if err := validateDigest(copyValue.GetConfigSha256(), "identity config"); err != nil { return nil, err } + if tool := copyValue.GetToolSha256(); len(tool) != 0 { + if err := validateDigest(tool, "identity tool"); err != nil { + return nil, err + } + } return copyValue, nil } diff --git a/internal/review/compiler_test.go b/internal/review/compiler_test.go index 5e46873..35f8a21 100644 --- a/internal/review/compiler_test.go +++ b/internal/review/compiler_test.go @@ -94,6 +94,7 @@ func TestHypothesisDigestRejectsInvalidAuthorityFields(t *testing.T) { {name: "invalid location", mutate: func(h *roomv1.ReviewHypothesis) { h.AffectedLocations[0].EndLine = 1 }}, {name: "missing producer", mutate: func(h *roomv1.ReviewHypothesis) { h.Producer = nil }}, {name: "short producer config", mutate: func(h *roomv1.ReviewHypothesis) { h.Producer.ConfigSha256 = []byte{1} }}, + {name: "short producer tool", mutate: func(h *roomv1.ReviewHypothesis) { h.Producer.ToolSha256 = []byte{1} }}, {name: "unknown severity", mutate: func(h *roomv1.ReviewHypothesis) { h.Severity = roomv1.Severity(99) }}, {name: "confidence overflow", mutate: func(h *roomv1.ReviewHypothesis) { h.ConfidenceBasisPoints = 10001 }}, {name: "missing timestamp", mutate: func(h *roomv1.ReviewHypothesis) { h.CreatedAt = nil }}, diff --git a/internal/review/registry.go b/internal/review/registry.go index 25b1b60..385b407 100644 --- a/internal/review/registry.go +++ b/internal/review/registry.go @@ -62,6 +62,9 @@ func canonicalVerifier(value *roomv1.ReviewVerifierIdentity, rejectDuplicateCove if len(copyValue.Analyzer.GetConfigSha256()) != sha256.Size { return nil, errors.New("analyzer config digest must be SHA-256") } + if tool := copyValue.Analyzer.GetToolSha256(); len(tool) != 0 && len(tool) != sha256.Size { + return nil, errors.New("analyzer tool digest must be SHA-256") + } if _, ok := roomv1.ReviewVerifierKind_name[int32(copyValue.GetKind())]; !ok || copyValue.GetKind() == roomv1.ReviewVerifierKind_REVIEW_VERIFIER_KIND_UNSPECIFIED { return nil, errors.New("verifier kind is required") } diff --git a/internal/review/registry_test.go b/internal/review/registry_test.go index b4f989e..c231f29 100644 --- a/internal/review/registry_test.go +++ b/internal/review/registry_test.go @@ -25,6 +25,7 @@ func TestRegistryConstruction(t *testing.T) { {name: "missing id", values: []*roomv1.ReviewVerifierIdentity{mutateVerifier(validDeterministic, func(v *roomv1.ReviewVerifierIdentity) { v.Analyzer.Id = "" })}}, {name: "missing version", values: []*roomv1.ReviewVerifierIdentity{mutateVerifier(validDeterministic, func(v *roomv1.ReviewVerifierIdentity) { v.Analyzer.Version = "" })}}, {name: "short config digest", values: []*roomv1.ReviewVerifierIdentity{mutateVerifier(validDeterministic, func(v *roomv1.ReviewVerifierIdentity) { v.Analyzer.ConfigSha256 = []byte{1} })}}, + {name: "short tool digest", values: []*roomv1.ReviewVerifierIdentity{mutateVerifier(validDeterministic, func(v *roomv1.ReviewVerifierIdentity) { v.Analyzer.ToolSha256 = []byte{1} })}}, {name: "unknown kind", values: []*roomv1.ReviewVerifierIdentity{mutateVerifier(validDeterministic, func(v *roomv1.ReviewVerifierIdentity) { v.Kind = roomv1.ReviewVerifierKind(99) })}}, {name: "empty coverage", values: []*roomv1.ReviewVerifierIdentity{mutateVerifier(validDeterministic, func(v *roomv1.ReviewVerifierIdentity) { v.CoveredClaims = nil })}}, {name: "unspecified coverage", values: []*roomv1.ReviewVerifierIdentity{mutateVerifier(validDeterministic, func(v *roomv1.ReviewVerifierIdentity) { diff --git a/proto/room/v1/rules.proto b/proto/room/v1/rules.proto index eb32e4d..5650e73 100644 --- a/proto/room/v1/rules.proto +++ b/proto/room/v1/rules.proto @@ -330,6 +330,7 @@ message AnalyzerIdentity { string id = 1; string version = 2; bytes config_sha256 = 3; + bytes tool_sha256 = 4; } message SecuritySignal {