Skip to content

Commit 68e337e

Browse files
committed
Security: Require a bearer token on the MCP HTTP server
The MCP server binds 127.0.0.1 but accepted requests with no Origin header, so any local process that read the world-readable `<data_dir>/mcp.port` could POST `delete`/`move`/`copy` with `autoConfirm:true` and bypass the user's confirmation dialog. Origin validation is browser-CSRF defense only — a non-browser process sets any header it likes — so it was no barrier to a local attacker. macOS doesn't isolate loopback between processes. - Generate a fresh per-instance bearer token (122-bit getrandom UUID) at server start; require it on every `/mcp` request via `validate_token`, using a constant-time comparison. `GET /mcp/health` stays open. Fails closed when no token is set. `validate_origin` stays as a secondary browser layer. - Write the token to `<data_dir>/mcp.token` at 0o600, and harden the `mcp.port` (and the wrapper-written `tauri-mcp.port`) to 0o600 too — they were 0o644. - `autoConfirm` on destructive tools is now transitively gated by the server token (the caller had to read the 0o600 token to connect). - Expose `get_mcp_token` IPC; the bundled `scripts/mcp-call.sh` and the E2E `mcp-client.ts` now send `Authorization: Bearer <token>`. The app's own frontend talks to the separate Tauri MCP bridge, not this HTTP server, so no frontend change. External MCP clients must now read `mcp.token` and present it (intended contract change). TDD: token-validation and 0o600-permission tests written red-first.
1 parent 6cabc94 commit 68e337e

13 files changed

Lines changed: 396 additions & 49 deletions

File tree

