From cde367b889399232e6809637d8dc8196aa33db8f Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 17:35:06 -0800 Subject: [PATCH] Plug-ins: only an approval records a grant, and installs need one registry::grant takes a GrantToken, which the engine makes after a person's approval and when it loads grants.json. registry::install, install_bytes and folder loads refuse a module that lists permissions no recorded grant gives this very module (Error::NeedsApproval), and leave the plug-in under its id and the revision as they were. A folder load's approve callback only narrows what installs. A module without permissions, or with an empty list, installs as before. A folder load in a session with a plug-in store records the store's grant for each module it approves, so those modules still load. docs/plugins.md says what the approval does not protect against: native code linked into the app, and another program running as the same user, which can edit the Plug-ins folder and its unsigned grants.json. --- crates/engine/src/cmd/plugin.rs | 9 +- crates/engine/src/store.rs | 32 ++++++- crates/engine/src/tests/plugins.rs | 64 +++++++++++-- crates/engine/tests/plugin_import.rs | 2 +- crates/plugins/src/effect.rs | 16 ++-- crates/plugins/src/lib.rs | 4 + crates/plugins/src/permissions.rs | 26 ++++++ crates/plugins/src/registry.rs | 134 +++++++++++++++++++++++---- crates/plugins/tests/grant_sites.rs | 82 ++++++++++++++++ crates/plugins/tests/safe_mode.rs | 4 +- docs/plugins.md | 7 ++ docs/target-app-parity.md | 1 + 12 files changed, 339 insertions(+), 42 deletions(-) create mode 100644 crates/plugins/tests/grant_sites.rs diff --git a/crates/engine/src/cmd/plugin.rs b/crates/engine/src/cmd/plugin.rs index a5b71d538..909c9de1c 100644 --- a/crates/engine/src/cmd/plugin.rs +++ b/crates/engine/src/cmd/plugin.rs @@ -52,7 +52,7 @@ use vectorcraft_plugins::{Budget, Command, Kind, Plugin, effect, export, objects use super::{CommandSpec, EngineError, NodeId, Result, Session, always, bad, cmd, has_doc, ids_param, str_param, switch_or}; -use vectorcraft_plugins::permissions::{self, Grant, Permission}; +use vectorcraft_plugins::permissions::{self, Grant, GrantToken, Permission}; /// What a plug-in error looks like to the user. A run the caller cancelled changed nothing. fn err(e: vectorcraft_plugins::Error) -> EngineError { @@ -748,7 +748,7 @@ fn install(s: &mut Session, p: &Value) -> Result { let mut stored = (previous.is_ok() || !m.asks_permissions()) && s.store_plugin(plugin.id(), &bytes); if m.asks_permissions() { let g = Grant { id: m.id.clone(), hash: plugin.hash().to_string(), permissions: wanted }; - registry::grant(g.clone()); + registry::grant(GrantToken::after_engine_approval(g.clone())); if stored && !s.store_grant(&m.id, Some(g)) { s.put_back_plugin(plugin.id(), previous.ok().flatten().as_deref()); stored = false; @@ -786,7 +786,8 @@ fn install(s: &mut Session, p: &Value) -> Result { if changed { s.store_disabled_plugins(); } - let mut v = describe_installed(®istry::install(plugin)); + let installed = registry::install(plugin).map_err(err)?; + let mut v = describe_installed(&installed); if !unknown.is_empty() { v["unknownCommands"] = json!(unknown); } @@ -885,7 +886,7 @@ fn load_folder(s: &Session, folder: &str) -> Value { None if crate::file_access::current().is_none() => Ok(()), None => Err(format!("automation path rejected: {} isn't valid text", f.display())), }; - match registry::load_folder_where(std::path::Path::new(folder), allow, s.plugin_approver()) { + match registry::load_folder_where(std::path::Path::new(folder), allow, s.plugin_folder_approver()) { Ok(r) => { for id in &r.loaded { s.attach_plugin_storage(id); diff --git a/crates/engine/src/store.rs b/crates/engine/src/store.rs index 01ebc4652..66501f6aa 100644 --- a/crates/engine/src/store.rs +++ b/crates/engine/src/store.rs @@ -39,6 +39,7 @@ use std::sync::{Arc, Mutex, MutexGuard}; use std::time::SystemTime; use vectorcraft_pdf::{PdfPreset, builtin_preset}; +use vectorcraft_plugins::permissions::GrantToken; use vectorcraft_plugins::safe_mode; use vectorcraft_plugins::storage::{self, Meter, Storage, StorageError}; @@ -383,9 +384,11 @@ impl Session { match grants.get(&m.id).filter(|g| grant_covers(g, &plugin)) { Some(g) => { asks_nothing.remove(&m.id); - vectorcraft_plugins::registry::grant(g.clone()); + vectorcraft_plugins::registry::grant(GrantToken::from_engine_grants_file(g.clone())); vectorcraft_plugins::registry::set_storage(&m.id, plugin_storage(Some(&dir), &m.id)); - vectorcraft_plugins::registry::install(plugin.with_source(path.display().to_string())); + if let Err(e) = vectorcraft_plugins::registry::install(plugin.with_source(path.display().to_string())) { + log::warn!("skipping the stored plug-in {}: {e}", path.display()); + } } None => { log::warn!("skipping the stored plug-in {}: the user hasn't granted this module the permissions it lists", path.display()) @@ -395,7 +398,9 @@ impl Session { Ok(plugin) => { asks_nothing.insert(plugin.id().to_string()); vectorcraft_plugins::registry::set_storage(plugin.id(), plugin_storage(Some(&dir), plugin.id())); - vectorcraft_plugins::registry::install(plugin.with_source(path.display().to_string())); + if let Err(e) = vectorcraft_plugins::registry::install(plugin.with_source(path.display().to_string())) { + log::warn!("skipping the stored plug-in {}: {e}", path.display()); + } } Err(e) => log::warn!("skipping the stored plug-in {}: {e}", path.display()), } @@ -469,6 +474,27 @@ impl Session { } } + /// [`Session::plugin_approver`] for a folder load: a module the store's grants approve gets + /// that grant recorded for the process too, since the registry installs only a module that a + /// recorded grant covers ([`vectorcraft_plugins::registry::install`]). + #[cfg(not(target_arch = "wasm32"))] + pub(crate) fn plugin_folder_approver(&self) -> impl Fn(&vectorcraft_plugins::Plugin) -> bool + use<> { + let grants = self.plugin_dir.as_deref().map(|dir| self.counted_grants(dir)); + move |plugin| { + let m = plugin.manifest(); + match &grants { + Some(grants) if m.asks_permissions() => match grants.get(&m.id).filter(|g| grant_covers(g, plugin)) { + Some(g) => { + vectorcraft_plugins::registry::grant(GrantToken::from_engine_grants_file(g.clone())); + true + } + None => false, + }, + _ => vectorcraft_plugins::registry::approved(plugin), + } + } + } + /// The grants kept in the plug-in store `dir` ([`stored_grants`]) without those this session /// doesn't count ([`Session::ignore_stored_grant`]). fn counted_grants(&self, dir: &Path) -> BTreeMap { diff --git a/crates/engine/src/tests/plugins.rs b/crates/engine/src/tests/plugins.rs index a4be6e554..324961a71 100644 --- a/crates/engine/src/tests/plugins.rs +++ b/crates/engine/src/tests/plugins.rs @@ -1587,7 +1587,7 @@ fn a_module_without_permissions_ends_the_grant_of_its_id() { /// that holds such a module and a grant for its id has that grant forgotten when it is attached. #[test] fn a_module_without_permissions_ends_the_grant_kept_in_the_store() { - use vectorcraft_plugins::permissions::{Grant, Permission, sha256_hex}; + use vectorcraft_plugins::permissions::{Grant, GrantToken, Permission, sha256_hex}; let folder = crate::tests_links::Folder::new("plugin-store-stale-grant"); let id = "test.engine.permissions.storedstale"; let other = "test.engine.permissions.storedother"; @@ -1634,7 +1634,11 @@ fn a_module_without_permissions_ends_the_grant_kept_in_the_store() { let future = json!({"id": id, "hash": "00", "permissions": ["doc.read", "ui.panel"]}); std::fs::write(&grants, serde_json::to_vec(&json!([future, other_entry])).unwrap()).unwrap(); vectorcraft_plugins::registry::remove(id); - vectorcraft_plugins::registry::grant(Grant { id: id.into(), hash: sha256_hex(&approved), permissions: vec![Permission::DocRead] }); + vectorcraft_plugins::registry::grant(GrantToken::after_engine_approval(Grant { + id: id.into(), + hash: sha256_hex(&approved), + permissions: vec![Permission::DocRead], + })); let mut fourth = session(); fourth.attach_plugin_store(folder.0.clone()); assert_eq!(ids(), vec![other.to_string()]); @@ -2046,7 +2050,7 @@ fn a_plugin_folder_ends_the_process_grant_of_an_id_it_replaces() { /// With a store attached, a matching grant held only by the process doesn't count. #[test] fn a_stored_grant_covers_only_the_stored_module_it_recorded() { - use vectorcraft_plugins::permissions::{Grant, Permission, sha256_hex}; + use vectorcraft_plugins::permissions::{Grant, GrantToken, Permission, sha256_hex}; let folder = crate::tests_links::Folder::new("plugin-store-recorded-module"); let id = "test.engine.permissions.storedbytes"; let first = asking(id, r#"["doc.read"]"#, &json!({"objects": []})); @@ -2091,7 +2095,11 @@ fn a_stored_grant_covers_only_the_stored_module_it_recorded() { assert!(starts()); let mut attached = session(); attached.attach_plugin_store(folder.0.clone()); - vectorcraft_plugins::registry::grant(Grant { id: id.into(), hash: sha256_hex(&changed), permissions: vec![Permission::DocRead] }); + vectorcraft_plugins::registry::grant(GrantToken::after_engine_approval(Grant { + id: id.into(), + hash: sha256_hex(&changed), + permissions: vec![Permission::DocRead], + })); let inspected = run(&mut attached, "plugin.inspect", json!({"dataBase64": vectorcraft_format::base64_encode(&changed)})); assert_eq!(inspected["needsApproval"], true, "a grant held only by the process doesn't count with a store attached"); run(&mut attached, "plugin.remove", json!({"id": id})); @@ -2361,8 +2369,15 @@ fn a_run_past_the_memory_budget_fails_before_the_plugin_starts() { params.join(",") ); let module = wat::parse_str(tpl::module(&manifest, "unreachable", "", 1)).unwrap(); - // One page of memory: the input and the parameters share 32 KiB. - registry::install(Plugin::load(&module, Limits { max_memory_bytes: 1 << 16, ..Limits::default() }).unwrap()); + // One page of memory: the input and the parameters share 32 KiB. The module lists permissions, + // so its grant is recorded first. + let plugin = Plugin::load(&module, Limits { max_memory_bytes: 1 << 16, ..Limits::default() }).unwrap(); + registry::grant(vectorcraft_plugins::permissions::GrantToken::after_engine_approval(vectorcraft_plugins::permissions::Grant { + id: id.into(), + hash: plugin.hash().to_string(), + permissions: plugin.manifest().permissions(), + })); + registry::install(plugin).unwrap(); let mut s = session(); let small = rect(&mut s, 10.0, "#ff0000"); let long = rect(&mut s, 30.0, "#00ff00"); @@ -2403,7 +2418,7 @@ fn a_run_past_the_memory_budget_fails_before_the_plugin_starts() { assert!(e.contains(says), "{says}: {e}"); assert_eq!((s.doc().unwrap().doc.clone(), s.doc().unwrap().history.undo.len()), before, "{says}"); } - assert!(registry::remove(id)); + assert!(registry::remove(id) && registry::revoke(id)); } /// What the user granted is kept in the plug-in store with the module: the next session installs @@ -2494,6 +2509,41 @@ fn granted_permissions_are_kept_and_folders_skip_the_rest() { run(&mut granted, "plugin.remove", json!({"id": id})); } +/// The registry installs a module that lists permissions only under a grant the process holds, so +/// a start and a `plugin.reload` record the store's grant for the module they load. A module in the +/// folder that nobody granted is reported and skipped, and records nothing. +#[test] +fn stored_and_reload_grants_still_install() { + let _folder = PREFS_FOLDER.lock().unwrap_or_else(|e| e.into_inner()); + let store = crate::tests_links::Folder::new("plugin-store-grants-reload"); + let id = "test.engine.permissions.reload"; + let lone = "test.engine.permissions.reload.lone"; + let wasm = asking(id, r#"["doc.read"]"#, &json!({"objects": []})); + let mut s = session(); + s.attach_plugin_store(store.0.clone()); + run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&wasm), "grant": ["doc.read"]})); + let forget = || { + vectorcraft_plugins::registry::remove(id); + vectorcraft_plugins::registry::revoke(id); + }; + forget(); + let mut next = session(); + next.attach_plugin_store(store.0.clone()); + assert!(vectorcraft_plugins::registry::get(id).is_some(), "installed again from the store"); + assert_eq!(vectorcraft_plugins::registry::granted(id).map(|g| g.hash), Some(vectorcraft_plugins::permissions::sha256_hex(&wasm))); + forget(); + let dir = crate::tests_links::Folder::new("plugin-folder-grants-reload"); + std::fs::write(dir.0.join("granted.wasm"), &wasm).unwrap(); + std::fs::write(dir.0.join("lone.wasm"), asking(lone, r#"["doc.read"]"#, &json!({"objects": []}))).unwrap(); + let r = run(&mut next, "plugin.reload", json!({"path": dir.0.display().to_string()})); + assert_eq!(r["loaded"], json!([id]), "{r}"); + assert!(vectorcraft_plugins::registry::granted(id).is_some(), "the store's grant is the process's too"); + let why = r["failed"][0]["error"].as_str().unwrap(); + assert!(why.contains("needs approval") && r["failed"][0]["file"].as_str().unwrap().ends_with("lone.wasm"), "{r}"); + assert!(vectorcraft_plugins::registry::get(lone).is_none() && vectorcraft_plugins::registry::granted(lone).is_none()); + run(&mut next, "plugin.remove", json!({"id": id})); +} + /// A grant covers the module it was given to: another module under the id, listing the same /// permissions, isn't installed from the store at the next start, a folder load reports it as /// needing approval, and `plugin.install` asks for `grant` again, even with `replace: true`. It diff --git a/crates/engine/tests/plugin_import.rs b/crates/engine/tests/plugin_import.rs index b780c115c..f09106230 100644 --- a/crates/engine/tests/plugin_import.rs +++ b/crates/engine/tests/plugin_import.rs @@ -326,7 +326,7 @@ fn an_importer_past_its_budget_opens_nothing() { ]; for (id, limits, _) in &cases { let wasm = wat::parse_str(tpl::module(&tpl::importer(id, r#"["tbg"]"#, "{}"), SPINS, "", 1)).unwrap(); - vectorcraft_plugins::registry::install(Plugin::load(&wasm, limits.clone()).unwrap()); + vectorcraft_plugins::registry::install(Plugin::load(&wasm, limits.clone()).unwrap()).unwrap(); } let mut s = Session::new(); for (id, _, why) in &cases { diff --git a/crates/plugins/src/effect.rs b/crates/plugins/src/effect.rs index f06d17e24..6d549ed07 100644 --- a/crates/plugins/src/effect.rs +++ b/crates/plugins/src/effect.rs @@ -552,7 +552,7 @@ mod tests { // Traps on a long input. let first = "(if (i32.gt_u (local.get $il) (i32.const 4096)) (then unreachable))"; let src = crate::wat::with_imports(&manifest, "", first, r#"{"objects":[]}"#); - registry::install(crate::Plugin::load(&wat::parse_str(&src).unwrap(), Limits::default()).unwrap()); + registry::install(crate::Plugin::load(&wat::parse_str(&src).unwrap(), Limits::default()).unwrap()).unwrap(); let doc = Document::new(100.0, 100.0); let line = |n: usize| PathData::single(SubPath::polyline(&(0..n).map(|i| Point::new(i as f64, 0.0)).collect::>(), false)); let draw = @@ -583,7 +583,7 @@ mod tests { // Traps on a long input. let first = "(if (i32.gt_u (local.get $il) (i32.const 4096)) (then unreachable))"; let src = crate::wat::with_imports(&manifest, "", first, &output); - registry::install(crate::Plugin::load(&wat::parse_str(&src).unwrap(), Limits::default()).unwrap()); + registry::install(crate::Plugin::load(&wat::parse_str(&src).unwrap(), Limits::default()).unwrap()).unwrap(); let doc = Document::new(100.0, 100.0); let line = |n: usize| PathData::single(SubPath::polyline(&(0..n).map(|i| Point::new(i as f64, 0.0)).collect::>(), false)); let draw = @@ -647,7 +647,7 @@ mod tests { crate::safe_mode::set_active(None); const ID: &str = "test.effect.budget"; let src = crate::wat::returning(&crate::wat::manifest(ID, "effect", "{}"), r#"{"objects":[]}"#); - registry::install(crate::Plugin::load(&wat::parse_str(&src).unwrap(), Limits::default()).unwrap()); + registry::install(crate::Plugin::load(&wat::parse_str(&src).unwrap(), Limits::default()).unwrap()).unwrap(); // Each run takes 900 ms by this clock (it is read before and after each run). let view = crate::deferred::Deferrals::with_clock(stepping(450)); let ran = view.view(|| (0..4).map(|i| drawn(ID, &line(i + 2), i)).collect::>()); @@ -677,7 +677,7 @@ mod tests { const ID: &str = "test.effect.deferfailed"; // Runs to its instruction budget. let src = crate::wat::spinning(&crate::wat::manifest(ID, "effect", "{}"), 1_000_000_000, r#"{"objects":[]}"#); - registry::install(crate::Plugin::load(&wat::parse_str(&src).unwrap(), Limits::default()).unwrap()); + registry::install(crate::Plugin::load(&wat::parse_str(&src).unwrap(), Limits::default()).unwrap()).unwrap(); let view = crate::deferred::Deferrals::with_clock(stepping(1)); assert!(!view.view(|| drawn(ID, &line(2), 0))); assert!(last_error(ID).is_some()); @@ -712,9 +712,9 @@ mod tests { const FAST: &str = "test.effect.cutshort.fast"; let src = crate::wat::spinning(&crate::wat::manifest(ID, "effect", "{}"), 10_000_000, r#"{"objects":[]}"#); let limits = Limits { fuel_base: 1_000_000_000, ..Limits::default() }; - registry::install(crate::Plugin::load(&wat::parse_str(&src).unwrap(), limits).unwrap()); + registry::install(crate::Plugin::load(&wat::parse_str(&src).unwrap(), limits).unwrap()).unwrap(); let fast = crate::wat::returning(&crate::wat::manifest(FAST, "effect", "{}"), r#"{"objects":[]}"#); - registry::install(crate::Plugin::load(&wat::parse_str(&fast).unwrap(), Limits::default()).unwrap()); + registry::install(crate::Plugin::load(&wat::parse_str(&fast).unwrap(), Limits::default()).unwrap()).unwrap(); // The first run starts the budget at 0 ms and ends at 1,999: the next has 1 ms left, and // ten million steps take longer. let reads = Arc::new(std::sync::atomic::AtomicU64::new(0)); @@ -737,7 +737,7 @@ mod tests { let _lock = crate::safe_mode::TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); crate::safe_mode::set_active(None); let src = crate::wat::spinning(&crate::wat::manifest("test.effect.cancelled", "effect", "{}"), 1_000_000_000, r#"{"objects":[]}"#); - let plugin = registry::install(crate::Plugin::load(&wat::parse_str(&src).unwrap(), Limits::default()).unwrap()); + let plugin = registry::install(crate::Plugin::load(&wat::parse_str(&src).unwrap(), Limits::default()).unwrap()).unwrap(); let ran_first = || RUNS.lock().unwrap_or_else(|e| e.into_inner()).as_ref().is_some_and(|r| matches!(r.get(&plugin.serial()), Some(Runs::Done))); let draw = || { @@ -777,7 +777,7 @@ mod tests { crate::safe_mode::parse_marker(&std::fs::read(&json[0]).unwrap()) }; let src = crate::wat::returning(&crate::wat::manifest("test.effect.overlap", "effect", "{}"), r#"{"objects":[]}"#); - let plugin = registry::install(crate::Plugin::load(&wat::parse_str(&src).unwrap(), Limits::default()).unwrap()); + let plugin = registry::install(crate::Plugin::load(&wat::parse_str(&src).unwrap(), Limits::default()).unwrap()).unwrap(); let named = Some(crate::safe_mode::Item::of(&plugin)); assert_eq!(marker(), None); diff --git a/crates/plugins/src/lib.rs b/crates/plugins/src/lib.rs index c71200187..b304e1721 100644 --- a/crates/plugins/src/lib.rs +++ b/crates/plugins/src/lib.rs @@ -89,6 +89,10 @@ pub enum Error { /// Safe Mode is on ([`safe_mode`]): no plug-in loads or runs. #[error("Safe Mode is on: plug-ins don't load or run until VectorCraft starts normally")] SafeMode, + /// The module lists permissions that no recorded grant gives this very module + /// ([`registry::approved`]): it names them, comma-separated. + #[error("needs approval: it asks for {0}, which the user grants when installing it on its own")] + NeedsApproval(String), #[error("{0}")] Io(String), } diff --git a/crates/plugins/src/permissions.rs b/crates/plugins/src/permissions.rs index 7380edd56..b8c867269 100644 --- a/crates/plugins/src/permissions.rs +++ b/crates/plugins/src/permissions.rs @@ -275,6 +275,32 @@ impl Grant { } } +/// A [`Grant`] that [`crate::registry::grant`] records. The engine makes one where it records what +/// a person approved and where it loads the grants kept in its plug-in folder; tests that set up a +/// grant use the same constructors. `tests/grant_sites.rs` fails when other shipped code makes one. +/// Native code linked into the app can still call these constructors, so the token does not +/// protect against such code. +#[derive(Debug)] +pub struct GrantToken(Grant); + +impl GrantToken { + /// The grant the engine records after a person approved the module it covers, or in a session + /// whose starter allowed requests to grant permissions (`--allow-plugin-grants`). + pub fn after_engine_approval(grant: Grant) -> Self { + Self(grant) + } + + /// A grant the engine read from the grants file of its plug-in folder, for a module it loads. + pub fn from_engine_grants_file(grant: Grant) -> Self { + Self(grant) + } + + /// The grant it holds. + pub fn grant(&self) -> &Grant { + &self.0 + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/plugins/src/registry.rs b/crates/plugins/src/registry.rs index be7f12180..ee3732908 100644 --- a/crates/plugins/src/registry.rs +++ b/crates/plugins/src/registry.rs @@ -7,9 +7,8 @@ use std::collections::{BTreeMap, HashSet}; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{Arc, Mutex, RwLock}; -#[cfg(not(target_arch = "wasm32"))] use crate::Error; -use crate::permissions::Grant; +use crate::permissions::{Grant, GrantToken}; use crate::storage::Storage; use crate::{Limits, Plugin, Result}; @@ -35,8 +34,11 @@ pub fn storage(id: &str) -> Option> { STORAGE.read().unwrap_or_else(|e| e.into_inner()).get(id).cloned() } -/// Records what the user approved for a plug-in, replacing an earlier grant for its id. -pub fn grant(g: Grant) { +/// Records what the user approved for a plug-in, replacing an earlier grant for its id. Only a +/// [`GrantToken`] is recorded, which the engine makes after an approval or when it loads its grants +/// file. +pub fn grant(token: GrantToken) { + let g = token.grant().clone(); GRANTS.write().unwrap_or_else(|e| e.into_inner()).insert(g.id.clone(), g); } @@ -63,20 +65,27 @@ pub fn approved(plugin: &Plugin) -> bool { !m.asks_permissions() || granted(&m.id).is_some_and(|g| g.covers(&m.id, plugin.hash(), &m.permissions())) } -/// Installs `plugin`, replacing one with the same id. -pub fn install(mut plugin: Plugin) -> Arc { +/// Installs `plugin`, replacing one with the same id, when it is [`approved`]: a module that lists +/// permissions no recorded grant gives this very module is refused ([`Error::NeedsApproval`]), and +/// the plug-in installed under its id, the grants and the revision stay as they were. +pub fn install(mut plugin: Plugin) -> Result> { // The revision moves while the write lock is held, so a cache that reads the new revision and - // then the plug-ins finds the new plug-in among them. + // then the plug-ins finds the new plug-in among them. The approval is checked while it is held + // too. let mut plugins = PLUGINS.write().unwrap_or_else(|e| e.into_inner()); + if !approved(&plugin) { + return Err(Error::NeedsApproval(crate::permissions::names(&plugin.manifest().permissions()).join(", "))); + } plugin.set_serial(REVISION.fetch_add(1, Ordering::Relaxed) + 1); let p = Arc::new(plugin); plugins.insert(p.id().to_string(), p.clone()); - p + Ok(p) } -/// Loads `bytes` with the default limits and installs the plug-in. +/// Loads `bytes` with the default limits and installs the plug-in ([`install`]: one that lists +/// permissions installs only when they were granted to this very module). pub fn install_bytes(bytes: &[u8]) -> Result> { - Ok(install(Plugin::load(bytes, Limits::default())?)) + install(Plugin::load(bytes, Limits::default())?) } pub fn get(id: &str) -> Option> { @@ -162,7 +171,8 @@ pub fn load_folder(dir: &std::path::Path) -> Result { } /// [`load_folder`], loading only the files `allow` accepts, and of the plug-ins that list -/// permissions only those `approve` accepts; the others are reported as failed with the reason. A plug-in the user disabled ([`crate::safe_mode::skipped`]) is reported and +/// permissions only those `approve` accepts and a recorded grant covers ([`install`]); the others +/// are reported as failed with the reason. A plug-in the user disabled ([`crate::safe_mode::skipped`]) is reported and /// skipped, by its file before it loads or by its id after (its file is then disabled too, so the /// next load skips it before it runs); the launch marker names each file while it loads /// ([`crate::safe_mode::loading`]). In Safe Mode nothing loads ([`Error::SafeMode`]). When the @@ -206,12 +216,14 @@ pub fn load_folder_where( } Ok(p) if p.manifest().asks_permissions() && !approve(&p) => { let asked = crate::permissions::names(&p.manifest().permissions()).join(", "); - report.failed.push(( - f.display().to_string(), - format!("needs approval: it asks for {asked}, which the user grants when installing it on its own"), - )); + report.failed.push((f.display().to_string(), Error::NeedsApproval(asked).to_string())); } - Ok(p) => report.loaded.push(install(p).id().to_string()), + // `approve` only narrows what installs: `install` still refuses a module that no + // recorded grant covers. + Ok(p) => match install(p) { + Ok(p) => report.loaded.push(p.id().to_string()), + Err(e) => report.failed.push((f.display().to_string(), e.to_string())), + }, Err(e) => report.failed.push((f.display().to_string(), e.to_string())), } } @@ -231,6 +243,7 @@ mod tests { /// waiting for the set leaves the revision alone until it holds the set. #[test] fn an_install_changes_the_revision_only_while_it_holds_the_set() { + let _lock = crate::safe_mode::TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let plugin = Plugin::load(include_bytes!("../tests/fixtures/desaturate.wasm"), Limits::default()).unwrap(); let id = plugin.id().to_string(); let held = PLUGINS.read().unwrap_or_else(|e| e.into_inner()); @@ -241,8 +254,95 @@ mod tests { revision() != before }); drop(held); - installing.join().unwrap(); + installing.join().unwrap().unwrap(); assert!(!moved, "the revision changed while the install waited for the set"); assert!(revision() > before && remove(&id)); } + + /// A module of a filter `id` whose manifest lists `permissions` (a JSON list, or nothing), with + /// `marker` in its code so that two markers give two SHA-256s. + fn module(id: &str, permissions: Option<&str>, marker: i64) -> Vec { + let listed = permissions.map(|p| format!(r#","permissions":{p}"#)).unwrap_or_default(); + let manifest = format!(r#"{{"id":"{id}","name":"T","kind":"filter"{listed}}}"#); + wat::parse_str(crate::wat::module(&manifest, &format!("(i64.const {marker}) (drop) (i64.const 0)"), "", 1)).unwrap() + } + + fn grant_for(id: &str, bytes: &[u8], permissions: Vec) -> Grant { + Grant { id: id.into(), hash: crate::permissions::sha256_hex(bytes), permissions } + } + + /// A module that lists permissions installs only once a recorded grant gives this very module + /// all of them: refused, nothing changes, neither the revision nor the plug-in under its id. + #[test] + fn install_bytes_refuses_unapproved_manifest_permissions() { + // The other tests that install or remove plug-ins hold it, so the revision moves only here. + let _lock = crate::safe_mode::TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let id = "test.registry.unapproved"; + let plain = install_bytes(&module(id, None, 1)).unwrap(); + let before = revision(); + let e = install_bytes(&module(id, Some(r#"["doc.read"]"#), 2)).unwrap_err(); + assert_eq!(e, Error::NeedsApproval("doc.read".into())); + assert_eq!(revision(), before, "nothing installed"); + assert_eq!(get(id).map(|p| p.hash().to_string()), Some(plain.hash().to_string()), "the plug-in before it stays"); + assert!(granted(id).is_none()); + assert!(remove(id)); + } + + /// `install` itself and a folder load whose `approve` accepts everything refuse the module too. + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn install_and_folder_callback_cannot_bypass_approval() { + let _lock = crate::safe_mode::TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + crate::safe_mode::set_active(None); + let id = "test.registry.bypass"; + let plain = install_bytes(&module(id, Some("[]"), 3)).unwrap(); + let asking = module(id, Some(r#"["doc.read","doc.write"]"#), 4); + let e = install(Plugin::load(&asking, Limits::default()).unwrap()).unwrap_err(); + assert_eq!(e, Error::NeedsApproval("doc.read, doc.write".into())); + let dir = std::env::temp_dir().join(format!("vc-registry-bypass-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("asking.wasm"), &asking).unwrap(); + let r = load_folder_where(&dir, |_| Ok(()), |_| true).unwrap(); + std::fs::remove_dir_all(&dir).unwrap(); + assert!(r.loaded.is_empty(), "{r:?}"); + assert_eq!(r.failed.len(), 1); + assert!(r.failed[0].1.starts_with("needs approval: it asks for doc.read, doc.write"), "{r:?}"); + assert_eq!(get(id).map(|p| p.hash().to_string()), Some(plain.hash().to_string()), "the plug-in before it stays"); + assert!(remove(id)); + } + + /// A recorded grant lets in only the module with its id and SHA-256 that lists nothing it + /// doesn't grant; a module whose manifest lists no permissions, or an empty list, needs none. + #[test] + fn matching_grant_allows_only_exact_bytes() { + use crate::permissions::Permission::{DocRead, DocWrite}; + let _lock = crate::safe_mode::TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let id = "test.registry.exact"; + let bytes = module(id, Some(r#"["doc.read","doc.write"]"#), 5); + let plugin = Plugin::load(&bytes, Limits::default()).unwrap(); + assert_eq!(plugin.hash(), crate::permissions::sha256_hex(&bytes), "a grant's hash is the loaded module's"); + let refused = |g: Grant, why: &str| { + let other = g.id.clone(); + revoke(id); + grant(GrantToken::after_engine_approval(g)); + assert!(matches!(install_bytes(&bytes), Err(Error::NeedsApproval(_))), "{why}"); + assert!(get(id).is_none(), "{why}"); + revoke(&other); + }; + refused(grant_for(id, &module(id, Some(r#"["doc.read","doc.write"]"#), 6), vec![DocRead, DocWrite]), "other bytes"); + refused(grant_for("test.registry.other", &bytes, vec![DocRead, DocWrite]), "another id"); + refused(Grant { id: id.into(), hash: "not a hash".into(), permissions: vec![DocRead, DocWrite] }, "a malformed hash"); + refused( + Grant { id: id.into(), hash: crate::permissions::sha256_hex(&bytes).to_uppercase(), permissions: vec![DocRead, DocWrite] }, + "an upper-case hash", + ); + refused(grant_for(id, &bytes, vec![DocRead]), "a missing permission"); + grant(GrantToken::from_engine_grants_file(grant_for(id, &bytes, vec![DocRead, DocWrite]))); + assert_eq!(install_bytes(&bytes).unwrap().hash(), plugin.hash()); + assert!(remove(id) && revoke(id)); + for listed in [None, Some("[]")] { + assert!(install_bytes(&module(id, listed, 7)).is_ok(), "{listed:?}"); + assert!(granted(id).is_none() && remove(id), "{listed:?}"); + } + } } diff --git a/crates/plugins/tests/grant_sites.rs b/crates/plugins/tests/grant_sites.rs new file mode 100644 index 000000000..d0c047933 --- /dev/null +++ b/crates/plugins/tests/grant_sites.rs @@ -0,0 +1,82 @@ +//! The shipped code that records a plug-in grant: only the engine's install approval and its load +//! of the plug-in folder's grants file make a `GrantToken` and call `registry::grant`. A new call +//! site fails this test until it is added to the list. It checks the source, not what native code +//! can do at run time. +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; + +/// `(text, file, how many times)` for every place shipped code may name it. +const EXPECTED: &[(&str, &str, usize)] = &[ + ("GrantToken::after_engine_approval", "crates/engine/src/cmd/plugin.rs", 1), + ("GrantToken::from_engine_grants_file", "crates/engine/src/store.rs", 2), + ("registry::grant", "crates/engine/src/cmd/plugin.rs", 1), + ("registry::grant", "crates/engine/src/store.rs", 2), +]; + +/// Every `.rs` file under `dir`, without test files and folders. +fn sources(dir: &Path, out: &mut Vec) { + let Ok(entries) = std::fs::read_dir(dir) else { return }; + for entry in entries.flatten() { + let path = entry.path(); + let name = entry.file_name().to_string_lossy().to_string(); + if path.is_dir() { + if !matches!(name.as_str(), "tests" | "benches" | "examples" | "target" | "fuzz") { + sources(&path, out); + } + } else if name.ends_with(".rs") && name != "tests.rs" && !name.starts_with("tests_") && !name.ends_with("_tests.rs") { + out.push(path); + } + } +} + +/// The lines of `text` that ship: not comments, and nothing from a top-level `#[cfg(test)] mod x {` +/// on. +fn shipped(text: &str) -> Vec<&str> { + let lines: Vec<&str> = text.lines().collect(); + let opens_test_module = |i: usize| { + lines[i] == "#[cfg(test)]" + && lines[i + 1..].iter().find(|l| !l.trim().is_empty()).is_some_and(|l| l.starts_with("mod ") && l.trim_end().ends_with('{')) + }; + let end = (0..lines.len()).find(|&i| opens_test_module(i)).unwrap_or(lines.len()); + lines[..end].iter().copied().filter(|l| !l.trim_start().starts_with("//")).collect() +} + +/// How many times `name` occurs in `line` as a whole path (`registry::grant`, not `registry::granted`). +fn count(line: &str, name: &str) -> usize { + line.match_indices(name).filter(|(at, _)| !line[at + name.len()..].starts_with(|c: char| c.is_alphanumeric() || c == '_')).count() +} + +#[test] +fn grant_token_call_sites_are_limited() { + let root = Path::new(env!("CARGO_MANIFEST_DIR")).join("../.."); + let mut files = Vec::new(); + for top in ["crates", "apps", "xtask"] { + sources(&root.join(top), &mut files); + } + assert!(files.len() > 100, "found only {} source files under {}", files.len(), root.display()); + let mut found: BTreeMap<(&str, String), usize> = BTreeMap::new(); + for file in &files { + let text = std::fs::read_to_string(file).unwrap(); + let rel = file.strip_prefix(&root).unwrap().to_string_lossy().replace('\\', "/"); + for line in shipped(&text) { + for name in ["GrantToken::after_engine_approval", "GrantToken::from_engine_grants_file", "registry::grant"] { + let n = count(line, name); + if n > 0 { + *found.entry((name, rel.clone())).or_default() += n; + } + } + } + } + let expected: BTreeMap<(&str, String), usize> = EXPECTED.iter().map(|&(name, file, n)| ((name, file.to_string()), n)).collect(); + assert_eq!(found, expected, "shipped code that makes a grant token or records a grant"); +} + +/// The audit's own filters: a test module and comments are left out, a longer name isn't counted. +#[test] +fn the_audit_skips_test_code_and_comments() { + let text = "fn a() { registry::grant(t); }\n// registry::grant(t)\nfn b() { registry::granted(id); }\n\n#[cfg(test)]\n\nmod tests {\n registry::grant(t);\n}\n"; + let lines = shipped(text); + assert_eq!(lines.iter().map(|l| count(l, "registry::grant")).sum::(), 1, "{lines:?}"); +} diff --git a/crates/plugins/tests/safe_mode.rs b/crates/plugins/tests/safe_mode.rs index f6278b8e3..8b3149cf8 100644 --- a/crates/plugins/tests/safe_mode.rs +++ b/crates/plugins/tests/safe_mode.rs @@ -58,9 +58,9 @@ fn temp_dir(name: &str) -> std::path::PathBuf { fn in_safe_mode_nothing_loads_or_runs() { let _f = fresh(); // Installed before Safe Mode was turned on. - let before = registry::install(Plugin::load(&filter("test.safe.before"), Limits::default()).unwrap()); + let before = registry::install(Plugin::load(&filter("test.safe.before"), Limits::default()).unwrap()).unwrap(); let fx = build(&tpl::returning(&tpl::manifest("test.safe.effect", "effect", "{}"), r#"{"objects":[]}"#)); - registry::install(Plugin::load(&fx, Limits::default()).unwrap()); + registry::install(Plugin::load(&fx, Limits::default()).unwrap()).unwrap(); let dir = temp_dir("gate"); std::fs::write(dir.join("a.wasm"), filter("test.safe.folder")).unwrap(); diff --git a/docs/plugins.md b/docs/plugins.md index 06fd89f28..eb26c0fa2 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -226,6 +226,13 @@ capability model proposed for PhotoCraft plug-ins `VECTORCRAFT_NO_PREFS` set there is no store, and a `grant` accepted under the flag lasts for that session only. With `--allow-plugin-grants`, pass `hash` set to the `plugin.inspect` entry's `hash` so that the module installed is the one the user saw. +- The approval protects against plug-ins and automation clients (MCP, scripts and CLI requests): none of them can + approve a module's permissions. VectorCraft's own code records a grant only after that approval and when it loads + `grants.json`, and installs a module that lists permissions only under a grant recorded for this very module, + also when it loads the Additional Plug-ins Folder. The approval does not protect against native code linked into + VectorCraft or against another program running as the same user, which can change the files in the `Plug-ins` + folder. `grants.json` is not signed, so such a program can add a grant to it. `--allow-plugin-grants` is a choice + the person starting a headless session makes. - **Replacing asks first.** When the file is another module than the plug-in installed with its id, or, with none installed, its id has settings and a cache in the `Plug-ins` folder, Install Plug-in… and File › Open show its id and SHA-256 and ask whether to install it in place of that plug-in, which gives it the settings and cache kept for diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index b4c949f74..d965a54e3 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1658,6 +1658,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - **Filter plug-ins read type, images and symbol instances (G17, #4333):** a filter at plug-in API level 2 lists `inputs` (`type`, `image`, `symbolInstance`) with `doc.read` in its `permissions`, so installing it asks for approval, and is also given the selected objects of those kinds, read only: type with its runs, character and paragraph attributes (no text past 256 KiB or 4,096 runs or paragraphs, or 16 MiB of runs over the input), images with their size, color mode and, when embedded and at most 1 MiB in base64 (16 MiB over the input), their RGBA pixels (a CMYK image's through the working CMYK profile; never a linked file's), and symbol instances with their symbol and transform. The objects come in paint order from the bottom, and an image that reaches the decoder counts toward the 16 MiB of pixels even when decoding fails. Every object counts toward the input's half of the memory cap; one whose fields besides its pixels and runs don't fit fails the run naming it and its size, a path or compound path that doesn't fit fails it too, and so do more than 100,000 of these objects. Its output returns each as `{id}` or exactly as given; any change or omission is refused before the document changes. Filters without `inputs` get the same input as before, except that compound paths that clip a clip group and paths inside a group that clips one are left out for every filter. Regressions: `crates/plugins/src/objects/tests.rs`, `crates/plugins/src/manifest.rs`, `crates/engine/src/tests/plugins.rs`, `crates/engine/tests/plugin_fuzz.rs`. Scores and hours unchanged. - **Plug-in grants cover one module (G17):** a grant counts only for the module with the id and SHA-256 it records. A module that lists permissions and has other bytes under the same id needs a new approval, even with the same permissions: `plugin.inspect` reports `needsApproval`, `plugin.install` refuses it without `grant`, Install Plug-in… and File › Open ask again, and the plug-in store and plug-in folders skip it. A module with a missing or empty permissions list needs no grant; without the list, it gets its kind's implied permissions. A grant with a malformed hash covers nothing. Regressions: `crates/plugins/src/permissions.rs`, `crates/engine/src/tests/plugins.rs`, `crates/ui-egui/src/tests/plugins.rs`. Scores and hours unchanged. - **Plug-in approval needs a person (G17):** a plug-in's permissions and heavy budget are approved in a dialog that only a person can confirm: the control channel, MCP and injected input are refused, and `plugin.install` takes `grant` or `allowHeavy` only with that approval. `vectorcraft-cli mcp` and `run` refuse them unless started with `--allow-plugin-grants`. Regressions: `crates/engine/src/tests/plugins.rs`, `crates/ui-egui/src/tests/automation.rs`, `apps/vectorcraft-cli/tests/it/cli.rs`. Scores and hours unchanged. +- **Only an approval records a plug-in grant (G17):** `vectorcraft_plugins::registry::grant` takes a `GrantToken`, which the engine makes after an approval and when it loads `grants.json`, and `registry::install`, `install_bytes` and folder loads refuse a module that lists permissions no recorded grant gives this very module (`Error::NeedsApproval`), leaving the plug-in under its id and the revision as they were. A module without `permissions`, or with an empty list, installs as before. `docs/plugins.md` says what the approval does not protect against (native code linked into the app, another program running as the same user, the unsigned `grants.json`). Regressions: `crates/plugins/src/registry.rs`, `crates/plugins/tests/grant_sites.rs`, `crates/engine/src/tests/plugins.rs`. Scores and hours unchanged. - **A plug-in with no permissions ends its id's grant (G17):** installing a module that lists no permissions under a granted id forgets the grant, in the process and in `grants.json`, so the formerly approved bytes need a new approval. Attaching a `Plug-ins` folder whose module for that id lists no permissions forgets it too, and plug-in folders forget the process's grant. A refused install leaves the grant. Regressions: `crates/engine/src/tests/plugins.rs`, `crates/engine/tests/plugin_safe_mode.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. - **Filter plug-ins call commands (G17, #2186):** a filter at plug-in API level 2 with the `commands` permission names the commands it may run in its manifest's `commandAllowlist` and runs them with `env.vc_call`, reading each result with `env.vc_call_result`. Each call sees what the calls before it changed; the calls and the filter's output are one undo step, and a run that fails, is canceled or makes a call that fails leaves every document, its selection and its history as they were. Commands that read or write files, use the clipboard, change preferences or the view, run scripts, actions or plug-ins, switch documents or undo are never called, and a manifest that lists one is refused when the module is installed. The install dialog lists the allowed commands. A run makes at most 1,000 calls, and each call costs 10,000 instructions of its budget. Regressions: `crates/plugins/tests/call.rs`, `crates/engine/tests/plugin_call.rs`, `crates/engine/src/cmd/plugin_call.rs`. Scores and hours unchanged.