fix: harden skill bundles and plugin API security

hermes_cli/web_server.py

@@ -225,7 +225,7 @@ async def host_header_middleware(request: Request, call_next):
 async def auth_middleware(request: Request, call_next):
     """Require the session token on all /api/ routes except the public list."""
     path = request.url.path
-    if path.startswith("/api/") and path not in _PUBLIC_API_PATHS and not path.startswith("/api/plugins/"):
+    if path.startswith("/api/") and path not in _PUBLIC_API_PATHS:
         if not _has_valid_session_token(request):
             return JSONResponse(
                 status_code=401,
@@ -3953,18 +3953,37 @@ async def serve_plugin_asset(plugin_name: str, file_path: str):
     return FileResponse(target, media_type=media_type)
 
 
+def _resolve_plugin_api_path(plugin_dir: str | Path, api_file_name: str) -> Path:
+    """Return a safe plugin API file path contained by the plugin dashboard dir."""
+    api_path = Path(api_file_name)
+    if api_path.is_absolute():
+        raise ValueError("absolute paths are not allowed")
+    if api_path.suffix != ".py":
+        raise ValueError("api file must be a Python module")
+
+    base = Path(plugin_dir).resolve()
+    resolved = (base / api_path).resolve()
+    if not resolved.is_relative_to(base):
+        raise ValueError("path escapes the plugin dashboard directory")
+    return resolved
+
+
 def _mount_plugin_api_routes():
     """Import and mount backend API routes from plugins that declare them.
 
-    Each plugin's ``api`` field points to a Python file that must expose
-    a ``router`` (FastAPI APIRouter).  Routes are mounted under
-    ``/api/plugins/<name>/``.
+    Each plugin's ``api`` field points to a Python file under the plugin's
+    dashboard directory that must expose a ``router`` (FastAPI APIRouter).
+    Routes are mounted under ``/api/plugins/<name>/``.
     """
     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
+        try:
+            api_path = _resolve_plugin_api_path(plugin["_dir"], api_file_name)
+        except ValueError as exc:
+            _log.warning("Plugin %s declares invalid api=%s: %s", plugin["name"], api_file_name, exc)
+            continue
         if not api_path.exists():
             _log.warning("Plugin %s declares api=%s but file not found", plugin["name"], api_file_name)
             continue

plugins/hermes-achievements/dashboard/dist/index.js

@@ -21,17 +21,7 @@
 
   async function api(path, options) {
     const url = "/api/plugins/hermes-achievements" + path;
-    const res = await fetch(url, options || {});
-    if (!res.ok) {
-      const text = await res.text().catch(function () { return res.statusText; });
-      throw new Error(res.status + ": " + text);
-    }
-    const text = await res.text();
-    try {
-      return JSON.parse(text);
-    } catch (_) {
-      return null;
-    }
+    return SDK.fetchJSON(url, options || {});
   }
 
   function AchievementIcon({ icon }) {

plugins/kanban/dashboard/plugin_api.py

@@ -13,15 +13,13 @@
 
 Security note
 -------------
-The dashboard's HTTP auth middleware (``web_server.auth_middleware``)
-explicitly skips ``/api/plugins/`` — plugin routes are unauthenticated by
-design because the dashboard binds to localhost by default. For the
-WebSocket we still require the session token as a ``?token=`` query
-parameter (browsers cannot set the ``Authorization`` header on an upgrade
-request), matching the established pattern used by the in-browser PTY
-bridge in ``hermes_cli/web_server.py``. If you run the dashboard with
-``--host 0.0.0.0``, every plugin route — kanban included — becomes
-reachable from the network. Don't do that on a shared host.
+Plugin HTTP routes are protected by the dashboard's session-token auth
+middleware (``web_server.auth_middleware``). Dashboard frontends should call
+these routes through ``window.__HERMES_PLUGIN_SDK__.fetchJSON`` or include the
+``X-Hermes-Session-Token`` header. The WebSocket also requires the session token
+as a ``?token=`` query parameter (browsers cannot set the ``Authorization``
+header on an upgrade request), matching the established pattern used by the
+in-browser PTY bridge in ``hermes_cli/web_server.py``.
 """
 
 from __future__ import annotations

pyproject.toml

@@ -13,8 +13,8 @@ license = { text = "MIT" }
 dependencies = [
   # Core — pinned to known-good ranges to limit supply chain attack surface
   "openai>=2.21.0,<3",
-  "anthropic>=0.39.0,<1",
-  "python-dotenv>=1.2.1,<2",
+  "anthropic>=0.87.0,<1",
+  "python-dotenv>=1.2.2,<2",
   "fire>=0.7.1,<1",
   "httpx[socks]>=0.28.1,<1",
   "rich>=14.3.3,<15",
@@ -42,8 +42,8 @@ dependencies = [
 modal = ["modal>=1.0.0,<2"]
 daytona = ["daytona>=0.148.0,<1"]
 vercel = ["vercel>=0.5.7,<0.6.0"]
-dev = ["debugpy>=1.8.0,<2", "pytest>=9.0.2,<10", "pytest-asyncio>=1.3.0,<2", "pytest-xdist>=3.0,<4", "mcp>=1.2.0,<2", "ty>=0.0.1a29,<0.0.22", "ruff"]
-messaging = ["python-telegram-bot[webhooks]>=22.6,<23", "discord.py[voice]>=2.7.1,<3", "aiohttp>=3.13.3,<4", "slack-bolt>=1.18.0,<2", "slack-sdk>=3.27.0,<4", "qrcode>=7.0,<8"]
+dev = ["debugpy>=1.8.0,<2", "pytest>=9.0.3,<10", "pytest-asyncio>=1.3.0,<2", "pytest-xdist>=3.0,<4", "mcp>=1.2.0,<2", "ty>=0.0.1a29,<0.0.22", "ruff"]
+messaging = ["python-telegram-bot[webhooks]>=22.6,<23", "discord.py>=2.7.1,<3", "aiohttp>=3.13.4,<4", "slack-bolt>=1.18.0,<2", "slack-sdk>=3.27.0,<4", "qrcode>=7.0,<8"]
 cron = []  # croniter is now a core dependency; this extra kept for back-compat
 slack = ["slack-bolt>=1.18.0,<2", "slack-sdk>=3.27.0,<4"]
 matrix = ["mautrix[encryption]>=0.20,<1", "Markdown>=3.6,<4", "aiosqlite>=0.20", "asyncpg>=0.29", "aiohttp-socks>=0.10,<1"]

tests/hermes_cli/test_web_server_host_header.py

@@ -146,3 +146,37 @@ def test_no_bound_host_skips_validation(self):
         resp = client.get("/api/status")
         # Should get through to the status endpoint, not a 400
         assert resp.status_code != 400
+
+
+class TestPluginApiSecurity:
+    def test_plugin_api_routes_require_session_token(self):
+        from fastapi.testclient import TestClient
+        from hermes_cli.web_server import app
+
+        client = TestClient(app)
+        resp = client.get("/api/plugins/example/anything")
+        assert resp.status_code == 401
+        assert resp.json()["detail"] == "Unauthorized"
+
+    def test_plugin_api_path_must_stay_under_dashboard_dir(self, tmp_path):
+        from hermes_cli.web_server import _resolve_plugin_api_path
+
+        plugin_dir = tmp_path / "plugin" / "dashboard"
+        plugin_dir.mkdir(parents=True)
+        api_file = plugin_dir / "api.py"
+        api_file.write_text("router = None\n")
+
+        assert _resolve_plugin_api_path(plugin_dir, "api.py") == api_file.resolve()
+        assert _resolve_plugin_api_path(plugin_dir, "nested/../api.py") == api_file.resolve()
+
+        for bad in ("../api.py", "/tmp/api.py", "api.txt"):
+            with pytest.raises(ValueError):
+                _resolve_plugin_api_path(plugin_dir, bad)
+
+    def test_bundled_plugin_api_fetches_use_session_aware_sdk(self):
+        repo = Path(__file__).resolve().parents[2]
+        achievements_js = repo / "plugins" / "hermes-achievements" / "dashboard" / "dist" / "index.js"
+        text = achievements_js.read_text(encoding="utf-8")
+
+        assert "SDK.fetchJSON(url, options || {})" in text
+        assert 'fetch(url, options || {})' not in text

tests/tools/test_docker_environment.py

@@ -1,7 +1,7 @@
 import logging
-from io import StringIO
 import subprocess
 import sys
+import tempfile
 import types
 
 import pytest
@@ -202,33 +202,51 @@ def test_auto_mount_replaces_persistent_workspace_bind(monkeypatch, tmp_path):
     assert "/sandboxes/docker/test-persistent-auto-mount/workspace:/workspace" not in run_args_str
 
 
-def test_non_persistent_cleanup_removes_container(monkeypatch):
-    """When persistent=false, cleanup() must schedule docker stop + rm."""
+def test_non_persistent_cleanup_removes_container_without_shell(monkeypatch):
+    """When persistent=false, cleanup() must schedule argv-safe docker cleanup."""
     monkeypatch.setattr(docker_env, "find_docker", lambda: "/usr/bin/docker")
-    calls = _mock_subprocess_run(monkeypatch)
+    _mock_subprocess_run(monkeypatch)
 
-    popen_cmds = []
-    monkeypatch.setattr(
-        docker_env.subprocess, "Popen",
-        lambda cmd, **kw: (popen_cmds.append(cmd), type("P", (), {"poll": lambda s: 0, "wait": lambda s, **k: None, "returncode": 0, "stdout": iter([]), "stdin": None})())[1],
-    )
+    popen_calls = []
+
+    class _FakeCleanupProcess:
+        def __init__(self):
+            self.returncode = 0
+
+        def poll(self):
+            return self.returncode
+
+    def _popen(cmd, **kwargs):
+        popen_calls.append((cmd, kwargs))
+        if isinstance(cmd, list) and cmd[:2] == [sys.executable, "-c"]:
+            return _FakeCleanupProcess()
+        return _FakePopen(cmd, **kwargs)
+
+    monkeypatch.setattr(docker_env.subprocess, "Popen", _popen)
 
     env = _make_dummy_env(persistent_filesystem=False, task_id="ephemeral-task")
     assert env._container_id
     container_id = env._container_id
 
     env.cleanup()
 
-    # Should have stop and rm calls via Popen
-    stop_cmds = [c for c in popen_cmds if container_id in str(c) and "stop" in str(c)]
-    assert len(stop_cmds) >= 1, f"cleanup() should schedule docker stop for {container_id}"
+    cleanup_popen_calls = [call for call in popen_calls if isinstance(call[0], list) and call[0][:2] == [sys.executable, "-c"]]
+    assert len(cleanup_popen_calls) == 1
+    cmd, kwargs = cleanup_popen_calls[0]
+    assert isinstance(cmd, list)
+    assert cmd[:2] == [sys.executable, "-c"]
+    assert cmd[-3:] == ["/usr/bin/docker", container_id, "0"]
+    assert kwargs["stdout"] is subprocess.DEVNULL
+    assert kwargs["stderr"] is subprocess.DEVNULL
+    assert kwargs["start_new_session"] is True
+    assert "shell" not in kwargs
 
 
 class _FakePopen:
     def __init__(self, cmd, **kwargs):
         self.cmd = cmd
         self.kwargs = kwargs
-        self.stdout = StringIO("")
+        self.stdout = tempfile.TemporaryFile(mode="w+b")
         self.stdin = None
         self.returncode = 0
 

tests/tools/test_skills_hub_bundle_hash.py

@@ -0,0 +1,35 @@
+from tools.skills_hub import SkillBundle, bundle_content_hash
+
+
+def test_bundle_content_hash_accepts_text_and_bytes_files():
+    bundle = SkillBundle(
+        name="mixed",
+        files={"SKILL.md": "# Skill\n", "assets/icon.png": b"\x89PNG\r\n"},
+        source="github",
+        identifier="owner/repo/skills/mixed",
+        trust_level="community",
+    )
+
+    digest = bundle_content_hash(bundle)
+
+    assert digest.startswith("sha256:")
+    assert len(digest) == len("sha256:") + 16
+
+
+def test_bundle_content_hash_is_deterministic_independent_of_file_order():
+    bundle_a = SkillBundle(
+        name="ordered",
+        files={"b.txt": "two", "a.txt": b"one"},
+        source="github",
+        identifier="owner/repo/skills/ordered",
+        trust_level="community",
+    )
+    bundle_b = SkillBundle(
+        name="ordered",
+        files={"a.txt": b"one", "b.txt": "two"},
+        source="github",
+        identifier="owner/repo/skills/ordered",
+        trust_level="community",
+    )
+
+    assert bundle_content_hash(bundle_a) == bundle_content_hash(bundle_b)

tools/environments/docker.py

@@ -618,25 +618,55 @@ def _storage_opt_supported() -> bool:
     def cleanup(self):
         """Stop and remove the container. Bind-mount dirs persist if persistent=True."""
         if self._container_id:
+            container_id = self._container_id
+            docker_exe = self._docker_exe
+            persistent_arg = "1" if self._persistent else "0"
+            helper_code = r"""
+import subprocess
+import sys
+import time
+
+docker_exe, container_id, persistent = sys.argv[1], sys.argv[2], sys.argv[3] == "1"
+try:
+    stopped = subprocess.run(
+        [docker_exe, "stop", container_id],
+        stdout=subprocess.DEVNULL,
+        stderr=subprocess.DEVNULL,
+        timeout=60,
+        check=False,
+    )
+    if stopped.returncode != 0:
+        subprocess.run(
+            [docker_exe, "rm", "-f", container_id],
+            stdout=subprocess.DEVNULL,
+            stderr=subprocess.DEVNULL,
+            timeout=30,
+            check=False,
+        )
+    elif not persistent:
+        time.sleep(3)
+        subprocess.run(
+            [docker_exe, "rm", "-f", container_id],
+            stdout=subprocess.DEVNULL,
+            stderr=subprocess.DEVNULL,
+            timeout=30,
+            check=False,
+        )
+except Exception:
+    pass
+"""
             try:
-                # Stop in background so cleanup doesn't block
-                stop_cmd = (
-                    f"(timeout 60 {self._docker_exe} stop {self._container_id} || "
-                    f"{self._docker_exe} rm -f {self._container_id}) >/dev/null 2>&1 &"
+                # Spawn an independent helper so cleanup survives parent exit,
+                # while passing docker paths and container IDs as argv rather
+                # than interpolating them into a shell command.
+                subprocess.Popen(
+                    [sys.executable, "-c", helper_code, docker_exe, container_id, persistent_arg],
+                    stdout=subprocess.DEVNULL,
+                    stderr=subprocess.DEVNULL,
+                    start_new_session=True,
                 )
-                subprocess.Popen(stop_cmd, shell=True)
             except Exception as e:
-                logger.warning("Failed to stop container %s: %s", self._container_id, e)
-
-            if not self._persistent:
-                # Also schedule removal (stop only leaves it as stopped)
-                try:
-                    subprocess.Popen(
-                        f"sleep 3 && {self._docker_exe} rm -f {self._container_id} >/dev/null 2>&1 &",
-                        shell=True,
-                    )
-                except Exception:
-                    pass
+                logger.warning("Failed to schedule cleanup for container %s: %s", container_id, e)
             self._container_id = None
 
         if not self._persistent:

tools/skills_hub.py

@@ -2801,7 +2801,11 @@ def bundle_content_hash(bundle: SkillBundle) -> str:
     """Compute a deterministic hash for an in-memory skill bundle."""
     h = hashlib.sha256()
     for rel_path in sorted(bundle.files):
-        h.update(bundle.files[rel_path].encode("utf-8"))
+        content = bundle.files[rel_path]
+        if isinstance(content, bytes):
+            h.update(content)
+        else:
+            h.update(content.encode("utf-8"))
     return f"sha256:{h.hexdigest()[:16]}"