From e2bea0abe6aae9dd1e9ff275c9240093c0d03245 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Mon, 22 Jun 2026 22:48:37 +0530 Subject: [PATCH] refactor(security): centralize non-bundled plugin sources in one constant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /simplify-code (LOW, flagged by two reviewers): the source tags 'user' / 'project' / 'bundled' were bare string literals scattered across the discovery scrub and the two mount-time refuse guards. A typo in any one site (e.g. 'users') would SILENTLY disable a security gate with no error — the exact failure mode this RCE boundary must not have. Introduce a shared module-level _NON_BUNDLED_PLUGIN_SOURCES frozenset referenced by both the discovery scrub and the (now single) mount guard, so the auto-import policy lives in one place. The two mount guards collapse into one gate that still emits the distinct per-source operator message via a map (no loss of guidance). Behavior unchanged: 39 RCE-bypass tests pass, and the constant is mutation-checked (typo'ing it fails the bypass tests). Defence-in-depth (discovery scrub + mount refuse) is retained intentionally. --- hermes_cli/web_server.py | 41 +++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index ece4620f05e..63ea7c5e06b 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -12178,6 +12178,13 @@ 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. @@ -12254,7 +12261,7 @@ 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 {"user", "project"} and safe_api: + 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 " @@ -12683,19 +12690,27 @@ def _mount_plugin_api_routes(): api_file_name = plugin.get("_api_file") if not api_file_name: continue - if plugin.get("source") == "user": + 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") _log.warning( - "Plugin %s: ignoring backend api=%s (user-installed " - "plugins may not auto-import Python code)", - plugin["name"], api_file_name, - ) - continue - if plugin.get("source") == "project": - _log.warning( - "Plugin %s: ignoring backend api=%s (project plugins may " - "not auto-import Python code; backend auto-import is " - "reserved for bundled plugins)", - plugin["name"], api_file_name, + "Plugin %s: ignoring backend api=%s (%s)", + plugin["name"], api_file_name, _reason, ) continue dashboard_dir = Path(plugin["_dir"])