From cc96ac617af512d4d30b093a70f30ab8302fbd18 Mon Sep 17 00:00:00 2001 From: yoniebans Date: Fri, 19 Jun 2026 16:48:54 +0200 Subject: [PATCH] fix(desktop): create control-socket dir before opening the SSH master MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenSSH does not create intermediate directories for ControlPath, so on a fresh box (no prior hermes-desktop-ssh dir under \$TMPDIR) the very first connect failed when ssh tried to create the master socket. Unit tests mock spawn and never hit real fs, so this was invisible. open() now mkdir -p (mode 0700 — the socket grants command execution on the master) the control-socket directory before spawning ssh. Mirrors ssh.py. Added a test that open() creates a non-existent control dir. --- apps/desktop/electron/ssh-connection.cjs | 10 ++++++++++ apps/desktop/electron/ssh-connection.test.cjs | 20 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/apps/desktop/electron/ssh-connection.cjs b/apps/desktop/electron/ssh-connection.cjs index 8a42e45b101..286ada6710e 100644 --- a/apps/desktop/electron/ssh-connection.cjs +++ b/apps/desktop/electron/ssh-connection.cjs @@ -36,6 +36,7 @@ const crypto = require('node:crypto') const net = require('node:net') const os = require('node:os') const path = require('node:path') +const fs = require('node:fs') const DEFAULT_CONNECT_TIMEOUT_MS = 15_000 const DEFAULT_EXEC_TIMEOUT_MS = 20_000 @@ -354,6 +355,15 @@ class SshConnection { this._opened = true return } + // Ensure the control-socket directory exists — OpenSSH will not create + // intermediate dirs for ControlPath, so a fresh box (no prior hermes-ssh + // socket dir under $TMPDIR) would otherwise fail before the first connect. + // 0o700: the socket grants command execution on the master; keep it private. + try { + fs.mkdirSync(path.dirname(this.controlPath), { recursive: true, mode: 0o700 }) + } catch { + // best effort — a pre-existing dir or a races-with-another-conn mkdir is fine + } const args = buildMasterArgs(this, this._connectTimeoutMs) this._logLine(`opening control master to ${target(this.user, this.host)}:${this.port}`) let result diff --git a/apps/desktop/electron/ssh-connection.test.cjs b/apps/desktop/electron/ssh-connection.test.cjs index 5af349304db..c69fbb47bf7 100644 --- a/apps/desktop/electron/ssh-connection.test.cjs +++ b/apps/desktop/electron/ssh-connection.test.cjs @@ -12,6 +12,9 @@ const test = require('node:test') const assert = require('node:assert/strict') const { EventEmitter } = require('node:events') +const fs = require('node:fs') +const os = require('node:os') +const path = require('node:path') const { SSH_ERROR, @@ -247,6 +250,23 @@ test('open() is a no-op when the master is already alive', async () => { assert.deepEqual(ops, ['check'], 'alive master → no second spawn to open it') }) +test('open() creates the control-socket directory if it does not exist', async () => { + const dir = path.join(os.tmpdir(), `hermes-ssh-test-${process.pid}-${Date.now()}`) + assert.ok(!fs.existsSync(dir), 'precondition: control dir absent') + const spawnFn = scriptedSpawn(args => (args.includes('check') ? { code: 255 } : { code: 0 })) + const conn = new SshConnection({ host: 'box', user: 'me' }, { spawnFn, controlDir: dir }) + try { + await conn.open() + assert.ok(fs.existsSync(dir), 'open() created the control-socket directory before spawning ssh') + } finally { + try { + fs.rmSync(dir, { recursive: true, force: true }) + } catch { + /* ignore */ + } + } +}) + test('open() surfaces a classified auth error', async () => { const spawnFn = scriptedSpawn(args => { if (args.includes('check')) return { code: 255 }