MCP: confine file access with --automation-read-root and --automation-write-root (#1448)

* Automation: check the read and write roots on every path a tool reads or writes

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

* Docs: document --automation-read-root and --automation-write-root for mcp

* #1448: count main's doc_batch helpers as resolved by their caller

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

apps/pdfcraft-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(), "report.pdf".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("--compact", &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("report.pdf"));
+        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("--root"), 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/pdfcraft-cli/src/main.rs

@@ -17,6 +17,8 @@
 
 #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
 
+#[cfg(feature = "mcp")]
+mod automation_args;
 mod batch;
 
 use std::io::Write;
@@ -33,8 +35,14 @@ use pdfcraft_render::{PageRenderer, RenderConfig, RenderRequest, RequestKind, in
 macro_rules! mcp_usage {
     () => {
         "\
-pdfcraft-cli mcp    [--root DIR] [--compact]              MCP server on stdin/stdout (opt-in)
+pdfcraft-cli mcp    [--root DIR] [--compact] [--automation-read-root DIR] [--automation-write-root DIR]
+                                                           MCP server on stdin/stdout (opt-in)
                                                            --compact lists a core set of tools plus tool_search and tool_call
+                                                           --automation-read-root / --automation-write-root confine the files the
+                                                           agent's tools read (open, combine, import, fonts, certificates…) and
+                                                           write (save, export, extract, split, reports…) to DIR, links followed;
+                                                           a root left out grants none of its access. --root, when also given,
+                                                           applies as well
 "
     };
 }
@@ -901,11 +909,42 @@ fn run(args: &[String]) -> Result<(), CliError> {
     print_output(&auto, content, flag(args, "--out"))
 }
 
+/// Take `--automation-read-root` / `--automation-write-root` (each `--flag DIR` or `--flag=DIR`,
+/// once) out of `args`, and open the roots, which must be existing folders. Returns the arguments
+/// that are left. A malformed root flag is an error, never a root silently left out.
+#[cfg(feature = "mcp")]
+fn automation_roots(command: &str, args: &[String]) -> Result<(Option<Arc<pdfcraft_automation::file_access::AutomationRoots>>, Vec<String>), String> {
+    let mut found = automation_args::AutomationArgs::new(None, None);
+    let mut left = Vec::new();
+    let mut it = args.iter().cloned();
+    while let Some(arg) = it.next() {
+        if !found.consume(&arg, &mut it).map_err(|e| format!("{command}: {e}"))? {
+            left.push(arg);
+        }
+    }
+    let roots = pdfcraft_automation::file_access::AutomationRoots::new(found.read_root.as_deref(), found.write_root.as_deref())
+        .map_err(|e| format!("{command}: {e}"))?;
+    Ok((roots, left))
+}
+
 #[cfg(feature = "mcp")]
 fn mcp(args: &[String]) -> Result<(), CliError> {
+    let (roots, args) = automation_roots("mcp", args)?;
+    let args = args.as_slice();
     validate_options("mcp", args, MCP_OPTIONS)?;
     let compact = args.iter().any(|a| a == "--compact");
-    let mut server = pdfcraft_automation::mcp::McpServer::new(automation(args)?).with_compact(compact);
+    let mut auto = automation(args)?;
+    if let Some(roots) = roots {
+        let show = |root: Option<&Path>| root.map_or_else(|| "none".to_string(), |r| r.display().to_string());
+        let _ = writeln!(
+            std::io::stderr().lock(),
+            "pdfcraft-cli: files confined: read root {}, write root {}",
+            show(roots.read_root()),
+            show(roots.write_root())
+        );
+        auto = auto.with_automation_roots(Some(roots));
+    }
+    let mut server = pdfcraft_automation::mcp::McpServer::new(auto).with_compact(compact);
     let _ = writeln!(
         std::io::stderr().lock(),
         "pdfcraft-cli: MCP server on stdio (protocol {}{}); close stdin to stop",

apps/pdfcraft-cli/tests/automation_roots.rs

@@ -0,0 +1,123 @@
+//! `pdfcraft-cli mcp --automation-read-root DIR --automation-write-root DIR`, on the real binary:
+//! both spellings of the flags confine the server, and a malformed flag stops it instead of
+//! leaving it unconfined.
+#![cfg(feature = "mcp")]
+
+use std::io::Write as _;
+use std::path::{Path, PathBuf};
+use std::process::{Command, Output, Stdio};
+
+use serde_json::{Value, json};
+
+const BIN: &str = env!("CARGO_BIN_EXE_pdfcraft-cli");
+
+/// `in/a.pdf` and `outside/secret.pdf`, blank one-page PDFs made by the binary itself.
+fn folders(tag: &str) -> PathBuf {
+    let base = std::env::temp_dir().join(format!("pdfcraft-cli-roots-{tag}-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&base);
+    for d in ["in", "out", "outside"] {
+        std::fs::create_dir_all(base.join(d)).unwrap();
+    }
+    for f in ["in/a.pdf", "outside/secret.pdf"] {
+        let script = json!([
+            {"tool": "doc_create", "args": {"from": "blank", "pages": 1, "width": 100, "height": 100}},
+            {"tool": "doc_save", "args": {"doc": 1, "path": f}},
+        ]);
+        std::fs::write(base.join("steps.json"), script.to_string()).unwrap();
+        let out = Command::new(BIN).args(["run", "--script"]).arg(base.join("steps.json")).arg("--root").arg(&base).output().unwrap();
+        assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr));
+    }
+    base
+}
+
+/// Run `mcp <flags>`, send `calls` as `tools/call` requests and return the replies.
+fn session(flags: &[String], calls: &[(&str, Value)]) -> (Vec<Value>, String) {
+    let mut child = Command::new(BIN).arg("mcp").args(flags).stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap();
+    let mut input = String::from("{\"jsonrpc\":\"2.0\",\"id\":0,\"method\":\"initialize\",\"params\":{}}\n");
+    for (i, (name, args)) in calls.iter().enumerate() {
+        let r = json!({"jsonrpc": "2.0", "id": i + 1, "method": "tools/call", "params": {"name": name, "arguments": args}});
+        input.push_str(&format!("{r}\n"));
+    }
+    child.stdin.take().unwrap().write_all(input.as_bytes()).unwrap();
+    let out = child.wait_with_output().unwrap();
+    assert!(out.status.success());
+    let replies = String::from_utf8(out.stdout).unwrap().lines().skip(1).map(|l| serde_json::from_str(l).unwrap()).collect();
+    (replies, String::from_utf8(out.stderr).unwrap())
+}
+
+fn s(p: &Path) -> String {
+    p.to_string_lossy().into_owned()
+}
+
+fn flags(spelling: &str, base: &Path) -> Vec<String> {
+    let (r, w) = (s(&base.join("in")), s(&base.join("out")));
+    if spelling == "equals" {
+        vec![format!("--automation-read-root={r}"), format!("--automation-write-root={w}")]
+    } else {
+        vec!["--automation-read-root".into(), r, "--automation-write-root".into(), w]
+    }
+}
+
+fn text(reply: &Value) -> String {
+    reply["result"]["content"][0]["text"].as_str().unwrap_or_default().to_owned()
+}
+
+#[test]
+fn both_spellings_confine_the_server() {
+    for spelling in ["equals", "separate"] {
+        let base = folders(spelling);
+        let calls = [
+            ("doc_open", json!({"path": s(&base.join("in/a.pdf"))})),
+            ("doc_open", json!({"path": s(&base.join("outside/secret.pdf"))})),
+            ("doc_save", json!({"doc": 1, "path": s(&base.join("out/copy.pdf"))})),
+            ("doc_save", json!({"doc": 1, "path": s(&base.join("outside/copy.pdf"))})),
+        ];
+        let (replies, stderr) = session(&flags(spelling, &base), &calls);
+        assert_eq!(replies[0]["result"]["isError"], false, "{spelling}: {replies:?}");
+        assert!(text(&replies[1]).contains("automation path rejected: outside the read root"), "{spelling}: {replies:?}");
+        assert_eq!(replies[2]["result"]["isError"], false, "{spelling}: {replies:?}");
+        assert!(text(&replies[3]).contains("automation path rejected: outside the write root"), "{spelling}: {replies:?}");
+        assert!(base.join("out/copy.pdf").is_file() && !base.join("outside/copy.pdf").exists());
+        assert!(stderr.contains("files confined: read root") && stderr.contains("write root"), "{stderr}");
+    }
+}
+
+#[test]
+fn a_root_left_out_grants_none_of_its_access() {
+    let base = folders("one");
+    let read_only = vec!["--automation-read-root".to_string(), s(&base.join("in"))];
+    let calls = [("doc_open", json!({"path": s(&base.join("in/a.pdf"))})), ("doc_save", json!({"doc": 1, "path": s(&base.join("out/x.pdf"))}))];
+    let (replies, _) = session(&read_only, &calls);
+    assert_eq!(replies[0]["result"]["isError"], false);
+    assert!(text(&replies[1]).contains("write authority is absent"), "{replies:?}");
+    assert!(!base.join("out/x.pdf").exists());
+}
+
+#[test]
+fn no_flags_means_no_confinement() {
+    let base = folders("none");
+    let (replies, stderr) = session(&[], &[("doc_open", json!({"path": s(&base.join("outside/secret.pdf"))}))]);
+    assert_eq!(replies[0]["result"]["isError"], false);
+    assert!(!stderr.contains("confined"), "{stderr}");
+}
+
+fn fails(args: &[&str]) -> String {
+    let out: Output = Command::new(BIN).arg("mcp").args(args).stdin(Stdio::null()).output().unwrap();
+    assert_eq!(out.status.code(), Some(1), "{args:?} should fail");
+    assert!(out.stdout.is_empty(), "{args:?} must not start a server");
+    String::from_utf8(out.stderr).unwrap()
+}
+
+#[test]
+fn malformed_flags_are_errors_not_a_missing_root() {
+    let dir = s(&std::env::temp_dir());
+    assert!(fails(&["--automation-read-root"]).contains("--automation-read-root needs a folder"));
+    assert!(fails(&["--automation-write-root", "--compact"]).contains("--automation-write-root needs a folder; use --automation-write-root=<dir>"));
+    assert!(fails(&["--automation-read-root="]).contains("--automation-read-root needs a non-empty folder"));
+    assert!(fails(&["--automation-read-root", &dir, "--automation-read-root", &dir]).contains("--automation-read-root given twice"));
+    assert!(fails(&["--automation-write-root", &dir, &format!("--automation-write-root={dir}")]).contains("--automation-write-root given twice"));
+    let missing = fails(&["--automation-read-root", "/definitely/not/a/folder"]);
+    assert!(missing.contains("mcp: cannot open the automation read root"), "{missing}");
+    let file = std::env::current_exe().unwrap();
+    assert!(fails(&["--automation-write-root", &s(&file)]).contains("cannot open the automation write root"));
+}