mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-27 11:22:03 +00:00
fix(tui): stop a cwd package named utils/proxy/ui from crashing the gateway child (#51693)
Launching Hermes from a directory that ships its own top-level package with a
Hermes-internal name (utils/, proxy/, ui/) crashed the gateway/TUI child with
an ImportError (exit 1, crash loop): from utils import atomic_replace resolved
to the user's package.
tui_gateway/entry.py already stripped the relative cwd forms ('' / '.'), but
the launch dir also reaches sys.path as its own ABSOLUTE path (venv activation
or a project that adds itself to PYTHONPATH), which the strip missed and which
sat ahead of the Hermes root.
Centralize a hardened guard in hermes_bootstrap.harden_import_path(): drop the
relative forms AND force the Hermes source root to the front even when an
absolute cwd entry is present. Wire it into tui_gateway/entry.py and
acp_adapter/entry.py (both spawn into arbitrary cwds); hermes_cli/main.py and
gateway/run.py already insert the root at front. gatewayClient.ts now also
exports HERMES_PYTHON_SRC_ROOT for defense in depth.
This commit is contained in:
parent
3d56807fbd
commit
c39b2b50ee
6 changed files with 208 additions and 99 deletions
|
|
@ -23,6 +23,11 @@ except ModuleNotFoundError:
|
|||
# new code but ``uv pip install -e .`` didn't finish. Missing bootstrap
|
||||
# means UTF-8 stdio setup is skipped on Windows; POSIX is unaffected.
|
||||
pass
|
||||
else:
|
||||
# Stop a ``utils/``/``proxy/``/``ui/`` package in the launch directory from
|
||||
# shadowing Hermes's own modules — ``hermes acp`` can be started from any
|
||||
# cwd, including a project that has same-named packages on its path.
|
||||
hermes_bootstrap.harden_import_path()
|
||||
|
||||
import argparse
|
||||
import asyncio
|
||||
|
|
|
|||
|
|
@ -122,6 +122,42 @@ def apply_windows_utf8_bootstrap() -> bool:
|
|||
return True
|
||||
|
||||
|
||||
def harden_import_path(src_root: str | None = None) -> None:
|
||||
"""Stop a package in the current directory from shadowing Hermes modules.
|
||||
|
||||
Hermes ships top-level modules with common names (``utils``, ``proxy``,
|
||||
``ui``). Python always seeds ``sys.path`` with the current directory, so
|
||||
launching an entry point from a project that has its own ``utils/`` package
|
||||
makes ``from utils import ...`` resolve to the *user's* package and crash
|
||||
with an ImportError before the gateway can even start.
|
||||
|
||||
The current directory reaches ``sys.path`` two ways, and a complete guard
|
||||
has to handle both:
|
||||
|
||||
- As the empty string ``""`` (or ``"."``) that Python inserts at
|
||||
``sys.path[0]`` for ``-m`` / script launches.
|
||||
- As its own *absolute* path, when a venv activation or a project that
|
||||
adds itself to ``PYTHONPATH`` puts the directory there explicitly.
|
||||
|
||||
We drop the relative forms outright, then force the real Hermes source root
|
||||
to the front — relocating it ahead of any absolute cwd entry rather than
|
||||
only inserting when absent, so an absolute cwd path can't keep winning.
|
||||
|
||||
``src_root`` defaults to the directory this module lives in, which is the
|
||||
repository root for every shipped entry point, so the guard is
|
||||
self-sufficient and does not depend on the spawner exporting an env var.
|
||||
"""
|
||||
root = src_root or os.environ.get("HERMES_PYTHON_SRC_ROOT") or os.path.dirname(
|
||||
os.path.abspath(__file__)
|
||||
)
|
||||
|
||||
sys.path[:] = [p for p in sys.path if p not in ("", ".")]
|
||||
|
||||
root_abs = os.path.abspath(root)
|
||||
sys.path[:] = [p for p in sys.path if os.path.abspath(p) != root_abs]
|
||||
sys.path.insert(0, root)
|
||||
|
||||
|
||||
# Apply on import — entry points just need ``import hermes_bootstrap``
|
||||
# (or ``from hermes_bootstrap import apply_windows_utf8_bootstrap``) at
|
||||
# the very top of their module, before importing anything else. The
|
||||
|
|
|
|||
|
|
@ -311,3 +311,88 @@ class TestEntryPointsImportBootstrap:
|
|||
f"configured before anything else initializes. Move the "
|
||||
f"'import hermes_bootstrap' line to be the first import."
|
||||
)
|
||||
|
||||
|
||||
class TestHardenImportPath:
|
||||
"""harden_import_path() must keep a same-named package in the launch
|
||||
directory from shadowing Hermes's own top-level modules — covering both
|
||||
the relative ('' / '.') and absolute-path forms the cwd can take on
|
||||
sys.path (issue #51286)."""
|
||||
|
||||
def _run(self, hb, path_seed, env=None):
|
||||
original = sys.path[:]
|
||||
original_env = os.environ.get("HERMES_PYTHON_SRC_ROOT")
|
||||
try:
|
||||
sys.path[:] = path_seed
|
||||
if env is not None:
|
||||
os.environ["HERMES_PYTHON_SRC_ROOT"] = env
|
||||
elif "HERMES_PYTHON_SRC_ROOT" in os.environ:
|
||||
del os.environ["HERMES_PYTHON_SRC_ROOT"]
|
||||
hb.harden_import_path(src_root="/opt/hermes")
|
||||
return sys.path[:]
|
||||
finally:
|
||||
sys.path[:] = original
|
||||
if original_env is None:
|
||||
os.environ.pop("HERMES_PYTHON_SRC_ROOT", None)
|
||||
else:
|
||||
os.environ["HERMES_PYTHON_SRC_ROOT"] = original_env
|
||||
|
||||
def test_relative_cwd_forms_removed(self):
|
||||
hb = _fresh_import()
|
||||
result = self._run(hb, ["", ".", "/opt/hermes", "/usr/lib/python"])
|
||||
assert "" not in result
|
||||
assert "." not in result
|
||||
|
||||
def test_src_root_forced_to_front(self):
|
||||
hb = _fresh_import()
|
||||
result = self._run(hb, ["", "/opt/hermes", "/usr/lib/python"])
|
||||
assert result[0] == "/opt/hermes"
|
||||
|
||||
def test_absolute_cwd_path_loses_to_src_root(self):
|
||||
# The real #51286 bug: the launch dir is present as its own absolute
|
||||
# path (venv activation / a project on PYTHONPATH), ahead of the
|
||||
# Hermes root. The guard must relocate Hermes to the front.
|
||||
hb = _fresh_import()
|
||||
result = self._run(hb, ["/home/user/tg-ws-proxy", "/opt/hermes"])
|
||||
assert result[0] == "/opt/hermes"
|
||||
# The cwd absolute path may still appear (it can hold legit deps),
|
||||
# but only AFTER the Hermes root.
|
||||
assert result.index("/opt/hermes") < result.index("/home/user/tg-ws-proxy")
|
||||
|
||||
def test_src_root_not_duplicated(self):
|
||||
hb = _fresh_import()
|
||||
result = self._run(hb, ["/opt/hermes", "/opt/hermes", ""])
|
||||
assert result.count("/opt/hermes") == 1
|
||||
|
||||
def test_env_var_used_when_no_arg(self):
|
||||
hb = _fresh_import()
|
||||
original = sys.path[:]
|
||||
original_env = os.environ.get("HERMES_PYTHON_SRC_ROOT")
|
||||
try:
|
||||
sys.path[:] = ["", "/cwd/proj", "/usr/lib"]
|
||||
os.environ["HERMES_PYTHON_SRC_ROOT"] = "/env/hermes"
|
||||
hb.harden_import_path()
|
||||
assert sys.path[0] == "/env/hermes"
|
||||
finally:
|
||||
sys.path[:] = original
|
||||
if original_env is None:
|
||||
os.environ.pop("HERMES_PYTHON_SRC_ROOT", None)
|
||||
else:
|
||||
os.environ["HERMES_PYTHON_SRC_ROOT"] = original_env
|
||||
|
||||
def test_defaults_to_module_dir(self):
|
||||
# With neither arg nor env var, the helper anchors on the bootstrap
|
||||
# module's own directory — the repo root for shipped entry points.
|
||||
hb = _fresh_import()
|
||||
original = sys.path[:]
|
||||
original_env = os.environ.get("HERMES_PYTHON_SRC_ROOT")
|
||||
try:
|
||||
sys.path[:] = ["", "/somewhere/else"]
|
||||
os.environ.pop("HERMES_PYTHON_SRC_ROOT", None)
|
||||
hb.harden_import_path()
|
||||
expected = os.path.dirname(os.path.abspath(hb.__file__))
|
||||
assert sys.path[0] == expected
|
||||
finally:
|
||||
sys.path[:] = original
|
||||
if original_env is not None:
|
||||
os.environ["HERMES_PYTHON_SRC_ROOT"] = original_env
|
||||
|
|
|
|||
|
|
@ -1,100 +1,81 @@
|
|||
"""Tests for tui_gateway/entry.py sys.path hardening (issue #15989).
|
||||
"""Tests for tui_gateway/entry.py sys.path hardening (issues #15989, #51286).
|
||||
|
||||
When the TUI backend is spawned by Node.js, the Python interpreter may have
|
||||
'' or '.' at the front of sys.path, allowing a local utils/ directory in CWD
|
||||
to shadow the installed utils module. entry.py must sanitize sys.path before
|
||||
any non-stdlib import is resolved.
|
||||
When the TUI backend is spawned by Node.js, the launch directory may shadow
|
||||
Hermes's own top-level modules (``utils``, ``proxy``, ``ui``). entry.py must
|
||||
neutralize this before any non-stdlib import is resolved, by delegating to the
|
||||
shared ``hermes_bootstrap.harden_import_path`` guard.
|
||||
|
||||
These tests assert the entry point wires up the real guard (rather than
|
||||
re-implementing it inline) and that the guard's behavior covers both the
|
||||
relative-cwd form and the absolute-cwd-path form that was the actual #51286
|
||||
failure.
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
from unittest.mock import patch
|
||||
import ast
|
||||
import pathlib
|
||||
|
||||
import hermes_bootstrap
|
||||
|
||||
|
||||
def _reload_entry_with_env(env_overrides: dict) -> None:
|
||||
"""Re-execute entry.py's module-level path setup under a controlled env."""
|
||||
# We only want to exercise the sys.path fixup block, not the signal/import
|
||||
# machinery that follows. We do this by running the fixup code verbatim in
|
||||
# a fresh copy of sys.path rather than importing the real module (which
|
||||
# would trigger tui_gateway.server imports requiring heavy mocks).
|
||||
original_path = sys.path[:]
|
||||
original_env = {k: os.environ.get(k) for k in env_overrides}
|
||||
try:
|
||||
with patch.dict(os.environ, env_overrides, clear=False):
|
||||
_src_root = os.environ.get("HERMES_PYTHON_SRC_ROOT", "")
|
||||
if _src_root and _src_root not in sys.path:
|
||||
sys.path.insert(0, _src_root)
|
||||
sys.path = [p for p in sys.path if p not in {"", "."}]
|
||||
return sys.path[:]
|
||||
finally:
|
||||
sys.path = original_path
|
||||
for k, v in original_env.items():
|
||||
if v is None:
|
||||
os.environ.pop(k, None)
|
||||
else:
|
||||
os.environ[k] = v
|
||||
def _entry_source() -> str:
|
||||
here = pathlib.Path(__file__).resolve()
|
||||
repo_root = here.parent.parent.parent # tests/tui_gateway/ -> repo root
|
||||
return (repo_root / "tui_gateway" / "entry.py").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def test_empty_string_and_dot_removed_from_sys_path():
|
||||
def test_entry_calls_shared_harden_guard_before_heavy_imports():
|
||||
"""entry.py must call hermes_bootstrap.harden_import_path() before it
|
||||
imports tui_gateway.server (which pulls ``from utils import ...``)."""
|
||||
source = _entry_source()
|
||||
tree = ast.parse(source)
|
||||
|
||||
harden_call_line = None
|
||||
server_import_line = None
|
||||
for node in ast.walk(tree):
|
||||
if (
|
||||
isinstance(node, ast.Call)
|
||||
and isinstance(node.func, ast.Attribute)
|
||||
and node.func.attr == "harden_import_path"
|
||||
):
|
||||
harden_call_line = node.lineno
|
||||
if isinstance(node, ast.ImportFrom) and (node.module or "").startswith(
|
||||
"tui_gateway"
|
||||
):
|
||||
if server_import_line is None:
|
||||
server_import_line = node.lineno
|
||||
|
||||
assert harden_call_line is not None, (
|
||||
"entry.py must call hermes_bootstrap.harden_import_path()"
|
||||
)
|
||||
assert server_import_line is not None, "entry.py must import from tui_gateway"
|
||||
assert harden_call_line < server_import_line, (
|
||||
"harden_import_path() must run before tui_gateway.server is imported"
|
||||
)
|
||||
|
||||
|
||||
def test_entry_does_not_reimplement_guard_inline():
|
||||
"""The old inline ``{'', '.'}`` strip lived in entry.py; the dedicated
|
||||
helper now owns it. Guard against the inline logic creeping back."""
|
||||
source = _entry_source()
|
||||
assert '{"", "."}' not in source and "{'', '.'}" not in source, (
|
||||
"entry.py should delegate to hermes_bootstrap.harden_import_path, "
|
||||
"not re-implement the sys.path strip inline"
|
||||
)
|
||||
|
||||
|
||||
def test_guard_handles_absolute_cwd_path():
|
||||
"""The #51286 case: the launch dir is on sys.path as its own absolute
|
||||
path, ahead of the Hermes root. harden_import_path must relocate the
|
||||
Hermes root to the front so ``from utils import ...`` resolves to Hermes."""
|
||||
import sys
|
||||
|
||||
original = sys.path[:]
|
||||
try:
|
||||
sys.path.insert(0, "")
|
||||
sys.path.insert(0, ".")
|
||||
assert "" in sys.path
|
||||
assert "." in sys.path
|
||||
|
||||
# Run the entry.py fixup logic directly
|
||||
sys.path = [p for p in sys.path if p not in {"", "."}]
|
||||
|
||||
assert "" not in sys.path
|
||||
assert "." not in sys.path
|
||||
sys.path[:] = ["/home/user/tg-ws-proxy", "/opt/hermes", "/usr/lib"]
|
||||
hermes_bootstrap.harden_import_path(src_root="/opt/hermes")
|
||||
assert sys.path[0] == "/opt/hermes"
|
||||
assert sys.path.index("/opt/hermes") < sys.path.index(
|
||||
"/home/user/tg-ws-proxy"
|
||||
)
|
||||
finally:
|
||||
sys.path = original
|
||||
|
||||
|
||||
def test_hermes_src_root_inserted_at_front():
|
||||
original = sys.path[:]
|
||||
try:
|
||||
fake_root = "/fake/hermes/src"
|
||||
with patch.dict(os.environ, {"HERMES_PYTHON_SRC_ROOT": fake_root}):
|
||||
_src_root = os.environ.get("HERMES_PYTHON_SRC_ROOT", "")
|
||||
if _src_root and _src_root not in sys.path:
|
||||
sys.path.insert(0, _src_root)
|
||||
sys.path = [p for p in sys.path if p not in {"", "."}]
|
||||
|
||||
assert sys.path[0] == fake_root
|
||||
finally:
|
||||
sys.path = original
|
||||
|
||||
|
||||
def test_src_root_not_duplicated_if_already_present():
|
||||
original = sys.path[:]
|
||||
try:
|
||||
fake_root = "/already/present"
|
||||
sys.path.insert(0, fake_root)
|
||||
count_before = sys.path.count(fake_root)
|
||||
|
||||
with patch.dict(os.environ, {"HERMES_PYTHON_SRC_ROOT": fake_root}):
|
||||
_src_root = os.environ.get("HERMES_PYTHON_SRC_ROOT", "")
|
||||
if _src_root and _src_root not in sys.path:
|
||||
sys.path.insert(0, _src_root)
|
||||
sys.path = [p for p in sys.path if p not in {"", "."}]
|
||||
|
||||
assert sys.path.count(fake_root) == count_before
|
||||
finally:
|
||||
sys.path = original
|
||||
|
||||
|
||||
def test_no_src_root_env_does_not_crash():
|
||||
original = sys.path[:]
|
||||
try:
|
||||
env = {k: v for k, v in os.environ.items() if k != "HERMES_PYTHON_SRC_ROOT"}
|
||||
with patch.dict(os.environ, {}, clear=True):
|
||||
os.environ.update(env)
|
||||
_src_root = os.environ.get("HERMES_PYTHON_SRC_ROOT", "")
|
||||
if _src_root and _src_root not in sys.path:
|
||||
sys.path.insert(0, _src_root)
|
||||
sys.path = [p for p in sys.path if p not in {"", "."}]
|
||||
# No exception raised
|
||||
finally:
|
||||
sys.path = original
|
||||
sys.path[:] = original
|
||||
|
|
|
|||
|
|
@ -1,15 +1,14 @@
|
|||
import os
|
||||
import sys
|
||||
|
||||
# Guard against a local utils/ (or other package) in CWD shadowing installed
|
||||
# hermes modules. hermes_cli sets HERMES_PYTHON_SRC_ROOT before spawning this
|
||||
# subprocess; inserting it first ensures the installed packages win.
|
||||
_src_root = os.environ.get("HERMES_PYTHON_SRC_ROOT", "")
|
||||
if _src_root and _src_root not in sys.path:
|
||||
sys.path.insert(0, _src_root)
|
||||
# Strip '' and '.' — both resolve to CWD at import time and can let a local
|
||||
# directory shadow installed packages.
|
||||
sys.path = [p for p in sys.path if p not in {"", "."}]
|
||||
# Stop a ``utils/`` (or ``proxy/``, ``ui/``) package in the launch directory
|
||||
# from shadowing Hermes's own top-level modules. ``hermes_bootstrap`` lives at
|
||||
# the repo root next to this package, so importing it is safe before the guard
|
||||
# runs (its name won't collide with a user package), and it owns the canonical
|
||||
# path-hardening logic shared with the other entry points.
|
||||
import hermes_bootstrap
|
||||
|
||||
hermes_bootstrap.harden_import_path()
|
||||
|
||||
import json
|
||||
import logging
|
||||
|
|
|
|||
|
|
@ -344,6 +344,9 @@ export class GatewayClient extends EventEmitter {
|
|||
const pyPath = env.PYTHONPATH?.trim()
|
||||
|
||||
env.PYTHONPATH = pyPath ? `${root}${delimiter}${pyPath}` : root
|
||||
// Tell the gateway child where the Hermes source root is so its import
|
||||
// guard can force it ahead of any same-named package in the launch cwd.
|
||||
env.HERMES_PYTHON_SRC_ROOT = root
|
||||
this.startReadyTimer(python, cwd)
|
||||
this.proc = spawn(python, ['-m', 'tui_gateway.entry'], { cwd, env, stdio: ['pipe', 'pipe', 'pipe'] })
|
||||
this.lifecycle(`[lifecycle] spawned gateway child ${describeChild(this.proc)} python=${python} cwd=${cwd}`)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue