From 96f6424aecbf9df5d7b451be19444fd76ba3650a Mon Sep 17 00:00:00 2001 From: Kevin Ravensberg Date: Wed, 14 Oct 2026 00:24:08 +0200 Subject: [PATCH] fix: confine MCP agents' files to allowed folders; recovery discard deletes only recovery entries (#205) (#578) Agent tool calls (MCP, and bridged control requests carrying allow_paths) reach files only in the open design's folder (strictly inside home or the temporary folder, not in a hidden folder, and never its hidden files), the app's recovery folder, the server's private scratch folder and each --allow-path; --allow-any-path lifts the limit. Paths are resolved first, so .. and symlinks that lead out (including a save through a link, #845) are refused. On Windows, UNC, device, NT, verbatim non-drive, drive-relative, alternate-stream, reserved-device and 8.3 short-name paths are refused before any file system call, and the resolved path must be on a local drive. doc.recovery_discard and doc.recover {discard} delete only real recovery entries. Docs: mcp.md, control-protocol.md, README. --- README.md | 4 +- apps/solidcraft-cli/src/main.rs | 25 +- crates/engine/src/cmd/file.rs | 16 +- crates/engine/src/cmd/motion.rs | 1 + crates/engine/src/cmd/motion_tests.rs | 9 + crates/engine/src/cmd/sketch_create.rs | 4 + crates/engine/src/recovery.rs | 69 +++- crates/engine/src/tests.rs | 1 + crates/engine/src/tests/path_scope.rs | 66 ++++ crates/io/src/design.rs | 1 + crates/io/src/design/thumbnail.rs | 1 + crates/io/src/safe_write.rs | 4 +- crates/io/src/vfs.rs | 494 ++++++++++++++++++++++++- crates/mcp/src/backend.rs | 15 +- crates/mcp/src/lib.rs | 47 +++ crates/mcp/src/server.rs | 16 +- crates/mcp/src/tests.rs | 115 ++++++ crates/mcp/src/tools.rs | 13 +- crates/sketch/src/fonts.rs | 7 + crates/sketch/src/lib.rs | 2 +- crates/ui-egui/src/control.rs | 73 +++- docs/control-protocol.md | 9 + docs/mcp.md | 45 +++ 23 files changed, 1000 insertions(+), 37 deletions(-) create mode 100644 crates/engine/src/tests/path_scope.rs diff --git a/README.md b/README.md index 59bf0728..937f157e 100644 --- a/README.md +++ b/README.md @@ -199,7 +199,9 @@ stdio. Headless it runs its own design session; with `--connect 127.0.0.1:PORT` started with `solidcraft --control PORT`, so you can watch the agent model. Tools: `list_commands`, `execute`, `batch`, `inspect_design`, `measure`, `body_topology`, `set_parameter`, `screenshot` (returns the image), `export`, `undo`, `redo`, `new_design`, `open`, -`save`. Claude Code: `claude mcp add solidcraft -- solidcraft-cli mcp`. Details, client +`save`. Tool calls reach files only in the open design's folder (when it lies inside your home +folder, never home itself or a hidden folder), SolidCraft's recovery folder and folders given with +`--allow-path DIR`. Claude Code: `claude mcp add solidcraft -- solidcraft-cli mcp`. Details, client configuration and an example session: [docs/mcp.md](docs/mcp.md). ## Architecture diff --git a/apps/solidcraft-cli/src/main.rs b/apps/solidcraft-cli/src/main.rs index 69581c7d..de72081b 100644 --- a/apps/solidcraft-cli/src/main.rs +++ b/apps/solidcraft-cli/src/main.rs @@ -9,7 +9,8 @@ //! solidcraft-cli commands the command registry as JSON //! solidcraft-cli recipe translate an oracle recipe to a script //! solidcraft-cli oracle ... replay recipes and compare with expected.json -//! solidcraft-cli mcp [--in design|script] [--connect HOST:PORT] MCP server on stdio (docs/mcp.md) +//! solidcraft-cli mcp [--in design|script] [--connect HOST:PORT] [--allow-path DIR]... [--allow-any-path] +//! MCP server on stdio (docs/mcp.md) //! solidcraft-cli licences SolidCraft's licence and the third-party licences //! solidcraft-cli icon --out F.png|F.ico [--size N] the app icon (packaging; drawn, not a file) //! ``` @@ -43,7 +44,7 @@ const USAGE: &str = "usage: solidcraft-cli recipe solidcraft-cli oracle ... [--json] solidcraft-cli fuzz [--from N] [--count N] [--jobs J] [--timeout S] [--steps N] [--minimise] [--out DIR] [--report FILE] - solidcraft-cli mcp [--in design.solidcraft|script.json] [--connect 127.0.0.1:PORT] + solidcraft-cli mcp [--in design.solidcraft|script.json] [--connect 127.0.0.1:PORT] [--allow-path DIR]... [--allow-any-path] solidcraft-cli crashes [list [--json] | show | replay | dir | clear []] solidcraft-cli licences solidcraft-cli icon --out icon.png [--size 256] | --out icon.ico @@ -312,8 +313,24 @@ fn cmd_exec(args: &[String]) -> Result<(), String> { } /// MCP server on stdio: headless (optionally starting from a design or script) or bridged to -/// a running app's control port. stdout carries only protocol messages. +/// a running app's control port. stdout carries only protocol messages. Files the agent's tool +/// calls reach are confined to `solidcraft_mcp::agent_scope` unless `--allow-any-path` is given. fn cmd_mcp(args: &[String]) -> Result<(), String> { + let scope = (!args.iter().any(|a| a == "--allow-any-path")) + .then(|| solidcraft_mcp::agent_scope(flags(args, "--allow-path").into_iter().map(std::path::PathBuf::from))); + if let Some(s) = &scope { + let roots: Vec = s.roots().iter().map(|r| r.display().to_string()).collect(); + eprintln!( + "solidcraft mcp: files limited to {} and the open design's folder (--allow-path DIR adds one, --allow-any-path lifts the limit)", + roots.join(", ") + ); + let design_dir = flag(args, "--in") + .and_then(|p| std::path::Path::new(p).parent()) + .map(|d| if d.as_os_str().is_empty() { std::path::Path::new(".") } else { d }); + if let Some(why) = design_dir.and_then(solidcraft_engine::io::vfs::design_folder_refusal) { + eprintln!("solidcraft mcp: the design's folder is left out because {why}; add it with --allow-path if you mean it"); + } + } let backend: Box = match (flag(args, "--connect"), flag(args, "--in")) { (Some(_), Some(_)) => return Err("mcp: use either --connect or --in, not both".into()), (Some(addr), None) => Box::new(solidcraft_mcp::Remote::connect(addr).map_err(|e| { @@ -328,7 +345,7 @@ fn cmd_mcp(args: &[String]) -> Result<(), String> { }; let stdin = std::io::stdin(); let stdout = std::io::stdout(); - solidcraft_mcp::Server::new(backend).serve(stdin.lock(), stdout.lock()).map_err(|e| e.to_string()) + solidcraft_mcp::Server::new(backend).with_path_scope(scope).serve(stdin.lock(), stdout.lock()).map_err(|e| e.to_string()) } /// `save-stress `: save a design of a few hundred kilobytes `rounds` times, its diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 3a3dc474..e09af4bd 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -170,6 +170,7 @@ fn rename_design(s: &mut Session, from: &str, to: &str) { } fn write(s: &mut Session, path: &str, format: &str, compression: &str, preview: bool, cache: bool) -> Result { + solidcraft_io::vfs::check(path).map_err(|e| EngineError::Other(e.to_string()))?; // Saving an unnamed design names it after its file (not an undoable edit). let renamed = default_name_for(&s.doc.name, path).map(|stem| (s.doc.name.clone(), stem)); if let Some((from, to)) = &renamed { @@ -337,10 +338,13 @@ fn save_stl(s: &mut Session, p: &Value) -> Result { } fn recovery_dir(p: &Value, cmd: &str) -> Result { - match str_(p, "dir").filter(|d| !d.trim().is_empty() && d.len() < 4096) { - Some(d) => Ok(d.into()), - None => crate::recovery::default_dir().ok_or_else(|| bad(cmd, "no recovery folder on this system (give `dir`)")), - } + let dir = match str_(p, "dir").filter(|d| !d.trim().is_empty() && d.len() < 4096) { + Some(d) => d.into(), + None => crate::recovery::default_dir().ok_or_else(|| bad(cmd, "no recovery folder on this system (give `dir`)"))?, + }; + // An agent's commands reach the recovery folder only when it is one of their folders. + solidcraft_io::vfs::check(&dir.to_string_lossy()).map_err(|e| EngineError::Other(e.to_string()))?; + Ok(dir) } fn recovery_list(_s: &mut Session, p: &Value) -> Result { @@ -360,7 +364,7 @@ fn recover(s: &mut Session, p: &Value) -> Result { s.path = entry.path.clone(); s.mark_unsaved(); if bool_(p, "discard").unwrap_or(false) { - crate::recovery::remove(&dir, id); + crate::recovery::remove(&dir, id)?; } let errors = s.model.results.iter().filter(|r| r.error.is_some()).count(); Ok(json!({"id": id, "path": entry.path, "name": entry.name, "features": s.doc.features.len(), "errors": errors})) @@ -375,7 +379,7 @@ fn recovery_discard(_s: &mut Session, p: &Value) -> Result { vec![str_(p, "id").ok_or_else(|| bad(cmd, "`id` or `all` is required"))?.to_string()] }; for id in &ids { - crate::recovery::remove(&dir, id); + crate::recovery::remove(&dir, id)?; } Ok(json!({"discarded": ids})) } diff --git a/crates/engine/src/cmd/motion.rs b/crates/engine/src/cmd/motion.rs index ab9b42f1..c189f243 100644 --- a/crates/engine/src/cmd/motion.rs +++ b/crates/engine/src/cmd/motion.rs @@ -589,6 +589,7 @@ fn export(s: &mut Session, p: &Value) -> Result { } else { serde_json::to_string_pretty(&out).unwrap_or_default() }; + solidcraft_io::vfs::check(path).map_err(|e| EngineError::Other(e.to_string()))?; solidcraft_io::write_atomic(std::path::Path::new(path), text.as_bytes(), false).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; return Ok(json!({"path": path, "samples": rows.len()})); } diff --git a/crates/engine/src/cmd/motion_tests.rs b/crates/engine/src/cmd/motion_tests.rs index 449448b1..934ceb7c 100644 --- a/crates/engine/src/cmd/motion_tests.rs +++ b/crates/engine/src/cmd/motion_tests.rs @@ -136,6 +136,15 @@ fn motion_study_plays_and_exports() { run(&mut s, "motion.export", json!({"study": "Push", "samples": 3, "path": csv.to_string_lossy()})); let text = std::fs::read_to_string(&csv).unwrap(); assert!(text.starts_with("step,occurrence,x,y,z") && text.lines().count() == 1 + 3 * 2, "{text}"); + // For an agent (#205) the export lands only inside its folders. + let root = dir.join("root"); + std::fs::create_dir_all(&root).unwrap(); + solidcraft_io::vfs::scoped(Some(solidcraft_io::vfs::PathScope::new([root.clone()])), || { + run(&mut s, "motion.export", json!({"study": "Push", "samples": 2, "path": root.join("in.json").to_string_lossy()})); + let e = s.execute("motion.export", &json!({"study": "Push", "samples": 2, "path": dir.join("out.json").to_string_lossy()})).unwrap_err(); + assert!(e.to_string().contains("outside the folders this agent may use"), "{e}"); + }); + assert!(root.join("in.json").exists() && !dir.join("out.json").exists()); let _ = std::fs::remove_dir_all(&dir); assert!(s.execute("motion.study", &json!({"keys": [{"joint": "Rail", "index": 3, "points": [[0, 1]]}]})).is_err()); run(&mut s, "motion.delete", json!({"study": "Push"})); diff --git a/crates/engine/src/cmd/sketch_create.rs b/crates/engine/src/cmd/sketch_create.rs index adacb0b3..7d53ba08 100644 --- a/crates/engine/src/cmd/sketch_create.rs +++ b/crates/engine/src/cmd/sketch_create.rs @@ -117,6 +117,10 @@ fn text_source(p: &Value, cmd: &str, old: Option<(String, Vec2, f64, f64, Option Some(Value::String(f)) => Some(f.trim().to_string()), Some(_) => return Err(bad(cmd, "`font` must be a font family name or a font file path")), }; + // 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 or an unreadable file is an error now, not a lost link later. solidcraft_sketch::font_data(font.as_deref()).map_err(|e| bad(cmd, e.to_string()))?; Ok(LinkSource::Text { text, at, height, angle, font }) diff --git a/crates/engine/src/recovery.rs b/crates/engine/src/recovery.rs index b36cf8b1..55adcdee 100644 --- a/crates/engine/src/recovery.rs +++ b/crates/engine/src/recovery.rs @@ -130,13 +130,19 @@ pub fn load(dir: &Path, id: &str) -> Result<(Document, Entry)> { Ok((solidcraft_io::read_design(&bytes)?, entry)) } -/// Delete an entry. -pub fn remove(dir: &Path, id: &str) { - if valid_id(id) { - for ext in ["json", solidcraft_io::DESIGN_EXT, solidcraft_io::LEGACY_DESIGN_EXT, "lock"] { - let _ = fs::remove_file(file(dir, id, ext)); - } +/// Delete an entry. Only a real one: `.json` must describe entry `id` and its design must +/// be there, so a folder and id that merely name someone's `part.solidcraft` delete nothing. +pub fn remove(dir: &Path, id: &str) -> Result<()> { + let is_entry = valid_id(id) + && fs::read(file(dir, id, "json")).ok().and_then(|b| serde_json::from_slice::(&b).ok()).is_some_and(|e| e.id == id) + && design_file(dir, id).is_file(); + if !is_entry { + return Err(EngineError::Other(format!("no recovered design `{id}`"))); } + for ext in ["json", solidcraft_io::DESIGN_EXT, solidcraft_io::LEGACY_DESIGN_EXT, "lock"] { + let _ = fs::remove_file(file(dir, id, ext)); + } + Ok(()) } /// One app's autosaver. @@ -241,7 +247,7 @@ mod tests { assert_eq!(orphans(&d).iter().map(|e| e.id.as_str()).collect::>(), [id]); let (_, loaded) = load(&d, id).unwrap(); assert_eq!(loaded, entry); - remove(&d, id); + remove(&d, id).unwrap(); assert!(orphans(&d).is_empty() && !d.join(format!("{id}.solvecraft")).exists()); let _ = fs::remove_dir_all(&d); } @@ -282,7 +288,7 @@ mod tests { let (doc, entry) = load(&d, &found[0].id).unwrap(); assert_eq!(doc, *s.doc); assert_eq!(entry.features, 1); - remove(&d, &entry.id); + remove(&d, &entry.id).unwrap(); assert!(orphans(&d).is_empty()); let _ = fs::remove_dir_all(&d); } @@ -302,6 +308,53 @@ mod tests { let _ = fs::remove_dir_all(&d); } + /// `doc.recovery_discard` deletes recovery entries only (#205): a folder and id that merely + /// name someone's design, or a `.json` describing another entry, delete nothing. + #[test] + fn discarding_refuses_what_is_not_a_recovery_entry() { + let d = dir("discard"); + fs::create_dir_all(&d).unwrap(); + let part = d.join("part.solidcraft"); + fs::write(&part, solidcraft_io::write_design(&Document::default())).unwrap(); + let mut s = Session::default(); + let discard = |s: &mut Session, id: &str| s.execute("doc.recovery_discard", &json!({"dir": d.to_string_lossy(), "id": id})); + let e = discard(&mut s, "part").unwrap_err(); + assert!(e.to_string().contains("no recovered design `part`"), "{e}"); + let other = Entry { id: "other".into(), path: None, name: "x".into(), saved_at: 1, features: 0 }; + fs::write(d.join("part.json"), serde_json::to_vec(&other).unwrap()).unwrap(); + assert!(discard(&mut s, "part").is_err()); + assert!(discard(&mut s, "../part").is_err()); + assert!(part.exists() && d.join("part.json").exists(), "nothing deleted"); + // A real entry still goes. + let mine = Entry { id: "part".into(), ..other }; + fs::write(d.join("part.json"), serde_json::to_vec(&mine).unwrap()).unwrap(); + assert_eq!(discard(&mut s, "part").unwrap()["discarded"], json!(["part"])); + assert!(!part.exists()); + let _ = fs::remove_dir_all(&d); + } + + /// `doc.recover {discard: true}` opens a real entry and then deletes it; given a folder and + /// id that only name someone's design, it deletes nothing (#205 review). + #[test] + fn recover_with_discard_deletes_only_the_entry_it_opened() { + let d = dir("recover-discard"); + let mut s = Session::default(); + let mut a = Autosaver::new(&d, Duration::from_secs(3600)).unwrap(); + s.execute("solid.box", &json!({"length": 10, "width": 10, "height": 10})).unwrap(); + assert!(a.save(&s).unwrap()); + let id = a.id.clone(); + drop(a); + let mut t = Session::default(); + let r = t.execute("doc.recover", &json!({"dir": d.to_string_lossy(), "id": id, "discard": true})).unwrap(); + assert_eq!(r["features"], 1, "{r}"); + assert!(orphans(&d).is_empty() && !file(&d, &id, "json").exists() && !file(&d, &id, "solidcraft").exists()); + let part = d.join("part.solidcraft"); + fs::write(&part, solidcraft_io::write_design(&Document::default())).unwrap(); + assert!(t.execute("doc.recover", &json!({"dir": d.to_string_lossy(), "id": "part", "discard": true})).is_err()); + assert!(part.exists()); + let _ = fs::remove_dir_all(&d); + } + #[test] fn saving_drops_the_entry_and_a_clean_close_leaves_nothing() { let d = dir("clean"); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 9e0d1a83..5021fd8b 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -198,6 +198,7 @@ fn join_meeting_nothing_keeps_the_first_body() { mod boolean_diagnostics; mod component_frames; mod mirror_planes; +mod path_scope; mod primitive_points; mod robustness; mod robustness_goldens; diff --git a/crates/engine/src/tests/path_scope.rs b/crates/engine/src/tests/path_scope.rs new file mode 100644 index 00000000..6b99c534 --- /dev/null +++ b/crates/engine/src/tests/path_scope.rs @@ -0,0 +1,66 @@ +//! Commands run for an agent (#205): with a `vfs::PathScope` active, every command that reads, +//! writes or deletes a file it was given a path for stays inside the scope's folders. + +use serde_json::json; +use solidcraft_io::vfs::{PathScope, scoped}; + +use crate::*; + +#[test] +fn file_commands_stay_inside_the_agents_folders() { + let base = std::env::temp_dir().join(format!("solidcraft-path-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 at = |d: &std::path::Path, f: &str| d.join(f).to_string_lossy().to_string(); + let mut s = Session::default(); + s.execute("solid.box", &json!({"length": 10, "width": 10, "height": 10})).unwrap(); + std::fs::write(out.join("part.solidcraft"), solidcraft_io::write_design(&s.doc)).unwrap(); + std::fs::write(out.join("p.csv"), "name,expression\nw,5 mm\n").unwrap(); + + scoped(Some(PathScope::new([root.clone()])), || { + // Inside: the usual workflow. + s.execute("file.save_as", &json!({"path": at(&root, "part.solidcraft")})).unwrap(); + s.execute("file.export", &json!({"path": at(&root, "part.step")})).unwrap(); + s.execute("doc.open", &json!({"path": at(&root, "part.solidcraft")})).unwrap(); + // Outside, directly or through `..`: refused, with the reason. + for (cmd, p) in [ + ("file.save_as", json!({"path": at(&out, "x.solidcraft")})), + ("file.save", json!({"path": at(&root, "../out/x.solidcraft")})), + ("file.export", json!({"path": at(&out, "x.stl")})), + ("file.export", json!({"path": at(&out, "x.json"), "format": "json"})), + ("file.save_as", json!({"path": at(&out, "x.json"), "format": "json", "preview": true})), + ("parameters.export", json!({"path": at(&out, "x.csv")})), + ("doc.open", json!({"path": at(&out, "part.solidcraft")})), + ("parameters.import", json!({"path": at(&out, "p.csv")})), + ("sketch.insert_dxf", json!({"path": at(&out, "a.dxf")})), + ("doc.recovery_list", json!({"dir": out.to_string_lossy()})), + ("doc.recovery_discard", json!({"dir": out.to_string_lossy(), "id": "part"})), + ("doc.recover", json!({"dir": out.to_string_lossy(), "id": "part", "discard": true})), + ] { + let e = s.execute(cmd, &p).unwrap_err().to_string(); + assert!(e.contains("outside the folders this agent may use"), "{cmd}: {e}"); + } + // A font file named by path is a file read too; the built-in font still works. + s.execute("sketch.create", &json!({"plane": "XY"})).unwrap(); + let e = s.execute("sketch.text", &json!({"text": "A", "at": [0, 0], "font": at(&out, "f.ttf")})).unwrap_err().to_string(); + assert!(e.contains("outside the folders this agent may use"), "sketch.text: {e}"); + s.execute("sketch.text", &json!({"text": "A", "at": [0, 0]})).unwrap(); + s.execute("sketch.finish", &json!({})).unwrap(); + // Saving follows a link to its file (#845), so a link that leads out is refused. + #[cfg(unix)] + { + std::os::unix::fs::symlink(out.join("part.solidcraft"), root.join("linked.solidcraft")).unwrap(); + let e = s.execute("file.save_as", &json!({"path": at(&root, "linked.solidcraft")})).unwrap_err().to_string(); + assert!(e.contains("outside the folders this agent may use"), "save through a link: {e}"); + } + // The default recovery folder isn't one of the agent's folders either. + assert!(s.execute("doc.recovery_list", &json!({})).is_err()); + }); + let left: Vec<_> = std::fs::read_dir(&out).unwrap().flatten().map(|e| e.file_name()).collect(); + assert_eq!(left.len(), 2, "nothing written or deleted outside: {left:?}"); + // Without a scope (the user's own dialogs) nothing changes. + s.execute("file.export", &json!({"path": at(&out, "x.stl")})).unwrap(); + let _ = std::fs::remove_dir_all(&base); +} diff --git a/crates/io/src/design.rs b/crates/io/src/design.rs index a0878523..5436e022 100644 --- a/crates/io/src/design.rs +++ b/crates/io/src/design.rs @@ -509,6 +509,7 @@ pub(super) fn read(bytes: &[u8]) -> Result { pub fn read_design_file(path: &str, notify: Option) -> Result { #[cfg(not(target_arch = "wasm32"))] let mut source = { + crate::vfs::check(path).map_err(invalid)?; let metadata = std::fs::metadata(path).map_err(invalid)?; if !metadata.is_file() { return Err(invalid("not a regular file")); diff --git a/crates/io/src/design/thumbnail.rs b/crates/io/src/design/thumbnail.rs index 5f86e941..ff375e67 100644 --- a/crates/io/src/design/thumbnail.rs +++ b/crates/io/src/design/thumbnail.rs @@ -60,6 +60,7 @@ pub fn read_design_thumbnail(bytes: &[u8]) -> Option> { pub fn read_design_thumbnail_file(path: &str) -> Option> { #[cfg(not(target_arch = "wasm32"))] let source = { + crate::vfs::check(path).ok()?; let metadata = std::fs::metadata(path).ok()?; if !metadata.is_file() || metadata.len() > MAX_DESIGN_BYTES as u64 { return None; diff --git a/crates/io/src/safe_write.rs b/crates/io/src/safe_write.rs index 9260b4c0..dc294da3 100644 --- a/crates/io/src/safe_write.rs +++ b/crates/io/src/safe_write.rs @@ -117,7 +117,9 @@ fn write_steps(path: &Path, bytes: &[u8], backup: bool, stop: Option) -> s /// Write `bytes` to `path` atomically; with `backup`, the file being replaced is kept as /// `.bak` first. A symbolic link at `path` is followed (the file it points to is replaced, -/// the link stays), and the file keeps the permissions it had. +/// the link stays), and the file keeps the permissions it had. The app's own folders (autosave, +/// crash reports) write through this without a [`crate::vfs::PathScope`]; a command writing a +/// path it was given calls [`crate::vfs::check`] first, which judges where the link leads. pub fn write_atomic(path: &Path, bytes: &[u8], backup: bool) -> std::io::Result<()> { // In the browser the host stores files (one write is already atomic there). if cfg!(target_arch = "wasm32") { diff --git a/crates/io/src/vfs.rs b/crates/io/src/vfs.rs index 493d68dd..958fc70f 100644 --- a/crates/io/src/vfs.rs +++ b/crates/io/src/vfs.rs @@ -14,6 +14,7 @@ mod imp { /// blocks the caller instead. Refuse anything that is not a regular file, and never read /// more than the caller's cap plus the byte that proves it was exceeded. pub fn read(path: &str, max: usize) -> Result> { + check(path)?; // Cheap and non-blocking: refuse before opening, in case the open itself would block. let meta = std::fs::metadata(path)?; if !meta.is_file() { @@ -35,15 +36,18 @@ mod imp { Ok(out) } pub fn write(path: &str, bytes: &[u8]) -> Result<()> { + check(path)?; std::fs::write(path, bytes) } pub fn len(path: &str) -> Result { + check(path)?; std::fs::metadata(path).map(|m| m.len()) } pub fn exists(path: &str) -> bool { - std::path::Path::new(path).exists() + check(path).is_ok() && std::path::Path::new(path).exists() } pub fn remove(path: &str) -> Result<()> { + check(path)?; std::fs::remove_file(path) } } @@ -104,6 +108,249 @@ mod imp { pub use imp::*; +/// The folders an agent's commands may read, write and delete in (an MCP server's, see +/// docs/mcp.md). While a scope is active on this thread ([`scoped`]), every file call here +/// refuses paths outside it; without one (the user's own file dialogs, the CLI) nothing changes. +/// +/// Two kinds of folder: those named on purpose (`--allow-path`, the app's own recovery folder), +/// where everything is reachable, and the open design's folder, which counts only when +/// [`design_folder_refusal`] has nothing against it and never reaches hidden files or folders +/// (`.env`, `.git`) inside it. +#[derive(Clone, Debug, Default, PartialEq)] +pub struct PathScope { + roots: Vec, + design_roots: Vec, +} + +thread_local! { + static SCOPE: std::cell::RefCell> = const { std::cell::RefCell::new(None) }; +} + +/// `dir` resolved, if it exists. +fn canonical(dir: &std::path::Path) -> Option { + std::fs::canonicalize(dir).ok() +} + +/// The user's home folder, resolved. +fn home_dir() -> Option { + let var = if cfg!(windows) { "USERPROFILE" } else { "HOME" }; + std::env::var_os(var).filter(|h| !h.is_empty()).and_then(|h| canonical(std::path::Path::new(&h))) +} + +/// A folder or file name an agent never reaches by default: hidden on Unix (`.ssh`, `.env`), +/// and on Windows also the per-user `AppData` folder. +fn hidden(name: &std::ffi::OsStr) -> bool { + let n = name.to_string_lossy(); + n.starts_with('.') || (cfg!(windows) && n.eq_ignore_ascii_case("AppData")) +} + +/// Why `dir` (an open design's folder) can't be one of an agent's folders by default, or `None` +/// when it can: it must lie strictly inside the home folder (or the temporary folder) and not +/// in a hidden folder. So a design saved in home itself, in `/`, on a drive root or in a system +/// folder never puts `~/.ssh`, `~/.bashrc` or anything else of the user's in reach. +pub fn design_folder_refusal(dir: &std::path::Path) -> Option { + design_folder_refusal_in(dir, home_dir().as_deref(), Some(&std::env::temp_dir())) +} + +/// [`design_folder_refusal`] with the home and temporary folders given (tests). +pub fn design_folder_refusal_in(dir: &std::path::Path, home: Option<&std::path::Path>, temp: Option<&std::path::Path>) -> Option { + let Some(d) = canonical(dir) else { return Some("it doesn't exist".into()) }; + if d.parent().is_none() { + return Some("it is a drive or file-system root".into()); + } + let home = home.and_then(canonical); + if home.as_ref().is_some_and(|h| h.starts_with(&d)) { + return Some("it is your home folder or a folder above it".into()); + } + let temp = temp.and_then(canonical).filter(|t| t.parent().is_some()); + // The closer of the two (a temporary folder may lie inside home, in a hidden folder). + let Some(base) = [home, temp].into_iter().flatten().filter(|b| d.starts_with(b) && d != *b).max_by_key(|b| b.components().count()) else { + return Some("it is not inside your home folder".into()); + }; + let rel = d.strip_prefix(&base).ok()?; + if rel.components().any(|c| hidden(c.as_os_str())) { + return Some("it is inside a hidden folder".into()); + } + None +} + +impl PathScope { + /// Confine file access to `roots` (resolved now; folders that don't exist are left out, so an + /// empty scope refuses every path). + pub fn new(roots: impl IntoIterator) -> PathScope { + let mut out = PathScope::default(); + for r in roots.into_iter().filter_map(|r| canonical(&r)) { + if !out.roots.contains(&r) { + out.roots.push(r); + } + } + out + } + + /// Also the folder of the design at `path` (the open design), when + /// [`design_folder_refusal`] allows it; hidden entries inside it stay out of reach. + pub fn with_design(mut self, path: Option<&str>) -> PathScope { + let dir = path + .map(std::path::Path::new) + .and_then(std::path::Path::parent) + .map(|d| if d.as_os_str().is_empty() { std::path::Path::new(".") } else { d }); + if let Some(d) = dir.filter(|d| design_folder_refusal(d).is_none()).and_then(canonical) + && !self.design_roots.contains(&d) + { + self.design_roots.push(d); + } + self + } + + /// The folders named on purpose. + pub fn roots(&self) -> &[std::path::PathBuf] { + &self.roots + } + + /// The open design's folder, when it counts. + pub fn design_roots(&self) -> &[std::path::PathBuf] { + &self.design_roots + } + + /// Whether `path` lies inside one of the folders, after following symlinks and `..`. + pub fn check(&self, path: &str) -> Result<()> { + let refused = || { + let roots: Vec = self.roots.iter().chain(&self.design_roots).map(|r| r.display().to_string()).collect(); + Error::new( + ErrorKind::PermissionDenied, + format!( + "{path}: outside the folders this agent may use ({}); allow another folder with `--allow-path DIR`", + if roots.is_empty() { "none".into() } else { roots.join(", ") } + ), + ) + }; + let windows = |why: &str| Error::new(ErrorKind::PermissionDenied, format!("{path}: not allowed for this agent: {why}")); + // Judged from the text before any filesystem call: resolving a UNC path would already + // make Windows connect out, and a device name opens the device. + if cfg!(windows) + && let Some(why) = windows_refusal(path) + { + return Err(windows(why)); + } + let resolved = resolve(std::path::Path::new(path)).map_err(|_| refused())?; + // Judged again once resolved: a link or junction inside an allowed folder may lead to a + // network share or a device even though the path given looked local. + if cfg!(windows) && !local_drive_path(&resolved) { + return Err(windows("only paths on a local drive")); + } + let inside_design = + || self.design_roots.iter().any(|r| resolved.strip_prefix(r).is_ok_and(|rel| !rel.components().any(|c| hidden(c.as_os_str())))); + if self.roots.iter().any(|r| resolved.starts_with(r)) || inside_design() { Ok(()) } else { Err(refused()) } + } +} + +/// Whether a resolved path is on a local drive (`C:\…` or `\\?\C:\…`) with no `:` or device +/// name in it. Always true off Windows, where these rules don't apply. +fn local_drive_path(p: &std::path::Path) -> bool { + #[cfg(windows)] + { + let drive = matches!( + p.components().next(), + Some(std::path::Component::Prefix(x)) if matches!(x.kind(), std::path::Prefix::Disk(_) | std::path::Prefix::VerbatimDisk(_)) + ); + drive && windows_refusal(&p.to_string_lossy()).is_none() + } + #[cfg(not(windows))] + { + let _ = p; + true + } +} + +/// Why Windows would not treat `path` as a plain file on a local drive, judged by Windows rules +/// from the text alone (so it is tested on every platform): UNC, device (`\\.\`), NT (`\??\`) and +/// verbatim paths other than a verbatim drive (`\\?\C:\`), drive-relative paths (`C:file`), a `:` +/// inside a name (alternate data streams), reserved device names (`NUL`, `COM1.txt`, …) and +/// short (8.3) names. +pub fn windows_refusal(path: &str) -> Option<&'static str> { + let norm = path.replace('/', "\\"); + let drive = |s: &str| matches!(s.as_bytes(), [d, b':', ..] if d.is_ascii_alphabetic()); + let drive_abs = |s: &str| matches!(s.as_bytes(), [d, b':', b'\\', ..] if d.is_ascii_alphabetic()); + let body = if let Some(v) = norm.strip_prefix("\\\\?\\") { + if !drive_abs(v) { + return Some("only paths on a local drive (no UNC, device or volume paths)"); + } + v.get(2..).unwrap_or("") + } else if norm.starts_with("\\\\") || norm.starts_with("\\??\\") { + return Some("network (UNC), device and NT namespace paths"); + } else if drive(&norm) { + if !drive_abs(&norm) { + return Some("drive-relative paths (`C:file`)"); + } + norm.get(2..).unwrap_or("") + } else { + &norm + }; + for name in body.split('\\').filter(|n| !n.is_empty()) { + if name.contains(':') { + return Some("`:` in a file name (alternate data streams)"); + } + // A short (8.3) name such as `PROGRA~1` or `SSH~1` is another spelling of a folder whose + // real name could be anything; canonicalising expands existing ones, and a new file's + // name must not be one. + if name.as_bytes().windows(2).any(|w| w[0] == b'~' && w[1].is_ascii_digit()) { + return Some("short (8.3) names such as `PROGRA~1`"); + } + let stem = name.split('.').next().unwrap_or("").trim_end_matches([' ', '.']).to_uppercase(); + let numbered = + |p: &str| stem.strip_prefix(p).is_some_and(|n| matches!(n, "0" | "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9" | "¹" | "²" | "³")); + if matches!(stem.as_str(), "CON" | "PRN" | "AUX" | "NUL" | "CONIN$" | "CONOUT$") || numbered("COM") || numbered("LPT") { + return Some("reserved device names (CON, NUL, COM1, …)"); + } + } + None +} + +/// `path` with symlinks and `..` resolved. A file that doesn't exist yet resolves through its +/// folder, which must exist; a symlink whose target is missing is refused (writing through it +/// would land wherever it points). +fn resolve(path: &std::path::Path) -> Result { + match std::fs::canonicalize(path) { + Ok(p) => Ok(p), + Err(e) if e.kind() == ErrorKind::NotFound && std::fs::symlink_metadata(path).is_err() => { + let (Some(parent), Some(name)) = (path.parent(), path.file_name()) else { return Err(e) }; + let parent = if parent.as_os_str().is_empty() { std::path::Path::new(".") } else { parent }; + Ok(std::fs::canonicalize(parent)?.join(name)) + } + Err(e) => Err(e), + } +} + +/// Run `f` with file access confined to `scope` on this thread (`None`: unconfined). +pub fn scoped(scope: Option, f: impl FnOnce() -> T) -> T { + struct Restore(Option); + impl Drop for Restore { + fn drop(&mut self) { + let prev = self.0.take(); + SCOPE.with(|s| *s.borrow_mut() = prev); + } + } + let _restore = Restore(SCOPE.with(|s| s.replace(scope))); + f() +} + +/// The scope active on this thread. +pub fn current_scope() -> Option { + SCOPE.with(|s| s.borrow().clone()) +} + +/// Refuse `path` if a scope is active and it lies outside. Code that reaches the disk without +/// going through this module calls it first. In the browser files are the host's, never the +/// disk, so there is nothing to confine. +// ponytail: checked then opened, so a local process racing a symlink swap in an allowed folder +// could still redirect one call; opening by handle (openat/O_NOFOLLOW) would close that. +pub fn check(path: &str) -> Result<()> { + if cfg!(target_arch = "wasm32") { + return Ok(()); + } + SCOPE.with(|s| s.borrow().as_ref().map_or(Ok(()), |scope| scope.check(path))) +} + /// A file's text, at most `max` bytes. pub fn read_to_string(path: &str, max: usize) -> Result { String::from_utf8(read(path, max)?).map_err(|e| Error::new(ErrorKind::InvalidData, e)) @@ -156,6 +403,251 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// An agent's scope (#205): inside its folders files work as before; outside, through `..` + /// or through a symlink that leads out, every call is refused with a clear error and the + /// disk is left alone. Without a scope nothing is confined. + #[cfg(all(unix, not(target_arch = "wasm32")))] + #[test] + fn a_scope_confines_every_call_to_its_folders() { + use super::{PathScope, scoped}; + let base = std::env::temp_dir().join(format!("solidcraft-vfs-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.join("sub")).unwrap(); + std::fs::create_dir_all(&out).unwrap(); + std::fs::write(out.join("secret.txt"), "secret").unwrap(); + std::os::unix::fs::symlink(&out, root.join("link")).unwrap(); + std::os::unix::fs::symlink(out.join("missing.txt"), root.join("dangling")).unwrap(); + let s = |p: std::path::PathBuf| p.to_string_lossy().to_string(); + let scope = PathScope::new([root.clone()]); + scoped(Some(scope.clone()), || { + let inside = s(root.join("sub/a.txt")); + super::write(&inside, b"ok").unwrap(); + assert_eq!(super::read(&inside, 16).unwrap(), b"ok"); + assert_eq!(super::len(&inside).unwrap(), 2); + assert!(super::exists(&inside)); + super::remove(&inside).unwrap(); + for escape in [out.join("secret.txt"), root.join("../out/secret.txt"), root.join("link/secret.txt"), root.join("dangling")] { + let p = s(escape); + let e = super::read(&p, 16).unwrap_err(); + assert_eq!(e.kind(), std::io::ErrorKind::PermissionDenied, "{p}: {e}"); + assert!(e.to_string().contains("outside the folders this agent may use"), "{e}"); + assert!(super::write(&p, b"x").is_err() && super::remove(&p).is_err() && super::len(&p).is_err() && !super::exists(&p), "{p}"); + } + assert!(super::write(&s(out.join("new.txt")), b"x").is_err()); + assert!(super::check(&s(root.join("sub/../sub/b.txt"))).is_ok(), "`..` that stays inside is fine"); + }); + assert_eq!(std::fs::read_to_string(out.join("secret.txt")).unwrap(), "secret"); + assert!(!out.join("new.txt").exists() && !out.join("missing.txt").exists()); + // The scope ends with the call; an empty one refuses everything. + assert!(super::current_scope().is_none()); + assert_eq!(super::read(&s(out.join("secret.txt")), 16).unwrap(), b"secret"); + assert!(PathScope::new([base.join("nonexistent")]).check(&s(root.join("sub/a.txt"))).is_err()); + let _ = std::fs::remove_dir_all(&base); + } + + /// Review of #205: a root is a folder, not a string prefix (`a` doesn't admit `ab`); a new + /// file needs an existing folder, and one whose folder is a symlink that leads out is refused. + #[cfg(all(unix, not(target_arch = "wasm32")))] + #[test] + fn roots_are_folders_and_new_files_resolve_through_their_folder() { + use super::PathScope; + let base = std::env::temp_dir().join(format!("solidcraft-vfs-prefix-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let (a, ab, out) = (base.join("a"), base.join("ab"), base.join("out")); + for d in [&a, &ab, &out] { + std::fs::create_dir_all(d).unwrap(); + } + std::os::unix::fs::symlink(&out, a.join("link")).unwrap(); + let scope = PathScope::new([a.clone()]); + let s = |p: std::path::PathBuf| p.to_string_lossy().to_string(); + assert!(scope.check(&s(a.join("new.txt"))).is_ok()); + for refused in [ab.join("x.txt"), a.join("missing/x.txt"), a.join("link/new.txt")] { + let e = scope.check(&s(refused.clone())).unwrap_err(); + assert!(e.to_string().contains("outside the folders this agent may use"), "{}: {e}", refused.display()); + } + super::scoped(Some(scope), || assert!(super::write(&s(a.join("link/new.txt")), b"x").is_err())); + assert!(!out.join("new.txt").exists() && !a.join("missing").exists()); + let _ = std::fs::remove_dir_all(&base); + } + + /// Windows rules, judged from the text alone, on every platform (#205 review). + #[test] + fn windows_paths_that_are_not_plain_local_files_are_refused() { + use super::windows_refusal as w; + for ok in [ + r"C:\work\part.step", + "C:/work/part.step", + r"\\?\C:\work\part.step", + r"work\part.step", + "part.step", + r"\work\a.b.step", + r"C:\com10\x.txt", + r"C:\console.txt", + r"\\?\c:\work\part.step", + r"C:\work\draft~v2.step", + ] { + assert_eq!(w(ok), None, "{ok}"); + } + for bad in [ + r"\\server\share\x.step", + "//server/share/x.step", + r"\\?\UNC\server\share\x.step", + r"\??\UNC\server\share\x.step", + r"\??\C:\x.step", + "/??/UNC/server/share/x.step", + r"\\.\COM1", + r"\\?\Volume{0}\x", + r"\\?\GLOBALROOT\Device\x", + "C:foo.step", + r"C:\work\a.step:stream", + r"C:\work\NUL", + r"C:\work\nul.txt", + r"C:\work\COM1.step", + r"C:\work\lpt9", + r"C:\work\Con .txt", + r"C:\work\COM¹", + r"C:\work\CONOUT$", + r"work\AUX\x.step", + // Every spelling of the NT, device and UNC prefixes. + r"\\?\unc\server\share\x.step", + "//?/UNC/server/share/x.step", + r"/\?\UNC\server\share\x.step", + r"\??/UNC/server/share/x.step", + r"\\.\C:\x.step", + "//./pipe/x", + r"\\?\C:", + r"\\server", + // Device names with any extension, in any case. + r"C:\work\aux.tar.gz", + r"C:\work\Prn.", + r"C:\work\lPt1.step", + r"C:\work\conin$.txt", + // Alternate data streams, and short (8.3) names. + r"C:\work\a.step::$DATA", + r"C:\Users\me\SSH~1\id_ed25519", + r"C:\PROGRA~1\x.step", + ] { + assert!(w(bad).is_some(), "{bad}"); + } + } + + /// The same, through a scope on Windows itself: refused before any filesystem call. + #[cfg(windows)] + #[test] + fn windows_unc_verbatim_drive_relative_and_device_paths_are_refused() { + let root = std::env::temp_dir().join(format!("solidcraft-vfs-win-{}", std::process::id())); + std::fs::create_dir_all(&root).unwrap(); + let scope = super::PathScope::new([root.clone()]); + let inside = |f: &str| root.join(f).to_string_lossy().to_string(); + assert!(scope.check(&inside("ok.step")).is_ok()); + for bad in [ + r"\\server\share\x.step".to_string(), + r"\\?\UNC\server\share\x.step".to_string(), + r"\??\UNC\server\share\x.step".to_string(), + r"\\.\COM1".to_string(), + "C:foo.step".to_string(), + inside("NUL"), + inside("COM1.txt"), + inside("con"), + inside("a.step:stream"), + ] { + let e = scope.check(&bad).unwrap_err(); + assert_eq!(e.kind(), std::io::ErrorKind::PermissionDenied, "{bad}: {e}"); + } + let _ = std::fs::remove_dir_all(&root); + } + + /// The open design's folder counts only when it is safe (#205): strictly inside home (or the + /// temporary folder), not home itself, a folder above it, a root or a hidden folder; and + /// hidden files inside it stay out of reach. Folders named on purpose reach everything. + #[cfg(all(unix, not(target_arch = "wasm32")))] + #[test] + fn the_designs_folder_counts_only_when_safe_and_never_reaches_hidden_files() { + use super::{PathScope, design_folder_refusal_in as refusal}; + let base = std::env::temp_dir().join(format!("solidcraft-vfs-design-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let home = base.join("home"); + let (proj, dotdir) = (home.join("work/proj"), home.join(".config/x")); + for d in [&proj.join("sub"), &dotdir, &home.join(".ssh"), &proj.join(".git")] { + std::fs::create_dir_all(d).unwrap(); + } + let sibling = base.join("homex"); + std::fs::create_dir_all(&sibling).unwrap(); + std::fs::write(home.join(".ssh/id_ed25519"), "key").unwrap(); + std::fs::write(proj.join(".env"), "x").unwrap(); + std::os::unix::fs::symlink(home.join(".ssh"), proj.join("keys")).unwrap(); + let h = Some(home.as_path()); + assert_eq!(refusal(&proj, h, None), None); + assert_eq!(refusal(&home.join("work/../work/proj"), h, None), None); + for (dir, why) in [ + (home.clone(), "home"), + (base.clone(), "above home"), + ("/".into(), "root"), + (sibling.clone(), "a name that only starts like home"), + (dotdir.clone(), "hidden"), + (base.join("missing"), "doesn't exist"), + ] { + assert!(refusal(&dir, h, None).is_some(), "{why}: {}", dir.display()); + } + assert!(refusal(&proj, None, None).is_some(), "no home known: nothing implicit"); + assert_eq!(refusal(&base.join("homex"), None, Some(&base)), None, "inside the temporary folder"); + assert!(refusal(&base, None, Some(&base)).is_some(), "the temporary folder itself"); + + // Through a scope: the design's folder reaches its own files, never hidden ones, and + // never a link that leads to them. + let s = |p: std::path::PathBuf| p.to_string_lossy().to_string(); + let mut scope = PathScope::new([]); + // `with_design` judges with the real home; build the folder list directly instead. + scope.design_roots.push(std::fs::canonicalize(&proj).unwrap()); + assert!(scope.check(&s(proj.join("part.step"))).is_ok()); + assert!(scope.check(&s(proj.join("sub/../part.step"))).is_ok()); + for bad in + [proj.join(".env"), proj.join(".git/config"), proj.join("keys/id_ed25519"), proj.join("../../.ssh/id_ed25519"), home.join(".bashrc")] + { + assert!(scope.check(&s(bad.clone())).is_err(), "{}", bad.display()); + } + // Named on purpose, a folder reaches its hidden files too. + assert!(PathScope::new([proj.clone()]).check(&s(proj.join(".env"))).is_ok()); + // A design saved at `/` or in a missing folder adds nothing. + assert!(PathScope::new([]).with_design(Some("/part.solidcraft")).design_roots().is_empty()); + assert!(PathScope::new([]).with_design(Some(&s(base.join("missing/p.solidcraft")))).design_roots().is_empty()); + let _ = std::fs::remove_dir_all(&base); + } + + /// A save follows a symbolic link to the file it points to (#845), so the link's target is + /// what a scope judges: a link inside an allowed folder that leads out is refused, a chain of + /// links that stays inside is fine. + #[cfg(all(unix, not(target_arch = "wasm32")))] + #[test] + fn a_link_is_judged_by_where_it_leads() { + use super::PathScope; + let base = std::env::temp_dir().join(format!("solidcraft-vfs-link-{}", 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(); + std::fs::write(out.join("part.solidcraft"), "outside").unwrap(); + std::fs::write(root.join("real.solidcraft"), "inside").unwrap(); + std::os::unix::fs::symlink(out.join("part.solidcraft"), root.join("part.solidcraft")).unwrap(); + std::os::unix::fs::symlink("real.solidcraft", root.join("a.solidcraft")).unwrap(); + std::os::unix::fs::symlink("a.solidcraft", root.join("b.solidcraft")).unwrap(); + std::os::unix::fs::symlink("../out/new.solidcraft", root.join("dangling.solidcraft")).unwrap(); + let scope = PathScope::new([root.clone()]); + let s = |p: std::path::PathBuf| p.to_string_lossy().to_string(); + assert!(scope.check(&s(root.join("part.solidcraft"))).is_err()); + assert!(scope.check(&s(root.join("dangling.solidcraft"))).is_err()); + assert!(scope.check(&s(root.join("b.solidcraft"))).is_ok()); + super::scoped(Some(scope), || { + let path = s(root.join("part.solidcraft")); + let refused = super::check(&path).and_then(|()| crate::safe_write::write_atomic(std::path::Path::new(&path), b"agent", false)); + assert!(refused.is_err()); + }); + assert_eq!(std::fs::read_to_string(out.join("part.solidcraft")).unwrap(), "outside"); + assert!(!out.join("new.solidcraft").exists()); + let _ = std::fs::remove_dir_all(&base); + } + /// A directory is not a file either. #[cfg(not(target_arch = "wasm32"))] #[test] diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs index acb245e6..aa3559d8 100644 --- a/crates/mcp/src/backend.rs +++ b/crates/mcp/src/backend.rs @@ -15,6 +15,11 @@ pub trait Backend { /// True when a real window is attached (`ui.screenshot` can capture it). fn has_ui(&self) -> bool; fn describe(&self) -> String; + /// The open design's file, when this side knows it (its folder joins the agent's folders, + /// see `vfs::PathScope::with_design`). A running app adds its own, so `Remote` says nothing. + fn design_path(&self) -> Option { + None + } } /// Longest reply line accepted from the app. @@ -85,6 +90,11 @@ impl Backend for Remote { fn call(&mut self, method: &str, params: Value) -> Result { let id = self.next_id; self.next_id += 1; + let mut params = params; + // The app confines the files this request reaches to the agent's folders (control.rs). + if let (Some(scope), Some(o)) = (solidcraft_engine::io::vfs::current_scope(), params.as_object_mut()) { + o.insert("allow_paths".into(), json!(scope.roots().iter().map(|r| r.to_string_lossy()).collect::>())); + } let line = json!({"id": id, "method": method, "params": params}).to_string(); let reply = match self.roundtrip(&line) { Ok(r) => r, @@ -170,7 +180,7 @@ impl Backend for Headless { let scene = solidcraft_engine::view::scene(s, &cam); let png = render_png(&scene, &cam, w, h).ok_or("render failed")?; let path = str_p("path").ok_or("missing `path`")?; - std::fs::write(path, &png).map_err(|e| format!("{path}: {e}"))?; + solidcraft_engine::io::vfs::write(path, &png).map_err(|e| format!("{path}: {e}"))?; Ok(json!({"path": path, "bytes": png.len(), "width": w, "height": h})) } "ui.screenshot" => Err("no window in headless mode; use ui.render".into()), @@ -183,4 +193,7 @@ impl Backend for Headless { fn describe(&self) -> String { "headless in-process SolidCraft session".into() } + fn design_path(&self) -> Option { + self.session.path.clone() + } } diff --git a/crates/mcp/src/lib.rs b/crates/mcp/src/lib.rs index ceebc49f..9957a649 100644 --- a/crates/mcp/src/lib.rs +++ b/crates/mcp/src/lib.rs @@ -17,5 +17,52 @@ pub use backend::{Backend, Headless, Remote}; pub use server::{PROTOCOL_VERSION, SUPPORTED_VERSIONS, Server}; pub use tools::{call_tool, tool_definitions}; +use std::path::PathBuf; + +use solidcraft_engine::io::vfs::PathScope; + +/// The folders `solidcraft-cli mcp` lets its agent read, write and delete in by default: the +/// app's own recovery folder, this server's private scratch folder (screenshots), and each +/// `allowed` folder (`--allow-path`). The open design's folder joins on every call when it is a +/// safe one (`vfs::design_folder_refusal`; the server adds it). The working folder, the home +/// folder and anything above it never count by themselves, so `~/.ssh`, `~/.bashrc` and other +/// dotfiles are out of reach unless a folder holding them is allowed on purpose. +pub fn agent_scope(allowed: impl IntoIterator) -> PathScope { + agent_scope_with(solidcraft_engine::recovery::default_dir(), allowed) +} + +/// [`agent_scope`] with the recovery folder given (tests). +pub fn agent_scope_with(recovery: Option, allowed: impl IntoIterator) -> PathScope { + let mut roots: Vec = Vec::new(); + if let Some(d) = recovery + && std::fs::create_dir_all(&d).is_ok() + { + roots.push(d); + } + if let Some(d) = scratch_dir() { + roots.push(d); + } + roots.extend(allowed); + PathScope::new(roots) +} + +/// This server's own folder for screenshots it reads back and deletes (`screenshot` without a +/// `path`): a new `/solidcraft-mcp--` made on first use (never one that was there +/// already, which could be someone else's), private to the user on Unix. +pub fn scratch_dir() -> Option { + static DIR: std::sync::OnceLock> = std::sync::OnceLock::new(); + DIR.get_or_init(|| { + let mut b = std::fs::DirBuilder::new(); + #[cfg(unix)] + std::os::unix::fs::DirBuilderExt::mode(&mut b, 0o700); + let seed = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).map(|d| d.subsec_nanos()).unwrap_or(0); + (0..16u32).find_map(|i| { + let d = std::env::temp_dir().join(format!("solidcraft-mcp-{}-{:x}", std::process::id(), seed.wrapping_add(i))); + b.create(&d).ok().map(|()| d) + }) + }) + .clone() +} + #[cfg(test)] mod tests; diff --git a/crates/mcp/src/server.rs b/crates/mcp/src/server.rs index 3d205457..769587a5 100644 --- a/crates/mcp/src/server.rs +++ b/crates/mcp/src/server.rs @@ -6,6 +6,8 @@ use serde_json::{Value, json}; /// Longest accepted request line, shared with the control server. use solidcraft_engine::MAX_LINE; +use solidcraft_engine::io::vfs::{self, PathScope}; + use crate::backend::Backend; use crate::tools::{call_tool, check_arguments, tool_definitions}; @@ -35,6 +37,7 @@ edge], radius}`, `solid.chamfer`, patterns, mirror, primitives). Use `batch` for pub struct Server { backend: Box, initialized: bool, + scope: Option, } fn response(id: Value, result: Value) -> Value { @@ -46,7 +49,13 @@ fn error(id: Value, code: i64, message: impl Into) -> Value { impl Server { pub fn new(backend: Box) -> Self { - Self { backend, initialized: false } + Self { backend, initialized: false, scope: None } + } + /// Confine the files tool calls may reach to `scope` (`None`: anywhere). `solidcraft-cli mcp` + /// always sets one unless started with `--allow-any-path` (docs/mcp.md). + pub fn with_path_scope(mut self, scope: Option) -> Self { + self.scope = scope; + self } pub fn backend(&mut self) -> &mut dyn Backend { self.backend.as_mut() @@ -146,7 +155,10 @@ impl Server { Some(a) => a.clone(), }; check_arguments(name, &args).map_err(|e| (INVALID_PARAMS, e))?; - Ok(call_tool(self.backend.as_mut(), name, &args)) + // The open design's folder counts as it is now (a save-as or open moves it). + let scope = self.scope.clone().map(|s| s.with_design(self.backend.design_path().as_deref())); + let backend = self.backend.as_mut(); + Ok(vfs::scoped(scope, || call_tool(backend, name, &args))) } "resources/list" => Ok(json!({"resources": [ {"uri": "solidcraft://design", "name": "Active design", "description": "document.inspect with measurements", "mimeType": "application/json"}, diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index e6913fa9..11d9217d 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -439,6 +439,121 @@ fn legacy_design_files_and_resource_uris() { assert!(d["result"]["contents"][0]["text"].as_str().unwrap().contains("timeline")); } +/// The default folders (#205): the recovery folder works out of the box, `screenshot` without a +/// `path` uses the server's own scratch folder, and the open design's folder counts while the +/// design is open there, without its hidden files. The working folder doesn't count at all. +#[test] +fn the_default_folders_are_the_designs_the_recovery_folder_and_the_scratch_folder() { + let base = std::env::temp_dir().join(format!("solidcraft-mcp-defaults-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let (proj, other, recovery) = (base.join("proj"), base.join("other"), base.join("recovery")); + std::fs::create_dir_all(&proj).unwrap(); + std::fs::create_dir_all(&other).unwrap(); + std::fs::write(proj.join(".env"), "TOKEN=secret").unwrap(); + let at = |d: &std::path::Path, f: &str| d.join(f).to_string_lossy().to_string(); + let scope = crate::agent_scope_with(Some(recovery.clone()), []); + let cwd = std::fs::canonicalize(std::env::current_dir().unwrap()).unwrap(); + assert!(!scope.roots().contains(&cwd), "the working folder is not a default folder: {scope:?}"); + assert!(scope.roots().contains(&std::fs::canonicalize(&recovery).unwrap()), "{scope:?}"); + assert!(scope.roots().contains(&crate::scratch_dir().unwrap()), "{scope:?}"); + + let mut session = solidcraft_engine::Session::default(); + session.execute("solid.box", &json!({"length": 10, "width": 10, "height": 10})).unwrap(); + session.execute("file.save_as", &json!({"path": at(&proj, "part.solidcraft")})).unwrap(); + let mut s = Server::new(Box::new(Headless::new(session))).with_path_scope(Some(scope)); + // Legitimate work in the design's folder: export, save, open, screenshots, the recovery folder. + tool(&mut s, "export", json!({"path": at(&proj, "part.step")})); + tool(&mut s, "execute", json!({"command": "parameters.export", "params": {"path": at(&proj, "params.csv")}})); + tool(&mut s, "save", json!({"path": at(&proj, "copy.solidcraft")})); + tool(&mut s, "open", json!({"path": at(&proj, "part.solidcraft")})); + tool(&mut s, "screenshot", json!({"width": 32, "height": 32})); + tool(&mut s, "execute", json!({"command": "doc.recovery_list", "params": {"dir": recovery.to_string_lossy()}})); + assert!(proj.join("part.step").exists() && proj.join("copy.solidcraft").exists()); + // Hidden files in it, and folders beside it, stay out of reach. + for (name, args) in [ + ("export", json!({"path": at(&proj, ".env")})), + ("execute", json!({"command": "parameters.import", "params": {"path": at(&proj, ".env")}})), + ("export", json!({"path": at(&proj, ".git/x.step")})), + ("export", json!({"path": at(&other, "x.step")})), + ] { + let r = raw_tool(&mut s, name, args); + assert_eq!(r["result"]["isError"], json!(true), "{name}: {r}"); + } + assert_eq!(std::fs::read_to_string(proj.join(".env")).unwrap(), "TOKEN=secret"); + assert_eq!(std::fs::read_dir(&other).unwrap().count(), 0); + let _ = std::fs::remove_dir_all(&base); +} + +/// Tool calls reach files only inside the server's folders (#205): the usual workflow (save, +/// export, open, screenshots) works there; a path outside, directly, through `..` or through a +/// symlink that leads out, is refused with a clear error and nothing is written. +#[test] +fn tool_calls_stay_inside_the_agents_folders() { + use solidcraft_engine::io::vfs::PathScope; + let base = std::env::temp_dir().join(format!("solidcraft-mcp-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 at = |d: &std::path::Path, f: &str| d.join(f).to_string_lossy().to_string(); + let defaults = crate::agent_scope_with(None, [root.clone()]); + assert!(defaults.roots().contains(&std::fs::canonicalize(&root).unwrap()), "{defaults:?}"); + assert!(!defaults.roots().contains(&std::fs::canonicalize(std::env::temp_dir()).unwrap()), "{defaults:?}"); + // Only `root` here, to tell inside from outside within the temporary folder. + let mut s = Server::new(Box::new(Headless::default())).with_path_scope(Some(PathScope::new([root.clone()]))); + tool(&mut s, "execute", json!({"command": "solid.box", "params": {"length": 10, "width": 10, "height": 10}})); + tool(&mut s, "save", json!({"path": at(&root, "part.solidcraft")})); + tool(&mut s, "export", json!({"path": at(&root, "part.step")})); + tool(&mut s, "open", json!({"path": at(&root, "part.solidcraft")})); + tool(&mut s, "screenshot", json!({"path": at(&root, "shot.png"), "width": 64, "height": 64})); + let mut escapes = vec![at(&out, "x.step"), at(&root, "../out/x.step")]; + #[cfg(unix)] + { + std::os::unix::fs::symlink(&out, root.join("link")).unwrap(); + escapes.push(at(&root, "link/x.step")); + } + for p in escapes { + for (name, args) in [ + ("export", json!({"path": p})), + ("save", json!({"path": p})), + ("open", json!({"path": p})), + ("screenshot", json!({"path": p, "width": 64, "height": 64})), + ("execute", json!({"command": "file.save_as", "params": {"path": p}})), + ("batch", json!({"commands": [{"command": "file.export", "params": {"path": p}}]})), + ] { + let r = raw_tool(&mut s, name, args); + assert_eq!(r["result"]["isError"], json!(true), "{name} {p}: {r}"); + assert!(r.to_string().contains("outside the folders this agent may use"), "{name} {p}: {r}"); + } + } + assert_eq!(std::fs::read_dir(&out).unwrap().count(), 0, "nothing written outside"); + let _ = std::fs::remove_dir_all(&base); +} + +/// A bridged server sends the agent's folders with every request, so the app confines the +/// request's files the same way (`allow_paths`, crates/ui-egui/src/control.rs). +#[test] +fn a_bridged_request_carries_the_agents_folders() { + use std::io::{BufRead, BufReader, Write}; + + use solidcraft_engine::io::vfs::{PathScope, scoped}; + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + let echo = std::thread::spawn(move || { + let (stream, _) = listener.accept().unwrap(); + let mut line = String::new(); + BufReader::new(stream.try_clone().unwrap()).read_line(&mut line).unwrap(); + let req: Value = serde_json::from_str(&line).unwrap(); + writeln!(&stream, "{}", json!({"id": req["id"], "ok": true, "result": req["params"]})).unwrap(); + }); + let mut b = crate::Remote::connect(&addr).unwrap(); + let dir = std::fs::canonicalize(std::env::temp_dir()).unwrap(); + let got = scoped(Some(PathScope::new([dir.clone()])), || crate::Backend::call(&mut b, "engine.execute", json!({"command": "file.new"}))).unwrap(); + assert_eq!(got["allow_paths"], json!([dir.to_string_lossy()]), "{got}"); + assert_eq!(got["command"], "file.new"); + echo.join().unwrap(); +} + /// #528: an agent saves the angle of a product photo as a named view once, then renders the /// model from that view after each change. The render must use the saved camera, not a /// standard view. diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 1b054dff..498790ca 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -4,13 +4,14 @@ use std::sync::atomic::{AtomicU64, Ordering}; use serde_json::{Map, Value, json}; +use solidcraft_engine::io::vfs; use crate::backend::Backend; /// Most commands in one `batch` call. const MAX_BATCH: usize = 10_000; /// Largest image returned by `screenshot`. -const MAX_IMAGE_BYTES: u64 = 32 << 20; +const MAX_IMAGE_BYTES: usize = 32 << 20; const VIEWS: &[&str] = &["iso", "home", "front", "back", "top", "bottom", "left", "right", "fit"]; @@ -540,7 +541,8 @@ fn capture(b: &mut dyn Backend, a: &Map) -> Result<(Vec, Valu let keep = str_arg(a, "path").filter(|p| !p.trim().is_empty()).map(str::to_string); let path = keep.clone().unwrap_or_else(|| { let n = SHOT.fetch_add(1, Ordering::Relaxed); - std::env::temp_dir().join(format!("solidcraft-mcp-{}-{n}.png", std::process::id())).to_string_lossy().to_string() + let dir = crate::scratch_dir().unwrap_or_else(std::env::temp_dir); + dir.join(format!("shot-{n}.png")).to_string_lossy().to_string() }); let window = match str_arg(a, "source") { Some("model") => false, @@ -558,12 +560,9 @@ fn capture(b: &mut dyn Backend, a: &Map) -> Result<(Vec, Valu } else { (render(b)?, "model") }; - let read = std::fs::metadata(&path) - .map_err(|e| format!("{path}: {e}")) - .and_then(|m| if m.len() > MAX_IMAGE_BYTES { Err(format!("{path}: image too large")) } else { Ok(()) }) - .and_then(|()| std::fs::read(&path).map_err(|e| format!("{path}: {e} (is the app on another machine?)"))); + let read = vfs::read(&path, MAX_IMAGE_BYTES).map_err(|e| format!("{path}: {e} (is the app on another machine?)")); if keep.is_none() { - let _ = std::fs::remove_file(&path); + let _ = vfs::remove(&path); } let png = read?; let (w, h) = match source { diff --git a/crates/sketch/src/fonts.rs b/crates/sketch/src/fonts.rs index de53524e..beb2ae0a 100644 --- a/crates/sketch/src/fonts.rs +++ b/crates/sketch/src/fonts.rs @@ -259,6 +259,13 @@ 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 { + 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) } diff --git a/crates/sketch/src/lib.rs b/crates/sketch/src/lib.rs index f3a1ebf3..b6487d23 100644 --- a/crates/sketch/src/lib.rs +++ b/crates/sketch/src/lib.rs @@ -42,7 +42,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, font_data, installed_families}; +pub use fonts::{DEFAULT_FONT_FAMILY, FontData, families_in, font_data, font_file_path, installed_families}; 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::{Constraint, ConstraintKind, Curve, CurveKind, MAX_CONSTRAINTS, MAX_CURVES, MAX_POINTS, SPoint, Sketch, SketchError, SketchView}; diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index efaa1ec3..7d77fb02 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -37,6 +37,7 @@ use std::sync::mpsc::Sender; use serde_json::{Value, json}; +use solidcraft_engine::io::vfs; use solidcraft_engine::render::{StandardView, render_png}; use crate::SolveApp; @@ -86,11 +87,27 @@ fn key_from(name: &str) -> Option { } pub fn handle(app: &mut SolveApp, ctx: &egui::Context, req: &ControlRequest) -> Outcome { - let p = &req.params; + // 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) }; + let Some(roots) = allowed.as_array().and_then(|a| a.iter().map(|r| r.as_str().map(std::path::PathBuf::from)).collect::>>()) else { + return err("`allow_paths` must be a list of folders"); + }; + 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)) +} + +fn dispatch(app: &mut SolveApp, ctx: &egui::Context, method: &str, p: &Value) -> Outcome { let s = |k: &str| p.get(k).and_then(Value::as_str); let f = |k: &str| p.get(k).and_then(Value::as_f64).filter(|x| x.is_finite()); let b = |k: &str| p.get(k).and_then(Value::as_bool).unwrap_or(false); - match req.method.as_str() { + match method { "engine.execute" | "command" => { let Some(id) = s("command").or(s("id")) else { return err("missing `command`") }; let params = p.get("params").cloned().filter(|v| !v.is_null()).unwrap_or(json!({})); @@ -235,7 +252,7 @@ pub fn handle(app: &mut SolveApp, ctx: &egui::Context, req: &ControlRequest) -> let (Some(x), Some(y)) = (f("x"), f("y")) else { return err("missing x/y") }; let pos = egui::pos2(x as f32, y as f32); app.synthetic.push(egui::Event::PointerMoved(pos)); - if req.method == "ui.click" { + if method == "ui.click" { let button = match s("button") { Some("right") => egui::PointerButton::Secondary, Some("middle") => egui::PointerButton::Middle, @@ -534,7 +551,10 @@ pub fn handle(app: &mut SolveApp, ctx: &egui::Context, req: &ControlRequest) -> ctx.send_viewport_cmd(egui::ViewportCommand::InnerSize(egui::vec2(w.clamp(320.0, 8192.0) as f32, h.clamp(240.0, 8192.0) as f32))); ok(Value::Null) } - "ui.screenshot" => Outcome::Screenshot { path: s("path").map(str::to_string) }, + "ui.screenshot" => match s("path").map(vfs::check) { + Some(Err(e)) => err(e), + _ => Outcome::Screenshot { path: s("path").map(str::to_string) }, + }, "ui.render" => { let w = f("width").unwrap_or(1280.0).clamp(16.0, 8192.0) as usize; let h = f("height").unwrap_or(800.0).clamp(16.0, 8192.0) as usize; @@ -543,7 +563,7 @@ pub fn handle(app: &mut SolveApp, ctx: &egui::Context, req: &ControlRequest) -> let scene = solidcraft_engine::view::scene_with_section(&app.session, &app.cam, crate::viewport::section_plane(app)); let Some(png) = render_png(&scene, &app.cam, w, h) else { return err("render failed") }; match s("path") { - Some(path) => match std::fs::write(path, &png) { + Some(path) => match vfs::check(path).and_then(|()| std::fs::write(path, &png)) { Ok(()) => ok(json!({"path": path, "bytes": png.len()})), Err(e) => err(e), }, @@ -621,4 +641,47 @@ mod tests { assert!(render(&mut app, &dir.join("committed.png")) == preview, "the committed section renders differently"); let _ = std::fs::remove_dir_all(&dir); } + + /// A request carrying `allow_paths` (from an MCP server, #205) reaches files only inside + /// those folders; the same request without it, like the user's own dialogs, is unconfined. + #[test] + fn requests_with_allow_paths_stay_inside_them() { + let mut s = Session::default(); + s.execute("solid.box", &json!({"length": 10, "width": 10, "height": 10})).unwrap(); + let mut app = SolveApp::new(s, Services::default()); + let base = std::env::temp_dir().join(format!("solidcraft-control-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 at = |d: &std::path::Path, f: &str| d.join(f).to_string_lossy().to_string(); + let mut call = |method: &str, mut params: Value, scoped: bool| { + if scoped { + params["allow_paths"] = json!([root.to_string_lossy()]); + } + let (req, _rx) = ControlRequest::new(method, params); + match handle(&mut app, &egui::Context::default(), &req) { + Outcome::Done(v) => v, + Outcome::Screenshot { path } => json!({"ok": true, "screenshot": path}), + } + }; + let export = |p: String| json!({"command": "file.export", "params": {"path": p}}); + assert_eq!(call("engine.execute", export(at(&root, "a.step")), true)["ok"], true); + assert_eq!(call("ui.render", json!({"path": at(&root, "a.png"), "width": 32, "height": 32}), true)["ok"], true); + assert_eq!(call("ui.screenshot", json!({"path": at(&root, "b.png")}), true)["ok"], true); + for (method, params) in [ + ("engine.execute", export(at(&out, "a.step"))), + ("engine.execute", export(at(&root, "../out/a.step"))), + ("engine.script", json!({"commands": [{"command": "file.export", "params": {"path": at(&out, "a.step")}}]})), + ("ui.render", json!({"path": at(&out, "a.png"), "width": 32, "height": 32})), + ("ui.screenshot", json!({"path": at(&out, "b.png")})), + ] { + let v = call(method, params, true); + assert!(v["ok"] == false && v.to_string().contains("outside the folders this agent may use"), "{method}: {v}"); + } + assert_eq!(std::fs::read_dir(&out).unwrap().count(), 0, "nothing written outside"); + assert_eq!(call("engine.execute", export(at(&out, "a.step")), false)["ok"], true, "without allow_paths: unconfined"); + let bad = call("engine.execute", json!({"command": "file.new", "allow_paths": "/"}), false); + assert!(bad["error"].as_str().is_some_and(|e| e.contains("allow_paths")), "{bad}"); + let _ = std::fs::remove_dir_all(&base); + } } diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 970a1a1a..0df5ea0d 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -14,6 +14,15 @@ run per frame so a flood cannot starve drawing, and a request that the UI thread answered within 60 seconds is replied to with `{"ok": false, "error": "timeout"}`. The same 16 MiB line limit applies to the MCP server. +Any request's `params` may carry `allow_paths: ["/abs/folder", …]`: every file that request +reads, writes or deletes (command paths such as `file.export`'s `path` or `doc.recovery_*`'s +`dir`, `ui.render`, `ui.screenshot`) must then lie inside one of those folders after `..` and +symlinks are resolved, or the request fails with `… outside the folders this agent may use …`. +The app adds its own recovery folder and, when it lies safely inside the home folder and not in +a hidden folder, the open design's folder (whose hidden files stay out of reach). The bridged MCP server sends its folders this way on every request ([mcp.md](mcp.md)). Without +`allow_paths` file paths are not limited, and the channel itself has no authentication beyond +listening on loopback only. + ## Engine | Method | Params | Result | diff --git a/docs/mcp.md b/docs/mcp.md index c9ef50f0..4978d7f2 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -22,6 +22,51 @@ solidcraft-cli mcp --connect 127.0.0.1:7878 stdout carries only protocol messages; errors go to stderr. Bridged screenshots are written to a temporary file by the app and read back by the server, so both must run on the same machine. +### Which files the agent can reach + +Tool calls read, write and delete files only inside the server's folders: + +- the folder of the open design (the `--in` design, or whatever the agent last opened or saved), + but only when it lies strictly inside your home folder (or the temporary folder) and not in a + hidden folder. Home itself, a folder above it (`/`, `/home`, `/Users`, a drive's root), + hidden folders such as `~/.config` and folders elsewhere don't count. Hidden files and folders + inside the design's folder (`.env`, `.git`) are out of reach too; +- SolidCraft's own recovery folder, so `doc.recovery_list`, `doc.recover` and + `doc.recovery_discard` work out of the box (discarding deletes only real recovery entries); +- the server's own scratch folder (a new private `solidcraft-mcp-*` folder in the temporary + folder), where `screenshot` without a `path` goes; +- every `--allow-path DIR` (repeat it for more). A folder named here is reached in full, + hidden files included. + +The working folder the server was started in does not count by itself, so `~/.ssh`, `~/.aws`, +`~/.bashrc` and other dotfiles are never in reach by default. stderr lists the folders at +start-up and says when the `--in` design's folder was left out; `--allow-path` adds it back. + +Paths are resolved before they are compared, so `..` and symlinks that lead out are refused +(saving through a link writes the file it points to, so the link's target is what counts), and +a new file must go into a folder that exists. On Windows a path must be on a local drive +(`C:\…` or `\\?\C:\…`): UNC, device and NT paths in any spelling (`\\server\share`, +`//server/share`, `\\?\UNC\…`, `\??\…`, `\\.\…`), drive-relative paths (`C:file`), a `:` in a +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. +A refused path is a tool error saying why. + +A hard link inside an allowed folder can't be told apart from an ordinary file, so a hard link +to a file elsewhere is reachable through it; don't keep such links in a folder you allow. + +```sh +solidcraft-cli mcp --allow-path ~/designs --allow-path ~/exports +solidcraft-cli mcp --allow-any-path # no limit (the behaviour before this option existed) +``` + +Bridged, the server sends its folders with every request (`allow_paths`, see +[control-protocol.md](control-protocol.md)) and the app adds its own recovery folder and the +folder of the design open in it, under the same rules, so give absolute paths: a relative one +resolves against the app's working folder, not the server's. The app's own +file dialogs are never limited. + ### Client configuration Claude Code (project `.mcp.json`, or `claude mcp add solidcraft -- solidcraft-cli mcp`; a setup