fix(scanner): redact mounted route findings

Signed-off-by: dongmucat <1127093059@qq.com>

scanner/skillhub_scanner_app.py

@@ -1,6 +1,7 @@
 """Runtime safeguards around the upstream Cisco Skill Scanner ASGI application."""
 
 import asyncio
+import inspect
 import logging
 import os
 import shutil
@@ -46,6 +47,7 @@ _upstream_router.MAX_UPLOAD_SIZE_BYTES = max(
 _active_scans = 0
 _active_scans_guard = asyncio.Lock()
 _SCAN_PATHS = {"/scan", "/scan-upload"}
+_REDACTION_MARKER = "_skillhub_redaction_installed"
 _log = logging.getLogger(__name__)
 
 
@@ -74,23 +76,78 @@ def _redact_supported_tokens(value):
     return value
 
 
-def _install_scan_response_redaction() -> None:
-    """Redact supported token forms before FastAPI serializes scan findings."""
-    for route in _upstream_router.router.routes:
-        if getattr(route, "path", None) not in _SCAN_PATHS or "POST" not in getattr(route, "methods", set()):
+def _iter_route_objects(container, seen=None):
+    """Walk FastAPI/Starlette route containers, including mounted child routers."""
+    seen = set() if seen is None else seen
+    routes = getattr(container, "routes", None)
+    if routes is None:
+        nested_router = getattr(container, "router", None)
+        if nested_router is not None and nested_router is not container:
+            yield from _iter_route_objects(nested_router, seen)
+        return
+    for route in routes:
+        route_id = id(route)
+        if route_id in seen:
             continue
-        endpoint = route.endpoint
+        seen.add(route_id)
+        yield route
+        for nested in (
+            getattr(route, "original_router", None),
+            getattr(route, "app", None),
+            getattr(route, "router", None),
+        ):
+            yield from _iter_route_objects(nested, seen)
 
+
+def _redact_scan_response(response):
+    findings = getattr(response, "findings", None)
+    if isinstance(findings, list):
+        response.findings = _redact_supported_tokens(findings)
+    return response
+
+
+def _make_redacting_endpoint(endpoint):
+    if inspect.iscoroutinefunction(endpoint):
         @wraps(endpoint)
         async def redacting_endpoint(*args, __endpoint=endpoint, **kwargs):
-            response = await __endpoint(*args, **kwargs)
-            findings = getattr(response, "findings", None)
-            if isinstance(findings, list):
-                response.findings = _redact_supported_tokens(findings)
-            return response
+            return _redact_scan_response(await __endpoint(*args, **kwargs))
+    else:
+        @wraps(endpoint)
+        def redacting_endpoint(*args, __endpoint=endpoint, **kwargs):
+            return _redact_scan_response(__endpoint(*args, **kwargs))
+    return redacting_endpoint
 
-        route.endpoint = redacting_endpoint
-        route.dependant.call = redacting_endpoint
+
+def _install_scan_response_redaction() -> None:
+    """Redact supported token forms before FastAPI serializes scan findings."""
+    upstream_routes = list(_iter_route_objects(_upstream_router.router))
+    target_routes = [
+        route
+        for route in upstream_routes
+        if getattr(route, "path", None) in _SCAN_PATHS
+        and "POST" in getattr(route, "methods", set())
+    ]
+    if not target_routes:
+        raise RuntimeError("Scanner routes /scan and /scan-upload were not found")
+    target_endpoints = {route.endpoint for route in target_routes}
+    target_paths = {route.path for route in target_routes}
+    all_routes = [
+        *list(_iter_route_objects(getattr(app, "router", app))),
+        *upstream_routes,
+    ]
+    for route in all_routes:
+        endpoint = getattr(route, "endpoint", None)
+        path = getattr(route, "path", None)
+        is_target_endpoint = endpoint in target_endpoints
+        is_target_path = path in target_paths and "POST" in getattr(route, "methods", set())
+        if not (is_target_endpoint or is_target_path) or getattr(route, _REDACTION_MARKER, False):
+            continue
+        wrapped_endpoint = _make_redacting_endpoint(endpoint)
+        route.endpoint = wrapped_endpoint
+        dependant = getattr(route, "dependant", None)
+        if dependant is not None:
+            dependant.call = wrapped_endpoint
+        setattr(route, _REDACTION_MARKER, True)
 
 
 def _restart_after_hard_timeout(request_path: str) -> NoReturn:

scanner/tests/test_skillhub_scanner_app.py

@@ -12,8 +12,9 @@ from unittest.mock import patch
 
 
 class _FakeRouter:
-    def __init__(self):
+    def __init__(self, routes=None):
         self.handlers = []
+        self.routes = routes or []
 
     def add_event_handler(self, _event, _handler):
         self.handlers.append((_event, _handler))
@@ -52,6 +53,9 @@ class _FakeApp:
                 _FakeScanResponse([{"description": "No credentials", "metadata": {"safe": True}}]),
             ),
         ]
+        mounted_routes = [route.clone("/mounted") for route in self.upstream_routes]
+        nested_routes = [route.clone("/nested") for route in self.upstream_routes]
+        self.router.routes = [_FakeIncludedRouter(mounted_routes), _FakeMount(nested_routes)]
 
     def middleware(self, _kind):
         return lambda function: function
@@ -75,6 +79,7 @@ class _FakeRoute:
     def __init__(self, path, response):
         self.path = path
         self.methods = {"POST"}
+        self._response = response
 
         async def endpoint():
             return response
@@ -82,6 +87,22 @@ class _FakeRoute:
         self.endpoint = endpoint
         self.dependant = types.SimpleNamespace(call=endpoint)
 
