test(main): cover updater fallback, ipc sender guard, log sanitizer, sftp timeouts

- updater-fallback.mjs (82 assertions): GitHub probe fallback to Gitea,
  timeout budgets, in-flight check never stuck in 'checking'; updater.ts
  gains a setUpdateTimeouts test seam, electron-updater aliased to a stub
- ipc-guard.mjs (43): trusted-frame guard exercised through real
  registerIpc handlers with forged senderFrames; electron-stub now records
  registrations via globalThis so bundle and test share one instance
- log-sanitizer.mjs (71): CSI/OSC/charset state machine, alt-screen fold,
  byte-split fuzz equal to whole-chunk output; fixes a wrong comment
- sftp-timeout.mjs (39): per-op timeouts (metadata 30s, transfer chunk 60s,
  open 10s) evict half-dead channels with one retry, slow-but-progressing
  transfers untouched, late rejections never unhandled
- reservedAccelerators.ts: single pure isReservedAccelerator shared by the
  settings recorder and applyGlobalShortcut (the two tables had drifted —
  main now also refuses Ctrl+=/-/0/PgUp/PgDn legacy values); 56 assertions
- ssh-loopback.mjs loads the real ssh.ts via a bundle (50 assertions):
  TOFU pinning, fail-closed stores, auth gate, connect budget

Offline suite grows 13 -> 17. Renderer test framework evaluated: not
introducing vitest/jsdom; pure logic keeps being extracted and tested
through the existing bundle harness.
This commit is contained in:
Bill committed 2026-10-07 22:57:16 +08:00
1 parent 5ff311ea0f
commit a0be827650
21 files changed
+2370 -174

No files matched your search

