feat(security): scanner-flow parity under sandbox isolation + docs (MCP-34.4, closes #71)#781
Merged
Merged
Conversation
D3 option (b) from the MCP-34 spike: clean, surfaced degradation. Under a non-Docker isolation mode (sandbox/none) the Docker-based scanner plugins (Spec 039) cannot run, so the scan engine now skips them with an honest, mode-specific reason pointing at MCPX_DOCKER_SNAP_APPARMOR instead of the misleading "pull the image" guidance. The always-on in-process tpa-descriptions scanner still runs, and the skipped Docker scanners stay in the resolved set (recorded as failed) so the existing coverage check downgrades the server's security_scan.status to "degraded" rather than a silent all-clear. - Engine: new isolationMode gate in resolveScanners/checkImage - Service: SetIsolationMode setter (+ startup WARN), wired in server.go from cfg.DockerIsolation.ResolvedMode() - Tests: engine skip under sandbox/none + docker-mode back-compat guard + service setter propagation - Docs: rewrite docs/docker-isolation.md to cover all three modes (docker/ sandbox/none), the snap-docker AppArmor failure mode, the honest uid/gid limitation, and a platform support matrix; add sandbox option to the MCPX_DOCKER_SNAP_APPARMOR error doc Co-Authored-By: Paperclip <noreply@paperclip.ing>
Closed
Deploying mcpproxy-docs with
|
| Latest commit: |
3f12faf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b362cc6e.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://feat-mcp-3235-scanner-sandbo.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 28346333806 --repo smart-mcp-proxy/mcpproxy-go
|
CI build/Cloudflare-Pages failed: onBrokenLinks:throw flagged the error doc's link to ../docker-isolation.md, which the website does not serve (its `include` list only builds features/**, errors/**, etc. — not the top-level docs/docker-isolation.md). Port the Isolation Modes / Sandbox / Scanner- behaviour / Honest-limitations / Platform-matrix content into the served docs/features/docker-isolation.md (the user-facing isolation page) and repoint the MCPX_DOCKER_SNAP_APPARMOR link to ../features/docker-isolation.md. Verified: website `npm run build` succeeds locally (no broken links). Co-Authored-By: Paperclip <noreply@paperclip.ing>
…(MCP-34.4) CodexReviewer correctness finding on #781: the scanner skip was wired from a single service-wide isolation mode (cfg.DockerIsolation.ResolvedMode()), but isolation resolves per-server — a per-server isolation.mode overrides the global (internal/upstream/core IsolationManager). So a server pinned to isolation.mode:docker would wrongly have its Docker scanners skipped under a global sandbox default, and vice versa, contradicting the per-server design and the served docs. Thread the scanned server's RESOLVED mode through the scan: - ScanRequest.IsolationMode carries the per-server mode; engine.StartScan uses effectiveIsolationMode(req) (per-server wins, else engine-wide default) and passes it to resolveScanners(ids, mode). - Service.SetIsolationModeResolver injects a per-server resolver; StartScan (both passes) populates req.IsolationMode via resolveIsolationMode(server). - server.go wires the resolver from core.IsolationManager.ResolveMode against the live per-server config; SetIsolationMode stays as the engine-wide default fallback. Scanner package stays decoupled from the resolver. Tests: per-server override cases (docker under global sandbox runs Docker scanners; sandbox/none under global docker skips), effectiveIsolationMode precedence, and Service.resolveIsolationMode fallback. go test -race + golangci-lint v2 green. Co-Authored-By: Paperclip <noreply@paperclip.ing>
Member
Author
GeminiCritic Review (fallback — MCP-3719)ACCEPT — head SummaryPR correctly implements MCP-34.4 D3 option (b): clean, surfaced degradation of Docker-based scanner plugins under What's correct
Minor observations (not blocking)
No correctness issues, no security regressions. Approve and ship. |
![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).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements MCP-34.4 — scanner-flow parity under
sandboxisolation mode, per the MCP-34 spike's decision D3 option (b): clean, surfaced degradation. Resolves #71 (snap-docker AppArmor blocks the Docker scanner).The security scanner plugins (Spec 039) are Docker-based and are the broken path on snap-docker / non-Docker hosts. Rather than fail silently (or print the misleading "pull the image locally" guidance), the scan engine now detects a non-Docker isolation mode and degrades cleanly:
isolation.mode: sandboxornone, Docker-based scanner plugins are skipped with an honest, mode-specific reason that points atMCPX_DOCKER_SNAP_APPARMOR.tpa-descriptionsscanner still runs.security_scan.statustodegraded— a low/zero risk score from incomplete coverage is never reported as a trustworthy all-clear.WARNlog surfaces the mode.A native (non-Docker) scanner runtime — D3 option (a) — is intentionally deferred as the larger follow-up the spike calls out.
Changes
Code
internal/security/scanner/engine.go— newisolationModegate inresolveScanners/checkImage.internal/security/scanner/service.go—SetIsolationMode(mode)setter + startup warn.internal/server/server.go— wire fromcfg.DockerIsolation.ResolvedMode().Tests (TDD, all green +
-race)TestEngineResolveScannersSkipsDockerUnderSandbox(sandbox + none): Docker scanner skipped with mode-named, MCPX_DOCKER_SNAP_APPARMOR-referencing reason; no "docker pull" guidance; in-process scanner still runs.TestEngineResolveScannersDockerModeUnaffected(""/docker): back-compat guard.TestServiceSetIsolationMode: setter propagates to engine.Docs
docs/docker-isolation.md— retitled to Security Isolation (Docker · Sandbox · None); new Isolation Modes section (three modes + legacyenabledback-compat + per-server precedence), Sandbox mode (Landlock), Scanner behaviour under each mode, Snap-docker (AppArmor) failure mode, Honest limitations (no uid/gid drop without privilege, Linux-only, FS+resources only), and a Platform support matrix.docs/errors/MCPX_DOCKER_SNAP_APPARMOR.md— added thesandbox-mode option (now four fixes).Verification
go build ./internal/... ./cmd/...✅go test -race ./internal/security/scanner/✅golangci-lint run --config .github/.golangci.yml ./internal/security/scanner/... ./internal/server/...→ 0 issues ✅Scope notes
origin/main(includes the MCP-34.3 sandbox launcher feat(upstream): native sandbox launcher (Landlock + rlimits) for stdio servers (MCP-34.3) #768).docker_isolation.modefrom MCP-34.2).Co-Authored-By: Paperclip noreply@paperclip.ing