fix(desktop): keep SSH ControlPath under sun_path on macOS

First real-hardware connect failed with: unix_listener: path
"/var/folders/8r/.../T/hermes-desktop-ssh/<hash>.sock.VSNDLDxh7gXySb0w" too long
for Unix domain socket.

Root cause: the default control-socket base was os.tmpdir(), which on macOS is
the deeply-nested per-user /var/folders/xx/yyyy.../T/ (~49 bytes). The socket
path itself fit 104, but OpenSSH binds a TEMPORARY listener at
<ControlPath>.<16 random chars> (a 17-byte suffix) while establishing the
master — 89 + 17 = 106 > 104, so bind failed.

Fix: default the POSIX control dir to a short, per-user base
(~/.hermes/desktop-ssh) instead of os.tmpdir(). Worst case is now ~72 bytes incl.
the temp suffix. Per-user (not a shared /tmp) avoids foreign-owned-dir/symlink
surface; still created 0700 in open(). Windows keeps os.tmpdir() (AF_UNIX has no
sun_path limit there). Deliberate divergence from ssh.py, which uses
gettempdir() and would hit this on macOS.

Test: default socket path + 17-byte temp suffix asserted <= 104 and not under
/var/folders/.
This commit is contained in:
yoniebans 2026-06-19 18:05:21 +02:00
parent e4cf51dbc0
commit a510e1132c
2 changed files with 38 additions and 3 deletions

View file

@ -80,14 +80,34 @@ function redactSecrets(text) {
// Hash user@host:port to a short, stable, filesystem-safe socket id. Stable
// across reconnects so ControlMaster reuse works; short so the full path stays
// well under sun_path's 104-byte limit even under macOS's deeply nested
// $TMPDIR (/var/folders/xx/yy/T/...). Mirrors ssh.py:53-67.
// under sun_path's 104-byte limit.
//
// CRITICAL (macOS): the base dir must be SHORT. os.tmpdir() on macOS is the
// per-user `/var/folders/xx/yyyy…/T/` (~49 bytes), and OpenSSH binds a
// TEMPORARY listener at `<ControlPath>.<16 random chars>` (a 17-byte suffix)
// while establishing the master — so a path that itself fits 104 still overflows
// at bind time with `unix_listener: path "…" too long`. We root under a short
// per-user base (`~/.hermes/desktop-ssh`) so even worst case
// (~/.hermes/desktop-ssh = ~33 on macOS + 1 + 16 + 5 + 17 ≈ 72) stays clear.
// Windows has no AF_UNIX sun_path limit, so os.tmpdir() is fine there. ssh.py
// uses gettempdir() and would hit this on macOS — deliberate divergence.
function controlSocketPath(user, host, port, baseDir) {
const dir = baseDir || path.join(os.tmpdir(), 'hermes-desktop-ssh')
const dir = baseDir || defaultControlDir()
const id = crypto.createHash('sha256').update(`${user}@${host}:${port}`).digest('hex').slice(0, 16)
return path.join(dir, `${id}.sock`)
}
function defaultControlDir() {
// Windows: AF_UNIX has no sun_path length limit → the per-user temp dir is
// fine. POSIX (macOS/Linux): a SHORT, PER-USER base — ~/.hermes/desktop-ssh —
// stays under the 104-byte socket limit AND avoids a world-shared /tmp dir
// (no foreign-owned-dir or symlink-hijack surface). Created 0700 in open().
if (process.platform === 'win32') {
return path.join(os.tmpdir(), 'hermes-desktop-ssh')
}
return path.join(os.homedir(), '.hermes', 'desktop-ssh')
}
// ---------------------------------------------------------------------------
// Command construction (pure — the unit tests exercise these directly)
// ---------------------------------------------------------------------------

View file

@ -76,6 +76,21 @@ test('controlSocketPath is stable, short, and host-distinct', () => {
assert.match(a, /\/[0-9a-f]{16}\.sock$/)
})
test('controlSocketPath default base stays under sun_path even with the temp-listener suffix', () => {
// OpenSSH binds a temporary listener at `<ControlPath>.<16 random chars>`
// (a 17-byte suffix) while opening the master. The macOS regression was the
// default base under os.tmpdir() (/var/folders/.../T/) pushing 89 → 106 bytes.
// The default base must keep socket + 17-byte suffix comfortably under 104.
const p = controlSocketPath('hermes', 'vbuddy-ubuntu', 22) // no baseDir → default
const worstCase = `${p}.0123456789abcdef` // mimic the .<16-char> temp suffix
assert.ok(
worstCase.length <= 104,
`default control socket + temp suffix must fit sun_path (got ${worstCase.length}: ${worstCase})`
)
// And it must NOT live under the deeply-nested macOS per-user temp dir.
assert.ok(!p.includes('/var/folders/'), 'default base must not be os.tmpdir() on macOS')
})
// --- command construction ---------------------------------------------------
test('baseSshOptions carries the house ControlMaster/BatchMode/accept-new policy', () => {