From bad23b542a95ac3551a397f53b454577a2a43085 Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Wed, 2 Sep 2026 09:22:03 -0400 Subject: [PATCH] fix(store): land the install_from_url gate, which never reached main #510 shows as merged, but into fix/gate-git-pull-updates -- #508's branch -- rather than main. #508 reached main first, so the sideload gate was left behind on a branch. Same failure as plugins #350/#351, which merged into each other's bases; worth knowing the pattern, because GitHub reports these as MERGED and `gh pr list` shows nothing outstanding. main today has two of the three routes gated: install_plugin (#431/#433) and update_plugin's git branch (#508). install_from_url validates required manifest fields and then installs whatever it found, never comparing the core version. Cherry-picked unchanged from the orphaned branch -- it applies to main with no conflict. TestSideloadGate pins the three cases the other routes pin: refuses a floor above this core leaving nothing behind, still allows a compatible plugin (the guard against a gate that refuses everything), and does not block a 2.0.0 floor on a core reporting an untrustworthy version. Full suite 3725 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 # --------------------------------------------------------------------------