apps/desktop/scripts/instance-id.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,11 @@ export function writePortFile(dir, name, port) {
281281
const tmpPath = join(dir, `${name}.tmp.${String(process.pid)}`)
282282
let fd = null
283283
try {
284-
fd = openSync(tmpPath, 'w')
284+
// 0o600 (owner-only) baked in at create time (no umask 0o644 window). The atomic
285+
// rename below preserves the mode, so the final file is owner-only too. Mirrors the
286+
// Rust `port_file.rs` hardening. The third `openSync` arg is ignored on platforms
287+
// without POSIX mode bits (Windows).
288+
fd = openSync(tmpPath, 'w', 0o600)
285289
writeSync(fd, `${String(port)}\n`)
286290
fsyncSync(fd)
287291
} finally {

apps/desktop/scripts/instance-id.test.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect } from 'vitest'
2-
import { mkdtempSync, readFileSync, existsSync, writeFileSync, readdirSync } from 'fs'
2+
import { mkdtempSync, readFileSync, existsSync, writeFileSync, readdirSync, statSync } from 'fs'
33
import { tmpdir } from 'os'
44
import { join } from 'path'
55
import {
@@ -301,6 +301,13 @@ describe('writePortFile + removePortFile', () => {
301301
expect(stragglers).toEqual([])
302302
})
303303

304+
it.skipIf(process.platform === 'win32')('writes the port file owner-only (0o600)', () => {
305+
const dir = mkdtempSync(join(tmpdir(), 'cmdr-port-file-test-'))
306+
writePortFile(dir, 'tauri-mcp.port', 54321)
307+
const mode = statSync(join(dir, 'tauri-mcp.port')).mode & 0o777
308+
expect(mode).toBe(0o600)
309+
})
310+
304311
it('rejects out-of-range port values', () => {
305312
const dir = mkdtempSync(join(tmpdir(), 'cmdr-port-file-test-'))
306313
expect(() => writePortFile(dir, 'tauri-mcp.port', -1)).toThrow(/u16/)

apps/desktop/src-tauri/src/commands/mcp.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,13 @@ pub fn get_mcp_running() -> bool {
4545
pub fn get_mcp_port() -> Option<u16> {
4646
mcp::get_mcp_actual_port()
4747
}
48+
49+
/// Returns the per-instance MCP bearer token, or null if the server isn't running.
50+
/// Used by the E2E harness (which runs outside the app) to authenticate `/mcp` requests
51+
/// after fetching it via the Tauri page. The in-app frontend never talks to the HTTP
52+
/// server (it uses the Tauri MCP bridge), so it doesn't need this.
53+
#[tauri::command]
54+
#[specta::specta]
55+
pub fn get_mcp_token() -> Option<String> {
56+
mcp::current_mcp_token()
57+
}

apps/desktop/src-tauri/src/ipc.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,7 @@ pub fn builder() -> Builder<tauri::Wry> {
464464
crate::commands::mcp::set_mcp_port,
465465
crate::commands::mcp::get_mcp_running,
466466
crate::commands::mcp::get_mcp_port,
467+
crate::commands::mcp::get_mcp_token,
467468
crate::commands::settings::check_port_available,
468469
crate::commands::settings::find_available_port,
469470
crate::commands::settings::update_file_watcher_debounce,

apps/desktop/src-tauri/src/ipc_collectors.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ pub(crate) fn collect_cross_platform_types(types: &mut Types) -> Vec<Function> {
159159
// set_mcp_enabled, set_mcp_port are generic (<R: Runtime>): excluded from specta
160160
crate::commands::mcp::get_mcp_running,
161161
crate::commands::mcp::get_mcp_port,
162+
crate::commands::mcp::get_mcp_token,
162163
crate::commands::settings::check_port_available,
163164
crate::commands::settings::find_available_port,
164165
crate::commands::settings::update_file_watcher_debounce,

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ Expose Cmdr functionality to AI agents via the Model Context Protocol (MCP). Age
1010

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.
13-
- Writes the actual bound port to `<data_dir>/mcp.port` atomically (tempfile + fsync + rename) after `bind`. External readers (the `scripts/mcp-call.sh` CLI, E2E fixtures, agent helpers) discover the port from that file; the FE still uses the `get_mcp_port` IPC to read the same in-process atomic. On clean shutdown the file is removed; on crash it stays and readers discover staleness via `ECONNREFUSED` on the contained port. See `port_file.rs` for the read/write/remove API and typed `PortDiscoveryError`.
13+
- 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.
1415
- Streamable HTTP transport (MCP spec 2025-11-25)
1516
- Endpoints: `POST /mcp` (JSON-RPC), `GET /mcp` (optional SSE), `GET /mcp/health`
1617

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

108109
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.
109110

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

112-
Binding to `0.0.0.0` would expose the server to the network. An attacker could quit the app, change settings, or navigate to sensitive directories. Localhost binding ensures only local processes can connect.
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.
114+
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.
116+
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.
113118

114119
### Why separate state stores?
115120

@@ -119,7 +124,7 @@ Binding to `0.0.0.0` would expose the server to the network. An attacker could q
119124

120125
### Server lifecycle is managed at runtime
121126

122-
`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` 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. 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.
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.
123128

124129
### Live MCP control only works from the settings window
125130

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ use super::{AckSignal, DEFAULT_ACK_TIMEOUT, PaneStateStore, ToolError, ToolResul
1515
/// is managed by the frontend. The validation happens in the frontend event handler
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 {
18+
// `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`.
1821
let auto_confirm = params.get("autoConfirm").and_then(|v| v.as_bool()).unwrap_or(false);
1922
let on_conflict = params.get("onConflict").and_then(|v| v.as_str()).unwrap_or("skip_all");
2023

@@ -57,6 +60,8 @@ pub async fn execute_copy<R: Runtime>(app: &AppHandle<R>, params: &Value) -> Too
5760
/// is managed by the frontend. The validation happens in the frontend event handler
5861
/// which will show an appropriate error if no files are selected.
5962
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.
6065
let auto_confirm = params.get("autoConfirm").and_then(|v| v.as_bool()).unwrap_or(false);
6166
let on_conflict = params.get("onConflict").and_then(|v| v.as_str()).unwrap_or("skip_all");
6267

@@ -101,6 +106,8 @@ pub async fn execute_move<R: Runtime>(app: &AppHandle<R>, params: &Value) -> Too
101106
/// is managed by the frontend. The validation happens in the frontend event handler
102107
/// which will show an appropriate error if no files are selected.
103108
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.
104111
let auto_confirm = params.get("autoConfirm").and_then(|v| v.as_bool()).unwrap_or(false);
105112

106113
let pre_gen = snapshot_generation(app);

apps/desktop/src-tauri/src/mcp/mod.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,7 @@ mod tests;
2020
pub use config::McpConfig;
2121
pub use dialog_state::SoftDialogTracker;
2222
pub use pane_state::PaneStateStore;
23-
pub use server::{get_mcp_actual_port, is_mcp_running, start_mcp_server, start_mcp_server_background, stop_mcp_server};
23+
pub use server::{
24+
current_mcp_token, get_mcp_actual_port, is_mcp_running, start_mcp_server, start_mcp_server_background,
25+
stop_mcp_server,
26+
};

0 commit comments

Comments
 (0)