+6 -26
View File
@@ -1,33 +1,13 @@
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('+')
}
import { isReservedAccelerator } from '@shared/reservedAccelerators'
/**
* 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.
* - A reserved chord (Ctrl+L, Ctrl+=/-/0/PgUp/PgDn) is skipped: see
* @shared/reservedAccelerators for why and for the shared table the settings
* recorder uses too.
* - 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
@@ -39,9 +19,9 @@ export function applyGlobalShortcut(accelerator: string | undefined): void {
globalShortcut.unregisterAll()
if (!accelerator) return
if (RESERVED_ACCELERATORS.has(normalizeAccelerator(accelerator))) {
if (isReservedAccelerator(accelerator)) {
console.warn(
`[global-shortcut] "${accelerator}" is reserved for the Ctrl+L lock shortcut; not registering`
`[global-shortcut] "${accelerator}" is reserved for an in-app shortcut; not registering`
)
return
}
+3 -1
View File
@@ -66,7 +66,9 @@ export class LogSanitizer {
} else if (c === '\n') {
out += this.emitLine()
} else if (c === '\t' || c >= ' ') {
// printable + tab; DEL and C0 controls (bell etc.) are dropped
// printable + tab; C0 controls (bell, backspace, …) are dropped.
// Note DEL (0x7F) is not a C0 control and passes this test, so it
// is kept as an ordinary character.
if (this.alt) this.altDirty = true
else this.line += c
}
+87 -18
View File
@@ -33,18 +33,87 @@ export function registerSftpClientProvider(provider: (id: string) => Client | un
}
function p<T>(fn: (cb: (err: Error | null, res: T) => void) => void): Promise<T> {
return bounded(rawP(fn), timeouts.op)
}
/** For ssh2 calls whose callback only yields an error. */
function pVoid(fn: (cb: (err: Error | null) => void) => void): Promise<void> {
return bounded(rawVoid(fn), timeouts.op)
}
/**
* Transfer-chunk variants: a 256KB read/write on a slow link is legitimately
* seconds, so these run on the wider `transfer` budget instead of the metadata
* one. Same class of failure, different tolerance.
*/
function pTransfer<T>(fn: (cb: (err: Error | null, res: T) => void) => void): Promise<T> {
return bounded(rawP(fn), timeouts.transfer)
}
function pVoidTransfer(fn: (cb: (err: Error | null) => void) => void): Promise<void> {
return bounded(rawVoid(fn), timeouts.transfer)
}
/**
* Operation budgets.
*
* Two classes, because one number cannot serve both: metadata round trips are
* milliseconds on any working link, while a 256KB transfer chunk on a slow link
* is legitimately seconds (and the upload path additionally waits on a peer
* that ACKs lazily — see the transfer section). The metadata budget is the
* "channel is dead" detector; the transfer budget is a backstop for the same
* failure at chunk granularity, set wide enough that it cannot fire on a merely
* slow transfer.
*/
const timeouts = { op: 30_000, transfer: 60_000, open: 10_000 }
/**
* Test seam: shrink the budgets so the offline harness does not have to wait
* them out. Mirrors setLocalPathPolicy — injected, never read from renderer
* input.
*/
export function setSftpTimeouts(patch: { op?: number; transfer?: number; open?: number }): void {
if (patch.op !== undefined) timeouts.op = patch.op
if (patch.transfer !== undefined) timeouts.transfer = patch.transfer
if (patch.open !== undefined) timeouts.open = patch.open
}
/** Unbounded primitive behind `p` / `pVoid`. */
function rawP<T>(fn: (cb: (err: Error | null, res: T) => void) => void): Promise<T> {
return new Promise((resolve, reject) => {
fn((err, res) => (err ? reject(err) : resolve(res)))
})
}
/** For ssh2 calls whose callback only yields an error. */
function pVoid(fn: (cb: (err: Error | null) => void) => void): Promise<void> {
function rawVoid(fn: (cb: (err: Error | null) => void) => void): Promise<void> {
return new Promise((resolve, reject) => {
fn(err => (err ? reject(err) : resolve()))
})
}
/**
* Reject `promise` once `ms` elapses. ssh2's SFTP callbacks are raw socket
* completions: a channel that is half-dead (peer gone, no FIN ever delivered,
* the case mobile / NAT'd links produce) accepts the request and then never
* calls back — the operation, and the UI spinner behind it, used to wait
* forever. The losing side of the race is left pending on purpose: it is only
* dropped, never cancelled, so a late reply cannot resurrect the operation.
*/
function bounded<T>(promise: Promise<T>, ms: number): Promise<T> {
// The race may already have been decided by the time this one rejects; an
// unhandled rejection would take the whole process down.
promise.catch(() => undefined)
let timer: NodeJS.Timeout | undefined
const expiry = new Promise<never>((_, reject) => {
timer = setTimeout(() => {
reject(new SftpTimeoutError(t('main.sftp.opTimeout', { seconds: Math.round(ms / 1000) })))
}, ms)
})
return Promise.race([promise, expiry]).finally(() => {
if (timer) clearTimeout(timer)
})
}
type StatLike = { isDirectory(): boolean; size: number; mtime: number; mode: number; uid: number; gid: number }
function lstat(sftp: SFTPWrapper, path: string): Promise<StatLike> {
@@ -79,9 +148,6 @@ export function formatMode(mode: number): string {
*/
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
@@ -117,7 +183,7 @@ async function sftpOf(sessionId: string): Promise<SFTPWrapper> {
// 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
timeouts.open
)
try {
client.sftp((err, sftp_) => {
@@ -189,13 +255,16 @@ export function closeSftp(sessionId: string): void {
const FAKE_FS = new Set(['tmpfs', 'overlay', 'udev', 'devtmpfs', 'none', 'squashfs', 'shm'])
/** readdir, both shapes ssh2 can yield (plain names or `{filename}` objects). */
function readdir(sftp: SFTPWrapper, dir: string): Promise<string[]> {
return p<Array<string | { filename: string }>>(cb => sftp.readdir(dir, cb)).then(raw =>
raw.map(n => (typeof n === 'string' ? n : n.filename))
)
}
export function listRemote(sessionId: string, dir: string): Promise<SftpEntry[]> {
return withSftp(sessionId, async sftp => {
const raw = await new Promise<Array<string | { filename: string }>>((resolve, reject) => {
sftp.readdir(dir, (err, names) => (err ? reject(err) : resolve(names)))
})
// ssh2 readdir yields plain strings OR {filename} objects depending on version/options
const names = raw.map(n => (typeof n === 'string' ? n : n.filename))
const names = await readdir(sftp, dir)
const entries: SftpEntry[] = []
const queue = [...names]
const base = dir.replace(/\/+$/, '') || '/'
@@ -240,10 +309,7 @@ async function deleteRecursive(sftp: SFTPWrapper, path: string, isDir: boolean):
await pVoid(cb => sftp.unlink(path, cb))
return
}
const raw = await new Promise<Array<string | { filename: string }>>((resolve, reject) => {
sftp.readdir(path, (err, names) => (err ? reject(err) : resolve(names)))
})
const names = raw.map(n => (typeof n === 'string' ? n : n.filename))
const names = await readdir(sftp, path)
const base = path.replace(/\/+$/, '') || '/'
for (const name of names) {
const child = `${base}/${name}`
@@ -472,7 +538,7 @@ export function uploadRemote(
lastEmit = now
emit({ transferId: id, kind: 'upload', state: 'running', file: name, bytes: pos, totalBytes: size })
}
const pr = pVoid(cb => sftp.write(handle, buf, 0, bytesRead, offset, cb))
const pr = pVoidTransfer(cb => sftp.write(handle, buf, 0, bytesRead, offset, cb))
inflight.add(
pr.catch((err: Error) => {
firstError = firstError ?? err
@@ -485,7 +551,10 @@ export function uploadRemote(
await localHandle.close()
}
await Promise.race([
pVoid(cb => sftp.close(handle, cb)),
// The stall guard owns this wait (10s), so the close itself stays
// unbounded-by-`bounded` — otherwise a timeout landing after the
// race is decided would reject with nothing listening.
rawVoid(cb => sftp.close(handle, cb)),
new Promise<void>((r) => setTimeout(r, 10_000))
])
await Promise.allSettled(inflight)
@@ -547,7 +616,7 @@ export function downloadRemote(
const buf = Buffer.alloc(CHUNK)
for (;;) {
if (transfer.cancelled) throw new OperationError(t('main.sftp.cancelled'))
const { bytesRead } = await p<{ bytesRead: number; buffer: Buffer }>(cb =>
const { bytesRead } = await pTransfer<{ bytesRead: number; buffer: Buffer }>(cb =>
sftp.read(handle, buf, 0, CHUNK, pos, cb)
)
if (bytesRead === 0) break
+29 -10
View File
@@ -23,11 +23,30 @@ const GITHUB_RELEASES_API =
'https://api.github.com/repos/billowliu2/OpenTerminal/releases?per_page=10'
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
/**
* Time budgets, kept in one mutable object so the offline harness
* (tests/updater-fallback.mjs) can drive the timeout and fallback paths without
* waiting out the production values. Nothing in the app calls
* `setUpdateTimeouts`; the numbers below are the shipping ones.
*
* - `probe`: GitHub connectivity probe. Short enough that a proxy-less user
* falls back to Gitea instead of hanging on a dead proxy/DNS.
* - `check`: overall budget for ONE check attempt — see withTimeout, which
* exists because electron-updater's own socket idle timeout only fires on
* silence, so a slow trickling response can hold an attempt open forever.
* - `fetch`: changelog / releases-API fetches; a stalled response must not
* hang the About tab.
*/
const budgets = {
probe: 20_000,
check: 30_000,
fetch: 15_000
}
export function setUpdateTimeouts(patch: Partial<typeof budgets>): void {
Object.assign(budgets, patch)
}
let state: UpdateState = { status: 'idle', currentVersion: app.getVersion() }
let activeFeed: 'gitea' | 'github' = 'gitea'
@@ -113,7 +132,7 @@ async function checkWithFallback(): Promise<void> {
if (await probeGithub()) {
useFeed('github')
try {
await withTimeout(autoUpdater.checkForUpdates(), CHECK_TIMEOUT_MS)
await withTimeout(autoUpdater.checkForUpdates(), budgets.check)
return
} catch (err) {
console.warn('[updater] github feed failed, falling back to gitea:', err)
@@ -126,7 +145,7 @@ async function checkWithFallback(): Promise<void> {
try {
// 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)
await withTimeout(autoUpdater.checkForUpdates(), budgets.check)
} catch (err) {
if (githubErr === undefined) throw err
const giteaErr = err instanceof Error ? err.message : String(err)
@@ -166,7 +185,7 @@ async function directFetch(url: string): Promise<Response> {
// to the releases APIs instead of leaving the view spinning.
return s.fetch(url, {
headers: { 'User-Agent': 'OpenTerminal' },
signal: AbortSignal.timeout(FETCH_TIMEOUT_MS)
signal: AbortSignal.timeout(budgets.fetch)
})
}
@@ -181,7 +200,7 @@ async function probeGithub(): Promise<boolean> {
await s.setProxy({ mode: 'system' })
const resp = await s.fetch(GITHUB_PROBE_URL, {
headers: { 'User-Agent': 'OpenTerminal' },
signal: AbortSignal.timeout(GITHUB_PROBE_TIMEOUT_MS)
signal: AbortSignal.timeout(budgets.probe)
})
return resp.ok
} catch {
@@ -215,7 +234,7 @@ async function fetchChangelog(): Promise<ReleaseNote[]> {
try {
const resp = await net.fetch(url, {
headers: { 'User-Agent': 'OpenTerminal' },
signal: AbortSignal.timeout(FETCH_TIMEOUT_MS)
signal: AbortSignal.timeout(budgets.fetch)
})
if (!resp.ok) continue
const data = (await resp.json()) as Array<{
+5 -21
View File
@@ -3,6 +3,7 @@ import { useMemo, useState } from 'react'
import { Input, InputNumber, Radio, Select, Switch } from 'antd'
import type { ThemeColors } from '@shared/theme'
import { DEFAULT_LANGUAGE, LANGUAGES, t, type Language } from '@shared/i18n'
import { isReservedAccelerator } from '@shared/reservedAccelerators'
import { useSettingsStore, useResolvedTheme } from './store'
import {
DEFAULT_FONT_STACK,
@@ -482,28 +483,11 @@ function acceleratorFromEvent(e: React.KeyboardEvent<HTMLInputElement>): string
}
/**
* Keys this app binds while Control is held: font size (Ctrl+=/-/0, main.tsx)
* and tab cycling (Ctrl+PgUp/PgDn, Workspace). Neither handler looks at the
* other modifiers, so any Control combo on one of these keys would shadow the
* in-app action — the recorder refuses it instead of saving a shortcut that
* silently loses its original meaning.
* The reserved-accelerator table (in-app font/tab chords and the Ctrl+L panic
* lock) lives in @shared/reservedAccelerators so this recorder and the main
* process's registration guard (`applyGlobalShortcut`) cannot drift apart —
* they are the two halves of one rule. See that module for the rationale.
*/
const RESERVED_CONTROL_KEYS = new Set(['=', '-', '0', 'PageUp', 'PageDown'])
/**
* Whole chords the app owns outright, matched exactly (modifier set included).
* Ctrl+L is the panic lock, captured in main's before-input-event: a global
* registration intercepts the key at the OS level even while this window is
* focused, so binding it here would silently disable the lock shortcut.
*/
const RESERVED_EXACT_ACCELERATORS = new Set(['Control+L'])
/** true when `accel` (e.g. "Control+Shift+=") collides with an in-app shortcut */
function isReservedAccelerator(accel: string): boolean {
if (RESERVED_EXACT_ACCELERATORS.has(accel)) return true
const parts = accel.split('+')
return parts.includes('Control') && RESERVED_CONTROL_KEYS.has(parts[parts.length - 1])
}
/** t() falls back to the key itself when a translation is missing. */
function tOr(key: string, fallback: string): string {
+1
View File
@@ -35,6 +35,7 @@ const main: Record<string, string> = {
'main.sftp.invalidUidGid': 'Invalid uid/gid',
'main.sftp.commandTimeout': 'Command timed out',
'main.sftp.openTimeout': 'Opening the SFTP channel timed out',
'main.sftp.opTimeout': 'The SFTP operation stopped responding ({seconds}s); the connection is probably dead — reconnect and try again',
'main.sftp.commandExitCode': 'Command exited with code {code}',
'main.sftp.cancelled': 'Cancelled',
'main.sftp.localNotAllowed': 'Local path not authorised: {path}. Pick it again with the "choose file / choose directory" dialog.',
+1
View File
@@ -35,6 +35,7 @@ const main: Record<string, string> = {
'main.sftp.invalidUidGid': '不正な uid/gid',
'main.sftp.commandTimeout': 'コマンドがタイムアウトしました',
'main.sftp.openTimeout': 'SFTP チャネルのオープンがタイムアウトしました',
'main.sftp.opTimeout': 'SFTP 操作が {seconds} 秒応答しません。接続が切断された可能性があります。再接続して再試行してください',
'main.sftp.commandExitCode': 'コマンドの終了コード {code}',
'main.sftp.cancelled': 'キャンセルしました',
'main.sftp.localNotAllowed': 'ローカルパスが許可されていません: {path}。「ファイルを選択 / ディレクトリを選択」ダイアログで選び直してください。',
+1
View File
@@ -35,6 +35,7 @@ const main: Record<string, string> = {
'main.sftp.invalidUidGid': '非法 uid/gid',
'main.sftp.commandTimeout': '命令执行超时',
'main.sftp.openTimeout': 'SFTP 通道打开超时',
'main.sftp.opTimeout': 'SFTP 操作无响应({seconds} 秒),连接可能已中断,请重新连接会话后重试',
'main.sftp.commandExitCode': '命令退出码 {code}',
'main.sftp.cancelled': '已取消',
'main.sftp.localNotAllowed': '本地路径未被授权: {path}。请通过「选择文件 / 选择目录」对话框重新选择。',
+1
View File
@@ -35,6 +35,7 @@ const main: Record<string, string> = {
'main.sftp.invalidUidGid': 'uid/gid 無效',
'main.sftp.commandTimeout': '命令執行逾時',
'main.sftp.openTimeout': 'SFTP 通道開啟逾時',
'main.sftp.opTimeout': 'SFTP 操作無回應({seconds} 秒),連線可能已中斷,請重新連線後再試',
'main.sftp.commandExitCode': '指令結束碼 {code}',
'main.sftp.cancelled': '已取消',
'main.sftp.localNotAllowed': '本機路徑未獲授權: {path}。請改用「選擇檔案 / 選擇目錄」對話框重新選擇。',
+61
View File
@@ -0,0 +1,61 @@
/**
* Accelerators the app binds itself, shared by the two places that must agree
* on them:
*
* - the settings recorder (`SettingsTabs.tsx`) refuses to SAVE such a chord, so
* a global registration never shadows the in-app action;
* - `applyGlobalShortcut` (main) refuses to REGISTER one. The recorder only
* guards values entered after it existed — a shortcut persisted by an older
* build still arrives there, and a globalShortcut registration intercepts the
* key at the OS level even while the window is focused.
*
* Both sides used to keep their own table and only the renderer's was covered by
* a test; keeping the decision here (pure, no imports) makes the two guards
* provably identical and table-testable under plain Node.
*/
/**
* Modifier aliases, folded to Electron's canonical spelling. `cmdorctrl`,
* `commandorctrl` and `commandorcontrol` all collapse to `control`: the only
* platform this app is packaged for is Windows, where CommandOrControl resolves
* to Control, and a legacy value saved with the portable spelling would
* otherwise slip past the reserved check into a real registration.
*/
function canonicalPart(part: string): string {
const p = part.trim().toLowerCase()
if (p === 'ctrl') return 'control'
if (p === 'cmdorctrl' || p === 'commandorctrl' || p === 'commandorcontrol') return 'control'
return p
}
/** Split an accelerator into its canonical parts (`Ctrl+L` -> `['control','l']`). */
export function acceleratorParts(accelerator: string): string[] {
return accelerator.split('+').map(canonicalPart)
}
/**
* Whole chords the app owns outright, matched exactly (modifier set included).
* Ctrl+L is the panic lock, captured in main's before-input-event: a global
* registration intercepts the key at the OS level even while this window is
* focused, so binding it would silently disable the lock shortcut.
*/
const RESERVED_EXACT = new Set(['control+l'])
/**
* Keys this app binds while Control is held: font size (Ctrl+=/-/0, main.tsx)
* and tab cycling (Ctrl+PgUp/PgDn, Workspace). Neither handler looks at the
* other modifiers, so any Control combo on one of these keys would shadow the
* in-app action — the recorder refuses it, and the main process must not
* register a legacy value that has the same effect.
*/
const RESERVED_CONTROL_KEYS = new Set(['=', '-', '0', 'pageup', 'pagedown'])
/**
* True when `accelerator` collides with a shortcut the app already answers
* itself (e.g. `Control+Shift+=`, `Ctrl+L`).
*/
export function isReservedAccelerator(accelerator: string): boolean {
const parts = acceleratorParts(accelerator)
if (RESERVED_EXACT.has(parts.join('+'))) return true
return parts.includes('control') && RESERVED_CONTROL_KEYS.has(parts[parts.length - 1])
}