fix(terminal): strip Hermes-venv site-packages from terminal subprocess PYTHONPATH to prevent cross-version ABI conflicts

The Desktop Electron process injects the Hermes venv's site-packages path
(e.g. .../python3.11/site-packages) into PYTHONPATH so the Python 3.11
backend can import its packages. When this PYTHONPATH leaks into terminal
subprocesses running a different Python version (e.g. Python 3.13), 3.11
C extension modules appear on sys.path ahead of the correct 3.13 versions
and crash with ImportError (PIL _imaging, cryptography, etc.).

Replace the existing blunt pop of PYTHONPATH from _ACTIVE_VENV_MARKER_VARS
with a surgical Hermes-venv-aware filter:

- Parse each PYTHONPATH entry by path
- Strip only paths under ~/.hermes/hermes-agent/venv/.../site-packages
- Preserve the Hermes source root (needed for import hermes_cli)
- Preserve all user-set PYTHONPATH entries

The same filter is applied in all three env builders:
- _make_run_env (foreground terminal commands)
- _sanitize_subprocess_env (background/PTY spawns)
- PTY env builder

This preserves env_passthrough semantics and never silently discards the
user's own PYTHONPATH configuration.
This commit is contained in:
mcjoys
2026-07-09 00:53:12 +08:00
committed by Teknium
parent 93ed11379b
commit 43c463fa95
3 changed files with 468 additions and 0 deletions

View File

