Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion apps/sim/blocks/blocks/guardrails.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,11 @@ export const GuardrailsBlock: BlockConfig<GuardrailsResponse> = {
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 ((?<=...), (?<!...)),
no backreferences (\\1), and no \\uXXXX escapes (use \\x41 or the literal character).
Patterns are matched by a linear-time engine that does not implement these, and one
that uses them fails every check. To express "must NOT contain X", match X and invert
the result downstream with a Condition block instead of using negative lookahead.
- Properly escaped for special characters
- Optimized for the use case

Expand Down
16 changes: 12 additions & 4 deletions apps/sim/components/pii/custom-patterns-editor.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,18 @@ describe('CustomPatternsEditor', () => {
expect(container.textContent).toMatch(/Invalid regex/)
})

it('shows an inline error for a catastrophic-backtracking pattern', () => {
renderEditor([row('(a+)+$')], vi.fn())
expect(container.textContent).toMatch(/potentially unsafe/)
})
it.each(['(a+)+$', '(?<=id: )\\w+', '(?:https?://)?example\\.com'])(
'accepts %s, which the removed safe-regex2 screen rejected',
(pattern) => {
// The editor no longer runs a catastrophic-backtracking screen. It caught
// `(a+)+$` but passed `a*a*b`, so it deterred only the obvious spelling
// while blocking valid Presidio patterns like lookbehind. These patterns
// run in Presidio, not in this process.
renderEditor([row(pattern)], vi.fn())
expect(container.textContent).not.toMatch(/potentially unsafe/)
expect(container.textContent).not.toMatch(/Invalid regex/)
}
)

it('appends an empty row when "Add pattern" is clicked', () => {
const onChange = vi.fn()
Expand Down
10 changes: 7 additions & 3 deletions apps/sim/components/pii/custom-patterns-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<CustomPiiPattern>) {
Expand Down
20 changes: 16 additions & 4 deletions apps/sim/lib/api/contracts/primitives.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
})

Expand Down
34 changes: 34 additions & 0 deletions apps/sim/lib/chunkers/regex-chunker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})
})
})
63 changes: 34 additions & 29 deletions apps/sim/lib/chunkers/regex-chunker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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) {
Expand All @@ -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')
}
Expand All @@ -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 ("(?!...)", "(?<!...)"), backreferences, and repeat counts above 1000. Positive lookaround is supported — "(?=X)" splits before a delimiter, "(?<=X)" after one, and "(?<=X)Y(?=Z)" consumes Y between them.`
)
}

async chunk(content: string): Promise<Chunk[]> {
Expand All @@ -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) {
Expand Down
7 changes: 6 additions & 1 deletion apps/sim/lib/copilot/tools/server/workflow/query-logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,12 +145,17 @@ export const queryLogsServerTool: BaseServerTool<QueryLogsArgs, unknown> = {

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,
}
Expand Down
32 changes: 32 additions & 0 deletions apps/sim/lib/copilot/vfs/operations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
44 changes: 34 additions & 10 deletions apps/sim/lib/copilot/vfs/operations.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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) {
Expand All @@ -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)
Expand Down
Loading
Loading