fix(ssh): corroborate an empty lsof answer before calling an endpoint free

#18304 decided enumerability after `lsof` runs, keying on stderr, non-numeric
output, and an abnormal exit. One failure carries none of those signals: probing
as a uid that does not own the socket's holder, `lsof -t -a -U <path>` exits 1
with no stdout and no stderr. Measured on Debian 12 against #18304's own probe, a
live relay owned by root probed as `nobody`:

  probe    uid      path         marker        pids
  merged   nobody   held.sock    lsof          []      <- live relay holds it
  merged   nobody   stale.sock   lsof          []      <- genuinely nobody
  merged   root     held.sock    lsof          [10]
  merged   root     stale.sock   lsof          []

The first two rows are byte-identical, so nothing about lsof's answer can
separate them. The first reaches `verdict: exited / evidence: no-holder`, which
`classifySupersededRelay` maps to `stale-endpoint-removed` and `rm -f` on an
inode a live relay is still holding. `hidepid=2` produces the same shape.

A positive control does not solve this. Controlling on something the probe itself
holds passes precisely when we are blind: as `nobody`, `lsof -t -p $$` returns a
pid while the socket query returns nothing. Blindness is to *other* uids, and
another uid's process is not ours to manufacture.

/proc/net/unix is. It is world-readable and lists every bound unix socket
regardless of owner, so an entry for the path alongside no reported pid proves
lsof was blind rather than that the path is free. Only an otherwise-clean empty
answer is corroborated; a reported pid still stands on its own, and the check is
skipped when the answer was already unavailable. Same run, with this change:

  fixed    nobody   held.sock    unavailable   []      <- no longer reapable
  fixed    nobody   stale.sock   lsof          []      <- still reapable
  fixed    root     held.sock    lsof          [10]    <- unchanged
  fixed    root     stale.sock   lsof          []      <- unchanged

The marker can only ever move from `lsof` toward `unavailable`, so this never
authorises an unlink that #18304 refuses.

