diff --git a/apps/figcraft-cli/src/args.rs b/apps/figcraft-cli/src/args.rs index 372082c2..4c162dbb 100644 --- a/apps/figcraft-cli/src/args.rs +++ b/apps/figcraft-cli/src/args.rs @@ -1,5 +1,6 @@ //! Minimal std-only argument parsing: positionals, `--key value` / `--key=value` / `-o value` -//! options, and boolean `--flag`s. Unknown options are usage errors. +//! options (given again: the last counts, or all with [`Args::values`]), and boolean `--flag`s. +//! Unknown options are usage errors. use std::collections::BTreeMap; @@ -8,7 +9,7 @@ use crate::{CliError, CliResult}; #[derive(Debug, Default)] pub struct Args { pub positional: Vec, - values: BTreeMap, + values: BTreeMap>, flags: Vec, } @@ -34,7 +35,7 @@ impl Args { Some(v) => v, None => it.next().cloned().ok_or_else(|| CliError::Usage(format!("`{arg}` needs a value")))?, }; - a.values.insert(name.to_string(), v); + a.values.entry(name.to_string()).or_default().push(v); } else if flags.contains(&name) && inline.is_none() { a.flags.push(name.to_string()); } else { @@ -45,7 +46,12 @@ impl Args { } pub fn value(&self, k: &str) -> Option { - self.values.get(k).cloned() + self.values.get(k).and_then(|v| v.last()).cloned() + } + + /// Every value of an option that may be given more than once, in order. + pub fn values(&self, k: &str) -> &[String] { + self.values.get(k).map_or(&[], Vec::as_slice) } pub fn flag(&self, k: &str) -> bool { @@ -62,9 +68,9 @@ impl Args { /// PDF print options: `--bleed LENGTH` (points, or "3mm", "0.125in"), `--crop-marks`, /// `--bleed-marks`, `--cmyk`. pub fn print(&self) -> CliResult { - let bleed = match self.values.get("bleed") { + let bleed = match self.value("bleed") { None => 0.0, - Some(s) => figcraft_doc::PrintSettings::parse_length(s) + Some(s) => figcraft_doc::PrintSettings::parse_length(&s) .filter(|b| *b <= figcraft_doc::PrintSettings::MAX_BLEED) .ok_or_else(|| CliError::Usage(format!("--bleed takes points or a length like 3mm or 0.125in (up to 2in), got `{s}`")))?, }; @@ -72,7 +78,7 @@ impl Args { } pub fn scale(&self) -> CliResult { - match self.values.get("scale") { + match self.value("scale") { None => Ok(1.0), Some(s) => s .trim_end_matches('x') @@ -85,7 +91,7 @@ impl Args { /// A comma-separated list of whole numbers (`--layers 1000,10_000`). pub fn usizes(&self, k: &str, default: &[usize]) -> CliResult> { - match self.values.get(k) { + match self.value(k) { None => Ok(default.to_vec()), Some(s) => s .split(',') @@ -110,6 +116,10 @@ mod tests { assert_eq!(a.value("o").as_deref(), Some("out")); assert_eq!(a.scale().unwrap(), 2.0); assert!(a.flag("json")); + let a = Args::parse(&v(&["--root", "a", "--root=b"]), &["root"], &[]).unwrap(); + assert_eq!(a.values("root"), ["a", "b"]); + assert_eq!(a.value("root").as_deref(), Some("b")); + assert!(a.values("none").is_empty()); } #[test] diff --git a/apps/figcraft-cli/src/main.rs b/apps/figcraft-cli/src/main.rs index 590b50f1..281d79dd 100644 --- a/apps/figcraft-cli/src/main.rs +++ b/apps/figcraft-cli/src/main.rs @@ -23,7 +23,7 @@ USAGE: figcraft-cli render -o [--scale N] [--page NAME] figcraft-cli run [--in ] [--out ] figcraft-cli commands [--json] [--filter TEXT] - figcraft-cli mcp [--attach PORT] + figcraft-cli mcp [--attach PORT] [--automation-read-root DIR]... [--automation-write-root DIR]... figcraft-cli plugin publish --server ws://host:port/[?key=KEY] figcraft-cli perf [--layers N[,N...]] [--instances N[,N...]] figcraft-cli help | --help | --version @@ -42,6 +42,10 @@ crop and bleed marks, CMYK colour). `run` executes a JSON array (or JSON lines) `plugin publish` sends a plugin folder (figcraft-plugin.json, its module, an optional assets/ folder) to a figcraft-server's plugin directory as a new version; the server's key comes from the address or the FIGCRAFT_SERVER_KEY environment variable (which keeps it out of the shell history). +`mcp --automation-read-root DIR` / `--automation-write-root DIR` (each may be repeated; the folders +must exist) confine the files the agent's commands touch: reads only inside a read or write root, +writes only inside a write root, everything else refused. A running app confines itself: start it +with `figcraft --control PORT` and the same flags (`--attach` refuses them). See docs/cli.md and docs/mcp.md."; fn main() -> ExitCode { @@ -133,11 +137,23 @@ fn run(argv: &[String]) -> CliResult { Ok(ExitCode::SUCCESS) } "mcp" => { - let a = Args::parse(rest, &["attach"], &[])?; + let a = Args::parse(rest, &["attach", "automation-read-root", "automation-write-root"], &[])?; if !a.positional.is_empty() { return Err(CliError::Usage("mcp takes no positional arguments".into())); } - mcp(a.value("attach").as_deref()) + let (read, write) = (a.values("automation-read-root"), a.values("automation-write-root")); + let roots = match (read.is_empty() && write.is_empty(), a.value("attach")) { + (true, _) => None, + // The app runs the commands: only its own roots can confine them. + (false, Some(_)) => { + return Err(CliError::Usage( + "--attach can't take automation roots: start the app with them instead (`figcraft --control PORT --automation-read-root DIR ...`)" + .into(), + )); + } + (false, None) => Some(figcraft_engine::file_access::Roots::new(read, write).map_err(CliError::Failed)?), + }; + mcp(a.value("attach").as_deref(), roots) } "plugin" => { let a = Args::parse(rest, &["server"], &[])?; @@ -205,7 +221,7 @@ fn publish_plugin(folder: &str, server: &str) -> CliResult { Ok(ExitCode::SUCCESS) } -fn mcp(attach: Option<&str>) -> CliResult { +fn mcp(attach: Option<&str>, roots: Option) -> CliResult { let backend: Box = match attach { Some(p) => { let addr = if p.contains(':') { @@ -220,7 +236,10 @@ fn mcp(attach: Option<&str>) -> CliResult { None => Box::new(figcraft_mcp::Headless::new()), }; eprintln!("figcraft MCP server ({}) on stdio", backend.describe()); - let mut server = figcraft_mcp::Server::new(backend); + if let Some(r) = &roots { + eprintln!("file access confined to {}", r.to_json()); + } + let mut server = figcraft_mcp::Server::new(backend).with_roots(roots); let stdin = std::io::stdin(); let stdout = std::io::stdout(); server.serve(stdin.lock(), stdout.lock()).map_err(|e| format!("stdio: {e}"))?; diff --git a/apps/figcraft-cli/tests/cli.rs b/apps/figcraft-cli/tests/cli.rs index 9dd52203..7eaf051f 100644 --- a/apps/figcraft-cli/tests/cli.rs +++ b/apps/figcraft-cli/tests/cli.rs @@ -25,6 +25,9 @@ fn bad_arguments_exit_2_without_panicking() { &["render", "a", "-o"], &["perf", "--shapes", "lots"], &["mcp", "--attach", "notaport"], + // An attached app confines itself: roots go to `figcraft --control`. + &["mcp", "--attach", "7979", "--automation-read-root", "."], + &["mcp", "--automation-write-root"], &["convert", "a"], ] { let out = bin().args(args).output().unwrap(); @@ -120,6 +123,49 @@ fn mcp_over_stdio() { assert_eq!(lines[2]["result"]["isError"], false); } +#[test] +fn mcp_over_stdio_confined_to_automation_roots() { + let d = tmp("roots"); + for sub in ["in", "out", "outside"] { + std::fs::create_dir_all(d.join(sub)).unwrap(); + } + let p = |rel: &str| d.join(rel).to_string_lossy().into_owned(); + // A root that isn't a folder stops the server before it serves. + let out = bin().args(["mcp", "--automation-read-root", &p("missing")]).output().unwrap(); + assert_eq!(out.status.code(), Some(1)); + assert!(String::from_utf8_lossy(&out.stderr).contains("--automation-read-root")); + let mut child = bin() + .args(["mcp", "--automation-read-root", &p("in"), "--automation-write-root", &p("out")]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn() + .unwrap(); + { + let stdin = child.stdin.as_mut().unwrap(); + let call = |id: u32, tool: &str, args: serde_json::Value| { + serde_json::json!({"jsonrpc": "2.0", "id": id, "method": "tools/call", "params": {"name": tool, "arguments": args}}).to_string() + }; + writeln!(stdin, r#"{{"jsonrpc":"2.0","id":1,"method":"initialize","params":{{"protocolVersion":"2025-06-18"}}}}"#).unwrap(); + writeln!(stdin, "{}", call(2, "create_node", serde_json::json!({"type": "ellipse"}))).unwrap(); + writeln!(stdin, "{}", call(3, "save_file", serde_json::json!({"path": p("outside/a.figcraft")}))).unwrap(); + writeln!(stdin, "{}", call(4, "save_file", serde_json::json!({"path": p("out/a.figcraft")}))).unwrap(); + writeln!(stdin, "{}", call(5, "open_file", serde_json::json!({"path": p("outside/a.figcraft")}))).unwrap(); + writeln!(stdin, "{}", call(6, "open_file", serde_json::json!({"path": p("out/a.figcraft")}))).unwrap(); + } + drop(child.stdin.take()); + let out = child.wait_with_output().unwrap(); + assert!(out.status.success()); + let lines: Vec = String::from_utf8_lossy(&out.stdout).lines().map(|l| serde_json::from_str(l).unwrap()).collect(); + assert_eq!(lines.len(), 6); + assert!(lines[0]["result"]["instructions"].as_str().unwrap().contains("only these folders may be read")); + let error = |i: usize| lines[i]["result"]["isError"] == true; + assert!(!error(1) && error(2) && !error(3) && error(4) && !error(5), "{lines:?}"); + assert!(lines[2]["result"]["content"][0]["text"].as_str().unwrap().contains("automation path rejected")); + assert!(d.join("out/a.figcraft").is_file() && !d.join("outside/a.figcraft").exists()); + let _ = std::fs::remove_dir_all(&d); +} + #[test] fn commands_json_lists_everything() { let out = bin().args(["commands", "--json"]).output().unwrap(); diff --git a/apps/figcraft/src/args.rs b/apps/figcraft/src/args.rs index 2e3099cb..3b4136d9 100644 --- a/apps/figcraft/src/args.rs +++ b/apps/figcraft/src/args.rs @@ -1,4 +1,5 @@ -//! The command line: `figcraft [--control ] [files…] [figcraft://join?…]`. An invite link +//! The command line: `figcraft [--control ] [--automation-read-root ]… +//! [--automation-write-root ]… [files…] [figcraft://join?…]`. An invite link //! (what the system passes when a `figcraft://` link is opened) joins that session once the window //! is up; everything else that isn't a flag is a file to open, as a path or a `file://` URL (the //! Linux desktop entry passes URLs, `%U`, so that it can also hand over invite links). @@ -11,6 +12,11 @@ pub struct Args { pub files: Vec, /// The last invite link given. pub join: Option, + /// `--automation-read-root ` / `--automation-write-root ` (each repeatable; `=` works + /// too): the folders the app's file access is confined to. A flag without a value gives `""`, + /// which is refused at startup. + pub read_roots: Vec, + pub write_roots: Vec, } /// An invite link rather than a file (`figcraft:` links, in any case). @@ -49,7 +55,15 @@ pub fn parse(args: &[String]) -> Args { let mut out = Args::default(); let mut it = args.iter(); while let Some(a) = it.next() { - if a == "--control" { + let (flag, inline) = a.split_once('=').map_or((a.as_str(), None), |(f, v)| (f, Some(v.to_string()))); + let roots = match flag { + "--automation-read-root" => Some(&mut out.read_roots), + "--automation-write-root" => Some(&mut out.write_roots), + _ => None, + }; + if let Some(roots) = roots { + roots.push(inline.or_else(|| it.next().cloned()).unwrap_or_default()); + } else if a == "--control" { out.control = it.next().and_then(|p| p.parse().ok()); } else if is_invite(a) { out.join = Some(a.clone()); diff --git a/apps/figcraft/src/args_tests.rs b/apps/figcraft/src/args_tests.rs index 0ed404c8..2ae29b3c 100644 --- a/apps/figcraft/src/args_tests.rs +++ b/apps/figcraft/src/args_tests.rs @@ -10,12 +10,22 @@ fn args(a: &[&str]) -> Vec { fn files_and_the_control_port() { assert_eq!(parse(&args(&[])), Args::default()); let a = parse(&args(&["--control", "4242", "a.figcraft", "--verbose", "b.svg"])); - assert_eq!(a, Args { control: Some(4242), files: args(&["a.figcraft", "b.svg"]), join: None }); + assert_eq!(a, Args { control: Some(4242), files: args(&["a.figcraft", "b.svg"]), ..Args::default() }); // A missing or bad port is ignored, not taken as a file. assert_eq!(parse(&args(&["--control"])), Args::default()); assert_eq!(parse(&args(&["--control", "nope"])).control, None); } +#[test] +fn automation_roots_repeat_and_are_never_files() { + let a = parse(&args(&["--automation-read-root", "in", "--automation-write-root=out", "--automation-read-root=more", "a.figcraft"])); + assert_eq!(a.read_roots, args(&["in", "more"])); + assert_eq!(a.write_roots, args(&["out"])); + assert_eq!(a.files, args(&["a.figcraft"])); + // A flag without its folder is kept as an empty one, which startup refuses. + assert_eq!(parse(&args(&["--automation-write-root"])).write_roots, args(&[""])); +} + #[test] fn invite_links_join_instead_of_opening() { let link = "figcraft://join?host=192.168.1.20:47900&file=abc&token=AbCdEfGhIjKlMnOpQrStUv"; diff --git a/apps/figcraft/src/main.rs b/apps/figcraft/src/main.rs index fdcc66f9..651cc521 100644 --- a/apps/figcraft/src/main.rs +++ b/apps/figcraft/src/main.rs @@ -6,6 +6,11 @@ //! `--control ` (or `FIGCRAFT_CONTROL_PORT`) starts a localhost JSON-lines control server: //! `{"id":1,"method":"engine.execute","params":{"command":"shape.rectangle","params":{"x":0,"y":0,"width":100,"height":100}}}` //! → `{"id":1,"ok":true,"result":{…}}`. Methods: `figcraft_ui_egui::control`. +//! +//! `--automation-read-root ` / `--automation-write-root ` (each repeatable) confine the +//! files the window and the control channel read and write (`figcraft_engine::file_access`); the +//! app's own folders (settings, recovery copies, installed plugins) and opening the files given on +//! the command line are not. A root that isn't an existing folder stops the app at startup. #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] #![cfg_attr(all(target_os = "windows", not(debug_assertions)), windows_subsystem = "windows")] @@ -119,6 +124,17 @@ fn main() -> eframe::Result { return Ok(()); } let args = args::parse(&args); + let roots = match ( + args.read_roots.is_empty() && args.write_roots.is_empty(), + figcraft_engine::file_access::Roots::new(&args.read_roots, &args.write_roots), + ) { + (true, _) => None, + (false, Ok(r)) => Some(Arc::new(r)), + (false, Err(e)) => { + eprintln!("FigCraft: {e}"); + std::process::exit(2); + } + }; let port = args.control.or_else(|| std::env::var("FIGCRAFT_CONTROL_PORT").ok().and_then(|p| p.parse().ok())); // System fonts: index them in the background, then wake the UI (an idle window draws no frames) // to lay text out again with them. @@ -174,6 +190,7 @@ fn main() -> eframe::Result { // open (no frame then). figcraft_ui_egui::i18n::set(&session.prefs.language, figcraft_ui_egui::Services::system_language(&svc).as_deref()); let mut fig = FigApp::new(session, Box::new(svc)); + fig.automation_roots = roots; fig.ui.home = false; fig.note_recovered(recovered.copies); if let Some(link) = &args.join { diff --git a/crates/engine/clippy.toml b/crates/engine/clippy.toml new file mode 100644 index 00000000..3ceac899 --- /dev/null +++ b/crates/engine/clippy.toml @@ -0,0 +1,39 @@ +# The workspace settings (../../clippy.toml; clippy reads only the nearest file), plus the engine's +# own rule: shipped engine code reaches the file system only through `file_access`, which enforces +# the automation roots (#715), so every command is confined without a check of its own. +allow-unwrap-in-tests = true +allow-expect-in-tests = true +allow-panic-in-tests = true +too-many-arguments-threshold = 10 +disallowed-methods = [ + { path = "std::fs::read", reason = "use file_access::read (automation roots)" }, + { path = "std::fs::read_to_string", reason = "use file_access::read (automation roots)" }, + { path = "std::fs::write", reason = "use file_access::write (automation roots)" }, + { path = "std::fs::metadata", reason = "use file_access::metadata (automation roots)" }, + { path = "std::fs::symlink_metadata", reason = "use file_access (automation roots)" }, + { path = "std::fs::read_dir", reason = "use file_access::read_dir (automation roots)" }, + { path = "std::fs::canonicalize", reason = "use file_access (automation roots)" }, + { path = "std::fs::copy", reason = "use file_access (automation roots)" }, + { path = "std::fs::rename", reason = "use file_access (automation roots)" }, + { path = "std::fs::create_dir", reason = "use file_access::create_dir_all (automation roots)" }, + { path = "std::fs::create_dir_all", reason = "use file_access::create_dir_all (automation roots)" }, + { path = "std::fs::remove_file", reason = "use file_access (automation roots)" }, + { path = "std::fs::remove_dir", reason = "use file_access (automation roots)" }, + { path = "std::fs::remove_dir_all", reason = "use file_access (automation roots)" }, + { path = "std::fs::hard_link", reason = "use file_access (automation roots)" }, + { path = "std::fs::File::open", reason = "use file_access::open (automation roots)" }, + { path = "std::fs::File::create", reason = "use file_access::write (automation roots)" }, + { path = "std::fs::File::create_new", reason = "use file_access::write (automation roots)" }, + { path = "std::fs::OpenOptions::open", reason = "use file_access (automation roots)" }, + { path = "std::path::Path::exists", reason = "use file_access::exists (automation roots)" }, + { path = "std::path::Path::try_exists", reason = "use file_access::exists (automation roots)" }, + { path = "std::path::Path::is_file", reason = "use file_access::metadata (automation roots)" }, + { path = "std::path::Path::is_dir", reason = "use file_access::is_dir (automation roots)" }, + { path = "std::path::Path::metadata", reason = "use file_access::metadata (automation roots)" }, + { path = "std::path::Path::symlink_metadata", reason = "use file_access (automation roots)" }, + { path = "std::path::Path::read_dir", reason = "use file_access::read_dir (automation roots)" }, + { path = "std::path::Path::canonicalize", reason = "use file_access (automation roots)" }, + { path = "figcraft_format::write_file_atomic", reason = "use file_access::write (automation roots)" }, + { path = "figcraft_format::save_file", reason = "use file_access::write (automation roots)" }, + { path = "figcraft_format::load_file", reason = "use file_access::read (automation roots)" }, +] diff --git a/crates/engine/src/cmd/create.rs b/crates/engine/src/cmd/create.rs index 9b471290..9b3f7067 100644 --- a/crates/engine/src/cmd/create.rs +++ b/crates/engine/src/cmd/create.rs @@ -7,7 +7,7 @@ use serde_json::{Value, json}; use super::{CommandSpec, ok}; use crate::params::*; -use crate::{EngineError, Result, Session, bad, spec}; +use crate::{Result, Session, bad, spec}; /// Frame presets shown by the Frame tool (category, name, width, height). pub const FRAME_PRESETS: &[(&str, &str, f64, f64)] = &[ @@ -313,12 +313,12 @@ fn free_spot(s: &Session, w: f64, h: f64) -> Result<(f64, f64)> { #[cfg(not(target_arch = "wasm32"))] fn read_path(path: &str) -> Result> { - std::fs::read(path).map_err(|e| EngineError::Other(format!("{path}: {e}"))) + crate::file_access::read(path) } #[cfg(target_arch = "wasm32")] fn read_path(path: &str) -> Result> { - Err(EngineError::Other(format!("{path}: files can't be read in the browser"))) + Err(crate::EngineError::Other(format!("{path}: files can't be read in the browser"))) } /// Place an image as a rectangle with an image fill (Figma's image layer). diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 6ba1ba6c..511e0fc6 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -435,7 +435,7 @@ pub(crate) fn recovered(path: &str) -> Result { #[cfg(not(target_arch = "wasm32"))] fn read(path: &str) -> Result> { - std::fs::read(path).map_err(|e| EngineError::Other(format!("{path}: {e}"))) + crate::file_access::read(path) } #[cfg(target_arch = "wasm32")] @@ -445,11 +445,7 @@ fn read(path: &str) -> Result> { /// The bytes of file `path`, refused when it's larger than `max`. pub(crate) fn read_capped(path: &str, max: u64) -> Result> { - let meta = std::fs::metadata(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; - if !meta.is_file() || meta.len() > max { - return Err(EngineError::Other(format!("{path}: not a file of at most {max} bytes"))); - } - read(path) + crate::file_access::read_capped(path, max) } /// Write `bytes` to `path` through a uniquely named, exclusively created staging file and one @@ -457,7 +453,7 @@ pub(crate) fn read_capped(path: &str, max: u64) -> Result> { /// truncates `path`, and no file or symlink at a staging name is ever written through. #[cfg(not(target_arch = "wasm32"))] pub(crate) fn write(path: &str, bytes: &[u8]) -> Result<()> { - figcraft_format::write_file_atomic(std::path::Path::new(path), bytes).map_err(|e| EngineError::Other(format!("{path}: {e}"))) + crate::file_access::write(path, bytes) } #[cfg(target_arch = "wasm32")] @@ -577,11 +573,9 @@ fn rest_images(path: &str) -> impl Fn(&str) -> Option> { return None; } let dir = dir.as_ref()?; - ["", ".png", ".jpg", ".jpeg", ".gif", ".webp"].iter().find_map(|e| { - let p = dir.join(format!("{r}{e}")); - let meta = std::fs::metadata(&p).ok()?; - if meta.is_file() && meta.len() <= MAX_IMAGE_FILE { std::fs::read(&p).ok() } else { None } - }) + ["", ".png", ".jpg", ".jpeg", ".gif", ".webp"] + .iter() + .find_map(|e| crate::file_access::read_capped(dir.join(format!("{r}{e}")), MAX_IMAGE_FILE).ok()) } } @@ -604,11 +598,7 @@ fn sibling_variables(path: &str) -> Option { let file = std::path::Path::new(path); let dir = file.parent()?; [format!("{}.variables.json", file_stem(path)), "variables.json".to_string()].iter().map(|n| dir.join(n)).filter(|c| c != file).find_map(|c| { - let meta = std::fs::metadata(&c).ok()?; - if !meta.is_file() || meta.len() > MAX_VARIABLES_FILE { - return None; - } - let v: Value = serde_json::from_slice(&std::fs::read(&c).ok()?).ok()?; + let v: Value = serde_json::from_slice(&crate::file_access::read_capped(c, MAX_VARIABLES_FILE).ok()?).ok()?; figcraft_format::rest::is_variables_json(&v).then_some(v) }) } @@ -646,20 +636,29 @@ fn remember(s: &mut Session, path: &str) { } fn open_recent(s: &mut Session, p: &Value) -> Result { + use crate::file_access::{Access, allow, allowed}; const CMD: &str = "file.openRecent"; + // Confined to automation roots, the recent files outside them are never listed, opened, removed + // or named: `index` counts the files listed. + let listed = |r: &String| allowed(r, Access::Read); if p.get("clear").and_then(Value::as_bool) == Some(true) { - s.prefs.recent_files.clear(); + s.prefs.recent_files.retain(|r| !listed(r)); } if let Some(gone) = string(p, "remove") { - s.prefs.recent_files.retain(|r| *r != gone); + s.prefs.recent_files.retain(|r| *r != gone || !listed(r)); } let path = match (string(p, "path"), num(p, "index")) { - (Some(path), _) => Some(path), - (None, Some(i)) => Some(s.prefs.recent_files.get(i.max(0.0) as usize).cloned().ok_or_else(|| bad(CMD, "no such recent file"))?), + (Some(path), _) => { + allow(&path, Access::Read)?; + Some(path) + } + (None, Some(i)) => { + Some(s.prefs.recent_files.iter().filter(|r| listed(r)).nth(i.max(0.0) as usize).cloned().ok_or_else(|| bad(CMD, "no such recent file"))?) + } _ => None, }; if let Some(path) = path { - if !std::path::Path::new(&path).exists() { + if !crate::file_access::exists(&path) { s.prefs.recent_files.retain(|r| *r != path); return Err(bad(CMD, format!("{path} no longer exists"))); } @@ -669,9 +668,10 @@ fn open_recent(s: &mut Session, p: &Value) -> Result { .prefs .recent_files .iter() + .filter(|r| listed(r)) .map(|r| { let name = std::path::Path::new(r).file_name().map_or_else(|| r.clone(), |f| f.to_string_lossy().into_owned()); - json!({"path": r, "name": name, "exists": std::path::Path::new(r).exists()}) + json!({"path": r, "name": name, "exists": crate::file_access::exists(r)}) }) .collect(); Ok(json!({"files": files})) @@ -1183,7 +1183,7 @@ fn export_layers(s: &mut Session, p: &Value) -> Result { let path = std::path::Path::new(d).join(&job.file); // A layer name with slashes exports into folders. if let Some(parent) = path.parent() { - let _ = std::fs::create_dir_all(parent); + let _ = crate::file_access::create_dir_all(parent); } let path = path.to_string_lossy().to_string(); write(&path, &bytes)?; diff --git a/crates/engine/src/file_access.rs b/crates/engine/src/file_access.rs new file mode 100644 index 00000000..8abeaee5 --- /dev/null +++ b/crates/engine/src/file_access.rs @@ -0,0 +1,293 @@ +//! Automation roots (#715): the folders the files an agent's commands read and write must lie in. +//! +//! `figcraft-cli mcp --automation-read-root DIR --automation-write-root DIR` and the desktop app +//! started with the same flags run their work [`confined`] to [`Roots`]: while a scope is active on +//! the thread, every file the engine reads must lie inside a read or a write root and every file it +//! writes inside a write root; anything else is an ordinary error. The engine reaches the file +//! system only through this module (`crates/engine/clippy.toml` denies `std::fs` and friends +//! anywhere else), so every command, present or future, is covered. Without a scope nothing changes. +//! +//! A path is made absolute against the working directory and resolved before it is compared: its +//! links are followed (`std::fs::canonicalize`), and for a file that doesn't exist yet its nearest +//! existing folder is resolved and the rest must be plain names. The comparison is by whole +//! components (`/work2` is not inside `/work`), and the I/O uses the resolved path, so what was +//! checked is what is opened. +#![allow(clippy::disallowed_methods)] + +use std::cell::RefCell; +use std::ffi::{OsStr, OsString}; +use std::path::{Component, Path, PathBuf}; +use std::sync::Arc; + +use crate::{EngineError, Result}; + +/// What a command does with a file. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Access { + Read, + Write, +} + +/// The folders automation may read (read and write roots) and write (write roots), resolved. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Roots { + read: Vec, + write: Vec, +} + +impl Roots { + /// Roots from the folders given (relative ones against the working directory). Each must be an + /// existing folder. + pub fn new(read: &[String], write: &[String]) -> std::result::Result { + let resolve = |flag: &str, dirs: &[String]| { + dirs.iter() + .map(|d| match std::fs::canonicalize(d) { + Ok(p) if p.is_dir() => Ok(p), + Ok(_) => Err(format!("{flag} {d}: not a folder")), + Err(e) => Err(format!("{flag} {d}: {e}")), + }) + .collect::, String>>() + }; + Ok(Self { read: resolve("--automation-read-root", read)?, write: resolve("--automation-write-root", write)? }) + } + + /// The resolved path to use for `access` to `path`, or why it is refused. + pub fn check(&self, path: &Path, access: Access) -> std::result::Result { + let asked = path.display(); + let (roots, what) = match access { + Access::Read => ([self.read.as_slice(), self.write.as_slice()].concat(), "read"), + Access::Write => (self.write.clone(), "written"), + }; + match resolve(path) { + Ok(resolved) if roots.iter().any(|r| resolved.starts_with(r)) => return Ok(resolved), + Ok(_) => {} + // Why a path can't be resolved says what exists there: only told for paths inside a root. + Err(why) if lexically_inside(path, &roots) => return Err(format!("automation path rejected: {asked}: {why}")), + Err(_) => {} + } + if roots.is_empty() { + return Err(format!("automation path rejected: no folder may be {what} (no root for it was given): {asked}")); + } + let shown: Vec = roots.iter().map(|r| shown(r)).collect(); + Err(format!("automation path rejected: {asked} is outside the folders that may be {what}: {}", shown.join(", "))) + } + + /// The folders as JSON, resolved: `{read: [every folder that may be read], write: [the write roots]}`. + pub fn to_json(&self) -> serde_json::Value { + let list = |v: &[PathBuf]| v.iter().map(|p| shown(p)).collect::>(); + serde_json::json!({"read": list(&[self.read.as_slice(), self.write.as_slice()].concat()), "write": list(&self.write)}) + } +} + +/// Whether `path`, made absolute with its `.` and `..` taken by name (no file system), lies in one +/// of `roots` (as resolved, or as shown without Windows' `\\?\` prefix). +fn lexically_inside(path: &Path, roots: &[PathBuf]) -> bool { + let Ok(abs) = std::path::absolute(path) else { return false }; + let mut lexical = PathBuf::new(); + for c in abs.components() { + match c { + Component::ParentDir => { + lexical.pop(); + } + Component::CurDir => {} + c => lexical.push(c), + } + } + roots.iter().any(|r| lexical.starts_with(r) || lexical.starts_with(shown(r))) +} + +/// Whether `path` lies in the app's own folder `folder`, both resolved (links and `..` followed); +/// false when either can't be resolved. +#[cfg_attr(target_arch = "wasm32", allow(dead_code))] +pub(crate) fn within_app_folder(path: &Path, folder: &Path) -> bool { + matches!((std::fs::canonicalize(path), std::fs::canonicalize(folder)), (Ok(p), Ok(f)) if p.starts_with(&f)) +} + +/// `p` without Windows' `\\?\` prefix, for messages. +fn shown(p: &Path) -> String { + let s = p.to_string_lossy(); + match s.strip_prefix(r"\\?\UNC\") { + Some(unc) => format!(r"\\{unc}"), + None => s.strip_prefix(r"\\?\").unwrap_or(&s).to_string(), + } +} + +/// `path` made absolute and resolved: links followed, and for a path that doesn't exist yet its +/// nearest existing folder resolved with the rest appended (plain names only). +fn resolve(path: &Path) -> std::result::Result { + if path.as_os_str().is_empty() { + return Err("not a file path".into()); + } + let abs = std::path::absolute(path).map_err(|e| e.to_string())?; + let mut rest: Vec = Vec::new(); + let mut at = abs.as_path(); + loop { + match std::fs::canonicalize(at) { + Ok(mut base) => { + base.extend(rest.iter().rev()); + return Ok(base); + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + // Writing through a link that leads nowhere would create its target, wherever it is. + if std::fs::symlink_metadata(at).is_ok() { + return Err("a link that leads nowhere".into()); + } + match at.components().next_back() { + Some(Component::Normal(name)) => { + plain_name(name)?; + rest.push(name.to_os_string()); + } + Some(Component::CurDir) => {} + _ => return Err("`..` (or a drive) below a folder that doesn't exist".into()), + } + at = at.parent().ok_or("no existing folder")?; + } + Err(e) => return Err(e.to_string()), + } + } +} + +/// A name for a file or folder that doesn't exist yet: on Windows no device (`CON`, `NUL`, `COM1`…) +/// and no alternate data stream (`a:b`). +fn plain_name(name: &OsStr) -> std::result::Result<(), String> { + if cfg!(windows) { + let n = name.to_string_lossy(); + let stem = n.split('.').next().unwrap_or_default().trim_end().to_ascii_uppercase(); + let device = matches!(stem.as_str(), "CON" | "PRN" | "AUX" | "NUL" | "CONIN$" | "CONOUT$") + || ((stem.starts_with("COM") || stem.starts_with("LPT")) + && stem.chars().count() == 4 + && stem.chars().last().is_some_and(|c| c.is_ascii_digit() || matches!(c, '¹' | '²' | '³'))); + if device || n.contains(':') { + return Err(format!("`{n}` is a device or stream name, not a file")); + } + } + Ok(()) +} + +thread_local! { + static ROOTS: RefCell>> = const { RefCell::new(None) }; +} + +/// Run `f` with this thread's file access confined to `roots` (unchanged when `None`). +pub fn confined(roots: Option<&Arc>, f: impl FnOnce() -> R) -> R { + match roots { + Some(roots) => scoped(Some(roots.clone()), f), + None => f(), + } +} + +/// Run `f` unconfined: only for the app's own files, never for a path a command was given. +// Only plugins read from disk use it, and the browser has no disk. +#[cfg_attr(target_arch = "wasm32", allow(dead_code))] +pub(crate) fn app_owned(f: impl FnOnce() -> R) -> R { + scoped(None, f) +} + +fn scoped(roots: Option>, f: impl FnOnce() -> R) -> R { + /// Puts the outer scope's roots back, also when `f` panics. + struct Restore(Option>); + impl Drop for Restore { + fn drop(&mut self) { + let outer = self.0.take(); + ROOTS.with(|r| *r.borrow_mut() = outer); + } + } + let _restore = Restore(ROOTS.with(|r| std::mem::replace(&mut *r.borrow_mut(), roots))); + f() +} + +/// The roots this thread's file access is confined to now, if any. +pub fn current() -> Option> { + ROOTS.with(|r| r.borrow().clone()) +} + +/// The path to use for `access` to `path`: resolved when confined, else as given. +pub fn allow(path: impl AsRef, access: Access) -> Result { + let path = path.as_ref(); + match current() { + Some(roots) => roots.check(path, access).map_err(EngineError::Other), + None => Ok(path.to_path_buf()), + } +} + +/// Whether `path` may be accessed so now. +pub fn allowed(path: impl AsRef, access: Access) -> bool { + allow(path, access).is_ok() +} + +fn io(path: &Path) -> impl Fn(std::io::Error) -> EngineError + '_ { + move |e| EngineError::Other(format!("{}: {e}", path.display())) +} + +/// The bytes of file `path`. +pub fn read(path: impl AsRef) -> Result> { + let path = path.as_ref(); + std::fs::read(allow(path, Access::Read)?).map_err(io(path)) +} + +/// The bytes of file `path`, refused when it isn't a file or is larger than `max`. +pub fn read_capped(path: impl AsRef, max: u64) -> Result> { + let path = path.as_ref(); + let p = allow(path, Access::Read)?; + let meta = std::fs::metadata(&p).map_err(io(path))?; + if !meta.is_file() || meta.len() > max { + return Err(EngineError::Other(format!("{}: not a file of at most {max} bytes", path.display()))); + } + std::fs::read(p).map_err(io(path)) +} + +/// File `path` opened for reading. +pub fn open(path: impl AsRef) -> Result { + let path = path.as_ref(); + std::fs::File::open(allow(path, Access::Read)?).map_err(io(path)) +} + +/// What the file system says about `path` (links followed). +pub fn metadata(path: impl AsRef) -> Result { + let path = path.as_ref(); + std::fs::metadata(allow(path, Access::Read)?).map_err(io(path)) +} + +/// Whether `path` is a folder (false when it may not be read). +pub fn is_dir(path: impl AsRef) -> bool { + metadata(path).is_ok_and(|m| m.is_dir()) +} + +/// Whether `path` exists (false when it may not be read). +pub fn exists(path: impl AsRef) -> bool { + metadata(path).is_ok() +} + +/// The paths of the entries of folder `path`. +pub fn read_dir(path: impl AsRef) -> Result> { + let path = path.as_ref(); + let dir = allow(path, Access::Read)?; + Ok(std::fs::read_dir(&dir).map_err(io(path))?.flatten().map(|e| e.path()).collect()) +} + +/// Write `bytes` to `path` through a uniquely named staging file and one atomic rename (see +/// [`figcraft_format::write_file_atomic`]). +pub fn write(path: impl AsRef, bytes: &[u8]) -> Result<()> { + let path = path.as_ref(); + write_atomic(&allow(path, Access::Write)?, bytes).map_err(io(path)) +} + +#[cfg(not(target_arch = "wasm32"))] +fn write_atomic(path: &Path, bytes: &[u8]) -> std::io::Result<()> { + figcraft_format::write_file_atomic(path, bytes) +} + +#[cfg(target_arch = "wasm32")] +fn write_atomic(_: &Path, _: &[u8]) -> std::io::Result<()> { + Err(std::io::Error::new(std::io::ErrorKind::Unsupported, "no file system in the browser")) +} + +/// Create folder `path` and the folders above it that are missing. +pub fn create_dir_all(path: impl AsRef) -> Result<()> { + let path = path.as_ref(); + std::fs::create_dir_all(allow(path, Access::Write)?).map_err(io(path)) +} + +#[cfg(test)] +#[path = "file_access_tests.rs"] +mod tests; diff --git a/crates/engine/src/file_access_tests.rs b/crates/engine/src/file_access_tests.rs new file mode 100644 index 00000000..a05ad88c --- /dev/null +++ b/crates/engine/src/file_access_tests.rs @@ -0,0 +1,371 @@ +//! Automation roots: how paths are resolved and compared, and that commands run confined can't +//! reach past them. + +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use serde_json::json; + +use super::{Access, Roots, allow, confined, current}; +use crate::Session; + +/// A fresh folder for one test: `/figcraft-roots--`, holding `root/`, `root2/` +/// (a sibling whose name starts like the root's) and `outside/`. +struct Fixture(PathBuf); + +impl Fixture { + fn new(name: &str) -> Self { + let d = std::env::temp_dir().join(format!("figcraft-roots-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + for sub in ["root/sub", "root2", "outside"] { + std::fs::create_dir_all(d.join(sub)).unwrap(); + } + std::fs::write(d.join("root/in.txt"), b"in").unwrap(); + std::fs::write(d.join("outside/secret.txt"), b"secret").unwrap(); + Self(d) + } + + fn p(&self, rel: &str) -> String { + self.0.join(rel).to_string_lossy().into_owned() + } + + /// The root as both read and write root. + fn rw(&self) -> Roots { + Roots::new(&[], &[self.p("root")]).unwrap() + } +} + +impl Drop for Fixture { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } +} + +#[cfg(unix)] +fn symlink_dir(target: &Path, link: &Path) -> bool { + std::os::unix::fs::symlink(target, link).is_ok() +} + +#[cfg(windows)] +fn symlink_dir(target: &Path, link: &Path) -> bool { + std::os::windows::fs::symlink_dir(target, link).is_ok() +} + +#[cfg(unix)] +fn symlink_file(target: &Path, link: &Path) -> bool { + std::os::unix::fs::symlink(target, link).is_ok() +} + +#[cfg(windows)] +fn symlink_file(target: &Path, link: &Path) -> bool { + std::os::windows::fs::symlink_file(target, link).is_ok() +} + +#[test] +fn inside_is_allowed_and_resolved_outside_is_refused() { + let f = Fixture::new("inside"); + let r = f.rw(); + let got = r.check(Path::new(&f.p("root/in.txt")), Access::Read).unwrap(); + assert_eq!(got, std::fs::canonicalize(f.p("root/in.txt")).unwrap(), "the I/O uses the resolved path"); + assert!(r.check(Path::new(&f.p("root/new.png")), Access::Write).is_ok()); + let e = r.check(Path::new(&f.p("outside/secret.txt")), Access::Read).unwrap_err(); + assert!(e.starts_with("automation path rejected:") && e.contains("outside the folders that may be read"), "{e}"); + assert!(r.check(Path::new(&f.p("outside/new.png")), Access::Write).is_err()); + // The root itself is inside. + assert!(r.check(Path::new(&f.p("root")), Access::Read).is_ok()); +} + +#[test] +fn a_sibling_whose_name_starts_like_the_root_is_outside() { + let f = Fixture::new("prefix"); + let r = f.rw(); + for p in ["root2/x.png", "root2", "root.png"] { + assert!(r.check(Path::new(&f.p(p)), Access::Write).is_err(), "{p}"); + assert!(r.check(Path::new(&f.p(p)), Access::Read).is_err(), "{p}"); + } +} + +#[test] +fn traversal_is_resolved_before_comparing() { + let f = Fixture::new("dots"); + let r = f.rw(); + assert!(r.check(Path::new(&f.p("root/sub/../../outside/secret.txt")), Access::Read).is_err()); + assert!(r.check(Path::new(&f.p("root/sub/../../outside/new.txt")), Access::Write).is_err()); + assert!(r.check(Path::new(&f.p("root/sub/../in.txt")), Access::Read).is_ok()); + assert!(r.check(Path::new(&f.p("root/./sub/../new.txt")), Access::Write).is_ok()); + // `..` below a folder that doesn't exist can't be resolved (on Windows it is taken lexically, + // as the system does, and stays inside). + let deep = f.p("root/missing/../../outside/x.txt"); + assert!(r.check(Path::new(&deep), Access::Write).is_err(), "{deep}"); +} + +#[test] +fn files_that_dont_exist_yet_resolve_through_their_nearest_folder() { + let f = Fixture::new("missing"); + let r = f.rw(); + let got = r.check(Path::new(&f.p("root/a/b/c.png")), Access::Write).unwrap(); + assert_eq!(got, std::fs::canonicalize(f.p("root")).unwrap().join("a").join("b").join("c.png")); + assert!(r.check(Path::new(&f.p("outside/a/b/c.png")), Access::Write).is_err()); +} + +#[test] +fn relative_paths_are_resolved_against_the_working_directory() { + // `cargo test` runs in the crate's folder, which holds Cargo.toml. + let cwd = std::env::current_dir().unwrap().to_string_lossy().into_owned(); + let here = Roots::new(std::slice::from_ref(&cwd), &[]).unwrap(); + assert_eq!(here.check(Path::new("Cargo.toml"), Access::Read).unwrap(), std::fs::canonicalize("Cargo.toml").unwrap()); + assert!(here.check(Path::new("src/../Cargo.toml"), Access::Read).is_ok()); + let f = Fixture::new("relative"); + assert!(f.rw().check(Path::new("Cargo.toml"), Access::Read).is_err()); + // Relative roots too. + assert_eq!(Roots::new(&[".".into()], &[]).unwrap(), here); +} + +#[test] +fn read_and_write_authority_are_separate() { + let f = Fixture::new("modes"); + let read_only = Roots::new(&[f.p("root")], &[]).unwrap(); + assert!(read_only.check(Path::new(&f.p("root/in.txt")), Access::Read).is_ok()); + let e = read_only.check(Path::new(&f.p("root/out.png")), Access::Write).unwrap_err(); + assert!(e.contains("no folder may be written"), "{e}"); + // A write root may be read too; a read root elsewhere doesn't make it writable. + let both = Roots::new(&[f.p("outside")], &[f.p("root")]).unwrap(); + assert!(both.check(Path::new(&f.p("root/in.txt")), Access::Read).is_ok()); + assert!(both.check(Path::new(&f.p("outside/secret.txt")), Access::Read).is_ok()); + assert!(both.check(Path::new(&f.p("outside/new.txt")), Access::Write).is_err()); + // Several roots of each kind. + let many = Roots::new(&[f.p("root"), f.p("root2")], &[f.p("root2"), f.p("outside")]).unwrap(); + assert!(many.check(Path::new(&f.p("outside/new.txt")), Access::Write).is_ok()); + assert!(many.check(Path::new(&f.p("root/new.txt")), Access::Write).is_err()); +} + +#[test] +fn roots_must_be_existing_folders() { + let f = Fixture::new("bad-roots"); + assert!(Roots::new(&[f.p("nope")], &[]).unwrap_err().contains("--automation-read-root")); + assert!(Roots::new(&[], &[f.p("root/in.txt")]).unwrap_err().contains("not a folder")); + assert!(Roots::new(&[], &[String::new()]).is_err()); + assert!(f.rw().check(Path::new(""), Access::Read).is_err()); +} + +#[test] +fn links_that_lead_out_of_a_root_are_outside() { + let f = Fixture::new("links"); + let r = f.rw(); + if !symlink_dir(&f.0.join("outside"), &f.0.join("root/escape")) || !symlink_file(&f.0.join("outside/secret.txt"), &f.0.join("root/secret.txt")) { + eprintln!("skipped: this system doesn't let tests make symbolic links"); + return; + } + assert!(r.check(Path::new(&f.p("root/escape/secret.txt")), Access::Read).is_err()); + assert!(r.check(Path::new(&f.p("root/escape/new.txt")), Access::Write).is_err()); + assert!(r.check(Path::new(&f.p("root/secret.txt")), Access::Read).is_err()); + assert!(r.check(Path::new(&f.p("root/secret.txt")), Access::Write).is_err()); + // A link that leads nowhere: writing through it would create its target. + assert!(symlink_file(&f.0.join("outside/not-yet.txt"), &f.0.join("root/dangling.txt"))); + let e = r.check(Path::new(&f.p("root/dangling.txt")), Access::Write).unwrap_err(); + assert!(e.contains("leads nowhere"), "{e}"); + // A link inside the root to the root is fine. + assert!(symlink_dir(&f.0.join("root/sub"), &f.0.join("root/alias"))); + assert!(r.check(Path::new(&f.p("root/alias/x.png")), Access::Write).is_ok()); +} + +#[cfg(windows)] +#[test] +fn windows_case_devices_and_streams() { + let f = Fixture::new("windows"); + let r = f.rw(); + // Case is taken from the disk: another spelling of the root is the root. + let upper = f.0.join("ROOT").join("IN.TXT"); + assert!(r.check(&upper, Access::Read).is_ok()); + for name in ["CON", "nul.txt", "Com1.png", "LPT9", "a.txt:stream", "conout$"] { + assert!(r.check(&f.0.join("root").join(name), Access::Write).is_err(), "{name}"); + } + assert!(r.check(&f.0.join("root").join("console.png"), Access::Write).is_ok()); + // The `\\?\` form of a path inside is inside; of a path outside, outside. + let verbatim = std::fs::canonicalize(f.p("root")).unwrap().join("v.png"); + assert!(verbatim.to_string_lossy().starts_with(r"\\?\")); + assert!(r.check(&verbatim, Access::Write).is_ok()); + assert!(r.check(&std::fs::canonicalize(f.p("outside")).unwrap().join("v.png"), Access::Write).is_err()); + // Messages don't show the prefix. + let e = r.check(Path::new(&f.p("outside/x.png")), Access::Write).unwrap_err(); + assert!(!e.contains(r"\\?\"), "{e}"); +} + +#[test] +fn a_scope_confines_the_thread_and_ends_with_it() { + let f = Fixture::new("scope"); + let roots = Arc::new(f.rw()); + assert!(current().is_none()); + confined(Some(&roots), || { + assert!(allow(f.p("outside/secret.txt"), Access::Read).is_err()); + assert!(super::read(f.p("outside/secret.txt")).is_err()); + assert_eq!(super::read(f.p("root/in.txt")).unwrap(), b"in"); + // Not confined: the app's own files. + assert_eq!(super::app_owned(|| super::read(f.p("outside/secret.txt")).unwrap()), b"secret"); + assert!(current().is_some(), "back in the scope after app_owned"); + // Another thread isn't confined by this one's scope. + let p = f.p("outside/secret.txt"); + assert!(std::thread::spawn(move || current().is_none() && allow(p, Access::Read).is_ok()).join().unwrap()); + }); + assert!(current().is_none()); + assert!(allow(f.p("outside/secret.txt"), Access::Read).is_ok()); + // A panic inside the scope still ends it. + let r = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| confined(Some(&roots), || panic!("boom")))); + assert!(r.is_err()); + assert!(current().is_none()); + // `None` changes nothing. + assert!(confined(None, || current().is_none())); +} + +#[test] +fn commands_run_confined_read_and_write_only_inside() { + let f = Fixture::new("commands"); + std::fs::create_dir_all(f.0.join("outside/readable")).unwrap(); + let roots = Arc::new(Roots::new(&[f.p("outside/readable")], &[f.p("root")]).unwrap()); + // Unconfined, the outside is writable: the bait files a confined command must not reach. + let bait = f.p("outside/bait.figcraft"); + let mut s = Session::new(); + s.execute("frame.create", &json!({"x": 0, "y": 0, "width": 20, "height": 10})).unwrap(); + s.execute("file.saveAs", &json!({"path": bait})).unwrap(); + std::fs::copy(&bait, f.0.join("outside/readable/ok.figcraft")).unwrap(); + let mut s = Session::new(); + s.execute("frame.create", &json!({"x": 0, "y": 0, "width": 20, "height": 10})).unwrap(); + s.prefs.recent_files = vec![bait.clone(), f.p("root/in.txt")]; + confined(Some(&roots), || { + let e = s.execute("file.export", &json!({"format": "png", "path": f.p("outside/x.png")})).unwrap_err().to_string(); + assert!(e.contains("automation path rejected"), "{e}"); + assert!(s.execute("file.export", &json!({"format": "png", "path": f.p("root/x.png")})).is_ok()); + assert!(s.execute("file.exportLayers", &json!({"dir": f.p("outside/layers")})).is_err()); + assert!(s.execute("file.open", &json!({"path": bait})).is_err()); + assert!(s.execute("file.import", &json!({"path": bait})).is_err()); + assert!(s.execute("file.placeImage", &json!({"path": f.p("outside/secret.txt")})).is_err()); + assert!(s.execute("file.open", &json!({"path": f.p("outside/readable/ok.figcraft")})).is_ok(), "read roots may be read"); + assert!(s.execute("file.save", &json!({})).is_err(), "but not written"); + assert!(s.execute("file.saveAs", &json!({"path": f.p("root/ok.figcraft")})).is_ok()); + // Recent files outside the roots aren't listed. + let recent = s.execute("file.openRecent", &json!({})).unwrap().to_string(); + assert!(!recent.contains("bait") && recent.contains("in.txt"), "{recent}"); + assert!(s.execute("file.openRecent", &json!({"path": bait})).is_err()); + }); + assert!(f.0.join("root/x.png").is_file() && !f.0.join("outside/x.png").exists() && !f.0.join("outside/layers").exists()); +} + +/// Every command, given paths outside the roots in its usual path parameters, writes nothing +/// there and opens nothing from there: they all reach files through `file_access`. +#[test] +fn no_command_reaches_outside_the_roots() { + let f = Fixture::new("sweep"); + let roots = Arc::new(f.rw()); + let outside = f.0.join("outside"); + // Bait the sweep could pick up: a document, an SVG, a PNG and variables JSON, each holding a + // name that must never show up in a document or a result. + let mut s = Session::new(); + s.execute("frame.create", &json!({"x": 0, "y": 0, "width": 20, "height": 10, "name": "BAIT-LAYER"})).unwrap(); + s.execute("file.saveAs", &json!({"path": f.p("outside/bait.figcraft")})).unwrap(); + s.execute("file.export", &json!({"format": "png", "path": f.p("outside/bait.png")})).unwrap(); + std::fs::write( + outside.join("bait.svg"), + r#""#, + ) + .unwrap(); + let before: Vec<_> = std::fs::read_dir(&outside).unwrap().flatten().map(|e| e.file_name()).collect(); + // The bait is what an unconfined command finds. + for (cmd, bait) in [("file.open", "bait.figcraft"), ("file.import", "bait.svg")] { + let mut s = Session::new(); + s.execute(cmd, &json!({"path": f.p(&format!("outside/{bait}"))})).unwrap(); + assert!(crate::inspect::document(&s, 64, true).to_string().contains("BAIT-LAYER"), "{cmd} {bait}"); + } + let mut s = Session::new(); + s.execute("file.placeImage", &json!({"path": f.p("outside/bait.png")})).unwrap(); + assert!(!s.doc().unwrap().images.is_empty()); + for bait in ["bait.figcraft", "bait.svg", "bait.png", "new.figcraft", "new.png"] { + let path = f.p(&format!("outside/{bait}")); + let params = json!({"path": path, "dir": f.p("outside"), "file": path, "variables": path, "folder": f.p("outside"), "confirm": true}); + for c in crate::all_commands() { + let mut s = Session::new(); + let r = confined(Some(&roots), || s.execute(c.id, ¶ms)); + let shown = format!("{r:?}"); + assert!(!shown.contains("BAIT-LAYER"), "{} read {bait}: {shown}", c.id); + let docs = crate::inspect::document(&s, 64, true).to_string(); + assert!(!docs.contains("BAIT-LAYER"), "{} opened {bait}", c.id); + assert!(s.docs.iter().all(|ds| ds.doc.images.is_empty()), "{} placed {bait}", c.id); + let now: Vec<_> = std::fs::read_dir(&outside).unwrap().flatten().map(|e| e.file_name()).collect(); + assert_eq!(now, before, "{} wrote outside the roots with {bait}", c.id); + } + } +} + +#[test] +fn recent_files_outside_the_roots_are_never_opened_removed_or_named() { + let f = Fixture::new("recents"); + let roots = Arc::new(f.rw()); + let (bait, gone, ok) = (f.p("outside/bait.figcraft"), f.p("outside/gone.figcraft"), f.p("root/ok.figcraft")); + let mut s = Session::new(); + s.execute("frame.create", &json!({"x": 0, "y": 0, "width": 20, "height": 10})).unwrap(); + s.execute("file.saveAs", &json!({"path": bait})).unwrap(); + s.execute("file.saveAs", &json!({"path": ok})).unwrap(); + let mut s = Session::new(); + let recents = vec![bait.clone(), gone.clone(), ok.clone()]; + s.prefs.recent_files = recents.clone(); + let quiet = |r: &crate::Result| { + let shown = format!("{r:?}"); + assert!(!shown.contains("bait") && !shown.contains("gone"), "an outside recent file was named: {shown}"); + }; + confined(Some(&roots), || { + let listing = s.execute("file.openRecent", &json!({})); + quiet(&listing); + assert_eq!(listing.unwrap()["files"].as_array().unwrap().len(), 1); + // `index` counts the files listed: 0 is the first one inside the roots. + let opened = s.execute("file.openRecent", &json!({"index": 0})); + quiet(&opened); + assert!(opened.is_ok(), "{opened:?}"); + assert_eq!(s.ds().unwrap().path.as_deref(), Some(ok.as_str())); + for i in [1, 2] { + let r = s.execute("file.openRecent", &json!({"index": i})); + quiet(&r); + assert!(r.is_err()); + } + // A path outside is refused like any outside path, and never dropped from the list. + let e = s.execute("file.openRecent", &json!({"path": gone})).unwrap_err().to_string(); + assert!(e.contains("is outside the folders that may be read") && !e.contains("no longer exists"), "{e}"); + quiet(&s.execute("file.openRecent", &json!({"remove": bait}))); + assert!(s.prefs.recent_files.contains(&bait) && s.prefs.recent_files.contains(&gone)); + // Clear clears what is listed. + quiet(&s.execute("file.openRecent", &json!({"clear": true}))); + }); + assert_eq!(s.prefs.recent_files, vec![bait, gone]); + // Unconfined, a recent file that is gone is still dropped. + s.execute("file.openRecent", &json!({"index": 1})).unwrap_err(); + assert_eq!(s.prefs.recent_files.len(), 1); +} + +#[test] +fn outside_paths_are_refused_alike_whatever_exists_there() { + let f = Fixture::new("oracle"); + let r = f.rw(); + let refusal = |p: &str| r.check(Path::new(p), Access::Write).unwrap_err(); + let standard = |p: &str| format!("automation path rejected: {} is outside the folders that may be written:", Path::new(p).display()); + let mut outside = vec![f.p("outside/secret.txt"), f.p("outside/none.txt"), f.p("outside/missing/../x.txt"), f.p("nowhere/deeper/x.txt")]; + if symlink_file(&f.0.join("outside/not-yet.txt"), &f.0.join("outside/dangling.txt")) { + outside.push(f.p("outside/dangling.txt")); + } + for p in &outside { + let e = refusal(p); + assert!(e.starts_with(&standard(p)), "{p}: {e}"); + } + // Inside a root the reason is given. + if symlink_file(&f.0.join("outside/not-yet.txt"), &f.0.join("root/dangling.txt")) { + assert!(refusal(&f.p("root/dangling.txt")).contains("leads nowhere")); + } +} + +#[test] +fn only_resolved_paths_count_as_the_apps_own_folder() { + let f = Fixture::new("own"); + let install = f.0.join("root"); + assert!(super::within_app_folder(&install.join("in.txt"), &install)); + assert!(!super::within_app_folder(&install.join("..").join("outside").join("secret.txt"), &install), "`..` leaves it"); + assert!(!super::within_app_folder(&f.0.join("root2"), &install), "a folder named like it is another"); + assert!(!super::within_app_folder(&install.join("missing.wasm"), &install)); + assert!(!super::within_app_folder(&install.join("in.txt"), &f.0.join("no-such-folder"))); +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index a0d87f9c..8796dc9c 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -6,6 +6,8 @@ //! change is one undo step. Pointer tools ([`tools`]) and text editing reduce to the same document //! edits and are journaled the same way. #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] +// Tests make and inspect files directly; shipped code goes through `file_access` (clippy.toml). +#![cfg_attr(test, allow(clippy::disallowed_methods))] pub mod al_keys; pub mod annotations; @@ -25,6 +27,7 @@ pub mod devcompare; pub mod devinspect; pub mod distance; pub mod drop; +pub mod file_access; pub mod guard; pub mod guides; pub mod inspect; diff --git a/crates/engine/src/library.rs b/crates/engine/src/library.rs index daa1f5bd..26863561 100644 --- a/crates/engine/src/library.rs +++ b/crates/engine/src/library.rs @@ -541,7 +541,7 @@ fn now() -> Option { #[cfg(not(target_arch = "wasm32"))] fn stat(path: &str) -> std::result::Result { - let m = std::fs::metadata(path).map_err(|e| format!("{path}: {e}"))?; + let m = crate::file_access::metadata(path).map_err(|e| e.to_string())?; if !m.is_file() { return Err(format!("{path} is not a file")); } @@ -555,7 +555,7 @@ fn stat(_path: &str) -> std::result::Result { #[cfg(not(target_arch = "wasm32"))] fn read_file(path: &str) -> std::result::Result { - let f = std::fs::File::open(path).map_err(|e| format!("{path}: {e}"))?; + let f = crate::file_access::open(path).map_err(|e| e.to_string())?; match figcraft_format::read_library(std::io::BufReader::new(f)) { Ok(Some(lib)) => Ok(lib), Ok(None) => Err(format!("{path} has no published library: open it and publish it first (Libraries ▸ Publish library…)")), diff --git a/crates/engine/src/playground.rs b/crates/engine/src/playground.rs index 0023a6c1..64286cef 100644 --- a/crates/engine/src/playground.rs +++ b/crates/engine/src/playground.rs @@ -75,7 +75,7 @@ pub fn specs() -> Vec { if let Some(path) = string(p, "path") { let scale = num(p, "scale").filter(|k| k.is_finite()).map_or(2.0, |k| k.clamp(0.01, 16.0)); let png = crate::cmd::file::export_bytes(©, &[inst], ExportFormat::Png, scale)?; - std::fs::write(&path, png).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + crate::file_access::write(&path, &png)?; out["path"] = json!(path); } Ok(out) diff --git a/crates/engine/src/plugin_directory.rs b/crates/engine/src/plugin_directory.rs index c3bf7dc4..2f721fea 100644 --- a/crates/engine/src/plugin_directory.rs +++ b/crates/engine/src/plugin_directory.rs @@ -147,6 +147,9 @@ fn install(s: &mut Session, p: &Value) -> Result { /// Keep an installed plugin's files in its own folder under the install folder (named by the hash /// of its id, so no id reaches the file system), its module renamed; the manifest's path. Without /// an install folder (the browser, the CLI and tests) the plugin lasts for the session. +// The install folder is the app's own (set by the desktop app, never by a command), so automation +// roots don't apply to it. +#[allow(clippy::disallowed_methods)] fn save(s: &Session, m: &mut Manifest, b: &Bundle) -> std::result::Result, String> { let Some(root) = s.plugins.install_dir.clone() else { return Ok(None) }; #[cfg(not(target_arch = "wasm32"))] @@ -189,6 +192,7 @@ fn save(s: &Session, m: &mut Manifest, b: &Bundle) -> std::result::Result