fix(tools): strip inherited PYTHONHOME + junction-aware Hermes-owned PYTHONPATH ownership (#82581 salvage) - #88285
Merged
Merged
Conversation
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.
The gateway runs inside its own venv; if its PYTHONHOME leaks into subprocesses (terminal commands, cron no_agent scripts, TTS providers), any child interpreter redirects its stdlib search to the Hermes venv and crashes with version-mismatch errors before importing anything. PYTHONHOME is now part of _ACTIVE_VENV_MARKER_VARS so all env builders (_make_run_env, _sanitize_subprocess_env, hermes_subprocess_env, and build_subprocess_env used by cron) drop it, consistent with Hermes' existing PYTHONHOME handling in managed_uv.py and sqlite_runtime.py. execute_code already scrubbed it via _SAFE_ENV_PREFIXES. Tests cover all four builders plus the marker constant.
Adversarial review of the previous two commits (and #78917 itself) found three ownership-boundary issues; this commit addresses them: 1. Repo direct-child over-strip (Finding A) No launcher injects <repo>/tools or another direct child as an independent PYTHONPATH entry - audited all four producers (Electron electron-main.mjs, gateway/run.py::_ensure_windows_gateway_venv_imports, cron/scheduler.py::_windows_cron_python_invocation, tui_gateway/host_supervisor.py). The depth<=1 rule deleted user paths that merely live under the repo directory; only the EXACT repo root is now stripped. 2. Windows junction/symlink alias (Finding B) The gateway launcher renders Hermes-owned paths under the configured HERMES_HOME spelling (gateway_windows.py::_preserve_hermes_home_path), which may be a junction to another drive, so it differs lexically from the resolved repo root. _hermes_repo_root_aliases now carries both the resolved and unresolved spellings; both are recognized as Hermes-owned. 3. Stale abstraction rename (Phase 4) _strip_mismatched_site_packages -> _strip_hermes_owned_pythonpath: the cross-version heuristic is gone, so the old name misdescribes the behavior (ownership-based, not version-based). Tests: direct-child now preserved; junction alias stripped (lexical pair monkeypatched); Windows-only real-semantics test added (POSIX test remains a safety test); mixed-ordering, duplicate-Hermes, and no-scrub PYTHONHOME contract tests added. Full file: 52 passed / 16 failed (identical failure set to base, all isolation-venv environment issues).
…ted inherited env Integration test for the #84500 + #82581 intersection: seeds a contaminated inherited PYTHONPATH (Hermes repo root + Hermes venv site-packages + user entries) through os.environ and drives execute_code to Popen. Asserts the staging tmpdir stays first, inherited Hermes site-packages never survive, the repo root is re-added exactly once for a same-env child (proving the inherited copy was stripped) and stays absent for an external-env child, and user entries survive in order.
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.
Confirmed on native Windows 11 with a real junction and the real startup chain: when the desktop/CLI spawns the backend with HERMES_HOME in the configured (lexical) spelling and --profile / sticky active_profile is in play, _apply_profile_override() re-homes HERMES_HOME through resolve_profile_env(), which resolves the junction under the platform default and returns the PHYSICAL spelling. tools.environments.local is imported after that mutation, so _hermes_repo_root_aliases is built from the physical home, the lexical repo-root spelling written into PYTHONPATH by the launcher (D:\hermes\hermes-agent) is not derivable, and the entry survives stripping (reproduced: cases --profile default / named / sticky active_profile / cross-drive junction all leave it in place; no-profile strips it). Two narrow changes, no heuristics, no new env vars: - hermes_cli/profiles.py::resolve_profile_env: when HERMES_HOME is set, the configured spelling IS the launch root (junction-transparent, physically identical dirs); keep it instead of re-deriving the native default. This is the same producer contract _preserve_hermes_home_path already follows. - tools/environments/local.py::_build_hermes_repo_root_aliases: when the configured home is a profile home (<root>/profiles/<name>), also derive the root spelling lexically (parent of the profiles component, same rule get_default_hermes_root uses) and run the exact-ownership mapping against it, so the launcher's lexical root is recovered after re-home without ever matching arbitrary descendants of HERMES_HOME. Regression test test_profile_rehome_keeps_junction_lexical_alias covers junction + profile re-home + inherited lexical PYTHONPATH end to end.
The junction fix made resolve_profile_env preserve the configured HERMES_HOME spelling as the launch root. Cover the four pre-existing resolution invariants so the spelling-preservation never regresses them: - root env + named profile -> <root>/profiles/<name> - profile-shaped env + named profile -> <root>/profiles/<name> (no nesting) - profile-shaped env + default -> <root> - custom root env never falls back to the platform default Plus existence/validation semantics (missing named profile still raises FileNotFoundError) and the unset-env fallback contract.
Second real-world topology reported and confirmed on native Windows 11: the repository itself is a cross-drive junction (D:\hermes\hermes-agent -> C:\...\hermes-agent) under a real HERMES_HOME directory. The editable import spelling resolves to the physical location, so _hermes_repo_root is physical while the launcher writes the lexical spelling into PYTHONPATH. The home-relative mapping cannot express a cross-drive link (commonpath raises on different drives), so the lexical repo root survives stripping; and with the repo alias missing, a lexical VIRTUAL_ENV (D:\hermes\hermes-agent\venv) also fails _validated_runtime_venv, so the venv site-packages survives too (uv-base gateway: both entries survive). Fix: after the existing home/profile-root mapping, try the single deterministic candidate <lexical root>/<repo dirname> for every trusted home candidate (configured home, plus the profile root when the configured home is a profile path) and accept it only when strict resolve proves it is the exact physical repo root (fail-closed: missing paths, real directories that are not the known repo, and unrelated spellings are never aliased). This also re-enables the VIRTUAL_ENV validation for lexical venv spellings, so uv-base gateway site-packages cleanup follows the repo alias. Tests: repo-level junction positive + negative control (same-named real directory preserved), profile-home + repo-level junction combination, lexical VIRTUAL_ENV validation after recovery (root + site-packages stripped, user entries kept), and a no-provenance lookalike preserved. The execute_code composition test now compares composed paths with os.path.normcase so a Windows case-only spelling difference (resolve() vs abspath() casing) can never fail the composition contract.
Same behavior, same coverage, less boilerplate (test file 1691 -> 1512 lines; PR diff unchanged in semantics). Production (mechanical only): - Extract _strip_hermes_owned_pythonpath_and_runtime_markers(): the three builders (_make_run_env, _sanitize_subprocess_env, hermes_subprocess_env) ran the identical strip-then-pop-markers sequence in the same order (ordering is load-bearing for VIRTUAL_ENV validation); the helper makes that explicit once instead of three times. Tests: - Non-owned preservation: 11 single-shape tests -> one parametrized matrix (user/Nix/other-version/python2.7/pythonX.Y-contained/raw spelling/empty component/empty PYTHONPATH) + one runtime-shaped matrix (other-version SP, venv-SP descendant, repo direct child, repo deep child). - Owned stripping: venv SP, repo root (independent parents[2] computation), duplicates, all-owned key removal, mixed ordering -> one matrix. - Builder integration: _make_run_env/_sanitize_subprocess_env/ hermes_subprocess_env venv-SP stripping -> one parametrized test; same for the four PYTHONHOME builders (incl. build_subprocess_env). - Junction: same-named non-owned negative control now covers both the configured-root location and an unrelated location; shared _physical_repo_root helper; profile resolution matrix (root->named, profile-shaped->named no nesting, profile-shaped->default, custom root). - Every independent proof preserved: home-level junction, repo-level junction, profile interaction, negative identity control, uv-base lexical VIRTUAL_ENV, validated/unrelated VIRTUAL_ENV, no-scrub escape hatch, #84500 same-env/external-env composition, PYTHONHOME removal, real Windows-only semantics, POSIX fail-closed backslash paths.
Shorter, single-source ownership explanation for _strip_hermes_owned_pythonpath (the code-level Check comments already carry the per-branch detail; the docstring only needs the contract).
Contributor
૮ >ﻌ< ა ci reviewran on 8bd33fd — chore: map contributor email for PYTHONHOME/junction salvage
|
alt-glitch
added
type/bug
This was referenced Aug 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Subprocess env builders now also strip inherited
PYTHONHOME(which redirects any child interpreter's stdlib search and crashes it, #75018), and the PYTHONPATH filter from #88182 is tightened to a strict ownership model: strip only entries proven Hermes-owned — including Windows junction/profile-re-home spellings of the repo root — while preserving every user-set entry verbatim, cross-version or not.Salvage of the delta half of #82581 by @Starfie1d1272 (11 commits, authorship preserved), which consolidated on top of the already-merged #78917/#61028 lineage. Independently verified on a real Windows 11 junction install by @vollegrewar (before/after repro of the cp311/cp314 numpy ABI crash).
Changes
tools/environments/local.py:PYTHONHOMEadded to_ACTIVE_VENV_MARKER_VARS;_strip_mismatched_site_packagesreplaced by_strip_hermes_owned_pythonpath— exact-ownership matching (repo root + validated runtime site-packages only; no cross-version heuristic, so a user'spython3.9/site-packagesentry is now preserved); repo-root alias set covers junction lexical spellings and profile re-home, fail-closed via strict resolve;VIRTUAL_ENVaccepted as provenance only when it validates as the exact<repo>/venvlayout with a realpyvenv.cfg.tools/code_execution_tool.py: sandbox path uses the renamed ownership filter.hermes_cli/profiles.py:resolve_profile_env()preserves the configured HERMES_HOME spelling through profile re-home so launcher lexical provenance survives.test_local_env_blocklist.pygrown to 76 tests (ownership, junction aliases, PYTHONHOME, Windows-portable separators); +5 profile-spelling invariants intest_profiles.py.Validation
import hermes_clivia venv python)Fixes the PYTHONHOME half of #24928 (@carterwayneskhizeine first reported the PYTHONHOME leak there). Completes #75018's PYTHONHOME vector. Salvaged from #82581 by @Starfie1d1272; follows #88182.
Infographic