Default host path binds to read-only

docs/server-entry-spec.md

@@ -38,10 +38,16 @@ allowHosts:
 |-------|------|----------|-------------|
 | `image` | string | Yes* | Docker image reference (can be SHA256 digest or tag). Required for `server` type. |
 | `command` | []string | No | Command-line arguments to pass to the container. |
-| `volumes` | []string | No | Volume mount specifications (format: `host:container` or `host:container:ro`). |
+| `volumes` | []string | No | Volume mount specifications (format: `host:container`, `host:container:ro`, or `host:container:rw`). |
 | `user` | string | No | User to run the container as (e.g., `1000:1000`). |
 | `longLived` | boolean | No | Whether the server should remain running (true) or start on-demand (false). Default: false. |
 
+#### Host Bind Mount Safety
+
+Host path bind mounts default to read-only when no mode is specified. Explicit writable host path bind mounts, and no-mode bind mounts that should become writable, are rejected unless the resolved host source exactly matches an entry in `MCP_GATEWAY_DOCKER_BIND_ALLOW_WRITABLE_PATHS`. Use the OS path-list separator when setting multiple entries.
+
+Setting `MCP_GATEWAY_DOCKER_BIND_ALLOW_WRITABLE_PATHS` means any catalog or profile you load can request that exact host path as writable. Scope writable entries to dedicated data directories, not source trees or broad project directories.
+
 ### Remote Configuration (for type: "remote")
 
 | Field | Type | Required | Description |
@@ -369,4 +375,3 @@ icon: https://www.cloudflare.com/favicon.ico
    - Use `allowHosts` to restrict network access
    - Use `disableNetwork: true` for tools that don't need network
    - Always use secrets for credentials, never hardcode in `env`
-

pkg/gateway/clientpool_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -157,6 +158,66 @@ volumes:
 	assert.Empty(t, env)
 }
 
