Skip to content

Commit 02605c7

Browse files
authored
fix(compliance): require explicit mount mode in volume mount validation (#1231)
Per MCP Gateway Specification v1.8.0 §4.1.5, volume mounts **MUST** use the `host:container:mode` 3-part format — mode is not optional. The implementation was accepting 2-part `host:container` mounts, which silently defaults to Docker's `rw` mode, violating least-privilege principles. ### Validation logic (`internal/config/rules/rules.go`, `validation_schema.go`) - `MountFormat`: changed `len(parts) < 2 || len(parts) > 3` → `len(parts) != 3`; removed optional-mode branch; mode is always validated against `ro`/`rw` - `mountPattern` regex: `^[^:]+:[^:]+(:(ro|rw))?$` → `^[^:]+:[^:]+:(ro|rw)$` ```go // Before — accepted both formats if len(parts) < 2 || len(parts) > 3 { ... } if mode != "" && mode != "ro" && mode != "rw" { ... } // After — exactly 3 parts required if len(parts) != 3 { ... } if mode != "ro" && mode != "rw" { ... } ``` ### Tests - Converted all `"valid mount without mode"` cases to `"invalid mount without mode"` (now expect error) across `rules_test.go`, `validation_test.go`, `validation_string_patterns_test.go`, `validation_schema_test.go` - Updated `config_stdin_test.go` fixtures to use valid 3-part mounts (`/tmp:/tmp:rw`, `/host:/container:ro`) > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### I tried to connect to the following addresses, but was blocked by firewall rules: > > - `example.com` > - Triggering command: `/tmp/go-build2535648412/b275/launcher.test /tmp/go-build2535648412/b275/launcher.test -test.testlogfile=/tmp/go-build2535648412/b275/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true go flate/deflate.go--64 64/pkg/tool/linu-o pull.rebase` (dns block) > - Triggering command: `/tmp/go-build2026882574/b279/launcher.test /tmp/go-build2026882574/b279/launcher.test -test.testlogfile=/tmp/go-build2026882574/b279/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 5648412/b223/cmd.test -I ker/cli-plugins/docker-buildx --gdwarf-5 --64 -o ker/cli-plugins/docker-buildx n-me�� ry=1 /opt/hostedtoolcache/go/1.25.6/x64/src/net ker/docker-init /tmp/go-build253/opt/hostedtoolcache/go/1.25.6/x64/pkg/tool/linux_amd64/compile -imultiarch x86_64-linux-gnu/tmp/go-build2026882574/b278/_pkg_.a ker/docker-init` (dns block) > - `invalid-host-that-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build2535648412/b260/config.test /tmp/go-build2535648412/b260/config.test -test.testlogfile=/tmp/go-build2535648412/b260/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true go ternal/fips140/c--64 64/pkg/tool/linu-o r` (dns block) > - Triggering command: `/tmp/go-build3414178539/b260/config.test /tmp/go-build3414178539/b260/config.test -test.testlogfile=/tmp/go-build3414178539/b260/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/cgo main x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build2026882574/b264/config.test /tmp/go-build2026882574/b264/config.test -test.testlogfile=/tmp/go-build2026882574/b264/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true se stmain.go ache/go/1.25.6/x64/pkg/tool/linux_amd64/link . ctor p=/opt/hostedtoo/tmp/go-build2784548960/b211/vet.cfg ache/go/1.25.6/x64/pkg/tool/linux_amd64/link -I 5648412/b223/cmd.test -I ker/cli-plugins/docker-buildx --gdwarf-5 --64 -o ker/cli-plugins/docker-buildx` (dns block) > - `nonexistent.local` > - Triggering command: `/tmp/go-build2535648412/b275/launcher.test /tmp/go-build2535648412/b275/launcher.test -test.testlogfile=/tmp/go-build2535648412/b275/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true go flate/deflate.go--64 64/pkg/tool/linu-o pull.rebase` (dns block) > - Triggering command: `/tmp/go-build2026882574/b279/launcher.test /tmp/go-build2026882574/b279/launcher.test -test.testlogfile=/tmp/go-build2026882574/b279/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 5648412/b223/cmd.test -I ker/cli-plugins/docker-buildx --gdwarf-5 --64 -o ker/cli-plugins/docker-buildx n-me�� ry=1 /opt/hostedtoolcache/go/1.25.6/x64/src/net ker/docker-init /tmp/go-build253/opt/hostedtoolcache/go/1.25.6/x64/pkg/tool/linux_amd64/compile -imultiarch x86_64-linux-gnu/tmp/go-build2026882574/b278/_pkg_.a ker/docker-init` (dns block) > - `slow.example.com` > - Triggering command: `/tmp/go-build2535648412/b275/launcher.test /tmp/go-build2535648412/b275/launcher.test -test.testlogfile=/tmp/go-build2535648412/b275/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true go flate/deflate.go--64 64/pkg/tool/linu-o pull.rebase` (dns block) > - Triggering command: `/tmp/go-build2026882574/b279/launcher.test /tmp/go-build2026882574/b279/launcher.test -test.testlogfile=/tmp/go-build2026882574/b279/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 5648412/b223/cmd.test -I ker/cli-plugins/docker-buildx --gdwarf-5 --64 -o ker/cli-plugins/docker-buildx n-me�� ry=1 /opt/hostedtoolcache/go/1.25.6/x64/src/net ker/docker-init /tmp/go-build253/opt/hostedtoolcache/go/1.25.6/x64/pkg/tool/linux_amd64/compile -imultiarch x86_64-linux-gnu/tmp/go-build2026882574/b278/_pkg_.a ker/docker-init` (dns block) > - `this-host-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build2535648412/b284/mcp.test /tmp/go-build2535648412/b284/mcp.test -test.testlogfile=/tmp/go-build2535648412/b284/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/c-p o ache/go/1.25.6/x-lang=go1.25 user.name` (dns block) > > If you need me to access, download, or install something from one of these locations, you can either: > > - Configure [Actions setup steps](https://gh.io/copilot/actions-setup-steps) to set up my environment, which run before the firewall is enabled > - Add the appropriate URLs or hosts to the custom allowlist in this repository's [Copilot coding agent settings](https://github.com/github/gh-aw-mcpg/settings/copilot/coding_agent) (admins only) > > </details> <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> ---- *This section details on the original issue you should resolve* <issue_title>[compliance] Compliance Gap: Volume Mount Mode Not Required by Validation</issue_title> <issue_description>## MCP Gateway Compliance Review - 2026-02-21 ## Summary Found 1 compliance issue (MUST violation) during daily review of commit `e3e8080`. ## Recent Changes Reviewed - Only commit `e3e8080` affected workflow/CI files; no changes to `internal/`, `main.go`, or core gateway logic in the last 10 commits - Full compliance audit of current codebase performed against MCP Gateway Specification v1.8.0 --- ## Critical Issues (MUST violations) ### 1. Volume Mount Mode Is Accepted As Optional But Spec Requires It **Specification Section:** 4.1.5 Volume Mounts for Stdio Servers **Deep Link:** https://github.com/github/gh-aw/blob/main/docs/src/content/docs/reference/mcp-gateway.md#415-volume-mounts-for-stdio-servers **Requirement:** > "Volume mounts MUST use the format: `"host:container:mode"`" > > "Each mount string MUST conform to the 'host:container:mode' format" > > "The mode MUST be either 'ro' (read-only) or 'rw' (read-write)" **Current State:** In `internal/config/rules/rules.go:123-131`, the function is documented and validated as accepting **either** the 2-part `"source:dest"` format **or** the 3-part `"source:dest:mode"` format: ```go // MountFormat validates a mount specification in the format "source:dest" or "source:dest:mode" // ... // - Mode (if provided) MUST be either "ro" (read-only) or "rw" (read-write) func MountFormat(mount, jsonPath string, index int) *ValidationError { parts := strings.Split(mount, ":") if len(parts) < 2 || len(parts) > 3 { // accepts 2 OR 3 parts ... } ... // Validate mode if provided (mode treated as optional) if mode != "" && mode != "ro" && mode != "rw" { ``` This behavior is tested and confirmed in multiple test files: - `internal/config/rules/rules_test.go:178`: `"valid mount without mode"` passes validation - `internal/config/validation_test.go:313`: `"valid mount without mode"` passes validation - `internal/config/validation_string_patterns_test.go:139`: same - `internal/config/validation_schema_test.go:395`: same **Gap:** The specification in Section 4.1.5 unambiguously requires the 3-part `"host:container:mode"` format with mode being mandatory. The compliance test **T-CFG-015** ("Valid volume mount format (host:container:mode)") is defined in Section 10.1.1 and describes the required format with an explicit mode component. The implementation diverges from this by treating mode as optional, accepting `"/host:/container"` where the spec requires `"/host:/container:ro"` or `"/host:/container:rw"`. **Security Impact:** This gap has security implications: without a required mode, a user may omit mode and rely on an undocumented default. Docker defaults to `rw` for mounts without a mode, meaning read-write access may be granted when read-only was intended. The spec explicitly recommends: _"Read-only mounts ('ro') SHOULD be preferred when the server only needs to read data."_ Forcing explicit mode declaration aligns with the principle of least privilege. **Severity:** Critical (MUST violation) **File References:** - `internal/config/rules/rules.go:123-128` — comment and function signature treating mode as optional - `internal/config/rules/rules.go:131` — condition `len(parts) < 2 || len(parts) > 3` accepts 2-part format - `internal/config/rules/rules.go:178-180` — mode-only validation is skipped if empty - `internal/config/rules/rules_test.go:178-186` — "valid mount without mode" test case - `internal/config/validation_test.go:313-320` — "valid mount without mode" test case **Suggested Fix:** 1. Update `MountFormat` in `internal/config/rules/rules.go` to require exactly 3 parts: ```go // MountFormat validates a mount specification in the format "source:dest:mode" // Per MCP Gateway specification v1.7.0 section 4.1.5: // - Host path MUST be an absolute path // - Container path MUST be an absolute path // - Mode MUST be either "ro" (read-only) or "rw" (read-write) func MountFormat(mount, jsonPath string, index int) *ValidationError { parts := strings.Split(mount, ":") if len(parts) != 3 { // require exactly 3 parts return &ValidationError{ Field: "mounts", Message: fmt.Sprintf("invalid mount format '%s' (expected 'source:dest:mode')", mount), JSONPath: fmt.Sprintf("%s.mounts[%d]", jsonPath, index), Suggestion: "Use format 'source:dest:mode' where mode is 'ro' (read-only) or 'rw' (read-write), e.g. '/host/path:/container/path:ro'", } } ... ``` 2. Update the related test cases to remove "valid mount without mode" cases and instead add tests that verify 2-part mounts are **rejected**. 3. Update the error message suggestion in `rules.go:136` to only show the 3-part format. --- ## Compliance Status | Section | Status | |---------|--------|... </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #1224 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).
2 parents e3e8080 + 28da41e commit 02605c7

7 files changed

Lines changed: 22 additions & 24 deletions

File tree

internal/config/config_stdin_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ func TestConvertStdinServerConfig_StdioServer(t *testing.T) {
1616
Entrypoint: "/bin/custom",
1717
EntrypointArgs: []string{"arg1", "arg2"},
1818
Args: []string{"--network", "host"},
19-
Mounts: []string{"/tmp:/tmp"},
19+
Mounts: []string{"/tmp:/tmp:rw"},
2020
Env: map[string]string{
2121
"TEST_VAR": "value123",
2222
"PASSTHROUGH": "",
@@ -46,7 +46,7 @@ func TestConvertStdinServerConfig_StdioServer(t *testing.T) {
4646

4747
// Check mounts
4848
assert.Contains(t, result.Args, "-v")
49-
assert.Contains(t, result.Args, "/tmp:/tmp")
49+
assert.Contains(t, result.Args, "/tmp:/tmp:rw")
5050

5151
// Check user environment variables
5252
assert.Contains(t, result.Args, "TEST_VAR=value123")
@@ -380,7 +380,7 @@ func TestConvertStdinServerConfig_ArgsOrdering(t *testing.T) {
380380
Entrypoint: "/custom/entry",
381381
EntrypointArgs: []string{"entry-arg1"},
382382
Args: []string{"--custom-flag", "value"},
383-
Mounts: []string{"/host:/container"},
383+
Mounts: []string{"/host:/container:ro"},
384384
Env: map[string]string{
385385
"MY_VAR": "my-value",
386386
},

internal/config/rules/rules.go

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -120,31 +120,28 @@ func TimeoutPositive(timeout int, fieldName, jsonPath string) *ValidationError {
120120
return nil
121121
}
122122

123-
// MountFormat validates a mount specification in the format "source:dest" or "source:dest:mode"
123+
// MountFormat validates a mount specification in the format "source:dest:mode"
124124
// Returns nil if valid, *ValidationError if invalid
125-
// Per MCP Gateway specification v1.7.0 section 4.1.5:
125+
// Per MCP Gateway specification v1.8.0 section 4.1.5:
126126
// - Host path MUST be an absolute path
127127
// - Container path MUST be an absolute path
128-
// - Mode (if provided) MUST be either "ro" (read-only) or "rw" (read-write)
128+
// - Mode MUST be either "ro" (read-only) or "rw" (read-write)
129129
func MountFormat(mount, jsonPath string, index int) *ValidationError {
130130
log.Printf("Validating mount format: mount=%s, jsonPath=%s, index=%d", mount, jsonPath, index)
131131
parts := strings.Split(mount, ":")
132-
if len(parts) < 2 || len(parts) > 3 {
132+
if len(parts) != 3 {
133133
log.Printf("Mount format validation failed: invalid part count=%d", len(parts))
134134
return &ValidationError{
135135
Field: "mounts",
136-
Message: fmt.Sprintf("invalid mount format '%s' (expected 'source:dest' or 'source:dest:mode')", mount),
136+
Message: fmt.Sprintf("invalid mount format '%s' (expected 'source:dest:mode')", mount),
137137
JSONPath: fmt.Sprintf("%s.mounts[%d]", jsonPath, index),
138-
Suggestion: "Use format 'source:dest' or 'source:dest:mode' where mode is 'ro' (read-only) or 'rw' (read-write)",
138+
Suggestion: "Use format 'source:dest:mode' where mode is 'ro' (read-only) or 'rw' (read-write), e.g. '/host/path:/container/path:ro'",
139139
}
140140
}
141141

142142
source := parts[0]
143143
dest := parts[1]
144-
mode := ""
145-
if len(parts) == 3 {
146-
mode = parts[2]
147-
}
144+
mode := parts[2]
148145

149146
// Validate source is not empty
150147
if source == "" {
@@ -186,8 +183,8 @@ func MountFormat(mount, jsonPath string, index int) *ValidationError {
186183
}
187184
}
188185

189-
// Validate mode if provided
190-
if mode != "" && mode != "ro" && mode != "rw" {
186+
// Validate mode
187+
if mode != "ro" && mode != "rw" {
191188
return &ValidationError{
192189
Field: "mounts",
193190
Message: fmt.Sprintf("invalid mount mode '%s' (must be 'ro' or 'rw')", mode),

internal/config/rules/rules_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,12 @@ func TestMountFormat(t *testing.T) {
175175
shouldErr: false,
176176
},
177177
{
178-
name: "valid mount without mode",
178+
name: "invalid mount without mode",
179179
mount: "/host/path:/container/path",
180180
jsonPath: "mcpServers.github",
181181
index: 0,
182-
shouldErr: false,
182+
shouldErr: true,
183+
errMsg: "invalid mount format",
183184
},
184185
{
185186
name: "invalid format - too many colons",

internal/config/validation_schema.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ var (
2020
// Compile regex patterns from schema for additional validation
2121
containerPattern = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9./_-]*(:([a-zA-Z0-9._-]+|latest))?$`)
2222
urlPattern = regexp.MustCompile(`^https?://.+`)
23-
mountPattern = regexp.MustCompile(`^[^:]+:[^:]+(:(ro|rw))?$`)
23+
mountPattern = regexp.MustCompile(`^[^:]+:[^:]+:(ro|rw)$`)
2424
domainVarPattern = regexp.MustCompile(`^\$\{[A-Z_][A-Z0-9_]*\}$`)
2525

2626
// logSchema is the debug logger for schema validation

internal/config/validation_schema_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ func TestValidateStringPatterns(t *testing.T) {
392392
shouldErr: false,
393393
},
394394
{
395-
name: "valid mount without mode",
395+
name: "invalid mount without mode",
396396
config: &StdinConfig{
397397
MCPServers: map[string]*StdinServerConfig{
398398
"test": {
@@ -402,7 +402,7 @@ func TestValidateStringPatterns(t *testing.T) {
402402
},
403403
},
404404
},
405-
shouldErr: false,
405+
shouldErr: true,
406406
},
407407
{
408408
name: "valid http url pattern",

internal/config/validation_string_patterns_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,9 @@ func TestValidateStringPatternsComprehensive(t *testing.T) {
136136
shouldError: false,
137137
},
138138
{
139-
name: "valid mount without mode",
139+
name: "invalid mount without mode",
140140
mounts: []string{"/host/path:/container/path"},
141-
shouldError: false,
141+
shouldError: true,
142142
},
143143
{
144144
name: "valid multiple mounts",

internal/config/validation_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -310,13 +310,13 @@ func TestValidateStdioServer(t *testing.T) {
310310
shouldErr: false,
311311
},
312312
{
313-
name: "valid mount without mode",
313+
name: "invalid mount without mode",
314314
server: &StdinServerConfig{
315315
Type: "stdio",
316316
Container: "test:latest",
317317
Mounts: []string{"/host:/container"},
318318
},
319-
shouldErr: false,
319+
shouldErr: true,
320320
},
321321
{
322322
name: "invalid mount format - too many parts",

0 commit comments

Comments
 (0)