ouyangbo/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 10 Packages Projects Releases Wiki Activity

fix(tools): preserve user PYTHONPATH entries (#74817 follow-up)

Browse Source 
Remove the cross-version heuristic from _strip_mismatched_site_packages:
the subprocess env builder cannot know which Python version a child will
run, so judging user PYTHONPATH entries against the backend interpreter's
version deletes legitimate paths meant for a different child Python
(e.g. /custom/lib/python3.13/site-packages while Hermes runs 3.11).

Also fix over-strip: entries merely containing a pythonX.Y path component
(e.g. /opt/tools/python3.13/bin) were stripped even though they are not
site-packages. Hermes-owned entries (repo root, own venv site-packages)
are now identified by path ownership, not by version.

Regression tests cover both cases; user paths with any pythonX.Y
component are preserved.
This commit is contained in:
Xinyu Du
2026-08-09 21:48:39 +08:00
committed by Teknium
parent 23a86594cc
commit ced80b2a20
2 changed files with 111 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/tools/test_local_env_blocklist.py
+81 -48
View File
@@ -315,13 +315,16 @@ class TestActiveVenvMarkerStripping:
class TestPythonpathSelectiveStrip:
"""PYTHONPATH site-packages stripping for cross-version ABI safety (#74817).
"""PYTHONPATH Hermes-owned entry stripping (#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.
The Desktop Electron app injects the Hermes repo root and 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
entries Hermes itself owns (repo root, own venv site-packages),
preserving user paths — including user paths whose names merely
contain another Python version.
"""
def test_hermes_venv_site_packages_stripped(self):
@@ -330,8 +333,8 @@ class TestPythonpathSelectiveStrip:
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).
# Use the running interpreter's version so it hits the Hermes-venv
# ownership check, not a user-path case.
pyver = f"python{sys.version_info[0]}.{sys.version_info[1]}"
venv_sp = str(
__import__("pathlib").Path(sys.prefix) / "lib" / pyver / "site-packages"
@@ -353,65 +356,92 @@ class TestPythonpathSelectiveStrip:
_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."""
def test_other_version_site_packages_preserved(self):
"""A user's pythonX.Y/site-packages entry is preserved even when its
version differs from the Hermes backend interpreter.
The env builder cannot know which Python the child will run, so a
user path intended for a child of another version must never be
judged against the BACKEND's interpreter version (P2, #74817
follow-up). Regression: prior Check 1 stripped these.
"""
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"
other_sp = f"/opt/other-venv/lib/{other_ver}/site-packages"
env = {
"PYTHONPATH": os.pathsep.join([mismatched_sp, "/home/user/my-lib"]),
"PYTHONPATH": os.pathsep.join([other_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 other_sp 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."""
def test_other_major_version_site_packages_preserved(self):
"""A user's python2.7/site-packages entry is preserved — path
ownership, not version, decides stripping (P2, #74817 follow-up).
"""
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)
assert "PYTHONPATH" in env
entries = env["PYTHONPATH"].split(os.pathsep)
assert "/old/lib/python2.7/site-packages" not in entries
assert "/old/lib/python2.7/site-packages" in entries
assert "/home/user/lib" in entries
def test_non_site_packages_python_version_paths_preserved(self):
"""Paths merely CONTAINING a pythonX.Y component (not site-packages)
must never be stripped (P1, #74817 follow-up). Regression: prior
Check 1 deleted these because it keyed on the version component alone.
"""
from tools.environments.local import _strip_mismatched_site_packages
user_pp = os.pathsep.join([
"/opt/tools/python3.13/bin",
"/opt/downloads/python3.13",
"/custom/python3.13",
])
env = {"PYTHONPATH": user_pp}
_strip_mismatched_site_packages(env)
assert env.get("PYTHONPATH") == user_pp
def test_windows_backslash_paths(self):
"""Windows-style backslash paths with site-packages are handled.
"""Windows-style backslash paths are handled for Hermes-owned entries.
On Windows, os.pathsep is ';'. We mock it so the test runs
correctly on POSIX CI."""
correctly on POSIX CI. On a POSIX host a backslash path is a
single path component, so _is_path_under cannot identify it as
Hermes-owned — the critical invariant is that user Windows paths
(including site-packages paths for another Python version) are
never destroyed. On a real Windows host, Path splits on
backslashes and Hermes venv site-packages entries are stripped
by the same Hermes-owned check.
"""
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"
pyver = f"python{sys.version_info[0]}.{sys.version_info[1]}"
hermes_win = f"C:\\\\Users\\\\u\\\\.hermes\\\\hermes-agent\\\\venv\\\\lib\\\\{pyver}\\\\site-packages"
user_win = "D:\\\\user\\\\lib"
env = {
"PYTHONPATH": ";".join([mismatched_win, user_win]),
"PYTHONPATH": ";".join([hermes_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
# Both survive on POSIX: user paths must always be preserved, and
# the Hermes-owned check cannot match a backslash path here.
assert hermes_win in entries
assert user_win in entries
def test_empty_pythonpath_unchanged(self):
@@ -431,16 +461,17 @@ class TestPythonpathSelectiveStrip:
assert "PYTHONPATH" not in env
def test_all_entries_stripped_removes_key(self):
"""If all entries are stripped, PYTHONPATH key is removed entirely."""
"""If all entries are Hermes-owned and 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}"
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": f"/a/lib/{other_ver}/site-packages"}
env = {"PYTHONPATH": venv_sp}
_strip_mismatched_site_packages(env)
assert "PYTHONPATH" not in env
@@ -503,24 +534,25 @@ class TestPythonpathSelectiveStrip:
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)."""
def test_scrub_child_env_strips_hermes_venv_pythonpath(self):
"""execute_code's _scrub_child_env path: after scrubbing, Hermes venv
site-packages entries should be stripped when
_strip_mismatched_site_packages is applied (as the spawn path does),
while user entries (even for another Python version) are preserved.
"""
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"
pyver = f"python{sys.version_info[0]}.{sys.version_info[1]}"
venv_sp = str(
__import__("pathlib").Path(sys.prefix) / "lib" / pyver / "site-packages"
)
other_sp = "/opt/other-venv/lib/python3.99/site-packages"
source = {
"PATH": "/usr/bin",
"HOME": "/home/user",
"PYTHONPATH": os.pathsep.join([mismatched_sp, "/home/user/my-lib"]),
"PYTHONPATH": os.pathsep.join([venv_sp, other_sp, "/home/user/my-lib"]),
}
scrubbed = _scrub_child_env(source)
# The scrubber passes PYTHONPATH through (it's in _SAFE_ENV_PREFIXES).
@@ -529,7 +561,8 @@ class TestPythonpathSelectiveStrip:
_strip_mismatched_site_packages(scrubbed)
pp = scrubbed.get("PYTHONPATH", "")
entries = pp.split(os.pathsep) if pp else []
assert mismatched_sp not in entries
assert venv_sp not in entries
assert other_sp in entries
assert "/home/user/my-lib" in entries
def test_repo_root_stripped(self):
tools/environments/local.py
+30 -58
View File
@@ -1421,63 +1421,52 @@ def _get_hermes_site_packages() -> list[Path]:
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.
"""Remove Hermes-owned PYTHONPATH entries from subprocess environments.
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.).
The Desktop Electron process (and other Hermes launchers) prepend the
Hermes repo root and the Hermes venv's ``site-packages`` to ``PYTHONPATH``
so the backend can ``import tools`` / ``import hermes_cli``. When that
``PYTHONPATH`` leaks into subprocesses, a child Python of a DIFFERENT
version (e.g. 3.13 vs the backend's 3.11) picks up the 3.11 C extensions
from ``sys.path`` ahead of its own and crashes with ``ImportError``
(``PIL._imaging``, ``cryptography``, ``numpy._core._multiarray_umath``,
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:
removes only the entries Hermes itself owns:
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.
1. **Hermes repo root** - the path the Electron app prepends so the
backend can ``import tools``. Subprocesses don't need it and it can
shadow local packages of the same name.
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.
2. **Hermes venv site-packages** - entries under the running
interpreter's own venv site-packages directory. Redundant for
subprocesses (they get their packages via ``sys.path``, not an
inherited env var) and a common leak vector. 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.
User ``PYTHONPATH`` entries (``/opt/my-lib``, Nix plugin paths, a
``/custom/lib/python3.13/site-packages`` intended for a child Python 3.13)
are always preserved. In particular we deliberately do NOT apply a
cross-version heuristic here: the subprocess env builder cannot know
which Python version the child will ultimately run, so judging a user
path against the BACKEND's interpreter version would delete legitimate
user entries meant for a different child Python (#74817 follow-up).
Hermes-owned entries are identified by path ownership, not by version.
"""
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] = []
@@ -1490,30 +1479,13 @@ def _strip_mismatched_site_packages(env: dict) -> None:
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 ---
# --- Check 1: 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):
if _SITE_PACKAGES_RE.search(entry):
for sp in hermes_site_packages:
if _is_path_under(entry_path, sp):
should_strip = True
@@ -1522,7 +1494,7 @@ def _strip_mismatched_site_packages(env: dict) -> None:
stripped.append(entry)
continue
# --- Check 3: Hermes repo root ---
# --- Check 2: 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.
@@ -1550,7 +1522,7 @@ def _strip_mismatched_site_packages(env: dict) -> None:
if stripped:
logger.debug(
"Stripped mismatched/Hermes-venv site-packages from PYTHONPATH: %s",
"Stripped Hermes-owned entries from PYTHONPATH: %s",
stripped,
)