diff --git a/.github/workflows/external-benchmark.yml b/.github/workflows/external-benchmark.yml new file mode 100644 index 0000000..6b610a2 --- /dev/null +++ b/.github/workflows/external-benchmark.yml @@ -0,0 +1,44 @@ +name: External repository precision benchmark + +on: + workflow_dispatch: + schedule: + - cron: '17 3 * * 1' + +permissions: + contents: read + +jobs: + benchmark: + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - name: Check out scanner + uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + cache: false + + - name: Prepare pinned upstream repositories + shell: bash + run: | + set -euo pipefail + root="$RUNNER_TEMP/agent-control-plane-benchmark" + mkdir -p "$root" + jq -r '.cases[] | [.directory, .repository, .ref] | @tsv' testdata/benchmark/manifest.json | + while IFS=$'\t' read -r directory repository ref; do + mkdir -p "$root/$directory" + git -C "$root/$directory" init -q + git -C "$root/$directory" remote add origin "$repository" + git -C "$root/$directory" -c protocol.version=2 fetch --filter=blob:none --depth 1 origin "$ref" + git -C "$root/$directory" checkout --detach FETCH_HEAD + actual=$(git -C "$root/$directory" rev-parse HEAD) + test "$actual" = "$ref" + done + echo "BENCHMARK_ROOT=$root" >> "$GITHUB_ENV" + + - name: Run precision benchmark + run: go run ./cmd/agentctl-benchmark --root "$BENCHMARK_ROOT" diff --git a/README.md b/README.md index d284288..3d80688 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,20 @@ The runtime fuzz targets are standalone safe parser seams. They classify in-memo `fuzzpayloadcheck` runs in strict mode by default. The `-allow-adversarial` option is only for the committed negative-case corpus: it permits expected malformed, duplicate-key, unsafe-URL, control-character, and inert command fixtures while keeping credential-like values, invalid UTF-8, resource-limit violations, unsafe paths, and filesystem errors blocking. +Run the pinned external-repository precision benchmark locally after cloning the +repositories listed in `testdata/benchmark/manifest.json`: + +```bash +go run ./cmd/agentctl-benchmark --root /path/to/benchmark-root +``` + +The benchmark is a regression gate for candidate count, non-policy ACP-005 +findings and code-like metadata values. It reports only aggregate counts; it +does not print source values. The weekly and manually triggered workflow +`.github/workflows/external-benchmark.yml` checks out the exact manifest refs +before running the gate. Thresholds are reviewed baselines, not a claim of +ground-truth precision or recall. + ## What it does The CLI scans an approved local directory in read-only mode and produces text or JSON inventory containing: @@ -78,6 +92,11 @@ The CLI scans an approved local directory in read-only mode and produces text or - SARIF 2.1.0 output for GitHub Code Scanning and other security tooling; - safe `.mcp.json` and `server.json` metadata such as server name, transport and auth method. +ACP-005 is emitted only for MCP policy sources such as `.mcp.json`, +`server.json`, and explicitly named MCP/server/manifest JSON or YAML files. +Source-code references to an MCP server are inventory evidence but do not create +a policy finding by themselves. + The default policy excludes common non-production paths such as tests, examples, samples, tutorials, fixtures, documentation and schemas. Agent definitions under tooling directories such as `.claude` and `.github`, Markdown agent files, and framework library layouts remain scannable when they contain strong agent signals. Use `.agentctl/config.yaml` to add workspace-specific exclusions and approved providers or MCP servers. `agentctl init` never overwrites an existing policy. ## Risk rules diff --git a/cmd/agentctl-benchmark/main.go b/cmd/agentctl-benchmark/main.go new file mode 100644 index 0000000..26fdcfb --- /dev/null +++ b/cmd/agentctl-benchmark/main.go @@ -0,0 +1,172 @@ +package main + +import ( + "encoding/json" + "errors" + "flag" + "fmt" + "io" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/HlinorAI/agent-control-plane/internal/scan" +) + +type manifest struct { + SchemaVersion string `json:"schema_version"` + Cases []bench `json:"cases"` +} + +type bench struct { + ID string `json:"id"` + Directory string `json:"directory"` + Repository string `json:"repository"` + Ref string `json:"ref"` + MinAgents int `json:"min_agents"` + MaxAgents int `json:"max_agents"` +} + +type result struct { + ID string `json:"id"` + Repository string `json:"repository"` + Ref string `json:"ref"` + Agents int `json:"agents"` + Findings int `json:"findings"` + SourceACP005 int `json:"source_acp005"` + CodeExpressionHits int `json:"code_expression_hits,omitempty"` + Pass bool `json:"pass"` + Errors []string `json:"errors,omitempty"` +} + +var benchmarkDottedIdentifierPattern = regexp.MustCompile(`^[a-z_][a-z0-9_]*(?:\.[a-z_][a-z0-9_]*)+$`) +var benchmarkBracketExpressionPattern = regexp.MustCompile(`(?i)\b(required|schema)\s*\[`) + +func main() { + if err := run(os.Args[1:], os.Stdout, os.Stderr); err != nil { + fmt.Fprintln(os.Stderr, "agentctl-benchmark:", err) + os.Exit(1) + } +} + +func run(args []string, stdout, stderr io.Writer) error { + fs := flag.NewFlagSet("agentctl-benchmark", flag.ContinueOnError) + fs.SetOutput(stderr) + root := fs.String("root", "", "directory containing the pinned benchmark repositories") + manifestPath := fs.String("manifest", "testdata/benchmark/manifest.json", "benchmark manifest") + if err := fs.Parse(args); err != nil { + return err + } + if *root == "" { + return errors.New("root is required") + } + data, err := os.ReadFile(*manifestPath) + if err != nil { + return fmt.Errorf("read manifest: %w", err) + } + var input manifest + if err := json.Unmarshal(data, &input); err != nil { + return fmt.Errorf("decode manifest: %w", err) + } + if input.SchemaVersion != "1" || len(input.Cases) == 0 { + return errors.New("unsupported or empty benchmark manifest") + } + + results := make([]result, 0, len(input.Cases)) + allPassed := true + for _, item := range input.Cases { + current := result{ID: item.ID, Repository: item.Repository, Ref: item.Ref} + path := filepath.Join(*root, item.Directory) + if info, err := os.Stat(path); err != nil || !info.IsDir() { + current.Errors = append(current.Errors, "benchmark repository directory is missing") + current.Pass = false + allPassed = false + results = append(results, current) + continue + } + report, err := scan.Run(path, scan.Options{}) + if err != nil { + current.Errors = append(current.Errors, err.Error()) + current.Pass = false + allPassed = false + results = append(results, current) + continue + } + current.Agents = len(report.Agents) + current.Findings = len(report.Findings) + for _, finding := range report.Findings { + if finding.RuleID == "ACP-005" && len(finding.Evidence) > 0 && !isMCPPolicyPath(finding.Evidence[0].Path) { + current.SourceACP005++ + } + } + for _, agent := range report.Agents { + if codeExpression(agent.Name) { + current.CodeExpressionHits++ + } + for _, model := range agent.Models { + if codeExpression(model) { + current.CodeExpressionHits++ + } + } + } + if current.Agents < item.MinAgents { + current.Errors = append(current.Errors, fmt.Sprintf("agent count %d is below minimum %d", current.Agents, item.MinAgents)) + } + if item.MaxAgents > 0 && current.Agents > item.MaxAgents { + current.Errors = append(current.Errors, fmt.Sprintf("agent count %d exceeds maximum %d", current.Agents, item.MaxAgents)) + } + if current.SourceACP005 > 0 { + current.Errors = append(current.Errors, "ACP-005 was emitted for a non-policy source") + } + if current.CodeExpressionHits > 0 { + current.Errors = append(current.Errors, "code expressions were emitted as agent metadata") + } + current.Pass = len(current.Errors) == 0 + if !current.Pass { + allPassed = false + } + results = append(results, current) + } + output := struct { + SchemaVersion string `json:"schema_version"` + Passed bool `json:"passed"` + Results []result `json:"results"` + }{SchemaVersion: "1", Passed: allPassed, Results: results} + encoder := json.NewEncoder(stdout) + encoder.SetIndent("", " ") + if err := encoder.Encode(output); err != nil { + return fmt.Errorf("encode benchmark report: %w", err) + } + if !allPassed { + return errors.New("benchmark thresholds failed") + } + return nil +} + +func isMCPPolicyPath(path string) bool { + base := strings.ToLower(filepath.Base(path)) + if base == ".mcp.json" || base == "server.json" { + return true + } + extension := strings.ToLower(filepath.Ext(base)) + return (extension == ".json" || extension == ".yaml" || extension == ".yml") && (strings.Contains(base, "mcp") || strings.Contains(base, "server") || strings.Contains(base, "manifest")) +} + +func codeExpression(value string) bool { + lower := strings.ToLower(value) + for _, marker := range []string{"(", ")", "[", "]", "|", "select_choice", "removeprefix"} { + if strings.Contains(lower, marker) { + return true + } + } + if benchmarkBracketExpressionPattern.MatchString(value) || benchmarkDottedIdentifierPattern.MatchString(lower) { + return true + } + switch lower { + case "str", "model", "name", "bool", "int", "model_name": + return true + default: + return false + } +} diff --git a/cmd/agentctl-benchmark/main_test.go b/cmd/agentctl-benchmark/main_test.go new file mode 100644 index 0000000..f6f756e --- /dev/null +++ b/cmd/agentctl-benchmark/main_test.go @@ -0,0 +1,66 @@ +package main + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "testing" +) + +func TestRunBenchmarkPassesPinnedFixture(t *testing.T) { + root := t.TempDir() + repo := filepath.Join(root, "fixture") + if err := os.MkdirAll(filepath.Join(repo, "agents"), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(repo, "agents", "demo_agent.py"), []byte("name: demo-agent\nmodel: openai\n"), 0o600); err != nil { + t.Fatal(err) + } + manifestPath := filepath.Join(t.TempDir(), "manifest.json") + manifest := `{"schema_version":"1","cases":[{"id":"fixture","directory":"fixture","repository":"local","ref":"test","min_agents":1,"max_agents":1}]}` + if err := os.WriteFile(manifestPath, []byte(manifest), 0o600); err != nil { + t.Fatal(err) + } + var output bytes.Buffer + if err := run([]string{"--root", root, "--manifest", manifestPath}, &output, &bytes.Buffer{}); err != nil { + t.Fatal(err) + } + var report struct { + Passed bool `json:"passed"` + } + if err := json.Unmarshal(output.Bytes(), &report); err != nil { + t.Fatal(err) + } + if !report.Passed { + t.Fatalf("benchmark did not pass: %s", output.String()) + } +} + +func TestRunBenchmarkRejectsMissingRepository(t *testing.T) { + manifestPath := filepath.Join(t.TempDir(), "manifest.json") + manifest := `{"schema_version":"1","cases":[{"id":"missing","directory":"missing","repository":"local","ref":"test","min_agents":1,"max_agents":1}]}` + if err := os.WriteFile(manifestPath, []byte(manifest), 0o600); err != nil { + t.Fatal(err) + } + var output bytes.Buffer + if err := run([]string{"--root", t.TempDir(), "--manifest", manifestPath}, &output, &bytes.Buffer{}); err == nil { + t.Fatal("expected missing repository failure") + } + if !bytes.Contains(output.Bytes(), []byte(`"passed": false`)) { + t.Fatalf("benchmark report did not record failure: %s", output.String()) + } +} + +func TestCodeExpressionClassification(t *testing.T) { + for _, value := range []string{"self.model", "args.model", "str", "model_name", "Required[str]", "schema[\"name\"]"} { + if !codeExpression(value) { + t.Errorf("expected code expression: %q", value) + } + } + for _, value := range []string{"schema-sync-agent", "openai/gpt-4o", "claude-3-5-sonnet"} { + if codeExpression(value) { + t.Errorf("expected metadata value: %q", value) + } + } +} diff --git a/internal/scan/scan.go b/internal/scan/scan.go index e1d5771..77eccbf 100644 --- a/internal/scan/scan.go +++ b/internal/scan/scan.go @@ -2,6 +2,7 @@ package scan import ( "bufio" + "bytes" "crypto/sha256" "encoding/json" "errors" @@ -50,6 +51,9 @@ var ( authMethodPattern = regexp.MustCompile(`(?im)^\s*["']?(auth[_ -]?method|authentication)["']?\s*[:=]`) staticNamePattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]{0,80}$`) staticNameYAMLPattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9 ._-]{0,80}$`) + staticModelPattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._:/@-]{0,120}$`) + codeExpressionPattern = regexp.MustCompile(`(?i)\b(getmodel|select_choice|removeprefix|field)\s*\(|\brequired\s*\[|schema\s*\[`) + dottedIdentifierPattern = regexp.MustCompile(`^[a-z_][a-z0-9_]*(?:\.[a-z_][a-z0-9_]*)+$`) ) type candidate struct { @@ -153,6 +157,10 @@ func Run(root string, options Options) (Report, error) { report.FilesSkipped++ return nil } + if isAgentctlReport(path, filepath.ToSlash(rel)) { + report.FilesSkipped++ + return nil + } report.FilesScanned++ report.ReadFiles = append(report.ReadFiles, filepath.ToSlash(rel)) totalBytes += fileInfo.Size() @@ -374,7 +382,7 @@ func Run(root string, options Options) (Report, error) { } } for _, server := range report.MCPServers { - if server.Approved { + if server.Approved || !isMCPPolicySource(server.SourcePath) { continue } evidence := mcpEvidence[strings.ToLower(server.Name)] @@ -434,9 +442,10 @@ func inspectFile(path, relative string) ([]candidate, error) { environmentExplicit := false nameExplicit := false var mcpTransport, mcpAuthMethod, verifiedAt string - identityLine, mcpLine, permissionLine := 0, 0, 0 + identityLine, mcpLine := 0, 0 modelLine, credentialLine, sensitiveLine, disableLine, verifiedLine := 0, 0, 0, 0, 0 - readOnly, writeScope := false, false + readOnly := false + readOnlyLine, writeScopeLine := 0, 0 productionCredential, sensitiveTool, approvalMetadata, disablePath := false, false, false, false approvedServers, approvedProviders, mcpTools := []string{}, []string{}, []string{} firstSignal := 0 @@ -474,20 +483,29 @@ func inspectFile(path, relative string) ([]candidate, error) { // Never copy or emit the line. The scanner only retains safe metadata. continue } - if name == "" && declarationMatch(line, namePattern) { - name = declarationValue(line, namePattern) + if name == "" && !isGitHubWorkflowPath(relative) && declarationMatch(line, namePattern) { + value := declarationValue(line, namePattern) + if value != "unknown" { + name = value + } nameExplicit = name != "" && name != "unknown" && staticNameDeclaration(line) if firstSignal == 0 && nameExplicit && isAgentPath(relative) && isAgentName(name) { firstSignal = lineNumber } } if identity == "" && declarationMatch(line, identityPattern) { - identity = declarationValue(line, identityPattern) - identityLine = lineNumber + value := declarationValue(line, identityPattern) + if value != "unknown" { + identity = value + identityLine = lineNumber + } } if mcpServer == "" && declarationMatch(line, mcpPattern) { - mcpServer = declarationValue(line, mcpPattern) - mcpLine = lineNumber + value := declarationValue(line, mcpPattern) + if value != "unknown" { + mcpServer = value + mcpLine = lineNumber + } } if mcpServer != "" && mcpTransport == "" && declarationMatch(line, transportPattern) { mcpTransport = declarationValue(line, transportPattern) @@ -497,10 +515,14 @@ func inspectFile(path, relative string) ([]candidate, error) { } if readOnlyPattern.MatchString(line) { readOnly = true + if readOnlyLine == 0 { + readOnlyLine = lineNumber + } } if writePattern.MatchString(line) && (declarationMatch(line, toolPattern) || declarationMatch(line, permissionDeclaration)) { - writeScope = true - permissionLine = lineNumber + if writeScopeLine == 0 { + writeScopeLine = lineNumber + } } if strings.Contains(strings.ToLower(relative), "approved") && strings.Contains(strings.ToLower(relative), "mcp") { if match := listItemPattern.FindStringSubmatch(line); len(match) == 2 { @@ -515,18 +537,20 @@ func inspectFile(path, relative string) ([]candidate, error) { if mcpServer != "" && declarationMatch(line, toolPattern) { mcpTools = appendUnique(mcpTools, toolLabel(line)) } - if firstSignal == 0 && (frameworkPattern.MatchString(line) || modelPattern.MatchString(line) || declarationMatch(line, modelDeclaration) || declarationMatch(line, toolPattern) || sensitiveTool) { + if firstSignal == 0 && (frameworkPattern.MatchString(line) || isModelReferenceLine(line) || declarationMatch(line, modelDeclaration) || declarationMatch(line, toolPattern) || sensitiveTool) { firstSignal = lineNumber } if declarationMatch(line, modelDeclaration) { - modelDeclarationSignal = true if value := declarationValue(line, modelDeclaration); value != "" && value != "unknown" { - models = appendUnique(models, value) - if modelLine == 0 { - modelLine = lineNumber + modelDeclarationSignal = true + if staticModelValue(value) { + models = appendUnique(models, value) + if modelLine == 0 { + modelLine = lineNumber + } } } - } else if modelPattern.MatchString(line) { + } else if isModelReferenceLine(line) { models = appendUnique(models, modelLabel(line)) if modelLine == 0 { modelLine = lineNumber @@ -539,18 +563,24 @@ func inspectFile(path, relative string) ([]candidate, error) { tools = appendUnique(tools, toolLabel(line)) } if owner == "" && declarationMatch(line, ownerPattern) { - owner = declarationValue(line, ownerPattern) + value := declarationValue(line, ownerPattern) + if value != "unknown" { + owner = value + } } if environment == "" && declarationMatch(line, environmentPattern) { - environmentExplicit = true - value := strings.ToLower(declarationValue(line, environmentPattern)) - switch value { - case "prod": - environment = "production" - case "dev": - environment = "development" - default: - environment = value + value := declarationValue(line, environmentPattern) + if value != "unknown" { + environmentExplicit = true + value = strings.ToLower(value) + switch value { + case "prod": + environment = "production" + case "dev": + environment = "development" + default: + environment = value + } } } if environment == "" && productionComment.MatchString(line) { @@ -582,8 +612,8 @@ func inspectFile(path, relative string) ([]candidate, error) { path: relative, line: firstSignal, name: name, kind: kind, models: models, tools: tools, identity: identity, identityLine: identityLine, mcpServer: mcpServer, mcpLine: mcpLine, mcpTransport: mcpTransport, mcpAuthMethod: mcpAuthMethod, mcpTools: mcpTools, - owner: owner, environment: environment, environmentExplicit: environmentExplicit, nameExplicit: nameExplicit, readOnly: readOnly, writeScope: writeScope, - permissionLine: permissionLine, approvedServers: approvedServers, approvedProviders: approvedProviders, + owner: owner, environment: environment, environmentExplicit: environmentExplicit, nameExplicit: nameExplicit, readOnly: readOnly, writeScope: permissionScope(readOnlyLine, writeScopeLine), + permissionLine: writeScopeLine, approvedServers: approvedServers, approvedProviders: approvedProviders, modelLine: modelLine, productionCredential: productionCredential, credentialLine: credentialLine, sensitiveTool: sensitiveTool, sensitiveLine: sensitiveLine, approvalMetadata: approvalMetadata, disablePath: disablePath, disableLine: disableLine, verifiedAt: verifiedAt, verifiedLine: verifiedLine, @@ -688,20 +718,40 @@ func lineForValue(data []byte, value string) int { return 1 } needle := []byte(value) - for index := 0; index+len(needle) <= len(data); index++ { - if string(data[index:index+len(needle)]) == value { - line := 1 - for _, current := range data[:index] { - if current == '\n' { - line++ - } - } - return line - } + index := bytes.Index(data, needle) + if index >= 0 { + return 1 + bytes.Count(data[:index], []byte{'\n'}) } return 1 } +func isMCPPolicySource(relative string) bool { + base := strings.ToLower(filepath.Base(relative)) + if base == ".mcp.json" || base == "server.json" { + return true + } + extension := strings.ToLower(filepath.Ext(base)) + if extension != ".json" && extension != ".yaml" && extension != ".yml" { + return false + } + return strings.Contains(base, "mcp") || strings.Contains(base, "server") || strings.Contains(base, "manifest") +} + +func permissionScope(readOnlyLine, writeScopeLine int) bool { + if readOnlyLine == 0 || writeScopeLine == 0 { + return false + } + distance := readOnlyLine - writeScopeLine + if distance < 0 { + distance = -distance + } + return distance <= 8 +} + +func isModelReferenceLine(line string) bool { + return modelPattern.MatchString(line) && !codeExpressionPattern.MatchString(line) +} + func supportedFile(path, name string) bool { if strings.EqualFold(name, "Dockerfile") || strings.EqualFold(name, "Makefile") { return true @@ -725,7 +775,26 @@ func ignoredDirectory(name string) bool { func ignoredSourceFile(relative string) bool { base := strings.ToLower(filepath.Base(relative)) - return strings.HasSuffix(base, "_test.go") || strings.Contains(base, ".test.") || strings.Contains(base, ".spec.") || strings.Contains(base, ".stories.") + return base == "dependabot.yml" || base == "dependabot.yaml" || strings.HasSuffix(base, "_test.go") || strings.Contains(base, ".test.") || strings.Contains(base, ".spec.") || strings.Contains(base, ".stories.") +} + +func isAgentctlReport(path, relative string) bool { + if strings.ToLower(filepath.Ext(relative)) != ".json" || filepath.Dir(relative) != "." { + return false + } + data, err := os.ReadFile(path) + if err != nil { + return false + } + var header struct { + SchemaVersion string `json:"schema_version"` + ReadOnly *bool `json:"read_only"` + MetadataOnly *bool `json:"metadata_only"` + } + if err := json.Unmarshal(data, &header); err != nil { + return false + } + return header.SchemaVersion == schemaVersion && header.ReadOnly != nil && header.MetadataOnly != nil } func runtimeMetadataPath(relative string) bool { @@ -741,9 +810,13 @@ func runtimeMetadataPath(relative string) bool { } func isLikelyAgentCandidate(relative, name string, nameExplicit, modelDeclarationSignal, frameworkSignal bool, models []string, identity, mcpServer string, environmentExplicit bool) bool { - if isGitHubWorkflowPath(relative) && identity == "" && mcpServer == "" && !environmentExplicit && len(models) == 0 { + if isGitHubWorkflowPath(relative) { return false } + extension := strings.ToLower(filepath.Ext(relative)) + if extension == ".md" || extension == ".json" { + return nameExplicit && (isAgentName(name) || isAgentMetadataPath(relative)) + } if nameExplicit && isAgentPath(relative) && isAgentName(name) { return true } @@ -753,7 +826,28 @@ func isLikelyAgentCandidate(relative, name string, nameExplicit, modelDeclaratio if identity != "" || mcpServer != "" || environmentExplicit { return true } - return len(models) > 0 && (modelDeclarationSignal || frameworkSignal) + if modelDeclarationSignal { + return hasAgentFileMarker(relative) || (nameExplicit && isAgentName(name)) + } + if len(models) == 0 { + return false + } + return frameworkSignal && hasAgentFileMarker(relative) +} + +func isAgentMetadataPath(relative string) bool { + path := strings.ToLower(filepath.ToSlash(relative)) + return strings.HasPrefix(path, "agents/") || strings.HasPrefix(path, ".claude/agents/") || strings.HasPrefix(path, ".github/agents/") || strings.Contains(path, "/.claude/agents/") || strings.Contains(path, "/.github/agents/") || strings.Contains(path, "/agents/") +} + +func hasAgentFileMarker(relative string) bool { + base := strings.TrimSuffix(strings.ToLower(filepath.Base(relative)), filepath.Ext(relative)) + for _, marker := range []string{"agent", "assistant", "copilot", "chatbot", "bot", "worker", "runner", "researcher"} { + if strings.Contains(base, marker) { + return true + } + } + return false } func isGitHubWorkflowPath(relative string) bool { @@ -788,7 +882,7 @@ func staticNameDeclaration(line string) bool { func isAgentEntryFile(relative string) bool { base := strings.TrimSuffix(strings.ToLower(filepath.Base(relative)), filepath.Ext(relative)) - for _, marker := range []string{"agent", "assistant", "copilot", "chatbot", "bot", "worker", "runner"} { + for _, marker := range []string{"agent", "assistant", "copilot", "chatbot", "bot", "worker", "runner", "researcher"} { if strings.Contains(base, marker) { return true } @@ -813,7 +907,7 @@ func isAgentName(name string) bool { func isAgentPath(relative string) bool { for _, part := range strings.Split(strings.ToLower(filepath.ToSlash(relative)), "/") { - for _, marker := range []string{"agent", "assistant", "copilot", "chatbot", "bot", "crew", "autogen", "langgraph"} { + for _, marker := range []string{"agent", "assistant", "copilot", "chatbot", "bot", "crew", "autogen", "langgraph", "researcher"} { if strings.Contains(part, marker) { return true } @@ -850,9 +944,65 @@ func declarationValue(line string, pattern *regexp.Regexp) string { if value == "" || secretPattern.MatchString(value) { return "unknown" } + if pattern == namePattern && !staticNameValue(line, value) { + return "unknown" + } + if pattern == mcpPattern && codeExpressionPattern.MatchString(value) { + return "unknown" + } return value } +func staticNameValue(line, value string) bool { + if !staticDeclarationValue(line, value) || strings.HasSuffix(strings.TrimSpace(value), ".") { + return false + } + if dottedIdentifierPattern.MatchString(strings.ToLower(value)) { + return false + } + switch strings.ToLower(value) { + case "str", "model", "name", "bool", "int", "model_name": + return false + } + upper := strings.ToUpper(value) + if value == upper && (strings.HasSuffix(upper, "_API_KEY") || strings.HasSuffix(upper, "_TOKEN") || strings.HasSuffix(upper, "_SECRET") || strings.HasSuffix(upper, "_CREDENTIAL")) { + return false + } + return true +} + +func staticDeclarationValue(line, value string) bool { + if value == "" || len(value) > 160 || strings.ContainsAny(value, "()[]{}|=<>") { + return false + } + trimmed := strings.TrimSpace(line) + separator := ":" + if !strings.Contains(trimmed, ":") { + separator = "=" + } + parts := strings.SplitN(trimmed, separator, 2) + if len(parts) != 2 { + return false + } + right := strings.TrimSpace(parts[1]) + if len(right) >= 2 && strings.ContainsRune("`\"'", rune(right[0])) { + return true + } + return separator == ":" && (staticNameYAMLPattern.MatchString(value) || staticModelPattern.MatchString(value)) +} + +func staticModelValue(value string) bool { + if !staticModelPattern.MatchString(value) || codeExpressionPattern.MatchString(value) || dottedIdentifierPattern.MatchString(strings.ToLower(value)) { + return false + } + switch strings.ToLower(value) { + case "str", "model", "name", "bool", "int", "model_name": + return false + default: + return true + } +} + func declarationMatch(line string, pattern *regexp.Regexp) bool { return pattern.MatchString(line) && !strings.Contains(strings.TrimSpace(line), ":=") } diff --git a/internal/scan/scan_test.go b/internal/scan/scan_test.go index 48e6d0f..b929b87 100644 --- a/internal/scan/scan_test.go +++ b/internal/scan/scan_test.go @@ -210,6 +210,187 @@ func TestRunCollectsAllMCPServersAndAppliesApprovalRule(t *testing.T) { } } +func TestRunDoesNotApplyACP005ToSourceCodeMCPReferences(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, "agents", "source_agent.py") + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + content := "name: source-agent\nmodel: openai\nmcp_server: internal-tool\n" + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + report, err := Run(root, Options{}) + if err != nil { + t.Fatal(err) + } + if len(report.MCPServers) != 1 { + t.Fatalf("expected source MCP reference in inventory: %+v", report.MCPServers) + } + for _, finding := range report.Findings { + if finding.RuleID == "ACP-005" { + t.Fatalf("source-code MCP reference produced ACP-005: %+v", finding) + } + } +} + +func TestRunRejectsCodeExpressionMetadata(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, "crewai", "mcp", "filters.py") + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + content := `from crewai import Agent +name: str | None = Field(default=None) +model: Required[str] +server_name = (item.get("name") or "").removeprefix("models/") +provider = "Provider name for error messages." +` + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + report, err := Run(root, Options{}) + if err != nil { + t.Fatal(err) + } + if len(report.Agents) != 0 || len(report.Findings) != 0 { + t.Fatalf("code expressions were inventoried as agent metadata: %+v", report) + } +} + +func TestRunKeepsDynamicModelDeclarationSignal(t *testing.T) { + root := t.TempDir() + files := map[string]string{ + "deep_researcher.py": "model: configurable.research_model\n", + "apps/api/src/controllers/v2/agent.ts": "model: req.body.model\n", + "agents/_assistant_agent.py": "name: str\nmodel: openai\n", + "agents/anthropic_agent.py": "name: chunk.content_block.name\nmodel: claude\n", + "agents/openai_agent.py": "name: str | None = Field(default=None)\nmodel: openai\n", + "agents/provider_agent.py": "name: Provider name for error messages.\nmodel: openai\n", + } + for relative, content := range files { + path := filepath.Join(root, relative) + 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) + } + if len(report.Agents) != len(files) { + t.Fatalf("dynamic model declarations or explicit agent files were not preserved: %+v", report.Agents) + } + for _, agent := range report.Agents { + if len(agent.Models) == 0 && agent.SourcePath == "deep_researcher.py" { + continue + } + if agent.SourcePath == "apps/api/src/controllers/v2/agent.ts" && len(agent.Models) == 0 { + continue + } + if agent.Name == "str" || agent.Name == "chunk.content_block.name" || agent.Name == "Provider name for error messages." { + t.Fatalf("code expression was emitted as agent name: %+v", agent) + } + } +} + +func TestRunIgnoresItsOwnJSONReport(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, "agents", "billing.yaml") + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("name: billing-agent\nmodel: openai\nidentity: svc-crm\nowner: payments-team\n"), 0o600); err != nil { + t.Fatal(err) + } + first, err := Run(root, Options{}) + if err != nil { + t.Fatal(err) + } + data, err := json.Marshal(first) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "report.json"), data, 0o600); err != nil { + t.Fatal(err) + } + second, err := Run(root, Options{}) + if err != nil { + t.Fatal(err) + } + if len(second.Agents) != 1 || len(second.Findings) != 0 { + t.Fatalf("agentctl report was inventoried: agents=%+v findings=%+v", second.Agents, second.Findings) + } +} + +func TestRunKeepsOwnerValuesWithParentheses(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, "agents", "owner_agent.py") + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + content := "name: owner-agent\nmodel: openai\nowner: payments-team (oncall)\n" + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + report, err := Run(root, Options{}) + if err != nil { + t.Fatal(err) + } + if len(report.Agents) != 1 || report.Agents[0].Owner != "payments-team (oncall)" { + t.Fatalf("owner value was filtered: %+v", report.Agents) + } + for _, finding := range report.Findings { + if finding.RuleID == "ACP-001" { + t.Fatalf("owner value produced ACP-001: %+v", finding) + } + } +} + +func TestRunDoesNotCombineDistantPermissionSignals(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, "agents", "mixed_agent.py") + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + content := `name: mixed-agent +model: openai +read_only: true + + + + + + + + + +permissions: ["write"] +` + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + report, err := Run(root, Options{}) + if err != nil { + t.Fatal(err) + } + for _, finding := range report.Findings { + if finding.RuleID == "ACP-004" { + t.Fatalf("distant permission signals produced ACP-004: %+v", finding) + } + } +} + +func TestLineForValueUsesByteIndexAndLineCount(t *testing.T) { + data := []byte("first\nsecond\nneedle\n") + if line := lineForValue(data, "needle"); line != 3 { + t.Fatalf("expected line 3, got %d", line) + } +} + func TestRunDiscoversAgentMarkdownUnderToolingDirectories(t *testing.T) { root := t.TempDir() files := map[string]string{ @@ -253,6 +434,30 @@ func TestRunDoesNotInventoryGenericGitHubWorkflow(t *testing.T) { } } +func TestRunIgnoresWorkflowJobNamesAndDependabotConfig(t *testing.T) { + root := t.TempDir() + files := map[string]string{ + ".github/workflows/claude-sync.yml": "name: 🔄 Check CLAUDE.md / AGENTS.md Sync\njobs:\n sync:\n runs-on: ubuntu-latest\n env:\n model: claude\n", + ".github/dependabot.yml": "version: 2\nupdates:\n - package-ecosystem: pip\n directory: /\n schedule:\n interval: weekly\n labels: [bot, openai, anthropic, ollama]\n", + } + for relative, content := range files { + path := filepath.Join(root, relative) + 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) + } + if len(report.Agents) != 0 || len(report.Findings) != 0 { + t.Fatalf("workflow or Dependabot metadata was inventoried as an agent: %+v", report) + } +} + func TestRunDetectsDeclarativeAgentRegistryEntry(t *testing.T) { root := t.TempDir() path := filepath.Join(root, "registry", "agents", "ticket-triage-agent.yaml") @@ -364,7 +569,7 @@ func TestRunDemoFixtureFindsRiskRulesAndCanonicalRelationships(t *testing.T) { t.Fatalf("demo fixture did not produce %s; findings=%+v", ruleID, report.Findings) } } - if len(report.Agents) != 3 || len(report.Sources) != 6 || len(report.Models) != 3 { + if len(report.Agents) != 3 || len(report.Sources) != 7 || len(report.Models) != 3 { t.Fatalf("unexpected canonical inventory sizes: agents=%d sources=%d models=%d", len(report.Agents), len(report.Sources), len(report.Models)) } if len(report.Identities) != 2 || report.Identities[0].Name != "shared-crm-role" && report.Identities[1].Name != "shared-crm-role" { diff --git a/testdata/benchmark/manifest.json b/testdata/benchmark/manifest.json new file mode 100644 index 0000000..58f251d --- /dev/null +++ b/testdata/benchmark/manifest.json @@ -0,0 +1,53 @@ +{ + "schema_version": "1", + "cases": [ + { + "id": "crewai", + "directory": "crewai", + "repository": "https://github.com/crewAIInc/crewAI.git", + "ref": "9b1f4938f02671d2066e336c8bfdcfb2f27c99ee", + "min_agents": 18, + "max_agents": 35 + }, + { + "id": "autogen", + "directory": "autogen", + "repository": "https://github.com/microsoft/autogen.git", + "ref": "027ecf0a379bcc1d09956d46d12d44a3ad9cee14", + "min_agents": 15, + "max_agents": 30 + }, + { + "id": "gpt-researcher", + "directory": "gpt-researcher", + "repository": "https://github.com/assafelovic/gpt-researcher.git", + "ref": "5d84d2f5553e70a2765a8ff3a0d2672d60437ce8", + "min_agents": 3, + "max_agents": 10 + }, + { + "id": "firecrawl", + "directory": "firecrawl", + "repository": "https://github.com/firecrawl/firecrawl.git", + "ref": "b304eff46e1218dafc1c8423f3b0c9f194ab9fb6", + "min_agents": 4, + "max_agents": 12 + }, + { + "id": "open-deep-research", + "directory": "open-deep-research", + "repository": "https://github.com/langchain-ai/open_deep_research.git", + "ref": "1b7d2e80db9faa586165c60e09096dbbfd483a64", + "min_agents": 1, + "max_agents": 5 + }, + { + "id": "langchain", + "directory": "langchain", + "repository": "https://github.com/langchain-ai/langchain.git", + "ref": "82fd04260c15a232af0bc9f4983413f0e63fef64", + "min_agents": 3, + "max_agents": 10 + } + ] +} diff --git a/testdata/demo/.mcp.json b/testdata/demo/.mcp.json new file mode 100644 index 0000000..5e9bd1c --- /dev/null +++ b/testdata/demo/.mcp.json @@ -0,0 +1,8 @@ +{ + "mcpServers": { + "unknown-crm": { + "type": "http", + "url": "https://example.test/mcp" + } + } +} diff --git a/testdata/demo/sample-report.sarif b/testdata/demo/sample-report.sarif index a4e339d..6896d45 100644 --- a/testdata/demo/sample-report.sarif +++ b/testdata/demo/sample-report.sarif @@ -83,6 +83,30 @@ } }, "results": [ + { + "ruleId": "ACP-002", + "level": "error", + "message": { + "text": "Runtime agent has no matching source inventory entry" + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "runtime/agents.json" + }, + "region": { + "startLine": 2 + } + } + } + ], + "properties": { + "agent_id": "runtime-agent_3c7012a6208aa5da", + "confidence": 0.75, + "remediation_hint": "Confirm the source repository and owner, or register this runtime agent explicitly." + } + }, { "ruleId": "ACP-007", "level": "error", @@ -133,7 +157,7 @@ }, { "ruleId": "ACP-001", - "level": "warning", + "level": "note", "message": { "text": "Potential owner gap: no owner/team declaration found in the scanned source" }, @@ -179,30 +203,6 @@ "remediation_hint": "Use an approved provider or update the workspace policy through an explicit review." } }, - { - "ruleId": "ACP-005", - "level": "error", - "message": { - "text": "MCP server is not present in the approved registry" - }, - "locations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": "agents/billing_agent.py" - }, - "region": { - "startLine": 5 - } - } - } - ], - "properties": { - "agent_id": "agent_d4c7feb8ccf186dd", - "confidence": 0.85, - "remediation_hint": "Review the server provenance and add it to the approved registry only after ownership and permission review." - } - }, { "ruleId": "ACP-009", "level": "error", @@ -275,30 +275,6 @@ "remediation_hint": "Use a dedicated least-privilege identity or document an approved shared-identity exception." } }, - { - "ruleId": "ACP-002", - "level": "error", - "message": { - "text": "Runtime agent has no matching source inventory entry" - }, - "locations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": "runtime/agents.json" - }, - "region": { - "startLine": 2 - } - } - } - ], - "properties": { - "agent_id": "runtime-agent_729109f2b322ef45", - "confidence": 0.75, - "remediation_hint": "Confirm the source repository and owner, or register this runtime agent explicitly." - } - }, { "ruleId": "ACP-003", "level": "error", @@ -396,51 +372,51 @@ } }, { - "ruleId": "ACP-005", + "ruleId": "ACP-001", "level": "error", "message": { - "text": "MCP server is not present in the approved registry" + "text": "Potential owner gap: no owner/team declaration found in the scanned source" }, "locations": [ { "physicalLocation": { "artifactLocation": { - "uri": "agents/crm_agent.py" + "uri": "agents/billing_agent.py" }, "region": { - "startLine": 6 + "startLine": 1 } } } ], "properties": { - "agent_id": "agent_57603bf821776970", - "confidence": 0.85, - "remediation_hint": "Review the server provenance and add it to the approved registry only after ownership and permission review." + "agent_id": "agent_d4c7feb8ccf186dd", + "confidence": 0.55, + "remediation_hint": "Confirm an accountable owner or team and record the ownership source." } }, { - "ruleId": "ACP-001", + "ruleId": "ACP-005", "level": "error", "message": { - "text": "Potential owner gap: no owner/team declaration found in the scanned source" + "text": "MCP server is not present in the approved registry" }, "locations": [ { "physicalLocation": { "artifactLocation": { - "uri": "agents/billing_agent.py" + "uri": ".mcp.json" }, "region": { - "startLine": 1 + "startLine": 3 } } } ], "properties": { - "agent_id": "agent_d4c7feb8ccf186dd", - "confidence": 0.55, - "remediation_hint": "Confirm an accountable owner or team and record the ownership source." + "agent_id": "mcp_b02885c87b9ae982", + "confidence": 0.85, + "remediation_hint": "Review the server provenance and add it to the approved registry only after ownership and permission review." } } ]