From 97f2c6c28eda8d0674be336be2cdebe3df96932d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=8E=A0=E8=93=9D?= <469906100@qq.com> Date: Tue, 26 May 2026 11:29:08 +0800 Subject: [PATCH 1/3] fix(scanner): strip hardcoded deepwiki URL from Cisco scanner raw stdout The upstream cisco-ai-mcp-scanner PyPI package emits a hardcoded "server_url: https://mcp.deepwiki.com/mcp" header in its raw-format stdout regardless of the actual scan target. mcpproxy was capturing this verbatim into ScannerJobStatus.Stdout and surfacing it in the scan report UI, making it look like tool definitions were being exfiltrated to an unrelated third-party URL. No network request is actually made (NetworkReq=false, static analyzers only). The findings parser (parseCiscoScannerOutput) already ignores this header, so findings stay unchanged. This commit strips the bogus line from the stdout shown to users and replaces it with a short annotation referencing the issue. Closes #383 --- docs/features/security-scanner-plugins.md | 2 +- internal/security/scanner/engine.go | 27 ++++++++++++ internal/security/scanner/engine_test.go | 54 +++++++++++++++++++++++ 3 files changed, 82 insertions(+), 1 deletion(-) 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..19437176d 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" @@ -606,6 +607,9 @@ type scannerLogs struct { func (e *Engine) setScannerLogs(job *ScanJob, scannerID string, logs scannerLogs) { e.mu.Lock() defer e.mu.Unlock() + if scannerID == "cisco-mcp-scanner" { + logs.Stdout = sanitizeCiscoStdout(logs.Stdout) + } for i := range job.ScannerStatuses { if job.ScannerStatuses[i].ScannerID == scannerID { job.ScannerStatuses[i].Stdout = truncate(logs.Stdout, MaxLogBytes) @@ -1041,3 +1045,26 @@ func truncate(s string, maxLen int) string { } return s[:maxLen] + "..." } + +// 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)^\s*"server_url"\s*:\s*"https?://[^"]*deepwiki[^"]*"\s*,?\s*\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. +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..44dda6629 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,56 @@ 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 TestParseCiscoScannerOutput_UnaffectedBySanitization(t *testing.T) { + raw := []byte(`{"scan_results": [{"tool_name": "t1", "is_safe": false, + "findings": {"yara_analyzer": {"severity": "HIGH", + "threat_names": ["test"], "total_findings": 1}}}]}`) + findings := parseCiscoScannerOutput(raw, "cisco-mcp-scanner") + if len(findings) != 1 { + t.Errorf("expected 1 finding, got %d", len(findings)) + } +} From 422e39225bb12d74af9f8e23c74dfabb2d4e3d25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=8E=A0=E8=93=9D?= <469906100@qq.com> Date: Tue, 26 May 2026 11:53:02 +0800 Subject: [PATCH 2/3] fix(scanner): tighten cisco stdout sanitizer regex and add integration tests Address review feedback on #383: - Change regex from \s* to [ \t]* around the URL line so it no longer eats the following line's indentation on multi-line JSON. - Extract scanner-id literal into ciscoScannerID const (keep in sync with registry_bundled.go). - Move sanitize call before the engine mutex; the function is pure and does not need the lock. - Add TestSetScannerLogs_CiscoStdoutSanitized and TestSetScannerLogs_NonCiscoScannerStdoutPreserved to cover the scannerID dispatch path directly. - Drop TestParseCiscoScannerOutput_UnaffectedBySanitization; the new integration tests cover the same intent more precisely. - Document in godoc that minified single-line JSON bypasses this filter. --- internal/security/scanner/engine.go | 15 +++-- internal/security/scanner/engine_test.go | 79 +++++++++++++++++++++--- 2 files changed, 83 insertions(+), 11 deletions(-) diff --git a/internal/security/scanner/engine.go b/internal/security/scanner/engine.go index 19437176d..886e76102 100644 --- a/internal/security/scanner/engine.go +++ b/internal/security/scanner/engine.go @@ -605,11 +605,11 @@ type scannerLogs struct { // setScannerLogs stores stdout/stderr on a scanner's job status func (e *Engine) setScannerLogs(job *ScanJob, scannerID string, logs scannerLogs) { - e.mu.Lock() - defer e.mu.Unlock() - if scannerID == "cisco-mcp-scanner" { + if scannerID == ciscoScannerID { logs.Stdout = sanitizeCiscoStdout(logs.Stdout) } + e.mu.Lock() + defer e.mu.Unlock() for i := range job.ScannerStatuses { if job.ScannerStatuses[i].ScannerID == scannerID { job.ScannerStatuses[i].Stdout = truncate(logs.Stdout, MaxLogBytes) @@ -1046,18 +1046,25 @@ func truncate(s string, maxLen int) string { return s[:maxLen] + "..." } +// ciscoScannerID matches the bundled Cisco AI Defense scanner registered +// in registry_bundled.go. Keep in sync with that file's ID literal. +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)^\s*"server_url"\s*:\s*"https?://[^"]*deepwiki[^"]*"\s*,?\s*\r?\n?`, + `(?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 diff --git a/internal/security/scanner/engine_test.go b/internal/security/scanner/engine_test.go index 44dda6629..7e2032af5 100644 --- a/internal/security/scanner/engine_test.go +++ b/internal/security/scanner/engine_test.go @@ -737,12 +737,77 @@ func TestSanitizeCiscoStdout_HandlesCRLF(t *testing.T) { } } -func TestParseCiscoScannerOutput_UnaffectedBySanitization(t *testing.T) { - raw := []byte(`{"scan_results": [{"tool_name": "t1", "is_safe": false, - "findings": {"yara_analyzer": {"severity": "HIGH", - "threat_names": ["test"], "total_findings": 1}}}]}`) - findings := parseCiscoScannerOutput(raw, "cisco-mcp-scanner") - if len(findings) != 1 { - t.Errorf("expected 1 finding, got %d", len(findings)) +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) } } From f3dbf343e56584d5f2c426288f70f8b759e48e96 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 26 May 2026 08:08:39 +0300 Subject: [PATCH 3/3] refactor(scanner): share cisco scanner ID const between engine and registry registry_bundled.go now references ciscoScannerID instead of duplicating the "cisco-mcp-scanner" literal. Removes the silent-no-op footgun where an ID drift would disable stdout sanitization without failing any test (both sides previously used the literal independently). --- internal/security/scanner/engine.go | 5 +++-- internal/security/scanner/registry_bundled.go | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/security/scanner/engine.go b/internal/security/scanner/engine.go index 886e76102..ef4e4a7bc 100644 --- a/internal/security/scanner/engine.go +++ b/internal/security/scanner/engine.go @@ -1046,8 +1046,9 @@ func truncate(s string, maxLen int) string { return s[:maxLen] + "..." } -// ciscoScannerID matches the bundled Cisco AI Defense scanner registered -// in registry_bundled.go. Keep in sync with that file's ID literal. +// 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 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.",