diff --git a/apps/figcraft-cli/src/files.rs b/apps/figcraft-cli/src/files.rs index 65b19ee46..9dc357b1b 100644 --- a/apps/figcraft-cli/src/files.rs +++ b/apps/figcraft-cli/src/files.rs @@ -29,11 +29,13 @@ fn image_format(e: &str) -> Option { } } +/// Writes through [`figcraft_engine::file_access`], so a run given automation roots can only +/// write inside a write root (#833). Unconfined it writes as before. fn write(path: &str, bytes: &[u8]) -> CliResult<()> { if let Some(dir) = Path::new(path).parent().filter(|d| !d.as_os_str().is_empty()) { - std::fs::create_dir_all(dir).map_err(|e| CliError::Failed(format!("{}: {e}", dir.display())))?; + figcraft_engine::file_access::create_dir_all(dir).map_err(|e| CliError::Failed(e.to_string()))?; } - std::fs::write(path, bytes).map_err(|e| CliError::Failed(format!("{path}: {e}"))) + figcraft_engine::file_access::write(path, bytes).map_err(|e| CliError::Failed(e.to_string())) } fn page_named(doc: &Document, name: Option<&str>) -> CliResult { @@ -244,7 +246,7 @@ pub fn export(file: &str, out: &str, t: &Target) -> CliResult { println!("{out}"); return Ok(ExitCode::SUCCESS); } - std::fs::create_dir_all(out).map_err(|e| CliError::Failed(format!("{out}: {e}")))?; + figcraft_engine::file_access::create_dir_all(out).map_err(|e| CliError::Failed(e.to_string()))?; let ids = match target { Some(t) => vec![t], None => { @@ -329,7 +331,8 @@ pub fn parse_script(text: &str) -> Result, String> { } pub fn run_script(script: &str, input: Option<&str>, out: Option<&str>) -> CliResult { - let text = std::fs::read_to_string(script).map_err(|e| CliError::Failed(format!("{script}: {e}")))?; + let bytes = figcraft_engine::file_access::read(script).map_err(|e| CliError::Failed(e.to_string()))?; + let text = String::from_utf8(bytes).map_err(|_| CliError::Failed(format!("{script}: not UTF-8")))?; let steps = parse_script(&text).map_err(|e| CliError::Failed(format!("{script}: {e}")))?; let mut s = match input { Some(path) => { diff --git a/apps/figcraft-cli/src/main.rs b/apps/figcraft-cli/src/main.rs index 281d79ddc..d9f757d0f 100644 --- a/apps/figcraft-cli/src/main.rs +++ b/apps/figcraft-cli/src/main.rs @@ -22,6 +22,7 @@ USAGE: figcraft-cli convert figcraft-cli render -o [--scale N] [--page NAME] figcraft-cli run [--in ] [--out ] + (info, export, convert, render and run also take --automation-read-root DIR / --automation-write-root DIR) figcraft-cli commands [--json] [--filter TEXT] figcraft-cli mcp [--attach PORT] [--automation-read-root DIR]... [--automation-write-root DIR]... figcraft-cli plugin publish --server ws://host:port/[?key=KEY] @@ -42,12 +43,34 @@ 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). +`--automation-read-root DIR` / `--automation-write-root DIR` (each may be repeated; the folders +must exist) confine the files a subcommand touches: reads only inside a read or write root, writes +only inside a write root, everything else refused -- so giving no write root denies writing +altogether. They are taken by `mcp`, to confine the agent's commands, and by `info`, `export`, +`convert`, `render` and `run`, to confine that one run; without them a subcommand reads and writes +as the user who started it, as before. 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."; +/// The automation roots a file subcommand was given, if any. Omitting both leaves the run +/// unconfined, as before; giving either confines every file the subcommand touches (#833). +const ROOT_FLAGS: [&str; 2] = ["automation-read-root", "automation-write-root"]; + +fn roots_of(a: &Args) -> CliResult>> { + let (read, write) = (a.values("automation-read-root"), a.values("automation-write-root")); + if read.is_empty() && write.is_empty() { + return Ok(None); + } + let roots = figcraft_engine::file_access::Roots::new(read, write).map_err(CliError::Failed)?; + Ok(Some(std::sync::Arc::new(roots))) +} + +/// Runs `f` confined to the roots `a` carries. +fn confined(a: &Args, f: impl FnOnce() -> CliResult) -> CliResult { + let roots = roots_of(a)?; + figcraft_engine::file_access::confined(roots.as_ref(), f) +} + fn main() -> ExitCode { let argv: Vec = std::env::args().skip(1).collect(); match run(&argv) { @@ -98,35 +121,38 @@ fn run(argv: &[String]) -> CliResult { Ok(ExitCode::SUCCESS) } "info" => { - let a = Args::parse(rest, &[], &["json"])?; + let a = Args::parse(rest, &ROOT_FLAGS, &["json"])?; let file = a.one_positional("info ")?; - files::info(&file, a.flag("json")) + confined(&a, || files::info(&file, a.flag("json"))) } "export" => { - let a = Args::parse(rest, &["format", "scale", "page", "node", "o", "out", "bleed"], &["crop-marks", "bleed-marks", "cmyk"])?; + let valued = [&["format", "scale", "page", "node", "o", "out", "bleed"][..], &ROOT_FLAGS[..]].concat(); + let a = Args::parse(rest, &valued, &["crop-marks", "bleed-marks", "cmyk"])?; let file = a.one_positional("export -o ")?; let out = a.value("o").or_else(|| a.value("out")).ok_or_else(|| CliError::Usage("export needs -o ".into()))?; let target = files::Target { format: a.value("format"), scale: a.scale()?, page: a.value("page"), node: a.value("node"), print: a.print()? }; - files::export(&file, &out, &target) + confined(&a, || files::export(&file, &out, &target)) } "convert" => { - let a = Args::parse(rest, &[], &[])?; + let a = Args::parse(rest, &ROOT_FLAGS, &[])?; match a.positional.as_slice() { - [i, o] => files::convert(i, o), + [i, o] => confined(&a, || files::convert(i, o)), _ => Err(CliError::Usage("convert ".into())), } } "render" => { - let a = Args::parse(rest, &["o", "out", "scale", "page"], &[])?; + let valued = [&["o", "out", "scale", "page"][..], &ROOT_FLAGS[..]].concat(); + let a = Args::parse(rest, &valued, &[])?; let file = a.one_positional("render -o ")?; let out = a.value("o").or_else(|| a.value("out")).ok_or_else(|| CliError::Usage("render needs -o ".into()))?; - files::render(&file, &out, a.scale()?, a.value("page").as_deref()) + confined(&a, || files::render(&file, &out, a.scale()?, a.value("page").as_deref())) } "run" => { - let a = Args::parse(rest, &["in", "out"], &[])?; + let valued = [&["in", "out"][..], &ROOT_FLAGS[..]].concat(); + let a = Args::parse(rest, &valued, &[])?; let script = a.one_positional("run ")?; - files::run_script(&script, a.value("in").as_deref(), a.value("out").as_deref()) + confined(&a, || files::run_script(&script, a.value("in").as_deref(), a.value("out").as_deref())) } "commands" => { let a = Args::parse(rest, &["filter"], &["json"])?; diff --git a/apps/figcraft-cli/tests/cli.rs b/apps/figcraft-cli/tests/cli.rs index 7eaf051f5..399e57bbd 100644 --- a/apps/figcraft-cli/tests/cli.rs +++ b/apps/figcraft-cli/tests/cli.rs @@ -172,3 +172,70 @@ fn commands_json_lists_everything() { let v: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap(); assert!(v.as_array().unwrap().len() > 200); } + +/// The automation roots confine the file subcommands too, not just `mcp` (#833): a read outside a +/// root, and a write outside a write root, are refused and leave no file behind. Without the flags +/// the same commands behave as they always did. +#[test] +fn file_subcommands_are_confined_to_automation_roots() { + let d = tmp("file-roots"); + let _ = std::fs::remove_dir_all(&d); + for sub in ["in", "out"] { + std::fs::create_dir_all(d.join(sub)).unwrap(); + } + let p = |rel: &str| d.join(rel).to_string_lossy().into_owned(); + let out_of = |o: &std::process::Output| String::from_utf8_lossy(&o.stderr).into_owned(); + + // A document to work on, made unconfined. + std::fs::write(d.join("in/make.json"), br#"[{"command":"shape.rectangle","params":{"x":0,"y":0,"width":40,"height":20}}]"#).unwrap(); + let made = bin().args(["run", &p("in/make.json"), "--out", &p("in/doc.figcraft")]).output().unwrap(); + assert!(made.status.success(), "{}", out_of(&made)); + + // Inside the roots: renders as usual. + let ok = bin() + .args(["render", &p("in/doc.figcraft"), "-o", &p("out/ok.png"), "--automation-read-root", &p("in"), "--automation-write-root", &p("out")]) + .output() + .unwrap(); + assert!(ok.status.success(), "{}", out_of(&ok)); + assert!(d.join("out/ok.png").is_file()); + + // A write outside the write root is refused, and nothing is written. + let escaped = d.join("escaped.png"); + let refused = bin() + .args([ + "render", + &p("in/doc.figcraft"), + "-o", + &escaped.to_string_lossy(), + "--automation-read-root", + &p("in"), + "--automation-write-root", + &p("out"), + ]) + .output() + .unwrap(); + assert!(!refused.status.success()); + assert!(out_of(&refused).contains("may be written"), "{}", out_of(&refused)); + assert!(!escaped.exists(), "the refused render wrote a file anyway"); + + // No write root at all: writing is refused outright, reading still works. + let no_write = bin().args(["render", &p("in/doc.figcraft"), "-o", &p("out/no.png"), "--automation-read-root", &p("in")]).output().unwrap(); + assert!(!no_write.status.success()); + assert!(out_of(&no_write).contains("no folder may be written"), "{}", out_of(&no_write)); + + // A read outside every root is refused: `info` on the document with only `out` readable. + let read_refused = bin().args(["info", &p("in/doc.figcraft"), "--automation-read-root", &p("out")]).output().unwrap(); + assert!(!read_refused.status.success()); + assert!(out_of(&read_refused).contains("may be read"), "{}", out_of(&read_refused)); + + // `convert` and `run` take them as well, and a root that is not a folder is refused. + let missing = bin().args(["convert", &p("in/doc.figcraft"), &p("out/c.svg"), "--automation-read-root", &p("missing")]).output().unwrap(); + assert!(!missing.status.success()); + assert!(out_of(&missing).contains("automation-read-root"), "{}", out_of(&missing)); + + // Unconfined, the same render still writes wherever it is told. + let plain = bin().args(["render", &p("in/doc.figcraft"), "-o", &escaped.to_string_lossy()]).output().unwrap(); + assert!(plain.status.success(), "{}", out_of(&plain)); + assert!(escaped.is_file()); + let _ = std::fs::remove_dir_all(&d); +} diff --git a/docs/cli.md b/docs/cli.md index 7af73fd60..699b6ba27 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -153,6 +153,28 @@ figcraft-cli run start.json --out dashboard.figcraft figcraft-cli export dashboard.figcraft -o dashboard.png ``` +### Confining a run: `--automation-read-root DIR` / `--automation-write-root DIR` + +`info`, `export`, `convert`, `render` and `run` take these two flags, with the same meaning they +have on [`mcp`](#mcp---attach-port---automation-read-root-dir---automation-write-root-dir): each is +repeatable, each folder must exist, and together they confine every file that subcommand touches. +Reads are allowed inside a read or write root, writes only inside a write root, and everything else +is refused — so a run given only a read root cannot write at all. Without either flag a subcommand +reads and writes as the user who started it. + +```sh +# Renders in/doc.figcraft to out/ok.png and can touch nothing else. +figcraft-cli render in/doc.figcraft -o out/ok.png \ + --automation-read-root in --automation-write-root out + +figcraft-cli render in/doc.figcraft -o ./escaped.png \ + --automation-read-root in --automation-write-root out +# error: automation path rejected: . is outside the folders that may be written: …/out +``` + +This is for driving the CLI from a script or an agent that should not reach the rest of the disk; +the flags leave the output identical when nothing strays outside the roots. + ### `commands [--json] [--filter TEXT]` Every command with its label, shortcut, undoable flag (`*`) and parameter docs; `--json` for