diff --git a/crates/engine/src/tests/plugins.rs b/crates/engine/src/tests/plugins.rs index fc46cfcd7..cba53c6b6 100644 --- a/crates/engine/src/tests/plugins.rs +++ b/crates/engine/src/tests/plugins.rs @@ -1026,8 +1026,9 @@ fn a_malformed_ids_is_refused_not_read_as_the_selection() { run(&mut s, "plugin.remove", json!({"id": id})); } -/// A live effect's commands are listed under Effect › Plug-ins, run under its permissions (it may -/// not change the document), and labels show bidirectional characters escaped. A malformed +/// A live effect's commands are listed under Effect › Plug-ins and run under its permissions (with +/// commands it gets `doc.write` unless its list leaves it out), and labels show bidirectional +/// characters escaped. A malformed /// `commands` entry is refused at install, and the error names it. #[test] fn effect_commands_labels_and_malformed_entries() { @@ -1038,10 +1039,28 @@ fn effect_commands_labels_and_malformed_entries() { let c = listed(&s, &crate::cmd::plugin::command_id(id, "flip")).expect("listed"); assert_eq!((c.label, c.menu.as_slice()), ("Flip\\u{202E}txt", &["Effect", "Plug-ins", "Cmd Test"][..])); run(&mut s, "select.none", json!({})); - let before = s.doc().unwrap().doc.clone(); + // Without `permissions`, a live effect with commands may change the document: its command adds + // the triangle, as one undo step. + let (before, undo) = (s.doc().unwrap().doc.clone(), s.doc().unwrap().history.undo.len()); + let out = run(&mut s, &crate::cmd::plugin::command_id(id, "flip"), json!({})); + assert_eq!(out["ids"].as_array().map(Vec::len), Some(1), "{out}"); + assert_ne!(s.doc().unwrap().doc, before); + assert_eq!(s.doc().unwrap().history.undo.len(), undo + 1); + run(&mut s, "plugin.remove", json!({"id": id})); + // One that lists its permissions without `doc.write` is refused the change, and the document + // stays as it was. + let listed = format!( + r#"{{"id":"{id}","name":"Cmd Test","version":"1","kind":"effect","permissions":["doc.read","effect.render"],"commands":[{{"id":"flip","label":"Flip"}}]}}"# + ); + let wasm = wat::parse_str(tpl::returning(&listed, triangle)).unwrap(); + let hash = vectorcraft_plugins::permissions::sha256_hex(&wasm); + let grant = json!({"dataBase64": vectorcraft_format::base64_encode(&wasm), "grant": ["doc.read", "effect.render"], "hash": hash}); + run(&mut s, "plugin.install", grant); + let (before, undo) = (s.doc().unwrap().doc.clone(), s.doc().unwrap().history.undo.len()); let e = s.execute(&crate::cmd::plugin::command_id(id, "flip"), &json!({})).unwrap_err().to_string(); assert!(e.contains("wasn't granted `doc.write`"), "{e}"); assert_eq!(s.doc().unwrap().doc, before); + assert_eq!(s.doc().unwrap().history.undo.len(), undo); run(&mut s, "plugin.remove", json!({"id": id})); let bad = id.replace("effect", "bad"); diff --git a/crates/plugins/src/manifest.rs b/crates/plugins/src/manifest.rs index d2f780bf6..0abdd6016 100644 --- a/crates/plugins/src/manifest.rs +++ b/crates/plugins/src/manifest.rs @@ -351,7 +351,7 @@ impl Manifest { return Err(Error::Manifest(format!("`api` is {}; this host implements plug-in API levels 1 to {API_LEVEL}", self.api))); } if let Some(names) = &self.permissions { - permissions::validate(self.kind, names)?; + permissions::validate(self.kind, !self.commands.is_empty(), names)?; } self.validate_extensions()?; match (self.budget, self.kind) { @@ -475,11 +475,14 @@ impl Manifest { self.params.iter().map(|(k, p)| (k.clone(), p.spec.default_value())).collect() } - /// What the plug-in may do: the permissions it lists, else the ones its kind implies. + /// What the plug-in may do: the permissions it lists, else every one its kind may list + /// ([`Permission::allowed`]: a live effect with commands also changes the document and keeps + /// data with it). pub fn permissions(&self) -> Vec { + let commands = !self.commands.is_empty(); match &self.permissions { - Some(names) => permissions::validate(self.kind, names).unwrap_or_default(), - None => Permission::implied(self.kind).to_vec(), + Some(names) => permissions::validate(self.kind, commands, names).unwrap_or_default(), + None => Permission::allowed(self.kind, commands).to_vec(), } } @@ -849,6 +852,16 @@ mod tests { assert!(serde_json::to_value(&plain).unwrap().get("permissions").is_none()); let effect = Manifest::parse(br#"{"id":"a","name":"x","kind":"effect"}"#).unwrap(); assert_eq!(effect.permissions(), [Permission::DocRead, Permission::EffectRender]); + // A live effect's commands may change the document: one with commands gets `doc.write` + // and `state` too, and may list them. + let commands = br#"{"id":"a","name":"x","kind":"effect","commands":[{"id":"c","label":"C"}]}"#; + let with_commands = Manifest::parse(commands).unwrap(); + assert_eq!(with_commands.permissions(), [Permission::DocRead, Permission::DocWrite, Permission::State, Permission::EffectRender]); + let listing = + br#"{"id":"a","name":"x","kind":"effect","permissions":["doc.read","effect.render","doc.write"],"commands":[{"id":"c","label":"C"}]}"#; + assert_eq!(Manifest::parse(listing).unwrap().permissions(), [Permission::DocRead, Permission::DocWrite, Permission::EffectRender]); + let without = br#"{"id":"a","name":"x","kind":"effect","permissions":["doc.read","effect.render","doc.write"]}"#; + assert!(Manifest::parse(without).unwrap_err().to_string().contains("`doc.write` doesn't apply")); let exporter = Manifest::parse(br#"{"id":"a","name":"x","kind":"exporter","extensions":["plt"]}"#).unwrap(); assert_eq!(exporter.permissions(), [Permission::DocRead, Permission::ExportWrite]); assert!(!Manifest::parse(br#"{"id":"a","name":"x","kind":"filter","permissions":[]}"#).unwrap().asks_permissions()); diff --git a/crates/plugins/src/permissions.rs b/crates/plugins/src/permissions.rs index 1d6b0664e..ac75e8e13 100644 --- a/crates/plugins/src/permissions.rs +++ b/crates/plugins/src/permissions.rs @@ -90,6 +90,17 @@ impl Permission { } } + /// The permissions a plug-in of `kind` that declares `commands` or not may list, which are also + /// the ones its manifest gets without `permissions`: those `kind` implies, and for a live effect + /// with commands also `doc.write` and `state`, which only its commands use (drawing the effect + /// changes no object and keeps no data). + pub fn allowed(kind: Kind, commands: bool) -> &'static [Permission] { + match kind { + Kind::Effect if commands => &[Permission::DocRead, Permission::DocWrite, Permission::State, Permission::EffectRender], + _ => Permission::implied(kind), + } + } + /// The permissions a plug-in of `kind` can't work without, which its list must hold: a live /// effect and an exporter are always handed objects, and only draw or write. A filter may /// leave any of its permissions out. @@ -101,10 +112,11 @@ impl Permission { } } -/// Checks the `permissions` a manifest of `kind` lists → them, in the order [`ALL`] has. A name -/// that is not a permission, a reserved one, one that doesn't apply to the kind, one listed twice -/// and a list without what the kind needs are errors that name it. -pub(crate) fn validate(kind: Kind, names: &[String]) -> Result> { +/// Checks the `permissions` a manifest of `kind` lists → them, in the order [`ALL`] has; `commands` +/// is whether it declares commands ([`Permission::allowed`]). A name that is not a permission, a +/// reserved one, one that doesn't apply to the kind, one listed twice and a list without what the +/// kind needs are errors that name it. +pub(crate) fn validate(kind: Kind, commands: bool, names: &[String]) -> Result> { if names.len() > MAX_PERMISSIONS { return Err(Error::Manifest(format!("at most {MAX_PERMISSIONS} `permissions`"))); } @@ -117,7 +129,7 @@ pub(crate) fn validate(kind: Kind, names: &[String]) -> Result> } return Err(Error::Manifest(format!("unknown permission `{shown}`"))); }; - if !Permission::implied(kind).contains(&p) { + if !Permission::allowed(kind, commands).contains(&p) { return Err(Error::Manifest(format!("permission `{shown}` doesn't apply to a plug-in of kind `{}`", kind.name()))); } if out.contains(&p) { @@ -214,23 +226,34 @@ mod tests { #[test] fn lists_are_checked_against_the_kind() { let list = |names: &[&str]| names.iter().map(|s| s.to_string()).collect::>(); - assert_eq!(validate(Kind::Filter, &list(&["doc.write", "doc.read"])).unwrap(), [Permission::DocRead, Permission::DocWrite]); - assert_eq!(validate(Kind::Filter, &[]).unwrap(), []); - assert!(validate(Kind::Effect, &list(&["doc.read", "effect.render"])).is_ok()); - assert!(validate(Kind::Exporter, &list(&["export.write", "doc.read"])).is_ok()); - for (kind, names, says) in [ - (Kind::Filter, &["doc.read", "clipboard"][..], "unknown permission `clipboard`"), - (Kind::Filter, &["progress"], "`progress` is reserved"), - (Kind::Filter, &["export.write"], "doesn't apply to a plug-in of kind `filter`"), - (Kind::Exporter, &["doc.read", "export.write", "doc.read"], "`doc.read` is listed twice"), - (Kind::Effect, &["doc.read"], "needs the permission `effect.render`"), - (Kind::Exporter, &["export.write"], "needs the permission `doc.read`"), + assert_eq!(validate(Kind::Filter, false, &list(&["doc.write", "doc.read"])).unwrap(), [Permission::DocRead, Permission::DocWrite]); + assert_eq!(validate(Kind::Filter, false, &[]).unwrap(), []); + assert!(validate(Kind::Effect, false, &list(&["doc.read", "effect.render"])).is_ok()); + assert!(validate(Kind::Exporter, false, &list(&["export.write", "doc.read"])).is_ok()); + // A live effect with commands may also change the document and keep data with it. + assert_eq!( + validate(Kind::Effect, true, &list(&["state", "effect.render", "doc.write", "doc.read"])).unwrap(), + [Permission::DocRead, Permission::DocWrite, Permission::State, Permission::EffectRender] + ); + assert_eq!(Permission::allowed(Kind::Effect, false), Permission::implied(Kind::Effect)); + assert_eq!(Permission::allowed(Kind::Filter, true), Permission::implied(Kind::Filter)); + for (kind, commands, names, says) in [ + (Kind::Filter, false, &["doc.read", "clipboard"][..], "unknown permission `clipboard`"), + (Kind::Filter, false, &["progress"], "`progress` is reserved"), + (Kind::Filter, false, &["export.write"], "doesn't apply to a plug-in of kind `filter`"), + (Kind::Exporter, false, &["doc.read", "export.write", "doc.read"], "`doc.read` is listed twice"), + (Kind::Effect, false, &["doc.read"], "needs the permission `effect.render`"), + (Kind::Effect, false, &["doc.read", "effect.render", "doc.write"], "`doc.write` doesn't apply to a plug-in of kind `effect`"), + (Kind::Effect, true, &["doc.read", "doc.write"], "needs the permission `effect.render`"), + (Kind::Effect, true, &["doc.read", "effect.render", "export.write"], "`export.write` doesn't apply"), + (Kind::Exporter, true, &["doc.read", "export.write", "doc.write"], "`doc.write` doesn't apply"), + (Kind::Exporter, false, &["export.write"], "needs the permission `doc.read`"), ] { - let e = validate(kind, &list(names)).unwrap_err().to_string(); + let e = validate(kind, commands, &list(names)).unwrap_err().to_string(); assert!(e.contains(says), "{names:?}: {e}"); } let many = list(&["doc.read"; MAX_PERMISSIONS + 1]); - assert!(validate(Kind::Filter, &many).unwrap_err().to_string().contains("at most")); + assert!(validate(Kind::Filter, false, &many).unwrap_err().to_string().contains("at most")); } /// Control, bidirectional override and separator characters show as escapes, so a name can't diff --git a/docs/plugins.md b/docs/plugins.md index 777dc69e8..656cde4a1 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -69,8 +69,8 @@ replaced by a module without it. - **Runs.** The parameters are checked against the command's declarations before the plug-in runs. The plug-in runs through `vc_run` on the selection (or `ids`) as a filter run does, with `_context.mode` `"command"` and `_context.command` set to the command's id ([Parameters](#parameters)), under the plug-in's - [permissions](#permissions): a live effect's command has no `doc.write`, so an output that changes the document is - refused. `ids` left out or null means the selection; an `ids` that is not a list of object ids is an error. One run + [permissions](#permissions): a live effect with commands may also hold `doc.write` and `state`, which only its + commands use. `ids` left out or null means the selection; an `ids` that is not a list of object ids is an error. One run is one undo step named after the command. A run that traps, runs out of its instruction budget or fails leaves the document and the undo history as they were, and the error starts with the plug-in's name and the command's label (`“Cmd Test” › Grow: …`). @@ -136,16 +136,18 @@ capability model proposed for PhotoCraft plug-ins | Permission | Kinds | What it allows | |---|---|---| | `doc.read` | all | Read the objects it is given. | -| `doc.write` | filter | Change, add and delete objects in the document. | -| `state` | filter | Keep data of its own with the document (`documentData`). | +| `doc.write` | filter, effect with `commands` | Change, add and delete objects in the document. | +| `state` | filter, effect with `commands` | Keep data of its own with the document (`documentData`). | | `effect.render` | effect | Change how the objects its effect is applied to are drawn. | | `export.write` | exporter | Write the document as a file in its own format. | - A manifest without `permissions` gets the ones its kind implies, as plug-ins did before the key existed: a filter - `doc.read`, `doc.write` and `state`; an effect `doc.read` and `effect.render`; an exporter `doc.read` and - `export.write`. Such a plug-in installs without a prompt. This rule is VectorCraft's own. A filter whose list is - empty may do none of these, and also installs without a prompt. -- An effect or an exporter that lists permissions lists both of its implied ones. A filter may list any of its three. + `doc.read`, `doc.write` and `state`; an effect `doc.read` and `effect.render`, and also `doc.write` and `state` + when it declares [commands](#commands); an exporter `doc.read` and `export.write`. Such a plug-in installs without + a prompt. This rule is VectorCraft's own. A filter whose list is empty may do none of these, and also installs + without a prompt. +- An effect or an exporter that lists permissions lists `doc.read` and its own (`effect.render`, `export.write`). An + effect with commands may add `doc.write` and `state`, which only its commands use. A filter may list any of its three. An unknown name, a name listed twice, a name that doesn't apply to the kind, more than 16 names, or one of the names kept for a later plug-in API (`progress`, `ui.panel`, `ui.tool`) is refused when the module is installed, and the error names it. @@ -162,7 +164,7 @@ capability model proposed for PhotoCraft plug-ins in it count for `plugin.install`, `plugin.inspect`, `plugin.reload` and the Additional Plug-ins Folder. At startup, a stored plug-in whose permissions are not granted is skipped. The Additional Plug-ins Folder and `plugin.reload` install only plug-ins whose permissions were granted, and report the others. -- The host enforces the grant on a filter run: without `doc.read` the filter is given no objects; without `state` its +- The host enforces the grant on a filter run and a command run: without `doc.read` the filter is given no objects; without `state` its `documentData` is `null` and an output that sets it is refused; without `doc.write` an output that changes the document is refused. A refused output leaves the document as it was. - `api` is the plug-in API level the manifest is written for: `1` (the default) is the only level this host diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 52d8aa2b9..b767e347b 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1429,7 +1429,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - **Plug-in data kept with the document (G17):** a filter writes JSON of at most 1 MiB with `documentData` (one undo step with its objects; `null` removes it) and reads it back in `_context.documentData`, which no other filter is given. It is the document's foreign data (`unknown.pluginData`, by plug-in id), as the perspective grid is, so the native file keeps it with no section of its own; an exporter is given the whole document, so it can read every plug-in's. Regressions: `crates/engine/src/tests/plugins.rs`, `crates/engine/tests/plugin_export.rs` (the exporter), `crates/doc/src/plugin_data.rs`, `crates/plugins/src/objects/tests.rs`. Scores and hours unchanged. - **Heavy filter plug-ins (G17):** a filter's manifest may ask for `"budget": "heavy"`: 1 G instructions (20 times a light run's) and 300 s a run, for routing, packing or simulation. Install Plug-in… and File › Open ask the user first, `plugin.install` needs `allowHeavy: true` (`plugin.inspect` tells beforehand), the Additional Plug-ins Folder skips such filters, and the plug-in store keeps the answer. Its dialog has no Preview; the app runs it on a thread of its own with a progress window (Cancel, Continue in Background) and applies the result as `plugin.run` with the same parameters, one undo step, unless the document or the plug-in changed meanwhile. Regressions: `crates/plugins/tests/abi.rs`, `crates/plugins/src/manifest.rs`, `crates/engine/src/tests/plugins.rs`, `crates/ui-egui/src/tests/plugin_runs.rs`. Scores and hours unchanged. - **Plug-in permissions (G17):** a manifest may list `permissions` (`doc.read`, `doc.write`, `state`, `effect.render`, `export.write`; the first three follow storytold/photocraft#662) and an `api` level; without the list a plug-in gets what its kind implies and installs as before. Install Plug-in… and File › Open show the name, id, version, file SHA-256 and each permission in plain words, with control and bidirectional characters escaped, and install nothing on Cancel; `plugin.install` needs `grant`, and folders install only granted plug-ins. The grant is kept with the module's hash, a module asking for more asks again, and a filter run refuses output its permissions don't allow. Regressions: `crates/plugins/src/permissions.rs`, `crates/plugins/src/manifest.rs`, `crates/engine/src/tests/plugins.rs`, `crates/engine/tests/plugin_fuzz.rs`, `crates/ui-egui/src/tests/plugins.rs`. Scores and hours unchanged. -- **Plug-in commands (G17):** a filter or live effect may declare up to 32 `commands` (`{id, label, params?}`); each is the command `plugin..` while the plug-in is installed, in a submenu named after the plug-in under Object › Plug-ins (Effect › Plug-ins for a live effect) whatever menu the manifest names, with its label escaped. Commands are recorded in actions, listed in Keyboard Shortcuts for the user to give a shortcut, and run through `run` and MCP with their parameters checked first (a command with parameters opens a dialog from its menu item or shortcut). A run uses the plug-in's permissions, is one undo step named after the command, and on failure (a trap or an exhausted instruction budget) leaves the document and the undo history as they were with an error naming the plug-in and the command. For a command and for `plugin.run`, an `ids` that is not a list of object ids is an error rather than the selection. A malformed entry is refused by name when the module is installed. Regressions: `crates/plugins/src/manifest.rs`, `crates/engine/src/tests/plugins.rs`, `crates/engine/tests/plugin_fuzz.rs`, `crates/ui-egui/src/tests/plugins.rs`. Scores and hours unchanged. +- **Plug-in commands (G17):** a filter or live effect may declare up to 32 `commands` (`{id, label, params?}`); each is the command `plugin..` while the plug-in is installed, in a submenu named after the plug-in under Object › Plug-ins (Effect › Plug-ins for a live effect) whatever menu the manifest names, with its label escaped. Commands are recorded in actions, listed in Keyboard Shortcuts for the user to give a shortcut, and run through `run` and MCP with their parameters checked first (a command with parameters opens a dialog from its menu item or shortcut). A run uses the plug-in's permissions (a live effect with commands may also hold `doc.write` and `state`, and gets them without a `permissions` list), is one undo step named after the command, and on failure (a trap or an exhausted instruction budget) leaves the document and the undo history as they were with an error naming the plug-in and the command. For a command and for `plugin.run`, an `ids` that is not a list of object ids is an error rather than the selection. A malformed entry is refused by name when the module is installed. Regressions: `crates/plugins/src/manifest.rs`, `crates/plugins/src/permissions.rs`, `crates/engine/src/tests/plugins.rs`, `crates/engine/tests/plugin_fuzz.rs`, `crates/ui-egui/src/tests/plugins.rs`. Scores and hours unchanged. - **Plug-in effects after a plug-in changes:** when a plug-in is installed for the first time, installed again or removed (`plugin.install`, `plugin.reload`, `plugin.remove`, File › Open of a `.wasm` file, the Additional Plug-ins Folder preference), the next frame the canvas draws uses the plug-ins then installed for objects with a plug-in effect and their raster effects, for the blends, envelopes and repeats that show such objects, and for pattern tiles and symbol instances whose art holds one.