diff --git a/src/main/ssh/relay-daemon-service-children.ts b/src/main/ssh/relay-daemon-service-children.ts new file mode 100644 index 00000000000..35e755ea518 --- /dev/null +++ b/src/main/ssh/relay-daemon-service-children.ts @@ -0,0 +1,62 @@ +/** + * Telling a relay daemon's own service processes apart from the work it holds. + * + * The reap gate used to ask `pgrep -P | grep -c .` and demand zero. But the daemon + * forks service children of its own — `relay-ai-vault-service.js` is spawned lazily and then + * never exits — so that count is permanently non-zero on any relay that has touched the AI + * Vault, whether or not it holds a single PTY. A superseded, disconnected relay holding + * nothing therefore reported `retained-live-work` forever, its version directory stayed + * pinned against GC by its own live socket, and the population grew without bound (#13614). + * + * The asymmetry below is the whole safety argument, and it follows + * docs/reference/ssh-execution-boundary.md: *subtracting a child we can positively identify + * as relay infrastructure is sound; assuming anything about a child we cannot identify is + * not.* An argv that does not match, an argv `ps` would not print, and a host without + * `pgrep` all count against the relay and keep it unreapable. Losing sight of a child is + * never evidence that it holds nothing. + */ +import { RELAY_DAEMON_SERVICE_ENTRY_FILENAMES } from '../../shared/relay-artifacts' +import { shellEscape } from './ssh-connection-utils' + +/** Shell variable set to the daemon's direct-child count, or `unknown`. */ +export const RELAY_CHILD_COUNT_VAR = 'kids' + +/** Shell variable set to the count of children not identified as relay services, or `unknown`. */ +export const RELAY_UNRECOGNIZED_CHILD_COUNT_VAR = 'unrecognized_kids' + +/** + * `case` patterns matching a service child's argv. Suffix-anchored on purpose: both entries + * are forked with no script arguments, so the argv ends at the filename, and the leading `/` + * requires the absolute path the daemon forks rather than a bare mention of the name. A + * future arg would stop matching and the relay would go back to being retained — the safe + * direction to fail in. + */ +function serviceChildArgvPatterns(): string { + return RELAY_DAEMON_SERVICE_ENTRY_FILENAMES.map( + (filename) => `*${shellEscape(`/${filename}`)}` + ).join('|') +} + +/** + * POSIX shell that censuses the direct children of `$pid`, setting `kids` and + * `unrecognized_kids`. Both stay `unknown` when the host cannot enumerate children at all. + */ +export function relayDaemonChildCensusShell(): string[] { + return [ + `${RELAY_CHILD_COUNT_VAR}=unknown`, + `${RELAY_UNRECOGNIZED_CHILD_COUNT_VAR}=unknown`, + 'if command -v pgrep >/dev/null 2>&1; then', + ` ${RELAY_CHILD_COUNT_VAR}=0`, + ` ${RELAY_UNRECOGNIZED_CHILD_COUNT_VAR}=0`, + ' for kid in $(pgrep -P "$pid" 2>/dev/null); do', + ` ${RELAY_CHILD_COUNT_VAR}=$((${RELAY_CHILD_COUNT_VAR}+1))`, + ' kid_args=$(ps -o args= -p "$kid" 2>/dev/null | tr -d "\\n")', + ' case "$kid_args" in', + ` ${serviceChildArgvPatterns()}) ;;`, + // An unreadable or unrecognised argv lands here, which is what keeps the relay retained. + ` *) ${RELAY_UNRECOGNIZED_CHILD_COUNT_VAR}=$((${RELAY_UNRECOGNIZED_CHILD_COUNT_VAR}+1)) ;;`, + ' esac', + ' done', + 'fi' + ] +} diff --git a/src/main/ssh/ssh-relay-endpoint-incumbent-shell.integration.test.ts b/src/main/ssh/ssh-relay-endpoint-incumbent-shell.integration.test.ts index 7ece0b6532e..a8975d0520b 100644 --- a/src/main/ssh/ssh-relay-endpoint-incumbent-shell.integration.test.ts +++ b/src/main/ssh/ssh-relay-endpoint-incumbent-shell.integration.test.ts @@ -4,7 +4,7 @@ * generated scripts through /bin/sh against real unix sockets and real processes. */ import { execFile, spawn, type ChildProcess } from 'node:child_process' -import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest' @@ -15,21 +15,32 @@ import { type RelayEndpointIncumbent } from './ssh-relay-endpoint-incumbent' import { reapEmptyRelayHuskCommand } from './ssh-relay-endpoint-takeover' +import { RELAY_DAEMON_SERVICE_ENTRY_FILENAMES } from '../../shared/relay-artifacts' const posixOnly = process.platform === 'win32' ? describe.skip : describe const FAKE_RELAY_SOURCE = ` const net = require('net') +const path = require('path') const sock = process.argv[process.argv.indexOf('--sock-path') + 1] +function spawnChild(args) { + require('child_process').spawn(process.execPath, args, { stdio: 'ignore' }) +} if (process.argv.includes('--with-child')) { - require('child_process').spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { - stdio: 'ignore' - }) + spawnChild(['-e', 'setTimeout(() => {}, 60000)']) +} +// Why forked the same way production does: the exclusion is argv-shaped, so a hand-written +// stand-in would test the test rather than the shell that runs on someone's host. +for (const name of process.argv.filter((arg) => arg.startsWith('--service-child='))) { + spawnChild([path.join(__dirname, name.slice('--service-child='.length))]) } net.createServer(() => {}).listen(sock, () => process.stdout.write('READY\\n')) process.on('SIGTERM', () => process.exit(0)) ` +// Self-limiting: these are orphaned when the relay under test is reaped. +const IDLE_SERVICE_SOURCE = 'setTimeout(() => {}, 60000)\n' + function sh(script: string): Promise { return new Promise((resolve, reject) => { execFile('/bin/sh', ['-c', script], { timeout: 20_000 }, (error, stdout) => { @@ -43,14 +54,21 @@ function sh(script: string): Promise { } let workDir: string +let pgreplessBinDir: string let hasLsof = false const running: ChildProcess[] = [] -function startFakeRelay(sockPath: string, withChild = false): Promise { +function startFakeRelay( + sockPath: string, + options: { withChild?: boolean; serviceChildren?: readonly string[] } = {} +): Promise { const args = [join(workDir, 'relay.js'), '--sock-path', sockPath] - if (withChild) { + if (options.withChild) { args.push('--with-child') } + for (const name of options.serviceChildren ?? []) { + args.push(`--service-child=${name}`) + } const child = spawn(process.execPath, args, { stdio: ['ignore', 'pipe', 'ignore'] }) running.push(child) return new Promise((resolve, reject) => { @@ -68,9 +86,31 @@ async function probe(sockPath: string): Promise { return parseRelayEndpointIncumbentProbe(sockPath, output) } +/** The relay forks its children after it starts listening, so the probe can race them. */ +async function waitForChildCount( + sockPath: string, + expected: number +): Promise { + let incumbent = await probe(sockPath) + for (let attempt = 0; attempt < 50 && incumbent.holders[0]?.childCount !== expected; attempt++) { + await new Promise((resolve) => setTimeout(resolve, 100)) + incumbent = await probe(sockPath) + } + return incumbent +} + beforeAll(async () => { workDir = mkdtempSync(join(tmpdir(), 'orca-relay-incumbent-')) writeFileSync(join(workDir, 'relay.js'), FAKE_RELAY_SOURCE) + for (const filename of RELAY_DAEMON_SERVICE_ENTRY_FILENAMES) { + writeFileSync(join(workDir, filename), IDLE_SERVICE_SOURCE) + } + writeFileSync(join(workDir, 'looks-like-relay-watcher.js'), IDLE_SERVICE_SOURCE) + pgreplessBinDir = join(workDir, 'pgrepless-bin') + mkdirSync(pgreplessBinDir) + for (const tool of ['ps', 'tr']) { + symlinkSync((await sh(`command -v ${tool}`)).trim(), join(pgreplessBinDir, tool)) + } hasLsof = await sh('command -v lsof >/dev/null 2>&1 && echo yes || echo no').then( (out) => out.trim() === 'yes' ) @@ -105,13 +145,51 @@ posixOnly('relay endpoint probe against a real socket', () => { return } expect(incumbent.holders.map((holder) => holder.pid)).toEqual([relay.pid]) - expect(incumbent.holders[0]).toMatchObject({ matchesRelayArgv: true, childCount: 0 }) + expect(incumbent.holders[0]).toMatchObject({ + matchesRelayArgv: true, + childCount: 0, + unrecognizedChildCount: 0 + }) expect(isReapableRelayHusk(incumbent)).toBe(true) }) + it("counts the daemon's own service children but does not hold them against it", async () => { + const sockPath = join(workDir, 'services.sock') + await startFakeRelay(sockPath, { serviceChildren: RELAY_DAEMON_SERVICE_ENTRY_FILENAMES }) + const incumbent = await waitForChildCount(sockPath, RELAY_DAEMON_SERVICE_ENTRY_FILENAMES.length) + + expect(incumbent.holders[0].childCount).toBe(RELAY_DAEMON_SERVICE_ENTRY_FILENAMES.length) + expect(incumbent.holders[0].unrecognizedChildCount).toBe(0) + expect(isReapableRelayHusk(incumbent)).toBe(true) + }) + + it('still retains a relay holding work alongside its service children', async () => { + const sockPath = join(workDir, 'services-and-work.sock') + await startFakeRelay(sockPath, { + withChild: true, + serviceChildren: RELAY_DAEMON_SERVICE_ENTRY_FILENAMES + }) + const incumbent = await waitForChildCount( + sockPath, + RELAY_DAEMON_SERVICE_ENTRY_FILENAMES.length + 1 + ) + + expect(incumbent.holders[0].unrecognizedChildCount).toBe(1) + expect(isReapableRelayHusk(incumbent)).toBe(false) + }) + + it('does not excuse a child that merely mentions a service entry name', async () => { + const sockPath = join(workDir, 'lookalike.sock') + await startFakeRelay(sockPath, { serviceChildren: ['looks-like-relay-watcher.js'] }) + const incumbent = await waitForChildCount(sockPath, 1) + + expect(incumbent.holders[0].unrecognizedChildCount).toBe(1) + expect(isReapableRelayHusk(incumbent)).toBe(false) + }) + it('refuses to call a relay with a live child an empty husk', async () => { const sockPath = join(workDir, 'busy.sock') - await startFakeRelay(sockPath, true) + await startFakeRelay(sockPath, { withChild: true }) const incumbent = await probe(sockPath) expect(incumbent.verdict).toBe('live') @@ -150,12 +228,34 @@ posixOnly('empty relay husk reap against a real process', () => { it('refuses to signal a relay that acquired a child after it was probed', async () => { const sockPath = join(workDir, 'raced.sock') - const relay = await startFakeRelay(sockPath, true) + const relay = await startFakeRelay(sockPath, { withChild: true }) const output = await sh(reapEmptyRelayHuskCommand(relay.pid!, sockPath)) expect(output.trim()).toBe('BUSY') expect(relay.killed).toBe(false) }) + it('terminates a relay whose only children are its own service processes (#13614)', async () => { + const sockPath = join(workDir, 'service-husk.sock') + const relay = await startFakeRelay(sockPath, { + serviceChildren: RELAY_DAEMON_SERVICE_ENTRY_FILENAMES + }) + await waitForChildCount(sockPath, RELAY_DAEMON_SERVICE_ENTRY_FILENAMES.length) + const output = await sh(reapEmptyRelayHuskCommand(relay.pid!, sockPath)) + expect(output.trim()).toBe('GONE') + }) + + it('refuses to signal when the host cannot enumerate children at all', async () => { + const sockPath = join(workDir, 'no-pgrep.sock') + const relay = await startFakeRelay(sockPath) + // A PATH carrying every tool the script needs except `pgrep`: the census answers + // `unknown`, which must reach BUSY rather than the zero a missing tool would imply. + const output = await sh( + `PATH=${pgreplessBinDir}\n${reapEmptyRelayHuskCommand(relay.pid!, sockPath)}` + ) + expect(output.trim()).toBe('BUSY') + expect(relay.killed).toBe(false) + }) + it('refuses to signal a pid whose argv is not this relay at this socket', async () => { const sockPath = join(workDir, 'mismatch.sock') await startFakeRelay(sockPath) diff --git a/src/main/ssh/ssh-relay-endpoint-incumbent.test.ts b/src/main/ssh/ssh-relay-endpoint-incumbent.test.ts index a65cb33fc57..de4cc28d170 100644 --- a/src/main/ssh/ssh-relay-endpoint-incumbent.test.ts +++ b/src/main/ssh/ssh-relay-endpoint-incumbent.test.ts @@ -32,17 +32,24 @@ describe('parseRelayEndpointIncumbentProbe', () => { it('reports live when the socket accepted a connection', () => { const incumbent = parseRelayEndpointIncumbentProbe( SOCK, - probeOutput(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=4242 yes 13']) + probeOutput([ + 'PRESENT=yes', + 'LISTEN=accepted', + 'HOLDERS_SOURCE=lsof', + 'HOLDER=4242 yes 13 11' + ]) ) expect(incumbent.verdict).toBe('live') expect(incumbent.evidence).toBe('accepted-connection') - expect(incumbent.holders).toEqual([{ pid: 4242, matchesRelayArgv: true, childCount: 13 }]) + expect(incumbent.holders).toEqual([ + { pid: 4242, matchesRelayArgv: true, childCount: 13, unrecognizedChildCount: 11 } + ]) }) it('reports live when a process still holds an inode that refuses connections', () => { const incumbent = parseRelayEndpointIncumbentProbe( SOCK, - probeOutput(['PRESENT=yes', 'LISTEN=refused', 'HOLDERS_SOURCE=lsof', 'HOLDER=91 yes 2']) + probeOutput(['PRESENT=yes', 'LISTEN=refused', 'HOLDERS_SOURCE=lsof', 'HOLDER=91 yes 2 2']) ) expect(incumbent.verdict).toBe('live') expect(incumbent.evidence).toBe('holder-process') @@ -85,7 +92,12 @@ describe('parseRelayEndpointIncumbentProbe', () => { it('drops holder lines that do not carry a usable pid', () => { const incumbent = parseRelayEndpointIncumbentProbe( SOCK, - probeOutput(['PRESENT=yes', 'LISTEN=refused', 'HOLDERS_SOURCE=lsof', 'HOLDER=- no unknown']) + probeOutput([ + 'PRESENT=yes', + 'LISTEN=refused', + 'HOLDERS_SOURCE=lsof', + 'HOLDER=- no unknown unknown' + ]) ) expect(incumbent.holders).toEqual([]) expect(incumbent.verdict).toBe('exited') @@ -94,9 +106,24 @@ describe('parseRelayEndpointIncumbentProbe', () => { it('keeps an unreadable child count as null rather than zero', () => { const [holder] = parseRelayEndpointIncumbentProbe( SOCK, - probeOutput(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=7 yes unknown']) + probeOutput([ + 'PRESENT=yes', + 'LISTEN=accepted', + 'HOLDERS_SOURCE=lsof', + 'HOLDER=7 yes unknown unknown' + ]) ).holders expect(holder.childCount).toBeNull() + expect(holder.unrecognizedChildCount).toBeNull() + }) + + it('keeps a holder line with no unrecognized-child field unreapable', () => { + const incumbent = parseRelayEndpointIncumbentProbe( + SOCK, + probeOutput(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=7 yes 0']) + ) + expect(incumbent.holders[0].unrecognizedChildCount).toBeNull() + expect(isReapableRelayHusk(incumbent)).toBe(false) }) }) @@ -172,27 +199,36 @@ describe('mayLaunchOverRelayEndpoint', () => { describe('isReapableRelayHusk', () => { const husk = parseRelayEndpointIncumbentProbe( SOCK, - probeOutput(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=500 yes 0']) + probeOutput(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=500 yes 0 0']) ) - it('accepts a single proven relay holder with zero children', () => { + it('accepts a single proven relay holder with no unaccounted-for children', () => { expect(isReapableRelayHusk(husk)).toBe(true) }) - it('refuses a relay that still holds children', () => { + it('accepts a relay whose only children are its own service processes (#13614)', () => { + const withServices = parseRelayEndpointIncumbentProbe( + SOCK, + probeOutput(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=500 yes 2 0']) + ) + expect(withServices.holders[0].childCount).toBe(2) + expect(isReapableRelayHusk(withServices)).toBe(true) + }) + + it('refuses a relay that still holds children it could not account for', () => { expect( isReapableRelayHusk({ ...husk, - holders: [{ pid: 500, matchesRelayArgv: true, childCount: 1 }] + holders: [{ pid: 500, matchesRelayArgv: true, childCount: 3, unrecognizedChildCount: 1 }] }) ).toBe(false) }) - it('refuses a holder whose child count could not be read', () => { + it('refuses a holder whose unrecognized-child count could not be read', () => { expect( isReapableRelayHusk({ ...husk, - holders: [{ pid: 500, matchesRelayArgv: true, childCount: null }] + holders: [{ pid: 500, matchesRelayArgv: true, childCount: 0, unrecognizedChildCount: null }] }) ).toBe(false) }) @@ -201,7 +237,7 @@ describe('isReapableRelayHusk', () => { expect( isReapableRelayHusk({ ...husk, - holders: [{ pid: 500, matchesRelayArgv: false, childCount: 0 }] + holders: [{ pid: 500, matchesRelayArgv: false, childCount: 0, unrecognizedChildCount: 0 }] }) ).toBe(false) }) @@ -211,8 +247,8 @@ describe('isReapableRelayHusk', () => { isReapableRelayHusk({ ...husk, holders: [ - { pid: 500, matchesRelayArgv: true, childCount: 0 }, - { pid: 501, matchesRelayArgv: true, childCount: 0 } + { pid: 500, matchesRelayArgv: true, childCount: 0, unrecognizedChildCount: 0 }, + { pid: 501, matchesRelayArgv: true, childCount: 0, unrecognizedChildCount: 0 } ] }) ).toBe(false) diff --git a/src/main/ssh/ssh-relay-endpoint-incumbent.ts b/src/main/ssh/ssh-relay-endpoint-incumbent.ts index 2688267f4c7..628a9558793 100644 --- a/src/main/ssh/ssh-relay-endpoint-incumbent.ts +++ b/src/main/ssh/ssh-relay-endpoint-incumbent.ts @@ -20,6 +20,11 @@ */ import type { SshConnection } from './ssh-connection' import { shellEscape } from './ssh-connection-utils' +import { + RELAY_CHILD_COUNT_VAR, + RELAY_UNRECOGNIZED_CHILD_COUNT_VAR, + relayDaemonChildCensusShell +} from './relay-daemon-service-children' import { execCommand, isUnconfirmedSshCommandTermination } from './ssh-relay-deploy-helpers' import { isWindowsRemoteHost, type RemoteHostPlatform } from './ssh-remote-platform' @@ -38,6 +43,12 @@ export type RelayEndpointHolder = { matchesRelayArgv: boolean /** Direct children, or null when `pgrep` could not answer. Never guessed. */ childCount: number | null + /** + * Direct children *not* positively identified as the daemon's own service processes, or + * null when the host could not enumerate them. This — not `childCount` — is what says + * whether the relay holds anything; see relay-daemon-service-children.ts. + */ + unrecognizedChildCount: number | null } export type RelayEndpointIncumbent = { @@ -95,11 +106,9 @@ export function relayEndpointIncumbentProbeCommand(nodePath: string, sockPath: s ' args=$(ps -o args= -p "$pid" 2>/dev/null | tr "\\n" " ")', ' match=no', ' case "$args" in *relay.js*"$sock"*) match=yes ;; esac', - ' kids=unknown', - ' if command -v pgrep >/dev/null 2>&1; then', - ' kids=$(pgrep -P "$pid" 2>/dev/null | grep -c .)', - ' fi', - ' printf \'HOLDER=%s %s %s\\n\' "$pid" "$match" "$kids"', + ...relayDaemonChildCensusShell().map((line) => ` ${line}`), + ' printf \'HOLDER=%s %s %s %s\\n\' "$pid" "$match" ' + + `"$${RELAY_CHILD_COUNT_VAR}" "$${RELAY_UNRECOGNIZED_CHILD_COUNT_VAR}"`, ' done', 'else', " printf 'HOLDERS_SOURCE=unavailable\\n'", @@ -159,19 +168,25 @@ export function parseRelayEndpointIncumbentProbe( } function parseHolder(value: string): RelayEndpointHolder | null { - const [rawPid, rawMatch, rawKids] = value.split(/\s+/) + const [rawPid, rawMatch, rawKids, rawUnrecognized] = value.split(/\s+/) const pid = Number.parseInt(rawPid ?? '', 10) if (!Number.isInteger(pid) || pid <= 0) { return null } - const childCount = Number.parseInt(rawKids ?? '', 10) return { pid, matchesRelayArgv: rawMatch === 'yes', - childCount: Number.isInteger(childCount) && childCount >= 0 ? childCount : null + childCount: parseChildCount(rawKids), + unrecognizedChildCount: parseChildCount(rawUnrecognized) } } +/** `unknown`, a missing field, and anything unparseable are all "could not tell" — never 0. */ +function parseChildCount(raw: string | undefined): number | null { + const count = Number.parseInt(raw ?? '', 10) + return Number.isInteger(count) && count >= 0 ? count : null +} + function unverifiableEndpoint(sockPath: string): RelayEndpointIncumbent { return { sockPath, @@ -239,8 +254,12 @@ export function mayLaunchOverRelayEndpoint(incumbent: RelayEndpointIncumbent): b /** * A live relay that provably holds nothing: identity confirmed against its argv, exactly one - * holder, and zero children. Reaping it destroys no user work. Anything less is retained — - * killing the wrong pid on someone's remote host is the worst outcome available here. + * holder, and no child the host could not account for as one of the daemon's own service + * processes. Reaping it destroys no user work. Anything less is retained — killing the wrong + * pid on someone's remote host is the worst outcome available here. + * + * Why not `childCount === 0`: the daemon's AI Vault sidecar never exits once spawned, so that + * gate was unreachable for any relay that had ever served a vault request (#13614). */ export function isReapableRelayHusk(incumbent: RelayEndpointIncumbent): boolean { if (incumbent.verdict !== 'live' || !incumbent.holdersEnumerable) { @@ -250,12 +269,16 @@ export function isReapableRelayHusk(incumbent: RelayEndpointIncumbent): boolean return false } const [holder] = incumbent.holders - return holder.matchesRelayArgv && holder.childCount === 0 + return holder.matchesRelayArgv && holder.unrecognizedChildCount === 0 } export function describeRelayEndpointIncumbent(incumbent: RelayEndpointIncumbent): string { const holders = incumbent.holders - .map((holder) => `${holder.pid}(children=${holder.childCount ?? 'unknown'})`) + .map( + (holder) => + `${holder.pid}(children=${holder.childCount ?? 'unknown'},` + + `unrecognized=${holder.unrecognizedChildCount ?? 'unknown'})` + ) .join(',') return ( `${incumbent.sockPath} verdict=${incumbent.verdict} evidence=${incumbent.evidence} ` + diff --git a/src/main/ssh/ssh-relay-endpoint-takeover.test.ts b/src/main/ssh/ssh-relay-endpoint-takeover.test.ts index 687d633b92b..d23f065f478 100644 --- a/src/main/ssh/ssh-relay-endpoint-takeover.test.ts +++ b/src/main/ssh/ssh-relay-endpoint-takeover.test.ts @@ -14,6 +14,7 @@ import { resolveRelayEndpointBeforeRelaunch } from './ssh-relay-endpoint-takeover' import { RelayVersionMismatchError } from './ssh-relay-version-mismatch-error' +import { RELAY_DAEMON_SERVICE_ENTRY_FILENAMES } from '../../shared/relay-artifacts' import type { SshConnection } from './ssh-connection' import { getRemoteHostPlatform } from './ssh-remote-platform' @@ -42,7 +43,7 @@ beforeEach(() => { describe('incumbent alive and refusing', () => { it('refuses to rebind a live relay holding PTYs, and signals nothing', async () => { execCommand.mockResolvedValueOnce( - probe(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=3669803 yes 13']) + probe(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=3669803 yes 13 11']) ) await expect(resolve()).rejects.toSatisfy(isRelayEndpointHeldError) // The whole point of #8585: the incumbent's socket must survive so it is not orphaned. @@ -52,9 +53,9 @@ describe('incumbent alive and refusing', () => { it('names the incumbent pid and the Reset Relay escape hatch in the error', async () => { execCommand.mockResolvedValue( - probe(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=3669803 yes 13']) + probe(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=3669803 yes 13 11']) ) - await expect(resolve()).rejects.toThrow(/3669803\(children=13\)/) + await expect(resolve()).rejects.toThrow(/3669803\(children=13,unrecognized=11\)/) await expect(resolve()).rejects.toThrow(/Reset Relay/) }) @@ -70,7 +71,7 @@ describe('incumbent alive and refusing', () => { it('reaps a live relay only when it provably holds nothing, and confirms it is gone', async () => { execCommand .mockResolvedValueOnce( - probe(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=80583 yes 0']) + probe(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=80583 yes 2 0']) ) .mockResolvedValueOnce('GONE\n') await expect(resolve()).resolves.toMatchObject({ verdict: 'live' }) @@ -80,7 +81,7 @@ describe('incumbent alive and refusing', () => { it('does not launch over an empty relay whose death could not be confirmed', async () => { execCommand .mockResolvedValueOnce( - probe(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=80583 yes 0']) + probe(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=80583 yes 2 0']) ) .mockResolvedValueOnce('LIVE\n') await expect(resolve()).rejects.toSatisfy(isRelayEndpointHeldError) @@ -89,7 +90,7 @@ describe('incumbent alive and refusing', () => { it('does not launch over a relay the host refused to signal on its own re-check', async () => { execCommand .mockResolvedValueOnce( - probe(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=80583 yes 0']) + probe(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=80583 yes 2 0']) ) .mockResolvedValueOnce('BUSY\n') await expect(resolve()).rejects.toSatisfy(isRelayEndpointHeldError) @@ -138,9 +139,19 @@ describe('reapEmptyRelayHuskCommand', () => { }) it('aborts without signalling when the host cannot count children', () => { - expect(reapEmptyRelayHuskCommand(4242, SOCK)).toContain( - "command -v pgrep >/dev/null 2>&1 || { printf 'BUSY\\n'; exit 0; }" - ) + const command = reapEmptyRelayHuskCommand(4242, SOCK) + // The census leaves both counters at `unknown` without pgrep, and the gate demands "0". + expect(command).toContain('unrecognized_kids=unknown') + expect(command).toContain('command -v pgrep >/dev/null 2>&1') + expect(command).toContain('[ "$unrecognized_kids" = "0" ] ||') + }) + + it('subtracts only the daemon service children it can name from the reap gate', () => { + const command = reapEmptyRelayHuskCommand(4242, SOCK) + for (const filename of RELAY_DAEMON_SERVICE_ENTRY_FILENAMES) { + expect(command).toContain(`*'/${filename}'`) + } + expect(command).toContain('unrecognized_kids=$((unrecognized_kids+1))') }) }) diff --git a/src/main/ssh/ssh-relay-endpoint-takeover.ts b/src/main/ssh/ssh-relay-endpoint-takeover.ts index f104aab5256..8f6130620cb 100644 --- a/src/main/ssh/ssh-relay-endpoint-takeover.ts +++ b/src/main/ssh/ssh-relay-endpoint-takeover.ts @@ -2,13 +2,17 @@ * Deciding whether a relay socket path is ours to take, and acting on the answer. * * The only destructive action available here is a SIGTERM to a relay that has been proven — - * by argv, by socket-holder enumeration, and by a zero child count re-checked on the host - * immediately before the signal — to hold nothing at all. Everything else is left running. + * by argv, by socket-holder enumeration, and by a child census re-run on the host immediately + * before the signal — to hold nothing at all. Everything else is left running. * Per docs/reference/ssh-execution-boundary.md, a relay we merely failed to reach is * `unverifiable`, and `unverifiable` never authorizes a kill or a rebind. */ import type { SshConnection } from './ssh-connection' import { shellEscape } from './ssh-connection-utils' +import { + RELAY_UNRECOGNIZED_CHILD_COUNT_VAR, + relayDaemonChildCensusShell +} from './relay-daemon-service-children' import { execCommand, isUnconfirmedSshCommandTermination } from './ssh-relay-deploy-helpers' import { describeRelayEndpointIncumbent, @@ -39,9 +43,10 @@ export function reapEmptyRelayHuskCommand(pid: number, sockPath: string): string `sock=${shellEscape(sockPath)}`, 'args=$(ps -o args= -p "$pid" 2>/dev/null | tr "\\n" " ")', 'case "$args" in *relay.js*"$sock"*) ;; *) printf \'MISMATCH\\n\'; exit 0 ;; esac', - "command -v pgrep >/dev/null 2>&1 || { printf 'BUSY\\n'; exit 0; }", - 'kids=$(pgrep -P "$pid" 2>/dev/null | grep -c .)', - '[ "$kids" = "0" ] || { printf \'BUSY\\n\'; exit 0; }', + // Why the same census as the probe: `unknown` (no pgrep) and any child this host could + // not account for as a relay service both land on BUSY, so nothing is signalled. + ...relayDaemonChildCensusShell(), + `[ "$${RELAY_UNRECOGNIZED_CHILD_COUNT_VAR}" = "0" ] || { printf 'BUSY\\n'; exit 0; }`, // SIGTERM only: the relay's own handler disposes and unlinks. SIGKILL would leave the // socket inode behind and skip that shutdown path for no gain on an empty daemon. 'kill -TERM "$pid" 2>/dev/null || true', diff --git a/src/main/ssh/ssh-relay-superseded-endpoints.test.ts b/src/main/ssh/ssh-relay-superseded-endpoints.test.ts index 874d9aae3fe..9168f4688bd 100644 --- a/src/main/ssh/ssh-relay-superseded-endpoints.test.ts +++ b/src/main/ssh/ssh-relay-superseded-endpoints.test.ts @@ -67,7 +67,7 @@ describe('classifySupersededRelay', () => { 'PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', - 'HOLDER=3669803 yes 13' + 'HOLDER=3669803 yes 13 11' ]) ) ).toBe('retained-live-work') @@ -76,7 +76,7 @@ describe('classifySupersededRelay', () => { it('nominates only a proven empty relay for reaping', () => { expect( classifySupersededRelay( - incumbent(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=80583 yes 0']) + incumbent(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=80583 yes 2 0']) ) ).toBe('reap-candidate') }) @@ -101,7 +101,7 @@ describe('sweepSupersededRelayEndpoints', () => { execCommand .mockResolvedValueOnce(`${OLD_SOCK}\n`) .mockResolvedValueOnce( - probe(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=3669803 yes 13']) + probe(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=3669803 yes 13 11']) ) const findings = await sweepSupersededRelayEndpoints(CONN, HOST, SWEEP) expect(findings).toHaveLength(1) @@ -114,7 +114,7 @@ describe('sweepSupersededRelayEndpoints', () => { execCommand .mockResolvedValueOnce(`${OLD_SOCK}\n`) .mockResolvedValueOnce( - probe(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=80583 yes 0']) + probe(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=80583 yes 2 0']) ) .mockResolvedValueOnce('GONE\n') const findings = await sweepSupersededRelayEndpoints(CONN, HOST, SWEEP) @@ -126,7 +126,7 @@ describe('sweepSupersededRelayEndpoints', () => { execCommand .mockResolvedValueOnce(`${OLD_SOCK}\n`) .mockResolvedValueOnce( - probe(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=80583 yes 0']) + probe(['PRESENT=yes', 'LISTEN=accepted', 'HOLDERS_SOURCE=lsof', 'HOLDER=80583 yes 2 0']) ) .mockResolvedValueOnce('LIVE\n') const findings = await sweepSupersededRelayEndpoints(CONN, HOST, SWEEP) diff --git a/src/shared/relay-artifacts.ts b/src/shared/relay-artifacts.ts index 273f6e059b8..2f9e563f839 100644 --- a/src/shared/relay-artifacts.ts +++ b/src/shared/relay-artifacts.ts @@ -37,6 +37,12 @@ export type RelayArtifact = { * optional one would loop forever redeploying a relay that is already correct. */ optional?: boolean + /** + * Forked by the relay daemon as a long-lived child of its own. These are relay + * infrastructure, never user work, and the reap gate subtracts them from a daemon's + * child census; see src/main/ssh/relay-daemon-service-children.ts. + */ + daemonServiceChild?: boolean } /** The bare Windows process-table addon; see docs/reference/windows-process-enumeration.md. */ @@ -44,8 +50,8 @@ export const RELAY_WINDOWS_PROCESS_TREE_FILENAME = 'windows-process-tree.node' export const RELAY_ARTIFACTS: readonly RelayArtifact[] = [ { filename: 'relay.js' }, - { filename: 'relay-watcher.js' }, - { filename: 'relay-ai-vault-service.js' }, + { filename: 'relay-watcher.js', daemonServiceChild: true }, + { filename: 'relay-ai-vault-service.js', daemonServiceChild: true }, { filename: 'managed-hook-runtime.js' }, // Forked by the AI Vault title reader; without it a relay answers every WSL // title request with no title and no error. @@ -62,6 +68,14 @@ export const RELAY_ARTIFACTS: readonly RelayArtifact[] = [ { filename: RELAY_WINDOWS_PROCESS_TREE_FILENAME, windowsOnly: true, optional: true } ] +/** + * The daemon's own service children, by entry filename. Anything else under a relay pid is + * either user work or unidentified, and both keep the relay unreapable. + */ +export const RELAY_DAEMON_SERVICE_ENTRY_FILENAMES: readonly string[] = RELAY_ARTIFACTS.filter( + (artifact) => artifact.daemonServiceChild +).map((artifact) => artifact.filename) + /** Written after the artifacts, so it is never an input to its own hash. */ export const RELAY_VERSION_FILENAME = '.version'