mirror of
https://github.com/stablyai/orca.git
synced 2026-10-22 16:03:47 +00:00
fix(ssh): ask who owns the socket, and let the daemon reclaim it
Two reviewers landed on the same remaining hole from opposite directions: the identity check and the unlink were still two operations, so a relay that bound the path in between was the one displaced. Adding a host-side lock would have to be shared with relay startup to mean anything. The unlink was never ours to do. A fresh daemon already reclaims a stale path itself, checking the socket's identity and binding in one process — and if it loses that race it fails to start rather than stranding anyone. So deploy now only asks who owns the path, and launches when the answer is that nothing does. The whole check-to-unlink window goes away, along with the identity bookkeeping that existed to narrow it. Proven by the journey where a killed relay's leftover socket is taken over by the production launch path. Reset drops the same distinction it could not support: an attempted signal is not evidence a process took it, so with no inventory to ask afterwards the stop is reported unproven whether or not something was signalled. The reset sandbox's lsof stub now answers the tool-works probe independently of the socket query — tying them together made a working lsof look broken the moment the relay stopped, which is not how a host behaves.
This commit is contained in:
@@ -77,7 +77,7 @@ import {
|
||||
import { detectRemoteHostPlatform } from './ssh-remote-platform-detection'
|
||||
import { powerShellCommand, powerShellLiteral, powerShellNativeArg } from './ssh-remote-powershell'
|
||||
import { relaySocketNameForInstanceId } from './ssh-relay-instance-id'
|
||||
import { isRelaySocketOwnerLiveError, releaseUnownedRelaySocket } from './ssh-relay-socket-owner'
|
||||
import { isRelaySocketOwnerLiveError, requireUnownedRelaySocket } from './ssh-relay-socket-owner'
|
||||
import { isSshSessionLimitError } from './ssh-session-limit-error'
|
||||
import {
|
||||
isWindowsRelayPipePath,
|
||||
@@ -1446,11 +1446,12 @@ export async function launchRelay(
|
||||
'[ssh-relay] Socket reconnect failed:',
|
||||
err instanceof Error ? err.message : String(err)
|
||||
)
|
||||
// Why: a refused --connect is loss of contact, never proof the relay died, so the socket
|
||||
// is released only once the host proves nothing owns it. Unlinking a live owner's path
|
||||
// cannot stop it — it just hides it from the fresh launch's EADDRINUSE check and orphans
|
||||
// it and its PTYs forever (STA-1756).
|
||||
await releaseUnownedRelaySocket(conn, nodePath, sockFile, { signal, cause: err })
|
||||
// Why: a refused --connect is loss of contact, never proof the relay died, so the path
|
||||
// is only taken once the host proves nothing owns it. Launching over a live owner
|
||||
// cannot stop it — it just hides it from the fresh daemon's EADDRINUSE check and
|
||||
// orphans it and its PTYs forever (STA-1756). The daemon reclaims a proven-stale
|
||||
// socket itself, under its own identity check.
|
||||
await requireUnownedRelaySocket(conn, nodePath, sockFile, { signal, cause: err })
|
||||
}
|
||||
}
|
||||
} catch (err) {
|
||||
|
||||
@@ -116,7 +116,10 @@ async function runReset(mode: HostMode): Promise<ResetRun> {
|
||||
if (mode !== 'no-process-tools') {
|
||||
// Why the -a split: without it lsof would also return an unrelated Unix-socket holder,
|
||||
// which reset must never pass to kill (#8762).
|
||||
const lsofBody =
|
||||
// Why the -p arm answers unconditionally: the script asks lsof about its own process
|
||||
// first to prove the tool works at all, and a stub that tied that to the socket's owner
|
||||
// would make a working lsof look broken the moment the relay stopped.
|
||||
const lsofSocketArm =
|
||||
mode === 'only-pgrep-finds-owner'
|
||||
? 'exit 1'
|
||||
: `${OWNER_ALIVE_GUARD}
|
||||
@@ -124,6 +127,10 @@ case " $* " in
|
||||
*" -a "*) printf '%s\\n' "$OWNER_PID" ;;
|
||||
*) printf '%s\\n' "$OWNER_PID" "$UNRELATED_PID" ;;
|
||||
esac`
|
||||
const lsofBody = `case " $* " in
|
||||
*" -p "*) printf '%s\\n' "$$" ; exit 0 ;;
|
||||
esac
|
||||
${lsofSocketArm}`
|
||||
writeExecutable(join(binDir, 'lsof'), lsofBody)
|
||||
writeExecutable(
|
||||
join(binDir, 'pgrep'),
|
||||
|
||||
@@ -64,10 +64,8 @@ export async function forceStopRelayForTarget(
|
||||
'if [ -d "$base" ]; then',
|
||||
' for sock in "$base"/relay-*/"$sock_name" "$base"/"$sock_name"; do',
|
||||
' [ -S "$sock" ] || continue',
|
||||
' signalled=no',
|
||||
' find_holder "$sock"',
|
||||
' if [ -n "$holder" ]; then',
|
||||
' signalled=yes',
|
||||
' kill -TERM $holder 2>/dev/null || true',
|
||||
' sleep 0.2',
|
||||
' kill -KILL $holder 2>/dev/null || true',
|
||||
@@ -80,7 +78,9 @@ export async function forceStopRelayForTarget(
|
||||
' find_holder "$sock"',
|
||||
' if [ "$listed" = 0 ] || [ -n "$holder" ]; then',
|
||||
` echo ${RESET_OWNER_SURVIVED_MARKER}`,
|
||||
' elif [ "$listed" = 2 ] && [ "$signalled" = no ]; then',
|
||||
// Why regardless of whether we signalled: sending a signal is not evidence the process
|
||||
// took it, so with no inventory to ask afterwards the stop stays unproven either way.
|
||||
' elif [ "$listed" = 2 ]; then',
|
||||
` echo ${RESET_NO_OWNER_PROOF_MARKER}`,
|
||||
' fi',
|
||||
' done',
|
||||
|
||||
@@ -117,7 +117,7 @@ describe('deployAndLaunchRelay socket displacement', () => {
|
||||
const conn = makeMockConnection()
|
||||
vi.mocked(waitForSentinel).mockRejectedValueOnce(new Error('relay reconnect refused'))
|
||||
queueDeployUpToOccupiedSocket()
|
||||
vi.mocked(execCommand).mockResolvedValueOnce('LIVE') // socket release: a live owner
|
||||
vi.mocked(execCommand).mockResolvedValueOnce('LIVE') // owner probe: a live owner
|
||||
|
||||
await expect(deployAndLaunchRelay(conn)).rejects.toSatisfy(isRelaySocketOwnerLiveError)
|
||||
|
||||
@@ -131,7 +131,7 @@ describe('deployAndLaunchRelay socket displacement', () => {
|
||||
const conn = makeMockConnection()
|
||||
vi.mocked(waitForSentinel).mockRejectedValueOnce(new Error('relay reconnect refused'))
|
||||
queueDeployUpToOccupiedSocket()
|
||||
vi.mocked(execCommand).mockResolvedValueOnce('') // release produced no verdict
|
||||
vi.mocked(execCommand).mockResolvedValueOnce('') // probe produced no verdict
|
||||
|
||||
await expect(deployAndLaunchRelay(conn)).rejects.toSatisfy(isRelaySocketOwnerLiveError)
|
||||
|
||||
@@ -139,14 +139,14 @@ describe('deployAndLaunchRelay socket displacement', () => {
|
||||
expect(writeRelayEndpointCredential).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('still launches fresh once the host reports the stale socket released', async () => {
|
||||
it('still launches fresh once the host proves the stale socket unowned', async () => {
|
||||
const conn = makeMockConnection()
|
||||
vi.mocked(waitForSentinel)
|
||||
.mockRejectedValueOnce(new Error('relay reconnect refused'))
|
||||
.mockResolvedValue({ write: vi.fn(), onData: vi.fn(), onClose: vi.fn() } as never)
|
||||
queueDeployUpToOccupiedSocket()
|
||||
vi.mocked(execCommand)
|
||||
.mockResolvedValueOnce('RELEASED') // socket release: nothing owned the path
|
||||
.mockResolvedValueOnce('EXITED') // owner probe: nothing owns the path
|
||||
.mockResolvedValueOnce('READY') // socket readiness poll
|
||||
|
||||
await expect(deployAndLaunchRelay(conn)).resolves.toMatchObject({ nodePath: '/usr/bin/node' })
|
||||
@@ -154,24 +154,21 @@ describe('deployAndLaunchRelay socket displacement', () => {
|
||||
expect(execChannelCommands(conn).some((command) => command.includes('--detached'))).toBe(true)
|
||||
})
|
||||
|
||||
// Moved here from ssh-relay-deploy.test.ts: the removal it guards now lives inside the
|
||||
// release command, so an unconfirmed close of that command must still stop the launch.
|
||||
it('does not launch fresh after an unconfirmed socket release', async () => {
|
||||
// Replaces the unconfirmed-cleanup case from ssh-relay-deploy.test.ts: deploy no longer
|
||||
// removes anything, so a probe it could not run must stop the launch on its own.
|
||||
it('does not launch fresh after a probe it could not run', async () => {
|
||||
const conn = makeMockConnection()
|
||||
const unconfirmedRelease = Object.assign(new Error('socket release still running'), {
|
||||
sshChannelCloseConfirmed: false
|
||||
})
|
||||
vi.mocked(waitForSentinel).mockRejectedValueOnce(new Error('relay reconnect refused'))
|
||||
queueDeployUpToOccupiedSocket()
|
||||
vi.mocked(execCommand).mockRejectedValueOnce(unconfirmedRelease)
|
||||
vi.mocked(execCommand).mockRejectedValueOnce(new Error('channel open failure'))
|
||||
|
||||
await expect(deployAndLaunchRelay(conn)).rejects.toBe(unconfirmedRelease)
|
||||
await expect(deployAndLaunchRelay(conn)).rejects.toSatisfy(isRelaySocketOwnerLiveError)
|
||||
|
||||
expect(execChannelCommands(conn)).toHaveLength(1)
|
||||
expect(execChannelCommands(conn).some((command) => command.includes('--detached'))).toBe(false)
|
||||
})
|
||||
|
||||
it('decides with one host command that probes and unlinks together', async () => {
|
||||
it('decides by asking the host who owns the socket, and removes nothing', async () => {
|
||||
const conn = makeMockConnection()
|
||||
vi.mocked(waitForSentinel).mockRejectedValueOnce(new Error('relay reconnect refused'))
|
||||
queueDeployUpToOccupiedSocket()
|
||||
@@ -179,13 +176,13 @@ describe('deployAndLaunchRelay socket displacement', () => {
|
||||
|
||||
await expect(deployAndLaunchRelay(conn)).rejects.toSatisfy(isRelaySocketOwnerLiveError)
|
||||
|
||||
// Why one command: probing here and unlinking in a second round-trip leaves a window
|
||||
// in which another client can bind the path, and the removal would then strand it.
|
||||
const release = execCommands().at(-1) ?? ''
|
||||
expect(release).toContain('/usr/bin/node')
|
||||
expect(release).toContain('connect')
|
||||
expect(release).toContain('unlinkSync')
|
||||
expect(release).not.toContain('test -S')
|
||||
expect(execCommands().some((command) => command.startsWith('rm -f'))).toBe(false)
|
||||
// Why nothing removed: the daemon reclaims a stale path under its own identity check,
|
||||
// and an unlink here would put an SSH round-trip between that check and the launch.
|
||||
const probe = execCommands().at(-1) ?? ''
|
||||
expect(probe).toContain('/usr/bin/node')
|
||||
expect(probe).toContain('connect')
|
||||
expect(probe).not.toContain('test -S')
|
||||
expect(execCommands().some((command) => command.includes('rm -f'))).toBe(false)
|
||||
expect(execCommands().some((command) => command.includes('unlinkSync'))).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
// while a PTY is held).
|
||||
//
|
||||
// The control case below is latest-main's behaviour: it performs the raw unlink and
|
||||
// asserts the orphan appears. The guarded case asserts releaseUnownedRelaySocket refuses
|
||||
// asserts the orphan appears. The guarded case asserts requireUnownedRelaySocket refuses
|
||||
// the same displacement, which restores the relay's own EADDRINUSE interlock. Which
|
||||
// production call site issues that release is pinned separately, by the mocked deploy
|
||||
// wiring in ssh-relay-socket-displacement.test.ts.
|
||||
@@ -27,7 +27,7 @@ import {
|
||||
relayBundleDirForHost,
|
||||
waitForExit
|
||||
} from './ssh-relay-live-daemon-harness'
|
||||
import { isRelaySocketOwnerLiveError, releaseUnownedRelaySocket } from './ssh-relay-socket-owner'
|
||||
import { isRelaySocketOwnerLiveError, requireUnownedRelaySocket } from './ssh-relay-socket-owner'
|
||||
|
||||
const REPO_ROOT = process.cwd()
|
||||
const BUNDLE_DIR = relayBundleDirForHost(REPO_ROOT)
|
||||
@@ -89,7 +89,7 @@ describe.skipIf(!BUNDLE_DIR || process.platform === 'win32')('relay socket owner
|
||||
expect(isProcessAlive(pid)).toBe(false)
|
||||
}
|
||||
|
||||
it('releases the socket of a relay that is really gone', async () => {
|
||||
it('clears a relay that is really gone, without touching its socket', async () => {
|
||||
const { fixture: live, pid } = await startRelayHoldingAPty()
|
||||
const conn = createLocalShellConnection()
|
||||
|
||||
@@ -99,18 +99,20 @@ describe.skipIf(!BUNDLE_DIR || process.platform === 'win32')('relay socket owner
|
||||
expect(existsSync(live.sockPath)).toBe(true)
|
||||
|
||||
await expect(
|
||||
releaseUnownedRelaySocket(conn, process.execPath, live.sockPath)
|
||||
requireUnownedRelaySocket(conn, process.execPath, live.sockPath)
|
||||
).resolves.toBeUndefined()
|
||||
|
||||
expect(existsSync(live.sockPath)).toBe(false)
|
||||
// Why still there: reclaiming the path belongs to the daemon that binds it, which does
|
||||
// the identity check and the bind in one process.
|
||||
expect(existsSync(live.sockPath)).toBe(true)
|
||||
})
|
||||
|
||||
it('refuses to release the socket of a relay that still holds a PTY', async () => {
|
||||
it('refuses to hand the socket of a relay that still holds a PTY to a replacement', async () => {
|
||||
const { fixture: live, pid } = await startRelayHoldingAPty()
|
||||
const conn = createLocalShellConnection()
|
||||
|
||||
await expect(
|
||||
releaseUnownedRelaySocket(conn, process.execPath, live.sockPath)
|
||||
requireUnownedRelaySocket(conn, process.execPath, live.sockPath)
|
||||
).rejects.toSatisfy(isRelaySocketOwnerLiveError)
|
||||
|
||||
expect(existsSync(live.sockPath)).toBe(true)
|
||||
@@ -136,7 +138,7 @@ describe.skipIf(!BUNDLE_DIR || process.platform === 'win32')('relay socket owner
|
||||
const conn = createLocalShellConnection({ path: '' })
|
||||
|
||||
await expect(
|
||||
releaseUnownedRelaySocket(conn, process.execPath, live.sockPath)
|
||||
requireUnownedRelaySocket(conn, process.execPath, live.sockPath)
|
||||
).rejects.toSatisfy(isRelaySocketOwnerLiveError)
|
||||
|
||||
expect(existsSync(live.sockPath)).toBe(true)
|
||||
|
||||
@@ -11,9 +11,9 @@ import type { SshConnection } from './ssh-connection'
|
||||
import { execCommand } from './ssh-relay-deploy-helpers'
|
||||
import {
|
||||
isRelaySocketOwnerLiveError,
|
||||
parseRelaySocketReleaseOutcome,
|
||||
relaySocketReleaseCommand,
|
||||
releaseUnownedRelaySocket
|
||||
parseRelaySocketOwnerVerdict,
|
||||
relaySocketOwnerProbeCommand,
|
||||
requireUnownedRelaySocket
|
||||
} from './ssh-relay-socket-owner'
|
||||
import {
|
||||
isRelayVersionMismatchError,
|
||||
@@ -24,8 +24,8 @@ const SOCK = '/home/u/.orca-remote/relay-0.1.0+abc/relay-deadbeef.sock'
|
||||
const NODE = '/usr/bin/node'
|
||||
const conn = {} as SshConnection
|
||||
|
||||
describe('relay socket release command', () => {
|
||||
const command = relaySocketReleaseCommand(NODE, SOCK)
|
||||
describe('relay socket owner probe command', () => {
|
||||
const command = relaySocketOwnerProbeCommand(NODE, SOCK)
|
||||
|
||||
it('proves an owner is present with lsof before falling back to a connect probe', () => {
|
||||
expect(command).toContain('lsof -t -a -U "$sock"')
|
||||
@@ -37,13 +37,12 @@ describe('relay socket release command', () => {
|
||||
expect(command).toContain(`'${SOCK}'`)
|
||||
})
|
||||
|
||||
it('unlinks inside the same host command, guarded by the inode it probed', () => {
|
||||
// Why: probing here and removing in a second round-trip leaves a window in which
|
||||
// another client can bind the path, and the removal would then strand it.
|
||||
expect(command).toContain('unlinkSync')
|
||||
expect(command).toContain('statSync')
|
||||
// Why ctime: inode numbers are recycled, so dev+ino alone can match a recreated socket.
|
||||
expect(command).toContain('st.dev+":"+st.ino+":"+st.ctimeNs')
|
||||
it('only looks, and never removes anything itself', () => {
|
||||
// Why: reclaiming the path is the fresh daemon's job, which checks the socket's identity
|
||||
// and binds in one process. Unlinking here would put an SSH round-trip between that check
|
||||
// and the launch, and a relay that bound the path in between would be the one displaced.
|
||||
expect(command).not.toContain('unlinkSync')
|
||||
expect(command).not.toContain('rm -f')
|
||||
})
|
||||
|
||||
it('retries a refused connect before concluding the path is unowned', () => {
|
||||
@@ -67,35 +66,35 @@ describe('relay socket release command', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('relay socket release outcome', () => {
|
||||
describe('relay socket owner verdict', () => {
|
||||
it('reads the host markers', () => {
|
||||
expect(parseRelaySocketReleaseOutcome('LIVE')).toBe('live')
|
||||
expect(parseRelaySocketReleaseOutcome('RELEASED\n')).toBe('released')
|
||||
expect(parseRelaySocketReleaseOutcome('UNVERIFIABLE')).toBe('unverifiable')
|
||||
expect(parseRelaySocketOwnerVerdict('LIVE')).toBe('live')
|
||||
expect(parseRelaySocketOwnerVerdict('EXITED\n')).toBe('exited')
|
||||
expect(parseRelaySocketOwnerVerdict('UNVERIFIABLE')).toBe('unverifiable')
|
||||
})
|
||||
|
||||
it('refuses to guess from ambiguous or empty output', () => {
|
||||
expect(parseRelaySocketReleaseOutcome('')).toBe('unverifiable')
|
||||
expect(parseRelaySocketReleaseOutcome('LIVE\nRELEASED')).toBe('unverifiable')
|
||||
expect(parseRelaySocketReleaseOutcome('command not found: node')).toBe('unverifiable')
|
||||
expect(parseRelaySocketOwnerVerdict('')).toBe('unverifiable')
|
||||
expect(parseRelaySocketOwnerVerdict('LIVE\nEXITED')).toBe('unverifiable')
|
||||
expect(parseRelaySocketOwnerVerdict('command not found: node')).toBe('unverifiable')
|
||||
})
|
||||
})
|
||||
|
||||
describe('releaseUnownedRelaySocket', () => {
|
||||
describe('requireUnownedRelaySocket', () => {
|
||||
beforeEach(() => {
|
||||
vi.mocked(execCommand).mockReset()
|
||||
})
|
||||
|
||||
it('succeeds once the host reports the path released', async () => {
|
||||
vi.mocked(execCommand).mockResolvedValueOnce('RELEASED')
|
||||
it('succeeds once the host proves nothing owns the path', async () => {
|
||||
vi.mocked(execCommand).mockResolvedValueOnce('EXITED')
|
||||
|
||||
await expect(releaseUnownedRelaySocket(conn, NODE, SOCK)).resolves.toBeUndefined()
|
||||
await expect(requireUnownedRelaySocket(conn, NODE, SOCK)).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
it('refuses when a live owner still holds the socket', async () => {
|
||||
vi.mocked(execCommand).mockResolvedValueOnce('LIVE')
|
||||
|
||||
await expect(releaseUnownedRelaySocket(conn, NODE, SOCK)).rejects.toSatisfy(
|
||||
await expect(requireUnownedRelaySocket(conn, NODE, SOCK)).rejects.toSatisfy(
|
||||
isRelaySocketOwnerLiveError
|
||||
)
|
||||
})
|
||||
@@ -103,7 +102,7 @@ describe('releaseUnownedRelaySocket', () => {
|
||||
it('refuses when ownership is merely unverifiable', async () => {
|
||||
vi.mocked(execCommand).mockResolvedValueOnce('UNVERIFIABLE')
|
||||
|
||||
await expect(releaseUnownedRelaySocket(conn, NODE, SOCK)).rejects.toSatisfy(
|
||||
await expect(requireUnownedRelaySocket(conn, NODE, SOCK)).rejects.toSatisfy(
|
||||
isRelaySocketOwnerLiveError
|
||||
)
|
||||
})
|
||||
@@ -111,20 +110,11 @@ describe('releaseUnownedRelaySocket', () => {
|
||||
it('refuses when the command could not run at all', async () => {
|
||||
vi.mocked(execCommand).mockRejectedValueOnce(new Error('channel open failure'))
|
||||
|
||||
await expect(releaseUnownedRelaySocket(conn, NODE, SOCK)).rejects.toSatisfy(
|
||||
await expect(requireUnownedRelaySocket(conn, NODE, SOCK)).rejects.toSatisfy(
|
||||
isRelaySocketOwnerLiveError
|
||||
)
|
||||
})
|
||||
|
||||
it('rethrows an unconfirmed termination so its in-flight state survives', async () => {
|
||||
const unconfirmed = Object.assign(new Error('socket cleanup still running'), {
|
||||
sshChannelCloseConfirmed: false
|
||||
})
|
||||
vi.mocked(execCommand).mockRejectedValueOnce(unconfirmed)
|
||||
|
||||
await expect(releaseUnownedRelaySocket(conn, NODE, SOCK)).rejects.toBe(unconfirmed)
|
||||
})
|
||||
|
||||
it('propagates an abort instead of reporting an outcome', async () => {
|
||||
const controller = new AbortController()
|
||||
vi.mocked(execCommand).mockImplementationOnce(() => {
|
||||
@@ -133,7 +123,7 @@ describe('releaseUnownedRelaySocket', () => {
|
||||
})
|
||||
|
||||
await expect(
|
||||
releaseUnownedRelaySocket(conn, NODE, SOCK, { signal: controller.signal })
|
||||
requireUnownedRelaySocket(conn, NODE, SOCK, { signal: controller.signal })
|
||||
).rejects.not.toSatisfy(isRelaySocketOwnerLiveError)
|
||||
})
|
||||
|
||||
@@ -141,10 +131,10 @@ describe('releaseUnownedRelaySocket', () => {
|
||||
vi.mocked(execCommand).mockResolvedValueOnce('LIVE')
|
||||
const cause = new Error('handshake refused')
|
||||
|
||||
const error = await releaseUnownedRelaySocket(conn, NODE, SOCK, { cause }).catch((err) => err)
|
||||
const error = await requireUnownedRelaySocket(conn, NODE, SOCK, { cause }).catch((err) => err)
|
||||
|
||||
expect(error.sockPath).toBe(SOCK)
|
||||
expect(error.outcome).toBe('live')
|
||||
expect(error.verdict).toBe('live')
|
||||
expect(error.cause).toBe(cause)
|
||||
expect(error.message).toContain('handshake refused')
|
||||
expect(error.message).toContain('Reset remote relay')
|
||||
@@ -156,7 +146,7 @@ describe('terminal causes carried by the owner error', () => {
|
||||
vi.mocked(execCommand).mockReset().mockResolvedValueOnce('LIVE')
|
||||
const mismatch = new RelayVersionMismatchError('0.1.0+new', '0.1.0+old')
|
||||
|
||||
const error = await releaseUnownedRelaySocket(conn, NODE, SOCK, { cause: mismatch }).catch(
|
||||
const error = await requireUnownedRelaySocket(conn, NODE, SOCK, { cause: mismatch }).catch(
|
||||
(err) => err
|
||||
)
|
||||
|
||||
@@ -168,7 +158,7 @@ describe('terminal causes carried by the owner error', () => {
|
||||
it('does not classify an ordinary reconnect failure as terminal', async () => {
|
||||
vi.mocked(execCommand).mockReset().mockResolvedValueOnce('LIVE')
|
||||
|
||||
const error = await releaseUnownedRelaySocket(conn, NODE, SOCK, {
|
||||
const error = await requireUnownedRelaySocket(conn, NODE, SOCK, {
|
||||
cause: new Error('relay sentinel timeout')
|
||||
}).catch((err) => err)
|
||||
|
||||
|
||||
@@ -1,90 +1,75 @@
|
||||
import type { SshConnection } from './ssh-connection'
|
||||
import { shellEscape } from './ssh-connection-utils'
|
||||
import { execCommand, isUnconfirmedSshCommandTermination } from './ssh-relay-deploy-helpers'
|
||||
import { execCommand } from './ssh-relay-deploy-helpers'
|
||||
|
||||
/**
|
||||
* What happened when we asked the host to release a relay socket path.
|
||||
* Who owns a relay socket path, as observed from the client.
|
||||
*
|
||||
* The vocabulary follows `src/shared/pty-liveness-verdict.ts`: `released` needs
|
||||
* positive evidence that nothing owns the path, and anything we could not
|
||||
* observe stays `unverifiable` — never read as absence.
|
||||
* The vocabulary follows `src/shared/pty-liveness-verdict.ts`: `exited` needs positive
|
||||
* evidence that nothing owns the path, and anything we could not observe stays
|
||||
* `unverifiable` — never read as absence.
|
||||
*/
|
||||
export type RelaySocketReleaseOutcome = 'live' | 'released' | 'unverifiable'
|
||||
export type RelaySocketOwnerVerdict = 'live' | 'exited' | 'unverifiable'
|
||||
|
||||
const PROBE_TIMEOUT_MS = 3_000
|
||||
const PROBE_ATTEMPTS = 3
|
||||
const PROBE_RETRY_MS = 250
|
||||
|
||||
// Why one host-side script rather than probe-then-remove from here: an SSH round-trip
|
||||
// between the two is wide enough for another client to bind the path, and the removal
|
||||
// would then unlink a live owner's socket. The unlink is guarded by the same
|
||||
// dev+ino+ctime identity the relay uses for its own stale-socket cleanup (relay.ts
|
||||
// unlinkIfStillStale).
|
||||
// Why this only looks: the fresh daemon reclaims a stale path itself, checking the socket's
|
||||
// identity and binding in one process (relay.ts unlinkIfStillStale). Unlinking here would
|
||||
// put an SSH round-trip between that check and the launch, and a relay that bound the path
|
||||
// in between would be the one we displaced.
|
||||
//
|
||||
// Why refusals alone never authorize the unlink: a live Unix listener whose accept
|
||||
// backlog is full also answers ECONNREFUSED, so N refusals are an inference, not the
|
||||
// positive evidence of absence this whole change is about. The unlink additionally
|
||||
// requires an owner inventory that ran and did not list the path — /proc/net/unix where
|
||||
// it exists, otherwise an lsof that ran and matched nothing. With neither, the answer is
|
||||
// unverifiable and the socket stays.
|
||||
const RELAY_SOCKET_RELEASE_SCRIPT =
|
||||
// Why refusals alone are not absence: a live Unix listener whose accept backlog is full also
|
||||
// answers ECONNREFUSED, so N refusals are an inference. A verdict of `exited` additionally
|
||||
// needs an owner inventory that ran and did not list the path — /proc/net/unix where it
|
||||
// exists, otherwise an lsof proven able to inspect this user's own processes.
|
||||
const RELAY_SOCKET_OWNER_PROBE_SCRIPT =
|
||||
'var net=require("net"),fs=require("fs"),p=process.argv[1],lsofProof=process.argv[2]==="lsof",' +
|
||||
`left=${PROBE_ATTEMPTS},mark=null,done=false,sock=null,timer=null;` +
|
||||
// "live" if an inventory lists the path, "absent" if one ran without it, null if none ran.
|
||||
// Why the field strip rather than a suffix test: the pathname is everything after the
|
||||
`left=${PROBE_ATTEMPTS},done=false,sock=null,timer=null;` +
|
||||
// "live" if an inventory lists the path, "exited" if one ran without it, null if none ran.
|
||||
// Why the column strip rather than a suffix test: the pathname is everything after the
|
||||
// seventh column, so a socket whose own path ends with " " + ours would match a suffix.
|
||||
'function inventory(){var data;try{data=fs.readFileSync("/proc/net/unix","utf8")}' +
|
||||
'catch(e){return lsofProof?"absent":null}' +
|
||||
'catch(e){return lsofProof?"exited":null}' +
|
||||
'var lines=data.split(String.fromCharCode(10));' +
|
||||
'for(var i=1;i<lines.length;i++){' +
|
||||
'if(lines[i].replace(/^(?:[^ ]+ +){7}/,"")===p){return "live"}}' +
|
||||
'return "absent"}' +
|
||||
// Why ctime too: inode numbers are recycled, so dev+ino alone can match a socket that
|
||||
// was unlinked and recreated inside the probe window. Same identity as relay.ts.
|
||||
'function ident(){try{var st=fs.statSync(p,{bigint:true});' +
|
||||
'return st.dev+":"+st.ino+":"+st.ctimeNs}catch(e){return null}}' +
|
||||
'return "exited"}' +
|
||||
'function say(v){if(done)return;done=true;if(timer)clearTimeout(timer);' +
|
||||
'if(sock)sock.destroy();process.stdout.write(v)}' +
|
||||
'function release(){if(mark===null){say("RELEASED");return}' +
|
||||
'if(ident()!==mark){say("UNVERIFIABLE");return}' +
|
||||
'try{fs.unlinkSync(p)}catch(e){if(e.code!=="ENOENT"){say("UNVERIFIABLE");return}}say("RELEASED")}' +
|
||||
'function attempt(){mark=ident();sock=net.connect(p);' +
|
||||
'function attempt(){sock=net.connect(p);' +
|
||||
`timer=setTimeout(function(){say("UNVERIFIABLE")},${PROBE_TIMEOUT_MS});` +
|
||||
'sock.on("connect",function(){say("LIVE")});' +
|
||||
'sock.on("error",function(e){clearTimeout(timer);' +
|
||||
'if(e.code==="ENOENT"){say("RELEASED");return}' +
|
||||
'if(e.code==="ENOENT"){say("EXITED");return}' +
|
||||
'if(e.code!=="ECONNREFUSED"){say("UNVERIFIABLE");return}' +
|
||||
`if(--left>0){setTimeout(attempt,${PROBE_RETRY_MS});return}` +
|
||||
'var inv=inventory();' +
|
||||
'if(inv==="live"){say("LIVE");return}' +
|
||||
'if(inv===null){say("UNVERIFIABLE");return}' +
|
||||
'release()})}attempt()'
|
||||
'say(inv==="live"?"LIVE":inv==="exited"?"EXITED":"UNVERIFIABLE")})}attempt()'
|
||||
|
||||
export function relaySocketReleaseCommand(nodePath: string, sockPath: string): string {
|
||||
// Why lsof first: on hosts without /proc it is the only owner inventory available, and it
|
||||
// answers both ways — a match proves an owner is there, and a clean run that matches
|
||||
// nothing is the absence evidence the unlink needs. A missing lsof proves neither.
|
||||
export function relaySocketOwnerProbeCommand(nodePath: string, sockPath: string): string {
|
||||
// Why ask lsof about our own process first: an lsof that is present but cannot inspect
|
||||
// anything reports nothing, and reading that silence as "no owner" is the inference this
|
||||
// whole change removes. The relay runs as this same user, so an lsof that can see our
|
||||
// files can see its socket.
|
||||
// Why -a: lsof ORs selectors by default, which would match every Unix-socket holder (#8762).
|
||||
return (
|
||||
`sock=${shellEscape(sockPath)}; holders=; proof=none; ` +
|
||||
// Why probe lsof against our own process first: an lsof that is present but cannot
|
||||
// inspect anything reports nothing, and reading that silence as "no owner" is the
|
||||
// inference this whole change exists to remove. The relay runs as this same user, so
|
||||
// an lsof that can see our files can see its socket.
|
||||
'if command -v lsof >/dev/null 2>&1 && [ -n "$(lsof -t -p $$ 2>/dev/null)" ]; then ' +
|
||||
'holders=$(lsof -t -a -U "$sock" 2>/dev/null); [ -n "$holders" ] || proof=lsof; fi; ' +
|
||||
'if [ -n "$holders" ]; then echo LIVE; ' +
|
||||
// Why argv: passing the path and proof as arguments dodges quoting issues inside -e.
|
||||
`else ${shellEscape(nodePath)} -e ${shellEscape(RELAY_SOCKET_RELEASE_SCRIPT)} "$sock" "$proof"; fi`
|
||||
`else ${shellEscape(nodePath)} -e ${shellEscape(RELAY_SOCKET_OWNER_PROBE_SCRIPT)} "$sock" "$proof"; fi`
|
||||
)
|
||||
}
|
||||
|
||||
export function parseRelaySocketReleaseOutcome(output: string): RelaySocketReleaseOutcome {
|
||||
export function parseRelaySocketOwnerVerdict(output: string): RelaySocketOwnerVerdict {
|
||||
const markers = new Set(
|
||||
output
|
||||
.split(/\r?\n/)
|
||||
.map((line) => line.trim())
|
||||
.filter((line) => line === 'LIVE' || line === 'RELEASED' || line === 'UNVERIFIABLE')
|
||||
.filter((line) => line === 'LIVE' || line === 'EXITED' || line === 'UNVERIFIABLE')
|
||||
)
|
||||
if (markers.size !== 1) {
|
||||
return 'unverifiable'
|
||||
@@ -92,7 +77,7 @@ export function parseRelaySocketReleaseOutcome(output: string): RelaySocketRelea
|
||||
if (markers.has('LIVE')) {
|
||||
return 'live'
|
||||
}
|
||||
return markers.has('RELEASED') ? 'released' : 'unverifiable'
|
||||
return markers.has('EXITED') ? 'exited' : 'unverifiable'
|
||||
}
|
||||
|
||||
export class RelaySocketOwnerLiveError extends Error {
|
||||
@@ -100,12 +85,12 @@ export class RelaySocketOwnerLiveError extends Error {
|
||||
|
||||
constructor(
|
||||
readonly sockPath: string,
|
||||
readonly outcome: Exclude<RelaySocketReleaseOutcome, 'released'>,
|
||||
readonly verdict: Exclude<RelaySocketOwnerVerdict, 'exited'>,
|
||||
cause?: unknown
|
||||
) {
|
||||
super(
|
||||
`Could not reach the relay at ${sockPath}, and its socket is ${
|
||||
outcome === 'live' ? 'still owned by a running relay' : 'of unproven ownership'
|
||||
verdict === 'live' ? 'still owned by a running relay' : 'of unproven ownership'
|
||||
}. Leaving it in place — replacing it would strand that relay and every terminal it ` +
|
||||
`holds. Retry, or use "Reset remote relay" on the SSH target to stop it.${
|
||||
cause instanceof Error ? ` Reconnect failed with: ${cause.message}` : ''
|
||||
@@ -120,42 +105,36 @@ export function isRelaySocketOwnerLiveError(err: unknown): err is RelaySocketOwn
|
||||
}
|
||||
|
||||
/**
|
||||
* Release a relay socket path so a fresh daemon can bind it — and only when the host
|
||||
* proves nothing owns it.
|
||||
* Establish that nothing owns this relay socket, so a fresh daemon may take the path.
|
||||
*
|
||||
* Why the guard: unlinking cannot stop a relay that already bound the path. It only hides
|
||||
* the live owner from the fresh launch's EADDRINUSE check, which is the one interlock that
|
||||
* refuses to displace a running daemon. A failed `--connect` is loss of contact, never
|
||||
* proof of death (STA-1756).
|
||||
* Why this gate: a failed `--connect` is loss of contact, never proof of death. Launching
|
||||
* over a path whose owner is still listening cannot stop that owner — it only hides it from
|
||||
* the fresh daemon's EADDRINUSE check, and it keeps running with its PTYs forever
|
||||
* (STA-1756). Reclaiming the path itself is the daemon's job, which it does under its own
|
||||
* identity check.
|
||||
*
|
||||
* POSIX hosts only: Windows relays take the named-pipe path in launchWindowsRelay, where
|
||||
* there is no inode to unlink.
|
||||
* POSIX hosts only: Windows relays take the named-pipe path in launchWindowsRelay.
|
||||
*/
|
||||
export async function releaseUnownedRelaySocket(
|
||||
export async function requireUnownedRelaySocket(
|
||||
conn: SshConnection,
|
||||
nodePath: string,
|
||||
sockPath: string,
|
||||
options?: { signal?: AbortSignal; cause?: unknown }
|
||||
): Promise<void> {
|
||||
const signal = options?.signal
|
||||
let outcome: RelaySocketReleaseOutcome
|
||||
let verdict: RelaySocketOwnerVerdict
|
||||
try {
|
||||
outcome = parseRelaySocketReleaseOutcome(
|
||||
await execCommand(conn, relaySocketReleaseCommand(nodePath, sockPath), { signal })
|
||||
verdict = parseRelaySocketOwnerVerdict(
|
||||
await execCommand(conn, relaySocketOwnerProbeCommand(nodePath, sockPath), { signal })
|
||||
)
|
||||
} catch (err) {
|
||||
} catch {
|
||||
signal?.throwIfAborted()
|
||||
// Why rethrow: an unconfirmed close means the removal may still be in flight, and the
|
||||
// caller must not launch a relay over a path whose state it cannot describe.
|
||||
if (isUnconfirmedSshCommandTermination(err)) {
|
||||
throw err
|
||||
}
|
||||
// Why: a command we could not run says nothing about the owner, and the caller must not
|
||||
// Why: a probe we could not run says nothing about the owner, and the caller must not
|
||||
// read that silence as death.
|
||||
outcome = 'unverifiable'
|
||||
verdict = 'unverifiable'
|
||||
}
|
||||
signal?.throwIfAborted()
|
||||
if (outcome !== 'released') {
|
||||
throw new RelaySocketOwnerLiveError(sockPath, outcome, options?.cause)
|
||||
if (verdict !== 'exited') {
|
||||
throw new RelaySocketOwnerLiveError(sockPath, verdict, options?.cause)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user