From 04c975e9cbaa37511e0476d0c14eb25f6aa2cbe7 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 21 Jul 2026 00:10:49 +0200 Subject: [PATCH] fix(store): uninstall must refuse a plugin's framework, not just odd names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard checked the package NAME shape, and `@punktfunk/plugin-kit` — the framework every kit-built plugin depends on — matches `@scope/plugin-*` exactly. Windows on-glass accepted an uninstall of it; bun no-ops removing a non-dependency so nothing broke, but the store was offering to pull a library out from under the plugins using it. Require membership in the top-level dependency list, the same authority that already keeps plugin-kit out of the installed listing. Same blind spot, two call sites — the listing was fixed during implementation, this one wasn't. Co-Authored-By: Claude Fable 5 --- crates/punktfunk-host/src/mgmt/store.rs | 24 +++++++++++++++++++ crates/punktfunk-host/src/store.rs | 32 +++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/crates/punktfunk-host/src/mgmt/store.rs b/crates/punktfunk-host/src/mgmt/store.rs index 21902a9f..849aa81b 100644 --- a/crates/punktfunk-host/src/mgmt/store.rs +++ b/crates/punktfunk-host/src/mgmt/store.rs @@ -534,6 +534,30 @@ pub(crate) async fn uninstall_plugin(ApiJson(req): ApiJson) -> if let Err(e) = store::valid_installed_pkg(&req.pkg) { return api_error(StatusCode::BAD_REQUEST, &format!("{e:#}")); } + // The name shape is not enough. `@punktfunk/plugin-kit` — the framework every kit-built plugin + // *depends on* — matches `@scope/plugin-*` exactly, so a syntactic guard waves it through and + // the store offers to uninstall a library out from under the plugins using it (accepted on + // Windows on-glass before this check existed). Only a package the operator actually installed + // may be removed, which is precisely what `installed_packages` reports: the plugins dir's + // top-level dependencies, transitive ones excluded. + let pkg = req.pkg.clone(); + let known = match blocking(move || { + store::installed_packages(&store::plugins_dir()) + .iter() + .any(|p| p.pkg == pkg) + }) + .await + { + Ok(k) => k, + Err(e) => return e, + }; + if !known { + return api_error( + StatusCode::BAD_REQUEST, + "that package is not an installed plugin — it may be a dependency of one, or already \ + removed", + ); + } match jobs::spawn_uninstall(req.pkg) { Ok(job) => (StatusCode::ACCEPTED, Json(JobRef { job })).into_response(), Err(e) => api_error(StatusCode::CONFLICT, &format!("{e:#}")), diff --git a/crates/punktfunk-host/src/store.rs b/crates/punktfunk-host/src/store.rs index 30279201..b79365d9 100644 --- a/crates/punktfunk-host/src/store.rs +++ b/crates/punktfunk-host/src/store.rs @@ -571,6 +571,38 @@ mod tests { assert!(!dir.path().join("bunfig.toml").exists()); } + /// The name-shape guard is necessary but NOT sufficient — see `mgmt::store::uninstall_plugin`. + /// + /// `@punktfunk/plugin-kit` is a plugin's *framework*, and it satisfies every syntactic rule + /// here. Windows on-glass accepted an uninstall of it. The real gate is membership in + /// [`installed_packages`], which excludes transitive dependencies; this test pins the fact that + /// the shape check alone lets it through, so nobody "simplifies" the handler back. + #[test] + fn name_shape_alone_does_not_protect_a_plugins_framework() { + assert!( + valid_installed_pkg("@punktfunk/plugin-kit").is_ok(), + "shape check passes plugin-kit — the handler must additionally require that the \ + package is a top-level install" + ); + + let dir = tempfile::tempdir().unwrap(); + touch_pkg(dir.path(), "@punktfunk/plugin-rom-manager", "0.3.1"); + touch_pkg(dir.path(), "@punktfunk/plugin-kit", "0.1.3"); + std::fs::write( + dir.path().join("package.json"), + r#"{"dependencies":{"@punktfunk/plugin-rom-manager":"0.3.1"}}"#, + ) + .unwrap(); + let installed = installed_packages(dir.path()); + assert!(installed + .iter() + .any(|p| p.pkg == "@punktfunk/plugin-rom-manager")); + assert!( + !installed.iter().any(|p| p.pkg == "@punktfunk/plugin-kit"), + "the framework is not an installed plugin, so uninstall must refuse it" + ); + } + #[test] fn plugin_id_derivation() { assert_eq!(