Merge pull request #50773 from NousResearch/salvage/43719-dashboard-plugin-rce

fix(security): restrict dashboard plugin backend auto-import to bundled plugins — defense-in-depth (#43719)

tests/hermes_cli/test_project_plugin_rce_bypass.py

@@ -24,7 +24,7 @@ These tests pin each layer of the new defence:
 * ``_safe_plugin_api_relpath`` rejects absolute paths, ``..``
   traversal, and non-string / empty values.
 * ``_mount_plugin_api_routes`` re-validates at import time and
-  refuses project-source plugins outright.
+  refuses user/project-source plugin backend code outright.
 * End-to-end the original PoC manifest no longer triggers
   ``importlib`` for ``/tmp/payload.py``.
 """
@@ -216,7 +216,7 @@ class TestDiscoveryScrubsApiField:
         assert entry["_api_file"] is None
         assert entry["has_api"] is False
 
-    def test_safe_api_path_survives(self, user_plugin_factory, tmp_path):
+    def test_user_safe_api_path_is_scrubbed(self, user_plugin_factory, tmp_path):
         user_plugin_factory("safe", {
             "name": "safe",
             "label": "Safe",
@@ -230,6 +230,86 @@ class TestDiscoveryScrubsApiField:
         )
         plugins = web_server._get_dashboard_plugins(force_rescan=True)
         entry = next(p for p in plugins if p["name"] == "safe")
+        assert entry["_api_file"] is None
+        assert entry["has_api"] is False
+
+    def test_project_safe_api_path_is_scrubbed(self, tmp_path, monkeypatch):
+        monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home"))
+        (tmp_path / "home").mkdir()
+        monkeypatch.setenv("HERMES_ENABLE_PROJECT_PLUGINS", "1")
+        cwd = tmp_path / "project"
+        cwd.mkdir()
+        monkeypatch.chdir(cwd)
+        dashboard = _write_plugin_manifest(
+            cwd / ".hermes" / "plugins",
+            "safe-project",
+            {
+                "name": "safe-project",
+                "label": "Safe Project",
+                "api": "api.py",
+                "entry": "dist/index.js",
+            },
+        )
+        (dashboard / "api.py").write_text("router = None\n")
+
+        plugins = web_server._get_dashboard_plugins(force_rescan=True)
+        entry = next(p for p in plugins if p["name"] == "safe-project")
+        assert entry["_api_file"] is None
+        assert entry["has_api"] is False
+
+    def test_bundled_safe_api_path_survives(self, tmp_path, monkeypatch):
+        hermes_home = tmp_path / "home"
+        monkeypatch.setenv("HERMES_HOME", str(hermes_home))
+        hermes_home.mkdir()
+        monkeypatch.setenv("HERMES_BUNDLED_PLUGINS", str(tmp_path / "bundled"))
+        dashboard = _write_plugin_manifest(
+            tmp_path / "bundled",
+            "safe-bundled",
+            {
+                "name": "safe-bundled",
+                "label": "Safe Bundled",
+                "api": "api.py",
+                "entry": "dist/index.js",
+            },
+        )
+        (dashboard / "api.py").write_text("router = None\n")
+
+        plugins = web_server._get_dashboard_plugins(force_rescan=True)
+        entry = next(p for p in plugins if p["name"] == "safe-bundled")
+        assert entry["_api_file"] == "api.py"
+        assert entry["has_api"] is True
+
+    def test_user_plugin_does_not_shadow_bundled_backend(self, tmp_path, monkeypatch):
+        hermes_home = tmp_path / "home"
+        monkeypatch.setenv("HERMES_HOME", str(hermes_home))
+        hermes_home.mkdir()
+        monkeypatch.setenv("HERMES_BUNDLED_PLUGINS", str(tmp_path / "bundled"))
+
+        bundled_dashboard = _write_plugin_manifest(
+            tmp_path / "bundled",
+            "shadowed",
+            {
+                "name": "shadowed",
+                "label": "Bundled Shadowed",
+                "api": "api.py",
+                "entry": "dist/index.js",
+            },
+        )
+        (bundled_dashboard / "api.py").write_text("router = None\n")
+        _write_plugin_manifest(
+            hermes_home / "plugins",
+            "shadowed",
+            {
+                "name": "shadowed",
+                "label": "User Shadowed",
+                "api": "api.py",
+                "entry": "dist/index.js",
+            },
+        )
+
+        plugins = web_server._get_dashboard_plugins(force_rescan=True)
+        entry = next(p for p in plugins if p["name"] == "shadowed")
+        assert entry["source"] == "bundled"
         assert entry["_api_file"] == "api.py"
         assert entry["has_api"] is True
 
@@ -276,6 +356,16 @@ class TestMountApiRoutesRefusesUntrusted:
             "GHSA-5qr3-c538-wm9j defence-in-depth regression"
         )
 
+    def test_user_source_api_is_not_imported(self, tmp_path):
+        plugin = self._payload_plugin(tmp_path, source="user")
+        web_server._dashboard_plugins_cache = [plugin]
+        with patch("importlib.util.spec_from_file_location") as spec:
+            web_server._mount_plugin_api_routes()
+        assert spec.call_count == 0, (
+            "user-installed plugin api file was imported — "
+            "third-party dashboard plugin backend code must stay inert"
+        )
+
     def test_bundled_source_api_imports_normally(self, tmp_path):
         plugin = self._payload_plugin(tmp_path, source="bundled")
         web_server._dashboard_plugins_cache = [plugin]

tests/hermes_cli/test_web_server.py

@@ -5070,14 +5070,8 @@ class TestPluginAPIAuth:
     """Tests that plugin API routes require the session token (issue #19533)."""
 
     @pytest.fixture(autouse=True)
-    def _setup_test_client(self, monkeypatch, _isolate_hermes_home, _install_example_plugin):
-        """Create a TestClient without the session token header.
-
-        Pulls in ``_install_example_plugin`` so ``test_plugin_route_allows_auth``
-        has the ``/api/plugins/example/hello`` endpoint available — the
-        example plugin is no longer a bundled plugin, so the fixture
-        installs it into the per-test ``HERMES_HOME``.
-        """
+    def _setup_test_client(self, monkeypatch, _isolate_hermes_home):
+        """Create TestClients with and without the session token header."""
         try:
             from starlette.testclient import TestClient
         except ImportError:
@@ -5102,19 +5096,15 @@ class TestPluginAPIAuth:
     def test_plugin_route_allows_auth(self):
         """Plugin API routes should work with a valid session token.
 
-        Uses ``/api/plugins/example/hello`` from the example-dashboard
-        test fixture (installed into HERMES_HOME by the class-level
-        ``_install_example_plugin`` fixture) — a stable, side-effect-free
-        GET that's only loaded for tests. With a valid token the handler
-        should run (200); without one the middleware should 401 before
-        the handler is reached.
+        Uses a bundled plugin route so the test covers authenticated plugin
+        API access without relying on user-installed plugin backend imports.
         """
         # Without auth: middleware blocks before reaching the handler.
-        resp = self.client.get("/api/plugins/example/hello")
+        resp = self.client.get("/api/plugins/kanban/board")
         assert resp.status_code == 401
 
         # With auth: handler runs.
-        resp = self.auth_client.get("/api/plugins/example/hello")
+        resp = self.auth_client.get("/api/plugins/kanban/board")
         assert resp.status_code == 200
 
     def test_plugin_post_requires_auth(self):