Security: --root confinement leaks and escapes (#136, #137) (#164)

* Automation: --root refuses every outside path the same way (#136)

With a root set, `resolve` canonicalized a path before checking it against
the root, so an existing outside path got "is outside the allowed
directory" while a missing one got the OS error ("No such file or
directory", "Not a directory", "invalid path"): a confined MCP client or
script could probe which files and folders exist anywhere on the disk.

Containment is now decided before anything about an outside path is
reported:
- `..` is resolved by name first (as Windows does), so a path can't pass
  through an outside folder on its way back in;
- on Windows another network share or device namespace (`\\host\share`,
  `\\?\UNC\...`, `\\.\...`) is refused by name, without contacting it;
- the deepest existing ancestor, links followed, must be inside the root
  before anything is said about the rest;
- a broken link is refused the same way, so whether its target exists
  stays hidden too.

Every outside path now gets `<path> is outside the allowed directory
<root>`, existing or not, file or folder, read or write. Filesystem errors
such as "not found" are still reported for paths inside the root.

Behaviour change: on Unix, `..` now means the parent by name, as on
Windows (`link/../x` is `x` beside `link`), and a write such as
`new/../x.pdf` that stays in the root now succeeds instead of failing with
"'..' is not allowed here".

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

* CLI: script images stay inside --root (#137)

`pdfcraft-cli run --script steps.json --root DIR` wrote each step's
`"out"` PNG with `std::fs::write`, never through the root check the tools
use: an absolute `"out"` created or replaced any file the process could
write, and a relative one landed in the working directory instead of the
root.

The image is now saved through the new `Automation::write_output`, which
uses the same resolver and atomic write as a tool's own output: outside
paths get the usual "is outside the allowed directory" refusal (the step
fails, nothing is written), relative paths resolve inside the root, and
without --root the path is written as given, in place as before (so
`/dev/stdout` still works). `run <tool> --out` goes the same way.

Found while testing this, the same "." problem in two tools:
- a write path naming a folder, such as "." (the root itself), staged its
  temporary file beside that folder, i.e. outside the root for ".",
  overwriting and then deleting any file there called
  `.root.pdfcraft-tmp`. `write_atomic` now refuses a folder before
  writing anything;
- `image_save` adds an extension to a path without one, so "." became
  `root.png` beside the root. It now refuses a folder first.

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

* Automation: files named after a document stay in the output folder

Found while reviewing the --root fixes for #136 and #137. Folder outputs
name their files after the document and joined that name straight onto the
resolved folder, without checking the result against the root:

- `doc_export_images` and `doc_export_all_images` used the whole document
  name, so `doc_create` with `name: "../../escape"` followed by
  `doc_export_images` wrote `escape_page_1.png` outside the root (an
  absolute name went anywhere);
- `page_extract` (separate files), `doc_split`, `ocr_recognize_files` and
  `action_run` took the last component, but on Windows a name such as
  `x/C:evil.pdf` gives `C:evil`, and `folder.join("C:evil-page1.pdf")`
  replaces the folder with a path relative to the current directory on C:.

Every such name now goes through `child()`, which turns separators,
colons and control characters into `_` (and prefixes a name of only dots),
so the file is always a direct child of the folder. `with_root` also
refuses a root that is not a folder.

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

README.md

@@ -282,6 +282,8 @@ Every engine feature is reachable without the GUI, through one table of JSON-Sch
   pdfcraft-cli run --script steps.json --root ./work        # several steps in one session
   ```
 
+  With `--root`, every file the script's steps read or write stays in that directory, including the PNG a step saves with `"out"` (the script itself is read from wherever you name it).
+
 - **An MCP server**, for AI agents such as Claude. **It is opt-in:** PdfCraft never starts it on its own, and it opens no network port. It runs only while an agent launches `pdfcraft-cli mcp`, talks over stdin/stdout, and stops when the agent disconnects. To enable it, add it to your agent's MCP configuration:
 
   ```json

apps/pdfcraft-cli/src/main.rs

@@ -20,7 +20,9 @@
 //!
 //! `run` and `mcp` drive the same tool table (`pdfcraft-automation`). In `run`, values parse as
 //! JSON when they can (`pages=[1,3]`, `degrees=90`) and are strings otherwise. A script runs its
-//! steps in one session, so `doc_open` returns id 1, the next document id 2, and so on.
+//! steps in one session, so `doc_open` returns id 1, the next document id 2, and so on. A step's
+//! `"out"` (like `--out`) saves its image there. With `--root DIR`, every path a tool or step
+//! names, `"out"` included, must be inside DIR, and relative paths resolve inside it.
 //!
 //! The MCP server never starts on its own: it runs only when this command is launched (normally
 //! by an agent configured to use it), talks only over stdio, and exits when stdin closes.
@@ -414,15 +416,16 @@ fn tools() -> Result<(), String> {
     Ok(())
 }
 
-/// Print a tool's result: JSON as JSON; images go to `--out` (or are summarised).
-fn print_output(content: Vec<pdfcraft_automation::Content>, out: Option<&str>) -> Result<(), String> {
+/// Print a tool's result: JSON as JSON; images go to `--out` (or are summarised). The image is
+/// written like a tool's own output, so `--root` confines it too.
+fn print_output(auto: &pdfcraft_automation::Automation, content: Vec<pdfcraft_automation::Content>, out: Option<&str>) -> Result<(), String> {
     for c in content {
         match c {
             pdfcraft_automation::Content::Json(v) => println!("{}", serde_json::to_string_pretty(&v).unwrap_or_default()),
             pdfcraft_automation::Content::Png { data, width, height } => match out {
                 Some(path) => {
-                    std::fs::write(path, &data).map_err(|e| format!("{path}: {e}"))?;
-                    println!("{}", serde_json::json!({ "image": path, "width": width, "height": height }));
+                    let written = auto.write_output(path, &data).map_err(|e| e.to_string())?;
+                    println!("{}", serde_json::json!({ "image": written.to_string_lossy(), "width": width, "height": height }));
                 }
                 None => println!(
                     "{}",
@@ -442,7 +445,7 @@ fn run(args: &[String]) -> Result<(), String> {
         for (i, step) in steps.iter().enumerate() {
             let tool = step["tool"].as_str().ok_or(format!("step {}: missing \"tool\"", i + 1))?;
             let content = auto.call(tool, &step["args"]).map_err(|e| format!("step {} ({tool}): {e}", i + 1))?;
-            print_output(content, step["out"].as_str())?;
+            print_output(&auto, content, step["out"].as_str()).map_err(|e| format!("step {} ({tool}): {e}", i + 1))?;
         }
         return Ok(());
     }
@@ -454,7 +457,7 @@ fn run(args: &[String]) -> Result<(), String> {
         obj.insert(k.to_string(), value);
     }
     let content = auto.call(tool, &serde_json::Value::Object(obj)).map_err(|e| e.to_string())?;
-    print_output(content, flag(args, "--out"))
+    print_output(&auto, content, flag(args, "--out"))
 }
 
 #[cfg(feature = "mcp")]

apps/pdfcraft-cli/tests/run_root.rs

@@ -0,0 +1,92 @@
+//! `pdfcraft-cli run --root`: a script's image output stays inside the root (#137).
+
+use std::path::{Path, PathBuf};
+use std::process::{Command, Output};
+
+const PNG: &[u8] = b"\x89PNG\r\n\x1a\n";
+
+/// `root/` next to `outside/` (holding `existing.txt`) in a fresh temporary directory.
+fn sandbox(test: &str) -> PathBuf {
+    let base = std::env::temp_dir().join(format!("pdfcraft-cli-{test}-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&base);
+    std::fs::create_dir_all(base.join("root")).unwrap();
+    std::fs::create_dir_all(base.join("outside")).unwrap();
+    std::fs::write(base.join("outside/existing.txt"), "SENTINEL\n").unwrap();
+    base
+}
+
+/// Run a two-step script (a blank page, then render it to `out`) from the working directory
+/// `cwd`, with `--root root` when `root` is given.
+fn render_to(base: &Path, cwd: &Path, root: Option<&Path>, out: &str) -> Output {
+    let script = base.join("steps.json");
+    let steps = serde_json::json!([
+        { "tool": "doc_create", "args": { "from": "blank", "width": 100, "height": 100 } },
+        { "tool": "page_render", "args": { "doc": 1, "page": 1, "dpi": 10 }, "out": out },
+    ]);
+    std::fs::write(&script, steps.to_string()).unwrap();
+    let mut cmd = Command::new(env!("CARGO_BIN_EXE_pdfcraft-cli"));
+    cmd.current_dir(cwd).arg("run").arg("--script").arg(&script);
+    if let Some(root) = root {
+        cmd.arg("--root").arg(root);
+    }
+    cmd.output().unwrap()
+}
+
+fn is_png(path: &Path) -> bool {
+    std::fs::read(path).is_ok_and(|b| b.starts_with(PNG))
+}
+
+#[test]
+fn script_image_output_is_confined_to_the_root() {
+    let base = sandbox("run-root");
+    let root = base.join("root");
+    let abs = |rel: &str| base.join(rel).to_str().unwrap().to_owned();
+
+    #[cfg_attr(not(windows), allow(unused_mut))]
+    let mut refused = vec![abs("outside/escaped.png"), abs("outside/existing.txt"), "../outside/escaped.png".into(), abs("outside/new/dir/x.png")];
+    #[cfg(windows)]
+    {
+        refused.push(r"..\outside\escaped.png".into());
+        refused.push(r"\\pdfcraft-test.invalid\share\x.png".into());
+        if let Some(drive) = (b'D'..=b'Z').rev().map(|d| format!("{}:\\", d as char)).find(|d| !Path::new(d).exists()) {
+            refused.push(format!("{drive}x.png"));
+        }
+    }
+    for out in &refused {
+        let o = render_to(&base, &base, Some(&root), out);
+        let stderr = String::from_utf8_lossy(&o.stderr);
+        assert!(!o.status.success(), "{out}: expected a refusal");
+        assert!(stderr.contains("step 2 (page_render)") && stderr.contains("is outside the allowed directory"), "{out}: {stderr}");
+    }
+    assert!(!base.join("outside/escaped.png").exists() && !base.join("outside/new").exists());
+    assert_eq!(std::fs::read_to_string(base.join("outside/existing.txt")).unwrap(), "SENTINEL\n");
+
+    // "." is the root itself: refused, without staging a temporary file beside the root.
+    std::fs::write(base.join(".root.pdfcraft-tmp"), "SENTINEL\n").unwrap();
+    let o = render_to(&base, &base, Some(&root), ".");
+    assert!(!o.status.success() && String::from_utf8_lossy(&o.stderr).contains("is a folder"), "{}", String::from_utf8_lossy(&o.stderr));
+    assert_eq!(std::fs::read_to_string(base.join(".root.pdfcraft-tmp")).unwrap(), "SENTINEL\n");
+
+    // A relative path resolves inside the root, not in the working directory.
+    let o = render_to(&base, &base, Some(&root), "relative.png");
+    assert!(o.status.success(), "{}", String::from_utf8_lossy(&o.stderr));
+    assert!(is_png(&root.join("relative.png")) && !base.join("relative.png").exists());
+    // So does an absolute one inside the root, and a new folder in it.
+    for (out, file) in [(abs("root/inside.png"), "inside.png"), ("pages/one.png".into(), "pages/one.png")] {
+        let o = render_to(&base, &base, Some(&root), &out);
+        assert!(o.status.success(), "{out}: {}", String::from_utf8_lossy(&o.stderr));
+        assert!(is_png(&root.join(file)), "{out}");
+    }
+
+    // Without --root, the path is used as given.
+    let o = render_to(&base, &base, None, "free.png");
+    assert!(o.status.success(), "{}", String::from_utf8_lossy(&o.stderr));
+    assert!(is_png(&base.join("free.png")));
+    // Written in place, not through a temporary file beside it, so a device still works.
+    #[cfg(unix)]
+    {
+        let o = render_to(&base, &base, None, "/dev/stdout");
+        assert!(o.status.success() && o.stdout.windows(PNG.len()).any(|w| w == PNG), "{}", String::from_utf8_lossy(&o.stderr));
+    }
+    let _ = std::fs::remove_dir_all(&base);
+}

crates/automation/README.md

@@ -10,6 +10,7 @@ Agent control for PdfCraft: a headless tool table over the engine, and an opt-in
 ```rust
 let mut a = Automation::new().with_root("/work")?;      // optional: confine all paths
 let doc = a.call("doc_open", &json!({ "path": "in.pdf" }))?;   // Vec<Content>
+a.write_output("page1.png", &png)?;                      // save a result yourself, under the same root rules
 tools() -> Vec<ToolDef>                                  // name, title, description, input_schema, read_only, destructive, command
 mcp::McpServer::new(a).serve(stdin, stdout)?             // newline-delimited JSON-RPC 2.0
 ```
@@ -23,7 +24,7 @@ mcp::McpServer::new(a).serve(stdin, stdout)?             // newline-delimited JS
 - Unknown arguments are rejected, so typos fail loudly.
 - Tools that change a document return its summary (`doc`, `pages`, `dirty`, `undo`, `redo`, …).
 - `doc_close` refuses to drop unsaved changes unless `discard_changes: true`. `doc_save` writes atomically, incrementally in place, and in full for a new path.
-- With a root set, every read and write path must resolve inside it (symlinks and `..` included).
+- With a root set, every read and write path must resolve inside it (symlinks and `..` included). Relative paths resolve inside the root, and `..` is resolved by name before the check. Every path outside the root gets the same refusal (`<path> is outside the allowed directory <root>`), whether or not it exists, so a confined agent can't probe the rest of the disk; on Windows another network share or device path (`\\host\share`, `\\?\UNC\…`, `\\.\…`) is refused without being contacted. "Not found" and other filesystem errors are reported only for paths inside the root. Tools that write several files into a folder name them after the document or input file, with separators, colons and control characters replaced by `_`, so a name can't lead them out of the folder. The root itself must be a folder.
 
 ## MCP server
 

crates/automation/src/a11y.rs

@@ -3,7 +3,7 @@
 use pdfcraft_engine::a11y::{Category, Options, Report, Rule, Status};
 use serde_json::{Value, json};
 
-use crate::{Args, Automation, Result, ToolError, failed, write_atomic};
+use crate::{Args, Automation, Result, ToolError, child, failed, write_atomic};
 
 fn status_id(s: Status) -> &'static str {
     match s {
@@ -167,7 +167,7 @@ impl Automation {
                 });
             out.push(match result {
                 Ok(r) => {
-                    let target = folder.join(&name);
+                    let target = child(&folder, &name);
                     write_atomic(&target, &r.bytes)?;
                     let skipped: Vec<usize> = r.pages.iter().filter(|p| p.skipped.is_some()).map(|p| p.page + 1).collect();
                     json!({ "path": p, "output": target.to_string_lossy(), "words": r.words(), "skipped_pages": skipped })
@@ -339,7 +339,7 @@ impl Automation {
                 .and_then(|bytes| run_on(&action, &name, std::sync::Arc::new(bytes), |_, _| {}));
             out.push(match result {
                 Ok(r) => {
-                    let target = folder.join(&name);
+                    let target = child(&folder, &name);
                     write_atomic(&target, &r.bytes)?;
                     json!({ "path": p, "output": target.to_string_lossy(), "log": r.log })
                 }

crates/automation/src/lib.rs

@@ -100,7 +100,11 @@ impl Automation {
 
     /// Confine every path the tools read or write to `root` (relative paths resolve inside it).
     pub fn with_root(mut self, root: impl Into<PathBuf>) -> std::io::Result<Self> {
-        self.root = Some(root.into().canonicalize()?);
+        let root = root.into().canonicalize()?;
+        if !root.is_dir() {
+            return Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, "the root must be a folder"));
+        }
+        self.root = Some(root);
         Ok(self)
     }
 
@@ -114,6 +118,20 @@ impl Automation {
         &self.session
     }
 
+    /// Save `bytes` that a caller writes on the session's behalf, such as `pdfcraft-cli run`
+    /// saving a rendered page. With a root, it is confined like a tool's own output (a relative
+    /// path resolves inside it) and written atomically. Without one, the path is written as
+    /// given, so `/dev/stdout` and the like still work. Returns where the file went.
+    pub fn write_output(&self, path: &str, bytes: &[u8]) -> Result<PathBuf> {
+        let target = self.resolve(path, true)?;
+        if self.root.is_some() {
+            write_atomic(&target, bytes)?;
+        } else {
+            std::fs::write(&target, bytes).map_err(|e| failed(format!("{path}: {e}")))?;
+        }
+        Ok(target)
+    }
+
     /// Run the tool `name` with JSON `args` (an object; `null` means no arguments).
     pub fn call(&mut self, name: &str, args: &Value) -> Result<Vec<Content>> {
         let empty = json!({});
@@ -326,6 +344,11 @@ impl Automation {
                 if name == "image_save" {
                     let (ext, bytes) = self.doc(&a)?.page_image_file(page, index).map_err(failed)?;
                     let mut path = self.resolve(a.str("path")?, true)?;
+                    if path.is_dir() {
+                        // Adding the extension to a folder's name would write beside it: "." is the
+                        // root, so that would land outside it.
+                        return Err(failed(format!("{}: is a folder, not a file", path.display())));
+                    }
                     if path.extension().is_none() {
                         path.set_extension(ext);
                     }
@@ -905,7 +928,7 @@ impl Automation {
             let out = pdfcraft_engine::export::extract_images(&doc.export_source(), &pages, min).map_err(failed)?;
             let mut files = Vec::new();
             for (k, img) in out.images.iter().enumerate() {
-                let path = folder.join(pdfcraft_engine::export::image_file_name(&stem, img, k + 1));
+                let path = child(&folder, &pdfcraft_engine::export::image_file_name(&stem, img, k + 1));
                 write_atomic(&path, &img.data)?;
                 files.push(json!({ "path": path.to_string_lossy(), "page": img.page + 1, "width": img.width, "height": img.height }));
             }
@@ -923,7 +946,7 @@ impl Automation {
         let mut files = Vec::new();
         for p in pages {
             let img = ex.image(p, dpi, format).map_err(failed)?;
-            let path = folder.join(format!("{stem}_page_{}.{}", p + 1, format.extension()));
+            let path = child(&folder, &format!("{stem}_page_{}.{}", p + 1, format.extension()));
             write_atomic(&path, &img)?;
             files.push(path.to_string_lossy().into_owned());
         }
@@ -1098,7 +1121,7 @@ impl Automation {
             let mut files = Vec::new();
             for &p in &pages {
                 let bytes = self.session.extract(id, &[p]).map_err(failed)?;
-                let path = dir.join(format!("{stem}-page{}.pdf", p + 1));
+                let path = child(&dir, &format!("{stem}-page{}.pdf", p + 1));
                 write_atomic(&path, &bytes)?;
                 files.push(path.to_string_lossy().into_owned());
             }
@@ -1177,7 +1200,7 @@ impl Automation {
                 Some((_, t)) => format!("{stem}-{}.pdf", safe(t)),
                 None => format!("{stem}-part{}.pdf", i + 1),
             };
-            let path = dir.join(file);
+            let path = child(&dir, &file);
             write_atomic(&path, bytes)?;
             files.push(json!({ "path": path.to_string_lossy(), "first_page": first, "last_page": last }));
         }
@@ -1322,37 +1345,89 @@ impl Automation {
 
     /// Resolve a user-supplied path, enforcing the root (if any). `for_write` allows a file that
     /// does not exist yet (its nearest existing ancestor must be inside the root).
+    ///
+    /// Every path outside the root gets the same refusal, so a confined client can't learn what
+    /// exists out there (#136): `..` is resolved by name first, another network share or device
+    /// namespace is refused without touching it, and the deepest existing ancestor (links
+    /// followed) must be inside the root before anything about the rest is reported.
     fn resolve(&self, path: &str, for_write: bool) -> Result<PathBuf> {
         let p = Path::new(path);
-        let joined = match &self.root {
-            Some(root) if p.is_relative() => root.join(p),
-            _ => p.to_path_buf(),
-        };
-        let Some(root) = &self.root else { return Ok(joined) };
-        let real = if for_write {
-            // Canonicalize the deepest existing ancestor, then re-append the rest.
-            let mut existing = joined.as_path();
-            let mut rest = Vec::new();
-            while !existing.exists() {
-                rest.push(existing.file_name().ok_or_else(|| failed(format!("{path}: invalid path")))?);
-                existing = existing.parent().ok_or_else(|| failed(format!("{path}: invalid path")))?;
-            }
-            if rest.iter().any(|c| *c == "..") {
-                return Err(failed(format!("{path}: '..' is not allowed here")));
-            }
-            let mut real = existing.canonicalize().map_err(|e| failed(format!("{path}: {e}")))?;
-            real.extend(rest.iter().rev());
-            real
-        } else {
-            joined.canonicalize().map_err(|e| failed(format!("{path}: {e}")))?
-        };
-        if !real.starts_with(root) {
-            return Err(failed(format!("{path} is outside the allowed directory {}", root.display())));
+        let Some(root) = &self.root else { return Ok(p.to_path_buf()) };
+        let outside = || failed(format!("{path} is outside the allowed directory {}", root.display()));
+        let joined = lexical(&root.join(p)).ok_or_else(outside)?;
+        if foreign_share(&joined, root) {
+            return Err(outside());
         }
+        let mut existing = joined.as_path();
+        let mut rest = Vec::new();
+        while !existing.exists() {
+            if existing.symlink_metadata().is_ok_and(|m| m.file_type().is_symlink()) {
+                return Err(outside()); // a broken link: going on would tell whether its target exists
+            }
+            match (existing.file_name(), existing.parent()) {
+                (Some(name), Some(parent)) => {
+                    rest.push(name);
+                    existing = parent;
+                }
+                _ => return Err(outside()), // not even a drive or share that exists
+            }
+        }
+        let mut real = existing.canonicalize().map_err(|_| outside())?;
+        if !real.starts_with(root) {
+            return Err(outside());
+        }
+        if !rest.is_empty() && !for_write {
+            // Missing, below a folder inside the root: say why, as the system reports it.
+            real = joined.canonicalize().map_err(|e| failed(format!("{path}: {e}")))?;
+            if !real.starts_with(root) {
+                return Err(outside()); // it appeared, as a link out, since the check above
+            }
+            return Ok(real);
+        }
+        real.extend(rest.iter().rev());
         Ok(real)
     }
 }
 
+/// `path` with `.` and `..` resolved by name, before the filesystem is consulted (Windows does
+/// the same, and so does joining onto a canonical root there). `None` if a `..` would climb
+/// above the start of the path.
+fn lexical(path: &Path) -> Option<PathBuf> {
+    use std::path::Component;
+    let mut parts: Vec<Component> = Vec::new();
+    for c in path.components() {
+        match c {
+            Component::CurDir => {}
+            Component::ParentDir => match parts.last() {
+                Some(Component::Normal(_)) => {
+                    parts.pop();
+                }
+                _ => return None,
+            },
+            other => parts.push(other),
+        }
+    }
+    Some(parts.iter().collect())
+}
+
+/// Whether `path` names another network share or device namespace than `root` (Windows
+/// `\\host\share`, `\\?\UNC\…`, `\\.\…`, `\\?\…`). Those are refused by name: even checking
+/// that one exists would contact the host. Drive letters are left to the normal check.
+fn foreign_share(path: &Path, root: &Path) -> bool {
+    use std::path::{Component, Prefix};
+    fn share(p: &Path) -> Option<String> {
+        let Some(Component::Prefix(prefix)) = p.components().next() else { return None };
+        let (kind, a, b) = match prefix.kind() {
+            Prefix::Disk(_) | Prefix::VerbatimDisk(_) => return None,
+            Prefix::UNC(host, share) | Prefix::VerbatimUNC(host, share) => ("unc", host, share),
+            Prefix::DeviceNS(name) => ("device", name, std::ffi::OsStr::new("")),
+            Prefix::Verbatim(name) => ("verbatim", name, std::ffi::OsStr::new("")),
+        };
+        Some(format!("{kind}\\{}\\{}", a.to_string_lossy(), b.to_string_lossy()).to_lowercase())
+    }
+    share(path).is_some_and(|s| share(root).as_ref() != Some(&s))
+}
+
 // ---- JSON helpers ----------------------------------------------------------------------------
 
 /// Typed access to validated arguments.
@@ -1577,8 +1652,24 @@ fn extract_parallel(bytes: &Arc<Vec<u8>>, password: Option<Arc<str>>, pages: &[u
     out
 }
 
+/// The file `name` inside `dir`, where `name` comes from a document or an argument: separators,
+/// colons (a Windows drive or stream) and control characters become `_`, and a name of only dots
+/// gets a leading `_`, so it is one plain file name and can't lead out of `dir`.
+fn child(dir: &Path, name: &str) -> PathBuf {
+    let mut safe: String = name.chars().map(|c| if matches!(c, '/' | '\\' | ':') || c.is_control() { '_' } else { c }).collect();
+    if safe.chars().all(|c| c == '.') {
+        safe.insert(0, '_');
+    }
+    dir.join(safe)
+}
+
 /// Write via a temporary file in the same directory, then rename over the target.
 fn write_atomic(path: &Path, bytes: &[u8]) -> Result<()> {
+    // A folder can't be replaced, and staging beside it could land outside the root: "." names
+    // the root itself, whose parent isn't ours to write in.
+    if path.is_dir() {
+        return Err(failed(format!("{}: is a folder, not a file", path.display())));
+    }
     let dir = path.parent().filter(|p| !p.as_os_str().is_empty()).unwrap_or(Path::new("."));
     let name = path.file_name().ok_or_else(|| failed(format!("{}: not a file path", path.display())))?;
     std::fs::create_dir_all(dir).map_err(|e| failed(format!("{}: {e}", dir.display())))?;
@@ -1589,3 +1680,59 @@ fn write_atomic(path: &Path, bytes: &[u8]) -> Result<()> {
         failed(format!("{}: {e}", path.display()))
     })
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn dot_dot_resolves_by_name_and_never_climbs_above_the_start() {
+        let base = std::env::temp_dir();
+        let l = |rel: &str| lexical(&base.join(rel));
+        assert_eq!(l("a/./b/../c"), Some(base.join("a").join("c")));
+        assert_eq!(l("missing/../../x"), lexical(&base.join("..").join("x")));
+        assert_eq!(l("a/.."), Some(lexical(&base).unwrap()));
+        let deep = "../".repeat(base.components().count() + 1);
+        assert_eq!(lexical(&base.join(&deep)), None);
+        assert_eq!(lexical(Path::new("..")), None);
+    }
+
+    #[test]
+    fn names_from_documents_become_one_plain_file_name() {
+        let dir = std::env::temp_dir().join("out");
+        for name in ["../../x.png", "/abs/x.png", r"..\..\x.png", "C:x.png", r"\\host\share\x.png", "a:stream", "..", ".", "", "tab\there", "ok.png"]
+        {
+            let p = child(&dir, name);
+            assert_eq!(p.parent(), Some(dir.as_path()), "{name:?} -> {}", p.display());
+            assert_eq!(p.components().count(), dir.components().count() + 1, "{name:?} -> {}", p.display());
+        }
+        assert_eq!(child(&dir, "ok.png"), dir.join("ok.png"));
+        assert_eq!(child(&dir, "../x.png"), dir.join(".._x.png"));
+    }
+
+    #[cfg(windows)]
+    #[test]
+    fn other_shares_and_device_paths_are_refused_by_name() {
+        let local = Path::new(r"\\?\C:\work\root");
+        for p in [
+            r"\\host\share\x.pdf",
+            "//host/share/x.pdf",
+            r"\/host/share/x.pdf",
+            r"\\?\UNC\host\share\x.pdf",
+            r"\\.\pipe\x",
+            r"\\.\C:\work\root\x.pdf",
+            r"\\?\GLOBALROOT\Device\x",
+        ] {
+            assert!(foreign_share(Path::new(p), local), "{p}");
+        }
+        for p in [r"C:\work\root\x.pdf", r"c:\elsewhere\x.pdf", r"\\?\C:\work\root\x.pdf", r"D:\x.pdf", r"C:x.pdf"] {
+            assert!(!foreign_share(Path::new(p), local), "{p}");
+        }
+        // A root on a share accepts that share, however it is spelled, and nothing else.
+        let shared = Path::new(r"\\?\UNC\Server\Docs\root");
+        assert!(!foreign_share(Path::new(r"\\server\docs\root\x.pdf"), shared));
+        assert!(!foreign_share(Path::new(r"\\?\UNC\SERVER\DOCS\x.pdf"), shared));
+        assert!(foreign_share(Path::new(r"\\server\other\x.pdf"), shared));
+        assert!(foreign_share(Path::new(r"\\attacker\docs\x.pdf"), shared));
+    }
+}

crates/automation/tests/automation.rs

@@ -218,6 +218,258 @@ fn errors_are_specific_and_safe() {
     let _ = std::fs::remove_file(outside);
 }
 
+/// `root/` (with `inside.pdf`) next to `outside/` (with the file `secret.pdf` and the folder
+/// `sub`), all in a fresh temporary directory. Returns (base, canonical root).
+fn sandbox(test: &str) -> (PathBuf, PathBuf) {
+    let base = std::env::temp_dir().join(format!("pdfcraft-automation-{test}-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&base);
+    std::fs::create_dir_all(base.join("root")).unwrap();
+    std::fs::create_dir_all(base.join("outside/sub")).unwrap();
+    std::fs::write(base.join("root/inside.pdf"), fixture(1)).unwrap();
+    std::fs::write(base.join("outside/secret.pdf"), fixture(1)).unwrap();
+    let root = base.join("root").canonicalize().unwrap();
+    (base, root)
+}
+
+/// A link `root/<name>` to the directory `target`: a symlink on Unix, a junction on Windows
+/// (which needs no privilege).
+fn link_dir(root: &Path, name: &str, target: &Path) {
+    #[cfg(unix)]
+    std::os::unix::fs::symlink(target, root.join(name)).unwrap();
+    #[cfg(windows)]
+    {
+        // Rebuilt from components so every separator is `\` (cmd reads `/x` as a switch).
+        let (link, target): (PathBuf, PathBuf) = (root.join(name).components().collect(), target.components().collect());
+        let status = std::process::Command::new("cmd").arg("/C").arg("mklink").arg("/J").arg(link).arg(target).output().unwrap();
+        assert!(status.status.success(), "mklink /J failed: {}", String::from_utf8_lossy(&status.stderr));
+    }
+}
+
+#[test]
+fn root_refusals_do_not_reveal_what_exists_outside() {
+    // Regression test for #136: every path outside the root gets the same refusal, whether it
+    // exists, is a file or a folder, or passes through a missing folder.
+    let (base, root) = sandbox("root-oracle");
+    let mut a = auto(&root);
+    let refusal = |p: &str| ToolError::Failed(format!("{p} is outside the allowed directory {}", root.display()));
+    let abs = |rel: &str| base.join(rel).to_str().unwrap().to_owned();
+
+    #[cfg_attr(not(windows), allow(unused_mut))]
+    let mut reads = vec![
+        "../outside/secret.pdf".to_owned(),
+        "../outside/nope.pdf".into(),
+        "../outside/secret.pdf/x".into(),
+        "../outside/sub".into(),
+        "../outside/sub/x".into(),
+        "../nowhere/at/all.pdf".into(),
+        "missing/../../outside/secret.pdf".into(),
+        "missing/../../outside/nope.pdf".into(),
+        "../../../../../../../../../../../../../../../../../../../../../../../../x.pdf".into(),
+        abs("outside/secret.pdf"),
+        abs("outside/nope.pdf"),
+        abs("outside/sub/x"),
+        abs("nowhere/x.pdf"),
+        abs("nowhere/../outside/nope.pdf"),
+        abs("outside/../outside/secret.pdf"),
+        abs("root/../outside/secret.pdf"),
+    ];
+    #[cfg(windows)]
+    {
+        reads.extend([
+            r"..\outside\secret.pdf".to_owned(),
+            r"..\outside/nope.pdf".into(),
+            "../outside/secret.pdf.".into(),
+            "../outside/secret.pdf ".into(),
+            // Another network share or device namespace is refused by name, without contacting
+            // it (`.invalid` never resolves, so a regression fails instead of reaching a host).
+            r"\\pdfcraft-test.invalid\share\secret.pdf".into(),
+            "//pdfcraft-test.invalid/share/secret.pdf".into(),
+            r"\\?\UNC\pdfcraft-test.invalid\share\secret.pdf".into(),
+            r"\\.\pipe\pdfcraft-test".into(),
+            r"\\?\GLOBALROOT\Device\Null".into(),
+        ]);
+        let other = base.join("outside/secret.pdf").canonicalize().unwrap();
+        reads.push(other.to_str().unwrap().to_owned()); // the verbatim \\?\C:\… form
+        if let Some(drive) = (b'D'..=b'Z').rev().map(|d| format!("{}:\\", d as char)).find(|d| !Path::new(d).exists()) {
+            reads.push(format!("{drive}secret.pdf")); // a drive that doesn't exist
+        }
+    }
+    for p in &reads {
+        assert_eq!(a.call("doc_open", &json!({ "path": p })).unwrap_err(), refusal(p), "reading {p}");
+    }
+
+    let writes = [
+        "../outside/sub/../y.pdf".to_owned(),
+        "../outside/nosub/../y.pdf".into(),
+        "../outside/y.pdf".into(),
+        "../outside/nosub/y.pdf".into(),
+        "../outside/secret.pdf".into(),
+        "../outside/secret.pdf/y.pdf".into(),
+        "new/../../escape.pdf".into(),
+        abs("outside/nosub/deeper/y.pdf"),
+    ];
+    let doc = ok(&mut a, "doc_open", json!({ "path": "inside.pdf" }))["doc"].as_u64().unwrap();
+    for p in &writes {
+        assert_eq!(a.call("doc_save", &json!({ "doc": doc, "path": p })).unwrap_err(), refusal(p), "writing {p}");
+    }
+    #[cfg(windows)]
+    {
+        // In a verbatim path `/` is not a separator, so `x/../..` can't climb out of it either.
+        for tail in [r"\x/../../outside/v.pdf", r"\x/../../outside/secret.pdf"] {
+            let p = format!("{}{tail}", root.display());
+            if let Ok(c) = a.call("doc_save", &json!({ "doc": doc, "path": p })) {
+                let Content::Json(v) = &c[0] else { panic!("{p}: expected JSON") };
+                assert!(Path::new(v["path"].as_str().unwrap()).starts_with(&root), "{p} wrote {v}");
+            }
+            // (What lands outside the root, if anything, is checked below.)
+        }
+    }
+
+    // A link inside the root that leads out of it is refused the same way, below it too.
+    link_dir(&base.join("root"), "link", &base.join("outside"));
+    for p in ["link/secret.pdf", "link/nope.pdf", "link/sub/x", "link"] {
+        assert_eq!(a.call("doc_open", &json!({ "path": p })).unwrap_err(), refusal(p), "reading {p}");
+    }
+    for p in ["link/new.pdf", "link/nosub/new.pdf", "link/secret.pdf"] {
+        assert_eq!(a.call("doc_save", &json!({ "doc": doc, "path": p })).unwrap_err(), refusal(p), "writing {p}");
+    }
+    // So is a link whose target is gone: whether a link's target exists stays hidden too.
+    std::fs::create_dir_all(base.join("outside/gone")).unwrap();
+    link_dir(&base.join("root"), "broken", &base.join("outside/gone"));
+    std::fs::remove_dir(base.join("outside/gone")).unwrap();
+    for p in ["broken", "broken/x.pdf", "broken/x/y.pdf"] {
+        assert_eq!(a.call("doc_open", &json!({ "path": p })).unwrap_err(), refusal(p), "reading {p}");
+        assert_eq!(a.call("doc_save", &json!({ "doc": doc, "path": p })).unwrap_err(), refusal(p), "writing {p}");
+    }
+
+    // Nothing was written outside the root, and the outside files are untouched.
+    let mut left: Vec<String> =
+        std::fs::read_dir(base.join("outside")).unwrap().map(|e| e.unwrap().file_name().to_string_lossy().into_owned()).collect();
+    left.sort();
+    assert_eq!(left, ["secret.pdf", "sub"]);
+    assert_eq!(std::fs::read(base.join("outside/secret.pdf")).unwrap(), fixture(1));
+    assert!(!base.join("escape.pdf").exists() && !base.join("y.pdf").exists());
+
+    // Inside the root nothing changes: missing files say so, and existing ones open and save,
+    // however the path is spelled.
+    let missing = a.call("doc_open", &json!({ "path": "missing-inside.pdf" })).unwrap_err();
+    assert!(matches!(&missing, ToolError::Failed(m) if m.starts_with("missing-inside.pdf: ") && !m.contains("outside")), "{missing:?}");
+    let not_dir = a.call("doc_open", &json!({ "path": "inside.pdf/x" })).unwrap_err();
+    assert!(matches!(&not_dir, ToolError::Failed(m) if !m.contains("outside")), "{not_dir:?}");
+    #[cfg_attr(not(windows), allow(unused_mut))]
+    let mut opens = vec![
+        "inside.pdf".to_owned(),
+        "./inside.pdf".into(),
+        "sub/../inside.pdf".into(),
+        "../root/inside.pdf".into(),
+        "../outside/../root/inside.pdf".into(),
+        "../nowhere/../root/inside.pdf".into(),
+        abs("root/inside.pdf"),
+        abs("outside/../root/inside.pdf"),
+        abs("nowhere/../root/inside.pdf"),
+        root.join("inside.pdf").to_str().unwrap().to_owned(),
+    ];
+    #[cfg(windows)]
+    {
+        let plain = abs("root/inside.pdf");
+        opens.extend([plain.to_lowercase(), plain.to_uppercase(), r"..\root\inside.pdf".into()]);
+    }
+    for p in &opens {
+        ok(&mut a, "doc_open", json!({ "path": p }));
+    }
+    for p in ["new.pdf", "fresh/dir/new.pdf", "fresh/../also-new.pdf"] {
+        ok(&mut a, "doc_save", json!({ "doc": doc, "path": p }));
+    }
+    ok(&mut a, "doc_save", json!({ "doc": doc, "path": abs("root/abs-new.pdf") }));
+    for f in ["new.pdf", "fresh/dir/new.pdf", "also-new.pdf", "abs-new.pdf"] {
+        assert!(root.join(f).is_file(), "{f} was written inside the root");
+    }
+
+    // Without a root, paths are used as given.
+    let mut free = Automation::new();
+    ok(&mut free, "doc_open", json!({ "path": abs("outside/secret.pdf") }));
+    let _ = std::fs::remove_dir_all(&base);
+}
+
+#[test]
+fn writing_to_a_folder_touches_nothing_beside_it() {
+    // "." names the root itself. Saving there used to stage its temporary file next to the
+    // root, outside it, overwriting and then deleting any file of that name.
+    let (base, root) = sandbox("root-itself");
+    let mut a = auto(&root);
+    let beside = base.join(".root.pdfcraft-tmp");
+    std::fs::write(&beside, "SENTINEL").unwrap();
+    std::fs::create_dir_all(root.join("folder")).unwrap();
+    let doc = ok(&mut a, "doc_open", json!({ "path": "inside.pdf" }))["doc"].as_u64().unwrap();
+    for p in [".", "", "folder", "folder/"] {
+        let e = a.call("doc_save", &json!({ "doc": doc, "path": p })).unwrap_err();
+        assert!(matches!(&e, ToolError::Failed(m) if m.contains("is a folder")), "{p:?}: {e:?}");
+    }
+    let png = vec![1, 2, 3];
+    assert!(a.write_output(".", &png).is_err());
+    assert_eq!(std::fs::read_to_string(&beside).unwrap(), "SENTINEL");
+    assert!(!root.join(".folder.pdfcraft-tmp").exists());
+    // `image_save` adds an extension when the path has none, which turned "." into `root.png`
+    // beside the root.
+    ok(&mut a, "doc_export_images", json!({ "doc": doc, "folder": "src", "dpi": 18 }));
+    let pic = ok(&mut a, "doc_create", json!({ "from": "images", "paths": ["src/inside_page_1.png"] }))["doc"].as_u64().unwrap();
+    for p in [".", "", "folder", "src/.."] {
+        let e = a.call("image_save", &json!({ "doc": pic, "page": 1, "image": 1, "path": p })).unwrap_err();
+        assert!(matches!(&e, ToolError::Failed(m) if m.contains("is a folder")), "{p:?}: {e:?}");
+    }
+    assert!(!base.join("root.png").exists() && !root.join("folder.png").exists());
+    ok(&mut a, "image_save", json!({ "doc": pic, "page": 1, "image": 1, "path": "folder/picture" }));
+    assert!(root.join("folder/picture.png").is_file());
+    // Folder outputs may still name the root.
+    ok(&mut a, "doc_split", json!({ "doc": doc, "every": 1, "out_dir": "." }));
+    let _ = std::fs::remove_dir_all(&base);
+}
+
+#[test]
+fn file_names_from_documents_stay_in_the_output_folder() {
+    // Folder outputs name their files after the document. A document name with separators,
+    // `..` or (on Windows) a drive letter used to take those files out of the folder, and out
+    // of the root.
+    let (base, root) = sandbox("doc-names");
+    let mut a = auto(&root);
+    let names = ["../../escape", "../../escape.pdf", "x/../../../escape.pdf", "/tmp/escape.pdf", r"x\C:escape.pdf", "C:escape.pdf", "..", "."];
+    for (i, name) in names.iter().enumerate() {
+        let doc = ok(&mut a, "doc_create", json!({ "from": "blank", "pages": 2, "name": name }))["doc"].as_u64().unwrap();
+        let out = format!("out{i}");
+        let mut files: Vec<String> = Vec::new();
+        let r = ok(&mut a, "doc_export_images", json!({ "doc": doc, "folder": out, "dpi": 10 }));
+        files.extend(r["files"].as_array().unwrap().iter().map(|f| f.as_str().unwrap().to_owned()));
+        let r = ok(&mut a, "page_extract", json!({ "doc": doc, "pages": [1], "separate": true, "out_dir": out }));
+        files.extend(r["files"].as_array().unwrap().iter().map(|f| f.as_str().unwrap().to_owned()));
+        let r = ok(&mut a, "doc_split", json!({ "doc": doc, "every": 1, "out_dir": out }));
+        files.extend(r["files"].as_array().unwrap().iter().map(|f| f["path"].as_str().unwrap().to_owned()));
+        assert_eq!(files.len(), 5, "{name:?}: {files:?}");
+        for f in &files {
+            let f = Path::new(f);
+            assert_eq!(f.parent(), Some(root.join(&out).as_path()), "{name:?} wrote {}", f.display());
+            assert!(f.is_file(), "{name:?}: {} exists", f.display());
+        }
+    }
+    // The same for the images a page uses, with a document that has one.
+    let doc = ok(&mut a, "doc_open", json!({ "path": "inside.pdf" }))["doc"].as_u64().unwrap();
+    ok(&mut a, "doc_export_images", json!({ "doc": doc, "folder": "src", "dpi": 18 }));
+    let pic =
+        ok(&mut a, "doc_create", json!({ "from": "images", "paths": ["src/inside_page_1.png"], "name": "../../escape" }))["doc"].as_u64().unwrap();
+    let r = ok(&mut a, "doc_export_all_images", json!({ "doc": pic, "folder": "all" }));
+    let files = r["files"].as_array().unwrap();
+    assert_eq!(files.len(), 1, "{r}");
+    for f in files {
+        assert_eq!(Path::new(f["path"].as_str().unwrap()).parent(), Some(root.join("all").as_path()), "{r}");
+    }
+    let mut left: Vec<String> = std::fs::read_dir(&base).unwrap().map(|e| e.unwrap().file_name().to_string_lossy().into_owned()).collect();
+    left.sort();
+    assert_eq!(left, ["outside", "root"]);
+
+    // The root itself must be a folder.
+    assert!(Automation::new().with_root(root.join("inside.pdf")).is_err());
+    let _ = std::fs::remove_dir_all(&base);
+}
+
 #[test]
 fn command_list_reports_enablement_and_tools() {
     let dir = workdir("commands");

parity/acrobat-features.toml

@@ -9350,8 +9350,8 @@ milestone = "M13"
 status = "shipped"
 commands = []
 tools = []
-evidence = ["crates/automation/tests/automation.rs::errors_are_specific_and_safe"]
-notes = ""
+evidence = ["crates/automation/tests/automation.rs::errors_are_specific_and_safe", "crates/automation/tests/automation.rs::root_refusals_do_not_reveal_what_exists_outside", "apps/pdfcraft-cli/tests/run_root.rs::script_image_output_is_confined_to_the_root", "crates/automation/tests/automation.rs::file_names_from_documents_stay_in_the_output_folder"]
+notes = "One refusal for every path outside the root, existing or not (#136). `run --script` images (`out`) are confined too (#137). Check-then-write, not openat-style: a process racing the tool on the same disk could swap a folder for a link between the check and the write."
 
 [[feature]]
 id = "misc.mcp-resources"