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.

Author: waleedlatif1

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', '(?<=</p>)|<hr>'],
+    ['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')))
+  })
 })

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<mid>${shape.middle})${ahead}`)
 
   let compiled: ReturnType<typeof RE2JS.compile>
   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