Skip to content

Commit 18cd4c3

Browse files
committed
Security: Gate only destructive auto-confirm on the MCP token
- Narrow the MCP bearer-token requirement to the calls that bypass the user's in-app confirmation dialog: `delete`/`move`/`copy` with `autoConfirm: true`, and the `dialog` tool's `action: "confirm"`. Reads, navigation, search, SSE, and destructive ops that still prompt the user all work without a token. - Add the pure, unit-tested `tool_call_requires_token` predicate and enforce it at the POST-handler boundary; remove the server-wide gate (GET/SSE no longer gated). - New 401 helper `auto_confirm_token_required_response` returns a uniform message that points callers at `CMDR_MCP_TOKEN` and the resolved `<data_dir>/mcp.token` path, never echoing the token. - Add a `CMDR_MCP_TOKEN` env override: a fixed token is stable across restarts (static client header) at the cost of per-launch replay protection. Opt-in for the dev workflow. - Document the whole scheme in `mcp/CLAUDE.md` (what's gated, why, how to obtain/send the token, the env override, and a `.mcp.json` recipe for an external client).
1 parent 68e337e commit 18cd4c3

4 files changed

Lines changed: 209 additions & 37 deletions

File tree

apps/desktop/src-tauri/src/commands/CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ immediately to business-logic modules. No significant logic lives here.
2222
| `file_viewer.rs` | File viewer | Session lifecycle, regex/literal search with mode flags, word wrap, menu state, encoding pickers (`viewer_set_encoding` / `viewer_get_encoding_options`), tail mode (`viewer_set_tail_mode`), `viewer_reload` |
2323
| `ui.rs` | UI / menu | Context menu (file/breadcrumb/tab/network host), Finder reveal, clipboard, Quick Look, Get Info, view mode, `set_menu_context` (enables/disables file-scoped menu items based on window focus), `cloud_make_available_offline` / `cloud_remove_download` (iCloud Drive eviction/download via `FileManager` ubiquity APIs; see `file_system/cloud_actions.rs`) |
2424
| `settings.rs` | Settings | Port availability check, watcher debounce, menu accelerator updates, and live-apply setters for `network.directSmbConnection`, `advanced.filterSafeSaveArtifacts`, `network.smbConcurrency` |
25-
| `mcp.rs` | MCP server | `set_mcp_enabled`, `set_mcp_port`: live start/stop/port-change of the MCP server without app restart |
25+
| `mcp.rs` | MCP server | `set_mcp_enabled`, `set_mcp_port`: live start/stop/port-change of the MCP server without app restart. `get_mcp_token`: returns the per-instance bearer token so in-process / E2E callers can authenticate the destructive-auto-confirm tools (see `mcp/CLAUDE.md` § Authentication) |
2626
| `licensing.rs` | Licensing | Status query, activation, expiry, reminder, key validation |
2727
| `indexing.rs` | Drive index | `start_drive_index`, `stop_drive_index`, `get_index_status`, `get_dir_stats`, `get_dir_stats_batch`, `clear_drive_index`, `set_indexing_enabled`, `get_index_debug_status` (dev-only extended stats). Uses `State<IndexManagerState>`. |
2828
| `clipboard.rs` | Clipboard file ops | `copy_files_to_clipboard`, `cut_files_to_clipboard`, `copy_paths_to_clipboard` / `cut_paths_to_clipboard` (paths-by-value siblings used by the search-results pane, which has no backend listing for index-based ops), `read_clipboard_files`, `clear_clipboard_cut_state`. macOS uses NSPasteboard via `clipboard::pasteboard`; non-macOS stubs return errors. |

apps/desktop/src-tauri/src/mcp/CLAUDE.md

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ Expose Cmdr functionality to AI agents via the Model Context Protocol (MCP). Age
1111
- Runs in a background tokio task spawned at app startup
1212
- Binds `127.0.0.1` only (localhost). Port defaults to ephemeral: when `developer.mcpPort` (the user setting) is 0 (the new default), the server binds `127.0.0.1:0` and asks the kernel for an unused port. When the setting (or `CMDR_MCP_PORT` env) is non-zero, the server binds that port and probes upward up to 100 ports on collision. The `resolve_bind_strategy` helper turns the resolved port into `BindStrategy::Ephemeral` or `BindStrategy::Pinned(port)` and is unit-tested in `server.rs`. The legacy fixed defaults (`19224` prod / `19225` dev) still live in `config.rs::DEFAULT_PORT` as the fallback for `CMDR_MCP_PORT` parse failures and are mirrored in the FE registry for users who want to pin a port.
1313
- Writes the actual bound port to `<data_dir>/mcp.port` atomically (tempfile + fsync + rename, mode 0o600) after `bind`, plus a fresh per-instance bearer token to `<data_dir>/mcp.token` (same atomic write, 0o600). External readers (the `scripts/mcp-call.sh` CLI, E2E fixtures, agent helpers) discover the port and token from those files and send `Authorization: Bearer <token>` on every request; the FE still uses the `get_mcp_port` / `get_mcp_token` IPC to read the same in-process state. On clean shutdown both files are removed and the in-process token is cleared; on crash they stay and readers discover staleness via `ECONNREFUSED`. See `port_file.rs` for the `write_port_file` / `write_secret_file` / read / remove API and typed `PortDiscoveryError`.
14-
- **Auth**: every `/mcp` request (POST + GET) is gated by `validate_origin` (browser layer) then `validate_token` (the real local-process gate). `validate_token` reads `Authorization: Bearer <token>`, compares against the in-process token in constant time, and rejects missing/empty/mismatched with 401. It fails closed if no token is set. `GET /mcp/health` is intentionally unauthenticated.
14+
- **Auth**: every `/mcp` request runs `validate_origin` (browser-CSRF / DNS-rebinding layer). The bearer token is then required only for the narrow set of calls that bypass the user's in-app confirmation dialog — see the Authentication section below. `validate_token` reads `Authorization: Bearer <token>`, compares against the in-process token in constant time, and returns `Err(())` on missing/empty/mismatched; the POST handler turns that into a friendly in-band JSON-RPC error via `auto_confirm_token_required_response` (HTTP 200, **not** 401 — see Authentication for why). It fails closed if no token is set. `GET /mcp/health` and `GET /mcp` (SSE) are unauthenticated.
1515
- Streamable HTTP transport (MCP spec 2025-11-25)
1616
- Endpoints: `POST /mcp` (JSON-RPC), `GET /mcp` (optional SSE), `GET /mcp/health`
1717

@@ -108,13 +108,60 @@ Without state, resources would need to query the frontend on every read (slow, a
108108

109109
Security via parity: agents can only do what users can do. Giving agents `fs.read`/`fs.write` would violate this. Agents navigate the UI just like users, using `move_cursor`, `open_under_cursor`, etc.
110110

111-
### Why localhost only, and why a bearer token on top?
111+
### Why localhost only?
112112

113-
Binding to `0.0.0.0` would expose the server to the network, so we bind `127.0.0.1` only. But localhost binding alone is **not** a security boundary against the real threat: a local non-Cmdr process. macOS doesn't isolate loopback between local processes, and a non-browser process can set any HTTP header (or none), so `validate_origin` (a browser-CSRF / DNS-rebinding defense) is no barrier to it. Without more, any local process that read the (formerly world-readable) `mcp.port` could POST a destructive `delete`/`move`/`copy` with `autoConfirm: true` and bypass the user's confirmation dialog.
113+
Binding to `0.0.0.0` would expose the server to the network, so we bind `127.0.0.1` only. But localhost binding alone is **not** a security boundary against the real threat: a local non-Cmdr process. macOS doesn't isolate loopback between local processes, and a non-browser process can set any HTTP header (or none), so `validate_origin` (a browser-CSRF / DNS-rebinding defense) is no barrier to it. That's why the bearer token exists — see Authentication.
114114

115-
The real gate is a **per-instance bearer token**: every `/mcp` request (POST and GET) must carry `Authorization: Bearer <token>`, validated in constant time against the in-process token (`validate_token` in `server.rs`). The token is a fresh CSPRNG value (`Uuid::new_v4`, 122 random bits) generated on every server start and written to `<data_dir>/mcp.token` at mode 0o600 (owner-only). Reading it therefore requires the user's own filesystem access — the same access an attacker would need to do damage directly. The port file (`mcp.port`) is also written 0o600 now. `GET /mcp/health` stays open (no token) so liveness probes work. With the whole server token-gated, `autoConfirm` is transitively protected: the caller already proved filesystem access by reading the token.
115+
## Authentication
116116

117-
Legit clients send the token: `scripts/mcp-call.sh` reads `<data_dir>/mcp.token` (or `CMDR_MCP_TOKEN`); the E2E harness fetches it via the `get_mcp_token` Tauri IPC. The app's own frontend does NOT talk to this HTTP server (it uses the separate Tauri MCP bridge), so it needs no token.
117+
### What's gated
118+
119+
The bearer token is required for **only the calls that bypass the user's in-app confirmation dialog**:
120+
121+
- `delete` / `move` / `copy` with `autoConfirm: true`, and
122+
- the `dialog` tool with `action: "confirm"` (programmatically confirming an open dialog).
123+
124+
**Everything else needs no token**: resource reads (`cmdr://state`, `cmdr://logs`, etc.), navigation, search, and the destructive ops that still pop the confirmation dialog (`autoConfirm` absent/false). The decision lives in one pure, unit-tested predicate, `tool_call_requires_token(method, params)` in `server.rs`: true iff `method == "tools/call"` and the tool+args match one of the two cases above.
125+
126+
The POST handler runs `if tool_call_requires_token(..) && validate_token(..).is_err() { reject }`. `GET /mcp` (SSE) carries no tool call, so it's never gated. `GET /mcp/health` stays open for liveness probes.
127+
128+
### Why only those
129+
130+
The threat is a **local non-Cmdr process silently auto-confirming a destructive op** — POSTing `delete`/`move`/`copy` with `autoConfirm: true` (or a `dialog` confirm) to wipe files without the user ever seeing the dialog. `validate_origin` is browser-CSRF defense only; it's no barrier to a local process that sets its own headers. Gating reads/nav/search bought no security (those mirror what the user can already do in the UI) while making the server annoying to drive — so the gate is now exactly the auto-confirm bypass, nothing more.
131+
132+
### How a client obtains and sends the token
133+
134+
The token is a fresh CSPRNG value (`Uuid::new_v4`, 122 random bits), generated on every server start and written to `<data_dir>/mcp.token` at mode 0o600 (owner-only). The port file (`mcp.port`) is 0o600 too. A client gets the token by either:
135+
136+
- reading `<data_dir>/mcp.token` (the same filesystem access an attacker would need to do damage directly), or
137+
- reading the `CMDR_MCP_TOKEN` env var (see override below).
138+
139+
It then sends `Authorization: Bearer <token>` on the gated calls. `scripts/mcp-call.sh` already does this (reads `<data_dir>/mcp.token`, or `CMDR_MCP_TOKEN`); the E2E harness fetches it via the `get_mcp_token` Tauri IPC. The app's own frontend does NOT talk to this HTTP server (it uses the separate Tauri MCP bridge), so it needs no token.
140+
141+
The rejection is returned as an **in-band JSON-RPC error at HTTP 200, not a 401**. The MCP Streamable-HTTP spec reserves HTTP 401 for an OAuth challenge, so a 401 makes clients (Claude Code, etc.) launch an OAuth discovery flow and surface a confusing "Invalid OAuth error response" instead of our message. A 200 + JSON-RPC `error` is the canonical application-error shape and renders client-side as `MCP error <code>: <message>` (the same path `nav_to_path`'s "path does not exist" takes). Our bearer gate isn't OAuth, so it must not look like one.
142+
143+
The message tells the caller exactly where the token lives (the `CMDR_MCP_TOKEN` env var and the resolved `<data_dir>/mcp.token` path). That's safe: the secret is the file's 0o600 contents and the env value, not the path, which is already discoverable. The message never echoes the token, and it's one uniform message for missing-vs-wrong token (no oracle).
144+
145+
### `CMDR_MCP_TOKEN` override
146+
147+
If `CMDR_MCP_TOKEN` is set and non-empty (after trim) when the server starts, that value is used as the token instead of a random one. The token file is still written 0o600 with the chosen value. Tradeoff: a fixed env token is **stable across restarts** (so a static `Authorization: Bearer ${CMDR_MCP_TOKEN}` client header keeps working) but **loses the per-launch replay protection** the random token gives. It's opt-in for the dev workflow, not the default.
148+
149+
### Letting an external MCP client auto-confirm
150+
151+
To let an external MCP client (for example Claude Code) issue auto-confirming destructive ops, export `CMDR_MCP_TOKEN` for the running Cmdr and add a matching header to the server entry in `.mcp.json`:
152+
153+
```json
154+
{
155+
"mcpServers": {
156+
"cmdr": {
157+
"url": "http://127.0.0.1:<port>/mcp",
158+
"headers": { "Authorization": "Bearer ${CMDR_MCP_TOKEN}" }
159+
}
160+
}
161+
}
162+
```
163+
164+
Without that header the client still works for everything else (reads, nav, search, and destructive ops that prompt in the app); only auto-confirm and `dialog` confirm are rejected (as the in-band JSON-RPC error above).
118165

119166
### Why separate state stores?
120167

@@ -124,7 +171,7 @@ Legit clients send the token: `scripts/mcp-call.sh` reads `<data_dir>/mcp.token`
124171

125172
### Server lifecycle is managed at runtime
126173

127-
`start_mcp_server()` binds the port and spawns a tokio task, storing the `JoinHandle` in a static `MCP_HANDLE`. Port-binding strategy comes from `resolve_bind_strategy(port)`: a 0 setting (the new default) means `BindStrategy::Ephemeral`, which binds `127.0.0.1:0` and reads the assigned port via `local_addr()`. A non-zero setting means `BindStrategy::Pinned(port)`, which uses `bind_with_probe()` to try tokio `TcpListener::bind` directly and retry up to 100 ports on collision (avoids the TOCTOU race of checking with a sync listener then re-binding async). The actual bound port is stored in `MCP_ACTUAL_PORT`. After `bind`, `write_port_file` lands `<data_dir>/mcp.port` (0o600) atomically (tempfile + fsync + rename, see `port_file.rs`) so external readers can discover the port without IPC; the data dir is cached in `MCP_PORT_FILE_DIR` for the shutdown path. A fresh CSPRNG bearer token is also generated and stored in the `MCP_TOKEN` static and written to `<data_dir>/mcp.token` (0o600) via `write_secret_file`, regenerated on every start. `stop_mcp_server()` and the crash path both clear `MCP_TOKEN` to None (so `validate_token` fails closed) and remove the token file alongside the port file. The frontend queries the in-process atomic via `get_mcp_port()` and shows "(ephemeral)" when the setting was 0 or "(port N was in use)" when the pinned port differs from the bound one. The server can be started/stopped live via `set_mcp_enabled` and `set_mcp_port` Tauri commands, no app restart needed. `stop_mcp_server()` aborts the task, resets `MCP_ACTUAL_PORT` to 0, and removes `<data_dir>/mcp.port` (best-effort: stale file is not a correctness bug). `is_mcp_running()` checks whether the handle exists. At startup, `start_mcp_server_background()` wraps the async start in a fire-and-forget spawn. If the server crashes mid-serve, `MCP_ACTUAL_PORT` resets to 0 but the on-disk file may linger; external readers retry on `ECONNREFUSED`. Check logs for "MCP server crashed" errors.
174+
`start_mcp_server()` binds the port and spawns a tokio task, storing the `JoinHandle` in a static `MCP_HANDLE`. Port-binding strategy comes from `resolve_bind_strategy(port)`: a 0 setting (the new default) means `BindStrategy::Ephemeral`, which binds `127.0.0.1:0` and reads the assigned port via `local_addr()`. A non-zero setting means `BindStrategy::Pinned(port)`, which uses `bind_with_probe()` to try tokio `TcpListener::bind` directly and retry up to 100 ports on collision (avoids the TOCTOU race of checking with a sync listener then re-binding async). The actual bound port is stored in `MCP_ACTUAL_PORT`. After `bind`, `write_port_file` lands `<data_dir>/mcp.port` (0o600) atomically (tempfile + fsync + rename, see `port_file.rs`) so external readers can discover the port without IPC; the data dir is cached in `MCP_PORT_FILE_DIR` for the shutdown path. A bearer token is also stored in the `MCP_TOKEN` static and written to `<data_dir>/mcp.token` (0o600) via `write_secret_file` on every start. It's a fresh CSPRNG value by default, or the `CMDR_MCP_TOKEN` env value when that's set non-empty (see Authentication § override). `stop_mcp_server()` and the crash path both clear `MCP_TOKEN` to None (so `validate_token` fails closed) and remove the token file alongside the port file. The frontend queries the in-process atomic via `get_mcp_port()` and shows "(ephemeral)" when the setting was 0 or "(port N was in use)" when the pinned port differs from the bound one. The server can be started/stopped live via `set_mcp_enabled` and `set_mcp_port` Tauri commands, no app restart needed. `stop_mcp_server()` aborts the task, resets `MCP_ACTUAL_PORT` to 0, and removes `<data_dir>/mcp.port` (best-effort: stale file is not a correctness bug). `is_mcp_running()` checks whether the handle exists. At startup, `start_mcp_server_background()` wraps the async start in a fire-and-forget spawn. If the server crashes mid-serve, `MCP_ACTUAL_PORT` resets to 0 but the on-disk file may linger; external readers retry on `ECONNREFUSED`. Check logs for "MCP server crashed" errors.
128175

129176
### Live MCP control only works from the settings window
130177

apps/desktop/src-tauri/src/mcp/executor/file_ops.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@ use super::{AckSignal, DEFAULT_ACK_TIMEOUT, PaneStateStore, ToolError, ToolResul
1616
/// which will show an appropriate error if no files are selected.
1717
pub async fn execute_copy<R: Runtime>(app: &AppHandle<R>, params: &Value) -> ToolResult {
1818
// `autoConfirm: true` skips the user's confirmation dialog. This is safe because the
19-
// whole MCP HTTP server is token-gated (see `mcp/server.rs::validate_token`): any caller
20-
// already proved filesystem access by reading the 0o600 `<data_dir>/mcp.token`.
19+
// POST-handler boundary gates exactly this case: `tool_call_requires_token` flags
20+
// destructive auto-confirm (and the `dialog` confirm action), so any caller that reaches
21+
// here already proved filesystem access by reading the 0o600 `<data_dir>/mcp.token`.
2122
let auto_confirm = params.get("autoConfirm").and_then(|v| v.as_bool()).unwrap_or(false);
2223
let on_conflict = params.get("onConflict").and_then(|v| v.as_str()).unwrap_or("skip_all");
2324

@@ -60,8 +61,9 @@ pub async fn execute_copy<R: Runtime>(app: &AppHandle<R>, params: &Value) -> Too
6061
/// is managed by the frontend. The validation happens in the frontend event handler
6162
/// which will show an appropriate error if no files are selected.
6263
pub async fn execute_move<R: Runtime>(app: &AppHandle<R>, params: &Value) -> ToolResult {
63-
// `autoConfirm: true` skips the user's confirmation dialog; the server-level token gate
64-
// (`mcp/server.rs::validate_token`) is what protects this now.
64+
// `autoConfirm: true` skips the user's confirmation dialog; the POST-handler token gate
65+
// (`tool_call_requires_token` in `mcp/server.rs`) is what protects this now — it flags
66+
// destructive auto-confirm (and the `dialog` confirm action), not the whole server.
6567
let auto_confirm = params.get("autoConfirm").and_then(|v| v.as_bool()).unwrap_or(false);
6668
let on_conflict = params.get("onConflict").and_then(|v| v.as_str()).unwrap_or("skip_all");
6769

@@ -106,8 +108,9 @@ pub async fn execute_move<R: Runtime>(app: &AppHandle<R>, params: &Value) -> Too
106108
/// is managed by the frontend. The validation happens in the frontend event handler
107109
/// which will show an appropriate error if no files are selected.
108110
pub async fn execute_delete<R: Runtime>(app: &AppHandle<R>, params: &Value) -> ToolResult {
109-
// `autoConfirm: true` skips the user's confirmation dialog; the server-level token gate
110-
// (`mcp/server.rs::validate_token`) is what protects this now.
111+
// `autoConfirm: true` skips the user's confirmation dialog; the POST-handler token gate
112+
// (`tool_call_requires_token` in `mcp/server.rs`) is what protects this now — it flags
113+
// destructive auto-confirm (and the `dialog` confirm action), not the whole server.
111114
let auto_confirm = params.get("autoConfirm").and_then(|v| v.as_bool()).unwrap_or(false);
112115

113116
let pre_gen = snapshot_generation(app);

0 commit comments

Comments
 (0)