Skip to content

Commit c033de6

Browse files
authored
Enforce a trusted host-path mount policy before launching container MCP servers (#10928)
Container-backed MCP servers were launched with whatever mount arguments upstream configuration supplied, validated only syntactically (`source:dest:mode` shape). 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)** - **Typed policy boundary**: `MountPolicy` / `MountRoot` (root path + explicit `Writable` flag) is owned by the `Launcher` and never derived from MCP server configuration. - **Default-deny allowlist**: `$GITHUB_WORKSPACE` and the gateway working directory (read-only), system temp dir (read-write, for logs/payload exchange). Operators may replace it with `MCP_GATEWAY_ALLOWED_MOUNT_ROOTS` (`path[:ro|:rw]`, comma-separated). Non-absolute entries and `/` are dropped; an empty allowlist denies all mounts. - **Structured parsing**: `-v` / `--volume` / `--volume=` are parsed into `source:dest:mode`; both paths must be absolute, only `ro`/`rw` options are accepted (`ro,rw` is rejected), and an omitted mode is treated as read-write (matching Docker). - **Canonicalization**: host sources are symlink- and `..`-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. - **Bypass rejection**: `--mount`, `--volumes-from`, `--privileged`, `--device`. **`internal/launcher/launcher.go`** - `launchStdioConnection` validates `serverCfg.Args` against the policy for container-backed servers before the process starts; the error identifies the declared mount source without leaking the resolved host path. **Docs** - New "Host Mount Policy" section in `docs/CONFIGURATION.md`; `MCP_GATEWAY_ALLOWED_MOUNT_ROOTS` added to the `README.md` / `AGENTS.md` env lists; `config.json` example mounts narrowed to allowed roots and the `--privileged` example arg removed. ## Behavior ``` $ awmg --config config.json # server mounting /etc [LAUNCHER] server "custom-app": mount "/etc" rejected: host source is outside the allowed mount roots ``` Symlink escapes are caught after canonicalization, e.g. `$GITHUB_WORKSPACE/link -> /etc` resolves 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. <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #10927
2 parents ee34630 + d9bbd28 commit c033de6

12 files changed

