CLI: add --automation-read-root and --automation-write-root to mcp

apps/designcraft-cli/src/automation_args.rs

@@ -0,0 +1,90 @@
+//! Parse `--automation-read-root` / `--automation-write-root` before the server starts. A malformed
+//! flag must not silently remove a root and leave the server unconfined.
+
+use std::path::PathBuf;
+
+pub struct AutomationArgs {
+    pub read_root: Option<PathBuf>,
+    pub write_root: Option<PathBuf>,
+    read_given: bool,
+    write_given: bool,
+}
+
+impl AutomationArgs {
+    pub fn new(read_root: Option<PathBuf>, write_root: Option<PathBuf>) -> Self {
+        Self { read_root, write_root, read_given: false, write_given: false }
+    }
+
+    /// Returns whether this argument is a root flag. A second occurrence of a flag is an error,
+    /// including when the two spellings (`--flag DIR`, `--flag=DIR`) are mixed.
+    pub fn consume(&mut self, arg: &str, args: &mut impl Iterator<Item = String>) -> Result<bool, String> {
+        let (flag, inline) = arg.split_once('=').map_or((arg, None), |(flag, value)| (flag, Some(value)));
+        let (root, given) = match flag {
+            "--automation-read-root" => (&mut self.read_root, &mut self.read_given),
+            "--automation-write-root" => (&mut self.write_root, &mut self.write_given),
+            _ => return Ok(false),
+        };
+        if *given {
+            return Err(format!("{flag} given twice"));
+        }
+        let value = match inline {
+            Some(value) => value.to_owned(),
+            None => {
+                let value = args.next().ok_or_else(|| format!("{flag} needs a folder"))?;
+                if value.starts_with('-') {
+                    return Err(format!("{flag} needs a folder; use {flag}=<dir> for a folder starting with '-'"));
+                }
+                value
+            }
+        };
+        if value.is_empty() {
+            return Err(format!("{flag} needs a non-empty folder"));
+        }
+        *root = Some(value.into());
+        *given = true;
+        Ok(true)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn roots_are_taken_once_and_other_arguments_are_left() {
+        let mut roots = AutomationArgs::new(None, None);
+        let mut args = ["a folder".to_owned(), "doc.designcraft".to_owned()].into_iter();
+        assert!(roots.consume("--automation-read-root", &mut args).unwrap());
+        assert!(roots.consume("--automation-write-root=out", &mut args).unwrap());
+        assert!(!roots.consume("--sample", &mut args).unwrap());
+        assert_eq!(roots.read_root, Some("a folder".into()));
+        assert_eq!(roots.write_root, Some("out".into()));
+        assert_eq!(args.next().as_deref(), Some("doc.designcraft"));
+        assert!(roots.consume("--automation-read-root=other", &mut args).is_err());
+        assert_eq!(roots.read_root, Some("a folder".into()));
+    }
+
+    #[test]
+    fn invalid_operands_leave_roots_unchanged() {
+        for flag in ["--automation-read-root", "--automation-write-root"] {
+            for operand in [None, Some(""), Some("--version"), Some("--connect"), Some("-folder")] {
+                let mut roots = AutomationArgs::new(Some("read".into()), Some("write".into()));
+                let mut args = operand.map(str::to_owned).into_iter();
+                assert!(roots.consume(flag, &mut args).is_err(), "{flag} {operand:?}");
+                assert_eq!(roots.read_root, Some("read".into()));
+                assert_eq!(roots.write_root, Some("write".into()));
+            }
+            let mut roots = AutomationArgs::new(None, None);
+            assert!(roots.consume(&format!("{flag}="), &mut std::iter::empty()).is_err());
+        }
+    }
+
+    #[test]
+    fn explicit_dash_folder_and_relative_dash_folder_are_valid() {
+        let mut roots = AutomationArgs::new(None, None);
+        assert!(roots.consume("--automation-read-root=-folder", &mut std::iter::empty()).unwrap());
+        assert!(roots.consume("--automation-write-root", &mut ["./-folder".to_owned()].into_iter()).unwrap());
+        assert_eq!(roots.read_root, Some("-folder".into()));
+        assert_eq!(roots.write_root, Some("./-folder".into()));
+    }
+}

apps/designcraft-cli/src/main.rs

@@ -9,7 +9,8 @@
 //!                                      # run a command script (crates/engine/src/script.rs): `$N.path` references
 //! designcraft-cli app [--port PORT] COMMAND [JSON]   # run a command in the running app (designcraft --control PORT)
 //! designcraft-cli app [--port PORT] --method METHOD [JSON]   # any control-channel method (ui.screenshot, ui.render, …)
-//! designcraft-cli mcp [--connect PORT] [--sample]  # MCP server over stdio (docs/mcp.md)
+//! designcraft-cli mcp [--connect PORT] [--sample] [--automation-read-root DIR] [--automation-write-root DIR]
+//!                                      # MCP server over stdio (docs/mcp.md); the roots confine the files it reads and writes
 //! designcraft-cli perf [--pages N] [--frames N] [--chars N] [--images N] [--runs N] [--strict]  # budgets on a synthetic stress document
 //! designcraft-cli bench FILE [--runs N]  # the same measurements on one document
 //! designcraft-cli validate-pdf FILE.pdf    # run the built-in PDF/X-4 conformance checks
@@ -30,13 +31,14 @@ macro_rules! outln {
     }};
 }
 
