diff --git a/crates/engine/src/file_access.rs b/crates/engine/src/file_access.rs index 73c8ba8efe..9d05b17272 100644 --- a/crates/engine/src/file_access.rs +++ b/crates/engine/src/file_access.rs @@ -215,6 +215,13 @@ pub fn check_read(path: &str) -> Result<(), String> { current().map_or(Ok(()), |r| r.check(path, Access::Read)) } +/// Does a command param named `key` hold a file or folder path: `path`, `paths`, `file`, `files`, +/// `folder`, or a name ending in `Path`, `Paths`, `File`, `Files` or `Folder` (`diffPath`, +/// `destinationFolder`)? +pub fn is_path_key(key: &str) -> bool { + matches!(key, "path" | "paths" | "file" | "files" | "folder") || ["Path", "Paths", "File", "Files", "Folder"].iter().any(|end| key.ends_with(end)) +} + /// May `path` be written here? Always, without roots in force. pub fn check_write(path: &str) -> Result<(), String> { current().map_or(Ok(()), |r| r.check(path, Access::Write))?; diff --git a/crates/testkit/src/strategies.rs b/crates/testkit/src/strategies.rs index 10b0608001..372152870d 100644 --- a/crates/testkit/src/strategies.rs +++ b/crates/testkit/src/strategies.rs @@ -502,15 +502,17 @@ pub fn param_keys(doc: &str) -> Vec { out } -/// Parameter-name keys that name a file or folder a command may write: fuzzing these with -/// arbitrary strings could write files anywhere, so callers substitute a temp path. +/// Parameter-name keys that name a file or folder a command may write +/// ([`vectorcraft_engine::file_access::is_path_key`]): fuzzing these with arbitrary strings could +/// write files anywhere, so callers substitute a temp path. pub fn is_path_key(k: &str) -> bool { - matches!(k, "path" | "diffPath") || is_folder_key(k) + vectorcraft_engine::file_access::is_path_key(k) } -/// Path keys that name a folder a command creates or writes files into. +/// Path keys that name a folder a command creates or writes files into: `folder` or a name ending in +/// `Folder` (`destinationFolder`). pub fn is_folder_key(k: &str) -> bool { - matches!(k, "folder" | "destinationFolder") + k == "folder" || k.ends_with("Folder") } /// The location to send under the path key `k`: `safe_path` for a file, a folder next to it for diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index d0be1a13ef..c38baf5165 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -308,23 +308,10 @@ pub fn inspect(app: &VectorcraftApp, ctx: &egui::Context) -> Value { // for the person at the computer: what asked for it runs again when it answers. "fileDialog": app.picks.waiting_kind(), }); - let ids = unreadable_library_ids(app); - for field in PANEL_FIELDS.iter().chain(&LIBRARY_ID_FIELDS) { - if let Some(v) = inspected["ui"].get_mut(*field) { - hide_library_ids(v, &ids); - } - } - let pairs: Vec<_> = ids.iter().map(|(id, shown)| (id.as_str(), shown.as_str())).collect(); - if let Some(sets) = inspected["ui"].get_mut("action_sets") { - swap_library_params(sets, &pairs); - } + hide_library_ids(&mut inspected, &unreadable_library_ids(app)); inspected } -/// The fields of the UI state, by their keys in `ui.inspect`'s `ui`, that hold library ids or panel -/// ids besides [`PANEL_FIELDS`]: the library panels, the Color Guide's limit and the workspaces. -const LIBRARY_ID_FIELDS: [&str; 4] = ["library_panels", "color_guide_limit", "custom_workspaces", "workspace_changes"]; - /// The fields of the UI state, by their keys in `ui.inspect`'s `ui`, that hold panel ids: the panel /// popped out of the collapsed column, the docking layout, closed panels' places and groups, the panels /// shown as icons or collapsed, and the older floating panels' record. @@ -336,8 +323,8 @@ const PANEL_FIELDS: [&str; 7] = /// loaded without a path is named. The UI state refers to a library by its bare id in a library panel /// and the Color Guide's limit, by a panel id (`library::loaded/`) in the fields of /// [`PANEL_FIELDS`] and the workspaces' layouts, and by either in the `library` and `panel` params of -/// recorded actions' steps. Text the person types (a command palette query, a workspace's or an -/// action's name) is not read. The ids are numbered with the session's loaded libraries as the +/// recorded actions' steps; the session lists the libraries it has loaded. Text the person types (a +/// command palette query, a workspace's or an action's name) is not read. The ids are numbered as the /// library commands number them ([`vectorcraft_engine::cmd::swatchlib::concealed_library_ids`]), so /// that two libraries stay two and `ui.inspect`, `ui.docking` and the library commands show a library /// under the same id. @@ -354,11 +341,16 @@ fn unreadable_library_ids(app: &VectorcraftApp) -> Vec<(String, String)> { } } collect_library_panel_ids(&json!(app.ui.transient_panel), &mut ids); - for id in app.ui.library_panels.iter().map(|p| p.id.as_str()).chain([app.ui.color_guide_limit.as_str()]) { + // The ids of the libraries loaded in the session: a hidden one does not take the id of one loaded + // without a path. + let s = &app.session; + let loaded = s.swatch_libraries.infos().chain(s.style_libraries.infos()).chain(s.symbol_libraries.infos()).chain(s.brush_libraries.infos()); + let loaded: Vec<&str> = loaded.map(|info| info.id.as_str()).collect(); + for id in app.ui.library_panels.iter().map(|p| p.id.as_str()).chain([app.ui.color_guide_limit.as_str()]).chain(loaded.iter().copied()) { add_library_id(id, &mut ids); } let mut hidden = vectorcraft_engine::cmd::swatchlib::concealed_library_ids(&app.session, ids.iter().cloned()); - // Only the ids the UI state refers to are replaced in the reply. + // Only the ids the UI state and the session refer to are replaced in the reply. hidden.retain(|(id, _)| ids.contains(id)); // The longest first, so that an id that begins another one does not replace part of it. hidden.sort_by_key(|(id, _)| std::cmp::Reverse(id.len())); @@ -393,11 +385,106 @@ fn collect_library_panel_ids(v: &Value, ids: &mut std::collections::BTreeSet`) and in keys. +/// Writes each id of `ids` as the id the reply shows instead, and the path in it as the file name +/// shown, wherever `v` holds them, in every string and key: a library's id, its panel's id +/// (`library:swatches:loaded/`), status text, a dialog, the command palette query, a script's +/// report and a recorded step. An occurrence followed by a character that continues a path +/// ([`continues_path`]) stays only when the request may read the longer path +/// ([`readable_continuation`]): a readable library's id that begins with a hidden one keeps its path. fn hide_library_ids(v: &mut Value, ids: &[(String, String)]) { - let pairs: Vec<_> = ids.iter().map(|(id, shown)| (id.as_str(), shown.as_str())).collect(); - swap_library_ids(v, &pairs); + replace_in_value(v, &hidden_text(ids)); +} + +/// The text [`hide_library_ids`] replaces, longest first: each hidden id with the id shown, and the +/// path in it with the file name shown, the path written with either separator too, and with each +/// `\` doubled (a path quoted with `{:?}`). +fn hidden_text(ids: &[(String, String)]) -> Vec<(String, String)> { + let mut pairs: Vec<(String, String)> = Vec::new(); + for (id, shown) in ids { + let path = &id["loaded/".len()..]; + let name = shown.strip_prefix("loaded/").unwrap_or(shown); + let back = path.replace('/', "\\"); + for path in [path.to_string(), path.replace('\\', "/"), back.replace('\\', "\\\\"), back] { + for (from, to) in [(format!("loaded/{path}"), shown.clone()), (path, name.to_string())] { + if !pairs.iter().any(|(f, _)| *f == from) { + pairs.push((from, to)); + } + } + } + } + pairs.sort_by_key(|(from, _)| std::cmp::Reverse(from.len())); + pairs +} + +/// Writes each `from` text of `pairs` as its `to` text in every string and key of `v`, at each +/// occurrence not followed by a character that continues a path ([`continues_path`]) into a path the +/// request may read ([`readable_continuation`]). Encoded file data, under a key ending in `Base64` +/// (`dataBase64`, `pngBase64`), stays as it is. +fn replace_in_value(v: &mut Value, pairs: &[(String, String)]) { + if pairs.is_empty() { + return; + } + match v { + Value::String(s) => { + if let Some(replaced) = replace_text(s, pairs) { + *s = replaced; + } + } + Value::Array(items) => items.iter_mut().for_each(|item| replace_in_value(item, pairs)), + Value::Object(fields) => { + for (key, mut item) in std::mem::take(fields) { + if key.ends_with("Base64") { + fields.insert(key, item); + continue; + } + replace_in_value(&mut item, pairs); + fields.insert(replace_text(&key, pairs).unwrap_or(key), item); + } + } + _ => {} + } +} + +/// `s` with the `from` texts of `pairs` written as their `to` texts, as [`replace_in_value`] writes +/// them. `None` when nothing changed. +fn replace_text(s: &str, pairs: &[(String, String)]) -> Option { + let mut out: Option = None; + for (from, to) in pairs { + let text = out.as_deref().unwrap_or(s); + if from.is_empty() || !text.contains(from.as_str()) { + continue; + } + let mut next = String::with_capacity(text.len()); + let mut copied = 0; + for (at, _) in text.match_indices(from.as_str()) { + let end = at + from.len(); + if !continues_path(&text[end..]) || !readable_continuation(&text[at..]) { + next.push_str(&text[copied..at]); + next.push_str(to); + copied = end; + } + } + if copied > 0 { + next.push_str(&text[copied..]); + out = Some(next); + } + } + out +} + +/// Does text that follows a path continue it: a letter, a digit, `_` or a separator, after any `.`, +/// `-` or `~`? A space, punctuation, a quote, a sentence's final period and the end of the text end it. +fn continues_path(rest: &str) -> bool { + rest.trim_start_matches(['.', '-', '~']).starts_with(|c: char| c.is_alphanumeric() || matches!(c, '_' | '/' | '\\')) +} + +/// May the request read the longer path that `text` starts with, up to a space, a quote, a bracket, +/// `,`, `;` or `|`, without a `loaded/` before it? A readable library's id that begins with a hidden +/// one stays; a hidden path followed by more path text (`.bak`) is still hidden. +fn readable_continuation(text: &str) -> bool { + let end = text.find(|c: char| c.is_whitespace() || "\"'`()[]{}<>,;|".contains(c)).unwrap_or(text.len()); + let path = &text[..end]; + readable(path.strip_prefix("loaded/").unwrap_or(path)) } /// Text that a person or a script wrote, by its key: a command palette query, a script's report and a @@ -484,10 +571,69 @@ fn readable(path: &str) -> bool { vectorcraft_engine::file_access::check_read(path).is_ok() } -/// Does a recorded step's param `key` hold a file or folder path: `path`, `folder` or a name ending in -/// `Folder` (`destinationFolder`, `searchFolder`)? -fn is_path_param(key: &str) -> bool { - matches!(key, "path" | "folder") || key.ends_with("Folder") +/// Is `s` a path the request may not read, written as an absolute path (either separator, with or +/// without a drive) or a `file:` URL ([`file_url_path`])? A `file:` URL that names another host is +/// never readable. +fn unreadable_absolute(s: &str) -> bool { + match file_url_path(s) { + Some(path) => !path.is_some_and(|p| readable(&p)), + None => { + let absolute = s.starts_with(['/', '\\']) || matches!(s.as_bytes(), [drive, b':', b'/' | b'\\', ..] if drive.is_ascii_alphabetic()); + absolute && !readable(s) + } + } +} + +/// May the request read the path `s` names, a path or a `file:` URL ([`file_url_path`])? +fn readable_value(s: &str) -> bool { + match file_url_path(s) { + Some(path) => path.is_some_and(|p| readable(&p)), + None => readable(s), + } +} + +/// The local path that `s` names when it is a `file:` URL, with the scheme in any case and its `%XX` +/// escapes decoded: `Some(None)` for a URL that names another host (`file://host/share`). +/// `file:///C:/x` names `C:/x`. +fn file_url_path(s: &str) -> Option> { + let rest = s.get(..5).filter(|scheme| scheme.eq_ignore_ascii_case("file:")).map(|_| &s[5..])?; + let Some(rest) = rest.strip_prefix("//") else { return Some(Some(percent_decoded(rest))) }; + let rest = if rest.get(..9).is_some_and(|host| host.eq_ignore_ascii_case("localhost")) { &rest[9..] } else { rest }; + if !rest.starts_with('/') { + return Some(None); + } + let path = match rest.as_bytes() { + [b'/', drive, b':', ..] if drive.is_ascii_alphabetic() => &rest[1..], + _ => rest, + }; + Some(Some(percent_decoded(path))) +} + +/// `s` with `%XX` escapes decoded, as the SVG importer decodes an href. +fn percent_decoded(s: &str) -> String { + if !s.contains('%') { + return s.to_string(); + } + let mut out = Vec::with_capacity(s.len()); + let mut rest = s.as_bytes(); + while let Some((&b, tail)) = rest.split_first() { + let hex = tail + .get(..2) + .filter(|h| h.iter().all(u8::is_ascii_hexdigit)) + .and_then(|h| std::str::from_utf8(h).ok()) + .and_then(|h| u8::from_str_radix(h, 16).ok()); + match hex { + Some(v) if b == b'%' => { + out.push(v); + rest = tail.get(2..).unwrap_or_default(); + } + _ => { + out.push(b); + rest = tail; + } + } + } + String::from_utf8_lossy(&out).into_owned() } /// The params of each step of each action of action set `set` (`ui.action_sets`). @@ -498,22 +644,35 @@ fn steps_mut(set: &mut Value) -> impl Iterator) { - params.retain(|key, v| !(is_path_param(key) && v.as_str().is_some_and(|p| !readable(p)))); - if let Some(Value::Array(paths)) = params.get_mut("paths") { - paths.retain(|p| p.as_str().is_none_or(readable)); - } - for v in params.values_mut() { + use vectorcraft_engine::file_access::is_path_key; + params.retain(|key, v| match v.as_str() { + Some(p) => !unreadable_absolute(p) && !(is_path_key(key) && !readable_value(p)), + None => true, + }); + for (key, v) in params.iter_mut() { + if let Value::Array(paths) = v + && is_path_key(key) + { + paths.retain(|p| p.as_str().is_none_or(readable_value)); + } leave_out_unreadable_values(v); } } -/// [`leave_out_unreadable_params`] on each object `v` holds. +/// [`leave_out_unreadable_params`] on each object `v` holds, and an absolute path or a `file:` URL +/// the request may not read taken out of each list. fn leave_out_unreadable_values(v: &mut Value) { match v { Value::Object(fields) => leave_out_unreadable_params(fields), - Value::Array(items) => items.iter_mut().for_each(leave_out_unreadable_values), + Value::Array(items) => { + items.retain(|item| !item.as_str().is_some_and(unreadable_absolute)); + items.iter_mut().for_each(leave_out_unreadable_values); + } _ => {} } } @@ -609,21 +768,45 @@ const DOCKING_ACTION: &str = "action"; /// Runs a control request. A library loaded from outside the read root keeps the id `ui.inspect` /// shows for it ([`unreadable_library_ids`]) both ways: a request may name it or its panel by that id -/// in a [`LIBRARY_PARAMS`] param, and the reply to such a request shows that id. +/// in a [`LIBRARY_PARAMS`] param, and the reply shows that id. Under a read root, the reply shows each +/// such library's id and path as `ui.inspect` does ([`hide_library_ids`]) in every string and key, +/// errors included: the libraries hidden after it ran, and before it for a request that names one. +/// A reply with no `/` or `\` in it is not searched, since every hidden path holds a separator. pub fn handle(app: &mut VectorcraftApp, ctx: &egui::Context, req: &ControlRequest) -> Outcome { - let ids = if names_a_loaded_library(&req.params) { unreadable_library_ids(app) } else { Vec::new() }; - let shown: Vec<_> = ids.iter().map(|(id, shown)| (shown.as_str(), id.as_str())).collect(); - let mut params = req.params.clone(); - if shown.is_empty() || !swap_library_params(&mut params, &shown) { + if vectorcraft_engine::file_access::current().is_none() { return dispatch(app, ctx, req); } - let named = ControlRequest { method: req.method.clone(), params, reply: req.reply.clone(), state: req.state.clone(), seq: req.seq }; - match dispatch(app, ctx, &named) { - Outcome::Done(mut reply) => { - hide_library_ids(&mut reply, &ids); - Outcome::Done(reply) + let before = if names_a_loaded_library(&req.params) { unreadable_library_ids(app) } else { Vec::new() }; + let shown: Vec<_> = before.iter().map(|(id, shown)| (shown.as_str(), id.as_str())).collect(); + let mut params = req.params.clone(); + let outcome = if !shown.is_empty() && swap_library_params(&mut params, &shown) { + let request = ControlRequest { method: req.method.clone(), params, reply: req.reply.clone(), state: req.state.clone(), seq: req.seq }; + dispatch(app, ctx, &request) + } else { + dispatch(app, ctx, req) + }; + let Outcome::Done(mut reply) = outcome else { return outcome }; + if !holds_a_separator(&reply) { + return Outcome::Done(reply); + } + let mut ids = unreadable_library_ids(app); + for pair in before { + if !ids.iter().any(|(id, _)| *id == pair.0) { + ids.push(pair); } - outcome => outcome, + } + hide_library_ids(&mut reply, &ids); + Outcome::Done(reply) +} + +/// Does a string or a key of `v`, at any depth, hold a `/` or a `\`? +fn holds_a_separator(v: &Value) -> bool { + let has = |s: &str| s.contains(['/', '\\']); + match v { + Value::String(s) => has(s), + Value::Array(items) => items.iter().any(holds_a_separator), + Value::Object(fields) => fields.iter().any(|(key, item)| has(key) || holds_a_separator(item)), + _ => false, } } diff --git a/crates/ui-egui/src/control/tests.rs b/crates/ui-egui/src/control/tests.rs index 1763fd679a..bc1dab6c42 100644 --- a/crates/ui-egui/src/control/tests.rs +++ b/crates/ui-egui/src/control/tests.rs @@ -12,6 +12,63 @@ fn input_request(app: &mut VectorcraftApp, method: &str, params: Value) -> Value r } +/// A reply hides a hidden library's id and path at every occurrence, in an error that wraps them in +/// spaces, brackets or quotes, follows them with more path text, or quotes the path with `{:?}`. +#[test] +fn a_reply_hides_a_hidden_library_whatever_surrounds_it() { + let path = std::env::temp_dir().join("vc-named-reply").join("b.svg").to_string_lossy().into_owned(); + let id = format!("loaded/{path}"); + let hidden = [(id.clone(), "loaded/b.svg".to_string())]; + let mut reply = json!({"ok": false, "error": format!("can't add ( {id} ) or '{id}', from {path}; also {id}.bak and {path:?}")}); + let root = std::env::temp_dir().join(format!("vc-named-reply-root-{}", std::process::id())); + std::fs::create_dir_all(&root).unwrap(); + let roots = vectorcraft_engine::file_access::AutomationRoots::new(&[&root], &[]).unwrap(); + vectorcraft_engine::file_access::confine(roots.as_ref(), || super::hide_library_ids(&mut reply, &hidden)); + let _ = std::fs::remove_dir_all(root); + assert_eq!(reply["error"], "can't add ( loaded/b.svg ) or 'loaded/b.svg', from b.svg; also loaded/b.svg.bak and \"b.svg\"", "{reply}"); +} + +/// Encoded file data in a reply (`dataBase64`) stays as it is when it holds a hidden library's path. +#[test] +fn a_reply_keeps_encoded_data_that_holds_a_hidden_path() { + let hidden = [("loaded/assets/icon".to_string(), "loaded/icon".to_string())]; + let data = "QUJDassets/iconREVG"; + let mut reply = json!({"ok": true, "result": {"files": [{"name": "a.png", "dataBase64": data}], "status": "from assets/icon"}}); + let root = std::env::temp_dir().join(format!("vc-base64-root-{}", std::process::id())); + std::fs::create_dir_all(&root).unwrap(); + let roots = vectorcraft_engine::file_access::AutomationRoots::new(&[&root], &[]).unwrap(); + vectorcraft_engine::file_access::confine(roots.as_ref(), || super::hide_library_ids(&mut reply, &hidden)); + let _ = std::fs::remove_dir_all(root); + assert_eq!(reply["result"]["files"][0]["dataBase64"], data, "{reply}"); + assert_eq!(reply["result"]["status"], "from icon", "{reply}"); +} + +/// Under a read root, an absolute path or a `file:` URL outside it is unreadable; an empty string, a +/// relative name and a path inside the root are not. +#[test] +fn an_absolute_path_outside_the_read_root_is_unreadable() { + let base = std::env::temp_dir().join(format!("vc-unreadable-absolute-{}", std::process::id())); + std::fs::create_dir_all(base.join("in")).unwrap(); + let roots = vectorcraft_engine::file_access::AutomationRoots::new(&[&base.join("in")], &[]).unwrap(); + let inside = base.join("in").join("a.svg").to_string_lossy().into_owned(); + vectorcraft_engine::file_access::confine(roots.as_ref(), || { + assert!(super::unreadable_absolute("/outside/root/x")); + assert!(super::unreadable_absolute("file:///outside/root/x")); + assert!(super::unreadable_absolute("FILE:///outside/root/x")); + assert!(super::unreadable_absolute("file://other-host/share/x")); + let url = if inside.starts_with('/') { format!("File://{inside}") } else { format!("File:///{}", inside.replace('\\', "/")) }; + assert!(!super::unreadable_absolute(&url), "{url}"); + assert!(super::readable_value(&url), "{url}"); + let escaped = url.replacen("/in/a.svg", "/%69n/a.svg", 1); + assert!(!super::unreadable_absolute(&escaped) && super::readable_value(&escaped), "{escaped}"); + assert!(super::unreadable_absolute("file:///outside%2Froot/x")); + assert!(!super::unreadable_absolute("")); + assert!(!super::unreadable_absolute("keep")); + assert!(!super::unreadable_absolute(&inside)); + }); + let _ = std::fs::remove_dir_all(base); +} + #[test] fn deferred_pointer_reply_uses_the_gesture_document_after_file_new() { let mut app = VectorcraftApp::new(Session::new(), Services::default()); diff --git a/crates/ui-egui/src/tests/automation.rs b/crates/ui-egui/src/tests/automation.rs index 8f6e0f5ab5..904068cfc8 100644 --- a/crates/ui-egui/src/tests/automation.rs +++ b/crates/ui-egui/src/tests/automation.rs @@ -487,22 +487,106 @@ fn ui_inspect_checks_a_folder_preference_as_written() { let _ = std::fs::remove_dir_all(base); } -/// `ui.inspect` rewrites a hidden library's id only in the fields that hold library and panel ids: -/// status text that is that id stays as written. +/// `ui.inspect` hides a hidden library's id and path in every string it holds: status text, the +/// command palette query and a script's report show the id and file name shown, and the path appears +/// nowhere in the reply. #[test] -fn ui_inspect_keeps_status_text_that_is_a_hidden_library_id() { +fn ui_inspect_hides_a_hidden_library_path_in_every_string() { let (mut app, tx, base) = confined("inspect-status-id"); - let theirs = format!("loaded/{}", s(base.join("outside/b.svg"))); + let path = s(base.join("outside/b.svg")); + let theirs = format!("loaded/{path}"); app.ui.color_guide_limit = theirs.clone(); - app.ui.status = theirs.clone(); + app.ui.status = format!("Loaded {path} (3 swatches), kept {path}.bak"); + app.ui.palette_query = format!("\"{theirs}\""); + let source = format!("vc.log({})", json!(format!("read {path}."))); + app.session.execute("script.run", &json!({"source": source, "name": "t.js"})).unwrap(); + for _ in 0..10_000 { + if app.session.script_pump().is_some() { + break; + } + } let r = request(&mut app, &tx, "ui.inspect", json!({})); assert_eq!(r["result"]["ui"]["color_guide_limit"], "loaded/b.svg", "{r}"); - assert_eq!(r["result"]["ui"]["status"], json!(theirs), "{r}"); + assert_eq!(r["result"]["ui"]["status"], "Loaded b.svg (3 swatches), kept b.svg.bak", "{r}"); + assert_eq!(r["result"]["ui"]["palette_query"], "\"loaded/b.svg\"", "{r}"); + assert_eq!(r["result"]["scriptReport"]["log"], json!(["read b.svg."]), "{r}"); + let outside = json!(s(base.join("outside"))).to_string(); + assert!(!r.to_string().contains(outside.trim_matches('"')), "no path outside the root: {r}"); + let _ = std::fs::remove_dir_all(base); +} + +/// The reply to a request that names a hidden library by the id `ui.inspect` shows hides that +/// library's id wherever the reply holds it, also between spaces and brackets and where a character +/// that continues a path follows it. +#[test] +fn a_reply_hides_a_library_the_request_named_by_its_shown_id_everywhere() { + let (mut app, tx, base) = confined("named-hidden-reply"); + let theirs = format!("loaded/{}", s(base.join("outside/b.svg"))); + app.ui.color_guide_limit = theirs.clone(); + app.ui.status = format!("Not added ( {theirs} ); kept {theirs}_old"); + let r = request(&mut app, &tx, "ui.inspect", json!({"library": "loaded/b.svg"})); + assert_eq!(r["result"]["ui"]["status"], "Not added ( loaded/b.svg ); kept loaded/b.svg_old", "{r}"); + let outside = json!(s(base.join("outside"))).to_string(); + assert!(!r.to_string().contains(outside.trim_matches('"')), "no path outside the root: {r}"); + let _ = std::fs::remove_dir_all(base); +} + +/// An error to a request that names a hidden library by the id `ui.inspect` shows names it by that id, +/// and its path appears nowhere in the reply. +#[test] +fn an_error_for_a_hidden_library_named_by_its_shown_id_shows_that_id() { + let (mut app, tx, base) = confined("named-hidden-error"); + app.ui.color_guide_limit = format!("loaded/{}", s(base.join("outside/b.svg"))); + let r = request(&mut app, &tx, "engine.execute", json!({"command": "ui.colorGuideLimit", "params": {"library": "loaded/b.svg"}})); + assert_eq!(r["ok"], false, "{r}"); + assert!(r["error"].as_str().is_some_and(|e| e.contains("loaded/b.svg")), "{r}"); + let outside = json!(s(base.join("outside"))).to_string(); + assert!(!r.to_string().contains(outside.trim_matches('"')), "no path outside the root: {r}"); + let _ = std::fs::remove_dir_all(base); +} + +/// `ui.inspect` leaves out of a recorded action's steps a path outside the read root under any path +/// param (`diffPath`, `file`, a name ending in `Files`), and an absolute path or a `file:` URL outside +/// it (the scheme in any case) under any other param or in any list. A `file:` URL inside it stays. +#[test] +fn ui_inspect_leaves_out_recorded_paths_under_any_key() { + let (mut app, tx, base) = confined("inspect-action-keys"); + let (a, b) = (s(base.join("in/a.svg")), s(base.join("outside/b.svg"))); + let to_url = |p: &str| if p.starts_with('/') { format!("file://{p}") } else { format!("file:///{}", p.replace('\\', "/")) }; + let (url, url_in) = (to_url(&b), to_url(&a)); + let set: crate::panels::actions::ActionSet = serde_json::from_value(json!({ + "name": "set", + "actions": [{"name": "files", "steps": [ + ["document.compare", {"diffPath": b, "target": a}], + ["file.open", {"file": b}], + ["type.fonts", {"fontFiles": [a, b, url_in]}], + ["x.y", {"target": b, "url": url, "upper": url.replacen("file:", "FILE:", 1), "other": "keep"}], + ["x.z", {"list": [b, "keep"]}], + ["x.w", {"share": "file://other-host/share/b.svg", "other": "keep"}], + ["x.v", {"file": url_in}], + ]}], + })) + .unwrap(); + app.ui.action_sets = vec![set]; + let r = request(&mut app, &tx, "ui.inspect", json!({})); + let steps = &r["result"]["ui"]["action_sets"][0]["actions"][0]["steps"]; + let want = json!([ + ["document.compare", {"target": a}], + ["file.open", {}], + ["type.fonts", {"fontFiles": [a, url_in]}], + ["x.y", {"other": "keep"}], + ["x.z", {"list": ["keep"]}], + ["x.w", {"other": "keep"}], + ["x.v", {"file": url_in}], + ]); + assert_eq!(steps, &want, "{r}"); + let outside = json!(s(base.join("outside"))).to_string(); + assert!(!r.to_string().contains(outside.trim_matches('"')), "no path outside the root: {r}"); let _ = std::fs::remove_dir_all(base); } /// `ui.inspect` hides a library loaded from outside the read root in the steps of a recorded action -/// (its `library` and `panel` params), also when nothing else names it, and keeps the action's name. +/// (its `library` and `panel` params) and in the action's name, also when nothing else names it. #[test] fn ui_inspect_hides_a_hidden_library_in_recorded_steps() { let (mut app, tx, base) = confined("inspect-action-steps"); @@ -518,7 +602,7 @@ fn ui_inspect_hides_a_hidden_library_in_recorded_steps() { app.ui.action_sets = vec![set]; let r = request(&mut app, &tx, "ui.inspect", json!({})); let action = &r["result"]["ui"]["action_sets"][0]["actions"][0]; - assert_eq!(action["name"], json!(theirs), "{r}"); + assert_eq!(action["name"], "loaded/b.svg", "{r}"); assert_eq!( action["steps"], json!([["swatch.library.add", {"library": "loaded/b.svg"}], ["window.panel", {"panel": "library:swatches:loaded/b.svg"}]]), @@ -731,7 +815,7 @@ fn ui_inspect_script_report_leaves_out_files_outside_the_read_root() { /// `ui.inspect` rewrites a hidden library's id only where a string is that whole id: a readable /// library's id that begins with it stays, and a script's report line and a command palette query -/// that hold it stay as written. +/// that end with it show the id shown. #[test] fn ui_inspect_rewrites_only_whole_hidden_library_ids() { let (mut app, tx, base) = confined("inspect-whole-id"); @@ -740,8 +824,7 @@ fn ui_inspect_rewrites_only_whole_hidden_library_ids() { assert!(readable.starts_with(&hidden)); let panel = |id: &str| -> crate::panels::library_panel::LibraryPanel { serde_json::from_value(json!({"kind": "swatches", "id": id})).unwrap() }; app.ui.library_panels = vec![panel(&hidden), panel(&readable)]; - let query = format!("library:swatches:{hidden}"); - app.ui.palette_query = query.clone(); + app.ui.palette_query = format!("library:swatches:{hidden}"); let text = format!("found {hidden}"); let source = format!("vc.log({})", json!(text)); app.session.execute("script.run", &json!({"source": source, "name": "t.js"})).unwrap(); @@ -753,8 +836,12 @@ fn ui_inspect_rewrites_only_whole_hidden_library_ids() { let r = request(&mut app, &tx, "ui.inspect", json!({})); let ids: Vec<_> = r["result"]["ui"]["library_panels"].as_array().unwrap().iter().map(|p| p["id"].clone()).collect(); assert_eq!(ids, [json!("loaded/i"), json!(readable)], "{r}"); - assert_eq!(r["result"]["scriptReport"]["log"], json!([text]), "{r}"); - assert_eq!(r["result"]["ui"]["palette_query"], json!(query), "{r}"); + assert_eq!(r["result"]["scriptReport"]["log"], json!(["found loaded/i"]), "{r}"); + assert_eq!(r["result"]["ui"]["palette_query"], "library:swatches:loaded/i", "{r}"); + // A request that names the hidden library by the id shown keeps the readable id too. + let r = request(&mut app, &tx, "ui.inspect", json!({"library": "loaded/i"})); + let ids: Vec<_> = r["result"]["ui"]["library_panels"].as_array().unwrap().iter().map(|p| p["id"].clone()).collect(); + assert_eq!(ids, [json!("loaded/i"), json!(readable)], "{r}"); let _ = std::fs::remove_dir_all(base); } diff --git a/docs/mcp.md b/docs/mcp.md index ac035e7cfc..a4e322818f 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -143,8 +143,11 @@ the suite) keep the agent to the folders you give it: `file.openRecentN` for a file outside the roots ends with `no such recent file`, as an empty slot does. - `ui_inspect` (the control channel's `ui.inspect`) leaves out the paths outside the read roots: in `opening`, the documents still being read; in `ui.recent_files`, the recent files; a library panel's `path` in `ui.library_panels`; - in a recorded action's steps in `ui.action_sets`, also in a batch step's commands, the `path` and `folder` params, - a param whose name ends in `Folder` (`destinationFolder`) and the entries of a `paths` param; and the folder + in a recorded action's steps in `ui.action_sets`, also in a batch step's commands, the `path`, `paths`, `file`, + `files` and `folder` params, a param whose name ends in `Path`, `Paths`, `File`, `Files` or `Folder` (`diffPath`, + `destinationFolder`), the entries of a list under such a param, and under any other param or in any list an + absolute path or a `file:` URL outside the roots (any case of `file:`, `%XX` escapes decoded; a URL that names + another host is always left out); and the folder preferences in `ui.engine_prefs` and in an open Preferences dialog's `ui.dialog.fields` (`fontsFolder`, `pluginsFolder`, `scratchPrimary`, `scratchSecondary`, `recoveryFolder`, `templatesFolder`; an empty folder and the scratch disk choices `Startup` and `None` stay). A library loaded from a file outside the @@ -159,9 +162,12 @@ the suite) keep the agent to the folders you give it: changed. A request may name such a library or its panel by the id shown in a `panel`, `library` or `panels` param, in a docking destination (`anchor`, `before`, `onto`, `above`, `below`) and in `window.panel.layout`'s `action` (`ui.set {panel: "library:swatches:loaded/"}` - reopens its panel), and the reply to such a request, an error included, shows that id too. `ui_inspect` rewrites only whole ids, and only in the fields that hold library and panel ids: - a longer id that begins with a hidden one, the status text (`ui.status`), the command palette query - (`ui.palette_query`) and a script's report (`scriptReport`) keep their text. + reopens its panel), and the reply to such a request shows that id and file name wherever the library's id or path + appears, an error included. Every reply under the read roots shows a hidden library's id as the id shown and its + path as the file name in every string and key, the status text (`ui.status`), dialogs, the command palette query + (`ui.palette_query`) and a script's report (`scriptReport`) included. A longer path that begins with a hidden one + keeps its text only when it lies inside the read roots, so a readable library's id that begins with a hidden one + stays. Encoded file data (`dataBase64`, `pngBase64`) is sent as it is. - `swatch.library.list`, `brush.libraries`, `symbol.libraries` and `graphicStyle.libraries` list every library but leave out the paths outside the read roots: a library's `path` and the user library folder in `userFolder` are `null` there. A library loaded from a file outside the roots is listed as `loaded/`, numbered