diff --git a/crates/engine/src/cmd/plugin.rs b/crates/engine/src/cmd/plugin.rs index 90a8cc827..8e7fc717e 100644 --- a/crates/engine/src/cmd/plugin.rs +++ b/crates/engine/src/cmd/plugin.rs @@ -379,13 +379,20 @@ fn install(s: &mut Session, p: &Value) -> Result { let asked = permissions::names(&wanted).join(", "); return Err(bad(C, format!("“{name}” asks for the permissions {asked}: install it with `grant` listing them once the user agrees"))); } + // Installed plug-ins stay installed after a restart (#2541). The module is written first and its + // grant only once it is there; when the grant can't be written, the store gets the module it + // held back, so the module and grant kept there always belong together. A stored module that + // can't be read back isn't replaced by one that needs a grant. + let previous = s.stored_plugin_bytes(plugin.id()); + 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()); - s.store_grant(&m.id, Some(g)); + if stored && !s.store_grant(&m.id, Some(g)) { + s.put_back_plugin(plugin.id(), previous.ok().flatten().as_deref()); + stored = false; + } } - // Installed plug-ins stay installed after a restart (#2541). - let stored = s.store_plugin(plugin.id(), &bytes); // Installing a disabled plug-in again (an updated version) turns it on for the next launch too, // also when it is disabled by its stored file alone (it stopped a launch while loading). When // the new version could not replace the stored file, the old one stays disabled. diff --git a/crates/engine/src/store.rs b/crates/engine/src/store.rs index 375ca4d02..e54f8deb9 100644 --- a/crates/engine/src/store.rs +++ b/crates/engine/src/store.rs @@ -372,6 +372,29 @@ impl Session { self.plugin_dir.as_deref().is_some_and(|dir| put(dir, &format!("{}.{PLUGIN_EXT}", file_stem(id)), bytes)) } + /// The module the store holds for the plug-in `id`: `None` without a folder or such a file, and + /// an error, with a log record, when the file can't be read or is larger than the module limit. + pub(crate) fn stored_plugin_bytes(&self, id: &str) -> Result>, String> { + let Some(dir) = self.plugin_dir.as_deref() else { return Ok(None) }; + let path = dir.join(format!("{}.{PLUGIN_EXT}", file_stem(id))); + if !path.exists() { + return Ok(None); + } + let max = vectorcraft_plugins::Limits::default().max_module_bytes as u64; + read_capped(&path, max).map(Some).inspect_err(|e| log::warn!("can't read the stored plug-in {}: {e}", path.display())) + } + + /// Return the store's module for the plug-in `id` to `previous`, the bytes it held before a + /// write ([`Session::stored_plugin_bytes`]): with `None`, the file is removed. + pub(crate) fn put_back_plugin(&self, id: &str, previous: Option<&[u8]>) { + match previous { + Some(bytes) => { + self.store_plugin(id, bytes); + } + None => self.unstore_plugin(id), + } + } + /// The file the plug-in `id` is stored in, spelled as [`Session::load_stored_plugins`] names it /// (`None` without a plug-in folder). pub(crate) fn stored_plugin_file(&self, id: &str) -> Option { @@ -382,14 +405,15 @@ impl Session { /// other grants as they are; with none left, remove the file. Entries this version can't use /// 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`. - pub(crate) fn store_grant(&self, id: &str, grant: Option) { - let Some(dir) = &self.plugin_dir else { return }; + /// 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). + pub(crate) fn store_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, Err(e) => { log::warn!("not updating the stored plug-in grants {}: {e}", dir.join(GRANTS_FILE).display()); - return; + return false; } }; let others = file.other.len(); @@ -399,22 +423,23 @@ impl Session { None => file.grants.remove(id).is_some(), } || file.other.len() != others; if !changed { - return; + return true; } let mut entries: Vec = file.grants.into_values().filter_map(|g| serde_json::to_value(g).ok()).collect(); entries.extend(file.other); if entries.is_empty() { take(dir, GRANTS_FILE); - return; + return true; } // The file is read back only up to its cap: written compactly when pretty-printed is larger, // and not at all when that is too. let fits = |bytes: &Vec| bytes.len() as u64 <= MAX_GRANTS_FILE; match serde_json::to_vec_pretty(&entries).ok().filter(fits).or_else(|| serde_json::to_vec(&entries).ok().filter(fits)) { - Some(bytes) => { - put(dir, GRANTS_FILE, &bytes); + Some(bytes) => put(dir, GRANTS_FILE, &bytes), + None => { + log::warn!("not updating the stored plug-in grants {}: {}", dir.join(GRANTS_FILE).display(), too_large(MAX_GRANTS_FILE)); + false } - None => log::warn!("not updating the stored plug-in grants {}: {}", dir.join(GRANTS_FILE).display(), too_large(MAX_GRANTS_FILE)), } } diff --git a/crates/engine/src/tests/plugins.rs b/crates/engine/src/tests/plugins.rs index 7d0983ef2..2dbcccb2b 100644 --- a/crates/engine/src/tests/plugins.rs +++ b/crates/engine/src/tests/plugins.rs @@ -690,6 +690,87 @@ fn a_stored_grant_covers_only_the_stored_module_it_recorded() { run(&mut attached, "plugin.remove", json!({"id": id})); } +/// A write to the plug-in store that fails leaves the store's module beside the grant that approved +/// it, so the old module loads at the next start: a module that can't be written doesn't update the +/// grant, and a grant that can't be written puts the old module back. On a first install, it takes +/// the new module out again. +#[test] +fn a_failed_store_write_keeps_the_stored_module_with_its_grant() { + use vectorcraft_plugins::permissions::sha256_hex; + let folder = crate::tests_links::Folder::new("plugin-store-failed-write"); + let id = "test.engine.permissions.failedwrite"; + let first = asking(id, r#"["doc.read"]"#, &json!({"objects": []})); + let changed = asking(id, r#"["doc.read","doc.write"]"#, &json!({"objects": []})); + let first_data = vectorcraft_format::base64_encode(&first); + let update = json!({"dataBase64": vectorcraft_format::base64_encode(&changed), "grant": ["doc.read", "doc.write"]}); + let mut s = session(); + s.attach_plugin_store(folder.0.clone()); + run(&mut s, "plugin.install", json!({"dataBase64": first_data, "grant": ["doc.read"]})); + let grants = folder.0.join("grants.json"); + let recorded = std::fs::read(&grants).unwrap(); + let module = std::path::PathBuf::from(s.stored_plugin_file(id).unwrap()); + let starts = || { + vectorcraft_plugins::registry::remove(id); + vectorcraft_plugins::registry::revoke(id); + let mut next = session(); + next.attach_plugin_store(folder.0.clone()); + vectorcraft_plugins::registry::get(id).map(|p| p.hash().to_string()) + }; + let block = |path: &std::path::Path| { + std::fs::remove_file(path).unwrap(); + std::fs::create_dir_all(path.join("block")).unwrap(); + }; + + // The module can't be written: the grant stays the old module's. + block(&module); + run(&mut s, "plugin.install", update.clone()); + assert_eq!(std::fs::read(&grants).unwrap(), recorded, "the grant isn't updated for a module that wasn't stored"); + std::fs::remove_dir_all(&module).unwrap(); + std::fs::write(&module, &first).unwrap(); + assert_eq!(starts(), Some(sha256_hex(&first)), "the old module loads after a failed module write"); + + // The grants file is full: the new grant, which is longer, doesn't fit. + let cap = 1 << 20; + let mut entries: Vec = serde_json::from_slice(&recorded).unwrap(); + entries.push(json!({"id": "", "pad": ""})); + let room = cap - serde_json::to_vec(&entries).unwrap().len(); + *entries.last_mut().unwrap() = json!({"id": "", "pad": "x".repeat(room)}); + let full = serde_json::to_vec(&entries).unwrap(); + assert_eq!(full.len(), cap); + std::fs::write(&grants, &full).unwrap(); + run(&mut s, "plugin.install", update.clone()); + assert_eq!(std::fs::read(&grants).unwrap(), full, "not written past the cap"); + assert_eq!(std::fs::read(&module).unwrap(), first, "the old module is put back"); + assert_eq!(starts(), Some(sha256_hex(&first)), "the old module loads after a failed grant write"); + + // The grants file can't be read, so nothing can be recorded in it. + block(&grants); + run(&mut s, "plugin.install", update.clone()); + assert_eq!(std::fs::read(&module).unwrap(), first, "the old module is put back"); + std::fs::remove_dir_all(&grants).unwrap(); + std::fs::write(&grants, &recorded).unwrap(); + assert_eq!(starts(), Some(sha256_hex(&first)), "the old module loads once the grants are readable"); + + // A stored module that can't be read back isn't replaced, so a failed grant write can't lose it. + let oversized = vectorcraft_plugins::Limits::default().max_module_bytes as u64 + 1; + std::fs::File::options().write(true).open(&module).unwrap().set_len(oversized).unwrap(); + block(&grants); + run(&mut s, "plugin.install", update); + assert_eq!(std::fs::metadata(&module).unwrap().len(), oversized, "the stored module stays"); + std::fs::remove_dir_all(&grants).unwrap(); + std::fs::write(&grants, &recorded).unwrap(); + std::fs::write(&module, &first).unwrap(); + + // A first install whose grant can't be written leaves no module without a grant in the store. + run(&mut s, "plugin.remove", json!({"id": id})); + assert!(!module.exists() && !grants.exists()); + std::fs::create_dir_all(grants.join("block")).unwrap(); + run(&mut s, "plugin.install", json!({"dataBase64": first_data, "grant": ["doc.read"]})); + assert!(!module.exists(), "the store holds no module its grants don't cover"); + vectorcraft_plugins::registry::remove(id); + vectorcraft_plugins::registry::revoke(id); +} + /// A plug-in folder reports a changed module as needing approval and still loads the other files; /// granted on its own, the changed module loads from the folder. A session without a store uses the /// grants held by the process. diff --git a/crates/engine/tests/plugin_safe_mode.rs b/crates/engine/tests/plugin_safe_mode.rs index 6923df616..fc9bfed29 100644 --- a/crates/engine/tests/plugin_safe_mode.rs +++ b/crates/engine/tests/plugin_safe_mode.rs @@ -198,6 +198,25 @@ fn installing_a_disabled_plug_in_again_turns_it_on() { let _ = std::fs::remove_dir_all(&dir); } +/// When the grant of an update can't be stored, the store keeps the old module, and it stays disabled. +#[test] +fn a_disabled_plug_in_whose_new_grant_cant_be_stored_stays_disabled() { + let _f = fresh(); + let dir = tmp_dir("reinstall-no-grant"); + let mut s = launch(&dir); + let manifest = r#"{"id":"test.sm.g","name":"Test","version":"1","kind":"filter","permissions":["doc.read"]}"#; + let module = |out: &str| vectorcraft_format::base64_encode(&wat::parse_str(tpl::returning(manifest, out)).unwrap()); + s.execute("plugin.install", &json!({"dataBase64": module(r#"{"objects":[]}"#), "grant": ["doc.read"]})).unwrap(); + s.execute("plugin.setEnabled", &json!({"id": "test.sm.g", "enabled": false})).unwrap(); + let grants = dir.join("grants.json"); + std::fs::remove_file(&grants).unwrap(); + std::fs::create_dir_all(grants.join("block")).unwrap(); + s.execute("plugin.install", &json!({"dataBase64": module(r#"{"objects":[],"error":null}"#), "grant": ["doc.read"]})).unwrap(); + assert_eq!(safe_mode::disabled().len(), 1, "the module the update could not replace stays disabled"); + safe_mode::set_disabled(vec![]); + let _ = std::fs::remove_dir_all(&dir); +} + /// A launch where the stored plug-in `id` stopped while loading: it is installed, then disabled by /// its stored file alone (as Disable and Continue saves it), and the next launch skips it. fn stored_plug_in_disabled_by_file(dir: &std::path::Path, id: &str) -> Session { diff --git a/docs/plugins.md b/docs/plugins.md index 2c581c425..0dc8f88e4 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -136,7 +136,9 @@ capability model proposed for PhotoCraft plug-ins - The grant is recorded with the plug-in's id, the module's hash and the permissions, in `grants.json` in the `Plug-ins` folder, and covers only that module. Installing the same bytes again needs no new approval. A module with other bytes asks again, even when it lists the same permissions. Until it is approved, the installed module and its - grant stay as they are; once it is, the grant records the new hash. A grant whose hash is not 64 lower-case + grant stay as they are; once it is, the grant records the new hash. When the new module or its grant can't be + written to the `Plug-ins` folder, the folder keeps the old module and its grant, and the old module loads at the + next start. A grant whose hash is not 64 lower-case hexadecimal digits covers nothing. `plugin.remove` forgets the grant. An entry of `grants.json` that this version can't use (a permission it doesn't know, an invalid id) is skipped and stays in the file until its plug-in is granted again or removed. A `grants.json` that is not a JSON list is never overwritten, and the file is never written larger than