mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-10-25 02:29:31 -06:00
* 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>
93 lines
4.5 KiB
Rust
93 lines
4.5 KiB
Rust
//! `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);
|
|
}
|