From ca578a8fb7d065e8a06c18b2dba6effea47488c3 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Thu, 22 Oct 2026 17:35:18 -0400 Subject: [PATCH] security: authenticate the local control server with a per-session token (CWE-306) (#1844) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * security: authenticate the local control server with a per-session token (CWE-306) The optional `--control PORT` server listens on loopback TCP and dispatched any JSON request without authenticating the peer. Loopback TCP does not distinguish operating-system users, so another local account could connect and drive the desktop user's command and file authority (for example reading files through native Lisp, or create/truncate writes). Authenticate the peer with an unguessable per-session token: - New `cadcraft_mcp::control_token` module: `issue(port)` generates a 256-bit token and writes it to `cadcraft-control-.token` in the per-user runtime directory with owner-only permissions (0600 on Unix; per-user LOCALAPPDATA on Windows). `read(port)` reads it; `matches` compares in constant time. - The control server issues the token at startup and requires every request to carry a matching `token` field; requests without one get an `unauthorized` reply and the connection is closed before dispatch. - The MCP bridge (`Remote`) reads the token from the owner-only file — which needs the desktop user's own filesystem access — and sends it on every request, so the bridge keeps working unchanged for the legitimate user. A different local user cannot read the token and cannot authenticate. Updates `docs/control-protocol.md`. New tests cover token issue/read/compare, file permissions, and that the server refuses unauthenticated requests while accepting authenticated ones. Addresses finding row 48. * fix: recover control authentication across server restarts --------- Co-authored-by: skylartaylor <26588250+skylartaylor@users.noreply.github.com> --- Cargo.lock | 2 + apps/cadcraft/Cargo.toml | 2 + apps/cadcraft/src/control_server.rs | 86 +++++++++++-- crates/mcp/Cargo.toml | 2 + crates/mcp/src/backend.rs | 119 ++++++++++++++++-- crates/mcp/src/control_token.rs | 180 ++++++++++++++++++++++++++++ crates/mcp/src/lib.rs | 1 + docs/control-protocol.md | 10 +- 8 files changed, 376 insertions(+), 26 deletions(-) create mode 100644 crates/mcp/src/control_token.rs diff --git a/Cargo.lock b/Cargo.lock index 4fc46f3c..0e7f5097 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -551,6 +551,7 @@ version = "0.11.0" dependencies = [ "cadcraft-engine", "cadcraft-io", + "cadcraft-mcp", "cadcraft-ui-egui", "chrono", "eframe", @@ -687,6 +688,7 @@ dependencies = [ "cadcraft-engine", "cadcraft-io", "cadcraft-render", + "getrandom 0.3.4", "log", "serde_json", ] diff --git a/apps/cadcraft/Cargo.toml b/apps/cadcraft/Cargo.toml index 886eac1e..7a46fa9b 100644 --- a/apps/cadcraft/Cargo.toml +++ b/apps/cadcraft/Cargo.toml @@ -14,6 +14,8 @@ workspace = true cadcraft-engine = { workspace = true } cadcraft-ui-egui = { workspace = true } cadcraft-io = { workspace = true } +# For the shared per-session control-token helper (control_server.rs authentication). +cadcraft-mcp = { workspace = true } eframe = { workspace = true, features = ["default_fonts", "wgpu", "accesskit", "wayland", "x11", "links", "persistence"] } egui = { workspace = true } serde_json = { workspace = true } diff --git a/apps/cadcraft/src/control_server.rs b/apps/cadcraft/src/control_server.rs index e678762c..d11671e1 100644 --- a/apps/cadcraft/src/control_server.rs +++ b/apps/cadcraft/src/control_server.rs @@ -51,8 +51,21 @@ impl Default for Limits { pub fn start(port: u16, ctx: egui::Context) -> Receiver { match TcpListener::bind(("127.0.0.1", port)) { Ok(l) => { - log::info!("control server listening on 127.0.0.1:{port}"); - serve_listener(l, Limits::default(), ctx) + // Loopback TCP cannot tell local users apart, so issue an unguessable per-session + // token in an owner-only file and require it on every request (CWE-306). A client + // (the MCP bridge) reads it from the file, which needs the desktop user's filesystem + // access; another local account cannot. + let token = match cadcraft_mcp::control_token::issue(port) { + Ok(t) => Arc::new(t), + Err(e) => { + log::error!("control server not started: could not write the control token: {e}"); + return sync_channel::(0).1; + } + }; + if let Ok(path) = cadcraft_mcp::control_token::token_path(port) { + log::info!("control server listening on 127.0.0.1:{port} (token in {})", path.display()); + } + serve_listener(l, Limits::default(), ctx, token) } Err(e) => { log::error!("control server failed to bind 127.0.0.1:{port}: {e}"); @@ -72,9 +85,9 @@ fn accepted(incoming: impl Iterator>, mut wait: imp } /// Accept connections on `listener` under `limits`; requests arrive on the returned receiver. -fn serve_listener(listener: TcpListener, limits: Limits, ctx: egui::Context) -> Receiver { +fn serve_listener(listener: TcpListener, limits: Limits, ctx: egui::Context, token: Arc) -> Receiver { let (tx, rx) = sync_channel::(limits.queue.max(1)); - std::thread::spawn(move || accept_loop(listener, tx, ctx, limits)); + std::thread::spawn(move || accept_loop(listener, tx, ctx, limits, token)); rx } @@ -88,7 +101,7 @@ impl Drop for Slot { } } -fn accept_loop(listener: TcpListener, tx: SyncSender, ctx: egui::Context, limits: Limits) { +fn accept_loop(listener: TcpListener, tx: SyncSender, ctx: egui::Context, limits: Limits, token: Arc) { let open = Arc::new(AtomicUsize::new(0)); for mut stream in accepted(listener.incoming(), || std::thread::sleep(ACCEPT_RETRY)) { let _ = stream.set_write_timeout(Some(limits.write_timeout)); @@ -100,9 +113,10 @@ fn accept_loop(listener: TcpListener, tx: SyncSender, ctx: egui: let slot = Slot(Arc::clone(&open)); let tx = tx.clone(); let ctx = ctx.clone(); + let token = Arc::clone(&token); let _ = std::thread::Builder::new().spawn(move || { let _slot = slot; - serve(stream, &tx, &ctx, limits); + serve(stream, &tx, &ctx, limits, &token); }); } } @@ -141,6 +155,7 @@ enum Line { id: Value, method: String, params: Value, + token: Option, }, Blank, /// Not a request: reply with this error and close the connection. @@ -172,6 +187,7 @@ fn parse_line(bytes: &[u8]) -> Line { id: obj.get("id").cloned().unwrap_or(Value::Null), method: method.to_string(), params: obj.get("params").cloned().unwrap_or(json!({})), + token: obj.get("token").and_then(Value::as_str).map(str::to_string), } } @@ -182,7 +198,7 @@ fn is_http_request_line(text: &str) -> bool { || text.rsplit_once(' ').is_some_and(|(_, version)| version.starts_with("HTTP/")) } -fn serve(stream: TcpStream, tx: &SyncSender, ctx: &egui::Context, limits: Limits) { +fn serve(stream: TcpStream, tx: &SyncSender, ctx: &egui::Context, limits: Limits, token: &str) { if stream.set_read_timeout(Some(limits.idle_timeout)).is_err() || stream.set_write_timeout(Some(limits.write_timeout)).is_err() { return; } @@ -200,7 +216,18 @@ fn serve(stream: TcpStream, tx: &SyncSender, ctx: &egui::Context Ok(true) => match parse_line(&buf) { Line::Blank => continue, Line::Reject(e) => e, - Line::Request { id, method, params } => { + Line::Request { id, method, params, token: given } => { + // Authenticate the peer before dispatching: loopback TCP does not identify the + // OS user, so only a client holding the owner-only session token may drive the + // desktop user's commands (CWE-306). + if !given.map(|g| cadcraft_mcp::control_token::matches(token, &g)).unwrap_or(false) { + let mut r = json!({"ok": false, "error": "unauthorized: missing or invalid control token", "state": "not-run"}); + if let Some(o) = r.as_object_mut() { + o.insert("id".into(), id); + } + let _ = writeln!(out, "{r}"); + break; + } let (req, pending) = ControlRequest::new(method, params); let mut r = match tx.try_send(req) { Ok(()) => { @@ -277,11 +304,14 @@ mod tests { } } + /// A fixed token for tests; real tokens are random (see `cadcraft_mcp::control_token`). + const TEST_TOKEN: &str = "test-control-token"; + /// The control server on a loopback port under `limits`; requests arrive on the receiver. fn serve_with(limits: Limits) -> (std::net::SocketAddr, Receiver) { let listener = TcpListener::bind(("127.0.0.1", 0)).unwrap(); let addr = listener.local_addr().unwrap(); - (addr, serve_listener(listener, limits, egui::Context::default())) + (addr, serve_listener(listener, limits, egui::Context::default(), Arc::new(TEST_TOKEN.to_string()))) } /// The control server on a loopback port, with a stand-in UI that echoes each method. @@ -295,6 +325,11 @@ mod tests { addr } + /// A request line carrying the test token, so it authenticates. + fn req_line(id: i64, method: &str) -> String { + json!({"id": id, "method": method, "token": TEST_TOKEN}).to_string() + "\n" + } + /// Send `input` on a new connection and read reply lines until the server closes it. fn exchange(addr: std::net::SocketAddr, input: &[u8]) -> Vec { let mut c = TcpStream::connect(addr).unwrap(); @@ -354,12 +389,12 @@ mod tests { fn a_full_queue_answers_busy() { let (addr, rx) = serve_with(short()); // No UI drains the one-place queue: the first request fills it, the second finds it full. - let first = std::thread::spawn(move || exchange(addr, b"{\"id\":1,\"method\":\"m\"}\n")); + let first = std::thread::spawn(move || exchange(addr, req_line(1, "m").as_bytes())); let deadline = std::time::Instant::now() + Duration::from_secs(4); let busy = loop { let mut c = TcpStream::connect(addr).unwrap(); c.set_read_timeout(Some(Duration::from_secs(5))).unwrap(); - c.write_all(b"{\"id\":2,\"method\":\"m\"}\n").unwrap(); + c.write_all(req_line(2, "m").as_bytes()).unwrap(); let mut line = String::new(); BufReader::new(c).read_line(&mut line).unwrap(); let v: Value = serde_json::from_str(&line).unwrap(); @@ -377,7 +412,8 @@ mod tests { let addr = serve_echo(); let mut c = TcpStream::connect(addr).unwrap(); c.set_read_timeout(Some(Duration::from_secs(5))).unwrap(); - c.write_all(b"{\"id\":3,\"method\":\"ping\"}\r\n\n{\"id\":4,\"method\":\"pong\"}\n").unwrap(); + let input = format!("{}\n{}", req_line(3, "ping").trim_end(), req_line(4, "pong")); + c.write_all(input.as_bytes()).unwrap(); let mut r = BufReader::new(c); for (id, method) in [(3, "ping"), (4, "pong")] { let mut line = String::new(); @@ -387,6 +423,32 @@ mod tests { } } + #[test] + fn a_request_without_the_token_is_unauthorized() { + // A local peer that does not hold the session token cannot drive the app (CWE-306). + let addr = serve_echo(); + let replies = exchange(addr, b"{\"id\":9,\"method\":\"ping\"}\n"); + assert_eq!(replies.len(), 1, "{replies:?}"); + assert_eq!(replies[0]["ok"], false); + assert!(replies[0]["error"].as_str().unwrap().contains("unauthorized"), "{replies:?}"); + // A wrong token is also refused. + let wrong = exchange(addr, b"{\"id\":9,\"method\":\"ping\",\"token\":\"nope\"}\n"); + assert_eq!(wrong[0]["ok"], false); + assert!(wrong[0]["error"].as_str().unwrap().contains("unauthorized"), "{wrong:?}"); + } + + #[test] + fn a_request_with_the_token_is_dispatched() { + let addr = serve_echo(); + let mut c = TcpStream::connect(addr).unwrap(); + c.set_read_timeout(Some(Duration::from_secs(5))).unwrap(); + c.write_all(req_line(10, "ping").as_bytes()).unwrap(); + let mut line = String::new(); + BufReader::new(c).read_line(&mut line).unwrap(); + let v: Value = serde_json::from_str(&line).unwrap(); + assert_eq!((v["ok"].as_bool(), v["method"].as_str()), (Some(true), Some("ping")), "{line}"); + } + #[test] fn extra_connections_are_refused() { let addr = serve_echo(); diff --git a/crates/mcp/Cargo.toml b/crates/mcp/Cargo.toml index 9bfc18e2..c4452955 100644 --- a/crates/mcp/Cargo.toml +++ b/crates/mcp/Cargo.toml @@ -14,3 +14,5 @@ cadcraft-render = { workspace = true } cadcraft-io = { workspace = true } serde_json = { workspace = true } log = { workspace = true } +# Secure randomness for the per-session control token (MIT OR Apache-2.0). +getrandom = "0.3" diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs index cbde5c7b..c758bac9 100644 --- a/crates/mcp/src/backend.rs +++ b/crates/mcp/src/backend.rs @@ -23,6 +23,14 @@ pub struct Remote { next_id: u64, /// Open the app when nothing answers (see [`Remote::lazy`]). launch: bool, + /// The per-session control token for this port, read from the owner-only token file on first + /// use so requests can authenticate (CWE-306). + token: Option, +} + +/// The port component of a `HOST:PORT` address, for locating the token file. +fn port_of(addr: &str) -> Option { + addr.rsplit(':').next().and_then(|p| p.parse::().ok()) } /// How long a launched app has to open its control port. @@ -30,7 +38,7 @@ const LAUNCH_WAIT: Duration = Duration::from_secs(30); impl Remote { pub fn connect(addr: &str) -> std::io::Result { - let mut r = Self { addr: addr.to_string(), conn: None, next_id: 1, launch: false }; + let mut r = Self { addr: addr.to_string(), conn: None, next_id: 1, launch: false, token: None }; r.reconnect()?; Ok(r) } @@ -38,7 +46,20 @@ impl Remote { /// the server before the app is open. With `launch`, a call that finds nothing at `addr` /// opens the app with that control port first (macOS) and waits for it. pub fn lazy(addr: &str, launch: bool) -> Self { - Self { addr: addr.to_string(), conn: None, next_id: 1, launch } + Self { addr: addr.to_string(), conn: None, next_id: 1, launch, token: None } + } + + /// The per-session token for this port, read (and cached) from the owner-only token file. A + /// missing file is not fatal here: the request is sent without a token and the server replies + /// with an authentication error the user can act on. + fn token(&mut self) -> Option<&str> { + if self.token.is_none() + && let Some(port) = port_of(&self.addr) + && let Ok(t) = crate::control_token::read(port) + { + self.token = Some(t); + } + self.token.as_deref() } /// Connect, opening the app first when asked to and nothing answers. fn reach(&mut self) -> std::io::Result<()> { @@ -91,7 +112,9 @@ impl Remote { let sent = writer.write_all(line.as_bytes()).and_then(|()| writer.write_all(b"\n")).and_then(|()| writer.flush()); if let Err(e) = sent { self.conn = None; - return Err((e, false)); + // write_all can fail after partial delivery. The server may have received the full + // JSON before the newline write failed, so a mutation must not be retried blindly. + return Err((e, true)); } let mut reply = String::new(); match reader.read_line(&mut reply) { @@ -162,17 +185,35 @@ impl Backend for Remote { fn call(&mut self, method: &str, params: Value) -> Result { let id = self.next_id; self.next_id += 1; - let line = json!({"id": id, "method": method, "params": params}).to_string(); - let reply = match self.roundtrip(&line) { + let token = self.token().map(str::to_string); + let mut req = json!({"id": id, "method": method, "params": params}); + if let (Some(obj), Some(token)) = (req.as_object_mut(), token) { + obj.insert("token".into(), Value::String(token)); + } + let reply = match self.roundtrip(&req.to_string()) { Ok(r) => r, - // Resend only when the request cannot have been applied. After a sent mutation with no reply the outcome is unknown, and - // resending could apply it twice (for example two circles from one call), so report that instead. - Err((_, sent)) if !sent || is_read_only(method) => self.roundtrip(&line).map_err(|(e, _)| format!("CADCraft at {}: {e}", self.addr))?, - Err((e, _)) => { - return Err(format!( - "CADCraft at {}: {e}; the reply was lost, so `{method}` may or may not have been applied. Inspect the drawing first", - self.addr - )); + Err((e, sent)) => { + self.token = None; + self.conn = None; + // A mutation whose reply was lost must never be sent twice. + if sent && !is_read_only(method) { + return Err(format!( + "CADCraft at {}: {e}; the reply was lost, so `{method}` may or may not have been applied. Inspect the drawing first", + self.addr + )); + } + // A restarted server may have rotated its token before dropping the old socket. + if let Some(obj) = req.as_object_mut() { + obj.remove("token"); + if let Some(token) = self.token() { + obj.insert("token".into(), Value::String(token.to_owned())); + } + } + self.roundtrip(&req.to_string()).map_err(|(e, _)| { + self.token = None; + self.conn = None; + format!("CADCraft at {}: {e}", self.addr) + })? } }; let v: Value = serde_json::from_str(&reply).map_err(|e| format!("bad reply: {e}"))?; @@ -180,6 +221,10 @@ impl Backend for Remote { Ok(v.get("result").cloned().unwrap_or(Value::Null)) } else { let e = v.get("error").and_then(Value::as_str).unwrap_or("error"); + if e.starts_with("unauthorized") { + self.token = None; + self.conn = None; + } // A timeout says whether the request can still have run (#345). Err(match v.get("state").and_then(Value::as_str) { Some("not-run") => format!("{e}: CADCraft did not start `{method}` and won't; it is safe to send again"), @@ -375,9 +420,12 @@ mod tests { requests }); let mut remote = Remote::connect(&addr).unwrap(); + remote.token = Some("stale-token".into()); let err = remote.call("engine.execute", json!({"command": "circle", "params": {}})).unwrap_err(); assert!(err.contains("may or may not have been applied"), "{err}"); assert_eq!(server.join().unwrap(), 1); + assert!(remote.token.is_none()); + assert!(remote.conn.is_none()); } /// The app's timeout reply says whether the request can still run; the error passes that on (#345). @@ -399,4 +447,49 @@ mod tests { server.join().unwrap(); } } + #[test] + fn safe_retry_rereads_a_rotated_token_after_a_lost_reply() { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let old = crate::control_token::issue(addr.port()).unwrap(); + let server = std::thread::spawn(move || { + let (conn, _) = listener.accept().unwrap(); + let mut line = String::new(); + let mut reader = BufReader::new(conn); + reader.read_line(&mut line).unwrap(); + let first: Value = serde_json::from_str(&line).unwrap(); + assert!(first["token"].as_str() == Some(old.as_str())); + let fresh = crate::control_token::issue(addr.port()).unwrap(); + drop(reader); + let (mut conn, _) = listener.accept().unwrap(); + line.clear(); + BufReader::new(conn.try_clone().unwrap()).read_line(&mut line).unwrap(); + let retry: Value = serde_json::from_str(&line).unwrap(); + assert!(retry["token"].as_str() == Some(fresh.as_str())); + writeln!(conn, "{}", json!({"ok": true, "result": "pong"})).unwrap(); + crate::control_token::revoke(addr.port()); + }); + let mut remote = Remote::connect(&addr.to_string()).unwrap(); + assert_eq!(remote.call("ping", json!({})).unwrap(), json!("pong")); + server.join().unwrap(); + } + #[test] + fn failed_mutation_write_is_not_resent() { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + let mut remote = Remote::connect(&addr).unwrap(); + remote.conn.as_ref().unwrap().1.shutdown(std::net::Shutdown::Write).unwrap(); + let server = std::thread::spawn(move || { + let (conn, _) = listener.accept().unwrap(); + let mut line = String::new(); + assert_eq!(BufReader::new(conn).read_line(&mut line).unwrap(), 0); + listener.set_nonblocking(true).unwrap(); + std::thread::sleep(Duration::from_millis(300)); + listener.accept().is_ok() + }); + let error = remote.call("engine.execute", json!({"command": "circle", "params": {}})).unwrap_err(); + assert!(error.contains("may or may not have been applied")); + assert!(!server.join().unwrap(), "a failed mutation write was retried"); + assert!(remote.token.is_none()); + } } diff --git a/crates/mcp/src/control_token.rs b/crates/mcp/src/control_token.rs new file mode 100644 index 00000000..84677ae4 --- /dev/null +++ b/crates/mcp/src/control_token.rs @@ -0,0 +1,180 @@ +//! A per-session credential for the loopback control channel. +//! +//! The control server listens on `127.0.0.1`, but loopback TCP does not distinguish operating +//! system users: any local account can connect and drive the desktop user's command and file +//! authority (CWE-306). To authenticate the peer without changing the transport, the app issues an +//! unguessable token at startup and writes it to a file only the desktop user can read. A client +//! (the MCP bridge) reads the token from that file — which requires the same user's filesystem +//! access — and sends it with every request. A different local user cannot read the token, so +//! cannot authenticate. + +use std::io; +use std::path::PathBuf; + +/// Length of the token in bytes (256 bits), rendered as lowercase hex. +const TOKEN_BYTES: usize = 32; + +/// The file holding the control token for `port`, inside a per-user private directory. +/// +/// On Unix the runtime dir (`XDG_RUNTIME_DIR`, already `0700` and per-user) is preferred, else the +/// user's home; on Windows the local app-data directory. The file itself is created with +/// owner-only permissions regardless. +pub fn token_path(port: u16) -> io::Result { + Ok(private_dir()?.join(format!("cadcraft-control-{port}.token"))) +} + +#[cfg(unix)] +fn private_dir() -> io::Result { + if let Some(d) = std::env::var_os("XDG_RUNTIME_DIR").filter(|v| !v.is_empty()) { + return Ok(PathBuf::from(d)); + } + if let Some(h) = std::env::var_os("HOME").filter(|v| !v.is_empty()) { + return Ok(PathBuf::from(h).join(".cache")); + } + Ok(std::env::temp_dir()) +} + +#[cfg(windows)] +fn private_dir() -> io::Result { + windows_private_dir(std::env::var_os("LOCALAPPDATA")) +} + +#[cfg(any(windows, test))] +fn windows_private_dir(local: Option) -> io::Result { + local + .filter(|p| !p.is_empty()) + .map(PathBuf::from) + .ok_or_else(|| io::Error::new(io::ErrorKind::NotFound, "LOCALAPPDATA is required for a private control token")) +} + +#[cfg(not(any(unix, windows)))] +fn private_dir() -> io::Result { + Err(io::Error::new(io::ErrorKind::Unsupported, "no private control token directory on this platform")) +} + +/// Generate a fresh token and write it to [`token_path`] with owner-only permissions, returning +/// the token. The file is replaced if it already exists. +pub fn issue(port: u16) -> io::Result { + let token = random_hex()?; + let path = token_path(port)?; + issue_at(&path, token.as_bytes())?; + Ok(token) +} + +fn issue_at(path: &std::path::Path, bytes: &[u8]) -> io::Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + write_private(path, bytes) +} + +/// Read the token for `port`, trimming trailing whitespace. Used by clients (the MCP bridge). +pub fn read(port: u16) -> io::Result { + let raw = std::fs::read_to_string(token_path(port)?)?; + let token = raw.trim().to_string(); + if token.is_empty() { + return Err(io::Error::new(io::ErrorKind::InvalidData, "empty control token")); + } + Ok(token) +} + +/// Remove the token file (best effort), called when the server stops. +pub fn revoke(port: u16) { + if let Ok(path) = token_path(port) { + let _ = std::fs::remove_file(path); + } +} + +/// Constant-time comparison of two tokens, so a wrong token cannot be guessed byte by byte. +pub fn matches(expected: &str, given: &str) -> bool { + let (a, b) = (expected.as_bytes(), given.as_bytes()); + if a.len() != b.len() { + return false; + } + let mut diff = 0u8; + for (x, y) in a.iter().zip(b.iter()) { + diff |= x ^ y; + } + diff == 0 +} + +fn random_hex() -> io::Result { + let mut bytes = [0u8; TOKEN_BYTES]; + getrandom::fill(&mut bytes).map_err(|e| io::Error::other(format!("no secure randomness: {e}")))?; + let mut s = String::with_capacity(TOKEN_BYTES * 2); + for b in bytes { + use std::fmt::Write as _; + let _ = write!(s, "{b:02x}"); + } + Ok(s) +} + +#[cfg(unix)] +fn write_private(path: &std::path::Path, bytes: &[u8]) -> io::Result<()> { + use std::io::Write; + use std::os::unix::fs::OpenOptionsExt; + // Replace any stale file, then create fresh with mode 0600 so no other user can read the token. + let _ = std::fs::remove_file(path); + let mut f = std::fs::OpenOptions::new().write(true).create_new(true).mode(0o600).open(path)?; + f.write_all(bytes)?; + f.flush() +} + +#[cfg(not(unix))] +fn write_private(path: &std::path::Path, bytes: &[u8]) -> io::Result<()> { + use std::io::Write; + // On Windows the per-user LOCALAPPDATA directory is already ACL-protected to the user. + let _ = std::fs::remove_file(path); + let mut f = std::fs::OpenOptions::new().write(true).create_new(true).open(path)?; + f.write_all(bytes)?; + f.flush() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn issue_then_read_round_trips() { + let port = 50000 + (std::process::id() % 10000) as u16; + let token = issue(port).unwrap(); + assert_eq!(token.len(), TOKEN_BYTES * 2); + assert_eq!(read(port).unwrap(), token); + revoke(port); + assert!(read(port).is_err()); + } + + #[test] + fn matches_is_exact() { + assert!(matches("abc", "abc")); + assert!(!matches("abc", "abd")); + assert!(!matches("abc", "ab")); + assert!(!matches("", "x")); + } + + #[cfg(unix)] + #[test] + fn token_file_is_owner_only() { + use std::os::unix::fs::PermissionsExt; + let port = 40000 + (std::process::id() % 10000) as u16; + issue(port).unwrap(); + let mode = std::fs::metadata(token_path(port).unwrap()).unwrap().permissions().mode(); + assert_eq!(mode & 0o077, 0, "token file is readable by group/other: {:o}", mode); + revoke(port); + } + #[test] + fn windows_requires_local_app_data() { + assert!(windows_private_dir(None).is_err()); + assert!(windows_private_dir(Some("".into())).is_err()); + assert_eq!(windows_private_dir(Some("private-app-data".into())).unwrap(), PathBuf::from("private-app-data")); + } + + #[test] + fn issuance_creates_missing_parent() { + let dir = std::env::temp_dir().join(format!("cadcraft-token-parent-{}", std::process::id())); + let path = dir.join("missing/cache/session.token"); + issue_at(&path, b"secret").unwrap(); + assert_eq!(std::fs::read(&path).unwrap(), b"secret"); + std::fs::remove_dir_all(dir).unwrap(); + } +} diff --git a/crates/mcp/src/lib.rs b/crates/mcp/src/lib.rs index 1d4d4162..7fcf4849 100644 --- a/crates/mcp/src/lib.rs +++ b/crates/mcp/src/lib.rs @@ -9,6 +9,7 @@ #![forbid(unsafe_code)] mod backend; +pub mod control_token; mod server; mod tools; diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 56cf4d99..b1e91a52 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -5,10 +5,18 @@ Start the desktop app with `--control PORT` (or `CADCRAFT_CONTROL_PORT=PORT`). I `--control` wins over the variable; a bare `--control` keeps the variable's port. ```json -{"id": 1, "method": "cmdline.input", "params": {"text": "circle 0,0 5"}} +{"id": 1, "method": "cmdline.input", "params": {"text": "circle 0,0 5"}, "token": ""} {"id": 1, "ok": true, "result": {"prompt": "Command:", "output": ["Command: CIRCLE"], ...}} ``` +Every request must carry the session `token`. Because loopback TCP cannot tell local operating +system users apart, the app writes an unguessable per-session token to an owner-only file when the +control server starts, and refuses (`unauthorized`) any request without a matching token. The file +is `cadcraft-control-.token` in the per-user runtime directory (`$XDG_RUNTIME_DIR`, else +`~/.cache`; `%LOCALAPPDATA%` on Windows); its path is logged at startup. A client reads the token +from that file — which requires the desktop user's own filesystem access — and includes it on each +line. The MCP bridge (`cadcraft-cli mcp --connect`) does this automatically. + Errors come back as `{"id": …, "ok": false, "error": "message"}`. An unknown method's error points to `control.methods`, which lists the methods the server answers.