fix(main): harden startup chain, updater/sftp timeouts, host-key guard, shell env scrub

src/main/globalShortcuts.ts

@@ -1,9 +1,33 @@
 import { BrowserWindow, globalShortcut } from 'electron'
 
+/**
+ * Chords the app owns outright: Ctrl+L is the panic lock, captured in the main
+ * window's before-input-event. The settings recorder (SettingsTabs.tsx,
+ * RESERVED_EXACT_ACCELERATORS) refuses to save it, but that guard only covers
+ * values entered after it existed — a shortcut persisted by an older build
+ * still arrives here, and a global registration intercepts the key at the OS
+ * level even while the window is focused, silently killing the lock shortcut.
+ * Normalized (modifier aliases + case folded) so every spelling is caught.
+ */
+const RESERVED_ACCELERATORS = new Set(['control+l', 'commandorcontrol+l'])
+
+function normalizeAccelerator(accelerator: string): string {
+  return accelerator
+    .split('+')
+    .map((part) => {
+      const p = part.trim().toLowerCase()
+      if (p === 'ctrl') return 'control'
+      if (p === 'cmdorctrl' || p === 'commandorctrl') return 'commandorcontrol'
+      return p
+    })
+    .join('+')
+}
+
 /**
  * Register the global show/hide toggle for the main window.
  *
  * - accelerator '' / undefined  => disabled (no global key bound).
+ * - A reserved chord (Ctrl+L) is skipped: see RESERVED_ACCELERATORS.
  * - Passing an invalid accelerator string makes Electron's register() throw;
  *   we swallow that here so a bad user-supplied value never crashes the app.
  * - register() returning false means the accelerator is already taken by
@@ -15,6 +39,13 @@ export function applyGlobalShortcut(accelerator: string | undefined): void {
   globalShortcut.unregisterAll()
   if (!accelerator) return
 
+  if (RESERVED_ACCELERATORS.has(normalizeAccelerator(accelerator))) {
+    console.warn(
+      `[global-shortcut] "${accelerator}" is reserved for the Ctrl+L lock shortcut; not registering`
+    )
+    return
+  }
+
   const handler = (): void => {
     const win = BrowserWindow.getAllWindows()[0]
     if (!win || win.isDestroyed()) return

src/main/index.ts

@@ -1,4 +1,4 @@
-import { app, BrowserWindow, globalShortcut, net, nativeImage, protocol, shell } from 'electron'
+import { app, BrowserWindow, dialog, globalShortcut, net, nativeImage, protocol, shell } from 'electron'
 import { existsSync } from 'fs'
 import { join } from 'path'
 import { pathToFileURL } from 'url'
@@ -12,7 +12,7 @@ import { isLockBlockedShortcut, isPanicLockChord } from './lockShortcuts'
 import { initTray, markQuitting, onMainWindowClose, refreshTrayMenu } from './tray'
 import { configureAutoUpdater, registerUpdateIpc } from './updater'
 import { applyWindowChrome } from './windowChrome'
-import { onLanguageChange } from '@shared/i18n'
+import { onLanguageChange, t } from '@shared/i18n'
 import { getThemeById } from '@shared/theme'
 
 /** Re-create the main window (tray restore path after all windows are gone). */
