MCP: an agent's folders hold for file loads through the app and for a design's font files (#1112)

doc.open and the STEP/mesh imports run on the file loader's worker with the request's path scope, and a font named by file path in a design is checked against the scope before it is read.

crates/engine/src/cmd/sketch_create.rs

@@ -186,11 +186,8 @@ fn text_source(p: &Value, cmd: &str, old: Option<(String, Vec2, f64, f64, Option
         Some(_) => return Err(bad(cmd, "`font` must be a font family name or a font file path")),
     };
     let style = text_style(p, cmd, s0)?;
-    // A font file an agent names must lie in its folders (installed families are found by name).
-    if let Some(path) = font.as_deref().and_then(solidcraft_sketch::font_file_path) {
-        solidcraft_io::vfs::check(&path.to_string_lossy()).map_err(|e| bad(cmd, e.to_string()))?;
-    }
-    // An unknown font, an unreadable file or a style the font lacks is an error now, not a lost link later.
+    // An unknown font, an unreadable file or a style the font lacks is an error now, not a lost
+    // link later; so is a font file outside an agent's folders (the font reader asks `vfs`).
     solidcraft_sketch::font_data_styled(font.as_deref(), style.bold, style.italic).map_err(|e| bad(cmd, e.to_string()))?;
     Ok(LinkSource::Text { text, at, height, angle, font, style })
 }

crates/engine/src/tests/path_scope.rs

@@ -64,3 +64,32 @@ fn file_commands_stay_inside_the_agents_folders() {
     s.execute("file.export", &json!({"path": at(&out, "x.stl")})).unwrap();
     let _ = std::fs::remove_dir_all(&base);
 }
+
+/// A design names the font file of its text, and opening it reads that file: for an agent, a
+/// font outside its folders stays unread, even one already read (and cached) for the user.
+#[test]
+fn a_designs_font_file_stays_inside_the_agents_folders() {
+    let base = solidcraft_io::vfs::temp_dir().join(format!("solidcraft-font-scope-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&base);
+    let (root, out) = (base.join("root"), base.join("out"));
+    std::fs::create_dir_all(&root).unwrap();
+    std::fs::create_dir_all(&out).unwrap();
+    let font = out.join("f.ttf");
+    std::fs::write(&font, solidcraft_sketch::font_data(None).unwrap().bytes()).unwrap();
+    let mut s = Session::default();
+    s.execute("sketch.create", &json!({"plane": "XY"})).unwrap();
+    s.execute("sketch.text", &json!({"text": "A", "at": [0, 0], "font": font.to_string_lossy()})).unwrap();
+    s.execute("sketch.finish", &json!({})).unwrap();
+    let design = root.join("text.solidcraft");
+    std::fs::write(&design, solidcraft_io::write_design(&s.doc)).unwrap();
+    let open = json!({"path": design.to_string_lossy()});
+    // The sketch's warning says what its text couldn't read.
+    let warnings = |s: &Session| s.model.results.iter().filter_map(|r| r.warning.clone()).collect::<Vec<_>>().join("; ");
+    s.execute("doc.open", &open).unwrap();
+    assert!(warnings(&s).is_empty(), "the user's own open reads it: {}", warnings(&s));
+    scoped(Some(PathScope::new([root.clone()])), || {
+        s.execute("doc.open", &open).unwrap();
+        assert!(warnings(&s).contains("outside the folders this agent may use"), "{}", warnings(&s));
+    });
+    let _ = std::fs::remove_dir_all(&base);
+}

crates/io/src/vfs.rs

@@ -369,6 +369,8 @@ pub fn scoped<T>(scope: Option<PathScope>, f: impl FnOnce() -> T) -> T {
             SCOPE.with(|s| *s.borrow_mut() = prev);
         }
     }
+    // Sketch text reads the font files a design names, below this crate: it asks here first.
+    solidcraft_doc::sketch::set_font_file_check(|p| check(&p.to_string_lossy()).map_err(|e| e.to_string()));
     let _restore = Restore(SCOPE.with(|s| s.replace(scope)));
     f()
 }

crates/sketch/src/fonts.rs

@@ -24,6 +24,15 @@ const MAX_CACHED_BYTES: usize = 128 << 20;
 /// Longest font name or path accepted.
 const MAX_FONT_NAME: usize = 1024;
 
+/// Asked before a font file named by its path is read. A design names such files, and an
+/// agent's folders (`vfs::PathScope`) live above this crate; the first check set stays.
+static FILE_CHECK: OnceLock<fn(&Path) -> std::result::Result<(), String>> = OnceLock::new();
+
+/// Set the check [`font_data`] runs on a font file's path before reading it.
+pub fn set_font_file_check(check: fn(&Path) -> std::result::Result<(), String>) {
+    let _ = FILE_CHECK.set(check);
+}
+
 /// The bytes of a font file and the face to use in it (collections hold several faces).
 #[derive(Clone)]
 pub struct FontData {
@@ -59,6 +68,10 @@ pub fn font_data(name: Option<&str>) -> Result<FontData> {
     }
     if is_path(name) {
         let path = expand(name);
+        // Before the cache too: a file read once for the user is not the agent's to read.
+        if let Some(check) = FILE_CHECK.get() {
+            check(&path).map_err(|e| SketchError::Invalid(format!("font file `{name}`: {e}")))?;
+        }
         let bytes = read_font(&path).map_err(|e| SketchError::Invalid(format!("font file `{name}`: {e}")))?;
         let index = 0;
         ttf_parser::Face::parse(&bytes, index)
@@ -306,13 +319,6 @@ fn normalize(s: &str) -> String {
     s.chars().filter(|c| c.is_alphanumeric()).flat_map(char::to_lowercase).collect()
 }
 
-/// The file a font name refers to (after `~` and `%VAR%` expansion), or `None` for the built-in
-/// font or an installed family, so callers can vet the path before [`font_data`] reads it.
-pub fn font_file_path(name: &str) -> Option<PathBuf> {
-    let name = name.trim();
-    (is_path(name) && name.len() <= MAX_FONT_NAME).then(|| expand(name))
-}
-
 fn is_path(name: &str) -> bool {
     name.contains('/') || name.contains('\\') || has_font_extension(name)
 }

crates/sketch/src/lib.rs

@@ -43,7 +43,7 @@ pub use curves::{
 };
 pub use decompose::Budget;
 pub use dims::{DimFrame, DimLayout, chain_centres, default_text, dim_frame, dim_layout, encode_text};
-pub use fonts::{DEFAULT_FONT_FAMILY, FontData, families_in, find_style_in, font_data, font_data_styled, font_file_path, installed_families};
+pub use fonts::{DEFAULT_FONT_FAMILY, FontData, families_in, find_style_in, font_data, font_data_styled, installed_families, set_font_file_check};
 pub use freeform_offset::{FreeformOffset, OFFSET_TOLERANCE};
 pub use link::{Link, LinkGeom, LinkKind, LinkSource, MAX_LINK_ENTITIES, SKETCH_AXIS_HALF, sketch_axis_geom};
 pub use model::{

crates/ui-egui/src/control.rs

@@ -159,20 +159,28 @@ fn batch(app: &mut SolveApp, p: &Value) -> Outcome {
     ok(json!({"completed": results.len(), "results": results, "undone": 0}))
 }
 
-pub fn handle(app: &mut SolveApp, ctx: &egui::Context, req: &ControlRequest) -> Outcome {
-    // A request from an MCP server carries the folders its agent may use; every file the
-    // request reaches must lie inside them (docs/mcp.md).
-    let Some(allowed) = req.params.get("allow_paths") else { return dispatch(app, ctx, &req.method, &req.params) };
+/// A request from an MCP server carries the folders its agent may use (`allow_paths`); every
+/// file the request reaches must lie inside them (docs/mcp.md). `None`: it names none.
+pub fn request_scope(app: &SolveApp, params: &Value) -> Result<Option<vfs::PathScope>, String> {
+    let Some(allowed) = params.get("allow_paths") else { return Ok(None) };
     let Some(roots) = allowed.as_array().and_then(|a| a.iter().map(|r| r.as_str().map(std::path::PathBuf::from)).collect::<Option<Vec<_>>>()) else {
-        return err("`allow_paths` must be a list of folders");
+        return Err("`allow_paths` must be a list of folders".into());
+    };
+    // Plus the app's own recovery folder and, when it is a safe one, the open design's folder.
+    let roots = roots.into_iter().chain(solidcraft_engine::recovery::default_dir());
+    Ok(Some(vfs::PathScope::new(roots).with_design(app.session.path.as_deref())))
+}
+
+pub fn handle(app: &mut SolveApp, ctx: &egui::Context, req: &ControlRequest) -> Outcome {
+    let scope = match request_scope(app, &req.params) {
+        Ok(Some(scope)) => scope,
+        Ok(None) => return dispatch(app, ctx, &req.method, &req.params),
+        Err(e) => return err(e),
     };
     let mut p = req.params.clone();
     if let Some(o) = p.as_object_mut() {
         o.remove("allow_paths");
     }
-    // Plus the app's own recovery folder and, when it is a safe one, the open design's folder.
-    let roots = roots.into_iter().chain(solidcraft_engine::recovery::default_dir());
-    let scope = vfs::PathScope::new(roots).with_design(app.session.path.as_deref());
     vfs::scoped(Some(scope), || dispatch(app, ctx, &req.method, &p))
 }
 
@@ -867,6 +875,33 @@ mod tests {
         let _ = std::fs::remove_dir_all(&base);
     }
 
+    /// An MCP server's `doc.open` and imports load off the window thread (`file_load`), where
+    /// the agent's folders hold as they do for every other request.
+    #[test]
+    fn file_loads_stay_inside_allow_paths() {
+        let mut h = crate::scenario::Harness::new();
+        let base = vfs::temp_dir().join(format!("solidcraft-load-scope-{}", std::process::id()));
+        let (root, out) = (base.join("root"), base.join("out"));
+        std::fs::create_dir_all(&root).unwrap();
+        std::fs::create_dir_all(&out).unwrap();
+        let mut s = Session::default();
+        s.execute("solid.box", &json!({"length": 10, "width": 10, "height": 10})).unwrap();
+        for d in [&root, &out] {
+            std::fs::write(d.join("part.solidcraft"), solidcraft_engine::io::write_design(&s.doc)).unwrap();
+        }
+        s.execute("file.export", &json!({"path": out.join("part.step").to_string_lossy()})).unwrap();
+        let load = |command: &str, d: &std::path::Path, file: &str| json!({"command": command, "params": {"path": d.join(file).to_string_lossy()}, "allow_paths": [root.to_string_lossy()]});
+        for (command, file) in [("doc.open", "part.solidcraft"), ("file.insert_step", "part.step")] {
+            let (v, _) = h.call_queued("engine.execute", load(command, &out, file));
+            assert!(v["ok"] == false && v.to_string().contains("outside the folders this agent may use"), "{command}: {v}");
+        }
+        let (v, _) = h.call_queued("engine.execute", load("doc.open", &root, "part.solidcraft"));
+        assert_eq!(v["ok"], true, "{v}");
+        let (v, _) = h.call_queued("engine.execute", json!({"command": "doc.open", "params": {"path": "x"}, "allow_paths": "/"}));
+        assert!(v["error"].as_str().is_some_and(|e| e.contains("allow_paths")), "{v}");
+        let _ = std::fs::remove_dir_all(&base);
+    }
+
     /// Each step runs as `engine.execute` runs it: same results, same design.
     #[test]
     fn engine_batch_matches_engine_execute() {

crates/ui-egui/src/file_load.rs

@@ -37,11 +37,13 @@ pub struct Request {
     pub kind: Kind,
     /// A control-channel caller waiting for the command's reply.
     pub reply: Option<Sender<ControlResponse>>,
+    /// The folders that caller's agent may use: the worker reads the file inside them.
+    pub scope: Option<solidcraft_engine::io::vfs::PathScope>,
 }
 
 impl Request {
     pub fn new(path: &str, kind: Kind) -> Self {
-        Request { path: path.into(), kind, reply: None }
+        Request { path: path.into(), kind, reply: None, scope: None }
     }
     fn insert(&self) -> bool {
         self.kind == Kind::Insert
@@ -180,7 +182,14 @@ pub fn control(app: &mut SolveApp, req: crate::control::ControlRequest) -> Optio
         _ => return Some(req),
     };
     let Some(path) = req.params.get("params").and_then(|p| p.get("path")).and_then(Value::as_str) else { return Some(req) };
-    let _ = begin_request(app, Request { path: path.into(), kind, reply: Some(req.reply.clone()) });
+    let scope = match crate::control::request_scope(app, &req.params) {
+        Ok(scope) => scope,
+        Err(e) => {
+            let _ = req.reply.send(json!({"ok": false, "error": e}));
+            return None;
+        }
+    };
+    let _ = begin_request(app, Request { path: path.into(), kind, reply: Some(req.reply.clone()), scope });
     None
 }
 
@@ -238,6 +247,7 @@ fn start(app: &mut SolveApp) {
     let (tx, rx) = std::sync::mpsc::channel();
     let (preview_tx, preview_rx) = std::sync::mpsc::channel();
     let (command, params) = request.command();
+    let scope = request.scope.clone();
     let thumbnail = request.kind == Kind::Open;
     let spawned = std::thread::Builder::new().name("file-load".into()).spawn(move || {
         let preview_began = std::time::Instant::now();
@@ -256,9 +266,10 @@ fn start(app: &mut SolveApp) {
                 }
             }));
         }
-        let result =
-            std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| compute(scratch, command, &params, thumbnail, &worker_phase, &worker_progress)))
-                .unwrap_or_else(|_| Err("the file operation failed; the current design is unchanged".into()));
+        let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+            compute(scratch, command, &params, scope, thumbnail, &worker_phase, &worker_progress)
+        }))
+        .unwrap_or_else(|_| Err("the file operation failed; the current design is unchanged".into()));
         let _ = tx.send(result);
     });
     match spawned {
@@ -364,15 +375,26 @@ fn preview_work_fits(scene: &solidcraft_engine::render::Scene, camera: &solidcra
 }
 
 #[cfg(not(target_arch = "wasm32"))]
-fn compute(mut session: Session, command: &str, params: &Value, thumbnail: bool, phase: &AtomicU8, progress: &Arc<Progress>) -> Job {
+fn compute(
+    mut session: Session,
+    command: &str,
+    params: &Value,
+    scope: Option<solidcraft_engine::io::vfs::PathScope>,
+    thumbnail: bool,
+    phase: &AtomicU8,
+    progress: &Arc<Progress>,
+) -> Job {
     let profile = solidcraft_engine::geom::env::var_os("PROFILE_OPEN").is_some();
     let began = std::time::Instant::now();
     let cancelled = || Err("file operation cancelled".to_string());
     if progress.is_cancelled() {
         return cancelled();
     }
-    // The timeline's rebuild reports each feature and stops at the next one after Cancel.
-    let result = solidcraft_engine::doc::progress::watch(progress.clone(), || session.execute(command, params)).map_err(|e| e.to_string())?;
+    // The timeline's rebuild reports each feature and stops at the next one after Cancel. An
+    // agent's scope is per thread, so it is set again here.
+    let result =
+        solidcraft_engine::io::vfs::scoped(scope, || solidcraft_engine::doc::progress::watch(progress.clone(), || session.execute(command, params)))
+            .map_err(|e| e.to_string())?;
     if progress.is_cancelled() {
         return cancelled();
     }

docs/mcp.md

@@ -55,7 +55,8 @@ a new file must go into a folder that exists. On Windows a path must be on a loc
 file name (alternate data streams), reserved device names with any extension (`NUL`, `CON`,
 `COM1.txt`, …) and short 8.3 names (`PROGRA~1`) are refused before any file system call, and the
 resolved path is checked again, so a link or junction that leads to a share is refused too. This
-includes a `sketch.text` font given as a file path; installed fonts named by family still work.
+includes a `sketch.text` font given as a file path and the font file a design's text names when
+the design is opened; installed fonts named by family still work.
 The server spells its own folders (scratch, recovery) with long names, so they work when the
 temporary folder's path has a short name (`…\NAME~1.XYZ\AppData\Local\Temp`, as Windows often
 gives when the user folder's name is long).