fix(scanner): always surface cisco static-analysis coverage caveat (MCP-2399)

docs/features/security-scanner-plugins.md

@@ -317,7 +317,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`). 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.
+- **Cisco scanner is static-analysis only — coverage caveat.** The bundled `cisco-mcp-scanner` runs `static --tools tools.json` (YARA + readiness rules over the exported tool definitions). It **never connects to or probes the live server endpoint and makes no network request**, so an `is_safe`/`SAFE` result reflects the analyzed tool definitions, not the server's live runtime behavior — do not read a clean Cisco result for a remote/URL server as proof the live endpoint was exercised. mcpproxy prepends this caveat to every Cisco execution log. Relatedly, the upstream tool hardcodes a placeholder `server_url` header (`https://mcp.deepwiki.com/mcp`) in its stdout; this is cosmetic and does not affect findings, and since #383 mcpproxy strips that 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

internal/security/scanner/engine.go

@@ -1073,19 +1073,34 @@ 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.
+// ciscoCoverageCaveat is prepended to every cisco-mcp-scanner execution log so
+// the scanner's coverage is never over-trusted. The bundled cisco scanner runs
+// `static --tools tools.json` (registry_bundled.go): it analyzes the exported
+// tool definitions with YARA + readiness rules and never connects to the live
+// server endpoint, so an is_safe/SAFE result reflects the tool definitions, not
+// the server's runtime behavior. Surfacing this unconditionally (not only when
+// the upstream deepwiki placeholder happens to appear) keeps the caveat honest
+// even if a future upstream release changes its raw output. See issue #383 / MCP-2399.
+const ciscoCoverageCaveat = "// [mcpproxy] coverage caveat: cisco-mcp-scanner performs STATIC analysis of " +
+	"the exported tool definitions only. It does not connect to or probe the live server endpoint and " +
+	"makes no network request, so a \"safe\"/is_safe result reflects the analyzed tool definitions, not " +
+	"the server's live runtime behavior (see issue #383).\n"
+
+// sanitizeCiscoStdout prepends a static-analysis coverage caveat and 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. The caveat leads the output so it survives MaxLogBytes truncation.
 //
 // 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
+	sanitized := stdout
+	if strings.Contains(sanitized, "deepwiki") {
+		sanitized = ciscoServerURLPattern.ReplaceAllString(
+			sanitized,
+			"// [mcpproxy] upstream cisco-ai-mcp-scanner emits a hardcoded "+
+				"placeholder server_url; no network request was made (see issue #383)\n",
+		)
 	}
-	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",
-	)
+	return ciscoCoverageCaveat + sanitized
 }

internal/security/scanner/engine_test.go

@@ -841,10 +841,43 @@ func TestSanitizeCiscoStdout_PreservesScanResults(t *testing.T) {
 	}
 }
 
-func TestSanitizeCiscoStdout_NoOpWithoutDeepwiki(t *testing.T) {
+// TestSanitizeCiscoStdout_SurfacesCoverageCaveatWithoutDeepwiki asserts the
+// static-analysis coverage caveat is surfaced even when the upstream output
+// carries no deepwiki placeholder line. The caveat must not depend on the
+// upstream placeholder string, which a future cisco-ai-mcp-scanner release may
+// change or drop — the static-only limitation is permanent regardless. See MCP-2399.
+func TestSanitizeCiscoStdout_SurfacesCoverageCaveatWithoutDeepwiki(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)
+	out := sanitizeCiscoStdout(in)
+	if !strings.Contains(out, ciscoCoverageCaveat) {
+		t.Errorf("expected static-analysis coverage caveat to be surfaced, got:\n%s", out)
+	}
+	// Original payload must be preserved verbatim after the caveat header.
+	if !strings.Contains(out, in) {
+		t.Errorf("expected original output preserved, got:\n%s", out)
+	}
+	// Caveat must state the key facts: static-only and no network request.
+	if !strings.Contains(ciscoCoverageCaveat, "static") && !strings.Contains(ciscoCoverageCaveat, "STATIC") {
+		t.Error("caveat must state the analysis is static")
+	}
+	if !strings.Contains(ciscoCoverageCaveat, "network request") {
+		t.Error("caveat must state no network request is made")
+	}
+}
+
+// TestSanitizeCiscoStdout_CaveatAlwaysLeadsOutput asserts the caveat is the
+// first thing in the sanitized output so it survives MaxLogBytes truncation.
+func TestSanitizeCiscoStdout_CaveatAlwaysLeadsOutput(t *testing.T) {
+	withDeepwiki := `{
+  "server_url": "https://mcp.deepwiki.com/mcp",
+  "scan_results": [{"tool_name": "x", "is_safe": true}]
+}`
+	out := sanitizeCiscoStdout(withDeepwiki)
+	if !strings.HasPrefix(out, ciscoCoverageCaveat) {
+		t.Errorf("expected caveat to lead the output, got:\n%s", out)
+	}
+	if strings.Contains(out, "deepwiki") {
+		t.Errorf("expected deepwiki line still stripped, got:\n%s", out)
 	}
 }