@@ -69,45 +69,63 @@ if (!gotSingleInstanceLock) {
 } else {
   app.on('second-instance', () => showOrCreate())
 
-  app.whenReady().then(() => {
+  app
-    // Serve the configured background image (path lives in settings; anything
+    .whenReady()
-    // else — including a path that is no longer configured — is refused, so the
+    .then(() => {
-    // protocol cannot be used to read arbitrary files).
+      // Serve the configured background image (path lives in settings; anything
-    protocol.handle('otimg', (request) => {
+      // else — including a path that is no longer configured — is refused, so the
-      const url = new URL(request.url)
+      // protocol cannot be used to read arbitrary files).
-      const requested = decodeURIComponent(url.pathname.replace(/^\//, ''))
+      protocol.handle('otimg', (request) => {
-      const allowed = loadSettings().terminal.backgroundImage
+        const url = new URL(request.url)
-      if (allowed === '' || requested !== allowed || !existsSync(requested)) {
+        const requested = decodeURIComponent(url.pathname.replace(/^\//, ''))
-        return new Response('', { status: 403 })
+        const allowed = loadSettings().terminal.backgroundImage
+        if (allowed === '' || requested !== allowed || !existsSync(requested)) {
+          return new Response('', { status: 403 })
+        }
+        return net.fetch(pathToFileURL(requested).toString())
+      })
+
+      registerIpc()
+      registerUpdateIpc()
+      // Before the window exists: a renderer-triggered check must not run against
+      // the updater's defaults (feed, proxy, autoDownload are set in here).
+      configureAutoUpdater()
+      // OS-level effects (login item, sleep blocker) must apply even if the
+      // settings dialog is never opened this run.
+      applyStartupSystemSettings(loadSettings())
+      // The lock state has to exist before the window loads, so a lockAtStartup
+      // lock is already in place when the renderer asks for it. It also starts the
+      // idle watcher, which is why it belongs after ready: powerMonitor cannot be
+      // touched before that. A restored startup lock engages before any publish,
+      // so the menu teardown is applied here from the flag itself.
+      const lock = initLockController()
+      applyMenuLockState(lock.isLocked())
+      createWindow()
+      initTray(showOrCreate)
+      // Tray labels are resolved from the dictionary at build time, so the menu has
+      // to be rebuilt whenever the interface language changes.
+      onLanguageChange(() => refreshTrayMenu())
+
+      app.on('activate', () => {
+        if (BrowserWindow.getAllWindows().length === 0) createWindow()
+      })
+    })
+    // A throw anywhere above (protocol, IPC, updater, lock controller, window,
+    // tray) used to leave the process running with the single-instance lock held
+    // and neither a window nor a tray to reach it — a zombie that refuses every
+    // later launch. Report and exit so a relaunch can retry from scratch.
+    .catch((err: unknown) => {
+      console.error('[main] startup failed:', err)
+      try {
+        dialog.showErrorBox(
+          'OpenTerminal',
+          t('main.startupFailed', { detail: err instanceof Error ? err.message : String(err) })
+        )
+      } catch {
+        // no display / dialog unavailable: the console line is all we have
       }
-      return net.fetch(pathToFileURL(requested).toString())
+      app.exit(1)
     })
-
-    registerIpc()
-    registerUpdateIpc()
-    // Before the window exists: a renderer-triggered check must not run against
-    // the updater's defaults (feed, proxy, autoDownload are set in here).
-    configureAutoUpdater()
-    // OS-level effects (login item, sleep blocker) must apply even if the
-    // settings dialog is never opened this run.
-    applyStartupSystemSettings(loadSettings())
-    // The lock state has to exist before the window loads, so a lockAtStartup
-    // lock is already in place when the renderer asks for it. It also starts the
-    // idle watcher, which is why it belongs after ready: powerMonitor cannot be
-    // touched before that. A restored startup lock engages before any publish,
-    // so the menu teardown is applied here from the flag itself.
-    const lock = initLockController()
-    applyMenuLockState(lock.isLocked())
-    createWindow()
-    initTray(showOrCreate)
-    // Tray labels are resolved from the dictionary at build time, so the menu has
-    // to be rebuilt whenever the interface language changes.
-    onLanguageChange(() => refreshTrayMenu())
-
-    app.on('activate', () => {
-      if (BrowserWindow.getAllWindows().length === 0) createWindow()
-    })
-  })
 }
 
 function createWindow(): void {
@@ -213,11 +231,17 @@ function createWindow(): void {
 
   // ELECTRON_RENDERER_URL is honored in dev builds only — a packaged build must
   // always load the bundled renderer file, no matter what the environment says.
+  // Both loads are fire-and-forget, so the rejection is surfaced explicitly
+  // instead of dying as a floating promise.
   const devUrl = devRendererUrl()
   if (devUrl) {
-    win.loadURL(devUrl)
+    void win.loadURL(devUrl).catch((err) => {
+      console.error('[main] loadURL failed:', err)
+    })
   } else {
-    win.loadFile(join(__dirname, '../renderer/index.html'))
+    void win.loadFile(join(__dirname, '../renderer/index.html')).catch((err) => {
+      console.error('[main] loadFile failed:', err)
+    })
   }
 }
 

src/main/pty.ts

@@ -189,6 +189,14 @@ export function createPty(opts: PtyCreateOptions = {}, owner?: number): PtyCreat
   // explicit FORCE_COLOR=0 form of the same request.
   delete env.NO_COLOR
   if (env.FORCE_COLOR === '0') delete env.FORCE_COLOR
+  // Electron's own plumbing must not leak into the user's shell either:
+  // ELECTRON_RUN_AS_NODE would turn this app's own exe into plain node, and
+  // NODE_OPTIONS would be applied to every node process started from here.
+  // OT_UPDATE_TOKEN is the updater credential — the updater is the only thing
+  // that has a reason to see it, not every shell the user opens.
+  delete env.ELECTRON_RUN_AS_NODE
+  delete env.NODE_OPTIONS
+  delete env.OT_UPDATE_TOKEN
 
   // Shell integration (opt-in): let the shell announce its own cwd over OSC 7 so
   // the remembered path is exact instead of inferred from typed `cd` commands.

src/main/sftp.ts

@@ -52,12 +52,18 @@ export function formatMode(mode: number): string {
 }
 
 /**
- * One SFTP channel PER SESSION, cached. Strict sshd builds (MaxSessions 2-3,
+ * One SFTP channel PER SESSION, cached for the session's whole lifetime. Strict
- * e.g. hardened cloud images) refuse extra channels and drop the whole
+ * sshd builds (MaxSessions 2-3, e.g. hardened cloud images) refuse extra
- * connection when every operation opens its own subsystem.
+ * channels and drop the whole connection when every operation opens its own
+ * subsystem. Only the session ending (pty.ts) or the channel itself erroring
+ * drops it: a finished transfer must NOT close it, or the next transfer pays
+ * for another subsystem.
  */
 const sftpCache = new Map<string, SFTPWrapper>()
 
+/** How long an SFTP subsystem open may take before the client counts as dead. */
+const SFTP_OPEN_TIMEOUT_MS = 10_000
+
 /**
  * Our own "session is gone" error. `isTransportError` classifies on the class,
  * not on the message text: the message is translated, the classifier must not
@@ -65,6 +71,13 @@ const sftpCache = new Map<string, SFTPWrapper>()
  */
 class SessionGoneError extends Error {}
 
+/**
+ * Our own "the client stopped answering" error: an SFTP subsystem open that
+ * outlived its budget. Transport-classified, so withSftp evicts the possibly
+ * half-dead channel and retries once on a fresh one.
+ */
+class SftpTimeoutError extends Error {}
+
 /**
  * Our own "operation failed for a non-transport reason" error (server-side
  * failure summary, user cancellation). isTransportError rejects these outright:
@@ -82,7 +95,23 @@ async function sftpOf(sessionId: string): Promise<SFTPWrapper> {
     throw new SessionGoneError(t('main.sftp.sessionGone'))
   }
   const sftp = await new Promise<SFTPWrapper>((resolve, reject) => {
-    client.sftp((err, sftp_) => (err != null ? reject(err) : resolve(sftp_)))
+    // A half-dead client can accept the subsystem request and then never call
+    // back; bound the wait (like execQuiet does) so withSftp can evict and retry.
+    const timer = setTimeout(
+      () => reject(new SftpTimeoutError(t('main.sftp.openTimeout'))),
+      SFTP_OPEN_TIMEOUT_MS
+    )
+    try {
+      client.sftp((err, sftp_) => {
+        clearTimeout(timer)
+        if (err != null) reject(err)
+        else resolve(sftp_)
+      })
+    } catch (err) {
+      // ssh2 throws synchronously when the socket is already gone
+      clearTimeout(timer)
+      reject(err)
+    }
   })
   // ssh2.d.ts declares only the used surface; the wrapper is an EventEmitter
   ;(sftp as SFTPWrapper & { on(event: 'error', cb: (err: Error) => void): void }).on('error', (err: Error) => {
@@ -109,14 +138,16 @@ function evictSftp(sessionId: string): void {
  * another subsystem channel, which strict sshd (MaxSessions 2) punishes by
  * dropping the whole connection. Only transport-level death retries.
  *
- * Classification is class-based for our own errors (SessionGoneError /
+ * Classification is class-based for our own errors (SessionGoneError and
- * OperationError) and matches only the ssh2 library's own message strings for
+ * SftpTimeoutError are transport-level, OperationError is not) and matches only
- * the rest: ssh2 is English-only and never translated, so the decision cannot
+ * the ssh2 library's own message strings for the rest: ssh2 is English-only and
- * drift with the UI locale (our translated messages arrive as OperationError
+ * never translated, so the decision cannot drift with the UI locale (our
- * and are rejected before any text is inspected).
+ * translated messages arrive as OperationError and are rejected before any text
+ * is inspected).
  */
 function isTransportError(err: unknown): boolean {
   if (err instanceof SessionGoneError) return true
+  if (err instanceof SftpTimeoutError) return true
   if (err instanceof OperationError) return false
   const msg = (err as Error | undefined)?.message ?? ''
   return /not connected|ECONNRESET|EPIPE|timed out|disconnected|channel|read past end|no response/i.test(msg)
@@ -399,8 +430,10 @@ export function uploadRemote(
           error: (err as Error).message
         })
       } finally {
+        // The cached channel belongs to the session, not to this transfer: only
+        // the session ending or the channel erroring drops it (see the cache note
+        // at the top of the file).
         activeTransfers.delete(id)
-        closeSftp(sessionId)
       }
     })()
 
@@ -472,8 +505,10 @@ export function downloadRemote(
           error: (err as Error).message
         })
       } finally {
+        // The cached channel belongs to the session, not to this transfer: only
+        // the session ending or the channel erroring drops it (see the cache note
+        // at the top of the file).
         activeTransfers.delete(id)
-        closeSftp(sessionId)
       }
     })()
 

src/main/ssh.ts

@@ -144,6 +144,14 @@ export async function connectSsh(
 
     promptUser(conn.host, conn.port, fingerprint, status, timeouts.prompt, deps)
       .then((accepted) => {
+        // The prompt can outlive the handshake: the connect timer is re-armed
+        // only after the decision, but a socket error meanwhile may have failed
+        // the attempt (and destroyed the client). Pinning the key then would
+        // record trust for a connection that never completed — drop the answer.
+        if (settled) {
+          console.warn('[ssh] host key decision arrived after the handshake failed; ignored')
+          return
+        }
         if (accepted) {
           try {
             deps.knownHosts.accept(conn.host, conn.port, hostKey, fingerprint)

src/main/updater.ts

@@ -24,6 +24,10 @@ const GITHUB_RELEASES_API =
 const GITHUB_PROBE_URL =
   'https://api.github.com/repos/billowliu2/OpenTerminal/releases/latest'
 const GITHUB_PROBE_TIMEOUT_MS = 20_000
+/** Overall budget for ONE check attempt — see withTimeout. */
+const CHECK_TIMEOUT_MS = 30_000
+/** Changelog / releases-API fetches: a stalled response must not hang the About tab. */
+const FETCH_TIMEOUT_MS = 15_000
 
 let state: UpdateState = { status: 'idle', currentVersion: app.getVersion() }
 let activeFeed: 'gitea' | 'github' = 'gitea'
@@ -75,13 +79,31 @@ function wireEvents(): void {
   )
 }
 
+/**
+ * Bound one updater call with an overall timeout. electron-updater's own socket
+ * idle timeout only fires on silence, so a slow trickling response can hold an
+ * attempt — and the "checking" state the UI shows — open indefinitely.
+ */
+function withTimeout<T>(promise: Promise<T>, ms: number): Promise<T> {
+  let timer: NodeJS.Timeout | undefined
+  const timeout = new Promise<never>((_, reject) => {
+    const handle = setTimeout(() => reject(new Error(t('main.updater.checkTimeout'))), ms)
+    handle.unref?.()
+    timer = handle
+  })
+  return Promise.race([promise, timeout]).finally(() => {
+    if (timer) clearTimeout(timer)
+  })
+}
+
 /**
  * Check updates: GitHub releases first (system proxy, gated by a
  * connectivity probe), domestic Gitea generic package as fallback.
  *
- * The feed is decided BEFORE touching the updater: checkForUpdates has no
+ * The feed is decided BEFORE touching the updater: a check abandoned on timeout
- * timeout, and racing it while mid-flight risks two concurrent checks
+ * cannot be cancelled, and electron-updater de-dupes concurrent checks by
- * polluting state.
+ * handing back the in-flight promise — so the fallback attempt below shares the
+ * abandoned check's fate instead of racing a second request.
  */
 async function checkWithFallback(): Promise<void> {
   // The feed choice is per attempt: a transient GitHub failure must not pin
@@ -91,7 +113,7 @@ async function checkWithFallback(): Promise<void> {
   if (await probeGithub()) {
     useFeed('github')
     try {
-      await autoUpdater.checkForUpdates()
+      await withTimeout(autoUpdater.checkForUpdates(), CHECK_TIMEOUT_MS)
       return
     } catch (err) {
       console.warn('[updater] github feed failed, falling back to gitea:', err)
@@ -102,7 +124,9 @@ async function checkWithFallback(): Promise<void> {
   }
   useFeed('gitea')
   try {
-    await autoUpdater.checkForUpdates()
+    // Bounded as well: a still-stuck GitHub check is handed back to us here, and
+    // it must not leave the state at "checking" forever.
+    await withTimeout(autoUpdater.checkForUpdates(), CHECK_TIMEOUT_MS)
   } catch (err) {
     if (githubErr === undefined) throw err
     const giteaErr = err instanceof Error ? err.message : String(err)
@@ -138,7 +162,12 @@ async function handleDownload(): Promise<void> {
 async function directFetch(url: string): Promise<Response> {
   const s = session.fromPartition('openterminal-update-direct', { cache: false })
   await s.setProxy({ mode: 'direct' })
-  return s.fetch(url, { headers: { 'User-Agent': 'OpenTerminal' } })
+  // Bounded: the changelog is on screen, so a stalled channel must fall through
+  // to the releases APIs instead of leaving the view spinning.
+  return s.fetch(url, {
+    headers: { 'User-Agent': 'OpenTerminal' },
+    signal: AbortSignal.timeout(FETCH_TIMEOUT_MS)
+  })
 }
 
 /**
@@ -184,7 +213,10 @@ async function fetchChangelog(): Promise<ReleaseNote[]> {
   }
   for (const url of [GITEA_RELEASES_API, GITHUB_RELEASES_API]) {
     try {
-      const resp = await net.fetch(url, { headers: { 'User-Agent': 'OpenTerminal' } })
+      const resp = await net.fetch(url, {
+        headers: { 'User-Agent': 'OpenTerminal' },
+        signal: AbortSignal.timeout(FETCH_TIMEOUT_MS)
+      })
       if (!resp.ok) continue
       const data = (await resp.json()) as Array<{
         tag_name?: string