From 53ea320171adef46fc97b92ec283cfd60d423ea7 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Mon, 9 Nov 2026 03:12:45 +0100 Subject: [PATCH] Check the remembered path of a pathless doc_save against the root (#1791) (#1879) --- crates/automation/src/documents.rs | 14 +++++++------- crates/automation/src/tests.rs | 24 ++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/crates/automation/src/documents.rs b/crates/automation/src/documents.rs index 95f94f3b88..8741b2829f 100644 --- a/crates/automation/src/documents.rs +++ b/crates/automation/src/documents.rs @@ -1,7 +1,7 @@ use super::error::{Result, ToolError, failed}; use super::json::{Args, nest_rows, security, summary}; use super::paths::{child, read_input, write_atomic}; -use super::{Arc, Automation, BlockKey, DocId, Document, Edit, Path, PathBuf, TextBlock, Value, comments, file_access, json}; +use super::{Arc, Automation, BlockKey, DocId, Document, Edit, Path, TextBlock, Value, comments, json}; /// How far a watermark may be moved from the page centre, in points (a page is at most 14,400). const WATERMARK_MAX_OFFSET: f64 = 20_000.0; @@ -135,18 +135,18 @@ impl Automation { pub(crate) fn doc_save_with(&mut self, a: &Args, write: impl FnOnce(&Path, &[u8]) -> Result<()>) -> Result { let doc = self.doc(a)?; let id = doc.id; - let target = match a.opt_str("path")? { + let explicit = a.opt_str("path")?; + let target = match explicit { Some(p) => self.resolve(p, true)?, None => { // Saving in place writes where the file was opened from, which the read root - // allows and the write root may not. + // allows and the write root may not. It also goes through --root again: a + // folder on the remembered path may have become a link out since (#1791). let own = doc.path.clone().ok_or_else(|| failed("the document has never been saved: pass a path"))?; - let own_path = PathBuf::from(&own); - self.check_roots(&own, &own_path, file_access::Access::Write)?; - own_path + self.resolve(&own, true)? } }; - let same_file = doc.path.as_deref().is_some_and(|p| Path::new(p) == target); + let same_file = explicit.is_none() || doc.path.as_deref().is_some_and(|p| Path::new(p) == target); // Saving to a new file is a full rewrite unless asked otherwise, like Save As. let full = a.opt_bool("full")?.unwrap_or(!same_file); let flatten_fill_sign = a.opt_bool("flatten_fill_sign")?.unwrap_or(false); diff --git a/crates/automation/src/tests.rs b/crates/automation/src/tests.rs index 0e97ffd540..408cb7cac0 100644 --- a/crates/automation/src/tests.rs +++ b/crates/automation/src/tests.rs @@ -270,6 +270,30 @@ fn a_bounded_text_find_reads_only_the_pages_it_needs_and_says_it_stopped() { let _ = std::fs::remove_dir_all(&dir); } +#[test] +fn a_save_without_a_path_stays_inside_the_root() { + let base = std::env::temp_dir().join(format!("pdfcraft-automation-save-root-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let root = base.join("root"); + let outside = base.join("outside"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::create_dir_all(&outside).unwrap(); + let mut a = Automation::new().with_root(&root).unwrap(); + let bytes = Arc::new(numbered_pages(1)); + // Remembered path inside the root: still saves. + let inside = root.canonicalize().unwrap().join("in.pdf"); + let id = a.session.open("in.pdf", Some(inside.to_string_lossy().into_owned()), bytes.clone(), None).unwrap(); + json(&mut a, "doc_save", json!({ "doc": id.0 })); + assert!(inside.exists()); + // Remembered path outside the root: refused, and nothing is written there. + let escaped = outside.join("out.pdf"); + let id = a.session.open("out.pdf", Some(escaped.to_string_lossy().into_owned()), bytes, None).unwrap(); + let error = a.call("doc_save", &json!({ "doc": id.0 })).unwrap_err(); + assert!(error.to_string().contains("outside the allowed directory"), "{error}"); + assert!(!escaped.exists()); + let _ = std::fs::remove_dir_all(&base); +} + #[test] fn dot_dot_resolves_by_name_and_never_climbs_above_the_start() { let base = std::env::temp_dir();