fix(store): uninstall must refuse a plugin's framework, not just odd names
apple / swift (push) Successful in 1m26s
apple / screenshots (push) Successful in 6m47s
ci / web (push) Successful in 1m9s
ci / docs-site (push) Successful in 1m5s
windows-host / package (push) Successful in 9m45s
android / android (push) Successful in 12m4s
arch / build-publish (push) Successful in 12m4s
decky / build-publish (push) Successful in 20s
docker / build-push (--build-arg FEDORA_VERSION=44, ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora44-rpm) (push) Successful in 10s
docker / build-push (., web/Dockerfile, punktfunk-web) (push) Successful in 40s
docker / build-push (ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora-rpm) (push) Successful in 9s
docker / build-push (ci, ci/rust-ci-noble.Dockerfile, punktfunk-rust-ci-noble) (push) Successful in 8s
docker / build-push (ci, ci/rust-ci.Dockerfile, punktfunk-rust-ci) (push) Successful in 10s
docker / build-push (docs-site, docs-site/Dockerfile, punktfunk-docs) (push) Successful in 10s
ci / bench (push) Successful in 5m57s
docker / deploy-docs (push) Successful in 24s
deb / build-publish (push) Successful in 9m0s
deb / build-publish-host (push) Successful in 9m32s
rpm / build-publish (43, bazzite, punktfunk-fedora-rpm) (push) Successful in 15m15s
ci / rust (push) Successful in 24m40s
rpm / build-publish (44, fedora-44, punktfunk-fedora44-rpm) (push) Successful in 19m55s
apple / swift (push) Successful in 1m26s
apple / screenshots (push) Successful in 6m47s
ci / web (push) Successful in 1m9s
ci / docs-site (push) Successful in 1m5s
windows-host / package (push) Successful in 9m45s
android / android (push) Successful in 12m4s
arch / build-publish (push) Successful in 12m4s
decky / build-publish (push) Successful in 20s
docker / build-push (--build-arg FEDORA_VERSION=44, ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora44-rpm) (push) Successful in 10s
docker / build-push (., web/Dockerfile, punktfunk-web) (push) Successful in 40s
docker / build-push (ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora-rpm) (push) Successful in 9s
docker / build-push (ci, ci/rust-ci-noble.Dockerfile, punktfunk-rust-ci-noble) (push) Successful in 8s
docker / build-push (ci, ci/rust-ci.Dockerfile, punktfunk-rust-ci) (push) Successful in 10s
docker / build-push (docs-site, docs-site/Dockerfile, punktfunk-docs) (push) Successful in 10s
ci / bench (push) Successful in 5m57s
docker / deploy-docs (push) Successful in 24s
deb / build-publish (push) Successful in 9m0s
deb / build-publish-host (push) Successful in 9m32s
rpm / build-publish (43, bazzite, punktfunk-fedora-rpm) (push) Successful in 15m15s
ci / rust (push) Successful in 24m40s
rpm / build-publish (44, fedora-44, punktfunk-fedora44-rpm) (push) Successful in 19m55s
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 <noreply@anthropic.com>
This commit is contained in:
@@ -534,6 +534,30 @@ pub(crate) async fn uninstall_plugin(ApiJson(req): ApiJson<UninstallRequest>) ->
|
|||||||
if let Err(e) = store::valid_installed_pkg(&req.pkg) {
|
if let Err(e) = store::valid_installed_pkg(&req.pkg) {
|
||||||
return api_error(StatusCode::BAD_REQUEST, &format!("{e:#}"));
|
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) {
|
match jobs::spawn_uninstall(req.pkg) {
|
||||||
Ok(job) => (StatusCode::ACCEPTED, Json(JobRef { job })).into_response(),
|
Ok(job) => (StatusCode::ACCEPTED, Json(JobRef { job })).into_response(),
|
||||||
Err(e) => api_error(StatusCode::CONFLICT, &format!("{e:#}")),
|
Err(e) => api_error(StatusCode::CONFLICT, &format!("{e:#}")),
|
||||||
|
|||||||
@@ -571,6 +571,38 @@ mod tests {
|
|||||||
assert!(!dir.path().join("bunfig.toml").exists());
|
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]
|
#[test]
|
||||||
fn plugin_id_derivation() {
|
fn plugin_id_derivation() {
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
|
|||||||
Reference in New Issue
Block a user