Plug-ins: a grant that can't be removed stops counting for the session

When loading the plug-in store can't remove the grant of an id whose stored
module lists no permissions, or can't read grants.json, the entry stays on
disk and still approved the bytes it covers. plugin.install has the same case
when the grant can't be removed and no earlier module can be put back.

The session now keeps those ids and does not count their grants in
plugin.inspect, plugin.install, plugin.reload or a later load of the store. A
successful update of the grant for the id ends that, and so does attaching a
different store.

crates/engine/src/cmd/plugin.rs

@@ -624,12 +624,18 @@ fn install(s: &mut Session, p: &Value) -> Result<Value> {
         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.

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<std::path::PathBuf>,
+    /// 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<std::collections::BTreeSet<String>>,
     /// 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<cmd::plugin::HandedIn>,

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(),

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<String, vectorcraft_plugins::permissions::Grant> {
+        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<vectorcraft_plugins::permissions::Grant>) -> 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<vectorcraft_plugins::permissions::Grant>) -> bool {
         let Some(dir) = &self.plugin_dir else { return false };
         let mut file = match read_grants(dir) {
             Ok(file) => file,

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::<Vec<Value>>(&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.

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