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.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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`.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user