From bf91d65d3d405580ea2257cbbf92b74c551857b0 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Fri, 9 Oct 2026 13:59:43 -0400 Subject: [PATCH] Merge land/451 into land/351; escape targets stay in the test's temp dir, and out/image paths are tried MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Absolute escape targets are now paths under the test's own temp base instead of /etc/passwd, C:\Windows\… and a UNC share, so a confinement bug can only touch files the test made. String 'out' and 'image' properties (and any described as a file path) are tried too. The backslash traversal runs only on Windows: elsewhere it is an ordinary file name inside the root. Co-Authored-By: Claude Opus 5.5 --- crates/automation/tests/hostile_args.rs | 30 ++++++++++++++++--------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/crates/automation/tests/hostile_args.rs b/crates/automation/tests/hostile_args.rs index 08975d7727..08b51b6a58 100644 --- a/crates/automation/tests/hostile_args.rs +++ b/crates/automation/tests/hostile_args.rs @@ -79,21 +79,26 @@ fn zoo() -> Vec { ] } -/// Paths that must never resolve inside the root. -fn escapes() -> Vec { - vec![ +/// Paths that must never resolve inside the root. Absolute ones point into the test's own temp +/// `base` (the root's parent), never at real system files or network shares, so a confinement +/// bug can only touch what the test made. +fn escapes(base: &std::path::Path) -> Vec { + let mut out: Vec = vec![ "../outside.pdf".into(), "../../outside.pdf".into(), "../".repeat(40) + "outside.pdf", "sub/../../outside.pdf".into(), "./././../outside.pdf".into(), - "/etc/passwd".into(), - "/tmp/outside.pdf".into(), - r"C:\Windows\System32\drivers\etc\hosts".into(), - r"\\server\share\outside.pdf".into(), - r"..\..\outside.pdf".into(), + base.join("outside.pdf").to_string_lossy().into_owned(), + base.join("new-outside.pdf").to_string_lossy().into_owned(), + base.join("sub").join("new-outside.pdf").to_string_lossy().into_owned(), "link-out/outside.pdf".into(), // through a planted symbolic link, when the system allows one - ] + ]; + // Backslashes separate only on Windows; elsewhere this is an ordinary file name in the root. + if cfg!(windows) { + out.push(r"..\..\outside.pdf".into()); + } + out } /// The properties of `def` whose values are paths, by name and whether they are a list. @@ -102,7 +107,10 @@ fn path_props(def: &pdfcraft_automation::ToolDef) -> Vec<(String, bool)> { props .iter() .filter(|(name, spec)| { - let is_pathish = matches!(name.as_str(), "path" | "paths" | "folder" | "out_dir" | "file"); + // `out` (an output file) and `image` (a picture to place) are paths when they are + // strings; `image` is an image number elsewhere, which the integer check skips. + let is_pathish = matches!(name.as_str(), "path" | "paths" | "folder" | "out_dir" | "file" | "out" | "image") + || spec["description"].as_str().is_some_and(|d| d.starts_with("A file path")); // `file_page` is a page number inside a file, not a path. is_pathish && spec["type"] != "integer" }) @@ -239,7 +247,7 @@ fn no_tool_escapes_the_root() { for def in tools() { for (prop, is_list) in path_props(&def) { - for escape in escapes() { + for escape in escapes(&base) { let mut obj = plausible_with(&def, Some(doc)); obj.insert(prop.clone(), if is_list { json!([escape]) } else { json!(escape) }); let args = Value::Object(obj);