From 9c4705bd76ec690f6ec1d90b34758a1867308bb7 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 20 Jul 2026 11:00:00 -0700 Subject: [PATCH] perf(ssh): install managed hooks inside remote relay Move managed agent-hook filesystem work behind one relay RPC so high-latency SSH connects pay one WAN round trip instead of hundreds. Keep installers serial, lock shared account config across relay processes, and fence cancelled connection generations from replacement state. Co-authored-by: nasagong Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- config/scripts/build-relay.mjs | 31 +- .../managed-hook-install-lock.test.ts | 397 ++++++++++++++++++ .../agent-hooks/managed-hook-install-lock.ts | 142 +++++++ .../managed-hook-local-filesystem.test.ts | 71 ++++ .../managed-hook-local-filesystem.ts | 61 +++ .../agent-hooks/managed-hook-lock-claims.ts | 216 ++++++++++ .../agent-hooks/managed-hook-lock-records.ts | 274 ++++++++++++ .../managed-hook-owner-identity.test.ts | 181 ++++++++ .../managed-hook-owner-identity.ts | 209 +++++++++ .../agent-hooks/managed-hook-runtime.test.ts | 22 + src/main/agent-hooks/managed-hook-runtime.ts | 106 +++++ .../managed-hook-stdin-lifecycle.test.ts | 26 +- .../agent-hooks/managed-hook-timeout.test.ts | 23 +- .../remote-hook-service-installers.test.ts | 29 ++ .../remote-managed-hook-installers.ts | 6 + src/main/ipc/ssh.test.ts | 43 ++ src/main/ipc/ssh.ts | 79 +++- src/main/ssh/ssh-connection-manager.test.ts | 74 ++++ src/main/ssh/ssh-connection-manager.ts | 25 +- src/main/ssh/ssh-connection.test.ts | 33 ++ src/main/ssh/ssh-connection.ts | 17 + src/main/ssh/ssh-relay-session.test.ts | 139 +++--- src/main/ssh/ssh-relay-session.ts | 109 +---- .../ssh/ssh-relay-versioned-install.test.ts | 3 +- src/main/ssh/ssh-relay-versioned-install.ts | 5 +- src/main/ssh/ssh-remote-commands.test.ts | 8 +- src/main/ssh/ssh-remote-commands.ts | 5 +- .../ssh-system-transport.integration.test.ts | 1 + src/relay/managed-hook-installer.test.ts | 63 +++ src/relay/managed-hook-installer.ts | 56 +++ src/relay/relay.ts | 4 + src/shared/agent-hook-relay.ts | 9 + 32 files changed, 2263 insertions(+), 204 deletions(-) create mode 100644 src/main/agent-hooks/managed-hook-install-lock.test.ts create mode 100644 src/main/agent-hooks/managed-hook-install-lock.ts create mode 100644 src/main/agent-hooks/managed-hook-local-filesystem.test.ts create mode 100644 src/main/agent-hooks/managed-hook-local-filesystem.ts create mode 100644 src/main/agent-hooks/managed-hook-lock-claims.ts create mode 100644 src/main/agent-hooks/managed-hook-lock-records.ts create mode 100644 src/main/agent-hooks/managed-hook-owner-identity.test.ts create mode 100644 src/main/agent-hooks/managed-hook-owner-identity.ts create mode 100644 src/main/agent-hooks/managed-hook-runtime.test.ts create mode 100644 src/main/agent-hooks/managed-hook-runtime.ts create mode 100644 src/main/ssh/ssh-connection-manager.test.ts create mode 100644 src/relay/managed-hook-installer.test.ts create mode 100644 src/relay/managed-hook-installer.ts diff --git a/config/scripts/build-relay.mjs b/config/scripts/build-relay.mjs index cb7e01c9700..6afd517af88 100644 --- a/config/scripts/build-relay.mjs +++ b/config/scripts/build-relay.mjs @@ -18,6 +18,14 @@ const __dirname = import.meta.dirname const ROOT = join(__dirname, '..', '..') const RELAY_ENTRY = join(ROOT, 'src', 'relay', 'relay.ts') const WATCHER_ENTRY = join(ROOT, 'src', 'main', 'ipc', 'parcel-watcher-process-entry.ts') +const MANAGED_HOOK_RUNTIME_ENTRY = join( + ROOT, + 'src', + 'main', + 'agent-hooks', + 'managed-hook-runtime.ts' +) +const JSONC_PARSER_ESM_ENTRY = join(ROOT, 'node_modules', 'jsonc-parser', 'lib', 'esm', 'main.js') const PLATFORMS = [ 'linux-x64', @@ -66,14 +74,33 @@ for (const platform of PLATFORMS) { } }) + await build({ + entryPoints: [MANAGED_HOOK_RUNTIME_ENTRY], + bundle: true, + platform: 'node', + target: 'node18', + format: 'cjs', + outfile: join(outDir, 'managed-hook-runtime.js'), + // Why: jsonc-parser's default UMD build keeps relative dynamic requires + // that break after bundling; its ESM entry is equivalent and self-contained. + alias: { 'jsonc-parser': JSONC_PARSER_ESM_ENTRY }, + sourcemap: false, + minify: true, + define: { + 'process.env.NODE_ENV': '"production"' + } + }) + // Why: include a content hash so the deploy check detects code changes - // even when RELAY_VERSION hasn't been bumped. Hash both process artifacts - // so a watcher-only change always deploys beside the matching relay host. + // even when RELAY_VERSION hasn't been bumped. Hash every executable module + // so a companion-only change always deploys beside the matching relay host. const relayContent = readFileSync(join(outDir, 'relay.js')) const watcherContent = readFileSync(join(outDir, 'relay-watcher.js')) + const managedHookRuntimeContent = readFileSync(join(outDir, 'managed-hook-runtime.js')) const hash = createHash('sha256') .update(relayContent) .update(watcherContent) + .update(managedHookRuntimeContent) .digest('hex') .slice(0, 12) writeFileSync(join(outDir, '.version'), `${RELAY_VERSION}+${hash}`) diff --git a/src/main/agent-hooks/managed-hook-install-lock.test.ts b/src/main/agent-hooks/managed-hook-install-lock.test.ts new file mode 100644 index 00000000000..d6563064f5e --- /dev/null +++ b/src/main/agent-hooks/managed-hook-install-lock.test.ts @@ -0,0 +1,397 @@ +import { + link, + lstat, + mkdir, + mkdtemp, + readFile, + readdir, + rename, + rm, + unlink, + utimes, + writeFile +} from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' +import * as ownerIdentity from './managed-hook-owner-identity' +import { withManagedHookInstallLock } from './managed-hook-install-lock' + +const tempHomes: string[] = [] +const STALE_TOKEN = '00000000-0000-4000-8000-000000000000' +const STALE_CLAIM_TOKEN = '11111111-1111-4111-8111-111111111111' +const fsFailure = vi.hoisted(() => ({ + canonicalUnlinkPath: null as string | null, + contendWithoutLock: null as (() => void) | null +})) + +vi.mock('node:fs/promises', async (importOriginal) => { + const original = await importOriginal<{ link: typeof link; unlink: typeof unlink }>() + return { + ...original, + link: async (...args: Parameters) => { + if (fsFailure.contendWithoutLock && String(args[1]).endsWith('managed-hook-install.lock')) { + fsFailure.contendWithoutLock() + throw Object.assign(new Error('injected publication contention'), { code: 'EEXIST' }) + } + await original.link(...args) + }, + unlink: async (path: Parameters[0]) => { + if (String(path) === fsFailure.canonicalUnlinkPath) { + fsFailure.canonicalUnlinkPath = null + throw Object.assign(new Error('injected unlink failure'), { code: 'EACCES' }) + } + await original.unlink(path) + } + } +}) + +async function createTempHome(): Promise { + const home = await mkdtemp(join(tmpdir(), 'orca-managed-hook-lock-')) + tempHomes.push(home) + return home +} + +async function requireHostIdentity(): Promise { + const hostIdentity = await ownerIdentity.readManagedHookHostIdentity() + if (!hostIdentity) { + throw new Error('test host identity is unavailable') + } + return hostIdentity +} + +async function createOwnedLock( + home: string, + processIdentity: string, + hostIdentity?: string +): Promise { + const lockHostIdentity = hostIdentity ?? (await requireHostIdentity()) + const lockParent = join(home, '.orca') + const ownerPath = join(lockParent, `managed-hook-install.owner-${STALE_TOKEN}.json`) + await mkdir(lockParent, { recursive: true }) + await writeFile( + ownerPath, + JSON.stringify({ + token: STALE_TOKEN, + pid: process.pid, + hostIdentity: lockHostIdentity, + processIdentity + }) + ) + await link(ownerPath, join(lockParent, 'managed-hook-install.lock')) +} + +afterEach(async () => { + fsFailure.canonicalUnlinkPath = null + fsFailure.contendWithoutLock = null + vi.restoreAllMocks() + vi.unstubAllEnvs() + await Promise.all(tempHomes.splice(0).map((home) => rm(home, { recursive: true, force: true }))) +}) + +describe.skipIf(process.platform === 'win32')('withManagedHookInstallLock', () => { + it('enforces the deadline when publication contention observes a missing lock', async () => { + const home = await createTempHome() + const controller = new AbortController() + const run = vi.fn() + let now = 0 + fsFailure.contendWithoutLock = () => controller.abort() + vi.spyOn(Date, 'now').mockImplementation(() => (now += 10_000)) + + await expect(withManagedHookInstallLock(home, controller.signal, run)).rejects.toThrow( + 'Timed out waiting for another managed-hook install to finish' + ) + expect(run).not.toHaveBeenCalled() + }) + + it('serializes installers across relay runtime instances', async () => { + const home = await createTempHome() + let releaseFirst!: () => void + let markFirstStarted!: () => void + const firstStarted = new Promise((resolve) => (markFirstStarted = resolve)) + const secondRun = vi.fn(async () => 'second') + const first = withManagedHookInstallLock(home, undefined, () => { + markFirstStarted() + return new Promise((resolve) => (releaseFirst = () => resolve('first'))) + }) + await firstStarted + const second = withManagedHookInstallLock(home, undefined, secondRun) + + await new Promise((resolve) => setTimeout(resolve, 40)) + expect(secondRun).not.toHaveBeenCalled() + releaseFirst() + + await expect(first).resolves.toBe('first') + await expect(second).resolves.toBe('second') + }) + + it('atomically publishes a complete owner record before entering the installer', async () => { + const home = await createTempHome() + const lockParent = join(home, '.orca') + const lockPath = join(lockParent, 'managed-hook-install.lock') + + await withManagedHookInstallLock(home, undefined, async () => { + const entries = await readdir(lockParent) + const ownerEntry = entries.find((entry) => entry.startsWith('managed-hook-install.owner-')) + expect(entries).toHaveLength(2) + expect(ownerEntry).toBeDefined() + if (!ownerEntry) { + throw new Error('owner entry was not published') + } + const ownerPath = join(lockParent, ownerEntry) + const owner = JSON.parse(await readFile(lockPath, 'utf8')) as { + token: string + pid: number + hostIdentity: string + processIdentity: string + } + expect(owner).toMatchObject({ pid: process.pid }) + expect(owner.token).toMatch(/^[\da-f-]{36}$/) + expect(owner.hostIdentity.length).toBeGreaterThan(0) + expect(owner.processIdentity.length).toBeGreaterThan(0) + const [lockStats, ownerStats] = await Promise.all([lstat(lockPath), lstat(ownerPath)]) + expect(lockStats.ino).toBe(ownerStats.ino) + expect(lockStats.nlink).toBe(2) + }) + }) + + it('fails fast instead of stealing an unverifiable legacy directory lock', async () => { + const home = await createTempHome() + await mkdir(join(home, '.orca', 'managed-hook-install.lock'), { recursive: true }) + const run = vi.fn() + + await expect(withManagedHookInstallLock(home, undefined, run)).rejects.toThrow( + 'unverifiable owner' + ) + expect(run).not.toHaveBeenCalled() + }) + + it('recovers after the same SSH host reboots even if its PID and start time are reused', async () => { + const home = await createTempHome() + await createOwnedLock(home, 'stale-process-incarnation') + + await expect( + withManagedHookInstallLock(home, undefined, async () => 'installed') + ).resolves.toBe('installed') + }) + + it('does not compare PID identities or steal a lock owned by another SSH host', async () => { + const home = await createTempHome() + const lockPath = join(home, '.orca', 'managed-hook-install.lock') + await createOwnedLock(home, 'stale-process-incarnation', 'another-host-boot') + const originalLock = await readFile(lockPath, 'utf8') + + await expect(withManagedHookInstallLock(home, undefined, vi.fn())).rejects.toThrow( + 'belongs to another host' + ) + expect(await readFile(lockPath, 'utf8')).toBe(originalLock) + }) + + it('does not steal a live lock when its owner process probe becomes unavailable', async () => { + const home = await createTempHome() + const lockPath = join(home, '.orca', 'managed-hook-install.lock') + await createOwnedLock(home, 'another-process-incarnation') + const originalLock = await readFile(lockPath, 'utf8') + const selfIdentity = await ownerIdentity.readManagedHookProcessIdentity(process.pid) + expect(selfIdentity).toBeTypeOf('string') + vi.spyOn(ownerIdentity, 'readManagedHookProcessIdentity') + .mockResolvedValueOnce(selfIdentity) + .mockResolvedValueOnce(undefined) + + await expect(withManagedHookInstallLock(home, undefined, vi.fn())).rejects.toThrow( + 'Could not verify the managed-hook lock owner process' + ) + expect(await readFile(lockPath, 'utf8')).toBe(originalLock) + }) + + it('serializes two contenders that concurrently recover the same dead owner', async () => { + const home = await createTempHome() + await createOwnedLock(home, 'stale-process-incarnation') + + let releaseFirst!: () => void + let activeRuns = 0 + let maxActiveRuns = 0 + const run = vi.fn(async () => { + activeRuns += 1 + maxActiveRuns = Math.max(maxActiveRuns, activeRuns) + if (run.mock.calls.length === 1) { + await new Promise((resolve) => (releaseFirst = resolve)) + } + activeRuns -= 1 + }) + const first = withManagedHookInstallLock(home, undefined, run) + const second = withManagedHookInstallLock(home, undefined, run) + + await vi.waitFor(() => expect(run).toHaveBeenCalledTimes(1)) + expect(maxActiveRuns).toBe(1) + releaseFirst() + await Promise.all([first, second]) + expect(run).toHaveBeenCalledTimes(2) + expect(maxActiveRuns).toBe(1) + }) + + it('recovers a canonical lock after its previous recovery claimant crashes', async () => { + const home = await createTempHome() + const lockParent = join(home, '.orca') + const ownerPath = join(lockParent, `managed-hook-install.owner-${STALE_TOKEN}.json`) + const claimedOwnerPath = join( + lockParent, + `managed-hook-install.claimed-${STALE_TOKEN}-${STALE_CLAIM_TOKEN}.json` + ) + const claimRecordPath = join( + lockParent, + `managed-hook-install.claim-${STALE_TOKEN}-${STALE_CLAIM_TOKEN}.json` + ) + const hostIdentity = await requireHostIdentity() + await createOwnedLock(home, 'stale-owner-incarnation', hostIdentity) + await writeFile( + claimRecordPath, + JSON.stringify({ + ownerToken: STALE_TOKEN, + claimToken: STALE_CLAIM_TOKEN, + pid: process.pid, + hostIdentity, + processIdentity: 'stale-claimant-incarnation' + }) + ) + await rename(ownerPath, claimedOwnerPath) + + await expect( + withManagedHookInstallLock(home, undefined, async () => 'installed') + ).resolves.toBe('installed') + expect(await readdir(lockParent)).toEqual([]) + }) + + it('fails fast when a recovery claimant disappears before removing the canonical lock', async () => { + const home = await createTempHome() + const lockParent = join(home, '.orca') + const lockPath = join(lockParent, 'managed-hook-install.lock') + const ownerPath = join(lockParent, `managed-hook-install.owner-${STALE_TOKEN}.json`) + await createOwnedLock(home, 'stale-process-incarnation') + await unlink(ownerPath) + + const startedAt = Date.now() + await expect(withManagedHookInstallLock(home, undefined, vi.fn())).rejects.toThrow( + 'unverifiable recovery claim' + ) + expect(Date.now() - startedAt).toBeLessThan(500) + expect(await readFile(lockPath, 'utf8')).toContain(STALE_TOKEN) + }) + + it('cleans an unlinked owner draft left by a crashed acquisition', async () => { + const home = await createTempHome() + const lockParent = join(home, '.orca') + const staleOwnerEntry = `managed-hook-install.owner-${STALE_TOKEN}.json` + const ownerPath = join(lockParent, staleOwnerEntry) + await mkdir(lockParent, { recursive: true }) + const hostIdentity = await requireHostIdentity() + await writeFile( + ownerPath, + JSON.stringify({ + token: STALE_TOKEN, + pid: process.pid, + hostIdentity, + processIdentity: 'stale-process-incarnation' + }) + ) + + await withManagedHookInstallLock(home, undefined, async () => { + expect(await readdir(lockParent)).not.toContain(staleOwnerEntry) + }) + expect(await readdir(lockParent)).toEqual([]) + }) + + it('does not delete a malformed final owner record that an older relay may still publish', async () => { + const home = await createTempHome() + const lockParent = join(home, '.orca') + const staleOwnerEntry = `managed-hook-install.owner-${STALE_TOKEN}.json` + const ownerPath = join(lockParent, staleOwnerEntry) + await mkdir(lockParent, { recursive: true }) + await writeFile(ownerPath, '{"token":') + const staleTime = new Date(Date.now() - 2_000) + await utimes(ownerPath, staleTime, staleTime) + + await withManagedHookInstallLock(home, undefined, async () => { + expect(await readdir(lockParent)).toContain(staleOwnerEntry) + }) + expect(await readdir(lockParent)).toEqual([staleOwnerEntry]) + }) + + it('does not let a late release delete a replacement owner lock', async () => { + const home = await createTempHome() + const lockPath = join(home, '.orca', 'managed-hook-install.lock') + let releaseFirst!: () => void + let markFirstStarted!: () => void + const firstStarted = new Promise((resolve) => (markFirstStarted = resolve)) + const first = withManagedHookInstallLock(home, undefined, () => { + markFirstStarted() + return new Promise((resolve) => (releaseFirst = resolve)) + }) + await firstStarted + await rename(lockPath, `${lockPath}.displaced`) + + let releaseSecond!: () => void + let markSecondStarted!: () => void + const secondStarted = new Promise((resolve) => (markSecondStarted = resolve)) + const second = withManagedHookInstallLock(home, undefined, () => { + markSecondStarted() + return new Promise((resolve) => (releaseSecond = resolve)) + }) + await secondStarted + + releaseFirst() + await first + const thirdRun = vi.fn() + const controller = new AbortController() + const third = withManagedHookInstallLock(home, controller.signal, thirdRun) + await new Promise((resolve) => setTimeout(resolve, 40)) + expect(thirdRun).not.toHaveBeenCalled() + controller.abort() + await expect(third).rejects.toMatchObject({ name: 'AbortError' }) + + releaseSecond() + await second + }) + + it('finishes an abandoned same-process release before the next install', async () => { + const home = await createTempHome() + const lockParent = join(home, '.orca') + const lockPath = join(lockParent, 'managed-hook-install.lock') + fsFailure.canonicalUnlinkPath = lockPath + + await expect(withManagedHookInstallLock(home, undefined, async () => 'first')).resolves.toBe( + 'first' + ) + expect(await readdir(lockParent)).toContain('managed-hook-install.lock') + + const startedAt = Date.now() + await expect(withManagedHookInstallLock(home, undefined, async () => 'second')).resolves.toBe( + 'second' + ) + expect(Date.now() - startedAt).toBeLessThan(500) + expect(await readdir(lockParent)).toEqual([]) + }) + + it('cancels a request waiting behind another relay runtime', async () => { + const home = await createTempHome() + let releaseFirst!: () => void + let markFirstStarted!: () => void + const firstStarted = new Promise((resolve) => (markFirstStarted = resolve)) + const first = withManagedHookInstallLock(home, undefined, () => { + markFirstStarted() + return new Promise((resolve) => (releaseFirst = resolve)) + }) + await firstStarted + const controller = new AbortController() + const secondRun = vi.fn() + const second = withManagedHookInstallLock(home, controller.signal, secondRun) + await new Promise((resolve) => setTimeout(resolve, 40)) + + controller.abort() + + await expect(second).rejects.toMatchObject({ name: 'AbortError' }) + expect(secondRun).not.toHaveBeenCalled() + releaseFirst() + await first + }) +}) diff --git a/src/main/agent-hooks/managed-hook-install-lock.ts b/src/main/agent-hooks/managed-hook-install-lock.ts new file mode 100644 index 00000000000..c283b99c41b --- /dev/null +++ b/src/main/agent-hooks/managed-hook-install-lock.ts @@ -0,0 +1,142 @@ +import { mkdir } from 'node:fs/promises' +import { join } from 'node:path' +import { setTimeout as delay } from 'node:timers/promises' +import { removeManagedHookLock } from './managed-hook-lock-claims' +import { + cleanupManagedHookLockFiles, + deactivateManagedHookLockOwner, + inspectManagedHookLock, + isManagedHookLockOwnerActive, + tryCreateManagedHookLock, + type ManagedHookLockOwner +} from './managed-hook-lock-records' +import { + readManagedHookHostIdentity, + readManagedHookProcessIdentity +} from './managed-hook-owner-identity' + +const LOCK_WAIT_TIMEOUT_MS = 10_000 +const LOCK_RETRY_MS = 20 + +async function releaseInstallLock( + lockPath: string, + lockParent: string, + owner: ManagedHookLockOwner, + hostIdentity: string, + processIdentity: string +): Promise { + try { + const removal = await removeManagedHookLock( + lockPath, + lockParent, + owner, + hostIdentity, + processIdentity + ) + if (removal !== 'removed') { + console.warn(`[agent-hooks] Failed to release managed-hook install lock: ${removal}`) + } + } catch (error) { + console.warn('[agent-hooks] Failed to release managed-hook install lock', error) + } finally { + deactivateManagedHookLockOwner(owner.token) + } +} + +async function acquireInstallLock( + home: string, + signal?: AbortSignal, + suppliedHostIdentity?: string +): Promise<() => Promise> { + const lockParent = join(home, '.orca') + const lockPath = join(lockParent, 'managed-hook-install.lock') + await mkdir(lockParent, { recursive: true }) + const hostIdentity = suppliedHostIdentity ?? (await readManagedHookHostIdentity()) + await cleanupManagedHookLockFiles(lockParent, hostIdentity) + const processIdentity = await readManagedHookProcessIdentity(process.pid) + if (typeof processIdentity !== 'string') { + throw new Error('Could not identify the managed-hook installer process') + } + const deadline = Date.now() + LOCK_WAIT_TIMEOUT_MS + + while (true) { + signal?.throwIfAborted() + const owner = await tryCreateManagedHookLock( + lockParent, + lockPath, + hostIdentity, + processIdentity + ) + if (owner) { + return async () => + await releaseInstallLock(lockPath, lockParent, owner, hostIdentity, processIdentity) + } + + const state = await inspectManagedHookLock(lockPath) + if (state.kind === 'unknown') { + // Why: unverifiable legacy locks cannot be stolen safely; hooks are best-effort, + // so fail fast instead of adding a recurring connection timeout. + throw new Error('Managed-hook install lock has an unverifiable owner') + } + if (state.kind === 'owned' && state.owner.hostIdentity !== hostIdentity) { + // Why: homes can be shared across SSH hosts, whose PID namespaces are unrelated. + throw new Error('Managed-hook install lock belongs to another host') + } + + if (state.kind === 'owned') { + const currentIdentity = await readManagedHookProcessIdentity(state.owner.pid) + if (currentIdentity === undefined) { + throw new Error('Could not verify the managed-hook lock owner process') + } + const abandonedOwnLock = + state.owner.pid === process.pid && + currentIdentity === processIdentity && + !isManagedHookLockOwnerActive(state.owner.token) + if ( + currentIdentity === null || + currentIdentity !== state.owner.processIdentity || + abandonedOwnLock + ) { + const removal = await removeManagedHookLock( + lockPath, + lockParent, + state.owner, + hostIdentity, + processIdentity + ) + if (removal === 'removed') { + if (Date.now() >= deadline) { + throw new Error('Timed out waiting for another managed-hook install to finish') + } + continue + } + if (removal === 'foreign') { + throw new Error('Managed-hook install lock recovery belongs to another host') + } + if (removal === 'unverifiable') { + throw new Error('Managed-hook install lock has an unverifiable recovery claim') + } + } + } + if (Date.now() >= deadline) { + throw new Error('Timed out waiting for another managed-hook install to finish') + } + await delay(LOCK_RETRY_MS, undefined, { signal }) + } +} + +/** Serialize config merges across relay daemons that target the same account. */ +export async function withManagedHookInstallLock( + home: string, + signal: AbortSignal | undefined, + run: () => Promise, + hostIdentity?: string +): Promise { + const release = await acquireInstallLock(home, signal, hostIdentity) + try { + signal?.throwIfAborted() + return await run() + } finally { + await release() + } +} diff --git a/src/main/agent-hooks/managed-hook-local-filesystem.test.ts b/src/main/agent-hooks/managed-hook-local-filesystem.test.ts new file mode 100644 index 00000000000..91a1ca62eb6 --- /dev/null +++ b/src/main/agent-hooks/managed-hook-local-filesystem.test.ts @@ -0,0 +1,71 @@ +import { mkdtemp, mkdir, readFile, readdir, rm, stat, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { installRemoteManagedAgentHooks } from './remote-managed-hook-installers' +import { createManagedHookLocalFilesystem } from './managed-hook-local-filesystem' + +const tempHomes: string[] = [] + +async function createTempHome(): Promise { + const home = await mkdtemp(join(tmpdir(), 'orca-managed-hooks-')) + tempHomes.push(home) + return home +} + +async function listFiles(root: string): Promise { + const entries = await readdir(root, { withFileTypes: true }) + const nested = await Promise.all( + entries.map(async (entry) => { + const path = join(root, entry.name) + return entry.isDirectory() ? await listFiles(path) : [path] + }) + ) + return nested.flat() +} + +afterEach(async () => { + await Promise.all(tempHomes.splice(0).map((home) => rm(home, { recursive: true, force: true }))) +}) + +describe('managed-hook local filesystem', () => { + it('supports cold and warm aggregate installs without SFTP or temp-file residue', async () => { + const home = await createTempHome() + const filesystem = createManagedHookLocalFilesystem() + const options = { grokHomeDir: join(home, '.grok') } + + const cold = await installRemoteManagedAgentHooks(filesystem, home, options) + const warm = await installRemoteManagedAgentHooks(filesystem, home, options) + + expect(cold).toHaveLength(14) + expect(cold.filter((result) => result.state === 'error')).toEqual([]) + expect(warm).toHaveLength(14) + expect(warm.filter((result) => result.state === 'error')).toEqual([]) + const files = await listFiles(home) + expect(files.filter((path) => path.endsWith('.tmp'))).toEqual([]) + const scripts = files.filter((path) => path.includes(join('.orca', 'agent-hooks'))) + expect(scripts.length).toBeGreaterThanOrEqual(10) + if (process.platform !== 'win32') { + for (const script of scripts) { + expect((await stat(script)).mode & 0o777).toBe(0o755) + } + } + }) + + it('isolates a malformed config while installing the remaining agents', async () => { + const home = await createTempHome() + const claudeConfig = join(home, '.claude', 'settings.json') + await mkdir(join(home, '.claude'), { recursive: true }) + await writeFile(claudeConfig, '{"hooks": }', 'utf8') + + const results = await installRemoteManagedAgentHooks(createManagedHookLocalFilesystem(), home, { + grokHomeDir: join(home, '.grok') + }) + + expect(results).toHaveLength(14) + expect(results.find((result) => result.agent === 'claude')?.state).toBe('error') + expect(results.find((result) => result.agent === 'openclaude')?.state).toBe('installed') + expect(results.find((result) => result.agent === 'kimi')?.state).toBe('installed') + expect(await readFile(claudeConfig, 'utf8')).toBe('{"hooks": }') + }) +}) diff --git a/src/main/agent-hooks/managed-hook-local-filesystem.ts b/src/main/agent-hooks/managed-hook-local-filesystem.ts new file mode 100644 index 00000000000..1a104a3a3b1 --- /dev/null +++ b/src/main/agent-hooks/managed-hook-local-filesystem.ts @@ -0,0 +1,61 @@ +import { chmod, mkdir, readFile, readdir, rename, stat, unlink, writeFile } from 'node:fs' +import type { SFTPWrapper } from 'ssh2' + +type Callback = (error: Error | null, value?: T) => void + +function asSftpError(error: NodeJS.ErrnoException): Error & { code: number } { + const translated = new Error(error.message, { cause: error }) as Error & { code: number } + translated.code = + error.code === 'ENOENT' ? 2 : error.code === 'ENOSYS' || error.code === 'ENOTSUP' ? 8 : 4 + return translated +} + +function finish(callback: Callback, error: NodeJS.ErrnoException | null, value?: T): void { + if (error) { + callback(asSftpError(error)) + return + } + callback(null, value) +} + +/** The managed installers only need this small callback-style SFTP surface. + * On the remote host it turns hundreds of WAN round trips into local fs calls. */ +export function createManagedHookLocalFilesystem(): SFTPWrapper { + const adapter = { + readFile(path: string, _encoding: string, callback: Callback): void { + readFile(path, 'utf8', (error, contents) => finish(callback, error, contents)) + }, + writeFile( + path: string, + contents: string, + options: { encoding?: BufferEncoding; mode?: number }, + callback: Callback + ): void { + writeFile(path, contents, options, (error) => finish(callback, error)) + }, + stat(path: string, callback: Callback<{ mode: number }>): void { + stat(path, (error, stats) => finish(callback, error, stats)) + }, + readdir(path: string, callback: Callback<[]>): void { + // Why: installers use readdir only as an existence check; names and + // attrs would allocate work that no caller consumes. + readdir(path, (error) => finish(callback, error, [])) + }, + mkdir(path: string, callback: Callback): void { + mkdir(path, (error) => finish(callback, error)) + }, + rename(source: string, destination: string, callback: Callback): void { + rename(source, destination, (error) => finish(callback, error)) + }, + ext_openssh_rename(source: string, destination: string, callback: Callback): void { + rename(source, destination, (error) => finish(callback, error)) + }, + unlink(path: string, callback: Callback): void { + unlink(path, (error) => finish(callback, error)) + }, + chmod(path: string, mode: number, callback: Callback): void { + chmod(path, mode, (error) => finish(callback, error)) + } + } + return adapter as unknown as SFTPWrapper +} diff --git a/src/main/agent-hooks/managed-hook-lock-claims.ts b/src/main/agent-hooks/managed-hook-lock-claims.ts new file mode 100644 index 00000000000..d3299c7bfd8 --- /dev/null +++ b/src/main/agent-hooks/managed-hook-lock-claims.ts @@ -0,0 +1,216 @@ +import { randomUUID } from 'node:crypto' +import { readdir, readFile, rename, unlink, writeFile } from 'node:fs/promises' +import { join } from 'node:path' +import { + CLAIMED_OWNER_PATTERN, + claimedOwnerFileName, + claimRecordFileName, + hasCode, + inspectManagedHookLock, + ownerFileName, + parseClaim, + parseOwner, + removeFileIfPresent, + type ManagedHookLockClaim, + type ManagedHookLockOwner +} from './managed-hook-lock-records' +import { readManagedHookProcessIdentity } from './managed-hook-owner-identity' + +export type ManagedHookLockRemoval = 'removed' | 'active' | 'foreign' | 'unverifiable' + +type AcquiredClaim = { + kind: 'claimed' + claim: ManagedHookLockClaim + claimRecordPath: string + claimedOwnerPath: string +} + +type ClaimResult = AcquiredClaim | { kind: 'active' | 'foreign' | 'unverifiable' | 'contended' } + +const activeClaimTokens = new Set() + +async function findClaimedOwner( + lockParent: string, + ownerToken: string +): Promise<{ claimToken: string; path: string } | null | undefined> { + const matches = (await readdir(lockParent)).flatMap((entry) => { + const match = CLAIMED_OWNER_PATTERN.exec(entry) + return match?.[1] === ownerToken && match[2] + ? [{ claimToken: match[2], path: join(lockParent, entry) }] + : [] + }) + return matches.length === 1 ? matches[0] : matches.length === 0 ? null : undefined +} + +async function resolveClaimSource( + lockParent: string, + owner: ManagedHookLockOwner, + hostIdentity: string, + processIdentity: string +): Promise< + | { kind: 'ready'; sourcePath: string; priorClaimRecordPath?: string } + | { kind: 'active' | 'foreign' | 'unverifiable' } +> { + const ownerPath = join(lockParent, ownerFileName(owner.token)) + try { + return parseOwner(await readFile(ownerPath, 'utf8'), owner.token) + ? { kind: 'ready', sourcePath: ownerPath } + : { kind: 'unverifiable' } + } catch (error) { + if (!hasCode(error, 'ENOENT')) { + throw error + } + } + + const claimedOwner = await findClaimedOwner(lockParent, owner.token) + if (claimedOwner === undefined || claimedOwner === null) { + return { kind: 'unverifiable' } + } + const priorClaimRecordPath = join( + lockParent, + claimRecordFileName(owner.token, claimedOwner.claimToken) + ) + let priorClaim: ManagedHookLockClaim | null + try { + priorClaim = parseClaim( + await readFile(priorClaimRecordPath, 'utf8'), + owner.token, + claimedOwner.claimToken + ) + } catch (error) { + if (hasCode(error, 'ENOENT')) { + return { kind: 'unverifiable' } + } + throw error + } + if (!priorClaim) { + return { kind: 'unverifiable' } + } + if (priorClaim.hostIdentity !== hostIdentity) { + return { kind: 'foreign' } + } + const currentIdentity = await readManagedHookProcessIdentity(priorClaim.pid) + const recoverOwnInactiveClaim = + priorClaim.pid === process.pid && + currentIdentity === processIdentity && + !activeClaimTokens.has(priorClaim.claimToken) + if (currentIdentity === undefined) { + return { kind: 'unverifiable' } + } + if (currentIdentity === priorClaim.processIdentity && !recoverOwnInactiveClaim) { + return { kind: 'active' } + } + return { kind: 'ready', sourcePath: claimedOwner.path, priorClaimRecordPath } +} + +async function claimManagedHookLock( + lockParent: string, + owner: ManagedHookLockOwner, + hostIdentity: string, + processIdentity: string +): Promise { + const source = await resolveClaimSource(lockParent, owner, hostIdentity, processIdentity) + if (source.kind !== 'ready') { + return source + } + const claimToken = randomUUID() + const claim = { + ownerToken: owner.token, + claimToken, + pid: process.pid, + hostIdentity, + processIdentity + } + const claimRecordPath = join(lockParent, claimRecordFileName(owner.token, claimToken)) + const claimedOwnerPath = join(lockParent, claimedOwnerFileName(owner.token, claimToken)) + await writeFile(claimRecordPath, JSON.stringify(claim), { + encoding: 'utf8', + flag: 'wx', + mode: 0o600 + }) + try { + try { + // Why: renaming the witness elects one claimant without ever dropping the hard link. + await rename(source.sourcePath, claimedOwnerPath) + } catch (error) { + if (hasCode(error, 'ENOENT')) { + return { kind: 'contended' } + } + throw error + } + activeClaimTokens.add(claimToken) + if (source.priorClaimRecordPath) { + try { + await removeFileIfPresent(source.priorClaimRecordPath) + } catch (error) { + console.warn('[agent-hooks] Failed to clean prior managed-hook claim record', error) + } + } + return { kind: 'claimed', claim, claimRecordPath, claimedOwnerPath } + } finally { + if (!activeClaimTokens.has(claimToken)) { + await removeFileIfPresent(claimRecordPath) + } + } +} + +async function cleanAcquiredClaim(claimed: AcquiredClaim): Promise { + await Promise.all([ + removeFileIfPresent(claimed.claimedOwnerPath), + removeFileIfPresent(claimed.claimRecordPath) + ]) +} + +export async function removeManagedHookLock( + lockPath: string, + lockParent: string, + owner: ManagedHookLockOwner, + hostIdentity: string, + processIdentity: string +): Promise { + const claimed = await claimManagedHookLock(lockParent, owner, hostIdentity, processIdentity) + if (claimed.kind === 'contended') { + return 'active' + } + if (claimed.kind === 'unverifiable') { + const state = await inspectManagedHookLock(lockPath) + if (state.kind === 'missing' || (state.kind === 'owned' && state.owner.token !== owner.token)) { + return 'removed' + } + } + if (claimed.kind !== 'claimed') { + return claimed.kind + } + + let removed = false + try { + const state = await inspectManagedHookLock(lockPath) + if (state.kind === 'unknown') { + return 'unverifiable' + } + if (state.kind === 'missing' || state.owner.token !== owner.token) { + removed = true + return 'removed' + } + try { + await unlink(lockPath) + } catch (error) { + if (!hasCode(error, 'ENOENT')) { + return 'unverifiable' + } + } + removed = true + return 'removed' + } finally { + activeClaimTokens.delete(claimed.claim.claimToken) + if (removed) { + try { + await cleanAcquiredClaim(claimed) + } catch (error) { + // Why: the canonical lock is already gone; residue cleanup must not + // turn a successful release into a failed installation. + console.warn('[agent-hooks] Failed to clean managed-hook recovery claim', error) + } + } + } +} diff --git a/src/main/agent-hooks/managed-hook-lock-records.ts b/src/main/agent-hooks/managed-hook-lock-records.ts new file mode 100644 index 00000000000..555c2a1a521 --- /dev/null +++ b/src/main/agent-hooks/managed-hook-lock-records.ts @@ -0,0 +1,274 @@ +import { randomUUID } from 'node:crypto' +import { link, lstat, readdir, readFile, rename, unlink, writeFile } from 'node:fs/promises' +import { join } from 'node:path' +import { readManagedHookProcessIdentity } from './managed-hook-owner-identity' + +const UUID_PATTERN = '[\\da-f]{8}-[\\da-f]{4}-[1-5][\\da-f]{3}-[89ab][\\da-f]{3}-[\\da-f]{12}' +const OWNER_FILE_PATTERN = new RegExp( + `^managed-hook-install\\.owner-(${UUID_PATTERN})\\.json$`, + 'i' +) +const OWNER_DRAFT_PATTERN = new RegExp( + `^managed-hook-install\\.owner-draft-(${UUID_PATTERN})\\.json$`, + 'i' +) +export const CLAIMED_OWNER_PATTERN = new RegExp( + `^managed-hook-install\\.claimed-(${UUID_PATTERN})-(${UUID_PATTERN})\\.json$`, + 'i' +) +const CLAIM_RECORD_PATTERN = new RegExp( + `^managed-hook-install\\.claim-(${UUID_PATTERN})-(${UUID_PATTERN})\\.json$`, + 'i' +) +const activeOwnerTokens = new Set() + +export type ManagedHookLockOwner = { + token: string + pid: number + hostIdentity: string + processIdentity: string +} + +export type ManagedHookLockClaim = { + ownerToken: string + claimToken: string + pid: number + hostIdentity: string + processIdentity: string +} + +export type ManagedHookLockState = + | { kind: 'missing' } + | { kind: 'owned'; owner: ManagedHookLockOwner } + | { kind: 'unknown' } + +export function hasCode(error: unknown, code: string): boolean { + return error instanceof Error && 'code' in error && error.code === code +} + +export function ownerFileName(token: string): string { + return `managed-hook-install.owner-${token}.json` +} + +function ownerDraftFileName(token: string): string { + return `managed-hook-install.owner-draft-${token}.json` +} + +export function claimedOwnerFileName(ownerToken: string, claimToken: string): string { + return `managed-hook-install.claimed-${ownerToken}-${claimToken}.json` +} + +export function claimRecordFileName(ownerToken: string, claimToken: string): string { + return `managed-hook-install.claim-${ownerToken}-${claimToken}.json` +} + +export function deactivateManagedHookLockOwner(ownerToken: string): void { + activeOwnerTokens.delete(ownerToken) +} + +export function isManagedHookLockOwnerActive(ownerToken: string): boolean { + return activeOwnerTokens.has(ownerToken) +} + +export function parseOwner(value: string, token: string): ManagedHookLockOwner | null { + try { + const owner = JSON.parse(value) as Partial + if ( + owner.token !== token || + !Number.isSafeInteger(owner.pid) || + (owner.pid ?? 0) <= 0 || + typeof owner.hostIdentity !== 'string' || + owner.hostIdentity.length === 0 || + typeof owner.processIdentity !== 'string' || + owner.processIdentity.length === 0 + ) { + return null + } + return owner as ManagedHookLockOwner + } catch { + return null + } +} + +export function parseClaim( + value: string, + ownerToken: string, + claimToken: string +): ManagedHookLockClaim | null { + try { + const claim = JSON.parse(value) as Partial + if ( + claim.ownerToken !== ownerToken || + claim.claimToken !== claimToken || + !Number.isSafeInteger(claim.pid) || + (claim.pid ?? 0) <= 0 || + typeof claim.hostIdentity !== 'string' || + claim.hostIdentity.length === 0 || + typeof claim.processIdentity !== 'string' || + claim.processIdentity.length === 0 + ) { + return null + } + return claim as ManagedHookLockClaim + } catch { + return null + } +} + +export async function removeFileIfPresent(path: string): Promise { + try { + await unlink(path) + return true + } catch (error) { + if (hasCode(error, 'ENOENT')) { + return false + } + throw error + } +} + +export async function inspectManagedHookLock(lockPath: string): Promise { + let rawOwner: string + try { + rawOwner = await readFile(lockPath, 'utf8') + } catch (error) { + if (hasCode(error, 'ENOENT')) { + return { kind: 'missing' } + } + // Why: an older relay may own the canonical path as a directory. + return { kind: 'unknown' } + } + try { + const parsed = JSON.parse(rawOwner) as Partial + const owner = typeof parsed.token === 'string' ? parseOwner(rawOwner, parsed.token) : null + return owner ? { kind: 'owned', owner } : { kind: 'unknown' } + } catch { + return { kind: 'unknown' } + } +} + +export async function tryCreateManagedHookLock( + lockParent: string, + lockPath: string, + hostIdentity: string, + processIdentity: string +): Promise { + const token = randomUUID() + const owner = { token, pid: process.pid, hostIdentity, processIdentity } + const ownerPath = join(lockParent, ownerFileName(token)) + const draftPath = join(lockParent, ownerDraftFileName(token)) + await writeFile(draftPath, JSON.stringify(owner), { encoding: 'utf8', flag: 'wx', mode: 0o600 }) + try { + // Why: the final owner name appears only after its complete record is durable. + await rename(draftPath, ownerPath) + try { + // Why: hard-link creation publishes metadata atomically and never replaces a lock. + await link(ownerPath, lockPath) + // Why: publish process-local activity before another same-process contender + // can mistake the newly linked owner for an abandoned release. + activeOwnerTokens.add(token) + return owner + } catch (error) { + if (hasCode(error, 'EEXIST')) { + return null + } + throw error + } + } finally { + await removeFileIfPresent(draftPath) + try { + if ((await lstat(ownerPath)).nlink === 1) { + await unlink(ownerPath) + } + } catch (error) { + if (!hasCode(error, 'ENOENT')) { + console.warn('[agent-hooks] Failed to clean managed-hook owner file', error) + } + } + } +} + +async function cleanOwnerEntry(path: string, token: string, hostIdentity: string): Promise { + const stats = await lstat(path) + if (stats.nlink !== 1) { + return + } + const owner = parseOwner(await readFile(path, 'utf8'), token) + if (!owner || owner.hostIdentity !== hostIdentity) { + return + } + const currentIdentity = await readManagedHookProcessIdentity(owner.pid) + if ( + currentIdentity === null || + (typeof currentIdentity === 'string' && currentIdentity !== owner.processIdentity) + ) { + await unlink(path) + } +} + +async function cleanLockEntry( + entry: string, + lockParent: string, + hostIdentity: string +): Promise { + const path = join(lockParent, entry) + const ownerToken = OWNER_FILE_PATTERN.exec(entry)?.[1] ?? OWNER_DRAFT_PATTERN.exec(entry)?.[1] + if (ownerToken) { + await cleanOwnerEntry(path, ownerToken, hostIdentity) + return + } + const claimedMatch = CLAIMED_OWNER_PATTERN.exec(entry) + if (claimedMatch?.[1] && claimedMatch[2] && (await lstat(path)).nlink === 1) { + await unlink(path) + await removeFileIfPresent( + join(lockParent, claimRecordFileName(claimedMatch[1], claimedMatch[2])) + ) + return + } + const claimMatch = CLAIM_RECORD_PATTERN.exec(entry) + if (!claimMatch?.[1] || !claimMatch[2]) { + return + } + const claim = parseClaim(await readFile(path, 'utf8'), claimMatch[1], claimMatch[2]) + if (!claim || claim.hostIdentity !== hostIdentity) { + return + } + try { + await lstat(join(lockParent, claimedOwnerFileName(claim.ownerToken, claim.claimToken))) + return + } catch (error) { + if (!hasCode(error, 'ENOENT')) { + throw error + } + } + const currentIdentity = await readManagedHookProcessIdentity(claim.pid) + if ( + currentIdentity === null || + (typeof currentIdentity === 'string' && currentIdentity !== claim.processIdentity) + ) { + await unlink(path) + } +} + +export async function cleanupManagedHookLockFiles( + lockParent: string, + hostIdentity: string +): Promise { + let entries: string[] + try { + entries = await readdir(lockParent) + } catch { + return + } + await Promise.all( + entries.map(async (entry) => { + try { + await cleanLockEntry(entry, lockParent, hostIdentity) + } catch (error) { + if (!hasCode(error, 'ENOENT')) { + console.warn('[agent-hooks] Failed to clean managed-hook lock file', error) + } + } + }) + ) +} diff --git a/src/main/agent-hooks/managed-hook-owner-identity.test.ts b/src/main/agent-hooks/managed-hook-owner-identity.test.ts new file mode 100644 index 00000000000..ae8ac15dd12 --- /dev/null +++ b/src/main/agent-hooks/managed-hook-owner-identity.test.ts @@ -0,0 +1,181 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +const originalPlatform = process.platform +const originalGetuidDescriptor = Object.getOwnPropertyDescriptor(process, 'getuid') + +type LinuxIdentityFixture = { + hostToken?: string + bootId?: string + pidNamespace?: string + statErrorCode?: string +} + +async function loadLinuxIdentity(fixture: LinuxIdentityFixture) { + Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' }) + Object.defineProperty(process, 'getuid', { configurable: true, value: () => 1000 }) + const statFields = ['S', ...Array.from({ length: 18 }, () => '0'), '4242'] + const readFile = vi.fn(async (path: string | URL) => { + if (String(path).endsWith('host-id') && fixture.hostToken) { + return fixture.hostToken + } + switch (String(path)) { + case '/proc/sys/kernel/random/boot_id': + if (fixture.bootId) { + return `${fixture.bootId}\n` + } + break + case `/proc/${process.pid}/stat`: + case '/proc/123/stat': + if (!fixture.statErrorCode) { + return `123 (node relay) ${statFields.join(' ')}` + } + break + default: + throw new Error(`unexpected path: ${String(path)}`) + } + throw Object.assign(new Error(`unavailable path: ${String(path)}`), { + code: fixture.statErrorCode ?? 'ENOENT' + }) + }) + const readlink = vi.fn(async (path: string | URL) => { + if (fixture.pidNamespace) { + return fixture.pidNamespace + } + throw Object.assign(new Error(`unavailable path: ${String(path)}`), { code: 'EACCES' }) + }) + const mkdir = vi.fn(async () => { + if (!fixture.hostToken) { + throw Object.assign(new Error('host-local storage unavailable'), { code: 'EACCES' }) + } + }) + const lstat = vi.fn(async (path: string | URL) => { + const isToken = String(path).endsWith('host-id') + return { + isDirectory: () => !isToken, + isFile: () => isToken, + mode: isToken ? 0o100600 : 0o040700, + uid: process.getuid?.() ?? 0 + } + }) + vi.doMock('node:fs/promises', async (importOriginal) => ({ + ...(await importOriginal>()), + lstat, + mkdir, + readFile, + readlink + })) + return { identity: await import('./managed-hook-owner-identity'), readFile } +} + +afterEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) + if (originalGetuidDescriptor) { + Object.defineProperty(process, 'getuid', originalGetuidDescriptor) + } else { + Reflect.deleteProperty(process, 'getuid') + } + vi.unstubAllEnvs() + vi.doUnmock('node:fs/promises') + vi.resetModules() +}) + +describe('managed hook owner identity', () => { + it('uses durable host-local identity without requiring Linux machine identity files', async () => { + vi.stubEnv('SSH_CONNECTION', '198.51.100.8 53100 10.0.0.7 2222') + const { identity, readFile } = await loadLinuxIdentity({ + hostToken: '00000000-0000-4000-8000-000000000001', + bootId: 'current-boot-id', + pidNamespace: 'pid:[4026533001]' + }) + + await expect(identity.readManagedHookHostIdentity()).resolves.toBe( + 'host-token:00000000-0000-4000-8000-000000000001' + ) + expect(readFile).not.toHaveBeenCalledWith('/etc/machine-id', 'utf8') + }) + + it('separates SSH backends that share a key, endpoint, and boot metadata', async () => { + vi.stubEnv('SSH_CONNECTION', '198.51.100.8 53100 10.0.0.7 2222') + const first = await loadLinuxIdentity({ + hostToken: '00000000-0000-4000-8000-000000000001', + bootId: 'shared-kernel-boot-id', + pidNamespace: 'pid:[4026533001]' + }) + const firstHost = await first.identity.readManagedHookHostIdentity() + const firstProcess = await first.identity.readManagedHookProcessIdentity(123) + const fingerprint = 'SHA256:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA' + const firstScopedHost = first.identity.scopeManagedHookHostIdentity(firstHost, fingerprint) + + vi.doUnmock('node:fs/promises') + vi.resetModules() + const second = await loadLinuxIdentity({ + hostToken: '00000000-0000-4000-8000-000000000002', + bootId: 'shared-kernel-boot-id', + pidNamespace: 'pid:[4026533002]' + }) + + const secondHost = await second.identity.readManagedHookHostIdentity() + expect(secondHost).not.toBe(firstHost) + expect(second.identity.scopeManagedHookHostIdentity(secondHost, fingerprint)).not.toBe( + firstScopedHost + ) + await expect(second.identity.readManagedHookProcessIdentity(123)).resolves.not.toBe( + firstProcess + ) + }) + + it('keeps one host scope stable across reboot while changing its process incarnation', async () => { + const fixture = { + hostToken: '00000000-0000-4000-8000-000000000001', + bootId: 'first-boot-id', + pidNamespace: 'pid:[4026533001]' + } + const first = await loadLinuxIdentity(fixture) + const firstHost = await first.identity.readManagedHookHostIdentity() + const firstProcess = await first.identity.readManagedHookProcessIdentity(123) + + vi.doUnmock('node:fs/promises') + vi.resetModules() + const second = await loadLinuxIdentity({ + ...fixture, + bootId: 'second-boot-id', + pidNamespace: 'pid:[4026534001]' + }) + + await expect(second.identity.readManagedHookHostIdentity()).resolves.toBe(firstHost) + await expect(second.identity.readManagedHookProcessIdentity(123)).resolves.not.toBe( + firstProcess + ) + }) + + it('falls back safely when machine, boot, and namespace probes are unavailable', async () => { + vi.stubEnv('SSH_CONNECTION', '') + const { identity } = await loadLinuxIdentity({ statErrorCode: 'EACCES' }) + + const hostIdentity = await identity.readManagedHookHostIdentity() + const processIdentity = await identity.readManagedHookProcessIdentity(process.pid) + expect(hostIdentity).toMatch(/^runtime:/) + expect(processIdentity).toMatch(/^runtime:/) + await expect(identity.readManagedHookHostIdentity()).resolves.toBe(hostIdentity) + await expect(identity.readManagedHookProcessIdentity(process.pid)).resolves.toBe( + processIdentity + ) + await expect(identity.readManagedHookProcessIdentity(123)).resolves.toBeUndefined() + }) + + it('includes namespace, boot, and start ticks when Linux exposes them', async () => { + vi.stubEnv('SSH_CONNECTION', '') + const { identity } = await loadLinuxIdentity({ + hostToken: '00000000-0000-4000-8000-000000000001', + bootId: 'current-boot-id', + pidNamespace: 'pid:[4026533001]' + }) + + await expect(identity.readManagedHookHostIdentity()).resolves.toBe( + 'host-token:00000000-0000-4000-8000-000000000001' + ) + await expect(identity.readManagedHookProcessIdentity(123)).resolves.toBe( + 'linux:pid:[4026533001]:current-boot-id:4242' + ) + }) +}) diff --git a/src/main/agent-hooks/managed-hook-owner-identity.ts b/src/main/agent-hooks/managed-hook-owner-identity.ts new file mode 100644 index 00000000000..e9801c3c489 --- /dev/null +++ b/src/main/agent-hooks/managed-hook-owner-identity.ts @@ -0,0 +1,209 @@ +import { execFile } from 'node:child_process' +import { randomUUID } from 'node:crypto' +import { link, lstat, mkdir, readFile, readlink, unlink, writeFile } from 'node:fs/promises' +import { join } from 'node:path' +import { promisify } from 'node:util' + +const execFileAsync = promisify(execFile) +const runtimeHostIdentity = `runtime:${randomUUID()}` +const runtimeProcessIdentity = `runtime:${randomUUID()}` +let hostIdentityPromise: Promise | undefined +let bootIdentityPromise: Promise | undefined +const HOST_TOKEN_PATTERN = /^[\da-f]{8}-[\da-f]{4}-4[\da-f]{3}-[89ab][\da-f]{3}-[\da-f]{12}$/i + +function hasCode(error: unknown, code: string): boolean { + return error instanceof Error && 'code' in error && error.code === code +} + +function parseLinuxStartTicks(statLine: string): string | null { + const commandEnd = statLine.lastIndexOf(') ') + if (commandEnd < 0) { + return null + } + // Field 22 is index 19 after removing pid and the parenthesized command. + const startTicks = statLine + .slice(commandEnd + 2) + .trim() + .split(/\s+/)[19] + return startTicks ?? null +} + +async function readLinuxPidNamespace(pid: number): Promise { + try { + const namespace = await readlink(`/proc/${pid}/ns/pid`) + return namespace.trim() || undefined + } catch { + return undefined + } +} + +async function readPublishedHostToken(path: string, uid: number): Promise { + try { + const stats = await lstat(path) + if (!stats.isFile() || stats.uid !== uid || (stats.mode & 0o077) !== 0) { + return undefined + } + const token = (await readFile(path, 'utf8')).trim() + return HOST_TOKEN_PATTERN.test(token) ? token : undefined + } catch { + return undefined + } +} + +async function readDurableHostToken(): Promise { + const uid = process.getuid?.() + if ((process.platform !== 'linux' && process.platform !== 'darwin') || uid === undefined) { + return undefined + } + const directory = join('/var', 'tmp', `orca-managed-hooks-${uid}`) + const tokenPath = join(directory, 'host-id') + try { + await mkdir(directory, { recursive: true, mode: 0o700 }) + const directoryStats = await lstat(directory) + if ( + !directoryStats.isDirectory() || + directoryStats.uid !== uid || + (directoryStats.mode & 0o077) !== 0 + ) { + return undefined + } + const existing = await readPublishedHostToken(tokenPath, uid) + if (existing) { + return existing + } + const token = randomUUID() + const draftPath = join(directory, `host-id-draft-${token}`) + await writeFile(draftPath, token, { encoding: 'utf8', flag: 'wx', mode: 0o600 }) + try { + // Why: the hard link publishes a complete token atomically across relay processes. + await link(draftPath, tokenPath).catch((error) => { + if (!hasCode(error, 'EEXIST')) { + throw error + } + }) + } finally { + await unlink(draftPath).catch(() => {}) + } + return await readPublishedHostToken(tokenPath, uid) + } catch { + return undefined + } +} + +async function readHostIdentity(): Promise { + const durableToken = await readDurableHostToken() + if (durableToken) { + return `host-token:${durableToken}` + } + if (process.platform === 'linux') { + // Why: an unverifiable host scope may acquire and release its own clean lock, + // but a later process gets a different identity and cannot steal its residue. + return runtimeHostIdentity + } + if (process.platform === 'darwin') { + try { + const { stdout } = await execFileAsync('ioreg', ['-rd1', '-c', 'IOPlatformExpertDevice'], { + encoding: 'utf8', + timeout: 1_000 + }) + const platformId = /"IOPlatformUUID"\s*=\s*"([^"]+)"/.exec(stdout)?.[1] + return platformId ? `darwin:${platformId}` : runtimeHostIdentity + } catch { + return runtimeHostIdentity + } + } + return runtimeHostIdentity +} + +async function readBootIdentity(): Promise { + if (process.platform === 'linux') { + try { + const bootId = (await readFile('/proc/sys/kernel/random/boot_id', 'utf8')).trim() + return bootId || undefined + } catch { + return undefined + } + } + + if (process.platform === 'darwin') { + try { + const { stdout } = await execFileAsync('sysctl', ['-n', 'kern.bootsessionuuid'], { + encoding: 'utf8', + timeout: 1_000 + }) + const bootSessionId = stdout.trim() + return bootSessionId || undefined + } catch { + return undefined + } + } + + return undefined +} + +export async function readManagedHookHostIdentity(): Promise { + hostIdentityPromise ??= readHostIdentity() + return await hostIdentityPromise +} + +/** Bind a stable SSH identity to one execution runtime before comparing its PIDs. */ +export function scopeManagedHookHostIdentity( + executionIdentity: string, + hostKeyFingerprint?: string +): string { + return hostKeyFingerprint + ? `ssh-host-key:${hostKeyFingerprint}:execution:${executionIdentity}` + : executionIdentity +} + +/** null means confirmed missing; undefined means the platform probe was unavailable. */ +export async function readManagedHookProcessIdentity( + pid: number +): Promise { + if (process.platform === 'linux') { + try { + const [statLine, pidNamespace, bootIdentity] = await Promise.all([ + readFile(`/proc/${pid}/stat`, 'utf8'), + readLinuxPidNamespace(pid), + (bootIdentityPromise ??= readBootIdentity()) + ]) + const startTicks = parseLinuxStartTicks(statLine) + return startTicks + ? `linux:${pidNamespace ?? 'unknown-namespace'}:${bootIdentity ?? 'unknown-boot'}:${startTicks}` + : undefined + } catch (error) { + if (hasCode(error, 'ENOENT')) { + return null + } + return pid === process.pid ? runtimeProcessIdentity : undefined + } + } + + bootIdentityPromise ??= readBootIdentity() + const bootIdentity = await bootIdentityPromise + try { + const { stdout } = await execFileAsync( + 'ps', + ['-o', 'lstart=', '-o', 'command=', '-p', String(pid)], + { + encoding: 'utf8', + timeout: 1_000, + // Why: lstart is locale-sensitive; a fixed locale keeps the identity + // stable across relay processes for the same remote account. + env: { ...process.env, LC_ALL: 'C', LANG: 'C' } + } + ) + const startedAt = stdout.trim() + return startedAt ? `${process.platform}:${bootIdentity ?? 'unknown-boot'}:${startedAt}` : null + } catch { + try { + process.kill(pid, 0) + return undefined + } catch (error) { + if (hasCode(error, 'ESRCH')) { + return null + } + return pid === process.pid ? runtimeProcessIdentity : undefined + } + } +} diff --git a/src/main/agent-hooks/managed-hook-runtime.test.ts b/src/main/agent-hooks/managed-hook-runtime.test.ts new file mode 100644 index 00000000000..456a741df87 --- /dev/null +++ b/src/main/agent-hooks/managed-hook-runtime.test.ts @@ -0,0 +1,22 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { resolveRelayGrokHome } from './managed-hook-runtime' + +afterEach(() => { + vi.unstubAllEnvs() +}) + +describe.runIf(process.platform !== 'win32')('resolveRelayGrokHome', () => { + it('uses the login-shell GROK_HOME and normalizes trailing separators', async () => { + vi.stubEnv('SHELL', '/bin/sh') + vi.stubEnv('GROK_HOME', '/srv/grok///') + + await expect(resolveRelayGrokHome('/home/orca')).resolves.toBe('/srv/grok') + }) + + it('falls back when the login-shell GROK_HOME is not an absolute POSIX path', async () => { + vi.stubEnv('SHELL', '/bin/sh') + vi.stubEnv('GROK_HOME', '../relative') + + await expect(resolveRelayGrokHome('/home/orca')).resolves.toBe('/home/orca/.grok') + }) +}) diff --git a/src/main/agent-hooks/managed-hook-runtime.ts b/src/main/agent-hooks/managed-hook-runtime.ts new file mode 100644 index 00000000000..b79c3b2186d --- /dev/null +++ b/src/main/agent-hooks/managed-hook-runtime.ts @@ -0,0 +1,106 @@ +import { execFile } from 'node:child_process' +import { basename } from 'node:path' +import { homedir, userInfo } from 'node:os' +import { promisify } from 'node:util' +import { installRemoteManagedAgentHooks } from './remote-managed-hook-installers' +import { createManagedHookLocalFilesystem } from './managed-hook-local-filesystem' +import { withManagedHookInstallLock } from './managed-hook-install-lock' +import { + readManagedHookHostIdentity, + scopeManagedHookHostIdentity +} from './managed-hook-owner-identity' + +const execFileAsync = promisify(execFile) +const GROK_HOME_MAX_LENGTH = 4096 +const GROK_HOME_PROBE_TIMEOUT_MS = 8_000 + +export type ManagedHookInstallSummary = { + installers: number + errors: number +} + +function defaultGrokHome(home: string): string { + return `${home.replace(/\/+$/, '') || home}/.grok` +} + +function hasControlCharacter(value: string): boolean { + return Array.from(value).some((character) => { + const code = character.charCodeAt(0) + return code <= 0x1f || code === 0x7f + }) +} + +function normalizeGrokHome(candidate: string): string | null { + if ( + candidate.length === 0 || + candidate.length > GROK_HOME_MAX_LENGTH || + candidate !== candidate.trim() || + !candidate.startsWith('/') || + candidate.includes('\\') || + hasControlCharacter(candidate) + ) { + return null + } + return candidate.replace(/\/+$/, '') || '/' +} + +function resolveLoginShell(): string { + const candidate = process.env.SHELL || userInfo().shell || '/bin/sh' + if (!candidate.startsWith('/') || candidate.includes('\\') || hasControlCharacter(candidate)) { + return '/bin/sh' + } + return candidate +} + +export async function resolveRelayGrokHome(home: string, signal?: AbortSignal): Promise { + const fallback = defaultGrokHome(home) + try { + const shell = resolveLoginShell() + const shellName = basename(shell) + const mode = shellName === 'sh' || shellName === 'dash' ? '-c' : '-lc' + // Why: agent PTYs start login shells, so read the same profile-derived + // GROK_HOME without opening two additional SSH exec channels. + const { stdout } = await execFileAsync( + shell, + [mode, `printenv GROK_HOME | head -c ${GROK_HOME_MAX_LENGTH + 1}`], + { encoding: 'utf8', timeout: GROK_HOME_PROBE_TIMEOUT_MS, signal } + ) + return normalizeGrokHome(stdout.split(/\r?\n/, 1)[0] ?? '') ?? fallback + } catch { + signal?.throwIfAborted() + return fallback + } +} + +export async function installManagedHooks(options?: { + signal?: AbortSignal + hostKeyFingerprint?: string +}): Promise { + options?.signal?.throwIfAborted() + const home = homedir() + const grokHomeDir = await resolveRelayGrokHome(home, options?.signal) + options?.signal?.throwIfAborted() + const hostIdentity = scopeManagedHookHostIdentity( + await readManagedHookHostIdentity(), + options?.hostKeyFingerprint + ) + return await withManagedHookInstallLock( + home, + options?.signal, + async () => { + const results = await installRemoteManagedAgentHooks( + createManagedHookLocalFilesystem(), + home, + { + grokHomeDir, + signal: options?.signal + } + ) + return { + installers: results.length, + errors: results.filter((result) => result.state === 'error').length + } + }, + hostIdentity + ) +} diff --git a/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts index 39639a36b49..bc11aba30f7 100644 --- a/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts +++ b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts @@ -2,12 +2,31 @@ // matrix catches an unread early exit without duplicating template assertions. import { describe, expect, it, vi } from 'vitest' import { spawn } from 'node:child_process' -import { mkdtempSync, readFileSync, readdirSync, rmSync } from 'node:fs' +import { existsSync, mkdtempSync, readFileSync, readdirSync, rmSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import type { SFTPWrapper } from 'ssh2' import type * as osModule from 'node:os' +function findGitBash(): string { + if (process.env.KIMI_SHELL_PATH) { + return process.env.KIMI_SHELL_PATH + } + const candidates = [ + process.env.ProgramFiles && join(process.env.ProgramFiles, 'Git', 'bin', 'bash.exe'), + process.env['ProgramFiles(x86)'] && + join(process.env['ProgramFiles(x86)'], 'Git', 'bin', 'bash.exe'), + process.env.LOCALAPPDATA && join(process.env.LOCALAPPDATA, 'Programs', 'Git', 'bin', 'bash.exe') + ] + const bash = candidates.find((candidate): candidate is string => + Boolean(candidate && existsSync(candidate)) + ) + if (!bash) { + throw new Error('Git Bash is required for the Windows Kimi hook lifecycle test') + } + return bash +} + const { homedirMock } = vi.hoisted(() => ({ homedirMock: vi.fn<() => string>() })) @@ -255,6 +274,7 @@ describe('Windows managed hook stdin structure', () => { const home = mkdtempSync(join(tmpdir(), 'orca-hook-stdin-windows-live-')) homedirMock.mockReturnValue(home) try { + const gitBash = findGitBash() for (const entry of LOCAL_INSTALLERS) { expect(entry.install().state, `${entry.agent} install status`).toBe('installed') } @@ -279,7 +299,7 @@ describe('Windows managed hook stdin structure', () => { 'v1.0', 'powershell.exe' ) - : process.env.KIMI_SHELL_PATH || 'bash.exe' + : gitBash const args = fileName.endsWith('.cmd') ? ['/d', '/c', scriptPath] : fileName.endsWith('.ps1') @@ -306,7 +326,7 @@ describe('Windows managed hook stdin structure', () => { }, { name: 'Git Bash fast path', - executable: process.env.KIMI_SHELL_PATH || 'bash.exe', + executable: gitBash, args: ['-lc', wrapWindowsGitBashHookCommand(missingScript)] } ] diff --git a/src/main/agent-hooks/managed-hook-timeout.test.ts b/src/main/agent-hooks/managed-hook-timeout.test.ts index a215210a577..c7d374573ff 100644 --- a/src/main/agent-hooks/managed-hook-timeout.test.ts +++ b/src/main/agent-hooks/managed-hook-timeout.test.ts @@ -118,10 +118,16 @@ const MANAGED_HOOKS_DIR_NEEDLE = '/.orca/agent-hooks/' // dir) has a positive config-level timeout sibling (`timeout` or the // provider-specific `timeoutSec`). Returns the count of managed carriers found // so callers can assert the scan was not vacuous. -function countManagedCarriersWithTimeout(node: unknown, expectedTimeout: number): number { +function countManagedCarriersWithTimeout( + node: unknown, + expectedTimeout: number, + isManagedCarrier = (value: string): boolean => + value.replaceAll('\\', '/').includes(MANAGED_HOOKS_DIR_NEEDLE) +): number { if (Array.isArray(node)) { return node.reduce( - (sum, child) => sum + countManagedCarriersWithTimeout(child, expectedTimeout), + (sum, child) => + sum + countManagedCarriersWithTimeout(child, expectedTimeout, isManagedCarrier), 0 ) } @@ -131,8 +137,7 @@ function countManagedCarriersWithTimeout(node: unknown, expectedTimeout: number) const record = node as Record let found = 0 const carrier = [record.command, record.bash, record.powershell].find( - (value): value is string => - typeof value === 'string' && value.includes(MANAGED_HOOKS_DIR_NEEDLE) + (value): value is string => typeof value === 'string' && isManagedCarrier(value) ) if (carrier !== undefined) { const timeout = typeof record.timeout === 'number' ? record.timeout : record.timeoutSec @@ -143,7 +148,7 @@ function countManagedCarriersWithTimeout(node: unknown, expectedTimeout: number) found += 1 } for (const value of Object.values(record)) { - found += countManagedCarriersWithTimeout(value, expectedTimeout) + found += countManagedCarriersWithTimeout(value, expectedTimeout, isManagedCarrier) } return found } @@ -182,7 +187,13 @@ describe('managed agent hook timeouts', () => { const status = new DroidHookService().install() expect(status.state).toBe('installed') const config = JSON.parse(readFileSync(join(homeDir, '.factory', 'settings.json'), 'utf8')) - const carriers = countManagedCarriersWithTimeout(config, MANAGED_HOOK_TIMEOUT_SECONDS) + const carriers = countManagedCarriersWithTimeout( + config, + MANAGED_HOOK_TIMEOUT_SECONDS, + (command) => + command.replaceAll('\\', '/').includes(MANAGED_HOOKS_DIR_NEEDLE) || + (process.platform === 'win32' && command.includes('-EncodedCommand')) + ) expect(carriers).toBeGreaterThan(0) } finally { homedirMock.mockImplementation(() => process.env.HOME ?? tmpdir()) diff --git a/src/main/agent-hooks/remote-hook-service-installers.test.ts b/src/main/agent-hooks/remote-hook-service-installers.test.ts index 8489048d0f5..3acca6a64ef 100644 --- a/src/main/agent-hooks/remote-hook-service-installers.test.ts +++ b/src/main/agent-hooks/remote-hook-service-installers.test.ts @@ -726,6 +726,35 @@ describe('remote hook service installers', () => { expect(byAgent.get('copilot')).toBe('installed') }) + it('stops before the next installer when its relay request is cancelled', async () => { + const controller = new AbortController() + const claudeInstall = vi + .spyOn(claudeHookService, 'installRemote') + .mockImplementation(async () => { + controller.abort() + return { + agent: 'claude', + state: 'installed', + configPath: '/home/dev/.claude/settings.json', + managedHooksPresent: true, + detail: null + } + }) + const openClaudeInstall = vi.spyOn(openClaudeHookService, 'installRemote') + try { + const { sftp } = createFakeSftp() + + await expect( + installRemoteManagedAgentHooks(sftp, '/home/dev', { signal: controller.signal }) + ).rejects.toMatchObject({ name: 'AbortError' }) + expect(claudeInstall).toHaveBeenCalledTimes(1) + expect(openClaudeInstall).not.toHaveBeenCalled() + } finally { + claudeInstall.mockRestore() + openClaudeInstall.mockRestore() + } + }) + it('installs remote Droid hooks into Factory settings.json (issue #7253)', async () => { const { sftp, fs } = createFakeSftp() diff --git a/src/main/agent-hooks/remote-managed-hook-installers.ts b/src/main/agent-hooks/remote-managed-hook-installers.ts index 39507f091f2..7f9e97f2f9f 100644 --- a/src/main/agent-hooks/remote-managed-hook-installers.ts +++ b/src/main/agent-hooks/remote-managed-hook-installers.ts @@ -23,6 +23,9 @@ export type RemoteManagedHookInstallOptions = { codexHomeDir?: string /** Explicit GROK_HOME for remote runtimes that redirect Grok's config. */ grokHomeDir?: string + /** Stops before starting the next installer when the owning relay request + * is cancelled. Individual filesystem mutations remain atomic. */ + signal?: AbortSignal } type RemoteManagedHookInstaller = readonly [ @@ -78,6 +81,9 @@ export async function installRemoteManagedAgentHooks( ): Promise { const results: AgentHookInstallStatus[] = [] for (const [agent, install] of REMOTE_MANAGED_HOOK_INSTALLERS) { + // Why: relay requests can disappear during reconnect; do not start more + // user-config mutations after their client has gone away. + options?.signal?.throwIfAborted() try { const result = await install(sftp, remoteHome, options) results.push(result) diff --git a/src/main/ipc/ssh.test.ts b/src/main/ipc/ssh.test.ts index cbca2db9d2a..49b6bcba83a 100644 --- a/src/main/ipc/ssh.test.ts +++ b/src/main/ipc/ssh.test.ts @@ -1225,6 +1225,49 @@ describe('SSH IPC handlers', () => { expect(mockConnectionManager.disconnect).toHaveBeenCalledWith('ssh-1') }) + it('invalidates a pending connect when disconnect wins and allows a fresh connect', async () => { + const target: SshTarget = { + id: 'ssh-1', + label: 'Server', + host: 'example.com', + port: 22, + username: 'deploy' + } + const staleConn = {} + const freshConn = {} + let resolveStaleConnect!: (connection: unknown) => void + mockSshStore.getTarget.mockReturnValue(target) + mockConnectionManager.connect.mockReturnValueOnce( + new Promise((resolve) => { + resolveStaleConnect = resolve + }) + ) + mockConnectionManager.getState.mockReturnValue({ + targetId: 'ssh-1', + status: 'connected', + error: null, + reconnectAttempt: 0 + }) + + const staleConnect = handlers.get('ssh:connect')!(null, { + targetId: 'ssh-1' + }) as Promise + await vi.waitFor(() => expect(mockConnectionManager.connect).toHaveBeenCalledTimes(1)) + + await handlers.get('ssh:disconnect')!(null, { targetId: 'ssh-1' }) + mockConnectionManager.connect.mockResolvedValueOnce(freshConn) + const freshConnect = handlers.get('ssh:connect')!(null, { + targetId: 'ssh-1' + }) as Promise + await vi.waitFor(() => expect(mockConnectionManager.connect).toHaveBeenCalledTimes(2)) + + resolveStaleConnect(staleConn) + + await expect(staleConnect).rejects.toThrow('SSH connection attempt was cancelled') + await expect(freshConnect).resolves.toMatchObject({ targetId: 'ssh-1', status: 'connected' }) + expect(mockDeployAndLaunchRelay).toHaveBeenCalledTimes(1) + }) + it('ssh:terminateSessions preserves tracking when relay shutdown fails', async () => { const target: SshTarget = { id: 'ssh-1', diff --git a/src/main/ipc/ssh.ts b/src/main/ipc/ssh.ts index d1c3e51c3fd..f8f55ecaae8 100644 --- a/src/main/ipc/ssh.ts +++ b/src/main/ipc/ssh.ts @@ -99,6 +99,7 @@ export function listRegisteredRemovedSshTargetLabels(): Record { } export async function disconnectRegisteredSshTarget(targetId: string): Promise { + invalidateConnectAttempt(targetId) if (!connectionManager) { return } @@ -110,7 +111,8 @@ export async function removeRegisteredSshTarget(targetId: string): Promise if (!sshStore) { return } - // Why: removal is destructive — dispose() (not detach()) so the relay shuts down and remote PTY leases are terminated, not preserved for a reattach. + invalidateConnectAttempt(targetId) + // Why: removal is destructive; dispose so remote PTYs cannot reattach to a deleted target. await disposeActiveSshSession(targetId) try { await connectionManager?.disconnect(targetId) @@ -172,8 +174,33 @@ function relayGracePeriodForTarget(target: SshTarget | null | undefined): number return target?.relayGracePeriodSeconds } -// Why: concurrent ssh:connect from multiple tabs would each create a session (leaking the first); hold the in-flight promise so the second awaits it. -const connectInFlight = new Map>() +// Why: tabs must share one connect, while a disconnect must invalidate that +// attempt so its late continuation cannot clobber a replacement. +type ConnectAttempt = { + generation: number + promise: Promise +} + +const connectInFlight = new Map() +const connectGenerationByTarget = new Map() + +function currentConnectGeneration(targetId: string): number { + return connectGenerationByTarget.get(targetId) ?? 0 +} + +function invalidateConnectAttempt(targetId: string): void { + connectGenerationByTarget.set(targetId, currentConnectGeneration(targetId) + 1) + connectInFlight.delete(targetId) + credentialRequestedForTarget.delete(targetId) +} + +function isCurrentConnectAttempt(targetId: string, generation: number): boolean { + return currentConnectGeneration(targetId) === generation +} + +function connectCancelledError(): Error { + return new Error('SSH connection attempt was cancelled') +} // Why: publish reset's teardown/force-stop/disconnect lifecycle so new connects and duplicate resets can't race it. const resetRelayInFlight = new Map>() @@ -732,6 +759,7 @@ export function registerSshHandlers( // ── Connection lifecycle ─────────────────────────────────────────── async function connectTarget(targetId: string): Promise { + const observedGeneration = currentConnectGeneration(targetId) const reset = resetRelayInFlight.get(targetId) if (reset) { await reset @@ -740,15 +768,23 @@ export function registerSshHandlers( // Why: serialize concurrent ssh:connect for the same target; interleaved connects otherwise leak the first session. const existing = connectInFlight.get(targetId) if (existing) { - return existing + return existing.promise + } + if (currentConnectGeneration(targetId) !== observedGeneration) { + throw connectCancelledError() } - const promise = doConnect(targetId) - connectInFlight.set(targetId, promise) + const generation = observedGeneration + 1 + connectGenerationByTarget.set(targetId, generation) + const promise = doConnect(targetId, generation) + const attempt = { generation, promise } + connectInFlight.set(targetId, attempt) try { return await promise } finally { - connectInFlight.delete(targetId) + if (connectInFlight.get(targetId) === attempt) { + connectInFlight.delete(targetId) + } } } @@ -759,7 +795,7 @@ export function registerSshHandlers( return connectTarget(args.targetId) }) - async function doConnect(targetId: string): Promise { + async function doConnect(targetId: string, generation: number): Promise { const target = sshStore!.getTarget(targetId) if (!target) { throw new Error(`SSH target "${targetId}" not found`) @@ -788,6 +824,9 @@ export function registerSshHandlers( if (existingSession) { // Why: await port teardown before disposing, else the new session's restorePortForwards can hit EADDRINUSE on not-yet-released ports. await portForwardManager!.removeAllForwards(targetId) + if (!isCurrentConnectAttempt(targetId, generation)) { + throw connectCancelledError() + } existingSession.detach() activeSessions.delete(targetId) clearRelayLostBackoff(targetId) @@ -805,14 +844,22 @@ export function registerSshHandlers( ) configureRelaySessionCallbacks(session) activeSessions.set(targetId, session) + const ownsSession = (): boolean => + isCurrentConnectAttempt(targetId, generation) && activeSessions.get(targetId) === session try { conn = await connectionManager!.connect(target) + if (!ownsSession()) { + throw connectCancelledError() + } } catch (err) { // Why: connect()'s internal state may not have reached the renderer; broadcast explicitly so the UI leaves 'connecting'. const errObj = err instanceof Error ? err : new Error(String(err)) const status: SshConnectionStatus = isAuthError(errObj) ? 'auth-failed' : 'error' - // Why: clear this failed connect's credential flag so a later non-prompting connect can't persist lastRequiredPassphrase=true. + if (!ownsSession()) { + throw connectCancelledError() + } + // Why: clear this failed connect's flag so a later non-prompting connect isn't deferred. credentialRequestedForTarget.delete(targetId) activeSessions.delete(targetId) clearRelayLostBackoff(targetId) @@ -835,6 +882,9 @@ export function registerSshHandlers( }) await session.establish(conn, relayGracePeriodForTarget(target)) + if (!ownsSession()) { + throw connectCancelledError() + } // Why: we manually pushed `deploying-relay`, so send `connected` straight to the renderer — routing through onStateChange would trigger reconnect logic. clearRelayStateOverride(targetId) @@ -846,6 +896,9 @@ export function registerSshHandlers( supportsFolderDownload: conn.usesSystemSshTransport?.() !== true }) } catch (err) { + if (!ownsSession()) { + throw connectCancelledError() + } activeSessions.delete(targetId) clearRelayLostBackoff(targetId) await connectionManager!.disconnect(targetId) @@ -865,6 +918,7 @@ export function registerSshHandlers( }) ipcMain.handle('ssh:terminateSessions', async (_event, args: { targetId: string }) => { + invalidateConnectAttempt(args.targetId) const session = activeSessions.get(args.targetId) const provider = getSshPtyProvider(args.targetId) const leasedIds = persistedStore! @@ -931,8 +985,8 @@ export function registerSshHandlers( const inFlightConnect = connectInFlight.get(targetId) if (inFlightConnect) { try { - // Why: await the in-flight connect first; tearing down activeSessions mid-deploy would dispose the session doConnect is about to use. - await inFlightConnect + // Why: resetting activeSessions mid-deploy would dispose the session doConnect will use. + await inFlightConnect.promise } catch { // The reset can still recover a stale remote relay after a failed connect. } @@ -1029,7 +1083,7 @@ export function registerSshHandlers( const inFlight = connectInFlight.get(args.targetId) if (inFlight) { try { - const state = await inFlight + const state = await inFlight.promise return { success: true, state } } catch (err) { return { @@ -1177,6 +1231,7 @@ export async function resetSshHandlerStateForTests(): Promise { } relayStateOverrides.clear() connectInFlight.clear() + connectGenerationByTarget.clear() resetRelayInFlight.clear() testingTargets.clear() credentialRequestedForTarget.clear() diff --git a/src/main/ssh/ssh-connection-manager.test.ts b/src/main/ssh/ssh-connection-manager.test.ts new file mode 100644 index 00000000000..2667dc28e1f --- /dev/null +++ b/src/main/ssh/ssh-connection-manager.test.ts @@ -0,0 +1,74 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { SshTarget } from '../../shared/ssh-types' + +const mockState = vi.hoisted(() => ({ + connectResults: [] as Promise[], + instances: [] as { + connect: ReturnType + disconnect: ReturnType + status: 'connecting' | 'connected' | 'disconnected' + }[] +})) + +vi.mock('./ssh-connection', () => ({ + SshConnection: class MockSshConnection { + status: 'connecting' | 'connected' | 'disconnected' = 'connecting' + connect = vi.fn(async () => { + await (mockState.connectResults.shift() ?? Promise.resolve()) + this.status = 'connected' + }) + disconnect = vi.fn(async () => { + this.status = 'disconnected' + }) + + constructor() { + mockState.instances.push(this) + } + + getState(): { status: 'connecting' | 'connected' | 'disconnected' } { + return { status: this.status } + } + + setCallbacks(): void {} + } +})) + +import { SshConnectionManager } from './ssh-connection-manager' + +const target = { + id: 'target-1', + label: 'Target 1', + host: 'example.test', + port: 22, + username: 'demo', + source: 'manual' +} as SshTarget + +describe('SshConnectionManager', () => { + beforeEach(() => { + mockState.connectResults.length = 0 + mockState.instances.length = 0 + }) + + it('lets disconnect start a new connect before the cancelled attempt settles', async () => { + let rejectFirst!: (error: Error) => void + mockState.connectResults.push( + new Promise((_resolve, reject) => { + rejectFirst = reject + }), + Promise.resolve() + ) + const manager = new SshConnectionManager({ + onStateChange: vi.fn() + }) + + const firstConnect = manager.connect(target) + await manager.disconnect(target.id) + const secondConnection = await manager.connect(target) + rejectFirst(new Error('cancelled')) + + await expect(firstConnect).rejects.toThrow('cancelled') + expect(mockState.instances).toHaveLength(2) + expect(manager.getConnection(target.id)).toBe(secondConnection) + }) +}) diff --git a/src/main/ssh/ssh-connection-manager.ts b/src/main/ssh/ssh-connection-manager.ts index d568d439409..8bc17d06b70 100644 --- a/src/main/ssh/ssh-connection-manager.ts +++ b/src/main/ssh/ssh-connection-manager.ts @@ -9,10 +9,9 @@ import { SshConnection, type SshConnectionCallbacks } from './ssh-connection' export class SshConnectionManager { private connections = new Map() private callbacks: SshConnectionCallbacks - // Why: two concurrent connect() calls for the same target would both pass - // the "existing" check, create two SshConnections, and orphan the first. - // This set prevents a second call from racing with an in-progress one. - private connectingTargets = new Set() + // Why: attempt identity lets disconnect unblock a replacement without the + // cancelled attempt later clearing the replacement's state. + private connectingTargets = new Map() constructor(callbacks: SshConnectionCallbacks) { this.callbacks = callbacks @@ -35,7 +34,8 @@ export class SshConnectionManager { throw new Error(`Connection to ${target.label} is already in progress`) } - this.connectingTargets.add(target.id) + const attempt = Symbol(target.id) + this.connectingTargets.set(target.id, attempt) try { if (existing) { @@ -48,23 +48,32 @@ export class SshConnectionManager { try { await conn.connect() } catch (err) { - this.connections.delete(target.id) + if (this.connections.get(target.id) === conn) { + this.connections.delete(target.id) + } throw err } return conn } finally { - this.connectingTargets.delete(target.id) + if (this.connectingTargets.get(target.id) === attempt) { + this.connectingTargets.delete(target.id) + } } } async disconnect(targetId: string): Promise { + // Why: disconnect invalidates the old attempt immediately so a reconnect + // need not wait for the cancelled socket's late completion. + this.connectingTargets.delete(targetId) const conn = this.connections.get(targetId) if (!conn) { return } await conn.disconnect() - this.connections.delete(targetId) + if (this.connections.get(targetId) === conn) { + this.connections.delete(targetId) + } } async reconnect(targetId: string): Promise { diff --git a/src/main/ssh/ssh-connection.test.ts b/src/main/ssh/ssh-connection.test.ts index 37bffc0d691..750932289d8 100644 --- a/src/main/ssh/ssh-connection.test.ts +++ b/src/main/ssh/ssh-connection.test.ts @@ -60,6 +60,9 @@ vi.mock('ssh2', () => { } connect(config?: unknown) { this.lastConnectConfig = config + const hostVerifier = (config as { hostVerifier?: (key: Buffer) => boolean } | undefined) + ?.hostVerifier + hostVerifier?.(Buffer.from('mock-ssh-host-key')) setTimeout(() => { const next = connectSequence.shift() if (next instanceof Error) { @@ -320,6 +323,35 @@ describe('SshConnection', () => { expect(clientInstances[0].setNoDelay).toHaveBeenCalledWith(true) }) + it('captures the negotiated SSH server key fingerprint', async () => { + const conn = new SshConnection(createTarget(), createCallbacks()) + await conn.connect() + + expect(conn.getHostKeyFingerprint()).toMatch(/^SHA256:[A-Za-z\d+/]{43}$/) + }) + + it('ignores a late host fingerprint from an obsolete connect generation', async () => { + const conn = new SshConnection(createTarget(), createCallbacks()) + await conn.connect() + const firstVerifier = ( + clientInstances[0].lastConnectConfig as { hostVerifier?: (key: Buffer) => boolean } + ).hostVerifier + + const privateConn = conn as unknown as { attemptConnect: () => Promise } + await privateConn.attemptConnect() + const secondVerifier = ( + clientInstances[1].lastConnectConfig as { hostVerifier?: (key: Buffer) => boolean } + ).hostVerifier + expect(firstVerifier).toBeTypeOf('function') + expect(secondVerifier).toBeTypeOf('function') + + secondVerifier?.(Buffer.from('newer-ssh-host-key')) + const currentFingerprint = conn.getHostKeyFingerprint() + firstVerifier?.(Buffer.from('obsolete-ssh-host-key')) + + expect(conn.getHostKeyFingerprint()).toBe(currentFingerprint) + }) + it('allows concurrent exec commands for ssh2 transport', async () => { const conn = new SshConnection(createTarget(), createCallbacks()) await conn.connect() @@ -1248,6 +1280,7 @@ describe('SshConnection', () => { expect(conn.getState().status).toBe('connected') expect(conn.usesSystemSshTransport()).toBe(true) + expect(conn.getHostKeyFingerprint()).toBeUndefined() expect(onCredentialRequest).not.toHaveBeenCalled() }) diff --git a/src/main/ssh/ssh-connection.ts b/src/main/ssh/ssh-connection.ts index eb2f79cf13a..baba633d79d 100644 --- a/src/main/ssh/ssh-connection.ts +++ b/src/main/ssh/ssh-connection.ts @@ -1,5 +1,6 @@ /* eslint-disable max-lines -- Why: SSH connection lifecycle, credential retries, reconnect policy, and transport fallback are intentionally co-located so state transitions stay auditable in one file. */ import * as net from 'node:net' +import { createHash } from 'node:crypto' import { Client as SshClient } from 'ssh2' import type { ChildProcess } from 'node:child_process' import type { ClientChannel, ConnectConfig, SFTPWrapper } from 'ssh2' @@ -82,6 +83,7 @@ export class SshConnection { private disposed = false private cachedPassphrase: string | null = null private cachedPassword: string | null = null + private hostKeyFingerprint: string | undefined private connectGeneration = 0 constructor(target: SshTarget, callbacks: SshConnectionCallbacks) { @@ -121,6 +123,11 @@ export class SshConnection { getSystemSshResolvedConfig(): SshResolvedConfig | null { return cloneResolvedConfig(this.systemSshResolvedConfig) } + getHostKeyFingerprint(): string | undefined { + // Why: system SSH does not expose its negotiated key; a fingerprint from a + // failed ssh2 attempt may identify a different load-balanced execution host. + return this.useSystemSshTransport ? undefined : this.hostKeyFingerprint + } setCallbacks(callbacks: SshConnectionCallbacks): void { this.callbacks = callbacks @@ -1075,6 +1082,16 @@ export class SshConnection { const client = new SshClient() let settled = false + // Why: the relay uses the negotiated server key to isolate shared-home + // install locks without comparing PIDs from an unrelated SSH host. + config.hostVerifier = (key: Buffer): boolean => { + if (!this.disposed && connectGeneration === this.connectGeneration) { + const digest = createHash('sha256').update(key).digest('base64').replace(/=+$/, '') + this.hostKeyFingerprint = `SHA256:${digest}` + } + return true + } + const cleanupStartupListeners = (): void => { client.off('ready', onReady) client.off('error', onStartupError) diff --git a/src/main/ssh/ssh-relay-session.test.ts b/src/main/ssh/ssh-relay-session.test.ts index 55abd17b9c6..826911bf0a6 100644 --- a/src/main/ssh/ssh-relay-session.test.ts +++ b/src/main/ssh/ssh-relay-session.test.ts @@ -1,14 +1,14 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { SshRelaySession } from './ssh-relay-session' import type { SshConnection } from './ssh-connection' -import { AGENT_HOOK_INSTALL_PLUGINS_METHOD } from '../../shared/agent-hook-relay' +import { + AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD, + AGENT_HOOK_INSTALL_PLUGINS_METHOD +} from '../../shared/agent-hook-relay' import { SSH_RELAY_CONFIGURE_GRACE_TIME_METHOD } from '../../shared/ssh-types' import { createMockDeps, mockDeploySuccess } from './ssh-relay-session-test-fixtures' -const { muxRequestMock, installRemoteManagedAgentHooksMock } = vi.hoisted(() => ({ - muxRequestMock: vi.fn(), - installRemoteManagedAgentHooksMock: vi.fn() -})) +const { muxRequestMock } = vi.hoisted(() => ({ muxRequestMock: vi.fn() })) vi.mock('./ssh-relay-deploy', () => ({ deployAndLaunchRelay: vi.fn() @@ -32,10 +32,6 @@ vi.mock('./ssh-channel-multiplexer', () => { } }) -vi.mock('../agent-hooks/remote-managed-hook-installers', () => ({ - installRemoteManagedAgentHooks: installRemoteManagedAgentHooksMock -})) - vi.mock('../providers/ssh-pty-provider', () => ({ isSshPtyNotFoundError: (err: unknown) => (err instanceof Error ? err.message : String(err)).includes('not found'), @@ -115,8 +111,6 @@ describe('SshRelaySession', () => { delete process.env.ORCA_FEATURE_REMOTE_AGENT_HOOKS muxRequestMock.mockReset() muxRequestMock.mockResolvedValue([]) - installRemoteManagedAgentHooksMock.mockReset() - installRemoteManagedAgentHooksMock.mockResolvedValue([]) mockDeploySuccess() vi.mocked(getPtyIdsForConnection).mockReturnValue([]) _resetHiddenRendererPtyDeliveryGateForTest() @@ -233,99 +227,86 @@ describe('SshRelaySession', () => { expect(registerSshGitProvider).toHaveBeenCalledWith('target-1', expect.anything()) }) - it.each([ - ['/bin/bash', "'/bin/bash' -lc 'printenv GROK_HOME | head -c 4097'"], - ['/usr/bin/fish', "'/usr/bin/fish' -lc 'printenv GROK_HOME | head -c 4097'"], - ['/bin/tcsh', "'/bin/tcsh' -lc 'printenv GROK_HOME | head -c 4097'"], - ['/bin/sh', "'/bin/sh' -c 'printenv GROK_HOME | head -c 4097'"] - ])('uses a shell-independent GROK_HOME probe with login shell %s', async (shell, command) => { + it('installs all managed hooks in one relay RPC before plugins and PTY registration', async () => { process.env.ORCA_FEATURE_REMOTE_AGENT_HOOKS = '1' - muxRequestMock.mockImplementation(async (method: string) => - method === 'session.resolveHome' ? { resolvedPath: '/home/orca' } : { ok: true } - ) - vi.mocked(execCommand).mockResolvedValueOnce(`${shell}\n`).mockResolvedValueOnce('/srv/grok\n') - const sftp = { end: vi.fn() } - const { mockStore, mockPortForward, getMainWindow } = createMockDeps() - const mockConn = { sftp: vi.fn().mockResolvedValue(sftp) } as unknown as SshConnection - const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward) - - await session.establish(mockConn) - - expect(execCommand).toHaveBeenNthCalledWith( - 1, - mockConn, - "printenv SHELL || printf '/bin/sh\\n'", - { timeoutMs: 8_000 } - ) - expect(execCommand).toHaveBeenNthCalledWith(2, mockConn, command, { - wrapCommand: false, - timeoutMs: 8_000 - }) - expect(installRemoteManagedAgentHooksMock).toHaveBeenCalledWith(sftp, '/home/orca', { - grokHomeDir: '/srv/grok' - }) - }) - - it('installs remote managed hooks and relay-owned plugin assets before registering the SSH PTY provider', async () => { - process.env.ORCA_FEATURE_REMOTE_AGENT_HOOKS = '1' - muxRequestMock.mockImplementation(async (method: string) => { - if (method === 'session.resolveHome') { - return { resolvedPath: '/home/orca' } - } - return { ok: true } - }) - const sftp = { end: vi.fn() } - vi.mocked(execCommand) - .mockResolvedValueOnce('/bin/bash\n') - .mockResolvedValueOnce('/srv/grok profile\n') + muxRequestMock.mockResolvedValue({ installers: 14, errors: 0 }) const { mockStore, mockPortForward, getMainWindow } = createMockDeps() + const sftp = vi.fn() const mockConn = { - sftp: vi.fn().mockResolvedValue(sftp) + sftp, + getHostKeyFingerprint: vi.fn(() => 'SHA256:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA') } as unknown as SshConnection const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward) await session.establish(mockConn) + const managedHookCalls = muxRequestMock.mock.calls.filter( + ([method]) => method === AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD + ) + const managedHookCallIndex = muxRequestMock.mock.calls.findIndex( + ([method]) => method === AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD + ) const installPluginsCallIndex = muxRequestMock.mock.calls.findIndex( ([method]) => method === AGENT_HOOK_INSTALL_PLUGINS_METHOD ) + expect(managedHookCalls).toEqual([ + [ + AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD, + { hostKeyFingerprint: 'SHA256:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA' } + ] + ]) expect(installPluginsCallIndex).toBeGreaterThanOrEqual(0) const installPluginsParams = muxRequestMock.mock.calls[installPluginsCallIndex]?.[1] expect(installPluginsParams).toMatchObject({ piExtensionSource: expect.stringContaining('/hook/pi'), ompExtensionSource: expect.stringContaining('/hook/omp') }) - expect(mockConn.sftp).toHaveBeenCalledTimes(1) - expect(installRemoteManagedAgentHooksMock).toHaveBeenCalledWith(sftp, '/home/orca', { - grokHomeDir: '/srv/grok profile' - }) - expect(sftp.end).toHaveBeenCalledTimes(1) - expect(installRemoteManagedAgentHooksMock.mock.invocationCallOrder[0]).toBeLessThan( - muxRequestMock.mock.invocationCallOrder[installPluginsCallIndex] - ) + expect(sftp).not.toHaveBeenCalled() + expect(managedHookCallIndex).toBeLessThan(installPluginsCallIndex) expect(muxRequestMock.mock.invocationCallOrder[installPluginsCallIndex]).toBeLessThan( vi.mocked(registerSshPtyProvider).mock.invocationCallOrder[0] ) }) - it('falls back to login-home Grok config when the remote env probe is invalid', async () => { + it('continues provider registration when the relay managed-hook request fails', async () => { process.env.ORCA_FEATURE_REMOTE_AGENT_HOOKS = '1' - muxRequestMock.mockImplementation(async (method: string) => - method === 'session.resolveHome' ? { resolvedPath: '/home/orca' } : { ok: true } - ) - vi.mocked(execCommand) - .mockResolvedValueOnce('/bin/bash\n') - .mockResolvedValueOnce('relative/grok-home\n') - const sftp = { end: vi.fn() } + muxRequestMock.mockImplementation(async (method: string) => { + if (method === AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD) { + throw new Error('runtime unavailable') + } + return { ok: true } + }) const { mockStore, mockPortForward, getMainWindow } = createMockDeps() - const mockConn = { sftp: vi.fn().mockResolvedValue(sftp) } as unknown as SshConnection const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward) - await session.establish(mockConn) + await session.establish({} as SshConnection) + + expect(registerSshPtyProvider).toHaveBeenCalledWith('target-1', expect.anything()) + expect( + muxRequestMock.mock.calls.some(([method]) => method === AGENT_HOOK_INSTALL_PLUGINS_METHOD) + ).toBe(true) + }) - expect(installRemoteManagedAgentHooksMock).toHaveBeenCalledWith(sftp, '/home/orca', { - grokHomeDir: '/home/orca/.grok' + it('suppresses expected managed-hook teardown errors during disconnect', async () => { + process.env.ORCA_FEATURE_REMOTE_AGENT_HOOKS = '1' + muxRequestMock.mockImplementation(async (method: string) => { + if (method === AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD) { + throw Object.assign(new Error('request disposed'), { code: 'DISPOSED' }) + } + return { ok: true } }) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const { mockStore, mockPortForward, getMainWindow } = createMockDeps() + const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward) + + try { + await session.establish({} as SshConnection) + + expect(registerSshPtyProvider).toHaveBeenCalledWith('target-1', expect.anything()) + expect(warn.mock.calls.flat().join(' ')).not.toContain('relay managed hook install failed') + } finally { + warn.mockRestore() + } }) it('does not run POSIX managed hook installers on Windows remotes', async () => { @@ -351,7 +332,11 @@ describe('SshRelaySession', () => { await session.establish(mockConn) - expect(installRemoteManagedAgentHooksMock).not.toHaveBeenCalled() + expect( + muxRequestMock.mock.calls.some( + ([method]) => method === AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD + ) + ).toBe(false) expect( muxRequestMock.mock.calls.some(([method]) => method === AGENT_HOOK_INSTALL_PLUGINS_METHOD) ).toBe(true) diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index 0428a4d3086..146f2b2dd5b 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -16,9 +16,9 @@ import { toAppSshPtyId, toRelaySshPtyId } from '../providers/ssh-pty-id' import { SshFilesystemProvider } from '../providers/ssh-filesystem-provider' import { SshGitProvider } from '../providers/ssh-git-provider' import { agentHookServer } from '../agent-hooks/server' -import { installRemoteManagedAgentHooks } from '../agent-hooks/remote-managed-hook-installers' import { isAgentStatusHooksEnabled } from '../agent-hooks/managed-agent-hook-controls' import { + AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD, AGENT_HOOK_INSTALL_PLUGINS_METHOD, AGENT_HOOK_NOTIFICATION_METHOD, AGENT_HOOK_REQUEST_REPLAY_METHOD, @@ -68,7 +68,6 @@ import { runRemoteOrcaCli } from './ssh-remote-orca-cli' import { toSshExecutionHostId, type ExecutionHostId } from '../../shared/execution-host' import { isTerminalLeafId, makePaneKey } from '../../shared/stable-pane-id' import { isValidTerminalTabId } from '../../shared/terminal-tab-id' -import { shellEscape } from './ssh-connection-utils' export type RelaySessionState = 'idle' | 'deploying' | 'ready' | 'reconnecting' | 'disposed' @@ -84,67 +83,6 @@ type RemoteCliBridgeEnv = { type ExpectedPtyIdentity = { paneKey?: string; tabId?: string } -const REMOTE_GROK_HOME_MAX_LENGTH = 4096 -const REMOTE_GROK_HOME_PROBE_TIMEOUT_MS = 8_000 - -function defaultRemoteGrokHome(remoteHome: string): string { - const home = remoteHome.replace(/\/+$/, '') || remoteHome - return `${home}/.grok` -} - -function normalizeRemoteGrokHome(candidate: string): string | null { - if ( - candidate.length === 0 || - candidate.length > REMOTE_GROK_HOME_MAX_LENGTH || - candidate !== candidate.trim() || - !candidate.startsWith('/') || - candidate.includes('\\') || - hasControlCharacter(candidate) - ) { - return null - } - return candidate.replace(/\/+$/, '') || '/' -} - -function hasControlCharacter(value: string): boolean { - return Array.from(value).some((character) => { - const code = character.charCodeAt(0) - return code <= 0x1f || code === 0x7f - }) -} - -function loginShellCommand(shell: string, command: string): string { - const shellName = shell.split('/').at(-1) - const mode = shellName === 'sh' || shellName === 'dash' ? '-c' : '-lc' - return `${shellEscape(shell)} ${mode} ${shellEscape(command)}` -} - -async function resolveRemoteGrokHome( - connection: SshConnection, - remoteHome: string -): Promise { - const fallback = defaultRemoteGrokHome(remoteHome) - try { - // Why: remote PTYs start login shells, so probe that profile-derived env, not the relay service or local Electron env. - const shellOutput = await execCommand(connection, "printenv SHELL || printf '/bin/sh\\n'", { - timeoutMs: REMOTE_GROK_HOME_PROBE_TIMEOUT_MS - }) - const shell = shellOutput.trim().split(/\r?\n/, 1)[0] - if (!shell?.startsWith('/') || hasControlCharacter(shell)) { - return fallback - } - // Why: runs in the user's login shell (maybe fish/tcsh), so use external commands to avoid shell-specific syntax. - const probe = `printenv GROK_HOME | head -c ${REMOTE_GROK_HOME_MAX_LENGTH + 1}` - const output = await execCommand(connection, loginShellCommand(shell, probe), { - wrapCommand: false, - timeoutMs: REMOTE_GROK_HOME_PROBE_TIMEOUT_MS - }) - return normalizeRemoteGrokHome(output.split(/\r?\n/, 1)[0] ?? '') ?? fallback - } catch { - return fallback - } -} - function expectedIdentityForLease(lease: { tabId?: string leafId?: string @@ -646,7 +584,7 @@ export class SshRelaySession { }) } - // Why: hook-script agents need remote config files (relay env alone isn't enough); install before the PTY provider so first prompts report status. + // Why: hooks must exist before PTY spawn; relay-local work keeps all managed installs to one SSH round trip. private async installManagedHooksOnRemote(mux: SshChannelMultiplexer): Promise { if (!isRemoteAgentHooksEnabled() || !this.areAgentStatusHooksEnabled()) { return @@ -659,41 +597,34 @@ export class SshRelaySession { return } - let remoteHome: string try { - const result = (await mux.request('session.resolveHome', { path: '~' })) as { - resolvedPath?: unknown + const hostKeyFingerprint = this.requireReadyConnection().getHostKeyFingerprint?.() + const params = hostKeyFingerprint ? { hostKeyFingerprint } : {} + const result = (await mux.request(AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD, params)) as { + errors?: unknown } - if (typeof result.resolvedPath !== 'string' || result.resolvedPath.length === 0) { + if (typeof result.errors === 'number' && result.errors > 0) { console.warn( - `[ssh-relay-session] skipped remote managed hook install for ${this.targetId}: could not resolve remote home` + `[ssh-relay-session] ${result.errors} remote managed hook installers failed for ${this.targetId}` ) - return } - remoteHome = result.resolvedPath - } catch (error) { - console.warn( - `[ssh-relay-session] skipped remote managed hook install for ${this.targetId}: ${ - error instanceof Error ? error.message : String(error) - }` - ) - return - } - - let sftp: Awaited> | null = null - try { - const connection = this.requireReadyConnection() - const remoteGrokHome = await resolveRemoteGrokHome(connection, remoteHome) - sftp = await connection.sftp() - await installRemoteManagedAgentHooks(sftp, remoteHome, { grokHomeDir: remoteGrokHome }) } catch (error) { + // Why: teardown routinely cancels this best-effort request; only warn for + // installer failures that survive the connection lifecycle. + const code = (error as { code?: unknown })?.code + if ( + code === -32601 || + code === 'CONNECTION_LOST' || + code === 'DISPOSED' || + mux.isDisposed() + ) { + return + } console.warn( - `[ssh-relay-session] remote managed hook install failed for ${this.targetId}: ${ + `[ssh-relay-session] relay managed hook install failed for ${this.targetId}: ${ error instanceof Error ? error.message : String(error) }` ) - } finally { - ;(sftp as { end?: () => void } | null)?.end?.() } } diff --git a/src/main/ssh/ssh-relay-versioned-install.test.ts b/src/main/ssh/ssh-relay-versioned-install.test.ts index 9bd3f73c8df..23d57757b5b 100644 --- a/src/main/ssh/ssh-relay-versioned-install.test.ts +++ b/src/main/ssh/ssh-relay-versioned-install.test.ts @@ -128,12 +128,13 @@ describe('isRelayAlreadyInstalled', () => { ).rejects.toBe(sessionLimitError) }) - it('checks for both relay process artifacts and .install-complete', async () => { + it('checks every relay runtime artifact and .install-complete', async () => { mockExec.mockResolvedValueOnce('OK') await isRelayAlreadyInstalled(conn, '/r') const cmd = mockExec.mock.calls.at(-1)?.[1] ?? '' expect(cmd).toContain('relay.js') expect(cmd).toContain('relay-watcher.js') + expect(cmd).toContain('managed-hook-runtime.js') expect(cmd).toContain('.install-complete') }) }) diff --git a/src/main/ssh/ssh-relay-versioned-install.ts b/src/main/ssh/ssh-relay-versioned-install.ts index def8840ca57..9945c997ef2 100644 --- a/src/main/ssh/ssh-relay-versioned-install.ts +++ b/src/main/ssh/ssh-relay-versioned-install.ts @@ -104,9 +104,8 @@ export function computeRemoteRelayDir( } /** - * Probe whether a fully-installed relay exists at remoteRelayDir — meaning - * relay.js, its relay-watcher.js child, and the .install-complete sentinel are - * all present. Missing artifacts force a complete re-deploy. + * Probe for relay.js, its watcher, the managed-hook runtime, and the + * completion sentinel. Any missing artifact forces a complete re-deploy. */ export async function isRelayAlreadyInstalled( conn: SshConnection, diff --git a/src/main/ssh/ssh-remote-commands.test.ts b/src/main/ssh/ssh-remote-commands.test.ts index 96cc8925515..594ed2864d5 100644 --- a/src/main/ssh/ssh-remote-commands.test.ts +++ b/src/main/ssh/ssh-remote-commands.test.ts @@ -108,7 +108,9 @@ describe('ssh remote command builders', () => { it('keeps POSIX deploy commands POSIX-native', () => { expect(readRemoteHomeCommand(posix)).toBe('echo $HOME') expect(makeRemoteDirectoryCommand(posix, '/home/me/.orca-remote')).toContain('mkdir -p') - expect(probeRelayInstalledCommand(posix, '/home/me/relay')).toContain('test -d') + const probe = probeRelayInstalledCommand(posix, '/home/me/relay') + expect(probe).toContain('test -d') + expect(probe).toContain('managed-hook-runtime.js') }) it('uses encoded PowerShell for Windows deploy commands', () => { @@ -116,7 +118,9 @@ describe('ssh remote command builders', () => { expect(makeRemoteDirectoryCommand(windows, 'C:/Users/me/.orca-remote')).toContain( '-EncodedCommand' ) - expect(probeRelayInstalledCommand(windows, 'C:/Users/me/relay')).toContain('-EncodedCommand') + const probe = probeRelayInstalledCommand(windows, 'C:/Users/me/relay') + expect(probe).toContain('-EncodedCommand') + expect(decodePowerShellCommand(probe)).toContain('managed-hook-runtime.js') }) it('uses a legacy-visible Windows lock directory with an exclusive owner file', () => { diff --git a/src/main/ssh/ssh-remote-commands.ts b/src/main/ssh/ssh-remote-commands.ts index b70761f8d54..155fb5ba66f 100644 --- a/src/main/ssh/ssh-remote-commands.ts +++ b/src/main/ssh/ssh-remote-commands.ts @@ -78,12 +78,14 @@ export function probeRelayInstalledCommand( ): string { const relayJs = joinRemotePath(host, remoteRelayDir, 'relay.js') const relayWatcherJs = joinRemotePath(host, remoteRelayDir, 'relay-watcher.js') + const managedHookRuntimeJs = joinRemotePath(host, remoteRelayDir, 'managed-hook-runtime.js') const installComplete = joinRemotePath(host, remoteRelayDir, '.install-complete') if (!isWindowsRemoteHost(host)) { return ( `test -d ${shellEscape(remoteRelayDir)} ` + `&& test -f ${shellEscape(relayJs)} ` + `&& test -f ${shellEscape(relayWatcherJs)} ` + + `&& test -f ${shellEscape(managedHookRuntimeJs)} ` + `&& test -f ${shellEscape(installComplete)} ` + `&& echo OK || echo MISSING` ) @@ -93,8 +95,9 @@ export function probeRelayInstalledCommand( `$dir = ${powerShellLiteral(remoteRelayDir)}`, `$relay = ${powerShellLiteral(relayJs)}`, `$watcher = ${powerShellLiteral(relayWatcherJs)}`, + `$managedHooks = ${powerShellLiteral(managedHookRuntimeJs)}`, `$complete = ${powerShellLiteral(installComplete)}`, - "if ((Test-Path -LiteralPath $dir -PathType Container) -and (Test-Path -LiteralPath $relay -PathType Leaf) -and (Test-Path -LiteralPath $watcher -PathType Leaf) -and (Test-Path -LiteralPath $complete -PathType Leaf)) { 'OK' } else { 'MISSING' }" + "if ((Test-Path -LiteralPath $dir -PathType Container) -and (Test-Path -LiteralPath $relay -PathType Leaf) -and (Test-Path -LiteralPath $watcher -PathType Leaf) -and (Test-Path -LiteralPath $managedHooks -PathType Leaf) -and (Test-Path -LiteralPath $complete -PathType Leaf)) { 'OK' } else { 'MISSING' }" ].join('; ') ) } diff --git a/src/main/ssh/ssh-system-transport.integration.test.ts b/src/main/ssh/ssh-system-transport.integration.test.ts index b949dc411af..33e5fb3c80d 100644 --- a/src/main/ssh/ssh-system-transport.integration.test.ts +++ b/src/main/ssh/ssh-system-transport.integration.test.ts @@ -142,6 +142,7 @@ function createRelayTree(root: string, remoteHome: string): void { writeFileSync(join(remoteDir, 'node_modules', 'node-pty', 'index.js'), '') writeFileSync(join(remoteDir, 'node_modules', '@parcel', 'watcher', 'index.js'), '') writeFileSync(join(remoteDir, '.install-complete'), '') + writeFileSync(join(remoteDir, 'managed-hook-runtime.js'), '') writeFakeRelay(remoteDir) } diff --git a/src/relay/managed-hook-installer.test.ts b/src/relay/managed-hook-installer.test.ts new file mode 100644 index 00000000000..a060114ffcc --- /dev/null +++ b/src/relay/managed-hook-installer.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, it, vi } from 'vitest' +import { AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD } from '../shared/agent-hook-relay' +import type { MethodHandler, RequestContext } from './dispatcher' +import { registerManagedHookInstaller, type ManagedHookRuntime } from './managed-hook-installer' + +function captureHandler(loadRuntime: () => ManagedHookRuntime): MethodHandler { + let handler: MethodHandler | undefined + registerManagedHookInstaller( + { + onRequest: (method, nextHandler) => { + expect(method).toBe(AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD) + handler = nextHandler + } + }, + loadRuntime + ) + return handler! +} + +function context(signal?: AbortSignal): RequestContext { + return { clientId: 1, isStale: () => signal?.aborted ?? false, signal } +} + +describe('registerManagedHookInstaller', () => { + it('forwards request cancellation to the remote runtime', async () => { + const controller = new AbortController() + const installManagedHooks = vi.fn().mockResolvedValue({ installers: 14, errors: 0 }) + const handler = captureHandler(() => ({ installManagedHooks })) + + await expect(handler({}, context(controller.signal))).resolves.toEqual({ + installers: 14, + errors: 0 + }) + expect(installManagedHooks).toHaveBeenCalledWith({ signal: controller.signal }) + }) + + it('does not load or start the runtime for an already-cancelled request', async () => { + const controller = new AbortController() + controller.abort() + const loadRuntime = vi.fn() + const handler = captureHandler(loadRuntime) + + await expect(handler({}, context(controller.signal))).rejects.toMatchObject({ + name: 'AbortError' + }) + expect(loadRuntime).not.toHaveBeenCalled() + }) + + it('forwards only a valid negotiated server-key fingerprint', async () => { + const installManagedHooks = vi.fn().mockResolvedValue({ installers: 14, errors: 0 }) + const handler = captureHandler(() => ({ installManagedHooks })) + const fingerprint = 'SHA256:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA' + + await handler({ hostKeyFingerprint: fingerprint }, context()) + await handler({ hostKeyFingerprint: 'ssh://untrusted-host' }, context()) + + expect(installManagedHooks).toHaveBeenNthCalledWith(1, { + signal: undefined, + hostKeyFingerprint: fingerprint + }) + expect(installManagedHooks).toHaveBeenNthCalledWith(2, { signal: undefined }) + }) +}) diff --git a/src/relay/managed-hook-installer.ts b/src/relay/managed-hook-installer.ts new file mode 100644 index 00000000000..a27a6b8bf49 --- /dev/null +++ b/src/relay/managed-hook-installer.ts @@ -0,0 +1,56 @@ +import { join } from 'node:path' +import { + AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD, + type AgentHookInstallManagedHooksParams +} from '../shared/agent-hook-relay' +import type { RelayDispatcher, RequestContext } from './dispatcher' + +export type ManagedHookInstallSummary = { + installers: number + errors: number +} + +export type ManagedHookRuntime = { + installManagedHooks: (options?: { + signal?: AbortSignal + hostKeyFingerprint?: string + }) => Promise +} + +const SHA256_HOST_KEY_PATTERN = /^SHA256:[A-Za-z\d+/]{43}$/ + +function readHostKeyFingerprint(params: unknown): string | undefined { + const fingerprint = (params as Partial | null) + ?.hostKeyFingerprint + return typeof fingerprint === 'string' && SHA256_HOST_KEY_PATTERN.test(fingerprint) + ? fingerprint + : undefined +} + +let managedHookRuntime: ManagedHookRuntime | null = null + +function loadManagedHookRuntime(): ManagedHookRuntime { + if (!managedHookRuntime) { + // Why: keep the sizeable installer implementation out of relay startup and + // its narrow TS project while still executing it in-process on the remote. + managedHookRuntime = require(join(__dirname, 'managed-hook-runtime.js')) as ManagedHookRuntime + } + return managedHookRuntime +} + +export function registerManagedHookInstaller( + dispatcher: Pick, + loadRuntime: () => ManagedHookRuntime = loadManagedHookRuntime +): void { + dispatcher.onRequest( + AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD, + async (params, context: RequestContext): Promise => { + context.signal?.throwIfAborted() + const hostKeyFingerprint = readHostKeyFingerprint(params) + return await loadRuntime().installManagedHooks({ + signal: context.signal, + ...(hostKeyFingerprint ? { hostKeyFingerprint } : {}) + }) + } + ) +} diff --git a/src/relay/relay.ts b/src/relay/relay.ts index 2e3c77bb4b3..e279bec49c7 100644 --- a/src/relay/relay.ts +++ b/src/relay/relay.ts @@ -52,6 +52,7 @@ import { pickRemoteCliEnv } from './remote-cli-env' import { relayLogLine } from './relay-diagnostic-log' import { remoteCliRequestTimeoutMs } from './remote-cli-timeout' import { shouldReadRemoteCliStdin } from './remote-cli-stdin' +import { registerManagedHookInstaller } from './managed-hook-installer' const DEFAULT_GRACE_MS = DEFAULT_SSH_RELAY_GRACE_PERIOD_SECONDS * 1000 const SOCK_NAME = 'relay.sock' @@ -540,6 +541,9 @@ async function main(): Promise { return { replayed } }) + // Why: relay-local installers collapse hundreds of SFTP request/response RTTs to one RPC. + registerManagedHookInstaller(dispatcher) + // Why: plugin sources ship over the wire so an Orca update doesn't force a relay redeploy; cache them per spawn. See docs/design/agent-status-over-ssh.md §4. // Why: bound per-source size so a buggy/hostile Orca can't OOM the relay by pushing a giant string. dispatcher.onRequest(AGENT_HOOK_INSTALL_PLUGINS_METHOD, async (params) => { diff --git a/src/shared/agent-hook-relay.ts b/src/shared/agent-hook-relay.ts index 6469a002270..9cbd6c18faa 100644 --- a/src/shared/agent-hook-relay.ts +++ b/src/shared/agent-hook-relay.ts @@ -108,6 +108,15 @@ export const AGENT_HOOK_REQUEST_REPLAY_METHOD = 'agent_hook.requestReplay' as co * overlay dirs on the remote. */ export const AGENT_HOOK_INSTALL_PLUGINS_METHOD = 'agent_hook.installPlugins' as const +/** JSON-RPC request method that asks the remote relay to install every + * managed hook using its local filesystem instead of WAN-bound SFTP. */ +export const AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD = 'agent_hook.installManagedHooks' as const + +export type AgentHookInstallManagedHooksParams = { + /** SHA-256 fingerprint of the server key negotiated by Orca's SSH transport. */ + hostKeyFingerprint?: string +} + /** Feature-flag env var. Read once at process start by Orca and the relay. * Remote agent hooks ship as the default SSH behavior; set "0" to opt out. */ export const ORCA_FEATURE_REMOTE_AGENT_HOOKS_ENV = 'ORCA_FEATURE_REMOTE_AGENT_HOOKS' as const