Plug-ins: a call reads no file, whatever param names it

A call whose arguments hold a file or folder key at any depth, such as an
effect's textureImage.path, is refused before the command runs; the walk
keeps its own stack. Each call also runs with automation file access
confined to no roots, so every checked read or write a command makes is
refused, whichever param carried the path.

crates/engine/src/cmd/plugin_call.rs

@@ -13,7 +13,9 @@
 //! changes preferences, runs a script, an action or a plug-in (a plug-in command, or an effect a
 //! plug-in adds), opens, closes or switches documents, or undoes or redoes, whatever its manifest
 //! lists. A command whose params name a file or folder is refused unless [`NOT_FILES`] says that
-//! key names part of the document in it. Every other command is refused too unless
+//! key names part of the document in it, and so is a call whose arguments carry such a key at any
+//! depth ([`names_file`]); a call runs with no file or folder readable or writable
+//! ([`execute_confined`]). Every other command is refused too unless
 //! [`reviewed::ALLOWED`] lists it, so a command added to the registry later is refused until
 //! someone decides.
 
@@ -180,6 +182,30 @@ pub(crate) fn names_key(text: &str, key: &str) -> bool {
     })
 }
 
+/// Does `args`, the arguments of a call of command `id`, carry a key in [`FILE_KEYS`] at any depth
+/// (in objects and in arrays), such as an effect's `textureImage.path`? Only a key [`NOT_FILES`]
+/// names for `id`, at the top level, isn't counted. The walk keeps its own stack, so deep
+/// arguments can't overflow the thread's.
+fn names_file(id: &str, args: &Value) -> bool {
+    let mut stack = vec![(args, true)];
+    while let Some((v, top)) = stack.pop() {
+        match v {
+            Value::Array(a) => stack.extend(a.iter().map(|x| (x, false))),
+            Value::Object(o) => {
+                for (k, x) in o {
+                    let k = k.as_str();
+                    if FILE_KEYS.contains(&k) && !(top && NOT_FILES.contains(&(id, k))) {
+                        return true;
+                    }
+                    stack.push((x, false));
+                }
+            }
+            _ => {}
+        }
+    }
+    false
+}
+
 /// Param keys whose value names an effect or a command (`id` is `effect.apply`'s other name for
 /// `effect`).
 const PLUGIN_KEYS: &[&str] = &["effect", "effects", "id", "command", "commands"];
@@ -255,11 +281,11 @@ pub(crate) fn call(s: &mut Session, run: &Calls, id: &str, args: &Value) -> std:
     let Some(spec) = super::find_command(id) else {
         return Err(CallFailure { code: CALL_UNKNOWN_COMMAND, why: super::params::unknown_command(id).to_string() });
     };
