diff --git a/apps/desktop/electron/managed-ssh-update.test.ts b/apps/desktop/electron/managed-ssh-update.test.ts index d2999590d9..0a4326f3fe 100644 --- a/apps/desktop/electron/managed-ssh-update.test.ts +++ b/apps/desktop/electron/managed-ssh-update.test.ts @@ -607,58 +607,27 @@ test.runIf(process.platform !== 'win32')( } ) -test.runIf(process.platform !== 'win32')( - 'managed observer judges a v2 claim by its owner and delegate, not as malformed', - async () => { - const home = await mkdtemp(path.join(os.tmpdir(), 'hermes-managed-v2-marker-')) - const marker = path.join(home, '.hermes-update-in-progress') - - const target = { - ssh: { exec: async () => '' }, - platform: 'Linux', - hermesPath: '/opt/hermes/hermes', - hermesHome: home - } - - const observe = async () => - parseRemoteUpdateObservation( - (await exec(buildRemoteUpdateObservationCommand(target as any, CORRELATION), { shell: 'sh' })).stdout, - CORRELATION - ) - - const exited = (await exec(`sh -c 'echo $$'`)).stdout.trim() - // The updater's v2 claim: pid, started_at, creation-time line (A2), then tagged lines. - const deadClaim = `${exited}\n${Math.floor(Date.now() / 1000)}\nct:1700000000.125\n` - // A delegate is live only at its real creation time (the judge checks pid AND ct, so a reused - // pid cannot impersonate it): record the spawn time, well inside the 2 s tolerance. - const delegateCt = (Date.now() / 1000).toFixed(3) - const delegate = spawn('sleep', ['30'], { stdio: 'ignore' }) - - try { - await writeFile(marker, deadClaim) - assert.equal((await observe()).marker, 'dead', 'a dead v2 claim is dead, never malformed') - await writeFile(marker, `${deadClaim}delegate:${delegate.pid} ct:${delegateCt}\n`) - const delegated = await observe() - assert.equal(delegated.marker, 'live', 'a live delegate keeps the update live') - assert.equal(delegated.markerPid, delegate.pid) - } finally { - delegate.kill() - await rm(home, { force: true, recursive: true }) - } - } -) - test.runIf(process.platform !== 'win32')( 'managed observer unwraps a named profile home for the install-wide marker', async () => { const root = await mkdtemp(path.join(os.tmpdir(), 'hermes-managed-profile-marker-')) const profileHome = path.join(root, 'profiles', 'research') + // A v2 claim whose owner is gone and whose delegate is alive at its creation time. + const delegate = spawn('python3', ['-c', 'import time;print(time.time(),flush=True);time.sleep(30)'], { + stdio: ['ignore', 'pipe', 'inherit'] + }) + try { + const ct = await new Promise(resolve => + delegate.stdout.once('data', chunk => resolve(String(chunk).trim())) + ) + + const now = Math.floor(Date.now() / 1000) await mkdir(profileHome, { recursive: true }) await writeFile( path.join(root, '.hermes-update-in-progress'), - `${process.pid}\n${Math.floor(Date.now() / 1000)}\n` + `0\n${now}\nct:${ct}\ndelegate:${delegate.pid} ct:${ct}\nrun:desk-1\n` ) const command = buildRemoteUpdateObservationCommand( @@ -675,8 +644,9 @@ test.runIf(process.platform !== 'win32')( const parsed = parseRemoteUpdateObservation(stdout, CORRELATION) assert.equal(parsed.marker, 'live') - assert.equal(parsed.markerPid, process.pid) + assert.equal(parsed.markerPid, delegate.pid) } finally { + delegate.kill('SIGKILL') await rm(root, { force: true, recursive: true }) } } diff --git a/apps/desktop/electron/managed-ssh-update.ts b/apps/desktop/electron/managed-ssh-update.ts index 0a8bde3907..b768399fa3 100644 --- a/apps/desktop/electron/managed-ssh-update.ts +++ b/apps/desktop/electron/managed-ssh-update.ts @@ -409,7 +409,6 @@ def marker_state(): try:raw=marker_path.read_bytes() except FileNotFoundError:return {'state':'absent'} except OSError:return {'state':'unavailable'} - # update_lock.judge_marker's parser and identity rule (owner or delegate, pid + creation time). verdict=marker_verdict(raw) if verdict=='UNCERTAIN':return {'state':'malformed'} if verdict=='CLEAR':return {'state':'dead'} diff --git a/apps/desktop/electron/remote-lifecycle-spawn.test.ts b/apps/desktop/electron/remote-lifecycle-spawn.test.ts index 8f2014698c..bda1a5f84b 100644 --- a/apps/desktop/electron/remote-lifecycle-spawn.test.ts +++ b/apps/desktop/electron/remote-lifecycle-spawn.test.ts @@ -8,6 +8,7 @@ import { promisify } from 'node:util' import { expect, test } from 'vitest' import { + assertRemoteInstallUpdateClear, buildSpawnCommand, cleanupStale, connectReservationPath, @@ -143,7 +144,7 @@ async function spawnFixture(launcher: 'setsid' | 'nohup', owned: boolean): Promi import json,os,signal,time from pathlib import Path pid=os.getpid() -mutex=Path(os.environ['HERMES_HOME'])/'.hermes-update-in-progress.mutex' +mutex=Path(os.environ['HERMES_HOME'])/'.hermes-update-in-progress.lock' identity=mutex.stat() fds=[] for fd in range(3,256): @@ -258,7 +259,7 @@ test.skipIf(process.platform === 'win32').each(variants)( // The child remains alive while another process acquires the exact mutex. await fixture.run( - `python3 -c 'import fcntl,sys;f=open(sys.argv[1],"a");fcntl.flock(f,fcntl.LOCK_EX|fcntl.LOCK_NB)' ${expandRemotePath(`${fixture.marker}.mutex`)}` + `python3 -c 'import fcntl,sys;f=open(sys.argv[1],"a");fcntl.flock(f,fcntl.LOCK_EX|fcntl.LOCK_NB)' ${expandRemotePath(`${fixture.marker}.lock`)}` ) // The published ownership record carries the profile the spawn argv // actually pinned — normalized like the CLI would (#88842) — never the @@ -298,30 +299,8 @@ test.skipIf(process.platform === 'win32').each(variants)( async ({ launcher, owned }: SpawnVariant): Promise => { const fixture: SpawnFixture = await spawnFixture(launcher, owned) - // A live delegate under a dead owner still guards the install (update_lock.judge_marker). - const exited: ChildProcess = spawn(process.execPath, ['-e', '']) - await new Promise(resolve => exited.once('exit', resolve)) - - const delegate: ChildProcess = spawn( - 'python3', - ['-c', 'import time;print(time.time(),flush=True);time.sleep(30)'], - { - stdio: ['ignore', 'pipe', 'ignore'] - } - ) - try { - const delegateCt: string = await new Promise(resolve => - delegate.stdout!.once('data', (chunk: Buffer) => resolve(String(chunk).trim())) - ) - - const now: number = Math.floor(Date.now() / 1000) - - for (const marker of [ - `${process.pid}\n${now}\n`, - 'not-a-pid\n', - `${exited.pid}\n${now}\nct:1700000000.125\ndelegate:${delegate.pid} ct:${delegateCt}\n` - ]) { + for (const marker of [`${process.pid}\n${Math.floor(Date.now() / 1000)}\n`, 'not-a-pid\n']) { await writeFile(fixture.marker, marker, 'utf8') await assert.rejects(fixture.run(fixture.command), { code: 75, stdout: '' }) assert.deepEqual(await reports(fixture), []) @@ -329,7 +308,96 @@ test.skipIf(process.platform === 'win32').each(variants)( await assert.rejects(readFile(fixture.localPath(lockfilePath(ownershipId))), { code: 'ENOENT' }) } } finally { - delegate.kill() + await fixture.dispose() + } + }, + 20_000 +) + +/** A python3 child that prints one line (after its setup) and stays alive. */ +async function pythonChild(fixture: SpawnFixture, script: string, ...args: string[]): Promise<[ChildProcess, string]> { + const child: ChildProcess = spawn('python3', ['-c', script, ...args], { + env: fixture.env, + stdio: ['ignore', 'pipe', 'inherit'] + }) + + const line: string = await new Promise((resolve, reject) => { + child.stdout!.once('data', (chunk: Buffer) => resolve(chunk.toString().trim())) + child.once('exit', code => reject(new Error(`python child exited ${code}`))) + }) + + return [child, line] +} + +async function deadPid(): Promise { + const child: ChildProcess = spawn('true') + await new Promise(resolve => child.once('exit', resolve)) + + return child.pid! +} + +test.skipIf(process.platform === 'win32')( + 'v2 claims are judged by pid + creation time: a dead claim is cleared, a live delegate refuses', + async (): Promise => { + const fixture: SpawnFixture = await spawnFixture(variants[0].launcher, false) + const [delegate, ct] = await pythonChild(fixture, 'import time;print(time.time(),flush=True);time.sleep(30)') + const dead: number = await deadPid() + const now: number = Math.floor(Date.now() / 1000) + + try { + await writeFile(fixture.marker, `${dead}\n${now}\nct:${ct}\ndelegate:${dead} ct:${ct}\nrun:desk-1\n`) + await assertRemoteInstallUpdateClear(fixture.ssh, fixture.lock.hermesHome) + await assert.rejects(readFile(fixture.marker), { code: 'ENOENT' }) + + const live: string = `${dead}\n${now}\nct:${ct}\ndelegate:${delegate.pid} ct:${ct}\n` + await writeFile(fixture.marker, live) + await assert.rejects( + assertRemoteInstallUpdateClear(fixture.ssh, fixture.lock.hermesHome), + (error: any) => error.kind === 'update-in-progress' && error.message.includes(`process ${delegate.pid}`) + ) + await assert.rejects(fixture.run(fixture.command), { code: 75, stdout: '' }) + assert.equal(await readFile(fixture.marker, 'utf8'), live) + + // The live pid at another incarnation (creation time 60 s off) is a dead claim. + await writeFile(fixture.marker, `${delegate.pid}\n${now}\nct:${Number(ct) - 60}\n`) + assert.match((await fixture.run(fixture.command)).stdout, /^[1-9][0-9]*\n$/) + await assert.rejects(readFile(fixture.marker), { code: 'ENOENT' }) + } finally { + delegate.kill('SIGKILL') + await fixture.dispose() + } + }, + 20_000 +) + +test.skipIf(process.platform === 'win32')( + "spawn waits on the updaters' .lock, so a claim made under it is never raced", + async (): Promise => { + const fixture: SpawnFixture = await spawnFixture(variants[0].launcher, false) + + // An updater: hold .lock (update_lock.py / marker.sh flock), then + // write its v2 claim and release, staying alive as the live owner. + const [updater] = await pythonChild( + fixture, + `import fcntl,os,sys,time +t=time.time() +fd=os.open(sys.argv[1]+'.lock',os.O_RDWR|os.O_CREAT) +fcntl.flock(fd,fcntl.LOCK_EX) +print('held',flush=True) +time.sleep(0.5) +open(sys.argv[1],'w').write('%d\\n%d\\nct:%.3f\\n'%(os.getpid(),int(t),t)) +time.sleep(0.5) +fcntl.flock(fd,fcntl.LOCK_UN) +time.sleep(30)`, + fixture.marker + ) + + try { + await assert.rejects(fixture.run(fixture.command), { code: 75, stdout: '' }) + assert.deepEqual(await reports(fixture), []) + assert.match(await readFile(fixture.marker, 'utf8'), new RegExp(`^${updater.pid}\\n`)) + } finally { + updater.kill('SIGKILL') await fixture.dispose() } }, diff --git a/apps/desktop/electron/remote-lifecycle.test.ts b/apps/desktop/electron/remote-lifecycle.test.ts index f2a4c97dcd..fc8ab96325 100644 --- a/apps/desktop/electron/remote-lifecycle.test.ts +++ b/apps/desktop/electron/remote-lifecycle.test.ts @@ -100,21 +100,25 @@ function fakeSsh(rules: any[] = []) { // Existing lifecycle fixtures predate the install-wide relaunch gate. // Their default remote has no update marker; focused marker tests below // use explicit SSH doubles to exercise live/uncertain transitions. - if (cmd.includes('.hermes-update-in-progress') && !cmd.includes('marker_clear()') && !/setsid|nohup/.test(cmd)) { + // The spawn payload (setsid/nohup) runs under the same marker gate as the relaunch probe. + const mutexWrapped = /setsid|nohup/.test(cmd) + + if (cmd.includes('.hermes-update-in-progress') && !mutexWrapped) { return 'CLEAR' } - const mutexWrapped = cmd.includes('fcntl.flock(fd,fcntl.LOCK_EX)') - const applicableRules = rules.filter(([matcher]) => { - if (cmd.includes('marker_clear()') && matcher instanceof RegExp && /kill -0/.test(matcher.source)) { + if (mutexWrapped && matcher instanceof RegExp && /kill -0/.test(matcher.source)) { return false } return !(mutexWrapped && matcher instanceof RegExp && /python3 -c/.test(matcher.source)) }) - if ((cmd.includes('os.kill(pid') && !/pidfd_open|marker_clear/.test(cmd)) || cmd.includes('printf TERMINATED')) { + if ( + (cmd.includes('os.kill(pid') && !cmd.includes('pidfd_open') && !mutexWrapped) || + cmd.includes('printf TERMINATED') + ) { return 'TERMINATED' } @@ -184,10 +188,7 @@ test('POSIX relaunch gate permits absent/dead markers and normalizes named-profi } await assertRemoteInstallUpdateClear(ssh, '/home/alice/.hermes/profiles/research') - assert.match(commands[0], /home\.parent\.name/) - assert.match(commands[0], /profiles/) - assert.match(commands[0], /\.hermes-update-in-progress/) - assert.match(commands[0], /marker\.unlink/) + assert.ok(commands[0].endsWith(" '/home/alice/.hermes/.hermes-update-in-progress'"), commands[0].slice(-80)) }) test('POSIX relaunch gate rechecks after token upload immediately before process creation', async () => { @@ -998,7 +999,7 @@ report=${expandRemotePath(reportPath)} for fd in /proc/$$/fd/*; do target=$(readlink "$fd" 2>/dev/null || true) case "$target" in - *hermes-update-in-progress.mutex) printf '%s\\n' "$target" >> "$report.tmp" ;; + *hermes-update-in-progress.lock) printf '%s\\n' "$target" >> "$report.tmp" ;; esac done mv "$report.tmp" "$report" @@ -2090,11 +2091,11 @@ test.skipIf(process.platform === 'win32')( }) const argv = (await readFile(argvFile, 'utf8')).split('\0') - // argv: ['-c', , , ] + // argv: ['-c', , , ] assert.equal( argv[2], - `${fakeHome}/.hermes/.hermes-update-in-progress.mutex`, - 'mutex path must reach python fully expanded, with no quote characters' + `${fakeHome}/.hermes/.hermes-update-in-progress`, + 'marker path must reach python fully expanded, with no quote characters' ) // The payload assigns reservation/lock/owner_file before its mkdir loop. diff --git a/apps/desktop/electron/remote-lifecycle.ts b/apps/desktop/electron/remote-lifecycle.ts index 02f64ec82f..9ba534210e 100644 --- a/apps/desktop/electron/remote-lifecycle.ts +++ b/apps/desktop/electron/remote-lifecycle.ts @@ -30,7 +30,7 @@ import crypto from 'node:crypto' import { READY_IN_MERGED_OUTPUT_RE } from './backend-ready' import { parseRemoteProfileListing } from './connection-registry' import { backendProfileArg } from './profile-id-guard' -import { REMOTE_MARKER_JUDGE_PY } from './remote-update-marker-programs' +import { REMOTE_MARKER_GATE_PY } from './remote-update-marker-programs' import { assertBootstrapNotSuperseded, withRemoteTimeout } from './ssh-connection' const LOCKFILE_SCHEMA_VERSION = 2 @@ -320,70 +320,23 @@ async function probeRemoteHermesHome(ssh) { } } -// The relaunch gate: REMOTE_MARKER_JUDGE_PY (update_lock.judge_marker's parser and identity rule) -// judges the marker; a dead claim is deleted under the updaters' marker lock. -const REMOTE_UPDATE_MARKER_PROBE = `${REMOTE_MARKER_JUDGE_PY}${String.raw` -import fcntl -from pathlib import Path - -home=Path(os.path.expanduser(sys.argv[1])) -if home.parent.name=='profiles':home=home.parent.parent -marker=home/'.hermes-update-in-progress' -def uncertain(): - print('UNCERTAIN');raise SystemExit -def clear(): - # Delete only under the updaters' marker mutex and only the bytes judged dead: - # a claim published since our read is a fresh claim, never ours to remove. - try: - try:fd=os.open(str(marker)+'.lock',os.O_RDWR|os.O_CREAT,0o644) - except PermissionError:fd=os.open(str(marker)+'.lock',os.O_RDONLY) - except OSError:uncertain() - try: - try:fcntl.flock(fd,fcntl.LOCK_EX|fcntl.LOCK_NB) - except OSError:uncertain() - try: - with marker.open('rb') as stream: - if stream.read(4097)!=raw:uncertain() - marker.unlink() - except FileNotFoundError:pass - except OSError:uncertain() - finally:os.close(fd) - print('CLEAR');raise SystemExit -try: - with marker.open('rb') as stream:raw=stream.read(4097) -except FileNotFoundError: - print('CLEAR');raise SystemExit -except OSError: - uncertain() -verdict=marker_verdict(raw) -if verdict=='CLEAR':clear() -print(verdict) -`}` - -// The spawn recheck: the same judgement, read-only (the relaunch gate above already reclaimed). -const REMOTE_MARKER_VERDICT_PY = `${REMOTE_MARKER_JUDGE_PY}${String.raw` -try: - with open(sys.argv[1],'rb') as stream:raw=stream.read(4097) -except FileNotFoundError:raw=None -print(marker_verdict(raw)) -`}` - /** * Refuse normal SSH reuse/spawn while the remote install is being mutated. * - * This probe intentionally uses only the host's system Python and raw marker - * bytes; it never imports or executes code from the changing Hermes checkout. - * Absence or a well-formed, confirmed-dead owner is clear. Every parse, read, - * probe, or transport uncertainty fails closed so a Desktop relaunch cannot - * start `serve` beside an updater that survived the old app process. + * The probe uses only the host's system Python and the shared marker judge + * (remote-update-marker-programs.ts); it never imports the changing checkout. + * Absence or a confirmed-dead claim (owner and delegate) is clear, and a dead + * claim is unlinked under `.lock`. Every parse, read, lock, or + * transport uncertainty fails closed so a Desktop relaunch cannot start + * `serve` beside an updater that survived the old app process. */ async function assertRemoteInstallUpdateClear(ssh, hermesHome) { - const home = assertSafeRemoteHome(hermesHome) + const marker = expandRemotePath(`${remoteInstallRoot(hermesHome)}/.hermes-update-in-progress`) let observation = '' try { observation = - String(await ssh.exec(`python3 -c ${shq(REMOTE_UPDATE_MARKER_PROBE)} ${expandRemotePath(home)}`)) + String(await ssh.exec(`python3 -c ${shq(REMOTE_MARKER_GATE_PY)} ${marker}`)) .trim() .split(/\r?\n/) .pop() || '' @@ -403,7 +356,7 @@ async function assertRemoteInstallUpdateClear(ssh, hermesHome) { const error: any = new Error( live ? `Remote Hermes update process ${live[1]} is still running; SSH startup is paused.` - : 'The remote Hermes update marker is unreadable or malformed; refusing SSH startup.' + : 'The remote Hermes update marker is unreadable, malformed, or locked; refusing SSH startup.' ) error.kind = 'update-in-progress' @@ -1005,32 +958,16 @@ finally: return `python3 -c ${shq(script)}` } -// The updater's Python _MarkerMutex uses the marker's .mutex sidecar and an -// advisory flock. Keep that same descriptor locked while the remote shell does -// the marker check, spawns the backend, and publishes its initial lockfile. -// Python keeps the descriptor close-on-exec by default and passes it explicitly -// only to the intended outer shell; each detached child closes it before -// execing Hermes. mutexPath is expandRemotePath() output — a complete shell -// word ("$HOME"'/…' or '/abs/…') embedded raw so $HOME expands remotely; a -// second shq() would hand python the quote characters as part of the path. -function withRemoteUpdateMutex(command, mutexPath) { - const script = ` -import fcntl,os,subprocess,sys -mutex_path=sys.argv[1] -payload=sys.argv[2] -parent=os.path.dirname(mutex_path) -if parent:os.makedirs(parent,exist_ok=True) -fd=os.open(mutex_path,os.O_RDWR|os.O_CREAT|os.O_CLOEXEC,0o600) -fcntl.flock(fd,fcntl.LOCK_EX) -result=None -try: - result=subprocess.run(["sh","-c",payload,"hermes-update-mutex",str(fd)],pass_fds=(fd,),check=False) -finally: - os.close(fd) -sys.exit(result.returncode if result is not None else 1) -`.trim() - - return `python3 -c ${shq(script)} ${mutexPath} ${shq(command)}` +// Run `command` under the updaters' marker lock: REMOTE_MARKER_GATE_PY holds +// `.lock` (the sidecar Python update_lock and marker.sh flock), clears a +// dead claim, and refuses (exit 75) a live or unreadable one, then keeps the +// lock across the spawn and the initial lockfile publication. The descriptor +// is close-on-exec and passed only to the outer shell; each detached child +// closes it before execing Hermes. markerPath is expandRemotePath() output — a +// complete shell word ("$HOME"'/…' or '/abs/…') embedded raw so $HOME expands +// remotely; a second shq() would hand python the quote characters. +function withRemoteUpdateMutex(command, markerPath) { + return `python3 -c ${shq(REMOTE_MARKER_GATE_PY)} ${markerPath} ${shq(command)}` } /** @@ -1175,18 +1112,14 @@ function buildSpawnCommand(hermesPath, profile, opts: any = {}) { const subCmd = `serve --isolated --host 127.0.0.1 --port 0${tokenArg}${ownerArg}` const marker = expandRemotePath(`${remoteInstallRoot(opts.hermesHome || '~/.hermes')}/.hermes-update-in-progress`) - const updateMutex = expandRemotePath( - `${remoteInstallRoot(opts.hermesHome || '~/.hermes')}/.hermes-update-in-progress.mutex` - ) - - // The marker probe, ownership reservation, process creation, and initial - // lockfile publication must be one remote command. A second Desktop process - // can therefore never observe an empty lock and spawn before this one records - // its PID. The reservation is an atomic mkdir and is reclaimed only when its - // owning remote shell is dead. - const markerClear = - `marker_clear() { if [ ! -e ${marker} ]; then return 0; fi; ` + - `[ "$(python3 -c ${shq(REMOTE_MARKER_VERDICT_PY)} ${marker} 2>/dev/null)" = CLEAR ]; }` + // The marker judge, ownership reservation, process creation, and initial + // lockfile publication are one remote command under the marker lock, so a + // second Desktop process never observes an empty lock and spawns before this + // one records its PID. The gate judges/clears the marker before the payload; + // a marker present after the spawn came from a writer that skipped the lock, + // so the child is reaped. The reservation is an atomic mkdir reclaimed only + // when its owning remote shell is dead. + const markerAbsent = `[ ! -e ${marker} ]` const dashCmd = `ulimit -n ${REMOTE_NOFILE_SOFT_LIMIT} 2>/dev/null || true; ` + @@ -1199,11 +1132,10 @@ function buildSpawnCommand(hermesPath, profile, opts: any = {}) { if (!opts.ownershipId || !opts.lockMetadata) { return withRemoteUpdateMutex( - `${markerClear}; marker_clear || exit 75; ` + - `mkdir -p "$(dirname ${logPath})" && ` + + `mkdir -p "$(dirname ${logPath})" && ` + `${detachedSpawn}; ` + - `marker_clear || { kill "$child" 2>/dev/null || true; wait "$child" 2>/dev/null || true; exit 75; }; echo "$child"`, - updateMutex + `${markerAbsent} || { kill "$child" 2>/dev/null || true; wait "$child" 2>/dev/null || true; exit 75; }; echo "$child"`, + marker ) } @@ -1232,9 +1164,9 @@ function buildSpawnCommand(hermesPath, profile, opts: any = {}) { `existing_pid=$(sed -n 's/.*"pid":\\([0-9][0-9]*\\).*/\\1/p' "$lock" | head -n 1); ` + `case "$existing_pid" in ''|*[!0-9]*) rm -f "$lock";; *) ` + `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})" && ` + + `mkdir -p "$(dirname ${logPath})" && ` + `${detachedSpawn}; ` + - `marker_clear || { kill "$child" 2>/dev/null || true; wait "$child" 2>/dev/null || true; exit 75; }; ` + + `${markerAbsent} || { 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 @@ -1244,7 +1176,7 @@ function buildSpawnCommand(hermesPath, profile, opts: any = {}) { `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 + marker ) } diff --git a/apps/desktop/electron/remote-update-marker-programs.ts b/apps/desktop/electron/remote-update-marker-programs.ts index 75d1c8f038..2eab8818cf 100644 --- a/apps/desktop/electron/remote-update-marker-programs.ts +++ b/apps/desktop/electron/remote-update-marker-programs.ts @@ -1,14 +1,13 @@ /** - * The update-marker judge as a program that runs ON an SSH remote. + * The update-marker judge as programs that run ON an SSH remote. * - * One parser and one identity rule for every Desktop remote reader (the POSIX - * relaunch gate, the spawn recheck, the managed-update observer), the same as - * `hermes_cli/update_lock.py::judge_marker` and `tests/fixtures/update_marker_corpus.json`: + * Same contract as `update-marker-judge.ts` (`tests/fixtures/update_marker_corpus.json`): * an identity (pid, ct) is live when the pid is alive and its creation time is * within 2 s of the recorded `ct:`; with no recorded/readable ct it is live only * while `now - started_at <= 1200`. Owner or delegate live => LIVE, both dead => - * CLEAR, malformed => UNCERTAIN. Only the host's system Python runs it: nothing - * imports the checkout an updater may be replacing. + * CLEAR, malformed => UNCERTAIN. Only the host's system Python (POSIX) or + * PowerShell/.NET (Windows relaunch/spawn) runs these: nothing imports the + * checkout an updater may be replacing. * * The Python stays free of double quotes: managed-ssh-update ships it to Windows * as a PowerShell native argument, and PowerShell 5.1 does not escape them. @@ -90,10 +89,9 @@ def marker_ct(pid): if sys.platform.startswith('linux'): with open('/proc/stat') as stat:btime=next(int(line.split()[1]) for line in stat if line.startswith('btime ')) return btime+int(marker_stat(pid)[19])/os.sysconf('SC_CLK_TCK') - # UTC like update_lock._stdlib_create_time: a local-time lstart is ambiguous in the repeated DST hour. - import calendar,subprocess - out=subprocess.check_output(['ps','-o','lstart=','-p',str(pid)],env=dict(os.environ,LC_ALL='C',TZ='UTC0'),universal_newlines=True) - return float(calendar.timegm(time.strptime(' '.join(out.split()),'%a %b %d %H:%M:%S %Y'))) + import subprocess + out=subprocess.check_output(['ps','-o','lstart=','-p',str(pid)],env=dict(os.environ,LC_ALL='C'),universal_newlines=True) + return time.mktime(time.strptime(out.strip(),'%a %b %d %H:%M:%S %Y')) except Exception:return None MARKER_ENV={'our_pid':os.getpid(),'our_ct':lambda:marker_ct(os.getpid()),'alive':marker_alive,'ct':marker_ct,'now':time.time()} @@ -104,3 +102,77 @@ def marker_verdict(raw): verdict,owner=marker_judge(raw.decode('utf-8','replace'),MARKER_ENV) return 'CLEAR' if verdict=='dead' else 'UNCERTAIN' if verdict=='malformed' else 'LIVE:%d'%owner ` + +/** + * POSIX gate: `python3 -c GATE [payload]`. Holds the updaters' kernel + * lock `.lock` (A7 rule 1: Python update_lock flock, marker.sh flock) + * for a bounded 10 s, judges the marker, unlinks a dead claim inside that hold, + * then either prints the verdict (no payload: the relaunch probe) or runs the + * payload as `sh -c payload hermes-update-mutex ` still holding the lock. + * A refused payload exits 75 with the verdict on stderr. The probe skips the + * lock when there is no marker, so it never creates files on a clean host. + */ +export const REMOTE_MARKER_GATE_PY = `${REMOTE_MARKER_JUDGE_PY} +import fcntl,subprocess +marker=sys.argv[1] +payload=sys.argv[2] if len(sys.argv)>2 else None +if payload is None and not os.path.lexists(marker): + print('CLEAR');sys.exit(0) + +def hold(fd): + deadline=time.monotonic()+10 + while True: + try: + fcntl.flock(fd,fcntl.LOCK_EX|fcntl.LOCK_NB);return True + except BlockingIOError: + if time.monotonic()>=deadline:return False + time.sleep(0.02) + +def read_marker(): + try: + with open(marker,'rb') as stream:return stream.read(4097) + except FileNotFoundError:return None + +os.makedirs(os.path.dirname(marker),exist_ok=True) +try:fd=os.open(marker+'.lock',os.O_RDWR|os.O_CREAT|os.O_CLOEXEC,0o644) +except PermissionError:fd=os.open(marker+'.lock',os.O_RDONLY|os.O_CLOEXEC) +verdict='UNCERTAIN' +if hold(fd): + raw=read_marker();verdict=marker_verdict(raw) + if verdict=='CLEAR' and raw is not None: + try:os.unlink(marker) + except FileNotFoundError:pass +if payload is None or verdict!='CLEAR': + print(verdict,file=sys.stderr if payload else sys.stdout);sys.exit(75 if payload else 0) +sys.exit(subprocess.run(['sh','-c',payload,'hermes-update-mutex',str(fd)],pass_fds=(fd,)).returncode) +` + +/** + * PowerShell `Get-MarkerVerdict $text` -> CLEAR | LIVE: | UNCERTAIN, the + * same rule via .NET process facts (Process.StartTime = psutil create_time on + * Windows; a process we may not query is alive with no ct). Windows runs this + * instead of the venv python so a probe never holds the runtime's python.exe + * open while an updater replaces it. Our own pid is never a remote owner. + */ +export const WINDOWS_MARKER_JUDGE_PS = [ + 'function Test-MarkerIdentity($ownerId,$ct,$started,$now){', + 'if($ownerId -eq 0 -or $ownerId -eq $PID -or $ownerId -gt [int]::MaxValue){return $false}', + 'try{$proc=[Diagnostics.Process]::GetProcessById([int]$ownerId)}catch [ArgumentException]{return $false}', + '$actual=$null', + 'try{if($proc.HasExited){return $false};if($null -ne $ct){$actual=([DateTimeOffset]$proc.StartTime).ToUnixTimeMilliseconds()/1000.0}}catch{}finally{$proc.Dispose()}', + 'if($null -eq $ct -or $null -eq $actual){return ($now-[double]$started) -le 1200}', + 'return [Math]::Abs($ct-$actual) -le 2.0', + '}', + 'function Get-MarkerCt([string]$digits){$value=0.0;if([double]::TryParse($digits,[Globalization.NumberStyles]::AllowDecimalPoint,[Globalization.CultureInfo]::InvariantCulture,[ref]$value)){return $value};return [double]::PositiveInfinity}', + 'function Get-MarkerVerdict([string]$text){', + '$lines=@(($text -replace "^\\uFEFF","") -split "`n" | ForEach-Object {($_ -replace "`r$","").Trim([char[]]" `t")})', + '$style=[Globalization.NumberStyles]::None;$culture=[Globalization.CultureInfo]::InvariantCulture;[uint32]$ownerId=0;[uint64]$started=0', + 'if($lines.Count -lt 2 -or $lines[0] -cnotmatch "^[0-9]+$" -or $lines[1] -cnotmatch "^[0-9]+$" -or -not [uint32]::TryParse($lines[0],$style,$culture,[ref]$ownerId) -or -not [uint64]::TryParse($lines[1],$style,$culture,[ref]$started)){return "UNCERTAIN"}', + '$ct=$null;if($lines.Count -gt 2 -and $lines[2] -cmatch "^ct:([0-9]+(\\.[0-9]+)?)$"){$ct=Get-MarkerCt $Matches[1]}', + '$ids=@(,@($ownerId,$ct))', + 'foreach($line in @($lines | Select-Object -Skip 3)){[uint32]$delegateId=0;if($line -cmatch "^delegate:([0-9]+) ct:([0-9]+(\\.[0-9]+)?)$" -and [uint32]::TryParse($Matches[1],$style,$culture,[ref]$delegateId)){$ids+=,@($delegateId,(Get-MarkerCt $Matches[2]));break}}', + '$now=[DateTimeOffset]::UtcNow.ToUnixTimeSeconds()', + 'foreach($id in $ids){if(Test-MarkerIdentity $id[0] $id[1] $started $now){return "LIVE:$($id[0])"}}', + 'return "CLEAR"', + '}' +].join('\n') diff --git a/apps/desktop/electron/windows-remote-lifecycle.test.ts b/apps/desktop/electron/windows-remote-lifecycle.test.ts index 670c4d5c9f..8885871304 100644 --- a/apps/desktop/electron/windows-remote-lifecycle.test.ts +++ b/apps/desktop/electron/windows-remote-lifecycle.test.ts @@ -1,5 +1,10 @@ import assert from 'node:assert/strict' +import { exec as execCallback, spawn } from 'node:child_process' import crypto from 'node:crypto' +import { mkdtemp, readFile, rm } from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' +import { promisify } from 'node:util' import { test } from 'vitest' @@ -31,8 +36,10 @@ test('Windows spawn holds the update mutex across marker check and helper spawn' const encoded = command.match(/-EncodedCommand\s+([^\s]+)$/)?.[1] const script = encoded ? Buffer.from(encoded, 'base64').toString('utf16le') : '' assert.match(script, /\.hermes-update-in-progress/) - assert.match(script, /\$mutexPath=\$marker\+"\.mutex"/) - assert.match(script, /\.Lock\(0,1\)/) + assert.match( + script, + /\$marker\+"\.lock",\[IO\.FileMode\]::OpenOrCreate,\[IO\.FileAccess\]::ReadWrite,\[IO\.FileShare\]::None/ + ) assert.match(script, /windows_ssh_runtime.*spawn/) }) @@ -60,9 +67,65 @@ test('Windows spawn publishes the initial ownership record before releasing the assert.match(script, /write-lock/) assert.match(script, /\$lock\s*\|\s*&.*write-lock/) assert.doesNotMatch(script, /write-lock[^;]*\$lock\|Out-Null/) - assert.ok(script.indexOf('write-lock') < script.indexOf('Unlock')) + assert.ok(script.indexOf('write-lock') < script.lastIndexOf('$mutex.Dispose()')) }) +test.runIf(process.platform === 'win32')( + 'Windows spawn waits on .lock and judges v2 claims: live refuses, dead is deleted under the lock', + async () => { + const root = await mkdtemp(path.join(os.tmpdir(), 'hermes-win-marker-')) + const marker = path.join(root, '.hermes-update-in-progress') + const run = (command: string) => promisify(execCallback)(command, { timeout: 30_000 }) + const command = atomicWindowsSpawnCommand({ hermesHome: root, python: path.join(root, 'missing-python.exe') }) + + // An updater: hold .lock sharing nothing (marker.ps1 Open-MarkerLock), + // write its v2 claim, release, and stay alive as the live owner. + const updater = spawn( + 'powershell.exe', + [ + '-NoProfile', + '-NonInteractive', + '-EncodedCommand', + encodedPowerShell( + [ + `$m=${psLiteral(marker)}`, + "$f=[IO.File]::Open($m+'.lock','OpenOrCreate','ReadWrite','None')", + "'held'", + 'Start-Sleep -Milliseconds 500', + '$ct=([DateTimeOffset](Get-Process -Id $PID).StartTime).ToUnixTimeMilliseconds()/1000.0', + '[IO.File]::WriteAllText($m,"$PID`n$([DateTimeOffset]::UtcNow.ToUnixTimeSeconds())`nct:$ct`n")', + 'Start-Sleep -Milliseconds 500', + '$f.Dispose()', + 'Start-Sleep -Seconds 30' + ].join(';') + ) + ], + { stdio: ['ignore', 'pipe', 'inherit'] } + ) + + try { + await new Promise(resolve => updater.stdout.once('data', resolve)) + await assert.rejects(run(command), (error: any) => + String(error.stderr).includes(`remote update marker is LIVE:${updater.pid}`) + ) + + updater.kill() + await new Promise(resolve => updater.once('exit', resolve)) + await assertWindowsRemoteInstallUpdateClear( + sshWith(async (probe: string) => (await run(probe)).stdout), + root + ) + // The dead claim is deleted inside the hold; the missing python then fails the spawn itself. + await assert.rejects(run(command)) + await assert.rejects(readFile(marker), { code: 'ENOENT' }) + } finally { + updater.kill() + await rm(root, { recursive: true, force: true }) + } + }, + 60_000 +) + function sshWith(exec) { return { exec } } @@ -175,7 +238,7 @@ test('Windows relaunch gate uses strict install-wide marker parsing and fail-clo assert.match(script, /\.hermes-update-in-progress/) assert.match(script, /Split-Path -Leaf \$parent.*profiles/) assert.match(script, /UTF8Encoding.*true/) - assert.match(script, /\\A\(\[1-9\]/) + assert.match(script, /\$result=Get-MarkerVerdict \$text/) assert.match(script, /GetProcessById/) assert.doesNotMatch(script, /ErrorAction SilentlyContinue/) }) diff --git a/apps/desktop/electron/windows-remote-lifecycle.ts b/apps/desktop/electron/windows-remote-lifecycle.ts index 830c623953..f78300f21f 100644 --- a/apps/desktop/electron/windows-remote-lifecycle.ts +++ b/apps/desktop/electron/windows-remote-lifecycle.ts @@ -1,6 +1,7 @@ import crypto from 'node:crypto' import { resolveReadyTimeoutMs } from './remote-lifecycle' +import { WINDOWS_MARKER_JUDGE_PS } from './remote-update-marker-programs' import { assertBootstrapNotSuperseded, redactSecrets, SSH_ERROR } from './ssh-connection' const LOCKFILE_SCHEMA_VERSION = 2 @@ -125,6 +126,7 @@ public static class HermesMarkerNoFollow { } '@ `, + WINDOWS_MARKER_JUDGE_PS, 'function Assert-NoReparse([string]$candidate,[bool]$allowMissing=$false){', 'if([string]::IsNullOrWhiteSpace($candidate)){return}', '$current=[IO.Path]::GetFullPath($candidate);$first=$true', @@ -146,21 +148,10 @@ public static class HermesMarkerNoFollow { 'if(-not (Test-Path -LiteralPath $marker -PathType Leaf)){$result="CLEAR"}else{$stream=[HermesMarkerNoFollow]::OpenRead($marker)', 'Assert-NoReparse $marker $false', '$memory=New-Object IO.MemoryStream;$stream.CopyTo($memory);$bytes=$memory.ToArray()', - 'if($bytes.Length -le 256){', + 'if($bytes.Length -le 4096){', '$utf8=[Text.UTF8Encoding]::new($false,$true)', '$text=$utf8.GetString($bytes)', - "$match=[regex]::Match($text,'\\A([1-9][0-9]*)\\r?\\n([0-9]+)(?:\\r?\\n)?\\z')", - '[uint32]$ownerPid=0', - '[uint64]$lease=0', - '$valid=$match.Success', - 'if($valid){$valid=[uint32]::TryParse($match.Groups[1].Value,[Globalization.NumberStyles]::None,[Globalization.CultureInfo]::InvariantCulture,[ref]$ownerPid)}', - 'if($valid){$valid=[uint64]::TryParse($match.Groups[2].Value,[Globalization.NumberStyles]::None,[Globalization.CultureInfo]::InvariantCulture,[ref]$lease)}', - 'if($valid -and $lease -le 9007199254740991){', - 'try{', - '$process=[Diagnostics.Process]::GetProcessById([int]$ownerPid)', - 'try{if($process.HasExited){$result="CLEAR"}else{$result="LIVE:"+[string]$ownerPid}}finally{$process.Dispose()}', - '}catch [ArgumentException]{$result="CLEAR"} catch{$result="UNCERTAIN"}', - '}', + '$result=Get-MarkerVerdict $text', '}', '}}catch [IO.FileNotFoundException]{$result="CLEAR"}catch{$result="UNCERTAIN"}finally{if($memory){$memory.Dispose()};if($stream){$stream.Dispose()}}', 'Write-Output $result' @@ -287,16 +278,19 @@ function atomicWindowsSpawnCommand(runtime, reservation: any = {}) { const script = [ '$ProgressPreference="SilentlyContinue"', '$ErrorActionPreference="Stop"', + WINDOWS_MARKER_JUDGE_PS, `$hermesHome=${psLiteral(runtime.hermesHome)}`, '$installRoot=$hermesHome', '$parent=Split-Path -Parent $hermesHome', 'if((Split-Path -Leaf $parent) -ieq "profiles"){$installRoot=Split-Path -Parent $parent}', '$marker=Join-Path $installRoot ".hermes-update-in-progress"', - '$mutexPath=$marker+".mutex"', - '$mutex=[IO.File]::Open($mutexPath,[IO.FileMode]::OpenOrCreate,[IO.FileAccess]::ReadWrite,[IO.FileShare]::ReadWrite)', + // The updaters' kernel lock (A7 rule 1): `.lock` opened sharing + // nothing, like marker.ps1 Open-MarkerLock / update_lock.py, bounded 10 s. + // A dead claim is deleted inside the hold; a live/malformed one refuses. + '$clock=[Diagnostics.Stopwatch]::StartNew();$mutex=$null', + 'while(-not $mutex){try{$mutex=[IO.File]::Open($marker+".lock",[IO.FileMode]::OpenOrCreate,[IO.FileAccess]::ReadWrite,[IO.FileShare]::None)}catch{if($clock.ElapsedMilliseconds -ge 10000){throw "remote update marker lock is busy"};Start-Sleep -Milliseconds 25}}', 'try{', - ' $mutex.Lock(0,1)', - ' if([IO.File]::Exists($marker)){throw "remote update marker is present"}', + ' if([IO.File]::Exists($marker)){$verdict=Get-MarkerVerdict ([IO.File]::ReadAllText($marker));if($verdict -ne "CLEAR"){throw "remote update marker is $verdict"};[IO.File]::Delete($marker)}', reservation.ownershipId ? ` $existingLines=@(& ${helper('read-lock').map(psLiteral).join(' ')} ${psLiteral(reservation.ownershipId)}); $existingExit=$LASTEXITCODE; ` + ' if($existingExit -eq 0 -and $existingLines.Count -gt 0){try{$existing=$existingLines[-1]|ConvertFrom-Json}catch{$existing=$null}; ' + @@ -315,7 +309,7 @@ function atomicWindowsSpawnCommand(runtime, reservation: any = {}) { ` $lock | & ${helper('write-lock').map(psLiteral).join(' ')} ${psLiteral(reservation.ownershipId)} | Out-Null; if($LASTEXITCODE -ne 0){exit $LASTEXITCODE}; $spawnLines|Write-Output` : '', ' if([IO.File]::Exists($marker)){throw "remote update marker claimed during backend spawn"}', - '}finally{try{$mutex.Unlock(0,1)}catch{};$mutex.Dispose()}' + '}finally{$mutex.Dispose()}' ] .filter(line => line !== '') .join(';')