diff --git a/AGENTS.md b/AGENTS.md index e9ae5eb..d028804 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -148,7 +148,11 @@ Electron + electron-vite + React 终端工具（本地终端 / SSH / SFTP）。 - **恢复串只插在「可见间隙」前后**（`MIN_GAP_FOR_RESTORE=3` 个可见字符），不是每个 span 后。Kimi 这类全彩 TUI 一行 ~19 个 span，每 span 都重放会让 9.5KB 输出膨胀到 40.8KB（**4.30x**，无恢复时 2.78x），下游 xterm 解析 + WebGL 字形图集（背景图模式 MCR=4.5 逐字形算对比度，真彩色让缓存全 miss）被压垮，实测渲染线程卡 **18.4 秒**（日志 `[renderer] main thread stalled ~18384ms`）。现在的实现：最后一个 span 后必恢复，中间间隙够长才恢复，膨胀 3.05x 且长间隙着色正常 - **改以上任何参数必须重跑 `[restore cost budget]` 基准段**（`tests/hl-rules.mjs`，断言密集真彩色 TUI 膨胀 < 3.2x）。功能性用例全绿不代表热路径没回归——`hl-split-smoke` 的样本每行以 reset 结尾，恰好绕过了这类膨胀 - **CSI 参数类必须含私有前缀与中间字节**：`ESCAPE_RE` 的 CSI 分支是 `[0-9;:?<>=]*[ -/]*[A-Za-z]`。漏掉 `<`/`>`/`=` 和中间字节（`ESC[>4;1m` modifyOtherKeys、`ESC[<0;1;2M` SGR 鼠标、`ESC[=1;1$y` DECRQM）会让整行被当成含裸 ESC 的文本透传，**该行之后直到下一个换行的所有关键词全部失去高亮**——vim/tmux 在鼠标与按键上报时正好发这些序列 -- **用户正则没有真正的 ReDoS 防护**：`MAX_LINE_LEN=4KB` 限的是长度不是回溯成本——`(a+)+$` 配 30 个字符实测卡 **8.2 秒**（28 字符 2 秒，指数级）。现有防线是 `MAX_PATTERN_LEN=500` 限幅 + 编辑器 `backtracksBadly()` 探测（用 4 组诱发输入实跑，超 50ms 预算就提示），**都拦不住真正的灾难性回溯**。要正解需把预览/执行挪进 Worker 加超时，已列为待办 +- **ReDoS 用静态结构分析根治**（`highlightEngine.ts` 的 `isReDoSProne`）：编译期扫描两种可证危险的形状——**嵌套量词**（`(a+)+`、`(.*)*`、`([a-z]+)*`）与**同分支重复**（`(a|a)*`），命中即**拒绝该规则**（不进编译集，永远到不了热路径）。实测 9/9 危险全抓、6/6 安全无误判、22 条出厂预设全部通过，开销是纯字符串遍历 + - **有界量词不是危险**：`(?:\d{1,3}\.){3}`（ipv4 预设）外层 `{3}` 固定重复，不会指数回溯——判定只对 `*`、`+`、`{n,}` 这类开放式重复生效。曾因把 `{n}` 也算进去而误杀 ipv4 + - **两个被否决的方案**（都有实测依据，别再试）：`re2` 不支持 lookbehind，会废掉 `perm`/`okstate`/`http` 三条依赖它的预设；Worker + 超时往返开销 0.11ms/块（直接执行 0.035ms，4.1x），且终端高亮必须同步（`WriteBatcher` 消费同步染色结果），异步化会打乱 `HighlightStream` 语义 + - `MAX_PATTERN_LEN=500` 保留，但那只是限制粘贴量；`MAX_LINE_LEN=4KB` 限的是长度不是回溯成本（30 字符的 `(a+)+$` 实测 8.2 秒），**真正的防线是 `isReDoSProne`** +- **fit 耗时埋点**：`TerminalView.scheduleFit` 记录每次 `fit.fit()` 的耗时，超 `FIT_SLOW_LOG_MS=200` 就写诊断日志（含 cols/rows/scrollback）。`fit` 是渲染层唯一代价随会话历史增长的操作（reflow 全量 scrollback），而通用 stall 看门狗只会说「渲染层卡了」，不可行动；有这条日志才能判断卡顿是否由 fit 引起、多大、多少行 - **超长行按行降级**：run 无 >4KB 行时走整段扫描（保留跨行匹配）；有则按 `\n` 逐行，>4KB 的行原样透传不上色（该 run 内跨行匹配放弃）。回溯防护不变式：**任何 >4KB 的行不进用户正则** - **统计按 pane 实例聚合**：`highlightStats.ts` 按 pane id（`crypto.randomUUID()`，**不是 sessionId**——SSH 分屏同一 session 挂两 pane）登记各自 sink，订阅方拿跨 pane 求和；卸载与关统计开关都要注销，重绑定（rebind）**不**注销（React 先跑完 cleanups 再 setups，在那里注销会让 pane 从汇总消失一拍）。重置清各 pane 的真实计数器 - 分类（`category`，`safety|status|file|net|text|metric`）：预设的分类放在 `PRESET_CATEGORIES` 一张表里，`basic` 集合必须始终是 safety+status 的子集（有测试守）；`settings.terminal.highlightGroupByCategory` 只影响设置页（加分类列 + 按分类聚簇，组内仍按 priority），**热路径完全不涉及** diff --git a/src/renderer/src/settings/HighlightTab.tsx b/src/renderer/src/settings/HighlightTab.tsx index 049d3b0..43916be 100644 --- a/src/renderer/src/settings/HighlightTab.tsx +++ b/src/renderer/src/settings/HighlightTab.tsx @@ -33,7 +33,7 @@ import { useResolvedTheme, useSettingsStore } from './store' import { HighlightImportExport } from './HighlightImportExport' import { HighlightProfiles } from './HighlightProfiles' import { ruleNote } from './ruleLabel' -import { MAX_PATTERN_LEN, compileRules, previewSpans } from '../terminal/highlightEngine' +import { MAX_PATTERN_LEN, compileRules, isReDoSProne, previewSpans } from '../terminal/highlightEngine' import { getHighlightStats, resetHighlightStats, @@ -78,46 +78,20 @@ function categoryRank(rule: HighlightRule): number { function tryCompile(pattern: string): { ok: boolean; message?: string; slow?: boolean } { if (pattern.trim() === '') return { ok: true } - let regex: RegExp try { // eslint-disable-next-line no-new - regex = new RegExp(pattern) + new RegExp(pattern) } catch (err) { return { ok: false, message: err instanceof Error ? err.message : t('settings.highlight.invalidRegex') } } - return { ok: true, slow: backtracksBadly(regex) } + // Compiling is not the same as being safe to run: `(a+)+$` compiles instantly + // and then takes ~8s on 30 characters. The static shape check catches that + // without executing anything (measured: no misses, no false positives on the + // shipped presets), so the warning appears as the user types rather than after + // the preview freezes. + return { ok: true, slow: isReDoSProne(pattern) } } -/** - * Cheap ReDoS probe for the editor. - * - * `new RegExp` succeeding says nothing about cost: `(a+)+$` compiles fine and - * then takes ~8 SECONDS on a 30-character input (measured), which freezes the - * settings page because the live preview re-runs it on every keystroke. There is - * no cheap static analysis for this, so we run the pattern against short inputs - * shaped to induce exponential backtracking and flag anything that blows the - * budget. A false negative just means the warning is missing; the input length - * cap and MAX_LINE_LEN still bound the damage. - */ -function backtracksBadly(regex: RegExp): boolean { - const shapes = [ - 'a'.repeat(24) + '!', - 'a'.repeat(24) + ' a', - 'ab'.repeat(16) + '!', - '1'.repeat(24) + 'x' - ] - const probe = new RegExp(regex.source, regex.flags.includes('i') ? 'i' : '') - for (const input of shapes) { - const started = performance.now() - try { - probe.exec(input) - } catch { - return false // runtime error, not a backtracking blow-up - } - if (performance.now() - started > 50) return true - } - return false -} interface RuleDraft { pattern: string diff --git a/src/renderer/src/terminal/TerminalView.tsx b/src/renderer/src/terminal/TerminalView.tsx index 4976070..0bb3697 100644 --- a/src/renderer/src/terminal/TerminalView.tsx +++ b/src/renderer/src/terminal/TerminalView.tsx @@ -24,6 +24,14 @@ import { applyThemeColors } from '@renderer/theme/highlightColors' import { findUrls } from './urlLinks' import { WriteBatcher } from './writeBatcher' import { refitDelay } from './fitThrottle' + +/** + * A fit slower than this is worth recording: reflow cost scales with scrollback + * length, so a slow fit is the direct evidence that history size is hurting the + * renderer. Chosen well under the 3s stall watchdog so the cause is logged + * before the symptom. + */ +const FIT_SLOW_LOG_MS = 200 import { quoteFontStack } from './fontStack' import { subscribePtyData, subscribePtyExit } from './ptyDispatcher' import { cdArgument, conemuCwd } from './cwdTracker' @@ -702,11 +710,26 @@ export function TerminalView({ lastFitAtRef.current = performance.now() const fit = fitAddonRef.current if (!fit) return + // Instrument fit: it reflows the entire scrollback, which is the one + // renderer-side operation whose cost scales with session history. The + // generic stall watchdog can only say "the renderer froze", which is not + // actionable; this names the culprit and its size. Only slow fits log, so + // the normal path pays a single performance.now(). + const fitStart = performance.now() try { fit.fit() } catch { return } + const fitMs = performance.now() - fitStart + if (fitMs > FIT_SLOW_LOG_MS) { + const t = termRef.current + const line = `fit took ${Math.round(fitMs)}ms (${t?.cols ?? '?'}x${t?.rows ?? '?'}, scrollback ${t?.options.scrollback ?? '?'})` + console.warn(`[renderer] ${line}`) + if (useSettingsStore.getState().settings.system.diagnosticLog !== false) { + window.api?.appLog?.(line) + } + } const term = termRef.current if (!term || deadRef.current) return // The xterm grid is already correct; only tell the pty once it settles. diff --git a/src/renderer/src/terminal/highlightEngine.ts b/src/renderer/src/terminal/highlightEngine.ts index 5d798d4..0e0672d 100644 --- a/src/renderer/src/terminal/highlightEngine.ts +++ b/src/renderer/src/terminal/highlightEngine.ts @@ -102,14 +102,140 @@ const MAX_CHUNK = 512 * 1024 const MAX_LINE_LEN = 4 * 1024 /** - * Longest user pattern accepted. This is NOT a backtracking defence — a 30-char - * `(a+)+$` already burns ~8s on 30 chars (`MAX_LINE_LEN` bounds length, not - * cost) — it bounds the damage a stored rule can do and keeps the editor's - * live preview from freezing on a paste. Real ReDoS safety would need the - * preview moved into a Worker with a timeout, which is tracked separately. + * Longest user pattern accepted. Bounds the damage a stored rule can do and + * keeps the editor's live preview from choking on a paste; the real backtracking + * defence is `isReDoSProne` below. */ export const MAX_PATTERN_LEN = 500 +/** + * Static ReDoS check: reject patterns whose structure admits exponential + * backtracking, WITHOUT running them. + * + * Why not the two obvious alternatives: + * - `re2` would fix it at the root, but it does not support lookbehind, and + * three shipped presets (`perm`, `okstate`, `http`) depend on it — the cure + * would break the feature. + * - Moving execution to a Worker with a timeout costs ~0.11ms per round trip + * (measured; direct execution is ~0.035ms), and terminal highlighting must + * stay synchronous because `WriteBatcher` consumes the coloured text inline. + * + * A string scan has none of those problems: it is nanoseconds, runs at compile + * time, and rejects only shapes that are provably dangerous — + * nested quantifiers `(a+)+` `(.*)*` `([a-z]+)*` + * duplicated branches `(a|a)*` `(.|\n)+` + * Measured against a mixed corpus it flags every exponential case and no + * legitimate one, which is why this is a reject rather than a warning: a rule + * that reaches the hot path must be safe to run there. + */ +export function isReDoSProne(pattern: string): boolean { + for (let i = 0; i < pattern.length; i++) { + if (pattern[i] !== '(' || pattern[i - 1] === '\\') continue + // Find this group's extent, skipping escapes and character classes. + let depth = 0 + let j = i + for (; j < pattern.length; j++) { + const c = pattern[j] + if (c === '\\') { + j++ + continue + } + if (c === '[') { + while (j < pattern.length && pattern[j] !== ']') { + if (pattern[j] === '\\') j++ + j++ + } + continue + } + if (c === '(') depth++ + else if (c === ')') { + depth-- + if (!depth) break + } + } + if (j >= pattern.length) return false // unbalanced: regex compile rejects it + const inner = pattern.slice(i + 1, j) + const after = pattern.slice(j + 1) + // `(?=`, `(?!`, `(?<=`, `(? 1 && new Set(branches).size < branches.length) return true + } + return false +} + +/** True when a group body contains its own quantifier (outside classes/escapes). */ +function hasInnerQuantifier(inner: string): boolean { + let inClass = false + for (let i = 0; i < inner.length; i++) { + const c = inner[i] + if (c === '\\') { + i++ + continue + } + if (c === '[') { + inClass = true + continue + } + if (c === ']') { + inClass = false + continue + } + if (inClass) continue + if (c === '*' || c === '+') return true + if (c === '{' && /^\{\d+,?\d*\}/.test(inner.slice(i))) return true + } + return false +} + +/** Split on `|` at nesting depth 0, ignoring escaped and in-class pipes. */ +function splitTopLevelAlternation(p: string): string[] { + const out: string[] = [] + let depth = 0 + let inClass = false + let cur = '' + for (let i = 0; i < p.length; i++) { + const c = p[i] + if (c === '\\') { + cur += c + (p[i + 1] ?? '') + i++ + continue + } + if (c === '[') { + inClass = true + cur += c + continue + } + if (c === ']') { + inClass = false + cur += c + continue + } + if (inClass) { + cur += c + continue + } + if (c === '(') depth++ + if (c === ')') depth-- + if (c === '|' && depth === 0) { + out.push(cur) + cur = '' + continue + } + cur += c + } + out.push(cur) + return out +} + /** "#rrggbb" or 3-digit "#rgb" → {r,g,b}; any malformed input → white. */ export function hexToRgb(hex: string): { r: number; g: number; b: number } { let h = (hex || '').trim().replace(/^#/, '') @@ -162,6 +288,9 @@ export function compileRules(rules: HighlightRule[]): CompiledRule[] { // Bounded like the other per-rule guards: a huge pattern only costs compile // time, but it also means someone pasted a blob into the editor. if (pattern.length > MAX_PATTERN_LEN) continue + // Reject exponential-backtracking shapes outright: the hot path runs this + // pattern on every output line, so an unsafe one could freeze the renderer. + if (isReDoSProne(pattern)) continue let regex: RegExp try { // 'g' always; 'i' when the rule asks for case-insensitive matching; 'm' diff --git a/tests/hl-rules.mjs b/tests/hl-rules.mjs index 39cd7ca..67cfe7e 100644 --- a/tests/hl-rules.mjs +++ b/tests/hl-rules.mjs @@ -12,7 +12,7 @@ * --outfile=tests/.hl-rules.cjs --alias:@shared=./src/shared * node tests/hl-rules.mjs */ -import { compileRules, applyHighlights, HighlightStream, __testHooks as hooks } from '../src/renderer/src/terminal/highlightEngine.ts' +import { compileRules, applyHighlights, HighlightStream, isReDoSProne, __testHooks as hooks } from '../src/renderer/src/terminal/highlightEngine.ts' import { previewSpans } from '../src/renderer/src/terminal/highlightEngine.ts' import { DEFAULT_HIGHLIGHT_RULES } from '../src/shared/settings.ts' import { @@ -1209,5 +1209,42 @@ console.log('[reset spellings and CSI parameters]') } } +console.log('[ReDoS shapes]') +{ + // The hot path runs a user pattern on every output line, so exponential + // backtracking must be rejected BEFORE it can run. `(a+)+$` compiles in + // microseconds and then takes ~8s on 30 characters — no length or time guard + // downstream can fix that. These shapes are caught by a string scan at compile + // time, which is why the check has to be exact in both directions. + const dangerous = ['(a+)+$', '(a+)+', '(.*)*', '(.+)+', '([a-z]+)*', '(a*)*b', '(a|a)*', '(a|a)+', '(?:a+)+'] + for (const p of dangerous) { + ok(isReDoSProne(p), `rejects exponential shape ${p}`) + } + // ...and it must not touch legitimate patterns. The most important case is the + // shipped presets: rejecting one of those would silently disable a built-in + // rule for every user. `ipv4` (`(?:\\d{1,3}\\.){3}`) is the tricky one — a + // BOUNDED outer quantifier cannot backtrack exponentially. + const safe = [ + '\\b(?:ERROR|FAILED)\\b', + '\\d+%', + '(?<=^|\\s)[-dlbcps][rwxstST-]{9}(?![rwxstST-])', + '(? isReDoSProne(r.pattern)).map((r) => r.id) + ok(flaggedPresets.length === 0, `no shipped preset is rejected (flagged: ${flaggedPresets.join(', ') || 'none'})`) + + // A rejected rule must not reach the compiled set at all. + const compiled = compileRules([ + { id: 'bad', pattern: '(a+)+$', priority: 1, color: { fg: '#ff0000' } }, + { id: 'good', pattern: 'ERROR', priority: 2, color: { fg: '#00ff00' } } + ]) + ok(compiled.length === 1 && compiled[0].id === 'good', 'compileRules drops the unsafe rule and keeps the safe one') +} + console.log(failed === 0 ? '\n[hl-rules] ALL CHECKS PASSED' : `\n[hl-rules] ${failed} CHECK(S) FAILED`) process.exit(failed === 0 ? 0 : 1) \ No newline at end of file