From d8f8759dea78b6ca0db860457dc28442aeecdb70 Mon Sep 17 00:00:00 2001 From: Matt Hill Date: Tue, 17 Mar 2026 17:13:04 -0600 Subject: [PATCH] docs: document excluded RPC methods + remove system.shell from MCP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add comprehensive excluded methods section to MCP ARCHITECTURE.md categorizing all 105 excluded RPC methods by reason (wrong context, CLI tooling, registry admin, tunnel mgmt, security, etc.). Remove system.shell tool — host-level shell access is too broad a privilege for MCP agents. Agents can still exec into package subcontainers via package.shell. Co-Authored-By: Claude Opus 4.6 (1M context) --- core/src/mcp/ARCHITECTURE.md | 107 +++++++++++++++++++++++++++-- core/src/mcp/mod.rs | 127 +---------------------------------- core/src/mcp/tools.rs | 25 +------ 3 files changed, 104 insertions(+), 155 deletions(-) diff --git a/core/src/mcp/ARCHITECTURE.md b/core/src/mcp/ARCHITECTURE.md index af687f964..9dbf67b47 100644 --- a/core/src/mcp/ARCHITECTURE.md +++ b/core/src/mcp/ARCHITECTURE.md @@ -39,7 +39,7 @@ core/src/mcp/ ├── mod.rs — HTTP handlers, routing, MCP method dispatch, shell execution, CORS ├── protocol.rs — JSON-RPC 2.0 types, MCP request/response structs, error codes ├── session.rs — Session map, create/remove/sweep, resource subscriptions with debounce -└── tools.rs — Tool registry (67 tools), HashMap mapping names → RPC methods + schemas +└── tools.rs — Tool registry (88 tools), HashMap mapping names → RPC methods + schemas ``` ## Tool Dispatch @@ -55,17 +55,16 @@ When `tools/call` arrives: 1. Look up the tool by name via HashMap O(1) lookup. 2. Convert arguments from `serde_json::Value` to `imbl_value::Value`. -3. **Special-case**: If `rpc_method` is `"__shell__"` or `"__package_shell__"`, dispatch to `handle_shell_exec` / `handle_package_shell_exec` directly (no RPC handler). Both set `kill_on_drop(true)` to ensure timed-out processes are terminated. +3. **Special-case**: If `rpc_method` is `"__package_shell__"`, dispatch to `handle_package_shell_exec` directly (no RPC handler). Sets `kill_on_drop(true)` to ensure timed-out processes are terminated. 4. Otherwise, optionally inject `__Auth_session` into params, then call `server.handle_command(rpc_method, params)`. 5. On success: if `sync_db` is true, flush the DB sequence. Return the result pretty-printed as a text content block. 6. On error: return the error as a text content block with `is_error: true`, using `McpResponse::ok` (MCP spec: tool errors are results, not JSON-RPC errors). ## Shell Execution -Two shell tools bypass the RPC layer entirely: +One shell tool bypasses the RPC layer entirely: -- **`system.shell`** (`__shell__`): Runs `/bin/bash -c ` on the host with `kill_on_drop(true)`. 30s default timeout, 300s max. -- **`package.shell`** (`__package_shell__`): Resolves the target package's subcontainer via `Service::resolve_subcontainer`, then runs `/bin/sh -c ` inside it via `lxc-attach` (also `kill_on_drop(true)`). Same timeout behavior. +- **`package.shell`** (`__package_shell__`): Resolves the target package's subcontainer via `Service::resolve_subcontainer`, then runs `/bin/sh -c ` inside it via `lxc-attach` with `kill_on_drop(true)`. 30s default timeout, 300s max. Host-level shell access (`system.shell`) is intentionally excluded — agents operate within package containers only. ## Resource Subscriptions @@ -88,6 +87,104 @@ Resource URIs are validated to only allow `/public/**` subtrees and the special - Normal responses (`apply_cors`): reflects the request's `Origin` header when present, falls back to `*` when absent. Exposes the `Mcp-Session-Id` header. This matches the behavior of the rpc-toolkit `Cors` middleware used by the main UI. - CORS headers are applied to all response types: POST JSON-RPC, GET SSE, DELETE, and error responses. +## Excluded RPC Methods + +Of the ~194 RPC methods registered in the StartOS backend, 87 are exposed as MCP tools (plus 1 MCP-only tool: `package.shell`). The remaining 105 are excluded for the following reasons. + +### Wrong context — Setup / Init / Diagnostic modes + +These methods belong to the setup wizard, initial install, or diagnostic recovery mode — entirely different server states that are not reachable during normal operation when the MCP server is running. + +| Method | Reason | +|--------|--------| +| `setup.*` (15 methods) | Setup wizard only runs during initial OS configuration | +| `init.*` (14 methods) | Initial disk/install flow, not reachable post-boot | +| `diagnostic.*` (7 methods) | Diagnostic recovery mode, separate HTTP server | +| `flash-os` | Bare-metal OS flashing | + +### Wrong context — CLI / Developer tooling + +These are developer-facing commands invoked via the CLI, not the web UI. They operate on local files or require local filesystem access. + +| Method | Reason | +|--------|--------| +| `s9pk.*` (9 methods) | Package building/inspection — CLI tool for developers | +| `util.b3sum` | BLAKE3 checksum utility — CLI helper | +| `init-key`, `pubkey` | Key management — CLI operations | + +### Wrong context — Registry administration + +These manage the package registry (a separate server-side component), not the local StartOS instance. + +| Method | Reason | +|--------|--------| +| `registry.*` (20 methods) | Registry server administration, not local device management | + +### Wrong context — Tunnel management + +These configure the Start9 tunnel service, which has its own management interface. + +| Method | Reason | +|--------|--------| +| `tunnel.*` (12 methods) | Tunnel server management, separate from local OS control | + +### Replaced by MCP-native functionality + +| Method | Reason | +|--------|--------| +| `db.subscribe` | Replaced by MCP `resources/subscribe` which calls `ctx.db.dump_and_sub()` directly with 500ms debounce | +| `server.metrics.follow` | WebSocket continuation for streaming metrics — use `server.metrics` (polling) instead | + +### Requires middleware injection not available via MCP dispatch + +| Method | Reason | +|--------|--------| +| `package.sideload` | Requires multipart file upload via middleware, not JSON-RPC params | + +### Security — host-level shell access excluded + +| Method | Reason | +|--------|--------| +| `system.shell` | Arbitrary host-level command execution is too broad a privilege for MCP agents. Agents can execute commands inside package subcontainers via `package.shell`, which is scoped to the service's filesystem and processes | + +### Auth methods — intentionally excluded + +| Method | Reason | +|--------|--------| +| `auth.login` | MCP clients authenticate via session cookie before reaching the MCP server — login is a prerequisite, not an MCP operation | +| `auth.logout` | Logging out the session that the MCP client is using would break the connection. Clients should disconnect (DELETE) instead | + +### Internal / low-value + +| Method | Reason | +|--------|--------| +| `echo` | Debug echo — no agent value | +| `git-info` | Build metadata — available via `server.device-info` | +| `state` | Returns server state enum — available via DB resources | +| `notification.create` | Internal: creates notifications from backend code, not user-facing | +| `db.apply` | Bulk DB mutation — CLI-specific params (`apply_receipt`) not suitable for MCP | +| `kiosk.set` | Kiosk mode toggle — physical display setting, not agent-relevant | + +### Deep host/binding management — not yet exposed + +These methods manage individual domain bindings and address assignments at a granular level. The list (`server.host.address.list`, `server.host.binding.list`, `package.host.list`) and read operations are exposed; the mutation operations below are deferred until agent workflows demonstrate a need. + +| Method | Reason | +|--------|--------| +| `server.host.address.domain.public.add` | Granular domain management — deferred | +| `server.host.address.domain.public.remove` | Granular domain management — deferred | +| `server.host.address.domain.private.add` | Granular domain management — deferred | +| `server.host.address.domain.private.remove` | Granular domain management — deferred | +| `server.host.binding.set-address-enabled` | Granular binding management — deferred | +| `package.host.address.domain.public.add` | Granular domain management — deferred | +| `package.host.address.domain.public.remove` | Granular domain management — deferred | +| `package.host.address.domain.private.add` | Granular domain management — deferred | +| `package.host.address.domain.private.remove` | Granular domain management — deferred | +| `package.host.address.list` | Per-package address listing — deferred | +| `package.host.binding.list` | Per-package binding listing — deferred | +| `package.host.binding.set-address-enabled` | Granular binding management — deferred | +| `net.gateway.set-default-outbound` | Gateway default route — deferred | + ## Body Size Limits POST request bodies are limited to 1 MiB: diff --git a/core/src/mcp/mod.rs b/core/src/mcp/mod.rs index 2001d3de5..3f04e1971 100644 --- a/core/src/mcp/mod.rs +++ b/core/src/mcp/mod.rs @@ -514,10 +514,7 @@ async fn handle_tools_call( } }; - // Special-case: shell execution (no RPC handler for these) - if tool.rpc_method == "__shell__" { - return handle_shell_exec(id, call_params.arguments, start).await; - } + // Special-case: package shell execution (no RPC handler) if tool.rpc_method == "__package_shell__" { return handle_package_shell_exec(ctx, id, call_params.arguments, start).await; } @@ -584,128 +581,6 @@ async fn handle_tools_call( } } -async fn handle_shell_exec( - id: Option, - arguments: JsonValue, - start: Instant, -) -> McpResponse { - let command = match arguments.get("command").and_then(|v| v.as_str()) { - Some(c) => c.to_string(), - None => { - return McpResponse::error( - id, - INVALID_PARAMS, - "Missing required parameter: command".into(), - None, - ) - } - }; - - let timeout_secs = arguments - .get("timeout") - .and_then(|v| v.as_u64()) - .unwrap_or(30) - .min(300); - - tracing::info!( - target: "mcp_audit", - command = %command, - timeout_secs = timeout_secs, - "MCP shell command executing" - ); - - let result = tokio::time::timeout( - Duration::from_secs(timeout_secs), - tokio::process::Command::new("/bin/bash") - .arg("-c") - .arg(&command) - .kill_on_drop(true) // N10: ensure timed-out processes are killed - .output(), - ) - .await; - - let duration = start.elapsed(); - - match result { - Ok(Ok(output)) => { - let stdout = String::from_utf8_lossy(&output.stdout); - let stderr = String::from_utf8_lossy(&output.stderr); - let exit_code = output.status.code().unwrap_or(-1); - - tracing::info!( - target: "mcp_audit", - command = %command, - exit_code = exit_code, - stdout_len = output.stdout.len(), - stderr_len = output.stderr.len(), - duration_ms = duration.as_millis() as u64, - "MCP shell command completed" - ); - - let mut text = String::new(); - if !stdout.is_empty() { - text.push_str(&stdout); - } - if !stderr.is_empty() { - if !text.is_empty() { - text.push('\n'); - } - text.push_str("[stderr]\n"); - text.push_str(&stderr); - } - if text.is_empty() { - text.push_str("(no output)"); - } - text.push_str(&format!("\n[exit code: {}]", exit_code)); - - McpResponse::ok( - id, - serde_json::to_value(&ToolsCallResult { - content: vec![ContentBlock::Text { text }], - is_error: if exit_code != 0 { Some(true) } else { None }, - }) - .unwrap(), - ) - } - Ok(Err(e)) => { - tracing::warn!( - target: "mcp_audit", - command = %command, - error = %e, - "MCP shell command failed to execute" - ); - McpResponse::ok( - id, - serde_json::to_value(&ToolsCallResult { - content: vec![ContentBlock::Text { - text: format!("Failed to execute command: {e}"), - }], - is_error: Some(true), - }) - .unwrap(), - ) - } - Err(_) => { - tracing::warn!( - target: "mcp_audit", - command = %command, - timeout_secs = timeout_secs, - "MCP shell command timed out" - ); - McpResponse::ok( - id, - serde_json::to_value(&ToolsCallResult { - content: vec![ContentBlock::Text { - text: format!("Command timed out after {timeout_secs} seconds"), - }], - is_error: Some(true), - }) - .unwrap(), - ) - } - } -} - async fn handle_package_shell_exec( ctx: &RpcContext, id: Option, diff --git a/core/src/mcp/tools.rs b/core/src/mcp/tools.rs index 94e3e152f..54ce0e71a 100644 --- a/core/src/mcp/tools.rs +++ b/core/src/mcp/tools.rs @@ -1311,31 +1311,8 @@ pub fn tool_registry() -> HashMap { needs_session: false, }, // ===================================================================== - // Shell execution + // Shell execution (package containers only — no host-level shell access) // ===================================================================== - ToolEntry { - definition: ToolDefinition { - name: "system.shell".into(), - description: "Execute a shell command on the StartOS server as the start9 user \ - with passwordless sudo. Returns stdout, stderr, and exit code. Use this for \ - diagnostics, inspecting files, checking processes, or any operation not \ - covered by other tools. THIS IS POWERFUL: the command runs with full system \ - access. Always be careful with destructive commands. Commands have a 30-second \ - timeout by default.".into(), - input_schema: json!({ - "type": "object", - "properties": { - "command": { "type": "string", "description": "Shell command to execute (passed to /bin/bash -c)" }, - "timeout": { "type": "integer", "description": "Timeout in seconds. Default: 30. Max: 300." } - }, - "required": ["command"], - "additionalProperties": false - }), - }, - rpc_method: "__shell__", - sync_db: false, - needs_session: false, - }, ToolEntry { definition: ToolDefinition { name: "package.shell".into(),