Plug-ins: a permission grant covers only the module that was approved

A grant records the module's SHA-256, but only its permissions were
compared, so a module with other bytes under a granted id inherited the
grant when it listed no new permission.

A grant now covers a module only when the id, the SHA-256 and every
listed permission match. plugin.inspect reports needsApproval for a
changed module, plugin.install refuses it without a new grant and leaves
the installed module and its grant as they are, Install Plug-in and
File > Open ask again, and the plug-in store and plug-in folders skip it.
The same bytes install again without asking, and a module with no
permissions list or an empty one needs no grant. A grant whose hash is
not 64 lower-case hexadecimal digits covers nothing.

plugin.install still trusts the client that sends grant; the docs say so.

crates/engine/src/cmd/plugin.rs

@@ -333,9 +333,10 @@ fn grant_param(c: &str, p: &Value) -> Result<Option<Vec<Permission>>> {
 }
 
 /// `plugin.install`: loads and checks the module, refuses one that asks for a heavy budget unless
-/// `allowHeavy` says the user agreed, and one that lists permissions the user hasn't granted its id
-/// unless `grant` lists them all, and one whose SHA-256 isn't the `hash` given; records the grant
-/// with the module's hash, keeps it for the next start and installs it → its `plugin.list` entry.
+/// `allowHeavy` says the user agreed, and one that lists permissions the user hasn't granted this
+/// very module unless `grant` lists them all, and one whose SHA-256 isn't the `hash` given; records
+/// the grant with the module's hash, keeps it for the next start and installs it → its
+/// `plugin.list` entry.
 fn install(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "plugin.install";
     let (replace, allow_heavy) = (switch_or(p, "replace", true, C)?, switch_or(p, "allowHeavy", false, C)?);
@@ -362,7 +363,7 @@ fn install(s: &mut Session, p: &Value) -> Result<Value> {
         ));
     }
     // A plug-in that lists permissions installs once the user grants them: whoever installs it
-    // asks them first, unless they already granted its id all of them.
+    // asks them first, unless they already granted this very module all of them.
     let m = plugin.manifest();
     let wanted = m.permissions();
     if !s.plugin_approved(&plugin) && !grant.as_ref().is_some_and(|g| wanted.iter().all(|w| g.contains(w))) {
@@ -526,7 +527,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Install Plug-in",
             [],
             None,
-            "{path: a .wasm file (desktop) | dataBase64: the module (+ name?: shown as its source), replace?: bool (true: a plug-in with the same id is replaced), name?: file name (with dataBase64), allowHeavy?: bool (true once the user agreed to a heavy budget: a filter whose manifest asks for one, budget \"heavy\" in plugin.inspect, installs only with it), grant?: [permission] (the permissions the user granted: a plug-in whose manifest lists permissions, needsApproval in plugin.inspect, installs only when grant holds every one of them; the grant is recorded with the module's hash, and replacing the plug-in with one that lists no new permission needs none; with a plug-in store attached, only the grants kept there count), hash?: string (the module's SHA-256 as plugin.inspect gave it: a module with other bytes is refused)} validate and install a WebAssembly plug-in for this session → its plugin.list entry",
+            "{path: a .wasm file (desktop) | dataBase64: the module (+ name?: shown as its source), replace?: bool (true: a plug-in with the same id is replaced), name?: file name (with dataBase64), allowHeavy?: bool (true once the user agreed to a heavy budget: a filter whose manifest asks for one, budget \"heavy\" in plugin.inspect, installs only with it), grant?: [permission] (the permissions the user granted: a plug-in whose manifest lists permissions, needsApproval in plugin.inspect, installs only when grant holds every one of them; the grant is recorded with the module's hash and covers only that module, so a module with other bytes needs a new grant even when it lists the same permissions; with a plug-in store attached, only the grants kept there count), hash?: string (the module's SHA-256 as plugin.inspect gave it: a module with other bytes is refused)} validate and install a WebAssembly plug-in for this session → its plugin.list entry",
             always,
             install
         ),
@@ -535,7 +536,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Inspect Plug-in",
             [],
             None,
-            "{path: a .wasm file (desktop) | dataBase64: the module, name?: the file name shown as its source (with dataBase64)} validate a WebAssembly plug-in without installing it → the plugin.list entry it would have, plus needsApproval: bool (true when it lists permissions the user hasn't granted its id: plugin.install then needs grant)",
+            "{path: a .wasm file (desktop) | dataBase64: the module, name?: the file name shown as its source (with dataBase64)} validate a WebAssembly plug-in without installing it → the plugin.list entry it would have, plus needsApproval: bool (true when it lists permissions the user hasn't granted this very module, its bytes included: plugin.install then needs grant)",
             always,
             inspect
         ),
