Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 42 additions & 11 deletions tests/tools/test_local_env_blocklist.py
Original file line number Diff line number Diff line change
Expand Up @@ -386,20 +386,51 @@ def test_sane_path_includes_homebrew_sbin(self):
assert "/opt/homebrew/sbin" in _SANE_PATH

def test_make_run_env_appends_homebrew_on_minimal_path(self):
"""When PATH is minimal (no /usr/bin), _make_run_env should append
_SANE_PATH which now includes Homebrew dirs."""
from tools.environments.local import _make_run_env
"""When PATH is minimal, _make_run_env appends missing sane entries."""
from tools.environments.local import _SANE_PATH, _make_run_env
minimal_env = {"PATH": "/some/custom/bin"}
with patch.dict(os.environ, minimal_env, clear=True):
result = _make_run_env({})
assert "/opt/homebrew/bin" in result["PATH"]
assert "/opt/homebrew/sbin" in result["PATH"]
path_entries = result["PATH"].split(":")
assert path_entries[0] == "/some/custom/bin"
for entry in _SANE_PATH.split(":"):
assert entry in path_entries

def test_make_run_env_fills_missing_homebrew_when_usr_bin_present(self):
"""macOS launchd PATH can include /usr/bin while missing Homebrew."""
from tools.environments.local import _make_run_env
launchd_env = {"PATH": "/usr/local/bin:/usr/bin:/bin"}
with patch.dict(os.environ, launchd_env, clear=True):
result = _make_run_env({})
path_entries = result["PATH"].split(":")
assert "/opt/homebrew/bin" in path_entries
assert "/opt/homebrew/sbin" in path_entries

def test_make_run_env_does_not_duplicate_existing_sane_entries(self):
from tools.environments.local import _make_run_env
existing_env = {"PATH": "/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin"}
with patch.dict(os.environ, existing_env, clear=True):
result = _make_run_env({})
path_entries = result["PATH"].split(":")
assert path_entries.count("/opt/homebrew/bin") == 1
assert path_entries.count("/usr/local/bin") == 1
assert path_entries.count("/usr/bin") == 1

def test_make_run_env_leaves_windows_path_unchanged(self, monkeypatch):
from tools.environments import local as local_mod
from tools.environments.local import _make_run_env
windows_env = {"PATH": r"C:\Windows\System32;C:\Program Files\Git\bin"}
monkeypatch.setattr(local_mod, "_IS_WINDOWS", True)
with patch.dict(os.environ, windows_env, clear=True):
result = _make_run_env({})
assert result["PATH"] == windows_env["PATH"]

def test_make_run_env_does_not_duplicate_on_full_path(self):
"""When PATH already has /usr/bin, _make_run_env should not append."""
def test_make_run_env_preserves_windows_mixed_case_path_key(self, monkeypatch):
from tools.environments import local as local_mod
from tools.environments.local import _make_run_env
full_env = {"PATH": "/usr/bin:/bin"}
with patch.dict(os.environ, full_env, clear=True):
windows_env = {"Path": r"C:\Windows\System32;C:\Program Files\Git\bin"}
monkeypatch.setattr(local_mod, "_IS_WINDOWS", True)
with patch.object(local_mod.os, "environ", windows_env):
result = _make_run_env({})
# Should keep existing PATH unchanged
assert result["PATH"] == "/usr/bin:/bin"
assert result["Path"] == windows_env["Path"]
assert "PATH" not in result
14 changes: 8 additions & 6 deletions tests/tools/test_windows_native_support.py
Original file line number Diff line number Diff line change
Expand Up @@ -790,13 +790,15 @@ def test_source_has_windows_branch_using_hermes_home(self):


class TestLocalEnvironmentPathInjectionGated:
"""The /usr/bin PATH injection in _make_run_env must be POSIX-only."""
"""Sane PATH completion must stay POSIX-only."""

def test_source_gates_path_injection(self):
root = Path(__file__).resolve().parents[2]
source = (root / "tools" / "environments" / "local.py").read_text(encoding="utf-8")
# The fix wraps the injection in `if not _IS_WINDOWS`.
assert 'not _IS_WINDOWS and "/usr/bin" not in existing_path.split(":")' in source
def test_windows_path_is_left_unchanged(self, monkeypatch):
from tools.environments import local as local_mod
from tools.environments.local import _append_missing_sane_path_entries

monkeypatch.setattr(local_mod, "_IS_WINDOWS", True)
path = r"C:\Windows\System32;C:\Program Files\Git\bin"
assert _append_missing_sane_path_entries(path) == path


# ---------------------------------------------------------------------------
Expand Down
41 changes: 30 additions & 11 deletions tools/environments/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,33 @@ def _find_bash() -> str:
)


def _append_missing_sane_path_entries(existing_path: str) -> str:
"""Return PATH with each missing POSIX sane entry appended once."""
if _IS_WINDOWS:
return existing_path

sane_entries = [entry for entry in _SANE_PATH.split(":") if entry]
if not existing_path:
return ":".join(sane_entries)

existing_entries = existing_path.split(":")
existing_set = {entry for entry in existing_entries if entry}
missing_entries = [entry for entry in sane_entries if entry not in existing_set]
if not missing_entries:
return existing_path
return f"{existing_path}:{':'.join(missing_entries)}"


def _path_env_key(run_env: dict) -> str | None:
"""Return the PATH env key to update without altering Windows casing."""
if not _IS_WINDOWS:
return "PATH"
for key in run_env:
if key.upper() == "PATH":
return key
return None


def _make_run_env(env: dict) -> dict:
"""Build a run environment with a sane PATH and provider-var stripping."""
try:
Expand All @@ -314,17 +341,9 @@ def _make_run_env(env: dict) -> dict:
run_env[real_key] = v
elif k not in _HERMES_PROVIDER_ENV_BLOCKLIST or _is_passthrough(k):
run_env[k] = v
existing_path = run_env.get("PATH", "")
# The "/usr/bin not already present β†’ inject sane POSIX path" heuristic
# only makes sense on POSIX. On Windows the PATH separator is ";"
# (the split(":") above turns a full Windows PATH into a single
# unrecognisable chunk, which then triggers prepending POSIX paths
# to a Windows PATH β€” completely wrong). Skip the injection entirely
# on Windows; the native PATH already points at whatever shell
# Hermes is driving via _find_bash (Git Bash), and Git Bash itself
# prepends its MSYS2 /usr/bin equivalent via the shell-init files.
if not _IS_WINDOWS and "/usr/bin" not in existing_path.split(":"):
run_env["PATH"] = f"{existing_path}:{_SANE_PATH}" if existing_path else _SANE_PATH
path_key = _path_env_key(run_env)
if path_key is not None:
run_env[path_key] = _append_missing_sane_path_entries(run_env.get(path_key, ""))

_inject_context_hermes_home(run_env)

Expand Down
Loading