Off Linux there is no /proc/net/unix, the check returns false, and behaviour is
exactly as before -- deliberately, because defaulting to `unavailable` there
would stop every macOS host from reaping a stale endpoint and trade a rare
destructive bug for a universal accumulation one. The tests are Linux-gated for
the same reason, with an assertion that the evidence they depend on is actually
present so the block cannot pass vacuously.
This commit is contained in:
Neil
2026-09-14 00:17:08 -07:00
parent 0c812843ef
commit 10bbcc2cd4
2 changed files with 116 additions and 4 deletions
@@ -1,4 +1,5 @@
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
import { createServer } from 'node:net'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { describe, expect, it } from 'vitest'
@@ -44,7 +45,14 @@ process.kill = function(pid, signal) {
};
`
async function runProbe(options: { signal?: string; census?: string; censusKillDenied?: boolean }) {
async function runProbe(options: {
signal?: string
census?: string
censusKillDenied?: boolean
/** Replaces the whole stub `lsof` body, for cases about what lsof answered. */
lsof?: string
sockPath?: string
}) {
const dir = mkdtempSync(join(tmpdir(), 'orca-lsof-lifecycle-'))
const pidFile = join(dir, 'lsof.pid')
const psPidFile = join(dir, 'ps.pid')
@@ -52,7 +60,7 @@ async function runProbe(options: { signal?: string; census?: string; censusKillD
writeFileSync(join(dir, 'preload.cjs'), PRELOAD)
writeFileSync(
join(dir, 'lsof'),
`#!/bin/sh\necho 123\n${options.signal ? 'exec sleep 60\n' : 'exit 2\n'}`,
options.lsof ?? `#!/bin/sh\necho 123\n${options.signal ? 'exec sleep 60\n' : 'exit 2\n'}`,
{ mode: 0o755 }
)
if (options.census !== undefined) {
@@ -61,7 +69,13 @@ async function runProbe(options: { signal?: string; census?: string; censusKillD
const started = performance.now()
const result = await runProcess({
program: process.execPath,
args: ['--require', join(dir, 'preload.cjs'), '-e', RELAY_LSOF_PROBE_JS, '/unused.sock'],
args: [
'--require',
join(dir, 'preload.cjs'),
'-e',
RELAY_LSOF_PROBE_JS,
options.sockPath ?? '/unused.sock'
],
env: {
...process.env,
ORCA_BACKGROUND_LAUNCH: '1',
@@ -151,3 +165,79 @@ describe.skipIf(process.platform === 'win32')('lsof supervisor lifecycle', () =>
expect(result.elapsedMs).toBeLessThan(4000)
})
})
/**
* When the probe runs as a uid that does not own the socket's holder, `lsof -t -a -U <path>`
* exits 1 with no stdout and no stderr. Measured on Debian 12: a live relay owned by root,
* probed as `nobody`, produced `exit=1 stdout=[] stderr=[]` — byte-identical to a genuinely
* stale socket, so no amount of inspecting lsof's answer can separate the two. That answer
* reaches `verdict: exited / evidence: no-holder`, which `classifySupersededRelay` turns into
* `rm -f` on an inode a live relay still holds.
*
* The stub reproduces that exact shape rather than switching uid, which needs root and two
* accounts. What separates the cases is /proc/net/unix, which is world-readable.
*/
const linuxOnly = describe.skipIf(process.platform !== 'linux')
linuxOnly('lsof blindness to another uid', () => {
const BLIND_LSOF = '#!/bin/sh\nexit 1\n'
it('has the evidence these assertions depend on', () => {
// Why asserted rather than assumed: without /proc/net/unix every case below passes
// vacuously, and a silently degraded suite would stop covering the only thing that
// separates "lsof could not see" from "nothing holds it".
expect(existsSync('/proc/net/unix')).toBe(true)
})
async function withBoundSocket<T>(run: (sockPath: string) => Promise<T>): Promise<T> {
const dir = mkdtempSync(join(tmpdir(), 'orca-lsof-bound-'))
const sockPath = join(dir, 'held.sock')
const server = createServer(() => {})
await new Promise<void>((resolve) => {
server.listen(sockPath, () => resolve())
})
try {
return await run(sockPath)
} finally {
server.close()
rmSync(dir, { recursive: true, force: true })
}
}
it('reports unavailable when nothing was reported for a socket that is still bound', async () => {
const result = await withBoundSocket((sockPath) => runProbe({ lsof: BLIND_LSOF, sockPath }))
expect(result.stdout).toBe('unavailable\n\n')
})
it('still reports lsof for an empty answer about a path nothing has bound', async () => {
// The control that keeps this from becoming a universal accumulation bug: on a healthy
// host a genuinely stale socket has no /proc/net/unix entry and must stay reapable.
const dir = mkdtempSync(join(tmpdir(), 'orca-lsof-stale-'))
try {
const result = await runProbe({ lsof: BLIND_LSOF, sockPath: join(dir, 'never-bound.sock') })
expect(result.stdout).toBe('lsof\n\n')
} finally {
rmSync(dir, { recursive: true, force: true })
}
})
it('keeps a reported pid even while the path is still bound', async () => {
const result = await withBoundSocket((sockPath) =>
runProbe({ lsof: '#!/bin/sh\necho 123\nexit 0\n', sockPath })
)
expect(result.stdout).toBe('lsof\n123\n')
})
// Both neighbours of the bound path, because a substring test would pass one and fail the
// other: `<sock>` is a prefix of the bound path, and the bound path is a prefix of
// `<sock>.other`. Only exact equality answers correctly for both.
it.each([
['a prefix of the bound path', (sockPath: string) => sockPath.slice(0, -1)],
['a path the bound one prefixes', (sockPath: string) => `${sockPath}.other`]
])('does not treat %s as bound', async (_name, derive) => {
const result = await withBoundSocket((sockPath) =>
runProbe({ lsof: BLIND_LSOF, sockPath: derive(sockPath) })
)
expect(result.stdout).toBe('lsof\n\n')
})
})
@@ -20,6 +20,24 @@ function groupExists(pid) {
try { process.kill(-pid, 0); return true; }
catch (error) { return error.code !== 'ESRCH'; }
}
// An empty lsof answer means "nobody holds it" only if lsof could see. Running as a uid that
// does not own the holder, lsof exits 1 with no stdout and no stderr -- byte-identical to a
// genuinely stale socket. /proc/net/unix is world-readable and lists every bound unix socket
// regardless of owner, so an entry for this path alongside no pid proves lsof was blind.
// Absent off Linux, where this returns false and the marker stays whatever it already was.
function pathStillBound(target) {
var text;
try { text = require('fs').readFileSync('/proc/net/unix', 'utf8'); }
catch (error) { return false; }
var lines = text.split('\n');
for (var i = 0; i < lines.length; i++) {
// Num: RefCount Protocol Flags Type St Inode Path -- the path is the line remainder, so
// one containing spaces survives intact.
var match = lines[i].match(/^\S+:(?:\s+\S+){5}\s+\d+ (.*)$/);
if (match && match[1] === target) return true;
}
return false;
}
function finish(unconfirmed) {
if (finished) return;
finished = true;
@@ -36,6 +54,10 @@ function finish(unconfirmed) {
unavailable = true;
return false;
});
// Only an otherwise-clean empty answer needs corroborating; a reported pid stands on its own.
if (!unconfirmed && !unavailable && !stderrSeen && !pids.length && pathStillBound(process.argv[1])) {
unavailable = true;
}
var marker = unconfirmed ? 'cleanup-unconfirmed' : (unavailable || stderrSeen ? 'unavailable' : 'lsof');
process.stdout.write(marker + '\n' + pids.join('\n') + '\n', function() { process.exit(0); });
}