From ac6435c1040780a485948762e992f3d04394ec71 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sat, 23 May 2026 17:39:30 +0800 Subject: [PATCH 1/5] feat(cli)!: remove mcp-server mode and add scan foundations - Remove the mcp-server and install-mcp-server commands and their package - Drop the now-unused go-sdk dependency and tidy modules - Add the X010 skipped-by-runtime-config code and category mapping - Accept serverUrl as an alias for url when parsing server configs - Introduce a no_mcp config type so empty configs parse without error BREAKING CHANGE: the mcp-server and install-mcp-server commands are removed; run agent-scanner via scan/inspect instead. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 28 -- go.mod | 7 - go.sum | 20 - internal/cli/flags.go | 14 +- internal/cli/install.go | 42 -- internal/cli/mcpserver.go | 67 --- internal/cli/root.go | 2 - internal/discovery/parser.go | 7 + internal/discovery/parser_test.go | 25 +- internal/discovery/parser_testdata_test.go | 18 +- internal/mcpserver/install.go | 140 ------ internal/mcpserver/install_test.go | 198 -------- internal/mcpserver/server.go | 268 ----------- internal/mcpserver/server_test.go | 514 --------------------- internal/models/config.go | 28 +- internal/models/config_test.go | 80 +++- internal/models/errors.go | 4 + internal/models/errors_test.go | 36 ++ internal/models/issue.go | 2 + internal/models/server.go | 19 + 20 files changed, 207 insertions(+), 1312 deletions(-) delete mode 100644 internal/cli/install.go delete mode 100644 internal/cli/mcpserver.go delete mode 100644 internal/mcpserver/install.go delete mode 100644 internal/mcpserver/install_test.go delete mode 100644 internal/mcpserver/server.go delete mode 100644 internal/mcpserver/server_test.go create mode 100644 internal/models/errors_test.go diff --git a/README.md b/README.md index a424b5e..b55554c 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,6 @@ Inspired by [snyk/agent-scan](https://github.com/snyk/agent-scan), reimplemented - **13 security rules** detecting prompt injections, tool shadowing, hardcoded secrets, malicious code, toxic flows, and more - **Skill scanning** for agent skill directories containing `SKILL.md` - **Direct scanning** from package managers (`npm:`, `pypi:`, `oci://`) and URLs (`sse://`, `streamable-http://`) -- **MCP server mode** — run agent-scanner itself as an MCP server with background periodic scanning - **Cross-platform** support (macOS, Linux, Windows) - **Single binary** with zero runtime dependencies @@ -95,33 +94,6 @@ List tools, prompts, and resources without security analysis: agent-scanner inspect ``` -### MCP Server Mode - -Run agent-scanner as an MCP server, exposing `scan` and `get_scan_results` tools: - -```bash -agent-scanner mcp-server -``` - -Run in tool-only mode (no background scanning): - -```bash -agent-scanner mcp-server --tool -``` - -Customize the background scan interval: - -```bash -agent-scanner mcp-server --scan-interval 60 -``` - -Install agent-scanner into Claude Desktop configuration: - -```bash -agent-scanner install-mcp-server -agent-scanner install-mcp-server ~/.config/claude/claude_desktop_config.json -``` - ### Options ```text diff --git a/go.mod b/go.mod index bcf3ae6..68fc010 100644 --- a/go.mod +++ b/go.mod @@ -3,19 +3,12 @@ module github.com/go-authgate/agent-scanner go 1.25.10 require ( - github.com/modelcontextprotocol/go-sdk v1.6.1 github.com/spf13/cobra v1.10.2 github.com/tidwall/jsonc v0.3.3 golang.org/x/sync v0.20.0 ) require ( - github.com/google/jsonschema-go v0.4.3 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect - github.com/segmentio/asm v1.2.1 // indirect - github.com/segmentio/encoding v0.5.4 // indirect github.com/spf13/pflag v1.0.10 // indirect - github.com/yosida95/uritemplate/v3 v3.0.2 // indirect - golang.org/x/oauth2 v0.36.0 // indirect - golang.org/x/sys v0.45.0 // indirect ) diff --git a/go.sum b/go.sum index 661e5e4..50df21b 100644 --- a/go.sum +++ b/go.sum @@ -1,19 +1,7 @@ github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= -github.com/golang-jwt/jwt/v5 v5.3.1 h1:kYf81DTWFe7t+1VvL7eS+jKFVWaUnK9cB1qbwn63YCY= -github.com/golang-jwt/jwt/v5 v5.3.1/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= -github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= -github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= -github.com/google/jsonschema-go v0.4.3 h1:/DBOLZTfDow7pe2GmaJNhltueGTtDKICi8V8p+DQPd0= -github.com/google/jsonschema-go v0.4.3/go.mod h1:r5quNTdLOYEz95Ru18zA0ydNbBuYoo9tgaYcxEYhJVE= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= -github.com/modelcontextprotocol/go-sdk v1.6.1 h1:0zOSupjKUxPKSocPT1Wtago+mUHU2/uZ4xSOY0FGReU= -github.com/modelcontextprotocol/go-sdk v1.6.1/go.mod h1:kzm3kzFL1/+AziGOE0nUs3gvPoNxMCvkxokMkuFapXQ= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/segmentio/asm v1.2.1 h1:DTNbBqs57ioxAD4PrArqftgypG4/qNpXoJx8TVXxPR0= -github.com/segmentio/asm v1.2.1/go.mod h1:BqMnlJP91P8d+4ibuonYZw9mfnzI9HfxselHZr5aAcs= -github.com/segmentio/encoding v0.5.4 h1:OW1VRern8Nw6ITAtwSZ7Idrl3MXCFwXHPgqESYfvNt0= -github.com/segmentio/encoding v0.5.4/go.mod h1:HS1ZKa3kSN32ZHVZ7ZLPLXWvOVIiZtyJnO1gPH1sKt0= github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU= github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4= github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= @@ -21,15 +9,7 @@ github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/tidwall/jsonc v0.3.3 h1:RVQqL3xFfDkKKXIDsrBiVQiEpBtxoKbmMXONb2H/y2w= github.com/tidwall/jsonc v0.3.3/go.mod h1:dw+3CIxqHi+t8eFSpzzMlcVYxKp08UP5CD8/uSFCyJE= -github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zIM+UJPGz4= -github.com/yosida95/uritemplate/v3 v3.0.2/go.mod h1:ILOh0sOhIJR3+L/8afwt/kE++YT040gmv5BQTMR2HP4= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= -golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= -golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= -golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= -golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/tools v0.42.0 h1:uNgphsn75Tdz5Ji2q36v/nsFSfR/9BRFvqhGBaJGd5k= -golang.org/x/tools v0.42.0/go.mod h1:Ma6lCIwGZvHK6XtgbswSoWroEkhugApmsXyrUmBhfr0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/internal/cli/flags.go b/internal/cli/flags.go index 74651a0..8516a19 100644 --- a/internal/cli/flags.go +++ b/internal/cli/flags.go @@ -26,13 +26,6 @@ type ScanFlags struct { ControlIdentifier []string } -// MCPServerFlags holds mcp-server subcommand flags. -type MCPServerFlags struct { - Tool bool - Background bool - ScanInterval int -} - // ClientsFlags holds clients-specific flags. type ClientsFlags struct { CurrentOS bool @@ -40,10 +33,9 @@ type ClientsFlags struct { } var ( - commonFlags CommonFlags - scanFlags ScanFlags - mcpServerFlags MCPServerFlags - clientsFlags ClientsFlags + commonFlags CommonFlags + scanFlags ScanFlags + clientsFlags ClientsFlags ) // addCommonFlags registers flags shared across scan/inspect commands. diff --git a/internal/cli/install.go b/internal/cli/install.go deleted file mode 100644 index a836250..0000000 --- a/internal/cli/install.go +++ /dev/null @@ -1,42 +0,0 @@ -package cli - -import ( - "fmt" - - "github.com/go-authgate/agent-scanner/internal/mcpserver" - "github.com/spf13/cobra" -) - -func newInstallCmd() *cobra.Command { - cmd := &cobra.Command{ - Use: "install-mcp-server [config-file]", - Short: "Install agent-scanner as an MCP server in client configs", - Long: "Adds agent-scanner as a stdio MCP server to the specified client configuration file.", - Args: cobra.MaximumNArgs(1), - RunE: runInstall, - } - return cmd -} - -func runInstall(cmd *cobra.Command, args []string) error { - var configPath string - if len(args) > 0 { - configPath = args[0] - } - - if configPath == "" { - defaultPath, err := mcpserver.DefaultConfigPath() - if err != nil { - return err - } - configPath = defaultPath - cmd.Printf("No config file specified, using default: %s\n", configPath) - } - - if err := mcpserver.InstallServer(configPath); err != nil { - return fmt.Errorf("installation failed: %w", err) - } - - cmd.Printf("Successfully installed agent-scanner as MCP server in %s\n", configPath) - return nil -} diff --git a/internal/cli/mcpserver.go b/internal/cli/mcpserver.go deleted file mode 100644 index 8047063..0000000 --- a/internal/cli/mcpserver.go +++ /dev/null @@ -1,67 +0,0 @@ -package cli - -import ( - "context" - "time" - - "github.com/go-authgate/agent-scanner/internal/analysis" - "github.com/go-authgate/agent-scanner/internal/discovery" - "github.com/go-authgate/agent-scanner/internal/inspect" - "github.com/go-authgate/agent-scanner/internal/mcpclient" - "github.com/go-authgate/agent-scanner/internal/mcpserver" - "github.com/go-authgate/agent-scanner/internal/models" - "github.com/go-authgate/agent-scanner/internal/pipeline" - "github.com/go-authgate/agent-scanner/internal/rules" - "github.com/spf13/cobra" -) - -func newMCPServerCmd() *cobra.Command { - cmd := &cobra.Command{ - Use: "mcp-server", - Short: "Run agent-scanner as an MCP server", - Long: "Starts agent-scanner as an MCP server for continuous background scanning or on-demand tool invocation.", - RunE: runMCPServer, - } - addCommonFlags(cmd) - cmd.Flags(). - BoolVar(&mcpServerFlags.Tool, "tool", false, "Run in tool-only mode (no background scanning)") - cmd.Flags(). - BoolVar(&mcpServerFlags.Background, "background", true, "Enable background periodic scanning") - cmd.Flags(). - IntVar(&mcpServerFlags.ScanInterval, "scan-interval", 30, "Background scan interval in minutes") - return cmd -} - -func runMCPServer(cmd *cobra.Command, _ []string) error { - setupLogging() - - // Build pipeline components - discoverer := discovery.NewDiscoverer() - client := mcpclient.NewClient(commonFlags.SkipSSLVerify) - inspector := inspect.NewInspector(client, commonFlags.ServerTimeout) - ruleEngine := rules.NewDefaultEngine() - analyzer := analysis.NewAnalyzer(commonFlags.AnalysisURL, commonFlags.SkipSSLVerify) - - // Create the scan function closure - scanFn := func(ctx context.Context, paths []string, skills bool) ([]models.ScanPathResult, error) { - p := pipeline.New(pipeline.Config{ - Discoverer: discoverer, - Inspector: inspector, - RuleEngine: ruleEngine, - Analyzer: analyzer, - Paths: paths, - ScanSkills: skills, - ScanAllUsers: commonFlags.ScanAllUsers, - Verbose: commonFlags.Verbose, - }) - return p.Run(ctx) - } - - background := mcpServerFlags.Background && !mcpServerFlags.Tool - - return mcpserver.RunServer(cmd.Context(), mcpserver.ServerConfig{ - ScanFn: scanFn, - Background: background, - ScanInterval: time.Duration(mcpServerFlags.ScanInterval) * time.Minute, - }) -} diff --git a/internal/cli/root.go b/internal/cli/root.go index c6d6f34..846cb15 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -31,8 +31,6 @@ func init() { rootCmd.AddCommand(versionCmd) rootCmd.AddCommand(newScanCmd()) rootCmd.AddCommand(newInspectCmd()) - rootCmd.AddCommand(newMCPServerCmd()) - rootCmd.AddCommand(newInstallCmd()) rootCmd.AddCommand(newClientsCmd()) } diff --git a/internal/discovery/parser.go b/internal/discovery/parser.go index 6bf3681..ed27baf 100644 --- a/internal/discovery/parser.go +++ b/internal/discovery/parser.go @@ -42,6 +42,13 @@ func ParseMCPConfigFile(path string) (models.MCPConfig, error) { } } + // No MCP servers were found. Distinguish a valid config that simply has no + // MCP section (parses clean → empty server set) from data we cannot parse + // at all (genuinely unknown format). + var probe any + if json.Unmarshal(data, &probe) == nil { + return &models.ConfigWithoutMCP{}, nil + } return &models.UnknownMCPConfig{}, nil } diff --git a/internal/discovery/parser_test.go b/internal/discovery/parser_test.go index b823551..f9ea63a 100644 --- a/internal/discovery/parser_test.go +++ b/internal/discovery/parser_test.go @@ -126,7 +126,8 @@ func TestParseMCPConfigFile_JSONC(t *testing.T) { func TestParseMCPConfigFile_Unknown(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "config.json") - content := `{"some_random_key": "value"}` + // Unparseable (not valid JSON) → unknown format. + content := `{not valid json` if err := os.WriteFile(path, []byte(content), 0o644); err != nil { t.Fatal(err) } @@ -141,6 +142,28 @@ func TestParseMCPConfigFile_Unknown(t *testing.T) { } } +func TestParseMCPConfigFile_WithoutMCP(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.json") + // Valid JSON with no MCP section → parses clean as no_mcp, not an error. + content := `{"some_random_key": "value"}` + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + + cfg, err := ParseMCPConfigFile(path) + if err != nil { + t.Fatal(err) + } + + if cfg.ConfigType() != "no_mcp" { + t.Errorf("expected config type 'no_mcp', got %s", cfg.ConfigType()) + } + if len(cfg.GetServers()) != 0 { + t.Errorf("expected 0 servers, got %d", len(cfg.GetServers())) + } +} + func TestParseMCPConfigFile_NotFound(t *testing.T) { _, err := ParseMCPConfigFile("/nonexistent/path.json") if err == nil { diff --git a/internal/discovery/parser_testdata_test.go b/internal/discovery/parser_testdata_test.go index 17a27f3..1eeea53 100644 --- a/internal/discovery/parser_testdata_test.go +++ b/internal/discovery/parser_testdata_test.go @@ -91,8 +91,12 @@ func TestParseMCPConfigFile_Testdata_Empty(t *testing.T) { if err != nil { t.Fatal(err) } - if cfg.ConfigType() != "unknown" { - t.Errorf("expected 'unknown', got %s", cfg.ConfigType()) + // Valid JSON ("{}") with no MCP section parses clean as no_mcp. + if cfg.ConfigType() != "no_mcp" { + t.Errorf("expected 'no_mcp', got %s", cfg.ConfigType()) + } + if len(cfg.GetServers()) != 0 { + t.Errorf("expected 0 servers, got %d", len(cfg.GetServers())) } } @@ -144,8 +148,9 @@ func TestParseMCPConfigFile_Testdata_VSCodeWithoutMCP(t *testing.T) { if err != nil { t.Fatal(err) } - if cfg.ConfigType() != "unknown" { - t.Errorf("expected 'unknown', got %s", cfg.ConfigType()) + // A valid settings.json without an mcp section is no_mcp, not unknown. + if cfg.ConfigType() != "no_mcp" { + t.Errorf("expected 'no_mcp', got %s", cfg.ConfigType()) } } @@ -155,7 +160,8 @@ func TestParseMCPConfigFile_Testdata_VSCodeWithEmptyMCP(t *testing.T) { if err != nil { t.Fatal(err) } - if cfg.ConfigType() != "unknown" { - t.Errorf("expected 'unknown', got %s", cfg.ConfigType()) + // An mcp section present but with no servers parses clean as no_mcp. + if cfg.ConfigType() != "no_mcp" { + t.Errorf("expected 'no_mcp', got %s", cfg.ConfigType()) } } diff --git a/internal/mcpserver/install.go b/internal/mcpserver/install.go deleted file mode 100644 index ca9094d..0000000 --- a/internal/mcpserver/install.go +++ /dev/null @@ -1,140 +0,0 @@ -package mcpserver - -import ( - "encoding/json" - "fmt" - "os" - "path/filepath" - "runtime" - "strings" - - "github.com/tidwall/jsonc" -) - -// DefaultConfigPath returns the default Claude Desktop config path for the current platform. -func DefaultConfigPath() (string, error) { - home, err := os.UserHomeDir() - if err != nil { - return "", fmt.Errorf("unable to determine home directory: %w", err) - } - - switch runtime.GOOS { - case "darwin": - return filepath.Join( - home, - "Library", - "Application Support", - "Claude", - "claude_desktop_config.json", - ), nil - case "windows": - return filepath.Join( - home, - "AppData", - "Roaming", - "Claude", - "claude_desktop_config.json", - ), nil - case "linux": - return filepath.Join(home, ".config", "Claude", "claude_desktop_config.json"), nil - default: - return "", fmt.Errorf("unsupported platform: %s", runtime.GOOS) - } -} - -// mcpServerEntry represents an MCP server entry in a config file. -type mcpServerEntry struct { - Command string `json:"command"` - Args []string `json:"args"` -} - -// InstallServer adds agent-scanner as an MCP server in the specified config file. -// If configPath is empty, it defaults to the Claude Desktop config path. -func InstallServer(configPath string) error { - if configPath == "" { - defaultPath, err := DefaultConfigPath() - if err != nil { - return err - } - configPath = defaultPath - } - - // Expand ~ in path - if strings.HasPrefix(configPath, "~/") { - home, err := os.UserHomeDir() - if err != nil { - return fmt.Errorf("unable to expand home directory: %w", err) - } - configPath = filepath.Join(home, configPath[2:]) - } - - // Find the agent-scanner binary path - binaryPath, err := os.Executable() - if err != nil { - return fmt.Errorf("unable to determine binary path: %w", err) - } - binaryPath, err = filepath.EvalSymlinks(binaryPath) - if err != nil { - return fmt.Errorf("unable to resolve binary path: %w", err) - } - - // Read existing config or start with empty object - var config map[string]any - - data, err := os.ReadFile(configPath) - switch { - case err != nil: - if !os.IsNotExist(err) { - return fmt.Errorf("reading config file: %w", err) - } - config = make(map[string]any) - case strings.TrimSpace(string(data)) == "": - config = make(map[string]any) - default: - if err := json.Unmarshal(jsonc.ToJSON(data), &config); err != nil { - return fmt.Errorf("parsing config file: %w", err) - } - } - - // Get or create mcpServers section - var mcpServers map[string]any - if existing, exists := config["mcpServers"]; !exists { - mcpServers = make(map[string]any) - } else { - var ok bool - mcpServers, ok = existing.(map[string]any) - if !ok { - return fmt.Errorf( - "config key %q has unexpected type %T; expected object", - "mcpServers", - existing, - ) - } - } - - // Add/update agent-scanner entry - mcpServers["agent-scanner"] = mcpServerEntry{ - Command: binaryPath, - Args: []string{"mcp-server"}, - } - config["mcpServers"] = mcpServers - - // Marshal with indentation - output, err := json.MarshalIndent(config, "", " ") - if err != nil { - return fmt.Errorf("marshaling config: %w", err) - } - - // Ensure parent directory exists - dir := filepath.Dir(configPath) - if err := os.MkdirAll(dir, 0o755); err != nil { - return fmt.Errorf("creating config directory: %w", err) - } - - // Write config file - if err := os.WriteFile(configPath, append(output, '\n'), 0o644); err != nil { - return fmt.Errorf("writing config file: %w", err) - } - - return nil -} diff --git a/internal/mcpserver/install_test.go b/internal/mcpserver/install_test.go deleted file mode 100644 index 187f76a..0000000 --- a/internal/mcpserver/install_test.go +++ /dev/null @@ -1,198 +0,0 @@ -package mcpserver - -import ( - "encoding/json" - "os" - "path/filepath" - "runtime" - "testing" -) - -func TestDefaultConfigPath(t *testing.T) { - path, err := DefaultConfigPath() - if err != nil { - t.Fatalf("DefaultConfigPath failed: %v", err) - } - - home, _ := os.UserHomeDir() - - switch runtime.GOOS { - case "darwin": - expected := filepath.Join( - home, - "Library", - "Application Support", - "Claude", - "claude_desktop_config.json", - ) - if path != expected { - t.Errorf("expected %q, got %q", expected, path) - } - case "linux": - expected := filepath.Join(home, ".config", "Claude", "claude_desktop_config.json") - if path != expected { - t.Errorf("expected %q, got %q", expected, path) - } - case "windows": - expected := filepath.Join( - home, - "AppData", - "Roaming", - "Claude", - "claude_desktop_config.json", - ) - if path != expected { - t.Errorf("expected %q, got %q", expected, path) - } - } -} - -func TestInstallServer_NewConfig(t *testing.T) { - tmpDir := t.TempDir() - configPath := filepath.Join(tmpDir, "config.json") - - if err := InstallServer(configPath); err != nil { - t.Fatalf("InstallServer failed: %v", err) - } - - // Verify the file was created - data, err := os.ReadFile(configPath) - if err != nil { - t.Fatalf("reading config file failed: %v", err) - } - - var config map[string]any - if err := json.Unmarshal(data, &config); err != nil { - t.Fatalf("parsing config file failed: %v", err) - } - - mcpServers, ok := config["mcpServers"].(map[string]any) - if !ok { - t.Fatal("expected mcpServers key in config") - } - - entry, ok := mcpServers["agent-scanner"].(map[string]any) - if !ok { - t.Fatal("expected agent-scanner entry in mcpServers") - } - - if _, ok := entry["command"].(string); !ok { - t.Error("expected command field in agent-scanner entry") - } - - args, ok := entry["args"].([]any) - if !ok { - t.Fatal("expected args field in agent-scanner entry") - } - if len(args) != 1 || args[0] != "mcp-server" { - t.Errorf("expected args [\"mcp-server\"], got %v", args) - } -} - -func TestInstallServer_ExistingConfig(t *testing.T) { - tmpDir := t.TempDir() - configPath := filepath.Join(tmpDir, "config.json") - - // Create existing config with another server - existingConfig := map[string]any{ - "mcpServers": map[string]any{ - "existing-server": map[string]any{ - "command": "existing-cmd", - "args": []string{"--existing"}, - }, - }, - "otherKey": "otherValue", - } - data, _ := json.MarshalIndent(existingConfig, "", " ") - if err := os.WriteFile(configPath, data, 0o644); err != nil { - t.Fatalf("writing existing config failed: %v", err) - } - - if err := InstallServer(configPath); err != nil { - t.Fatalf("InstallServer failed: %v", err) - } - - // Verify existing entries are preserved - updatedData, err := os.ReadFile(configPath) - if err != nil { - t.Fatalf("reading updated config failed: %v", err) - } - - var config map[string]any - if err := json.Unmarshal(updatedData, &config); err != nil { - t.Fatalf("parsing updated config failed: %v", err) - } - - // Check other keys are preserved - if config["otherKey"] != "otherValue" { - t.Error("existing config key 'otherKey' was not preserved") - } - - mcpServers := config["mcpServers"].(map[string]any) - - // Check existing server is preserved - if _, ok := mcpServers["existing-server"]; !ok { - t.Error("existing-server entry was not preserved") - } - - // Check agent-scanner was added - if _, ok := mcpServers["agent-scanner"]; !ok { - t.Error("agent-scanner entry was not added") - } -} - -func TestInstallServer_NestedDirectory(t *testing.T) { - tmpDir := t.TempDir() - configPath := filepath.Join(tmpDir, "subdir", "nested", "config.json") - - if err := InstallServer(configPath); err != nil { - t.Fatalf("InstallServer failed for nested path: %v", err) - } - - // Verify the file was created - if _, err := os.Stat(configPath); os.IsNotExist(err) { - t.Error("config file was not created in nested directory") - } -} - -func TestInstallServer_UpdateExistingEntry(t *testing.T) { - tmpDir := t.TempDir() - configPath := filepath.Join(tmpDir, "config.json") - - // Create config with an old agent-scanner entry - existingConfig := map[string]any{ - "mcpServers": map[string]any{ - "agent-scanner": map[string]any{ - "command": "/old/path/agent-scanner", - "args": []string{"mcp-server", "--old-flag"}, - }, - }, - } - data, _ := json.MarshalIndent(existingConfig, "", " ") - if err := os.WriteFile(configPath, data, 0o644); err != nil { - t.Fatalf("writing existing config failed: %v", err) - } - - if err := InstallServer(configPath); err != nil { - t.Fatalf("InstallServer failed: %v", err) - } - - // Verify the entry was updated - updatedData, err := os.ReadFile(configPath) - if err != nil { - t.Fatalf("reading updated config failed: %v", err) - } - - var config map[string]any - if err := json.Unmarshal(updatedData, &config); err != nil { - t.Fatalf("parsing updated config failed: %v", err) - } - - mcpServers := config["mcpServers"].(map[string]any) - entry := mcpServers["agent-scanner"].(map[string]any) - - args := entry["args"].([]any) - if len(args) != 1 || args[0] != "mcp-server" { - t.Errorf("expected updated args [\"mcp-server\"], got %v", args) - } -} diff --git a/internal/mcpserver/server.go b/internal/mcpserver/server.go deleted file mode 100644 index 1e83692..0000000 --- a/internal/mcpserver/server.go +++ /dev/null @@ -1,268 +0,0 @@ -package mcpserver - -import ( - "context" - "encoding/json" - "errors" - "fmt" - "log/slog" - "sync" - "time" - - "github.com/go-authgate/agent-scanner/internal/models" - "github.com/go-authgate/agent-scanner/internal/redact" - "github.com/go-authgate/agent-scanner/internal/version" - "github.com/modelcontextprotocol/go-sdk/mcp" -) - -// ScanFunc is a function that runs the scanner pipeline and returns results. -type ScanFunc func(ctx context.Context, paths []string, skills bool) ([]models.ScanPathResult, error) - -// ServerConfig holds the configuration for the MCP server. -type ServerConfig struct { - ScanFn ScanFunc - Background bool - ScanInterval time.Duration -} - -// ScanState holds the cached scan results and provides thread-safe access. -type ScanState struct { - mu sync.RWMutex - results []models.ScanPathResult -} - -// Set stores scan results in the cache. -func (s *ScanState) Set(results []models.ScanPathResult) { - s.mu.Lock() - defer s.mu.Unlock() - s.results = copyScanResults(results) -} - -// Get retrieves the cached scan results. -func (s *ScanState) Get() []models.ScanPathResult { - s.mu.RLock() - defer s.mu.RUnlock() - return copyScanResults(s.results) -} - -// copyScanResults returns a shallow copy of the provided scan results slice -// to avoid sharing the underlying array between callers and the internal cache. -func copyScanResults(src []models.ScanPathResult) []models.ScanPathResult { - if src == nil { - return nil - } - dst := make([]models.ScanPathResult, len(src)) - copy(dst, src) - return dst -} - -// redactResults returns a deep copy of results with sensitive fields redacted. -func redactResults(results []models.ScanPathResult) []models.ScanPathResult { - redacted := make([]models.ScanPathResult, len(results)) - copy(redacted, results) - for i := range redacted { - redact.ScanPathResult(&redacted[i]) - } - return redacted -} - -// scanInput is the typed input for the scan tool. -type scanInput struct { - Paths []string `json:"paths,omitempty" jsonschema:"optional list of config file paths or directories to scan"` - Skills bool `json:"skills,omitempty" jsonschema:"whether to include skill scanning"` -} - -// scanOutput is the typed output from the scan tool. -type scanOutput struct { - Results []models.ScanPathResult `json:"results"` - Summary scanSummary `json:"summary"` -} - -// scanSummary provides a high-level overview of scan results. -type scanSummary struct { - TotalPaths int `json:"total_paths"` - TotalServers int `json:"total_servers"` - TotalIssues int `json:"total_issues"` - Critical int `json:"critical"` - High int `json:"high"` - Medium int `json:"medium"` - Low int `json:"low"` - Info int `json:"info"` -} - -// getResultsInput is the typed input for the get_scan_results tool (empty). -type getResultsInput struct{} - -// getResultsOutput is the typed output from the get_scan_results tool. -type getResultsOutput struct { - Results []models.ScanPathResult `json:"results"` - Summary scanSummary `json:"summary"` -} - -// buildSummary creates a summary from scan results. -func buildSummary(results []models.ScanPathResult) scanSummary { - summary := scanSummary{ - TotalPaths: len(results), - } - for _, r := range results { - summary.TotalServers += len(r.Servers) - for _, issue := range r.Issues { - summary.TotalIssues++ - switch issue.GetSeverity() { - case models.SeverityCritical: - summary.Critical++ - case models.SeverityHigh: - summary.High++ - case models.SeverityMedium: - summary.Medium++ - case models.SeverityLow: - summary.Low++ - case models.SeverityInfo: - summary.Info++ - } - } - } - return summary -} - -// NewServer creates a configured MCP server with scan and get_scan_results tools. -// It returns the server and the scan state used for caching results. -func NewServer(cfg ServerConfig) (*mcp.Server, *ScanState) { - state := &ScanState{} - - server := mcp.NewServer( - &mcp.Implementation{ - Name: "agent-scanner", - Version: version.Version, - }, - &mcp.ServerOptions{ - Instructions: "Agent Scanner is a security scanner for AI agents, MCP servers, and agent skills. " + - "Use the 'scan' tool to discover and analyze MCP servers for security threats. " + - "Use the 'get_scan_results' tool to retrieve the results of the last scan.", - }, - ) - - // Register scan tool - mcp.AddTool(server, &mcp.Tool{ - Name: "scan", - Description: "Scan MCP servers and agent skills for security issues. Discovers installed AI agent clients, connects to their configured MCP servers, and detects prompt injections, tool poisoning, toxic flows, and other security threats.", - }, func(ctx context.Context, req *mcp.CallToolRequest, input scanInput) (*mcp.CallToolResult, scanOutput, error) { - if cfg.ScanFn == nil { - return nil, scanOutput{}, errors.New("scan function not configured") - } - - results, err := cfg.ScanFn(ctx, input.Paths, input.Skills) - if err != nil { - return nil, scanOutput{}, fmt.Errorf("scan failed: %w", err) - } - - // Redact sensitive data (env vars, headers) before caching and returning - redacted := redactResults(results) - - // Cache the redacted results - state.Set(redacted) - - output := scanOutput{ - Results: redacted, - Summary: buildSummary(redacted), - } - - // Also provide a text summary in the content for easy consumption - jsonBytes, err := json.MarshalIndent(output, "", " ") - if err != nil { - return nil, output, fmt.Errorf("failed to marshal scan results: %w", err) - } - - return &mcp.CallToolResult{ - Content: []mcp.Content{ - &mcp.TextContent{Text: string(jsonBytes)}, - }, - }, output, nil - }) - - // Register get_scan_results tool - mcp.AddTool(server, &mcp.Tool{ - Name: "get_scan_results", - Description: "Get the results of the last security scan. Returns cached results from the most recent scan, or empty results if no scan has been performed yet.", - }, func(_ context.Context, _ *mcp.CallToolRequest, _ getResultsInput) (*mcp.CallToolResult, getResultsOutput, error) { - results := state.Get() - if results == nil { - results = []models.ScanPathResult{} - } - - output := getResultsOutput{ - Results: results, - Summary: buildSummary(results), - } - - jsonBytes, err := json.MarshalIndent(output, "", " ") - if err != nil { - return nil, output, fmt.Errorf("failed to marshal scan results: %w", err) - } - - return &mcp.CallToolResult{ - Content: []mcp.Content{ - &mcp.TextContent{Text: string(jsonBytes)}, - }, - }, output, nil - }) - - return server, state -} - -// RunServer creates and runs the MCP server over stdio. -// The provided context controls the server lifetime and background scanning. -func RunServer(ctx context.Context, cfg ServerConfig) error { - server, state := NewServer(cfg) - - ctx, cancel := context.WithCancel(ctx) - defer cancel() - - // If background scanning is enabled, run initial scan and start periodic scanning - if cfg.Background && cfg.ScanFn != nil { - interval := cfg.ScanInterval - if interval <= 0 { - interval = 30 * time.Minute - } - - // Run initial scan - go func() { - slog.Info("running initial background scan") - results, err := cfg.ScanFn(ctx, nil, false) - if err != nil { - slog.Error("initial background scan failed", "error", err) - return - } - state.Set(redactResults(results)) - slog.Info("initial background scan complete", - "paths", len(results), - ) - }() - - // Start periodic scanning - go func() { - ticker := time.NewTicker(interval) - defer ticker.Stop() - for { - select { - case <-ctx.Done(): - return - case <-ticker.C: - slog.Info("running periodic background scan") - results, err := cfg.ScanFn(ctx, nil, false) - if err != nil { - slog.Error("periodic background scan failed", "error", err) - continue - } - state.Set(redactResults(results)) - slog.Info("periodic background scan complete", - "paths", len(results), - ) - } - } - }() - } - - slog.Info("starting MCP server", "name", "agent-scanner", "version", version.Version) - return server.Run(ctx, &mcp.StdioTransport{}) -} diff --git a/internal/mcpserver/server_test.go b/internal/mcpserver/server_test.go deleted file mode 100644 index 57aa8c8..0000000 --- a/internal/mcpserver/server_test.go +++ /dev/null @@ -1,514 +0,0 @@ -package mcpserver - -import ( - "context" - "encoding/json" - "fmt" - "strings" - "sync" - "sync/atomic" - "testing" - "time" - - "github.com/go-authgate/agent-scanner/internal/models" - "github.com/modelcontextprotocol/go-sdk/mcp" -) - -// mockScanResults returns test scan results. -func mockScanResults() []models.ScanPathResult { - return []models.ScanPathResult{ - { - Client: "test-client", - Path: "/tmp/test-config.json", - Servers: []models.ServerScanResult{ - { - Name: "test-server", - Server: &models.StdioServer{ - Command: "test-cmd", - Args: []string{"--flag"}, - Env: map[string]string{ - "API_KEY": "sk-secret-12345", - }, - }, - Signature: &models.ServerSignature{ - Metadata: models.InitializeResult{ - ServerInfo: models.ServerInfo{ - Name: "test-server", - Version: "1.0.0", - }, - }, - Tools: []models.Tool{ - {Name: "test-tool", Description: "A test tool"}, - }, - }, - }, - }, - Issues: []models.Issue{ - { - Code: models.CodeSuspiciousWords, - Message: "Found suspicious words in tool description", - }, - { - Code: models.CodePromptInjection, - Message: "Prompt injection detected", - }, - }, - }, - } -} - -func TestNewServer_RegistersTools(t *testing.T) { - cfg := ServerConfig{ - ScanFn: func(_ context.Context, _ []string, _ bool) ([]models.ScanPathResult, error) { - return nil, nil - }, - } - - server, _ := NewServer(cfg) - - // Connect a test client to verify tools are registered - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - - client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "v1.0.0"}, nil) - - t1, t2 := mcp.NewInMemoryTransports() - if _, err := server.Connect(ctx, t1, nil); err != nil { - t.Fatalf("server.Connect failed: %v", err) - } - session, err := client.Connect(ctx, t2, nil) - if err != nil { - t.Fatalf("client.Connect failed: %v", err) - } - defer session.Close() - - // List tools - toolsResult, err := session.ListTools(ctx, nil) - if err != nil { - t.Fatalf("ListTools failed: %v", err) - } - - toolNames := make(map[string]bool) - for _, tool := range toolsResult.Tools { - toolNames[tool.Name] = true - } - - if !toolNames["scan"] { - t.Error("expected 'scan' tool to be registered") - } - if !toolNames["get_scan_results"] { - t.Error("expected 'get_scan_results' tool to be registered") - } - if len(toolsResult.Tools) != 2 { - t.Errorf("expected 2 tools, got %d", len(toolsResult.Tools)) - } -} - -func TestScanTool_CallsScanFunc(t *testing.T) { - var scanCalled atomic.Bool - expectedResults := mockScanResults() - - cfg := ServerConfig{ - ScanFn: func(_ context.Context, paths []string, skills bool) ([]models.ScanPathResult, error) { - scanCalled.Store(true) - if len(paths) != 1 || paths[0] != "/tmp/config.json" { - t.Errorf("unexpected paths: %v", paths) - } - if !skills { - t.Error("expected skills=true") - } - return expectedResults, nil - }, - } - - server, _ := NewServer(cfg) - - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - - client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "v1.0.0"}, nil) - - t1, t2 := mcp.NewInMemoryTransports() - if _, err := server.Connect(ctx, t1, nil); err != nil { - t.Fatalf("server.Connect failed: %v", err) - } - session, err := client.Connect(ctx, t2, nil) - if err != nil { - t.Fatalf("client.Connect failed: %v", err) - } - defer session.Close() - - // Call the scan tool - result, err := session.CallTool(ctx, &mcp.CallToolParams{ - Name: "scan", - Arguments: map[string]any{ - "paths": []string{"/tmp/config.json"}, - "skills": true, - }, - }) - if err != nil { - t.Fatalf("CallTool scan failed: %v", err) - } - - if !scanCalled.Load() { - t.Error("scan function was not called") - } - - if result.IsError { - t.Error("expected no error in result") - } - - // Verify the structured content has the expected JSON - if len(result.Content) == 0 { - t.Fatal("expected content in result") - } - - // Parse the text content to verify structure - textContent, ok := result.Content[0].(*mcp.TextContent) - if !ok { - t.Fatalf("expected TextContent, got %T", result.Content[0]) - } - - // Parse into a generic map since ServerConfig is an interface - var output map[string]any - if err := json.Unmarshal([]byte(textContent.Text), &output); err != nil { - t.Fatalf("failed to parse scan output: %v", err) - } - - results, ok := output["results"].([]any) - if !ok { - t.Fatal("expected results array in output") - } - if len(results) != 1 { - t.Errorf("expected 1 result, got %d", len(results)) - } - - summary, ok := output["summary"].(map[string]any) - if !ok { - t.Fatal("expected summary in output") - } - if totalIssues := summary["total_issues"].(float64); totalIssues != 2 { - t.Errorf("expected 2 total issues, got %v", totalIssues) - } - if totalServers := summary["total_servers"].(float64); totalServers != 1 { - t.Errorf("expected 1 total server, got %v", totalServers) - } -} - -func TestGetScanResults_EmptyInitially(t *testing.T) { - cfg := ServerConfig{ - ScanFn: func(_ context.Context, _ []string, _ bool) ([]models.ScanPathResult, error) { - return nil, nil - }, - } - - server, _ := NewServer(cfg) - - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - - client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "v1.0.0"}, nil) - - t1, t2 := mcp.NewInMemoryTransports() - if _, err := server.Connect(ctx, t1, nil); err != nil { - t.Fatalf("server.Connect failed: %v", err) - } - session, err := client.Connect(ctx, t2, nil) - if err != nil { - t.Fatalf("client.Connect failed: %v", err) - } - defer session.Close() - - // Call get_scan_results before any scan - result, err := session.CallTool(ctx, &mcp.CallToolParams{ - Name: "get_scan_results", - }) - if err != nil { - t.Fatalf("CallTool get_scan_results failed: %v", err) - } - - if result.IsError { - t.Error("expected no error in result") - } - - // Parse the content - if len(result.Content) == 0 { - t.Fatal("expected content in result") - } - - textContent, ok := result.Content[0].(*mcp.TextContent) - if !ok { - t.Fatalf("expected TextContent, got %T", result.Content[0]) - } - - var output getResultsOutput - if err := json.Unmarshal([]byte(textContent.Text), &output); err != nil { - t.Fatalf("failed to parse output: %v", err) - } - - if len(output.Results) != 0 { - t.Errorf("expected 0 results initially, got %d", len(output.Results)) - } - if output.Summary.TotalIssues != 0 { - t.Errorf("expected 0 issues initially, got %d", output.Summary.TotalIssues) - } -} - -func TestGetScanResults_ReturnsCachedResults(t *testing.T) { - expectedResults := mockScanResults() - - cfg := ServerConfig{ - ScanFn: func(_ context.Context, _ []string, _ bool) ([]models.ScanPathResult, error) { - return expectedResults, nil - }, - } - - server, _ := NewServer(cfg) - - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - - client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "v1.0.0"}, nil) - - t1, t2 := mcp.NewInMemoryTransports() - if _, err := server.Connect(ctx, t1, nil); err != nil { - t.Fatalf("server.Connect failed: %v", err) - } - session, err := client.Connect(ctx, t2, nil) - if err != nil { - t.Fatalf("client.Connect failed: %v", err) - } - defer session.Close() - - // First, run a scan to populate the cache - _, err = session.CallTool(ctx, &mcp.CallToolParams{ - Name: "scan", - }) - if err != nil { - t.Fatalf("CallTool scan failed: %v", err) - } - - // Now get cached results - result, err := session.CallTool(ctx, &mcp.CallToolParams{ - Name: "get_scan_results", - }) - if err != nil { - t.Fatalf("CallTool get_scan_results failed: %v", err) - } - - textContent, ok := result.Content[0].(*mcp.TextContent) - if !ok { - t.Fatalf("expected TextContent, got %T", result.Content[0]) - } - - var output map[string]any - if err := json.Unmarshal([]byte(textContent.Text), &output); err != nil { - t.Fatalf("failed to parse output: %v", err) - } - - results, ok := output["results"].([]any) - if !ok { - t.Fatal("expected results array in output") - } - if len(results) != 1 { - t.Errorf("expected 1 cached result, got %d", len(results)) - } - - firstResult, ok := results[0].(map[string]any) - if !ok { - t.Fatal("expected first result to be an object") - } - if firstResult["client"] != "test-client" { - t.Errorf("expected client 'test-client', got %v", firstResult["client"]) - } - - summary, ok := output["summary"].(map[string]any) - if !ok { - t.Fatal("expected summary in output") - } - if totalIssues := summary["total_issues"].(float64); totalIssues != 2 { - t.Errorf("expected 2 issues in cached results, got %v", totalIssues) - } -} - -func TestScanTool_RedactsSensitiveData(t *testing.T) { - cfg := ServerConfig{ - ScanFn: func(_ context.Context, _ []string, _ bool) ([]models.ScanPathResult, error) { - return mockScanResults(), nil - }, - } - - server, state := NewServer(cfg) - - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - - client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "v1.0.0"}, nil) - - t1, t2 := mcp.NewInMemoryTransports() - if _, err := server.Connect(ctx, t1, nil); err != nil { - t.Fatalf("server.Connect failed: %v", err) - } - session, err := client.Connect(ctx, t2, nil) - if err != nil { - t.Fatalf("client.Connect failed: %v", err) - } - defer session.Close() - - // Call scan - result, err := session.CallTool(ctx, &mcp.CallToolParams{ - Name: "scan", - }) - if err != nil { - t.Fatalf("CallTool scan failed: %v", err) - } - - // Verify the response JSON has redacted env values - textContent, ok := result.Content[0].(*mcp.TextContent) - if !ok { - t.Fatalf("expected TextContent, got %T", result.Content[0]) - } - - if strings.Contains(textContent.Text, "sk-secret-12345") { - t.Error("scan response should not contain raw API key") - } - - // Verify cached results are also redacted - cached := state.Get() - if len(cached) == 0 { - t.Fatal("expected cached results") - } - stdio, ok := cached[0].Servers[0].Server.(*models.StdioServer) - if !ok { - t.Fatal("expected StdioServer") - } - if v, exists := stdio.Env["API_KEY"]; exists && v == "sk-secret-12345" { - t.Error("cached results should have redacted API_KEY") - } -} - -func TestRunServer_NegativeScanInterval(t *testing.T) { - cfg := ServerConfig{ - ScanFn: func(_ context.Context, _ []string, _ bool) ([]models.ScanPathResult, error) { - return nil, nil - }, - Background: true, - ScanInterval: -1 * time.Second, - } - - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) - defer cancel() - - // RunServer should not panic with negative interval - _ = RunServer(ctx, cfg) -} - -func TestBuildSummary(t *testing.T) { - results := []models.ScanPathResult{ - { - Servers: []models.ServerScanResult{ - {Name: "server-1"}, - {Name: "server-2"}, - }, - Issues: []models.Issue{ - { - Code: models.CodePromptInjection, - Message: "injection", - }, // high - { - Code: models.CodeBehaviorHijack, - Message: "hijack", - }, // critical - { - Code: models.CodeSuspiciousWords, - Message: "suspicious", - }, // medium - { - Code: models.CodeDataLeakFlow, - Message: "leak", - }, // high (TF) - { - Code: models.CodeServerStartup, - Message: "startup", - ExtraData: map[string]any{"severity": "info"}, - }, // info (custom) - }, - }, - { - Servers: []models.ServerScanResult{ - {Name: "server-3"}, - }, - Issues: []models.Issue{ - {Code: models.CodeSkillInjection, Message: "skill injection"}, // critical - }, - }, - } - - summary := buildSummary(results) - - if summary.TotalPaths != 2 { - t.Errorf("expected 2 paths, got %d", summary.TotalPaths) - } - if summary.TotalServers != 3 { - t.Errorf("expected 3 servers, got %d", summary.TotalServers) - } - if summary.TotalIssues != 6 { - t.Errorf("expected 6 issues, got %d", summary.TotalIssues) - } - if summary.Critical != 2 { - t.Errorf("expected 2 critical, got %d", summary.Critical) - } - if summary.High != 2 { - t.Errorf("expected 2 high, got %d", summary.High) - } - if summary.Medium != 1 { - t.Errorf("expected 1 medium, got %d", summary.Medium) - } - if summary.Info != 1 { - t.Errorf("expected 1 info, got %d", summary.Info) - } -} - -func TestScanState_Concurrency(t *testing.T) { - state := &ScanState{} - - // Verify initial state - got := state.Get() - if got != nil { - t.Errorf("expected nil initially, got %v", got) - } - - // Set results - expected := mockScanResults() - state.Set(expected) - - // Verify retrieval - got = state.Get() - if len(got) != len(expected) { - t.Errorf("expected %d results, got %d", len(expected), len(got)) - } - - // Overwrite with empty - state.Set([]models.ScanPathResult{}) - got = state.Get() - if len(got) != 0 { - t.Errorf("expected 0 results after overwrite, got %d", len(got)) - } - - // Exercise concurrent Set/Get to verify thread safety under -race. - var wg sync.WaitGroup - for i := range 10 { - wg.Add(2) - go func(n int) { - defer wg.Done() - state.Set([]models.ScanPathResult{ - {Client: fmt.Sprintf("client-%d", n), Path: "/p"}, - }) - }(i) - go func() { - defer wg.Done() - _ = state.Get() - }() - } - wg.Wait() -} diff --git a/internal/models/config.go b/internal/models/config.go index d2182fd..7f8b5f1 100644 --- a/internal/models/config.go +++ b/internal/models/config.go @@ -92,12 +92,20 @@ func (c *VSCodeMCPConfigFile) GetServers() map[string]ServerConfig { return servers } -// UnknownMCPConfig is a placeholder for unrecognized config formats. +// UnknownMCPConfig is a placeholder for unrecognized or unparseable config data. type UnknownMCPConfig struct{} func (c *UnknownMCPConfig) ConfigType() string { return "unknown" } func (c *UnknownMCPConfig) GetServers() map[string]ServerConfig { return nil } +// ConfigWithoutMCP represents a valid config file that contains no MCP servers +// (e.g. a settings.json without an "mcp" section, or an empty "{}"). It parses +// cleanly and yields an empty server set rather than being treated as an error. +type ConfigWithoutMCP struct{} + +func (c *ConfigWithoutMCP) ConfigType() string { return "no_mcp" } +func (c *ConfigWithoutMCP) GetServers() map[string]ServerConfig { return nil } + // ServerConfigJSON is the raw JSON representation that gets parsed into a typed ServerConfig. type ServerConfigJSON struct { // Common @@ -108,21 +116,27 @@ type ServerConfigJSON struct { Args []string `json:"args,omitempty"` Env map[string]string `json:"env,omitempty"` - // RemoteServer fields - URL string `json:"url,omitempty"` - Headers map[string]string `json:"headers,omitempty"` + // RemoteServer fields. "serverUrl" is accepted as an alias for "url". + URL string `json:"url,omitempty"` + ServerURL string `json:"serverUrl,omitempty"` + Headers map[string]string `json:"headers,omitempty"` } // ToServerConfig converts the raw JSON to a typed ServerConfig. func (s *ServerConfigJSON) ToServerConfig() ServerConfig { - // If URL is present, it's a remote server - if s.URL != "" { + // "serverUrl" is an alias for "url"; prefer the canonical "url". + url := s.URL + if url == "" { + url = s.ServerURL + } + // If a URL is present, it's a remote server + if url != "" { st := ServerType(s.Type) if st == "" { st = ServerTypeHTTP } return &RemoteServer{ - URL: s.URL, + URL: url, Type: st, Headers: s.Headers, } diff --git a/internal/models/config_test.go b/internal/models/config_test.go index 09fcc76..96a5b75 100644 --- a/internal/models/config_test.go +++ b/internal/models/config_test.go @@ -1,6 +1,9 @@ package models -import "testing" +import ( + "encoding/json" + "testing" +) func TestServerConfigJSONToStdioServer(t *testing.T) { raw := ServerConfigJSON{ @@ -44,6 +47,81 @@ func TestServerConfigJSONToRemoteServer(t *testing.T) { } } +func TestServerConfigJSONServerURLAlias(t *testing.T) { + tests := []struct { + name string + raw ServerConfigJSON + wantURL string + }{ + { + name: "serverUrl only", + raw: ServerConfigJSON{ServerURL: "https://alias.example.com/mcp"}, + wantURL: "https://alias.example.com/mcp", + }, + { + name: "url wins over serverUrl", + raw: ServerConfigJSON{ + URL: "https://canonical.example.com", + ServerURL: "https://alias.example.com", + }, + wantURL: "https://canonical.example.com", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := tt.raw.ToServerConfig() + remote, ok := cfg.(*RemoteServer) + if !ok { + t.Fatalf("expected *RemoteServer, got %T", cfg) + } + if remote.URL != tt.wantURL { + t.Errorf("expected URL=%s, got %s", tt.wantURL, remote.URL) + } + }) + } +} + +func TestRemoteServerUnmarshalServerURLAlias(t *testing.T) { + tests := []struct { + name string + data string + wantURL string + }{ + { + `serverUrl only`, + `{"serverUrl":"https://alias.example.com/mcp"}`, + "https://alias.example.com/mcp", + }, + {`url only`, `{"url":"https://canonical.example.com"}`, "https://canonical.example.com"}, + { + `url wins`, + `{"url":"https://canonical.example.com","serverUrl":"https://alias.example.com"}`, + "https://canonical.example.com", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var s RemoteServer + if err := json.Unmarshal([]byte(tt.data), &s); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if s.URL != tt.wantURL { + t.Errorf("expected URL=%s, got %s", tt.wantURL, s.URL) + } + }) + } +} + +func TestConfigWithoutMCP(t *testing.T) { + cfg := ConfigWithoutMCP{} + if cfg.ConfigType() != "no_mcp" { + t.Errorf("expected config type 'no_mcp', got %s", cfg.ConfigType()) + } + if servers := cfg.GetServers(); servers != nil { + t.Error("expected nil servers") + } +} + func TestClaudeConfigFileGetServers(t *testing.T) { cfg := ClaudeConfigFile{ MCPServers: map[string]ServerConfigJSON{ diff --git a/internal/models/errors.go b/internal/models/errors.go index ec41423..ef1d1ad 100644 --- a/internal/models/errors.go +++ b/internal/models/errors.go @@ -11,6 +11,8 @@ const ( ErrCatServerHTTP ErrorCategory = "server_http_error" ErrCatAnalysisError ErrorCategory = "analysis_error" ErrCatSkillScan ErrorCategory = "skill_scan_error" + // ErrCatSkippedByRuntimeConfig marks a server skipped via runtime_config.skip_servers. + ErrCatSkippedByRuntimeConfig ErrorCategory = "skipped_by_runtime_config" ) // ScanError is a structured error from the scan pipeline. @@ -57,6 +59,8 @@ func ErrorToIssueCode(category ErrorCategory) string { return CodeServerHTTPError case ErrCatAnalysisError: return CodeAnalysisError + case ErrCatSkippedByRuntimeConfig: + return CodeSkippedByRuntimeConfig default: return CodeUnknownError } diff --git a/internal/models/errors_test.go b/internal/models/errors_test.go new file mode 100644 index 0000000..bc2fe90 --- /dev/null +++ b/internal/models/errors_test.go @@ -0,0 +1,36 @@ +package models + +import "testing" + +func TestErrorToIssueCode(t *testing.T) { + tests := []struct { + category ErrorCategory + want string + }{ + {ErrCatServerStartup, CodeServerStartup}, + {ErrCatSkillScan, CodeSkillScanError}, + {ErrCatFileNotFound, CodeFileNotFound}, + {ErrCatUnknownConfig, CodeUnknownConfig}, + {ErrCatParseError, CodeParseError}, + {ErrCatServerHTTP, CodeServerHTTPError}, + {ErrCatAnalysisError, CodeAnalysisError}, + {ErrCatSkippedByRuntimeConfig, CodeSkippedByRuntimeConfig}, + {ErrorCategory("something_else"), CodeUnknownError}, + } + for _, tt := range tests { + t.Run(string(tt.category), func(t *testing.T) { + if got := ErrorToIssueCode(tt.category); got != tt.want { + t.Errorf("ErrorToIssueCode(%q) = %s, want %s", tt.category, got, tt.want) + } + }) + } +} + +func TestSkippedByRuntimeConfigCodeValue(t *testing.T) { + if CodeSkippedByRuntimeConfig != "X010" { + t.Errorf("expected X010, got %s", CodeSkippedByRuntimeConfig) + } + if ErrCatSkippedByRuntimeConfig != "skipped_by_runtime_config" { + t.Errorf("expected skipped_by_runtime_config, got %s", ErrCatSkippedByRuntimeConfig) + } +} diff --git a/internal/models/issue.go b/internal/models/issue.go index 30225f9..bae2bd0 100644 --- a/internal/models/issue.go +++ b/internal/models/issue.go @@ -106,4 +106,6 @@ const ( CodeServerHTTPError = "X006" // Server HTTP error CodeAnalysisError = "X007" // Analysis error CodeUnknownError = "X008" // Unknown error + // X009 is reserved. + CodeSkippedByRuntimeConfig = "X010" // Server skipped by runtime config (skip_servers) ) diff --git a/internal/models/server.go b/internal/models/server.go index 63086e4..33ddff5 100644 --- a/internal/models/server.go +++ b/internal/models/server.go @@ -63,6 +63,25 @@ func (s *RemoteServer) MarshalJSON() ([]byte, error) { }) } +// UnmarshalJSON accepts "serverUrl" as an alias for "url" when decoding. +func (s *RemoteServer) UnmarshalJSON(data []byte) error { + type Alias RemoteServer + aux := &struct { + ServerURL string `json:"serverUrl"` + *Alias + }{ + Alias: (*Alias)(s), + } + if err := json.Unmarshal(data, aux); err != nil { + return err + } + // "serverUrl" is an alias; canonical "url" wins when both are present. + if s.URL == "" { + s.URL = aux.ServerURL + } + return nil +} + // SkillServer represents a local skill directory containing SKILL.md. type SkillServer struct { Path string `json:"path"` From 251fedc51961d29861178c1830b7c3104b05a737 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sat, 23 May 2026 17:52:03 +0800 Subject: [PATCH 2/5] fix(discovery): classify malformed configs as unknown not no_mcp - Treat JSON type mismatches in recognized config shapes as unknown format - Restrict the no_mcp fallback to JSON objects, excluding arrays and scalars - Add regression tests for malformed and non-object config inputs Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/discovery/parser.go | 33 ++++++++++++++++++++++--------- internal/discovery/parser_test.go | 31 +++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/internal/discovery/parser.go b/internal/discovery/parser.go index ed27baf..cd19c75 100644 --- a/internal/discovery/parser.go +++ b/internal/discovery/parser.go @@ -32,22 +32,37 @@ func ParseMCPConfigFile(path string) (models.MCPConfig, error) { {"vscode_mcp", parseVSCodeMCPConfig}, } + // sawTypeError records whether a recognized config shape failed to decode + // because of a JSON type mismatch (e.g. "mcpServers": [] or a non-string + // command). That signals a malformed-but-intended config rather than one + // that simply lacks an MCP section. + var sawTypeError bool for _, p := range parsers { cfg, err := p.parse(data) - if err == nil && cfg != nil { - servers := cfg.GetServers() - if len(servers) > 0 { + if err != nil { + var typeErr *json.UnmarshalTypeError + if errors.As(err, &typeErr) { + sawTypeError = true + } + continue + } + if cfg != nil { + if servers := cfg.GetServers(); len(servers) > 0 { return cfg, nil } } } - // No MCP servers were found. Distinguish a valid config that simply has no - // MCP section (parses clean → empty server set) from data we cannot parse - // at all (genuinely unknown format). - var probe any - if json.Unmarshal(data, &probe) == nil { - return &models.ConfigWithoutMCP{}, nil + // No MCP servers were found. Distinguish a valid config object that simply + // has no MCP section (parses clean → empty server set) from data we cannot + // trust: malformed configs (type mismatches) and non-object JSON + // (arrays/scalars/syntax errors) are reported as an unknown format so a + // real problem is not silently hidden as "no servers". + if !sawTypeError { + var probe map[string]any + if json.Unmarshal(data, &probe) == nil { + return &models.ConfigWithoutMCP{}, nil + } } return &models.UnknownMCPConfig{}, nil } diff --git a/internal/discovery/parser_test.go b/internal/discovery/parser_test.go index f9ea63a..55d86a8 100644 --- a/internal/discovery/parser_test.go +++ b/internal/discovery/parser_test.go @@ -164,6 +164,37 @@ func TestParseMCPConfigFile_WithoutMCP(t *testing.T) { } } +func TestParseMCPConfigFile_MalformedOrNonObject(t *testing.T) { + // Valid JSON that is malformed-but-intended (type mismatch) or not a config + // object must be reported as unknown, not silently treated as no_mcp. + tests := []struct { + name string + content string + }{ + {"mcpServers wrong type (array)", `{"mcpServers": []}`}, + {"server command wrong type", `{"mcpServers": {"foo": {"command": 123}}}`}, + {"top-level array", `[1, 2, 3]`}, + {"top-level scalar", `42`}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.json") + if err := os.WriteFile(path, []byte(tt.content), 0o644); err != nil { + t.Fatal(err) + } + + cfg, err := ParseMCPConfigFile(path) + if err != nil { + t.Fatal(err) + } + if cfg.ConfigType() != "unknown" { + t.Errorf("expected config type 'unknown', got %s", cfg.ConfigType()) + } + }) + } +} + func TestParseMCPConfigFile_NotFound(t *testing.T) { _, err := ParseMCPConfigFile("/nonexistent/path.json") if err == nil { From b6809fce7a2b8ac53a33a18b935a63883b37afd0 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sat, 23 May 2026 17:59:21 +0800 Subject: [PATCH 3/5] refactor(models): return empty map from ConfigWithoutMCP.GetServers - Return a non-nil empty map for consistency with other MCPConfig types - Update the test to assert a non-nil empty server set Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/models/config.go | 9 +++++++-- internal/models/config_test.go | 8 ++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/internal/models/config.go b/internal/models/config.go index 7f8b5f1..574524b 100644 --- a/internal/models/config.go +++ b/internal/models/config.go @@ -103,8 +103,13 @@ func (c *UnknownMCPConfig) GetServers() map[string]ServerConfig { return nil } // cleanly and yields an empty server set rather than being treated as an error. type ConfigWithoutMCP struct{} -func (c *ConfigWithoutMCP) ConfigType() string { return "no_mcp" } -func (c *ConfigWithoutMCP) GetServers() map[string]ServerConfig { return nil } +func (c *ConfigWithoutMCP) ConfigType() string { return "no_mcp" } + +// GetServers returns a non-nil empty map, matching the concrete config types +// (ClaudeConfigFile, VSCodeConfigFile, …) so callers never need a nil check. +func (c *ConfigWithoutMCP) GetServers() map[string]ServerConfig { + return map[string]ServerConfig{} +} // ServerConfigJSON is the raw JSON representation that gets parsed into a typed ServerConfig. type ServerConfigJSON struct { diff --git a/internal/models/config_test.go b/internal/models/config_test.go index 96a5b75..74b0a6d 100644 --- a/internal/models/config_test.go +++ b/internal/models/config_test.go @@ -117,8 +117,12 @@ func TestConfigWithoutMCP(t *testing.T) { if cfg.ConfigType() != "no_mcp" { t.Errorf("expected config type 'no_mcp', got %s", cfg.ConfigType()) } - if servers := cfg.GetServers(); servers != nil { - t.Error("expected nil servers") + servers := cfg.GetServers() + if servers == nil { + t.Error("expected non-nil empty map, got nil") + } + if len(servers) != 0 { + t.Errorf("expected 0 servers, got %d", len(servers)) } } From 7f06c2232877826799c88168dfa15e39a7075515 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sat, 23 May 2026 18:06:10 +0800 Subject: [PATCH 4/5] fix(discovery): treat explicit null MCP keys as unknown config - Classify configs with an explicit null for a known MCP key as unknown - Keep valid empty objects classified as no_mcp - Add tests for null MCP keys and the empty-object regression Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/discovery/parser.go | 19 ++++++++++++++++++- internal/discovery/parser_test.go | 22 ++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/internal/discovery/parser.go b/internal/discovery/parser.go index cd19c75..cc906ab 100644 --- a/internal/discovery/parser.go +++ b/internal/discovery/parser.go @@ -60,13 +60,30 @@ func ParseMCPConfigFile(path string) (models.MCPConfig, error) { // real problem is not silently hidden as "no servers". if !sawTypeError { var probe map[string]any - if json.Unmarshal(data, &probe) == nil { + // An explicit null for a known MCP key (e.g. {"mcpServers": null}) is a + // non-object value that decodes without a type error, so it is treated + // as malformed rather than "no servers". + if json.Unmarshal(data, &probe) == nil && !hasNullMCPKey(probe) { return &models.ConfigWithoutMCP{}, nil } } return &models.UnknownMCPConfig{}, nil } +// mcpConfigKeys are the top-level keys the recognized parsers read. A config +// that declares one of these but holds an explicit null (rather than a valid +// object) is malformed rather than simply lacking an MCP section. +var mcpConfigKeys = []string{"mcpServers", "mcp", "servers"} + +func hasNullMCPKey(obj map[string]any) bool { + for _, key := range mcpConfigKeys { + if v, ok := obj[key]; ok && v == nil { + return true + } + } + return false +} + func parseClaudeConfig(data []byte) (models.MCPConfig, error) { var cfg models.ClaudeConfigFile if err := json.Unmarshal(data, &cfg); err != nil { diff --git a/internal/discovery/parser_test.go b/internal/discovery/parser_test.go index 55d86a8..bf35cbb 100644 --- a/internal/discovery/parser_test.go +++ b/internal/discovery/parser_test.go @@ -175,6 +175,9 @@ func TestParseMCPConfigFile_MalformedOrNonObject(t *testing.T) { {"server command wrong type", `{"mcpServers": {"foo": {"command": 123}}}`}, {"top-level array", `[1, 2, 3]`}, {"top-level scalar", `42`}, + {"mcpServers explicit null", `{"mcpServers": null}`}, + {"mcp explicit null", `{"mcp": null}`}, + {"servers explicit null", `{"servers": null}`}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -195,6 +198,25 @@ func TestParseMCPConfigFile_MalformedOrNonObject(t *testing.T) { } } +func TestParseMCPConfigFile_EmptyObjectStaysNoMCP(t *testing.T) { + // A known MCP key holding a valid empty object means zero servers, not a + // malformed config — it must stay no_mcp (unlike an explicit null). + for _, content := range []string{`{"mcpServers": {}}`, `{"mcp": {"servers": {}}}`} { + dir := t.TempDir() + path := filepath.Join(dir, "config.json") + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + cfg, err := ParseMCPConfigFile(path) + if err != nil { + t.Fatal(err) + } + if cfg.ConfigType() != "no_mcp" { + t.Errorf("content %s: expected 'no_mcp', got %s", content, cfg.ConfigType()) + } + } +} + func TestParseMCPConfigFile_NotFound(t *testing.T) { _, err := ParseMCPConfigFile("/nonexistent/path.json") if err == nil { From 96c71f06a2c22db2c5a5300d52e667e2ec852174 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sat, 23 May 2026 18:28:20 +0800 Subject: [PATCH 5/5] docs(claude): drop deleted mcpserver package from structure list Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1071042..df6b523 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,7 +65,6 @@ make build_windows_amd64 # Windows x86-64 - `internal/redact/` — Sensitive data redaction - `internal/upload/` — Control server upload - `internal/signed/` — macOS code signature verification -- `internal/mcpserver/` — Self-as-MCP-server mode - `internal/cli/` — Cobra CLI commands - `internal/version/` — Build-time version info