fix: time out Claude keychain security calls (#3813)

src/main/claude-accounts/keychain.test.ts

@@ -45,6 +45,7 @@ describe('Claude Keychain credentials', () => {
   })
 
   afterEach(() => {
+    vi.useRealTimers()
     if (originalPlatform) {
       Object.defineProperty(process, 'platform', originalPlatform)
     }
@@ -175,6 +176,36 @@ describe('Claude Keychain credentials', () => {
     ])
   })
 
+  it('rejects when a keychain read never reports completion', async () => {
+    vi.useFakeTimers()
+    const configDir = '/tmp/orca-claude-login-test'
+    const killMock = vi.fn()
+    execFileMock.mockImplementationOnce(() => ({ kill: killMock }) as never)
+
+    let settled = false
+    let rejected: unknown
+    const readPromise = readActiveClaudeKeychainCredentialsStrict(configDir).then(
+      (credentials) => {
+        settled = true
+        return credentials
+      },
+      (error: unknown) => {
+        settled = true
+        rejected = error
+        return null
+      }
+    )
+
+    await vi.advanceTimersByTimeAsync(3000)
+
+    expect(settled).toBe(true)
+    await readPromise
+    expect(rejected).toEqual(
+      expect.objectContaining({ message: 'security timed out after 3000ms' })
+    )
+    expect(killMock).toHaveBeenCalled()
+  })
+
   it('deletes both scoped and legacy active credentials for config-dir cleanup', async () => {
     const configDir = '/tmp/orca-claude-login-test'
     const scopedService = serviceForConfigDir(configDir)

src/main/claude-accounts/keychain.ts

@@ -3,6 +3,12 @@ import { createHash } from 'node:crypto'
 
 const ACTIVE_CLAUDE_SERVICE = 'Claude Code-credentials'
 const ORCA_CLAUDE_SERVICE = 'Orca Claude Code Managed Credentials'
+const KEYCHAIN_COMMAND_TIMEOUT_MS = 3_000
+
+type SecurityCommandResult = {
+  stdout: string
+  stderr: string
+}
 
 export async function readActiveClaudeKeychainCredentials(
   configDir?: string
@@ -97,30 +103,25 @@ async function readKeychainPassword(service: string, account: string): Promise<s
   if (process.platform !== 'darwin') {
     return null
   }
-  return new Promise((resolve, reject) => {
-    execFile(
-      'security',
-      ['find-generic-password', '-s', service, '-a', account, '-w'],
-      { timeout: 3_000 },
-      (error, stdout, stderr) => {
-        if (!error && stdout.trim()) {
-          resolve(stdout.trim())
-          return
-        }
-        const message = `${stderr} ${error?.message ?? ''}`.toLowerCase()
-        const code = (error as { code?: unknown } | null)?.code
-        if (
-          code === 44 ||
-          message.includes('could not be found') ||
-          message.includes('not be found')
-        ) {
-          resolve(null)
-          return
-        }
-        reject(error ?? new Error(`Could not read macOS Keychain item ${service}/${account}.`))
-      }
-    )
-  })
+  try {
+    const { stdout } = await execSecurityCommand([
+      'find-generic-password',
+      '-s',
+      service,
+      '-a',
+      account,
+      '-w'
+    ])
+    if (stdout.trim()) {
+      return stdout.trim()
+    }
+    throw new Error(`Could not read macOS Keychain item ${service}/${account}.`)
+  } catch (error) {
+    if (isKeychainNotFoundError(error)) {
+      return null
+    }
+    throw error
+  }
 }
 
 async function writeKeychainPassword(
@@ -152,26 +153,81 @@ function execSecurity(
   args: string[],
   options?: { ignoreFailure?: boolean; ignoreNotFound?: boolean }
 ): Promise<void> {
-  return new Promise((resolve, reject) => {
-    execFile('security', args, { timeout: 3_000 }, (error, _stdout, stderr) => {
-      if (!error) {
-        resolve()
-        return
-      }
-      const code = (error as { code?: unknown }).code
-      const message = `${stderr} ${error.message}`.toLowerCase()
-      if (
-        options?.ignoreNotFound &&
-        (code === 44 || message.includes('could not be found') || message.includes('not be found'))
-      ) {
-        resolve()
-        return
-      }
-      if (!options?.ignoreFailure) {
-        reject(error)
-        return
-      }
-      resolve()
-    })
+  return execSecurityCommand(args).then(undefined, (error: unknown) => {
+    if (options?.ignoreNotFound && isKeychainNotFoundError(error)) {
+      return
+    }
+    if (!options?.ignoreFailure) {
+      throw error
+    }
+  })
+}
+
+function isKeychainNotFoundError(error: unknown): boolean {
+  const code =
+    error && typeof error === 'object' && 'code' in error
+      ? (error as { code?: unknown }).code
+      : undefined
+  const message =
+    error && typeof error === 'object'
+      ? `${(error as { stderr?: unknown }).stderr ?? ''} ${
+          (error as { message?: unknown }).message ?? ''
+        }`.toLowerCase()
+      : String(error).toLowerCase()
+  return code === 44 || message.includes('could not be found') || message.includes('not be found')
+}
+
+function execSecurityCommand(args: string[]): Promise<SecurityCommandResult> {
+  return new Promise((resolve, reject) => {
+    let settled = false
+    let child: ReturnType<typeof execFile> | undefined
+    const timer = setTimeout(() => {
+      if (settled) {
+        return
+      }
+      settled = true
+      child?.kill()
+      reject(
+        Object.assign(new Error(`security timed out after ${KEYCHAIN_COMMAND_TIMEOUT_MS}ms`), {
+          code: 'ETIMEDOUT',
+          stderr: ''
+        })
+      )
+    }, KEYCHAIN_COMMAND_TIMEOUT_MS)
+
+    const settle = (callback: () => void): void => {
+      if (settled) {
+        return
+      }
+      settled = true
+      clearTimeout(timer)
+      callback()
+    }
+
+    // Why: Node's execFile timeout only signals the `security` process; a
+    // stuck callback would otherwise leave auth/keychain operations pending.
+    try {
+      child = execFile(
+        'security',
+        args,
+        { timeout: KEYCHAIN_COMMAND_TIMEOUT_MS },
+        (error, stdout, stderr) => {
+          if (error) {
+            settle(() =>
+              reject(
+                Object.assign(error, {
+                  stdout: String(stdout),
+                  stderr: String(stderr)
+                })
+              )
+            )
+            return
+          }
+          settle(() => resolve({ stdout: String(stdout), stderr: String(stderr) }))
+        }
+      )
+    } catch (error) {
+      settle(() => reject(error))
+    }
   })
 }