Skip to content

Commit f980a3d

Browse files
committed
fix(plugin): replace bootstrap marker regex with linear scan to close ReDoS
The non-greedy regex /<SYSTEMATIC_WORKFLOWS>[\s\S]*?<\/SYSTEMATIC_WORKFLOWS>/ was flagged by CodeQL as polynomial-time on uncontrolled input — when the opening tag repeats and the closing tag is absent, the engine backtracks through every prefix. With per-load registration now letting any plugin source contribute system prompt fragments, this regex sees content the plugin itself didn't author. Replaces the regex with a small indexOf/slice helper. Fixed literal delimiters never needed regex; the linear scan is provably immune to ReDoS and unchanged in behavior for the existing seven marker-replacement tests. Adds a regression test that runs the helper against 10000 repeated opening markers with no closing tag and asserts completion in well under 1s. Closes CodeQL alerts #42 and #43.
1 parent 071c886 commit f980a3d

2 files changed

Lines changed: 39 additions & 4 deletions

File tree

src/index.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,32 @@ const bundledAgentsDir = path.join(packageRoot, 'agents')
1818
const bundledCommandsDir = path.join(packageRoot, 'commands')
1919
const packageJsonPath = path.join(packageRoot, 'package.json')
2020

21-
const BOOTSTRAP_MARKER_PATTERN =
22-
/<SYSTEMATIC_WORKFLOWS>[\s\S]*?<\/SYSTEMATIC_WORKFLOWS>/
21+
const BOOTSTRAP_MARKER_OPEN = '<SYSTEMATIC_WORKFLOWS>'
22+
const BOOTSTRAP_MARKER_CLOSE = '</SYSTEMATIC_WORKFLOWS>'
23+
24+
const findBootstrapMarkerBlock = (
25+
entry: string,
26+
): { start: number; end: number } | null => {
27+
const start = entry.indexOf(BOOTSTRAP_MARKER_OPEN)
28+
if (start === -1) return null
29+
const closeStart = entry.indexOf(
30+
BOOTSTRAP_MARKER_CLOSE,
31+
start + BOOTSTRAP_MARKER_OPEN.length,
32+
)
33+
if (closeStart === -1) return null
34+
return { start, end: closeStart + BOOTSTRAP_MARKER_CLOSE.length }
35+
}
2336

2437
export const applyBootstrapContent = (
2538
output: { system: string[] },
2639
content: string,
2740
): void => {
2841
for (let i = 0; i < output.system.length; i++) {
2942
const entry = output.system[i]
30-
if (BOOTSTRAP_MARKER_PATTERN.test(entry)) {
31-
output.system[i] = entry.replace(BOOTSTRAP_MARKER_PATTERN, content)
43+
const block = findBootstrapMarkerBlock(entry)
44+
if (block !== null) {
45+
output.system[i] =
46+
entry.slice(0, block.start) + content + entry.slice(block.end)
3247
return
3348
}
3449
}

tests/unit/plugin.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -575,4 +575,24 @@ describe('applyBootstrapContent marker-based idempotency', () => {
575575
expect(joined).toContain('SECOND REGISTRATION')
576576
expect(joined).not.toContain('FIRST REGISTRATION')
577577
})
578+
579+
test('completes in linear time on malicious input with many opening markers and no closing tag', () => {
580+
// Regression: the prior regex implementation was vulnerable to ReDoS on
581+
// inputs starting with the opening marker repeated and missing a closing
582+
// tag. The linear-scan helper is provably immune; this pins the failure
583+
// mode without relying on benchmarking.
584+
const malicious = `${MARKER_OPEN.repeat(10000)} trailing content with no close`
585+
const output = { system: [malicious] }
586+
const startedAt = Date.now()
587+
applyBootstrapContent(output, wrap('NEW CONTENT'))
588+
const elapsedMs = Date.now() - startedAt
589+
// No marker block matched (no closing tag), so content is appended.
590+
expect(output.system).toHaveLength(1)
591+
expect(output.system[0]).toContain('NEW CONTENT')
592+
expect(output.system[0]).toContain(MARKER_OPEN.repeat(10000))
593+
// Loose upper bound — linear scan finishes in well under 100ms even on
594+
// slow CI runners. A polynomial-backtracking regex would take seconds or
595+
// longer on this input shape.
596+
expect(elapsedMs).toBeLessThan(1000)
597+
})
578598
})

0 commit comments

Comments
 (0)