fix(scanner): resolve source for runtime-added Docker-isolated stdio servers (MCP-2123 Defect A)#643
Conversation
…servers Two independent defects broke security scans of Docker-isolated stdio servers added at runtime from the official registry (slash/dot names like com.pulsemcp/google-flights), surfacing as "No Source Available" (source_method=none) with no tools.json and a skipped Pass 2. 1. Stale config snapshot (primary). configServerInfoProvider iterated a config captured at boot, so servers added at runtime were invisible to GetServerInfo. The scanner then ran with no ServerInfo: source stayed "none", tool definitions were never exported, and Pass 2 was skipped with "No server info available". The provider now resolves against the live config snapshot (runtime.Config), falling back to the boot snapshot. 2. Container-name sanitizer mismatch. The scanner's docker-ps lookup mapped '.' to '-' while the launcher preserves it, so the 'mcpproxy-<sanitized>-' filter never matched the real container for any dotted name and docker_extract silently fell through. Both sides now share internal/dockernaming.SanitizeServerName so the rule can't drift. Adds regression tests for both. Defect B (frontend scan-report link not URL-encoding the scan id) is tracked separately. Related #MCP-2123
Deploying mcpproxy-docs with
|
| Latest commit: |
bde3dcb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://23703e92.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-mcp-2123-docker-scan-sou.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 27457860138 --repo smart-mcp-proxy/mcpproxy-go
|
🔎 Verification (CEO cockpit) — CHANGES REQUESTEDBuilt this branch merged with #644 and ran a real end-to-end scan of a runtime-added, Docker-isolated What this PR fixes (confirmed): stale config snapshot → ServerInfo found; sanitizer unified → container now matches Newly-exposed, blocking:
With both applied (verified): Recommend folding MCP-2149's patch into this branch so it ships as one PR; QA will re-bless the new head SHA. |
…ode scan-report jobId Follow-up to the same MCP-2123 branch: once the sanitizer fix makes the container actually match, two more defects surface for official-registry slash-named servers (verified end-to-end by CEO on com.pulsemcp/google-flights): 1. os.MkdirTemp rejects a pattern containing a path separator, so the three extraction temp-dir patterns (mcpproxy-scan-%s-, -scan-full-%s-, -scan-tools-%s-) failed for '/'-containing names and source fell back to 'none'. Sanitize the name via dockernaming.SanitizeServerName first. 2. handleGetScanReportByJobID read the raw chi jobId param. Scan ids are scan-<serverName>-<ts> and contain '/', arriving percent-encoded, so the report endpoint 404'd. Decode via decodePathParam (same pattern the /servers subtree uses). Verified result: source_method=docker_extract, 23 files extracted, tools_exported=3, report endpoint HTTP 200. Adds regression tests: TestScanTempDirPatternHandlesSlashNames, TestDecodePathParamScanJobID. Related #MCP-2123 Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
Pushed follow-up commit
Verified result: |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ection) The user-controlled server name in the os.MkdirTemp patterns was purely cosmetic — the random suffix already guarantees a unique dir. Embedding it introduced a go/path-injection CodeQL high alert (taint flows into the path expression even through dockernaming.SanitizeServerName) and was the original cause of the slash-name rejection bug. Make all three scan temp-dir patterns constant, killing the taint at the source and trivially handling slash-named servers. Drop the now-unused dockernaming import from service.go; keep it in source_resolver.go (still used by findServerContainer). Update the MCP-2123 regression test to assert the patterns are constant and carry no user data. Related #643
![mcpproxy-gatekeeper[bot]](https://avatars.githubusercontent.com/u/216428285?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Gatekeeper approval — Codex review verdict: ACCEPT.
This approval is posted automatically by the MCPProxy Gatekeeper App on behalf of the Codex reviewer (verdict of record lives in the Paperclip review thread). Author≠approver satisfied; QA + CI gates enforced separately.
Auto-approved per Model B (MCP-1249).
…644) Scan ids embed the raw upstream server name, so official-registry servers whose names contain '/' (e.g. com.pulsemcp/google-flights) produce a scan id like scan-com.pulsemcp/google-flights-<ts> with a '/' in it. The scan-report route (/security/scans/:jobId) is a single segment, so the unencoded ServerDetail "View Full Report" link split the path and fell through to the catch-all 404. Add a scanReportPath() helper that percent-encodes the id (vue-router v4 decodes it back on read, so ScanReport.vue needs no manual decode) and use it in both ServerDetail.vue (the bug) and Security.vue (DRY; was already encoding inline). Same class as MCP-1112 / serverDetailPath. Adds a vitest covering the encoded link target and a route round-trip. Related #643
3 participants
Summary
Fixes Defect A of MCP-2123: security scans of a Docker-isolated
stdioserver added at runtime from the official registry (slash/dot names likecom.pulsemcp/google-flights) reported "No Source Available" (source_method=none), exported notools.json, and skipped Pass 2 with "No server info available".Two independent root causes, both confirmed by static trace from the issue's evidence:
1. Stale config snapshot (primary — matches all reported evidence)
configServerInfoProvideriterated a*config.Configcaptured at server boot.runtime.Config()returns a fresh immutable snapshot on every reload, so a server added at runtime (exactly the registry case) was absent from the boot snapshot →GetServerInforeturned not found → the scanner ran with noServerInfo:source_methodstayednone;/scan/source/tools.jsonmissing →tpa-descriptions/mcp-scanfailed;serverInfo == nil→ "No server info available for Pass 2, skipping".The provider now resolves against the live config (
runtime.Config), falling back to the boot snapshot when no accessor is wired.2. Container-name sanitizer mismatch (secondary)
The scanner's
docker ps --filter name=mcpproxy-<sanitized>-lookup mapped.→-, while the launcher (internal/upstream/core) preserves.(Docker allows it). So for any dotted name the filter prefix never matched the real container (mcpproxy-com.pulsemcp-google-flights-umqu) anddocker_extractsilently fell through. Both sides now shareinternal/dockernaming.SanitizeServerNameso the rule can't drift again.With (1) fixed the scan resolves
ServerInfoand (worst case) exportstools.jsonvia the existingtool_definitions_onlypath; with (2) fixeddocker_extractmatches the container and scans the real package source.Tests
internal/dockernaming:TestSanitizeServerName— locks thecom.pulsemcp/google-flights→com.pulsemcp-google-flightscase plus full parity with the launcher's prior rules.internal/server:TestConfigServerInfoProvider_UsesLiveConfig/_FallsBackToSnapshot— regression guard for the stale-snapshot bug.internal/upstream/coresanitizer tests still pass (behavior preserved via delegation).Verification
go build ./...✅go test -race ./internal/dockernaming/... ./internal/security/scanner/... ./internal/upstream/core/... ./internal/server/...✅./scripts/run-linter.sh→0 issues✅gofmt -lclean,go vetclean ✅Scope
internal/only.encodeURIComponent-ing the scan id) → separate FrontendEngineer child issue.No public API/CLI/config contract change; no docs change required (restores the behavior
source_methoddocs already describe).Related #MCP-2123