+    def clone(self, prefix=""):
+        clone = _FakeRoute(f"{prefix}{self.path}", self._response)
+        clone.endpoint = self.endpoint
+        clone.dependant = types.SimpleNamespace(call=self.endpoint)
+        return clone
+
+
+class _FakeIncludedRouter:
+    def __init__(self, routes):
+        self.original_router = types.SimpleNamespace(routes=routes)
+
+
+class _FakeMount:
+    def __init__(self, routes):
+        self.app = types.SimpleNamespace(router=types.SimpleNamespace(routes=routes))
+
 
 class _Request:
     method = "POST"
@@ -235,6 +256,39 @@ class SkillHubScannerAppTest(unittest.IsolatedAsyncioTestCase):
         self.assertEqual(200, response.status_code)
         self.assertEqual({"X-Contract": "preserved"}, response.headers)
 
+    async def test_mounted_app_route_also_redacts_findings(self):
+        included = next(route for route in self.module.app.router.routes if isinstance(route, _FakeIncludedRouter))
+        route = next(route for route in included.original_router.routes if route.path == "/mounted/scan")
+
+        response = await route.endpoint()
+
+        self.assertNotIn("ghp_", str(response.findings))
+        self.assertNotIn("sk-proj-", str(response.findings))
+        self.assertEqual(200, response.status_code)
+        self.assertEqual({"X-Contract": "preserved"}, response.headers)
+
+    async def test_nested_fastapi_app_route_also_redacts_findings(self):
+        mount = next(route for route in self.module.app.router.routes if isinstance(route, _FakeMount))
+        route = next(route for route in mount.app.router.routes if route.path == "/nested/scan")
+
+        response = await route.endpoint()
+
+        self.assertNotIn("ghp_", str(response.findings))
+        self.assertNotIn("sk-proj-", str(response.findings))
+        self.assertEqual(200, response.status_code)
+        self.assertEqual({"X-Contract": "preserved"}, response.headers)
+
+    async def test_sync_endpoint_wrapper_redacts_findings(self):
+        response = _FakeScanResponse([{"description": "api_key=custom-secret-1234567890"}])
+
+        def endpoint():
+            return response
+
+        wrapped = self.module._make_redacting_endpoint(endpoint)
+
+        self.assertIs(response, wrapped())
+        self.assertNotIn("custom-secret-1234567890", str(response.findings))
+
     async def test_safe_scan_findings_are_unchanged(self):
         route = next(route for route in self.module._router_stub.router.routes if route.path == "/scan-upload")
         expected = [{"description": "No credentials", "metadata": {"safe": True}}]

scripts/tests/scanner-2-1-contract-test.sh

@@ -187,7 +187,13 @@ MAIN_URL="http://127.0.0.1:$MAIN_PORT"
 wait_for_health "$MAIN_URL"
 
 docker exec -i "$MAIN_CONTAINER" python - <<'PY'
+from types import SimpleNamespace
+
+from fastapi import APIRouter, FastAPI, Response
+from fastapi.testclient import TestClient
+from pydantic import BaseModel
 from skill_scanner.core.analyzers.llm_analyzer import LLMProvider
+import skillhub_scanner_app as scanner_app
 from skillhub_scanner_app import _redact_supported_tokens
 
 canaries = {
@@ -217,6 +223,54 @@ if _redact_supported_tokens(mixed) != "before\napi_key=<redacted>\tafter":
     raise SystemExit("redaction changed non-secret characters around a credential")
 if not LLMProvider.is_valid_provider("azure-openai") or LLMProvider.is_valid_provider("azure"):
     raise SystemExit("unexpected Scanner 2.1 Azure provider contract")
+
+
+class ScanResponse(BaseModel):
+    findings: list[dict]
+    safe_text: str
+
+
+upstream_router = APIRouter()
+
+
+def install_http_canary(path):
+    @upstream_router.post(path, response_model=ScanResponse)
+    async def scan(response: Response):
+        response.headers["X-Contract"] = "preserved"
+        return ScanResponse(
+            findings=[{"description": f"api_key={canaries['labeled']}"}],
+            safe_text="line one\n" + ("x" * 5000),
+        )
+
+install_http_canary("/scan")
+install_http_canary("/scan-upload")
+http_app = FastAPI()
+http_app.include_router(upstream_router)
+nested_app = FastAPI()
+nested_app.include_router(upstream_router)
+http_app.mount("/nested", nested_app)
+
+previous_app = scanner_app.app
+previous_router = scanner_app._upstream_router
+scanner_app.app = http_app
+scanner_app._upstream_router = SimpleNamespace(router=upstream_router)
+try:
+    scanner_app._install_scan_response_redaction()
+    with TestClient(http_app) as client:
+        for path in ("/scan", "/scan-upload", "/nested/scan", "/nested/scan-upload"):
+            response = client.post(path)
+            if response.status_code != 200:
+                raise SystemExit(f"HTTP canary failed for {path}: {response.status_code}")
+            if response.headers.get("X-Contract") != "preserved":
+                raise SystemExit(f"HTTP canary changed headers for {path}")
+            body = response.text
+            if canaries["labeled"] in body:
+                raise SystemExit(f"HTTP canary leaked secret for {path}")
+            if "line one\\n" not in body or ("x" * 5000) not in body:
+                raise SystemExit(f"HTTP canary changed safe text for {path}")
+finally:
+    scanner_app.app = previous_app
+    scanner_app._upstream_router = previous_router
 PY
 
 docker exec "$MAIN_CONTAINER" python -c \