Skip to content

Commit 918df05

Browse files
feat(security): enforce E2B sandbox, fail-closed defaults [EPIC 0.2]
Closes #4, closes #5, closes #6, closes #7, closes #8
1 parent 802921d commit 918df05

File tree

12 files changed

+654
-40
lines changed

12 files changed

+654
-40
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,4 @@ frontend/node_modules
7777
# Coverage
7878
.coverage
7979
htmlcov/
80+
coverage.xml

openspace/.env.example

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,15 @@ OPENSPACE_API_KEY=sk_xxxxxxxxxxxxxxxx
4040
# EMBEDDING_API_KEY=
4141
# EMBEDDING_MODEL= "openai/text-embedding-3-small"
4242

43-
# ---- E2B Sandbox (Optional) ----
44-
# Required only if sandbox mode is enabled in security config.
45-
# E2B_API_KEY=
43+
# ---- E2B Sandbox (Required for MCP stdio servers) ----
44+
# E2B sandbox is ENFORCED by default for all stdio-based MCP servers.
45+
# You MUST provide an API key for E2B to execute skills securely.
46+
# Get your API key from https://e2b.dev/
47+
E2B_API_KEY=
48+
49+
# ---- Sandbox Configuration ----
50+
# Sandbox is mandatory. To opt out (development only, NOT recommended):
51+
# OPENSPACE_ALLOW_UNSANDBOXED=1
4652

4753
# ---- Local Server (Optional) ----
4854
# Override the default local server URL (default: http://127.0.0.1:5000)

openspace/config/README.md

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ Layered system — later files override earlier ones:
9999
|---------|-----------|-------------|
100100
| `shell` | `mode`, `timeout`, `conda_env`, `working_dir` | `"local"` (default) or `"server"`, command timeout (default: `60`s) |
101101
| `gui` | `mode`, `timeout`, `driver_type`, `screenshot_on_error` | Local/server mode, automation driver (default: `pyautogui`) |
102-
| `mcp` | `timeout`, `sandbox`, `eager_sessions` | Request timeout (`30`s), E2B sandbox, lazy/eager server init |
102+
| `mcp` | `timeout`, `sandbox`, `eager_sessions` | Request timeout (`30`s), E2B sandbox (**enforced by default**), lazy/eager server init |
103103
| `tool_search` | `search_mode`, `max_tools`, `enable_llm_filter` | `"hybrid"` (semantic + LLM), max tools to return (`40`), embedding cache |
104104
| `tool_quality` | `enabled`, `enable_persistence`, `evolve_interval` | Quality tracking, self-evolution every N calls (default: `5`) |
105105
| `skills` | `enabled`, `skill_dirs`, `max_select` | Directories to scan, max skills injected per task (default: `2`) |
@@ -110,6 +110,26 @@ Layered system — later files override earlier ones:
110110
|-------|-------------|---------|
111111
| `allow_shell_commands` | Enable shell execution | `true` |
112112
| `blocked_commands` | Platform-specific blacklists (common/linux/darwin/windows) | `rm -rf`, `shutdown`, `dd`, etc. |
113-
| `sandbox_enabled` | Enable sandboxing for all operations | `false` |
113+
| `sandbox_enabled` | Enable sandboxing for all operations | **`true`** |
114114
| Per-backend overrides | Shell, MCP, GUI, Web each have independent security policies | Inherit global |
115115

