From d3d0a0f955f7bbbaea9cef08e283952bcf6c96a7 Mon Sep 17 00:00:00 2001 From: Sergey Kozyrenko Date: Tue, 30 Jun 2026 21:01:11 +0700 Subject: [PATCH] =?UTF-8?q?fix(editor):=20kill=20ReDoS=20in=20the=20variab?= =?UTF-8?q?le=20cycle=20=E2=80=94=20block-first=20instead=20of=20lazy=20re?= =?UTF-8?q?gex?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit variableUseRegex built `{{[^{}]*?\.Name\b[^{}]*?}}` with two lazy spans; on an unclosed `{{` with many `.Name` anchors it backtracked O(n²) (measured 25KB 49ms / 100KB 782ms / 250KB 4.9s — a multi-second main-thread freeze from one variable-panel click). The panel COUNT path already extracted `{{…}}` blocks linearly via VARIABLE_RE then probed each; apply the same block-first shape to the two CYCLE consumers (findVariableOccurrences for the editor, the plain-mode textarea cycle in settings-prompt) via shared findVariableUseRanges/variableProbe. Same inputs now 0.16/0.21/0.48ms (~10000x at 250KB). Also escape the interpolated variable name (a `.` in a name would otherwise match any char). Drop the dead variableUseRegex; re-add InlineMatch.text so the doc scan can probe per block. Pinned by a linear-time regression test on the pathological input + an escape test; de-stales the editor-highlight-regex test header (scan is per-textblock since 69f86ae, not per-text-node). Co-Authored-By: Claude Opus 4.8 --- .../shared/editor-highlight-regex.test.ts | 31 +++++++++++++++-- .../components/shared/editor-inline-scan.ts | 3 +- .../shared/editor-variable-highlight.ts | 33 +++++++++++++++---- .../src/pages/settings/settings-prompt.tsx | 10 +++--- 4 files changed, 62 insertions(+), 15 deletions(-) diff --git a/frontend/src/components/shared/editor-highlight-regex.test.ts b/frontend/src/components/shared/editor-highlight-regex.test.ts index 73ffff30..3f861473 100644 --- a/frontend/src/components/shared/editor-highlight-regex.test.ts +++ b/frontend/src/components/shared/editor-highlight-regex.test.ts @@ -1,10 +1,10 @@ import { describe, expect, it } from 'vitest'; import { TAG_RE } from './editor-tag-highlight'; -import { VARIABLE_RE } from './editor-variable-highlight'; +import { findVariableUseRanges, VARIABLE_RE, variableProbe } from './editor-variable-highlight'; -// Decorations highlight per text node via `node.text.matchAll(RE)`; assert the RE itself matches the -// right spans and — critically — does NOT false-positive on prose that merely contains < or {{. +// Assert the highlight regexes match the right spans and — critically — do NOT false-positive on prose +// that merely contains < or {{. const matches = (re: RegExp, text: string) => [...text.matchAll(re)].map((m) => m[0]); describe('VARIABLE_RE — {{ go-template actions }}', () => { @@ -55,3 +55,28 @@ describe('TAG_RE — ', () => { expect(matches(TAG_RE, 'if a then')).toEqual(['']); }); }); + +describe('findVariableUseRanges — block-first {{ … .Var … }} ranges (ReDoS-safe)', () => { + it('returns each use as {index, length}', () => { + expect(findVariableUseRanges('a {{.Foo}} b {{ .Foo | upper }} c {{.Bar}}', 'Foo')).toEqual([ + { index: 2, length: 8 }, + { index: 13, length: 18 }, + ]); + }); + + it('stays linear on an unclosed {{ with many anchors (the old lazy regex froze here)', () => { + const pathological = `{{ ${' .Foo'.repeat(20000)}`; + const start = performance.now(); + const ranges = findVariableUseRanges(pathological, 'Foo'); + + expect(performance.now() - start).toBeLessThan(200); + expect(ranges).toEqual([]); + }); +}); + +describe('variableProbe — escapes the interpolated name', () => { + it('treats a dot in the name as literal, not any-char', () => { + expect(variableProbe('Foo').test('.Foo')).toBe(true); + expect(variableProbe('F.o').test('.Fxo')).toBe(false); + }); +}); diff --git a/frontend/src/components/shared/editor-inline-scan.ts b/frontend/src/components/shared/editor-inline-scan.ts index 5cacceb7..0ad7765f 100644 --- a/frontend/src/components/shared/editor-inline-scan.ts +++ b/frontend/src/components/shared/editor-inline-scan.ts @@ -2,6 +2,7 @@ import type { Node as PMNode } from '@tiptap/pm/model'; export interface InlineMatch { from: number; + text: string; to: number; } @@ -41,7 +42,7 @@ export const collectInlineMatches = (doc: PMNode, regex: RegExp): InlineMatch[] const last = positions[start + match[0].length - 1]; if (from !== undefined && last !== undefined) { - matches.push({ from, to: last + 1 }); + matches.push({ from, text: match[0], to: last + 1 }); } } diff --git a/frontend/src/components/shared/editor-variable-highlight.ts b/frontend/src/components/shared/editor-variable-highlight.ts index 3308fd97..428af626 100644 --- a/frontend/src/components/shared/editor-variable-highlight.ts +++ b/frontend/src/components/shared/editor-variable-highlight.ts @@ -15,10 +15,13 @@ const variableHighlightKey = new PluginKey('variableHighlight'); // `[^{}]` keeps the scan linear (no catastrophic backtracking); Go actions never nest braces. export const VARIABLE_RE = /\{\{[^{}]*\}\}/g; -// Matches one variable's `{{ … .Name … }}` use. Shared with settings-prompt.tsx (panel cycle + count) so the -// panel's "used" badge and the editor cycle agree on what counts as a use. -export const variableUseRegex = (variable: string): RegExp => - new RegExp(`\\{\\{[^{}]*?\\.${variable}\\b[^{}]*?\\}\\}`, 'g'); +const escapeRegExp = (value: string): string => value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + +// Tests whether a single `{{ … }}` block references `variable` (`.Name` on a word boundary). Always used +// block-first — extract `{{ … }}` blocks with the linear VARIABLE_RE, THEN probe each — never a lazy +// `[^{}]*?` around the name, which backtracks O(n²) on an unclosed `{{`. Shared with settings-prompt.tsx +// (panel cycle + count) so the "used" badge and the editor cycle agree on what counts as a use. +export const variableProbe = (variable: string): RegExp => new RegExp(`\\.${escapeRegExp(variable)}\\b`); const buildDecorations = (doc: PMNode): DecorationSet => DecorationSet.create( @@ -28,8 +31,26 @@ const buildDecorations = (doc: PMNode): DecorationSet => ), ); -export const findVariableOccurrences = (doc: PMNode, variable: string): { from: number; to: number }[] => - collectInlineMatches(doc, variableUseRegex(variable)).map(({ from, to }) => ({ from, to })); +export const findVariableOccurrences = (doc: PMNode, variable: string): { from: number; to: number }[] => { + const probe = variableProbe(variable); + + return collectInlineMatches(doc, VARIABLE_RE) + .filter(({ text }) => probe.test(text)) + .map(({ from, to }) => ({ from, to })); +}; + +export const findVariableUseRanges = (value: string, variable: string): { index: number; length: number }[] => { + const probe = variableProbe(variable); + const ranges: { index: number; length: number }[] = []; + + for (const match of value.matchAll(VARIABLE_RE)) { + if (match.index !== undefined && probe.test(match[0])) { + ranges.push({ index: match.index, length: match[0].length }); + } + } + + return ranges; +}; export const VariableHighlight = Extension.create({ addProseMirrorPlugins() { diff --git a/frontend/src/pages/settings/settings-prompt.tsx b/frontend/src/pages/settings/settings-prompt.tsx index 4c2fe70b..b40e3305 100644 --- a/frontend/src/pages/settings/settings-prompt.tsx +++ b/frontend/src/pages/settings/settings-prompt.tsx @@ -52,7 +52,7 @@ import { AppHeaderTitle, } from '@/components/layouts/app/app-header'; import ConfirmationDialog from '@/components/shared/confirmation-dialog'; -import { VARIABLE_RE, variableUseRegex } from '@/components/shared/editor-variable-highlight'; +import { findVariableUseRanges, VARIABLE_RE, variableProbe } from '@/components/shared/editor-variable-highlight'; import { UnsavedChangesDialog, useUnsavedChangesGuard } from '@/components/shared/unsaved-changes'; import { Alert, AlertDescription, AlertTitle } from '@/components/ui/alert'; import { Badge } from '@/components/ui/badge'; @@ -126,7 +126,7 @@ type SystemFormData = z.infer; // One pass over the `{{ … }}` blocks — the naive per-variable `.match` over the whole template is // O(variables × length) on every keystroke. function countVariableUses(template: string, variables: string[]): Record { - const probes = variables.map((variable) => [variable, new RegExp(`\\.${variable}\\b`)] as const); + const probes = variables.map((variable) => [variable, variableProbe(variable)] as const); const counts: Record = {}; for (const block of template.match(VARIABLE_RE) ?? []) { @@ -317,12 +317,12 @@ function SettingsPrompt() { if (textarea) { const currentValue = field.value || ''; const variablePattern = `{{.${variable}}}`; - const matches = [...currentValue.matchAll(variableUseRegex(variable))]; + const matches = findVariableUseRanges(currentValue, variable); if (matches.length > 0) { const { selectionEnd, selectionStart } = textarea; const currentIndex = matches.findIndex( - (match) => match.index === selectionStart && match.index + match[0].length === selectionEnd, + (match) => match.index === selectionStart && match.index + match.length === selectionEnd, ); const target = currentIndex >= 0 @@ -332,7 +332,7 @@ function SettingsPrompt() { if (target) { const matchStart = target.index; textarea.focus(); - textarea.setSelectionRange(matchStart, matchStart + target[0].length); + textarea.setSelectionRange(matchStart, matchStart + target.length); textarea.scrollTop = Math.max( 0, caretOffsetTop(textarea, matchStart) - textarea.clientHeight / 2,