From bded38dd2363b5ea9fc422814ff9fe95412f7ad5 Mon Sep 17 00:00:00 2001 From: Lucio Date: Sat, 10 Oct 2026 12:56:41 +0200 Subject: [PATCH] Control port: require a per-window key (split from #54) (#124) * MCP bridge: send the control key, to loopback addresses only `wordcraft-cli mcp --connect` now sends the app's control key with every request. It takes the key from WORDCRAFT_CONTROL_KEY, or else from the key file the app writes for that port, and reads it again on every new connection, also after an `unauthorized` reply (the app restarted with a new key). The key goes only to loopback addresses: 127.0.0.0/8, ::1 and `localhost`, and only when the connection really leads to a loopback address. For any other host the bridge sends no key, prints why at start, and turns the app's `unauthorized` into an error that says the key was not sent and how to reach the app (through 127.0.0.1, for example an SSH tunnel). Other host names are never resolved for this. The new `wordcraft_mcp::control_key` module holds the rule the app and the bridge share: the settings folder (the one with ui.json), the Flatpak instance id and `/control-key.` (`local-` outside Flatpak). No new dependency; wasm-safe. * Control port: every request needs the window's key Any local program, and possibly a web page, could drive the app through its loopback control port. Each window started with --control now has a key: - once the port is bound, the app writes a random key (32 bytes from the OS, as hex) to /control-key., mode 0600 on Unix, replacing a stale file; it removes the file on a clean exit. A failed bind writes nothing. is the Flatpak instance id, or local- outside Flatpak (wordcraft_mcp::control_key, the rule the MCP bridge reads it with). The settings folder is the one that holds ui.json; prefs_path now uses the same helper. - every request needs the key: top-level "key", or {"method":"auth","key":...} once per connection. Only auth reads params.key; elsewhere it stays an ordinary parameter (ui.key). The key is checked on every request, in time that does not depend on where a wrong key differs. A missing or wrong key gets {"ok":false,"error":"unauthorized"} and the connection is closed. - a line over 1 MiB gets "line too long" and the connection is closed (the rest of the line is drained first, so the client reads the error). - a connection that sends no authorised request within 30 s of being accepted is closed; blank lines and bad JSON do not extend that limit. After that it may stay idle. * Docs: control-port keys docs/control-protocol.md gets a Keys section: where the key file is (per platform and per instance), top-level "key" or auth once per connection, `unauthorized` and the closed connection, the 1 MiB line limit, the 30 s limit for the first authorised request, and how the MCP bridge finds the key and sends it only to loopback addresses (an SSH tunnel for an app on another machine). docs/mcp.md points there, and the agent guides (AGENTS.md, CLAUDE.md) show the key in their control channel example. * Control key: the key-file helpers in their own small crate wordcraft-control-key (standard library only, Standalone in the layering table) holds where the key file lives and how to read it. The app and the MCP bridge both use it, so the app no longer depends on wordcraft-mcp. * Cargo.lock: wordcraft-control-key follows the workspace version (0.4.0) Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: lpgn <2556951+lpgn@users.noreply.github.com> Co-authored-by: Brandon Thomas Co-authored-by: Claude Opus 5.5 --- AGENTS.md | 2 +- CLAUDE.md | 2 +- Cargo.lock | 7 + Cargo.toml | 1 + apps/wordcraft-cli/src/main.rs | 8 +- apps/wordcraft/Cargo.toml | 4 + apps/wordcraft/src/control_server.rs | 522 ++++++++++++++++++++++++--- apps/wordcraft/src/main.rs | 35 +- crates/control-key/Cargo.toml | 9 + crates/control-key/src/lib.rs | 83 +++++ crates/mcp/Cargo.toml | 1 + crates/mcp/src/backend.rs | 314 +++++++++++++++- docs/control-protocol.md | 38 +- docs/mcp.md | 4 + xtask/src/layers.rs | 1 + 15 files changed, 932 insertions(+), 99 deletions(-) create mode 100644 crates/control-key/Cargo.toml create mode 100644 crates/control-key/src/lib.rs diff --git a/AGENTS.md b/AGENTS.md index ac74d60..17fdc1f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -53,7 +53,7 @@ Typing, Enter, Backspace, Tab, lists, AutoCorrect and AutoFormat follow Word's o ## Running and looking at the app - `cargo run --release -p wordcraft -- --sample --control 7981` (sample document + control channel). -- Drive it with JSON lines on `127.0.0.1:7981`, e.g. `{"id":1,"method":"engine.execute","params":{"command":"text.insert","params":{"text":"Hello"}}}` then `{"id":2,"method":"ui.screenshot","params":{"path":"/tmp/shot.png"}}`. Methods: `crates/ui-egui/src/control.rs`; docs: `docs/control-protocol.md`. +- Drive it with JSON lines on `127.0.0.1:7981`, each with the window's key from `~/.config/wordcraft/control-key.local-7981` (macOS: `~/Library/Application Support/WordCraft/`), e.g. `{"id":1,"key":"…","method":"engine.execute","params":{"command":"text.insert","params":{"text":"Hello"}}}` then `{"id":2,"key":"…","method":"ui.screenshot","params":{"path":"/tmp/shot.png"}}`. Methods: `crates/ui-egui/src/control.rs`; docs: `docs/control-protocol.md` (Keys). - **Headless window screenshots** (no focus stealing, works with a locked screen): `cargo run --release -p wordcraft-ui-egui --example ui_shot -- script.jsonl` (see the example's header). - **For UI work, look at the result** (screenshot, read the PNG) and compare with Word side by side. - CLI: `wordcraft-cli convert in.docx out.pdf`, `wordcraft-cli run --template sample --cmd 'select.text={"text":"Studio"}' --cmd format.bold --save out.docx`, `wordcraft-cli commands`, `wordcraft-cli parity`. diff --git a/CLAUDE.md b/CLAUDE.md index a852ec1..586520e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -53,7 +53,7 @@ Typing, Enter, Backspace, Tab, lists, AutoCorrect and AutoFormat follow Word's o ## Running and looking at the app - `cargo run --release -p wordcraft -- --sample --control 7981` (sample document + control channel). -- Drive it with JSON lines on `127.0.0.1:7981`, e.g. `{"id":1,"method":"engine.execute","params":{"command":"text.insert","params":{"text":"Hello"}}}` then `{"id":2,"method":"ui.screenshot","params":{"path":"/tmp/shot.png"}}`. Methods: `crates/ui-egui/src/control.rs`; docs: `docs/control-protocol.md`. +- Drive it with JSON lines on `127.0.0.1:7981`, each with the window's key from `~/.config/wordcraft/control-key.local-7981` (macOS: `~/Library/Application Support/WordCraft/`), e.g. `{"id":1,"key":"…","method":"engine.execute","params":{"command":"text.insert","params":{"text":"Hello"}}}` then `{"id":2,"key":"…","method":"ui.screenshot","params":{"path":"/tmp/shot.png"}}`. Methods: `crates/ui-egui/src/control.rs`; docs: `docs/control-protocol.md` (Keys). - **Headless window screenshots** (no focus stealing, works with a locked screen): `cargo run --release -p wordcraft-ui-egui --example ui_shot -- script.jsonl` (see the example's header). - **For UI work, look at the result** (screenshot, read the PNG) and compare with Word side by side. - CLI: `wordcraft-cli convert in.docx out.pdf`, `wordcraft-cli run --template sample --cmd 'select.text={"text":"Studio"}' --cmd format.bold --save out.docx`, `wordcraft-cli commands`, `wordcraft-cli parity`. diff --git a/Cargo.lock b/Cargo.lock index a47fa33..4c86807 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4845,10 +4845,12 @@ version = "0.4.0" dependencies = [ "eframe", "egui", + "getrandom 0.3.4", "log", "rfd", "serde_json", "winresource", + "wordcraft-control-key", "wordcraft-doc", "wordcraft-engine", "wordcraft-ui-egui", @@ -4865,6 +4867,10 @@ dependencies = [ "wordcraft-render", ] +[[package]] +name = "wordcraft-control-key" +version = "0.4.0" + [[package]] name = "wordcraft-doc" version = "0.4.0" @@ -4966,6 +4972,7 @@ version = "0.4.0" dependencies = [ "log", "serde_json", + "wordcraft-control-key", "wordcraft-doc", "wordcraft-engine", "wordcraft-render", diff --git a/Cargo.toml b/Cargo.toml index f0d9c52..0cefa19 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,7 @@ wordcraft-pdf = { path = "crates/pdf" } wordcraft-proof = { path = "crates/proof" } wordcraft-engine = { path = "crates/engine" } wordcraft-mcp = { path = "crates/mcp" } +wordcraft-control-key = { path = "crates/control-key" } wordcraft-ui-egui = { path = "crates/ui-egui" } # foundation diff --git a/apps/wordcraft-cli/src/main.rs b/apps/wordcraft-cli/src/main.rs index cefe6dd..f0b83ec 100644 --- a/apps/wordcraft-cli/src/main.rs +++ b/apps/wordcraft-cli/src/main.rs @@ -192,7 +192,13 @@ fn run(args: &[String]) -> Result<(), String> { "mcp" => { validate_options("mcp", &rest, MCP_OPTIONS)?; let backend: Box = match arg_value(&rest, "--connect") { - Some(addr) => Box::new(wordcraft_mcp::Remote::connect(&addr).map_err(|e| format!("can't reach the WordCraft app at {addr}: {e}"))?), + Some(addr) => { + let remote = wordcraft_mcp::Remote::connect(&addr).map_err(|e| format!("can't reach the WordCraft app at {addr}: {e}"))?; + if let Some(w) = remote.key_warning() { + eprintln!("wordcraft-cli: {w}"); + } + Box::new(remote) + } None => Box::new(wordcraft_mcp::Headless::default()), }; let mut server = wordcraft_mcp::Server::new(backend); diff --git a/apps/wordcraft/Cargo.toml b/apps/wordcraft/Cargo.toml index 2323e9f..776bee5 100644 --- a/apps/wordcraft/Cargo.toml +++ b/apps/wordcraft/Cargo.toml @@ -14,6 +14,8 @@ workspace = true wordcraft-doc = { workspace = true } wordcraft-engine = { workspace = true } wordcraft-ui-egui = { workspace = true } +# The control key file's location (shared with the MCP bridge). +wordcraft-control-key = { workspace = true } eframe = { workspace = true, features = ["default_fonts", "wgpu", "accesskit", "wayland", "x11", "links"] } egui = { workspace = true } serde_json = { workspace = true } @@ -25,3 +27,5 @@ winresource = "0.1" [target.'cfg(not(target_arch = "wasm32"))'.dependencies] rfd = { workspace = true } +# The control server's random key. +getrandom = "0.3" diff --git a/apps/wordcraft/src/control_server.rs b/apps/wordcraft/src/control_server.rs index ce9f9dc..54828d5 100644 --- a/apps/wordcraft/src/control_server.rs +++ b/apps/wordcraft/src/control_server.rs @@ -1,50 +1,207 @@ //! Loopback JSON-lines control server: one request per line, one reply per line. -//! This is the transport the MCP server (`wordcraft mcp --connect`) wraps. +//! This is the transport the MCP server (`wordcraft-cli mcp --connect`) wraps. +//! +//! Every request needs the window's key (`docs/control-protocol.md`, Keys): once the port is +//! bound, the app writes a random key to `/control-key.` (mode 0600) and +//! removes the file when the window closes. -use std::io::{BufRead, BufReader, Write}; +use std::io::{BufRead, BufReader, Read, Write}; use std::net::{TcpListener, TcpStream}; +use std::path::{Path, PathBuf}; +use std::sync::Arc; use std::sync::mpsc::{Receiver, Sender, channel}; -use std::time::Duration; +use std::time::{Duration, Instant}; use serde_json::{Value, json}; use wordcraft_ui_egui::ControlRequest; -pub fn start(port: u16, ctx: egui::Context) -> Receiver { - let (tx, rx) = channel::(); +/// Longer request lines are refused (and the connection closed). +const MAX_LINE: usize = 1 << 20; + +/// A connection that sends no authorised request in this time is closed. +const FIRST_REQUEST: Duration = Duration::from_secs(30); + +/// The key file of a running control server; [`KeyFile::remove`] it when the window closes. +pub struct KeyFile(PathBuf); + +impl KeyFile { + pub fn path(&self) -> &Path { + &self.0 + } + pub fn remove(&self) { + let _ = std::fs::remove_file(&self.0); + } +} + +/// Bind `127.0.0.1:port`, write the key file into `settings` and serve. `None` when the port +/// cannot be bound or there is no random key (the control channel is off); the key file is +/// `None` when it could not be written (requests are then refused). +pub fn start(port: u16, ctx: egui::Context, settings: Option<&Path>) -> Option<(Receiver, Option)> { let listener = match TcpListener::bind(("127.0.0.1", port)) { Ok(l) => l, Err(e) => { log::error!("control server failed to bind 127.0.0.1:{port}: {e}"); - return rx; + return None; } }; - log::info!("control server listening on 127.0.0.1:{port}"); + let key = match random_key() { + Ok(k) => k, + Err(e) => { + log::error!("control server off: no random key ({e})"); + return None; + } + }; + let key_file = match settings { + Some(dir) => { + let p = wordcraft_control_key::key_path(dir, port); + match write_key_file(&p, &key) { + Ok(()) => Some(KeyFile(p)), + Err(e) => { + log::error!("control key file not written ({e}): requests will be refused"); + None + } + } + } + None => { + log::error!("no settings folder for the control key file: requests will be refused"); + None + } + }; + match &key_file { + Some(f) => log::info!("control server listening on 127.0.0.1:{port}, key in {}", f.path().display()), + None => log::info!("control server listening on 127.0.0.1:{port}"), + } + Some((serve_on(listener, ctx, key, FIRST_REQUEST), key_file)) +} + +/// A new key: 32 random bytes from the operating system, as hex. +fn random_key() -> Result { + let mut b = [0u8; 32]; + getrandom::fill(&mut b).map_err(|e| e.to_string())?; + Ok(b.iter().map(|x| format!("{x:02x}")).collect()) +} + +/// Write the key file: a stale one (from a run that did not exit cleanly) is replaced; on Unix +/// the new file is created with mode 0600. +fn write_key_file(p: &Path, key: &str) -> Result<(), String> { + if let Some(d) = p.parent() { + std::fs::create_dir_all(d).map_err(|e| format!("cannot create {}: {e}", d.display()))?; + } + let _ = std::fs::remove_file(p); + let mut open = std::fs::OpenOptions::new(); + open.write(true).create_new(true); + #[cfg(unix)] + std::os::unix::fs::OpenOptionsExt::mode(&mut open, 0o600); + let mut f = open.open(p).map_err(|e| format!("cannot create {}: {e}", p.display()))?; + f.write_all(key.as_bytes()).map_err(|e| format!("cannot write {}: {e}", p.display())) +} + +/// Compare a key with the window's in time that does not depend on where they differ. An +/// empty key never matches. +fn keys_match(given: &str, key: &str) -> bool { + let (a, b) = (given.as_bytes(), key.as_bytes()); + !b.is_empty() && a.len() == b.len() && a.iter().zip(b).fold(0u8, |acc, (x, y)| acc | (x ^ y)) == 0 +} + +fn serve_on(listener: TcpListener, ctx: egui::Context, key: String, first: Duration) -> Receiver { + let (tx, rx) = channel::(); + let key: Arc = key.into(); std::thread::spawn(move || { for stream in listener.incoming().flatten() { - let tx = tx.clone(); - let ctx = ctx.clone(); - std::thread::spawn(move || serve(stream, tx, ctx)); + let (tx, ctx, key) = (tx.clone(), ctx.clone(), key.clone()); + std::thread::spawn(move || serve(stream, tx, ctx, &key, first)); } }); rx } -fn serve(stream: TcpStream, tx: Sender, ctx: egui::Context) { - let Ok(read) = stream.try_clone() else { return }; - serve_lines(BufReader::new(read), stream, &tx, &ctx); +/// The read side of a connection with an optional overall deadline: each read waits only for +/// the time left, so blank lines, bad JSON or a slow drip of bytes cannot extend it. +struct Timed { + s: TcpStream, + deadline: Option, } -/// Answer requests until the peer hangs up or sends a line that isn't JSON. Closing on bad -/// input matters: an HTTP request (say, a web page's `fetch` to this port) starts with a -/// request line that never parses, so it can't carry a command in its body. -fn serve_lines(input: impl BufRead, mut out: impl Write, tx: &Sender, ctx: &egui::Context) { - for line in input.lines() { - let Ok(line) = line else { break }; - if line.trim().is_empty() { +impl Read for Timed { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + if let Some(d) = self.deadline { + let left = d.checked_duration_since(Instant::now()).filter(|l| !l.is_zero()).ok_or(std::io::ErrorKind::TimedOut)?; + self.s.set_read_timeout(Some(left))?; + } + self.s.read(buf) + } +} + +impl Timed { + fn set_deadline(&mut self, d: Option) { + self.deadline = d; + if d.is_none() { + let _ = self.s.set_read_timeout(None); + } + } +} + +/// The next line (trimmed), `None` at the end of the stream or after a read error. +fn read_line(r: &mut BufReader) -> Option> { + let mut buf = Vec::new(); + let n = r.by_ref().take(MAX_LINE as u64 + 1).read_until(b'\n', &mut buf).ok()?; + if n == 0 { + return None; + } + if buf.len() > MAX_LINE { + return Some(Err("line too long")); + } + Some(String::from_utf8(buf).map(|s| s.trim().to_string()).map_err(|_| "not UTF-8")) +} + +/// Read and drop the rest of an oversize line, so closing the socket does not reset the +/// connection before the client has read the error. At most 64 MiB, within 2 s. +fn discard_rest_of_line(r: &mut BufReader) { + r.get_mut().set_deadline(Some(Instant::now() + Duration::from_secs(2))); + let mut left: usize = 64 << 20; + while left > 0 { + let (used, done) = match r.fill_buf() { + Ok([]) | Err(_) => return, + Ok(b) => match b.iter().position(|&c| c == b'\n') { + Some(i) => (i + 1, true), + None => (b.len(), false), + }, + }; + r.consume(used); + if done { + return; + } + left = left.saturating_sub(used); + } +} + +fn serve(stream: TcpStream, tx: Sender, ctx: egui::Context, key: &str, first: Duration) { + let Ok(read) = stream.try_clone() else { return }; + let mut out = stream; + // One overall deadline from accept to the first authorised request; after it, none. + let mut reader = BufReader::new(Timed { s: read, deadline: Some(Instant::now() + first) }); + let mut authorised = false; + // The key an `auth` request gave this connection. + let mut conn_key: Option = None; + while let Some(line) = read_line(&mut reader) { + let line = match line { + Ok(l) => l, + Err(e) => { + let _ = writeln!(out, "{}", json!({"ok": false, "error": e})); + if e == "line too long" { + discard_rest_of_line(&mut reader); + } + break; + } + }; + if line.is_empty() { continue; } let msg = match serde_json::from_str::(&line) { - Ok(msg) => msg, + Ok(m) => m, + // Close on a line that isn't JSON: an HTTP request (say, a web page's `fetch` to this + // port) starts with a request line that never parses, so it can't carry a command in + // its body, even before the key is checked. Err(e) => { let _ = writeln!(out, "{}", json!({"ok": false, "error": format!("bad JSON: {e}; closing the connection")})); break; @@ -53,16 +210,40 @@ fn serve_lines(input: impl BufRead, mut out: impl Write, tx: &Sender bool { + if let Some(o) = v.as_object_mut() { + o.insert("id".into(), id.clone()); + } + writeln!(out, "{v}").is_ok() + }; + // Checked on every request. + let k = given.or(conn_key.as_deref()).unwrap_or(""); + if !keys_match(k, key) { + let _ = reply(json!({"ok": false, "error": "unauthorized"})); + break; + } + if !authorised { + authorised = true; + reader.get_mut().set_deadline(None); + } + if method == "auth" { + conn_key = Some(k.to_string()); + if !reply(json!({"ok": true, "result": {}})) { + break; + } + continue; + } let (req, rrx) = ControlRequest::new(method, params); if tx.send(req).is_err() { break; } ctx.request_repaint(); - let mut reply = rrx.recv_timeout(Duration::from_secs(60)).unwrap_or_else(|_| json!({"ok": false, "error": "timeout"})); - if let Some(o) = reply.as_object_mut() { - o.insert("id".into(), id); - } - if writeln!(out, "{reply}").is_err() { + let r = rrx.recv_timeout(Duration::from_secs(60)).unwrap_or_else(|_| json!({"ok": false, "error": "timeout"})); + if !reply(r) { break; } } @@ -71,50 +252,275 @@ fn serve_lines(input: impl BufRead, mut out: impl Write, tx: &Sender (Vec, Vec) { - let (tx, rx) = channel::(); - let app = std::thread::spawn(move || { - let mut seen = Vec::new(); - for req in rx { - let _ = req.reply.send(json!({"ok": true, "result": req.method})); - seen.push(req.method); - } - seen - }); - let mut out = Vec::new(); - serve_lines(input.as_bytes(), &mut out, &tx, &egui::Context::default()); - drop(tx); - let seen = app.join().unwrap(); - let replies = String::from_utf8(out).unwrap().lines().map(|l| serde_json::from_str(l).unwrap()).collect(); - (seen, replies) + const KEY: &str = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + + fn boot_with(first: Duration) -> (u16, Receiver) { + let listener = TcpListener::bind(("127.0.0.1", 0)).unwrap(); + let port = listener.local_addr().unwrap().port(); + (port, serve_on(listener, egui::Context::default(), KEY.to_string(), first)) + } + + fn boot() -> (u16, Receiver) { + boot_with(FIRST_REQUEST) + } + + /// Answer the next request that reaches the UI with `ok`, and return it. + fn answer(rx: &Receiver) -> ControlRequest { + let req = rx.recv_timeout(Duration::from_secs(5)).unwrap(); + let _ = req.reply.send(json!({"ok": true, "result": {}})); + req + } + + /// One connection, several lines. + struct Conn(BufReader, TcpStream); + + impl Conn { + fn open(port: u16) -> Self { + let s = TcpStream::connect(("127.0.0.1", port)).unwrap(); + s.set_read_timeout(Some(Duration::from_secs(5))).unwrap(); + Self(BufReader::new(s.try_clone().unwrap()), s) + } + fn send(&mut self, line: &str) { + writeln!(self.1, "{line}").unwrap(); + } + fn recv(&mut self) -> Value { + let mut r = String::new(); + let _ = self.0.read_line(&mut r); + serde_json::from_str(&r).unwrap_or_default() + } + fn call(&mut self, line: &str) -> Value { + self.send(line); + self.recv() + } + /// The server closed the connection. + fn closed(&mut self) -> bool { + let mut r = String::new(); + matches!(self.0.read_line(&mut r), Ok(0)) + } } #[test] - fn json_lines_get_replies() { - let (seen, replies) = run("{\"id\":1,\"method\":\"format.bold\"}\n\n{\"id\":2,\"method\":\"ui.inspect\"}\n"); - assert_eq!(seen, ["format.bold", "ui.inspect"]); - assert_eq!(replies.len(), 2); - assert_eq!(replies[1]["id"], 2); - assert_eq!(replies[1]["result"], "ui.inspect"); + fn a_request_without_the_key_is_refused_and_closed() { + let (port, rx) = boot(); + let mut c = Conn::open(port); + let r = c.call(r#"{"id":1,"method":"document.inspect"}"#); + assert_eq!((r["ok"].clone(), r["error"].clone(), r["id"].clone()), (json!(false), json!("unauthorized"), json!(1))); + assert!(c.closed()); + assert!(rx.try_recv().is_err(), "nothing reached the UI"); + } + + #[test] + fn a_wrong_key_is_refused_and_closed() { + let (port, rx) = boot(); + for key in ["nope", &KEY[..63], &format!("{KEY}0"), &KEY.to_uppercase()] { + let mut c = Conn::open(port); + assert_eq!(c.call(&format!(r#"{{"id":1,"key":"{key}","method":"document.inspect"}}"#))["error"], "unauthorized", "{key}"); + assert!(c.closed()); + } + assert!(rx.try_recv().is_err()); + } + + #[test] + fn the_key_on_each_request() { + let (port, rx) = boot(); + let mut c = Conn::open(port); + c.send(&format!(r#"{{"id":7,"key":"{KEY}","method":"document.inspect","params":{{"text":true}}}}"#)); + let req = answer(&rx); + assert_eq!((req.method.as_str(), &req.params), ("document.inspect", &json!({"text": true}))); + let r = c.recv(); + assert_eq!((r["ok"].clone(), r["id"].clone()), (json!(true), json!(7))); + // Every request needs it: one without the key is refused, also after good ones. + assert_eq!(c.call(r#"{"id":8,"method":"document.inspect"}"#)["error"], "unauthorized"); + assert!(c.closed()); + } + + #[test] + fn auth_once_per_connection() { + let (port, rx) = boot(); + let mut c = Conn::open(port); + assert_eq!(c.call(&format!(r#"{{"id":1,"method":"auth","key":"{KEY}"}}"#))["ok"], true); + for id in 2..4 { + c.send(&format!(r#"{{"id":{id},"method":"document.inspect"}}"#)); + answer(&rx); + assert_eq!(c.recv()["ok"], true); + } + // `params.key` works for `auth` too. + let mut c = Conn::open(port); + assert_eq!(c.call(&format!(r#"{{"id":1,"method":"auth","params":{{"key":"{KEY}"}}}}"#))["ok"], true); + // A wrong key on an authenticated connection is still refused, and closes it. + assert_eq!(c.call(r#"{"id":2,"key":"nope","method":"document.inspect"}"#)["error"], "unauthorized"); + assert!(c.closed()); + // A failed auth closes the connection. + let mut c = Conn::open(port); + assert_eq!(c.call(r#"{"id":1,"method":"auth","key":"nope"}"#)["error"], "unauthorized"); + assert!(c.closed()); + } + + #[test] + fn params_key_is_only_a_secret_for_auth() { + let (port, rx) = boot(); + // `params.key` does not authenticate other methods. + let mut c = Conn::open(port); + assert_eq!(c.call(&format!(r#"{{"id":1,"method":"document.inspect","params":{{"key":"{KEY}"}}}}"#))["error"], "unauthorized"); + // It is an ordinary parameter for them (`ui.key` presses a key). + let mut c = Conn::open(port); + c.send(&format!(r#"{{"id":1,"key":"{KEY}","method":"ui.key","params":{{"key":"Enter"}}}}"#)); + let req = answer(&rx); + assert_eq!((req.method.as_str(), &req.params), ("ui.key", &json!({"key": "Enter"}))); + assert_eq!(c.recv()["ok"], true); + } + + #[test] + fn a_line_over_1_mib_is_refused_and_closed() { + let (port, rx) = boot(); + let mut c = Conn::open(port); + let big = format!(r#"{{"id":1,"key":"{KEY}","method":"document.inspect","pad":"{}"}}"#, "x".repeat(MAX_LINE)); + assert_eq!(c.call(&big)["error"], "line too long"); + assert!(c.closed()); + assert!(rx.try_recv().is_err()); + } + + #[test] + fn a_silent_connection_is_closed() { + let (port, rx) = boot_with(Duration::from_millis(300)); + let mut quiet = Conn::open(port); + let t0 = Instant::now(); + assert!(quiet.closed()); + assert!(t0.elapsed() < Duration::from_secs(3)); + // After an authorised request the connection may stay idle. + let mut busy = Conn::open(port); + assert_eq!(busy.call(&format!(r#"{{"id":1,"method":"auth","key":"{KEY}"}}"#))["ok"], true); + std::thread::sleep(Duration::from_millis(600)); + busy.send(r#"{"id":2,"method":"document.inspect"}"#); + answer(&rx); + assert_eq!(busy.recv()["ok"], true); + } + + #[test] + fn the_first_request_limit_is_one_overall_deadline() { + // Blank lines and bad JSON do not keep a connection without the key open. + let (port, _rx) = boot_with(Duration::from_millis(300)); + let s = TcpStream::connect(("127.0.0.1", port)).unwrap(); + let mut w = s.try_clone().unwrap(); + let feeder = std::thread::spawn(move || { + for i in 0..40 { + if w.write_all(if i % 2 == 0 { b"\n" } else { b"not json\n" }).is_err() { + return; + } + std::thread::sleep(Duration::from_millis(50)); + } + }); + let t0 = Instant::now(); + let mut r = BufReader::new(s); + r.get_ref().set_read_timeout(Some(Duration::from_secs(5))).unwrap(); + let mut line = String::new(); + loop { + line.clear(); + if matches!(r.read_line(&mut line), Ok(0) | Err(_)) { + break; + } + assert!(line.contains("bad JSON"), "{line}"); + } + let _ = feeder.join(); + assert!(t0.elapsed() < Duration::from_millis(1500), "{:?}", t0.elapsed()); } #[test] fn bad_json_closes_the_connection() { - let (seen, replies) = run("not json\n{\"id\":1,\"method\":\"format.bold\"}\n"); - assert!(seen.is_empty()); - assert_eq!(replies.len(), 1); - assert_eq!(replies[0]["ok"], false); + let (port, rx) = boot(); + let mut c = Conn::open(port); + assert_eq!(c.call("not json")["ok"], false); + assert!(c.closed()); + assert!(rx.try_recv().is_err()); } #[test] fn http_request_cannot_smuggle_a_command() { - // What a web page's `fetch("http://127.0.0.1:/", {method: "POST", body})` sends. - let body = "\n{\"method\":\"file.saveAs\",\"params\":{\"path\":\"/tmp/x.docx\"}}\n"; + // What a web page's `fetch("http://127.0.0.1:/", {method: "POST", body})` sends, + // even with the key in the body. + let (port, rx) = boot(); + let body = format!("\n{{\"key\":\"{KEY}\",\"method\":\"file.saveAs\",\"params\":{{\"path\":\"/tmp/x.docx\"}}}}\n"); let req = format!("POST / HTTP/1.1\r\nHost: 127.0.0.1:7981\r\nContent-Type: text/plain\r\nContent-Length: {}\r\n\r\n{body}", body.len()); - let (seen, _) = run(&req); - assert!(seen.is_empty(), "{seen:?}"); + let mut c = Conn::open(port); + c.1.write_all(req.as_bytes()).unwrap(); + let _ = c.recv(); + assert!(c.closed()); + assert!(rx.recv_timeout(Duration::from_millis(300)).is_err()); + } + + #[test] + fn keys_compare_whole() { + assert!(keys_match(KEY, KEY)); + assert!(!keys_match(KEY, &KEY[..63])); + assert!(!keys_match(&KEY[..63], KEY)); + assert!(!keys_match(KEY, &KEY.replace('f', "e"))); + assert!(!keys_match("", ""), "an empty key never matches"); + } + + #[test] + fn random_keys_are_256_bits() { + let (a, b) = (random_key().unwrap(), random_key().unwrap()); + assert_eq!(a.len(), 64); + assert!(a.bytes().all(|c| c.is_ascii_hexdigit())); + assert_ne!(a, b); + } + + fn temp_dir(name: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!("wordcraft-control-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + dir + } + + #[test] + fn the_key_file_is_written_after_the_bind_and_removed_on_exit() { + let dir = temp_dir("life"); + // The port is taken: no key file. + let taken = TcpListener::bind(("127.0.0.1", 0)).unwrap(); + let port = taken.local_addr().unwrap().port(); + assert!(start(port, egui::Context::default(), Some(&dir)).is_none()); + assert!(!dir.exists() || std::fs::read_dir(&dir).unwrap().next().is_none(), "no key file after a failed bind"); + drop(taken); + // Bound: the key file holds the key that opens the port. + let (rx, key_file) = start(port, egui::Context::default(), Some(&dir)).unwrap(); + let key_file = key_file.unwrap(); + assert_eq!(key_file.path(), wordcraft_control_key::key_path(&dir, port)); + let key = std::fs::read_to_string(key_file.path()).unwrap(); + assert_eq!(key.len(), 64); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + assert_eq!(std::fs::metadata(key_file.path()).unwrap().permissions().mode() & 0o777, 0o600); + } + let mut c = Conn::open(port); + c.send(&format!(r#"{{"id":1,"key":"{key}","method":"document.inspect"}}"#)); + answer(&rx); + assert_eq!(c.recv()["ok"], true); + key_file.remove(); + assert!(!key_file.path().exists()); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn a_stale_key_file_is_replaced() { + let dir = temp_dir("stale"); + std::fs::create_dir_all(&dir).unwrap(); + let p = dir.join("control-key.test"); + std::fs::write(&p, "old").unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&p, std::fs::Permissions::from_mode(0o644)).unwrap(); + } + write_key_file(&p, KEY).unwrap(); + assert_eq!(std::fs::read_to_string(&p).unwrap(), KEY); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + assert_eq!(std::fs::metadata(&p).unwrap().permissions().mode() & 0o777, 0o600); + } + let _ = std::fs::remove_dir_all(&dir); } } diff --git a/apps/wordcraft/src/main.rs b/apps/wordcraft/src/main.rs index 8762929..18c4905 100644 --- a/apps/wordcraft/src/main.rs +++ b/apps/wordcraft/src/main.rs @@ -3,8 +3,9 @@ //! Usage: `wordcraft [--control ] [--sample] [files…]` //! //! `--control ` (or `WORDCRAFT_CONTROL_PORT`) starts a localhost JSON-lines control server: -//! `{"id":1,"method":"engine.execute","params":{"command":"text.insert","params":{"text":"Hi"}}}`. -//! See `docs/control-protocol.md`. +//! `{"id":1,"key":"…","method":"engine.execute","params":{"command":"text.insert","params":{"text":"Hi"}}}`. +//! Every request needs the key the app writes to `/control-key.` (mode 0600, +//! removed on exit). See `docs/control-protocol.md`. #![cfg_attr(all(target_os = "windows", not(debug_assertions)), windows_subsystem = "windows")] #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] @@ -19,8 +20,9 @@ use wordcraft_ui_egui::{ window_geometry::{WindowGeometry, take_rescue}, }; -/// The app, and the restored window geometry until the first frame has checked it. -struct App(WordApp, Option); +/// The app, the restored window geometry until the first frame has checked it, and the control +/// server's key file (removed on exit). +struct App(WordApp, Option, Option); impl eframe::App for App { fn logic(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) { @@ -42,21 +44,15 @@ impl eframe::App for App { } fn on_exit(&mut self) { save_prefs(&self.0); + if let Some(key_file) = &self.2 { + key_file.remove(); + } } } +/// `ui.json` in the settings folder, which the MCP bridge also uses to find the control key. fn prefs_path() -> Option { - let base = if cfg!(target_os = "macos") { - std::env::var_os("HOME").map(|h| std::path::PathBuf::from(h).join("Library/Application Support/WordCraft")) - } else if cfg!(windows) { - std::env::var_os("APPDATA").map(|a| std::path::PathBuf::from(a).join("WordCraft")) - } else { - std::env::var_os("XDG_CONFIG_HOME") - .map(std::path::PathBuf::from) - .or_else(|| std::env::var_os("HOME").map(|h| std::path::PathBuf::from(h).join(".config"))) - .map(|c| c.join("wordcraft")) - }; - base.map(|b| b.join("ui.json")) + wordcraft_control_key::settings_dir().map(|b| b.join("ui.json")) } /// Where the log files live: `logs` in the preferences folder (see `logging`). @@ -200,16 +196,19 @@ fn main() -> eframe::Result { load_prefs(&mut app); app.ui.window = restored; app.integrated_titlebar = cfg!(target_os = "macos"); - if let Some(port) = control_port { - let rx = control_server::start(port, cc.egui_ctx.clone()); + let mut key_file = None; + if let Some(port) = control_port + && let Some((rx, file)) = control_server::start(port, cc.egui_ctx.clone(), wordcraft_control_key::settings_dir().as_deref()) + { app = app.with_control(rx); + key_file = file; } for f in files { if let Err(e) = app.run("file.open", serde_json::json!({"path": f})) { log::warn!("{f}: {e}"); } } - Ok(Box::new(App(app, restored))) + Ok(Box::new(App(app, restored, key_file))) }), ) } diff --git a/crates/control-key/Cargo.toml b/crates/control-key/Cargo.toml new file mode 100644 index 0000000..0884e52 --- /dev/null +++ b/crates/control-key/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "wordcraft-control-key" +version.workspace = true +edition.workspace = true +license.workspace = true +description = "WordCraft control-port key file: where it lives and how to read it (shared by the app and the MCP bridge)" + +[lints] +workspace = true diff --git a/crates/control-key/src/lib.rs b/crates/control-key/src/lib.rs new file mode 100644 index 0000000..1cb7a1e --- /dev/null +++ b/crates/control-key/src/lib.rs @@ -0,0 +1,83 @@ +//! The control channel's key file, one rule for the app (which writes it) and the MCP bridge +//! (which reads it): `/control-key.`. See `docs/control-protocol.md`, Keys. + +use std::path::{Path, PathBuf}; + +/// The app's settings folder (preferences, logs, key files): macOS +/// `~/Library/Application Support/WordCraft`, Windows `%APPDATA%\WordCraft`, otherwise +/// `$XDG_CONFIG_HOME/wordcraft` or `~/.config/wordcraft`. +pub fn settings_dir() -> Option { + if cfg!(target_os = "macos") { + std::env::var_os("HOME").map(|h| PathBuf::from(h).join("Library/Application Support/WordCraft")) + } else if cfg!(windows) { + std::env::var_os("APPDATA").map(|a| PathBuf::from(a).join("WordCraft")) + } else { + std::env::var_os("XDG_CONFIG_HOME") + .map(PathBuf::from) + .or_else(|| std::env::var_os("HOME").map(|h| PathBuf::from(h).join(".config"))) + .map(|c| c.join("wordcraft")) + } +} + +/// The `instance-id=` value of a `/.flatpak-info` file, only when it is `[A-Za-z0-9_-]+` (it +/// becomes part of a file name). +pub fn parse_instance_id(info: &str) -> Option { + let v = info.lines().find_map(|l| l.strip_prefix("instance-id="))?.trim(); + (!v.is_empty() && v.chars().all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-')).then(|| v.to_string()) +} + +/// The Flatpak instance id, or `None` outside Flatpak. +pub fn instance_id() -> Option { + parse_instance_id(&std::fs::read_to_string("/.flatpak-info").ok()?) +} + +/// `/control-key.`, with `` the Flatpak instance id or +/// `local-`. +pub fn key_path(settings: &Path, port: u16) -> PathBuf { + key_path_for(settings, instance_id().as_deref(), port) +} + +fn key_path_for(settings: &Path, instance: Option<&str>, port: u16) -> PathBuf { + match instance { + Some(id) => settings.join(format!("control-key.{id}")), + None => settings.join(format!("control-key.local-{port}")), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::Path; + + #[test] + fn instance_id_only_safe_values() { + assert_eq!(parse_instance_id("[Instance]\ninstance-id=abc_123-X\n").as_deref(), Some("abc_123-X")); + assert_eq!(parse_instance_id("instance-id= 42 \n").as_deref(), Some("42")); + // It ends up in a file name. + assert_eq!(parse_instance_id("instance-id=../etc\n"), None); + assert_eq!(parse_instance_id("instance-id=a/b\n"), None); + assert_eq!(parse_instance_id("instance-id=a b\n"), None); + assert_eq!(parse_instance_id("instance-id=..\n"), None); + assert_eq!(parse_instance_id("instance-id=\n"), None); + assert_eq!(parse_instance_id("app=ai.storyteller.wordcraft\n"), None); + } + + #[test] + fn key_file_per_instance() { + let dir = Path::new("/settings/wordcraft"); + assert_eq!(key_path_for(dir, Some("1234"), 7981), dir.join("control-key.1234")); + assert_eq!(key_path_for(dir, None, 7981), dir.join("control-key.local-7981")); + assert_eq!(key_path_for(dir, None, 7982), dir.join("control-key.local-7982")); + if instance_id().is_none() { + assert_eq!(key_path(dir, 7981), dir.join("control-key.local-7981")); + } + } + + #[test] + fn settings_dir_is_the_apps_folder() { + if let Some(d) = settings_dir() { + let name = d.file_name().and_then(|n| n.to_str()).unwrap_or(""); + assert!(name == "wordcraft" || name == "WordCraft", "{}", d.display()); + } + } +} diff --git a/crates/mcp/Cargo.toml b/crates/mcp/Cargo.toml index 8044e76..a767a20 100644 --- a/crates/mcp/Cargo.toml +++ b/crates/mcp/Cargo.toml @@ -12,5 +12,6 @@ workspace = true wordcraft-doc = { workspace = true } wordcraft-engine = { workspace = true } wordcraft-render = { workspace = true } +wordcraft-control-key = { workspace = true } serde_json = { workspace = true } log = { workspace = true } diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs index b458727..ac32f5d 100644 --- a/crates/mcp/src/backend.rs +++ b/crates/mcp/src/backend.rs @@ -1,7 +1,8 @@ //! Where MCP tool calls end up: a control-channel method call. use std::io::{BufRead, BufReader, Write}; -use std::net::{TcpStream, ToSocketAddrs}; +use std::net::{IpAddr, SocketAddr, TcpStream, ToSocketAddrs}; +use std::path::Path; use std::time::Duration; use serde_json::{Value, json}; @@ -17,18 +18,76 @@ pub trait Backend { fn describe(&self) -> String; } +/// Looks up the control key for an address (`HOST:PORT`). +type KeyFinder = Box Result + Send>; + /// A running WordCraft app, reached through its loopback control port. pub struct Remote { addr: String, conn: Option<(BufReader, TcpStream)>, next_id: u64, + /// The connection leads to a loopback address: requests carry the control key. + send_key: bool, + /// The control key, looked up once per connection (the app picks a new one at every start). + key: Option, + find_key: KeyFinder, +} + +/// The control key: `env` (`WORDCRAFT_CONTROL_KEY`) when set, else the app's key file for +/// `port` in `settings`. The error says where it looked. +fn lookup_key(env: Option, settings: Option<&Path>, port: Option) -> Result { + if let Some(k) = env.map(|k| k.trim().to_string()).filter(|k| !k.is_empty()) { + return Ok(k); + } + let (Some(settings), Some(port)) = (settings, port) else { + return Err("no control key: set WORDCRAFT_CONTROL_KEY (no settings folder or port to find the key file)".to_string()); + }; + let path = wordcraft_control_key::key_path(settings, port); + match std::fs::read_to_string(&path).map(|k| k.trim().to_string()) { + Ok(k) if !k.is_empty() => Ok(k), + _ => Err(format!("no control key: set WORDCRAFT_CONTROL_KEY, or start the app with --control {port} (it writes {})", path.display())), + } +} + +/// The key for the app at `addr`, from the environment or the app's key file. +fn find_key(addr: &str) -> Result { + let port = addr.rsplit_once(':').and_then(|(_, p)| p.parse().ok()); + lookup_key(std::env::var("WORDCRAFT_CONTROL_KEY").ok(), wordcraft_control_key::settings_dir().as_deref(), port) +} + +/// Whether `addr` (`HOST:PORT`, as given to `--connect`) names a loopback address: 127.0.0.0/8, +/// ::1 or `localhost`. Other host names are not resolved: they never count. +fn is_loopback_addr(addr: &str) -> bool { + let loopback = |ip: IpAddr| ip.to_canonical().is_loopback(); + if let Ok(sa) = addr.parse::() { + return loopback(sa.ip()); + } + if let Ok(ip) = addr.parse::() { + return loopback(ip); + } + let host = addr.rsplit_once(':').map_or(addr, |(h, _)| h); + let host = host.strip_prefix('[').and_then(|h| h.strip_suffix(']')).unwrap_or(host); + host.parse::().map_or_else(|_| host.eq_ignore_ascii_case("localhost"), loopback) +} + +/// The key goes only to a loopback `--connect` address, and only when the connection really +/// leads to one (`peer`, so a `localhost` that resolves elsewhere gets no key either). +fn may_send_key(addr: &str, peer: Option) -> bool { + is_loopback_addr(addr) && peer.is_some_and(|p| p.ip().to_canonical().is_loopback()) } impl Remote { /// Connect to `addr` (`127.0.0.1:7981`), failing fast when nothing is listening. pub fn connect(addr: &str) -> std::io::Result { - let mut r = Self { addr: addr.to_string(), conn: None, next_id: 1 }; + Self::connect_with(addr, Box::new(find_key)) + } + + fn connect_with(addr: &str, find_key: KeyFinder) -> std::io::Result { + let mut r = Self { addr: addr.to_string(), conn: None, next_id: 1, send_key: false, key: None, find_key }; r.reconnect()?; + if let Some(w) = r.key_warning() { + log::warn!("{w}"); + } Ok(r) } @@ -36,8 +95,19 @@ impl Remote { &self.addr } + /// Why requests go without the control key (the address is not loopback), if they do. + pub fn key_warning(&self) -> Option { + (!self.send_key).then(|| { + format!( + "{} is not a loopback address, so the control key is not sent (it only goes to 127.0.0.0/8, ::1 and localhost) and the app refuses requests; reach the app through 127.0.0.1, for example with an SSH tunnel", + self.addr + ) + }) + } + fn reconnect(&mut self) -> std::io::Result<()> { self.conn = None; + self.key = None; let mut last = std::io::Error::new(std::io::ErrorKind::NotFound, format!("cannot resolve {}", self.addr)); for sa in self.addr.to_socket_addrs()? { match TcpStream::connect_timeout(&sa, Duration::from_millis(800)) { @@ -45,6 +115,7 @@ impl Remote { s.set_nodelay(true).ok(); // The app answers within 60 s (its own timeout); leave headroom. s.set_read_timeout(Some(Duration::from_secs(90))).ok(); + self.send_key = may_send_key(&self.addr, s.peer_addr().ok()); let read = s.try_clone()?; self.conn = Some((BufReader::new(read), s)); return Ok(()); @@ -55,10 +126,22 @@ impl Remote { Err(last) } - fn roundtrip(&mut self, line: &str) -> std::io::Result { - if self.conn.is_none() { - self.reconnect()?; + /// The request line; it carries the key when this connection may (looked up once per + /// connection). + fn request_line(&mut self, id: u64, method: &str, params: &Value) -> Result { + let mut req = json!({"id": id, "method": method, "params": params}); + if self.send_key { + let key = match &self.key { + Some(k) => k.clone(), + None => (self.find_key)(&self.addr)?, + }; + req["key"] = json!(key); + self.key = Some(key); } + Ok(req.to_string()) + } + + fn roundtrip(&mut self, line: &str) -> std::io::Result { let Some((reader, writer)) = self.conn.as_mut() else { return Err(std::io::Error::new(std::io::ErrorKind::NotConnected, "not connected")); }; @@ -71,27 +154,61 @@ impl Remote { } Ok(reply) } + + /// One request and its reply, with one retry on a new connection (the app may have + /// restarted; a new connection also reads the key again). + fn exchange(&mut self, id: u64, method: &str, params: &Value) -> Result { + let unreachable = |addr: &str, e: std::io::Error| format!("WordCraft app at {addr} is not reachable: {e}"); + let mut first = true; + let reply = loop { + let tried = if self.conn.is_some() { Ok(()) } else { self.reconnect() }; + let result = match tried { + Ok(()) => { + let line = self.request_line(id, method, params)?; + self.roundtrip(&line) + } + Err(e) => Err(e), + }; + match result { + Ok(r) => break r, + Err(e) => { + self.conn = None; + if !first { + return Err(unreachable(&self.addr, e)); + } + first = false; + } + } + }; + serde_json::from_str(reply.trim()).map_err(|e| format!("bad reply from app: {e}")) + } } 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(); - // One retry with a fresh connection (the app may have restarted). - let reply = match self.roundtrip(&line) { - Ok(r) => r, - Err(_) => { - self.conn = None; - self.roundtrip(&line).map_err(|e| { - self.conn = None; - format!("WordCraft app at {} is not reachable: {e}", self.addr) - })? - } - }; - let v: Value = serde_json::from_str(reply.trim()).map_err(|e| format!("bad reply from app: {e}"))?; + let unauthorized = |v: &Value| v.get("error").and_then(Value::as_str) == Some("unauthorized"); + let mut v = self.exchange(id, method, ¶ms)?; + if unauthorized(&v) && self.send_key { + // A stale key (the app restarted on this port): the app closed the connection; a new + // one reads the key again. Once. + self.conn = None; + v = self.exchange(id, method, ¶ms)?; + } if v.get("ok").and_then(Value::as_bool) == Some(true) { Ok(v.get("result").cloned().unwrap_or(Value::Null)) + } else if unauthorized(&v) { + self.conn = None; + Err(self.key_warning().map_or_else( + || { + format!( + "unauthorized: the WordCraft app at {} refused the control key (from WORDCRAFT_CONTROL_KEY or the app's key file)", + self.addr + ) + }, + |w| format!("unauthorized: {w}"), + )) } else { Err(v.get("error").and_then(Value::as_str).unwrap_or("unknown error").to_string()) } @@ -105,3 +222,164 @@ impl Backend for Remote { format!("connected to the WordCraft app at {}", self.addr) } } + +#[cfg(test)] +mod tests { + use super::*; + use std::net::{SocketAddr, TcpListener}; + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::mpsc::{Receiver, channel}; + + #[test] + fn key_from_env_then_key_file() { + let dir = std::env::temp_dir().join(format!("wordcraft-mcp-key-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let path = wordcraft_control_key::key_path(&dir, 7981); + // Neither: the error names the variable and the file it looked for. + let e = lookup_key(None, Some(&dir), Some(7981)).unwrap_err(); + assert!(e.contains("WORDCRAFT_CONTROL_KEY") && e.contains(&path.display().to_string()), "{e}"); + // The key file, trimmed; an empty variable does not count. + std::fs::write(&path, " file-key\n").unwrap(); + assert_eq!(lookup_key(None, Some(&dir), Some(7981)).unwrap(), "file-key"); + assert_eq!(lookup_key(Some(" ".into()), Some(&dir), Some(7981)).unwrap(), "file-key"); + // The variable wins. + assert_eq!(lookup_key(Some(" env-key ".into()), Some(&dir), Some(7981)).unwrap(), "env-key"); + assert_eq!(lookup_key(Some("env-key".into()), None, None).unwrap(), "env-key"); + // Read again every time: the app writes a new key at every start. + std::fs::write(&path, "new-key").unwrap(); + assert_eq!(lookup_key(None, Some(&dir), Some(7981)).unwrap(), "new-key"); + // Another port, another file. + assert!(lookup_key(None, Some(&dir), Some(7982)).is_err()); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn key_only_for_loopback_addresses() { + let peer = |s: &str| s.parse::().ok(); + for (addr, at) in [ + ("127.0.0.1:7981", "127.0.0.1:7981"), + ("127.0.0.2:7981", "127.0.0.2:7981"), + ("[::1]:7981", "[::1]:7981"), + ("[::ffff:127.0.0.1]:7981", "[::ffff:127.0.0.1]:7981"), + ("localhost:7981", "127.0.0.1:7981"), + ("localhost:7981", "[::1]:7981"), + ("LocalHost:7981", "127.0.0.1:7981"), + ] { + assert!(may_send_key(addr, peer(at)), "{addr} at {at}"); + } + for (addr, at) in [ + ("192.0.2.20:7981", "192.0.2.20:7981"), + ("10.0.0.7:7981", "10.0.0.7:7981"), + ("[fe80::1]:7981", "[fe80::1]:7981"), + ("0.0.0.0:7981", "127.0.0.1:7981"), + // Host names other than localhost: even when they lead to this machine. + ("workstation:7981", "127.0.1.1:7981"), + ("example.org:7981", "93.184.215.14:7981"), + ("localhost.example.org:7981", "127.0.0.1:7981"), + // `localhost` that does not lead to a loopback address. + ("localhost:7981", "192.0.2.20:7981"), + ] { + assert!(!may_send_key(addr, peer(at)), "{addr} at {at}"); + } + // No peer address: no key. + assert!(!may_send_key("127.0.0.1:7981", None)); + } + + /// A stand-in for the app on 127.0.0.1: every request line goes to the returned channel and + /// gets `answer(request)`: a reply and whether to close the connection after it, or `None` + /// to close it without a reply (the app quit). + fn fake_app(answer: impl Fn(&Value) -> Option<(Value, bool)> + Send + Sync + 'static) -> (u16, Receiver) { + let listener = TcpListener::bind(("127.0.0.1", 0)).unwrap(); + let port = listener.local_addr().unwrap().port(); + let (tx, rx) = channel(); + let answer = Arc::new(answer); + std::thread::spawn(move || { + for stream in listener.incoming().flatten() { + let (tx, answer) = (tx.clone(), answer.clone()); + std::thread::spawn(move || { + let mut out = stream.try_clone().unwrap(); + for line in BufReader::new(stream).lines() { + let Ok(req) = serde_json::from_str::(&line.unwrap_or_default()) else { return }; + let _ = tx.send(req.clone()); + let Some((mut reply, close)) = answer(&req) else { return }; + reply["id"] = req["id"].clone(); + if writeln!(out, "{reply}").is_err() || close { + return; + } + } + }); + } + }); + (port, rx) + } + + fn fixed_key(key: &'static str) -> KeyFinder { + Box::new(move |_| Ok(key.to_string())) + } + + const WAIT: Duration = Duration::from_secs(5); + + #[test] + fn sends_the_key_to_loopback() { + let (port, seen) = fake_app(|_| Some((json!({"ok": true, "result": 7}), false))); + for host in ["127.0.0.1", "localhost"] { + let mut r = Remote::connect_with(&format!("{host}:{port}"), fixed_key("k1")).unwrap(); + assert!(r.key_warning().is_none(), "{host}"); + assert_eq!(r.call("document.inspect", json!({})).unwrap(), json!(7)); + assert_eq!(seen.recv_timeout(WAIT).unwrap()["key"], "k1", "{host}"); + } + } + + #[test] + fn reads_the_key_again_on_a_new_connection() { + // The app restarted with a new key: the old key is refused (and the connection closed). + let (port, seen) = fake_app(|req| { + Some(if req["key"] == "new" { (json!({"ok": true, "result": 1}), false) } else { (json!({"ok": false, "error": "unauthorized"}), true) }) + }); + let reads = Arc::new(AtomicUsize::new(0)); + let n = reads.clone(); + let finder: KeyFinder = Box::new(move |_| Ok(if n.fetch_add(1, Ordering::SeqCst) == 0 { "old" } else { "new" }.to_string())); + let mut r = Remote::connect_with(&format!("127.0.0.1:{port}"), finder).unwrap(); + assert_eq!(r.call("document.inspect", json!({})).unwrap(), json!(1)); + assert_eq!(seen.recv_timeout(WAIT).unwrap()["key"], "old"); + assert_eq!(seen.recv_timeout(WAIT).unwrap()["key"], "new"); + assert_eq!(reads.load(Ordering::SeqCst), 2); + // The new key is kept for the next requests on the same connection. + assert_eq!(r.call("document.inspect", json!({})).unwrap(), json!(1)); + assert_eq!(reads.load(Ordering::SeqCst), 2); + + // The app quits mid-session (connection closed): the retry reads the key again. + let (port, seen) = fake_app(|req| if req["id"] == 2 { None } else { Some((json!({"ok": true, "result": 1}), false)) }); + let reads = Arc::new(AtomicUsize::new(0)); + let n = reads.clone(); + let finder: KeyFinder = Box::new(move |_| Ok(format!("key-{}", n.fetch_add(1, Ordering::SeqCst)))); + let mut r = Remote::connect_with(&format!("127.0.0.1:{port}"), finder).unwrap(); + r.call("document.inspect", json!({})).unwrap(); + r.call("document.inspect", json!({})).unwrap_err(); + let keys: Vec = seen.try_iter().map(|v| v["key"].clone()).collect(); + assert_eq!(keys.first(), Some(&json!("key-0")), "{keys:?}"); + assert_eq!(keys.last(), Some(&json!("key-1")), "{keys:?}"); + } + + #[test] + fn no_key_for_other_hosts_and_a_clear_error() { + // There is no LAN address to test with here: same stand-in, with the decision a LAN + // address gets. + let (port, seen) = fake_app(|req| { + Some(if req.get("key").is_some() { + (json!({"ok": true, "result": 1}), false) + } else { + (json!({"ok": false, "error": "unauthorized"}), true) + }) + }); + let finder: KeyFinder = Box::new(|_| Err("the key must not be read for another host".to_string())); + let mut r = Remote::connect_with(&format!("127.0.0.1:{port}"), finder).unwrap(); + r.send_key = false; + assert!(r.key_warning().is_some_and(|w| w.contains("not a loopback address"))); + let e = r.call("document.inspect", json!({})).unwrap_err(); + assert!(e.contains("not a loopback address") && e.contains("not sent"), "{e}"); + assert!(seen.recv_timeout(WAIT).unwrap().get("key").is_none()); + } +} diff --git a/docs/control-protocol.md b/docs/control-protocol.md index fb8c30e..8399780 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -1,10 +1,11 @@ # Control protocol Start the app with a control port: `wordcraft --control 7981` (or `WORDCRAFT_CONTROL_PORT=7981`). -It listens on `127.0.0.1` only. Send one JSON object per line; each gets one reply line: +It listens on `127.0.0.1` only. Send one JSON object per line, with the window's key (see +[Keys](#keys)); each gets one reply line: ```json -{"id": 1, "method": "engine.execute", "params": {"command": "text.insert", "params": {"text": "Hello"}}} +{"id": 1, "key": "…", "method": "engine.execute", "params": {"command": "text.insert", "params": {"text": "Hello"}}} {"id": 1, "ok": true, "result": {"anchor": {...}, "focus": {...}}} ``` @@ -12,6 +13,39 @@ Errors come back as `{"id": …, "ok": false, "error": "message"}`. A failed com document unchanged. A line that isn't valid JSON gets one error reply, then the server closes the connection (so an HTTP request sent to the port can't run a command); reconnect to continue. +## Keys + +Any local program (and possibly a web page) can reach a loopback port, so every request needs the +window's key. Once the port is bound, the app writes a random key (256 bits, as hex) to +`/control-key.` and removes the file when the window closes; a new start +writes a new key. `` is the folder that holds the preferences: on Linux +`$XDG_CONFIG_HOME/wordcraft` (by default `~/.config/wordcraft`), on macOS +`~/Library/Application Support/WordCraft`, on Windows `%APPDATA%\WordCraft`. `` is the +Flatpak instance id inside Flatpak, and `local-` otherwise. On Unix the file has mode 0600. + +```sh +cat ~/.config/wordcraft/control-key.local-7981 # Linux, outside Flatpak, port 7981 +``` + +- Send the key as top-level `"key"` with every request, or once per connection with + `{"method": "auth", "key": "…"}` (reply `{"ok": true, "result": {}}`); later requests on that + connection may leave `key` out. Only `auth` reads `params.key`; for other methods `params.key` + is an ordinary parameter (`ui.key {"key": "Enter"}` presses Enter). +- The key is checked on every request. A missing or wrong key gets + `{"ok": false, "error": "unauthorized"}`, and the app closes the connection. +- A line over 1 MiB gets `{"ok": false, "error": "line too long"}`, and the app closes the + connection. +- A connection that sends no authorised request within 30 s of being accepted is closed. That is + one limit: blank lines and bad JSON do not extend it. After an authorised request the + connection may stay idle. + +`wordcraft-cli mcp --connect` takes the key from `WORDCRAFT_CONTROL_KEY`, or else from the key +file for the port it connects to, and reads it again on every new connection (the app may have +restarted). It sends the key only to loopback addresses (`127.0.0.0/8`, `::1`, `localhost`). For +any other host it sends no key, says so, and the app refuses its requests: to drive an app on +another machine, forward its port to `127.0.0.1` (for example `ssh -L 7981:127.0.0.1:7981 host`) +and set `WORDCRAFT_CONTROL_KEY` to that app's key. + ## Methods | Method | Params | Result | diff --git a/docs/mcp.md b/docs/mcp.md index cf039b4..559e767 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -11,6 +11,10 @@ wordcraft-cli mcp --connect 127.0.0.1:7981 # …including clicks, keys and scree Claude Code: `claude mcp add wordcraft -- wordcraft-cli mcp`. +With `--connect`, the bridge sends the app's control key, which it reads from +`WORDCRAFT_CONTROL_KEY` or from the key file the app writes at start. It sends the key only to +loopback addresses; see [Keys](control-protocol.md#keys). + ## Tools | Tool | What it does | diff --git a/xtask/src/layers.rs b/xtask/src/layers.rs index 401ffcc..64705c6 100644 --- a/xtask/src/layers.rs +++ b/xtask/src/layers.rs @@ -34,6 +34,7 @@ impl Class { /// prefix. pub const TABLE: &[(&str, Class)] = &[ ("geom", Class::Layer(0)), + ("control-key", Class::Standalone), ("doc", Class::Layer(1)), ("fonts", Class::Layer(1)), ("proof", Class::Layer(1)),