@@ -313,6 +313,238 @@ class TestActiveVenvMarkerStripping:
assert "CONDA_PREFIX" in _ACTIVE_VENV_MARKER_VARS
class TestPythonpathSelectiveStrip:
"""PYTHONPATH site-packages stripping for cross-version ABI safety (#74817).
The Desktop Electron app injects the Hermes venv's site-packages
(Python 3.11) into PYTHONPATH. When this leaks into subprocesses
running a different Python (e.g. 3.13), 3.11 C extensions appear on
sys.path and crash with ImportError. ``_strip_mismatched_site_packages``
surgically removes only the dangerous entries, preserving user paths.
"""
def test_hermes_venv_site_packages_stripped(self):
"""A site-packages entry under the Hermes venv is removed."""
from tools.environments.local import _strip_mismatched_site_packages
import sys
# Construct a path that looks like the Hermes venv site-packages.
# Use the running interpreter's version so it hits the "under Hermes
# venv" check (check 2), not the cross-version check (check 1).
pyver = f"python{sys.version_info[0]}.{sys.version_info[1]}"
venv_sp = str(
__import__("pathlib").Path(sys.prefix) / "lib" / pyver / "site-packages"
)
env = {
"PYTHONPATH": os.pathsep.join([venv_sp, "/home/user/my-lib"]),
}
_strip_mismatched_site_packages(env)
assert "PYTHONPATH" in env
entries = env["PYTHONPATH"].split(os.pathsep)
assert venv_sp not in entries
assert "/home/user/my-lib" in entries
def test_user_pythonpath_preserved(self):
"""User PYTHONPATH entries pass through untouched."""
from tools.environments.local import _strip_mismatched_site_packages
user_pp = os.pathsep.join(["/opt/my-lib", "/another/path"])
env = {"PYTHONPATH": user_pp}
_strip_mismatched_site_packages(env)
assert env.get("PYTHONPATH") == user_pp
def test_cross_version_site_packages_stripped(self):
"""A python3.12/site-packages entry is stripped even if NOT under the
Hermes venv path - simulates a leak from systemd or another source."""
from tools.environments.local import _strip_mismatched_site_packages
import sys
# Use a version different from the running interpreter.
running_major = sys.version_info[0]
running_minor = sys.version_info[1]
# Pick a guaranteed-different version.
other_minor = running_minor + 1 if running_minor < 20 else running_minor - 1
other_ver = f"python{running_major}.{other_minor}"
mismatched_sp = f"/opt/other-venv/lib/{other_ver}/site-packages"
env = {
"PYTHONPATH": os.pathsep.join([mismatched_sp, "/home/user/my-lib"]),
}
_strip_mismatched_site_packages(env)
assert "PYTHONPATH" in env
entries = env["PYTHONPATH"].split(os.pathsep)
assert mismatched_sp not in entries
assert "/home/user/my-lib" in entries
def test_cross_major_version_stripped(self):
"""A python2.7/site-packages entry is always stripped."""
from tools.environments.local import _strip_mismatched_site_packages
env = {
"PYTHONPATH": "/old/lib/python2.7/site-packages:/home/user/lib",
}
_strip_mismatched_site_packages(env)
entries = env["PYTHONPATH"].split(os.pathsep)
assert "/old/lib/python2.7/site-packages" not in entries
assert "/home/user/lib" in entries
def test_windows_backslash_paths(self):
"""Windows-style backslash paths with site-packages are handled.
On Windows, os.pathsep is ';'. We mock it so the test runs
correctly on POSIX CI."""
from tools.environments.local import _strip_mismatched_site_packages
import sys
# Construct a Windows-style path with a different Python version.
running_major = sys.version_info[0]
running_minor = sys.version_info[1]
other_minor = running_minor + 1 if running_minor < 20 else running_minor - 1
other_ver = f"python{running_major}.{other_minor}"
mismatched_win = f"C:\\venv\\lib\\{other_ver}\\site-packages"
user_win = "D:\\user\\lib"
env = {
"PYTHONPATH": ";".join([mismatched_win, user_win]),
}
# Mock os.pathsep to ';' (Windows) just for the strip call.
with patch("os.pathsep", ";"):
_strip_mismatched_site_packages(env)
assert "PYTHONPATH" in env
entries = env["PYTHONPATH"].split(";")
assert mismatched_win not in entries
assert user_win in entries
def test_empty_pythonpath_unchanged(self):
"""An empty PYTHONPATH is a no-op (falsy -> early return)."""
from tools.environments.local import _strip_mismatched_site_packages
env = {"PYTHONPATH": ""}
_strip_mismatched_site_packages(env)
# Empty string is falsy, so the function returns early without
# modifying the dict. The key stays as-is (empty string).
assert env.get("PYTHONPATH") == ""
def test_no_pythonpath_key(self):
"""Missing PYTHONPATH key is a no-op."""
from tools.environments.local import _strip_mismatched_site_packages
env = {"PATH": "/usr/bin"}
_strip_mismatched_site_packages(env)
assert "PYTHONPATH" not in env
def test_all_entries_stripped_removes_key(self):
"""If all entries are stripped, PYTHONPATH key is removed entirely."""
from tools.environments.local import _strip_mismatched_site_packages
import sys
running_major = sys.version_info[0]
running_minor = sys.version_info[1]
other_minor = running_minor + 1 if running_minor < 20 else running_minor - 1
other_ver = f"python{running_major}.{other_minor}"
env = {"PYTHONPATH": f"/a/lib/{other_ver}/site-packages"}
_strip_mismatched_site_packages(env)
assert "PYTHONPATH" not in env
def test_make_run_env_strips_hermes_venv_pythonpath(self):
"""_make_run_env strips Hermes venv site-packages from PYTHONPATH."""
from tools.environments.local import _make_run_env
import sys
pyver = f"python{sys.version_info[0]}.{sys.version_info[1]}"
venv_sp = str(
__import__("pathlib").Path(sys.prefix) / "lib" / pyver / "site-packages"
)
with patch.dict(os.environ, {
"PATH": "/usr/bin:/bin",
"PYTHONPATH": os.pathsep.join([venv_sp, "/home/user/my-lib"]),
}, clear=True):
run_env = _make_run_env({})
pp = run_env.get("PYTHONPATH", "")
entries = pp.split(os.pathsep) if pp else []
assert venv_sp not in entries
assert "/home/user/my-lib" in entries
def test_sanitize_subprocess_env_strips_hermes_venv_pythonpath(self):
"""_sanitize_subprocess_env strips Hermes venv site-packages."""
from tools.environments.local import _sanitize_subprocess_env
import sys
pyver = f"python{sys.version_info[0]}.{sys.version_info[1]}"
venv_sp = str(
__import__("pathlib").Path(sys.prefix) / "lib" / pyver / "site-packages"
)
base = {
"PATH": "/usr/bin",
"HOME": "/home/user",
"PYTHONPATH": os.pathsep.join([venv_sp, "/home/user/my-lib"]),
}
result = _sanitize_subprocess_env(base)
pp = result.get("PYTHONPATH", "")
entries = pp.split(os.pathsep) if pp else []
assert venv_sp not in entries
assert "/home/user/my-lib" in entries
def test_hermes_subprocess_env_strips_hermes_venv_pythonpath(self):
"""hermes_subprocess_env strips Hermes venv site-packages."""
from tools.environments.local import hermes_subprocess_env
import sys
pyver = f"python{sys.version_info[0]}.{sys.version_info[1]}"
venv_sp = str(
__import__("pathlib").Path(sys.prefix) / "lib" / pyver / "site-packages"
)
with patch.dict(os.environ, {
"PATH": "/usr/bin:/bin",
"HOME": "/home/user",
"PYTHONPATH": os.pathsep.join([venv_sp, "/home/user/my-lib"]),
}, clear=True):
result = hermes_subprocess_env()
pp = result.get("PYTHONPATH", "")
entries = pp.split(os.pathsep) if pp else []
assert venv_sp not in entries
assert "/home/user/my-lib" in entries
def test_scrub_child_env_strips_mismatched_pythonpath(self):
"""execute_code's _scrub_child_env path: after scrubbing, mismatched
site-packages entries should be stripped when _strip_mismatched_site_packages
is applied (as the spawn path does)."""
from tools.code_execution_tool import _scrub_child_env
from tools.environments.local import _strip_mismatched_site_packages
import sys
running_major = sys.version_info[0]
running_minor = sys.version_info[1]
other_minor = running_minor + 1 if running_minor < 20 else running_minor - 1
other_ver = f"python{running_major}.{other_minor}"
mismatched_sp = f"/opt/other-venv/lib/{other_ver}/site-packages"
source = {
"PATH": "/usr/bin",
"HOME": "/home/user",
"PYTHONPATH": os.pathsep.join([mismatched_sp, "/home/user/my-lib"]),
}
scrubbed = _scrub_child_env(source)
# The scrubber passes PYTHONPATH through (it's in _SAFE_ENV_PREFIXES).
assert "PYTHONPATH" in scrubbed
# Now apply the selective strip (as the spawn path does).
_strip_mismatched_site_packages(scrubbed)
pp = scrubbed.get("PYTHONPATH", "")
entries = pp.split(os.pathsep) if pp else []
assert mismatched_sp not in entries
assert "/home/user/my-lib" in entries
def test_repo_root_stripped(self):
"""The Hermes repo root entry is stripped from PYTHONPATH."""
from tools.environments.local import _strip_mismatched_site_packages, _hermes_repo_root
repo_root = str(_hermes_repo_root)
env = {
"PYTHONPATH": os.pathsep.join([repo_root, "/home/user/my-lib"]),
}
_strip_mismatched_site_packages(env)
pp = env.get("PYTHONPATH", "")
entries = pp.split(os.pathsep) if pp else []
assert repo_root not in entries
assert "/home/user/my-lib" in entries
class TestProfileScopedPassthrough:
def test_make_run_env_uses_active_profile_for_passthrough(self, monkeypatch):
"""Allowlisted values must come from the routed profile, not os.environ."""

