diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 33a4f27fcfa..d5d319dda09 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -4047,6 +4047,43 @@ async def set_dashboard_theme(body: ThemeSetBody): # Dashboard plugin system # --------------------------------------------------------------------------- +def _safe_plugin_api_relpath(api_field: Any, *, dashboard_dir: Path) -> Optional[str]: + """Validate the manifest's ``api`` field for the plugin loader. + + The web server later imports this file as a Python module via + ``importlib.util.spec_from_file_location`` (arbitrary code + execution by design — that's how plugins extend the backend). + Pre-#29156 the field was used as-is, which meant: + + * An absolute path swallowed the plugin's dashboard directory + entirely — ``Path('safe/dashboard') / '/tmp/evil.py'`` resolves + to ``/tmp/evil.py``, so any attacker-controlled manifest could + point the import at any Python file on disk (GHSA-5qr3-c538-wm9j). + * A ``../..`` traversal could climb out of the plugin into + neighbouring directories on the search path. + + Return the original string when the resolved path stays under + ``dashboard_dir``; return ``None`` (with a warning logged at the + call site) otherwise so the plugin still loads its static JS/CSS + but its backend ``api`` is rejected. + """ + if not isinstance(api_field, str) or not api_field.strip(): + return None + candidate = Path(api_field) + if candidate.is_absolute(): + return None + try: + resolved = (dashboard_dir / candidate).resolve() + base = dashboard_dir.resolve() + except (OSError, RuntimeError): + return None + try: + resolved.relative_to(base) + except ValueError: + return None + return api_field + + def _discover_dashboard_plugins() -> list: """Scan plugins/*/dashboard/manifest.json for dashboard extensions. @@ -4113,6 +4150,23 @@ def _discover_dashboard_plugins() -> list: slots: List[str] = [] if isinstance(slots_src, list): slots = [s for s in slots_src if isinstance(s, str) and s] + # Validate ``api`` at discovery time so the value cached + # on the plugin entry is already safe to feed into the + # importer. An attacker-controlled manifest can name + # any absolute path or ``..`` traversal here — the + # web server then imports that file as a Python module + # (RCE, GHSA-5qr3-c538-wm9j). + raw_api = data.get("api") + dashboard_dir = child / "dashboard" + safe_api = _safe_plugin_api_relpath(raw_api, dashboard_dir=dashboard_dir) + if raw_api and safe_api is None: + _log.warning( + "Plugin %s: refusing unsafe api path %r (must be a " + "relative file inside the plugin's dashboard/ " + "directory); backend routes from this plugin will " + "not be mounted", + name, raw_api, + ) plugins.append({ "name": name, "label": data.get("label", name), @@ -4123,10 +4177,10 @@ def _discover_dashboard_plugins() -> list: "slots": slots, "entry": data.get("entry", "dist/index.js"), "css": data.get("css"), - "has_api": bool(data.get("api")), + "has_api": bool(safe_api), "source": source, - "_dir": str(child / "dashboard"), - "_api_file": data.get("api"), + "_dir": str(dashboard_dir), + "_api_file": safe_api, }) except Exception as exc: _log.warning("Bad dashboard plugin manifest %s: %s", manifest_file, exc) @@ -4481,12 +4535,42 @@ def _mount_plugin_api_routes(): Each plugin's ``api`` field points to a Python file that must expose a ``router`` (FastAPI APIRouter). Routes are mounted under ``/api/plugins//``. + + 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 - api_path = Path(plugin["_dir"]) / api_file_name + if plugin.get("source") == "project": + _log.warning( + "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"]) + api_path = dashboard_dir / api_file_name + try: + resolved_api = api_path.resolve() + resolved_base = dashboard_dir.resolve() + resolved_api.relative_to(resolved_base) + except (OSError, RuntimeError, ValueError): + # Discovery already filters this, but re-check here in case + # ``_dir`` was tampered with after caching or a future caller + # bypasses the validator. Defence in depth keeps the import + # primitive contained even if the upstream check regresses. + _log.warning( + "Plugin %s: refusing to import api file outside its " + "dashboard directory (%s)", plugin["name"], api_path, + ) + continue if not api_path.exists(): _log.warning("Plugin %s declares api=%s but file not found", plugin["name"], api_file_name) continue