From c42d44cb2fc2e04dcaf04253d6f5bfa8b66a0264 Mon Sep 17 00:00:00 2001 From: kshitij <82637225+kshitijk4poor@users.noreply.github.com> Date: Wed, 24 Jun 2026 20:16:54 +0530 Subject: [PATCH] revert(plugins): restore user dashboard plugin backend API auto-import (#43719) (#51950) * Revert "refactor(security): centralize non-bundled plugin sources in one constant" This reverts commit e2bea0abe6aae9dd1e9ff275c9240093c0d03245. * Revert "fix(security): restrict dashboard plugin backend import to bundled plugins (#43719)" This reverts commit 8845f3316c26732cb758d7f7300b9dbf83ef2728. --- hermes_cli/web_server.py | 63 ++++--------- plugins/hermes-achievements/README.md | 13 +-- .../test_project_plugin_rce_bypass.py | 94 +------------------ tests/hermes_cli/test_web_server.py | 22 +++-- .../docs/reference/environment-variables.md | 2 +- .../features/extending-the-dashboard.md | 27 ++---- 6 files changed, 47 insertions(+), 174 deletions(-) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 3c38717e01c..2f98c0ffbda 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -12277,20 +12277,12 @@ def _safe_plugin_api_relpath(api_field: Any, *, dashboard_dir: Path) -> Optional return api_field -# Plugin sources whose Python backend (dashboard manifest `api` file) must NEVER -# be auto-imported by the dashboard web server — only bundled plugins may. Shared -# by the discovery-time scrub and the mount-time refuse guards so a typo in one -# site cannot silently disable a security gate (GHSA-5qr3-c538-wm9j / #43719). -_NON_BUNDLED_PLUGIN_SOURCES = frozenset({"user", "project"}) - - def _discover_dashboard_plugins() -> list: """Scan plugins/*/dashboard/manifest.json for dashboard extensions. - Checks three plugin sources. Bundled dashboard plugins win name conflicts - so non-bundled plugins cannot shadow trusted backend-capable routes: - 1. Bundled plugins: /plugins//dashboard/manifest.json (memory/, etc.) - 2. User plugins: ~/.hermes/plugins//dashboard/manifest.json + Checks three plugin sources (same as hermes_cli.plugins): + 1. User plugins: ~/.hermes/plugins//dashboard/manifest.json + 2. Bundled plugins: /plugins//dashboard/manifest.json (memory/, etc.) 3. Project plugins: ./.hermes/plugins/ (only if HERMES_ENABLE_PROJECT_PLUGINS) """ plugins = [] @@ -12299,9 +12291,9 @@ def _discover_dashboard_plugins() -> list: from hermes_cli.plugins import get_bundled_plugins_dir bundled_root = get_bundled_plugins_dir() search_dirs = [ + (get_hermes_home() / "plugins", "user"), (bundled_root / "memory", "bundled"), (bundled_root, "bundled"), - (get_hermes_home() / "plugins", "user"), ] # GHSA-5qr3-c538-wm9j (#29156): the previous ``os.environ.get(...)`` # check treated *any* non-empty string as truthy, so ``=0``, ``=false``, @@ -12360,20 +12352,10 @@ def _discover_dashboard_plugins() -> list: raw_api = data.get("api") dashboard_dir = child / "dashboard" safe_api = _safe_plugin_api_relpath(raw_api, dashboard_dir=dashboard_dir) - if source in _NON_BUNDLED_PLUGIN_SOURCES and safe_api: - _log.warning( - "Plugin %s: refusing dashboard backend api=%s " - "(only bundled plugins may auto-import Python " - "backend routes; non-bundled plugins may extend " - "the dashboard with static UI assets only)", - name, safe_api, - ) - safe_api = None - raw_api = None if raw_api and safe_api is None: _log.warning( "Plugin %s: refusing unsafe api path %r (must be a " - "relative file inside a bundled plugin's dashboard/ " + "relative file inside the plugin's dashboard/ " "directory); backend routes from this plugin will " "not be mounted", name, raw_api, @@ -12780,36 +12762,23 @@ def _mount_plugin_api_routes(): a ``router`` (FastAPI APIRouter). Routes are mounted under ``/api/plugins//``. - Backend import is restricted to bundled plugins. User and project - plugins can extend the dashboard UI via static JS/CSS, but their - Python ``api`` files are never auto-imported by the web server. - See GHSA-5qr3-c538-wm9j (#29156) and #43719. + Backend import is restricted to ``bundled`` and ``user`` sources. + Project plugins (``./.hermes/plugins/``) ship with the CWD and are + therefore attacker-controlled in any threat model where the user + opens a malicious repo; they can extend the dashboard UI via + static JS/CSS but their Python ``api`` file is never auto-imported + by the web server. See GHSA-5qr3-c538-wm9j (#29156). """ for plugin in _get_dashboard_plugins(): api_file_name = plugin.get("_api_file") if not api_file_name: continue - source = plugin.get("source") - if source in _NON_BUNDLED_PLUGIN_SOURCES: - # Backend Python auto-import is reserved for bundled plugins; user - # and project plugins extend the dashboard with static UI assets - # only (GHSA-5qr3-c538-wm9j / #43719). Defence-in-depth: discovery - # already nulls _api_file for these sources, but re-refusing here — - # at the actual importlib call site — keeps the import primitive - # contained even if a future caller or a tampered cache entry slips - # a non-bundled plugin through with an _api_file set. - _reason = { - "user": ( - "user-installed plugins may not auto-import Python code" - ), - "project": ( - "project plugins may not auto-import Python code; backend " - "auto-import is reserved for bundled plugins" - ), - }.get(source, "only bundled plugins may auto-import Python code") + if plugin.get("source") == "project": _log.warning( - "Plugin %s: ignoring backend api=%s (%s)", - plugin["name"], api_file_name, _reason, + "Plugin %s: ignoring backend api=%s (project plugins may " + "not auto-import Python code; move the plugin to " + "~/.hermes/plugins/ if you trust it)", + plugin["name"], api_file_name, ) continue dashboard_dir = Path(plugin["_dir"]) diff --git a/plugins/hermes-achievements/README.md b/plugins/hermes-achievements/README.md index b2deb52d397..2c1ed638281 100644 --- a/plugins/hermes-achievements/README.md +++ b/plugins/hermes-achievements/README.md @@ -77,9 +77,7 @@ Then rescan dashboard plugins: curl http://127.0.0.1:9119/api/dashboard/plugins/rescan ``` -When installed as a user plugin, the dashboard UI loads but Python backend API -routes are not auto-imported. Backend routes are available when this plugin is -bundled with Hermes. +If backend API routes 404, restart `hermes dashboard`; plugin APIs are mounted at dashboard startup. ## Updating @@ -91,11 +89,7 @@ git pull --ff-only curl http://127.0.0.1:9119/api/dashboard/plugins/rescan ``` -For a user-installed plugin at `~/.hermes/plugins/hermes-achievements`, a plugin -rescan is enough because Python backend routes are not auto-imported. If you -update the bundled plugin by pulling changes in the hermes-agent repository, and -that bundled plugin update changes backend routes or `plugin_api.py`, restart -`hermes dashboard` after pulling. +If the update changes backend routes or `plugin_api.py`, restart `hermes dashboard` after pulling. As of 2026-04-29, updating is strongly recommended because scan performance changed significantly: - removed duplicate `/overview` scan path @@ -124,9 +118,6 @@ dashboard/ ## API -These backend routes are mounted for the bundled plugin. User-installed copies -load their dashboard UI but do not auto-import Python backend routes. - Routes are mounted under: ```text diff --git a/tests/hermes_cli/test_project_plugin_rce_bypass.py b/tests/hermes_cli/test_project_plugin_rce_bypass.py index fa3457b1ed0..1e12b47eb9d 100644 --- a/tests/hermes_cli/test_project_plugin_rce_bypass.py +++ b/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 user/project-source plugin backend code outright. + refuses project-source plugins 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_user_safe_api_path_is_scrubbed(self, user_plugin_factory, tmp_path): + def test_safe_api_path_survives(self, user_plugin_factory, tmp_path): user_plugin_factory("safe", { "name": "safe", "label": "Safe", @@ -230,86 +230,6 @@ 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 @@ -356,16 +276,6 @@ 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] diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index 76ba0e5f488..3ebe6a05c1d 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -5095,8 +5095,14 @@ 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): - """Create TestClients with and without the session token header.""" + 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``. + """ try: from starlette.testclient import TestClient except ImportError: @@ -5121,15 +5127,19 @@ class TestPluginAPIAuth: def test_plugin_route_allows_auth(self): """Plugin API routes should work with a valid session token. - Uses a bundled plugin route so the test covers authenticated plugin - API access without relying on user-installed plugin backend imports. + 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. """ # Without auth: middleware blocks before reaching the handler. - resp = self.client.get("/api/plugins/kanban/board") + resp = self.client.get("/api/plugins/example/hello") assert resp.status_code == 401 # With auth: handler runs. - resp = self.auth_client.get("/api/plugins/kanban/board") + resp = self.auth_client.get("/api/plugins/example/hello") assert resp.status_code == 200 def test_plugin_post_requires_auth(self): diff --git a/website/docs/reference/environment-variables.md b/website/docs/reference/environment-variables.md index 31a8c0f1c28..3387c80c70d 100644 --- a/website/docs/reference/environment-variables.md +++ b/website/docs/reference/environment-variables.md @@ -625,7 +625,7 @@ Advanced per-platform knobs for throttling the outbound message batcher. Most us | `HERMES_AGENT_NOTIFY_INTERVAL` | Gateway: interval in seconds between progress notifications on long-running agent turns. | | `HERMES_CHECKPOINT_TIMEOUT` | Timeout for filesystem checkpoint creation in seconds (default: `30`). | | `HERMES_EXEC_ASK` | Enable execution approval prompts in gateway mode (`true`/`false`) | -| `HERMES_ENABLE_PROJECT_PLUGINS` | Enable auto-discovery of repo-local plugins from `./.hermes/plugins/` for both the agent loader and the dashboard web server. Accepts the standard truthy set: `1` / `true` / `yes` / `on` (case-insensitive). Everything else — including `0`, `false`, `no`, `off`, and the empty string — is treated as **disabled** (default). Note: as of GHSA-5qr3-c538-wm9j (#29156) and #43719, the dashboard web server refuses to auto-import Python `api` files from project or user-installed plugins — they may extend the UI via static JS/CSS, while backend routes are reserved for bundled plugins. | +| `HERMES_ENABLE_PROJECT_PLUGINS` | Enable auto-discovery of repo-local plugins from `./.hermes/plugins/` for both the agent loader and the dashboard web server. Accepts the standard truthy set: `1` / `true` / `yes` / `on` (case-insensitive). Everything else — including `0`, `false`, `no`, `off`, and the empty string — is treated as **disabled** (default). Note: as of GHSA-5qr3-c538-wm9j (#29156) the dashboard web server refuses to auto-import a project plugin's Python `api` file even when this var is enabled — project plugins may extend the UI via static JS/CSS but their backend routes are only loaded when moved under `~/.hermes/plugins/`. | | `HERMES_PLUGINS_DEBUG` | `1`/`true` to surface verbose plugin-discovery logs on stderr — directories scanned, manifests parsed, skip reasons, and full tracebacks on parse or `register()` failure. Aimed at plugin authors. | | `HERMES_BACKGROUND_NOTIFICATIONS` | Background process notification mode in gateway: `all` (default), `result`, `error`, `off` | | `HERMES_EPHEMERAL_SYSTEM_PROMPT` | Ephemeral system prompt injected at API-call time (never persisted to sessions) | diff --git a/website/docs/user-guide/features/extending-the-dashboard.md b/website/docs/user-guide/features/extending-the-dashboard.md index b0119495174..79b84a73efb 100644 --- a/website/docs/user-guide/features/extending-the-dashboard.md +++ b/website/docs/user-guide/features/extending-the-dashboard.md @@ -431,14 +431,14 @@ If you prefer JSX, use any bundler (esbuild, Vite, rollup) with React as an exte ├── dist/ │ ├── index.js # required — pre-built JS bundle (IIFE) │ └── style.css # optional — custom CSS - └── plugin_api.py # bundled plugins only — backend API routes (FastAPI) + └── plugin_api.py # optional — backend API routes (FastAPI) ``` A single plugin directory can carry three orthogonal extensions: - `plugin.yaml` + `__init__.py` — CLI/gateway plugin ([see plugins page](./plugins)). - `dashboard/manifest.json` + `dashboard/dist/index.js` — dashboard UI plugin. -- `dashboard/plugin_api.py` — bundled plugins only; backend API routes. +- `dashboard/plugin_api.py` — dashboard backend routes. None of them are required; include only the layers you need. @@ -743,10 +743,7 @@ Routes are mounted under `/api/plugins//`, so the above becomes: - `GET /api/plugins/my-plugin/data` - `POST /api/plugins/my-plugin/action` -Security notes: - -- Bundled plugin API routes bypass session-token authentication. The dashboard server binds to localhost by default, which mitigates the risks of this bypass. -- User-installed and project dashboard plugins may still extend the UI with static JS/CSS, but their Python `api` files are not auto-imported by the dashboard server. Backend routes are reserved for bundled plugins. +Plugin API routes bypass session-token authentication since the dashboard server binds to localhost by default. **Don't expose the dashboard on a public interface with `--host 0.0.0.0` if you run untrusted plugins** — their routes become reachable too. #### Accessing Hermes internals @@ -807,14 +804,11 @@ The dashboard scans three directories for `dashboard/manifest.json`: | Priority | Directory | Source label | |----------|-----------|--------------| -| 1 (wins on conflict) | `/plugins/memory//dashboard/` | `bundled` | -| 1 (wins on conflict) | `/plugins//dashboard/` | `bundled` | -| 2 | `~/.hermes/plugins//dashboard/` | `user` | +| 1 (wins on conflict) | `~/.hermes/plugins//dashboard/` | `user` | +| 2 | `/plugins/memory//dashboard/` | `bundled` | +| 2 | `/plugins//dashboard/` | `bundled` | | 3 | `./.hermes/plugins//dashboard/` | `project` — only when `HERMES_ENABLE_PROJECT_PLUGINS` is set | -Bundled dashboard plugins win name conflicts because only bundled plugins may -register backend routes. Give user and project dashboard plugins unique names. - Discovery results are cached per dashboard process. After adding a new plugin, either: ```bash @@ -914,11 +908,10 @@ Check that the file is in `~/.hermes/dashboard-themes/` and ends in `.yaml` or ` The `sidebar` slot only renders when the active theme has `layoutVariant: cockpit`. Other slots always render. If you're registering into a slot with no hits, add `console.log` inside `registerSlot` to confirm the plugin bundle ran at all. **Plugin backend routes return 404.** -1. Confirm the plugin is bundled with Hermes. User-installed and project dashboard plugins can extend the UI, but their Python backend routes are not auto-imported. -2. Confirm the manifest has `"api": "plugin_api.py"` pointing to an existing file inside `dashboard/`. -3. Restart `hermes dashboard` — plugin API routes are mounted once at startup, **not** on rescan. -4. Check that `plugin_api.py` exports a module-level `router = APIRouter()`. Other export names are not picked up. -5. Tail `~/.hermes/logs/errors.log` for `Failed to load plugin API routes` — import errors are logged there. +1. Confirm the manifest has `"api": "plugin_api.py"` pointing to an existing file inside `dashboard/`. +2. Restart `hermes dashboard` — plugin API routes are mounted once at startup, **not** on rescan. +3. Check that `plugin_api.py` exports a module-level `router = APIRouter()`. Other export names are not picked up. +4. Tail `~/.hermes/logs/errors.log` for `Failed to load plugin API routes` — import errors are logged there. **Theme change drops my color overrides.** `colorOverrides` are scoped to the active theme and cleared on theme switch — that's by design. If you want overrides that persist, put them in your theme's YAML, not in the live switcher.