From 6aa3e0e29b3444ee98d974875595fccce2d85ead Mon Sep 17 00:00:00 2001 From: Waleed Date: Mon, 27 Jul 2026 19:57:30 -0700 Subject: [PATCH] fix(logs): match copilot log-grep patterns with RE2 (linear time) (#5987) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(logs): match copilot log-grep patterns literally, never as a regex `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. * improvement(logs): only flag genuine regex intent, document the truncated invariant 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. * fix(logs): match log-grep patterns with RE2 instead of dropping regex support 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. * fix(security): route every caller-supplied regex through the linear-time engine 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`. * fix(security): drop safe-regex2, keep lookaround splits on the linear engine 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. * test(pii): drop the custom-pattern editor's backtracking-screen assertion 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". * fix(chunkers): support lookaround with an affix, and keep JS whitespace semantics 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. * fix(security): correct three defects in the lookaround split decomposition 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. * test(security): pin engine parity with a differential suite 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. --- apps/sim/blocks/blocks/guardrails.ts | 6 +- .../pii/custom-patterns-editor.test.tsx | 16 +- .../components/pii/custom-patterns-editor.tsx | 10 +- apps/sim/lib/api/contracts/primitives.test.ts | 20 +- apps/sim/lib/chunkers/regex-chunker.test.ts | 34 ++ apps/sim/lib/chunkers/regex-chunker.ts | 63 +-- .../tools/server/workflow/query-logs.ts | 7 +- apps/sim/lib/copilot/vfs/operations.test.ts | 32 ++ apps/sim/lib/copilot/vfs/operations.ts | 44 ++- .../linear-regex.differential.test.ts | 241 ++++++++++++ .../lib/core/security/linear-regex.test.ts | 200 ++++++++++ apps/sim/lib/core/security/linear-regex.ts | 367 ++++++++++++++++++ .../sim/lib/guardrails/validate_regex.test.ts | 67 ++++ apps/sim/lib/guardrails/validate_regex.ts | 65 +++- apps/sim/lib/logs/log-views.test.ts | 111 +++++- apps/sim/lib/logs/log-views.ts | 130 ++++++- apps/sim/package.json | 2 +- bun.lock | 8 +- 18 files changed, 1324 insertions(+), 99 deletions(-) create mode 100644 apps/sim/lib/core/security/linear-regex.differential.test.ts 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/blocks/blocks/guardrails.ts b/apps/sim/blocks/blocks/guardrails.ts index 1b89324395..6b857f690f 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 ((?<=...), (? { 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() diff --git a/apps/sim/components/pii/custom-patterns-editor.tsx b/apps/sim/components/pii/custom-patterns-editor.tsx index 23ba59f658..4be1204f5b 100644 --- a/apps/sim/components/pii/custom-patterns-editor.tsx +++ b/apps/sim/components/pii/custom-patterns-editor.tsx @@ -15,9 +15,13 @@ interface CustomPatternsEditorProps { /** * Editor for user-supplied custom regex patterns. Each row is a name label, the - * regex (validated inline for syntax + catastrophic-backtracking safety), and the - * verbatim replacement token that matches are redacted to. Shared by the Data - * Retention settings and any other PII-policy surface. + * regex (validated inline for syntax only), and the verbatim replacement token + * that matches are redacted to. Shared by the Data Retention settings and any + * other PII-policy surface. + * + * Syntax is the only check because these patterns execute in Presidio, not in + * this process — see `validateRegexPattern` for why a backtracking screen here + * was removed rather than kept. */ export function CustomPatternsEditor({ patterns, onChange }: CustomPatternsEditorProps) { function updateRow(index: number, patch: Partial) { diff --git a/apps/sim/lib/api/contracts/primitives.test.ts b/apps/sim/lib/api/contracts/primitives.test.ts index 82e5e42514..727a42a65f 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.test.ts b/apps/sim/lib/chunkers/regex-chunker.test.ts index f4f55112e5..96d84cc5ed 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', 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({ + 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 0cafa47b0d..b294a504ee 100644 --- a/apps/sim/lib/chunkers/regex-chunker.ts +++ b/apps/sim/lib/chunkers/regex-chunker.ts @@ -10,6 +10,11 @@ import { splitAtWordBoundaries, tokensToChars, } from '@/lib/chunkers/utils' +import { + compileLinearRegex, + compileLookaroundSplit, + type LinearRegex, +} from '@/lib/core/security/linear-regex' const logger = createLogger('RegexChunker') @@ -56,7 +61,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 +72,24 @@ 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. + * + * 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) { throw new Error('Regex pattern is required') } @@ -77,34 +99,18 @@ export class RegexChunker { } 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') - } - } - - regex.lastIndex = 0 - return regex + new RegExp(pattern) } catch (error) { - if (error instanceof Error && error.message.includes('catastrophic')) { - throw 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. Unsupported: negative lookaround ("(?!...)", "(? { @@ -119,8 +125,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/tools/server/workflow/query-logs.ts b/apps/sim/lib/copilot/tools/server/workflow/query-logs.ts index 4e4af600e8..9ab073f7dd 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/copilot/vfs/operations.test.ts b/apps/sim/lib/copilot/vfs/operations.test.ts index 32d1ad3900..e626b70284 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 bd7208719b..624707b43e 100644 --- a/apps/sim/lib/copilot/vfs/operations.ts +++ b/apps/sim/lib/copilot/vfs/operations.ts @@ -1,4 +1,13 @@ +import { createLogger } from '@sim/logger' import micromatch from 'micromatch' +import { + compileLinearRegex, + isPlainText, + type LinearRegex, + literalRegex, +} from '@/lib/core/security/linear-regex' + +const logger = createLogger('VfsOperations') export interface GrepMatch { path: string @@ -125,7 +134,16 @@ 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` (see `@/lib/core/security/linear-regex`). + * + * A pattern RE2 cannot represent — negative lookaround, backreferences — is + * matched as a literal instead of on the backtracking engine, as is a pattern + * that does not compile at all (which previously returned no results). Both + * cases log a warning: the return shape carries results only, so there is + * nowhere to tell the caller inline, and a literal fallback can match the + * pattern's own text when grepping source that contains regexes. + * * `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 +160,27 @@ 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. + 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[] = [] 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 +196,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 +213,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.differential.test.ts b/apps/sim/lib/core/security/linear-regex.differential.test.ts new file mode 100644 index 0000000000..02c5abbf51 --- /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.test.ts b/apps/sim/lib/core/security/linear-regex.test.ts new file mode 100644 index 0000000000..c6edab0541 --- /dev/null +++ b/apps/sim/lib/core/security/linear-regex.test.ts @@ -0,0 +1,200 @@ +/** + * @vitest-environment node + */ + +import { describe, expect, it } from 'vitest' +import { + compileLinearRegex, + compileLookaroundSplit, + 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, 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([ + ['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.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', () => { + 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([ + ['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'], + ['negative lookbehind', '(? { + 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 new file mode 100644 index 0000000000..a22134473c --- /dev/null +++ b/apps/sim/lib/core/security/linear-regex.ts @@ -0,0 +1,367 @@ +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. 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 +} + +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. + * + * 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. */ +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 !METACHARACTERS.test(pattern) +} + +/** + * ECMAScript's `\s` as an RE2 character-class body. + * + * 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. + */ +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 +} + +interface SplitShape { + behind: string + middle: string + 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 + * 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 + if (hasTopLevelAlternation(rest)) return null + return { behind, middle: rest, ahead } +} + +/** + * Compile a lookaround *split* pattern onto RE2, which has no lookaround. + * + * 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` 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). + * + * 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, + options: LinearRegexOptions = {} +): LinearRegex | 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}(?P${shape.middle})${ahead}`) + + let compiled: ReturnType + try { + compiled = RE2JS.compile(source, options.ignoreCase ? RE2JS.CASE_INSENSITIVE : 0) + } catch { + 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) => delimiterAt(text, 0)?.start ?? -1, + split: (text) => { + const segments: string[] = [] + let cursor = 0 + 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 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 + }, + } +} + +/** + * 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 + * 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( + translateToRe2(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 0000000000..905b869edb --- /dev/null +++ b/apps/sim/lib/guardrails/validate_regex.test.ts @@ -0,0 +1,67 @@ +/** + * @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.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 442daa7b3e..f8a90dc6e5 100644 --- a/apps/sim/lib/guardrails/validate_regex.ts +++ b/apps/sim/lib/guardrails/validate_regex.ts @@ -1,4 +1,8 @@ -import safe from 'safe-regex2' +import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' +import { compileLinearRegex } from '@/lib/core/security/linear-regex' + +const logger = createLogger('ValidateRegex') /** * Validate if input matches regex pattern @@ -15,9 +19,24 @@ 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. + * + * 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. */ export function validateRegexPattern(pattern: string): RegexPatternValidation { if (pattern.length === 0) { @@ -26,34 +45,42 @@ 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 } } +/** + * 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) { + // 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: '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.test.ts b/apps/sim/lib/logs/log-views.test.ts index 9b31ed4feb..de444485ad 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,114 @@ describe('grepSpans', () => { expect(result.matches.some((m) => m.field === 'output')).toBe(true) }) - it('falls back to literal substring on invalid regex', async () => { - const spans = [span({ output: { v: 'value a(b found' } })] - const result = await grepSpans(spans, '(', ctx) + 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('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'], + ])('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. + // 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.truncated).toBe(false) + }) + + 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('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. + 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 e7e4feacee..1f679ce53b 100644 --- a/apps/sim/lib/logs/log-views.ts +++ b/apps/sim/lib/logs/log-views.ts @@ -1,3 +1,4 @@ +import { compileLinearRegex, isPlainText, literalRegex } from '@/lib/core/security/linear-regex' import { materializeLargeArrayManifest, readLargeArrayManifestSlice, @@ -23,6 +24,27 @@ 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 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. 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 + // --------------------------------------------------------------------------- // Overview (Level 2): block tree with timing + cost, NO input/output. // --------------------------------------------------------------------------- @@ -171,40 +193,86 @@ export interface GrepSpanMatch { export interface GrepSpansResult { matches: GrepSpanMatch[] + /** + * 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 + /** + * 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 } 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 - regex: RegExp + maxScannedChars: number + matchTimeBudgetMs: number + find: FindMatch } -function escapeRegExp(input: string): string { - return input.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') -} +/** Index of the first case-insensitive match in `text`, or -1. */ +type FindMatch = (text: string) => number -function buildRegex(pattern: string): RegExp { - try { - return new RegExp(pattern, 'i') - } catch { - return new RegExp(escapeRegExp(pattern), 'i') +/** + * 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. See `@/lib/core/security/linear-regex` for why + * the engine changed rather than the pattern being screened. + * + * 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 (isPlainText(pattern)) return { find: literalRegex(pattern, { ignoreCase: true }).find } + + 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.', } } -function snippetAround(text: string, regex: RegExp, maxChars: number): string { - const m = regex.exec(text) - const index = m ? m.index : 0 +function findTimed(text: string, state: GrepState): number { + const started = performance.now() + try { + return state.find(text) + } finally { + state.matchTimeMs += performance.now() - started + } +} + +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) const end = Math.min(text.length, start + maxChars) @@ -214,7 +282,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 +297,19 @@ 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 + 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.regex, state.maxSnippetChars), + snippet: snippetAround(text, index, state), }) if (state.matches.length >= state.maxMatches) state.truncated = true } @@ -305,6 +382,12 @@ 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 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[], @@ -312,14 +395,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, + find: compiled.find, } const walk = async (list: TraceSpan[]): Promise => { @@ -335,5 +423,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 } : {}), + } } diff --git a/apps/sim/package.json b/apps/sim/package.json index b0205c85f7..8ef25a4b95 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", @@ -206,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 20cc0d013c..597bc798df 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", @@ -281,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", @@ -3598,6 +3598,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=="], @@ -3696,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=="], @@ -3734,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=="],