diff --git a/crates/engine/src/cmd/plugin.rs b/crates/engine/src/cmd/plugin.rs index 86b3281f2..c0293d42e 100644 --- a/crates/engine/src/cmd/plugin.rs +++ b/crates/engine/src/cmd/plugin.rs @@ -624,12 +624,18 @@ fn install(s: &mut Session, p: &Value) -> Result { registry::revoke(&m.id); // When the write failed, the old stored module stays in the folder and needs its grant at // the next launch. A grant that can't be removed still approves the old bytes, so the store - // gets the module it held back, when that could be read. A grants file that is too large or + // gets the module it held back, when that could be read; when there is no module to go with + // the grant, the grant stops counting for this session. A grants file that is too large or // doesn't hold a list approves nothing, and the new module stays; one that can't be opened or // read may approve the old bytes once it reads again. - if stored && !s.store_grant(&m.id, None) && previous.is_ok() && s.holds_plugin_grant(&m.id) { - s.put_back_plugin(plugin.id(), previous.ok().flatten().as_deref()); - stored = false; + if stored && !s.store_grant(&m.id, None) && s.holds_plugin_grant(&m.id) { + if !matches!(previous, Ok(Some(_))) { + s.ignore_stored_grant(&m.id); + } + if let Ok(held) = previous { + s.put_back_plugin(plugin.id(), held.as_deref()); + stored = false; + } } } // It gets the settings and cache kept for its id. diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 381109adc..779d78f41 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -141,6 +141,10 @@ pub struct Session { /// Where the plug-ins `plugin.install` installs are kept; `None`: only for the process /// ([`Session::attach_plugin_store`]). plugin_dir: Option, + /// The plug-in ids whose grant in the plug-in store should be gone but could not be removed: + /// [`Session::plugin_approver`] and a load of the store do not count those grants, and a successful + /// [`Session::store_grant`] for the id takes it out. + ignored_plugin_grants: std::sync::Mutex>, /// A filter's output that a run made in the background handed in for `plugin.run` to apply /// ([`cmd::plugin::FilterRun::hand_in`]); the next `plugin.run` takes it. plugin_output: Option, diff --git a/crates/engine/src/session/documents.rs b/crates/engine/src/session/documents.rs index 832a22961..bde2a7d24 100644 --- a/crates/engine/src/session/documents.rs +++ b/crates/engine/src/session/documents.rs @@ -77,6 +77,7 @@ impl Session { pdf_preset_dir: None, profile_dir: None, plugin_dir: None, + ignored_plugin_grants: Default::default(), plugin_output: None, recent_urls: vec![], trace_preset_changes: Default::default(), diff --git a/crates/engine/src/store.rs b/crates/engine/src/store.rs index 7be385967..5738290d9 100644 --- a/crates/engine/src/store.rs +++ b/crates/engine/src/store.rs @@ -293,6 +293,9 @@ impl Session { let disabled = read_capped(&dir.join(DISABLED_FILE), safe_mode::MAX_DISABLED_FILE_BYTES).map(|b| safe_mode::parse_disabled_file(&b)).unwrap_or_default(); safe_mode::set_disabled(disabled); + if self.plugin_dir.as_ref() != Some(&dir) { + self.ignored_plugin_grants.lock().unwrap_or_else(|e| e.into_inner()).clear(); + } self.plugin_dir = Some(dir); self.load_stored_plugins(); } @@ -302,13 +305,14 @@ impl Session { /// file while it loads. A module that doesn't load, and one that lists permissions the grants /// there don't grant that very module ([`grant_covers`]), is skipped with a log record, and /// stays in the folder with its grant. A grant for an id whose stored module lists no - /// permissions is forgotten. + /// permissions is forgotten; one that can't be removed from the grants file doesn't count for + /// the rest of the session ([`Session::plugin_approver`]). pub fn load_stored_plugins(&mut self) { let Some(dir) = self.plugin_dir.clone() else { return }; if safe_mode::active() { return; } - let grants = stored_grants(&dir); + let grants = self.counted_grants(&dir); let limits = vectorcraft_plugins::Limits::default(); let mut left = MAX_PLUGIN_STORE; let mut asks_nothing = BTreeSet::new(); @@ -355,11 +359,17 @@ impl Session { Err(e) => log::warn!("skipping the stored plug-in {}: {e}", path.display()), } } - let kept = read_grants(&dir).unwrap_or_default(); + let kept = read_grants(&dir); for id in &asks_nothing { vectorcraft_plugins::registry::revoke(id); - if kept.grants.contains_key(id) || kept.other.iter().any(|e| e.get("id").and_then(serde_json::Value::as_str) == Some(id.as_str())) { - self.store_grant(id, None); + let named = match &kept { + Ok(file) => { + file.grants.contains_key(id) || file.other.iter().any(|e| e.get("id").and_then(serde_json::Value::as_str) == Some(id.as_str())) + } + Err(e) => e.unread, + }; + if named && !self.store_grant(id, None) && self.holds_plugin_grant(id) { + self.ignore_stored_grant(id); } } } @@ -400,7 +410,8 @@ impl Session { /// Whether `plugin` installs without asking the user: it lists no permissions, or this very /// module (its id and SHA-256) was granted every one it lists, in the grants of the attached - /// plug-in store when there is one ([`grant_covers`]), else in this process's + /// plug-in store when there is one ([`grant_covers`]), not counting a grant that should be gone + /// and could not be removed ([`Session::ignore_stored_grant`]), else in this process's /// ([`vectorcraft_plugins::registry::approved`]). pub(crate) fn plugin_approved(&self, plugin: &vectorcraft_plugins::Plugin) -> bool { self.plugin_approver()(plugin) @@ -409,7 +420,7 @@ impl Session { /// [`Session::plugin_approved`] for many plug-ins: reads the store's grants once, when it is /// made, and checks each plug-in against them. pub(crate) fn plugin_approver(&self) -> impl Fn(&vectorcraft_plugins::Plugin) -> bool + use<> { - let grants = self.plugin_dir.as_deref().map(stored_grants); + let grants = self.plugin_dir.as_deref().map(|dir| self.counted_grants(dir)); move |plugin| { let m = plugin.manifest(); match &grants { @@ -419,6 +430,15 @@ impl Session { } } + /// 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 { + let ignored = self.ignored_plugin_grants.lock().unwrap_or_else(|e| e.into_inner()); + let mut grants = stored_grants(dir); + grants.retain(|id, _| !ignored.contains(id)); + grants + } + /// Write the module `bytes` of the plug-in `id` to the store → whether they went there (`false` /// without a folder). pub(crate) fn store_plugin(&self, id: &str, bytes: &[u8]) -> bool { @@ -459,8 +479,24 @@ impl Session { /// are written back as they were, except one for `id`. A grants file that is too large, can't /// be read or doesn't hold a list is left as it is, with a log record, and nothing is recorded; /// so is one whose new content would be larger than `MAX_GRANTS_FILE`. → whether the store now - /// holds what was asked (`false` without a folder). + /// holds what was asked (`false` without a folder). When it does, the grant kept for `id` counts + /// again ([`Session::ignore_stored_grant`]). pub(crate) fn store_grant(&self, id: &str, grant: Option) -> bool { + let done = self.write_grant(id, grant); + if done { + self.ignored_plugin_grants.lock().unwrap_or_else(|e| e.into_inner()).remove(id); + } + done + } + + /// Stop counting the store's grant for the plug-in `id` until [`Session::store_grant`] next + /// succeeds for it or another store is attached: for a grant that should be gone and could not + /// be removed, which would otherwise approve the bytes it covers. + pub(crate) fn ignore_stored_grant(&self, id: &str) { + self.ignored_plugin_grants.lock().unwrap_or_else(|e| e.into_inner()).insert(id.to_string()); + } + + fn write_grant(&self, id: &str, grant: Option) -> bool { let Some(dir) = &self.plugin_dir else { return false }; let mut file = match read_grants(dir) { Ok(file) => file, diff --git a/crates/engine/src/tests/plugins.rs b/crates/engine/src/tests/plugins.rs index d1d9cc63e..41a6467ca 100644 --- a/crates/engine/src/tests/plugins.rs +++ b/crates/engine/src/tests/plugins.rs @@ -1463,6 +1463,9 @@ fn a_grant_that_cannot_be_removed_keeps_the_stored_module() { std::fs::File::options().write(true).open(&file).unwrap().set_len(oversized).unwrap(); run(&mut s, "plugin.install", replace.clone()); assert_eq!(std::fs::read(&file).unwrap(), plain, "a module that can't be read back can't be put back"); + assert_eq!(std::fs::read(&grants).unwrap(), full); + let inspect = json!({"dataBase64": vectorcraft_format::base64_encode(&approved)}); + assert_eq!(run(&mut s, "plugin.inspect", inspect)["needsApproval"], true, "the grant left behind stops counting"); std::fs::write(&file, &approved).unwrap(); std::fs::write(&grants, b"not a list").unwrap(); @@ -1478,6 +1481,98 @@ fn a_grant_that_cannot_be_removed_keeps_the_stored_module() { run(&mut s, "plugin.remove", json!({"id": id})); } +/// A grant that can't be removed when no module is stored for its id has no module to go with, so +/// it stops counting for the session. +#[test] +fn a_grant_that_cannot_be_removed_and_has_no_stored_module_stops_counting() { + let folder = crate::tests_links::Folder::new("plugin-store-grant-no-module"); + let id = "test.engine.permissions.forgetnomodule"; + let empty = json!({"objects": []}); + let approved = asking(id, r#"["doc.read"]"#, &empty); + let approved_data = vectorcraft_format::base64_encode(&approved); + let plain = asking(id, "null", &empty); + let grants = folder.0.join("grants.json"); + let mut s = session(); + s.attach_plugin_store(folder.0.clone()); + run(&mut s, "plugin.install", json!({"dataBase64": approved_data, "grant": ["doc.read"]})); + let file = s.stored_plugin_file(id).unwrap(); + let entry = serde_json::from_slice::>(&std::fs::read(&grants).unwrap()).unwrap().remove(0); + let full = format!(r#"[{entry},{{"n":[{}]}}]"#, vec!["1e9"; 200_000].join(",")).into_bytes(); + std::fs::write(&grants, &full).unwrap(); + std::fs::remove_file(&file).unwrap(); + + run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&plain), "replace": true})); + assert!(!std::path::Path::new(&file).exists(), "the store keeps no module"); + assert_eq!(std::fs::read(&grants).unwrap(), full); + assert_eq!(run(&mut s, "plugin.inspect", json!({"dataBase64": approved_data}))["needsApproval"], true); + + std::fs::remove_file(&grants).unwrap(); + run(&mut s, "plugin.remove", json!({"id": id})); +} + +/// A grant that a load of the plug-in store should forget and can't stays out of the approvals for +/// the session: both when `grants.json` can't be rewritten and when it can't be read at the load. +/// A grant recorded later counts again, and another store starts with nothing ignored. +#[test] +fn a_grant_the_load_cannot_remove_stops_counting_for_the_session() { + use vectorcraft_plugins::permissions::sha256_hex; + let folder = crate::tests_links::Folder::new("plugin-store-load-forget"); + let elsewhere = crate::tests_links::Folder::new("plugin-store-load-forget-other"); + let id = "test.engine.permissions.loadforget"; + let empty = json!({"objects": []}); + let approved = asking(id, r#"["doc.read"]"#, &empty); + let approved_data = vectorcraft_format::base64_encode(&approved); + let plain = asking(id, "null", &empty); + let needs_approval = json!({"dataBase64": approved_data}); + let grants = folder.0.join("grants.json"); + let entry = json!({"id": id, "hash": sha256_hex(&approved), "permissions": ["doc.read"]}); + let recorded = serde_json::to_vec(&json!([entry])).unwrap(); + let mut s = session(); + s.attach_plugin_store(folder.0.clone()); + run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&plain)})); + + // `1e9` is written back as `1000000000.0`, which makes the rewritten file larger than the 1 MiB that is read. + let full = format!(r#"[{entry},{{"n":[{}]}}]"#, vec!["1e9"; 200_000].join(",")).into_bytes(); + std::fs::write(&grants, &full).unwrap(); + vectorcraft_plugins::registry::remove(id); + let mut next = session(); + next.attach_plugin_store(folder.0.clone()); + assert_eq!(std::fs::read(&grants).unwrap(), full, "the load can't rewrite the file"); + assert_eq!(run(&mut next, "plugin.inspect", needs_approval.clone())["needsApproval"], true); + let refused = next.execute("plugin.install", &json!({"dataBase64": approved_data, "replace": true})).unwrap_err().to_string(); + assert!(refused.contains("doc.read"), "{refused}"); + + std::fs::write(elsewhere.0.join("grants.json"), &recorded).unwrap(); + next.attach_plugin_store(elsewhere.0.clone()); + assert_eq!(run(&mut next, "plugin.inspect", needs_approval.clone())["needsApproval"], false, "that store's grant counts"); + + // A directory at the file's path gives a read error on every platform. + std::fs::remove_file(&grants).unwrap(); + std::fs::create_dir(&grants).unwrap(); + vectorcraft_plugins::registry::remove(id); + let mut third = session(); + third.attach_plugin_store(folder.0.clone()); + std::fs::remove_dir(&grants).unwrap(); + std::fs::write(&grants, &recorded).unwrap(); + assert_eq!(run(&mut third, "plugin.inspect", needs_approval.clone())["needsApproval"], true, "the file reads again, and the grant doesn't count"); + + // Attaching the same store again, with no stored module to try the removal for, keeps it out. + let file = third.stored_plugin_file(id).unwrap(); + std::fs::remove_file(&file).unwrap(); + third.attach_plugin_store(folder.0.clone()); + assert_eq!(run(&mut third, "plugin.inspect", needs_approval.clone())["needsApproval"], true, "the same store's grant stays out"); + + // A later load in the session doesn't count it either, for the bytes it covers. + std::fs::write(&file, &approved).unwrap(); + vectorcraft_plugins::registry::remove(id); + third.load_stored_plugins(); + assert!(vectorcraft_plugins::registry::get(id).is_none(), "the formerly approved module isn't loaded"); + + run(&mut third, "plugin.install", json!({"dataBase64": approved_data, "grant": ["doc.read"], "replace": true})); + assert_eq!(run(&mut third, "plugin.inspect", needs_approval)["needsApproval"], false, "the user granted it again"); + run(&mut third, "plugin.remove", json!({"id": id})); +} + /// A grants file that can't be opened or read may name the old module once it reads again, so /// neither a module listing no permissions nor `plugin.remove` gets past it. A directory at the /// file's path gives a read error on every platform. diff --git a/docs/plugins.md b/docs/plugins.md index 328a1d85e..f8c8d5546 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -218,11 +218,15 @@ capability model proposed for PhotoCraft plug-ins tries again. A refused install leaves the grant as it is. When an install of a module that lists no permissions can't write the module to the `Plug-ins` folder, or can't remove the grant from `grants.json`, the folder keeps the module it held and `grants.json` keeps the grant. The new module runs until the session ends. When the grant can't be - removed and the module already stored can't be read back, the new module stays in the folder. A `grants.json` larger - than 1 MiB, or one that is not a JSON list, grants nothing: the new module stays in the folder, and `plugin.remove` - removes the plug-in. A `grants.json` that can't be opened or read counts as a grant that can't be removed. When - VectorCraft loads the `Plug-ins` folder, it forgets any grant for an id whose stored module lists no permissions. In - Safe Mode, and for a stored module that the user disabled, + removed and the module already stored can't be read back, the new module stays in the folder, and the grant does not + count for the rest of the session. The grant also does not count when the folder held no module for that id. A + `grants.json` larger than 1 MiB, or one that is not a JSON list, grants nothing: the new module stays in the folder, + and `plugin.remove` removes the plug-in. A `grants.json` that can't be opened or read counts as a grant that can't be + removed. When VectorCraft loads the `Plug-ins` folder, it forgets any grant for an id whose stored module lists no + permissions. When it can't remove that grant from `grants.json`, or can't read the file, the grant does not count for + the rest of the session, or until the plug-in is granted again, including for the next load, which tries the removal + again. In Safe Mode, and for + a stored module that the user disabled, VectorCraft does not read the module, and `grants.json` keeps the grant for its id. The Additional Plug-ins Folder and `plugin.reload` do not change the `Plug-ins` folder: when they leave a module that lists no permissions under an id, the process forgets its own grant for that id, and a grant kept in `grants.json` stays with the stored module it