diff --git a/apps/desktop/electron/remote-lifecycle.test.ts b/apps/desktop/electron/remote-lifecycle.test.ts index bbabd65bb7f4a..0e94422b9a0a3 100644 --- a/apps/desktop/electron/remote-lifecycle.test.ts +++ b/apps/desktop/electron/remote-lifecycle.test.ts @@ -1,5 +1,5 @@ import assert from 'node:assert/strict' -import { exec as execCallback, spawn } from 'node:child_process' +import { exec as execCallback, execFile as execFileCallback, spawn } from 'node:child_process' import { chmod, mkdir, mkdtemp, readFile, rm, symlink, writeFile } from 'node:fs/promises' import os from 'node:os' import path from 'node:path' @@ -11,6 +11,7 @@ import { profileSshOverride } from './connection-config' import { assertRemoteInstallUpdateClear, buildSpawnCommand, + buildSpawnPayload, classifySshReuseProof, cleanupStale, connect, @@ -47,6 +48,7 @@ import type { SshConnection } from './ssh-connection' const OWNERSHIP_ID = '0123456789abcdef0123456789abcdef' const SPAWN_NONCE = '0123456789abcdef' const exec = promisify(execCallback) +const execFile = promisify(execFileCallback) test('SSH reuse proof rejects a backend whose runtime was replaced', () => { assert.equal( @@ -1447,6 +1449,117 @@ test('buildSpawnCommand includes --ssh-session-token-file when tokenFilePath is assert.match(cmd, /\.hermes\/desktop-ssh\//) }) +test('buildSpawnPayload lockfile publication is POSIX sh (no bash substitution)', () => { + const cmd = buildSpawnPayload('/x/hermes', 'work', { + hermesHome: '~/.hermes', + logPath: spawnLogPath(OWNERSHIP_ID, SPAWN_NONCE), + ownershipId: OWNERSHIP_ID, + reservationNonce: SPAWN_NONCE, + spawnNonce: SPAWN_NONCE, + tokenFilePath: spawnTokenPath(OWNERSHIP_ID, SPAWN_NONCE), + lockMetadata: { ownershipId: OWNERSHIP_ID, pid: '__PID__' } + }) + + // ${var//pat/rep} is bash-only; dash aborts the payload on it AFTER the + // serve was spawned, so the client sees an unknown failure, deletes the + // token file, and orphans the backend. The sed swap must target only the + // quoted JSON pid field so the published pid is a JSON integer (readLockfile + // rejects a quoted-string pid as malformed-pid skew and fails closed). + assert.doesNotMatch(cmd, /\$\{lock_json\/\//, 'must not use ${var//} substitution under sh') + assert.ok(cmd.includes(`sed 's/"pid":"__PID__"/"pid":'"$child"'/`), 'pid substitution must target the JSON pid field') +}) + +test.skipIf(process.platform !== 'linux')( + 'buildSpawnPayload publishes a runtime-valid lockfile pid under dash', + async () => { + const directory = await mkdtemp(path.join(os.tmpdir(), 'hermes-remote-payload-runtime-')) + const home = path.join(directory, 'home') + const hermesPath = path.join(directory, 'hermes') + const pidPath = path.join(directory, 'hermes.pid') + const hermesHome = '~/.hermes' + const logPath = spawnLogPath(OWNERSHIP_ID, SPAWN_NONCE) + const lockDirectory = path.join(home, '.hermes', 'desktop-ssh', OWNERSHIP_ID) + const lockPath = path.join(lockDirectory, 'backend.lock.json') + const tokenPath = path.join(lockDirectory, `${SPAWN_NONCE}.token`) + + const payload = buildSpawnPayload(hermesPath, 'work', { + hermesHome, + logPath, + ownershipId: OWNERSHIP_ID, + reservationNonce: SPAWN_NONCE, + spawnNonce: SPAWN_NONCE, + tokenFilePath: spawnTokenPath(OWNERSHIP_ID, SPAWN_NONCE), + lockMetadata: { + ownershipId: OWNERSHIP_ID, + spawnNonce: SPAWN_NONCE, + port: 0, + profile: 'work', + hermesPath, + hermesHome, + logPath, + tokenFingerprint: fingerprintToken('stored-token'), + protocolVersion: PROTOCOL_VERSION, + startedAt: '2026-07-14T00:00:00.000Z' + } + }) + + let hermesPid = 0 + + try { + await mkdir(lockDirectory, { recursive: true, mode: 0o700 }) + await writeFile(tokenPath, 'token', { mode: 0o600 }) + await writeFile(hermesPath, '#!/bin/sh\nprintf \'%s\\n\' "$$" > "$PID_FILE"\nexec sleep 30\n', { mode: 0o700 }) + + const { stdout } = await execFile('dash', ['-c', payload, 'hermes-update-mutex', '3'], { + env: { ...process.env, HOME: home, PID_FILE: pidPath }, + timeout: 10_000 + }) + + assert.match(stdout.trim(), /^[0-9]+$/) + const lock = JSON.parse(await readFile(lockPath, 'utf8')) + assert.equal(Number.isInteger(lock.pid), true) + + for (let attempt = 0; attempt < 40; attempt += 1) { + try { + const candidate = Number((await readFile(pidPath, 'utf8')).trim()) + + if (Number.isInteger(candidate) && candidate > 0) { + hermesPid = candidate + + break + } + } catch (error: any) { + if (error?.code !== 'ENOENT') { + throw error + } + } + + await new Promise(resolve => setTimeout(resolve, 25)) + } + + assert.ok(hermesPid > 0, 'the fake Hermes backend did not start') + assert.equal(lock.pid, hermesPid) + } finally { + if (!hermesPid) { + try { + hermesPid = Number((await readFile(pidPath, 'utf8')).trim()) + } catch { + void 0 + } + } + + if (Number.isInteger(hermesPid) && hermesPid > 0) { + try { + process.kill(hermesPid, 'SIGTERM') + } catch { + void 0 + } + } + + await rm(directory, { recursive: true, force: true }) + } + } +) test('spawnRemoteDashboard removes a token file when upload reporting fails', async () => { const failure = new Error('channel closed') @@ -2022,3 +2135,5 @@ test('connect() post-spawn cleanup that cannot prove ownership keeps the origina 'record must survive for the next connect to reap' ) }) + + diff --git a/apps/desktop/electron/remote-lifecycle.ts b/apps/desktop/electron/remote-lifecycle.ts index ca83ccecb901a..e3b4a849b34cc 100644 --- a/apps/desktop/electron/remote-lifecycle.ts +++ b/apps/desktop/electron/remote-lifecycle.ts @@ -1106,7 +1106,7 @@ async function terminateOwnedDashboardForUpdate(ssh, expected) { // Detach so the backend survives the SSH channel closing: setsid (Linux) // starts a new session; macOS has no setsid, so fall back to nohup (HUP-immune; // fd-detachment is already handled by /dev/null || true; wait "$child" 2>/dev/null || true; exit 75; }; echo "$child"`, - updateMutex - ) + `marker_clear || { kill "$child" 2>/dev/null || true; wait "$child" 2>/dev/null || true; exit 75; }; echo "$child"` + } } const reservation = expandRemotePath(connectReservationPath(opts.ownershipId)) @@ -1157,8 +1158,10 @@ function buildSpawnCommand(hermesPath, profile, opts: any = {}) { const metadata = JSON.stringify({ schemaVersion: LOCKFILE_SCHEMA_VERSION, ...opts.lockMetadata, pid: '__PID__' }) const reservationNonce = validateSpawnNonce(opts.reservationNonce || crypto.randomBytes(8).toString('hex')) - return withRemoteUpdateMutex( - `(umask 077 && mkdir -p "$(dirname ${reservation})"); ` + + return { + updateMutex, + payload: + `(umask 077 && mkdir -p "$(dirname ${reservation})"); ` + // reservation/lockPath/ownerPath are expandRemotePath() output — already // shell-quoted fragments ("$HOME"'/…'). Embed raw so the assignment // expands $HOME; shq() here would store the quote characters literally @@ -1177,18 +1180,31 @@ function buildSpawnCommand(hermesPath, profile, opts: any = {}) { `if kill -0 "$existing_pid" 2>/dev/null; then ${tokenPath ? `rm -f ${tokenPath}; ` : ''}printf EXISTING; exit 0; fi; rm -f "$lock";; esac; fi; ` + `${markerClear}; marker_clear || exit 75; mkdir -p "$(dirname ${logPath})" && ` + `${detachedSpawn}; ` + + `case "$child" in ''|*[!0-9]*) exit 76;; esac; ` + `marker_clear || { kill "$child" 2>/dev/null || true; wait "$child" 2>/dev/null || true; exit 75; }; ` + // ${var//pat/rep} is a bashism — this payload runs under plain sh (dash // on Ubuntu), which aborts the whole script on it with "Bad // substitution" AFTER the child was spawned, orphaning the backend and - // skipping the lockfile publication. Replace the quoted PID field with - // a JSON number so readLockfile and concurrent spawns accept the record. - `lock_json=$(printf '%s' ${shq(metadata)} | sed "s/\\"pid\\":\\"__PID__\\"/\\"pid\\":\${child}/"); ` + + // skipping the lockfile publication. Substitute with sed instead, and + // replace only the JSON pid field's quoted placeholder so the published + // record carries a real JSON number pid: readLockfile requires an integer + // (malformed-pid skew fails closed otherwise) and the reuse regex only + // matches "pid":. + `lock_json=$(printf '%s' ${shq(metadata)} | sed 's/"pid":"__PID__"/"pid":'"$child"'/') || { kill "$child" 2>/dev/null || true; wait "$child" 2>/dev/null || true; exit 76; }; ` + `temporary_lock="\${lock}.${reservationNonce}.tmp"; ` + `printf '%s' "$lock_json" > "$temporary_lock" && mv -f "$temporary_lock" "$lock" || { kill "$child" 2>/dev/null || true; wait "$child" 2>/dev/null || true; exit 76; }; ` + - `echo "$child"`, - updateMutex - ) + `echo "$child"` + } +} + +function buildSpawnPayload(hermesPath, profile, opts: any = {}) { + return buildSpawnCommandParts(hermesPath, profile, opts).payload +} + +function buildSpawnCommand(hermesPath, profile, opts: any = {}) { + const { updateMutex, payload } = buildSpawnCommandParts(hermesPath, profile, opts) + + return withRemoteUpdateMutex(payload, updateMutex) } async function remoteSupportsSshOwnership(ssh, hermesPath) { @@ -1751,6 +1767,7 @@ export { adoptOwnedServedToken, assertRemoteInstallUpdateClear, buildSpawnCommand, + buildSpawnPayload, classifySshReuseProof, cleanupStale, connect,