From f20d16fbf168a3cb2b0814aa61addfba515102d3 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 12 Aug 2026 02:56:33 -0700 Subject: [PATCH] fix(windows): SSH ControlMaster gating + stop hijacking the user's python (#84452) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(windows): SSH ControlMaster gating + stop hijacking the user's python Two Windows environment-integrity fixes: 1. tools/environments/ssh.py (#73927): Windows OpenSSH has no Unix-domain-socket ControlMaster support, so unconditionally passing ControlPath/ControlMaster/ControlPersist failed EVERY tool call on a Windows-hosted ssh terminal backend with 'getsockname failed: Not a socket'. Gate the three multiplexing options behind a module-level _SSH_MULTIPLEX = (os.name != 'nt'); the scp upload path is gated the same way. On Windows the backend now works without connection pooling (each command a fresh connection); POSIX behavior is unchanged. The teardown 'ssh -O exit' is naturally inert because the socket never exists on Windows. 2. scripts/install.ps1 (#83797): the installer put the whole venv\Scripts directory on the user PATH, which contains python.exe / pythonw.exe / pip.exe and so silently hijacked the 'python' command in every terminal on the machine — unrelated projects started resolving python to Hermes' runtime interpreter. Now copy only the launchers (hermes.exe, hermes-acp.exe) into a dedicated $InstallDir\bin and put THAT on PATH. Existing installs are migrated: the legacy venv\Scripts entry is stripped from the user PATH on the next install/update. The new bin dir is under $InstallDir (…\hermes-agent), which the uninstall PATH sweep already matches via its \hermes-agent marker. Updated the stale hermes_cli/update_cmd.py docstring that described the old venv\Scripts-on-PATH layout. Tests: SSH ControlMaster gating pinned both directions (multiplex on → flags present; off → absent but BatchMode/StrictHostKeyChecking retained). install.ps1 parses clean via the PowerShell AST parser. * docs: update windows-native install docs for the bin\ launcher layout CI (test_windows_native_docs) pins the docs and installer to the same PATH layout. The #83797 fix moved the PATH entry from venv\Scripts to a dedicated $InstallDir\bin holding only the hermes launchers, so update the Windows-native guide to match: PATH-after-install section, the install-steps list, the directory-layout table, the Get-Command verification line, and the 'command not found' pitfall. Test now asserts the bin\ layout and guards against a regression back to venv\Scripts on PATH. * fix: keep install.ps1 pure ASCII (PowerShell 5.1 codepage safety) The two comments I added in the #83797 PATH-hijack fix used em-dashes, tripping tests/test_install_ps1_ascii_only.py — Windows PowerShell 5.1 reads a BOM-less .ps1 in the system ANSI codepage (not UTF-8), so a non-ASCII byte can misdecode into a stray quote and desync the parser (issues #66994/#67000). Replace the em-dashes with ASCII '--'. --- hermes_cli/update_cmd.py | 6 ++-- scripts/install.ps1 | 30 ++++++++++++++++++-- tests/hermes_cli/test_windows_native_docs.py | 15 ++++++++-- tests/tools/test_ssh_environment.py | 21 +++++++++++++- tools/environments/ssh.py | 17 ++++++++--- website/docs/user-guide/windows-native.md | 10 +++---- 6 files changed, 81 insertions(+), 18 deletions(-) diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 5b810d04ab..78eed1d2c6 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -2522,8 +2522,10 @@ def _ensure_acp_launcher() -> None: (venv wrapper, FHS symlink, pipx/pip console script) without having to reconstruct interpreter/entrypoint paths. - No-op on Windows (install.ps1 puts ``venv\\Scripts`` on the user PATH, so - ``hermes-acp.exe`` already resolves) and wherever a ``hermes-acp`` is + No-op on Windows (install.ps1 copies ``hermes.exe`` + ``hermes-acp.exe`` + into ``$InstallDir\bin`` and puts THAT on the user PATH — never the whole + ``venv\Scripts`` dir, which would shadow the user's ``python`` (#83797) — + so ``hermes-acp.exe`` already resolves) and wherever a ``hermes-acp`` is already present next to the ``hermes`` command. Unwritable directories (e.g. ``/usr/local/bin`` as non-root) are skipped silently. Idempotent. """ diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 90a7b2c1ef..6ca4de190a 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -2715,12 +2715,36 @@ function Set-PathVariable { if ($NoVenv) { $hermesBin = "$InstallDir" } else { - $hermesBin = "$InstallDir\venv\Scripts" + # Expose ONLY the hermes launchers on PATH -- never the whole + # venv\Scripts directory. venv\Scripts contains python.exe / + # pythonw.exe / pip.exe, and putting it on the user PATH silently + # hijacks the `python` command in every terminal on the machine + # (#83797): unrelated projects start resolving python to Hermes' + # runtime interpreter. A dedicated bin dir with copies of the + # launcher exes keeps `hermes` globally available without + # shadowing anything. (Launcher exes embed the venv interpreter + # path, so they work from any location and survive updates.) + $hermesBin = "$InstallDir\bin" + New-Item -ItemType Directory -Force -Path $hermesBin | Out-Null + foreach ($launcher in @("hermes.exe", "hermes-acp.exe")) { + $src = "$InstallDir\venv\Scripts\$launcher" + if (Test-Path $src) { + Copy-Item -Force $src "$hermesBin\$launcher" + } + } } - # Add the venv Scripts dir to user PATH so hermes is globally available - # On Windows, the hermes.exe in venv\Scripts\ has the venv Python baked in $currentPath = [Environment]::GetEnvironmentVariable("Path", "User") + + # Migrate installs that got venv\Scripts onto PATH from earlier + # installer versions -- remove it so the python shadowing stops. + $legacyBin = "$InstallDir\venv\Scripts" + if ((-not $NoVenv) -and $currentPath -like "*$legacyBin*") { + $cleaned = ($currentPath -split ';' | Where-Object { $_ -and $_ -ne $legacyBin }) -join ';' + [Environment]::SetEnvironmentVariable("Path", $cleaned, "User") + $currentPath = $cleaned + Write-Info "Removed legacy venv\Scripts from user PATH (kept hermes via $hermesBin)" + } if ($currentPath -notlike "*$hermesBin*") { [Environment]::SetEnvironmentVariable( diff --git a/tests/hermes_cli/test_windows_native_docs.py b/tests/hermes_cli/test_windows_native_docs.py index 10d52394b9..6e53ff7a0f 100644 --- a/tests/hermes_cli/test_windows_native_docs.py +++ b/tests/hermes_cli/test_windows_native_docs.py @@ -5,6 +5,15 @@ def test_windows_native_install_path_docs_match_installer() -> None: doc = Path("website/docs/user-guide/windows-native.md").read_text() install = Path("scripts/install.ps1").read_text() - assert "%LOCALAPPDATA%\\hermes\\hermes-agent\\venv\\Scripts" in doc - assert "Get-Command hermes # should print C:\\Users\\\\AppData\\Local\\hermes\\hermes-agent\\venv\\Scripts\\hermes.exe" in doc - assert '$hermesBin = "$InstallDir\\venv\\Scripts"' in install + # The launchers live in a dedicated bin/ dir on PATH — NOT the whole + # venv\Scripts (which would shadow the user's python, #83797). + assert "%LOCALAPPDATA%\\hermes\\hermes-agent\\bin" in doc + assert ( + "Get-Command hermes # should print " + "C:\\Users\\\\AppData\\Local\\hermes\\hermes-agent\\bin\\hermes.exe" + ) in doc + # Installer exposes $InstallDir\bin, and must copy the launchers into it. + assert '$hermesBin = "$InstallDir\\bin"' in install + assert "hermes.exe" in install and "hermes-acp.exe" in install + # Guard against a regression back to putting venv\Scripts on PATH. + assert '$hermesBin = "$InstallDir\\venv\\Scripts"' not in install diff --git a/tests/tools/test_ssh_environment.py b/tests/tools/test_ssh_environment.py index f4efabad94..6d4dc4f7d4 100644 --- a/tests/tools/test_ssh_environment.py +++ b/tests/tools/test_ssh_environment.py @@ -45,13 +45,32 @@ class TestBuildSSHCommand: stdin=MagicMock())) monkeypatch.setattr("tools.environments.base.time.sleep", lambda _: None) - def test_base_flags(self): + def test_base_flags(self, monkeypatch): + # ControlMaster flags are POSIX-only (#73927): assert them only + # where multiplexing is enabled so the test passes on Windows too. + monkeypatch.setattr(ssh_env, "_SSH_MULTIPLEX", True) env = SSHEnvironment(host="h", user="u") cmd = " ".join(env._build_ssh_command()) for flag in ("ControlMaster=auto", "ControlPersist=300", "BatchMode=yes", "StrictHostKeyChecking=accept-new"): assert flag in cmd + def test_controlmaster_gated_off_on_windows(self, monkeypatch): + """#73927: Windows OpenSSH has no Unix-domain ControlMaster, so the + ControlPath/ControlMaster/ControlPersist options must be omitted — + passing them fails the connection with 'getsockname failed'.""" + monkeypatch.setattr(ssh_env, "_SSH_MULTIPLEX", False) + env = SSHEnvironment(host="h", user="u") + cmd = " ".join(env._build_ssh_command()) + assert "ControlMaster" not in cmd + assert "ControlPath" not in cmd + assert "ControlPersist" not in cmd + # Non-multiplex flags must still be present — the backend works, + # just without connection pooling. + assert "BatchMode=yes" in cmd + assert "StrictHostKeyChecking=accept-new" in cmd + assert env._build_ssh_command()[-1] == "u@h" + def test_user_host_suffix(self): env = SSHEnvironment(host="h", user="u") diff --git a/tools/environments/ssh.py b/tools/environments/ssh.py index 9fff7c95cf..1260c070b0 100644 --- a/tools/environments/ssh.py +++ b/tools/environments/ssh.py @@ -3,6 +3,12 @@ import hashlib import logging import os + +# Windows OpenSSH has no Unix-domain-socket ControlMaster support — +# passing ControlPath/ControlMaster options fails the connection outright +# ('getsockname failed: Not a socket', #73927). Skip multiplexing there; +# each command pays a fresh connection but the backend works. +_SSH_MULTIPLEX = os.name != "nt" import shlex import shutil import subprocess @@ -86,9 +92,10 @@ class SSHEnvironment(BaseEnvironment): def _build_ssh_command(self, extra_args: list | None = None) -> list: cmd = ["ssh"] - cmd.extend(["-o", f"ControlPath={self.control_socket}"]) - cmd.extend(["-o", "ControlMaster=auto"]) - cmd.extend(["-o", "ControlPersist=300"]) + if _SSH_MULTIPLEX: + cmd.extend(["-o", f"ControlPath={self.control_socket}"]) + cmd.extend(["-o", "ControlMaster=auto"]) + cmd.extend(["-o", "ControlPersist=300"]) cmd.extend(["-o", "BatchMode=yes"]) cmd.extend(["-o", "StrictHostKeyChecking=accept-new"]) cmd.extend(["-o", "ConnectTimeout=10"]) @@ -186,7 +193,9 @@ class SSHEnvironment(BaseEnvironment): stdin=subprocess.DEVNULL, ) - scp_cmd = ["scp", "-o", f"ControlPath={self.control_socket}"] + scp_cmd = ["scp"] + if _SSH_MULTIPLEX: + scp_cmd.extend(["-o", f"ControlPath={self.control_socket}"]) if self.port != 22: scp_cmd.extend(["-P", str(self.port)]) if self.key_path: diff --git a/website/docs/user-guide/windows-native.md b/website/docs/user-guide/windows-native.md index de369fc08a..4e0a48c0b0 100644 --- a/website/docs/user-guide/windows-native.md +++ b/website/docs/user-guide/windows-native.md @@ -75,7 +75,7 @@ Top-to-bottom, in order: 6. **Tiered `uv pip install`** — tries `.[all]` first, falls back to progressively smaller sets (`[messaging,dashboard,ext]` → `[messaging]` → `.`) if a `git+https` dep flakes on rate-limited GitHub. Prevents "single flake drops you to a bare install" failure mode. 7. **Auto-installs messaging SDKs** keyed off `.env` — if `TELEGRAM_BOT_TOKEN` / `DISCORD_BOT_TOKEN` / `SLACK_BOT_TOKEN` / `SLACK_APP_TOKEN` / `WHATSAPP_ENABLED` are present, runs `python -m ensurepip --upgrade` and targeted `pip install` calls so each platform's SDK is actually importable. 8. **Sets `HERMES_GIT_BASH_PATH`** to the resolved `bash.exe` so Hermes finds it deterministically in fresh shells. -9. **Adds `%LOCALAPPDATA%\hermes\hermes-agent\venv\Scripts` to User PATH and sets `HERMES_HOME=%LOCALAPPDATA%\hermes`** — exposes the `hermes` command (and points it at your data dir) after you open a new terminal. +9. **Adds `%LOCALAPPDATA%\hermes\hermes-agent\bin` to User PATH and sets `HERMES_HOME=%LOCALAPPDATA%\hermes`** — exposes the `hermes` command (and points it at your data dir) after you open a new terminal. Only the `hermes.exe` / `hermes-acp.exe` launchers are copied into this `bin` directory; the full `venv\Scripts` is deliberately **not** placed on PATH so Hermes never shadows your own `python` command. 10. **Runs `hermes setup`** — the normal first-run wizard (model, provider, toolsets). Skip with `-SkipSetup`. :::tip Skip provider hunting on Windows @@ -202,7 +202,7 @@ Services require admin rights to install and tie the gateway's lifecycle to mach | Path | Contents | |---|---| -| `%LOCALAPPDATA%\hermes\hermes-agent\` | Git checkout + venv. `venv\Scripts\hermes.exe` is the command added to User PATH. Safe to `Remove-Item -Recurse` and reinstall. | +| `%LOCALAPPDATA%\hermes\hermes-agent\` | Git checkout + venv. The `bin\hermes.exe` launcher (copied from `venv\Scripts\hermes.exe`) is the command added to User PATH. Safe to `Remove-Item -Recurse` and reinstall. | | `%LOCALAPPDATA%\hermes\git\` | PortableGit (only if the installer provisioned it). | | `%LOCALAPPDATA%\hermes\node\` | Portable Node.js (only if the installer provisioned it). | | `%LOCALAPPDATA%\hermes\bin\` | Hermes's managed `uv.exe` (the Python manager it uses for updates). | @@ -224,12 +224,12 @@ The browser tool uses `agent-browser` (a Node helper) to drive Chromium. On Wind ### PATH after install -The installer adds `%LOCALAPPDATA%\hermes\hermes-agent\venv\Scripts` to your **User PATH** via `[Environment]::SetEnvironmentVariable`. Existing terminals don't pick this up — open a new PowerShell window (or Windows Terminal tab) after installation. Close-and-reopen, don't `$env:PATH += …` by hand unless you know what you're doing. +The installer adds `%LOCALAPPDATA%\hermes\hermes-agent\bin` to your **User PATH** via `[Environment]::SetEnvironmentVariable`. Existing terminals don't pick this up — open a new PowerShell window (or Windows Terminal tab) after installation. Close-and-reopen, don't `$env:PATH += …` by hand unless you know what you're doing. Verify: ```powershell -Get-Command hermes # should print C:\Users\\AppData\Local\hermes\hermes-agent\venv\Scripts\hermes.exe +Get-Command hermes # should print C:\Users\\AppData\Local\hermes\hermes-agent\bin\hermes.exe hermes --version ``` @@ -288,7 +288,7 @@ Consequence: any codepath that said "check if this PID is alive" via `os.kill(pi ## Common pitfalls **`hermes: command not found` right after install.** -Open a new PowerShell window. The installer added `%LOCALAPPDATA%\hermes\hermes-agent\venv\Scripts` to User PATH, but existing shells need to be restarted to pick it up. In the meantime you can run `& "$env:LOCALAPPDATA\hermes\hermes-agent\venv\Scripts\hermes.exe"`. +Open a new PowerShell window. The installer added `%LOCALAPPDATA%\hermes\hermes-agent\bin` to User PATH, but existing shells need to be restarted to pick it up. In the meantime you can run `& "$env:LOCALAPPDATA\hermes\hermes-agent\bin\hermes.exe"`. **`WinError 193: %1 is not a valid Win32 application` when running a tool.** You hit a shebang-script invocation that bypassed the `.cmd` shim. Hermes resolves commands through `shutil.which(cmd, path=local_bin)` so PATHEXT picks up `.CMD` — if you're invoking the tool via a hardcoded path instead, switch to the `.cmd` variant (e.g., `npx.cmd`, not `npx`).