From 664ef4111634d63e0ee5613ddaf046d5f9218022 Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Mon, 31 Aug 2026 18:37:41 -0400 Subject: [PATCH] fix(store): gate install_from_url, the last ungated route in Sideloading a plugin from a URL validated required manifest fields, warned on version-field and schema problems, and then installed whatever it had found -- never once comparing the core version. With this, all three routes in refuse on the same rule: install_plugin (#431/#433), the git-pull update path (the commit before this one), and now sideloading. The argument for gating it is not the same as for the other two, and worth stating. An automatic store update is something that happens TO a user; a sideload is something they chose. But the floor is not advice about intent -- it is a statement that the plugin cannot run on this core. Letting it through produces exactly the silent PluginState.ERROR at load that the store gate exists to prevent, and someone who pasted a URL is no better placed to diagnose that than someone who pressed Update. Checked after the download, like the others, because the manifest is the only place the floor is written. Placed before the move into plugins/, so the existing `finally` removes the temp tree and nothing half-installed survives a refusal. The reason string is returned to the caller, so the web UI shows the same actionable message the store path already gives. TestSideloadGate pins the three cases the other routes pin: refuses a floor above this core and leaves nothing behind, still allows a compatible plugin (the guard against a gate that refuses everything and passes the first test), and does not block a 2.0.0 floor on a core reporting an untrustworthy version -- the v3.1.0-reports-1.0.0 population, who must not be locked out. TestInstallGate's docstring said this route was still ungated. It no longer is, so it now points at TestSideloadGate rather than describing a gap. Verified the tests bite: removing the check fails the refusal case. Full suite 3723 passed, 6 skipped. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 --- src/plugin_system/store_manager.py | 20 +++++++ test/test_plugin_compatibility_gate.py | 72 +++++++++++++++++++++++++- 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/src/plugin_system/store_manager.py b/src/plugin_system/store_manager.py index 623270e3..80a454fc 100644 --- a/src/plugin_system/store_manager.py +++ b/src/plugin_system/store_manager.py @@ -1602,6 +1602,26 @@ def install_from_url(self, repo_url: str, plugin_id: str = None, plugin_path: st 'error': f'Manifest missing required fields: {", ".join(missing_fields)}' } + # Refuse a plugin that needs a newer core than this one, exactly as + # _install_plugin_impl does after its download. Sideloading is an + # explicit act rather than an automatic store update, but the floor + # is not advice about intent -- it is a statement that the plugin + # cannot run here, and letting it through produces the same silent + # PluginState.ERROR at load. This was the last of the three routes + # in that skipped the check. + # + # Before the move, so the `finally` below removes the temp tree and + # nothing half-installed is left behind. + from src import __version__ as core_version + from src.plugin_system import compatibility + + compatible, reason = compatibility.check(manifest, core_version) + if not compatible: + self.logger.error( + "Refusing to install %s from %s: %s", + plugin_id, repo_url, reason) + return {'success': False, 'error': reason} + # Validate version fields consistency (warnings only, not required) validation_errors = self._validate_manifest_version_fields(manifest) if validation_errors: diff --git a/test/test_plugin_compatibility_gate.py b/test/test_plugin_compatibility_gate.py index 91a84b8e..9ab858a9 100644 --- a/test/test_plugin_compatibility_gate.py +++ b/test/test_plugin_compatibility_gate.py @@ -155,8 +155,9 @@ class TestInstallGate: rediscovering it. `update_plugin` also has a git branch that pulls in place and never re-downloads; that one is gated separately by `_gate_pulled_commit` and pinned in `TestGitPullGate` below. A third - route, `install_from_url`, is still ungated — sideloading from a URL - checks required fields but not the floor. + route, `install_from_url` (sideloading from a URL), is gated in that + function and pinned in `TestSideloadGate`. All three refuse on the same + rule. """ def _install_with_manifest(self, store, manifest, core_version, monkeypatch): @@ -217,6 +218,73 @@ def test_untrustworthy_core_does_not_block_installs(self, store, monkeypatch): assert (path / "manifest.json").exists() +class TestSideloadGate: + """`install_from_url` never looked at the core version. + + Sideloading is an explicit act rather than an automatic store update, so + the argument for gating it is different: not "the user did not choose + this", but that the floor states the plugin *cannot run here*. Letting it + through produces the same silent PluginState.ERROR at load that the store + gate exists to prevent, and the user who typed the URL is no better placed + to diagnose it than one who pressed Update. + """ + + def _sideload(self, store, manifest, core_version, monkeypatch): + mgr, plugins_dir = store + plugin_id = manifest["id"] + + def fake_clone(repo_url, target_path, branches=None): + target_path.mkdir(parents=True, exist_ok=True) + (target_path / "manifest.json").write_text( + json.dumps(manifest), encoding="utf-8") + (target_path / "manager.py").write_text( + "class P: pass\n", encoding="utf-8") + return "main" + + monkeypatch.setattr(mgr, "_install_via_git", fake_clone) + monkeypatch.setattr(mgr, "_install_dependencies", lambda *a, **k: True) + + import src + monkeypatch.setattr(src, "__version__", core_version) + return mgr.install_from_url("https://example.invalid/plugin"), \ + plugins_dir / plugin_id + + def test_refuses_a_plugin_that_needs_a_newer_core(self, store, monkeypatch): + manifest = { + "id": "sideload-newer", "name": "Sideload", "class_name": "P", + "display_modes": ["a"], "min_ledmatrix_version": "9.9.9", + } + result, path = self._sideload(store, manifest, "3.2.0", monkeypatch) + + assert result["success"] is False + assert "9.9.9" in result["error"], result["error"] + assert not path.exists(), ( + "a refused sideload must not leave the plugin installed") + + def test_allows_a_compatible_plugin(self, store, monkeypatch): + """The guard against over-refusing: a gate that blocks everything + passes the test above and breaks sideloading entirely.""" + manifest = { + "id": "sideload-fine", "name": "Sideload", "class_name": "P", + "display_modes": ["a"], "min_ledmatrix_version": "3.0.0", + } + result, path = self._sideload(store, manifest, "3.2.0", monkeypatch) + + assert result["success"] is True, result.get("error") + assert (path / "manifest.json").exists() + + def test_untrustworthy_core_does_not_block_a_2_0_0_floor( + self, store, monkeypatch): + """Same rule as the other two routes: a v3.1.0 release reports 1.0.0, + and nearly every published manifest floors at 2.0.0.""" + manifest = { + "id": "sideload-floored", "name": "Sideload", "class_name": "P", + "display_modes": ["a"], "versions": [{"ledmatrix_min": "2.0.0"}], + } + result, _ = self._sideload(store, manifest, "1.0.0", monkeypatch) + + assert result["success"] is True, result.get("error") + # -------------------------------------------------------------------------- # The git-pull update path — the one route that does not re-download # --------------------------------------------------------------------------