Lines changed: 820 additions & 9 deletions

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,7 @@ DEBUG_COLORS=0 DEBUG=* ./awmg --config config.toml
410410
- `MCP_GATEWAY_AGENT_ID` - Used by environment validation (`--validate-env`) and containerized startup checks; to enable auth set `gateway.agentId` (commonly `"${MCP_GATEWAY_AGENT_ID}"` in JSON stdin config)
411411
- `MCP_GATEWAY_API_KEY` - *Deprecated alias for `MCP_GATEWAY_AGENT_ID`*; still accepted with a deprecation warning (lower precedence when both are set). Use `MCP_GATEWAY_AGENT_ID` instead.
412412
- `MCP_GATEWAY_CONTAINER_RUNTIME` - Overrides stdio container runtime selection for JSON stdin `container` servers (`docker` default, `podman` supported)
413+
- `MCP_GATEWAY_ALLOWED_MOUNT_ROOTS` - Comma-separated allowlist of host roots (`path[:ro|:rw]`, default `ro`) that container-backed MCP servers may bind-mount. Overrides the default roots (`$GITHUB_WORKSPACE` and working directory read-only, system temp dir read-write). Enforced by the launcher immediately before container launch.
413414
- `DEBUG` - Enable debug logging (e.g., `DEBUG=*`, `DEBUG=server:*,launcher:*`)
414415
- `DEBUG_COLORS` - Control colored output (0 to disable, auto-disabled when piping)
415416
- `MCP_GATEWAY_LOG_DIR` - Log file directory (sets default for `--log-dir` flag, default: `/tmp/gh-aw/mcp-logs`)

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ Key operator-facing environment variables (see [AGENTS.md](AGENTS.md) for the fu
8383
- `MCP_GATEWAY_SESSION_TIMEOUT` — session timeout for stateful unified/routed MCP sessions (default: `6h`)
8484
- `MCP_GATEWAY_TOOL_TIMEOUT` — global tool invocation timeout fallback when JSON stdin `gateway.toolTimeout` is not set (built-in default: `60`)
8585
- `MCP_GATEWAY_CONTAINER_RUNTIME` — overrides stdio container runtime selection for JSON stdin `container` servers (`docker` default, `podman` supported)
86+
- `MCP_GATEWAY_ALLOWED_MOUNT_ROOTS` — comma-separated allowlist of host roots (`path[:ro|:rw]`) that container-backed MCP servers may bind-mount (default: `$GITHUB_WORKSPACE` and working directory read-only, system temp dir read-write)
8687
- `MCP_GATEWAY_FORCE_PUBLIC_REPOS` — when `true` (default), auto-forces `repos="public"` allow-only policy when workflow repo is public
8788
- `MCP_GATEWAY_GUARDS_MODE`, `MCP_GATEWAY_WASM_GUARDS_DIR` — default guard enforcement mode and per-server WASM guard discovery root
8889
- `MCP_GATEWAY_ALLOWONLY_SCOPE_PUBLIC`, `MCP_GATEWAY_ALLOWONLY_SCOPE_OWNER`, `MCP_GATEWAY_ALLOWONLY_SCOPE_REPO`, `MCP_GATEWAY_ALLOWONLY_MIN_INTEGRITY` — environment defaults for allow-only policy override flags

config.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@
3737
"entrypoint": "/custom/entrypoint.sh",
3838
"entrypointArgs": ["--verbose", "--debug"],
3939
"mounts": [
40-
"/host/config:/app/config:ro",
41-
"/host/data:/app/data:rw"
40+
"${PWD}/config:/app/config:ro",
41+
"/tmp/custom-app-data:/app/data:rw"
4242
],
4343
"env": {
4444
"API_KEY": "${CUSTOM_API_KEY}",

docs/CONFIGURATION.md

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,9 @@ Run `./awmg --help` for full CLI options. Selected frequently-used flags (run `.
155155

156156
- **`args`** (optional): Additional Docker runtime arguments inserted before the container image name
157157
- Array of strings passed to `<runtime> run` before the container image
158-
- Example: `["--network", "host", "--privileged"]`
158+
- Example: `["--network", "host"]`
159159
- Useful for advanced container runtime configurations
160+
- Options that would bypass the host mount policy (`--mount`, `--volumes-from`, `--privileged`, `--device`) are rejected at launch time
160161

161162
### Rootless Podman Notes
162163

@@ -168,6 +169,32 @@ When using `gateway.containerRuntime: "podman"` in containerized/Kubernetes envi
168169
- `dest` - Container path where the volume is mounted
169170
- `mode` - Either `"ro"` (read-only) or `"rw"` (read-write)
170171
- Example: `["/host/config:/app/config:ro", "/host/data:/app/data:rw"]`
172+
- **Enforced at launch time**: before a container-backed MCP server is started, the launcher independently validates every mount against a trusted host-path allowlist (see [Host mount policy](#host-mount-policy)). Mounts outside the allowed roots, symlink or `..` escapes, and read-write mounts under read-only roots are rejected with a configuration error.
173+
174+
### Host Mount Policy
175+
176+
The gateway applies a default-deny host-path policy immediately before launching a container-backed (stdio) MCP server. The allowlist is owned by the launcher and is never derived from MCP server configuration.
177+
178+
Default allowed roots:
179+
180+
- `$GITHUB_WORKSPACE` (read-only)
181+
- The gateway working directory (read-only)
182+
- The system temporary directory, e.g. `/tmp` (read-write; used for gateway logs and large payload exchange)
183+
184+
Enforcement rules:
185+
186+
- Host sources are canonicalized (symlinks and `..` components resolved) before the allowlist check.
187+
- Mounts whose canonicalized source is outside every allowed root are rejected.
188+
- Read-write mounts (`:rw`, or a declaration with no mode) are only permitted under roots explicitly marked writable.
189+
- Container runtime options that bypass structured mount declarations are rejected: `--mount`, `--volumes-from`, `--privileged`, and `--device`.
190+
191+
Operators can replace the default allowlist with `MCP_GATEWAY_ALLOWED_MOUNT_ROOTS`, a comma-separated list of `path[:ro|:rw]` entries (default `ro`), for example:
192+
193+
```bash
194+
MCP_GATEWAY_ALLOWED_MOUNT_ROOTS="/srv/workspace:ro,/var/lib/mcp-data:rw"
195+
```
196+
197+
Non-absolute entries and the filesystem root (`/`) are ignored. When the resulting allowlist is empty, all mounts are denied.
171198

172199
- **`env`** (optional): Environment variables
173200
- Set to `""` (empty string) for passthrough from host environment

internal/config/config_core.go

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,11 @@ type ServerConfig struct {
279279
// This is internal-only metadata used by the launcher for diagnostics.
280280
Containerized bool `toml:"-" json:"-"`
281281

282+
// ContainerRuntimeArgs contains only the arguments before the container image.
283+
// It is internal-only metadata used to distinguish runtime options from
284+
// arguments passed to the container process.
285+
ContainerRuntimeArgs []string `toml:"-" json:"-"`
286+
282287
// Command is the executable command (for stdio servers)
283288
Command string `toml:"command" json:"command,omitempty"`
284289

@@ -520,6 +525,9 @@ func LoadFromFile(path string) (*Config, error) {
520525
for _, serverCfg := range cfg.Servers {
521526
if IsStdioServerType(serverCfg.Type) {
522527
serverCfg.Containerized = true
528+
if runtimeArgs, ok := containerRuntimeArgs(serverCfg.Args); ok {
529+
serverCfg.ContainerRuntimeArgs = runtimeArgs
530+
}
523531
}
524532
}
525533

@@ -600,6 +608,116 @@ func LoadFromFile(path string) (*Config, error) {
600608
return &cfg, nil
601609
}
602610

611+
// containerRuntimeArgs returns the portion of a Docker-compatible command
612+
// argument list preceding the image. It understands the runtime options used
613+
// by gateway configuration; callers retain the original arguments if an image
614+
// boundary cannot be derived.
615+
func containerRuntimeArgs(args []string) ([]string, bool) {
616+
runIndex := -1
617+
for i, arg := range args {
618+
if strings.EqualFold(arg, "run") {
619+
runIndex = i
620+
break
621+
}
622+
}
623+
if runIndex == -1 {
624+
return nil, false
625+
}
626+
627+
for i := runIndex + 1; i < len(args); i++ {
628+
arg := args[i]
629+
if arg == "--" {
630+
return append([]string(nil), args[:i+1]...), i+1 < len(args)
631+
}
632+
if strings.HasPrefix(arg, "--") {
633+
if !strings.Contains(arg, "=") && containerRunOptionTakesValue[arg] {
634+
i++
635+
}
636+
continue
637+
}
638+
if strings.HasPrefix(arg, "-") {
639+
if containerRunOptionTakesValue[arg] {
640+
i++
641+
}
642+
continue
643+
}
644+
return append([]string(nil), args[:i]...), true
645+
}
646+
return nil, false
647+
}
648+
649+
var containerRunOptionTakesValue = map[string]bool{
650+
"--add-host": true,
651+
"--annotation": true,
652+
"--attach": true,
653+
"--blkio-weight": true,
654+
"--cap-add": true,
655+
"--cap-drop": true,
656+
"--cgroup-parent": true,
657+
"--cgroupns": true,
658+
"--cidfile": true,
659+
"--cpu-period": true,
660+
"--cpu-quota": true,
661+
"--cpu-shares": true,
662+
"--cpus": true,
663+
"--cpuset-cpus": true,
664+
"--cpuset-mems": true,
665+
"--detach-keys": true,
666+
"--device": true,
667+
"--dns": true,
668+
"--dns-option": true,
669+
"--dns-search": true,
670+
"--entrypoint": true,
671+
"--env": true,
672+
"--env-file": true,
673+
"--expose": true,
674+
"--gpus": true,
675+
"--group-add": true,
676+
"--health-cmd": true,
677+
"--hostname": true,
678+
"--ipc": true,
679+
"--label": true,
680+
"--label-file": true,
681+
"--log-driver": true,
682+
"--log-opt": true,
683+
"--mac-address": true,
684+
"--memory": true,
685+
"--memory-reservation": true,
686+
"--memory-swap": true,
687+
"--mount": true,
688+
"--name": true,
689+
"--network": true,
690+
"--oom-score-adj": true,
691+
"--pid": true,
692+
"--pids-limit": true,
693+
"--platform": true,
694+
"--publish": true,
695+
"--restart": true,
696+
"--rootfs": true,
697+
"--runtime": true,
698+
"--security-opt": true,
699+
"--shm-size": true,
700+
"--stop-signal": true,
701+
"--stop-timeout": true,
702+
"--sysctl": true,
703+
"--tmpfs": true,
704+
"--ulimit": true,
705+
"--user": true,
706+
"--userns": true,
707+
"--uts": true,
708+
"--volume": true,
709+
"--volumes-from": true,
710+
"--workdir": true,
711+
"-a": true,
712+
"-e": true,
713+
"-h": true,
714+
"-l": true,
715+
"-p": true,
716+
"-u": true,
717+
"-v": true,
718+
"-w": true,
719+
}
720+
603721
// logConfig is the debug logger for the config package.
604722
// Enable with DEBUG=config:* or DEBUG=*.
605723
var logConfig = logger.New("config:config")

internal/config/config_core_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,27 @@ type = "http"
3535
url = "http://localhost:9090/mcp"
3636
`
3737

38+
func TestLoadFromFileSeparatesContainerRuntimeArgs(t *testing.T) {
39+
path := writeTempTOML(t, `
40+
[servers.github]
41+
command = "docker"
42+
args = ["run", "--rm", "-e", "NO_COLOR=1", "ghcr.io/github/github-mcp-server:latest", "--privileged"]
43+
`)
44+
45+
cfg, err := LoadFromFile(path)
46+
require.NoError(t, err)
47+
assert.Equal(t, []string{"run", "--rm", "-e", "NO_COLOR=1"}, cfg.Servers["github"].ContainerRuntimeArgs)
48+
}
49+
50+
func TestContainerRuntimeArgsRecognizesValueOptions(t *testing.T) {
51+
args := []string{"run", "--ipc", "host", "--pids-limit", "100", "--volume", "/tmp:/tmp", "image:latest", "--privileged"}
52+
53+
runtimeArgs, ok := containerRuntimeArgs(args)
54+
55+
assert.True(t, ok)
56+
assert.Equal(t, args[:7], runtimeArgs)
57+
}
58+
3859
// TestLoadFromFile_FileNotFound verifies that LoadFromFile returns an error
3960
// when the specified file path does not exist.
4061
func TestLoadFromFile_FileNotFound(t *testing.T) {

internal/config/config_stdin.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,8 @@ func buildStdioServerConfigWithRuntime(name string, server *StdinServerConfig, r
639639
}
640640
args = append(args, server.Args...)
641641

642+
runtimeArgs := append([]string(nil), args...)
643+
642644
// Add container name
643645
args = append(args, server.Container)
644646

@@ -652,11 +654,12 @@ func buildStdioServerConfigWithRuntime(name string, server *StdinServerConfig, r
652654
logConfig.Printf("Configured stdio MCP server: name=%s, container=%s", name, server.Container)
653655

654656
serverCfg := &ServerConfig{
655-
Type: "stdio",
656-
Containerized: true,
657-
Command: runtimeCfg.Command,
658-
Args: args,
659-
Env: make(map[string]string),
657+
Type: "stdio",
658+
Containerized: true,
659+
ContainerRuntimeArgs: runtimeArgs,
660+
Command: runtimeCfg.Command,
661+
Args: args,
662+
Env: make(map[string]string),
660663
}
661664
applyCommonServerConfigFields(serverCfg, server)
662665
return serverCfg

internal/config/config_stdin_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -916,6 +916,7 @@ func TestBuildStdioServerConfig_WithEntrypointArgs(t *testing.T) {
916916
serveIdx := indexOf(args, "--serve")
917917
require.True(t, serveIdx >= 0, "--serve must be in args")
918918
assert.Greater(t, serveIdx, containerIdx, "--serve must appear after container name")
919+
assert.Equal(t, args[:containerIdx], result.ContainerRuntimeArgs)
919920

920921
assert.Contains(t, args, "--port")
921922
assert.Contains(t, args, "8080")

internal/config/validation_schema_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -912,5 +912,5 @@ func TestValidateJSONSchema_CustomSchemasPropertyNamesLocation(t *testing.T) {
912912

913913
err := validateJSONSchema([]byte(config))
914914
require.Error(t, err)
915-
assert.ErrorContains(t, err, "Location: /customSchemas\n Error: invalid propertyName 'Bad_Name'")
915+
assert.ErrorContains(t, err, "Error: 'Bad_Name' does not match pattern '^[a-z][a-z0-9-]*$'")
916916
}

internal/launcher/launcher.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ type Launcher struct {
5151
oidcProvider *oidc.Provider
5252
serverStartTimes map[string]time.Time // tracks when each server was successfully launched
5353
serverErrors map[string]string // tracks the most recent error per server
54+
mountPolicy MountPolicy // trusted allowlist of host paths for container mounts
5455

5556
// hookAfterFirstPoolMiss is called in GetOrLaunchForSession after the initial
5657
// session-pool miss and before the write-lock is acquired. It is nil in
@@ -109,6 +110,7 @@ func New(ctx context.Context, cfg *config.Config) *Launcher {
109110
oidcProvider: oidcProvider,
110111
serverStartTimes: make(map[string]time.Time),
111112
serverErrors: serverErrors,
113+
mountPolicy: DefaultMountPolicy(),
112114
}
113115
}
114116

@@ -265,6 +267,20 @@ func (l *Launcher) launchStdioConnection(serverID, sessionID string, serverCfg *
265267
l.logSecurityWarning(serverID, serverCfg)
266268
}
267269

270+
// Defense-in-depth: independently enforce the host-path mount policy before
271+
// the backend process is launched. Configuration may originate from other
272+
// producers or compiler versions, so syntactic validation is not sufficient.
273+
if !isDirectCommand {
274+
runtimeArgs := serverCfg.Args
275+
if serverCfg.ContainerRuntimeArgs != nil {
276+
runtimeArgs = serverCfg.ContainerRuntimeArgs
277+
}
278+
if err := l.mountPolicy.ValidateContainerArgs(runtimeArgs); err != nil {
279+
logger.LogErrorToServer(serverID, "backend", "Mount policy violation: %v", err)
280+
return nil, fmt.Errorf("server %q: %w", serverID, err)
281+
}
282+
}
283+
268284
// Log the command being executed
269285
l.logLaunchStart(serverID, sessionID, serverCfg, isDirectCommand)
270286

0 commit comments

Comments
 (0)