collective
Browse / github.com/storytold/designcraft / patch
Control channel: a connection presents this launch's token first (#1585)
Control channel: a connection presents this launch's token first (#1585) Audit **DC-04** (#1585), Confirmed: the control channel ran every command for anyone who could connect. ## Why it matters `--control <port>` reaches **every** engine command — open, save and export with arbitrary paths, and quitting without saving. The existing hardening is good (opt-in, loopback only, non-JSON first lines rejected, line and connection limits), but loopback is not a boundary *between users*: on a shared machine, or beside other local software, a process that is not the user could act with the user's file permissions while the channel was on. ## The token ```json {"token":
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- Hamza0101 (imported from pull request #1910, open)
- Base
50650f74d7328745cfcdd16098313a9fc31b23d5- Size
- +304 −15 in 8 files
- Review
- pending
- Shared
- 2026-10-28
- Id
2ae94d82127bdbe80e7a7447
How it went
No other operator has reported using it yet.
Diff
raw · Cargo.lock apps/designcraft-cli/src/main.rs apps/designcraft/Cargo.toml apps/designcraft/src/control_server.rs apps/designcraft/src/control_token.rs apps/designcraft/src/main.rs crates/mcp/src/backend.rs docs/control-protocol.md
@@ -1302,6 +1302,7 @@ dependencies = [ "eframe", "egui", "fmv-macos-events",+ "getrandom 0.4.3", "log", "muda", "objc2 0.6.5",@@ -144,6 +144,21 @@ fn main() -> ExitCode { } } +/// The control token a client presents: `DESIGNCRAFT_CONTROL_TOKEN`, else the file the app wrote+/// in its profile directory, named by `DESIGNCRAFT_CONTROL_TOKEN_FILE`. The app prints where that+/// file is when it opens the channel (#1585).+fn control_token() -> Option<String> {+ if let Ok(t) = std::env::var("DESIGNCRAFT_CONTROL_TOKEN") {+ let t = t.trim().to_string();+ if !t.is_empty() {+ return Some(t);+ }+ }+ let path = std::env::var_os("DESIGNCRAFT_CONTROL_TOKEN_FILE")?;+ let t = std::fs::read_to_string(path).ok()?.trim().to_string();+ (!t.is_empty()).then_some(t)+}+ fn report(r: Result<(), String>) -> ExitCode { match r { Ok(()) => ExitCode::SUCCESS,@@ -223,7 +238,7 @@ so drop --connect (or use --connect without the roots)" Some(c) => { let addr = control_addr(&c); Box::new(- Remote::connect(&addr)+ Remote::connect_with_token(&addr, control_token()) .map_err(|e| format!("cannot connect to the DesignCraft app at {addr}: {e} (start it with `designcraft --control PORT`)"))?, ) }@@ -506,7 +521,7 @@ fn script(args: &[String]) -> Result<(), String> { Some(c) => { let addr = control_addr(c); Box::new(- Remote::connect(&addr)+ Remote::connect_with_token(&addr, control_token()) .map_err(|e| format!("cannot connect to the DesignCraft app at {addr}: {e} (start it with `designcraft --control PORT`)"))?, ) }@@ -567,7 +582,7 @@ fn app(args: &[String]) -> Result<(), String> { } } let addr = control_addr(&port);- let mut r = Remote::connect(&addr)+ let mut r = Remote::connect_with_token(&addr, control_token()) .map_err(|e| format!("cannot connect to the DesignCraft app at {addr}: {e} (start it with `designcraft --control {port}`)"))?; let json_arg = |s: Option<&String>| -> Result<Value, String> { s.map_or(Ok(json!({})), |t| serde_json::from_str(t).map_err(|e| format!("bad JSON: {e}"))) };@@ -11,6 +11,8 @@ default-run = "designcraft" workspace = true [dependencies]+# The control channel's per-launch token comes from the OS random source (#1585).+getrandom = "0.4" designcraft-engine = { workspace = true } designcraft-ui-egui = { workspace = true } eframe = { workspace = true, features = ["default_fonts", "wgpu", "accesskit", "wayland", "x11", "links", "clipboard"] }@@ -23,7 +23,7 @@ pub const MAX_LINE: usize = 4 * 1024 * 1024; /// Connections served at once; further ones get an error line and are closed. pub const MAX_CONNECTIONS: usize = 16; -pub fn start(port: u16, ctx: egui::Context) -> Receiver<ControlRequest> {+pub fn start(port: u16, token: String, ctx: egui::Context) -> Receiver<ControlRequest> { let (tx, rx) = channel::<ControlRequest>(); let listener = match TcpListener::bind(("127.0.0.1", port)) { Ok(l) => l,@@ -33,11 +33,11 @@ pub fn start(port: u16, ctx: egui::Context) -> Receiver<ControlRequest> { } }; eprintln!("designcraft: control server listening on 127.0.0.1:{port}");- std::thread::spawn(move || accept_loop(listener, tx, ctx));+ std::thread::spawn(move || accept_loop(listener, tx, ctx, token)); rx } -fn accept_loop(listener: TcpListener, tx: Sender<ControlRequest>, ctx: egui::Context) {+fn accept_loop(listener: TcpListener, tx: Sender<ControlRequest>, ctx: egui::Context, token: String) { let open = Arc::new(AtomicUsize::new(0)); for mut stream in listener.incoming().flatten() { if open.fetch_add(1, Ordering::SeqCst) >= MAX_CONNECTIONS {@@ -48,16 +48,17 @@ fn accept_loop(listener: TcpListener, tx: Sender<ControlRequest>, ctx: egui::Con let tx = tx.clone(); let ctx = ctx.clone(); let open = Arc::clone(&open);+ let token = token.clone(); std::thread::spawn(move || {- serve(stream, &tx, &ctx);+ serve(stream, &tx, &ctx, &token); open.fetch_sub(1, Ordering::SeqCst); }); } } -fn serve(stream: TcpStream, tx: &Sender<ControlRequest>, ctx: &egui::Context) {+fn serve(stream: TcpStream, tx: &Sender<ControlRequest>, ctx: &egui::Context, token: &str) { let Ok(read) = stream.try_clone() else { return };- serve_lines(read, stream, |method, params| {+ serve_lines_with_token(read, stream, token, |method, params| { let (req, rrx) = ControlRequest::new(method, params); tx.send(req).ok()?; ctx.request_repaint();@@ -126,6 +127,49 @@ fn read_bounded_line(reader: &mut impl BufRead, buf: &mut Vec<u8>, max: usize) - Ok(true) } +/// [`serve_lines`] behind the launch's token: the first line of a connection must be the token+/// and nothing else, so a process that cannot read the token file runs nothing (#1585).+///+/// The reply to the token line says only whether it was accepted. A wrong one closes the+/// connection, as any other line that is not a request does.+fn serve_lines_with_token<R: Read, W: Write>(read: R, mut out: W, token: &str, handle: impl FnMut(String, Value) -> Option<Value>) {+ let mut reader = BufReader::new(read);+ let mut buf = Vec::new();+ loop {+ match read_bounded_line(&mut reader, &mut buf, MAX_LINE) {+ Ok(false) => return,+ Err(e) => {+ let _ = writeln!(out, "{}", json!({"ok": false, "error": e})).and_then(|()| out.flush());+ return;+ }+ Ok(true) => {}+ }+ let text = String::from_utf8_lossy(&buf);+ let text = text.trim();+ if text.is_empty() {+ continue;+ }+ // The token may come alone on the line or as `{"token": "..."}`, so a client can send it+ // as JSON like everything else on this channel.+ let given = match serde_json::from_str::<Value>(text) {+ Ok(v) => v.get("token").and_then(Value::as_str).map(str::to_string).unwrap_or_default(),+ Err(_) => text.to_string(),+ };+ if crate::control_token::matches(token, &given) {+ let _ = writeln!(out, "{}", json!({"ok": true})).and_then(|()| out.flush());+ break;+ }+ let _ = writeln!(+ out,+ "{}",+ json!({"ok": false, "error": "the control channel needs this launch's token as the first line; closing the connection"})+ )+ .and_then(|()| out.flush());+ return;+ }+ serve_lines(reader, out, handle);+}+ /// Serve requests from `read`, answering on `out`, until the peer closes, sends something that is /// not a request, or `handle` returns `None` (app gone). `handle` gets `(method, params)` and /// returns the reply object (the request's `id` is added here).@@ -171,6 +215,54 @@ mod tests { (replies, calls) } + /// The channel reaches every command, so a connection runs nothing until it has presented+ /// this launch's token (#1585).+ #[test]+ fn nothing_runs_until_the_token_is_presented() {+ const TOKEN: &str = "0123456789abcdef0123456789abcdef";+ let serve = |input: &str| {+ let mut out = Vec::new();+ let mut calls: Vec<String> = Vec::new();+ serve_lines_with_token(input.as_bytes(), &mut out, TOKEN, |m, _p| {+ calls.push(m);+ Some(json!({"ok": true, "result": null}))+ });+ let replies: Vec<Value> =+ String::from_utf8(out).unwrap().lines().filter(|l| !l.is_empty()).map(|l| serde_json::from_str(l).unwrap()).collect();+ (replies, calls)+ };+ let request = r#"{"id":1,"method":"engine.execute","params":{}}"#;++ // The token, then a request: the token is acknowledged and the request runs.+ let (replies, calls) = serve(&format!("{{\"token\":\"{TOKEN}\"}}\n{request}\n"));+ assert_eq!(calls, ["engine.execute"], "the request ran");+ assert_eq!(replies[0]["ok"], json!(true), "the token was acknowledged");+ assert_eq!(replies[1]["id"], json!(1), "and the reply carries the request's id");++ // The bare token on a line works too, for a client that is not sending JSON yet.+ let (_, calls) = serve(&format!("{TOKEN}\n{request}\n"));+ assert_eq!(calls, ["engine.execute"]);++ // No token: the request never reaches the engine, and the connection closes.+ let (replies, calls) = serve(&format!("{request}\n{request}\n"));+ assert!(calls.is_empty(), "nothing ran without the token");+ assert_eq!(replies.len(), 1, "one error and the connection is closed");+ assert_eq!(replies[0]["ok"], json!(false));+ assert!(replies[0]["error"].as_str().unwrap_or_default().contains("token"), "{:?}", replies[0]);++ // A wrong token, including one that is a prefix of the real one: nothing runs.+ for wrong in [&TOKEN[..31], "0123456789abcdef0123456789abcdee", "", "{\"token\":null}"] {+ let (replies, calls) = serve(&format!("{wrong}\n{request}\n"));+ assert!(calls.is_empty(), "nothing ran for {wrong:?}");+ assert_eq!(replies[0]["ok"], json!(false), "{wrong:?}");+ }++ // A browser's cross-origin request still gets one error and no execution.+ let (replies, calls) = serve("GET / HTTP/1.1\r\n");+ assert!(calls.is_empty());+ assert_eq!(replies[0]["ok"], json!(false));+ }+ #[test] fn requests_are_answered_in_order_with_ids() { let (r, calls) = run(b"{\"id\":1,\"method\":\"a\"}\n\n{\"id\":2,\"method\":\"b\",\"params\":{}}\r\n{\"method\":\"c\"}");@@ -266,7 +358,7 @@ mod tests { let listener = TcpListener::bind(("127.0.0.1", 0)).unwrap(); let addr = listener.local_addr().unwrap(); let (tx, _rx) = channel::<ControlRequest>();- std::thread::spawn(move || accept_loop(listener, tx, egui::Context::default()));+ std::thread::spawn(move || accept_loop(listener, tx, egui::Context::default(), "token-for-the-test".into())); // Idle connections are accepted in order and held open. let held: Vec<TcpStream> = (0..16).map(|_| TcpStream::connect(addr).unwrap()).collect(); let mut extra = TcpStream::connect(addr).unwrap();new file mode 100644@@ -0,0 +1,105 @@+//! The token a control-channel client presents before it can run anything.+//!+//! `--control <port>` reaches every engine command: opening, saving and exporting with arbitrary+//! paths, and quitting without saving. The channel is opt-in and loopback only, but loopback is+//! not a boundary between users -- on a shared machine, or beside other local software, any+//! process that can connect could act with the user's file permissions (#1585).+//!+//! Each launch makes a fresh token, writes it to the profile directory readable only by its owner,+//! and prints where it is. A connection's first line must be that token.++use std::io::Write;+use std::path::{Path, PathBuf};++/// A client may pass the token here instead of reading the file.+pub const TOKEN_ENV: &str = "DESIGNCRAFT_CONTROL_TOKEN";+/// The file's name inside the profile directory.+pub const TOKEN_FILE: &str = "control-token";+/// Token length in hex characters (128 bits).+const TOKEN_HEX: usize = 32;++/// A fresh token from the operating system's random source, or `None` when it cannot be read --+/// in which case the channel must not start, rather than start without a token.+pub fn new_token() -> Option<String> {+ let mut bytes = [0u8; TOKEN_HEX / 2];+ getrandom::fill(&mut bytes).ok()?;+ Some(bytes.iter().map(|b| format!("{b:02x}")).collect())+}++/// `<profile>/control-token`.+pub fn token_path(config_dir: &Path) -> PathBuf {+ config_dir.join(TOKEN_FILE)+}++/// Write `token` where only its owner can read it, replacing any previous one.+pub fn write_token(config_dir: &Path, token: &str) -> std::io::Result<PathBuf> {+ std::fs::create_dir_all(config_dir)?;+ let path = token_path(config_dir);+ // Create with the mode set, rather than writing and then narrowing: a token must never exist+ // readable, however briefly.+ let mut options = std::fs::OpenOptions::new();+ options.write(true).create(true).truncate(true);+ #[cfg(unix)]+ {+ use std::os::unix::fs::OpenOptionsExt;+ options.mode(0o600);+ }+ let mut file = options.open(&path)?;+ file.write_all(token.as_bytes())?;+ file.flush()?;+ Ok(path)+}++/// Whether `given` is `expected`, compared without giving away where they differ.+pub fn matches(expected: &str, given: &str) -> bool {+ let (a, b) = (expected.as_bytes(), given.as_bytes());+ // The length is not a secret; the bytes are, so every one of them is looked at.+ if a.len() != b.len() {+ return false;+ }+ a.iter().zip(b).fold(0u8, |acc, (x, y)| acc | (x ^ y)) == 0+}++#[cfg(test)]+mod tests {+ use super::*;++ #[test]+ fn a_token_is_128_random_bits_in_hex() {+ let a = new_token().expect("the OS random source");+ let b = new_token().expect("the OS random source");+ assert_eq!(a.len(), TOKEN_HEX);+ assert!(a.chars().all(|c| c.is_ascii_hexdigit()));+ assert_ne!(a, b, "a fresh token each time");+ }++ #[test]+ fn a_token_matches_only_itself() {+ let t = "0123456789abcdef0123456789abcdef";+ assert!(matches(t, t));+ assert!(!matches(t, &t[..31]), "a prefix is not the token");+ assert!(!matches(t, &format!("{t}0")), "nor is a longer string");+ assert!(!matches(t, "0123456789abcdef0123456789abcdee"), "nor one byte off");+ assert!(!matches(t, ""), "nor nothing");+ }++ #[test]+ fn the_token_file_is_written_for_its_owner_only() {+ let dir = std::env::temp_dir().join(format!("dc-token-{}", std::process::id()));+ let _ = std::fs::remove_dir_all(&dir);+ let token = new_token().unwrap();+ let path = write_token(&dir, &token).expect("the token is written");+ assert_eq!(std::fs::read_to_string(&path).unwrap(), token);+ #[cfg(unix)]+ {+ use std::os::unix::fs::PermissionsExt;+ let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777;+ assert_eq!(mode, 0o600, "only its owner can read the token: {mode:o}");+ }+ // Writing again replaces it, and leaves no second file behind.+ let next = new_token().unwrap();+ write_token(&dir, &next).unwrap();+ assert_eq!(std::fs::read_to_string(&path).unwrap(), next);+ let _ = std::fs::remove_dir_all(&dir);+ }+}@@ -16,6 +16,7 @@ #[cfg(target_os = "macos")] mod apple_events; mod control_server;+mod control_token; mod gpu; mod gpu_limits; mod logging;@@ -575,8 +576,26 @@ fn main() -> std::process::ExitCode { app.integrated_titlebar = cfg!(target_os = "macos"); app.custom_titlebar = cfg!(target_os = "windows"); if let Some(port) = control_port {- let rx = control_server::start(port, cc.egui_ctx.clone());- app = app.with_control(rx);+ // The channel reaches every command, so it opens only with a token a client+ // has to read from the profile directory (#1585). Without a random source+ // there is no token, and the channel does not open at all: starting it+ // unauthenticated would be the thing the token is for.+ match control_token::new_token() {+ Some(token) => {+ match profile::config_dir().map(|dir| control_token::write_token(&dir, &token)) {+ Some(Ok(path)) => eprintln!("designcraft: the control token is in {}", path.display()),+ Some(Err(e)) => {+ eprintln!("designcraft: the control token could not be written: {e} (pass it with {})", control_token::TOKEN_ENV)+ }+ None => {+ eprintln!("designcraft: no profile directory for the control token (pass it with {})", control_token::TOKEN_ENV)+ }+ }+ let rx = control_server::start(port, token, cc.egui_ctx.clone());+ app = app.with_control(rx);+ }+ None => eprintln!("designcraft: the control server did not start: no random source for its token"),+ } } if sample { let _ = app.run("file.newSample", serde_json::json!({}));@@ -28,12 +28,22 @@ pub struct Remote { addr: String, conn: Option<(BufReader<TcpStream>, TcpStream)>, next_id: u64,+ /// The launch's control token, sent as the first line of every connection (#1585).+ token: Option<String>, } impl Remote { /// Connect to `addr` (`127.0.0.1:7979`), failing fast when nothing is listening.+ ///+ /// The app's control channel asks for the launch's token first (#1585), so pass it with+ /// [`Self::connect_with_token`]; this stays for callers talking to a channel without one. pub fn connect(addr: &str) -> std::io::Result<Self> {- let mut r = Self { addr: addr.to_string(), conn: None, next_id: 1 };+ Self::connect_with_token(addr, None)+ }++ /// [`Self::connect`], presenting `token` as the first line of the connection.+ pub fn connect_with_token(addr: &str, token: Option<String>) -> std::io::Result<Self> {+ let mut r = Self { addr: addr.to_string(), conn: None, next_id: 1, token }; r.reconnect()?; Ok(r) }@@ -53,6 +63,7 @@ impl Remote { s.set_read_timeout(Some(Duration::from_secs(90))).ok(); let read = s.try_clone()?; self.conn = Some((BufReader::new(read), s));+ self.greet()?; return Ok(()); } Err(e) => last = e,@@ -61,6 +72,31 @@ impl Remote { Err(last) } + /// Present the token, if there is one, and read the app's answer. A channel that asks for a+ /// token and does not get one closes, so a clear error here beats a mystery later.+ fn greet(&mut self) -> std::io::Result<()> {+ let Some(token) = self.token.clone() else { return Ok(()) };+ let Some((reader, writer)) = self.conn.as_mut() else {+ return Err(std::io::Error::new(std::io::ErrorKind::NotConnected, "no connection"));+ };+ writeln!(writer, "{}", serde_json::json!({"token": token}))?;+ writer.flush()?;+ let mut line = String::new();+ reader.read_line(&mut line)?;+ let ok = serde_json::from_str::<serde_json::Value>(line.trim())+ .ok()+ .and_then(|v| v.get("ok").and_then(serde_json::Value::as_bool))+ .unwrap_or(false);+ if ok {+ return Ok(());+ }+ self.conn = None;+ Err(std::io::Error::new(+ std::io::ErrorKind::PermissionDenied,+ "the app refused the control token (it is in the profile directory, or pass DESIGNCRAFT_CONTROL_TOKEN)",+ ))+ }+ fn prepare_connection(&mut self) -> std::io::Result<()> { if let Some((reader, writer)) = self.conn.take() { writer.set_nonblocking(true)?;@@ -3,6 +3,24 @@ Start the app with `--control <port>` (or `DESIGNCRAFT_CONTROL_PORT`). The server listens on `127.0.0.1` only and speaks JSON lines: one request object per line, one reply per line. +**The first line of every connection is this launch's token.** The channel reaches every command+-- open, save and export with any path, and quitting without saving -- and loopback is not a+boundary between users, so a connection runs nothing until it presents the token:++```json+{"token": "0f8c…"}+{"ok": true}+```++A bare token on its own line is accepted too. A wrong or missing token gets one error reply and+the connection is closed, like any other line that is not a request.++Each launch makes a fresh 128-bit token and writes it to the profile directory as `control-token`,+readable only by its owner (mode 600 on unix); the app prints the path on startup. Clients:+`designcraft-cli mcp --connect` reads `DESIGNCRAFT_CONTROL_TOKEN`, else the file named by+`DESIGNCRAFT_CONTROL_TOKEN_FILE`. Without a random source for a token the channel does not open at+all, rather than opening without one.+ For an isolated native test profile, set `DESIGNCRAFT_CONFIG_DIR` to a dedicated directory before launch. UI/engine preferences and the `Recovery` directory then live under that root. An unset override preserves the usual platform paths (or `DesignCraftData` next to the executable when a@@ -22,8 +40,9 @@ nothing sent after it on that connection runs. That covers text that isn't JSON, an object without `method`, invalid UTF-8, and a line longer than 4 MiB. An HTTP request (for example a web page's cross-origin `fetch` to `127.0.0.1:<port>`) therefore can't smuggle a command in its body: its request line is rejected first. At most 16 connections are served at once; further ones get an error-line and are closed. Clients that get an error reply should reconnect. The port has no authentication,-so only enable it while you use it. Transport: `apps/designcraft/src/control_server.rs`.+line and are closed. Clients that get an error reply should reconnect. The token above is what+stands between the channel and any other local process, so still only enable the port while you+use it. Transport: `apps/designcraft/src/control_server.rs`. ```json {"id": 1, "method": "engine.execute", "params": {"command": "frame.create", "params": {"rect": [36, 36, 300, 200], "content": "text"}}}