-    if denied(spec).is_some() || names_plugin(args) {
+    if denied(spec).is_some() || names_plugin(args) || names_file(id, args) {
         return Err(refused());
     }
     let before = s.active().map(|d| d.doc.clone());
-    let r = s.execute(id, args);
+    let r = execute_confined(s, id, args);
     let value = match r {
         Ok(v) => v,
         Err(e @ EngineError::Disabled(..)) => return Err(CallFailure { code: CALL_DISABLED, why: e.to_string() }),
@@ -272,6 +298,13 @@ pub(crate) fn call(s: &mut Session, run: &Calls, id: &str, args: &Value) -> std:
     Ok(value)
 }
 
+/// Runs command `id` with `args` in `s` with no file or folder readable or writable: every path the
+/// command reaches through the checked file access ([`crate::file_access`]) is refused, whatever
+/// param carries it.
+fn execute_confined(s: &mut Session, id: &str, args: &Value) -> Result<Value> {
+    crate::file_access::confine(Some(&crate::file_access::AutomationRoots::none()), || s.execute(id, args))
+}
+
 /// Runs `f`, a plug-in run that makes command calls, as one undo step named `label` in the active
 /// document: the commands it calls and the output it applies change the document inside one
 /// interaction, which is kept when `f` succeeds. When `f` fails, every document goes back to how it
@@ -487,6 +520,51 @@ mod tests {
         assert!(!names_plugin(&json!({"effect": {"name": "plugin.style"}})));
     }
 
+    /// A file or folder key counts at any depth, in objects and in arrays; a key [`NOT_FILES`] names
+    /// for the command only at the top level; ordinary nested params don't count.
+    #[test]
+    fn a_file_key_counts_at_any_depth() {
+        use serde_json::json;
+        let texture = json!({"effect": "distort.glass", "params": {"textureImage": {"path": "/tmp/t.png"}}});
+        assert!(names_file("effect.apply", &texture));
+        assert!(names_file("effect.setParams", &texture));
+        assert!(names_file("effect.apply", &json!({"params": {"layers": [{"name": "a"}, {"folder": "/tmp"}]}})));
+        assert!(names_file("select.all", &json!([[[{"url": "file:///etc/hosts"}]]])));
+        assert!(!names_file("effect.apply", &json!({"effect": "distort.glass", "params": {"distortion": 5, "textureImage": "frosted"}})));
+        assert!(!names_file("object.move", &json!({"dx": 5, "dy": 0, "ids": [1, 2], "opts": {"copy": false}})));
+        // `path` is a shape's path in object.setLiveShape, but only as its own param.
+        assert!(!names_file("object.setLiveShape", &json!({"path": [[0, 0], [1, 1]]})));
+        assert!(names_file("object.setLiveShape", &json!({"options": {"path": "/tmp/x"}})));
+        assert!(names_file("object.move", &json!({"path": [[0, 0]]})));
+        // Deep nesting is walked without recursion.
+        let mut deep = json!({"file": "/tmp/x"});
+        for _ in 0..10_000 {
+            deep = Value::Array(vec![deep]);
+        }
+        assert!(names_file("select.all", &deep));
+        std::mem::forget(deep);
+    }
+
+    /// A call reads no file even when a param the checks above don't know carries its path: the
+    /// command runs with no file readable. Run as an ordinary request, the same texture loads.
+    #[cfg(not(target_arch = "wasm32"))]
+    #[test]
+    fn a_confined_call_reads_no_file() {
+        use serde_json::json;
+        let file = std::env::temp_dir().join(format!("vc-plugin-call-texture-{}.png", std::process::id()));
+        let png = vectorcraft_trace::Raster::from_fn(8, 8, |x, y| if (x + y) % 2 == 0 { [0, 0, 0, 255] } else { [255, 255, 255, 255] }).encode_png();
+        std::fs::write(&file, png).unwrap();
+        let mut s = Session::new();
+        s.execute("file.new", &json!({"width": 200, "height": 200})).unwrap();
+        s.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 50, "height": 50})).unwrap();
+        let p = json!({"effect": "distort.glass", "params": {"textureImage": {"path": file.to_string_lossy()}}});
+        let e = execute_confined(&mut s, "effect.apply", &p).map(|_| ()).unwrap_err().to_string();
+        let read = s.execute("effect.apply", &p).map(|_| ());
+        let _ = std::fs::remove_file(&file);
+        assert!(e.contains("read authority is absent"), "{e}");
+        read.unwrap();
+    }
+
     #[test]
     fn keys_are_found_as_whole_keys() {
         assert!(names_key("{path: a .wasm file}", "path"));

crates/engine/src/file_access.rs

@@ -85,6 +85,12 @@ impl AutomationRoots {
         Ok(Some(Arc::new(Self { read: open(read, Access::Read)?, write: open(write, Access::Write)? })))
     }
 
+    /// Roots that grant no access of either kind: inside [`confine`] with them, every path a
+    /// command reads or writes is refused.
+    pub(crate) fn none() -> Arc<Self> {
+        Arc::new(Self { read: Vec::new(), write: Vec::new() })
+    }
+
     /// May the file or folder at `path` be read (or written)? It must lie inside one of the roots
     /// of that kind. `Err` says why not.
     pub fn check(&self, path: &str, access: Access) -> Result<(), String> {

crates/engine/tests/plugin_call.rs

@@ -176,6 +176,51 @@ fn without_doc_write_a_call_that_edits_is_refused() {
     run(&mut s, "plugin.remove", json!({"id": "test.call.reader"}));
 }
 
+/// Installs filter `id` that may read and change the document and call `allowlist`, whose `vc_run`
+/// makes `calls` (command id and arguments) in order, dropping their codes, then returns nothing.
+fn install_calls(s: &mut Session, id: &str, allowlist: &str, calls: &[(&str, &str)]) {
+    let mut imports = r#"(import "env" "vc_call" (func $vc_call (param i32 i32 i32 i32) (result i32)))"#.to_string();
+    let mut first = String::new();
+    for (i, (cmd, args)) in calls.iter().enumerate() {
+        let at = 0x3000 + i * 0x400;
+        imports += &format!(r#" (data (i32.const {at}) "{}") (data (i32.const {}) "{}")"#, tpl::string(cmd), at + 0x100, tpl::string(args));
+        first += &format!("(drop (call $vc_call (i32.const {at}) (i32.const {}) (i32.const {}) (i32.const {})))", cmd.len(), at + 0x100, args.len());
+    }
+    let manifest = format!(
+        r#"{{"id":"{id}","name":"Test {id}","kind":"filter","api":2,"permissions":["doc.read","doc.write","commands"],"commandAllowlist":{allowlist}}}"#
+    );
+    let wasm = wat::parse_str(tpl::with_imports(&manifest, &imports, &first, EMPTY)).unwrap();
+    let data = vectorcraft_format::base64_encode(&wasm);
+    let grant = json!(["doc.read", "doc.write", "commands"]);
+    run(s, "plugin.install", json!({"dataBase64": data, "grant": grant, "replace": true}));
+}
+
+/// Arguments that carry a file or folder key at any depth, such as an effect's
+/// `textureImage.path`, are refused before the command runs, so no file is read; arguments nested
+/// without one run.
+#[test]
+fn a_call_whose_arguments_name_a_file_is_refused() {
+    let mut s = document();
+    let before = state(&mut s);
+    let allowlist = r#"["select.all","effect.apply","effect.setParams"]"#;
+    let select = ("select.all", "");
+    for (cmd, args) in [
+        ("effect.apply", r#"{"effect":"distort.glass","params":{"textureImage":{"path":"missing-texture.png"}}}"#),
+        ("effect.setParams", r#"{"index":0,"params":{"textureImage":{"path":"missing-texture.png"}}}"#),
+        ("effect.apply", r#"{"effect":"distort.glass","params":{"layers":[{"name":"a"},{"file":"missing-texture.png"}]}}"#),
+    ] {
+        install_calls(&mut s, "test.call.files", allowlist, &[select, (cmd, args)]);
+        let e = run_plugin(&mut s, "test.call.files").unwrap_err();
+        assert!(e.contains("may not call") && e.contains(cmd), "{args}: {e}");
+        assert_eq!(state(&mut s), before, "{args}");
+    }
+    // The same effect with ordinary nested params runs.
+    install_calls(&mut s, "test.call.files", allowlist, &[select, ("effect.apply", r#"{"effect":"distort.glass","params":{"distortion":5}}"#)]);
+    run_plugin(&mut s, "test.call.files").unwrap();
+    assert_eq!(state(&mut s).1, before.1 + 1, "one undo step");
+    run(&mut s, "plugin.remove", json!({"id": "test.call.files"}));
+}
+
 /// A command's result describes the document, so a plug-in without `doc.read` may call nothing.
 #[test]
 fn without_doc_read_every_call_is_refused() {

docs/plugins.md

@@ -422,6 +422,10 @@ the module's start function, or in a run that isn't a filter's run on a document
   whole app, and for a command VectorCraft adds later until it has been checked for plug-in calls. Installing a
   manifest that lists such a command is refused, and the error names it; `plugin.inspect` lists them as
   `refusedCommands`.
+- A call whose arguments hold a file or folder key (`path`, `paths`, `file`, `files`, `folder`, `folders`, `dir`,
+  `directory`, `destinationFolder`, `sourceFolder` or `url`) at any depth, such as an effect's `textureImage.path`,
+  returns `-6`; only a key that names part of the document in that command, as its own param, is let through. A
+  command a plug-in calls can't read or write any file or folder.
 - An id in `commandAllowlist` that no command has installs, so a plug-in can list a command a later version adds;
   `plugin.install` and `plugin.inspect` list such ids as `unknownCommands`, and calling one returns `-5` (`-6` without
   `doc.read`).