fix(windows): SSH ControlMaster gating + stop hijacking the user's python (#84452)
* 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 '--'.
This commit is contained in:
Teknium
@@ -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.
|
||||
"""
|
||||
|
||||
+27
-3
@@ -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(
|
||||
|
||||
@@ -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\\<you>\\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\\<you>\\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
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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\<you>\AppData\Local\hermes\hermes-agent\venv\Scripts\hermes.exe
|
||||
Get-Command hermes # should print C:\Users\<you>\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`).
|
||||
|
||||
Reference in New Issue
Block a user