Enforce a trusted host-path mount policy before launching container MCP servers - #10928
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Adds runtime enforcement of trusted host-path mount policies for container-backed MCP servers.
Changes:
- Adds mount parsing, canonicalization, allowlisting, and bypass prevention.
- Enforces policy immediately before container launch.
- Adds tests, configuration examples, and operator documentation.
Show a summary per file
| File | Description |
|---|---|
internal/launcher/mount_policy.go |
Implements mount-policy validation. |
internal/launcher/mount_policy_test.go |
Tests policy enforcement and launch rejection. |
internal/launcher/launcher.go |
Enforces policy before process launch. |
docs/CONFIGURATION.md |
Documents host-mount policy behavior. |
README.md |
Lists the policy environment variable. |
AGENTS.md |
Documents the environment variable for contributors. |
config.json |
Updates example mounts to allowed roots. |
Review details
Tip
Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Suppressed comments (1)
internal/launcher/mount_policy.go:276
- The short volume flag can carry its value in the same token (for example,
-v/etc:/host-etc:ro), but this switch only recognizes an exact-v. Docker/Podman parse the attached form as a bind mount while this validator falls through, allowing a host path outside the policy roots. Parse non-empty text attached to-vand cover this form in the bypass tests.
case strings.HasPrefix(arg, "--volume="):
spec = strings.TrimPrefix(arg, "--volume=")
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Balanced
| var mountBypassOptions = map[string]string{ | ||
| "--mount": "use the structured 'mounts' field ('source:dest:mode') instead of --mount", | ||
| "--volumes-from": "volumes cannot be inherited from other containers", | ||
| "--privileged": "privileged containers can access arbitrary host devices", | ||
| "--device": "host devices cannot be exposed to MCP servers", | ||
| } |
| func (p MountPolicy) ValidateContainerArgs(args []string) error { | ||
| for i := 0; i < len(args); i++ { | ||
| arg := args[i] |
This comment has been minimized.
This comment has been minimized.
|
@copilot address review feedback and fix this failing ci check https://github.com/github/gh-aw-mcpg/actions/runs/31323340276/job/93271591061?pr=10928 |
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Addressed in d9bbd28: Podman |
🔒 mcpg Read-Only Stress — default AWFSurface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Overall: PASS
References: §31325088063
|
🔒 mcpg Read-Only Stress — docker-sbxSurface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Overall: PASS
References: §31325088103
|
🔒 mcpg Read-Only Stress — gVisorSurface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Gateway enforcement detail (Part B): The gateway serves a 22-tool read-only manifest. All 7 write tool calls returned MCP JSON-RPC error CLI/GraphQL (Parts D/E): Overall: PASS References: §31325088062
|
Container-backed MCP servers were launched with whatever mount arguments upstream configuration supplied, validated only syntactically (
source:dest:modeshape). Since the gateway is the component that actually starts the backend process, it should independently decide which host paths may be exposed — configurations can originate from other producers or older compiler versions.Changes
internal/launcher/mount_policy.go(new)MountPolicy/MountRoot(root path + explicitWritableflag) is owned by theLauncherand never derived from MCP server configuration.$GITHUB_WORKSPACEand the gateway working directory (read-only), system temp dir (read-write, for logs/payload exchange). Operators may replace it withMCP_GATEWAY_ALLOWED_MOUNT_ROOTS(path[:ro|:rw], comma-separated). Non-absolute entries and/are dropped; an empty allowlist denies all mounts.-v/--volume/--volume=are parsed intosource:dest:mode; both paths must be absolute, onlyro/rwoptions are accepted (ro,rwis rejected), and an omitted mode is treated as read-write (matching Docker)...-resolved before the containment check, resolving the longest existing ancestor so not-yet-created leaf directories still validate. Roots are ordered most-specific-first, so a read-only root nested in a writable one narrows access rather than inheriting it.--mount,--volumes-from,--privileged,--device.internal/launcher/launcher.golaunchStdioConnectionvalidatesserverCfg.Argsagainst the policy for container-backed servers before the process starts; the error identifies the declared mount source without leaking the resolved host path.Docs
docs/CONFIGURATION.md;MCP_GATEWAY_ALLOWED_MOUNT_ROOTSadded to theREADME.md/AGENTS.mdenv lists;config.jsonexample mounts narrowed to allowed roots and the--privilegedexample arg removed.Behavior
Symlink escapes are caught after canonicalization, e.g.
$GITHUB_WORKSPACE/link -> /etcresolves outside the workspace root and is rejected. Writable mounts under a read-only root ($GITHUB_WORKSPACE:/workspace:rw) are rejected as well.Tests cover allowed workspace/temp mounts, disallowed host paths, symlink and traversal escapes, malformed declarations, env-override precedence, runtime-argument bypasses, and end-to-end rejection before process launch.