116+
### E2B Sandbox Configuration
117+
118+
Sandbox execution is **enforced by default** for all stdio-based MCP servers. This prevents
119+
untrusted skill code from executing directly on the host.
120+
121+
| Setting | Source | Description |
122+
|---------|--------|-------------|
123+
| `E2B_API_KEY` | **Environment variable** (required) | API key from [e2b.dev](https://e2b.dev). Never passed via config. |
124+
| `mcp.sandbox` | `config_grounding.json` | Default: `true`. Controls sandbox enforcement for MCP backend. |
125+
| `sandbox_enabled` | `config_security.json` | Default: `true`. Global and per-backend sandbox policy. |
126+
| `sandbox_template_id` | Config only | E2B sandbox template (default: `"base"`). |
127+
| `timeout` | Config only | Sandbox command timeout in seconds (default: `600`). |
128+
| `OPENSPACE_ALLOW_UNSANDBOXED` | Environment variable | Set to `1` to allow unsandboxed execution (dev only, **NOT recommended**). |
129+
130+
**Fail-closed behavior:**
131+
- Missing `E2B_API_KEY` → startup fails (no silent fallback)
132+
- Missing E2B SDK → startup fails with install instructions
133+
- Invalid config → startup fails (no default-config fallback)
134+
- Sandbox start failure → execution aborted (no downgrade to direct execution)
135+

openspace/config/config_grounding.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
"timeout": 30,
1515
"max_retries": 3,
1616
"retry_interval": 2.0,
17-
"sandbox": false,
17+
"sandbox": true,
1818
"auto_initialize": true,
1919
"eager_sessions": false,
2020
"sse_read_timeout": 300.0,

openspace/config/config_security.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
"darwin": ["diskutil", "dd", "pfctl", "launchctl", "killall"],
1111
"windows": ["del", "format", "rd", "rmdir", "/s", "/q", "taskkill", "/f"]
1212
},
13-
"sandbox_enabled": false
13+
"sandbox_enabled": true
1414
},
1515
"backend": {
1616
"shell": {
@@ -54,10 +54,10 @@
5454
"wmic"
5555
]
5656
},
57-
"sandbox_enabled": false
57+
"sandbox_enabled": true
5858
},
5959
"mcp": {
60-
"sandbox_enabled": false
60+
"sandbox_enabled": true
6161
},
6262
"web": {
6363
"allow_network_access": true,

openspace/config/grounding.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ class WebConfig(BackendConfig):
105105

106106
class MCPConfig(BackendConfig):
107107
"""MCP backend configuration"""
108-
sandbox: bool = Field(False, description="Whether to enable sandbox")
108+
sandbox: bool = Field(True, description="Whether to enable sandbox (enforced by default for security)")
109109
auto_initialize: bool = Field(True, description="Whether to auto initialize")
110110
eager_sessions: bool = Field(False, description="Whether to eagerly create sessions for all servers on initialization")
111111
retry_interval: float = Field(2.0, ge=0.1, le=60.0, description="Wait time between retries in seconds")

openspace/config/loader.py

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,24 @@ def _deep_merge_dict(base: dict, update: dict) -> dict:
3333
result[key] = value
3434
return result
3535

36-
def _load_json_file(path: Path) -> Dict[str, Any]:
36+
def _load_json_file(path: Path, *, critical: bool = False) -> Dict[str, Any]:
3737
"""Load single JSON configuration file.
3838
39-
This function wraps the generic load_json_file and adds global configuration specific error handling and logging.
39+
This function wraps the generic load_json_file and adds global
40+
configuration specific error handling and logging.
41+
42+
Args:
43+
path: Path to JSON file.
44+
critical: If True, raise on parse errors instead of returning {}.
45+
Use for security-critical config files where silent
46+
fallback to empty dict could weaken security posture.
4047
"""
4148
if not path.exists():
49+
if critical:
50+
raise FileNotFoundError(
51+
f"Critical configuration file missing: {path}. "
52+
"Cannot start with potentially insecure defaults."
53+
)
4254
logger.debug(f"Configuration file does not exist, skipping: {path}")
4355
return {}
4456

@@ -47,18 +59,33 @@ def _load_json_file(path: Path) -> Dict[str, Any]:
4759
logger.info(f"Loaded configuration file: {path}")
4860
return data
4961
except Exception as e:
62+
if critical:
63+
raise RuntimeError(
64+
f"Failed to parse critical configuration file {path}: {e}. "
65+
"Refusing to start with potentially insecure defaults."
66+
) from e
5067
logger.warning(f"Failed to load configuration file {path}: {e}")
5168
return {}
5269

53-
def _load_multiple_files(paths: Iterable[Path]) -> Dict[str, Any]:
54-
"""Load configuration from multiple files"""
70+
def _load_multiple_files(paths: Iterable[Path], critical_files: frozenset[str] = frozenset()) -> Dict[str, Any]:
71+
"""Load configuration from multiple files.
72+
73+
Args:
74+
paths: Config file paths to load and merge.
75+
critical_files: Filenames (not full paths) that must not fail silently.
76+
"""
5577
merged = {}
5678
for path in paths:
57-
data = _load_json_file(path)
79+
is_critical = path.name in critical_files
80+
data = _load_json_file(path, critical=is_critical)
5881
if data:
5982
merged = _deep_merge_dict(merged, data)
6083
return merged
6184

85+
# Security config must not fail silently — malformed security config
86+
# could silently disable sandbox enforcement.
87+
_CRITICAL_CONFIG_FILES = frozenset({CONFIG_SECURITY})
88+
6289
def load_config(*config_paths: Union[str, Path]) -> GroundingConfig:
6390
"""
6491
Load configuration files
@@ -76,7 +103,9 @@ def load_config(*config_paths: Union[str, Path]) -> GroundingConfig:
76103
]
77104

78105
# Load and merge configuration
79-
raw_data = _load_multiple_files(paths)
106+
# Security config is marked critical — parse errors raise instead of
107+
# silently falling back to defaults (which could disable sandbox).
108+
raw_data = _load_multiple_files(paths, critical_files=_CRITICAL_CONFIG_FILES)
80109

81110
# Load MCP configuration (separate processing)
82111
# Check if mcpServers already provided in merged custom configs
@@ -98,11 +127,17 @@ def load_config(*config_paths: Union[str, Path]) -> GroundingConfig:
98127
logger.debug(f"Loaded MCP servers from default config_mcp.json ({len(raw_data['mcp']['servers'])} servers)")
99128

100129
# Validate and create configuration object
130+
# Fail-closed: invalid config raises instead of silently falling back
131+
# to defaults (which could disable sandbox enforcement)
101132
try:
102133
_config = GroundingConfig.model_validate(raw_data)
103134
except Exception as e:
104-
logger.error(f"Validation failed, using default configuration: {e}")
105-
_config = GroundingConfig()
135+
logger.error(f"Configuration validation failed: {e}")
136+
raise RuntimeError(
137+
f"GroundingConfig validation failed — refusing to start with "
138+
f"potentially insecure defaults. Fix the config or remove it "
139+
f"to use secure defaults. Error: {e}"
140+
) from e
106141

107142
# Adjust log level according to configuration
108143
if _config.debug:

openspace/grounding/backends/mcp/client.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class MCPClient:
2929
def __init__(
3030
self,
3131
config: str | dict[str, Any] | None = None,
32-
sandbox: bool = False,
32+
sandbox: bool = True,
3333
sandbox_options: SandboxOptions | None = None,
3434
timeout: float = 30.0,
3535
sse_read_timeout: float = 300.0,
@@ -94,7 +94,7 @@ def _get_mcp_servers(self) -> dict[str, Any]:
9494
def from_dict(
9595
cls,
9696
config: dict[str, Any],
97-
sandbox: bool = False,
97+
sandbox: bool = True,
9898
sandbox_options: SandboxOptions | None = None,
9999
timeout: float = 30.0,
100100
sse_read_timeout: float = 300.0,
@@ -118,7 +118,7 @@ def from_dict(
118118

119119
@classmethod
120120
def from_config_file(
121-
cls, filepath: str, sandbox: bool = False, sandbox_options: SandboxOptions | None = None,
121+
cls, filepath: str, sandbox: bool = True, sandbox_options: SandboxOptions | None = None,
122122
timeout: float = 30.0, sse_read_timeout: float = 300.0,
123123
max_retries: int = 3, retry_interval: float = 2.0,
124124
) -> "MCPClient":

openspace/grounding/backends/mcp/config.py

Lines changed: 75 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,12 @@
22
Configuration loader for MCP session.
33
44
This module provides functionality to load MCP configuration from JSON files.
5+
6+
Security: Sandbox is enforced by default for all stdio-based MCP servers.
7+
Unsandboxed execution requires explicit OPENSPACE_ALLOW_UNSANDBOXED=1 env var.
58
"""
69

10+
import os
711
from typing import Any, Optional
812

913
from openspace.grounding.core.types import SandboxOptions
@@ -26,10 +30,39 @@
2630
E2BSandbox = None
2731
E2B_AVAILABLE = False
2832

33+
34+
# Trusted sandbox config keys that may be sourced from config/env
35+
_TRUSTED_SANDBOX_KEYS = frozenset({
36+
"timeout", "sse_read_timeout", "supergateway_command", "port",
37+
"sandbox_template_id",
38+
})
39+
40+
41+
def _build_trusted_sandbox_options(
42+
caller_options: SandboxOptions | None,
43+
default_timeout: float,
44+
default_sse_timeout: float,
45+
) -> dict[str, Any]:
46+
"""Build sandbox options from trusted sources only.
47+
48+
Strips caller-supplied api_key (must come from env E2B_API_KEY).
49+
Only allows known config keys through; ignores anything else.
50+
"""
51+
base: dict[str, Any] = {
52+
"timeout": default_timeout,
53+
"sse_read_timeout": default_sse_timeout,
54+
}
55+
if caller_options:
56+
for key in _TRUSTED_SANDBOX_KEYS:
57+
if key in caller_options:
58+
base[key] = caller_options[key]
59+
return base
60+
61+
2962
async def create_connector_from_config(
3063
server_config: dict[str, Any],
3164
server_name: str = "unknown",
32-
sandbox: bool = False,
65+
sandbox: bool = True,
3366
sandbox_options: SandboxOptions | None = None,
3467
timeout: float = 30.0,
3568
sse_read_timeout: float = 300.0,
@@ -40,10 +73,15 @@ async def create_connector_from_config(
4073
) -> MCPBaseConnector:
4174
"""Create a connector based on server configuration.
4275
76+
For stdio-based servers, sandbox is ENFORCED. Unsandboxed stdio execution
77+
is denied by default. Set OPENSPACE_ALLOW_UNSANDBOXED=1 to explicitly
78+
opt out (development/testing only — NOT recommended for production).
79+
4380
Args:
4481
server_config: The server configuration section
4582
server_name: Name of the MCP server (for display purposes)
4683
sandbox: Whether to use sandboxed execution mode for running MCP servers.
84+
Defaults to True (enforced).
4785
sandbox_options: Optional sandbox configuration options.
4886
timeout: Timeout for operations in seconds (default: 30.0)
4987
sse_read_timeout: SSE read timeout in seconds (default: 300.0)
@@ -56,13 +94,40 @@ async def create_connector_from_config(
5694
A configured connector instance
5795
5896
Raises:
59-
RuntimeError: If dependencies are not installed and user declines installation
97+
RuntimeError: If sandbox is required but not available, or if
98+
dependencies are not installed and user declines installation
6099
"""
61100

62101
# Get original command and args from config
63102
original_command = get_config_value(server_config, "command")
64103
original_args = get_config_value(server_config, "args", [])
65104

105+
# --- Sandbox enforcement BEFORE any host-side operations ---
106+
# Reject unsandboxed stdio early, before ensure_dependencies runs
107+
# npm/pip install on the host. This prevents a malicious server config
108+
# from triggering host-side package installs before being denied.
109+
if is_stdio_server(server_config) and not sandbox:
110+
allow_unsandboxed = os.environ.get("OPENSPACE_ALLOW_UNSANDBOXED", "").strip()
111+
if allow_unsandboxed != "1":
112+
raise RuntimeError(
113+
f"Unsandboxed stdio execution denied for server '{server_name}'. "
114+
"Sandbox is required for all stdio-based MCP servers. "
115+
"Set OPENSPACE_ALLOW_UNSANDBOXED=1 to override (NOT recommended)."
116+
)
117+
import logging
118+
logging.getLogger(__name__).warning(
119+
"SECURITY: Running server '%s' WITHOUT sandbox (OPENSPACE_ALLOW_UNSANDBOXED=1). "
120+
"This is NOT recommended for production use.",
121+
server_name,
122+
)
123+
124+
if is_stdio_server(server_config) and sandbox and not E2B_AVAILABLE:
125+
raise ImportError(
126+
"E2B sandbox support not available. Please install e2b-code-interpreter: "
127+
"'pip install e2b-code-interpreter'"
128+
)
129+
130+
# --- Host-side operations (only after sandbox enforcement passes) ---
66131
# Check and install dependencies if needed (only for stdio servers)
67132
if is_stdio_server(server_config) and check_dependencies:
68133
# Use provided installer or get global instance
@@ -73,27 +138,23 @@ async def create_connector_from_config(
73138
# Ensure dependencies are installed (using original command/args)
74139
await installer.ensure_dependencies(server_name, original_command, original_args)
75140

76-
# Stdio connector (command-based)
141+
# Stdio connector — unsandboxed (only reachable with explicit opt-out)
77142
if is_stdio_server(server_config) and not sandbox:
78143
return StdioConnector(
79144
command=get_config_value(server_config, "command"),
80145
args=get_config_value(server_config, "args"),
81146
env=get_config_value(server_config, "env", None),
82147
)
83148

84-
# Sandboxed connector
85-
elif is_stdio_server(server_config) and sandbox:
86-
if not E2B_AVAILABLE:
87-
raise ImportError(
88-
"E2B sandbox support not available. Please install e2b-code-interpreter: "
89-
"'pip install e2b-code-interpreter'"
90-
)
91-
92-
# Create E2B sandbox instance
93-
_sandbox_options = sandbox_options or {}
149+
# Sandboxed connector (E2B_AVAILABLE already verified above)
150+
elif is_stdio_server(server_config):
151+
# Build sandbox options from trusted config/env only (never user input)
152+
_sandbox_options = _build_trusted_sandbox_options(
153+
sandbox_options, timeout, sse_read_timeout
154+
)
94155
e2b_sandbox = E2BSandbox(_sandbox_options)
95156

96-
# Extract timeout values from sandbox_options or use defaults
157+
# Extract timeout values from trusted options
97158
connector_timeout = _sandbox_options.get("timeout", timeout)
98159
connector_sse_timeout = _sandbox_options.get("sse_read_timeout", sse_read_timeout)
99160

openspace/grounding/backends/mcp/provider.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ def __init__(self, config: Dict | None = None, installer: Optional[MCPInstallerM
3939
super().__init__(BackendType.MCP, config)
4040

4141
# Extract MCP-specific configuration
42-
sandbox = get_config_value(config, "sandbox", False)
42+
sandbox = get_config_value(config, "sandbox", True)
4343
timeout = get_config_value(config, "timeout", 30)
4444
sse_read_timeout = get_config_value(config, "sse_read_timeout", 300.0)
4545
max_retries = get_config_value(config, "max_retries", 3)

0 commit comments

Comments
 (0)