+mod automation_args;
 mod perf;
 
 use designcraft_engine::Session;
 use serde_json::{Value, json};
 
 /// The usage block. Shared by `--help` (stdout, success) and an unknown command (stderr, failure).
-const USAGE: &str = "usage: designcraft-cli run [--in FILE | --sample] [--cmd ID[=JSON]]... [--page N] [--scale S] [--pdf-options JSON] [--export OUT] [--all-pages DIR]\n         (--page, --scale and --pdf-options apply to the exports that follow them)\n       designcraft-cli commands [FILTER]\n       designcraft-cli describe COMMAND\n       designcraft-cli script [FILE|-] [--in FILE | --sample] [--connect PORT] [--save OUT] [--export OUT] [--keep-going]\n       designcraft-cli app [--port PORT] COMMAND [JSON] | --method METHOD [JSON]\n       designcraft-cli mcp [--connect PORT] [--sample]\n       designcraft-cli perf [--pages N] [--runs N] [--strict]\n       designcraft-cli bench FILE [--runs N]\n       designcraft-cli links\n       designcraft-cli --version";
+const USAGE: &str = "usage: designcraft-cli run [--in FILE | --sample] [--cmd ID[=JSON]]... [--page N] [--scale S] [--pdf-options JSON] [--export OUT] [--all-pages DIR]\n         (--page, --scale and --pdf-options apply to the exports that follow them)\n       designcraft-cli commands [FILTER]\n       designcraft-cli describe COMMAND\n       designcraft-cli script [FILE|-] [--in FILE | --sample] [--connect PORT] [--save OUT] [--export OUT] [--keep-going]\n       designcraft-cli app [--port PORT] COMMAND [JSON] | --method METHOD [JSON]\n       designcraft-cli mcp [--connect PORT] [--sample] [--automation-read-root DIR] [--automation-write-root DIR]\n         the two roots confine every file the agent's commands read (open, placed images, data sources, libraries) and\n         write (save, export, package) to those folders, links followed. A root left out grants none of its access.\n         They run the in-process server, so they conflict with --connect (a running app is not confined by them).\n       designcraft-cli perf [--pages N] [--runs N] [--strict]\n       designcraft-cli bench FILE [--runs N]\n       designcraft-cli links\n       designcraft-cli --version";
 
 /// The links line under the usage block.
 fn usage_footer() -> String {
@@ -129,16 +131,31 @@ fn validate_pdf(path: Option<&str>) -> Result<(), String> {
 /// `mcp` (headless, in-process engine) or `mcp --connect PORT|HOST:PORT` (drive a running app
 /// started with `designcraft --control PORT`). JSON-RPC on stdin/stdout; logs on stderr.
 fn mcp(args: &[String]) -> Result<(), String> {
+    use designcraft_engine::file_access::{self, AutomationRoots};
     use designcraft_mcp::{Backend, Headless, Remote, Server, control_addr};
     let mut connect: Option<String> = None;
     let mut sample = false;
-    let mut it = args.iter();
+    let mut roots_args = automation_args::AutomationArgs::new(None, None);
+    let mut it = args.iter().cloned();
     while let Some(a) = it.next() {
-        match a.as_str() {
-            "--connect" => connect = Some(it.next().cloned().ok_or("--connect needs a port or host:port")?),
-            "--sample" => sample = true,
-            other => return Err(format!("unknown mcp option `{other}` (usage: designcraft-cli mcp [--connect PORT] [--sample])")),
+        if roots_args.consume(&a, &mut it)? {
+            continue;
         }
+        match a.as_str() {
+            "--connect" => connect = Some(it.next().ok_or("--connect needs a port or host:port")?),
+            "--sample" => sample = true,
+            other => {
+                return Err(format!(
+                    "unknown mcp option `{other}` (usage: designcraft-cli mcp [--connect PORT] [--sample] [--automation-read-root DIR] [--automation-write-root DIR])"
+                ));
+            }
+        }
+    }
+    let roots = AutomationRoots::new(roots_args.read_root.as_deref(), roots_args.write_root.as_deref())?;
+    if roots.is_some() && connect.is_some() {
+        return Err("--automation-read-root and --automation-write-root confine the in-process server; a running app is not confined by them, \
+so drop --connect (or use --connect without the roots)"
+            .into());
     }
     let backend: Box<dyn Backend> = match connect {
         Some(c) => {
@@ -150,9 +167,17 @@ fn mcp(args: &[String]) -> Result<(), String> {
         }
         None => {
             let mut h = Headless::with_document();
+            // `--sample` is the operator's own choice, made before the roots come into force.
             if sample {
                 h.session.execute("file.newSample", &json!({})).map_err(|e| e.to_string())?;
             }
+            if let Some(roots) = roots {
+                // Every thread of this process, not only the server's: nothing reaches past the roots.
+                file_access::confine_process(roots.clone());
+                let show = |root: Option<&std::path::Path>| root.map_or_else(|| "none".to_string(), |r| r.display().to_string());
+                eprintln!("designcraft-cli: files confined: read root {}, write root {}", show(roots.read_root()), show(roots.write_root()));
+                h = h.with_automation_roots(Some(roots));
+            }
             Box::new(h)
         }
     };

apps/designcraft-cli/tests/cli.rs

@@ -154,3 +154,124 @@ fn raster_export_preserves_an_existing_reader_and_private_symlink_target() {
     assert_eq!(std::fs::metadata(&target).unwrap().permissions().mode() & 0o777, 0o600);
     assert!(std::fs::symlink_metadata(alias).unwrap().file_type().is_symlink());
 }
+
+/// Send `calls` (tool name, arguments) to `designcraft-cli mcp <args>`; returns whether it exited
+/// cleanly, its stderr, and the stdout lines after `initialize`, one per call.
+fn mcp_session(args: &[&str], calls: &[(&str, serde_json::Value)]) -> (bool, String, Vec<serde_json::Value>) {
+    let mut child = cli().arg("mcp").args(args).stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap();
+    {
+        let stdin = child.stdin.as_mut().unwrap();
+        writeln!(stdin, r#"{{"jsonrpc":"2.0","id":1,"method":"initialize","params":{{"protocolVersion":"2025-06-18"}}}}"#).unwrap();
+        for (i, (name, arguments)) in calls.iter().enumerate() {
+            let call = serde_json::json!({"jsonrpc": "2.0", "id": i + 2, "method": "tools/call", "params": {"name": name, "arguments": arguments}});
+            writeln!(stdin, "{call}").unwrap();
+        }
+    }
+    let out = child.wait_with_output().unwrap();
+    let stdout = String::from_utf8(out.stdout).unwrap();
+    let replies = stdout.lines().skip(1).map(|l| serde_json::from_str(l).unwrap()).collect();
+    (out.status.success(), String::from_utf8_lossy(&out.stderr).into_owned(), replies)
+}
+
+fn text(reply: &serde_json::Value) -> &str {
+    reply["result"]["content"][0]["text"].as_str().unwrap()
+}
+
+/// A `.designcraft` file with one page, made with the CLI itself.
+fn make_document(path: &std::path::Path) {
+    let out = cli().args(["run", "--cmd", "file.new={}", "--cmd"]).arg(format!("file.save={}", serde_json::json!({"path": path}))).output().unwrap();
+    assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr));
+    assert!(path.exists());
+}
+
+#[test]
+fn mcp_automation_roots_confine_the_server() {
+    let base = std::env::temp_dir().join(format!("dc-cli-roots-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&base);
+    let (work, elsewhere) = (base.join("work"), base.join("elsewhere"));
+    std::fs::create_dir_all(&work).unwrap();
+    std::fs::create_dir_all(&elsewhere).unwrap();
+    make_document(&work.join("in.designcraft"));
+    make_document(&elsewhere.join("secret.designcraft"));
+    let work_s = work.to_str().unwrap();
+    let secret = elsewhere.join("secret.designcraft").to_str().unwrap().to_string();
+
+    // Both spellings of the flags; inside works, outside is refused with a readable message.
+    let read_flag = format!("--automation-read-root={work_s}");
+    let calls = [
+        ("open_document", serde_json::json!({"path": work.join("in.designcraft").to_str().unwrap()})),
+        ("open_document", serde_json::json!({"path": secret})),
+        ("save_document", serde_json::json!({"path": work.join("saved.designcraft").to_str().unwrap()})),
+        ("save_document", serde_json::json!({"path": elsewhere.join("saved.designcraft").to_str().unwrap()})),
+        ("export_png", serde_json::json!({"path": elsewhere.join("page.png").to_str().unwrap()})),
+        ("render_page", serde_json::json!({"path": elsewhere.join("render.png").to_str().unwrap()})),
+    ];
+    let (success, stderr, replies) = mcp_session(&[&read_flag, "--automation-write-root", work_s], &calls);
+    assert!(success, "{stderr}");
+    assert!(stderr.contains("files confined"), "{stderr}");
+    assert_eq!(replies[0]["result"]["isError"], false, "{}", replies[0]);
+    assert_eq!(replies[1]["result"]["isError"], true);
+    assert!(text(&replies[1]).contains("outside the read root"), "{}", text(&replies[1]));
+    assert_eq!(replies[2]["result"]["isError"], false, "{}", replies[2]);
+    assert!(work.join("saved.designcraft").exists());
+    for reply in &replies[3..] {
+        assert_eq!(reply["result"]["isError"], true, "{reply}");
+        assert!(text(reply).contains("outside the write root"), "{}", text(reply));
+    }
+    for name in ["saved.designcraft", "page.png", "render.png"] {
+        assert!(!elsewhere.join(name).exists(), "{name} was written outside the write root");
+    }
+
+    // A root left out grants none of its access.
+    let save = [("new_document", serde_json::json!({})), calls[2].clone()];
+    let (_, _, replies) = mcp_session(&[&read_flag], &save);
+    assert_eq!(replies[1]["result"]["isError"], true);
+    assert!(text(&replies[1]).contains("write authority is absent"), "{}", text(&replies[1]));
+    let (_, _, replies) = mcp_session(&["--automation-write-root", work_s], &calls[..1]);
+    assert_eq!(replies[0]["result"]["isError"], true);
+    assert!(text(&replies[0]).contains("read authority is absent"), "{}", text(&replies[0]));
+
+    // No flags: nothing is confined.
+    let (success, _, replies) = mcp_session(&[], &calls[1..2]);
+    assert!(success);
+    assert_eq!(replies[0]["result"]["isError"], false, "{}", replies[0]);
+    let _ = std::fs::remove_dir_all(base);
+}
+
+#[test]
+fn mcp_automation_root_flags_are_checked_up_front() {
+    let base = std::env::temp_dir().join(format!("dc-cli-roots-args-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&base);
+    std::fs::create_dir_all(&base).unwrap();
+    let dir = base.to_str().unwrap();
+    let missing = base.join("missing");
+    let fails = |args: &[&str], needle: &str| {
+        let mut full = vec!["mcp"];
+        full.extend_from_slice(args);
+        let o = cli().args(&full).output().unwrap();
+        assert!(!o.status.success(), "{args:?}");
+        let err = String::from_utf8_lossy(&o.stderr).into_owned();
+        assert!(err.contains(needle), "{args:?}: {err}");
+    };
+    // The flags confine the in-process server, so they cannot be combined with --connect.
+    fails(&["--connect", "7979", "--automation-read-root", dir], "--connect");
+    fails(&["--automation-write-root", dir, "--connect", "7979"], "--connect");
+    fails(&["--automation-read-root", dir, "--automation-read-root", dir], "given twice");
+    fails(&["--automation-write-root", dir, "--automation-write-root", dir], "given twice");
+    fails(&["--automation-read-root", dir, "--automation-read-root=x"], "given twice");
+    fails(&["--automation-read-root"], "needs a folder");
+    fails(&["--automation-write-root", "--sample"], "needs a folder");
+    fails(&["--automation-read-root", missing.to_str().unwrap()], "cannot open the automation read root");
+    fails(&["--automation-write-root="], "needs a non-empty folder");
+    // A file is not a folder.
+    std::fs::write(base.join("f.txt"), "x").unwrap();
+    fails(&["--automation-write-root", base.join("f.txt").to_str().unwrap()], "cannot open the automation write root");
+    let _ = std::fs::remove_dir_all(base);
+}
+
+#[test]
+fn usage_lists_the_automation_root_flags() {
+    let out = cli().arg("--help").output().unwrap();
+    let usage = String::from_utf8_lossy(&out.stdout);
+    assert!(usage.contains("--automation-read-root DIR") && usage.contains("--automation-write-root DIR"), "{usage}");
+}