+func TestApplyConfigVolumeFilterDefaultsHostBindsToReadOnly(t *testing.T) {
+	hostPath := t.TempDir()
+	expectedHostPath, err := cleanDockerHostPath(hostPath)
+	require.NoError(t, err)
+	catalogYAML := `
+volumes:
+  - '{{filesystem.paths|volume|into}}'
+  `
+	configYAML := fmt.Sprintf(`
+filesystem:
+  paths:
+    - %s
+`, hostPath)
+
+	args, env := argsAndEnv(t, "filesystem", catalogYAML, configYAML, nil)
+
+	mountIndex := -1
+	for i, arg := range args {
+		if arg == "-v" {
+			mountIndex = i + 1
+			break
+		}
+	}
+	require.NotEqual(t, -1, mountIndex)
+	require.Less(t, mountIndex, len(args))
+	assert.True(t, strings.HasPrefix(args[mountIndex], expectedHostPath+":"))
+	assert.True(t, strings.HasSuffix(args[mountIndex], ":ro"))
+	assert.Empty(t, env)
+}
+
+func TestApplyConfigVolumeFilterAllowsWritableHostBindOverride(t *testing.T) {
+	hostPath := t.TempDir()
+	expectedHostPath, err := cleanDockerHostPath(hostPath)
+	require.NoError(t, err)
+	t.Setenv(dockerBindWritableAllowedPathsEnv, hostPath)
+	catalogYAML := `
+volumes:
+  - '{{filesystem.paths|volume|into}}'
+  `
+	configYAML := fmt.Sprintf(`
+filesystem:
+  paths:
+    - %s
+`, hostPath)
+
+	args, env := argsAndEnv(t, "filesystem", catalogYAML, configYAML, nil)
+
+	mountIndex := -1
+	for i, arg := range args {
+		if arg == "-v" {
+			mountIndex = i + 1
+			break
+		}
+	}
+	require.NotEqual(t, -1, mountIndex)
+	require.Less(t, mountIndex, len(args))
+	assert.Equal(t, expectedHostPath+":"+filepath.ToSlash(hostPath)+":rw", args[mountIndex])
+	assert.Empty(t, env)
+}
+
 func TestApplyConfigMountAsReadOnly(t *testing.T) {
 	hostPath := t.TempDir()
 	expectedHostPath, err := cleanDockerHostPath(hostPath)
@@ -222,7 +283,7 @@ func TestApplyConfigLongLivedRejectsWritableHostBind(t *testing.T) {
 	catalogYAML := `
 longLived: true
 volumes:
-  - '/local/data:/data'
+  - '/local/data:/data:rw'
   `
 
 	_, _, err := argsAndEnvErr(t, "longlived", catalogYAML, "", nil)
@@ -232,7 +293,7 @@ volumes:
 func TestApplyConfigShortLivedRejectsWritableHostBind(t *testing.T) {
 	catalogYAML := `
 volumes:
-  - '/local/data:/data'
+  - '/local/data:/data:rw'
   `
 
 	_, _, err := argsAndEnvErr(t, "shortlived", catalogYAML, "", nil)
@@ -310,11 +371,48 @@ func TestValidateDockerVolumeBinds(t *testing.T) {
 		require.NoError(t, validateDockerVolumeBinds([]string{t.TempDir() + ":/data:ro"}))
 	})
 
+	t.Run("defaults host binds without mode to read-only", func(t *testing.T) {
+		hostPath := t.TempDir()
+		expectedSource, err := cleanDockerHostPath(hostPath)
+		require.NoError(t, err)
+
+		normalized, err := normalizeDockerVolumeBind(hostPath + ":/data")
+		require.NoError(t, err)
+		require.Equal(t, expectedSource+":/data:ro", normalized)
+	})
+
 	t.Run("allows read-only binds from configured roots", func(t *testing.T) {
 		t.Setenv(dockerBindAllowedPathsEnv, "/opt/mcp-data")
 		require.NoError(t, validateDockerVolumeBinds([]string{"/opt/mcp-data/project:/data:ro"}))
 	})
 
+	t.Run("allows writable binds from configured writable paths", func(t *testing.T) {
+		hostPath := t.TempDir()
+		expectedSource, err := cleanDockerHostPath(hostPath)
+		require.NoError(t, err)
+		t.Setenv(dockerBindWritableAllowedPathsEnv, hostPath)
+
+		require.NoError(t, validateDockerVolumeBinds([]string{hostPath + ":/data:rw"}))
+
+		normalized, err := normalizeDockerVolumeBind(hostPath + ":/data")
+		require.NoError(t, err)
+		require.Equal(t, expectedSource+":/data:rw", normalized)
+
+		normalized, err = normalizeDockerVolumeBind(hostPath + ":/data:rw")
+		require.NoError(t, err)
+		require.Equal(t, expectedSource+":/data:rw", normalized)
+	})
+
+	t.Run("configured writable paths do not allow child paths", func(t *testing.T) {
+		root := t.TempDir()
+		project := filepath.Join(root, "project")
+		require.NoError(t, os.MkdirAll(project, 0o755))
+		t.Setenv(dockerBindWritableAllowedPathsEnv, root)
+
+		err := validateDockerVolumeBinds([]string{project + ":/data:rw"})
+		require.ErrorContains(t, err, "host path bind mounts must be read-only")
+	})
+
 	t.Run("allows home paths from configured roots", func(t *testing.T) {
 		home := t.TempDir()
 		project := filepath.Join(home, "trusted", "project")
@@ -333,27 +431,30 @@ func TestValidateDockerVolumeBinds(t *testing.T) {
 	t.Run("rejects host paths outside allowed roots", func(t *testing.T) {
 		err := validateDockerVolumeBinds([]string{"/opt/mcp-data:/data:ro"})
 		require.ErrorContains(t, err, "outside allowed roots")
+		require.ErrorContains(t, err, "MCP_GATEWAY_DOCKER_BIND_ALLOWED_PATHS=/opt/mcp-data")
+		require.ErrorContains(t, err, "MCP_GATEWAY_DOCKER_BIND_ALLOW_WRITABLE_PATHS=/opt/mcp-data")
 	})
 
 	t.Run("rejects relative host paths with slash", func(t *testing.T) {
 		err := validateDockerVolumeBinds([]string{"foo/bar:/data"})
-		require.ErrorContains(t, err, "host path bind mounts must be read-only")
+		require.ErrorContains(t, err, "outside allowed roots")
 
 		err = validateDockerVolumeBinds([]string{"foo/bar:/data:ro"})
 		require.ErrorContains(t, err, "outside allowed roots")
 	})
 
 	t.Run("rejects relative host paths escaping upward", func(t *testing.T) {
 		err := validateDockerVolumeBinds([]string{"subdir/../../../etc:/data"})
-		require.ErrorContains(t, err, "host path bind mounts must be read-only")
+		require.ErrorContains(t, err, "outside allowed roots")
 
 		err = validateDockerVolumeBinds([]string{"subdir/../../../etc:/data:ro"})
 		require.ErrorContains(t, err, "outside allowed roots")
 	})
 
-	t.Run("rejects writable host path binds", func(t *testing.T) {
-		err := validateDockerVolumeBinds([]string{t.TempDir() + ":/data"})
+	t.Run("rejects explicitly writable host path binds", func(t *testing.T) {
+		err := validateDockerVolumeBinds([]string{t.TempDir() + ":/data:rw"})
 		require.ErrorContains(t, err, "host path bind mounts must be read-only")
+		require.ErrorContains(t, err, "MCP_GATEWAY_DOCKER_BIND_ALLOW_WRITABLE_PATHS=")
 	})
 
 	t.Run("rejects docker socket binds", func(t *testing.T) {
@@ -376,6 +477,33 @@ func TestValidateDockerVolumeBinds(t *testing.T) {
 		err := validateDockerVolumeBinds([]string{filepath.Join(t.TempDir(), ".ssh") + ":/ssh:ro"})
 		require.ErrorContains(t, err, "credential path")
 	})
+
+	t.Run("rejects credential paths even under writable paths", func(t *testing.T) {
+		sshPath := filepath.Join(t.TempDir(), ".ssh")
+		t.Setenv(dockerBindWritableAllowedPathsEnv, sshPath)
+
+		err := validateDockerVolumeBinds([]string{sshPath + ":/ssh:rw"})
+		require.ErrorContains(t, err, "credential path")
+	})
+
+	t.Run("rejects symlink system paths even under writable paths", func(t *testing.T) {
+		root := t.TempDir()
+		link := filepath.Join(root, "etc-link")
+		if err := os.Symlink("/etc", link); err != nil {
+			t.Skipf("cannot create symlink: %v", err)
+		}
+		t.Setenv(dockerBindWritableAllowedPathsEnv, link)
+
+		err := validateDockerVolumeBinds([]string{filepath.ToSlash(link) + ":/data:rw"})
+		require.ErrorContains(t, err, "sensitive system path")
+	})
+
+	t.Run("rejects docker socket even under writable paths", func(t *testing.T) {
+		t.Setenv(dockerBindWritableAllowedPathsEnv, "/var/run/docker.sock")
+
+		err := validateDockerVolumeBinds([]string{"/var/run/docker.sock:/var/run/docker.sock:rw"})
+		require.ErrorContains(t, err, "blocked")
+	})
 }
 
 func argsAndEnv(t *testing.T, name, catalogYAML, configYAML string, secrets map[string]string) ([]string, []string) {

pkg/gateway/docker_binds.go

@@ -17,6 +17,10 @@ import (
 // default read-only temp-root allowlist. Use the OS path-list separator.
 const dockerBindAllowedPathsEnv = "MCP_GATEWAY_DOCKER_BIND_ALLOWED_PATHS"
 
+// MCP_GATEWAY_DOCKER_BIND_ALLOW_WRITABLE_PATHS adds trusted host paths that may
+// be mounted writable. Use the OS path-list separator.
+const dockerBindWritableAllowedPathsEnv = "MCP_GATEWAY_DOCKER_BIND_ALLOW_WRITABLE_PATHS"
+
 type dockerVolumeBind struct {
 	raw        string
 	source     string
@@ -29,35 +33,46 @@ type dockerVolumeBind struct {
 
 func validateDockerVolumeBinds(volumes []string) error {
 	allowedRoots := dockerBindAllowedRoots()
+	writableAllowedPaths := dockerBindWritableAllowedPaths()
 	for _, raw := range volumes {
-		if _, err := normalizeDockerVolumeBindWithRoots(raw, allowedRoots); err != nil {
+		if _, err := normalizeDockerVolumeBindWithRoots(raw, allowedRoots, writableAllowedPaths); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
 func normalizeDockerVolumeBind(raw string) (string, error) {
-	return normalizeDockerVolumeBindWithRoots(raw, dockerBindAllowedRoots())
+	return normalizeDockerVolumeBindWithRoots(raw, dockerBindAllowedRoots(), dockerBindWritableAllowedPaths())
 }
 
-func normalizeDockerVolumeBindWithRoots(raw string, allowedRoots []string) (string, error) {
+func normalizeDockerVolumeBindWithRoots(raw string, allowedRoots, writableAllowedPaths []string) (string, error) {
 	bind, err := parseDockerVolumeBind(raw)
 	if err != nil {
 		return "", err
 	}
 	if !bind.hostPath {
 		return bind.raw, nil
 	}
-	if !bind.readOnly {
-		return "", fmt.Errorf("unsafe docker volume %q: host path bind mounts must be read-only", bind.raw)
-	}
 	if disallowed, reason := disallowedDockerHostPath(bind.sourcePath); disallowed {
 		return "", fmt.Errorf("unsafe docker volume %q: host path %q is blocked (%s)", bind.raw, bind.source, reason)
 	}
-	if !isPathUnderAnyRoot(bind.sourcePath, allowedRoots) {
-		return "", fmt.Errorf("unsafe docker volume %q: host path %q is outside allowed roots %s",
-			bind.raw, bind.source, strings.Join(allowedRoots, ", "))
+	writableAllowed := slices.Contains(writableAllowedPaths, bind.sourcePath)
+	if bind.mode == "" {
+		if writableAllowed {
+			bind.mode = "rw"
+		} else {
+			bind.mode = "ro"
+			bind.readOnly = true
+		}
+	}
+	if !bind.readOnly && !writableAllowed {
+		return "", fmt.Errorf("unsafe docker volume %q: host path bind mounts must be read-only unless the exact host path is allowed for writable binds. To allow this writable host path, run with %s=%s",
+			bind.raw, dockerBindWritableAllowedPathsEnv, bind.source)
+	}
+	if !isPathUnderAnyRoot(bind.sourcePath, allowedRoots) && !slices.Contains(writableAllowedPaths, bind.sourcePath) {
+		return "", fmt.Errorf("unsafe docker volume %q: host path %q is outside allowed roots %s. To allow read-only access, run with %s=%s or another trusted parent directory. To allow writable access, run with %s=%s",
+			bind.raw, bind.source, strings.Join(allowedRoots, ", "), dockerBindAllowedPathsEnv, bind.source, dockerBindWritableAllowedPathsEnv, bind.source)
 	}
 	return bind.sourcePath + ":" + bind.target + ":" + bind.mode, nil
 }
@@ -258,7 +273,17 @@ func dockerBindAllowedRoots() []string {
 	if env := os.Getenv(dockerBindAllowedPathsEnv); env != "" {
 		roots = append(roots, filepath.SplitList(env)...)
 	}
+	return cleanDockerBindRoots(roots)
+}
+
+func dockerBindWritableAllowedPaths() []string {
+	if env := os.Getenv(dockerBindWritableAllowedPathsEnv); env != "" {
+		return cleanDockerBindRoots(filepath.SplitList(env))
+	}
+	return nil
+}
 
+func cleanDockerBindRoots(roots []string) []string {
 	out := make([]string, 0, len(roots))
 	for _, root := range roots {
 		root = strings.TrimSpace(root)