fix(bg): revalidate process identity before signals (#1937)

* fix(bg): revalidate process identity before signals

* fix(bg): sanitize process identity probe failures

* fix(bg): skip signals for terminal sessions
This commit is contained in:
Bogdan
2026-07-14 14:40:32 +08:00
committed by GitHub
parent 6b2a1d8aef
commit af0885d8ec
4 changed files with 673 additions and 33 deletions
+375
View File
@@ -7,12 +7,18 @@ import {
buildBackgroundSessionLaunch,
buildBackgroundChildProcessConfig,
followLogFile,
killBackgroundSession,
printExistingLog,
terminateBackgroundSessionProcessTree,
terminateBackgroundProcessTree,
LOG_STREAM_CHUNK_SIZE,
parseBackgroundInvocation,
parseLogsInvocation,
} from './bg.js'
import type {
BackgroundSession,
BackgroundSessionProcessIdentity,
} from './bgRegistry.js'
class TestOutput extends EventEmitter {
chunks: Buffer[] = []
@@ -498,6 +504,375 @@ describe('background session CLI parsing', () => {
})
})
describe('background session process termination safety', () => {
const session: BackgroundSession = {
id: 'bg-safety',
name: 'safety',
pid: 4242,
cwd: '/repo',
status: 'running',
startedAt: '2026-07-10T08:00:00.000Z',
updatedAt: '2026-07-10T08:00:00.000Z',
sessionId: 'conversation-safety',
command: ['node', 'openclaude', '--session-id', 'conversation-safety'],
stdoutLogPath: '/tmp/stdout.log',
stderrLogPath: '/tmp/stderr.log',
}
function identity(
state: BackgroundSessionProcessIdentity['state'],
overrides: Partial<BackgroundSessionProcessIdentity> = {},
): BackgroundSessionProcessIdentity {
return {
state,
backgroundSessionId: session.id,
pid: session.pid,
...overrides,
}
}
it('verifies the selected session immediately before SIGTERM', async () => {
const calls: string[] = []
let aliveChecks = 0
await terminateBackgroundSessionProcessTree(session, {
isProcessAlive: () => ++aliveChecks <= 2,
getProcessCommand: pid => {
calls.push(`verify:${pid}`)
return 'node openclaude --session-id conversation-safety'
},
killTree: async (pid, signal) => {
calls.push(`signal:${pid}:${signal}`)
},
sleep: async () => {},
termGraceMs: 1,
pollIntervalMs: 1,
})
expect(calls).toEqual([
'verify:4242',
'signal:4242:SIGTERM',
])
})
it('refuses a mismatched identity before SIGTERM and does not mark killed', async () => {
const calls: string[] = []
let refusal: unknown
try {
await killBackgroundSession(
{
...session,
status: 'running',
command: [...session.command, 'private prompt value'],
},
{
isProcessAlive: () => true,
verifySessionIdentity: () => identity('mismatch'),
killTree: async (_pid, signal) => {
calls.push(`signal:${signal}`)
},
markKilled: async selected => {
calls.push(`mark:${selected.id}`)
return { ...selected, status: 'killed' }
},
},
)
} catch (error) {
refusal = error
}
expect(String(refusal)).toContain('refused to signal an unverified process')
expect(String(refusal)).not.toContain('private prompt value')
expect(calls).toEqual([])
})
it('does not verify or signal terminal records when a reused PID would match', async () => {
for (const status of ['stale', 'killed', 'exited', 'failed'] as const) {
const calls: string[] = []
const killed = await killBackgroundSession(
{ ...session, status },
{
verifySessionIdentity: () => {
calls.push('verify')
return identity('matches')
},
killTree: async (_pid, signal) => {
calls.push(`signal:${signal}`)
},
markKilled: async selected => {
calls.push(`mark:${selected.id}`)
return { ...selected, status: 'killed' }
},
},
)
expect(killed.status).toBe('killed')
expect(calls).toEqual(['mark:bg-safety'])
}
})
it('freshly verifies an unknown session that becomes readable before killing', async () => {
const calls: string[] = []
const states: BackgroundSessionProcessIdentity['state'][] = [
'matches',
'matches',
]
const killed = await killBackgroundSession(
{ ...session, status: 'unknown' },
{
isProcessAlive: () => false,
verifySessionIdentity: () => {
const state = states.shift()!
calls.push(`verify:${state}`)
return identity(state)
},
killTree: async (_pid, signal) => {
calls.push(`signal:${signal}`)
},
markKilled: async selected => {
calls.push(`mark:${selected.id}`)
return { ...selected, status: 'killed' }
},
},
)
expect(killed.status).toBe('killed')
expect(calls).toEqual([
'verify:matches',
'verify:matches',
'signal:SIGTERM',
'mark:bg-safety',
])
})
it('treats an unknown session that exits during fresh verification as terminated', async () => {
const calls: string[] = []
let aliveChecks = 0
const killed = await killBackgroundSession(
{ ...session, status: 'unknown' },
{
isProcessAlive: () => ++aliveChecks === 1,
getProcessCommand: () => null,
killTree: async (_pid, signal) => {
calls.push(`signal:${signal}`)
},
markKilled: async selected => {
calls.push(`mark:${selected.id}`)
return { ...selected, status: 'killed' }
},
},
)
expect(killed.status).toBe('killed')
expect(calls).toEqual(['mark:bg-safety'])
})
it('does not escalate when identity changes during the SIGTERM grace period', async () => {
const calls: string[] = []
const states: BackgroundSessionProcessIdentity['state'][] = [
'matches',
'mismatch',
]
await expect(
terminateBackgroundSessionProcessTree(session, {
isProcessAlive: () => true,
verifySessionIdentity: () => {
const state = states.shift()!
calls.push(`verify:${state}`)
return identity(state)
},
killTree: async (_pid, signal) => {
calls.push(`signal:${signal}`)
},
sleep: async () => {
calls.push('sleep')
},
termGraceMs: 1,
pollIntervalMs: 1,
}),
).rejects.toThrow('refused to signal an unverified process')
expect(calls).toEqual([
'verify:matches',
'signal:SIGTERM',
'sleep',
'verify:mismatch',
])
})
it('fails closed when the live process identity is unreadable', async () => {
const signals: Array<string | number> = []
await expect(
terminateBackgroundSessionProcessTree(session, {
isProcessAlive: () => true,
getProcessCommand: () => null,
killTree: async (_pid, signal) => {
signals.push(signal)
},
}),
).rejects.toThrow('refused to signal an unverified process')
expect(signals).toEqual([])
})
it('refuses signalling when liveness becomes unreadable during command lookup', async () => {
const calls: string[] = []
let probes = 0
await expect(
terminateBackgroundSessionProcessTree(session, {
signalProcess: () => {
probes++
calls.push(`probe:${probes}`)
if (probes > 1) {
throw Object.assign(new Error('access denied'), { code: 'EPERM' })
}
},
getProcessCommand: () => {
calls.push('command')
return 'node openclaude --session-id conversation-safety'
},
killTree: async (_pid, signal) => {
calls.push(`signal:${signal}`)
},
sleep: async () => {},
termGraceMs: 1,
pollIntervalMs: 1,
}),
).rejects.toThrow('refused to signal an unverified process')
expect(calls).toEqual(['probe:1', 'command', 'probe:2'])
})
it('succeeds without signalling when the process exits before verification', async () => {
const calls: string[] = []
const killed = await killBackgroundSession(session, {
isProcessAlive: () => false,
getProcessCommand: () => {
calls.push('command')
return null
},
killTree: async (_pid, signal) => {
calls.push(`signal:${signal}`)
},
markKilled: async selected => {
calls.push(`mark:${selected.id}`)
return { ...selected, status: 'killed' }
},
})
expect(killed.status).toBe('killed')
expect(calls).toEqual(['mark:bg-safety'])
})
it('accepts a natural exit after verification without a misleading error', async () => {
const calls: string[] = []
await terminateBackgroundSessionProcessTree(session, {
isProcessAlive: () => false,
verifySessionIdentity: () => {
calls.push('verify')
return identity('matches')
},
killTree: async (_pid, signal) => {
calls.push(`signal:${signal}`)
},
sleep: async () => {
calls.push('sleep')
},
termGraceMs: 1,
pollIntervalMs: 1,
})
expect(calls).toEqual(['verify', 'signal:SIGTERM'])
})
it('revalidates a stable identity immediately before SIGKILL', async () => {
const calls: string[] = []
let aliveChecks = 0
await terminateBackgroundSessionProcessTree(session, {
isProcessAlive: () => ++aliveChecks < 4,
verifySessionIdentity: () => {
calls.push('verify')
return identity('matches')
},
killTree: async (_pid, signal) => {
calls.push(`signal:${signal}`)
},
sleep: async () => {
calls.push('sleep')
},
termGraceMs: 1,
killGraceMs: 1,
pollIntervalMs: 1,
})
expect(calls).toEqual([
'verify',
'signal:SIGTERM',
'sleep',
'verify',
'signal:SIGKILL',
'sleep',
])
})
it('rejects stale verifier results for another session or PID', async () => {
for (const staleIdentity of [
identity('matches', { backgroundSessionId: 'bg-unrelated' }),
identity('matches', { pid: 9001 }),
]) {
const signals: Array<string | number> = []
await expect(
terminateBackgroundSessionProcessTree(session, {
isProcessAlive: () => true,
verifySessionIdentity: () => staleIdentity,
killTree: async (_pid, signal) => {
signals.push(signal)
},
}),
).rejects.toThrow('refused to signal an unverified process')
expect(signals).toEqual([])
}
})
it('sanitizes throwing injected identity verifiers without signalling or marking', async () => {
const calls: string[] = []
let refusal: unknown
try {
await killBackgroundSession(session, {
verifySessionIdentity: () => {
throw new Error('private verifier details')
},
killTree: async (_pid, signal) => {
calls.push(`signal:${signal}`)
},
markKilled: async selected => {
calls.push(`mark:${selected.id}`)
return { ...selected, status: 'killed' }
},
})
} catch (error) {
refusal = error
}
expect(String(refusal)).toContain('refused to signal an unverified process')
expect(String(refusal)).not.toContain('private verifier details')
expect(calls).toEqual([])
})
})
describe('background session log streaming', () => {
it('emits a multi-megabyte existing log exactly with bounded allocations', async () => {
await withTempFile('stdout.log', async path => {
+130 -13
View File
@@ -16,6 +16,12 @@ import {
markBackgroundSessionKilled,
refreshBackgroundSessionStatuses,
resolveBackgroundSession,
getBackgroundSessionProcessLiveness,
isTerminalBackgroundSession,
verifyBackgroundSessionProcessIdentity,
type BackgroundSession,
type BackgroundSessionProcessIdentity,
type BackgroundSessionProcessIdentityOptions,
} from './bgRegistry.js'
export type ParsedBackgroundInvocation = {
@@ -720,6 +726,10 @@ export async function terminateBackgroundProcessTree(
termGraceMs?: number
killGraceMs?: number
pollIntervalMs?: number
verifyBeforeSignal?: (
pid: number,
signal: string | number,
) => Promise<'matches' | 'not-running'>
},
): Promise<void> {
const isProcessAlive = options?.isProcessAlive ?? isProcessRunning
@@ -728,7 +738,13 @@ export async function terminateBackgroundProcessTree(
const pollIntervalMs =
options?.pollIntervalMs ?? DEFAULT_KILL_POLL_INTERVAL_MS
if (!isProcessAlive(pid)) return
if (options?.verifyBeforeSignal) {
if ((await options.verifyBeforeSignal(pid, 'SIGTERM')) === 'not-running') {
return
}
} else if (!isProcessAlive(pid)) {
return
}
await killTree(pid, 'SIGTERM')
if (
await waitForProcessExit(pid, {
@@ -741,6 +757,9 @@ export async function terminateBackgroundProcessTree(
return
}
if ((await options?.verifyBeforeSignal?.(pid, 'SIGKILL')) === 'not-running') {
return
}
await killTree(pid, 'SIGKILL')
if (
await waitForProcessExit(pid, {
@@ -756,6 +775,113 @@ export async function terminateBackgroundProcessTree(
throw new Error(`Process ${pid} did not exit after SIGKILL`)
}
type BackgroundSessionTerminationOptions = BackgroundSessionProcessIdentityOptions & {
killTree?: (pid: number, signal: string | number) => Promise<void>
sleep?: (ms: number) => Promise<void>
termGraceMs?: number
killGraceMs?: number
pollIntervalMs?: number
verifySessionIdentity?: (
session: BackgroundSession,
) => BackgroundSessionProcessIdentity
}
function unverifiedProcessError(
session: BackgroundSession,
reason: string,
): Error {
return new Error(
`OpenClaude refused to signal an unverified process for background session ${session.id} (PID ${session.pid}): ${reason}. Re-run \`openclaude ps\` and retry after confirming the session identity.`,
)
}
export async function terminateBackgroundSessionProcessTree(
session: BackgroundSession,
options: BackgroundSessionTerminationOptions = {},
): Promise<void> {
const getLiveness = (pid: number) =>
getBackgroundSessionProcessLiveness(pid, options)
await terminateBackgroundProcessTree(session.pid, {
...options,
isProcessAlive: pid => getLiveness(pid) !== 'not-running',
verifyBeforeSignal: async pid => {
const identity = verifySelectedBackgroundSessionIdentity(session, options)
if (pid !== session.pid) {
throw unverifiedProcessError(
session,
'the identity check did not correspond to the selected session and PID',
)
}
return authorizeBackgroundSessionSignal(session, identity)
},
})
}
function verifySelectedBackgroundSessionIdentity(
session: BackgroundSession,
options: BackgroundSessionTerminationOptions,
): BackgroundSessionProcessIdentity {
let identity: BackgroundSessionProcessIdentity
if (options.verifySessionIdentity) {
try {
identity = options.verifySessionIdentity(session)
} catch {
throw unverifiedProcessError(
session,
'the live process identity could not be read',
)
}
} else {
identity = verifyBackgroundSessionProcessIdentity(session, options)
}
if (
identity.backgroundSessionId !== session.id ||
identity.pid !== session.pid
) {
throw unverifiedProcessError(
session,
'the identity check did not correspond to the selected session and PID',
)
}
return identity
}
function authorizeBackgroundSessionSignal(
session: BackgroundSession,
identity: BackgroundSessionProcessIdentity,
): 'matches' | 'not-running' {
if (identity.state === 'not-running') return 'not-running'
if (identity.state === 'matches') return 'matches'
throw unverifiedProcessError(
session,
identity.state === 'mismatch'
? 'the PID now belongs to a different process'
: 'the live process identity could not be read',
)
}
export async function killBackgroundSession(
session: BackgroundSession,
options: BackgroundSessionTerminationOptions & {
markKilled?: (session: BackgroundSession) => Promise<BackgroundSession>
} = {},
): Promise<BackgroundSession> {
const markKilled =
options.markKilled ??
(async (selected: BackgroundSession) =>
await markBackgroundSessionKilled(selected.id))
if (isTerminalBackgroundSession(session)) return await markKilled(session)
const identity = verifySelectedBackgroundSessionIdentity(session, options)
if (authorizeBackgroundSessionSignal(session, identity) === 'matches') {
await terminateBackgroundSessionProcessTree(session, options)
}
return await markKilled(session)
}
export async function psHandler(_args: string[]): Promise<void> {
const sessions = await refreshBackgroundSessionStatuses()
printSessionTable(sessions)
@@ -811,20 +937,11 @@ export async function killHandler(
await refreshBackgroundSessionStatuses()
const session = await resolveSessionOrExit(target)
if (session.status === 'unknown' && isProcessRunning(session.pid)) {
const killed = await killBackgroundSession(session).catch(error => {
fail(
`Cannot safely kill background session ${session.id}: process identity could not be verified`,
`Failed to kill background session ${session.id}: ${errorMessage(error)}`,
)
}
if (session.status === 'running' && isProcessRunning(session.pid)) {
await terminateBackgroundProcessTree(session.pid).catch(error => {
fail(
`Failed to kill background session ${session.id}: ${errorMessage(error)}`,
)
})
}
const killed = await markBackgroundSessionKilled(session.id)
})
console.log(`Killed background session ${killed.id}.`)
}
+87
View File
@@ -12,6 +12,7 @@ import {
markBackgroundSessionKilled,
refreshBackgroundSessionStatuses,
resolveBackgroundSession,
verifyBackgroundSessionProcessIdentity,
type BackgroundSession,
} from './bgRegistry.js'
@@ -956,4 +957,90 @@ describe('isBackgroundSessionProcessAlive process identity', () => {
})
expect(alive).toBe(false)
})
it('distinguishes missing, matching, mismatched, and unreadable live processes', () => {
const states = [
verifyBackgroundSessionProcessIdentity(session, {
isProcessAlive: () => false,
getProcessCommand: () => 'not read',
}),
verifyBackgroundSessionProcessIdentity(session, {
isProcessAlive: () => true,
getProcessCommand: () => 'node openclaude 1642 --serve',
}),
verifyBackgroundSessionProcessIdentity(session, {
isProcessAlive: () => true,
getProcessCommand: () => 'node unrelated --serve',
}),
verifyBackgroundSessionProcessIdentity(session, {
isProcessAlive: () => true,
getProcessCommand: () => null,
}),
]
expect(states.map(result => result.state)).toEqual([
'not-running',
'matches',
'mismatch',
'unreadable',
])
expect(
states.every(result => result.backgroundSessionId === session.id),
).toBe(true)
expect(states.every(result => result.pid === session.pid)).toBe(true)
})
it('treats exit during command lookup as not running', () => {
let aliveChecks = 0
const result = verifyBackgroundSessionProcessIdentity(session, {
isProcessAlive: () => ++aliveChecks === 1,
getProcessCommand: () => null,
})
expect(result.state).toBe('not-running')
expect(aliveChecks).toBe(2)
})
it('treats an access-denied liveness probe as unreadable', () => {
const result = verifyBackgroundSessionProcessIdentity(session, {
signalProcess: () => {
throw Object.assign(new Error('access denied'), { code: 'EPERM' })
},
getProcessCommand: () => {
throw new Error('command lookup must not run without confirmed liveness')
},
})
expect(result.state).toBe('unreadable')
})
it('maps throwing injected liveness and command probes to unreadable', () => {
const livenessError = verifyBackgroundSessionProcessIdentity(session, {
isProcessAlive: () => {
throw new Error('private liveness details')
},
getProcessCommand: () => 'not read',
})
const commandError = verifyBackgroundSessionProcessIdentity(session, {
isProcessAlive: () => true,
getProcessCommand: () => {
throw new Error('private command details')
},
})
expect(livenessError.state).toBe('unreadable')
expect(commandError.state).toBe('unreadable')
})
it('treats empty command output as unreadable', () => {
for (const command of ['', ' ']) {
const result = verifyBackgroundSessionProcessIdentity(session, {
isProcessAlive: () => true,
getProcessCommand: () => command,
})
expect(result.state).toBe('unreadable')
}
})
})
+81 -20
View File
@@ -478,11 +478,14 @@ export async function refreshBackgroundSessionStatuses(options?: {
continue
}
const processState = getBackgroundSessionProcessState(session, options)
const processState = verifyBackgroundSessionProcessIdentity(
session,
options,
).state
const nextStatus: BackgroundSessionStatus =
processState === 'alive'
processState === 'matches'
? 'running'
: processState === 'unknown'
: processState === 'unreadable'
? 'unknown'
: 'stale'
@@ -503,7 +506,22 @@ export async function refreshBackgroundSessionStatuses(options?: {
return refreshed
}
type BackgroundSessionProcessState = 'alive' | 'dead' | 'unknown'
export type BackgroundSessionProcessIdentity = {
state: 'not-running' | 'matches' | 'mismatch' | 'unreadable'
backgroundSessionId: string
pid: number
}
export type BackgroundSessionProcessLiveness =
| 'alive'
| 'not-running'
| 'unreadable'
export type BackgroundSessionProcessIdentityOptions = {
isProcessAlive?: (pid: number) => boolean
signalProcess?: (pid: number, signal: 0) => unknown
getProcessCommand?: (pid: number) => string | null
}
// A spaced path or prompt is a single argv entry, but the raw command line
// quotes it, so a whitespace split fuses a quote onto the edge tokens. Windows
@@ -570,30 +588,73 @@ function commandLineMatchesBackgroundSession(
return commandLineContainsArgs(commandLine, session.command)
}
function getBackgroundSessionProcessState(
export function verifyBackgroundSessionProcessIdentity(
session: BackgroundSession,
options?: {
isProcessAlive?: (pid: number) => boolean
getProcessCommand?: (pid: number) => string | null
},
): BackgroundSessionProcessState {
const isAlive = options?.isProcessAlive ?? isProcessRunning
if (!isAlive(session.pid)) return 'dead'
options?: BackgroundSessionProcessIdentityOptions,
): BackgroundSessionProcessIdentity {
const result = (
state: BackgroundSessionProcessIdentity['state'],
): BackgroundSessionProcessIdentity => ({
state,
backgroundSessionId: session.id,
pid: session.pid,
})
const getLiveness = () =>
getBackgroundSessionProcessLiveness(session.pid, options)
const liveness = getLiveness()
if (liveness !== 'alive') return result(liveness)
const readCommand = options?.getProcessCommand ?? getProcessCommand
const command = readCommand(session.pid)
if (command == null) return 'unknown'
return commandLineMatchesBackgroundSession(command, session) ? 'alive' : 'dead'
let command: string | null
try {
command = readCommand(session.pid)
} catch {
const latestLiveness = getLiveness()
return result(
latestLiveness === 'alive' ? 'unreadable' : latestLiveness,
)
}
const latestLiveness = getLiveness()
if (latestLiveness !== 'alive') return result(latestLiveness)
if (command == null || command.trim() === '') {
return result('unreadable')
}
return result(
commandLineMatchesBackgroundSession(command, session)
? 'matches'
: 'mismatch',
)
}
export function getBackgroundSessionProcessLiveness(
pid: number,
options?: BackgroundSessionProcessIdentityOptions,
): BackgroundSessionProcessLiveness {
if (options?.isProcessAlive) {
try {
return options.isProcessAlive(pid) ? 'alive' : 'not-running'
} catch {
return 'unreadable'
}
}
if (pid <= 1) return 'not-running'
const signalProcess = options?.signalProcess ?? process.kill
try {
signalProcess(pid, 0)
return 'alive'
} catch (error) {
return isErrno(error, 'ESRCH') ? 'not-running' : 'unreadable'
}
}
export function isBackgroundSessionProcessAlive(
session: BackgroundSession,
options?: {
isProcessAlive?: (pid: number) => boolean
getProcessCommand?: (pid: number) => string | null
},
options?: BackgroundSessionProcessIdentityOptions,
): boolean {
return getBackgroundSessionProcessState(session, options) === 'alive'
return (
verifyBackgroundSessionProcessIdentity(session, options).state === 'matches'
)
}
export async function markBackgroundSessionKilled(