From da18c20226e6de243e9f279686d272224413619b Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Tue, 15 Sep 2026 14:23:59 -0700 Subject: [PATCH] fix: kanban dispatcher prefers module argv over PATH hermes Review finding: hermes_cli/kanban_db_dispatch.py::_resolve_hermes_argv still resolved which('hermes') before sys.executable -m hermes_cli.main while claiming to mirror gateway.run._resolve_hermes_bin, which this PR made module-first (#111569). Keep the explicit HERMES_BIN override first, then the module argv whenever hermes_cli is importable, PATH only as fallback; docstring updated. --- hermes_cli/kanban_db_dispatch.py | 20 +++++++++++++++----- tests/hermes_cli/test_kanban_db.py | 22 ++++++++++++++++++++-- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/hermes_cli/kanban_db_dispatch.py b/hermes_cli/kanban_db_dispatch.py index e9713be2ce..6f87913ec4 100644 --- a/hermes_cli/kanban_db_dispatch.py +++ b/hermes_cli/kanban_db_dispatch.py @@ -2158,12 +2158,16 @@ def _hermes_path_argv(path: str) -> list[str]: def _resolve_hermes_argv() -> list[str]: """Resolve the ``hermes`` invocation as argv for ``Popen``: ``$HERMES_BIN`` (path-like -> absolute; bare names keep PATH semantics, never a - same-directory file), then ``which("hermes")`` (Windows: safe PATH search, - batch shims fall back to the module form), then ``sys.executable -m - hermes_cli.main`` for shim-less environments (cron, systemd ``User=``, - launchd). Mirrors ``gateway.run._resolve_hermes_bin``; local because - ``hermes_cli`` sits below ``gateway`` in the dependency order. + same-directory file), then the running interpreter's ``sys.executable -m + hermes_cli.main`` (exactly this install; also covers shim-less cron, + systemd ``User=``, launchd), then ``which("hermes")`` (Windows: safe PATH + search, batch shims fall back to the module form) only when ``hermes_cli`` + is not importable. The module argv must win over PATH: a PATH-first lookup + lets an attacker-planted ``hermes`` shadow the running install (#111569). + Mirrors ``gateway.run._resolve_hermes_bin``; local because ``hermes_cli`` + sits below ``gateway`` in the dependency order. """ + import importlib.util import shutil env_bin = os.environ.get("HERMES_BIN", "").strip() @@ -2175,6 +2179,12 @@ def _resolve_hermes_argv() -> list[str]: return _hermes_path_argv(resolved_env_bin) return _module_hermes_argv() + try: + if importlib.util.find_spec("hermes_cli") is not None: + return _module_hermes_argv() + except Exception: + pass + hermes_bin = _safe_which_no_cwd("hermes") if _kb._IS_WINDOWS else shutil.which("hermes") if hermes_bin: return _hermes_path_argv(hermes_bin) diff --git a/tests/hermes_cli/test_kanban_db.py b/tests/hermes_cli/test_kanban_db.py index 28dd29c750..65c9f0ed59 100644 --- a/tests/hermes_cli/test_kanban_db.py +++ b/tests/hermes_cli/test_kanban_db.py @@ -1360,11 +1360,29 @@ def test_migrate_add_optional_columns_tolerates_concurrent_migration(kanban_home # launchd jobs, and other detached processes routinely run with a stripped # $PATH that doesn't include the venv's bin/, so a bare `["hermes", ...]` # spawn fails with FileNotFoundError and the task gets stuck. The resolver -# prefers the PATH shim (familiar `ps` output) but falls back to the module -# form so the spawn keeps working when PATH is missing the shim. +# prefers the interpreter-bound module form (exactly this install; a PATH +# shim could be attacker-planted or belong to another install, #111569) and +# only falls back to the PATH shim when ``hermes_cli`` is not importable. # --------------------------------------------------------------------------- +def test_resolve_hermes_argv_prefers_module_form_over_path_shim(monkeypatch): + """A `hermes` on PATH must not shadow the running install (#111569): + the module argv wins whenever ``hermes_cli`` is importable; only an + explicit ``$HERMES_BIN`` overrides it.""" + import shutil + import sys + from hermes_cli import kanban_db_dispatch as kbd + + monkeypatch.delenv("HERMES_BIN", raising=False) + monkeypatch.setattr(shutil, "which", lambda name: "/tmp/planted/hermes") + monkeypatch.setattr(kbd, "_safe_which_no_cwd", lambda name: "/tmp/planted/hermes") + assert kbd._resolve_hermes_argv() == [sys.executable, "-m", "hermes_cli.main"] + + monkeypatch.setenv("HERMES_BIN", "/opt/hermes/bin/hermes") + assert kbd._resolve_hermes_argv() == ["/opt/hermes/bin/hermes"] + + def test_resolve_hermes_argv_falls_back_to_module_form_when_no_path_shim(monkeypatch): """When the shim is not on PATH, fall back to `python -m hermes_cli.main`.