Merge origin/main into bb/gui (2026-05-24)

Bring 313 commits of upstream main into the bb/gui dashboard refactor branch. Eight conflicts resolved by hand, the rest auto-merged. One missing class (_StreamErrorEvent) restored from main after the auto-merger dropped it. Conflict resolutions: apps/dashboard/README.md take HEAD: main's text described the pre-rename web/ layout that bb/gui refactored away. apps/dashboard/package.json combine: keep HEAD's @hermes/shared workspace dep, take main's @nous-research/ui 0.16.0 bump. apps/dashboard/package-lock.json regenerate via npm install --package-lock-only. Root lock also regenerated; only dashboard and apps/desktop entries moved (apps/desktop version 0.0.1 → 0.0.2 to match bb/gui's package.json bump). apps/dashboard/src/pages/ take main (4 hunks): text-xs EnvPage.tsx replaces text-[0.65rem] per the typography rule HEAD's own README documents. hermes_cli/gateway.py take main (2 hunks): Discord setup metadata moved to plugin (architectural migration); s6 service-manager dispatch helpers additive. hermes_cli/main.py combine (2 hunks): take main's Termux-aware _sync_bundled_skills_for_startup; combine gui + portal subcommands in the known-subcommand list. hermes_cli/web_server.py mixed (10 hunks): - take main on _PUBLIC_API_PATHS (bb/gui's own test asserts the rescan endpoint must require auth) - combine WS helpers: keep HEAD's _ws_client_label + main's Host/Origin guard + composing _ws_request_is_allowed - take HEAD's debug-level broadcast drop log (matches the comment "subscriber went away mid-send") - take main's _safe_plugin_api_relpath GHSA-5qr3-c538-wm9j fix and the paired discovery-time validation - take main's {name:path} route converter for plugin visibility tui_gateway/server.py take main: PR #31379's verbose- args gating supersedes HEAD's unconditional args dump on tool.start. Post-merge restoration: run_agent.py restored class _StreamErrorEvent (40 lines, from origin/main:288). Auto-merge silently dropped it, breaking imports in agent/codex_runtime.py and three test files (test_codex_xai_oauth_recovery.py, test_streaming.py). Restored verbatim from main. Sanity checks: * git diff --check / --cached --check: clean (no stray markers) * ast.parse + import on all touched .py files: clean * targeted pytest on resolved files: 756 passed, 1 pre-existing Windows-curses failure unrelated to the merge * full pytest_parallel run: 105 files / 391 failures vs baseline 98 files / 346. Differential vs origin/bb/gui shows all 11 "new" failure files come from main's added tests/code and reproduce identically against origin/main on the same Windows host (pure Windows path-separator / perms / git-bash issues in upstream tests, not merge regressions). 4 baseline failures fixed: 3 in test_codex_xai_oauth_recovery (the _StreamErrorEvent restoration), 1 each in test_pairing, test_runner_startup_failures, test_stream_consumer. * sentinel-token sweep on main's eight largest commits: every audited symbol present in the merged tree at expected counts (TTSProvider 61, NtfyAdapter 29, S6ServiceManager 70, install_bws 12, security_audit 16, register_image_gen_provider 23, list_profile_gateways 22, DISCORD_FREE_RESPONSE_CHANNELS 48, …). * byte-diff sweep: 30/30 sampled main-only-modified files byte-identical to origin/main; the four bb/gui-only files that drifted (i18n/types.ts, i18n/ru.ts, ThemeSwitcher.tsx, ToolCall.tsx) correctly absorbed main's web/ → apps/dashboard/ edits through git's rename detection (main's added lines all present, removed lines all absent).
2026-05-25 00:39:46 -04:00
parent 3adb74269a 7e165e843d
commit e1338265c1
521 changed files with 40613 additions and 3961 deletions
@@ -18,6 +18,7 @@ import json
 import logging
 import os
 import secrets
+import stat
 import subprocess
 import sys
 import tempfile
@@ -51,6 +52,7 @@ from hermes_cli.config import (
    redact_key,
 )
 from gateway.status import get_running_pid, read_runtime_status
+from utils import env_var_enabled

 try:
    from fastapi import FastAPI, HTTPException, Request, WebSocket, WebSocketDisconnect
@@ -141,17 +143,14 @@ app.add_middleware(
 # /api/ is gated by the auth middleware below.  Keep this list minimal —
 # only truly non-sensitive, read-only endpoints belong here.
 # ---------------------------------------------------------------------------
-_PUBLIC_API_PATHS: frozenset = frozenset(
-    {
-        "/api/status",
-        "/api/config/defaults",
-        "/api/config/schema",
-        "/api/model/info",
-        "/api/dashboard/themes",
-        "/api/dashboard/plugins",
-        "/api/dashboard/plugins/rescan",
-    }
-)
+_PUBLIC_API_PATHS: frozenset = frozenset({
+    "/api/status",
+    "/api/config/defaults",
+    "/api/config/schema",
+    "/api/model/info",
+    "/api/dashboard/themes",
+    "/api/dashboard/plugins",
+})


 def _has_valid_session_token(request: Request) -> bool:
@@ -1375,11 +1374,13 @@ _AUX_TASK_SLOTS: Tuple[str, ...] = (
    "vision",
    "web_extract",
    "compression",
-    "session_search",
    "skills_hub",
    "approval",
    "mcp",
    "title_generation",
+    "triage_specifier",
+    "kanban_decomposer",
+    "profile_describer",
    "curator",
 )

@@ -2772,7 +2773,25 @@ def _save_anthropic_oauth_creds(
        "expiresAt": expires_at_ms,
    }
    _HERMES_OAUTH_FILE.parent.mkdir(parents=True, exist_ok=True)
-    _HERMES_OAUTH_FILE.write_text(json.dumps(payload, indent=2), encoding="utf-8")
+    tmp_path = _HERMES_OAUTH_FILE.with_name(
+        f"{_HERMES_OAUTH_FILE.name}.tmp.{os.getpid()}.{secrets.token_hex(8)}"
+    )
+    try:
+        with tmp_path.open("w", encoding="utf-8") as handle:
+            handle.write(json.dumps(payload, indent=2))
+            handle.flush()
+            os.fsync(handle.fileno())
+        os.replace(tmp_path, _HERMES_OAUTH_FILE)
+        try:
+            _HERMES_OAUTH_FILE.chmod(stat.S_IRUSR | stat.S_IWUSR)
+        except OSError:
+            pass
+    finally:
+        try:
+            if tmp_path.exists():
+                tmp_path.unlink()
+        except OSError:
+            pass
    # Best-effort credential-pool insert. Failure here doesn't invalidate
    # the file write — pool registration only matters for the rotation
    # strategy, not for runtime credential resolution.
@@ -4510,19 +4529,11 @@ _VALID_CHANNEL_RE = re.compile(r"^[A-Za-z0-9._-]{1,128}$")
 _LOOPBACK_HOSTS = frozenset({"127.0.0.1", "::1", "localhost", "testclient"})


-def _is_public_bind() -> bool:
-    """True when bound to all-interfaces (operator used --insecure)."""
-    return getattr(app.state, "bound_host", "") in {"0.0.0.0", "::"}
-
-
 def _ws_client_is_allowed(ws: "WebSocket") -> bool:
    """Check if the WebSocket client IP is acceptable.

-    Allows loopback always; allows any IP when bound to all-interfaces
-    (--insecure mode, guarded by session token auth).
+    Allows loopback clients only.
    """
-    if _is_public_bind():
-        return True
    client_host = ws.client.host if ws.client else ""
    if not client_host:
        return True
@@ -4538,6 +4549,39 @@ def _ws_client_label(ws: "WebSocket") -> str:
    return f"{host}:{port}" if port is not None else host


+def _ws_host_origin_is_allowed(ws: "WebSocket") -> bool:
+    """Apply the dashboard Host/Origin guard to WebSocket upgrades.
+
+    FastAPI HTTP middleware does not run for WebSocket routes, so the
+    DNS-rebinding Host check used for normal dashboard HTTP requests must be
+    repeated here before accepting the upgrade.  Browsers also send an Origin
+    header on WebSocket handshakes; when present, require it to target the
+    same bound dashboard host.
+    """
+    bound_host = getattr(app.state, "bound_host", None)
+    if not bound_host:
+        return True
+
+    host_header = ws.headers.get("host", "")
+    if not _is_accepted_host(host_header, bound_host):
+        return False
+
+    origin = ws.headers.get("origin", "")
+    if not origin:
+        return True
+
+    parsed = urllib.parse.urlparse(origin)
+    if parsed.scheme not in {"http", "https"} or not parsed.netloc:
+        return False
+
+    return _is_accepted_host(parsed.netloc, bound_host)
+
+
+def _ws_request_is_allowed(ws: "WebSocket") -> bool:
+    """Return True when the WebSocket upgrade matches dashboard boundaries."""
+    return _ws_host_origin_is_allowed(ws) and _ws_client_is_allowed(ws)
+
+
 # Per-channel subscriber registry used by /api/pub (PTY-side gateway → dashboard)
 # and /api/events (dashboard → browser sidebar).  Keyed by an opaque channel id
 # the chat tab generates on mount; entries auto-evict when the last subscriber
@@ -4671,8 +4715,8 @@ async def pty_ws(ws: WebSocket) -> None:
        await ws.close(code=4401)
        return

-    if not _ws_client_is_allowed(ws):
-        _log.warning("pty-ws reject peer=%s reason=non_loopback_client", peer)
+    if not _ws_request_is_allowed(ws):
+        _log.warning("pty-ws reject peer=%s reason=non_loopback_or_bad_origin", peer)
        await ws.close(code=4403)
        return

@@ -4839,9 +4883,9 @@ async def gateway_ws(ws: WebSocket) -> None:
        await ws.close(code=4401)
        return

-    if not _ws_client_is_allowed(ws):
+    if not _ws_request_is_allowed(ws):
        _log.warning(
-            "gateway-ws reject peer=%s reason=non_loopback_client "
+            "gateway-ws reject peer=%s reason=non_loopback_or_bad_origin "
            "bound_host=%s close_code=4403",
            peer,
            getattr(app.state, "bound_host", ""),
@@ -4894,8 +4938,8 @@ async def pub_ws(ws: WebSocket) -> None:
        await ws.close(code=4401)
        return

-    if not _ws_client_is_allowed(ws):
-        _log.warning("pub-ws reject peer=%s reason=non_loopback_client", peer)
+    if not _ws_request_is_allowed(ws):
+        _log.warning("pub-ws reject peer=%s reason=non_loopback_or_bad_origin", peer)
        await ws.close(code=4403)
        return

@@ -4934,8 +4978,8 @@ async def events_ws(ws: WebSocket) -> None:
        await ws.close(code=4401)
        return

-    if not _ws_client_is_allowed(ws):
-        _log.warning("events-ws reject peer=%s reason=non_loopback_client", peer)
+    if not _ws_request_is_allowed(ws):
+        _log.warning("events-ws reject peer=%s reason=non_loopback_or_bad_origin", peer)
        await ws.close(code=4403)
        return

@@ -5473,6 +5517,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.
@@ -5493,7 +5574,16 @@ def _discover_dashboard_plugins() -> list:
        (bundled_root / "memory", "bundled"),
        (bundled_root, "bundled"),
    ]
-    if os.environ.get("HERMES_ENABLE_PROJECT_PLUGINS"):
+    # GHSA-5qr3-c538-wm9j (#29156): the previous ``os.environ.get(...)``
+    # check treated *any* non-empty string as truthy, so ``=0``, ``=false``,
+    # and ``=no`` — all of which the agent loader and operators correctly
+    # read as "disabled" — silently *enabled* the untrusted project source
+    # in the web server.  Combined with the absolute-path RCE primitive on
+    # the manifest's ``api`` field (now patched below), this turned the
+    # opt-in into a sticky always-on switch.  Use the shared truthy
+    # semantics (``1`` / ``true`` / ``yes`` / ``on``) so the gate matches
+    # ``hermes_cli/plugins.py`` and the documented user contract.
+    if env_var_enabled("HERMES_ENABLE_PROJECT_PLUGINS"):
        search_dirs.append((Path.cwd() / ".hermes" / "plugins", "project"))

    for plugins_root, source in search_dirs:
@@ -5534,23 +5624,38 @@ 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]
-                plugins.append(
-                    {
-                        "name": name,
-                        "label": data.get("label", name),
-                        "description": data.get("description", ""),
-                        "icon": data.get("icon", "Puzzle"),
-                        "version": data.get("version", "0.0.0"),
-                        "tab": tab_info,
-                        "slots": slots,
-                        "entry": data.get("entry", "dist/index.js"),
-                        "css": data.get("css"),
-                        "has_api": bool(data.get("api")),
-                        "source": source,
-                        "_dir": str(child / "dashboard"),
-                        "_api_file": data.get("api"),
-                    }
-                )
+                # 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),
+                    "description": data.get("description", ""),
+                    "icon": data.get("icon", "Puzzle"),
+                    "version": data.get("version", "0.0.0"),
+                    "tab": tab_info,
+                    "slots": slots,
+                    "entry": data.get("entry", "dist/index.js"),
+                    "css": data.get("css"),
+                    "has_api": bool(safe_api),
+                    "source": source,
+                    "_dir": str(dashboard_dir),
+                    "_api_file": safe_api,
+                })
            except Exception as exc:
                _log.warning("Bad dashboard plugin manifest %s: %s", manifest_file, exc)
                continue