View File

@@ -1483,6 +1483,16 @@ def execute_code(
# external venv; exposing Hermes's site-packages to that interpreter
# can mix incompatible compiled extensions (for example, Python 3.12
# NumPy with a Python 3.9 project interpreter).
# Before re-injecting PYTHONPATH, strip any mismatched site-packages
# entries that leaked through _scrub_child_env (PYTHONPATH is in
# _SAFE_ENV_PREFIXES so it passes the scrub). Cross-version entries
# (e.g. python3.11 site-packages injected by systemd/Electron) would
# poison the sandbox's sys.path with ABI-incompatible C extensions
# (#74817); Hermes venv/repo-root entries are redundant because the
# correct ones are re-added below, gated on the child interpreter
# actually being the Hermes environment.
from tools.environments.local import _strip_mismatched_site_packages
_strip_mismatched_site_packages(child_env)
_hermes_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
_existing_pp = child_env.get("PYTHONPATH", "")
_pp_parts = [tmpdir]

View File

@@ -346,6 +346,10 @@ _HERMES_PROVIDER_ENV_BLOCKLIST = _build_provider_env_blocklist()
# to a different Python version overwrites it and breaks the gateway). The
# Hermes venv stays reachable via PATH (its bin dir is first), so stripping
# these markers is safe and only prevents the cross-project clobber (#23473).
#
# PYTHONPATH is NOT included here — it's handled by
# _strip_mismatched_site_packages() which surgically removes only site-packages
# paths that don't match the current Python ABI, preserving user-set entries.
_ACTIVE_VENV_MARKER_VARS = ("VIRTUAL_ENV", "CONDA_PREFIX")
@@ -506,6 +510,8 @@ def _sanitize_subprocess_env(base_env: dict | None, extra_env: dict | None = Non
for _marker in _ACTIVE_VENV_MARKER_VARS:
sanitized.pop(_marker, None)
_strip_mismatched_site_packages(sanitized)
_apply_windows_msys_bash_env_defaults(sanitized)
sanitized = _scrub_delegated_child_kanban_env(sanitized)
@@ -635,6 +641,8 @@ def hermes_subprocess_env(*, inherit_credentials: bool = False) -> dict[str, str
for _marker in _ACTIVE_VENV_MARKER_VARS:
env.pop(_marker, None)
_strip_mismatched_site_packages(env)
_apply_windows_msys_bash_env_defaults(env)
# Cross-session leak guard, same as the terminal spawn paths: this helper
@@ -1321,6 +1329,8 @@ def _make_run_env(env: dict) -> dict:
for _marker in _ACTIVE_VENV_MARKER_VARS:
run_env.pop(_marker, None)
_strip_mismatched_site_packages(run_env)
_apply_windows_msys_bash_env_defaults(run_env)
run_env = _scrub_delegated_child_kanban_env(run_env)
@@ -1328,6 +1338,222 @@ def _make_run_env(env: dict) -> dict:
return run_env
def _is_path_under(child: Path, parent: Path) -> bool:
"""Return True if *child* is the same as or under *parent*.
Uses ``os.path.normcase`` so the comparison is case-insensitive on
Windows (NTFS is case-insensitive by default) and case-sensitive on
POSIX, matching filesystem semantics. Paths are NOT resolved against
disk (no ``.resolve()``) so non-existent paths - common in test mocks
and in PYTHONPATH entries pointing at yet-to-be-created dirs - work
correctly. ``Path.resolve(strict=False)`` would also touch the
filesystem to resolve symlinks, which we deliberately avoid.
"""
c_parts = [os.path.normcase(p) for p in child.parts]
p_parts = [os.path.normcase(p) for p in parent.parts]
if len(c_parts) < len(p_parts):
return False
return c_parts[: len(p_parts)] == p_parts
# --- Hermes venv / repo-root detection (module-level, computed once) ---
#: The running interpreter's own venv root. On a venv Python ``sys.prefix``
#: points at the venv root (e.g. ``.../venv``); on a system Python it points
#: at ``/usr`` or similar. We only strip site-packages under this path when
#: the interpreter is actually inside a venv (``sys.prefix != sys.base_prefix``).
_hermes_venv_root: Path = Path(sys.prefix)
#: The Hermes repository root - two levels up from this file
#: (``tools/environments/local.py`` -> ``tools/`` -> repo root). This is the
#: directory the Electron app prepends to PYTHONPATH so the backend can do
#: ``import tools``, ``import hermes_cli``, etc. Subprocesses that are NOT
#: the Hermes backend don't need it and it can shadow local packages.
_hermes_repo_root: Path = Path(__file__).resolve().parents[1]
#: Whether the current interpreter is running inside a venv. On Python 3.3+
#: ``sys.base_prefix != sys.prefix`` indicates a venv (or virtualenv).
#: ``sys.real_prefix`` is the old virtualenv (<20) marker.
_in_venv: bool = (
getattr(sys, "base_prefix", sys.prefix) != sys.prefix
or hasattr(sys, "real_prefix")
)
#: Cached set of site-packages directories that belong to the running
#: interpreter's own venv. Computed lazily (once) because ``site`` import
#: and path construction are not free and this function is called on every
#: subprocess spawn.
_hermes_site_packages: list[Path] | None = None
def _get_hermes_site_packages() -> list[Path]:
"""Return the site-packages dirs of the running interpreter's venv.
Uses ``site.getsitepackages()`` when available for robustness (it respects
``.pth`` rewrites and platform conventions), with a manual fallback that
constructs the canonical path from ``sys.prefix`` for POSIX and Windows.
"""
global _hermes_site_packages
if _hermes_site_packages is not None:
return _hermes_site_packages
result: list[Path] = []
try:
import site
for sp in site.getsitepackages():
result.append(Path(sp))
except Exception:
pass
# Fallback: construct manually. On POSIX:
# sys.prefix / lib / python{X.Y} / site-packages
# On Windows:
# sys.prefix / Lib / site-packages
if not result:
if _IS_WINDOWS:
result.append(Path(sys.prefix) / "Lib" / "site-packages")
else:
pyver = f"python{sys.version_info[0]}.{sys.version_info[1]}"
result.append(Path(sys.prefix) / "lib" / pyver / "site-packages")
_hermes_site_packages = result
return result
# Regex to extract a Python version marker (e.g. ``python3.11``) from a path.
# Matches ``python3.11``, ``python3.13``, etc. as a path component - i.e.
# preceded by a path separator (``/`` or ``\``) or string start, and followed
# by a separator or string end. This is cross-platform: it works with both
# POSIX forward-slash paths and Windows backslash paths regardless of the
# host OS, so a POSIX host correctly detects version markers in Windows-style
# paths (important for testing and for edge cases like WSL).
_PYVER_IN_PATH_RE = re.compile(r"(?:^|[\\/])python(\d+)\.(\d+)(?:[\\/]|$)")
# Regex to detect ``site-packages`` as a path component (not a substring of
# a longer directory name). Same cross-platform separator handling.
_SITE_PACKAGES_RE = re.compile(r"(?:^|[\\/])site-packages(?:[\\/]|$)")
def _strip_mismatched_site_packages(env: dict) -> None:
"""Remove mismatched site-packages paths from PYTHONPATH.
The Desktop Electron process (and systemd units, gateway VBS launchers,
etc.) inject the Hermes venv's site-packages path (e.g.
``.../venv/lib/python3.11/site-packages``) into ``PYTHONPATH`` so the
Hermes backend can import its packages. When this ``PYTHONPATH`` leaks
into subprocesses running a **different** Python version (e.g. 3.13),
the 3.11 C extensions appear on ``sys.path`` ahead of the correct 3.13
versions and crash with ``ImportError`` (``PIL._imaging``,
``cryptography``, etc.).
Rather than stripping ``PYTHONPATH`` entirely - which would discard
legitimate user entries (Nix uses ``PYTHONPATH`` for plugin discovery,
users set it for custom library paths) - this function surgically
removes only the dangerous entries:
1. **Cross-version site-packages** - any entry whose path contains a
``python{X.Y}/site-packages`` component where ``{X.Y}`` differs from
the running interpreter's version. This catches ALL leak sources
(Electron, systemd, gateway scripts) with a single version check,
regardless of the venv path.
2. **Hermes venv site-packages** (no version marker or same-version) -
entries that live under the running interpreter's own venv
site-packages directory. These are redundant for subprocesses: the
Hermes backend discovers its packages via ``sys.path``, not via an
inherited env var. Only checked when running inside a venv.
3. **Hermes repo root** - the Electron app prepends the repo root
(parent of ``tools/``) to ``PYTHONPATH``. Subprocesses don't need
it and it can shadow local packages.
User ``PYTHONPATH`` entries (``/opt/my-lib``, Nix plugin paths, etc.)
are always preserved.
"""
pp = env.get("PYTHONPATH")
if not pp:
return
hermes_site_packages = _get_hermes_site_packages() if _in_venv else []
running_major = sys.version_info[0]
running_minor = sys.version_info[1]
kept: list[str] = []
stripped: list[str] = []
for entry in pp.split(os.pathsep):
entry = entry.strip()
if not entry:
continue
entry_path = Path(entry)
should_strip = False
# --- Check 1: cross-version site-packages ---
# Look for a ``python{X.Y}`` path component and compare its version
# against the running interpreter. If they differ, the entry's
# C extensions are ABI-incompatible - strip unconditionally.
# We search the full entry string (not ``entry_path.parts``) because
# ``Path.parts`` only splits on the host OS separator, so a Windows
# backslash path on a POSIX host would be a single un-split part.
m = _PYVER_IN_PATH_RE.search(entry)
if m:
entry_major = int(m.group(1))
entry_minor = int(m.group(2))
if (entry_major, entry_minor) != (running_major, running_minor):
should_strip = True
if should_strip:
stripped.append(entry)
continue
# --- Check 2: under Hermes venv site-packages ---
# The entry lives under the running interpreter's own venv
# site-packages. Redundant for subprocesses (they get their packages
# via sys.path, not PYTHONPATH) and a common leak vector.
# Use the regex (not ``entry_path.parts``) for cross-platform detection
# so Windows backslash paths are caught on a POSIX host.
if not should_strip and _SITE_PACKAGES_RE.search(entry):
for sp in hermes_site_packages:
if _is_path_under(entry_path, sp):
should_strip = True
break
if should_strip:
stripped.append(entry)
continue
# --- Check 3: Hermes repo root ---
# The Electron app prepends the repo root so ``import tools`` works
# in the backend. Subprocesses don't need it and it can shadow
# local packages of the same name.
if not should_strip and _is_path_under(entry_path, _hermes_repo_root):
# Only strip if the entry IS the repo root or a direct package
# dir under it (e.g. ``.../hermes-agent/tools``). Don't strip
# arbitrary user paths that happen to be nested deeper.
rel = entry_path.resolve() if entry_path.exists() else entry_path
try:
depth = len(rel.relative_to(_hermes_repo_root).parts)
except (ValueError, OSError):
depth = -1
if depth <= 1:
should_strip = True
if should_strip:
stripped.append(entry)
else:
kept.append(entry)
if kept:
env["PYTHONPATH"] = os.pathsep.join(kept)
else:
env.pop("PYTHONPATH", None)
if stripped:
logger.debug(
"Stripped mismatched/Hermes-venv site-packages from PYTHONPATH: %s",
stripped,
)
def _read_terminal_shell_init_config() -> tuple[list[str], bool]:
"""Return (shell_init_files, auto_source_bashrc) from config.yaml.