@@ -545,7 +546,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Reload Plug-ins",
             ["Object", "Plug-ins"],
             None,
-            "{path?: a folder (default: the Additional Plug-ins Folder preference)} install every *.wasm in it (desktop; a bad module is reported and skipped, and so are a heavy filter and a plug-in that lists permissions the user hasn't granted its id) → {folder, loaded: [id], failed: [{file, error}]}",
+            "{path?: a folder (default: the Additional Plug-ins Folder preference)} install every *.wasm in it (desktop; a bad module is reported and skipped, and so are a heavy filter and a plug-in that lists permissions the user hasn't granted this very module, its bytes included) → {folder, loaded: [id], failed: [{file, error}]}",
             always,
             reload
         ),

crates/engine/src/store.rs

@@ -224,8 +224,8 @@ impl Session {
     }
 
     /// Keep the plug-ins this session installs in `dir`, and install the ones already there (a
-    /// module that doesn't load, and one that lists permissions the grants there don't grant it,
-    /// is skipped with a log record).
+    /// module that doesn't load, and one that lists permissions the grants there don't grant that
+    /// very module, is skipped with a log record, and stays in `dir` with its grant).
     pub fn attach_plugin_store(&mut self, dir: PathBuf) {
         let grants = stored_grants(&dir);
         let limits = vectorcraft_plugins::Limits::default();
@@ -244,12 +244,12 @@ impl Session {
                 // module stored here load.
                 Ok(plugin) if plugin.manifest().asks_permissions() => {
                     let m = plugin.manifest();
-                    match grants.get(&m.id).filter(|g| g.covers(&m.permissions())) {
+                    match grants.get(&m.id).filter(|g| g.covers(&m.id, plugin.hash(), &m.permissions())) {
                         Some(g) => {
                             vectorcraft_plugins::registry::grant(g.clone());
                             vectorcraft_plugins::registry::install(plugin.with_source(path.display().to_string()));
                         }
-                        None => log::warn!("skipping the stored plug-in {}: the user hasn't granted the permissions it lists", path.display()),
+                        None => log::warn!("skipping the stored plug-in {}: the user hasn't approved this module's permissions", path.display()),
                     }
                 }
                 Ok(plugin) => {
@@ -261,9 +261,10 @@ impl Session {
         self.plugin_dir = Some(dir);
     }
 
-    /// Whether `plugin` installs without asking the user: it lists no permissions, or its id was
-    /// granted every one it lists, in the grants of the attached plug-in store when there is one,
-    /// else in this process's ([`vectorcraft_plugins::registry::approved`]).
+    /// 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, else in this process's
+    /// ([`vectorcraft_plugins::registry::approved`]).
     pub(crate) fn plugin_approved(&self, plugin: &vectorcraft_plugins::Plugin) -> bool {
         self.plugin_approver()(plugin)
     }
@@ -275,7 +276,7 @@ impl Session {
         move |plugin| {
             let m = plugin.manifest();
             match &grants {
-                Some(grants) if m.asks_permissions() => grants.get(&m.id).is_some_and(|g| g.covers(&m.permissions())),
+                Some(grants) if m.asks_permissions() => grants.get(&m.id).is_some_and(|g| g.covers(&m.id, plugin.hash(), &m.permissions())),
                 _ => vectorcraft_plugins::registry::approved(plugin),
             }
         }

crates/engine/src/tests/plugins.rs

@@ -532,9 +532,9 @@ fn asking(id: &str, permissions: &str, output: &Value) -> Vec<u8> {
 
 /// A plug-in whose manifest lists permissions installs only once they are all granted (`grant`),
 /// which `plugin.inspect` tells beforehand (`needsApproval`) with the permissions and the module's
-/// hash. The grant is recorded with that hash: replacing the plug-in with one that lists the same
-/// permissions needs no new grant, and one that lists another needs one. Removing it forgets the
-/// grant. A plug-in without `permissions` installs as before, with the ones its kind implies.
+/// hash. The grant is recorded with that hash and covers only that module: one with other bytes
+/// needs a new grant. Removing it forgets the grant. A plug-in without `permissions` installs as
+/// before, with the ones its kind implies.
 #[test]
 fn a_plugin_that_lists_permissions_installs_once_they_are_granted() {
     let mut s = session();
@@ -570,11 +570,10 @@ fn a_plugin_that_lists_permissions_installs_once_they_are_granted() {
     assert_eq!(installed["permissions"], json!(["doc.read"]));
     let g = vectorcraft_plugins::registry::granted(id).unwrap();
     assert_eq!((g.hash.as_str(), g.permissions.as_slice()), (hash.as_str(), &[vectorcraft_plugins::permissions::Permission::DocRead][..]));
-    // Replaced with another module that lists the same permissions: no new grant needed, and the
-    // grant now records the new module's hash.
+    // Replaced with another module that lists the same permissions: granted again, the grant now
+    // records the new module's hash.
     let other = asking(id, r#"["doc.read"]"#, &json!({"objects": [], "error": null}));
-    assert_eq!(run(&mut s, "plugin.inspect", json!({"dataBase64": vectorcraft_format::base64_encode(&other)}))["needsApproval"], false);
-    run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&other)}));
+    run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&other), "grant": ["doc.read"]}));
     assert_eq!(vectorcraft_plugins::registry::granted(id).unwrap().hash, vectorcraft_plugins::permissions::sha256_hex(&other));
     // One that asks for more is refused until it is granted all of them.
     let more = data(r#"["doc.read","doc.write"]"#);
@@ -594,6 +593,140 @@ fn a_plugin_that_lists_permissions_installs_once_they_are_granted() {
     assert_eq!(run(&mut s, "plugin.inspect", json!({"dataBase64": vectorcraft_format::base64_encode(DESATURATE)}))["needsApproval"], false);
 }
 
+/// A module with other bytes under an id that was granted is not covered, even when it lists the
+/// same permissions: `plugin.inspect` says so, `plugin.install` refuses it and leaves the installed
+/// module and its grant alone, and a new `grant` installs it and records its hash. The same bytes
+/// install again without asking, and a module that lists no permissions never asks.
+#[test]
+fn a_grant_covers_only_the_module_that_was_approved() {
+    use vectorcraft_plugins::permissions::sha256_hex;
+    let mut s = session();
+    let id = "test.engine.permissions.bytes";
+    let first = asking(id, r#"["doc.read"]"#, &json!({"objects": []}));
+    let changed = asking(id, r#"["doc.read"]"#, &json!({"objects": [], "error": null}));
+    let (first_data, changed_data) = (vectorcraft_format::base64_encode(&first), vectorcraft_format::base64_encode(&changed));
+    let (first_hash, changed_hash) = (sha256_hex(&first), sha256_hex(&changed));
+    run(&mut s, "plugin.install", json!({"dataBase64": first_data, "grant": ["doc.read"]}));
+    assert_eq!(run(&mut s, "plugin.inspect", json!({"dataBase64": changed_data}))["needsApproval"], true);
+    let e = s.execute("plugin.install", &json!({"dataBase64": changed_data})).unwrap_err().to_string();
+    assert!(e.contains("doc.read") && e.contains("`grant`"), "{e}");
+    let e = s.execute("plugin.install", &json!({"dataBase64": changed_data, "hash": changed_hash})).unwrap_err().to_string();
+    assert!(e.contains("`grant`"), "`hash` is not an approval: {e}");
+    assert_eq!(vectorcraft_plugins::registry::get(id).unwrap().hash(), first_hash, "the installed module stays");
+    assert_eq!(vectorcraft_plugins::registry::granted(id).unwrap().hash, first_hash, "its grant stays");
+    // The same bytes need no new grant.
+    assert_eq!(run(&mut s, "plugin.inspect", json!({"dataBase64": first_data}))["needsApproval"], false);
+    run(&mut s, "plugin.install", json!({"dataBase64": first_data}));
+    // A new grant installs the changed module, and the old bytes are no longer covered.
+    run(&mut s, "plugin.install", json!({"dataBase64": changed_data, "grant": ["doc.read"], "hash": changed_hash}));
+    assert_eq!(vectorcraft_plugins::registry::get(id).unwrap().hash(), changed_hash);
+    assert_eq!(vectorcraft_plugins::registry::granted(id).unwrap().hash, changed_hash);
+    assert_eq!(run(&mut s, "plugin.inspect", json!({"dataBase64": first_data}))["needsApproval"], true);
+    run(&mut s, "plugin.remove", json!({"id": id}));
+
+    // A manifest without a list, or with an empty one, asks for nothing, whatever its bytes.
+    let exempt = "test.engine.permissions.exempt";
+    for (permissions, out) in [("null", json!({"objects": []})), ("null", json!({"objects": [], "error": null})), ("[]", json!({"objects": []}))] {
+        let data = vectorcraft_format::base64_encode(&asking(exempt, permissions, &out));
+        assert_eq!(run(&mut s, "plugin.inspect", json!({"dataBase64": data}))["needsApproval"], false, "{permissions}");
+        run(&mut s, "plugin.install", json!({"dataBase64": data}));
+    }
+    run(&mut s, "plugin.remove", json!({"id": exempt}));
+}
+
+/// A stored module whose bytes changed is skipped at the next start and stays in the store with its
+/// grant, and the original bytes load again. A stored grant whose hash is malformed covers nothing.
+/// 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};
+    let folder = crate::tests_links::Folder::new("plugin-store-grant-hash");
+    let id = "test.engine.permissions.storedbytes";
+    let first = asking(id, r#"["doc.read"]"#, &json!({"objects": []}));
+    let changed = asking(id, r#"["doc.read"]"#, &json!({"objects": [], "error": null}));
+    let mut s = session();
+    s.attach_plugin_store(folder.0.clone());
+    run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&first), "grant": ["doc.read"]}));
+    let grants = folder.0.join("grants.json");
+    let recorded = std::fs::read(&grants).unwrap();
+    let module = std::fs::read_dir(&folder.0)
+        .unwrap()
+        .map(|e| e.unwrap().path())
+        .find(|p| p.extension().is_some_and(|x| x == "wasm"))
+        .expect("the stored module");
+    let forget = || {
+        vectorcraft_plugins::registry::remove(id);
+        vectorcraft_plugins::registry::revoke(id);
+    };
+    let starts = || {
+        forget();
+        let mut next = session();
+        next.attach_plugin_store(folder.0.clone());
+        vectorcraft_plugins::registry::get(id).is_some()
+    };
+    assert!(starts(), "the module that was approved loads");
+    std::fs::write(&module, &changed).unwrap();
+    assert!(!starts(), "other bytes under the old grant are skipped");
+    assert_eq!(std::fs::read(&module).unwrap(), changed, "the module stays in the store");
+    assert_eq!(std::fs::read(&grants).unwrap(), recorded, "its grant stays");
+    std::fs::write(&module, &first).unwrap();
+    assert!(starts(), "the approved bytes load again");
+
+    // A malformed hash is no wildcard, even for the module on disk.
+    let upper = sha256_hex(&first).to_uppercase();
+    for hash in ["", "00", upper.as_str()] {
+        std::fs::write(&grants, serde_json::to_vec(&json!([{"id": id, "hash": hash, "permissions": ["doc.read"]}])).unwrap()).unwrap();
+        assert!(!starts(), "hash {hash:?}");
+    }
+
+    // The store's grants are the ones that count, even when the process holds one for these bytes.
+    std::fs::write(&grants, &recorded).unwrap();
+    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] });
+    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}));
+}
+
+/// 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.
+#[test]
+fn a_plugin_folder_skips_a_changed_module_until_it_is_approved() {
+    let mut s = session();
+    let id = "test.engine.permissions.folderbytes";
+    let plain = "test.engine.permissions.folderplain";
+    let first = asking(id, r#"["doc.read"]"#, &json!({"objects": []}));
+    let changed = asking(id, r#"["doc.read"]"#, &json!({"objects": [], "error": null}));
+    let dir = crate::tests_links::Folder::new("plugin-folder-grant-hash");
+    let file = dir.0.join("asks.wasm");
+    std::fs::write(&file, &first).unwrap();
+    std::fs::write(dir.0.join("plain.wasm"), asking(plain, "null", &json!({"objects": []}))).unwrap();
+    run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&first), "grant": ["doc.read"]}));
+    let path = json!({"path": dir.0.display().to_string()});
+    let r = run(&mut s, "plugin.reload", path.clone());
+    assert_eq!((r["loaded"].as_array().unwrap().len(), r["failed"].as_array().unwrap().len()), (2, 0), "{r}");
+
+    std::fs::write(&file, &changed).unwrap();
+    let r = run(&mut s, "plugin.reload", path.clone());
+    assert_eq!(r["loaded"], json!([plain]), "the other file still loads");
+    let failed = r["failed"].as_array().unwrap();
+    assert_eq!(failed.len(), 1, "{r}");
+    assert!(failed[0]["file"].as_str().unwrap().ends_with("asks.wasm"), "{r}");
+    let why = failed[0]["error"].as_str().unwrap();
+    assert!(why.contains("needs approval") && why.contains("doc.read"), "{why}");
+    let hash = vectorcraft_plugins::permissions::sha256_hex(&first);
+    assert_eq!(vectorcraft_plugins::registry::get(id).unwrap().hash(), hash, "the module installed earlier stays");
+
+    run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&changed), "grant": ["doc.read"]}));
+    let r = run(&mut s, "plugin.reload", path);
+    assert_eq!(r["failed"], json!([]), "approved on its own, it loads: {r}");
+    run(&mut s, "plugin.remove", json!({"id": id}));
+    run(&mut s, "plugin.remove", json!({"id": plain}));
+}
+
 /// The host enforces what a filter was granted: without `doc.write` an output that changes the
 /// document is refused and nothing changes, without `doc.read` it is given no objects, and without
 /// `state` it is given no data kept with the document and may not write any.