diff --git a/docs/features/security-scanner-plugins.md b/docs/features/security-scanner-plugins.md index 0b57b0187..b30a31357 100644 --- a/docs/features/security-scanner-plugins.md +++ b/docs/features/security-scanner-plugins.md @@ -313,7 +313,7 @@ The Security page at `/security` in the Web UI mirrors the CLI and provides: ## Known limitations - **Ramparts on arm64 macOS** — the upstream scanner image ships a binary linked against a newer GLIBC than the image base and fails every run on arm64. Track the [scanner-ramparts image rebuild](https://github.com/smart-mcp-proxy/mcpproxy-go/issues) for a fix. Other 6 of 7 scanners work out of the box on arm64 macOS. -- **Cisco scanner output has a hardcoded `server_url`** header in its stdout (`https://mcp.deepwiki.com/mcp`). Cosmetic, does not affect findings. +- **Cisco scanner output has a hardcoded `server_url`** header in its stdout (`https://mcp.deepwiki.com/mcp`). This is cosmetic and does not affect findings. Since #383, mcpproxy strips this line from the user-visible execution log and replaces it with an annotation explaining no network request was made. - **Pass 2 (supply-chain audit)** currently requires Docker isolation to be enabled, otherwise it fails source resolution. The UI doesn't yet surface this precondition. ## Related reading diff --git a/internal/security/scanner/engine.go b/internal/security/scanner/engine.go index e04b27f50..ef4e4a7bc 100644 --- a/internal/security/scanner/engine.go +++ b/internal/security/scanner/engine.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path/filepath" + "regexp" "strings" "sync" "time" @@ -604,6 +605,9 @@ type scannerLogs struct { // setScannerLogs stores stdout/stderr on a scanner's job status func (e *Engine) setScannerLogs(job *ScanJob, scannerID string, logs scannerLogs) { + if scannerID == ciscoScannerID { + logs.Stdout = sanitizeCiscoStdout(logs.Stdout) + } e.mu.Lock() defer e.mu.Unlock() for i := range job.ScannerStatuses { @@ -1041,3 +1045,34 @@ func truncate(s string, maxLen int) string { } return s[:maxLen] + "..." } + +// ciscoScannerID is the single source of truth for the bundled Cisco AI Defense +// scanner's plugin ID. registry_bundled.go references this const, and +// setScannerLogs uses it to gate Cisco-specific stdout sanitization. +const ciscoScannerID = "cisco-mcp-scanner" + +// ciscoServerURLPattern matches the placeholder server_url line emitted by +// the upstream cisco-ai-mcp-scanner PyPI package in its raw stdout output. +// The URL is hardcoded in the upstream tool and does not represent a real +// network request; mcpproxy strips it from the user-visible execution log +// to avoid the false impression of data exfiltration. See issue #383. +var ciscoServerURLPattern = regexp.MustCompile( + `(?m)^[ \t]*"server_url"[ \t]*:[ \t]*"https?://[^"]*deepwiki[^"]*"[ \t]*,?[ \t]*\r?\n?`, +) + +// sanitizeCiscoStdout replaces the upstream-hardcoded deepwiki placeholder +// URL line with a short annotation referencing the tracking issue. The rest +// of the raw output is preserved for debugging. +// +// Assumes pretty-printed multi-line output; minified single-line JSON +// bypasses this filter (acceptable since the cisco scanner emits multi-line). +func sanitizeCiscoStdout(stdout string) string { + if !strings.Contains(stdout, "deepwiki") { + return stdout + } + return ciscoServerURLPattern.ReplaceAllString( + stdout, + "// [mcpproxy] upstream cisco-ai-mcp-scanner emits a hardcoded "+ + "placeholder server_url; no network request was made (see issue #383)\n", + ) +} diff --git a/internal/security/scanner/engine_test.go b/internal/security/scanner/engine_test.go index 465760fbf..7e2032af5 100644 --- a/internal/security/scanner/engine_test.go +++ b/internal/security/scanner/engine_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "os" "path/filepath" + "strings" "testing" "time" @@ -692,3 +693,121 @@ func TestPrepareReportDirForEngine(t *testing.T) { t.Error("directory should exist") } } + +func TestSanitizeCiscoStdout_RemovesHardcodedDeepwikiURL(t *testing.T) { + in := `{ + "server_url": "https://mcp.deepwiki.com/mcp", + "scan_results": [{"tool_name": "test_tool", "is_safe": true}] +}` + out := sanitizeCiscoStdout(in) + if strings.Contains(out, "deepwiki") { + t.Errorf("expected deepwiki URL stripped, got: %s", out) + } + if !strings.Contains(out, "// [mcpproxy] upstream cisco-ai-mcp-scanner") { + t.Error("expected explanatory annotation") + } +} + +func TestSanitizeCiscoStdout_PreservesScanResults(t *testing.T) { + in := `{ + "server_url": "https://mcp.deepwiki.com/mcp", + "scan_results": [{"tool_name": "test_tool", "is_safe": true}] +}` + out := sanitizeCiscoStdout(in) + if !strings.Contains(out, `"scan_results"`) { + t.Error("scan_results lost") + } + if !strings.Contains(out, "test_tool") { + t.Error("tool_name lost") + } +} + +func TestSanitizeCiscoStdout_NoOpWithoutDeepwiki(t *testing.T) { + in := `{"scan_results": [{"tool_name": "x", "is_safe": true}]}` + if got := sanitizeCiscoStdout(in); got != in { + t.Errorf("expected no-op, got diff: %s", got) + } +} + +func TestSanitizeCiscoStdout_HandlesCRLF(t *testing.T) { + in := " \"server_url\": \"https://mcp.deepwiki.com/mcp\",\r\n{...}\r\n" + out := sanitizeCiscoStdout(in) + if strings.Contains(out, "deepwiki") { + t.Error("CRLF variant not handled") + } +} + +func TestSetScannerLogs_CiscoStdoutSanitized(t *testing.T) { + dir := t.TempDir() + logger := zap.NewNop() + registry := NewRegistry(dir, logger) + docker := NewDockerRunner(logger) + engine := NewEngine(docker, registry, dir, logger) + + job := &ScanJob{ + ID: "job-sanitize", + ServerName: "test-server", + ScannerStatuses: []ScannerJobStatus{ + {ScannerID: ciscoScannerID, Status: ScanJobStatusRunning}, + }, + } + + rawStdout := `{ + "server_url": "https://mcp.deepwiki.com/mcp", + "scan_results": [{"tool_name": "test_tool", "is_safe": true}] +}` + engine.setScannerLogs(job, ciscoScannerID, scannerLogs{ + Stdout: rawStdout, + Stderr: "some stderr", + ExitCode: 0, + }) + + got := job.ScannerStatuses[0].Stdout + if strings.Contains(got, "deepwiki") { + t.Errorf("expected deepwiki URL removed from cisco stdout, got:\n%s", got) + } + if !strings.Contains(got, "// [mcpproxy]") { + t.Error("expected annotation comment in sanitized output") + } + if !strings.Contains(got, `"scan_results"`) { + t.Error("scan_results should be preserved after sanitization") + } + // Verify indentation of the line following the removed URL is intact. + if !strings.Contains(got, ` "scan_results"`) { + t.Errorf("next line indentation damaged, got:\n%s", got) + } +} + +func TestSetScannerLogs_NonCiscoScannerStdoutPreserved(t *testing.T) { + dir := t.TempDir() + logger := zap.NewNop() + registry := NewRegistry(dir, logger) + docker := NewDockerRunner(logger) + engine := NewEngine(docker, registry, dir, logger) + + const otherScanner = "semgrep" + job := &ScanJob{ + ID: "job-preserve", + ServerName: "test-server", + ScannerStatuses: []ScannerJobStatus{ + {ScannerID: otherScanner, Status: ScanJobStatusRunning}, + }, + } + + // Stdout happens to contain "deepwiki" but should NOT be sanitized + // because this is not the cisco scanner. + rawStdout := `{"server_url": "https://mcp.deepwiki.com/mcp", "results": []}` + engine.setScannerLogs(job, otherScanner, scannerLogs{ + Stdout: rawStdout, + Stderr: "", + ExitCode: 0, + }) + + got := job.ScannerStatuses[0].Stdout + if !strings.Contains(got, "deepwiki") { + t.Errorf("non-cisco scanner stdout should not be sanitized, got:\n%s", got) + } + if got != rawStdout { + t.Errorf("stdout should be preserved verbatim for non-cisco scanner\nwant: %s\ngot: %s", rawStdout, got) + } +} diff --git a/internal/security/scanner/registry_bundled.go b/internal/security/scanner/registry_bundled.go index 504dfc38d..c7a051267 100644 --- a/internal/security/scanner/registry_bundled.go +++ b/internal/security/scanner/registry_bundled.go @@ -16,7 +16,7 @@ package scanner // deterministic across API, CLI, and UI. var bundledScanners = []*ScannerPlugin{ { - ID: "cisco-mcp-scanner", + ID: ciscoScannerID, Name: "Cisco MCP Scanner", Vendor: "Cisco AI Defense", Description: "YARA rules + readiness analysis. Detects tool poisoning, prompt injection, credential harvesting, and data exfiltration. No API key needed for offline mode.",