From 6dda4cede579cd728960a9a8b2ca0731d72b2d16 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 27 Jul 2026 13:47:06 -0700 Subject: [PATCH 1/9] fix(logs): match copilot log-grep patterns literally, never as a regex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `grepSpans` compiled a caller-supplied pattern with a bare `new RegExp`, whose only guard was a `catch` for syntax errors. Trace text is attacker-influenced — a workflow can emit arbitrarily long uniform runs into its own block outputs — and matching runs synchronously on the shared event loop, so an authenticated caller could choose both the pattern and the input and stall every request on the instance. Patterns are now matched as an escaped, case-insensitive literal. Screening was implemented first and abandoned, because each rule only excludes the shapes someone thought to enumerate: - `safe-regex2` (already used by the guardrails validator) documents itself as having false negatives. It screens star height only, so it passes `(a|a)*b`, measured >61s on V8 at 30 characters. - Rejecting quantified groups on top of that catches `(a|a)*b`, and every attack `safe-regex2` catches, but still passes `a*a*b` — adjacent quantifiers over overlapping character sets — measured 213s on JSC and 132s on V8 over a 10k-character run, well inside what a block output can hold. An escaped literal is linear on every engine, needs no dependency, and does not depend on which runtime serves the request. `safe-regex2` stays a dependency for its existing guardrails/PII callers; it is simply not relied on here. `query_logs` loses regex matching. Its catalog entry describes `pattern` as "greps" rather than promising regex, but it is generated from a contract in another repository and cannot be updated here, so a `patternNotice` is returned whenever a pattern contains regex syntax — the caller is told the pattern was taken literally instead of reading zero matches as "not in the trace". Two bounds are kept as backstops: a cumulative match-time budget charging only time inside `test`/`exec` (never the blob-store reads the scan awaits between matches, which would truncate slow-but-legitimate greps under load), and a total scanned-character cap. --- .../tools/server/workflow/query-logs.ts | 7 +- apps/sim/lib/logs/log-views.test.ts | 73 ++++++++++- apps/sim/lib/logs/log-views.ts | 114 ++++++++++++++++-- 3 files changed, 179 insertions(+), 15 deletions(-) diff --git a/apps/sim/lib/copilot/tools/server/workflow/query-logs.ts b/apps/sim/lib/copilot/tools/server/workflow/query-logs.ts index 4e4af600e85..9ab073f7dd8 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/query-logs.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/query-logs.ts @@ -145,12 +145,17 @@ export const queryLogsServerTool: BaseServerTool = { if (args.pattern) { logger.info('query_logs grep', { workspaceId, executionId: args.executionId }) - const { matches, truncated } = await grepSpans(traceSpans, args.pattern, viewCtx) + const { matches, truncated, patternNotice } = await grepSpans( + traceSpans, + args.pattern, + viewCtx + ) return { executionId: detail.executionId, workflowId: detail.workflowId, status: detail.status, pattern: args.pattern, + ...(patternNotice ? { patternNotice } : {}), matches, truncated, } diff --git a/apps/sim/lib/logs/log-views.test.ts b/apps/sim/lib/logs/log-views.test.ts index 9b31ed4feb5..e1b2340e636 100644 --- a/apps/sim/lib/logs/log-views.test.ts +++ b/apps/sim/lib/logs/log-views.test.ts @@ -32,6 +32,7 @@ vi.mock('@/lib/execution/payloads/store', () => ({ materializeLargeValueRef: materializeLargeValueRefMock, })) +import { sleep } from '@sim/utils/helpers' import type { TraceSpan } from '@/lib/logs/types' import { grepSpans, type LogViewContext, toFull, toOverview } from './log-views' @@ -169,10 +170,80 @@ describe('grepSpans', () => { expect(result.matches.some((m) => m.field === 'output')).toBe(true) }) - it('falls back to literal substring on invalid regex', async () => { + it('matches regex syntax literally rather than interpreting it', async () => { const spans = [span({ output: { v: 'value a(b found' } })] const result = await grepSpans(spans, '(', ctx) expect(result.matches.some((m) => m.field === 'output')).toBe(true) + expect(result.patternNotice).toContain('literal') + }) + + it('does not interpret a regex pattern, and says so', async () => { + const spans = [span({ output: { v: 'status=503' } })] + + // Literal 'status=\d+' is not present; the regex it denotes would have matched. + const asRegex = await grepSpans(spans, 'status=\\d+', ctx) + expect(asRegex.matches).toEqual([]) + expect(asRegex.patternNotice).toContain('literal') + + const asLiteral = await grepSpans(spans, 'status=503', ctx) + expect(asLiteral.matches.some((m) => m.field === 'output')).toBe(true) + expect(asLiteral.patternNotice).toBeUndefined() + }) + + it.each([ + ['nested quantifier', '(a+)+$'], + ['duplicate alternation, passes safe-regex2', '(a|a)*b'], + ['adjacent quantifiers, passes every structural screen', 'a*a*b'], + ])('never executes a catastrophic pattern (%s)', async (_label, pattern) => { + // Each of these blocks the event loop for minutes if compiled as a regex: + // `a*a*b` measured 213s on JSC / 132s on V8 over a 10k-character run. + const spans = [span({ output: { v: `${'a'.repeat(5000)}!` } })] + + const start = Date.now() + const result = await grepSpans(spans, pattern, ctx) + const elapsedMs = Date.now() - start + + expect(elapsedMs).toBeLessThan(1000) + expect(result.matches).toEqual([]) + }) + + it('matches a long pattern literally without a length cap', async () => { + const pattern = `${'x'.repeat(600)}needle` + const spans = [span({ output: { v: pattern } })] + const result = await grepSpans(spans, pattern, ctx) + expect(result.matches.some((m) => m.field === 'output')).toBe(true) + }) + + it('stops scanning and marks truncated once the character budget is exhausted', async () => { + const spans = [ + span({ id: 'a', output: { v: 'x'.repeat(400) } }), + span({ id: 'b', output: { v: 'needle' } }), + ] + const result = await grepSpans(spans, 'needle', ctx, { maxScannedChars: 100 }) + expect(result.matches).toEqual([]) + expect(result.truncated).toBe(true) + }) + + it('stops scanning and marks truncated once the match-time budget is exhausted', async () => { + const spans = [span({ output: { v: 'needle' } })] + const result = await grepSpans(spans, 'needle', ctx, { matchTimeBudgetMs: 0 }) + expect(result.matches).toEqual([]) + expect(result.truncated).toBe(true) + }) + + it('does not charge blob-store I/O to the match-time budget', async () => { + // Each slice read sleeps well past the budget: only time spent matching + // counts, so a slow-but-legitimate grep must still return complete results. + readLargeArrayManifestSliceMock.mockImplementation(async (_m: unknown, start: number) => { + await sleep(30) + return start === 400 ? [{ v: 'found the needle here' }] : [{ v: 'nothing' }] + }) + const spans = [span({ output: manifest(500) as any })] + + const result = await grepSpans(spans, 'needle', ctx, { matchTimeBudgetMs: 50 }) + + expect(result.matches.some((m) => m.field === 'output')).toBe(true) + expect(result.truncated).toBe(false) }) it('returns empty for empty traceSpans', async () => { diff --git a/apps/sim/lib/logs/log-views.ts b/apps/sim/lib/logs/log-views.ts index e7e4feaceee..060d92caf9c 100644 --- a/apps/sim/lib/logs/log-views.ts +++ b/apps/sim/lib/logs/log-views.ts @@ -23,6 +23,23 @@ const DEFAULT_MAX_MATCHES = 50 const DEFAULT_MAX_SNIPPET_CHARS = 500 const DEFAULT_MAX_SLICES_SCANNED = 200 +/** + * Cumulative time the pattern itself may spend matching, across all spans/fields. + * + * Deliberately counts only time inside `test`/`exec`, not the grep's wall clock: + * the scan awaits blob-store reads (array slices, large-value refs) between + * matches, and charging that I/O to the budget would truncate slow-but-legitimate + * greps under load. Matching is the only part that occupies the event loop, so + * it is the only part worth bounding. + */ +const DEFAULT_MATCH_TIME_BUDGET_MS = 5_000 +/** + * Total characters a single grep may run the pattern over. A backstop against a + * pattern that slips past the backtracking screen being amplified across every + * slice — set well above any realistic trace so normal greps never trip it. + */ +const DEFAULT_MAX_SCANNED_CHARS = 64 * 1024 * 1024 + // --------------------------------------------------------------------------- // Overview (Level 2): block tree with timing + cost, NO input/output. // --------------------------------------------------------------------------- @@ -172,21 +189,34 @@ export interface GrepSpanMatch { export interface GrepSpansResult { matches: GrepSpanMatch[] truncated: boolean + /** + * Present when the pattern contained regex syntax, which is matched literally. + * The tool catalog cannot say so up front — it is generated from a contract in + * another repository — so the caller is told here instead of silently reading + * zero matches as "not present in the trace". + */ + patternNotice?: string } export interface GrepSpansOptions { maxMatches?: number maxSnippetChars?: number maxSlicesScanned?: number + maxScannedChars?: number + matchTimeBudgetMs?: number } interface GrepState { matches: GrepSpanMatch[] slicesScanned: number + scannedChars: number + matchTimeMs: number truncated: boolean maxMatches: number maxSnippetChars: number maxSlicesScanned: number + maxScannedChars: number + matchTimeBudgetMs: number regex: RegExp } @@ -194,16 +224,50 @@ function escapeRegExp(input: string): string { return input.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') } -function buildRegex(pattern: string): RegExp { +/** Regex syntax in a pattern signals the caller expected regex semantics. */ +const REGEX_METACHARACTERS = /[.*+?^${}()|[\]\\]/ + +/** + * Compile a caller-supplied grep pattern as a case-insensitive literal. + * + * Caller patterns are deliberately never executed as regexes. Trace text is + * attacker-influenced — a workflow can emit arbitrarily long uniform runs into + * its own block outputs — and matching runs synchronously on the shared event + * loop, so a backtracking pattern stalls every request on the instance. + * + * Screening was tried and abandoned: `safe-regex2` (which the guardrails + * validator uses) documents itself as having false negatives and passes + * `(a|a)*b`; rejecting quantified groups on top of it still passes `a*a*b`, + * measured at 213s on JSC and 132s on V8 over a 10k-character run. Each rule + * only excludes the shapes someone thought to enumerate. An escaped literal is + * linear on every engine, which is the one property worth relying on here. + */ +function compilePattern(pattern: string): { regex: RegExp; notice?: string } { + const regex = new RegExp(escapeRegExp(pattern), 'i') + if (!REGEX_METACHARACTERS.test(pattern)) return { regex } + return { + regex, + notice: + 'Matched as a literal, case-insensitive substring — regex syntax is not interpreted. Search for the literal text you expect to see in the trace.', + } +} + +/** + * Charge the elapsed matching time to the grep's budget. Every `test`/`exec` on + * caller-supplied patterns goes through here. + */ +function runTimed(state: GrepState, match: (regex: RegExp) => T): T { + const started = performance.now() try { - return new RegExp(pattern, 'i') - } catch { - return new RegExp(escapeRegExp(pattern), 'i') + state.regex.lastIndex = 0 + return match(state.regex) + } finally { + state.matchTimeMs += performance.now() - started } } -function snippetAround(text: string, regex: RegExp, maxChars: number): string { - const m = regex.exec(text) +function snippetAround(text: string, state: GrepState, maxChars: number): string { + const m = runTimed(state, (regex) => regex.exec(text)) const index = m ? m.index : 0 const half = Math.floor(maxChars / 2) const start = Math.max(0, index - half) @@ -214,7 +278,12 @@ function snippetAround(text: string, regex: RegExp, maxChars: number): string { } function done(state: GrepState): boolean { - return state.truncated || state.matches.length >= state.maxMatches + if (state.truncated || state.matches.length >= state.maxMatches) return true + if (state.matchTimeMs >= state.matchTimeBudgetMs) { + state.truncated = true + return true + } + return false } function recordIfMatch( @@ -224,15 +293,18 @@ function recordIfMatch( state: GrepState ): void { if (done(state)) return - state.regex.lastIndex = 0 - if (!state.regex.test(text)) return - state.regex.lastIndex = 0 + if (state.scannedChars + text.length > state.maxScannedChars) { + state.truncated = true + return + } + state.scannedChars += text.length + if (!runTimed(state, (regex) => regex.test(text))) return state.matches.push({ spanId: span.id, blockId: span.blockId, name: span.name, field, - snippet: snippetAround(text, state.regex, state.maxSnippetChars), + snippet: snippetAround(text, state, state.maxSnippetChars), }) if (state.matches.length >= state.maxMatches) state.truncated = true } @@ -305,6 +377,13 @@ function safeStringify(value: unknown): string { * directly; large-array I/O is streamed slice-by-slice (each released before the * next); single large refs are materialized under a byte cap (falling back to * the ref preview). Only bounded match snippets are accumulated. + * + * `pattern` is matched as a literal substring, never as a regex — see + * `compilePattern`. Two budgets bound the scan on top of that: a total + * character budget and a cumulative match-time budget. Neither counts the + * blob-store I/O this scan awaits, so a slow-but-legitimate grep is not + * truncated for being slow. With literal matching both are backstops rather + * than load-bearing, and they stay to bound total work per request. */ export async function grepSpans( spans: TraceSpan[], @@ -312,14 +391,19 @@ export async function grepSpans( ctx: LogViewContext, opts?: GrepSpansOptions ): Promise { + const compiled = compilePattern(pattern) const state: GrepState = { matches: [], slicesScanned: 0, + scannedChars: 0, + matchTimeMs: 0, truncated: false, maxMatches: opts?.maxMatches ?? DEFAULT_MAX_MATCHES, maxSnippetChars: opts?.maxSnippetChars ?? DEFAULT_MAX_SNIPPET_CHARS, maxSlicesScanned: opts?.maxSlicesScanned ?? DEFAULT_MAX_SLICES_SCANNED, - regex: buildRegex(pattern), + maxScannedChars: opts?.maxScannedChars ?? DEFAULT_MAX_SCANNED_CHARS, + matchTimeBudgetMs: opts?.matchTimeBudgetMs ?? DEFAULT_MATCH_TIME_BUDGET_MS, + regex: compiled.regex, } const walk = async (list: TraceSpan[]): Promise => { @@ -335,5 +419,9 @@ export async function grepSpans( } await walk(spans) - return { matches: state.matches, truncated: state.truncated } + return { + matches: state.matches, + truncated: state.truncated, + ...(compiled.notice ? { patternNotice: compiled.notice } : {}), + } } From 0347b177b4644c4ac5e43b0de0dab31a37f9aa3e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 27 Jul 2026 16:01:09 -0700 Subject: [PATCH 2/9] improvement(logs): only flag genuine regex intent, document the truncated invariant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the literal log-grep. The `patternNotice` fired on any regex metacharacter, which includes `.` — so ordinary literal searches (`example.com`, `file.pdf`, `v1.2.3`, `block_1.output`) were told their pattern "was not interpreted as a regex", inviting the agent to retry a search that had in fact worked. Measured 10 false notices across 19 realistic literal searches. `REGEX_INTENT` now keys on actual regex intent — escape classes, character class, group, alternation, a leading/trailing anchor, or a repetition quantifier — which drops that to 0/19 while still flagging all 10 regex attempts tested. `truncated` gains a TSDoc block stating its invariant: it reports that trace was left unread, not that a budget was reached. Every point that skips work goes through `done()`, which sets it, so a budget exhausted by the final match correctly reports `false`. Raised by Cursor Bugbot; behavior is right, the contract was just implicit. Also drops `snippetAround`'s `maxChars` parameter, which every caller passed from the state object it already receives. --- apps/sim/lib/logs/log-views.test.ts | 10 ++++++++++ apps/sim/lib/logs/log-views.ts | 26 +++++++++++++++++++++----- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/apps/sim/lib/logs/log-views.test.ts b/apps/sim/lib/logs/log-views.test.ts index e1b2340e636..5617a94e40e 100644 --- a/apps/sim/lib/logs/log-views.test.ts +++ b/apps/sim/lib/logs/log-views.test.ts @@ -177,6 +177,16 @@ describe('grepSpans', () => { expect(result.patternNotice).toContain('literal') }) + it.each(['example.com', 'file.pdf', 'v1.2.3', 'block_1.output', '$0.42', 'why?'])( + 'does not warn about regex on the ordinary literal %s', + async (pattern) => { + const spans = [span({ output: { v: `saw ${pattern} here` } })] + const result = await grepSpans(spans, pattern, ctx) + expect(result.matches.some((m) => m.field === 'output')).toBe(true) + expect(result.patternNotice).toBeUndefined() + } + ) + it('does not interpret a regex pattern, and says so', async () => { const spans = [span({ output: { v: 'status=503' } })] diff --git a/apps/sim/lib/logs/log-views.ts b/apps/sim/lib/logs/log-views.ts index 060d92caf9c..277db8369a8 100644 --- a/apps/sim/lib/logs/log-views.ts +++ b/apps/sim/lib/logs/log-views.ts @@ -188,6 +188,12 @@ export interface GrepSpanMatch { export interface GrepSpansResult { matches: GrepSpanMatch[] + /** + * Whether the scan stopped with trace left unread — not whether a budget was + * reached. Every point that skips work goes through `done`, which sets this, + * so a budget exhausted by the very last match reports `false`: the caller's + * results are complete, and nothing was withheld. + */ truncated: boolean /** * Present when the pattern contained regex syntax, which is matched literally. @@ -224,8 +230,17 @@ function escapeRegExp(input: string): string { return input.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') } -/** Regex syntax in a pattern signals the caller expected regex semantics. */ -const REGEX_METACHARACTERS = /[.*+?^${}()|[\]\\]/ +/** + * Signals that the caller wrote a pattern *intending* regex semantics: escape + * classes, a character class, a group, alternation, a leading/trailing anchor, + * or a repetition quantifier. + * + * Deliberately narrower than the set `escapeRegExp` escapes. A bare `.` or `?` + * is ordinary text in the things people actually grep for — `example.com`, + * `file.pdf`, `v1.2.3`, `block_1.output` — and flagging those would tell the + * caller its correct search was misinterpreted, prompting a pointless retry. + */ +const REGEX_INTENT = /\\[dwsbDWSB]|[[\]()|*+]|^\^|\$$|\{\d+,?\d*\}/ /** * Compile a caller-supplied grep pattern as a case-insensitive literal. @@ -244,7 +259,7 @@ const REGEX_METACHARACTERS = /[.*+?^${}()|[\]\\]/ */ function compilePattern(pattern: string): { regex: RegExp; notice?: string } { const regex = new RegExp(escapeRegExp(pattern), 'i') - if (!REGEX_METACHARACTERS.test(pattern)) return { regex } + if (!REGEX_INTENT.test(pattern)) return { regex } return { regex, notice: @@ -266,9 +281,10 @@ function runTimed(state: GrepState, match: (regex: RegExp) => T): T { } } -function snippetAround(text: string, state: GrepState, maxChars: number): string { +function snippetAround(text: string, state: GrepState): string { const m = runTimed(state, (regex) => regex.exec(text)) const index = m ? m.index : 0 + const maxChars = state.maxSnippetChars const half = Math.floor(maxChars / 2) const start = Math.max(0, index - half) const end = Math.min(text.length, start + maxChars) @@ -304,7 +320,7 @@ function recordIfMatch( blockId: span.blockId, name: span.name, field, - snippet: snippetAround(text, state, state.maxSnippetChars), + snippet: snippetAround(text, state), }) if (state.matches.length >= state.maxMatches) state.truncated = true } From e3852520ad5c3264b27e1e11d3db85e85716158b Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 27 Jul 2026 16:20:29 -0700 Subject: [PATCH 3/9] fix(logs): match log-grep patterns with RE2 instead of dropping regex support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restores full regex support on copilot log-grep, which the previous commits in this PR had removed to close the ReDoS. Deleting the capability was the wrong trade: RE2JS is a port of RE2 that matches in time linear in the input, so no pattern can blow up regardless of the text, and callers keep their regexes. Measured through the real compile path, against a 100k-character adversarial input — 10x the run that took 213s on JSC / 132s on V8: (a+)+$ 13.0ms (\w+\s?)*$ 10.7ms (a|a)*b 6.7ms (x+x+)+y 4.0ms a*a*b 8.1ms ^(\d+)*$ 0.0ms Two escape hatches keep this honest: - A pattern with no regex metacharacter takes the built-in engine. Semantics are identical when there is nothing to interpret, and RE2JS costs ~100x more per byte (~25ms/MB), so the common plain-text search stays native. - Lookaround and backreferences are not in RE2. RE2JS rejects them at compile time, so those fall back to a literal and return a `patternNotice` naming the constructs — a narrow, reported gap rather than a silent behavior change. The match-time and scanned-character budgets stop being formalities: RE2JS's throughput is what they now bound, not backtracking. Also collapses the old test-then-exec double scan — `compilePattern` returns a single `find` returning a match index, which `recordIfMatch` passes straight to `snippetAround`, so each field is scanned once instead of twice. re2js@2.8.6 is MIT, pure JavaScript (no native addon, so nothing changes for the oven/bun image), server-only, and published 2026-07-05 — clearing the 7-day bunfig minimumReleaseAge gate without an exclusion. --- apps/sim/lib/logs/log-views.test.ts | 68 ++++++++------ apps/sim/lib/logs/log-views.ts | 140 ++++++++++++++++------------ apps/sim/package.json | 1 + bun.lock | 3 + 4 files changed, 123 insertions(+), 89 deletions(-) diff --git a/apps/sim/lib/logs/log-views.test.ts b/apps/sim/lib/logs/log-views.test.ts index 5617a94e40e..d942ffb9236 100644 --- a/apps/sim/lib/logs/log-views.test.ts +++ b/apps/sim/lib/logs/log-views.test.ts @@ -170,51 +170,59 @@ describe('grepSpans', () => { expect(result.matches.some((m) => m.field === 'output')).toBe(true) }) - it('matches regex syntax literally rather than interpreting it', async () => { - const spans = [span({ output: { v: 'value a(b found' } })] - const result = await grepSpans(spans, '(', ctx) - expect(result.matches.some((m) => m.field === 'output')).toBe(true) - expect(result.patternNotice).toContain('literal') + it.each([ + ['character class', 'status=\\d+'], + ['anchor', '^Agent'], + ['alternation', '(openai|anthropic)'], + ['word boundary', '\\bstatus\\b'], + ['bounded quantifier', '\\d{4}-\\d{2}-\\d{2}'], + ['wildcard', 'called.*status'], + ])('interprets %s regex syntax', async (_label, pattern) => { + // None of these patterns occur literally in the span, so a match proves the + // regex was interpreted. `^Agent` anchors against the name field, the rest + // against output — hence the field-agnostic assertion. + const spans = [span({ output: { v: 'called api.openai.com -> status=503 on 2026-01-01' } })] + const result = await grepSpans(spans, pattern, ctx) + expect(result.matches.length).toBeGreaterThan(0) + expect(result.patternNotice).toBeUndefined() + }) + + it('falls back to a literal, with a notice, for syntax RE2 does not implement', async () => { + const spans = [span({ output: { v: 'id: abc and (?=x) literally here' } })] + + const lookahead = await grepSpans(spans, '(?=x)', ctx) + expect(lookahead.matches.some((m) => m.field === 'output')).toBe(true) + expect(lookahead.patternNotice).toContain('RE2') + + // Unbalanced paren is invalid in both engines; still degrades to a literal. + const invalid = await grepSpans([span({ output: { v: 'value a(b' } })], '(', ctx) + expect(invalid.matches.some((m) => m.field === 'output')).toBe(true) + expect(invalid.patternNotice).toContain('RE2') }) - it.each(['example.com', 'file.pdf', 'v1.2.3', 'block_1.output', '$0.42', 'why?'])( - 'does not warn about regex on the ordinary literal %s', - async (pattern) => { - const spans = [span({ output: { v: `saw ${pattern} here` } })] - const result = await grepSpans(spans, pattern, ctx) - expect(result.matches.some((m) => m.field === 'output')).toBe(true) - expect(result.patternNotice).toBeUndefined() - } - ) - - it('does not interpret a regex pattern, and says so', async () => { - const spans = [span({ output: { v: 'status=503' } })] - - // Literal 'status=\d+' is not present; the regex it denotes would have matched. - const asRegex = await grepSpans(spans, 'status=\\d+', ctx) - expect(asRegex.matches).toEqual([]) - expect(asRegex.patternNotice).toContain('literal') - - const asLiteral = await grepSpans(spans, 'status=503', ctx) - expect(asLiteral.matches.some((m) => m.field === 'output')).toBe(true) - expect(asLiteral.patternNotice).toBeUndefined() + it('takes the built-in engine for a metacharacter-free pattern', async () => { + const spans = [span({ output: { v: 'saw ECONNREFUSED here' } })] + const result = await grepSpans(spans, 'ECONNREFUSED', ctx) + expect(result.matches.some((m) => m.field === 'output')).toBe(true) + expect(result.patternNotice).toBeUndefined() }) it.each([ ['nested quantifier', '(a+)+$'], ['duplicate alternation, passes safe-regex2', '(a|a)*b'], ['adjacent quantifiers, passes every structural screen', 'a*a*b'], - ])('never executes a catastrophic pattern (%s)', async (_label, pattern) => { - // Each of these blocks the event loop for minutes if compiled as a regex: + ])('runs a catastrophic pattern in linear time (%s)', async (_label, pattern) => { + // Each blocks the event loop for minutes on a backtracking engine: // `a*a*b` measured 213s on JSC / 132s on V8 over a 10k-character run. - const spans = [span({ output: { v: `${'a'.repeat(5000)}!` } })] + // RE2 has no backtracking, so these are matched normally and stay fast. + const spans = [span({ output: { v: `${'a'.repeat(10000)}!` } })] const start = Date.now() const result = await grepSpans(spans, pattern, ctx) const elapsedMs = Date.now() - start expect(elapsedMs).toBeLessThan(1000) - expect(result.matches).toEqual([]) + expect(result.truncated).toBe(false) }) it('matches a long pattern literally without a length cap', async () => { diff --git a/apps/sim/lib/logs/log-views.ts b/apps/sim/lib/logs/log-views.ts index 277db8369a8..176d20ad023 100644 --- a/apps/sim/lib/logs/log-views.ts +++ b/apps/sim/lib/logs/log-views.ts @@ -1,3 +1,4 @@ +import { RE2JS } from 're2js' import { materializeLargeArrayManifest, readLargeArrayManifestSlice, @@ -26,17 +27,21 @@ const DEFAULT_MAX_SLICES_SCANNED = 200 /** * Cumulative time the pattern itself may spend matching, across all spans/fields. * - * Deliberately counts only time inside `test`/`exec`, not the grep's wall clock: - * the scan awaits blob-store reads (array slices, large-value refs) between - * matches, and charging that I/O to the budget would truncate slow-but-legitimate - * greps under load. Matching is the only part that occupies the event loop, so - * it is the only part worth bounding. + * Deliberately counts only time spent matching, not the grep's wall clock: the + * scan awaits blob-store reads (array slices, large-value refs) between matches, + * and charging that I/O to the budget would truncate slow-but-legitimate greps + * under load. Matching is the only part that occupies the event loop, so it is + * the only part worth bounding. + * + * RE2JS trades throughput for its linear-time guarantee — roughly 100x slower + * than the built-in engine, ~25ms per megabyte — so on a very large trace this + * budget is what actually caps the scan rather than a formality. */ const DEFAULT_MATCH_TIME_BUDGET_MS = 5_000 /** - * Total characters a single grep may run the pattern over. A backstop against a - * pattern that slips past the backtracking screen being amplified across every - * slice — set well above any realistic trace so normal greps never trip it. + * Total characters a single grep may run the pattern over. Bounds the work one + * request can demand across every span and slice; set well above any realistic + * trace so normal greps never trip it. */ const DEFAULT_MAX_SCANNED_CHARS = 64 * 1024 * 1024 @@ -196,10 +201,10 @@ export interface GrepSpansResult { */ truncated: boolean /** - * Present when the pattern contained regex syntax, which is matched literally. - * The tool catalog cannot say so up front — it is generated from a contract in - * another repository — so the caller is told here instead of silently reading - * zero matches as "not present in the trace". + * Present only when the pattern used syntax RE2 does not implement and was + * therefore matched literally. The tool catalog cannot warn up front — it is + * generated from a contract in another repository — so the caller is told + * here rather than reading zero matches as "not present in the trace". */ patternNotice?: string } @@ -223,67 +228,84 @@ interface GrepState { maxSlicesScanned: number maxScannedChars: number matchTimeBudgetMs: number - regex: RegExp + find: FindMatch } function escapeRegExp(input: string): string { return input.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') } -/** - * Signals that the caller wrote a pattern *intending* regex semantics: escape - * classes, a character class, a group, alternation, a leading/trailing anchor, - * or a repetition quantifier. - * - * Deliberately narrower than the set `escapeRegExp` escapes. A bare `.` or `?` - * is ordinary text in the things people actually grep for — `example.com`, - * `file.pdf`, `v1.2.3`, `block_1.output` — and flagging those would tell the - * caller its correct search was misinterpreted, prompting a pointless retry. - */ -const REGEX_INTENT = /\\[dwsbDWSB]|[[\]()|*+]|^\^|\$$|\{\d+,?\d*\}/ +/** Any regex metacharacter — a pattern without one behaves the same either way. */ +const REGEX_METACHARACTERS = /[.*+?^${}()|[\]\\]/ + +/** Index of the first case-insensitive match in `text`, or -1. */ +type FindMatch = (text: string) => number + +/** Escaped-literal search on the built-in engine: linear, and allocation-free. */ +function literalFinder(pattern: string): FindMatch { + const regex = new RegExp(escapeRegExp(pattern), 'i') + return (text) => { + const match = regex.exec(text) + return match ? match.index : -1 + } +} /** - * Compile a caller-supplied grep pattern as a case-insensitive literal. + * Compile a caller-supplied grep pattern into a matcher that cannot backtrack. * - * Caller patterns are deliberately never executed as regexes. Trace text is - * attacker-influenced — a workflow can emit arbitrarily long uniform runs into - * its own block outputs — and matching runs synchronously on the shared event - * loop, so a backtracking pattern stalls every request on the instance. + * Trace text is attacker-influenced — a workflow can emit arbitrarily long + * uniform runs into its own block outputs — and matching runs synchronously on + * the shared event loop, so a backtracking engine lets one request stall every + * other request on the instance. RE2JS is a port of RE2: it matches in time + * linear in the input, so no pattern can blow up regardless of the text. * - * Screening was tried and abandoned: `safe-regex2` (which the guardrails - * validator uses) documents itself as having false negatives and passes - * `(a|a)*b`; rejecting quantified groups on top of it still passes `a*a*b`, - * measured at 213s on JSC and 132s on V8 over a 10k-character run. Each rule - * only excludes the shapes someone thought to enumerate. An escaped literal is - * linear on every engine, which is the one property worth relying on here. + * Screening the pattern instead was tried and abandoned. `safe-regex2` (used by + * the guardrails validator) documents itself as having false negatives and + * passes `(a|a)*b`; rejecting quantified groups on top of it still passes + * `a*a*b`, measured at 213s on JSC and 132s on V8 over a 10k-character run. + * Every syntactic rule only excludes the shapes someone thought to enumerate, + * which is why the engine changed instead of the filter. + * + * Two escape hatches keep the common path fast and the rare one honest: + * a pattern with no metacharacter takes the built-in engine (identical + * semantics, ~100x quicker), and syntax RE2 does not implement — lookaround, + * backreferences — falls back to a literal with a notice, since RE2JS rejects + * those at compile time rather than matching them. */ -function compilePattern(pattern: string): { regex: RegExp; notice?: string } { - const regex = new RegExp(escapeRegExp(pattern), 'i') - if (!REGEX_INTENT.test(pattern)) return { regex } - return { - regex, - notice: - 'Matched as a literal, case-insensitive substring — regex syntax is not interpreted. Search for the literal text you expect to see in the trace.', +function compilePattern(pattern: string): { find: FindMatch; notice?: string } { + if (!REGEX_METACHARACTERS.test(pattern)) return { find: literalFinder(pattern) } + + try { + const compiled = RE2JS.compile(pattern, RE2JS.CASE_INSENSITIVE) + return { + find: (text) => { + const matcher = compiled.matcher(text) + return matcher.find() ? matcher.start() : -1 + }, + } + } catch { + return { + find: literalFinder(pattern), + notice: + 'Pattern is not valid RE2 syntax (lookahead, lookbehind and backreferences are unsupported), so it was matched as a literal string. Rewrite it without those constructs to search by regex.', + } } } /** - * Charge the elapsed matching time to the grep's budget. Every `test`/`exec` on - * caller-supplied patterns goes through here. + * Run the pattern over `text`, charging the elapsed matching time to the grep's + * budget. Every match on a caller-supplied pattern goes through here. */ -function runTimed(state: GrepState, match: (regex: RegExp) => T): T { +function findTimed(text: string, state: GrepState): number { const started = performance.now() try { - state.regex.lastIndex = 0 - return match(state.regex) + return state.find(text) } finally { state.matchTimeMs += performance.now() - started } } -function snippetAround(text: string, state: GrepState): string { - const m = runTimed(state, (regex) => regex.exec(text)) - const index = m ? m.index : 0 +function snippetAround(text: string, index: number, state: GrepState): string { const maxChars = state.maxSnippetChars const half = Math.floor(maxChars / 2) const start = Math.max(0, index - half) @@ -314,13 +336,14 @@ function recordIfMatch( return } state.scannedChars += text.length - if (!runTimed(state, (regex) => regex.test(text))) return + const index = findTimed(text, state) + if (index < 0) return state.matches.push({ spanId: span.id, blockId: span.blockId, name: span.name, field, - snippet: snippetAround(text, state), + snippet: snippetAround(text, index, state), }) if (state.matches.length >= state.maxMatches) state.truncated = true } @@ -394,12 +417,11 @@ function safeStringify(value: unknown): string { * next); single large refs are materialized under a byte cap (falling back to * the ref preview). Only bounded match snippets are accumulated. * - * `pattern` is matched as a literal substring, never as a regex — see - * `compilePattern`. Two budgets bound the scan on top of that: a total - * character budget and a cumulative match-time budget. Neither counts the - * blob-store I/O this scan awaits, so a slow-but-legitimate grep is not - * truncated for being slow. With literal matching both are backstops rather - * than load-bearing, and they stay to bound total work per request. + * `pattern` is matched by a non-backtracking engine — see `compilePattern` — so + * no pattern can blow up on any input. Two budgets bound total work on top of + * that: a character budget and a cumulative match-time budget. Neither counts + * the blob-store I/O this scan awaits, so a slow-but-legitimate grep is not + * truncated merely for being slow. */ export async function grepSpans( spans: TraceSpan[], @@ -419,7 +441,7 @@ export async function grepSpans( maxSlicesScanned: opts?.maxSlicesScanned ?? DEFAULT_MAX_SLICES_SCANNED, maxScannedChars: opts?.maxScannedChars ?? DEFAULT_MAX_SCANNED_CHARS, matchTimeBudgetMs: opts?.matchTimeBudgetMs ?? DEFAULT_MATCH_TIME_BUDGET_MS, - regex: compiled.regex, + find: compiled.find, } const walk = async (list: TraceSpan[]): Promise => { diff --git a/apps/sim/package.json b/apps/sim/package.json index b0205c85f7f..533de127400 100644 --- a/apps/sim/package.json +++ b/apps/sim/package.json @@ -194,6 +194,7 @@ "posthog-node": "5.28.9", "pptxgenjs": "4.0.1", "prismjs": "^1.30.0", + "re2js": "2.8.6", "react": "19.2.4", "react-dom": "19.2.4", "react-hook-form": "^7.54.2", diff --git a/bun.lock b/bun.lock index 20cc0d013c2..6733be244ec 100644 --- a/bun.lock +++ b/bun.lock @@ -269,6 +269,7 @@ "posthog-node": "5.28.9", "pptxgenjs": "4.0.1", "prismjs": "^1.30.0", + "re2js": "2.8.6", "react": "19.2.4", "react-dom": "19.2.4", "react-hook-form": "^7.54.2", @@ -3598,6 +3599,8 @@ "rc9": ["rc9@2.1.2", "", { "dependencies": { "defu": "^6.1.4", "destr": "^2.0.3" } }, "sha512-btXCnMmRIBINM2LDZoEmOogIZU7Qe7zn4BpomSKZ/ykbLObuBdvG+mFq11DL6fjH1DRwHhrlgtYWG96bJiC7Cg=="], + "re2js": ["re2js@2.8.6", "", {}, "sha512-xLgQil4kIUCrAzVk9fRSkxkFNwmygLFjVxXrLc65aE1F0+Zsb8rxumFBy4XKyvgMCTL6kilDq3EZ0piE2dP/Dg=="], + "react": ["react@19.2.4", "", {}, "sha512-9nfp2hYpCwOjAN+8TZFGhtWEwgvWHXqESH8qT89AT/lWklpLON22Lc8pEtnpsZz7VmawabSU0gCjnj8aC0euHQ=="], "react-dom": ["react-dom@19.2.4", "", { "dependencies": { "scheduler": "^0.27.0" }, "peerDependencies": { "react": "^19.2.4" } }, "sha512-AXJdLo8kgMbimY95O2aKQqsz2iWi9jMgKJhRBAxECE4IFxfcazB2LmzloIoibJI3C12IlY20+KFaLv+71bUJeQ=="], From 0b5136dbfe1dcd3e6ea2b5f2f49eb4ee37765e50 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 27 Jul 2026 16:34:14 -0700 Subject: [PATCH 4/9] fix(security): route every caller-supplied regex through the linear-time engine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the RE2 fix to the three other places that compiled a caller-supplied pattern and ran it on the shared event loop, behind one shared helper — `lib/core/security/linear-regex.ts` — rather than four local copies. - `lib/copilot/vfs/operations.ts` — copilot VFS grep took a caller pattern over caller-supplied file content with no screen at all. - `lib/guardrails/validate_regex.ts` (`validateRegex`) — a guardrail rule's pattern matched against LLM output, screened only by `safe-regex2`. - `lib/chunkers/regex-chunker.ts` — the KB chunker's split pattern. This one screened for catastrophic backtracking by running the pattern against six probes including `'a'.repeat(10000)` and rejecting anything over 50ms — but it measured elapsed time *after* the match returned, so the screen was the denial of service it existed to prevent (`a*a*b` on that probe: 213s). The probe is deleted rather than repaired. Deliberately unchanged: `validateRegexPattern`. Those patterns are persisted for Presidio, a separate service where a slow pattern times out instead of stalling this loop, and Presidio's Python engine accepts constructs RE2 does not — so narrowing it to the RE2 subset would reject patterns that work. Its TSDoc now says plainly that its `safe-regex2` screen is a courtesy, not a ReDoS defense, and points in-process callers at `compileLinearRegex`. `safe-regex2` therefore stays a dependency, used only there. Lookaround is the standard way to split while keeping the delimiter (`(?=#\s)`) and RE2 cannot represent it, so the chunker keeps the built-in engine for those patterns — the only remaining path that can backtrack, bounded by the existing 500-character cap. RE2JS `split` was verified identical to `String.split` across six representative chunking patterns. Adds 27 tests for the shared helper (every catastrophic pattern asserted linear over a 50k-character input) plus regression tests at each site, plus the first tests for `validateRegex`. Also removes the now-duplicated `escapeRegExp` and literal-finder locals from `log-views.ts`. --- apps/sim/lib/chunkers/regex-chunker.test.ts | 34 +++++++ apps/sim/lib/chunkers/regex-chunker.ts | 64 ++++++------ apps/sim/lib/copilot/vfs/operations.test.ts | 32 ++++++ apps/sim/lib/copilot/vfs/operations.ts | 21 ++-- .../lib/core/security/linear-regex.test.ts | 98 +++++++++++++++++++ apps/sim/lib/core/security/linear-regex.ts | 91 +++++++++++++++++ .../sim/lib/guardrails/validate_regex.test.ts | 64 ++++++++++++ apps/sim/lib/guardrails/validate_regex.ts | 43 +++++--- apps/sim/lib/logs/log-views.ts | 65 +++--------- 9 files changed, 412 insertions(+), 100 deletions(-) create mode 100644 apps/sim/lib/core/security/linear-regex.test.ts create mode 100644 apps/sim/lib/core/security/linear-regex.ts create mode 100644 apps/sim/lib/guardrails/validate_regex.test.ts diff --git a/apps/sim/lib/chunkers/regex-chunker.test.ts b/apps/sim/lib/chunkers/regex-chunker.test.ts index f4f55112e55..c5da910fb36 100644 --- a/apps/sim/lib/chunkers/regex-chunker.test.ts +++ b/apps/sim/lib/chunkers/regex-chunker.test.ts @@ -387,5 +387,39 @@ describe('RegexChunker', () => { ) } }) + it.concurrent( + 'chunks with a catastrophic pattern without hanging', + async () => { + // `a*a*b` defeats every syntactic backtracking screen. The previous + // guard probed it against 'a'.repeat(10000) and measured the elapsed + // time only after the match returned, so the guard itself hung for + // ~213s. RE2 has no backtracking, so this simply completes. + const chunker = new RegexChunker({ pattern: 'a*a*b', chunkSize: 100, chunkOverlap: 0 }) + + const start = Date.now() + await chunker.chunk(`${'a'.repeat(10000)}!`) + + expect(Date.now() - start).toBeLessThan(2000) + }, + 10000 + ) + + it.concurrent('still supports lookahead split patterns via the built-in engine', async () => { + // RE2 cannot represent lookaround, and splitting *before* a delimiter so + // it is kept is the standard use for it, so these must keep working. + // (No `m` flag is applied, so anchor the lookahead on the delimiter + // itself rather than on `^`.) + const chunker = new RegexChunker({ + pattern: '(?=#\\s)', + chunkSize: 1024, + chunkOverlap: 0, + strictBoundaries: true, + }) + const chunks = await chunker.chunk('# One\nalpha\n# Two\nbeta') + + expect(chunks.length).toBe(2) + expect(chunks[0].text).toContain('# One') + expect(chunks[1].text).toContain('# Two') + }) }) }) diff --git a/apps/sim/lib/chunkers/regex-chunker.ts b/apps/sim/lib/chunkers/regex-chunker.ts index 0cafa47b0d3..c9d384b04c3 100644 --- a/apps/sim/lib/chunkers/regex-chunker.ts +++ b/apps/sim/lib/chunkers/regex-chunker.ts @@ -10,6 +10,7 @@ import { splitAtWordBoundaries, tokensToChars, } from '@/lib/chunkers/utils' +import { compileLinearRegex, type LinearRegex } from '@/lib/core/security/linear-regex' const logger = createLogger('RegexChunker') @@ -56,7 +57,7 @@ function toNonCapturing(pattern: string): string { export class RegexChunker { private readonly chunkSize: number private readonly chunkOverlap: number - private readonly regex: RegExp + private readonly regex: LinearRegex private readonly strictBoundaries: boolean constructor(options: RegexChunkerOptions) { @@ -67,7 +68,22 @@ export class RegexChunker { this.strictBoundaries = options.strictBoundaries ?? false } - private compilePattern(pattern: string): RegExp { + /** + * Compile the caller's split pattern on an engine that cannot backtrack. + * + * This previously screened for catastrophic backtracking by running the + * pattern against six probe strings — including `'a'.repeat(10000)` — and + * rejecting anything slower than 50ms. It measured the elapsed time *after* + * the match returned, so the screen was the denial of service it existed to + * prevent: `a*a*b` against that probe measured 213s on JSC. RE2 removes the + * failure mode outright, so the probe is gone rather than repaired. + * + * Lookaround is the standard way to split while keeping the delimiter + * (`(?=^#\s)`), and RE2 does not implement it, so those patterns still use + * the built-in engine. They are the only ones that can backtrack now, and the + * length cap is the sole remaining bound on them. + */ + private compilePattern(pattern: string): LinearRegex { if (!pattern) { throw new Error('Regex pattern is required') } @@ -76,33 +92,26 @@ export class RegexChunker { throw new Error(`Regex pattern exceeds maximum length of ${MAX_PATTERN_LENGTH} characters`) } + const source = toNonCapturing(pattern) + + const linear = compileLinearRegex(source) + if (linear) return linear + try { - const regex = new RegExp(toNonCapturing(pattern), 'g') - - const testStrings = [ - 'a'.repeat(10000), - ' '.repeat(10000), - 'a '.repeat(5000), - 'aB1 xY2\n'.repeat(1250), - `${'a'.repeat(30)}!`, - `${'a b '.repeat(25)}!`, - ] - for (const testStr of testStrings) { - regex.lastIndex = 0 - const start = Date.now() - regex.test(testStr) - const elapsed = Date.now() - start - if (elapsed > 50) { - throw new Error('Regex pattern appears to have catastrophic backtracking') - } + const fallback = new RegExp(source, 'g') + return { + test: (text) => { + fallback.lastIndex = 0 + return fallback.test(text) + }, + find: (text) => { + fallback.lastIndex = 0 + const match = fallback.exec(text) + return match ? match.index : -1 + }, + split: (text) => text.split(new RegExp(source, 'g')), } - - regex.lastIndex = 0 - return regex } catch (error) { - if (error instanceof Error && error.message.includes('catastrophic')) { - throw error - } throw new Error(`Invalid regex pattern "${pattern}": ${toError(error).message}`) } } @@ -119,8 +128,7 @@ export class RegexChunker { return buildChunks([cleaned], 0) } - this.regex.lastIndex = 0 - const segments = cleaned.split(this.regex).filter((s) => s.trim().length > 0) + const segments = this.regex.split(cleaned).filter((s) => s.trim().length > 0) if (segments.length <= 1) { if (this.strictBoundaries) { diff --git a/apps/sim/lib/copilot/vfs/operations.test.ts b/apps/sim/lib/copilot/vfs/operations.test.ts index 32d1ad39007..e626b702842 100644 --- a/apps/sim/lib/copilot/vfs/operations.test.ts +++ b/apps/sim/lib/copilot/vfs/operations.test.ts @@ -138,3 +138,35 @@ describe('grep', () => { expect(hits).toHaveLength(1) }) }) + +describe('grep regex safety', () => { + it('runs a catastrophic pattern in linear time', () => { + // `a*a*b` takes minutes on a backtracking engine against this content; + // both the pattern and the file content are caller-supplied. + const files = vfsFromEntries([['notes.md', `${'a'.repeat(10000)}!`]]) + + const start = Date.now() + grep(files, 'a*a*b') + + expect(Date.now() - start).toBeLessThan(2000) + }) + + it('still interprets regex syntax', () => { + const files = vfsFromEntries([['log.txt', 'req finished status=503']]) + expect(grep(files, 'status=\\d+')).toHaveLength(1) + expect(grep(files, 'status=\\d+', undefined, { outputMode: 'count' })).toEqual([ + { path: 'log.txt', count: 1 }, + ]) + }) + + it('matches syntax RE2 cannot represent literally instead of not at all', () => { + const files = vfsFromEntries([['log.txt', 'contains (?=x) verbatim']]) + expect(grep(files, '(?=x)')).toHaveLength(1) + }) + + it('honours ignoreCase across repeated line tests', () => { + const files = vfsFromEntries([['log.txt', 'Alpha\nALPHA\nalpha']]) + expect(grep(files, 'alpha', undefined, { ignoreCase: true })).toHaveLength(3) + expect(grep(files, 'alpha')).toHaveLength(1) + }) +}) diff --git a/apps/sim/lib/copilot/vfs/operations.ts b/apps/sim/lib/copilot/vfs/operations.ts index bd7208719b8..2f2e1e8c7bb 100644 --- a/apps/sim/lib/copilot/vfs/operations.ts +++ b/apps/sim/lib/copilot/vfs/operations.ts @@ -1,4 +1,5 @@ import micromatch from 'micromatch' +import { compileLinearRegex, isPlainText, literalRegex } from '@/lib/core/security/linear-regex' export interface GrepMatch { path: string @@ -125,7 +126,9 @@ export function pathWithinGrepScope(filePath: string, scope: string): boolean { } /** - * Regex search over VFS file contents using ECMAScript `RegExp` syntax. + * Regex search over VFS file contents using RE2 syntax — a subset of + * ECMAScript `RegExp` without lookaround or backreferences, which are matched + * literally instead (see `@/lib/core/security/linear-regex`). * `content` and `count` are line-oriented (split on newline, CR stripped per line). * `files_with_matches` tests the entire file string once, so multiline patterns can match there * but not in line modes. @@ -142,19 +145,17 @@ export function grep( const showLineNumbers = opts?.lineNumbers ?? true const contextLines = opts?.context ?? 0 - const flags = ignoreCase ? 'gi' : 'g' - let regex: RegExp - try { - regex = new RegExp(pattern, flags) - } catch { - return [] - } + // Caller-supplied pattern over caller-supplied file content on the shared + // event loop — matched by RE2 so it cannot backtrack. Syntax RE2 cannot + // represent degrades to a literal rather than to the backtracking engine. + const regex = isPlainText(pattern) + ? literalRegex(pattern, { ignoreCase }) + : (compileLinearRegex(pattern, { ignoreCase }) ?? literalRegex(pattern, { ignoreCase })) if (outputMode === 'files_with_matches') { const matchingFiles: string[] = [] for (const [filePath, content] of files) { if (path && !pathWithinGrepScope(filePath, path)) continue - regex.lastIndex = 0 if (regex.test(content)) { matchingFiles.push(filePath) if (matchingFiles.length >= maxResults) break @@ -170,7 +171,6 @@ export function grep( const lines = splitLinesForGrep(content) let count = 0 for (const line of lines) { - regex.lastIndex = 0 if (regex.test(line)) count++ } if (count > 0) { @@ -188,7 +188,6 @@ export function grep( const lines = splitLinesForGrep(content) for (let i = 0; i < lines.length; i++) { - regex.lastIndex = 0 if (regex.test(lines[i])) { if (contextLines > 0) { const start = Math.max(0, i - contextLines) diff --git a/apps/sim/lib/core/security/linear-regex.test.ts b/apps/sim/lib/core/security/linear-regex.test.ts new file mode 100644 index 00000000000..70bccfcccfa --- /dev/null +++ b/apps/sim/lib/core/security/linear-regex.test.ts @@ -0,0 +1,98 @@ +/** + * @vitest-environment node + */ + +import { describe, expect, it } from 'vitest' +import { + compileLinearRegex, + escapeRegExp, + isPlainText, + literalRegex, +} from '@/lib/core/security/linear-regex' + +/** + * Patterns that take exponential time on a backtracking engine. `a*a*b` is the + * important one: it defeats `safe-regex2`'s star-height screen and a + * quantified-group screen alike, and measured 213s on JSC / 132s on V8 against + * the input below. + */ +const CATASTROPHIC = ['(a+)+$', '(a|a)*b', 'a*a*b', '(x+x+)+y', '(\\w+\\s?)*$', '^(\\d+)*$'] + +describe('compileLinearRegex', () => { + it.each(CATASTROPHIC)('matches %s in linear time on adversarial input', (pattern) => { + const regex = compileLinearRegex(pattern) + expect(regex).not.toBeNull() + + const adversarial = `${'a'.repeat(50000)}!` + const start = Date.now() + regex?.test(adversarial) + expect(Date.now() - start).toBeLessThan(2000) + }) + + it('interprets regex syntax rather than matching it literally', () => { + const regex = compileLinearRegex('status=\\d+') + expect(regex?.test('http status=503 here')).toBe(true) + expect(regex?.test('status=abc')).toBe(false) + expect(regex?.find('http status=503')).toBe(5) + }) + + it('honours ignoreCase only when asked', () => { + expect(compileLinearRegex('ERROR', { ignoreCase: true })?.test('an error here')).toBe(true) + expect(compileLinearRegex('ERROR')?.test('an error here')).toBe(false) + }) + + it('splits equivalently to String.prototype.split', () => { + const doc = '# One\ntext a\n\n# Two\ntext b' + expect(compileLinearRegex('\\n\\n+')?.split(doc)).toEqual(doc.split(/\n\n+/g)) + }) + + it.each([ + ['lookahead', '(?=foo)bar'], + ['lookbehind', '(?<=id: )\\w+'], + ['backreference', '(ab)\\1'], + ['invalid syntax', '('], + ])('returns null for %s so the caller must choose how to degrade', (_label, pattern) => { + expect(compileLinearRegex(pattern)).toBeNull() + }) + + it('returns -1 from find when there is no match', () => { + expect(compileLinearRegex('zzz')?.find('abc')).toBe(-1) + }) +}) + +describe('literalRegex', () => { + it('treats regex syntax as ordinary characters', () => { + const regex = literalRegex('a+b') + expect(regex.test('xxa+bxx')).toBe(true) + expect(regex.test('aaab')).toBe(false) + }) + + it('is unaffected by repeated calls (no lastIndex carry-over)', () => { + const regex = literalRegex('needle') + const text = 'needle here and needle again' + expect([regex.test(text), regex.test(text), regex.test(text)]).toEqual([true, true, true]) + expect([regex.find(text), regex.find(text)]).toEqual([0, 0]) + }) + + it('matches case-insensitively when asked', () => { + expect(literalRegex('Needle', { ignoreCase: true }).test('a NEEDLE')).toBe(true) + expect(literalRegex('Needle').test('a NEEDLE')).toBe(false) + }) +}) + +describe('isPlainText / escapeRegExp', () => { + it.each(['timeout', 'ECONNREFUSED', 'status=503', 'GET /api/logs'])( + 'treats %s as plain text', + (pattern) => expect(isPlainText(pattern)).toBe(true) + ) + + it.each(['example.com', 'a+b', '^x', '(a|b)', '[abc]'])( + 'treats %s as containing metacharacters', + (pattern) => expect(isPlainText(pattern)).toBe(false) + ) + + it('escapes every metacharacter so the pattern matches only itself', () => { + const raw = 'a.*+?^${}()|[]\\b' + expect(new RegExp(escapeRegExp(raw)).test(raw)).toBe(true) + }) +}) diff --git a/apps/sim/lib/core/security/linear-regex.ts b/apps/sim/lib/core/security/linear-regex.ts new file mode 100644 index 00000000000..efb2cd6db12 --- /dev/null +++ b/apps/sim/lib/core/security/linear-regex.ts @@ -0,0 +1,91 @@ +import { RE2JS } from 're2js' + +/** + * Linear-time matching for caller-supplied regex patterns. + * + * The built-in engine backtracks, so a pattern chosen by a caller can take + * exponential time on input the same caller controls — `a*a*b` against a 10k + * run of `a` measured 213s on JSC and 132s on V8. Anywhere that runs on a + * shared event loop, that is a denial of service against every other tenant. + * + * Screening the pattern instead does not work. `safe-regex2` documents itself + * as having false negatives and passes `(a|a)*b`; rejecting quantified groups + * on top of it still passes `a*a*b`. Every syntactic rule only excludes the + * shapes someone thought to enumerate, so the engine has to change instead. + * + * RE2 has no backtracking and matches in time linear in the input. The cost is + * throughput (~100x the built-in engine, roughly 25ms/MB) and syntax: RE2 + * implements neither lookaround nor backreferences, so `compileLinearRegex` + * returns `null` for those and each caller decides how to degrade. + */ +export interface LinearRegexOptions { + ignoreCase?: boolean +} + +export interface LinearRegex { + /** Whether the pattern matches anywhere in `text`. */ + test(text: string): boolean + /** Index of the first match in `text`, or -1. */ + find(text: string): number + /** Split `text` around every match, like `String.prototype.split(regex)`. */ + split(text: string): string[] +} + +/** Escape every regex metacharacter so `input` matches only itself. */ +export function escapeRegExp(input: string): string { + return input.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') +} + +/** True when `pattern` has no metacharacter, so both engines behave identically. */ +export function isPlainText(pattern: string): boolean { + return !/[.*+?^${}()|[\]\\]/.test(pattern) +} + +/** + * Match `pattern` as an escaped literal on the built-in engine. + * + * Safe because an escaped literal cannot backtrack, and ~100x quicker than RE2 + * — worth taking whenever the pattern has no metacharacter to interpret, or as + * a degradation path when RE2 rejects the syntax. + */ +export function literalRegex(pattern: string, options: LinearRegexOptions = {}): LinearRegex { + const flags = options.ignoreCase ? 'gi' : 'g' + const source = escapeRegExp(pattern) + const at = (text: string): number => { + const regex = new RegExp(source, flags) + const match = regex.exec(text) + return match ? match.index : -1 + } + return { + test: (text) => at(text) >= 0, + find: at, + split: (text) => text.split(new RegExp(source, flags)), + } +} + +/** + * Compile `pattern` into a matcher that cannot backtrack. + * + * Returns `null` when RE2 cannot represent the pattern — invalid syntax, or the + * lookaround and backreference constructs RE2 does not implement. Callers must + * handle `null` explicitly rather than silently falling back to the built-in + * engine, which would reintroduce the exposure this exists to remove. + */ +export function compileLinearRegex( + pattern: string, + options: LinearRegexOptions = {} +): LinearRegex | null { + try { + const compiled = RE2JS.compile(pattern, options.ignoreCase ? RE2JS.CASE_INSENSITIVE : 0) + return { + test: (text) => compiled.matcher(text).find(), + find: (text) => { + const matcher = compiled.matcher(text) + return matcher.find() ? matcher.start() : -1 + }, + split: (text) => compiled.split(text), + } + } catch { + return null + } +} diff --git a/apps/sim/lib/guardrails/validate_regex.test.ts b/apps/sim/lib/guardrails/validate_regex.test.ts new file mode 100644 index 00000000000..78feb044aec --- /dev/null +++ b/apps/sim/lib/guardrails/validate_regex.test.ts @@ -0,0 +1,64 @@ +/** + * @vitest-environment node + */ + +import { describe, expect, it } from 'vitest' +import { validateRegex, validateRegexPattern } from '@/lib/guardrails/validate_regex' + +describe('validateRegex', () => { + it('passes when the input matches', () => { + expect(validateRegex('order 12345 shipped', '\\d{5}')).toEqual({ passed: true }) + }) + + it('fails with a reason when the input does not match', () => { + expect(validateRegex('no digits', '\\d{5}')).toEqual({ + passed: false, + error: 'Input does not match regex pattern', + }) + }) + + it('runs a catastrophic pattern in linear time', () => { + // Both the guardrail pattern and the text it checks are caller-influenced, + // and this executes on the shared event loop. `a*a*b` against this input + // measured 213s on JSC before the engine change. + const start = Date.now() + const result = validateRegex(`${'a'.repeat(10000)}!`, 'a*a*b') + + expect(Date.now() - start).toBeLessThan(2000) + expect(result.passed).toBe(false) + }) + + it('reports syntax RE2 cannot evaluate rather than running it', () => { + const result = validateRegex('anything', '(?=foo)bar') + expect(result.passed).toBe(false) + expect(result.error).toContain('lookahead') + }) + + it('reports invalid syntax distinctly from unsupported syntax', () => { + const result = validateRegex('anything', '(') + expect(result.passed).toBe(false) + expect(result.error).toContain('Invalid regex pattern') + }) +}) + +describe('validateRegexPattern', () => { + it('accepts a valid pattern', () => { + expect(validateRegexPattern('\\d{3}-\\d{4}')).toEqual({ valid: true }) + }) + + it('rejects an empty pattern', () => { + expect(validateRegexPattern('')).toMatchObject({ valid: false }) + }) + + it('rejects invalid syntax', () => { + expect(validateRegexPattern('(')).toMatchObject({ valid: false }) + }) + + it('is unchanged by the RE2 migration — patterns here go to Presidio, not this process', () => { + // Left on `safe-regex2` deliberately: these patterns execute in Presidio, + // where a slow one times out rather than stalling this event loop, and + // Presidio's Python engine supports constructs RE2 does not. + expect(validateRegexPattern('(?:https?://)?example\\.com')).toMatchObject({ valid: false }) + expect(validateRegexPattern('(?<=id: )\\w+')).toMatchObject({ valid: false }) + }) +}) diff --git a/apps/sim/lib/guardrails/validate_regex.ts b/apps/sim/lib/guardrails/validate_regex.ts index 442daa7b3ee..189310d2e69 100644 --- a/apps/sim/lib/guardrails/validate_regex.ts +++ b/apps/sim/lib/guardrails/validate_regex.ts @@ -1,4 +1,6 @@ +import { getErrorMessage } from '@sim/utils/errors' import safe from 'safe-regex2' +import { compileLinearRegex } from '@/lib/core/security/linear-regex' /** * Validate if input matches regex pattern @@ -15,9 +17,19 @@ export interface RegexPatternValidation { } /** - * Validate a regex pattern's syntax and safety without matching it against input: - * it must compile (`new RegExp`) and pass `safe-regex2`'s catastrophic-backtracking - * screen. Shared by the custom-pattern editor UI and any pre-flight boundary check. + * Validate a PII custom pattern's syntax before it is persisted and handed to + * Presidio. Shared by the custom-pattern editor UI and the write boundary. + * + * The `safe-regex2` screen here is a courtesy, NOT a ReDoS defense: it screens + * star height only and is documented as having false negatives — it passes + * `(a|a)*b`, and `a*a*b` defeats every syntactic rule of this kind. It is kept + * because these patterns execute in Presidio, a separate service where a slow + * pattern times out and silently fails open (leaving PII unredacted) rather + * than stalling this event loop, and because Presidio's Python engine supports + * lookaround — so gating on RE2 here would reject patterns that work. + * + * Anything that matches a caller-supplied pattern *in this process* must use + * `compileLinearRegex` from `@/lib/core/security/linear-regex` instead. */ export function validateRegexPattern(pattern: string): RegexPatternValidation { if (pattern.length === 0) { @@ -37,23 +49,32 @@ export function validateRegexPattern(pattern: string): RegexPatternValidation { return { valid: true } } +/** + * Match `inputStr` against a caller-defined guardrail `pattern`. + * + * Both the pattern and the input are caller-influenced and this runs on the + * shared event loop, so matching goes through RE2 — a backtracking engine here + * lets one guardrail rule stall every other request on the instance. Patterns + * RE2 cannot represent (lookaround, backreferences) are reported rather than + * run on the built-in engine, which would reintroduce that exposure. + */ export function validateRegex(inputStr: string, pattern: string): ValidationResult { - let regex: RegExp try { - regex = new RegExp(pattern) - } catch (error: any) { - return { passed: false, error: `Invalid regex pattern: ${error.message}` } + new RegExp(pattern) + } catch (error) { + return { passed: false, error: `Invalid regex pattern: ${getErrorMessage(error)}` } } - if (!safe(pattern)) { + const regex = compileLinearRegex(pattern) + if (!regex) { return { passed: false, - error: 'Regex pattern rejected: potentially unsafe (catastrophic backtracking)', + error: + 'Regex pattern uses syntax that cannot be evaluated safely (lookahead, lookbehind and backreferences are unsupported). Rewrite it without those constructs.', } } - const match = regex.test(inputStr) - if (match) { + if (regex.test(inputStr)) { return { passed: true } } return { passed: false, error: 'Input does not match regex pattern' } diff --git a/apps/sim/lib/logs/log-views.ts b/apps/sim/lib/logs/log-views.ts index 176d20ad023..cbc1ac0f19a 100644 --- a/apps/sim/lib/logs/log-views.ts +++ b/apps/sim/lib/logs/log-views.ts @@ -1,4 +1,4 @@ -import { RE2JS } from 're2js' +import { compileLinearRegex, isPlainText, literalRegex } from '@/lib/core/security/linear-regex' import { materializeLargeArrayManifest, readLargeArrayManifestSlice, @@ -231,71 +231,36 @@ interface GrepState { find: FindMatch } -function escapeRegExp(input: string): string { - return input.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') -} - -/** Any regex metacharacter — a pattern without one behaves the same either way. */ -const REGEX_METACHARACTERS = /[.*+?^${}()|[\]\\]/ - /** Index of the first case-insensitive match in `text`, or -1. */ type FindMatch = (text: string) => number -/** Escaped-literal search on the built-in engine: linear, and allocation-free. */ -function literalFinder(pattern: string): FindMatch { - const regex = new RegExp(escapeRegExp(pattern), 'i') - return (text) => { - const match = regex.exec(text) - return match ? match.index : -1 - } -} - /** * Compile a caller-supplied grep pattern into a matcher that cannot backtrack. * * Trace text is attacker-influenced — a workflow can emit arbitrarily long * uniform runs into its own block outputs — and matching runs synchronously on * the shared event loop, so a backtracking engine lets one request stall every - * other request on the instance. RE2JS is a port of RE2: it matches in time - * linear in the input, so no pattern can blow up regardless of the text. + * other request on the instance. See `@/lib/core/security/linear-regex` for why + * the engine changed rather than the pattern being screened. * - * Screening the pattern instead was tried and abandoned. `safe-regex2` (used by - * the guardrails validator) documents itself as having false negatives and - * passes `(a|a)*b`; rejecting quantified groups on top of it still passes - * `a*a*b`, measured at 213s on JSC and 132s on V8 over a 10k-character run. - * Every syntactic rule only excludes the shapes someone thought to enumerate, - * which is why the engine changed instead of the filter. - * - * Two escape hatches keep the common path fast and the rare one honest: - * a pattern with no metacharacter takes the built-in engine (identical - * semantics, ~100x quicker), and syntax RE2 does not implement — lookaround, - * backreferences — falls back to a literal with a notice, since RE2JS rejects - * those at compile time rather than matching them. + * A pattern with no metacharacter takes the built-in engine, which is ~100x + * quicker and identical in meaning when there is nothing to interpret. Syntax + * RE2 cannot represent degrades to a literal with a notice, so the caller knows + * its regex was not applied instead of reading zero matches as "not present". */ function compilePattern(pattern: string): { find: FindMatch; notice?: string } { - if (!REGEX_METACHARACTERS.test(pattern)) return { find: literalFinder(pattern) } + if (isPlainText(pattern)) return { find: literalRegex(pattern, { ignoreCase: true }).find } - try { - const compiled = RE2JS.compile(pattern, RE2JS.CASE_INSENSITIVE) - return { - find: (text) => { - const matcher = compiled.matcher(text) - return matcher.find() ? matcher.start() : -1 - }, - } - } catch { - return { - find: literalFinder(pattern), - notice: - 'Pattern is not valid RE2 syntax (lookahead, lookbehind and backreferences are unsupported), so it was matched as a literal string. Rewrite it without those constructs to search by regex.', - } + const compiled = compileLinearRegex(pattern, { ignoreCase: true }) + if (compiled) return { find: compiled.find } + + return { + find: literalRegex(pattern, { ignoreCase: true }).find, + notice: + 'Pattern is not valid RE2 syntax (lookahead, lookbehind and backreferences are unsupported), so it was matched as a literal string. Rewrite it without those constructs to search by regex.', } } -/** - * Run the pattern over `text`, charging the elapsed matching time to the grep's - * budget. Every match on a caller-supplied pattern goes through here. - */ function findTimed(text: string, state: GrepState): number { const started = performance.now() try { From 5641a5b57b4921dae67ea29633194e8f696b6cb1 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 27 Jul 2026 16:47:28 -0700 Subject: [PATCH 5/9] fix(security): drop safe-regex2, keep lookaround splits on the linear engine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit follow-ups, plus the Cursor round-4 finding. Removes `safe-regex2` entirely. Its last caller was `validateRegexPattern`, the PII custom-pattern boundary, where it was pure cost: it screens star height alone and passes `(a|a)*b` and `a*a*b`, while rejecting patterns that work — lookbehind and optional groups could not be saved as custom PII rules at all. Nor could any screen be sound there: those patterns execute in Presidio's Python engine, which backtracks on shapes RE2 accepts, so RE2-representability says nothing about their runtime. Validation is now syntax-only and the dependency is gone. Cursor flagged the chunker's lookaround fallback as reintroducing backtracking, which was correct. The fallback is removed. Verified first that no bounded probe can replace it: at a probe size small enough to survive an exponential pattern (24 chars), `(?=a*a*b)` completes in 0.0ms and passes, because polynomial blowup only shows on long input — small probes miss it and large probes hang, the same trap as the original guard. Instead `compileLookaroundSplit` puts the two idioms that actually need lookaround onto RE2: splitting on `(?=X)` is slicing at every match start of X, and on `(?<=X)` at every match end, neither of which needs the assertion. Both `(?=#\s)` and the pre-existing `(?<=)` case keep working, now in linear time; anything else RE2 cannot represent is rejected with an actionable message rather than run on the backtracking engine. Also from the audit: - `literalRegex` recompiled its `RegExp` on every call to dodge `lastIndex` state from the `g` flag. VFS grep calls it per line, so that was a compile per line. It now keeps one non-global instance for `test`/`find` and a global one for `split`. - `validateRegex` and VFS grep log a warning when a pattern degrades. A guardrail rule that used lookaround now fails closed, which reads as the guardrail tripping on every input, and VFS grep's return shape has nowhere to report that a pattern was taken literally — both need to be findable in logs. Measured: RE2 costs 6-13x native on per-line matching (20k-line, 1.66MB file greps in 7ms), against ~100x on single multi-megabyte strings. --- apps/sim/lib/api/contracts/primitives.test.ts | 20 ++++- apps/sim/lib/chunkers/regex-chunker.ts | 43 +++++----- apps/sim/lib/copilot/vfs/operations.ts | 26 +++++- .../lib/core/security/linear-regex.test.ts | 36 ++++++++ apps/sim/lib/core/security/linear-regex.ts | 85 +++++++++++++++++-- .../sim/lib/guardrails/validate_regex.test.ts | 15 ++-- apps/sim/lib/guardrails/validate_regex.ts | 36 ++++---- apps/sim/package.json | 1 - bun.lock | 5 -- 9 files changed, 201 insertions(+), 66 deletions(-) diff --git a/apps/sim/lib/api/contracts/primitives.test.ts b/apps/sim/lib/api/contracts/primitives.test.ts index 82e5e425140..727a42a65f1 100644 --- a/apps/sim/lib/api/contracts/primitives.test.ts +++ b/apps/sim/lib/api/contracts/primitives.test.ts @@ -35,10 +35,22 @@ describe('customPatternSchema', () => { } }) - it('rejects a catastrophic-backtracking regex at the boundary', () => { - expect( - customPatternSchema.safeParse({ name: 'evil', regex: '(a+)+$', replacement: '' }).success - ).toBe(false) + it('no longer screens for catastrophic backtracking, which never worked here', () => { + // This used to reject `(a+)+$` via `safe-regex2`. The screen was removed: + // it caught that shape but passed `a*a*b`, which is just as catastrophic, + // so it only ever deterred the obvious spelling of a misconfiguration a + // user can inflict on their own workspace. It also rejected valid patterns + // (lookbehind, optional groups) that Presidio accepts. + // + // These patterns run in Presidio, not in this process, so they cannot + // stall this event loop; Presidio's own timeout is the bound. In-process + // matching uses `compileLinearRegex`, which cannot backtrack at all. + for (const regex of ['(a+)+$', 'a*a*b', '(?<=id: )\\w+']) { + expect( + customPatternSchema.safeParse({ name: 'p', regex, replacement: '' }).success, + `pattern ${regex} should be accepted` + ).toBe(true) + } }) }) diff --git a/apps/sim/lib/chunkers/regex-chunker.ts b/apps/sim/lib/chunkers/regex-chunker.ts index c9d384b04c3..20681f6dd31 100644 --- a/apps/sim/lib/chunkers/regex-chunker.ts +++ b/apps/sim/lib/chunkers/regex-chunker.ts @@ -10,7 +10,11 @@ import { splitAtWordBoundaries, tokensToChars, } from '@/lib/chunkers/utils' -import { compileLinearRegex, type LinearRegex } from '@/lib/core/security/linear-regex' +import { + compileLinearRegex, + compileLookaroundSplit, + type LinearRegex, +} from '@/lib/core/security/linear-regex' const logger = createLogger('RegexChunker') @@ -78,10 +82,12 @@ export class RegexChunker { * prevent: `a*a*b` against that probe measured 213s on JSC. RE2 removes the * failure mode outright, so the probe is gone rather than repaired. * - * Lookaround is the standard way to split while keeping the delimiter - * (`(?=^#\s)`), and RE2 does not implement it, so those patterns still use - * the built-in engine. They are the only ones that can backtrack now, and the - * length cap is the sole remaining bound on them. + * Keeping the delimiter — `(?=X)` before a chunk, `(?<=X)` after one — is the + * reason a split pattern reaches for lookaround, and `compileLookaroundSplit` + * runs both on RE2 without it. Anything else RE2 cannot represent is rejected + * rather than run on the built-in engine: no probe can tell a safe pattern + * from an unsafe one without running it, which is what made the old guard + * hang, so there is nothing to fall back *to*. */ private compilePattern(pattern: string): LinearRegex { if (!pattern) { @@ -92,28 +98,19 @@ export class RegexChunker { throw new Error(`Regex pattern exceeds maximum length of ${MAX_PATTERN_LENGTH} characters`) } - const source = toNonCapturing(pattern) - - const linear = compileLinearRegex(source) - if (linear) return linear - try { - const fallback = new RegExp(source, 'g') - return { - test: (text) => { - fallback.lastIndex = 0 - return fallback.test(text) - }, - find: (text) => { - fallback.lastIndex = 0 - const match = fallback.exec(text) - return match ? match.index : -1 - }, - split: (text) => text.split(new RegExp(source, 'g')), - } + new RegExp(pattern) } catch (error) { throw new Error(`Invalid regex pattern "${pattern}": ${toError(error).message}`) } + + const source = toNonCapturing(pattern) + const compiled = compileLinearRegex(source) ?? compileLookaroundSplit(source) + if (compiled) return compiled + + throw new Error( + `Regex pattern "${pattern}" uses syntax that cannot be evaluated safely (backreferences and negative or embedded lookaround are unsupported). Use a plain delimiter, "(?=...)" to split before one and keep it, or "(?<=...)" to split after one.` + ) } async chunk(content: string): Promise { diff --git a/apps/sim/lib/copilot/vfs/operations.ts b/apps/sim/lib/copilot/vfs/operations.ts index 2f2e1e8c7bb..b298b33112e 100644 --- a/apps/sim/lib/copilot/vfs/operations.ts +++ b/apps/sim/lib/copilot/vfs/operations.ts @@ -1,5 +1,13 @@ +import { createLogger } from '@sim/logger' import micromatch from 'micromatch' -import { compileLinearRegex, isPlainText, literalRegex } from '@/lib/core/security/linear-regex' +import { + compileLinearRegex, + isPlainText, + type LinearRegex, + literalRegex, +} from '@/lib/core/security/linear-regex' + +const logger = createLogger('VfsOperations') export interface GrepMatch { path: string @@ -148,9 +156,19 @@ export function grep( // Caller-supplied pattern over caller-supplied file content on the shared // event loop — matched by RE2 so it cannot backtrack. Syntax RE2 cannot // represent degrades to a literal rather than to the backtracking engine. - const regex = isPlainText(pattern) - ? literalRegex(pattern, { ignoreCase }) - : (compileLinearRegex(pattern, { ignoreCase }) ?? literalRegex(pattern, { ignoreCase })) + let regex: LinearRegex + if (isPlainText(pattern)) { + regex = literalRegex(pattern, { ignoreCase }) + } else { + const linear = compileLinearRegex(pattern, { ignoreCase }) + if (!linear) { + // The return shape carries results only, so the caller cannot be told + // inline that its regex was taken literally — log it, since silently + // returning "no matches" reads as "not in the file". + logger.warn('Grep pattern is not RE2-representable; matching it literally', { pattern }) + } + regex = linear ?? literalRegex(pattern, { ignoreCase }) + } if (outputMode === 'files_with_matches') { const matchingFiles: string[] = [] diff --git a/apps/sim/lib/core/security/linear-regex.test.ts b/apps/sim/lib/core/security/linear-regex.test.ts index 70bccfcccfa..3bf44167704 100644 --- a/apps/sim/lib/core/security/linear-regex.test.ts +++ b/apps/sim/lib/core/security/linear-regex.test.ts @@ -5,6 +5,7 @@ import { describe, expect, it } from 'vitest' import { compileLinearRegex, + compileLookaroundSplit, escapeRegExp, isPlainText, literalRegex, @@ -96,3 +97,38 @@ describe('isPlainText / escapeRegExp', () => { expect(new RegExp(escapeRegExp(raw)).test(raw)).toBe(true) }) }) + +describe('compileLookaroundSplit', () => { + it('splits before each delimiter for (?=X), matching String.split', () => { + const doc = '# One\nalpha\n# Two\nbeta' + expect(compileLookaroundSplit('(?=#\\s)')?.split(doc)).toEqual( + doc.split(/(?=#\s)/g).filter(Boolean) + ) + }) + + it('splits after each delimiter for (?<=X)', () => { + const doc = 'onetwothree' + expect(compileLookaroundSplit('(?<=)')?.split(doc)).toEqual([ + 'one', + 'two', + 'three', + ]) + }) + + it('stays linear on a catastrophic body', () => { + const regex = compileLookaroundSplit('(?=a*a*b)') + expect(regex).not.toBeNull() + + const start = Date.now() + regex?.split(`${'a'.repeat(20000)}!`) + expect(Date.now() - start).toBeLessThan(2000) + }) + + it.each([ + ['negative lookahead', '(?!x)y'], + ['embedded lookahead', 'a(?=b)c'], + ['plain pattern', '\\n\\n'], + ])('returns null for %s', (_label, pattern) => { + expect(compileLookaroundSplit(pattern)).toBeNull() + }) +}) diff --git a/apps/sim/lib/core/security/linear-regex.ts b/apps/sim/lib/core/security/linear-regex.ts index efb2cd6db12..8e4ee15186d 100644 --- a/apps/sim/lib/core/security/linear-regex.ts +++ b/apps/sim/lib/core/security/linear-regex.ts @@ -49,17 +49,86 @@ export function isPlainText(pattern: string): boolean { * a degradation path when RE2 rejects the syntax. */ export function literalRegex(pattern: string, options: LinearRegexOptions = {}): LinearRegex { - const flags = options.ignoreCase ? 'gi' : 'g' const source = escapeRegExp(pattern) - const at = (text: string): number => { - const regex = new RegExp(source, flags) - const match = regex.exec(text) - return match ? match.index : -1 + const caseFlag = options.ignoreCase ? 'i' : '' + // Non-global, so `exec`/`test` keep no `lastIndex` between calls and one + // instance is reusable — callers scan line-by-line, and recompiling per line + // would dominate the cost. `split` needs the global form, which + // `String.prototype.split` clones rather than mutating. + const scanner = new RegExp(source, caseFlag) + const splitter = new RegExp(source, `g${caseFlag}`) + return { + test: (text) => scanner.test(text), + find: (text) => { + const match = scanner.exec(text) + return match ? match.index : -1 + }, + split: (text) => text.split(splitter), + } +} + +/** A pattern that is exactly one lookahead, or exactly one lookbehind. */ +const WHOLE_PATTERN_LOOKAHEAD = /^\(\?=([\s\S]*)\)$/ +const WHOLE_PATTERN_LOOKBEHIND = /^\(\?<=([\s\S]*)\)$/ + +/** + * Compile the two zero-width split idioms — `(?=X)` (split before each X) and + * `(?<=X)` (split after each X) — onto RE2, which has no lookaround. + * + * Neither idiom actually needs lookaround to *split*: splitting on `(?=X)` is + * slicing at the start of every match of `X`, and on `(?<=X)` at every match + * end. RE2 can locate both. These two cover the reason a split pattern reaches + * for lookaround at all — keeping the delimiter attached to the leading or + * trailing chunk — so they stay on the linear engine instead of forcing a + * fallback to the backtracking one. + * + * Returns `null` unless the whole pattern is a single such assertion whose body + * RE2 can represent; negative lookaround, lookbehind mixed with other syntax, + * and backreferences are not covered. + * + * `test`/`find` report on the body itself: a zero-width assertion matches + * exactly when its body does, so `test` is exact, while `find` gives the body's + * start rather than the (zero-width) assertion position. + */ +export function compileLookaroundSplit( + pattern: string, + options: LinearRegexOptions = {} +): LinearRegex | null { + const ahead = WHOLE_PATTERN_LOOKAHEAD.exec(pattern)?.[1] + const behind = ahead === undefined ? WHOLE_PATTERN_LOOKBEHIND.exec(pattern)?.[1] : undefined + const body = ahead ?? behind + if (!body) return null + + let compiled: ReturnType + try { + compiled = RE2JS.compile(body, options.ignoreCase ? RE2JS.CASE_INSENSITIVE : 0) + } catch { + return null } + const sliceAtMatchStart = ahead !== undefined + return { - test: (text) => at(text) >= 0, - find: at, - split: (text) => text.split(new RegExp(source, flags)), + test: (text) => compiled.matcher(text).find(), + find: (text) => { + const matcher = compiled.matcher(text) + return matcher.find() ? matcher.start() : -1 + }, + split: (text) => { + const segments: string[] = [] + const matcher = compiled.matcher(text) + let cursor = 0 + while (matcher.find()) { + const boundary = sliceAtMatchStart ? matcher.start() : matcher.end() + // Skip a boundary at the cursor or at the very end: both would emit an + // empty segment, which `String.prototype.split` also produces and every + // caller discards. + if (boundary <= cursor || boundary >= text.length) continue + segments.push(text.slice(cursor, boundary)) + cursor = boundary + } + segments.push(text.slice(cursor)) + return segments + }, } } diff --git a/apps/sim/lib/guardrails/validate_regex.test.ts b/apps/sim/lib/guardrails/validate_regex.test.ts index 78feb044aec..905b869edb2 100644 --- a/apps/sim/lib/guardrails/validate_regex.test.ts +++ b/apps/sim/lib/guardrails/validate_regex.test.ts @@ -54,11 +54,14 @@ describe('validateRegexPattern', () => { expect(validateRegexPattern('(')).toMatchObject({ valid: false }) }) - it('is unchanged by the RE2 migration — patterns here go to Presidio, not this process', () => { - // Left on `safe-regex2` deliberately: these patterns execute in Presidio, - // where a slow one times out rather than stalling this event loop, and - // Presidio's Python engine supports constructs RE2 does not. - expect(validateRegexPattern('(?:https?://)?example\\.com')).toMatchObject({ valid: false }) - expect(validateRegexPattern('(?<=id: )\\w+')).toMatchObject({ valid: false }) + it.each([ + ['lookbehind', '(?<=id: )\\w+'], + ['optional group', '(?:https?://)?example\\.com'], + ['nested quantifier', '(a+)+$'], + ])('accepts %s, which the removed safe-regex2 screen rejected', (_label, pattern) => { + // These are valid Presidio patterns that could not be saved before. The + // screen that blocked them caught no real ReDoS (it passes `a*a*b`), and + // these run in Presidio rather than in this process. + expect(validateRegexPattern(pattern)).toEqual({ valid: true }) }) }) diff --git a/apps/sim/lib/guardrails/validate_regex.ts b/apps/sim/lib/guardrails/validate_regex.ts index 189310d2e69..f8a90dc6e5e 100644 --- a/apps/sim/lib/guardrails/validate_regex.ts +++ b/apps/sim/lib/guardrails/validate_regex.ts @@ -1,7 +1,9 @@ +import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' -import safe from 'safe-regex2' import { compileLinearRegex } from '@/lib/core/security/linear-regex' +const logger = createLogger('ValidateRegex') + /** * Validate if input matches regex pattern */ @@ -20,13 +22,18 @@ export interface RegexPatternValidation { * Validate a PII custom pattern's syntax before it is persisted and handed to * Presidio. Shared by the custom-pattern editor UI and the write boundary. * - * The `safe-regex2` screen here is a courtesy, NOT a ReDoS defense: it screens - * star height only and is documented as having false negatives — it passes - * `(a|a)*b`, and `a*a*b` defeats every syntactic rule of this kind. It is kept - * because these patterns execute in Presidio, a separate service where a slow - * pattern times out and silently fails open (leaving PII unredacted) rather - * than stalling this event loop, and because Presidio's Python engine supports - * lookaround — so gating on RE2 here would reject patterns that work. + * Syntax only, deliberately. This previously also ran a `safe-regex2` + * catastrophic-backtracking screen, which was removed because it was pure cost: + * it screens star height alone and is documented as having false negatives — it + * passes `(a|a)*b`, and `a*a*b` defeats it and every syntactic rule of its kind + * — while rejecting patterns that work perfectly well, including lookbehind + * (`(?<=id: )\w+`) and optional groups (`(?:https?://)?example\.com`). It + * blocked valid rules and stopped nothing. + * + * Nor could a screen here be made sound: these patterns execute in Presidio's + * Python engine, which backtracks on shapes RE2 accepts, so RE2-representability + * says nothing about their runtime there. Presidio's own request timeout is the + * real bound — note it fails open on timeout, leaving PII unredacted. * * Anything that matches a caller-supplied pattern *in this process* must use * `compileLinearRegex` from `@/lib/core/security/linear-regex` instead. @@ -38,13 +45,7 @@ export function validateRegexPattern(pattern: string): RegexPatternValidation { try { new RegExp(pattern) } catch (error) { - return { valid: false, error: `Invalid regex: ${(error as Error).message}` } - } - if (!safe(pattern)) { - return { - valid: false, - error: 'Pattern rejected: potentially unsafe (catastrophic backtracking)', - } + return { valid: false, error: `Invalid regex: ${getErrorMessage(error)}` } } return { valid: true } } @@ -67,6 +68,11 @@ export function validateRegex(inputStr: string, pattern: string): ValidationResu const regex = compileLinearRegex(pattern) if (!regex) { + // A rule that used lookaround worked before this became RE2-only and now + // fails closed, which reads to the workspace as the guardrail tripping on + // every input. Log it so an operator can find and rewrite the rule from + // logs rather than from user reports. + logger.warn('Guardrail regex uses syntax RE2 cannot evaluate; failing closed', { pattern }) return { passed: false, error: diff --git a/apps/sim/package.json b/apps/sim/package.json index 533de127400..8ef25a4b95b 100644 --- a/apps/sim/package.json +++ b/apps/sim/package.json @@ -207,7 +207,6 @@ "remark-gfm": "4.0.1", "resend": "^4.1.2", "rss-parser": "3.13.0", - "safe-regex2": "5.1.0", "sharp": "0.35.3", "socket.io-client": "4.8.1", "ssh2": "^1.17.0", diff --git a/bun.lock b/bun.lock index 6733be244ec..597bc798df4 100644 --- a/bun.lock +++ b/bun.lock @@ -282,7 +282,6 @@ "remark-gfm": "4.0.1", "resend": "^4.1.2", "rss-parser": "3.13.0", - "safe-regex2": "5.1.0", "sharp": "0.35.3", "socket.io-client": "4.8.1", "ssh2": "^1.17.0", @@ -3699,8 +3698,6 @@ "restore-cursor": ["restore-cursor@5.1.0", "", { "dependencies": { "onetime": "^7.0.0", "signal-exit": "^4.1.0" } }, "sha512-oMA2dcrw6u0YfxJQXm342bFKX/E4sG9rbTzO9ptUcR/e8A33cHuvStiYOwH7fszkZlZ1z/ta9AAoPk2F4qIOHA=="], - "ret": ["ret@0.5.0", "", {}, "sha512-I1XxrZSQ+oErkRR4jYbAyEEu2I0avBvvMM5JN+6EBprOGRCs63ENqZ3vjavq8fBw2+62G5LF5XelKwuJpcvcxw=="], - "retry": ["retry@0.12.0", "", {}, "sha512-9LkiTwjUh6rT555DtE9rTX+BKByPfrMzEAtnlEtdEwr3Nkffwiihqe2bWADg+OQRjt9gl6ICdmB/ZFDCGAtSow=="], "retry-request": ["retry-request@7.0.2", "", { "dependencies": { "@types/request": "^2.48.8", "extend": "^3.0.2", "teeny-request": "^9.0.0" } }, "sha512-dUOvLMJ0/JJYEn8NrpOaGNE7X3vpI5XlZS/u0ANjqtcZVKnIxP7IgCFwrKTxENw29emmwug53awKtaMm4i9g5w=="], @@ -3737,8 +3734,6 @@ "safe-buffer": ["safe-buffer@5.2.1", "", {}, "sha512-rp3So07KcdmmKbGvgaNxQSJr7bGVSVk5S9Eq1F+ppbRo70+YeaDxkw5Dd8NPN+GD6bjnYm2VuPuCXmpuYvmCXQ=="], - "safe-regex2": ["safe-regex2@5.1.0", "", { "dependencies": { "ret": "~0.5.0" }, "bin": { "safe-regex2": "bin/safe-regex2.js" } }, "sha512-pNHAuBW7TrcleFHsxBr5QMi/Iyp0ENjUKz7GCcX1UO7cMh+NmVK6HxQckNL1tJp1XAJVjG6B8OKIPqodqj9rtw=="], - "safe-stable-stringify": ["safe-stable-stringify@2.5.0", "", {}, "sha512-b3rppTKm9T+PsVCBEOUR46GWI7fdOs00VKZ1+9c1EWDaDMvjQc6tUwuFyIprgGgTcWoVHSKrU8H31ZHA2e0RHA=="], "safer-buffer": ["safer-buffer@2.1.2", "", {}, "sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg=="], From de3e5fdbee8e6d7d416e82cc2989a569130fb788 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 27 Jul 2026 17:06:24 -0700 Subject: [PATCH 6/9] test(pii): drop the custom-pattern editor's backtracking-screen assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught this; my local runs only covered `lib/**` and missed the `.tsx` counterpart of removing `safe-regex2`. The editor asserted that `(a+)+$` renders "potentially unsafe". That screen is gone, so the assertion is inverted: the test now pins that `(a+)+$`, lookbehind and optional groups all render cleanly, which is the point of the removal — they are valid Presidio patterns that could not be saved before. Syntactically invalid patterns still surface "Invalid regex". --- .../pii/custom-patterns-editor.test.tsx | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/apps/sim/components/pii/custom-patterns-editor.test.tsx b/apps/sim/components/pii/custom-patterns-editor.test.tsx index 769f45dc92a..2336c644d32 100644 --- a/apps/sim/components/pii/custom-patterns-editor.test.tsx +++ b/apps/sim/components/pii/custom-patterns-editor.test.tsx @@ -49,10 +49,18 @@ describe('CustomPatternsEditor', () => { expect(container.textContent).toMatch(/Invalid regex/) }) - it('shows an inline error for a catastrophic-backtracking pattern', () => { - renderEditor([row('(a+)+$')], vi.fn()) - expect(container.textContent).toMatch(/potentially unsafe/) - }) + it.each(['(a+)+$', '(?<=id: )\\w+', '(?:https?://)?example\\.com'])( + 'accepts %s, which the removed safe-regex2 screen rejected', + (pattern) => { + // The editor no longer runs a catastrophic-backtracking screen. It caught + // `(a+)+$` but passed `a*a*b`, so it deterred only the obvious spelling + // while blocking valid Presidio patterns like lookbehind. These patterns + // run in Presidio, not in this process. + renderEditor([row(pattern)], vi.fn()) + expect(container.textContent).not.toMatch(/potentially unsafe/) + expect(container.textContent).not.toMatch(/Invalid regex/) + } + ) it('appends an empty row when "Add pattern" is clicked', () => { const onChange = vi.fn() From b56faa156c3d8292c99e7e10f3db62f15940bd4e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 27 Jul 2026 17:50:12 -0700 Subject: [PATCH 7/9] fix(chunkers): support lookaround with an affix, and keep JS whitespace semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three independent reviewers with no shared context audited the RE2 migration. Two found the same high-severity regression, and one found the divergence that silently rewrites stored data. Both are fixed here. **Lookaround with an affix threw.** `compileLookaroundSplit` only accepted a pattern that was *entirely* one assertion, so `(?<=\.)\s+` — split after the period — and `(?<=[.!?])\s+(?=[A-Z])` — the textbook sentence splitter — fell through to the chunker's `throw`. Those are persisted KB configs, and the compile runs in the constructor, so an existing knowledge base would stop re-ingesting with a hard error. The TSDoc claiming the bare assertions "cover the reason a split pattern reaches for lookaround" was simply wrong. Fixed generally rather than by adding another special case: splitting never needs the assertion, only the span the delimiter consumes, so `(?<=X)Y(?=Z)` compiles to `(?:X)(Y)(?:Z)` and group 1's span is exactly what `String.split` removes. One rule now covers every combination, and the four shapes above match `String.prototype.split` output exactly. **`\s` silently lost 14 codepoints.** RE2's `\s` is ASCII-only; ECMAScript's also covers `\v`, NBSP, U+2028/9, U+202F, U+3000, U+FEFF and the rest of `\p{Zs}`. A reviewer measured 9 of 35 realistic pattern/document pairs chunking differently through the real `RegexChunker` — NBSP from PDF/DOCX/HTML extraction, U+3000 in CJK, U+202F in French text. That rewrites stored chunk boundaries and embeddings for a document that never changed, with nothing thrown and nothing logged. `translateToRe2` now rewrites `\s`/`\S` to the verified-equivalent RE2 class, and `\uXXXX` to `\x{XXXX}` (RE2 rejects the former outright, turning a non-ASCII delimiter into a hard failure). Also from the audit: - `truncated`'s new TSDoc asserted an invariant the code violates — reaching `maxMatches` sets it even when nothing was left unread. Reworded to what it actually means. - The chunker's rejection message blamed "backreferences and lookaround" for repeat-count and `\uXXXX` rejections too. - `escapeRegExp` was a byte-identical copy of `executor/constants.ts:472` and exported for nobody; now module-private. - The guardrails wand prompt told the LLM to emit "valid JavaScript regex", which reliably produces `(?=.*\d)`-style patterns that now fail every check. - The PII editor's TSDoc still described the removed backtracking screen. - VFS grep's TSDoc omitted that invalid patterns now match literally. Test gaps the audit exposed: the match-time budget's only mechanism was unguarded (deleting the accumulation left every test green) — now covered by a mutation-verified test; the escape test survived dropping `.` from the class; the split-parity test dodged the one real divergence; and two test names described the built-in engine that no longer runs. --- apps/sim/blocks/blocks/guardrails.ts | 6 +- .../components/pii/custom-patterns-editor.tsx | 10 +- apps/sim/lib/chunkers/regex-chunker.test.ts | 6 +- apps/sim/lib/chunkers/regex-chunker.ts | 2 +- apps/sim/lib/copilot/vfs/operations.ts | 11 +- .../lib/core/security/linear-regex.test.ts | 51 +++- apps/sim/lib/core/security/linear-regex.ts | 283 ++++++++++++++---- apps/sim/lib/logs/log-views.test.ts | 16 + apps/sim/lib/logs/log-views.ts | 9 +- 9 files changed, 310 insertions(+), 84 deletions(-) diff --git a/apps/sim/blocks/blocks/guardrails.ts b/apps/sim/blocks/blocks/guardrails.ts index 1b893243951..6b857f690f8 100644 --- a/apps/sim/blocks/blocks/guardrails.ts +++ b/apps/sim/blocks/blocks/guardrails.ts @@ -76,7 +76,11 @@ export const GuardrailsBlock: BlockConfig = { enabled: true, prompt: `Generate a regular expression pattern based on the user's description. The regex should be: -- Valid JavaScript regex syntax +- Valid RE2 syntax: no lookahead ((?=...), (?!...)), no lookbehind ((?<=...), (?) { diff --git a/apps/sim/lib/chunkers/regex-chunker.test.ts b/apps/sim/lib/chunkers/regex-chunker.test.ts index c5da910fb36..96d84cc5ed4 100644 --- a/apps/sim/lib/chunkers/regex-chunker.test.ts +++ b/apps/sim/lib/chunkers/regex-chunker.test.ts @@ -404,9 +404,9 @@ describe('RegexChunker', () => { 10000 ) - it.concurrent('still supports lookahead split patterns via the built-in engine', async () => { - // RE2 cannot represent lookaround, and splitting *before* a delimiter so - // it is kept is the standard use for it, so these must keep working. + it.concurrent('still supports lookahead split patterns', async () => { + // RE2 has no lookaround; compileLookaroundSplit expresses this as slicing + // at each match start, so the idiom keeps working on the linear engine. // (No `m` flag is applied, so anchor the lookahead on the delimiter // itself rather than on `^`.) const chunker = new RegexChunker({ diff --git a/apps/sim/lib/chunkers/regex-chunker.ts b/apps/sim/lib/chunkers/regex-chunker.ts index 20681f6dd31..b294a504eee 100644 --- a/apps/sim/lib/chunkers/regex-chunker.ts +++ b/apps/sim/lib/chunkers/regex-chunker.ts @@ -109,7 +109,7 @@ export class RegexChunker { if (compiled) return compiled throw new Error( - `Regex pattern "${pattern}" uses syntax that cannot be evaluated safely (backreferences and negative or embedded lookaround are unsupported). Use a plain delimiter, "(?=...)" to split before one and keep it, or "(?<=...)" to split after one.` + `Regex pattern "${pattern}" uses syntax that cannot be evaluated safely. Unsupported: negative lookaround ("(?!...)", "(? { expect(compileLinearRegex('ERROR')?.test('an error here')).toBe(false) }) - it('splits equivalently to String.prototype.split', () => { + it('splits equivalently to String.prototype.split, minus the trailing empty', () => { const doc = '# One\ntext a\n\n# Two\ntext b' expect(compileLinearRegex('\\n\\n+')?.split(doc)).toEqual(doc.split(/\n\n+/g)) + + // The documented divergence: a trailing delimiter yields no empty tail. + expect(compileLinearRegex(',')?.split('a,b,')).toEqual(['a', 'b']) + expect('a,b,'.split(/,/g)).toEqual(['a', 'b', '']) + }) + + it.each([ + ['non-breaking space', '\u00a0'], + ['narrow no-break space', '\u202f'], + ['ideographic space', '\u3000'], + ['line separator', '\u2028'], + ['vertical tab', '\v'], + ])('treats %s as whitespace, matching the built-in engine', (_label, ws) => { + // RE2's own \\s is ASCII-only. Untranslated, a \\s document splitter stops + // splitting on the whitespace that PDF/HTML extraction emits, silently + // changing stored chunk boundaries. + const doc = `alpha${ws}beta` + expect(compileLinearRegex('\\s+')?.split(doc)).toEqual(doc.split(/\s+/g)) + expect(compileLinearRegex('\\s')?.test(doc)).toBe(true) }) it.each([ @@ -92,10 +110,17 @@ describe('isPlainText / escapeRegExp', () => { (pattern) => expect(isPlainText(pattern)).toBe(false) ) - it('escapes every metacharacter so the pattern matches only itself', () => { - const raw = 'a.*+?^${}()|[]\\b' - expect(new RegExp(escapeRegExp(raw)).test(raw)).toBe(true) - }) + it.each(['.', '*', '+', '?', '^', '$', '{', '}', '(', ')', '|', '[', ']', '\\'])( + 'escapes %s so a literal pattern matches only itself', + (meta) => { + const regex = literalRegex(`a${meta}b`) + expect(regex.test(`a${meta}b`)).toBe(true) + // Under-escaping shows up here: an unescaped metacharacter would let the + // pattern match text that does not contain it verbatim. + expect(regex.test('aXb')).toBe(false) + expect(regex.test('ab')).toBe(false) + } + ) }) describe('compileLookaroundSplit', () => { @@ -124,9 +149,21 @@ describe('compileLookaroundSplit', () => { expect(Date.now() - start).toBeLessThan(2000) }) + it.each([ + ['split after a period', '(?<=\\.)\\s+', 'One. Two. Three.'], + ['split before a heading', '\\n(?=Chapter )', 'intro\nChapter 1\nChapter 2'], + ['sentence splitter', '(?<=[.!?])\\s+(?=[A-Z])', 'One. Two! Three? four.'], + ])('handles %s, where the assertion has an affix', (_label, pattern, doc) => { + // These are the common shapes: a lookaround combined with other syntax. + // Handling only a whole-pattern assertion would reject them outright. + const split = compileLookaroundSplit(pattern)?.split(doc) + expect(split).toEqual(doc.split(new RegExp(pattern, 'g'))) + }) + it.each([ ['negative lookahead', '(?!x)y'], - ['embedded lookahead', 'a(?=b)c'], + ['negative lookbehind', '(? { expect(compileLookaroundSplit(pattern)).toBeNull() diff --git a/apps/sim/lib/core/security/linear-regex.ts b/apps/sim/lib/core/security/linear-regex.ts index 8e4ee15186d..a39df4f67ec 100644 --- a/apps/sim/lib/core/security/linear-regex.ts +++ b/apps/sim/lib/core/security/linear-regex.ts @@ -13,10 +13,16 @@ import { RE2JS } from 're2js' * on top of it still passes `a*a*b`. Every syntactic rule only excludes the * shapes someone thought to enumerate, so the engine has to change instead. * - * RE2 has no backtracking and matches in time linear in the input. The cost is - * throughput (~100x the built-in engine, roughly 25ms/MB) and syntax: RE2 - * implements neither lookaround nor backreferences, so `compileLinearRegex` - * returns `null` for those and each caller decides how to degrade. + * RE2 has no backtracking and matches in time linear in the input. Two costs + * follow, and this module works to keep both off the caller: + * + * - **Syntax.** RE2 implements neither lookaround nor backreferences, and + * spells some escapes differently. `translateToRe2` bridges the mechanical + * differences and `compileLookaroundSplit` recovers the lookaround *split* + * idioms; genuine gaps return `null` so each caller decides how to degrade. + * - **Throughput.** Measured 0.5–270ms per megabyte depending on the pattern, + * against roughly 0.04ms/MB for the built-in engine. Callers matching a + * pattern with no metacharacter should take `literalRegex` instead. */ export interface LinearRegexOptions { ignoreCase?: boolean @@ -27,104 +33,225 @@ export interface LinearRegex { test(text: string): boolean /** Index of the first match in `text`, or -1. */ find(text: string): number - /** Split `text` around every match, like `String.prototype.split(regex)`. */ + /** + * Split `text` around every match. + * + * Follows `String.prototype.split` except that a trailing empty segment is + * omitted — RE2 drops it, and every caller here discards empties anyway. + */ split(text: string): string[] } +const METACHARACTERS = /[.*+?^${}()|[\]\\]/ + /** Escape every regex metacharacter so `input` matches only itself. */ -export function escapeRegExp(input: string): string { +function escapeRegExp(input: string): string { return input.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') } /** True when `pattern` has no metacharacter, so both engines behave identically. */ export function isPlainText(pattern: string): boolean { - return !/[.*+?^${}()|[\]\\]/.test(pattern) + return !METACHARACTERS.test(pattern) } /** - * Match `pattern` as an escaped literal on the built-in engine. + * ECMAScript's `\s` as an RE2 character-class body. * - * Safe because an escaped literal cannot backtrack, and ~100x quicker than RE2 - * — worth taking whenever the pattern has no metacharacter to interpret, or as - * a degradation path when RE2 rejects the syntax. + * RE2's `\s` is ASCII-only (`[\t\n\f\r ]`), while ECMAScript's also covers + * `\v`, NBSP, U+1680, U+2000–U+200A, U+2028, U+2029, U+202F, U+205F, U+3000 + * and U+FEFF. Left untranslated, a `\s`-based document splitter silently stops + * splitting on the non-breaking spaces that PDF, DOCX and HTML extraction put + * everywhere — producing different chunks, and so different embeddings, for a + * document that has not changed. Verified equivalent to ECMAScript `\s` across + * a full sweep of the BMP. */ -export function literalRegex(pattern: string, options: LinearRegexOptions = {}): LinearRegex { - const source = escapeRegExp(pattern) - const caseFlag = options.ignoreCase ? 'i' : '' - // Non-global, so `exec`/`test` keep no `lastIndex` between calls and one - // instance is reusable — callers scan line-by-line, and recompiling per line - // would dominate the cost. `split` needs the global form, which - // `String.prototype.split` clones rather than mutating. - const scanner = new RegExp(source, caseFlag) - const splitter = new RegExp(source, `g${caseFlag}`) - return { - test: (text) => scanner.test(text), - find: (text) => { - const match = scanner.exec(text) - return match ? match.index : -1 - }, - split: (text) => text.split(splitter), +const JS_WHITESPACE_BODY = '\\t\\n\\v\\f\\r\\x{2028}\\x{2029}\\x{feff}\\p{Zs}' + +/** + * Rewrite the mechanical ECMAScript-vs-RE2 spelling differences. + * + * Two substitutions, both verified equivalent: + * - `\s`/`\S` → the Unicode set above, so whitespace splitting is unchanged. + * - `\uXXXX` → `\x{XXXX}`, RE2's spelling. Untranslated, RE2 rejects the + * pattern outright, which turns the ordinary way of writing a non-ASCII + * delimiter (`•`) into a hard failure. + * + * Scans with escape and character-class state so `\\s` (a literal backslash + * followed by `s`) is left alone and `[\s\d]` splices the set body rather than + * nesting a class. `\S` *inside* a class is left as RE2's ASCII form: negation + * within a set cannot be spliced, and `[^\S\n]` is rare enough not to justify + * a full class parser. + */ +function translateToRe2(pattern: string): string { + let out = '' + let inClass = false + for (let i = 0; i < pattern.length; i++) { + const char = pattern[i] + + if (char === '\\') { + const next = pattern[i + 1] + if (next === 'u' && /^[0-9a-fA-F]{4}$/.test(pattern.slice(i + 2, i + 6))) { + out += `\\x{${pattern.slice(i + 2, i + 6)}}` + i += 5 + continue + } + if (next === 's') { + out += inClass ? JS_WHITESPACE_BODY : `[${JS_WHITESPACE_BODY}]` + i += 1 + continue + } + if (next === 'S' && !inClass) { + out += `[^${JS_WHITESPACE_BODY}]` + i += 1 + continue + } + out += next === undefined ? char : char + next + i += 1 + continue + } + + if (inClass) { + if (char === ']') inClass = false + } else if (char === '[') { + inClass = true + } + out += char + } + return out +} + +/** Index of the `)` closing the group opened at `open`, or -1. */ +function closingParen(pattern: string, open: number): number { + let depth = 0 + let inClass = false + for (let i = open; i < pattern.length; i++) { + const char = pattern[i] + if (char === '\\') { + i += 1 + continue + } + if (inClass) { + if (char === ']') inClass = false + continue + } + if (char === '[') inClass = true + else if (char === '(') depth += 1 + else if (char === ')') { + depth -= 1 + if (depth === 0) return i + } } + return -1 } -/** A pattern that is exactly one lookahead, or exactly one lookbehind. */ -const WHOLE_PATTERN_LOOKAHEAD = /^\(\?=([\s\S]*)\)$/ -const WHOLE_PATTERN_LOOKBEHIND = /^\(\?<=([\s\S]*)\)$/ +interface SplitShape { + behind: string + middle: string + ahead: string +} /** - * Compile the two zero-width split idioms — `(?=X)` (split before each X) and - * `(?<=X)` (split after each X) — onto RE2, which has no lookaround. + * Decompose a split pattern into `(?<=behind) middle (?=ahead)`, with either + * assertion optional. Returns `null` when neither is present (the caller should + * compile normally) or when the shape is anything else. + */ +function parseSplitShape(pattern: string): SplitShape | null { + let rest = pattern + let behind = '' + + if (rest.startsWith('(?<=')) { + const close = closingParen(rest, 0) + if (close === -1) return null + behind = rest.slice(4, close) + rest = rest.slice(close + 1) + } + + let ahead = '' + let depth = 0 + let inClass = false + for (let i = 0; i < rest.length; i++) { + const char = rest[i] + if (char === '\\') { + i += 1 + continue + } + if (inClass) { + if (char === ']') inClass = false + continue + } + if (char === '[') { + inClass = true + continue + } + if (char === ')') depth -= 1 + if (char !== '(') continue + depth += 1 + if (depth !== 1 || !rest.startsWith('(?=', i)) continue + const close = closingParen(rest, i) + if (close !== rest.length - 1) continue + ahead = rest.slice(i + 3, close) + rest = rest.slice(0, i) + break + } + + if (!behind && !ahead) return null + return { behind, middle: rest, ahead } +} + +/** + * Compile a lookaround *split* pattern onto RE2, which has no lookaround. * - * Neither idiom actually needs lookaround to *split*: splitting on `(?=X)` is - * slicing at the start of every match of `X`, and on `(?<=X)` at every match - * end. RE2 can locate both. These two cover the reason a split pattern reaches - * for lookaround at all — keeping the delimiter attached to the leading or - * trailing chunk — so they stay on the linear engine instead of forcing a - * fallback to the backtracking one. + * Splitting never needs the assertion itself — only the span the delimiter + * consumes. `(?<=X)Y(?=Z)` becomes `(?:X)(Y)(?:Z)`, and the span of group 1 is + * exactly what `String.prototype.split` would remove. That covers every + * combination in one rule: `(?=Z)` alone splits before a delimiter and keeps + * it, `(?<=X)` alone splits after one, and `(?<=[.!?])\s+(?=[A-Z])` — the + * sentence splitter — consumes the whitespace between them. * - * Returns `null` unless the whole pattern is a single such assertion whose body - * RE2 can represent; negative lookaround, lookbehind mixed with other syntax, - * and backreferences are not covered. + * Returns `null` for negative lookaround, backreferences, or any body RE2 + * cannot represent. * - * `test`/`find` report on the body itself: a zero-width assertion matches - * exactly when its body does, so `test` is exact, while `find` gives the body's - * start rather than the (zero-width) assertion position. + * Two divergences from the built-in engine, both narrow: RE2 iterates + * non-overlapping matches, so a self-overlapping delimiter (`(?=aa)` over + * `aaaa`) yields fewer boundaries; and `test`/`find` report on the compiled + * form rather than the zero-width assertion position. */ export function compileLookaroundSplit( pattern: string, options: LinearRegexOptions = {} ): LinearRegex | null { - const ahead = WHOLE_PATTERN_LOOKAHEAD.exec(pattern)?.[1] - const behind = ahead === undefined ? WHOLE_PATTERN_LOOKBEHIND.exec(pattern)?.[1] : undefined - const body = ahead ?? behind - if (!body) return null + const shape = parseSplitShape(pattern) + if (!shape) return null + + const behind = shape.behind ? `(?:${shape.behind})` : '' + const ahead = shape.ahead ? `(?:${shape.ahead})` : '' + const source = translateToRe2(`${behind}(${shape.middle})${ahead}`) let compiled: ReturnType try { - compiled = RE2JS.compile(body, options.ignoreCase ? RE2JS.CASE_INSENSITIVE : 0) + compiled = RE2JS.compile(source, options.ignoreCase ? RE2JS.CASE_INSENSITIVE : 0) } catch { return null } - const sliceAtMatchStart = ahead !== undefined return { test: (text) => compiled.matcher(text).find(), find: (text) => { const matcher = compiled.matcher(text) - return matcher.find() ? matcher.start() : -1 + return matcher.find() ? matcher.start(1) : -1 }, split: (text) => { const segments: string[] = [] const matcher = compiled.matcher(text) let cursor = 0 while (matcher.find()) { - const boundary = sliceAtMatchStart ? matcher.start() : matcher.end() - // Skip a boundary at the cursor or at the very end: both would emit an - // empty segment, which `String.prototype.split` also produces and every - // caller discards. - if (boundary <= cursor || boundary >= text.length) continue - segments.push(text.slice(cursor, boundary)) - cursor = boundary + const from = matcher.start(1) + const to = matcher.end(1) + // Skip a boundary behind the cursor, or one that would only emit an + // empty leading/trailing segment. + if (from < cursor || (from === cursor && to === cursor) || from >= text.length) continue + segments.push(text.slice(cursor, from)) + cursor = to } segments.push(text.slice(cursor)) return segments @@ -132,20 +259,50 @@ export function compileLookaroundSplit( } } +/** + * Match `pattern` as an escaped literal on the built-in engine. + * + * Safe because an escaped literal cannot backtrack, and far quicker than RE2 — + * worth taking whenever the pattern has no metacharacter to interpret, or as a + * degradation path when RE2 rejects the syntax. + */ +export function literalRegex(pattern: string, options: LinearRegexOptions = {}): LinearRegex { + const source = escapeRegExp(pattern) + const caseFlag = options.ignoreCase ? 'i' : '' + // Non-global, so `exec`/`test` keep no `lastIndex` between calls and one + // instance is reusable — callers scan line-by-line, and recompiling per line + // would dominate the cost. + const scanner = new RegExp(source, caseFlag) + return { + test: (text) => scanner.test(text), + find: (text) => { + const match = scanner.exec(text) + return match ? match.index : -1 + }, + // Built lazily: no caller splits on a literal, so the second compile is + // only paid if one ever does. + split: (text) => text.split(new RegExp(source, `g${caseFlag}`)), + } +} + /** * Compile `pattern` into a matcher that cannot backtrack. * - * Returns `null` when RE2 cannot represent the pattern — invalid syntax, or the - * lookaround and backreference constructs RE2 does not implement. Callers must - * handle `null` explicitly rather than silently falling back to the built-in - * engine, which would reintroduce the exposure this exists to remove. + * Returns `null` when RE2 cannot represent the pattern — invalid syntax, or + * constructs RE2 does not implement (lookaround, backreferences, repeat counts + * above 1000). Callers must handle `null` explicitly rather than silently + * falling back to the built-in engine, which would reintroduce the exposure + * this exists to remove. */ export function compileLinearRegex( pattern: string, options: LinearRegexOptions = {} ): LinearRegex | null { try { - const compiled = RE2JS.compile(pattern, options.ignoreCase ? RE2JS.CASE_INSENSITIVE : 0) + const compiled = RE2JS.compile( + translateToRe2(pattern), + options.ignoreCase ? RE2JS.CASE_INSENSITIVE : 0 + ) return { test: (text) => compiled.matcher(text).find(), find: (text) => { diff --git a/apps/sim/lib/logs/log-views.test.ts b/apps/sim/lib/logs/log-views.test.ts index d942ffb9236..de444485adc 100644 --- a/apps/sim/lib/logs/log-views.test.ts +++ b/apps/sim/lib/logs/log-views.test.ts @@ -249,6 +249,22 @@ describe('grepSpans', () => { expect(result.truncated).toBe(true) }) + it('accumulates match time and truncates once the budget is spent', async () => { + // Guards the accumulation in `findTimed`: with that line removed, + // matchTimeMs stays 0, the budget never trips, and every span is scanned. + const big = 'x'.repeat(400_000) + const spans = [ + span({ id: 'a', output: { v: big } }), + span({ id: 'b', output: { v: big } }), + span({ id: 'c', output: { v: `${big} needle` } }), + ] + + const result = await grepSpans(spans, 'needle|nomatch', ctx, { matchTimeBudgetMs: 1 }) + + expect(result.truncated).toBe(true) + expect(result.matches).toEqual([]) + }) + it('does not charge blob-store I/O to the match-time budget', async () => { // Each slice read sleeps well past the budget: only time spent matching // counts, so a slow-but-legitimate grep must still return complete results. diff --git a/apps/sim/lib/logs/log-views.ts b/apps/sim/lib/logs/log-views.ts index cbc1ac0f19a..1f679ce53b0 100644 --- a/apps/sim/lib/logs/log-views.ts +++ b/apps/sim/lib/logs/log-views.ts @@ -194,10 +194,11 @@ export interface GrepSpanMatch { export interface GrepSpansResult { matches: GrepSpanMatch[] /** - * Whether the scan stopped with trace left unread — not whether a budget was - * reached. Every point that skips work goes through `done`, which sets this, - * so a budget exhausted by the very last match reports `false`: the caller's - * results are complete, and nothing was withheld. + * Whether the scan stopped early — because a budget was exhausted, the slice + * cap was hit, or `maxMatches` was reached. It is a "there may be more" flag, + * not proof that trace was left unread: reaching `maxMatches` on the final + * match sets it even when nothing remained. Treat it as a prompt to narrow + * the pattern, never as a count. */ truncated: boolean /** From 76d896de1ae3d6cce34f9a2d1f1b600ceba1b7ac Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 27 Jul 2026 19:43:41 -0700 Subject: [PATCH 8/9] fix(security): correct three defects in the lookaround split decomposition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fourth reviewer, given no context beyond "these scanners are new and unreviewed", differential-tested them against the built-in engine over ~62k comparisons and found three real defects in code added an hour earlier. All were reachable through RegexChunker with plausible patterns, and all silently produced different chunks rather than failing. 1. **Top-level alternation was reshaped into a different pattern.** `(?<=\.)\s+|\n\n` means `((?<=\.)\s+)|(\n\n)`, but decomposing it produced `(?:\.)(\s+|\n\n)` — demanding the period before *both* branches. It could lose boundaries (`"Heading\n\nAlpha beta.\nGamma delta."` split in 2, not 3) and invent them. No grouping recovers the original meaning, so patterns whose middle alternates at the top level are now declined outright, which also removes an asymmetry: `\n\n|(?<=\.)\s` already threw. 2. **A capturing group inside the lookbehind stole group 1.** `(?<=(a))b` cut at the assertion instead of the delimiter, and where the group did not participate `start(1)` was -1, so `find` and `test` contradicted each other. The middle is now captured by name, immune to numbering. 3. **Consuming the assertion text swallowed every other boundary.** The compiled form matched `behind + middle + ahead`, so any boundary whose lookahead doubles as the next one's lookbehind was skipped: `(?<=\w)\s+(?=[A-Z])` over `"A B C D"` split 2 of 3 gaps. Each iteration now resumes at the end of the delimiter rather than the end of the match. This was documented as affecting only self-overlapping delimiters; that was wrong and far too narrow. The reviewer confirmed `translateToRe2` and `closingParen` clean across exhaustive escape-state, character-class, and paren-matching probes, and that nothing added is superlinear. One divergence remains and is now documented rather than overstated: when delimiters themselves overlap (a single-character middle whose matches abut, as in `(?<=\w).(?=\w)`), a match starting behind the cursor is dropped instead of emitting the empty segment the built-in engine produces. Splitters that consume whitespace or punctuation between tokens do not overlap and match exactly. --- .../lib/core/security/linear-regex.test.ts | 29 +++++++ apps/sim/lib/core/security/linear-regex.ts | 85 ++++++++++++++----- 2 files changed, 95 insertions(+), 19 deletions(-) diff --git a/apps/sim/lib/core/security/linear-regex.test.ts b/apps/sim/lib/core/security/linear-regex.test.ts index 74ec29496c8..c6edab05414 100644 --- a/apps/sim/lib/core/security/linear-regex.test.ts +++ b/apps/sim/lib/core/security/linear-regex.test.ts @@ -168,4 +168,33 @@ describe('compileLookaroundSplit', () => { ])('returns null for %s', (_label, pattern) => { expect(compileLookaroundSplit(pattern)).toBeNull() }) + + it.each([ + ['leading assertion', '(?<=\\.)\\s+|\\n\\n'], + ['empty middle', '(?<=

)|
'], + ['trailing assertion', 'a|b(?=c)'], + ])('rejects %s with top-level alternation rather than reshaping it', (_label, pattern) => { + // `(?<=X)A|B` means `((?<=X)A)|B`; rebuilt as `(?:X)(A|B)` it would demand + // the assertion before both branches. No grouping recovers that, so the + // only correct answer is to decline the pattern. + expect(compileLookaroundSplit(pattern)).toBeNull() + }) + + it('is unaffected by a capturing group inside the lookbehind', () => { + // The middle is captured by name, so group numbering cannot shift. + expect(compileLookaroundSplit('(?<=(a))b')?.split('xaby')).toEqual(['xa', 'y']) + + const optional = compileLookaroundSplit('(?<=(a)|b)c') + expect(optional?.find('bc')).toBe(1) + expect(optional?.test('bc')).toBe(true) + }) + + it.each([ + ['(?<=\\w)\\s+(?=[A-Z])', 'A B C D'], + ['(?<=\\w)\\s+(?=\\w)', 'a b c d e'], + ])('does not consume assertion text between boundaries (%s)', (pattern, doc) => { + // The lookahead of one boundary is the lookbehind of the next. Consuming + // it would swallow every other split. + expect(compileLookaroundSplit(pattern)?.split(doc)).toEqual(doc.split(new RegExp(pattern, 'g'))) + }) }) diff --git a/apps/sim/lib/core/security/linear-regex.ts b/apps/sim/lib/core/security/linear-regex.ts index a39df4f67ec..19e248eaa40 100644 --- a/apps/sim/lib/core/security/linear-regex.ts +++ b/apps/sim/lib/core/security/linear-regex.ts @@ -150,6 +150,36 @@ interface SplitShape { ahead: string } +/** + * True when `pattern` has a `|` outside any group or character class. + * + * A split shape cannot be decomposed when its middle alternates at the top + * level: `(?<=\.)\s+|\n\n` means `((?<=\.)\s+)|(\n\n)`, so the assertion binds + * to the first branch only. Rebuilding it as `(?:\.)(\s+|\n\n)` would require + * the period before *either* branch — a silently different pattern. No grouping + * fixes that, so such patterns are rejected rather than reshaped. + */ +function hasTopLevelAlternation(pattern: string): boolean { + let depth = 0 + let inClass = false + for (let i = 0; i < pattern.length; i++) { + const char = pattern[i] + if (char === '\\') { + i += 1 + continue + } + if (inClass) { + if (char === ']') inClass = false + continue + } + if (char === '[') inClass = true + else if (char === '(') depth += 1 + else if (char === ')') depth -= 1 + else if (char === '|' && depth === 0) return true + } + return false +} + /** * Decompose a split pattern into `(?<=behind) middle (?=ahead)`, with either * assertion optional. Returns `null` when neither is present (the caller should @@ -195,6 +225,7 @@ function parseSplitShape(pattern: string): SplitShape | null { } if (!behind && !ahead) return null + if (hasTopLevelAlternation(rest)) return null return { behind, middle: rest, ahead } } @@ -208,13 +239,22 @@ function parseSplitShape(pattern: string): SplitShape | null { * it, `(?<=X)` alone splits after one, and `(?<=[.!?])\s+(?=[A-Z])` — the * sentence splitter — consumes the whitespace between them. * - * Returns `null` for negative lookaround, backreferences, or any body RE2 - * cannot represent. + * Returns `null` for negative lookaround, backreferences, a middle that + * alternates at the top level, or any body RE2 cannot represent. + * + * Two details make the reconstruction faithful rather than approximate. The + * middle is captured by *name*, so a capturing group inside `behind` cannot + * shift the index out from under it. And each iteration resumes the search at + * the end of the previous delimiter rather than at the end of the whole match, + * so the assertion text is not consumed — without that, every boundary whose + * lookahead text doubles as the next boundary's lookbehind would be swallowed + * (`(?<=\w)\s+(?=[A-Z])` over `A B C D` would split only half the gaps). * - * Two divergences from the built-in engine, both narrow: RE2 iterates - * non-overlapping matches, so a self-overlapping delimiter (`(?=aa)` over - * `aaaa`) yields fewer boundaries; and `test`/`find` report on the compiled - * form rather than the zero-width assertion position. + * Known divergence: when delimiters themselves overlap — a single-character + * middle whose matches abut, as in `(?<=\w).(?=\w)` — a match starting behind + * the cursor is dropped instead of emitting the empty segment the built-in + * engine would. Splitters that consume whitespace or punctuation between + * tokens do not overlap and are unaffected. */ export function compileLookaroundSplit( pattern: string, @@ -225,7 +265,7 @@ export function compileLookaroundSplit( const behind = shape.behind ? `(?:${shape.behind})` : '' const ahead = shape.ahead ? `(?:${shape.ahead})` : '' - const source = translateToRe2(`${behind}(${shape.middle})${ahead}`) + const source = translateToRe2(`${behind}(?P${shape.middle})${ahead}`) let compiled: ReturnType try { @@ -234,24 +274,31 @@ export function compileLookaroundSplit( return null } + /** Span of the delimiter itself — what `String.prototype.split` removes. */ + const delimiterAt = (text: string, from: number): { start: number; end: number } | null => { + const matcher = compiled.matcher(text) + if (!matcher.find(from)) return null + return { start: matcher.start('mid'), end: matcher.end('mid') } + } + return { test: (text) => compiled.matcher(text).find(), - find: (text) => { - const matcher = compiled.matcher(text) - return matcher.find() ? matcher.start(1) : -1 - }, + find: (text) => delimiterAt(text, 0)?.start ?? -1, split: (text) => { const segments: string[] = [] - const matcher = compiled.matcher(text) let cursor = 0 - while (matcher.find()) { - const from = matcher.start(1) - const to = matcher.end(1) + let searchFrom = 0 + while (searchFrom <= text.length) { + const span = delimiterAt(text, searchFrom) + if (!span) break + // Always advance, so a zero-width delimiter cannot spin. + searchFrom = span.end > span.start ? span.end : span.start + 1 // Skip a boundary behind the cursor, or one that would only emit an - // empty leading/trailing segment. - if (from < cursor || (from === cursor && to === cursor) || from >= text.length) continue - segments.push(text.slice(cursor, from)) - cursor = to + // empty leading or trailing segment. + if (span.start < cursor || span.start >= text.length) continue + if (span.start === cursor && span.end === cursor) continue + segments.push(text.slice(cursor, span.start)) + cursor = span.end } segments.push(text.slice(cursor)) return segments From 9ba9e6532c458be795375d5cfc4d3a6bd3e88ea7 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 27 Jul 2026 19:54:22 -0700 Subject: [PATCH 9/9] test(security): pin engine parity with a differential suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every defect in this module has been a silent semantic divergence rather than a crash, and each was found by comparing engines over a corpus in a throwaway script. That comparison now lives in the suite: ~700 pattern/document pairs plus match/index/ignore-case parity, checked against the built-in engine on every run, with the accepted divergences enumerated and individually pinned. It earned its place immediately by falsifying my own documentation. The "self-overlapping delimiter" caveat was stale — fixing the boundary-advance in the previous commit also made `(?=#{1,6}\s)`, `(?=aa)` and `(?=##)` exact, so the entry asserting they still diverge failed. One real divergence remains and is now stated precisely: a lookbehind whose body self-overlaps (`(?<=aa)` over `aaaaa`). Making that exact means restarting the scan one character past each match start, which is quadratic on a multi-megabyte document and forfeits the linear guarantee — so it is a deliberate trade, not an oversight. Mutation-verified against the three bugs that actually escaped review: disabling the `\s` translation fails 12 tests, changing the boundary-advance fails 2, and removing the top-level-alternation guard fails 3. That last one initially passed, because the corpus had no alternation pattern in it — the gap is closed and the mutation now fails as it should. --- .../linear-regex.differential.test.ts | 241 ++++++++++++++++++ apps/sim/lib/core/security/linear-regex.ts | 13 +- 2 files changed, 249 insertions(+), 5 deletions(-) create mode 100644 apps/sim/lib/core/security/linear-regex.differential.test.ts diff --git a/apps/sim/lib/core/security/linear-regex.differential.test.ts b/apps/sim/lib/core/security/linear-regex.differential.test.ts new file mode 100644 index 00000000000..02c5abbf516 --- /dev/null +++ b/apps/sim/lib/core/security/linear-regex.differential.test.ts @@ -0,0 +1,241 @@ +/** + * @vitest-environment node + * + * Differential test: every pattern the linear engine accepts must behave like + * the built-in engine. + * + * Every defect found in this module has been a silent semantic divergence, not + * a crash — `\s` losing Unicode whitespace, a decomposed lookaround binding to + * the wrong alternation branch, a consumed lookahead swallowing boundaries. + * Each was caught by comparing engines over a corpus, and each would have + * shipped otherwise, because the code was self-consistent and every + * hand-written example passed. + * + * So the comparison lives here rather than in a scratch script. Any future + * change to `translateToRe2` or the split decomposition is checked against + * `RegExp` across the corpus below, with the known divergences enumerated + * explicitly — a divergence that is not on that list is a bug. + */ + +import { describe, expect, it } from 'vitest' +import { compileLinearRegex, compileLookaroundSplit } from '@/lib/core/security/linear-regex' + +/** How a caller compiles: linear engine first, split decomposition second. */ +function compile(pattern: string) { + return compileLinearRegex(pattern) ?? compileLookaroundSplit(pattern) +} + +/** + * Delimiters people actually write, plus the shapes that have broken before. + * Anything here that the linear engine accepts must split like `RegExp`. + */ +const SPLIT_PATTERNS = [ + // Plain delimiters + '\\n\\n', + '\\n\\n+', + '\\s+', + '\\s{2,}', + '\\.\\s+', + '[.!?]\\s+', + '---+', + '\\|', + ',\\s*', + '\\t', + // Structural + '\\n#{1,6}\\s', + '', + '', + '\\r?\\n', + // Lookaround: keep the delimiter + '(?=#\\s)', + '(?=#{1,6}\\s)', + '(?<=\\.)', + '(?<=)', + '(?<=\\.)\\s+', + '(?<=[.!?])\\s+', + '(?<=[.!?])\\s+(?=[A-Z])', + '(?<=\\w)\\s+(?=[A-Z])', + '(?<=\\w)\\s+(?=\\w)', + '\\n(?=Chapter )', + '\\n(?=\\d+\\.)', + '(?<=;)\\s*', + // Top-level alternation beside an assertion: must be declined, never + // reshaped — `(?<=X)A|B` is not `(?:X)(A|B)`. + '(?<=\\.)\\s+|\\n\\n', + '(?<=

)|
', + 'a|b(?=c)', + '\\n\\n|(?<=\\.)\\s', + // Quantifier and class shapes + '[-=]{3,}', + '\\s*\\n\\s*\\n\\s*', + '(?:\\r\\n|\\n){2}', + '[\\s\\u00b7]+', +] + +/** + * Documents chosen to exercise the divergences that have bitten: non-ASCII + * whitespace from PDF/HTML extraction, CJK, and adjacent delimiters. + */ +const DOCUMENTS = [ + 'Section one\n\nSection two\n\nSection three', + 'One. Two. Three. Four.', + 'A B C D', + 'Heading\n\nAlpha beta.\nGamma delta.', + '# One\nalpha\n## Two\nbeta\n### Three', + 'onetwothree', + 'Chapter 1\nintro\nChapter 2\nmore', + // Non-breaking and exotic whitespace — the stored-data divergence + 'Section 1. Overview The agreement', + 'Bullet one  Bullet two  Bullet three', + 'Prix : 100 EUR. Livraison offerte.', + '第一章　概要　第二章　詳細', + 'line
separator
paragraph', + 'tab\tseparated\tvalues', + // Degenerate + '', + ' ', + '\n\n\n', + 'nodelimiterhere', + 'a,b,', + 'trailing delimiter.\n\n', + 'Heading\n\nAlpha beta.\nGamma delta.\n\nMore.', + 'one

two
three', + 'zabzabz', +] + +/** + * Divergences that are known, documented on the API, and accepted. + * + * Keep this list short and specific — it is the honest boundary of the + * guarantee, so every entry needs a reason, not just a pattern. + */ +const KNOWN_DIVERGENCES: Array<{ pattern: string; doc: string; reason: string }> = [ + { + pattern: '(?<=aa)', + doc: 'aaaaa', + reason: + 'Lookbehind whose body self-overlaps. Matching every position would mean restarting the scan one character past each match start, which is quadratic on a multi-megabyte document and forfeits the linear guarantee this module exists for. Delimiters that do not self-overlap — punctuation, tags, whitespace — are exact.', + }, +] + +function isKnownDivergence(pattern: string): boolean { + return KNOWN_DIVERGENCES.some((entry) => entry.pattern === pattern) +} + +/** + * `LinearRegex.split` omits the trailing empty segment `RegExp` produces, and + * every caller filters empties anyway — so compare on the filtered forms. + */ +function normalize(segments: string[]): string[] { + return segments.filter((segment) => segment !== '') +} + +describe('differential: split parity with the built-in engine', () => { + const cases = SPLIT_PATTERNS.flatMap((pattern) => + DOCUMENTS.map((doc) => ({ pattern, doc })) + ).filter(({ pattern }) => !isKnownDivergence(pattern)) + + it(`covers ${cases.length} pattern/document pairs`, () => { + expect(cases.length).toBeGreaterThan(400) + }) + + it.each(SPLIT_PATTERNS.filter((pattern) => !isKnownDivergence(pattern)))( + 'splits %s identically to RegExp across every document', + (pattern) => { + const compiled = compile(pattern) + // A pattern the linear engine declines is handled by the caller (notice, + // literal fallback, or a thrown config error) — not a parity concern. + if (!compiled) return + + for (const doc of DOCUMENTS) { + expect( + normalize(compiled.split(doc)), + `pattern ${pattern} on ${JSON.stringify(doc)}` + ).toEqual(normalize(doc.split(new RegExp(pattern, 'g')))) + } + } + ) +}) + +describe('differential: test/find parity with the built-in engine', () => { + /** Grep-style patterns, where `test` and the match index are what matter. */ + const MATCH_PATTERNS = [ + 'timeout', + 'ECONNREFUSED', + 'status=\\d+', + 'status=5\\d\\d', + '^Agent', + 'agent$', + '(openai|anthropic)', + '\\bstatus\\b', + '\\d{4}-\\d{2}-\\d{2}', + '[Ee]xception', + 'https?://[^\\s"]+', + '\\$\\{[^}]*\\}', + 'block_\\d+.*output', + '\\s+$', + '^\\s*\\{.*\\}\\s*$', + 'a.*b.*c', + '.*', + '.+', + 'sk-[A-Za-z0-9]{20,}', + 'rate.?limit', + '\\w+@\\w+\\.\\w+', + '[0-9a-f]{8}-[0-9a-f]{4}', + '\\s', + '\\S+', + ] + + const HAYSTACKS = [ + 'Agent 1 called api.openai.com -> status=503 at 2026-01-01', + 'request timeout occurred after 30s', + 'ECONNREFUSED connecting to db', + 'user@example.com signed in', + 'sk-abcdefghijklmnopqrstuvwxyz012345', + 'value with non-breaking space', + '第一章　概要', + '{ "ok": true }', + '', + ' ', + ] + + it.each(MATCH_PATTERNS)('matches %s identically to RegExp', (pattern) => { + const compiled = compile(pattern) + if (!compiled) return + + const oracle = new RegExp(pattern) + for (const text of HAYSTACKS) { + const label = `pattern ${pattern} on ${JSON.stringify(text)}` + expect(compiled.test(text), label).toBe(oracle.test(text)) + + const match = oracle.exec(text) + expect(compiled.find(text), label).toBe(match ? match.index : -1) + } + }) + + it.each(MATCH_PATTERNS)('matches %s identically to RegExp when ignoring case', (pattern) => { + const compiled = compileLinearRegex(pattern, { ignoreCase: true }) + if (!compiled) return + + const oracle = new RegExp(pattern, 'i') + for (const text of HAYSTACKS) { + expect(compiled.test(text), `pattern ${pattern} on ${JSON.stringify(text)}`).toBe( + oracle.test(text) + ) + } + }) +}) + +describe('differential: every known divergence is still exactly as documented', () => { + // Pinned so the list cannot rot. If a divergence is silently fixed this + // fails and the entry must be deleted; if one spreads, the parity suites + // above fail. Either way the list stays honest about the real boundary. + it.each(KNOWN_DIVERGENCES)('$pattern still diverges on $doc — $reason', ({ pattern, doc }) => { + const compiled = compile(pattern) + expect(compiled).not.toBeNull() + + expect(normalize(compiled?.split(doc) ?? [])).not.toEqual( + normalize(doc.split(new RegExp(pattern, 'g'))) + ) + }) +}) diff --git a/apps/sim/lib/core/security/linear-regex.ts b/apps/sim/lib/core/security/linear-regex.ts index 19e248eaa40..a22134473cb 100644 --- a/apps/sim/lib/core/security/linear-regex.ts +++ b/apps/sim/lib/core/security/linear-regex.ts @@ -250,11 +250,14 @@ function parseSplitShape(pattern: string): SplitShape | null { * lookahead text doubles as the next boundary's lookbehind would be swallowed * (`(?<=\w)\s+(?=[A-Z])` over `A B C D` would split only half the gaps). * - * Known divergence: when delimiters themselves overlap — a single-character - * middle whose matches abut, as in `(?<=\w).(?=\w)` — a match starting behind - * the cursor is dropped instead of emitting the empty segment the built-in - * engine would. Splitters that consume whitespace or punctuation between - * tokens do not overlap and are unaffected. + * Known divergence: a delimiter that self-overlaps — `(?<=aa)` over `aaaaa`, + * or a single-character middle whose matches abut as in `(?<=\w).(?=\w)` — + * yields fewer boundaries than the built-in engine. Matching every position + * would mean restarting the scan one character past each match start, which is + * quadratic on a multi-megabyte document and forfeits the linear guarantee + * this module exists for. Delimiters that do not self-overlap — punctuation, + * tags, whitespace between tokens — are exact, and + * `linear-regex.differential.test.ts` pins that. */ export function compileLookaroundSplit( pattern: string,