From 95dcbfe59fc5b8d251d90ae0c0844148a0159271 Mon Sep 17 00:00:00 2001 From: Don Omondi <282319807+mockydevs@users.noreply.github.com> Date: Fri, 6 Nov 2026 20:11:52 -0500 Subject: [PATCH 01/13] EPS import: an image's transient data stops counting against the memory budget The sample planes an image is decoded from, and the strings a procedure data source hands back to fill them, are freed once the image is decoded, so they stop counting against the 1 GiB budget then; the pixels kept still count. A file of many images whose procedure returns one large buffer each time no longer runs out of budget. --- crates/eps/src/import/data/ops.rs | 6 ++++++ crates/eps/src/import/interp.rs | 11 +++++++++++ crates/eps/src/import/tests.rs | 14 ++++++++++++++ docs/file-format-parity.md | 3 ++- 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/crates/eps/src/import/data/ops.rs b/crates/eps/src/import/data/ops.rs index baca4d0ed..9e4cc3a8b 100644 --- a/crates/eps/src/import/data/ops.rs +++ b/crates/eps/src/import/data/ops.rs @@ -657,8 +657,12 @@ impl Interp<'_> { return ps_err("rangecheck", "DataSource"); } let plane = row.checked_mul(s.h).filter(|p| *p <= MAX_DATA).ok_or(PsError::Limit("decoded data is too large"))?; + // The planes and the strings a procedure gives to fill them live only while the image is + // decoded: they stop counting once it is (the pixels made from them still count). + let before = self.allocated(); self.alloc(plane.saturating_mul(sources.len()))?; let mut planes = self.read_sources(sources, row * s.h, if multi { row } else { row * s.h })?; + let transient = self.allocated().saturating_sub(before); for p in &mut planes { p.resize(row * s.h, 0); } @@ -734,6 +738,8 @@ impl Interp<'_> { rgba.extend([rgb[0], rgb[1], rgb[2], if masked { 0 } else { 255 }]); } } + drop(planes); + self.unalloc(transient); Ok(Some(Pixels { w: s.w as u32, h: s.h as u32, rgba, inks, matrix: s.matrix })) } diff --git a/crates/eps/src/import/interp.rs b/crates/eps/src/import/interp.rs index 75f7c7d13..da558f174 100644 --- a/crates/eps/src/import/interp.rs +++ b/crates/eps/src/import/interp.rs @@ -341,6 +341,17 @@ impl<'a> Interp<'a> { Ok(()) } + /// The bytes counted against [`MAX_MEMORY`] so far. + pub(crate) fn allocated(&self) -> usize { + self.allocated + } + + /// Stop counting `bytes` the program no longer holds: buffers that lived only while one + /// operator ran, freed when it returned. + pub(crate) fn unalloc(&mut self, bytes: usize) { + self.allocated = self.allocated.saturating_sub(bytes); + } + fn tick(&mut self) -> Res { self.spend(1) } diff --git a/crates/eps/src/import/tests.rs b/crates/eps/src/import/tests.rs index 86fa8e35d..40cc4edc7 100644 --- a/crates/eps/src/import/tests.rs +++ b/crates/eps/src/import/tests.rs @@ -1381,6 +1381,20 @@ fn the_planes_of_multi_source_images_are_charged() { assert!(r.warnings.iter().any(|w| w.contains("too much memory")), "{:?}", r.warnings); } +/// The planes an image is decoded from, and the strings a procedure gives to fill them, stop +/// counting against the memory budget once the image is decoded: they are freed then. A procedure +/// that hands back one large buffer for every image (1 MiB here, for a one-pixel image) charged the +/// whole buffer each time, and a file of many images came in as its preview. +#[test] +fn an_images_transient_data_is_not_charged_after_it() { + let body = "/buf 1048576 string def\n1100 { 1 1 8 [1 0 0 1 0 0] { buf } image } repeat\n"; + let r = import(&eps(body)).unwrap(); + assert!(!r.warnings.iter().any(|w| w.contains("too much memory")), "{:?}", r.warnings); + let mut images = 0; + r.document.walk(|n| images += usize::from(matches!(n.kind, NodeKind::Image(_)))); + assert_eq!(images, 1100); +} + /// #3568: a single sample plane over the data limit is refused. #[test] fn an_oversized_image_plane_is_refused() { diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index d3e2f0303..038eb8a7b 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -1,6 +1,6 @@ # File format parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-11-06 · **Change:** minor (SVG import: filter and mask regions clip, #5876, #5875; SVG import: Gaussian blurs keep `color-interpolation-filters`, #5877; PDF and `.ai` export: objects' URLs as links, #5785; SVG import: `color-interpolation="linearRGB"` gradients, #5878; `.ai` import: paths with a Calligraphic brush open brushed, #372) · **Target:** Adobe Illustrator 2026 (30.x) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-11-06 · **Change:** minor (EPS import: an image's transient data stops counting against the memory budget once it is decoded; SVG import: filter and mask regions clip, #5876, #5875; SVG import: Gaussian blurs keep `color-interpolation-filters`, #5877; PDF and `.ai` export: objects' URLs as links, #5785; SVG import: `color-interpolation="linearRGB"` gradients, #5878; `.ai` import: paths with a Calligraphic brush open brushed, #372) · **Target:** Adobe Illustrator 2026 (30.x) Every format Illustrator opens, places, saves or exports, against VectorCraft's format table (`FORMATS`, `OPEN_EXTS`, `PLACE_EXTS` in `crates/engine/src/cmd/fileio/formats.rs`). Illustrator's list comes from its public @@ -191,6 +191,7 @@ the app's own editing copy of a file (an EPS after its `%%EOF`, a `.ai`'s `AIPri | Date | Change | Summary | |---|---|---| +| 2026-11-06 | minor | EPS import: the buffers an image is decoded from (its sample planes, and the strings a procedure data source hands back to fill them) stop counting against the PostScript reader's 1 GiB memory budget once the image is decoded, as they are freed then; the pixels kept still count. A user's Illustrator 25.2 EPS with 71 CMYK images (used locally, not committed) used to stop with "the program uses too much memory" and come in as its screen preview; it now opens with its art. The budget itself is unchanged. Scores and estimates unchanged. | | 2026-11-06 | minor | SVG import: filter and mask regions clip (SVG 1.1 §15.5, §14.4). An object whose imported blur, shadow or glow reaches past its filter's region comes in inside a clip group cut to the region (#5876), and a mask's art is cut to the mask's region, in user space or the object's bounding box (#5875). Objects that fit their regions, and our own exports, are unchanged; primitive subregions are still ignored. Tests `import_live::a_blur_is_cut_at_its_filter_region`, `import_live::a_mask_counts_only_inside_its_region` (crate `vectorcraft-svg`); scores unchanged | | 2026-11-06 | minor | SVG import: a Gaussian blur keeps the colour space its filter asks for with `color-interpolation-filters` (initially `linearRGB`, SVG 1.1 §15.7.6): a linear-light blur becomes Gaussian Blur with the new `linear` parameter, which blurs the colours in linear light (black beside white meets at about 188/255 instead of 128), and SVG export writes it back on the `feGaussianBlur` (#5877). Blurs that ask for sRGB, and the blurs our own export writes, are as before. CSS export still writes `blur()`. Tests `pixel::tests::linear_light_blur_mixes_light_not_values` (crate `vectorcraft-effects`), `import_live::a_gaussian_blur_keeps_its_colour_space` (crate `vectorcraft-svg`); scores unchanged | | 2026-11-06 | minor | PDF export and PDF-compatible `.ai` saves write each object's URL (Attributes panel) as a link annotation with a URI action (web and mail URLs only) over the bounds of the object that shows, trimmed by its clipping groups (ISO 32000-1, 12.5.6.5 and 12.6.4.7); hidden objects, layers that don't print and prints have none, and PDF/X files leave them out with a warning (#5785). Own-fixture tests in `tests::links`; PDF import doesn't read links back as URLs yet; scores unchanged | From 53f60823b2081f2378e2087bb279081120746b6f Mon Sep 17 00:00:00 2001 From: Don Omondi <282319807+mockydevs@users.noreply.github.com> Date: Sat, 7 Nov 2026 07:10:41 -0500 Subject: [PATCH 02/13] EPS import: release only the image decoder's own buffers from the memory budget What a data source procedure allocates stays counted: only the sample planes, the decoder's copies of the strings a procedure returns, and the split buffers of an interleaved masked image stop counting once the image is decoded. --- crates/eps/src/import/data/ops.rs | 37 ++++++++++++++++++++++--------- crates/eps/src/import/interp.rs | 9 ++------ crates/eps/src/import/tests.rs | 19 ++++++++++++---- docs/file-format-parity.md | 4 ++-- 4 files changed, 46 insertions(+), 23 deletions(-) diff --git a/crates/eps/src/import/data/ops.rs b/crates/eps/src/import/data/ops.rs index 9b758e252..0375de876 100644 --- a/crates/eps/src/import/data/ops.rs +++ b/crates/eps/src/import/data/ops.rs @@ -451,6 +451,10 @@ impl Interp<'_> { continue; } let want = step.min(n - o.len()); + // The bytes of this read's own copy of a procedure's string: counted while it + // lives, released once it is appended (the string itself is the program's, and + // stays counted as long as the program can hold it). + let mut copied = 0; let chunk = match src { Source::File(f) => self.read(f, want)?, // A string gives its bytes over and over. @@ -462,11 +466,14 @@ impl Interp<'_> { self.call(p.clone())?; let s = self.pop_str()?.to_vec(); self.alloc(s.len())?; + copied = s.len(); s } }; *e = chunk.is_empty(); o.extend_from_slice(chunk.get(..chunk.len().min(n - o.len())).unwrap_or_default()); + drop(chunk); + self.unalloc(copied); } } Ok(out) @@ -549,12 +556,16 @@ impl Interp<'_> { } return Ok(None); } + // Interleaved data is read into this decoder's own buffers, split into the two images' + // samples: those stop counting once both are decoded (their pixels still count). + let mut charged = 0; if interleave != 3.0 { let src = image.sources.first().cloned().ok_or(PsError::Ps("undefined", "DataSource".into()))?; let (pixels, samples) = if interleave == 2.0 { let (mrow, drow) = (stencil.row_bytes(1), image.row_bytes(n)); let total = mrow.saturating_mul(stencil.h).saturating_add(drow.saturating_mul(image.h)); self.alloc(total)?; + charged = total; let raw = self.source_bytes(&src, total.min(MAX_DATA))?; by_rows(&raw, (mrow, stencil.h), (drow, image.h)) } else { @@ -562,15 +573,21 @@ impl Interp<'_> { stencil.bpc = image.bpc; (stencil.w, stencil.h) = (image.w, image.h); let row = (image.w * (n + 1) * usize::from(image.bpc)).div_ceil(8); - self.alloc(row.saturating_mul(image.h))?; - let raw = self.source_bytes(&src, row.saturating_mul(image.h).min(MAX_DATA))?; + charged = row.saturating_mul(image.h); + self.alloc(charged)?; + let raw = self.source_bytes(&src, charged.min(MAX_DATA))?; by_samples(&raw, image.w, image.h, n, image.bpc) }; stencil.sources = vec![Source::Str(pixels)]; image.sources = vec![Source::Str(samples)]; } - let Some(mut img) = self.decode_image(image)? else { return Ok(None) }; - let Some(m) = self.decode_image(stencil)? else { return Ok(Some(img)) }; + let decoded = match self.decode_image(image) { + Ok(Some(img)) => self.decode_image(stencil).map(|m| Some((img, m))), + other => other.map(|_| None), + }; + self.unalloc(charged); + let Some((mut img, m)) = decoded? else { return Ok(None) }; + let Some(m) = m else { return Ok(Some(img)) }; let (w, h, mw, mh) = (img.w as usize, img.h as usize, m.w as usize, m.h as usize); for (i, px) in img.rgba.as_chunks_mut::<4>().0.iter_mut().enumerate() { let (x, y) = (i % w, i / w); @@ -689,12 +706,12 @@ impl Interp<'_> { return ps_err("rangecheck", "DataSource"); } let plane = row.checked_mul(s.h).filter(|p| *p <= MAX_DATA).ok_or(PsError::Limit("decoded data is too large"))?; - // The planes and the strings a procedure gives to fill them live only while the image is - // decoded: they stop counting once it is (the pixels made from them still count). - let before = self.allocated(); - self.alloc(plane.saturating_mul(sources.len()))?; + // The sample planes are this decoder's own and live only while the image is decoded: they + // stop counting once it is (the pixels made from them still count). What the data source + // procedures allocate is the program's and stays counted. + let plane_bytes = plane.saturating_mul(sources.len()); + self.alloc(plane_bytes)?; let mut planes = self.read_sources(sources, row * s.h, if multi { row } else { row * s.h })?; - let transient = self.allocated().saturating_sub(before); for p in &mut planes { p.resize(row * s.h, 0); } @@ -771,7 +788,7 @@ impl Interp<'_> { } } drop(planes); - self.unalloc(transient); + self.unalloc(plane_bytes); Ok(Some(Pixels { w: s.w as u32, h: s.h as u32, rgba, inks, matrix: s.matrix })) } diff --git a/crates/eps/src/import/interp.rs b/crates/eps/src/import/interp.rs index 783bf0a1a..9252d827c 100644 --- a/crates/eps/src/import/interp.rs +++ b/crates/eps/src/import/interp.rs @@ -350,13 +350,8 @@ impl<'a> Interp<'a> { Ok(()) } - /// The bytes counted against [`MAX_MEMORY`] so far. - pub(crate) fn allocated(&self) -> usize { - self.allocated - } - - /// Stop counting `bytes` the program no longer holds: buffers that lived only while one - /// operator ran, freed when it returned. + /// Stop counting `bytes` the program no longer holds: buffers an operator made for itself and + /// freed before it returned. Never for what the program's own procedures allocated. pub(crate) fn unalloc(&mut self, bytes: usize) { self.allocated = self.allocated.saturating_sub(bytes); } diff --git a/crates/eps/src/import/tests.rs b/crates/eps/src/import/tests.rs index 7f2f10471..395ee130e 100644 --- a/crates/eps/src/import/tests.rs +++ b/crates/eps/src/import/tests.rs @@ -1419,10 +1419,10 @@ fn the_planes_of_multi_source_images_are_charged() { assert!(r.warnings.iter().any(|w| w.contains("too much memory")), "{:?}", r.warnings); } -/// The planes an image is decoded from, and the strings a procedure gives to fill them, stop -/// counting against the memory budget once the image is decoded: they are freed then. A procedure -/// that hands back one large buffer for every image (1 MiB here, for a one-pixel image) charged the -/// whole buffer each time, and a file of many images came in as its preview. +/// The planes an image is decoded from, and the decoder's copies of the strings a procedure gives +/// to fill them, stop counting against the memory budget once the image is decoded: they are freed +/// then. A procedure that hands back one large buffer for every image (1 MiB here, for a one-pixel +/// image) charged the whole buffer each time, and a file of many images came in as its preview. #[test] fn an_images_transient_data_is_not_charged_after_it() { let body = "/buf 1048576 string def\n1100 { 1 1 8 [1 0 0 1 0 0] { buf } image } repeat\n"; @@ -1433,6 +1433,17 @@ fn an_images_transient_data_is_not_charged_after_it() { assert_eq!(images, 1100); } +/// What a data source procedure allocates stays counted, whatever image it fills: a procedure +/// that keeps each new string (in an array here) still runs into the memory budget, however small +/// the images it fills are. +#[test] +fn strings_a_data_source_keeps_stay_charged() { + let body = "/keep 1100 array def /i 0 def\n\ + 1100 { 1 1 8 [1 0 0 1 0 0] { keep i 1048576 string put keep i get /i i 1 add def } image } repeat\n"; + let r = import(&eps(body)).unwrap(); + assert!(r.warnings.iter().any(|w| w.contains("too much memory")), "{:?}", r.warnings); +} + /// #3568: a single sample plane over the data limit is refused. #[test] fn an_oversized_image_plane_is_refused() { diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 67b12e70c..e971ce731 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -1,6 +1,6 @@ # File format parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-11-06 · **Change:** minor (EPS import: an image's transient data stops counting against the memory budget once it is decoded; EPS and `.ai` editing data: a transparency set inside a clipping group stays after its `Q`; SVG import: filter and mask regions clip, #5876, #5875; SVG import: Gaussian blurs keep `color-interpolation-filters`, #5877; PDF and `.ai` export: objects' URLs as links, #5785; SVG import: `color-interpolation="linearRGB"` gradients, #5878; `.ai` import: paths with a Calligraphic brush open brushed, #372) · **Target:** Adobe Illustrator 2026 (30.x) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-11-06 · **Change:** minor (EPS import: an image decoder's own buffers stop counting against the memory budget once it is decoded; EPS and `.ai` editing data: a transparency set inside a clipping group stays after its `Q`; SVG import: filter and mask regions clip, #5876, #5875; SVG import: Gaussian blurs keep `color-interpolation-filters`, #5877; PDF and `.ai` export: objects' URLs as links, #5785; SVG import: `color-interpolation="linearRGB"` gradients, #5878; `.ai` import: paths with a Calligraphic brush open brushed, #372) · **Target:** Adobe Illustrator 2026 (30.x) Every format Illustrator opens, places, saves or exports, against VectorCraft's format table (`FORMATS`, `OPEN_EXTS`, `PLACE_EXTS` in `crates/engine/src/cmd/fileio/formats.rs`). Illustrator's list comes from its public @@ -212,7 +212,7 @@ the app's own editing copy of a file (an EPS after its `%%EOF`, a `.ai`'s `AIPri | Date | Change | Summary | |---|---|---| -| 2026-11-06 | minor | EPS import: the buffers an image is decoded from (its sample planes, and the strings a procedure data source hands back to fill them) stop counting against the PostScript reader's 1 GiB memory budget once the image is decoded, as they are freed then; the pixels kept still count. A user's Illustrator 25.2 EPS with 71 CMYK images (used locally, not committed) used to stop with "the program uses too much memory" and come in as its screen preview; it now opens with its art. The budget itself is unchanged. Scores and estimates unchanged. | +| 2026-11-06 | minor | EPS import: the buffers an image decoder makes for itself (its sample planes, its copies of the strings a procedure data source hands back, and the split buffers of an interleaved masked image) stop counting against the PostScript reader's 1 GiB memory budget once the image is decoded, as they are freed then; the pixels kept, and anything the program's own procedures allocate, still count. A user's Illustrator 25.2 EPS with 71 CMYK images (used locally, not committed) used to stop with "the program uses too much memory" and come in as its screen preview; it now opens with its art. The budget itself is unchanged. Scores and estimates unchanged. | | 2026-11-06 | minor | EPS and `.ai` editing data: `Q` restores the painting state but not the transparency (`Xy`), which the writer only sets when the next object's differs from the last one written. A Screen glow followed by a clipping group set back to Normal no longer leaves the type after the group in Screen: two users' Illustrator 26.5 and 30 EPS posters (used locally, not committed) now show their gold heading and crescent, and both open from their layers (2.5% and 2.7% from the page before, under the limit after). Scores and estimates unchanged. | | 2026-11-06 | minor | EPS and `.ai` editing data: a blend whose spine was never edited, written with its keys and no spine part, comes in as a live blend along the straight lines between its keys' centres (in its record's order) instead of as its drawn steps. Checked on six users' Illustrator EPS files (used locally, not committed): every blend in four of them now comes in live, their renders unchanged; two still have blends the reader can't match to their drawn steps. Scores and estimates unchanged. | | 2026-11-06 | minor | EPS and `.ai` editing data: the art styles are found before the art is read, so objects in symbols defined ahead of `%AI9_BeginArtStyles` (as Illustrator 24 writes it) come in as themselves with their style instead of their drawn look. Checked on two users' EPS files (used locally, not committed): one's Symbols panel gains live Feather and Round Corners, its drawn-look count drops from 5 to 2, and both render unchanged; own-fixture test in `import::ai::tests`; scores and hours unchanged | From 22a6edc9167e607646a9d254baf1508791f7d5d4 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 8 Nov 2026 10:17:58 -0800 Subject: [PATCH 03/13] Plug-ins: a module that lists no permissions ends the grant of its id Installing a module that lists no permissions under a granted id now forgets the grant, in the process and in grants.json, so the formerly approved bytes need a new approval. A refused install leaves the grant. Attaching a plug-in store forgets the grant of an id whose stored module lists no permissions. A plug-in folder load forgets the process's grant for such an id and leaves grants.json alone, because the store keeps the module its grant covers. --- crates/engine/src/cmd/plugin.rs | 8 +- crates/engine/src/store.rs | 15 ++- crates/engine/src/tests/plugins.rs | 141 +++++++++++++++++++++++++++++ crates/plugins/src/registry.rs | 7 ++ docs/mcp.md | 3 +- docs/plugins.md | 11 ++- docs/target-app-parity.md | 1 + 7 files changed, 178 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/cmd/plugin.rs b/crates/engine/src/cmd/plugin.rs index 6457e2cc9..c8c639148 100644 --- a/crates/engine/src/cmd/plugin.rs +++ b/crates/engine/src/cmd/plugin.rs @@ -336,7 +336,7 @@ fn grant_param(c: &str, p: &Value) -> Result>> { /// `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. +/// `plugin.list` entry. A module that lists no permissions ends any grant held for its id. fn install(s: &mut Session, p: &Value) -> Result { const C: &str = "plugin.install"; let (replace, allow_heavy) = (switch_or(p, "replace", true, C)?, switch_or(p, "allowHeavy", false, C)?); @@ -375,6 +375,10 @@ fn install(s: &mut Session, p: &Value) -> Result { 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)); + } else { + // This module takes the id over, so nothing the user approved for the id applies any more. + registry::revoke(&m.id); + s.store_grant(&m.id, None); } // Installed plug-ins stay installed after a restart (#2541). s.store_plugin(plugin.id(), &bytes); @@ -527,7 +531,7 @@ pub fn specs() -> Vec { "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 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", + "{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; installing a module that lists no permissions forgets the grant held for its id; 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 ), diff --git a/crates/engine/src/store.rs b/crates/engine/src/store.rs index 3d8259361..14f9b21d9 100644 --- a/crates/engine/src/store.rs +++ b/crates/engine/src/store.rs @@ -21,7 +21,7 @@ //! The files are untrusted input like any other: sizes and counts are capped, a file that does not //! parse is skipped with a log record, and every write is atomic. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::io::Read; use std::path::{Path, PathBuf}; @@ -225,11 +225,13 @@ 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 that - /// very module, is skipped with a log record, and stays in `dir` with its grant). + /// very module, is skipped with a log record, and stays in `dir` with its grant). A grant for + /// an id whose module there lists no permissions is forgotten. pub fn attach_plugin_store(&mut self, dir: PathBuf) { let grants = stored_grants(&dir); let limits = vectorcraft_plugins::Limits::default(); let mut left = MAX_PLUGIN_STORE; + let mut asks_nothing = BTreeSet::new(); for path in list_files(&dir, PLUGIN_EXT) { if left == 0 { log::warn!("skipping the other stored plug-ins: they hold more than {MAX_PLUGIN_STORE} bytes together"); @@ -246,6 +248,7 @@ impl Session { let m = plugin.manifest(); match grants.get(&m.id).filter(|g| g.covers(&m.id, plugin.hash(), &m.permissions())) { Some(g) => { + asks_nothing.remove(&m.id); vectorcraft_plugins::registry::grant(g.clone()); vectorcraft_plugins::registry::install(plugin.with_source(path.display().to_string())); } @@ -253,12 +256,20 @@ impl Session { } } Ok(plugin) => { + asks_nothing.insert(plugin.id().to_string()); vectorcraft_plugins::registry::install(plugin.with_source(path.display().to_string())); } Err(e) => log::warn!("skipping the stored plug-in {}: {e}", path.display()), } } + let kept = read_grants(&dir).unwrap_or_default(); self.plugin_dir = Some(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); + } + } } /// Whether `plugin` installs without asking the user: it lists no permissions, or this very diff --git a/crates/engine/src/tests/plugins.rs b/crates/engine/src/tests/plugins.rs index 7d0983ef2..5425b9d31 100644 --- a/crates/engine/src/tests/plugins.rs +++ b/crates/engine/src/tests/plugins.rs @@ -634,6 +634,147 @@ fn a_grant_covers_only_the_module_that_was_approved() { run(&mut s, "plugin.remove", json!({"id": exempt})); } +/// Installing a module that lists no permissions under a granted id forgets the grant, so the +/// formerly approved bytes need a new approval. A refused install leaves the grant, and a module +/// that lists permissions keeps replacing it as before. +#[test] +fn a_module_without_permissions_ends_the_grant_of_its_id() { + use vectorcraft_plugins::permissions::sha256_hex; + let mut s = session(); + let id = "test.engine.permissions.replaced"; + let empty = json!({"objects": []}); + let approved = asking(id, r#"["doc.read"]"#, &empty); + let approved_data = vectorcraft_format::base64_encode(&approved); + let granted = || vectorcraft_plugins::registry::granted(id).map(|g| g.hash); + for permissions in ["null", "[]"] { + let plain = vectorcraft_format::base64_encode(&asking(id, permissions, &empty)); + run(&mut s, "plugin.install", json!({"dataBase64": approved_data, "grant": ["doc.read"]})); + // Refused installs change nothing. + let e = s.execute("plugin.install", &json!({"dataBase64": plain, "hash": "0".repeat(64)})).unwrap_err().to_string(); + assert!(e.contains("SHA-256 `hash`"), "{e}"); + let e = s.execute("plugin.install", &json!({"dataBase64": plain, "replace": false})).unwrap_err().to_string(); + assert!(e.contains("already installed"), "{e}"); + assert_eq!(granted(), Some(sha256_hex(&approved)), "{permissions}: a refused install leaves the grant"); + + run(&mut s, "plugin.install", json!({"dataBase64": plain})); + assert_eq!(granted(), None, "{permissions}"); + assert_eq!(run(&mut s, "plugin.inspect", json!({"dataBase64": approved_data}))["needsApproval"], true, "{permissions}"); + let e = s.execute("plugin.install", &json!({"dataBase64": approved_data})).unwrap_err().to_string(); + assert!(e.contains("`grant`"), "{permissions}: {e}"); + assert!(!vectorcraft_plugins::registry::get(id).unwrap().manifest().asks_permissions(), "{permissions}: the module installed stays"); + } + // With no module installed under the id, the grant ends the same way. + run(&mut s, "plugin.install", json!({"dataBase64": approved_data, "grant": ["doc.read"]})); + vectorcraft_plugins::registry::remove(id); + run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&asking(id, "null", &empty))})); + assert_eq!(granted(), None); + // A module that lists permissions records its own grant. + run(&mut s, "plugin.install", json!({"dataBase64": approved_data, "grant": ["doc.read"]})); + assert_eq!(granted(), Some(sha256_hex(&approved))); + run(&mut s, "plugin.remove", json!({"id": id})); +} + +/// In a plug-in store, a module that lists no permissions removes only its own id's entry from +/// `grants.json`, and the formerly approved bytes need a new approval after a restart. A store +/// 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}; + let folder = crate::tests_links::Folder::new("plugin-store-stale-grant"); + let id = "test.engine.permissions.storedstale"; + let other = "test.engine.permissions.storedother"; + 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 other_wasm = asking(other, r#"["doc.read"]"#, &empty); + let grants = folder.0.join("grants.json"); + let ids = || -> Vec { + serde_json::from_slice::>(&std::fs::read(&grants).unwrap()) + .unwrap() + .iter() + .map(|e| e["id"].as_str().unwrap().to_string()) + .collect() + }; + let mut s = session(); + s.attach_plugin_store(folder.0.clone()); + run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&other_wasm), "grant": ["doc.read"]})); + run(&mut s, "plugin.install", json!({"dataBase64": approved_data, "grant": ["doc.read"]})); + assert_eq!(ids().len(), 2); + run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&plain)})); + assert_eq!(ids(), vec![other.to_string()], "only this id's entry goes"); + assert!(vectorcraft_plugins::registry::granted(id).is_none()); + assert!(vectorcraft_plugins::registry::granted(other).is_some(), "the other plug-in's grant stays"); + + vectorcraft_plugins::registry::remove(id); + let mut next = session(); + next.attach_plugin_store(folder.0.clone()); + assert_eq!(run(&mut next, "plugin.inspect", json!({"dataBase64": approved_data}))["needsApproval"], true); + + // A grant the store already holds for an id whose stored module lists no permissions. + let entry = json!({"id": id, "hash": sha256_hex(&approved), "permissions": ["doc.read"]}); + let other_entry = json!({"id": other, "hash": sha256_hex(&other_wasm), "permissions": ["doc.read"]}); + std::fs::write(&grants, serde_json::to_vec(&json!([entry, other_entry])).unwrap()).unwrap(); + vectorcraft_plugins::registry::remove(id); + let mut third = session(); + third.attach_plugin_store(folder.0.clone()); + assert!(vectorcraft_plugins::registry::get(id).is_some(), "the module that lists nothing still loads"); + assert_eq!(ids(), vec![other.to_string()]); + assert!(vectorcraft_plugins::registry::get(other).is_some()); + + // So does an entry this version can't read, and a grant held only by the process. + 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] }); + let mut fourth = session(); + fourth.attach_plugin_store(folder.0.clone()); + assert_eq!(ids(), vec![other.to_string()]); + assert!(vectorcraft_plugins::registry::granted(id).is_none()); + run(&mut fourth, "plugin.remove", json!({"id": id})); + run(&mut fourth, "plugin.remove", json!({"id": other})); +} + +/// A plug-in folder that leaves a module listing no permissions under an id forgets the process's +/// grant for it, unless a later file in the folder leaves a granted module there. A grant kept in +/// the plug-in store stays, with the stored module it covers. +#[test] +fn a_plugin_folder_ends_the_process_grant_of_an_id_it_replaces() { + let mut s = session(); + let id = "test.engine.permissions.folderstale"; + 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 dir = crate::tests_links::Folder::new("plugin-folder-stale-grant"); + std::fs::write(dir.0.join("a-plain.wasm"), &plain).unwrap(); + std::fs::write(dir.0.join("b-approved.wasm"), &approved).unwrap(); + let path = json!({"path": dir.0.display().to_string()}); + run(&mut s, "plugin.install", json!({"dataBase64": approved_data, "grant": ["doc.read"]})); + + let r = run(&mut s, "plugin.reload", path.clone()); + assert_eq!(r["failed"], json!([]), "{r}"); + assert!(vectorcraft_plugins::registry::granted(id).is_some(), "the granted module is the one left"); + + std::fs::remove_file(dir.0.join("b-approved.wasm")).unwrap(); + let r = run(&mut s, "plugin.reload", path.clone()); + assert_eq!(r["loaded"], json!([id]), "{r}"); + assert!(vectorcraft_plugins::registry::granted(id).is_none()); + assert_eq!(run(&mut s, "plugin.inspect", json!({"dataBase64": approved_data}))["needsApproval"], true); + run(&mut s, "plugin.remove", json!({"id": id})); + + let store = crate::tests_links::Folder::new("plugin-folder-stale-grant-store"); + let mut stored = session(); + stored.attach_plugin_store(store.0.clone()); + run(&mut stored, "plugin.install", json!({"dataBase64": approved_data, "grant": ["doc.read"]})); + let grants = store.0.join("grants.json"); + let recorded = std::fs::read(&grants).unwrap(); + run(&mut stored, "plugin.reload", path); + assert_eq!(std::fs::read(&grants).unwrap(), recorded, "the store still holds the module its grant covers"); + assert_eq!(run(&mut stored, "plugin.inspect", json!({"dataBase64": approved_data}))["needsApproval"], false); + run(&mut stored, "plugin.remove", json!({"id": id})); +} + /// 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. diff --git a/crates/plugins/src/registry.rs b/crates/plugins/src/registry.rs index 8e4635c3f..986a30e17 100644 --- a/crates/plugins/src/registry.rs +++ b/crates/plugins/src/registry.rs @@ -133,6 +133,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. +/// When the module a load leaves under an id lists no permissions, the process's grant for that id +/// is forgotten. #[cfg(not(target_arch = "wasm32"))] pub fn load_folder_where( dir: &std::path::Path, @@ -166,5 +168,10 @@ pub fn load_folder_where( Err(e) => report.failed.push((f.display().to_string(), e.to_string())), } } + for id in &report.loaded { + if get(id).is_some_and(|p| !p.manifest().asks_permissions()) { + revoke(id); + } + } Ok(report) } diff --git a/docs/mcp.md b/docs/mcp.md index 71e7978bd..d83db18ca 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -6176,7 +6176,8 @@ the entry's `hash` as `hash` so that a module that changed since is refused. Wit an error naming the permissions needed. The grant is kept with the module's hash and covers only that module: the same bytes install again without `grant`, and a module with other bytes has `needsApproval: true` and needs a new `grant` even when it lists the same permissions. `plugin.install` does not check that a person approved `grant`: it -trusts the client that sends it. `plugin.remove` forgets the grant. A +trusts the client that sends it. `plugin.remove` forgets the grant, and so does installing a module that lists no +permissions under that id: the earlier bytes then need a new `grant`. A filter without `doc.write` or `state` that returns output needing them gets an error and the document is unchanged. A plug-in of API level 2 (`api: 2` in its entry) that lists `progress` may report its progress, see Cancel and write log lines: `notifications/cancelled` makes its `vc_cancelled` return 1, and its log lines go to the server's log diff --git a/docs/plugins.md b/docs/plugins.md index 80544ad2b..2865f1dbf 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -137,9 +137,14 @@ capability model proposed for PhotoCraft plug-ins `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 - 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 + hexadecimal digits covers nothing. `plugin.remove` forgets the grant, and so does a successful `plugin.install` of a + module that lists no permissions under that id, so the earlier bytes need a new approval. A refused install leaves + the grant as it is. A stored module that lists no permissions has any grant for its id forgotten when the `Plug-ins` + folder is attached. 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 covers. 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, + replaced by a module that lists no permissions, or removed. A `grants.json` that is not a JSON list is never overwritten, and the file is never written larger than 1 MiB, the most that is read. When a `Plug-ins` folder is in use, only the grants kept in it count for `plugin.install`, `plugin.inspect`, `plugin.reload` and the Additional Plug-ins Folder. At startup, a stored plug-in is skipped when no grant covers its bytes and the permissions it lists. The Additional Plug-ins diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 816df8d0f..bb6e532f2 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1505,6 +1505,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - **Plug-in instruction budget at a bulk memory instruction:** a run that pauses where one slice of its instruction budget ends keeps the instructions the slice still held. A `memory.fill` or `memory.copy` that the slice can't pay for in full runs on the next slice, so a run whose budget covers its work finishes; a run ends with `Limit("instruction budget")` only when the instructions it has left can't pay for the next instruction. Regressions: `crates/plugins/tests/abi.rs`. Scores and hours unchanged. - **Plug-in permissions (G17):** a manifest may list `permissions` (`doc.read`, `doc.write`, `state`, `effect.render`, `export.write`; the first three follow storytold/photocraft#662) and an `api` level; without the list a plug-in gets what its kind implies and installs as before. Install Plug-in… and File › Open show the name, id, version, file SHA-256 and each permission in plain words, with control and bidirectional characters escaped, and install nothing on Cancel; `plugin.install` needs `grant`, and folders install only granted plug-ins. The grant is kept with the module's hash, a module asking for more asks again, and a filter run refuses output its permissions don't allow. Regressions: `crates/plugins/src/permissions.rs`, `crates/plugins/src/manifest.rs`, `crates/engine/src/tests/plugins.rs`, `crates/engine/tests/plugin_fuzz.rs`, `crates/ui-egui/src/tests/plugins.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 with 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 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. +- **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`. Scores and hours unchanged. - **Plug-in progress, Cancel and log lines (G17):** at plug-in API level 2 a plug-in with the `progress` permission imports `env.vc_progress`, `env.vc_cancelled` and `env.vc_log`. A heavy filter's and an export's progress windows show its progress as a percentage bar, their Cancel reaches it through `vc_cancelled`, and its log lines go to the app's log with its id, capped per line and per run. Installing checks every import and names the one it refuses. Regressions: `crates/plugins/src/host.rs`, `crates/plugins/tests/abi.rs`, `crates/engine/src/tests/plugins.rs`, `crates/engine/tests/plugin_fuzz.rs`, `crates/ui-egui/src/tests/plugin_runs.rs`. Scores and hours unchanged. - **Plug-in effects after a plug-in changes:** when a plug-in is installed for the first time, installed again or removed (`plugin.install`, `plugin.reload`, `plugin.remove`, File › Open of a `.wasm` file, the Additional Plug-ins Folder preference), the next frame the canvas draws uses the plug-ins then installed for objects with a plug-in effect and their raster effects, for the blends, envelopes and repeats that show such objects, and for pattern tiles and symbol instances whose art holds one. From cb3108a9b4079ffd23fdb028889892dd5bc087f2 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 8 Nov 2026 11:05:25 -0800 Subject: [PATCH 04/13] Plug-ins: a failed store write keeps the stored grant A module that lists no permissions now removes the grant from grants.json only after it has replaced the stored module. When the write fails, the old module stays stored and keeps the grant it needs at the next launch. --- crates/engine/src/cmd/plugin.rs | 6 +++++- crates/engine/src/tests/plugins.rs | 26 ++++++++++++++++++++++++++ docs/plugins.md | 3 ++- 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/plugin.rs b/crates/engine/src/cmd/plugin.rs index c8bcc8964..75dd5d497 100644 --- a/crates/engine/src/cmd/plugin.rs +++ b/crates/engine/src/cmd/plugin.rs @@ -386,10 +386,14 @@ fn install(s: &mut Session, p: &Value) -> Result { } else { // This module takes the id over, so nothing the user approved for the id applies any more. registry::revoke(&m.id); - s.store_grant(&m.id, None); } // Installed plug-ins stay installed after a restart (#2541). let stored = s.store_plugin(plugin.id(), &bytes); + // When the write fails, the old stored module stays in the folder and needs its grant at the + // next launch. + if stored && !m.asks_permissions() { + s.store_grant(&m.id, None); + } // 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/tests/plugins.rs b/crates/engine/src/tests/plugins.rs index 5425b9d31..d11f13bee 100644 --- a/crates/engine/src/tests/plugins.rs +++ b/crates/engine/src/tests/plugins.rs @@ -735,6 +735,32 @@ fn a_module_without_permissions_ends_the_grant_kept_in_the_store() { run(&mut fourth, "plugin.remove", json!({"id": other})); } +/// A module listing no permissions that can't be written to the plug-in store leaves the stored +/// module in place, so `grants.json` keeps the grant that module needs at the next launch. +#[test] +fn a_failed_store_write_keeps_the_stored_grant() { + let folder = crate::tests_links::Folder::new("plugin-store-failed-write"); + let id = "test.engine.permissions.storefail"; + let empty = json!({"objects": []}); + let approved = asking(id, r#"["doc.read"]"#, &empty); + 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": vectorcraft_format::base64_encode(&approved), "grant": ["doc.read"]})); + let before = std::fs::read(&grants).unwrap(); + let file = s.stored_plugin_file(id).unwrap(); + std::fs::remove_file(&file).unwrap(); + std::fs::create_dir(&file).unwrap(); + + run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&plain)})); + assert_eq!(std::fs::read(&grants).unwrap(), before, "the grant stays with the module left in the store"); + assert!(vectorcraft_plugins::registry::granted(id).is_none(), "the process runs the module that lists nothing"); + + std::fs::remove_dir(&file).unwrap(); + run(&mut s, "plugin.remove", json!({"id": id})); +} + /// A plug-in folder that leaves a module listing no permissions under an id forgets the process's /// grant for it, unless a later file in the folder leaves a granted module there. A grant kept in /// the plug-in store stays, with the stored module it covers. diff --git a/docs/plugins.md b/docs/plugins.md index e35d6d648..04a18c9f4 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -139,7 +139,8 @@ capability model proposed for PhotoCraft plug-ins grant stay as they are; once it is, the grant records the new hash. A grant whose hash is not 64 lower-case hexadecimal digits covers nothing. `plugin.remove` forgets the grant, and so does a successful `plugin.install` of a module that lists no permissions under that id, so the earlier bytes need a new approval. A refused install leaves - the grant as it is. A stored module that lists no permissions has any grant for its id forgotten when the `Plug-ins` + the grant as it is. When such an install can't write the module to the `Plug-ins` folder, the module already stored + there stays and `grants.json` keeps its grant. A stored module that lists no permissions has any grant for its id forgotten when the `Plug-ins` folder is attached. 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 covers. An entry of `grants.json` that this version can't use (a From 0a4c037fb181c96a6ade4a353f6812d929993a9c Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 8 Nov 2026 16:04:58 -0800 Subject: [PATCH 05/13] Plug-ins: docs state that a stored module not read keeps its id's grant In Safe Mode and for a disabled stored module, load_stored_plugins does not read the module, so grants.json keeps the grant for its id. The next load that reads the module forgets the grant. --- crates/engine/tests/plugin_safe_mode.rs | 31 +++++++++++++++++++++++++ docs/plugins.md | 5 ++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/crates/engine/tests/plugin_safe_mode.rs b/crates/engine/tests/plugin_safe_mode.rs index 7fe54fb53..1f49fea15 100644 --- a/crates/engine/tests/plugin_safe_mode.rs +++ b/crates/engine/tests/plugin_safe_mode.rs @@ -217,6 +217,37 @@ fn a_disabled_plug_in_whose_new_grant_cant_be_stored_stays_disabled() { let _ = std::fs::remove_dir_all(&dir); } +/// A stored module that lists no permissions and isn't read, because it is disabled or Safe Mode is +/// on, leaves the grant for its id in the store. The load that reads it forgets the grant. +#[test] +fn a_stored_module_that_is_not_read_keeps_the_grant_for_its_id() { + let _f = fresh(); + let dir = tmp_dir("unread-grant"); + let id = "test.sm.unread"; + let mut s = launch(&dir); + s.execute("plugin.install", &json!({"dataBase64": vectorcraft_format::base64_encode(&filter(id))})).unwrap(); + let grants = dir.join("grants.json"); + let entry = serde_json::to_vec(&json!([{"id": id, "hash": "0".repeat(64), "permissions": ["doc.read"]}])).unwrap(); + std::fs::write(&grants, &entry).unwrap(); + + s.execute("plugin.setEnabled", &json!({"id": id, "enabled": false})).unwrap(); + let mut s = launch(&dir); + assert!(!installed(id), "the disabled module is skipped"); + assert_eq!(std::fs::read(&grants).unwrap(), entry, "a disabled module keeps the grant"); + + s.execute("plugin.setEnabled", &json!({"id": id, "enabled": true})).unwrap(); + vectorcraft_engine::cmd::plugin::start_in_safe_mode(); + let mut s = launch(&dir); + assert!(!installed(id), "Safe Mode loads nothing"); + assert_eq!(std::fs::read(&grants).unwrap(), entry, "Safe Mode keeps the grant"); + + safe_mode::set_active(None); + vectorcraft_engine::cmd::plugin::load_prefs_folder(&mut s); + assert!(installed(id)); + assert!(!grants.exists(), "the load that reads the module forgets the grant"); + 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 29ab56311..45ef94e16 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -156,8 +156,9 @@ capability model proposed for PhotoCraft plug-ins hexadecimal digits covers nothing. `plugin.remove` forgets the grant, and so does a successful `plugin.install` of a module that lists no permissions under that id, so the earlier bytes need a new approval. A refused install leaves the grant as it is. When such an install can't write the module to the `Plug-ins` folder, the module already stored - there stays and `grants.json` keeps its grant. A stored module that lists no permissions has any grant for - its id forgotten when the `Plug-ins` folder is attached. The Additional Plug-ins Folder + there stays and `grants.json` keeps its grant. 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, + 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 covers. An entry of `grants.json` that this version can't use (a permission it doesn't know, an invalid id) is From b79e89de1df25013e25158cfd3bb397456aa412d Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 8 Nov 2026 17:25:09 -0800 Subject: [PATCH 06/13] Plug-ins: a grant that can't be removed keeps the module it covers plugin.install of a module that lists no permissions ignored the result of removing the id's grant. When grants.json could not be updated, the formerly approved bytes could still be installed without approval. The store now gets the old module back, so the grant and the module it covers stay together. The new module stays when the old one can't be read back and when grants.json can't be read, since such a file approves nothing. Deleting the last grant now reports a failure. plugin.remove had the same fault. It now moves the settings and cache aside and removes the grant before anything is deleted. When the grant can't be removed, the folder is moved back, the command returns an error and the plug-in stays installed. --- crates/engine/src/cmd/plugin.rs | 32 ++++--- crates/engine/src/store.rs | 47 ++++++++--- crates/engine/src/tests/plugins.rs | 129 ++++++++++++++++++++++++++++- docs/mcp.md | 3 +- docs/plugins.md | 12 ++- 5 files changed, 194 insertions(+), 29 deletions(-) diff --git a/crates/engine/src/cmd/plugin.rs b/crates/engine/src/cmd/plugin.rs index 6a64774b1..c802d0e3e 100644 --- a/crates/engine/src/cmd/plugin.rs +++ b/crates/engine/src/cmd/plugin.rs @@ -608,9 +608,12 @@ fn install(s: &mut Session, p: &Value) -> Result { // This module takes the id over, so nothing the user approved for the id applies any more. registry::revoke(&m.id); // When the write failed, the old stored module stays in the folder and needs its grant at - // the next launch. - if stored { - s.store_grant(&m.id, None); + // 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 can't be read + // approves nothing, and the new module stays. + 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; } } // It gets the settings and cache kept for its id. @@ -822,19 +825,24 @@ fn remove(s: &mut Session, p: &Value) -> Result { let installed = registry::get(id).is_some(); // Settings and cache the store keeps for an id that isn't installed are deleted too. let stored = !installed && vectorcraft_plugins::manifest::valid_id(id) && s.has_plugin_storage(id); - // Its settings and cache go first: when they can't be moved aside, the plug-in stays installed, - // as it was, and a second `plugin.remove` tries again. - if (installed || stored) - && let Err(e) = s.remove_plugin_storage(id) - { + // Installed again, it asks again. A grant that can't be removed still approves the module; a + // grants file that can't be read approves nothing and doesn't stop the removal. + let forget = || s.store_grant(id, None) || !s.holds_plugin_grant(id); + // Its settings and cache are moved aside and its grant removed before anything is deleted: when + // either can't be done, the plug-in stays installed, as it was, and a second `plugin.remove` + // tries again. + let failed = if installed || stored { + s.remove_plugin_storage(id, forget).err().map(|e| e.to_string()) + } else { + (!forget()).then(|| crate::store::GRANT_NOT_REMOVED.to_string()) + }; + if let Some(e) = failed { if installed { s.attach_plugin_storage(id); } - return Err(EngineError::Other(e.to_string())); + return Err(EngineError::Other(e)); } - // Installed again, it asks again. registry::revoke(id); - s.store_grant(id, None); s.unstore_plugin(id); if !stored && (!installed || !registry::remove(id)) { return Err(err(vectorcraft_plugins::Error::NotFound(id.into()))); @@ -913,7 +921,7 @@ pub fn specs() -> Vec { "Remove Plug-in", [], None, - "{id} uninstall a plug-in (its live effects then leave the geometry as it is) and delete the settings and cache it kept (when their folder can't be moved aside, the command fails and the plug-in stays installed); for an id that isn't installed, deletes the settings and cache the plug-in store still keeps for it → {removed}", + "{id} uninstall a plug-in (its live effects then leave the geometry as it is) and delete the settings and cache it kept (when their folder can't be moved aside, or its grant can't be removed from the plug-in store, the command fails and the plug-in stays installed); for an id that isn't installed, deletes the settings and cache the plug-in store still keeps for it → {removed}", always, remove ), diff --git a/crates/engine/src/store.rs b/crates/engine/src/store.rs index 824479d73..6ebdea6a7 100644 --- a/crates/engine/src/store.rs +++ b/crates/engine/src/store.rs @@ -61,6 +61,8 @@ pub const DISABLED_FILE: &str = "disabled.json"; pub const DISABLED_LOCK: &str = "disabled.lock"; /// The file in the plug-in store that holds the permissions the user granted. const GRANTS_FILE: &str = "grants.json"; +/// The error of a `plugin.remove` that can't remove the plug-in's grant from [`GRANTS_FILE`]. +pub(crate) const GRANT_NOT_REMOVED: &str = "the plug-in's grant can't be removed from the plug-in store"; /// The largest grants file read. const MAX_GRANTS_FILE: u64 = 1 << 20; /// The folder in the plug-in store that holds each plug-in's settings and cache. @@ -167,13 +169,15 @@ fn lock_disabled(_: &Path) -> Option { None } -/// Remove `dir/file`; a file that isn't there is what was wanted. -fn take(dir: &Path, file: &str) { +/// Remove `dir/file` → whether it is gone; a file that isn't there is what was wanted. +fn take(dir: &Path, file: &str) -> bool { let path = dir.join(file); - if let Err(e) = std::fs::remove_file(&path) - && e.kind() != std::io::ErrorKind::NotFound - { - log::warn!("can't remove {}: {e}", path.display()); + match std::fs::remove_file(&path) { + Err(e) if e.kind() != std::io::ErrorKind::NotFound => { + log::warn!("can't remove {}: {e}", path.display()); + false + } + _ => true, } } @@ -470,8 +474,7 @@ impl Session { 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 true; + return take(dir, GRANTS_FILE); } // 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. @@ -485,6 +488,12 @@ impl Session { } } + /// Whether the store holds a grant this version can use for the plug-in `id` (`false` without a + /// folder, and when the grants file can't be read). + pub(crate) fn holds_plugin_grant(&self, id: &str) -> bool { + self.plugin_dir.as_deref().is_some_and(|dir| stored_grants(dir).contains_key(id)) + } + /// Remove the plug-in `id` from the store. pub(crate) fn unstore_plugin(&self, id: &str) { if let Some(dir) = &self.plugin_dir { @@ -510,10 +519,15 @@ impl Session { /// Delete the settings and cache of plug-in `id` from the store, and stop keeping any for it. /// The folder is first renamed out of the way (`.removed`), and that fails, leaving it /// as it was, when the rename does; what the deletion then leaves stays under that name, with a - /// log record, and is deleted at the plug-in's next removal. - pub(crate) fn remove_plugin_storage(&self, id: &str) -> Result<(), StorageError> { + /// log record, and is deleted at the plug-in's next removal. `forget` runs once the folder is out + /// of the way, before anything is deleted: when it returns `false`, the folder is renamed back + /// and this fails. + pub(crate) fn remove_plugin_storage(&self, id: &str, forget: impl FnOnce() -> bool) -> Result<(), StorageError> { + let not_forgotten = || StorageError::Io(GRANT_NOT_REMOVED.to_string()); vectorcraft_plugins::registry::set_storage(id, None); - let Some(dir) = &self.plugin_dir else { return Ok(()) }; + let Some(dir) = &self.plugin_dir else { + return if forget() { Ok(()) } else { Err(not_forgotten()) }; + }; let _held = PLUGIN_DATA_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let path = plugin_data_dir(dir, id); // Every storage handed out for the folder, including those runs still hold, is closed once @@ -528,6 +542,9 @@ impl Session { let data = dir.join(PLUGIN_DATA_DIR); if is_link(&data) { log::warn!("{} is a link: the plug-in settings and cache it leads to are left", data.display()); + if !forget() { + return Err(not_forgotten()); + } close(); return Ok(()); } @@ -540,6 +557,14 @@ impl Session { Err(e) => return Err(StorageError::Io(format!("{}: {e}", path.display()))), Ok(()) => {} } + if !forget() { + if std::fs::symlink_metadata(&aside).is_ok() + && let Err(e) = std::fs::rename(&aside, &path) + { + log::warn!("can't move {} back: {e}", aside.display()); + } + return Err(not_forgotten()); + } close(); if std::fs::symlink_metadata(&aside).is_ok() && let Err(e) = std::fs::remove_dir_all(&aside) diff --git a/crates/engine/src/tests/plugins.rs b/crates/engine/src/tests/plugins.rs index 67b4b29c0..4ced0fcb3 100644 --- a/crates/engine/src/tests/plugins.rs +++ b/crates/engine/src/tests/plugins.rs @@ -811,6 +811,133 @@ fn a_failed_store_write_of_a_module_without_permissions_keeps_the_stored_grant() run(&mut s, "plugin.remove", json!({"id": id})); } +/// A module listing no permissions whose id's grant can't be removed from `grants.json` doesn't stay +/// in the store, so the grant and the module it covers stay together. It does stay when the module +/// before it can't be read back, and when the grants file can't be read and so approves nothing. +#[test] +fn a_grant_that_cannot_be_removed_keeps_the_stored_module() { + let folder = crate::tests_links::Folder::new("plugin-store-failed-forget"); + let id = "test.engine.permissions.forgetfail"; + let empty = json!({"objects": []}); + let approved = asking(id, r#"["doc.read"]"#, &empty); + let plain = asking(id, "null", &empty); + let replace = json!({"dataBase64": vectorcraft_format::base64_encode(&plain), "replace": true}); + 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": vectorcraft_format::base64_encode(&approved), "grant": ["doc.read"]})); + let file = s.stored_plugin_file(id).unwrap(); + // An entry this version can't use is written back as it was read: `1e9` as `1000000000.0`, which + // makes the rewritten file larger than the 1 MiB that is read. + 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(); + + run(&mut s, "plugin.install", replace.clone()); + assert_eq!(std::fs::read(&file).unwrap(), approved, "the store gets the module the grant covers back"); + assert_eq!(std::fs::read(&grants).unwrap(), full); + assert!(vectorcraft_plugins::registry::granted(id).is_none(), "the process runs the module that lists nothing"); + + let oversized = vectorcraft_plugins::Limits::default().max_module_bytes as u64 + 1; + 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"); + + std::fs::write(&file, &approved).unwrap(); + std::fs::write(&grants, b"not a list").unwrap(); + run(&mut s, "plugin.install", replace); + assert_eq!(std::fs::read(&file).unwrap(), plain, "a grants file that can't be read approves nothing"); + + std::fs::remove_file(&grants).unwrap(); + run(&mut s, "plugin.remove", json!({"id": id})); +} + +/// `plugin.remove` returns an error and keeps the plug-in when its grant can't be removed from +/// `grants.json`; a second one tries again. A grants file that can't be read doesn't stop it. +#[test] +fn a_grant_that_cannot_be_removed_stops_plugin_remove() { + let folder = crate::tests_links::Folder::new("plugin-store-failed-remove"); + let id = "test.engine.permissions.removefail"; + let approved = asking(id, r#"["doc.read"]"#, &json!({"objects": []})); + let approved_data = vectorcraft_format::base64_encode(&approved); + 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 recorded = std::fs::read(&grants).unwrap(); + // `1e9` is written back as `1000000000.0`, which makes the rewritten file larger than the 1 MiB that is read. + let entry = serde_json::from_slice::>(&recorded).unwrap().remove(0); + let full = format!(r#"[{entry},{{"n":[{}]}}]"#, vec!["1e9"; 200_000].join(",")).into_bytes(); + std::fs::write(&grants, &full).unwrap(); + let data = crate::store::plugin_data_dir(&folder.0, id); + std::fs::create_dir_all(&data).unwrap(); + std::fs::write(data.join("settings.json"), b"{}").unwrap(); + + assert!(s.execute("plugin.remove", &json!({"id": id})).is_err()); + assert!(vectorcraft_plugins::registry::get(id).is_some(), "the plug-in stays installed"); + assert!(vectorcraft_plugins::registry::granted(id).is_some(), "it keeps its grant"); + assert_eq!(std::fs::read(&file).unwrap(), approved, "the store keeps the module the grant covers"); + assert_eq!(std::fs::read(&grants).unwrap(), full); + assert!(data.join("settings.json").is_file(), "it keeps its settings"); + + // Settings that can't be moved aside leave the grant recorded. A file at the aside path stops + // the move on Unix, and a file held open in the folder stops it on Windows. + std::fs::write(&grants, &recorded).unwrap(); + let mut aside = data.clone().into_os_string(); + aside.push(".removed"); + let aside = std::path::PathBuf::from(aside); + std::fs::write(&aside, b"in the way").unwrap(); + #[cfg(windows)] + let held = { + use std::os::windows::fs::OpenOptionsExt; + std::fs::File::options().read(true).share_mode(1).open(data.join("settings.json")).unwrap() + }; + assert!(s.execute("plugin.remove", &json!({"id": id})).is_err()); + #[cfg(windows)] + drop(held); + assert_eq!(std::fs::read(&grants).unwrap(), recorded, "the grant stays while the settings do"); + assert!(vectorcraft_plugins::registry::get(id).is_some()); + let _ = std::fs::remove_file(&aside); + + run(&mut s, "plugin.remove", json!({"id": id})); + assert!(vectorcraft_plugins::registry::get(id).is_none()); + assert!(!grants.exists(), "the second removal forgets the grant"); + + run(&mut s, "plugin.install", json!({"dataBase64": approved_data, "grant": ["doc.read"]})); + std::fs::write(&grants, b"not a list").unwrap(); + run(&mut s, "plugin.remove", json!({"id": id})); + assert!(vectorcraft_plugins::registry::get(id).is_none(), "a grants file that can't be read approves nothing"); + assert!(!std::path::Path::new(&file).exists()); +} + +/// Windows keeps a file another program holds open without delete sharing, so the last grant can't +/// be removed by deleting `grants.json`. +#[cfg(windows)] +#[test] +fn a_grants_file_that_cannot_be_deleted_keeps_the_stored_module() { + use std::os::windows::fs::OpenOptionsExt; + const FILE_SHARE_READ: u32 = 1; + let folder = crate::tests_links::Folder::new("plugin-store-held-grants"); + let id = "test.engine.permissions.heldgrants"; + let empty = json!({"objects": []}); + let approved = asking(id, r#"["doc.read"]"#, &empty); + 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": vectorcraft_format::base64_encode(&approved), "grant": ["doc.read"]})); + let file = s.stored_plugin_file(id).unwrap(); + + let held = std::fs::File::options().read(true).share_mode(FILE_SHARE_READ).open(&grants).unwrap(); + run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&plain), "replace": true})); + drop(held); + assert_eq!(std::fs::read(&file).unwrap(), approved, "the store gets the module the grant covers back"); + assert!(grants.exists()); + + run(&mut s, "plugin.remove", json!({"id": id})); +} + /// A plug-in folder that leaves a module listing no permissions under an id forgets the process's /// grant for it, unless a later file in the folder leaves a granted module there. A grant kept in /// the plug-in store stays, with the stored module it covers. @@ -1893,7 +2020,7 @@ fn plugin_storage_is_capped_and_survives_a_damaged_settings_file() { assert_eq!(st.set_cache_entry("tile", b"again", m), Err(StorageError::Unavailable)); assert!(!dir.exists()); assert_eq!(upper.setting("k0", m).unwrap(), Some(b"upper".to_vec())); - s.remove_plugin_storage(upper_id).unwrap(); + s.remove_plugin_storage(upper_id, || true).unwrap(); } /// Files larger than the storage reads are not read: a settings file larger than 1 MiB, although diff --git a/docs/mcp.md b/docs/mcp.md index 7f009244a..75e3ce4f1 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -6570,7 +6570,8 @@ an error naming the permissions needed. The grant is kept with the module's hash same bytes install again without `grant`, and a module with other bytes has `needsApproval: true` and needs a new `grant` even when it lists the same permissions. `plugin.install` does not check that a person approved `grant`: it trusts the client that sends it. `plugin.remove` forgets the grant, and so does installing a module that lists no -permissions under that id: the earlier bytes then need a new `grant`. A +permissions under that id: the earlier bytes then need a new `grant`. When `plugin.remove` can't remove the grant from +the plug-in store, it returns an error and the plug-in stays installed. A filter without `doc.write` or `state` that returns output needing them gets an error and the document is unchanged. A plug-in of API level 2 (`api: 2` in its entry) that lists `progress` may report its progress, see Cancel and write log lines: `notifications/cancelled` makes its `vc_cancelled` return 1, and its log lines go to the server's log diff --git a/docs/plugins.md b/docs/plugins.md index dc3adea40..97789dd41 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -181,10 +181,14 @@ capability model proposed for PhotoCraft plug-ins fails as well, the folder holds the new module with the old grant, the grant does not cover that module, and the plug-in is skipped at the next start until it is approved again. A grant whose hash is not 64 lower-case hexadecimal digits covers nothing. `plugin.remove` forgets the grant, and so does a successful `plugin.install` of a - module that lists no permissions under that id, so the earlier bytes need a new approval. A refused install leaves - the grant as it is. When such an install can't write the module to the `Plug-ins` folder, the module already stored - there stays and `grants.json` keeps its grant. 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, + module that lists no permissions under that id, so the earlier bytes need a new approval. When `plugin.remove` can't + remove the grant from `grants.json`, it returns an error, the plug-in stays installed, and a second `plugin.remove` + tries again. A refused install leaves + the grant as it is. When such an install 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` that can't be read grants nothing, and the new module stays in the folder then + too. 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, 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 From 63a483492b4f372fb3472062f91f61278f62d3f3 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 8 Nov 2026 18:59:22 -0800 Subject: [PATCH 07/13] Plug-ins: a grants file that can't be read keeps the grant it may hold A grants file that can't be opened or read may name the old module once it reads again. plugin.remove and an install of a module listing no permissions now fail on it and keep the module. A file over 1 MiB or one that doesn't hold a list still approves nothing and doesn't stop them. --- crates/engine/src/cmd/plugin.rs | 10 +++--- crates/engine/src/store.rs | 49 ++++++++++++++++++++++-------- crates/engine/src/tests/plugins.rs | 43 +++++++++++++++++++++++--- docs/plugins.md | 14 +++++---- 4 files changed, 89 insertions(+), 27 deletions(-) diff --git a/crates/engine/src/cmd/plugin.rs b/crates/engine/src/cmd/plugin.rs index 03c9f97ab..213b167af 100644 --- a/crates/engine/src/cmd/plugin.rs +++ b/crates/engine/src/cmd/plugin.rs @@ -621,8 +621,9 @@ 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 can't be read - // approves nothing, and the new module stays. + // gets the module it held back, when that could be read. 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; @@ -837,8 +838,9 @@ fn remove(s: &mut Session, p: &Value) -> Result { let installed = registry::get(id).is_some(); // Settings and cache the store keeps for an id that isn't installed are deleted too. let stored = !installed && vectorcraft_plugins::manifest::valid_id(id) && s.has_plugin_storage(id); - // Installed again, it asks again. A grant that can't be removed still approves the module; a - // grants file that can't be read approves nothing and doesn't stop the removal. + // Installed again, it asks again. A grant that can't be removed still approves the module, and + // so may a grants file that can't be opened or read. One that is too large or doesn't hold a + // list approves nothing and doesn't stop the removal. let forget = || s.store_grant(id, None) || !s.holds_plugin_grant(id); // Its settings and cache are moved aside and its grant removed before anything is deleted: when // either can't be done, the plug-in stays installed, as it was, and a second `plugin.remove` diff --git a/crates/engine/src/store.rs b/crates/engine/src/store.rs index 6ebdea6a7..92a6507de 100644 --- a/crates/engine/src/store.rs +++ b/crates/engine/src/store.rs @@ -111,15 +111,21 @@ fn list_files(dir: &Path, ext: &str) -> Vec { /// The content of `path` when it is at most `max` bytes; the file is never read past `max`. pub(crate) fn read_capped(path: &Path, max: u64) -> Result, String> { - let file = std::fs::File::open(path).map_err(|e| e.to_string())?; - let mut bytes = vec![]; - file.take(max + 1).read_to_end(&mut bytes).map_err(|e| e.to_string())?; + let bytes = read_past_cap(path, max).map_err(|e| e.to_string())?; if bytes.len() as u64 > max { return Err(too_large(max)); } Ok(bytes) } +/// The first `max + 1` bytes of `path` at most, so that a result longer than `max` shows the file is +/// over its cap. +fn read_past_cap(path: &Path, max: u64) -> std::io::Result> { + let mut bytes = vec![]; + std::fs::File::open(path)?.take(max + 1).read_to_end(&mut bytes)?; + Ok(bytes) +} + /// What a file over its cap is told (a detail after the file's name, in the log or an error). fn too_large(max: u64) -> String { format!("larger than {max} bytes") @@ -458,7 +464,7 @@ impl Session { 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()); + log::warn!("not updating the stored plug-in grants {}: {}", dir.join(GRANTS_FILE).display(), e.why); return false; } }; @@ -488,10 +494,14 @@ impl Session { } } - /// Whether the store holds a grant this version can use for the plug-in `id` (`false` without a - /// folder, and when the grants file can't be read). + /// Whether the store holds, or may hold, a grant this version can use for the plug-in `id`. + /// `true` when the grants file can't be opened or read, because it may name `id` once it reads + /// again; `false` without a folder, and when the file is too large or doesn't hold a list. pub(crate) fn holds_plugin_grant(&self, id: &str) -> bool { - self.plugin_dir.as_deref().is_some_and(|dir| stored_grants(dir).contains_key(id)) + self.plugin_dir.as_deref().is_some_and(|dir| match read_grants(dir) { + Ok(file) => file.grants.contains_key(id), + Err(e) => e.unread, + }) } /// Remove the plug-in `id` from the store. @@ -901,16 +911,29 @@ struct GrantsFile { other: Vec, } +/// Why the grants file of a plug-in store gave no grants. +struct GrantsError { + /// Opening or reading the file failed, so what it holds is unknown. When `false`, the file is + /// too large or doesn't hold a list, and approves nothing. + unread: bool, + /// The detail for a log record. + why: String, +} + /// The grants file of the plug-in store `dir`: empty when there is none, an error when it is too /// large, can't be read or doesn't hold a list. An entry that doesn't parse as a grant, or names an /// invalid id, is kept in [`GrantsFile::other`] with a log record. -fn read_grants(dir: &Path) -> Result { +fn read_grants(dir: &Path) -> Result { let path = dir.join(GRANTS_FILE); - if !path.exists() { - return Ok(GrantsFile::default()); + let bytes = match read_past_cap(&path, MAX_GRANTS_FILE) { + Ok(bytes) => bytes, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(GrantsFile::default()), + Err(e) => return Err(GrantsError { unread: true, why: e.to_string() }), + }; + if bytes.len() as u64 > MAX_GRANTS_FILE { + return Err(GrantsError { unread: false, why: too_large(MAX_GRANTS_FILE) }); } - let entries = - read_capped(&path, MAX_GRANTS_FILE).and_then(|bytes| serde_json::from_slice::>(&bytes).map_err(|e| e.to_string()))?; + let entries = serde_json::from_slice::>(&bytes).map_err(|e| GrantsError { unread: false, why: e.to_string() })?; let mut file = GrantsFile::default(); let mut first_error = None; for entry in entries { @@ -938,7 +961,7 @@ fn stored_grants(dir: &Path) -> BTreeMap file.grants, Err(e) => { - log::warn!("skipping the stored plug-in grants {}: {e}", dir.join(GRANTS_FILE).display()); + log::warn!("skipping the stored plug-in grants {}: {}", dir.join(GRANTS_FILE).display(), e.why); BTreeMap::new() } } diff --git a/crates/engine/src/tests/plugins.rs b/crates/engine/src/tests/plugins.rs index 4ced0fcb3..e367708a5 100644 --- a/crates/engine/src/tests/plugins.rs +++ b/crates/engine/src/tests/plugins.rs @@ -813,7 +813,7 @@ fn a_failed_store_write_of_a_module_without_permissions_keeps_the_stored_grant() /// A module listing no permissions whose id's grant can't be removed from `grants.json` doesn't stay /// in the store, so the grant and the module it covers stay together. It does stay when the module -/// before it can't be read back, and when the grants file can't be read and so approves nothing. +/// before it can't be read back, and when the grants file doesn't hold a list and so approves nothing. #[test] fn a_grant_that_cannot_be_removed_keeps_the_stored_module() { let folder = crate::tests_links::Folder::new("plugin-store-failed-forget"); @@ -846,14 +846,49 @@ fn a_grant_that_cannot_be_removed_keeps_the_stored_module() { std::fs::write(&file, &approved).unwrap(); std::fs::write(&grants, b"not a list").unwrap(); run(&mut s, "plugin.install", replace); - assert_eq!(std::fs::read(&file).unwrap(), plain, "a grants file that can't be read approves nothing"); + assert_eq!(std::fs::read(&file).unwrap(), plain, "a grants file that isn't a list approves nothing"); std::fs::remove_file(&grants).unwrap(); run(&mut s, "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. +#[test] +fn a_grants_file_that_cannot_be_read_keeps_the_grant_and_the_module_it_covers() { + let folder = crate::tests_links::Folder::new("plugin-store-unread-grants"); + let id = "test.engine.permissions.unreadgrants"; + let empty = json!({"objects": []}); + let approved = asking(id, r#"["doc.read"]"#, &empty); + 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": vectorcraft_format::base64_encode(&approved), "grant": ["doc.read"]})); + let file = s.stored_plugin_file(id).unwrap(); + let recorded = std::fs::read(&grants).unwrap(); + std::fs::remove_file(&grants).unwrap(); + std::fs::create_dir(&grants).unwrap(); + + run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&plain), "replace": true})); + assert_eq!(std::fs::read(&file).unwrap(), approved, "the store gets the module the grant covers back"); + + run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&approved), "grant": ["doc.read"]})); + let refused = s.execute("plugin.remove", &json!({"id": id})).unwrap_err().to_string(); + assert!(refused.contains(crate::store::GRANT_NOT_REMOVED), "{refused}"); + assert!(vectorcraft_plugins::registry::get(id).is_some(), "the plug-in stays installed"); + assert_eq!(std::fs::read(&file).unwrap(), approved, "the store keeps the module"); + + std::fs::remove_dir(&grants).unwrap(); + std::fs::write(&grants, &recorded).unwrap(); + run(&mut s, "plugin.remove", json!({"id": id})); + assert!(vectorcraft_plugins::registry::get(id).is_none()); + assert!(!grants.exists(), "the file reads again and the grant goes"); +} + /// `plugin.remove` returns an error and keeps the plug-in when its grant can't be removed from -/// `grants.json`; a second one tries again. A grants file that can't be read doesn't stop it. +/// `grants.json`; a second one tries again. A grants file that doesn't hold a list doesn't stop it. #[test] fn a_grant_that_cannot_be_removed_stops_plugin_remove() { let folder = crate::tests_links::Folder::new("plugin-store-failed-remove"); @@ -907,7 +942,7 @@ fn a_grant_that_cannot_be_removed_stops_plugin_remove() { run(&mut s, "plugin.install", json!({"dataBase64": approved_data, "grant": ["doc.read"]})); std::fs::write(&grants, b"not a list").unwrap(); run(&mut s, "plugin.remove", json!({"id": id})); - assert!(vectorcraft_plugins::registry::get(id).is_none(), "a grants file that can't be read approves nothing"); + assert!(vectorcraft_plugins::registry::get(id).is_none(), "a grants file that isn't a list approves nothing"); assert!(!std::path::Path::new(&file).exists()); } diff --git a/docs/plugins.md b/docs/plugins.md index adbd128cc..178bfd927 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -214,12 +214,14 @@ capability model proposed for PhotoCraft plug-ins hexadecimal digits covers nothing. `plugin.remove` forgets the grant, and so does a successful `plugin.install` of a module that lists no permissions under that id, so the earlier bytes need a new approval. When `plugin.remove` can't remove the grant from `grants.json`, it returns an error, the plug-in stays installed, and a second `plugin.remove` - tries again. A refused install leaves - the grant as it is. When such an install 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` that can't be read grants nothing, and the new module stays in the folder then - too. 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, + 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, 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 From 93f98257e43ec009ed40b5458c430a498372a6d4 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 8 Nov 2026 19:27:47 -0800 Subject: [PATCH 08/13] Plug-ins: test a grants file over its size limit and the unread case The unread grants test replaces the plain module it installed earlier, as plugin.install now asks. A grants file over 1 MiB approves nothing, so the new module stays in the store. --- crates/engine/src/tests/plugins.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/tests/plugins.rs b/crates/engine/src/tests/plugins.rs index b46add1dc..34f3be01c 100644 --- a/crates/engine/src/tests/plugins.rs +++ b/crates/engine/src/tests/plugins.rs @@ -1433,7 +1433,8 @@ fn a_failed_store_write_of_a_module_without_permissions_keeps_the_stored_grant() /// A module listing no permissions whose id's grant can't be removed from `grants.json` doesn't stay /// in the store, so the grant and the module it covers stay together. It does stay when the module -/// before it can't be read back, and when the grants file doesn't hold a list and so approves nothing. +/// before it can't be read back, and when the grants file is too large or doesn't hold a list and so +/// approves nothing. #[test] fn a_grant_that_cannot_be_removed_keeps_the_stored_module() { let folder = crate::tests_links::Folder::new("plugin-store-failed-forget"); @@ -1465,9 +1466,14 @@ fn a_grant_that_cannot_be_removed_keeps_the_stored_module() { std::fs::write(&file, &approved).unwrap(); std::fs::write(&grants, b"not a list").unwrap(); - run(&mut s, "plugin.install", replace); + run(&mut s, "plugin.install", replace.clone()); assert_eq!(std::fs::read(&file).unwrap(), plain, "a grants file that isn't a list approves nothing"); + std::fs::write(&file, &approved).unwrap(); + std::fs::write(&grants, vec![b' '; (1 << 20) + 1]).unwrap(); + run(&mut s, "plugin.install", replace); + assert_eq!(std::fs::read(&file).unwrap(), plain, "a grants file over its size limit approves nothing"); + std::fs::remove_file(&grants).unwrap(); run(&mut s, "plugin.remove", json!({"id": id})); } @@ -1494,7 +1500,7 @@ fn a_grants_file_that_cannot_be_read_keeps_the_grant_and_the_module_it_covers() run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&plain), "replace": true})); assert_eq!(std::fs::read(&file).unwrap(), approved, "the store gets the module the grant covers back"); - run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&approved), "grant": ["doc.read"]})); + run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&approved), "grant": ["doc.read"], "replace": true})); let refused = s.execute("plugin.remove", &json!({"id": id})).unwrap_err().to_string(); assert!(refused.contains(crate::store::GRANT_NOT_REMOVED), "{refused}"); assert!(vectorcraft_plugins::registry::get(id).is_some(), "the plug-in stays installed"); From 623d8fdd83094b8a1edcbdb4334bf6822a8aff7b Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 8 Nov 2026 19:37:01 -0800 Subject: [PATCH 09/13] Plug-ins: the settings file test passes remove_plugin_storage its callback --- crates/engine/src/tests/plugins.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/src/tests/plugins.rs b/crates/engine/src/tests/plugins.rs index afd7535e7..d1d9cc63e 100644 --- a/crates/engine/src/tests/plugins.rs +++ b/crates/engine/src/tests/plugins.rs @@ -2864,7 +2864,7 @@ fn removing_a_plugin_drops_its_settings_from_memory() { let mut m = Meter::unlimited(); assert_eq!(st.setting("zoom", &mut m).unwrap(), Some(b"hello".to_vec())); assert_eq!(m.used(), bytes.len() as u64, "read from the file"); - s.remove_plugin_storage(id).unwrap(); + s.remove_plugin_storage(id, || true).unwrap(); } /// The cache's total stays in memory while the cache folder keeps its time; a write and a clear From 6cce39dd7f884e1bcc27fe268abbd37a6767c469 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 8 Nov 2026 20:38:03 -0800 Subject: [PATCH 10/13] 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 | 14 ++-- crates/engine/src/lib.rs | 4 ++ crates/engine/src/session/documents.rs | 1 + crates/engine/src/store.rs | 52 +++++++++++--- crates/engine/src/tests/plugins.rs | 95 ++++++++++++++++++++++++++ docs/plugins.md | 14 ++-- 6 files changed, 163 insertions(+), 17 deletions(-) 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 From 7177438feb4beb3168f3d23e1c46a8043384469d Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 8 Nov 2026 21:26:13 -0800 Subject: [PATCH 11/13] Plug-ins: plugin.install's params text and docs/mcp.md state when a grant is kept A refused install keeps the grant, a failed write of the module to the plug-in store leaves the grant in the store, and a stored module that is disabled or not read in Safe Mode keeps the grant for its id. This matches docs/plugins.md. --- crates/engine/src/cmd/plugin.rs | 2 +- docs/mcp.md | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/plugin.rs b/crates/engine/src/cmd/plugin.rs index de864f8bd..9cad35cdf 100644 --- a/crates/engine/src/cmd/plugin.rs +++ b/crates/engine/src/cmd/plugin.rs @@ -942,7 +942,7 @@ pub fn specs() -> Vec { "Install Plug-in", [], None, - "{path: a .wasm file (desktop) | dataBase64: the module (+ name?: shown as its source), replace?: bool (true once the user agreed: a module with other bytes than the plug-in installed with its id, or with none installed, one whose id has settings or a cache in the plug-in store, installs only with it, takes that plug-in's place and gets its settings and cache; false: one whose id is installed is refused; left out, the same module installs again), 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; installing a module that lists no permissions forgets the grant held for its id; 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 once the user agreed: a module with other bytes than the plug-in installed with its id, or with none installed, one whose id has settings or a cache in the plug-in store, installs only with it, takes that plug-in's place and gets its settings and cache; false: one whose id is installed is refused; left out, the same module installs again), 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; installing a module that lists no permissions forgets the grant held for its id; a refused install keeps the grant, a failed write of the module to the plug-in store leaves the grant in the store, and a stored module that is disabled or not read in Safe Mode keeps the grant for its id; 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 ), diff --git a/docs/mcp.md b/docs/mcp.md index 99df46c6a..6b044edb5 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -6710,7 +6710,9 @@ an error naming the permissions needed. The grant is kept with the module's hash same bytes install again without `grant`, and a module with other bytes has `needsApproval: true` and needs a new `grant` even when it lists the same permissions. `plugin.install` does not check that a person approved `grant`: it trusts the client that sends it. `plugin.remove` forgets the grant, and so does installing a module that lists no -permissions under that id: the earlier bytes then need a new `grant`. When `plugin.remove` can't remove the grant from +permissions under that id: the earlier bytes then need a new `grant`. A refused install keeps the grant. A failed write +of the module to the plug-in store leaves the grant in the store. A stored module that is disabled, or not read in Safe +Mode, keeps the grant for its id. When `plugin.remove` can't remove the grant from the plug-in store, it returns an error and the plug-in stays installed. A filter without `doc.write` or `state` that returns output needing them gets an error and the document is unchanged. A plug-in of API level 2 (`api: 2` in its entry) that lists `progress` may report its progress, see Cancel and write From 65ff9df84995c80ad6595498f5105f2792c53108 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 8 Nov 2026 22:00:26 -0800 Subject: [PATCH 12/13] Plug-ins: a grant left behind stops counting when its module can't go back plugin.install and plugin.remove count an entry of grants.json that this version can't use when it can't be removed. plugin.install stops counting a grant it can't remove when the old module can't be written back to the Plug-ins folder. --- crates/engine/src/cmd/plugin.rs | 16 +++---- crates/engine/src/store.rs | 47 +++++++++++++-------- crates/engine/src/tests/plugins.rs | 68 ++++++++++++++++++++++++++++++ docs/plugins.md | 8 ++-- 4 files changed, 110 insertions(+), 29 deletions(-) diff --git a/crates/engine/src/cmd/plugin.rs b/crates/engine/src/cmd/plugin.rs index 9cad35cdf..5985c43d9 100644 --- a/crates/engine/src/cmd/plugin.rs +++ b/crates/engine/src/cmd/plugin.rs @@ -640,18 +640,16 @@ 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; 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. + // gets the module it held back. When that module can't be read or written back, or there + // was none, no module goes with the grant, and 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) && s.holds_plugin_grant(&m.id) { - if !matches!(previous, Ok(Some(_))) { + let back = previous.as_ref().is_ok_and(|held| s.put_back_plugin(plugin.id(), held.as_deref())); + stored = !back; + if !back || matches!(previous, Ok(None)) { 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/store.rs b/crates/engine/src/store.rs index 5738290d9..ae98bdf4f 100644 --- a/crates/engine/src/store.rs +++ b/crates/engine/src/store.rs @@ -176,6 +176,12 @@ fn lock_disabled(_: &Path) -> Option { None } +#[cfg(test)] +thread_local! { + /// Tests: [`Session::put_back_plugin`] fails on this thread, as on a full volume. + pub(crate) static PUT_BACK_FAILS: std::cell::Cell = const { std::cell::Cell::new(false) }; +} + /// Remove `dir/file` → whether it is gone; a file that isn't there is what was wanted. fn take(dir: &Path, file: &str) -> bool { let path = dir.join(file); @@ -363,9 +369,7 @@ impl Session { for id in &asks_nothing { vectorcraft_plugins::registry::revoke(id); 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())) - } + Ok(file) => file.names(id), Err(e) => e.unread, }; if named && !self.store_grant(id, None) && self.holds_plugin_grant(id) { @@ -458,12 +462,15 @@ impl Session { } /// 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]>) { + /// write ([`Session::stored_plugin_bytes`]): with `None`, the file is removed. → whether the + /// store now holds `previous`. + pub(crate) fn put_back_plugin(&self, id: &str, previous: Option<&[u8]>) -> bool { + #[cfg(test)] + if PUT_BACK_FAILS.get() { + return false; + } match previous { - Some(bytes) => { - self.store_plugin(id, bytes); - } + Some(bytes) => self.store_plugin(id, bytes), None => self.unstore_plugin(id), } } @@ -531,21 +538,20 @@ impl Session { } } - /// Whether the store holds, or may hold, a grant this version can use for the plug-in `id`. - /// `true` when the grants file can't be opened or read, because it may name `id` once it reads - /// again; `false` without a folder, and when the file is too large or doesn't hold a list. + /// Whether the store holds, or may hold, an entry for the plug-in `id`, including one this + /// version can't use ([`GrantsFile::names`]). `true` when the grants file can't be opened or + /// read, because it may name `id` once it reads again; `false` without a folder, and when the + /// file is too large or doesn't hold a list. pub(crate) fn holds_plugin_grant(&self, id: &str) -> bool { self.plugin_dir.as_deref().is_some_and(|dir| match read_grants(dir) { - Ok(file) => file.grants.contains_key(id), + Ok(file) => file.names(id), Err(e) => e.unread, }) } - /// Remove the plug-in `id` from the store. - pub(crate) fn unstore_plugin(&self, id: &str) { - if let Some(dir) = &self.plugin_dir { - take(dir, &format!("{}.{PLUGIN_EXT}", file_stem(id))); - } + /// Remove the plug-in `id` from the store → whether its file is gone (`false` without a folder). + pub(crate) fn unstore_plugin(&self, id: &str) -> bool { + self.plugin_dir.as_deref().is_some_and(|dir| take(dir, &format!("{}.{PLUGIN_EXT}", file_stem(id)))) } /// Keep the settings and cache of plug-in `id` in the attached plug-in store from now on; without @@ -1042,6 +1048,13 @@ struct GrantsFile { other: Vec, } +impl GrantsFile { + /// Whether the file has an entry for the plug-in `id`, usable by this version or not. + fn names(&self, id: &str) -> bool { + self.grants.contains_key(id) || self.other.iter().any(|e| e.get("id").and_then(serde_json::Value::as_str) == Some(id)) + } +} + /// Why the grants file of a plug-in store gave no grants. struct GrantsError { /// Opening or reading the file failed, so what it holds is unknown. When `false`, the file is diff --git a/crates/engine/src/tests/plugins.rs b/crates/engine/src/tests/plugins.rs index 83b57a6f6..08ed19db1 100644 --- a/crates/engine/src/tests/plugins.rs +++ b/crates/engine/src/tests/plugins.rs @@ -1517,6 +1517,74 @@ 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 stops counting for the session when the module it covers can't be +/// written back either: the store holds the new module, and the old bytes would otherwise install +/// without approval. +#[test] +fn a_grant_left_behind_when_its_module_cannot_go_back_stops_counting() { + let folder = crate::tests_links::Folder::new("plugin-store-failed-put-back"); + let id = "test.engine.permissions.failedputback"; + 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(); + // `1e9` is written back as `1000000000.0`, which makes the rewritten file larger than the 1 MiB that is read. + 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(); + + crate::store::PUT_BACK_FAILS.set(true); + run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&plain), "replace": true})); + crate::store::PUT_BACK_FAILS.set(false); + assert_eq!(std::fs::read(&file).unwrap(), plain, "the store keeps the new module"); + assert_eq!(std::fs::read(&grants).unwrap(), full); + assert_eq!(run(&mut s, "plugin.inspect", json!({"dataBase64": approved_data}))["needsApproval"], true, "the grant left behind stops counting"); + let refused = s.execute("plugin.install", &json!({"dataBase64": approved_data, "replace": true})).unwrap_err().to_string(); + assert!(refused.contains("doc.read"), "{refused}"); + + std::fs::remove_file(&grants).unwrap(); + run(&mut s, "plugin.remove", json!({"id": id})); +} + +/// An entry of `grants.json` that this version can't use is kept for a version that can, so when it +/// can't be removed, neither `plugin.remove` nor a replacing module gets past it. +#[test] +fn an_unusable_entry_that_cannot_be_removed_stops_plugin_remove() { + let folder = crate::tests_links::Folder::new("plugin-store-failed-remove-other"); + let id = "test.engine.permissions.removeother"; + let empty = json!({"objects": []}); + let first = asking(id, "null", &empty); + let second = asking(id, "[]", &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": vectorcraft_format::base64_encode(&first)})); + let file = s.stored_plugin_file(id).unwrap(); + let entry = json!({"id": id, "hash": "0".repeat(64), "permissions": ["no.such.permission"]}); + // `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(); + + let refused = s.execute("plugin.remove", &json!({"id": id})).unwrap_err().to_string(); + assert!(refused.contains(crate::store::GRANT_NOT_REMOVED), "{refused}"); + assert!(vectorcraft_plugins::registry::get(id).is_some(), "the plug-in stays installed"); + assert_eq!(std::fs::read(&file).unwrap(), first, "the store keeps the module"); + assert_eq!(std::fs::read(&grants).unwrap(), full); + + run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&second), "replace": true})); + assert_eq!(std::fs::read(&file).unwrap(), first, "the store gets its module back"); + + std::fs::write(&grants, serde_json::to_vec(&json!([entry])).unwrap()).unwrap(); + run(&mut s, "plugin.remove", json!({"id": id})); + assert!(vectorcraft_plugins::registry::get(id).is_none()); + assert!(!grants.exists(), "the removal takes the entry out of the file"); +} + /// 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] diff --git a/docs/plugins.md b/docs/plugins.md index d5f4e8f4a..f197f96ac 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -218,8 +218,9 @@ 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, 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 + removed and the module already stored can't be read back or written 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 @@ -232,7 +233,8 @@ capability model proposed for PhotoCraft plug-ins id, the process forgets its own grant for that id, and a grant kept in `grants.json` stays with the stored module it covers. 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, replaced by a module that lists no permissions, or - removed. A `grants.json` that is not a JSON list is never overwritten, and the file is never written larger than + removed. When such an entry can't be removed from the file, `plugin.install` and `plugin.remove` treat it as a grant + that can't be removed. A `grants.json` that is not a JSON list is never overwritten, and the file is never written larger than 1 MiB, the most that is read. When a `Plug-ins` folder is in use, only the grants kept in it count for `plugin.install`, `plugin.inspect`, `plugin.reload` and the Additional Plug-ins Folder. At startup, a stored plug-in is skipped when no grant covers its bytes and the permissions it lists. The Additional Plug-ins From 004805172ac4100321dfc79a761cb08e0d41d842 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 8 Nov 2026 23:00:11 -0800 Subject: [PATCH 13/13] Docs: cite the Safe Mode regression for the plug-in grant entry --- docs/target-app-parity.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 331bef556..4a6da699f 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1582,7 +1582,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - **Plug-in permissions (G17):** a manifest may list `permissions` (`doc.read`, `doc.write`, `state`, `effect.render`, `export.write`; the first three follow storytold/photocraft#662) and an `api` level; without the list a plug-in gets what its kind implies and installs as before. Install Plug-in… and File › Open show the name, id, version, file SHA-256 and each permission in plain words, with control and bidirectional characters escaped, and install nothing on Cancel; for a plug-in that lists permissions, `plugin.install` needs `grant` and folders install it only when it is granted. The grant is kept with the module's hash, a module asking for more asks again, and a filter run refuses output its permissions don't allow. Regressions: `crates/plugins/src/permissions.rs`, `crates/plugins/src/manifest.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 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. 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. -- **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`. 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. - **Plug-in progress, Cancel and log lines (G17):** at plug-in API level 2 a plug-in with the `progress` permission imports `env.vc_progress`, `env.vc_cancelled` and `env.vc_log`. A heavy filter's and an export's progress windows show its progress as a percentage bar, their Cancel reaches it through `vc_cancelled`, and its log lines go to the app's log with its id, capped per line and per run. Installing checks every import and names the one it refuses. Regressions: `crates/plugins/src/host.rs`, `crates/plugins/tests/abi.rs`, `crates/engine/src/tests/plugins.rs`, `crates/engine/tests/plugin_fuzz.rs`, `crates/ui-egui/src/tests/plugin_runs.rs`. Scores and hours unchanged. - **Live-effect plug-ins paint their output (G17, #121):** a live effect whose manifest has `"api": 2` and `"paintOutput": true` returns pieces, each drawn by itself in order with fills and strokes of its own in the filter's paint forms (a piece without them keeps the object's; an empty list paints none; an effect on a fill returns only fills, one on a stroke only strokes). The whole result is checked first: limits, numbers in range with nothing clamped, known keys only, and the swatches and patterns it names in the document; one fault draws that step of the stack unchanged, keeps the effect record and puts the plug-in's name and the field in `plugin.info`'s `lastError`. The pieces are drawn, measured and exported to PDF and SVG from the appearance stack without changing the document, and Expand Appearance writes them as ordinary paths in one undo step. Effects without the key behave as before. Regressions: `crates/plugins/src/painted/tests.rs`, `crates/plugins/src/manifest.rs`, `crates/engine/src/tests/plugins.rs`, `crates/engine/tests/plugin_fuzz.rs`. Scores and hours unchanged.