@@ -5762,12 +5867,13 @@ async def post_agent_plugin_install(request: Request, body: _AgentPluginInstallB

 def _validate_plugin_name(name: str) -> str:
    """Reject path-traversal attempts in plugin name URL parameters."""
-    if not name or "/" in name or "\\" in name or ".." in name:
+    name = name.strip("/")
+    if not name or ".." in name or "\\" in name:
        raise HTTPException(status_code=400, detail="Invalid plugin name.")
    return name


-@app.post("/api/dashboard/agent-plugins/{name}/enable")
+@app.post("/api/dashboard/agent-plugins/{name:path}/enable")
 async def post_agent_plugin_enable(request: Request, name: str):
    _require_token(request)
    name = _validate_plugin_name(name)
@@ -5781,7 +5887,7 @@ async def post_agent_plugin_enable(request: Request, name: str):
    return result


-@app.post("/api/dashboard/agent-plugins/{name}/disable")
+@app.post("/api/dashboard/agent-plugins/{name:path}/disable")
 async def post_agent_plugin_disable(request: Request, name: str):
    _require_token(request)
    name = _validate_plugin_name(name)
@@ -5795,7 +5901,7 @@ async def post_agent_plugin_disable(request: Request, name: str):
    return result


-@app.post("/api/dashboard/agent-plugins/{name}/update")
+@app.post("/api/dashboard/agent-plugins/{name:path}/update")
 async def post_agent_plugin_update(request: Request, name: str):
    _require_token(request)
    name = _validate_plugin_name(name)
@@ -5810,7 +5916,7 @@ async def post_agent_plugin_update(request: Request, name: str):
    return result


-@app.delete("/api/dashboard/agent-plugins/{name}")
+@app.delete("/api/dashboard/agent-plugins/{name:path}")
 async def delete_agent_plugin(request: Request, name: str):
    _require_token(request)
    name = _validate_plugin_name(name)
@@ -5850,10 +5956,8 @@ class _PluginVisibilityBody(BaseModel):
    hidden: bool


-@app.post("/api/dashboard/plugins/{name}/visibility")
-async def post_plugin_visibility(
-    request: Request, name: str, body: _PluginVisibilityBody
-):
+@app.post("/api/dashboard/plugins/{name:path}/visibility")
+async def post_plugin_visibility(request: Request, name: str, body: _PluginVisibilityBody):
    """Toggle a plugin's sidebar visibility (persists to config.yaml dashboard.hidden_plugins)."""
    _require_token(request)
    name = _validate_plugin_name(name)
@@ -5923,12 +6027,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/<name>/``.
+
+    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",