Fix ssh watcher isolation (#8463)

* fix(ssh): isolate relay filesystem watchers * Fix relay watcher fault-harness pid file and in-process fallback isolati - Use exclusive ('wx') creation for the fault-harness pid file so a leaked ORCA_WATCHER_CHILD_PID_FILE env var can't clobber an existing file, and have the harness remove the file after reading a replacement pid. - Force useInProcessVitestFallback to false in the relay watcher pool so a leaked VITEST env var can never load the native watcher addon in-process on the relay; fail closed instead when the isolated child is missing. - Thread an injectable RelayWatcherProcessPool into FsHandler/ RelayFilesystemWatchRegistry for tests, and add coverage for both fixes.
2026-09-22 16:02:32 +00:00 · 2026-07-12 21:34:59 -07:00
parent a5faf19631
commit e3c47eff17
22 changed files with 1092 additions and 194 deletions
@@ -1,9 +1,9 @@
 #!/usr/bin/env node
 /**
- * Bundle the relay daemon into a single relay.js file per platform.
+ * Bundle the relay daemon and its crash-isolated watcher child per platform.
 *
- * The relay runs on remote hosts via `node relay.js`, so it must be a
- * self-contained CommonJS bundle with no external dependencies beyond
+ * The relay runs on remote hosts via `node relay.js`, so both outputs use
+ * self-contained CommonJS bundles with no external dependencies beyond
 * Node.js built-ins. Native addons (node-pty, @parcel/watcher) are
 * marked external and expected to be installed on the remote or
 * gracefully degraded.
@@ -17,6 +17,7 @@ const __dirname = import.meta.dirname
 // Why: the script lives under config/scripts, so go two levels up to reach the repo root.
 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 PLATFORMS = [
  'linux-x64',
@@ -42,7 +43,22 @@ for (const platform of PLATFORMS) {
    outfile: join(outDir, 'relay.js'),
    // Native addons cannot be bundled — they must exist on the remote host.
    // The relay gracefully degrades when they are absent.
-    external: ['node-pty', '@parcel/watcher'],
+    external: ['node-pty', '@parcel/watcher', 'electron'],
+    sourcemap: false,
+    minify: true,
+    define: {
+      'process.env.NODE_ENV': '"production"'
+    }
+  })
+
+  await build({
+    entryPoints: [WATCHER_ENTRY],
+    bundle: true,
+    platform: 'node',
+    target: 'node18',
+    format: 'cjs',
+    outfile: join(outDir, 'relay-watcher.js'),
+    external: ['@parcel/watcher'],
    sourcemap: false,
    minify: true,
    define: {
@@ -51,9 +67,15 @@ for (const platform of PLATFORMS) {
  })

  // Why: include a content hash so the deploy check detects code changes
-  // even when RELAY_VERSION hasn't been bumped (common during development).
+  // even when RELAY_VERSION hasn't been bumped. Hash both process artifacts
+  // so a watcher-only change always deploys beside the matching relay host.
  const relayContent = readFileSync(join(outDir, 'relay.js'))
-  const hash = createHash('sha256').update(relayContent).digest('hex').slice(0, 12)
+  const watcherContent = readFileSync(join(outDir, 'relay-watcher.js'))
+  const hash = createHash('sha256')
+    .update(relayContent)
+    .update(watcherContent)
+    .digest('hex')
+    .slice(0, 12)
  writeFileSync(join(outDir, '.version'), `${RELAY_VERSION}+${hash}`)

  console.log(`Built relay for ${platform} → ${outDir}/relay.js`)
@@ -122,6 +122,46 @@ describe('Electron runtime package contract', () => {
    )
  })

+  it('packages and release-gates the SSH relay watcher child', () => {
+    const relayBuild = readFileSync(join(projectDir, 'config/scripts/build-relay.mjs'), 'utf8')
+    const builderConfig = readFileSync(
+      join(projectDir, 'config/electron-builder.config.cjs'),
+      'utf8'
+    )
+    const remoteCommands = readFileSync(
+      join(projectDir, 'src/main/ssh/ssh-remote-commands.ts'),
+      'utf8'
+    )
+    const releaseWorkflow = parse(
+      readFileSync(join(projectDir, '.github/workflows/release-cut.yml'), 'utf8')
+    )
+    const macWorkflow = parse(
+      readFileSync(join(projectDir, '.github/workflows/release-mac-build.yml'), 'utf8')
+    )
+
+    expect(relayBuild).toContain("'parcel-watcher-process-entry.ts'")
+    expect(relayBuild).toContain("outfile: join(outDir, 'relay-watcher.js')")
+    expect(relayBuild).toContain("readFileSync(join(outDir, 'relay-watcher.js'))")
+    expect(builderConfig).toContain("from: 'out/relay'")
+    expect(remoteCommands).toContain("joinRemotePath(host, remoteRelayDir, 'relay-watcher.js')")
+
+    const assertRelayGate = (steps, publishStepName) => {
+      const names = steps.map((step) => step.name)
+      const gate = steps.find((step) => step.name === 'Gate SSH relay watcher process isolation')
+      expect(gate['continue-on-error']).toBeUndefined()
+      expect(gate.run).toContain('node config/scripts/relay-watcher-fault-harness.mjs')
+      expect(names.indexOf('Build app')).toBeLessThan(names.indexOf(gate.name))
+      expect(names.indexOf(gate.name)).toBeLessThan(names.indexOf(publishStepName))
+    }
+
+    assertRelayGate(releaseWorkflow.jobs.build.steps, 'Publish release artifacts (Linux)')
+    assertRelayGate(macWorkflow.jobs['build-mac'].steps, 'Publish release artifacts (macOS)')
+    const releaseNames = releaseWorkflow.jobs.build.steps.map((step) => step.name)
+    expect(releaseNames.indexOf('Gate SSH relay watcher process isolation')).toBeLessThan(
+      releaseNames.indexOf('Build Windows release artifacts')
+    )
+  })
+
  it('pins the Windows release builder to the VS 2022 runner image', () => {
    const releaseWorkflow = parse(
      readFileSync(join(projectDir, '.github/workflows/release-cut.yml'), 'utf8')
@@ -0,0 +1,286 @@
+import { build } from 'esbuild'
+import { spawn } from 'node:child_process'
+import { createRequire } from 'node:module'
+import { existsSync } from 'node:fs'
+import { mkdtemp, readFile, realpath, rm, writeFile } from 'node:fs/promises'
+import { tmpdir } from 'node:os'
+import { dirname, join, resolve } from 'node:path'
+
+const WAIT_TIMEOUT_MS = 20_000
+const require = createRequire(import.meta.url)
+
+function withTimeout(promise, label, stderr) {
+  return new Promise((resolvePromise, rejectPromise) => {
+    const timer = setTimeout(() => {
+      rejectPromise(new Error(`Timed out waiting for ${label}\n${stderr()}`))
+    }, WAIT_TIMEOUT_MS)
+    promise.then(
+      (value) => {
+        clearTimeout(timer)
+        resolvePromise(value)
+      },
+      (error) => {
+        clearTimeout(timer)
+        rejectPromise(error)
+      }
+    )
+  })
+}
+
+function pollUntil(readValue, label, stderr) {
+  const deadline = Date.now() + WAIT_TIMEOUT_MS
+  return new Promise((resolveValue, rejectValue) => {
+    const poll = async () => {
+      try {
+        const value = await readValue()
+        if (value !== undefined) {
+          resolveValue(value)
+          return
+        }
+      } catch {}
+      if (Date.now() >= deadline) {
+        rejectValue(new Error(`Timed out waiting for ${label}\n${stderr()}`))
+        return
+      }
+      setTimeout(poll, 10)
+    }
+    void poll()
+  })
+}
+
+function waitForExit(proc) {
+  if (proc.exitCode !== null || proc.signalCode !== null) {
+    return Promise.resolve()
+  }
+  return new Promise((resolveExit) => proc.once('exit', resolveExit))
+}
+
+async function loadProtocol(bundleDir) {
+  const outfile = join(bundleDir, 'relay-protocol.cjs')
+  await build({
+    entryPoints: [resolve('src/relay/protocol.ts')],
+    bundle: true,
+    platform: 'node',
+    format: 'cjs',
+    outfile,
+    logLevel: 'silent'
+  })
+  return require(outfile)
+}
+
+function createRelayClient(entryPath, args, env, protocol) {
+  const proc = spawn(process.execPath, [entryPath, ...args], {
+    cwd: dirname(entryPath),
+    env,
+    stdio: ['pipe', 'pipe', 'pipe']
+  })
+  const messages = []
+  let stderr = ''
+  let nextSequence = 1
+  let stdoutBuffer = Buffer.alloc(0)
+  let ready = false
+  let resolveReady
+  const sentinelReceived = new Promise((resolvePromise) => {
+    resolveReady = resolvePromise
+  })
+  const decoder = new protocol.FrameDecoder((frame) => {
+    if (frame.type === protocol.MessageType.Regular) {
+      messages.push(protocol.parseJsonRpcMessage(frame.payload))
+    }
+  })
+  proc.stderr.on('data', (chunk) => {
+    stderr = `${stderr}${String(chunk)}`.slice(-8_000)
+  })
+  proc.stdout.on('data', (chunk) => {
+    if (ready) {
+      decoder.feed(chunk)
+      return
+    }
+    stdoutBuffer = Buffer.concat([stdoutBuffer, chunk])
+    const sentinel = Buffer.from(protocol.RELAY_SENTINEL)
+    const index = stdoutBuffer.indexOf(sentinel)
+    if (index < 0) {
+      return
+    }
+    ready = true
+    resolveReady()
+    const remainder = stdoutBuffer.subarray(index + sentinel.length)
+    if (remainder.length > 0) {
+      decoder.feed(remainder)
+    }
+  })
+
+  const waitForMessage = (startIndex, predicate, label) =>
+    pollUntil(
+      () => messages.slice(startIndex).find(predicate),
+      label,
+      () => stderr
+    )
+
+  const request = async (method, params = {}) => {
+    const id = nextSequence++
+    const startIndex = messages.length
+    proc.stdin.write(protocol.encodeJsonRpcFrame({ jsonrpc: '2.0', id, method, params }, id, 0))
+    const response = await waitForMessage(
+      startIndex,
+      (message) => message.id === id,
+      `response to ${method}`
+    )
+    if (response.error) {
+      throw new Error(`${method} failed: ${response.error.message}`)
+    }
+    return response.result
+  }
+
+  const notify = (method, params = {}) => {
+    const sequence = nextSequence++
+    proc.stdin.write(protocol.encodeJsonRpcFrame({ jsonrpc: '2.0', method, params }, sequence, 0))
+  }
+
+  return {
+    proc,
+    request,
+    notify,
+    sentinelReceived: withTimeout(sentinelReceived, 'relay sentinel', () => stderr),
+    messageCount: () => messages.length,
+    waitForNotification: (startIndex, method, predicate = () => true) =>
+      waitForMessage(
+        startIndex,
+        (message) => message.method === method && predicate(message.params ?? {}),
+        `${method} notification`
+      ),
+    stderr: () => stderr
+  }
+}
+
+async function waitForWatcherPid(pidFile, previousPid, stderr) {
+  return pollUntil(
+    async () => {
+      const pid = Number((await readFile(pidFile, 'utf8')).trim())
+      if (!Number.isInteger(pid) || pid <= 0 || pid === previousPid) {
+        return undefined
+      }
+      // Why: replacement children reuse this exclusive path after fault injection.
+      await rm(pidFile, { force: true })
+      return pid
+    },
+    previousPid ? 'replacement watcher child pid' : 'initial watcher child pid',
+    stderr
+  )
+}
+
+function includesWatchPath(params, targetPath) {
+  return Array.isArray(params.events)
+    ? params.events.some((event) => event.absolutePath === targetPath)
+    : false
+}
+
+async function main() {
+  const platform = `${process.platform}-${process.arch}`
+  const relayEntry = resolve('out', 'relay', platform, 'relay.js')
+  const watcherEntry = resolve('out', 'relay', platform, 'relay-watcher.js')
+  if (!existsSync(relayEntry) || !existsSync(watcherEntry)) {
+    throw new Error(`Missing built relay artifacts for ${platform}; run pnpm run build:relay first`)
+  }
+
+  let tempRoot
+  let relay
+  try {
+    tempRoot = await mkdtemp(join(tmpdir(), 'orca-relay-watcher-fault-'))
+    const watchRoot = await realpath(tempRoot)
+    const pidFile = join(tempRoot, 'watcher.pid')
+    const protocol = await loadProtocol(tempRoot)
+    const socketPath =
+      process.platform === 'win32'
+        ? `\\\\.\\pipe\\orca-relay-watcher-fault-${process.pid}-${Date.now()}`
+        : join(tempRoot, 'relay.sock')
+    relay = createRelayClient(
+      relayEntry,
+      ['--sock-path', socketPath, '--endpoint-dir', join(tempRoot, 'agent-hooks')],
+      { ...process.env, ORCA_WATCHER_CHILD_PID_FILE: pidFile },
+      protocol
+    )
+    await relay.sentinelReceived
+
+    const spawned = await relay.request('pty.spawn', { cols: 80, rows: 24, cwd: watchRoot })
+    const beforePtyMarker = `ORCA_PTY_BEFORE_${Date.now()}`
+    let startIndex = relay.messageCount()
+    relay.notify('pty.data', { id: spawned.id, data: `echo ${beforePtyMarker}\r` })
+    await relay.waitForNotification(
+      startIndex,
+      'pty.data',
+      (params) => params.id === spawned.id && String(params.data).includes(beforePtyMarker)
+    )
+
+    await relay.request('fs.watch', { rootPath: watchRoot })
+    const firstWatcherPid = await waitForWatcherPid(pidFile, undefined, relay.stderr)
+    const beforePath = join(watchRoot, 'before.txt')
+    startIndex = relay.messageCount()
+    await writeFile(beforePath, 'before')
+    await relay.waitForNotification(startIndex, 'fs.changed', (params) =>
+      includesWatchPath(params, beforePath)
+    )
+
+    const faultSignal = process.platform === 'win32' ? 'SIGTERM' : 'SIGSEGV'
+    startIndex = relay.messageCount()
+    process.kill(firstWatcherPid, faultSignal)
+    const replacementWatcherPid = await waitForWatcherPid(pidFile, firstWatcherPid, relay.stderr)
+    await relay.waitForNotification(startIndex, 'fs.changed', (params) =>
+      Array.isArray(params.events)
+        ? params.events.some(
+            (event) => event.kind === 'overflow' && event.absolutePath === watchRoot
+          )
+        : false
+    )
+
+    const status = await relay.request('relay.status')
+    if (status.pid !== relay.proc.pid) {
+      throw new Error('relay.status did not come from the original surviving relay process')
+    }
+    const afterPtyMarker = `ORCA_PTY_AFTER_${Date.now()}`
+    startIndex = relay.messageCount()
+    relay.notify('pty.data', { id: spawned.id, data: `echo ${afterPtyMarker}\r` })
+    await relay.waitForNotification(
+      startIndex,
+      'pty.data',
+      (params) => params.id === spawned.id && String(params.data).includes(afterPtyMarker)
+    )
+
+    const afterPath = join(watchRoot, 'after.txt')
+    startIndex = relay.messageCount()
+    await writeFile(afterPath, 'after')
+    await relay.waitForNotification(startIndex, 'fs.changed', (params) =>
+      includesWatchPath(params, afterPath)
+    )
+
+    relay.notify('fs.unwatch', { rootPath: watchRoot })
+    await relay.request('pty.shutdown', { id: spawned.id })
+    console.log(
+      JSON.stringify({
+        relayPid: relay.proc.pid,
+        killedWatcherPid: firstWatcherPid,
+        replacementWatcherPid,
+        faultSignal,
+        relaySurvived: true,
+        existingPtySurvived: true,
+        overflowRefreshDelivered: true,
+        postCrashEventDelivered: true
+      })
+    )
+  } finally {
+    if (relay && relay.proc.exitCode === null && relay.proc.signalCode === null) {
+      relay.proc.kill('SIGTERM')
+      try {
+        await withTimeout(waitForExit(relay.proc), 'relay shutdown', relay.stderr)
+      } catch {
+        relay.proc.kill('SIGKILL')
+        await withTimeout(waitForExit(relay.proc), 'forced relay shutdown', relay.stderr)
+      }
+    }
+    if (tempRoot) {
+      await rm(tempRoot, { recursive: true, force: true })
+    }
+  }
+}
+
+await main()