fix(tools): keep SSH ControlMaster socket path under macOS 104-byte limit
On macOS, Unix domain socket paths are capped at 104 bytes (sun_path). SSH appends a 16-byte random suffix to the ControlPath when operating in ControlMaster mode. With an IPv6 host embedded literally in the filename and a deeply-nested macOS $TMPDIR like /var/folders/XX/YYYYYYYYYYYY/T/, the full path reliably exceeds the limit — every terminal/file-op tool call then fails immediately with ``unix_listener: path "…" too long for Unix domain socket``. Swap the ``user@host:port.sock`` filename for a sha256-derived 16-char hex digest. The digest is deterministic for a given (user, host, port) triple, so ControlMaster reuse across reconnects is preserved, and the full path fits comfortably under the limit even after SSH's random suffix. Collision space is 2^64 — effectively unreachable for the handful of concurrent connections any single Hermes process holds. Regression tests cover: path length under realistic macOS $TMPDIR with the IPv6 host from the issue report, determinism for reconnects, and distinctness across different (user, host, port) triples. Closes #11840
This commit is contained in:
@@ -67,6 +67,74 @@ class TestBuildSSHCommand:
|
||||
assert env._build_ssh_command()[-1] == "u@h"
|
||||
|
||||
|
||||
class TestControlSocketPath:
|
||||
"""Regression tests for issue #11840.
|
||||
|
||||
macOS caps Unix domain socket paths at 104 bytes (sun_path). SSH
|
||||
appends a 16-byte random suffix to the control socket path when
|
||||
operating in ControlMaster mode. An IPv6 host embedded in the
|
||||
filename plus the deeply-nested macOS $TMPDIR easily blows past
|
||||
the limit, causing every tool call to fail immediately.
|
||||
"""
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _mock_connection(self, monkeypatch):
|
||||
monkeypatch.setattr("tools.environments.ssh.subprocess.run",
|
||||
lambda *a, **k: subprocess.CompletedProcess([], 0))
|
||||
monkeypatch.setattr("tools.environments.ssh.subprocess.Popen",
|
||||
lambda *a, **k: MagicMock(stdout=iter([]),
|
||||
stderr=iter([]),
|
||||
stdin=MagicMock()))
|
||||
monkeypatch.setattr("tools.environments.base.time.sleep", lambda _: None)
|
||||
|
||||
# SSH appends ``.XXXXXXXXXXXXXXXX`` (17 bytes) to the ControlPath in
|
||||
# ControlMaster mode; the macOS sun_path field is 104 bytes including
|
||||
# the NUL terminator, so the usable path length is 103 bytes.
|
||||
_SSH_CONTROLMASTER_SUFFIX = 17
|
||||
_MAX_SUN_PATH = 103
|
||||
|
||||
def test_fits_under_macos_socket_limit_with_ipv6_host(self, monkeypatch):
|
||||
"""A realistic macOS $TMPDIR + IPv6 host must still produce a
|
||||
control socket path that fits once SSH appends its ControlMaster
|
||||
suffix (see issue #11840)."""
|
||||
# Simulate the macOS $TMPDIR shape from the issue traceback —
|
||||
# 48 bytes, the typical length of ``/var/folders/XX/YYYYYYYYY/T``.
|
||||
fake_tmp = "/var/folders/2t/wbkw5yb158jc3zhswgl7tz9c0000gn/T"
|
||||
monkeypatch.setattr("tools.environments.ssh.tempfile.gettempdir",
|
||||
lambda: fake_tmp)
|
||||
# The simulated path doesn't exist on the test host — skip the
|
||||
# real mkdir so __init__ can proceed.
|
||||
from pathlib import Path as _Path
|
||||
monkeypatch.setattr(_Path, "mkdir", lambda *a, **k: None)
|
||||
|
||||
env = SSHEnvironment(
|
||||
host="9373:9b91:4480:558d:708e:e601:24e8:d8d0",
|
||||
user="hermes",
|
||||
port=22,
|
||||
)
|
||||
|
||||
total_len = len(str(env.control_socket)) + self._SSH_CONTROLMASTER_SUFFIX
|
||||
assert total_len <= self._MAX_SUN_PATH, (
|
||||
f"control socket path would exceed the {self._MAX_SUN_PATH}-byte "
|
||||
f"Unix domain socket limit once SSH appends its 16-byte suffix: "
|
||||
f"{env.control_socket} (+{self._SSH_CONTROLMASTER_SUFFIX} = {total_len})"
|
||||
)
|
||||
|
||||
def test_path_is_deterministic_across_instances(self):
|
||||
"""Same (user, host, port) must yield the same control socket so
|
||||
ControlMaster reuse works across reconnects."""
|
||||
first = SSHEnvironment(host="example.com", user="alice", port=2222)
|
||||
second = SSHEnvironment(host="example.com", user="alice", port=2222)
|
||||
assert first.control_socket == second.control_socket
|
||||
|
||||
def test_path_differs_for_different_targets(self):
|
||||
"""Different (user, host, port) triples must produce different paths."""
|
||||
base = SSHEnvironment(host="h", user="u", port=22).control_socket
|
||||
assert SSHEnvironment(host="h", user="u", port=23).control_socket != base
|
||||
assert SSHEnvironment(host="h", user="v", port=22).control_socket != base
|
||||
assert SSHEnvironment(host="g", user="u", port=22).control_socket != base
|
||||
|
||||
|
||||
class TestTerminalToolConfig:
|
||||
def test_ssh_persistent_default_true(self, monkeypatch):
|
||||
"""SSH persistent defaults to True (via TERMINAL_PERSISTENT_SHELL)."""
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
"""SSH remote execution environment with ControlMaster connection persistence."""
|
||||
|
||||
import hashlib
|
||||
import logging
|
||||
import os
|
||||
import shlex
|
||||
@@ -47,7 +48,18 @@ class SSHEnvironment(BaseEnvironment):
|
||||
|
||||
self.control_dir = Path(tempfile.gettempdir()) / "hermes-ssh"
|
||||
self.control_dir.mkdir(parents=True, exist_ok=True)
|
||||
self.control_socket = self.control_dir / f"{user}@{host}:{port}.sock"
|
||||
# Keep the socket filename short and deterministic so the full path
|
||||
# stays under the 104-byte sun_path limit that macOS enforces on
|
||||
# Unix domain sockets. A raw ``user@host:port`` — especially with an
|
||||
# IPv6 host — plus the 16-byte random suffix SSH appends in
|
||||
# ControlMaster mode easily exceeds the limit under macOS's
|
||||
# deeply-nested $TMPDIR (e.g. /var/folders/xx/yy/T/). Hashing the
|
||||
# triple keeps the path stable across reconnects so ControlMaster
|
||||
# reuse still works.
|
||||
_socket_id = hashlib.sha256(
|
||||
f"{user}@{host}:{port}".encode()
|
||||
).hexdigest()[:16]
|
||||
self.control_socket = self.control_dir / f"{_socket_id}.sock"
|
||||
_ensure_ssh_available()
|
||||
self._establish_connection()
|
||||
self._remote_home = self._detect_remote_home()
|
||||
|
||||
Reference in New Issue
Block a user