From b024d139187e7ff63d93ce92e4919b81409172d4 Mon Sep 17 00:00:00 2001 From: Ishavn Date: Tue, 18 Aug 2026 17:31:21 +0300 Subject: [PATCH 1/2] test: enforce adversarial security invariants --- .github/workflows/ci.yml | 3 + internal/adversarial/harness_test.go | 31 ++- .../adversarial/invariant_helpers_test.go | 237 ++++++++++++++++++ testdata/adversarial/manifest.yaml | 6 +- 4 files changed, 271 insertions(+), 6 deletions(-) create mode 100644 internal/adversarial/invariant_helpers_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 64a975a..c36aa00 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,6 +38,9 @@ jobs: - name: Build CLI run: go build ./cmd/agentctl + - name: Build all packages + run: go build ./... + - name: Scan repository run: go run ./cmd/agentctl scan . --format sarif --fail-on high --output agentctl.sarif diff --git a/internal/adversarial/harness_test.go b/internal/adversarial/harness_test.go index 28fd60a..3a6d1df 100644 --- a/internal/adversarial/harness_test.go +++ b/internal/adversarial/harness_test.go @@ -63,6 +63,10 @@ func TestAdversarialCorpus(t *testing.T) { if tc.Description == "" || tc.Root == "" || len(tc.Formats) == 0 { t.Fatal("case requires description, root, and at least one format") } + assertManifestInvariantCoverage(t, tc.SecurityInvariants) + if err := validateSecretInvariant(tc.SecurityInvariants, tc.Secrets); err != nil { + t.Fatal(err) + } if tc.Expected.ExitCode != 0 { t.Fatalf("package-level harness only supports expected exit code 0, got %d", tc.Expected.ExitCode) } @@ -85,6 +89,9 @@ func TestAdversarialCorpus(t *testing.T) { if err != nil { t.Fatal(err) } + if !containsInvariant(tc.SecurityInvariants, "deterministic") { + return + } for i := 0; i < 2; i++ { repeated, err := runAndAssert(t, fixtureRoot, tc, format) if err != nil { @@ -102,10 +109,26 @@ func TestAdversarialCorpus(t *testing.T) { func runAndAssert(t *testing.T, root string, tc testCase, format string) ([]byte, error) { t.Helper() + var before treeSnapshot + var err error + if containsInvariant(tc.SecurityInvariants, "no_filesystem_mutation") { + before, err = snapshotTree(root) + if err != nil { + return nil, fmt.Errorf("snapshot fixture before scan: %w", err) + } + } report, err := scan.Run(root, scan.Options{}) if err != nil { return nil, err } + assertReportInvariants(t, report, tc.SecurityInvariants) + if containsInvariant(tc.SecurityInvariants, "no_filesystem_mutation") { + after, err := snapshotTree(root) + if err != nil { + return nil, fmt.Errorf("snapshot fixture after scan: %w", err) + } + assertTreeUnchanged(t, before, after) + } if len(report.Agents) < tc.Expected.MinAgents { return nil, fmt.Errorf("case %s: expected at least %d agents, got %d", tc.ID, tc.Expected.MinAgents, len(report.Agents)) } @@ -136,9 +159,11 @@ func runAndAssert(t *testing.T, root string, tc testCase, format string) ([]byte if err != nil { return nil, fmt.Errorf("render %s: %w", format, err) } - for _, secret := range tc.Secrets { - if bytes.Contains(output, []byte(secret)) { - return nil, fmt.Errorf("case %s leaked synthetic secret in %s output", tc.ID, format) + if containsInvariant(tc.SecurityInvariants, "no_secret_leak") { + for _, secret := range tc.Secrets { + if bytes.Contains(output, []byte(secret)) { + return nil, fmt.Errorf("case %s leaked synthetic secret in %s output", tc.ID, format) + } } } if containsInvariant(tc.SecurityInvariants, "no_marker_created") { diff --git a/internal/adversarial/invariant_helpers_test.go b/internal/adversarial/invariant_helpers_test.go new file mode 100644 index 0000000..0279ef5 --- /dev/null +++ b/internal/adversarial/invariant_helpers_test.go @@ -0,0 +1,237 @@ +package adversarial + +import ( + "crypto/sha256" + "fmt" + "io/fs" + "os" + "path" + "path/filepath" + "sort" + "strings" + "testing" + + "github.com/HlinorAI/agent-control-plane/internal/scan" +) + +var knownSecurityInvariants = map[string]bool{ + "deterministic": true, + "no_marker_created": true, + "no_secret_leak": true, + "no_filesystem_mutation": true, + "safe_evidence": true, + "severity_integrity": true, +} + +func validateSecurityInvariants(t *testing.T, invariants []string) { + t.Helper() + for _, invariant := range invariants { + if !knownSecurityInvariants[invariant] { + t.Fatalf("unknown security invariant %q", invariant) + } + } +} + +func validateSecretInvariant(invariants, secrets []string) error { + declared := containsInvariant(invariants, "no_secret_leak") + if declared && len(secrets) == 0 { + return fmt.Errorf("no_secret_leak requires at least one secret value") + } + if !declared && len(secrets) > 0 { + return fmt.Errorf("manifest secrets require the no_secret_leak invariant") + } + return nil +} + +func assertReportInvariants(t *testing.T, report scan.Report, invariants []string) { + t.Helper() + if !report.ReadOnly || !report.MetadataOnly { + t.Fatalf("report lost read-only metadata contract: read_only=%t metadata_only=%t", report.ReadOnly, report.MetadataOnly) + } + if containsInvariant(invariants, "safe_evidence") { + for _, readFile := range report.ReadFiles { + assertRelativePath(t, "read file", readFile) + } + for _, source := range report.Sources { + assertRelativePath(t, "source", source.Path) + } + for _, agent := range report.Agents { + assertRelativePath(t, "agent", agent.SourcePath) + } + for _, model := range report.Models { + assertRelativePath(t, "model", model.SourcePath) + } + for _, identity := range report.Identities { + assertRelativePath(t, "identity", identity.SourcePath) + } + for _, server := range report.MCPServers { + assertRelativePath(t, "MCP server", server.SourcePath) + } + for _, relationship := range report.Relationships { + assertRelativePath(t, "relationship evidence", relationship.Evidence.Path) + if relationship.Evidence.Line < 1 { + t.Fatalf("relationship %s has invalid evidence line %d", relationship.ID, relationship.Evidence.Line) + } + } + for _, finding := range report.Findings { + for _, evidence := range finding.Evidence { + assertRelativePath(t, "finding evidence", evidence.Path) + if evidence.Line < 1 { + t.Fatalf("finding %s has invalid evidence line %d", finding.ID, evidence.Line) + } + } + } + } + if containsInvariant(invariants, "severity_integrity") { + for _, agent := range report.Agents { + assertConfidence(t, "agent "+agent.ID, agent.Confidence) + } + for _, relationship := range report.Relationships { + assertConfidence(t, "relationship "+relationship.ID, relationship.Confidence) + } + for _, finding := range report.Findings { + if !knownSeverity(finding.Severity) { + t.Fatalf("finding %s has invalid severity %q", finding.ID, finding.Severity) + } + assertConfidence(t, "finding "+finding.ID, finding.Confidence) + } + } +} + +func assertRelativePath(t *testing.T, kind, value string) { + t.Helper() + if err := validateRelativePath(value); err != nil { + t.Fatalf("%s path is not safe: %q: %v", kind, value, err) + } +} + +func assertConfidence(t *testing.T, kind string, value float64) { + t.Helper() + if err := validateConfidence(value); err != nil { + t.Fatalf("%s confidence is outside [0,1]: %v: %v", kind, value, err) + } +} + +func validateRelativePath(value string) error { + clean := path.Clean(filepath.ToSlash(value)) + if value == "" { + return fmt.Errorf("path is empty") + } + if path.IsAbs(clean) || clean == "." || clean == ".." || strings.HasPrefix(clean, "../") { + return fmt.Errorf("path escapes the scan root") + } + return nil +} + +func validateConfidence(value float64) error { + if value < 0 || value > 1 { + return fmt.Errorf("value must be between 0 and 1") + } + return nil +} + +func knownSeverity(value string) bool { + switch value { + case "Critical", "High", "Medium", "Note": + return true + default: + return false + } +} + +type treeSnapshot map[string]string + +func snapshotTree(root string) (treeSnapshot, error) { + snapshot := treeSnapshot{} + err := filepath.WalkDir(root, func(filePath string, entry fs.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if entry.IsDir() { + return nil + } + if !entry.Type().IsRegular() { + return fmt.Errorf("unsupported fixture entry %s", filePath) + } + data, err := os.ReadFile(filePath) + if err != nil { + return err + } + info, err := entry.Info() + if err != nil { + return err + } + digest := sha256.Sum256(data) + relative, err := filepath.Rel(root, filePath) + if err != nil { + return err + } + snapshot[filepath.ToSlash(relative)] = fmt.Sprintf("%o:%d:%x", info.Mode().Perm(), info.Size(), digest) + return nil + }) + return snapshot, err +} + +func assertTreeUnchanged(t *testing.T, before, after treeSnapshot) { + t.Helper() + if err := compareSnapshots(before, after); err != nil { + t.Fatal(err) + } +} + +func compareSnapshots(before, after treeSnapshot) error { + if len(before) != len(after) { + return fmt.Errorf("scanner mutated fixture file set: before=%d after=%d", len(before), len(after)) + } + keys := make([]string, 0, len(before)) + for key := range before { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + if before[key] != after[key] { + return fmt.Errorf("scanner mutated fixture file %s: before=%s after=%s", key, before[key], after[key]) + } + } + return nil +} + +func assertManifestInvariantCoverage(t *testing.T, invariants []string) { + t.Helper() + validateSecurityInvariants(t, invariants) + for _, wanted := range []string{"deterministic", "no_filesystem_mutation", "safe_evidence", "severity_integrity"} { + if !containsInvariant(invariants, wanted) { + t.Fatalf("manifest case does not declare required invariant %q", wanted) + } + } +} + +func TestInvariantHelpersRejectUnsafeValues(t *testing.T) { + for _, value := range []string{"", "/tmp/secret", "../outside", "a/../../outside"} { + if err := validateRelativePath(value); err == nil { + t.Errorf("validateRelativePath(%q) accepted unsafe path", value) + } + } + for _, value := range []float64{-0.01, 1.01} { + if err := validateConfidence(value); err == nil { + t.Errorf("validateConfidence(%v) accepted out-of-range value", value) + } + } + for _, value := range []string{"Critical", "High", "Medium", "Note"} { + if !knownSeverity(value) { + t.Errorf("knownSeverity(%q) rejected supported value", value) + } + } + if knownSeverity("") || knownSeverity("Low") { + t.Fatal("knownSeverity accepted unsupported severity") + } + if err := compareSnapshots(treeSnapshot{"a": "one"}, treeSnapshot{"a": "two"}); err == nil { + t.Fatal("compareSnapshots accepted changed file content") + } + if err := validateSecretInvariant([]string{"no_secret_leak"}, nil); err == nil { + t.Fatal("validateSecretInvariant accepted a secret invariant without secrets") + } + if err := validateSecretInvariant(nil, []string{"synthetic-secret"}); err == nil { + t.Fatal("validateSecretInvariant accepted undeclared secrets") + } +} diff --git a/testdata/adversarial/manifest.yaml b/testdata/adversarial/manifest.yaml index 361bbdb..0a82162 100644 --- a/testdata/adversarial/manifest.yaml +++ b/testdata/adversarial/manifest.yaml @@ -10,7 +10,7 @@ "min_agents": 3, "min_sources": 6 }, - "security_invariants": ["deterministic"], + "security_invariants": ["deterministic", "no_filesystem_mutation", "safe_evidence", "severity_integrity"], "formats": ["json", "sarif", "text"] }, { @@ -23,7 +23,7 @@ "min_agents": 2, "min_sources": 2 }, - "security_invariants": ["deterministic"], + "security_invariants": ["deterministic", "no_filesystem_mutation", "safe_evidence", "severity_integrity"], "formats": ["json", "sarif"] }, { @@ -37,7 +37,7 @@ "min_sources": 1 }, "secrets": ["prod-api-token-SYNTHETIC-NOT-REAL"], - "security_invariants": ["no_secret_leak", "no_marker_created", "deterministic"], + "security_invariants": ["no_secret_leak", "no_marker_created", "deterministic", "no_filesystem_mutation", "safe_evidence", "severity_integrity"], "formats": ["json", "sarif", "text"] } ] From d8cd73e138974347458382dd62005aa8c6cf2e6a Mon Sep 17 00:00:00 2001 From: Ishavn Date: Wed, 19 Aug 2026 15:29:37 +0300 Subject: [PATCH 2/2] feat: add auditable finding suppressions --- .agentctl/suppressions.example.json | 9 ++ README.md | 32 ++++++ cmd/agentctl/main.go | 142 ++++++++++++++++++++++- cmd/agentctl/main_test.go | 169 ++++++++++++++++++++++++++++ internal/scan/model.go | 35 ++++-- internal/scan/sarif.go | 44 +++++--- internal/scan/scan.go | 77 ++++++++++--- internal/scan/scan_test.go | 67 +++++++++++ 8 files changed, 534 insertions(+), 41 deletions(-) create mode 100644 .agentctl/suppressions.example.json diff --git a/.agentctl/suppressions.example.json b/.agentctl/suppressions.example.json new file mode 100644 index 0000000..be338cf --- /dev/null +++ b/.agentctl/suppressions.example.json @@ -0,0 +1,9 @@ +{ + "suppressions": [ + { + "finding_id": "finding_0123456789abcdef", + "reason": "Accepted until the permission migration is complete", + "expires_at": "2026-12-31T23:59:59Z" + } + ] +} diff --git a/README.md b/README.md index 3d80688..68ffecd 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,38 @@ The baseline matches stable finding IDs. It does not weaken the scanner rules; it only removes findings already present in the supplied report from the current output. Review and update the baseline deliberately. +For a time-bound exception, use a separate suppression document. Every entry +requires a stable finding ID, a reviewable reason, and an RFC 3339 expiry: + +```bash +go run ./cmd/agentctl scan ./workspace --format sarif \ + --suppressions .agentctl/suppressions.json --fail-on high --output agentctl.sarif +``` + +Example document: + +```json +{ + "suppressions": [ + { + "finding_id": "finding_0123456789abcdef", + "reason": "Accepted until the permission migration is complete", + "expires_at": "2026-12-31T23:59:59Z" + } + ] +} +``` + +Suppressed findings remain visible in JSON, text, and SARIF with their reason +and expiry; they are excluded from `--fail-on`. SARIF uses the standard +`result.suppressions` field with `kind: external`. Unknown finding IDs, +unknown JSON fields, a misspelled top-level key, expired, duplicate, or +incomplete entries fail closed. Finding IDs are deterministic opaque IDs tied +to the source path/entity and rule, so renaming a source file or changing the +rule context requires reviewing the suppression again. Use `--debug` to print +safe discovery decisions to stderr while keeping JSON and SARIF on stdout or +the selected output path. + Output paths may be relative or absolute. The parent directory must already exist, output directories are rejected, final output symlinks are rejected, and reports are written atomically with mode `0600`. diff --git a/cmd/agentctl/main.go b/cmd/agentctl/main.go index 27e662d..2fc1e8b 100644 --- a/cmd/agentctl/main.go +++ b/cmd/agentctl/main.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "strings" + "time" "github.com/HlinorAI/agent-control-plane/internal/config" "github.com/HlinorAI/agent-control-plane/internal/scan" @@ -26,10 +27,12 @@ Usage: Scan flags: --baseline file suppress findings already present in a JSON report --config file workspace policy file + --debug print safe discovery decisions to stderr --dry-run list approved files without parsing content --fail-on severity fail when findings meet severity: none, low, medium, high, critical --format format output format: text, json or sarif --output file write the report to a file instead of stdout + --suppressions file suppress active findings with reason and expiry from a JSON file The scanner is read-only and metadata-only. It does not execute scanned content. ` @@ -81,6 +84,8 @@ func run(args []string, stdout, stderr io.Writer) error { output := fs.String("output", "", "write the report to a file instead of stdout") baseline := fs.String("baseline", "", "suppress findings already present in a JSON report") configFile := fs.String("config", "", "workspace policy file; defaults to /.agentctl/config.yaml when present") + debug := fs.Bool("debug", false, "print safe discovery decisions to stderr") + suppressions := fs.String("suppressions", "", "suppress active findings with reason and expiry from a JSON file") if err := fs.Parse(args[2:]); err != nil { return err } @@ -96,7 +101,18 @@ func run(args []string, stdout, stderr io.Writer) error { if err != nil { return err } - report, err := scan.Run(root, scan.Options{DryRun: *dryRun, ConfigPath: policyPath}) + suppressionPath := "" + if *suppressions != "" { + suppressionPath, err = resolveWorkspaceFile(root, *suppressions, "suppressions") + if err != nil { + return err + } + } + options := scan.Options{DryRun: *dryRun, ConfigPath: policyPath, SuppressionPath: suppressionPath} + if *debug { + options.DebugWriter = stderr + } + report, err := scan.Run(root, options) if err != nil { return err } @@ -105,6 +121,11 @@ func run(args []string, stdout, stderr io.Writer) error { return err } } + if *suppressions != "" { + if err := applySuppressions(&report, suppressionPath, time.Now()); err != nil { + return err + } + } if *output != "" { var payload []byte @@ -195,6 +216,96 @@ func applyBaseline(report *scan.Report, path string) error { return nil } +const maxSuppressionReasonLength = 500 + +type suppressionEntry struct { + FindingID string `json:"finding_id"` + Reason string `json:"reason"` + ExpiresAt string `json:"expires_at"` +} + +type suppressionDocument struct { + Suppressions *[]suppressionEntry `json:"suppressions"` +} + +func applySuppressions(report *scan.Report, path string, now time.Time) error { + if report == nil { + return errors.New("suppressions require a scan report") + } + suppressionPath, err := filepath.Abs(path) + if err != nil { + return fmt.Errorf("resolve suppressions path: %w", err) + } + file, err := os.Open(suppressionPath) + if err != nil { + return fmt.Errorf("open suppressions: %w", err) + } + defer file.Close() + + var document suppressionDocument + decoder := json.NewDecoder(file) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&document); err != nil { + return fmt.Errorf("decode suppressions: %w", err) + } + var trailing json.RawMessage + if err := decoder.Decode(&trailing); err != io.EOF { + if err == nil { + return errors.New("decode suppressions: multiple JSON values are not allowed") + } + return fmt.Errorf("decode suppressions: trailing data: %w", err) + } + if document.Suppressions == nil { + return errors.New("suppression document must contain a suppressions array") + } + if len(*document.Suppressions) == 0 { + return errors.New("suppression document must contain at least one suppression") + } + knownFindings := make(map[string]struct{}, len(report.Findings)) + for _, finding := range report.Findings { + knownFindings[finding.ID] = struct{}{} + } + active := make(map[string]suppressionEntry, len(*document.Suppressions)) + for index, entry := range *document.Suppressions { + id := strings.TrimSpace(entry.FindingID) + reason := strings.TrimSpace(entry.Reason) + if id == "" { + return fmt.Errorf("suppression %d has no finding_id", index+1) + } + if reason == "" { + return fmt.Errorf("suppression %q has no reason", id) + } + if len(reason) > maxSuppressionReasonLength { + return fmt.Errorf("suppression %q reason exceeds %d characters", id, maxSuppressionReasonLength) + } + expiresAt, err := time.Parse(time.RFC3339, strings.TrimSpace(entry.ExpiresAt)) + if err != nil { + return fmt.Errorf("suppression %q has invalid expires_at: %w", id, err) + } + if !expiresAt.After(now) { + return fmt.Errorf("suppression %q is expired", id) + } + if _, exists := knownFindings[id]; !exists { + return fmt.Errorf("suppression %q does not match a finding in this scan", id) + } + if _, exists := active[id]; exists { + return fmt.Errorf("duplicate suppression for %q", id) + } + entry.FindingID = id + entry.Reason = reason + entry.ExpiresAt = expiresAt.UTC().Format(time.RFC3339) + active[id] = entry + } + for index := range report.Findings { + if entry, exists := active[report.Findings[index].ID]; exists { + report.Findings[index].Suppressed = true + report.Findings[index].SuppressionReason = entry.Reason + report.Findings[index].SuppressionExpiresAt = entry.ExpiresAt + } + } + return nil +} + func validSeverity(value string) bool { switch value { case "none", "low", "medium", "high", "critical": @@ -210,6 +321,9 @@ func findingsMeetThreshold(findings []scan.Finding, threshold string) bool { return false } for _, finding := range findings { + if finding.Suppressed { + continue + } if severityRank(strings.ToLower(finding.Severity)) >= minimum { return true } @@ -293,6 +407,32 @@ func resolveConfigPath(root, requested string) (string, error) { return absPath, nil } +func resolveWorkspaceFile(root, requested, label string) (string, error) { + absRoot, err := filepath.Abs(root) + if err != nil { + return "", fmt.Errorf("resolve scan root: %w", err) + } + absPath, err := filepath.Abs(requested) + if err != nil { + return "", fmt.Errorf("resolve %s path: %w", label, err) + } + rel, err := filepath.Rel(absRoot, absPath) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("%s must stay inside the scan root", label) + } + info, err := os.Lstat(absPath) + if err != nil { + return "", fmt.Errorf("stat %s: %w", label, err) + } + if info.Mode()&os.ModeSymlink != 0 { + return "", fmt.Errorf("%s path must not be a symlink", label) + } + if !info.Mode().IsRegular() { + return "", fmt.Errorf("%s path must be a regular file", label) + } + return absPath, nil +} + func writeOutputFile(path string, payload []byte) error { outputPath, err := resolveOutputPath(path) if err != nil { diff --git a/cmd/agentctl/main_test.go b/cmd/agentctl/main_test.go index 0f47577..573de85 100644 --- a/cmd/agentctl/main_test.go +++ b/cmd/agentctl/main_test.go @@ -2,10 +2,13 @@ package main import ( "bytes" + "encoding/json" "os" "path/filepath" "strings" "testing" + + "github.com/HlinorAI/agent-control-plane/internal/scan" ) func TestRunUsesProvidedScanPath(t *testing.T) { @@ -90,6 +93,172 @@ func TestRunBaselineSuppressesKnownFindings(t *testing.T) { } } +func TestRunSuppressionsRequireReasonAndExpiry(t *testing.T) { + root := t.TempDir() + agentPath := filepath.Join(root, "agents", "demo_agent.py") + if err := os.MkdirAll(filepath.Dir(agentPath), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(agentPath, []byte("name: demo-agent\nmodel: openai/gpt-4o\nenvironment: production\ndisable_path: runbook/disable-agent\n"), 0o600); err != nil { + t.Fatal(err) + } + var reportOutput bytes.Buffer + var errors bytes.Buffer + if err := run([]string{"scan", root, "--format", "json"}, &reportOutput, &errors); err != nil { + t.Fatal(err) + } + var report scan.Report + if err := json.Unmarshal(reportOutput.Bytes(), &report); err != nil { + t.Fatal(err) + } + if len(report.Findings) == 0 { + t.Fatal("demo report has no finding to suppress") + } + selectedID := "" + for _, finding := range report.Findings { + if finding.Severity == "High" { + selectedID = finding.ID + break + } + } + if selectedID == "" { + t.Fatal("demo report has no high finding to suppress") + } + suppressionPath := filepath.Join(root, "agents", "agents-suppressions.json") + if err := os.MkdirAll(filepath.Dir(suppressionPath), 0o700); err != nil { + t.Fatal(err) + } + payload := `{"suppressions":[{"finding_id":"` + selectedID + `","reason":"accepted until remediation","expires_at":"2099-01-01T00:00:00Z"}]}` + if err := os.WriteFile(suppressionPath, []byte(payload), 0o600); err != nil { + t.Fatal(err) + } + var output bytes.Buffer + if err := run([]string{"scan", root, "--format", "json", "--suppressions", suppressionPath}, &output, &errors); err != nil { + t.Fatal(err) + } + var suppressed scan.Report + if err := json.Unmarshal(output.Bytes(), &suppressed); err != nil { + t.Fatal(err) + } + if len(suppressed.Findings) != len(report.Findings) { + t.Fatalf("suppressed findings must remain visible, before=%d after=%d", len(report.Findings), len(suppressed.Findings)) + } + for _, path := range suppressed.ReadFiles { + if path == "agents/agents-suppressions.json" { + t.Fatalf("suppression file must stay out of discovery read_files: %+v", suppressed.ReadFiles) + } + } + var suppressedFinding *scan.Finding + for index := range suppressed.Findings { + if suppressed.Findings[index].ID == selectedID { + suppressedFinding = &suppressed.Findings[index] + break + } + } + if suppressedFinding == nil || !suppressedFinding.Suppressed || suppressedFinding.SuppressionReason != "accepted until remediation" || suppressedFinding.SuppressionExpiresAt != "2099-01-01T00:00:00Z" { + t.Fatalf("suppression metadata was not preserved: %+v", suppressedFinding) + } + var sarifOutput bytes.Buffer + if err := run([]string{"scan", root, "--format", "sarif", "--suppressions", suppressionPath, "--fail-on", "high"}, &sarifOutput, &errors); err != nil { + t.Fatalf("suppressed high finding should not fail threshold: %v", err) + } + if !strings.Contains(sarifOutput.String(), `"suppressions"`) || !strings.Contains(sarifOutput.String(), `"kind": "external"`) || !strings.Contains(sarifOutput.String(), "accepted until remediation") { + t.Fatalf("SARIF did not preserve suppression metadata: %s", sarifOutput.String()) + } + var textOutput bytes.Buffer + if err := run([]string{"scan", root, "--format", "text", "--suppressions", suppressionPath}, &textOutput, &errors); err != nil { + t.Fatal(err) + } + if !strings.Contains(textOutput.String(), "(suppressed)") || !strings.Contains(textOutput.String(), "accepted until remediation") { + t.Fatalf("text output did not preserve suppression metadata: %s", textOutput.String()) + } +} + +func TestRunRejectsInvalidSuppression(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, "suppressions.json") + if err := os.WriteFile(path, []byte(`{"suppressions":[{"finding_id":"finding-1","reason":"","expires_at":"2099-01-01T00:00:00Z"}]}`), 0o600); err != nil { + t.Fatal(err) + } + var output bytes.Buffer + var errors bytes.Buffer + err := run([]string{"scan", root, "--suppressions", path}, &output, &errors) + if err == nil || !strings.Contains(err.Error(), "has no reason") { + t.Fatalf("expected suppression validation error, got %v", err) + } +} + +func TestRunRejectsSuppressionOutsideScanRoot(t *testing.T) { + root := t.TempDir() + path := filepath.Join(t.TempDir(), "suppressions.json") + if err := os.WriteFile(path, []byte(`{"suppressions":[]}`), 0o600); err != nil { + t.Fatal(err) + } + var output bytes.Buffer + var errors bytes.Buffer + err := run([]string{"scan", root, "--suppressions", path}, &output, &errors) + if err == nil || !strings.Contains(err.Error(), "must stay inside the scan root") { + t.Fatalf("expected suppression boundary error, got %v", err) + } +} + +func TestRunRejectsExpiredSuppression(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, "suppressions.json") + if err := os.WriteFile(path, []byte(`{"suppressions":[{"finding_id":"finding-1","reason":"temporary exception","expires_at":"2020-01-01T00:00:00Z"}]}`), 0o600); err != nil { + t.Fatal(err) + } + var output bytes.Buffer + var errors bytes.Buffer + err := run([]string{"scan", root, "--suppressions", path}, &output, &errors) + if err == nil || !strings.Contains(err.Error(), "is expired") { + t.Fatalf("expected expired suppression error, got %v", err) + } +} + +func TestRunRejectsUnknownSuppressionFieldsAndFindingIDs(t *testing.T) { + tests := []struct { + name string + payload string + want string + }{ + {name: "unknown field", payload: `{"suppressions":[{"finding_id":"finding-1","reason":"temporary exception","expires_at":"2099-01-01T00:00:00Z","approved_by":"security"}]}`, want: "unknown field"}, + {name: "unknown finding", payload: `{"suppressions":[{"finding_id":"finding-1","reason":"temporary exception","expires_at":"2099-01-01T00:00:00Z"}]}`, want: "does not match a finding"}, + {name: "misspelled top level", payload: `{"supressions":[]}`, want: "unknown field"}, + {name: "empty list", payload: "{\"suppressions\":[]}", want: "at least one suppression"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, "suppressions.json") + if err := os.WriteFile(path, []byte(test.payload), 0o600); err != nil { + t.Fatal(err) + } + var output bytes.Buffer + var errors bytes.Buffer + err := run([]string{"scan", root, "--suppressions", path}, &output, &errors) + if err == nil || !strings.Contains(err.Error(), test.want) { + t.Fatalf("expected %q, got %v", test.want, err) + } + }) + } +} + +func TestRunDebugWritesSafeDecisionsToStderr(t *testing.T) { + var output bytes.Buffer + var errors bytes.Buffer + if err := run([]string{"scan", "../../testdata/demo", "--format", "json", "--debug"}, &output, &errors); err != nil { + t.Fatal(err) + } + var report scan.Report + if err := json.Unmarshal(output.Bytes(), &report); err != nil { + t.Fatalf("debug output corrupted JSON: %v", err) + } + if !strings.Contains(errors.String(), "decision=") || strings.Contains(errors.String(), "prod-api-token") { + t.Fatalf("debug output is missing a decision or contains sensitive material: %s", errors.String()) + } +} + func TestRunOutputAllowsExplicitPathAndUsesPrivateMode(t *testing.T) { outputPath := filepath.Join(t.TempDir(), "report.json") var output bytes.Buffer diff --git a/internal/scan/model.go b/internal/scan/model.go index dad8b9f..217857d 100644 --- a/internal/scan/model.go +++ b/internal/scan/model.go @@ -2,6 +2,7 @@ package scan import ( "fmt" + "io" "sort" "strings" ) @@ -9,8 +10,10 @@ import ( const schemaVersion = "0.1" type Options struct { - DryRun bool - ConfigPath string + DryRun bool + ConfigPath string + DebugWriter io.Writer + SuppressionPath string } type Report struct { @@ -85,14 +88,17 @@ type Relationship struct { } type Finding struct { - ID string `json:"id"` - RuleID string `json:"rule_id"` - Severity string `json:"severity"` - Message string `json:"message"` - AgentID string `json:"agent_id"` - Confidence float64 `json:"confidence"` - Evidence []Evidence `json:"evidence"` - RemediationHint string `json:"remediation_hint"` + ID string `json:"id"` + RuleID string `json:"rule_id"` + Severity string `json:"severity"` + Message string `json:"message"` + AgentID string `json:"agent_id"` + Confidence float64 `json:"confidence"` + Evidence []Evidence `json:"evidence"` + RemediationHint string `json:"remediation_hint"` + Suppressed bool `json:"suppressed,omitempty"` + SuppressionReason string `json:"suppression_reason,omitempty"` + SuppressionExpiresAt string `json:"suppression_expires_at,omitempty"` } type Evidence struct { @@ -131,7 +137,14 @@ func (r Report) Text() string { if len(r.Findings) > 0 { b.WriteString("\nFindings:\n") for _, finding := range r.Findings { - fmt.Fprintf(&b, "- [%s] %s: %s\n", finding.Severity, finding.RuleID, finding.Message) + status := "" + if finding.Suppressed { + status = " (suppressed)" + } + fmt.Fprintf(&b, "- [%s] %s%s: %s\n", finding.Severity, finding.RuleID, status, finding.Message) + if finding.Suppressed { + fmt.Fprintf(&b, " suppression: %s (expires %s)\n", finding.SuppressionReason, finding.SuppressionExpiresAt) + } for _, evidence := range finding.Evidence { fmt.Fprintf(&b, " evidence: %s:%d\n", evidence.Path, evidence.Line) } diff --git a/internal/scan/sarif.go b/internal/scan/sarif.go index 6318b9c..30cd3fd 100644 --- a/internal/scan/sarif.go +++ b/internal/scan/sarif.go @@ -36,11 +36,17 @@ type sarifRule struct { } type sarifResult struct { - RuleID string `json:"ruleId"` - Level string `json:"level"` - Message sarifMessage `json:"message"` - Locations []sarifLocation `json:"locations,omitempty"` - Properties map[string]any `json:"properties,omitempty"` + RuleID string `json:"ruleId"` + Level string `json:"level"` + Message sarifMessage `json:"message"` + Locations []sarifLocation `json:"locations,omitempty"` + Properties map[string]any `json:"properties,omitempty"` + Suppressions []sarifSuppression `json:"suppressions,omitempty"` +} + +type sarifSuppression struct { + Kind string `json:"kind"` + Justification string `json:"justification,omitempty"` } type sarifMessage struct { @@ -95,17 +101,23 @@ func (r Report) SARIF() ([]byte, error) { Region: sarifRegion{StartLine: line}, }}) } - results = append(results, sarifResult{ - RuleID: finding.RuleID, - Level: sarifLevel(finding.Severity), - Message: sarifMessage{Text: finding.Message}, - Locations: locations, - Properties: map[string]any{ - "agent_id": finding.AgentID, - "confidence": finding.Confidence, - "remediation_hint": finding.RemediationHint, - }, - }) + properties := map[string]any{ + "agent_id": finding.AgentID, + "confidence": finding.Confidence, + "remediation_hint": finding.RemediationHint, + } + result := sarifResult{ + RuleID: finding.RuleID, + Level: sarifLevel(finding.Severity), + Message: sarifMessage{Text: finding.Message}, + Locations: locations, + Properties: properties, + } + if finding.Suppressed { + result.Suppressions = []sarifSuppression{{Kind: "external", Justification: finding.SuppressionReason}} + properties["suppression_expires_at"] = finding.SuppressionExpiresAt + } + results = append(results, result) } payload := sarifLog{ diff --git a/internal/scan/scan.go b/internal/scan/scan.go index 77eccbf..ce063a4 100644 --- a/internal/scan/scan.go +++ b/internal/scan/scan.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "io/fs" "os" "path/filepath" @@ -115,6 +116,13 @@ func Run(root string, options Options) (Report, error) { if freshnessDays <= 0 { freshnessDays = 30 } + suppressionPath := "" + if options.SuppressionPath != "" { + suppressionPath, err = filepath.Abs(options.SuppressionPath) + if err != nil { + return Report{}, fmt.Errorf("resolve suppression path: %w", err) + } + } report := Report{ SchemaVersion: schemaVersion, @@ -161,13 +169,17 @@ func Run(root string, options Options) (Report, error) { report.FilesSkipped++ return nil } + if suppressionPath != "" && path == suppressionPath { + report.FilesSkipped++ + return nil + } report.FilesScanned++ report.ReadFiles = append(report.ReadFiles, filepath.ToSlash(rel)) totalBytes += fileInfo.Size() if options.DryRun { return nil } - found, err := inspectFile(path, filepath.ToSlash(rel)) + found, err := inspectFile(path, filepath.ToSlash(rel), options.DebugWriter) if err != nil { report.FilesSkipped++ return nil @@ -421,11 +433,24 @@ func Run(root string, options Options) (Report, error) { return report, nil } -func inspectFile(path, relative string) ([]candidate, error) { +func debugf(writer io.Writer, format string, args ...any) { + if writer == nil { + return + } + fmt.Fprintf(writer, format, args...) +} + +func inspectFile(path, relative string, debugWriter io.Writer) ([]candidate, error) { if ignoredSourceFile(relative) { + debugf(debugWriter, "agentctl debug: path=%s decision=skip reason=ignored_source_file\n", relative) return nil, nil } if candidates, err := inspectJSONMCPMetadata(path, relative); err != nil || candidates != nil { + if err != nil { + debugf(debugWriter, "agentctl debug: path=%s decision=skip reason=mcp_metadata_error\n", relative) + } else { + debugf(debugWriter, "agentctl debug: path=%s decision=accept kind=mcp_metadata candidates=%d\n", relative, len(candidates)) + } return candidates, err } file, err := os.Open(path) @@ -591,6 +616,7 @@ func inspectFile(path, relative string) ([]candidate, error) { return nil, err } if firstSignal == 0 && identity == "" && mcpServer == "" && len(approvedServers) == 0 && len(approvedProviders) == 0 && !runtimeMetadataPath(relative) { + debugf(debugWriter, "agentctl debug: path=%s decision=skip reason=no_discovery_signals\n", relative) return nil, nil } if name == "" { @@ -602,8 +628,15 @@ func inspectFile(path, relative string) ([]candidate, error) { } else if runtimeMetadataPath(relative) { kind = "runtime" } - if kind == "source" && !isLikelyAgentCandidate(relative, name, nameExplicit, modelDeclarationSignal, frameworkSignal, models, identity, mcpServer, environmentExplicit) { - return nil, nil + if kind == "source" { + candidate, reason := agentCandidateDecision(relative, name, nameExplicit, modelDeclarationSignal, frameworkSignal, models, identity, mcpServer, environmentExplicit) + if !candidate { + debugf(debugWriter, "agentctl debug: path=%s decision=skip reason=%s signals=name:%t model:%t framework:%t identity:%t mcp:%t environment:%t\n", relative, reason, nameExplicit, modelDeclarationSignal, frameworkSignal, identity != "", mcpServer != "", environmentExplicit) + return nil, nil + } + debugf(debugWriter, "agentctl debug: path=%s decision=accept kind=agent signals=name:%t model:%t framework:%t identity:%t mcp:%t environment:%t\n", relative, nameExplicit, modelDeclarationSignal, frameworkSignal, identity != "", mcpServer != "", environmentExplicit) + } else { + debugf(debugWriter, "agentctl debug: path=%s decision=accept kind=%s\n", relative, kind) } if firstSignal == 0 { firstSignal = 1 @@ -737,6 +770,8 @@ func isMCPPolicySource(relative string) bool { return strings.Contains(base, "mcp") || strings.Contains(base, "server") || strings.Contains(base, "manifest") } +const permissionSignalWindow = 8 + func permissionScope(readOnlyLine, writeScopeLine int) bool { if readOnlyLine == 0 || writeScopeLine == 0 { return false @@ -745,7 +780,9 @@ func permissionScope(readOnlyLine, writeScopeLine int) bool { if distance < 0 { distance = -distance } - return distance <= 8 + // Keep permission signals local to one component declaration; a wider window + // can correlate unrelated read-only and write scopes in the same file. + return distance <= permissionSignalWindow } func isModelReferenceLine(line string) bool { @@ -810,29 +847,43 @@ func runtimeMetadataPath(relative string) bool { } func isLikelyAgentCandidate(relative, name string, nameExplicit, modelDeclarationSignal, frameworkSignal bool, models []string, identity, mcpServer string, environmentExplicit bool) bool { + candidate, _ := agentCandidateDecision(relative, name, nameExplicit, modelDeclarationSignal, frameworkSignal, models, identity, mcpServer, environmentExplicit) + return candidate +} + +func agentCandidateDecision(relative, name string, nameExplicit, modelDeclarationSignal, frameworkSignal bool, models []string, identity, mcpServer string, environmentExplicit bool) (bool, string) { if isGitHubWorkflowPath(relative) { - return false + return false, "github_workflow" } extension := strings.ToLower(filepath.Ext(relative)) if extension == ".md" || extension == ".json" { - return nameExplicit && (isAgentName(name) || isAgentMetadataPath(relative)) + if nameExplicit && (isAgentName(name) || isAgentMetadataPath(relative)) { + return true, "metadata_signal" + } + return false, "metadata_without_agent_signal" } if nameExplicit && isAgentPath(relative) && isAgentName(name) { - return true + return true, "explicit_agent_name" } if !isAgentPath(relative) || !isAgentEntryFile(relative) { - return false + return false, "path_or_entry_not_agent_like" } if identity != "" || mcpServer != "" || environmentExplicit { - return true + return true, "governance_signal" } if modelDeclarationSignal { - return hasAgentFileMarker(relative) || (nameExplicit && isAgentName(name)) + if hasAgentFileMarker(relative) || (nameExplicit && isAgentName(name)) { + return true, "model_and_agent_signal" + } + return false, "model_without_agent_signal" } if len(models) == 0 { - return false + return false, "no_static_model_signal" + } + if frameworkSignal && hasAgentFileMarker(relative) { + return true, "framework_and_file_signal" } - return frameworkSignal && hasAgentFileMarker(relative) + return false, "model_without_framework_signal" } func isAgentMetadataPath(relative string) bool { diff --git a/internal/scan/scan_test.go b/internal/scan/scan_test.go index b929b87..c630ac7 100644 --- a/internal/scan/scan_test.go +++ b/internal/scan/scan_test.go @@ -2,6 +2,7 @@ package scan import ( "encoding/json" + "fmt" "os" "path/filepath" "strings" @@ -593,6 +594,72 @@ func TestRunDemoFixtureFindsRiskRulesAndCanonicalRelationships(t *testing.T) { } } +func TestRunACP003FindsSharedIdentityAcrossAgents(t *testing.T) { + root := t.TempDir() + for name, identity := range map[string]string{ + "first_agent.py": "shared-role", + "second_agent.py": "shared-role", + "isolated_agent.py": "isolated-role", + } { + path := filepath.Join(root, "agents", name) + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + content := fmt.Sprintf("name: %s\nmodel: openai/gpt-4o\nidentity: %s\n", strings.TrimSuffix(name, filepath.Ext(name)), identity) + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + report, err := Run(root, Options{}) + if err != nil { + t.Fatal(err) + } + count := 0 + for _, finding := range report.Findings { + if finding.RuleID == "ACP-003" { + count++ + } + } + if count != 2 { + t.Fatalf("expected one ACP-003 finding per shared-identity agent, got %d: %+v", count, report.Findings) + } +} + +func TestRunACP009RequiresProductionDisablePath(t *testing.T) { + root := t.TempDir() + files := map[string]string{ + "production_agent.py": "name: production-agent\nmodel: openai/gpt-4o\nenvironment: production\n", + "safe_production_agent.py": "name: safe-production-agent\nmodel: openai/gpt-4o\nenvironment: production\ndisable_path: runbook/disable-agent\n", + } + for name, content := range files { + path := filepath.Join(root, "agents", name) + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + report, err := Run(root, Options{}) + if err != nil { + t.Fatal(err) + } + var productionFinding *Finding + for index := range report.Findings { + finding := &report.Findings[index] + if finding.RuleID != "ACP-009" { + continue + } + if productionFinding != nil { + t.Fatalf("expected only the unprotected production agent to trigger ACP-009: %+v", report.Findings) + } + productionFinding = finding + } + if productionFinding == nil || !strings.Contains(productionFinding.Evidence[0].Path, "production_agent.py") { + t.Fatalf("expected ACP-009 for the production agent without a disable path: %+v", report.Findings) + } +} + func TestRunProducesStableReport(t *testing.T) { root := filepath.Join("..", "..", "testdata", "demo") first, err := Run(root, Options{})