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

test(tools): make subprocess env regressions Windows-portable

Browse Source 
The PYTHONPATH/PATH sanitization suite was written POSIX-centric and
failed on real Windows 11 (reproduced natively: 4 failures before this
change).  Fix the tests to express the true per-platform contract:

- test_other_major_version_site_packages_preserved /
  test_make_run_env_injects_hermes_bin_dir: build inputs with
  os.pathsep instead of hardcoded ':'.
- test_make_run_env_appends_homebrew_on_minimal_path: split on
  os.pathsep, neutralise Git Bash dir prepending, and assert the
  documented Windows passthrough (_append_missing_sane_path_entries is
  a no-op off POSIX) instead of the Homebrew append.
- test_make_run_env_real_launchd_path_gains_homebrew: mark
  macos_only per repo OS-marker policy (the regression is the macOS
  launchd PATH; the merge is a passthrough on Windows).
- test_configured_home_alias_matches_launcher_output: create the
  configured-home link via a helper that falls back to an unprivileged
  directory junction (cmd /c mklink /J) when symlink creation raises
  WinError 1314, and skips with a clear reason if no mechanism exists.

Also correct a stale comment in execute_code: the child is not always
the same Python as Hermes (project mode can select an external venv),
so the strip is about compatibility, not redundancy.
This commit is contained in:
Xinyu Du
2026-08-14 00:52:56 +08:00
committed by Teknium
parent ba85da4898
commit deb4953776
2 changed files with 70 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/tools/test_local_env_blocklist.py
+66 -11
View File
@@ -9,6 +9,7 @@ See: https://github.com/NousResearch/hermes-agent/issues/1264
"""
import os
import subprocess
import sys
import threading
from pathlib import Path
@@ -323,6 +324,33 @@ class TestActiveVenvMarkerStripping:
assert "CONDA_PREFIX" in _ACTIVE_VENV_MARKER_VARS
def _make_directory_link(link: Path, target: Path) -> None:
"""Create a directory link without requiring symlink privileges.
POSIX: Path.symlink_to. Windows: try symlink_to first (works with
Developer Mode enabled), then fall back to an unprivileged directory
junction via `cmd /c mklink /J` -- junctions do not require the
SeCreateSymbolicLinkPrivilege. Raises the original error when no
mechanism is available so callers can skip with a clear reason.
"""
try:
link.symlink_to(target, target_is_directory=True)
return
except OSError:
if sys.platform != "win32":
raise
# Binary capture: on a localized Windows the junction message is in the
# console code page (e.g. GBK), which would raise UnicodeDecodeError in
# the reader thread under UTF-8 mode. Only the exit code matters.
result = subprocess.run(
["cmd", "/c", "mklink", "/J", str(link), str(target)],
capture_output=True,
)
if result.returncode != 0:
detail = result.stderr.decode("utf-8", errors="replace").strip()
raise OSError(detail or f"mklink /J failed: {result.returncode}")
class TestPythonpathSelectiveStrip:
"""PYTHONPATH Hermes-owned entry stripping (#74817).
@@ -418,7 +446,10 @@ class TestPythonpathSelectiveStrip:
"""
from tools.environments.local import _strip_hermes_owned_pythonpath
env = {
"PYTHONPATH": "/old/lib/python2.7/site-packages:/home/user/lib",
"PYTHONPATH": os.pathsep.join([
"/old/lib/python2.7/site-packages",
"/home/user/lib",
]),
}
_strip_hermes_owned_pythonpath(env)
assert "PYTHONPATH" in env
@@ -865,7 +896,10 @@ class TestPythonpathSelectiveStrip:
physical_root = physical_home / "hermes-agent"
physical_root.mkdir(parents=True)
configured_home = tmp_path / "configured-home"
configured_home.symlink_to(physical_home, target_is_directory=True)
try:
_make_directory_link(configured_home, physical_home)
except OSError as exc:
pytest.skip(f"directory link unavailable on this host: {exc}")
monkeypatch.setenv("HERMES_HOME", str(configured_home))
launcher_entry = Path(_preserve_hermes_home_path(physical_root))
@@ -1217,25 +1251,42 @@ class TestSanePathIncludesHomebrew:
assert "/opt/homebrew/bin" in _SANE_PATH
def test_make_run_env_appends_homebrew_on_minimal_path(self):
"""When PATH is minimal, _make_run_env appends missing sane entries."""
def test_make_run_env_appends_homebrew_on_minimal_path(self, monkeypatch):
"""When PATH is minimal, _make_run_env appends missing sane entries.
POSIX: the sane-path merge appends the Homebrew dirs. Windows:
_append_missing_sane_path_entries is a documented passthrough (the
native PATH must not be touched), so the assertion is the unchanged
input. Git Bash dir prepending is neutralised so the merged PATH
layout is deterministic on every host.
"""
from tools.environments import local as local_mod
from tools.environments.local import _SANE_PATH, _make_run_env
monkeypatch.setattr(local_mod, "_git_bash_bin_dirs", lambda: [])
minimal_env = {"PATH": "/some/custom/bin"}
with patch.dict(os.environ, minimal_env, clear=True):
result = _make_run_env({})
path_entries = result["PATH"].split(":")
path_entries = result["PATH"].split(os.pathsep)
assert path_entries[0] == "/some/custom/bin"
for entry in _SANE_PATH.split(":"):
assert entry in path_entries
if sys.platform == "win32":
assert result["PATH"] == "/some/custom/bin"
else:
for entry in _SANE_PATH.split(os.pathsep):
assert entry in path_entries
@pytest.mark.macos_only
def test_make_run_env_real_launchd_path_gains_homebrew(self):
"""The literal macOS launchd PATH is the production trigger for #35613."""
"""The literal macOS launchd PATH is the production trigger for #35613.
macOS-only: the regression is the launchd environment on macOS, and
the sane-path merge is a documented passthrough on Windows.
"""
from tools.environments.local import _make_run_env
launchd_env = {"PATH": "/usr/bin:/bin:/usr/sbin:/sbin"}
launchd_env = {"PATH": os.pathsep.join(["/usr/bin", "/bin", "/usr/sbin", "/sbin"])}
with patch.dict(os.environ, launchd_env, clear=True):
result = _make_run_env({})
path_entries = result["PATH"].split(":")
path_entries = result["PATH"].split(os.pathsep)
assert "/opt/homebrew/bin" in path_entries
assert "/opt/homebrew/sbin" in path_entries
# Original entries keep their leading precedence.
@@ -1298,7 +1349,11 @@ class TestHermesBinDirOnPath:
from tools.environments.local import _make_run_env
self._reset_cache()
local_mod._HERMES_BIN_DIR = "/opt/hermes/bin"
with patch.dict(os.environ, {"PATH": "/usr/bin:/bin"}, clear=True):
with patch.dict(
os.environ,
{"PATH": os.pathsep.join(["/usr/bin", "/bin"])},
clear=True,
):
result = _make_run_env({})
entries = result["PATH"].split(os.pathsep)
assert entries[0] == "/opt/hermes/bin"
tools/code_execution_tool.py
+4 -4
View File
@@ -1486,10 +1486,10 @@ def execute_code(
#
# Before re-injecting PYTHONPATH, strip Hermes-owned entries that
# leaked through _scrub_child_env (PYTHONPATH is in _SAFE_ENV_PREFIXES
# so it passes the scrub). The sandbox runs the SAME Python as
# Hermes, so the Hermes venv entries are redundant — and if they
# came from a different Hermes venv they would poison the sandbox's
# sys.path with ABI-incompatible C extensions (#74817).
# so it passes the scrub). They are redundant for same-Hermes-
# environment children and may be incompatible with external
# interpreters (project mode can select a different venv), so they
# must not shadow or poison the child's sys.path (#74817).
from tools.environments.local import _strip_hermes_owned_pythonpath
_strip_hermes_owned_pythonpath(child_env)
_hermes_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))