Merge branch 'main' into fix/sec-mcp-token-endpoint

The bridge picked the token by port alone and sent it to whatever the address resolved to, so a listener another account opened on ::1 with the same port received the desktop's credential. The token is now attached only when the connected peer is exactly 127.0.0.1:PORT, where the app listens. Reading the token refuses a symlink, a non-plain file and a file others can read, and the shared temporary directory is no longer a fallback location for it.

Cargo.lock

@@ -772,6 +772,7 @@ dependencies = [
  "cadcraft-render",
  "clap",
  "getrandom 0.4.3",
+ "libc",
  "log",
  "serde_json",
 ]

crates/constraints/src/auto.rs

@@ -232,10 +232,6 @@ pub fn infer_placed(d: &Drawing, placed: &[Handle], others: &[Handle], o: &Infer
     let mut extra = Vec::new();
     let mut on = Vec::new();
     for (rp, p) in g.points.iter().filter(|(r, _)| placed.contains(&r.handle)) {
-        // Nothing is added once the candidates are full, so the points left need no search.
-        if extra.len() + on.len() + out.len() >= MAX_CANDIDATES {
-            break;
-        }
         if out.iter().any(|c| c.kind == ConstraintKind::Coincident && c.refs.contains(rp)) {
             continue;
         }
@@ -271,22 +267,20 @@ pub fn infer_placed(d: &Drawing, placed: &[Handle], others: &[Handle], o: &Infer
     // Perpendicular lines meeting in a T (a perpendicular snap onto the middle of a line).
     let atol = if o.angle_tolerance.is_finite() { o.angle_tolerance.clamp(0.0, 45.0).to_radians() } else { 0.0 };
     let allow_per = o.types.is_empty() || o.types.iter().any(|t| t.eq_ignore_ascii_case(ConstraintKind::Perpendicular.name()));
-    'per: for (r1, a1, b1) in g.lines.iter().filter(|(r, _, _)| allow_per && placed.contains(&r.handle)) {
+    for (r1, a1, b1) in g.lines.iter().filter(|(r, _, _)| placed.contains(&r.handle)) {
         for (r2, a2, b2) in g.lines.iter().filter(|(r, _, _)| r.handle != r1.handle) {
-            // Full: nothing more can be added, so the pairs left need no geometry or duplicate search.
-            if out.len() >= MAX_CANDIDATES {
-                break 'per;
-            }
             let (l1, l2) = (Line::new(*a1, *b1), Line::new(*a2, *b2));
             let touches = |l: &Line, p: Vec2| l.dist(p) <= tol && (0.0..=1.0).contains(&l.param_of(p));
             let tee = [*a1, *b1].iter().any(|p| touches(&l2, *p)) || [*a2, *b2].iter().any(|p| touches(&l1, *p));
             let ang = ang_between(*b1 - *a1, *b2 - *a2);
-            // The duplicate search scans every candidate, so it comes after the cheap tests.
-            if tee
+            let dup = out.iter().any(|c| c.kind == ConstraintKind::Perpendicular && c.refs.contains(r1) && c.refs.contains(r2));
+            if allow_per
+                && tee
+                && !dup
                 && (std::f64::consts::FRAC_PI_2 - ang).abs() <= atol
                 && l1.len() > tol
                 && l2.len() > tol
-                && !out.iter().any(|c| c.kind == ConstraintKind::Perpendicular && c.refs.contains(r1) && c.refs.contains(r2))
+                && out.len() < MAX_CANDIDATES
             {
                 out.push(Constraint { id: 0, kind: ConstraintKind::Perpendicular, refs: vec![*r1, *r2], name: String::new(), expr: String::new() });
             }

crates/fonts/src/ttf.rs

@@ -89,11 +89,7 @@ impl Face {
         let Some(f) = self.font() else { return Vec::new() };
         [skrifa::string::StringId::FAMILY_NAME, skrifa::string::StringId::TYPOGRAPHIC_FAMILY_NAME, skrifa::string::StringId::FULL_NAME]
             .into_iter()
-            // A name table can hold many records that point at one big string: at most
-            // MAX_NAME_RECORDS per name id, each decoded only up to MAX_NAME_CHARS characters.
-            .flat_map(|id| {
-                f.localized_strings(id).take(MAX_NAME_RECORDS).map(|s| norm(&s.chars().take(MAX_NAME_CHARS).collect::<String>())).collect::<Vec<_>>()
-            })
+            .flat_map(|id| f.localized_strings(id).map(|s| norm(&s.to_string())).collect::<Vec<_>>())
             .collect()
     }
 }
@@ -108,10 +104,6 @@ pub fn faces(bytes: &Arc<Vec<u8>>) -> Vec<Face> {
 }
 
 const MAX_FACES_PER_FILE: u32 = 32;
-/// Name records read per name id of a face, and characters read of each: real fonts have a few
-/// dozen short ones.
-const MAX_NAME_RECORDS: usize = 64;
-const MAX_NAME_CHARS: usize = 256;
 /// Bounds of the missing-glyph search: files loaded, bytes loaded, faces checked per character.
 const MAX_FALLBACK_FILES: usize = 24;
 const MAX_FALLBACK_BYTES: usize = 160 << 20;

crates/geom/src/region.rs

@@ -110,10 +110,6 @@ fn build(polys: &[Vec<Vec2>], tol: f64) -> Option<Graph> {
     };
     let mut grid: HashMap<(usize, usize), Vec<usize>> = HashMap::new();
     let mut long: Vec<usize> = Vec::new();
-    // Every cell a segment is registered in is an index kept: past this many in all (each segment
-    // may take up to 4096 cells), the input is refused as it is for too much pair work.
-    const MAX_REGISTRATIONS: usize = 20_000_000;
-    let mut registered = 0usize;
     for (i, (a, b)) in segs.iter().enumerate() {
         let (x0, y0) = cell(a.min(*b));
         let (x1, y1) = cell(a.max(*b));
@@ -122,10 +118,6 @@ fn build(polys: &[Vec<Vec2>], tol: f64) -> Option<Graph> {
             long.push(i);
             continue;
         }
-        registered = registered.saturating_add((x1 - x0 + 1) * (y1 - y0 + 1));
-        if registered > MAX_REGISTRATIONS {
-            return None;
-        }
         for gx in x0..=x1 {
             for gy in y0..=y1 {
                 grid.entry((gx, gy)).or_default().push(i);
@@ -399,10 +391,10 @@ fn outside(core: &Graph, u: usize, steps: usize) -> Option<Vec<Vec2>> {
 }
 
 /// Outlines of the groups of curves inside the face `outer` (vertex indices into `g`).
-fn islands(g: &Graph, outer: &[usize], p: Vec2) -> Option<Vec<Island>> {
+fn islands(g: &Graph, outer: &[usize], p: Vec2) -> Vec<Island> {
     let comp = components(g);
     let outer_comp: std::collections::HashSet<usize> = outer.iter().filter_map(|&i| comp.get(i).copied()).collect();
-    let outer_pts = points(g, outer)?;
+    let Some(outer_pts) = points(g, outer) else { return Vec::new() };
     let ob = Bounds2::from_points(outer_pts.iter().copied());
     let core = prune(g);
     // Leftmost (then lowest) vertex still on a cycle, per component.
@@ -432,32 +424,19 @@ fn islands(g: &Graph, outer: &[usize], p: Vec2) -> Option<Vec<Island>> {
     let mut starts: Vec<(usize, usize)> = start.into_iter().collect();
     starts.sort_unstable();
     let mut out = Vec::new();
-    // Each containment test scans a whole polygon, accepted or not: they share one allowance of
-    // polygon edges, and the search ends when it is spent.
-    let mut edges_left: usize = 100_000_000;
     for (c, u) in starts {
         if out.len() >= MAX_ISLANDS {
             break;
         }
         let Some(&pu) = core.pts.get(u) else { continue };
-        if !ob.contains(pu) {
-            continue;
-        }
-        let left = edges_left.checked_sub(outer_pts.len())?;
-        edges_left = left;
-        if !point_in_polygon(&outer_pts, pu) {
+        if !ob.contains(pu) || !point_in_polygon(&outer_pts, pu) {
             continue;
         }
         let steps = edges.get(&c).copied().unwrap_or(0).saturating_add(4);
         let Some(lp) = outside(&core, u, steps) else { continue };
         let area = shoelace(&lp);
         let ib = Bounds2::from_points(lp.iter().copied());
-        if lp.len() < 3 || !area.is_finite() || area <= 0.0 || !ob.contains_box(&ib) {
-            continue;
-        }
-        let left = edges_left.checked_sub(lp.len())?;
-        edges_left = left;
-        if point_in_polygon(&lp, p) {
+        if lp.len() < 3 || !area.is_finite() || area <= 0.0 || !ob.contains_box(&ib) || point_in_polygon(&lp, p) {
             continue;
         }
         let mut src = sources.remove(&c).unwrap_or_default();
@@ -465,7 +444,7 @@ fn islands(g: &Graph, outer: &[usize], p: Vec2) -> Option<Vec<Island>> {
         src.dedup();
         out.push(Island { outline: lp, sources: src });
     }
-    Some(out)
+    out
 }
 
 /// The point of the nearest other curve within `snap` of the end `e` of curve `i`, as (curve,
@@ -478,10 +457,6 @@ fn nearest_curve(polys: &[Vec<Vec2>], i: usize, e: Vec2, snap: f64, tol: f64, bu
         if j == i {
             continue;
         }
-        // A curve with no segment costs a visit too, so many of them can't multiply the searches.
-        if other.len() < 2 {
-            *budget = budget.checked_sub(1).ok_or(())?;
-        }
         for (k, w) in other.windows(2).enumerate() {
             if *budget == 0 {
                 return Err(());
@@ -583,7 +558,7 @@ pub fn boundary_at_gap(polys: &[Vec<Vec2>], p: Vec2, tol: f64, gap: f64) -> Opti
     let g = build_gap(polys, tol, gap)?;
     let ids = outer_face(&g, p)?;
     let outer = points(&g, &ids)?;
-    let islands = islands(&g, &ids, p)?;
+    let islands = islands(&g, &ids, p);
     let mut outer_sources: Vec<usize> =
         ids.iter().zip(ids.iter().cycle().skip(1)).filter_map(|(&u, &v)| g.edge_src.get(&(u.min(v), u.max(v))).copied()).collect();
     outer_sources.sort_unstable();

crates/mcp/Cargo.toml

@@ -17,3 +17,7 @@ serde_json = { workspace = true }
 log = { workspace = true }
 # Secure randomness for the per-session control token (MIT OR Apache-2.0).
 getrandom = "0.4"
+
+[target.'cfg(unix)'.dependencies]
+# O_NOFOLLOW when reading the token file (MIT OR Apache-2.0).
+libc = "0.2"

crates/mcp/src/backend.rs

@@ -1,7 +1,7 @@
 //! Where MCP tool calls end up: a control-channel method call.
 
 use std::io::{BufRead, BufReader, Read, Write};
-use std::net::{TcpStream, ToSocketAddrs};
+use std::net::{Ipv4Addr, SocketAddr, TcpStream, ToSocketAddrs};
 use std::time::Duration;
 
 use cadcraft_engine::Session;
@@ -76,9 +76,17 @@ impl Remote {
     /// 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.
+    ///
+    /// The desktop app listens only on `127.0.0.1:PORT`, so the token goes only to that exact
+    /// peer: a name that also resolves elsewhere (`localhost` → `::1`, where another local account
+    /// may listen on the same port) must not be handed the desktop's credential (CWE-522).
     fn token(&mut self) -> Option<&str> {
+        let port = port_of(&self.addr)?;
+        let peer = self.conn.as_ref().and_then(|(_, s)| s.peer_addr().ok());
+        if peer != Some(SocketAddr::from((Ipv4Addr::LOCALHOST, port))) {
+            return None;
+        }
         if self.token.is_none()
-            && let Some(port) = port_of(&self.addr)
             && let Ok(t) = crate::control_token::read(port)
         {
             self.token = Some(t);
@@ -207,6 +215,14 @@ fn is_read_only(method: &str) -> bool {
 
 impl Remote {
     /// Put the current token (if any) on `req`, replacing the one it carries.
+    /// Connect if needed, put the token for the peer actually connected to on `req`, and send it.
+    fn sign_and_send(&mut self, req: &mut Value) -> Result<String, (std::io::Error, bool)> {
+        if self.conn.is_none() {
+            self.reach().map_err(|e| (e, false))?;
+        }
+        self.set_token(req);
+        self.roundtrip(&req.to_string())
+    }
     fn set_token(&mut self, req: &mut Value) {
         let token = self.token().map(str::to_string);
         if let Some(obj) = req.as_object_mut() {
@@ -219,7 +235,7 @@ impl Remote {
     /// Send `req` and parse the reply, refreshing the token before a safe retry after a transport
     /// error. A mutation that may have reached the server is never resent automatically.
     fn exchange(&mut self, method: &str, req: &mut Value) -> Result<Value, String> {
-        let reply = match self.roundtrip(&req.to_string()) {
+        let reply = match self.sign_and_send(req) {
             Ok(r) => r,
             Err((e, sent)) => {
                 self.token = None;
@@ -232,8 +248,7 @@ impl Remote {
                     ));
                 }
                 // A restarted server may have rotated its token before dropping the old socket.
-                self.set_token(req);
-                self.roundtrip(&req.to_string()).map_err(|(e, _)| {
+                self.sign_and_send(req).map_err(|(e, _)| {
                     self.token = None;
                     self.conn = None;
                     format!("CADCraft at {}: {e}", self.addr)
@@ -256,14 +271,12 @@ impl Backend for Remote {
         let id = self.next_id;
         self.next_id += 1;
         let mut req = json!({"id": id, "method": method, "params": params});
-        self.set_token(&mut req);
         let mut v = self.exchange(method, &mut req)?;
         // The token is read before `--launch` opens the app, which then writes a new one (#2087); the
         // file can also be left over from an earlier session. Once, so a wrong token can't loop.
         if unauthorized(&v) && v.get("state").and_then(Value::as_str) == Some("not-run") {
             self.token = None;
             self.conn = None;
-            self.set_token(&mut req);
             v = self.exchange(method, &mut req)?;
         }
         if v.get("ok").and_then(Value::as_bool) == Some(true) {
@@ -589,6 +602,24 @@ mod tests {
         })
     }
 
+    /// A listener on another loopback address with the desktop's port (here `::1`) never receives
+    /// the desktop's token.
+    #[test]
+    fn token_is_not_sent_to_another_loopback_listener() {
+        let Ok(listener) = TcpListener::bind("[::1]:0") else { return };
+        let addr = listener.local_addr().unwrap();
+        crate::control_token::issue(addr.port()).unwrap();
+        let server = fake_app(listener, move |req, _| {
+            assert!(req.get("token").is_none(), "token leaked to {addr}");
+            json!({"ok": true, "result": "x"})
+        });
+        let mut remote = Remote::lazy(&addr.to_string(), false);
+        let got = remote.call("ping", json!({}));
+        crate::control_token::revoke(addr.port());
+        assert_eq!(got, Ok(json!("x")));
+        assert_eq!(server.join().unwrap(), 1);
+    }
+
     /// `--launch` reads the token before it opens the app, so the first request carries the token
     /// of an earlier session (or none); the app refuses it without running it, and the call is
     /// resent once with the token the app has just written (#2087).

crates/mcp/src/control_token.rs

@@ -32,7 +32,9 @@ fn private_dir() -> io::Result<PathBuf> {
     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())
+    // The shared temporary directory would put the credential at a path any local account can
+    // predict and plant a link at: refuse instead.
+    Err(io::Error::new(io::ErrorKind::NotFound, "XDG_RUNTIME_DIR or HOME is required for a private control token"))
 }
 
 /// Require the Windows per-user local app-data directory for session token storage.
@@ -75,7 +77,31 @@ fn issue_at(path: &std::path::Path, bytes: &[u8]) -> io::Result<()> {
 
 /// Read the token for `port`, trimming trailing whitespace. Used by clients (the MCP bridge).
 pub fn read(port: u16) -> io::Result<String> {
-    let raw = std::fs::read_to_string(token_path(port)?)?;
+    read_at(&token_path(port)?)
+}
+
+/// Read a token file the issuer wrote: not through a link, a plain file, and (Unix) readable by
+/// its owner only, so a file another account planted or left open is not taken as the credential.
+fn read_at(path: &std::path::Path) -> io::Result<String> {
+    use std::io::Read;
+    let mut options = std::fs::OpenOptions::new();
+    options.read(true);
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::OpenOptionsExt;
+        options.custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK);
+    }
+    let file = options.open(path)?;
+    let meta = file.metadata()?;
+    if !meta.is_file() {
+        return Err(io::Error::new(io::ErrorKind::InvalidData, "the control token is not a plain file"));
+    }
+    #[cfg(unix)]
+    if std::os::unix::fs::PermissionsExt::mode(&meta.permissions()) & 0o077 != 0 {
+        return Err(io::Error::new(io::ErrorKind::PermissionDenied, "the control token file is readable by other users"));
+    }
+    let mut raw = String::new();
+    file.take(1024).read_to_string(&mut raw)?;
     let token = raw.trim().to_string();
     if token.is_empty() {
         return Err(io::Error::new(io::ErrorKind::InvalidData, "empty control token"));
@@ -181,6 +207,24 @@ mod tests {
         assert_eq!(windows_private_dir(Some("private-app-data".into())).unwrap(), PathBuf::from("private-app-data"));
     }
 
+    /// A link, or a file other users can read, is not accepted as the token.
+    #[cfg(unix)]
+    #[test]
+    fn read_refuses_links_and_shared_files() {
+        use std::os::unix::fs::PermissionsExt;
+        let dir = std::env::temp_dir().join(format!("cadcraft-token-read-{}", std::process::id()));
+        std::fs::create_dir_all(&dir).unwrap();
+        let secret = dir.join("secret");
+        issue_at(&secret, b"abc").unwrap();
+        assert_eq!(read_at(&secret).unwrap(), "abc");
+        let link = dir.join("link");
+        std::os::unix::fs::symlink(&secret, &link).unwrap();
+        assert!(read_at(&link).is_err());
+        std::fs::set_permissions(&secret, std::fs::Permissions::from_mode(0o644)).unwrap();
+        assert!(read_at(&secret).is_err());
+        std::fs::remove_dir_all(dir).unwrap();
+    }
+
     /// Token issuance creates its missing directory hierarchy before writing the credential.
     #[test]
     fn issuance_creates_missing_parent() {