Skip to content

Commit 5ed2c30

Browse files
DumbrisPaperclip-Paperclip
Dumbris
and
Paperclip-Paperclip
authored
feat(config): isolation.mode enum + mode-aware selector (MCP-34.2) (#759)
* feat(config): add isolation.mode enum + mode-aware selector (MCP-34.2) Introduce a three-way isolation mode (docker | sandbox | none) on both the global DockerIsolationConfig and per-server IsolationConfig, laying the config foundation for native (non-Docker) sandbox isolation (MCP-34). - New `config.IsolationMode` type + constants and `IsValid()` helper. - Global `DockerIsolationConfig.Mode` with `ResolvedMode()` back-compat mapping: explicit Mode wins; else legacy enabled:true ⇒ docker, enabled:false ⇒ none. - Per-server `IsolationConfig.Mode` (*IsolationMode, nil = inherit global). - `IsolationManager.ResolveMode()` mode resolver; `ShouldIsolate()` reimplemented as `ResolveMode() == docker` so existing Docker behavior is unchanged. Precedence: per-server explicit Mode > global gate > per-server bool opt-out; structural gates (HTTP / docker-command servers) apply to all modes. - Config validation rejects unknown global/per-server mode strings. - Per-server Mode round-trips through BBolt (UpstreamRecord) — test asserts it. - Regenerated oas/swagger.yaml + docs.go for the new fields. Sandbox-mode launch wiring (consuming ResolveMode==sandbox) is deferred to MCP-34.3; no consumer acts on the sandbox value yet. Co-Authored-By: Paperclip <noreply@paperclip.ing> * docs(config): clarify isolation mode schema for global and per-server The shared config.IsolationMode enum schema (and the inlined global DockerIsolationConfig.mode field) inherited the per-server field's "Per-server isolation mode override / nil = inherit global" wording via swaggo, which is inaccurate for the global field: the global Mode is a non-pointer whose empty value falls back to the legacy Enabled bool, not "inherit global". Rewrite the per-server Mode field comment (the swaggo source for the shared schema) to a neutral description accurate for both contexts: unset per-server inherits the global mode; unset globally falls back to the legacy enabled flag. Regenerate OpenAPI. Related #MCP-3233 --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
1 parent ef7b53e commit 5ed2c30

7 files changed

Lines changed: 413 additions & 37 deletions

File tree

‎internal/config/config.go‎

Lines changed: 80 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -437,9 +437,43 @@ type OAuthConfig struct {
437437
ExtraParams map[string]string `json:"extra_params,omitempty" mapstructure:"extra_params"` // Additional OAuth parameters (e.g., RFC 8707 resource)
438438
}
439439

440+
// IsolationMode selects how an stdio MCP server's process is isolated (MCP-34.2).
441+
//
442+
// - "docker" — run the server inside a Docker container (the original behavior).
443+
// - "sandbox" — run under a native OS sandbox (Landlock LSM + rlimits on Linux;
444+
// see MCP-34). No daemon required.
445+
// - "none" — no isolation; the server process runs directly on the host.
446+
//
447+
// The empty string means "unset": back-compat code falls back to the legacy
448+
// boolean Enabled flag (enabled:true ⇒ docker, enabled:false ⇒ none).
449+
type IsolationMode string
450+
451+
const (
452+
// IsolationModeDocker runs the server inside a Docker container.
453+
IsolationModeDocker IsolationMode = "docker"
454+
// IsolationModeSandbox runs the server under a native OS sandbox (Landlock/rlimits).
455+
IsolationModeSandbox IsolationMode = "sandbox"
456+
// IsolationModeNone disables isolation; the process runs directly.
457+
IsolationModeNone IsolationMode = "none"
458+
)
459+
460+
// IsValid reports whether the mode is a recognized value. The empty string
461+
// ("unset") is treated as valid because the global config falls back to the
462+
// legacy Enabled bool; callers that require an explicit mode should check for
463+
// emptiness separately.
464+
func (m IsolationMode) IsValid() bool {
465+
switch m {
466+
case IsolationModeDocker, IsolationModeSandbox, IsolationModeNone, "":
467+
return true
468+
default:
469+
return false
470+
}
471+
}
472+
440473
// DockerIsolationConfig represents global Docker isolation settings
441474
type DockerIsolationConfig struct {
442-
Enabled bool `json:"enabled" mapstructure:"enabled"` // Global enable/disable for Docker isolation
475+
Enabled bool `json:"enabled" mapstructure:"enabled"` // Global enable/disable for Docker isolation (legacy; superseded by Mode)
476+
Mode IsolationMode `json:"mode,omitempty" mapstructure:"mode"` // Isolation mode: "docker" | "sandbox" | "none". Empty falls back to Enabled (MCP-34.2)
443477
EnableCacheVolume bool `json:"enable_cache_volume" mapstructure:"enable_cache_volume"` // Mount shared cache volumes for faster restarts (default: true)
444478
DefaultImages map[string]string `json:"default_images" mapstructure:"default_images"` // Map of runtime type to Docker image
445479
Registry string `json:"registry,omitempty" mapstructure:"registry"` // Custom registry (defaults to docker.io)
@@ -455,14 +489,34 @@ type DockerIsolationConfig struct {
455489

456490
// IsolationConfig represents per-server isolation settings
457491
type IsolationConfig struct {
458-
Enabled *bool `json:"enabled,omitempty" mapstructure:"enabled"` // Enable Docker isolation for this server (nil = inherit global)
459-
Image string `json:"image,omitempty" mapstructure:"image"` // Custom Docker image (overrides default)
460-
NetworkMode string `json:"network_mode,omitempty" mapstructure:"network_mode"` // Custom network mode for this server
461-
ExtraArgs []string `json:"extra_args,omitempty" mapstructure:"extra_args"` // Additional docker run arguments for this server
462-
WorkingDir string `json:"working_dir,omitempty" mapstructure:"working_dir"` // Custom working directory in container
463-
LogDriver string `json:"log_driver,omitempty" mapstructure:"log_driver"` // Docker log driver override for this server
464-
LogMaxSize string `json:"log_max_size,omitempty" mapstructure:"log_max_size"` // Maximum size of log files override
465-
LogMaxFiles string `json:"log_max_files,omitempty" mapstructure:"log_max_files"` // Maximum number of log files override
492+
Enabled *bool `json:"enabled,omitempty" mapstructure:"enabled"` // Enable Docker isolation for this server (nil = inherit global; legacy, superseded by Mode)
493+
Mode *IsolationMode `json:"mode,omitempty" mapstructure:"mode"` // Isolation mode: "docker" | "sandbox" | "none" (MCP-34.2). Unset per-server inherits the global mode; unset globally falls back to the legacy "enabled" flag (true ⇒ docker, false ⇒ none)
494+
Image string `json:"image,omitempty" mapstructure:"image"` // Custom Docker image (overrides default)
495+
NetworkMode string `json:"network_mode,omitempty" mapstructure:"network_mode"` // Custom network mode for this server
496+
ExtraArgs []string `json:"extra_args,omitempty" mapstructure:"extra_args"` // Additional docker run arguments for this server
497+
WorkingDir string `json:"working_dir,omitempty" mapstructure:"working_dir"` // Custom working directory in container
498+
LogDriver string `json:"log_driver,omitempty" mapstructure:"log_driver"` // Docker log driver override for this server
499+
LogMaxSize string `json:"log_max_size,omitempty" mapstructure:"log_max_size"` // Maximum size of log files override
500+
LogMaxFiles string `json:"log_max_files,omitempty" mapstructure:"log_max_files"` // Maximum number of log files override
501+
}
502+
503+
// ResolvedMode returns the effective global isolation mode, applying back-compat
504+
// mapping from the legacy Enabled bool (MCP-34.2):
505+
//
506+
// - an explicit Mode always wins;
507+
// - otherwise enabled:true ⇒ docker, enabled:false ⇒ none;
508+
// - a nil config resolves to none.
509+
func (dic *DockerIsolationConfig) ResolvedMode() IsolationMode {
510+
if dic == nil {
511+
return IsolationModeNone
512+
}
513+
if dic.Mode != "" {
514+
return dic.Mode
515+
}
516+
if dic.Enabled {
517+
return IsolationModeDocker
518+
}
519+
return IsolationModeNone
466520
}
467521

468522
// IsEnabled returns true if isolation is explicitly enabled, false otherwise.
@@ -1540,11 +1594,28 @@ func (c *Config) ValidateDetailed() []ValidationError {
15401594
}
15411595
}
15421596

1597+
// Validate global isolation mode (MCP-34.2). Empty is allowed (back-compat
1598+
// fallback to the Enabled bool); only a non-empty unknown value is invalid.
1599+
if c.DockerIsolation != nil && !c.DockerIsolation.Mode.IsValid() {
1600+
errors = append(errors, ValidationError{
1601+
Field: "docker_isolation.mode",
1602+
Message: fmt.Sprintf("invalid isolation mode: %s (must be docker, sandbox, or none)", c.DockerIsolation.Mode),
1603+
})
1604+
}
1605+
15431606
// Validate server configurations
15441607
serverNames := make(map[string]bool)
15451608
for i, server := range c.Servers {
15461609
fieldPrefix := fmt.Sprintf("mcpServers[%d]", i)
15471610

1611+
// Validate per-server isolation mode override (MCP-34.2).
1612+
if server.Isolation != nil && server.Isolation.Mode != nil && !server.Isolation.Mode.IsValid() {
1613+
errors = append(errors, ValidationError{
1614+
Field: fmt.Sprintf("%s.isolation.mode", fieldPrefix),
1615+
Message: fmt.Sprintf("invalid isolation mode: %s (must be docker, sandbox, or none)", *server.Isolation.Mode),
1616+
})
1617+
}
1618+
15481619
// Validate server name
15491620
if server.Name == "" {
15501621
errors = append(errors, ValidationError{

‎internal/config/isolation_mode_test.go‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
package config
2+
3+
import "testing"
4+
5+
// TestDockerIsolationConfig_ResolvedMode_BackCompat verifies the back-compat
6+
// mapping from the legacy global Enabled bool to the new isolation.mode enum
7+
// (MCP-34.2): old enabled:true ⇒ mode:docker, enabled:false ⇒ mode:none, and
8+
// an explicit Mode always wins over the legacy bool.
9+
func TestDockerIsolationConfig_ResolvedMode_BackCompat(t *testing.T) {
10+
tests := []struct {
11+
name string
12+
cfg *DockerIsolationConfig
13+
want IsolationMode
14+
}{
15+
{"nil config resolves to none", nil, IsolationModeNone},
16+
{"legacy enabled:true ⇒ docker", &DockerIsolationConfig{Enabled: true}, IsolationModeDocker},
17+
{"legacy enabled:false ⇒ none", &DockerIsolationConfig{Enabled: false}, IsolationModeNone},
18+
{"explicit mode:docker", &DockerIsolationConfig{Mode: IsolationModeDocker}, IsolationModeDocker},
19+
{"explicit mode:sandbox", &DockerIsolationConfig{Mode: IsolationModeSandbox}, IsolationModeSandbox},
20+
{"explicit mode:none", &DockerIsolationConfig{Mode: IsolationModeNone}, IsolationModeNone},
21+
{"explicit mode wins over legacy enabled:false", &DockerIsolationConfig{Enabled: false, Mode: IsolationModeSandbox}, IsolationModeSandbox},
22+
{"explicit mode:none wins over legacy enabled:true", &DockerIsolationConfig{Enabled: true, Mode: IsolationModeNone}, IsolationModeNone},
23+
}
24+
for _, tt := range tests {
25+
t.Run(tt.name, func(t *testing.T) {
26+
if got := tt.cfg.ResolvedMode(); got != tt.want {
27+
t.Errorf("ResolvedMode() = %q, want %q", got, tt.want)
28+
}
29+
})
30+
}
31+
}
32+
33+
// TestIsolationMode_IsValid checks the enum validation helper.
34+
func TestIsolationMode_IsValid(t *testing.T) {
35+
valid := []IsolationMode{IsolationModeDocker, IsolationModeSandbox, IsolationModeNone}
36+
for _, m := range valid {
37+
if !m.IsValid() {
38+
t.Errorf("IsValid(%q) = false, want true", m)
39+
}
40+
}
41+
// Empty string is treated as "unset" (back-compat) and is considered valid
42+
// at the global level; only a non-empty unknown value is invalid.
43+
if !IsolationMode("").IsValid() {
44+
t.Errorf("IsValid(\"\") = false, want true (unset is valid)")
45+
}
46+
for _, m := range []IsolationMode{"vm", "gvisor", "DOCKER"} {
47+
if m.IsValid() {
48+
t.Errorf("IsValid(%q) = true, want false", m)
49+
}
50+
}
51+
}
52+
53+
// TestConfig_Validate_InvalidIsolationMode ensures an unknown global isolation
54+
// mode is rejected by config validation.
55+
func TestConfig_Validate_InvalidIsolationMode(t *testing.T) {
56+
c := &Config{
57+
DockerIsolation: &DockerIsolationConfig{Mode: IsolationMode("gvisor")},
58+
}
59+
errs := c.ValidateDetailed()
60+
found := false
61+
for _, e := range errs {
62+
if e.Field == "docker_isolation.mode" {
63+
found = true
64+
}
65+
}
66+
if !found {
67+
t.Errorf("expected validation error for docker_isolation.mode, got %+v", errs)
68+
}
69+
}
70+
71+
// TestConfig_Validate_InvalidPerServerIsolationMode ensures an unknown
72+
// per-server isolation mode is rejected by config validation.
73+
func TestConfig_Validate_InvalidPerServerIsolationMode(t *testing.T) {
74+
bad := IsolationMode("vm")
75+
c := &Config{
76+
Servers: []*ServerConfig{
77+
{
78+
Name: "srv",
79+
Command: "npx",
80+
Protocol: "stdio",
81+
Isolation: &IsolationConfig{Mode: &bad},
82+
},
83+
},
84+
}
85+
errs := c.ValidateDetailed()
86+
found := false
87+
for _, e := range errs {
88+
if e.Field == "mcpServers[0].isolation.mode" {
89+
found = true
90+
}
91+
}
92+
if !found {
93+
t.Errorf("expected validation error for per-server isolation.mode, got %+v", errs)
94+
}
95+
}

‎internal/storage/async_ops_test.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ func TestSaveServerSyncPreservesAllFields(t *testing.T) {
5151
Updated: time.Now(),
5252
Isolation: &config.IsolationConfig{
5353
Enabled: config.BoolPtr(true),
54+
Mode: func() *config.IsolationMode { m := config.IsolationModeSandbox; return &m }(),
5455
Image: "python:3.11",
5556
NetworkMode: "bridge",
5657
ExtraArgs: []string{"-v", "/host:/container"},
@@ -120,6 +121,10 @@ func TestSaveServerSyncPreservesAllFields(t *testing.T) {
120121
if record.Isolation.IsEnabled() != serverConfig.Isolation.IsEnabled() {
121122
t.Errorf("Isolation.Enabled mismatch: got %v, want %v", record.Isolation.IsEnabled(), serverConfig.Isolation.IsEnabled())
122123
}
124+
// MCP-34.2: per-server isolation.mode must survive the BBolt round-trip.
125+
if !reflect.DeepEqual(record.Isolation.Mode, serverConfig.Isolation.Mode) {
126+
t.Errorf("Isolation.Mode mismatch: got %v, want %v", record.Isolation.Mode, serverConfig.Isolation.Mode)
127+
}
123128
if record.Isolation.Image != serverConfig.Isolation.Image {
124129
t.Errorf("Isolation.Image mismatch: got %s, want %s", record.Isolation.Image, serverConfig.Isolation.Image)
125130
}

‎internal/upstream/core/isolation.go‎

Lines changed: 59 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -153,41 +153,75 @@ func (im *IsolationManager) GetDockerIsolationWarning(serverConfig *config.Serve
153153
return ""
154154
}
155155

156-
// ShouldIsolate determines if a server should be isolated based on global and server config
156+
// ShouldIsolate determines if a server should be isolated via Docker, based on
157+
// global and server config. It is the legacy boolean view of ResolveMode and
158+
// stays in lockstep with it: it returns true iff the resolved mode is "docker".
159+
// Callers that need to distinguish sandbox/none should call ResolveMode.
157160
func (im *IsolationManager) ShouldIsolate(serverConfig *config.ServerConfig) bool {
158-
// Check if global isolation is disabled
159-
if im.globalConfig == nil || !im.globalConfig.Enabled {
160-
// If the user explicitly opted THIS server into isolation (via
161-
// isolation.enabled: true), warn them once that the per-server
162-
// setting is being ignored because the global flag is off. Silent
163-
// short-circuits here are a common cause of confusion — telemetry
164-
// shows many users configure per-server opt-ins and then wonder why
165-
// nothing runs in a container.
166-
if im.hasExplicitPerServerOptIn(serverConfig) {
167-
im.warnPerServerIgnoredOnce(serverConfig.Name)
168-
}
169-
return false
170-
}
161+
return im.ResolveMode(serverConfig) == config.IsolationModeDocker
162+
}
171163

172-
// Check if server has isolation config and it's explicitly disabled
173-
// With *bool: nil means "inherit global", explicit false means "disabled"
174-
if serverConfig.Isolation != nil && serverConfig.Isolation.Enabled != nil && !*serverConfig.Isolation.Enabled {
175-
return false
164+
// ResolveMode resolves the effective isolation mode for a server (MCP-34.2),
165+
// combining the global config (with legacy Enabled⇒docker back-compat), an
166+
// optional per-server override, and structural gates.
167+
//
168+
// Precedence:
169+
// 1. A per-server explicit Mode wins outright (even over a disabled global) —
170+
// mirroring how other per-server overrides (image, network) take priority.
171+
// 2. Otherwise, when the global mode resolves to none, per-server bool opt-ins
172+
// are ignored (and warned about once), preserving the pre-mode behavior.
173+
// 3. When the global mode is active, a per-server bool opt-out (enabled:false)
174+
// downgrades the server to none.
175+
//
176+
// Structural gates then apply to ALL non-none modes: HTTP servers (no command)
177+
// and servers that already invoke docker are never isolated.
178+
func (im *IsolationManager) ResolveMode(serverConfig *config.ServerConfig) config.IsolationMode {
179+
mode := im.resolveConfiguredMode(serverConfig)
180+
if mode == config.IsolationModeNone {
181+
return config.IsolationModeNone
176182
}
177183

178-
// Only isolate stdio servers (HTTP servers don't need Docker isolation)
179-
if serverConfig.Command == "" {
180-
return false
184+
// Only isolate stdio servers (HTTP servers don't need a sandbox/container).
185+
if serverConfig == nil || serverConfig.Command == "" {
186+
return config.IsolationModeNone
181187
}
182188

183-
// Skip isolation for servers that are already using Docker
184-
// These are typically pre-configured Docker containers that don't need additional isolation
189+
// Skip isolation for servers that already invoke Docker — these are
190+
// typically pre-configured containers, and wrapping them (in a container
191+
// or a Landlock sandbox) would break their access to the Docker socket.
185192
cmdName := filepath.Base(serverConfig.Command)
186193
if cmdName == "docker" || strings.Contains(serverConfig.Command, "docker") {
187-
return false
194+
return config.IsolationModeNone
195+
}
196+
197+
return mode
198+
}
199+
200+
// resolveConfiguredMode applies the global + per-server config precedence to
201+
// produce the desired mode, before the structural gates in ResolveMode.
202+
func (im *IsolationManager) resolveConfiguredMode(serverConfig *config.ServerConfig) config.IsolationMode {
203+
globalMode := im.globalConfig.ResolvedMode() // nil-safe; returns none for nil
204+
205+
// (1) A per-server explicit Mode override wins outright.
206+
if serverConfig != nil && serverConfig.Isolation != nil && serverConfig.Isolation.Mode != nil {
207+
return *serverConfig.Isolation.Mode
208+
}
209+
210+
// (2) Global isolation off: per-server bool opt-ins are ignored (warn once).
211+
if globalMode == config.IsolationModeNone {
212+
if im.hasExplicitPerServerOptIn(serverConfig) {
213+
im.warnPerServerIgnoredOnce(serverConfig.Name)
214+
}
215+
return config.IsolationModeNone
216+
}
217+
218+
// (3) Global isolation active: honor a per-server bool opt-out.
219+
if serverConfig != nil && serverConfig.Isolation != nil &&
220+
serverConfig.Isolation.Enabled != nil && !*serverConfig.Isolation.Enabled {
221+
return config.IsolationModeNone
188222
}
189223

190-
return true
224+
return globalMode
191225
}
192226

193227
// hasExplicitPerServerOptIn returns true when the server config explicitly

0 commit comments

Comments
 (0)