Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 100 additions & 5 deletions apps/desktop/electron/remote-lifecycle.test.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -11,6 +11,7 @@ import { profileSshOverride } from './connection-config'
import {
assertRemoteInstallUpdateClear,
buildSpawnCommand,
buildSpawnPayload,
classifySshReuseProof,
cleanupStale,
connect,
Expand Down Expand Up @@ -46,6 +47,7 @@ import {
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(
Expand Down Expand Up @@ -1547,8 +1549,8 @@ test('buildSpawnCommand payload variables keep $HOME expandable (no double quoti
}
})

test('buildSpawnCommand lockfile publication is POSIX sh (no bash substitution)', () => {
const cmd = buildSpawnCommand('/x/hermes', 'work', {
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,
Expand All @@ -1560,9 +1562,11 @@ test('buildSpawnCommand lockfile publication is POSIX sh (no bash substitution)'

// ${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.
// 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__/${child}/"'), 'pid substitution must use sed')
assert.ok(cmd.includes(`sed 's/"pid":"__PID__"/"pid":'"$child"'/`), 'pid substitution must target the JSON pid field')
})

test('buildSpawnCommand scopes umask 077 to the mkdir subshell (no leak into serve)', () => {
Expand All @@ -1586,6 +1590,97 @@ test('buildSpawnCommand scopes umask 077 to the mkdir subshell (no leak into ser
'scoped mkdir must still precede the serve spawn'
)
})
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')
Expand Down
50 changes: 36 additions & 14 deletions apps/desktop/electron/remote-lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 + redirect + &).
function buildSpawnCommand(hermesPath, profile, opts: any = {}) {
function buildSpawnCommandParts(hermesPath, profile, opts: any = {}) {
const hermes = expandRemotePath(hermesPath)
const profileArgs = profile ? `--profile ${shq(profile)} ` : ''
const logPath = expandRemotePath(opts.logPath)
Expand Down Expand Up @@ -1135,17 +1135,22 @@ function buildSpawnCommand(hermesPath, profile, opts: any = {}) {
`ulimit -n ${REMOTE_NOFILE_SOFT_LIMIT} 2>/dev/null || true; ` +
`exec env HERMES_DESKTOP=1${opts.guestOnboarding === true ? ' HERMES_GUEST_ONBOARDING=1' : ''} ${hermes} ${profileArgs}${subCmd}`

const detachedShell = `eval "exec $1>&-"; ${dashCmd} </dev/null >> ${logPath} 2>&1 & echo $!`
// Keep Hermes in the foreground of the detached setsid/nohup shell. The outer
// shell backgrounds that process and emits the only PID. If this inner shell
// also echoes `$!`, command substitution captures two PIDs and the lockfile
// substitution becomes invalid under POSIX sh.
const detachedShell = `eval "exec $1>&-"; ${dashCmd} </dev/null >> ${logPath} 2>&1`
const detachedSpawn = `child=$("$(command -v setsid || echo nohup)" sh -c ${shq(detachedShell)} hermes-update-child "$1" & echo $!)`

if (!opts.ownershipId || !opts.lockMetadata) {
return withRemoteUpdateMutex(
`${markerClear}; marker_clear || exit 75; ` +
return {
updateMutex,
payload:
`${markerClear}; marker_clear || exit 75; ` +
`mkdir -p "$(dirname ${logPath})" && ` +
`${detachedSpawn}; ` +
`marker_clear || { kill "$child" 2>/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))
Expand All @@ -1155,8 +1160,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
Expand All @@ -1175,17 +1182,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. Substitute with sed instead.
`lock_json=$(printf '%s' ${shq(metadata)} | sed "s/__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":<digits>.
`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) {
Expand Down Expand Up @@ -1748,6 +1769,7 @@ export {
adoptOwnedServedToken,
assertRemoteInstallUpdateClear,
buildSpawnCommand,
buildSpawnPayload,
classifySshReuseProof,
cleanupStale,
connect,
Expand Down