From baa7ada7a13df33fccec8f7151be6e4dcc862f67 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 12 Nov 2026 06:05:56 -0800 Subject: [PATCH] Automation: errors name a path outside the read roots by its file name Under --automation-read-root, an error about a file or folder the client did not give now names it whole only when it is inside the read roots. Outside them it names the file or folder by its file name, or as "a file or folder outside the read roots" when it has none, and keeps the roots and the reason. A path the client gives stays whole. This covers a save without a path (the document's own file and the files written beside it, also inside action.batch, and the refusal to write over a file read with losses), file.revert, script.listInstalled, plugin.reload without a path and plugin.list's folder report, and links.relink with copy (the document's folder, its error when the document's path has no folder, and the copies in the reply). Export for Screens' refusal for an export preset's folder uses the same helper. --- crates/engine/src/cmd/fileio.rs | 3 +- crates/engine/src/cmd/fileio/access.rs | 89 ++++++++++++++++++++ crates/engine/src/cmd/fileio/action_batch.rs | 36 ++++++++ crates/engine/src/cmd/fileio/save.rs | 14 ++- crates/engine/src/cmd/fileio/screens.rs | 42 +-------- crates/engine/src/cmd/links.rs | 26 ++++-- crates/engine/src/cmd/plugin.rs | 43 ++++++++-- crates/engine/src/cmd/scripts.rs | 3 +- crates/engine/src/tests/links.rs | 50 +++++++++++ crates/engine/src/tests/plugins.rs | 31 +++++++ crates/engine/src/tests/save.rs | 49 +++++++++++ crates/engine/src/tests/script_folder.rs | 4 +- docs/mcp.md | 6 ++ docs/target-app-parity.md | 1 + 14 files changed, 342 insertions(+), 55 deletions(-) diff --git a/crates/engine/src/cmd/fileio.rs b/crates/engine/src/cmd/fileio.rs index 329988cb34..e7dc59a951 100644 --- a/crates/engine/src/cmd/fileio.rs +++ b/crates/engine/src/cmd/fileio.rs @@ -127,7 +127,8 @@ pub use access::write_file_with; #[cfg(not(target_arch = "wasm32"))] pub(crate) use access::{Placed, copy_new, file_metadata}; pub(crate) use access::{ - absolute_path, create_dir, document_path, file_created, file_stamp, read_file, read_file_head, shown_link, shown_path, write_file, write_new_file, + absolute_path, create_dir, document_path, file_created, file_stamp, naming_in_error, read_file, read_file_head, shown_link, shown_path, + write_file, write_new_file, }; #[cfg(all(test, windows))] pub(crate) use access::{document_path_on, document_path_with}; diff --git a/crates/engine/src/cmd/fileio/access.rs b/crates/engine/src/cmd/fileio/access.rs index 7b20326106..14251dee28 100644 --- a/crates/engine/src/cmd/fileio/access.rs +++ b/crates/engine/src/cmd/fileio/access.rs @@ -65,6 +65,50 @@ pub(crate) fn shown_link<'a>(stored: &'a str, resolved: Option<&str>) -> std::bo std::borrow::Cow::Borrowed(if matches!(name, "." | "..") { "" } else { name }) } +/// What an error names a file or folder outside the read roots by when [`shown_path`] gives no +/// name for it, or gives one that is not valid text. +pub(crate) const UNNAMED: &str = "a file or folder outside the read roots"; + +/// An error about `path`, a file or folder the client did not give, as the request being served +/// may see it: where the message names `path` ([`naming_in_message`]), it names it as +/// [`shown_path`] does, or as [`UNNAMED`] when that is empty or holds a replacement character. The +/// roots the message names and the reason stay whole. Unchanged when `path` is [`shown`]. +pub(crate) fn naming_in_error(e: EngineError, path: &str) -> EngineError { + if path.is_empty() || shown(path) { + return e; + } + let name = shown_path(path); + let name = if name.is_empty() || name.contains('\u{FFFD}') { UNNAMED } else { &name }; + match e { + EngineError::Other(msg) => EngineError::Other(naming_in_message(&msg, path, name)), + EngineError::BadParams { cmd, msg } => EngineError::BadParams { cmd, msg: naming_in_message(&msg, path, name) }, + e => e, + } +} + +/// `msg` with `path` given as `name` where it begins a path the message is about: at the start of +/// `msg` (`: `), after a `: ` (`... outside the write root : `) or after +/// `cannot resolve `, followed by the end, a separator, a `:` or a space. A root the message names +/// stays whole, even one inside `path`. +pub(crate) fn naming_in_message(msg: &str, path: &str, name: &str) -> String { + const BEFORE: [&str; 2] = [": ", "cannot resolve "]; + let mut out = String::with_capacity(msg.len()); + let mut rest = msg; + loop { + if let Some(after) = rest.strip_prefix(path) + && after.chars().next().is_none_or(|c| matches!(c, '/' | '\\' | ':' | ' ')) + { + out.push_str(name); + rest = after; + } + let Some((i, len)) = BEFORE.iter().filter_map(|b| rest.find(b).map(|i| (i, b.len()))).min() else { break }; + out.push_str(&rest[..i + len]); + rest = &rest[i + len..]; + } + out.push_str(rest); + out +} + /// `path` checked for writing against the automation roots in force. #[cfg(not(target_arch = "wasm32"))] pub(crate) fn may_write(path: &str) -> Result<()> { @@ -441,3 +485,48 @@ pub(crate) fn create_dir(path: &str) -> Result<()> { pub(crate) fn write_new_file(path: &str, _: &[u8]) -> Result<()> { Err(no_fs(path)) } + +#[cfg(all(test, not(target_arch = "wasm32")))] +mod tests { + use super::*; + use crate::file_access::{AutomationRoots, confine}; + + /// An error about a path the client did not give, under roots that don't let it be read: the + /// path is named by its file name (after a `/` or a `\`), or as [`UNNAMED`] when it ends in no + /// name or its name is not valid text, and every root and the reason stay whole. A path inside the + /// read roots, and any path without roots in force, stay whole. + #[test] + fn an_error_names_a_path_outside_the_read_roots_by_its_name() { + let base = std::env::temp_dir().join(format!("vc-access-naming-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let (a, b, out) = (base.join("a"), base.join("b"), base.join("kept-out")); + for f in [&a, &b, &out] { + std::fs::create_dir_all(f).unwrap(); + } + let text = |p: &std::path::Path| p.to_string_lossy().into_owned(); + let roots = AutomationRoots::new(&[a.as_path(), b.as_path()], &[a.as_path(), b.as_path()]).unwrap(); + let file = text(&out.join("doc.svg")); + let raw = confine(roots.as_ref(), || crate::file_access::check_write(&file)).unwrap_err(); + let shown_root = |p: &std::path::Path| crate::cmd::findfiles::plain(std::fs::canonicalize(p).unwrap()).display().to_string(); + let (ra, rb) = (shown_root(&a), shown_root(&b)); + let named = |e: EngineError, path: &str| confine(roots.as_ref(), || naming_in_error(e, path)).to_string(); + + assert_eq!(named(EngineError::Other(raw.clone()), &file), format!("automation path rejected: outside the write roots {ra}, {rb}: doc.svg")); + let io = named(EngineError::Other(format!("{file}: disk full")), &file); + assert_eq!(io, "doc.svg: disk full"); + let bad = named(EngineError::BadParams { cmd: "x".into(), msg: format!("{file}: not a file") }, &file); + assert_eq!(bad, "invalid parameters for `x`: doc.svg: not a file"); + // A Windows path written with `\`, a path that ends in no name and a name that isn't text. + let win = r"C:\Users\me\secret\doc.svg"; + assert_eq!(named(EngineError::Other(format!("{win}: denied")), win), "doc.svg: denied"); + let folder = format!("{}/", text(&out)); + assert_eq!(named(EngineError::Other(format!("{folder}: denied")), &folder), format!("{UNNAMED}: denied")); + let lossy = text(&out.join("bad\u{FFFD}.svg")); + assert_eq!(named(EngineError::Other(format!("{lossy}: denied")), &lossy), format!("{UNNAMED}: denied")); + // Readable, or nothing confined: whole. + let inside = text(&a.join("x.svg")); + assert_eq!(named(EngineError::Other(format!("{inside}: denied")), &inside), format!("{inside}: denied")); + assert_eq!(naming_in_error(EngineError::Other(raw.clone()), &file).to_string(), raw); + let _ = std::fs::remove_dir_all(base); + } +} diff --git a/crates/engine/src/cmd/fileio/action_batch.rs b/crates/engine/src/cmd/fileio/action_batch.rs index 4953785189..232d5ba1f4 100644 --- a/crates/engine/src/cmd/fileio/action_batch.rs +++ b/crates/engine/src/cmd/fileio/action_batch.rs @@ -1429,6 +1429,42 @@ mod tests { assert_eq!(written.len(), 2, "{written:?}"); } + /// A Data Sets batch whose Save step has no path saves the document's own file. When that file is + /// outside the client's read and write roots, each row's error keeps the write root and names the + /// file by its file name, with Stop on Error on and off. A source folder the client gives stays + /// whole in the error. + #[test] + fn dataset_batch_step_error_hides_document_path() { + use crate::file_access::{AutomationRoots, confine}; + + let base = tmp("roots-error"); + let (inside, outside) = (base.join("in"), base.join("kept-out")); + for f in [&inside, &outside] { + std::fs::create_dir_all(f).unwrap(); + } + let mut s = with_data_sets(&["Alpha", "Beta"]); + let own = outside.join("doc.vectorcraft").to_string_lossy().into_owned(); + s.execute("document.save", &json!({"path": own})).unwrap(); + let roots = AutomationRoots::new(&[inside.as_path()], &[inside.as_path()]).unwrap(); + let root = crate::cmd::findfiles::plain(std::fs::canonicalize(&inside).unwrap()).display().to_string(); + let hidden = outside.to_string_lossy().into_owned(); + for (stop, rows) in [(true, 1), (false, 2)] { + let p = json!({"source": "datasets", "stopOnError": stop, "commands": [{"command": "document.save", "params": {}}]}); + let r = confine(roots.as_ref(), || s.execute("action.batch", &p)).unwrap(); + assert_eq!((r["failed"].as_u64(), r["processed"].as_array().map(Vec::len)), (Some(rows), Some(rows as usize)), "{r}"); + for row in r["processed"].as_array().unwrap() { + let e = row["error"].as_str().unwrap_or_default(); + let want = format!("batch step 0 (`document.save`) failed: automation path rejected: outside the write root {root}: doc.vectorcraft"); + assert_eq!(e, want, "{r}"); + } + assert!(!r.to_string().contains(&hidden), "{r}"); + } + let p = json!({"source": "folder", "folder": hidden, "commands": [{"command": "object.unlockAll", "params": {}}]}); + let e = confine(roots.as_ref(), || s.execute("action.batch", &p)).unwrap_err().to_string(); + assert!(e.contains(&hidden), "{e}"); + let _ = std::fs::remove_dir_all(base); + } + /// The names of the files in `dir`, sorted. fn file_names(dir: &std::path::Path) -> Vec { let mut names: Vec = diff --git a/crates/engine/src/cmd/fileio/save.rs b/crates/engine/src/cmd/fileio/save.rs index 5d7783665b..0fa346dd5b 100644 --- a/crates/engine/src/cmd/fileio/save.rs +++ b/crates/engine/src/cmd/fileio/save.rs @@ -298,7 +298,9 @@ pub fn save_plan(s: &Session, mode: SaveMode, p: &Value) -> Result { }; let modified = date_param(p, "modified", cmd)?; if let Some(path) = &path { - super::check_not_lossy_overwrite(st, path, p, cmd)?; + // Without a path, it is the document's own file: the error names it as the request may see it. + let given = str_param(p, "path").is_some_and(|g| !g.is_empty()); + super::check_not_lossy_overwrite(st, path, p, cmd).map_err(|e| if given { e } else { super::access::naming_in_error(e, path) })?; } let written = written_as(cmd, format, &Value::Object(options.clone())); let top = NATIVE_OPTIONS.iter().filter(|k| p.get(**k).is_some_and(|v| !v.is_null())).map(|k| k.to_string()); @@ -1043,7 +1045,12 @@ pub(crate) fn save(s: &mut Session, mode: SaveMode, p: &Value) -> Result let plan = save_plan(s, mode, p)?; let job = save_job(s, plan)?; let uid = s.doc()?.uid; - let mut out = job.write_streamed(&mut super::write_file_with)?; + let given = str_param(p, "path").is_some_and(|g| !g.is_empty()); + // Without a path, the files written are the document's own file and those beside it. + let mut write = |path: &str, fill: &mut dyn FnMut(&mut dyn std::io::Write) -> std::io::Result<()>| { + super::write_file_with(path, fill).map_err(|e| if given { e } else { super::access::naming_in_error(e, path) }) + }; + let mut out = job.write_streamed(&mut write)?; job.complete(s, uid); leave_out_unreadable_save_paths(&mut out, str_param(p, "path")); Ok(out) @@ -1098,7 +1105,8 @@ fn can_revert(s: &Session) -> std::result::Result<(), String> { fn revert(s: &mut Session, _: &Value) -> Result { let index = s.active_index().ok_or(crate::EngineError::NoDocument)?; let path = s.doc()?.path.clone().ok_or_else(|| bad("file.revert", "the document has never been saved"))?; - let Loaded { mut doc, native_pdf_compatible, .. } = load(&path, &read_file(&path)?)?.with_grid_prefs(&s.prefs).with_unit_prefs(&s.prefs); + let bytes = read_file(&path).map_err(|e| super::access::naming_in_error(e, &path))?; + let Loaded { mut doc, native_pdf_compatible, .. } = load(&path, &bytes)?.with_grid_prefs(&s.prefs).with_unit_prefs(&s.prefs); doc.template = false; // Back to the saved file: its recovery copy has nothing left to recover. let uid = s.doc()?.uid; diff --git a/crates/engine/src/cmd/fileio/screens.rs b/crates/engine/src/cmd/fileio/screens.rs index b48aef68c3..b991dc0bec 100644 --- a/crates/engine/src/cmd/fileio/screens.rs +++ b/crates/engine/src/cmd/fileio/screens.rs @@ -649,7 +649,7 @@ pub(super) fn export_for_screens(s: &mut Session, p: &Value) -> Result { let p = with_preset(s, p)?; let hidden = if given { None } else { str_param(&p, "folder").filter(|f| !f.is_empty() && !super::access::shown(f)) }; let mut r = export(s, &p).map_err(|e| match hidden { - Some(folder) => leave_out_folder_from_error(e, folder), + Some(folder) => super::access::naming_in_error(e, folder), None => e, })?; remember(s, &p); @@ -659,41 +659,6 @@ pub(super) fn export_for_screens(s: &mut Session, p: &Value) -> Result { Ok(r) } -/// An error of an export into `folder`, a folder outside the read roots that the client did not -/// give: `folder` as the path the error is about names it by its own name -/// ([`super::access::shown_path`]), and the rest of the message stays. -fn leave_out_folder_from_error(e: EngineError, folder: &str) -> EngineError { - let name = super::access::shown_path(folder); - match e { - EngineError::Other(msg) => EngineError::Other(leave_out_folder_from_message(&msg, folder, &name)), - EngineError::BadParams { cmd, msg } => EngineError::BadParams { cmd, msg: leave_out_folder_from_message(&msg, folder, &name) }, - e => e, - } -} - -/// `msg` with `folder` given as `name` where it begins a path the message is about: at the start -/// of `msg` (`: `), after a `: ` (`... outside the write root : `) or -/// after `cannot resolve `, followed by the end, a separator, a `:` or a space. A root the message -/// names stays whole, even one inside `folder`. -fn leave_out_folder_from_message(msg: &str, folder: &str, name: &str) -> String { - const BEFORE: [&str; 2] = [": ", "cannot resolve "]; - let mut out = String::with_capacity(msg.len()); - let mut rest = msg; - loop { - if let Some(after) = rest.strip_prefix(folder) - && after.chars().next().is_none_or(|c| matches!(c, '/' | '\\' | ':' | ' ')) - { - out.push_str(name); - rest = after; - } - let Some((i, len)) = BEFORE.iter().filter_map(|b| rest.find(b).map(|i| (i, b.len()))).min() else { break }; - out.push_str(&rest[..i + len]); - rest = &rest[i + len..]; - } - out.push_str(rest); - out -} - /// The reply `r` of an export into `folder`, a folder the client did not give, as the request being /// served may see it: when `folder` is outside the read roots ([`super::access::shown`]), the ZIP's /// `path` is left out and `files` names each file written relative to `folder` (`2x/Logo@2x.png`), @@ -1475,7 +1440,8 @@ pub(super) fn export_settings(s: &mut Session, _: &Value) -> Result { #[cfg(test)] mod tests { - use super::{Output, ScreenSize, leave_out_folder_from_message, safe_file_name}; + use super::super::access::naming_in_message; + use super::{Output, ScreenSize, safe_file_name}; /// A hidden export folder is named by its name where a message is about it, and a root that /// starts with the same text, or a longer path, stays whole. @@ -1492,7 +1458,7 @@ mod tests { ("automation path rejected: outside the write root /a: /work/outer", "automation path rejected: outside the write root /a: /work/outer"), ]; for (msg, want) in cases { - assert_eq!(leave_out_folder_from_message(msg, f, "out"), want, "{msg}"); + assert_eq!(naming_in_message(msg, f, "out"), want, "{msg}"); } } diff --git a/crates/engine/src/cmd/links.rs b/crates/engine/src/cmd/links.rs index 89c0afefcf..43b7c3dfd4 100644 --- a/crates/engine/src/cmd/links.rs +++ b/crates/engine/src/cmd/links.rs @@ -68,7 +68,7 @@ pub fn specs() -> Vec { "Relink", [], None, - "{ids?: [image or placed document ids] (default: the selected ones; with folder and nothing selected, every missing link), path | folder, scope?: selected (default: ids) | all (ids and every other object in the layers linked to the same file as one of them) | missingInFolder (with path: ids, then every other missing link whose file name is in path's folder, each to the file of its name there; names not there are in notFound; with copy, only images are looked for) | each (with paths), paths?: [{ids, path}] (with scope each, in place of ids, path and folder: each link to its own file; an id in two entries is an error), copy?: false (with path or paths, for images of a saved document: copy the file into the document's folder and link to the copy; the names Name.png, Name 2.png … Name 99.png are tried in turn until one is free, which gets the copy, or is a file with the same contents, which is used), preserve?: transforms|bounds|fileDimensions|fit|fill (how the new file takes each image's place this time, as links.placementOptions describes; default: each image's Placement Options; fit keeps the new file's aspect ratio inside the old bounds: Replace)} link images (or placed documents, each showing the same artboard or page of the new file) to the file at path, or each to the file of its link's name in folder; embedded images become linked; each keeps its bounds. One undo step → {relinked: [ids], notFound: [file names], copied?: [{from, to}], kept?: [{from, to}] (with copy)}", + "{ids?: [image or placed document ids] (default: the selected ones; with folder and nothing selected, every missing link), path | folder, scope?: selected (default: ids) | all (ids and every other object in the layers linked to the same file as one of them) | missingInFolder (with path: ids, then every other missing link whose file name is in path's folder, each to the file of its name there; names not there are in notFound; with copy, only images are looked for) | each (with paths), paths?: [{ids, path}] (with scope each, in place of ids, path and folder: each link to its own file; an id in two entries is an error), copy?: false (with path or paths, for images of a saved document: copy the file into the document's folder and link to the copy; the names Name.png, Name 2.png … Name 99.png are tried in turn until one is free, which gets the copy, or is a file with the same contents, which is used), preserve?: transforms|bounds|fileDimensions|fit|fill (how the new file takes each image's place this time, as links.placementOptions describes; default: each image's Placement Options; fit keeps the new file's aspect ratio inside the old bounds: Replace)} link images (or placed documents, each showing the same artboard or page of the new file) to the file at path, or each to the file of its link's name in folder; embedded images become linked; each keeps its bounds. One undo step → {relinked: [ids], notFound: [file names], copied?: [{from, to}], kept?: [{from, to}] (with copy; to is the file name alone when the document's folder is outside the automation read roots)}", has_doc, relink ), @@ -1337,7 +1337,9 @@ fn relink(s: &mut Session, p: &Value) -> Result { (true, false) => return Err(bad(C, "copy needs path: the file to copy into the document's folder")), (true, true) => { let doc = st.path.as_deref().map(absolute_path).ok_or_else(|| bad(C, "save the document first: copy puts the file in its folder"))?; - Some(Path::new(&doc).parent().map(Path::to_path_buf).ok_or_else(|| bad(C, format!("{doc} has no folder")))?) + // The document's path is not the client's: the error names it as the request may see it. + let no_folder = || fileio::naming_in_error(bad(C, format!("{doc} has no folder")), &doc); + Some(Path::new(&doc).parent().map(Path::to_path_buf).ok_or_else(no_folder)?) } }; // An image or a placed document, with its link. @@ -1472,6 +1474,12 @@ fn relink(s: &mut Session, p: &Value) -> Result { not_found.dedup(); let mut out = json!({ "relinked": relinked, "notFound": not_found }); if copy_to.is_some() { + // The copies are in the document's folder, which the client did not give. + for pair in copied.iter_mut().chain(kept.iter_mut()) { + if let Some(to) = pair.get("to").and_then(Value::as_str).map(str::to_string) { + pair["to"] = json!(fileio::shown_path(&to)); + } + } out["copied"] = json!(copied); out["kept"] = json!(kept); } @@ -1521,9 +1529,14 @@ fn copy_into_folder(from: &str, bytes: &[u8], dir: &Path) -> Result<(String, boo use std::io::Write as _; let source = Path::new(from); let name = source.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); - // Each name it takes is checked against the automation roots in force. + // Each name it takes is checked against the automation roots in force. The folder is the + // document's, so a refusal names it and the file as the request may see them. let create = |to: &Path| { - crate::file_access::check_write(&to.to_string_lossy()).map_err(|e| std::io::Error::new(std::io::ErrorKind::PermissionDenied, e))?; + let to_text = to.to_string_lossy(); + crate::file_access::check_write(&to_text).map_err(|e| { + let e = fileio::naming_in_error(EngineError::Other(e), &to_text); + std::io::Error::new(std::io::ErrorKind::PermissionDenied, e.to_string()) + })?; vectorcraft_format::write_new_with(to, |f| f.write_all(bytes)) }; // A file there that automation may not read isn't taken for the same file. @@ -1534,7 +1547,10 @@ fn copy_into_folder(from: &str, bytes: &[u8], dir: &Path) -> Result<(String, boo Ok(Some(fileio::Placed::Copied(to))) => Ok((to.to_string_lossy().into_owned(), true)), Ok(Some(fileio::Placed::Kept(to))) => Ok((to.to_string_lossy().into_owned(), false)), Ok(None) => Err(EngineError::Other(format!("every numbered name for {name} is taken in the document's folder"))), - Err(e) => Err(EngineError::Other(format!("{}: {e}", dir.display()))), + Err(e) => { + let dir = dir.to_string_lossy(); + Err(fileio::naming_in_error(EngineError::Other(format!("{dir}: {e}")), &dir)) + } } } diff --git a/crates/engine/src/cmd/plugin.rs b/crates/engine/src/cmd/plugin.rs index 2d16bbc683..04a065ba1b 100644 --- a/crates/engine/src/cmd/plugin.rs +++ b/crates/engine/src/cmd/plugin.rs @@ -917,12 +917,43 @@ fn reload(s: &mut Session, p: &Value) -> Result { if folder.is_empty() { return Err(bad(C, "no folder: pass `path` or set Preferences › Plug-ins & Scratch Disks › Additional Plug-ins Folder")); } + let given = p.get("path").is_some(); let report = load_folder(s, &folder); - if let Some(e) = report.get("error").and_then(Value::as_str) { + let shown = if given { report.clone() } else { report_as_shown(report.clone()) }; + if let Some(e) = shown.get("error").and_then(Value::as_str) { return Err(EngineError::Other(e.to_string())); } - *FOLDER.lock().unwrap_or_else(|e| e.into_inner()) = Some((folder, s.plugin_dir.clone(), report.clone())); - Ok(report) + *FOLDER.lock().unwrap_or_else(|e| e.into_inner()) = Some((folder, s.plugin_dir.clone(), report)); + Ok(shown) +} + +/// The report of a plug-in folder load as the request being served may see it: the folder and each +/// file it names that is outside the read roots is named by its file name +/// ([`super::fileio::shown_path`]), also in the errors ([`super::fileio::naming_in_error`]). +fn report_as_shown(mut report: Value) -> Value { + use super::fileio::{naming_in_error, shown_path}; + + if crate::file_access::current().is_none() { + return report; + } + let naming = |msg: &str, path: &str| naming_in_error(EngineError::Other(msg.to_string()), path).to_string(); + let Some(o) = report.as_object_mut() else { return report }; + if let Some(folder) = o.get("folder").and_then(Value::as_str).map(str::to_string) { + if let Some(Value::String(e)) = o.get_mut("error") { + *e = naming(e, &folder); + } + o.insert("folder".into(), json!(shown_path(&folder))); + } + if let Some(Value::Array(rows)) = o.get_mut("failed") { + for row in rows.iter_mut().filter_map(Value::as_object_mut) { + let Some(file) = row.get("file").and_then(Value::as_str).map(str::to_string) else { continue }; + if let Some(Value::String(e)) = row.get_mut("error") { + *e = naming(e, &file); + } + row.insert("file".into(), json!(shown_path(&file))); + } + } + report } /// [`describe`] for an installed plug-in, with the bytes its cache holds (`cacheBytes`). @@ -933,7 +964,7 @@ fn describe_installed(p: &Plugin) -> Value { } fn list(_: &mut Session, _: &Value) -> Result { - let folder = FOLDER.lock().unwrap_or_else(|e| e.into_inner()).as_ref().map(|(_, _, r)| r.clone()); + let folder = FOLDER.lock().unwrap_or_else(|e| e.into_inner()).as_ref().map(|(_, _, r)| report_as_shown(r.clone())); let disabled: Vec = safe_mode::disabled().iter().map(safe_mode::Item::to_json).collect(); Ok( json!({"plugins": registry::list().iter().map(|p| describe_installed(p)).collect::>(), "folder": folder, "safeMode": safe_mode::active(), "disabled": disabled}), @@ -1055,7 +1086,7 @@ pub fn specs() -> Vec { "List Plug-ins", [], None, - "{} → {plugins: [{id, name, version, kind: \"filter\"|\"effect\"|\"exporter\"|\"importer\", author, description, params (doc), paramsSchema, effect? (the effect id of a live effect: plugin.), format? and extensions? (an exporter's or importer's format id, plugin., for document.export or document.open's importer, and its file extensions), budget?: \"heavy\" (a filter that may compute for longer, see plugin.install's allowHeavy), inputs?: [\"type\"|\"image\"|\"symbolInstance\"] (the objects besides paths a filter reads, see plugin.run), size, source, permissions: [name] (what it may do: the ones its manifest lists, else the ones its kind implies; progress: it may report its progress, see Cancel and write log lines; settings: it may keep settings of its own; cache: it may keep a cache of its own; commands: a filter may run the commands its commandAllowlist names, see docs/plugins.md), commandAllowlist?: [command id] (the commands a filter with the commands permission may run while it runs, with vc_call), hash (the module's SHA-256), api (the plug-in API level it was built for, 1 or 2), paintOutput?: true (a live effect whose pieces have paints of their own, see docs/plugins.md), cacheBytes (the bytes its cache holds; see plugin.clearCache), commands?: [{id, command (its command id, plugin.., which runs it), label, params (doc), paramsSchema}] (the menu commands a filter or live effect adds)}], folder: the Additional Plug-ins Folder load report {folder, loaded, failed: [{file, error}]} | null, safeMode: bool (no plug-in loads or runs, see plugin.safeMode), disabled: [{id, file, name, label}] (turned off for the next launch, see plugin.setEnabled)}", + "{} → {plugins: [{id, name, version, kind: \"filter\"|\"effect\"|\"exporter\"|\"importer\", author, description, params (doc), paramsSchema, effect? (the effect id of a live effect: plugin.), format? and extensions? (an exporter's or importer's format id, plugin., for document.export or document.open's importer, and its file extensions), budget?: \"heavy\" (a filter that may compute for longer, see plugin.install's allowHeavy), inputs?: [\"type\"|\"image\"|\"symbolInstance\"] (the objects besides paths a filter reads, see plugin.run), size, source, permissions: [name] (what it may do: the ones its manifest lists, else the ones its kind implies; progress: it may report its progress, see Cancel and write log lines; settings: it may keep settings of its own; cache: it may keep a cache of its own; commands: a filter may run the commands its commandAllowlist names, see docs/plugins.md), commandAllowlist?: [command id] (the commands a filter with the commands permission may run while it runs, with vc_call), hash (the module's SHA-256), api (the plug-in API level it was built for, 1 or 2), paintOutput?: true (a live effect whose pieces have paints of their own, see docs/plugins.md), cacheBytes (the bytes its cache holds; see plugin.clearCache), commands?: [{id, command (its command id, plugin.., which runs it), label, params (doc), paramsSchema}] (the menu commands a filter or live effect adds)}], folder: the Additional Plug-ins Folder load report {folder, loaded, failed: [{file, error}]} | null (a folder or file outside the automation read roots is named by its file name, also in an error), safeMode: bool (no plug-in loads or runs, see plugin.safeMode), disabled: [{id, file, name, label}] (turned off for the next launch, see plugin.setEnabled)}", always, list ), @@ -1118,7 +1149,7 @@ pub fn specs() -> Vec { "Reload Plug-ins", ["Object", "Plug-ins"], None, - "{path?: a folder (default: the Additional Plug-ins Folder preference)} install every *.wasm in it (desktop; a bad module, and one disabled with plugin.setEnabled, is reported and skipped, and so are a heavy filter and a plug-in that lists permissions the user hasn't granted this very module, its bytes included; an error in Safe Mode) → {folder, loaded: [id], failed: [{file, error}]}", + "{path?: a folder (default: the Additional Plug-ins Folder preference)} install every *.wasm in it (desktop; a bad module, and one disabled with plugin.setEnabled, is reported and skipped, and so are a heavy filter and a plug-in that lists permissions the user hasn't granted this very module, its bytes included; an error in Safe Mode; without path, a folder or file outside the automation read roots is named by its file name in the reply and in an error) → {folder, loaded: [id], failed: [{file, error}]}", always, reload ), diff --git a/crates/engine/src/cmd/scripts.rs b/crates/engine/src/cmd/scripts.rs index 8c86f0dfe3..772e1cdc0c 100644 --- a/crates/engine/src/cmd/scripts.rs +++ b/crates/engine/src/cmd/scripts.rs @@ -253,7 +253,8 @@ fn list_installed(s: &mut Session, _: &Value) -> Result { return Ok(json!({"folder": null, "scripts": [], "truncated": false, "unreadable": false})); }; let folder = root.to_string_lossy(); - crate::file_access::check_read(&folder).map_err(EngineError::Other)?; + // The Scripts folder is the app's: a refusal names it as the request may see it. + crate::file_access::check_read(&folder).map_err(|e| super::fileio::naming_in_error(EngineError::Other(e), &folder))?; let listing = script::folder::list(root); let scripts: Vec = listing .scripts() diff --git a/crates/engine/src/tests/links.rs b/crates/engine/src/tests/links.rs index f0a0674f08..a3c7a72aee 100644 --- a/crates/engine/src/tests/links.rs +++ b/crates/engine/src/tests/links.rs @@ -728,6 +728,56 @@ fn relink_each_with_copy_copies_nothing_when_a_file_cant_be_read() { assert_eq!(link_path(&s, a), one); } +/// With copy, for a client whose read roots don't hold the document's folder: the reply names the +/// copy by its file name, and a copy refused there keeps the write root and names the folder and +/// the file by their names. +#[test] +fn relink_with_copy_names_the_documents_folder_by_its_name_outside_the_read_roots() { + use crate::file_access::{AutomationRoots, confine}; + + let dir = Folder::new("relink-copy-roots"); + let one = dir.file("doc/one.png"); + write(&one, &png(60, 30, RED)); + let mut s = session(); + let a = place(&mut s, &one); + save(&mut s, &dir.file("doc/poster.vectorcraft")); + let x = dir.file("new/x.png"); + write(&x, &png(60, 30, BLUE)); + let (new, doc) = (Path::new(&x).parent().unwrap().to_path_buf(), Path::new(&one).parent().unwrap().to_path_buf()); + let hidden = doc.to_string_lossy().into_owned(); + let p = json!({"ids": [a.0], "path": x, "copy": true}); + + let writes_there = AutomationRoots::new(&[new.as_path()], &[doc.as_path()]).unwrap(); + let r = confine(writes_there.as_ref(), || s.execute("links.relink", &p)).unwrap(); + assert_eq!(r["copied"], json!([{"from": x, "to": "x.png"}]), "{r}"); + assert!(!r.to_string().contains(&hidden), "{r}"); + std::fs::remove_file(dir.file("doc/x.png")).unwrap(); + + let elsewhere = AutomationRoots::new(&[new.as_path()], &[new.as_path()]).unwrap(); + let root = crate::cmd::findfiles::plain(std::fs::canonicalize(&new).unwrap()).display().to_string(); + let e = confine(elsewhere.as_ref(), || s.execute("links.relink", &p)).unwrap_err().to_string(); + assert_eq!(e, format!("doc: automation path rejected: outside the write root {root}: x.png")); + assert!(!Path::new(&dir.file("doc/x.png")).exists()); +} + +/// With copy, for a document whose path has no folder: outside the read roots the error does not +/// name the path, and without roots it names it whole. +#[test] +fn relink_with_copy_names_a_document_path_without_a_folder_as_the_request_may_see_it() { + use crate::file_access::{AutomationRoots, confine}; + + let dir = Folder::new("relink-copy-no-folder"); + let mut s = session(); + s.doc_mut().unwrap().path = Some("/".into()); + let p = json!({"path": dir.file("x.png"), "copy": true}); + let roots = AutomationRoots::new(&[dir.0.as_path()], &[dir.0.as_path()]).unwrap(); + let e = confine(roots.as_ref(), || s.execute("links.relink", &p)).unwrap_err().to_string(); + assert_eq!(e, "invalid parameters for `links.relink`: a file or folder outside the read roots has no folder"); + let whole = s.execute("links.relink", &p).unwrap_err().to_string(); + let doc = crate::cmd::fileio::absolute_path("/"); + assert_eq!(whole, format!("invalid parameters for `links.relink`: {doc} has no folder")); +} + /// A scope or paths that doesn't fit is refused, and nothing is relinked. #[test] fn relink_refuses_scopes_and_paths_that_dont_fit() { diff --git a/crates/engine/src/tests/plugins.rs b/crates/engine/src/tests/plugins.rs index a1e615a08d..52820be60f 100644 --- a/crates/engine/src/tests/plugins.rs +++ b/crates/engine/src/tests/plugins.rs @@ -1433,6 +1433,37 @@ fn plugins_folder_set_again_after_clearing_installs_its_plugins() { run(&mut s, "plugin.remove", json!({"id": id})); } +/// The Additional Plug-ins Folder, seen by a client whose read roots don't hold it: `plugin.list` +/// names the folder and the files that failed by their names, and `plugin.reload` without a path is +/// refused naming the folder by its name and keeping the root. A folder the client gives stays whole. +#[test] +fn a_plugins_folder_outside_the_read_roots_is_named_by_its_name() { + use crate::file_access::{AutomationRoots, confine}; + + let _folder = PREFS_FOLDER.lock().unwrap_or_else(|e| e.into_inner()); + let dir = crate::tests_links::Folder::new("plugin-folder-hidden"); + let work = crate::tests_links::Folder::new("plugin-folder-work"); + std::fs::write(dir.0.join("broken.wasm"), b"\0asm?").unwrap(); + let mut s = session(); + let shown = dir.0.display().to_string(); + let name = dir.0.file_name().unwrap().to_string_lossy().into_owned(); + run(&mut s, "prefs.set", json!({"key": "pluginsFolder", "value": shown})); + let roots = AutomationRoots::new(&[work.0.as_path()], &[]).unwrap(); + let root = crate::cmd::findfiles::plain(std::fs::canonicalize(&work.0).unwrap()).display().to_string(); + + let list = confine(roots.as_ref(), || run(&mut s, "plugin.list", json!({}))); + assert_eq!(list["folder"]["folder"], json!(name), "{list}"); + assert_eq!(list["folder"]["failed"][0]["file"], json!("broken.wasm"), "{list}"); + assert!(!list.to_string().contains(&shown), "{list}"); + let e = confine(roots.as_ref(), || s.execute("plugin.reload", &json!({}))).unwrap_err().to_string(); + assert_eq!(e, format!("automation path rejected: outside the read root {root}: {name}")); + let e = confine(roots.as_ref(), || s.execute("plugin.reload", &json!({"path": shown}))).unwrap_err().to_string(); + assert_eq!(e, format!("automation path rejected: outside the read root {root}: {shown}")); + // Unconfined, the report names the folder whole. + assert_eq!(run(&mut s, "plugin.list", json!({}))["folder"]["folder"], json!(shown)); + run(&mut s, "prefs.set", json!({"key": "pluginsFolder", "value": ""})); +} + /// #2541: a plug-in installed in a session with a plug-in store is installed again in the next /// session that attaches the store, and Remove Plug-in takes it out of the store. #[test] diff --git a/crates/engine/src/tests/save.rs b/crates/engine/src/tests/save.rs index 9932055652..8d557bb4ff 100644 --- a/crates/engine/src/tests/save.rs +++ b/crates/engine/src/tests/save.rs @@ -84,6 +84,55 @@ fn a_confined_save_leaves_out_paths_outside_the_read_root() { let _ = std::fs::remove_dir_all(d); } +/// A save without a path, and Revert, refused because the document's own file is outside a confined +/// client's roots: the error keeps the root and the reason and names the file by its file name, also +/// for the files a save writes beside it. A path the client gives stays whole in the error. +#[test] +fn pathless_save_rejection_hides_document_folder() { + use crate::file_access::{AutomationRoots, confine}; + + let d = dir("roots-error"); + let (inside, outside) = (d.join("in"), d.join("kept-out")); + for f in [&inside, &outside] { + std::fs::create_dir_all(f).unwrap(); + } + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 200, "height": 100, "artboards": 2})).unwrap(); + rect(&mut s); + let own = path(&outside, "doc.vectorcraft"); + s.execute("document.save", &json!({"path": own})).unwrap(); + rect(&mut s); + let roots = AutomationRoots::new(&[inside.as_path()], &[inside.as_path()]).unwrap(); + let root = crate::cmd::findfiles::plain(std::fs::canonicalize(&inside).unwrap()).display().to_string(); + let hidden = outside.to_string_lossy().into_owned(); + let mut refused = |id: &str, p: Value| confine(roots.as_ref(), || s.execute(id, &p)).unwrap_err().to_string(); + + let e = refused("document.save", json!({})); + assert_eq!(e, format!("automation path rejected: outside the write root {root}: doc.vectorcraft")); + let e = refused("document.save", json!({"separateArtboards": true})); + assert!(e.starts_with(&format!("automation path rejected: outside the write root {root}: ")) && !e.contains(&hidden), "{e}"); + let e = refused("file.revert", json!({})); + assert_eq!(e, format!("automation path rejected: outside the read root {root}: doc.vectorcraft")); + // Given by the client: whole. + let e = refused("document.save", json!({"path": own})); + assert_eq!(e, format!("automation path rejected: outside the write root {root}: {own}")); + // Without read roots, a write refusal still names the file by its file name. + let write_only = AutomationRoots::new(&[], &[inside.as_path()]).unwrap(); + let e = confine(write_only.as_ref(), || s.execute("document.save", &json!({}))).unwrap_err().to_string(); + assert_eq!(e, format!("automation path rejected: outside the write root {root}: doc.vectorcraft")); + assert!(s.doc().unwrap().is_dirty(), "nothing was saved"); + // Read with losses from its own file: the refusal to write over it names the file by its file name. + let st = s.doc_mut().unwrap(); + st.imported_from = Some(own.clone()); + st.import_losses = vec!["hidden text".into()]; + let e = confine(roots.as_ref(), || s.execute("document.save", &json!({}))).unwrap_err().to_string(); + assert!(e.starts_with("invalid parameters for `document.save`: doc.vectorcraft is the file this document was read from"), "{e}"); + assert!(!e.contains(&hidden), "{e}"); + let e = confine(roots.as_ref(), || s.execute("document.save", &json!({"path": own}))).unwrap_err().to_string(); + assert!(e.contains(&format!("{own} is the file this document was read from")), "{e}"); + let _ = std::fs::remove_dir_all(d); +} + #[test] fn an_svg_path_writes_svg_and_save_writes_it_back() { let d = dir("svg"); diff --git a/crates/engine/src/tests/script_folder.rs b/crates/engine/src/tests/script_folder.rs index b176b4f143..312db3e0fc 100644 --- a/crates/engine/src/tests/script_folder.rs +++ b/crates/engine/src/tests/script_folder.rs @@ -311,7 +311,9 @@ fn the_automation_read_root_keeps_an_agent_out_of_the_scripts_folder() { s.attach_scripts_dir(root.clone()); let roots = crate::file_access::AutomationRoots::new(&[&work], &[]).unwrap().unwrap(); let e = crate::file_access::confine(Some(&roots), || s.execute("script.listInstalled", &json!({}))).unwrap_err(); - assert!(e.to_string().contains("outside the read root"), "{e}"); + // The Scripts folder is the app's: the error keeps the root and names the folder by its name. + let shown_root = crate::cmd::findfiles::plain(std::fs::canonicalize(&work).unwrap()).display().to_string(); + assert_eq!(e.to_string(), format!("automation path rejected: outside the read root {shown_root}: Scripts")); let e = crate::file_access::confine(Some(&roots), || s.installed_script(&script.to_string_lossy())).unwrap_err(); assert!(e.to_string().contains("outside the read root"), "{e}"); // A read root that holds the folder lets both through. diff --git a/docs/mcp.md b/docs/mcp.md index fa22c66414..4931fed70d 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -202,6 +202,12 @@ the suite) keep the agent to the folders you give it: `outside the write roots /work/project/out, /tmp/scratch: …`) or `automation filesystem access is not granted: write authority is absent: /work/project/out/x.png`. Links in an opened document that point outside the read roots show as missing. +- An error names a path the client gave as given. A path the client did not give (the document's own file + in a save without `path` and in `file.revert`, also inside `action.batch`, an export preset's folder, the + Scripts folder, the Additional Plug-ins Folder, the document's folder in `links.relink {copy}`) is named + whole only inside the read roots; outside them the error names it by its file name, or as + `a file or folder outside the read roots` when it has none, and keeps the roots and the reason, for example + `automation path rejected: outside the write root /work/project/out: poster.vectorcraft`. - The preferences that move folders the app reads or writes on its own (`fontsFolder`, `pluginsFolder`, `recoveryFolder`, `templatesFolder`) can only be set inside the roots. The app's own folders (preferences, Data Recovery, the User Defined library folders, VectorCraft's Fonts folder) are not diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index d42a4d988b..ef1059c692 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -764,6 +764,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - Scripts folder: File › Scripts lists the `.js` and `.jsx` files of the `Scripts` folder next to the preferences, each named after its file without the extension, with each subfolder as a submenu, above Check Script… and Other Script… (https://helpx.adobe.com/illustrator/desktop/automate-visualize-data/automate-actions/install-and-run-scripts.html). VectorCraft's own rules: the folder is read again when the window becomes active and when the File menu opens (in the macOS menu bar, when any of its menus opens, and the new listing shows the next time the menu opens), links are not followed, and one read stops at 2,048 entries, 512 scripts, 128 subfolders, 4 levels or 250 ms. `script.listInstalled` returns the listing. `crates/engine/src/tests/script_folder.rs`, `crates/ui-egui/src/tests/scripts_menu.rs`. - Open Recent Files under `--automation-read-root`: the control channel's `ui.menu.list` and `file.recentFiles` leave out the recent files outside the read roots, the other rows keep their ids, and `file.openRecentN` for one outside them answers `no such recent file`, as an empty slot does. `crates/ui-egui/src/tests/automation.rs`. - Saves, Export for Screens and Package under `--automation-read-root`: a save without a path leaves the document's own file out of its reply, and a save without a path known leaves out the suggested folder, when they are outside the read roots; Export for Screens into an export preset's folder outside them lists its files relative to that folder and names that folder by its name in an error, and the export settings and saved presets leave out a folder outside them when read or exported; Package's report and warnings, and Document Info's linked images, name a file outside them by its file name. A path the client gives stays in the reply. `crates/engine/src/tests/save.rs`, `crates/engine/src/cmd/fileio/tests_screens.rs`, `crates/engine/src/tests/package.rs`, `crates/ui-egui/src/tests/automation.rs`. + - Errors under `--automation-read-root`: a refused save without a path and `file.revert` (also as an `action.batch` step), `script.listInstalled`, `plugin.reload` without a path and `links.relink {copy}` name the document's own file, the Scripts folder, the Additional Plug-ins Folder or the document's folder by its file name when it is outside the read roots, and keep the roots and the reason; `plugin.list`'s folder report and `links.relink`'s copies name such a folder or file by its file name too. A path the client gives stays whole. `crates/engine/src/tests/save.rs` (`pathless_save_rejection_hides_document_folder`), `crates/engine/src/cmd/fileio/action_batch.rs`, `crates/engine/src/tests/plugins.rs`, `crates/engine/src/tests/links.rs`, `crates/engine/src/tests/script_folder.rs`. - `ui.inspect` under `--automation-read-root`: the documents still opening, the recent files, library panels' files and folder preferences (saved, and in an open Preferences dialog) outside the read roots are left out of the reply, a library loaded from outside them shows by its file name in `ui.inspect` and `ui.docking`, and the app's state is not changed. `crates/ui-egui/src/tests/automation.rs` (`ui_inspect_leaves_out_paths_outside_the_read_root`). - Link replies under `--automation-read-root`: `links.list`, `links.check`, `links.info` and `document.open`'s link rows leave out the path, found path and folder of a linked file outside the read roots, and the stored size and date of it in `links.info`; each row keeps its id, file name and status, and the document's links are not changed. `crates/engine/src/tests/linkspanel.rs` (`link_paths_outside_the_read_roots_are_left_out`), `crates/engine/src/tests/placed_document.rs` (`link_info_leaves_out_a_placed_documents_path_outside_the_read_roots`). - Linked-file and font-file searches under `--automation-read-root`: `links.findFiles` and `text.findFontFiles` replies, also later polls and stops of a search another client started, leave out the files found, a link's saved path and the folder searched outside the caller's read roots, and a failed search gives its reason without the path; `image.info` gives `link: null` for a linked file outside them; `text.addFontFiles` gives `folder: null` and the copies' file names when the Fonts folder is outside them. `crates/engine/src/tests/linkfiles.rs`, `crates/engine/src/tests/fontfiles.rs`, `crates/engine/tests/font_folder.rs`.