From 01b691fae8db27d90a14c0dc5273c39a9b8c9a00 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 10 Jul 2026 10:42:18 -0400 Subject: [PATCH] fix(#2128): route scans through a hardened shared seam (root-cause ReDoS fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A convergence audit showed the `[\s\S]*?` lazy-scan ReDoS was pervasive (a dozen+ bespoke copies across roadmap-parser/check-command-router/verify), each a distinct quadratic vector on a large document with unclosed tags. Rather than whack-a-mole, single-source them (maintainer-directed): - markdown-sectionizer: extractTaggedBlocks now shares one ReDoS-safe `taggedBlockPattern` (stop-at-next-open, bounded optional attributes) and gains a `stripTaggedBlocks` companion for block removal. - roadmap-parser: 3 `
` strips -> stripTaggedBlocks (behavior-identical — no
here carries attributes). - verify: actionZones + both loops + their nested //gate/req extractions -> extractTaggedBlocks (behavior byte-equivalent, verified). - check-command-router: the objective|tasks?|action alternation hardened in place (distinct multi-tag shape); HTML-comment strip gains a `$` fallback. Every vector now linear (<3ms on 1.5MB adversarial); real content unchanged (end-to-end verify/roadmap resolution + task extraction confirmed). The only remaining `` scan (uat.cts:201) is anchored + non-global — one scan, safe. Co-Authored-By: Claude Opus 4.8 --- src/check-command-router.cts | 4 ++-- src/markdown-sectionizer.cts | 35 ++++++++++++++++++++++++++---- src/roadmap-parser.cts | 10 ++++----- src/verify.cts | 42 +++++++++++------------------------- 4 files changed, 50 insertions(+), 41 deletions(-) diff --git a/src/check-command-router.cts b/src/check-command-router.cts index 3a712bb14..329424138 100644 --- a/src/check-command-router.cts +++ b/src/check-command-router.cts @@ -139,11 +139,11 @@ function loadPlanContents(phaseDir: string): string[] { } const DESIGNATED_HEADINGS_RE = /^#{1,6}\s+(?:must[_ ]haves?|truths?|tasks?|objective)\b/i; -const XML_DECISION_TAGS_RE = /<(?:objective|tasks?|action)(?:\s[^>]*)?>([\s\S]*?)<\/(?:objective|tasks?|action)>/gi; +const XML_DECISION_TAGS_RE = /<(?:objective|tasks?|action)(?:\s[^>]{0,1000})?>((?:(?!<(?:objective|tasks?|action)[\s>])[\s\S])*?)<\/(?:objective|tasks?|action)>/gi; function stripCommentsAndFences(text: string): string { // HTML-comment stripping stays caller-side (the seam does not strip HTML comments). - const htmlStripped = text.replace(//g, ' '); + const htmlStripped = text.replace(/|$)/g, ' '); // Fenced-code stripping: delegate to the canonical CommonMark-correct seam. // replaces the prior independent regex copy (```` ``` ``` ```` + `~~~ ~~~`). return stripFencedCode(htmlStripped).text; diff --git a/src/markdown-sectionizer.cts b/src/markdown-sectionizer.cts index 8a745d37a..c152f8acc 100644 --- a/src/markdown-sectionizer.cts +++ b/src/markdown-sectionizer.cts @@ -535,10 +535,7 @@ export function extractTaggedBlocks(content: string, tagName: string): string[] if (typeof content !== 'string' || content.length === 0) return []; if (typeof tagName !== 'string' || tagName.length === 0) return []; - // Escape the tag name for safe interpolation into a RegExp. - const escapedTag = tagName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - const pattern = new RegExp(`<${escapedTag}>((?:(?!<${escapedTag}>)[\\s\\S])*?)`, 'g'); - + const pattern = taggedBlockPattern(tagName, 'g'); const results: string[] = []; let match: RegExpExecArray | null; while ((match = pattern.exec(content)) !== null) { @@ -547,6 +544,36 @@ export function extractTaggedBlocks(content: string, tagName: string): string[] return results; } +/** + * Build the single, ReDoS-safe `…` block regex shared by + * `extractTaggedBlocks` (extract bodies) and `stripTaggedBlocks` (remove blocks). + * + * Safety: the body uses a `(?:(?!])[\s\S])*?` negative-lookahead scan + * that terminates at the NEXT opening `` instead of lazily rescanning the + * whole remaining document for a `` that may never appear — so a large + * document full of unclosed `` openings stays LINEAR, not quadratic + * (#2128). The opening tag tolerates optional attributes (``), + * bounded to 1000 chars so the attribute scan cannot itself ReDoS. + * Group 1 is the block body. + */ +function taggedBlockPattern(tagName: string, flags: string): RegExp { + const esc = tagName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + return new RegExp(`<${esc}(?:\\s[^>]{0,1000})?>((?:(?!<${esc}[\\s>])[\\s\\S])*?)`, flags); +} + +/** + * Remove every `…` block (opening tag, body, and closing tag) + * from `content`. The ReDoS-safe counterpart to `extractTaggedBlocks` — same + * hardened pattern, `.replace(…, '')` instead of body extraction. Case-insensitive + * by default (matching the `
` strip call sites); pass `caseSensitive` + * to force exact-case matching. + */ +export function stripTaggedBlocks(content: string, tagName: string, caseSensitive = false): string { + if (typeof content !== 'string' || content.length === 0) return ''; + if (typeof tagName !== 'string' || tagName.length === 0) return content; + return content.replace(taggedBlockPattern(tagName, caseSensitive ? 'g' : 'gi'), ''); +} + // ─── replaceSection ─────────────────────────────────────────────────────────── /** diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index 4cadfdd75..b0236d065 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -32,7 +32,7 @@ const { import planningWorkspace = require('./planning-workspace.cjs'); const { planningDir } = planningWorkspace; import { platformReadSync } from './shell-command-projection.cjs'; -import { tokenizeHeadings } from './markdown-sectionizer.cjs'; +import { tokenizeHeadings, stripTaggedBlocks } from './markdown-sectionizer.cjs'; // ─── Roadmap milestone scoping ─────────────────────────────────────────────── @@ -40,7 +40,7 @@ import { tokenizeHeadings } from './markdown-sectionizer.cjs'; * Strip shipped milestone content wrapped in
blocks. */ function stripShippedMilestones(content: string): string { - return content.replace(/
[\s\S]*?<\/details>/gi, ''); + return stripTaggedBlocks(content, 'details'); } /** @@ -96,8 +96,7 @@ function extractCurrentMilestone(content: string, cwd?: string): string { const anyMilestoneOrDetails = /^#{1,3}\s+(?!Phase\s+\S)(?:.*v\d+\.\d+|✅|📋|🚧|🔄)|
[\s\S]*?<\/details>/gi, '') + const preamble = stripTaggedBlocks(content.slice(0, preambleCutoff), 'details') // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). .replace(/^#{2,4}\s*Phase\s+[\w][\w.-]*(?:\s*\([^)\n]{0,200}\))?\s*:[^\n]*(?:\n(?!#{1,6}\s)[^\n]*)*\n?/gim, '') .replace(/^#{1,4}\s*Phase Details\b[^\n]*\n?/gim, ''); @@ -177,8 +176,7 @@ function extractCurrentMilestone(content: string, cwd?: string): string { ); } - const preamble = beforeMilestones - .replace(/
[\s\S]*?<\/details>/gi, '') + const preamble = stripTaggedBlocks(beforeMilestones, 'details') // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). .replace(/^#{2,4}\s*Phase\s+[\w][\w.-]*(?:\s*\([^)\n]{0,200}\))?\s*:[^\n]*(?:\n(?!#{1,6}\s)[^\n]*)*\n?/gim, '') .replace(/^#{1,4}\s*Phase Details\b[^\n]*\n?/gim, ''); diff --git a/src/verify.cts b/src/verify.cts index 184344cf8..8aa3a4778 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -26,6 +26,7 @@ import { PACKAGE_NAME } from './package-identity.cjs'; import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs'; import { detectSchemaFiles, checkSchemaDrift } from './schema-detect.cjs'; import { isCanonicalPlanningFile } from './artifacts.cjs'; +import { extractTaggedBlocks } from './markdown-sectionizer.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports -- agent-install-check.cjs is an export= CommonJS module import agentInstallCheck = require('./agent-install-check.cjs'); const { checkAgentsInstalled } = agentInstallCheck; @@ -205,10 +206,7 @@ function scanNegativeGrepCommentEcho(content: string): { errors: string[]; warni // while a prose echo on the same line is still caught. const cmdSpanRe = /grep(?:\s+-{1,2}[A-Za-z][A-Za-z-]*)+\s+(?:'[^']*'|"[^"]*"|[^\s'"|>&;]+)[^\n]*?(?:==|-eq|=)\s*0\b/g; - const actionZones: string[] = []; - const actionRe = /([\s\S]*?)<\/action>/g; - let acm: RegExpExecArray | null; - while ((acm = actionRe.exec(text)) !== null) actionZones.push(acm[1]); + const actionZones = extractTaggedBlocks(text, 'action'); const scannableActionText = actionZones.map((zone) => zone.replace(cmdSpanRe, ' ')).join('\n'); // 3. Per shell SEGMENT (split lines on && / ||) extract count-grep literals and @@ -364,32 +362,21 @@ function scanFileWideNegativeGateConflict(content: string): { warnings: string[] gateText: string; // ++ text reqText: string; // + text (requirement side) } - const taskRe = /]*>([\s\S]*?)<\/task>/g; const tasks: TaskInfo[] = []; - let tm: RegExpExecArray | null; - while ((tm = taskRe.exec(text)) !== null) { - const tc = tm[1]; + for (const tc of extractTaggedBlocks(text, 'task')) { // Extract task name. - const namem = tc.match(/([\s\S]*?)<\/name>/); - const name = namem ? namem[1].trim() : 'unnamed'; + const namem = extractTaggedBlocks(tc, 'name'); + const name = namem.length ? namem[0].trim() : 'unnamed'; // Extract entries. - const filesm = tc.match(/((?:(?!)[\s\S])*?)<\/files>/); - const filesText = filesm ? filesm[1] : ''; + const filesArr = extractTaggedBlocks(tc, 'files'); + const filesText = filesArr.length ? filesArr[0] : ''; const files = filesText.split(/[,\s]+/).map(s => s.trim()).filter(Boolean); // Gate text: //. const gateFragments: string[] = []; - for (const tag of ['verify', 'automated', 'acceptance_criteria']) { - const re = new RegExp(`<${tag}>((?:(?!<${tag}>)[\\s\\S])*?)<\\/${tag}>`, 'g'); - let mm: RegExpExecArray | null; - while ((mm = re.exec(tc)) !== null) gateFragments.push(mm[1]); - } + for (const tag of ['verify', 'automated', 'acceptance_criteria']) gateFragments.push(...extractTaggedBlocks(tc, tag)); // Requirement text: /. const reqFragments: string[] = []; - for (const tag of ['action', 'acceptance_criteria']) { - const re = new RegExp(`<${tag}>((?:(?!<${tag}>)[\\s\\S])*?)<\\/${tag}>`, 'g'); - let mm: RegExpExecArray | null; - while ((mm = re.exec(tc)) !== null) reqFragments.push(mm[1]); - } + for (const tag of ['action', 'acceptance_criteria']) reqFragments.push(...extractTaggedBlocks(tc, tag)); // Strip XML tags from gate text so segments containing embedded // XML closing tags (e.g. cmd nested inside ) // don't bleed into the file-path token extraction. @@ -577,19 +564,16 @@ function cmdVerifyPlanStructure(cwd: string, filePath: string, raw: boolean): vo if (fm[field] === undefined) errors.push(`Missing required frontmatter field: ${field}`); } - const taskPattern = /]*>([\s\S]*?)<\/task>/g; const tasks: Record[] = []; - let taskMatch: RegExpExecArray | null; - while ((taskMatch = taskPattern.exec(content)) !== null) { - const taskContent = taskMatch[1]; - const nameMatch = taskContent.match(/([\s\S]*?)<\/name>/); - const taskName = nameMatch ? nameMatch[1].trim() : 'unnamed'; + for (const taskContent of extractTaggedBlocks(content, 'task')) { + const nameArr = extractTaggedBlocks(taskContent, 'name'); + const taskName = nameArr.length ? nameArr[0].trim() : 'unnamed'; const hasFiles = //.test(taskContent); const hasAction = //.test(taskContent); const hasVerify = //.test(taskContent); const hasDone = //.test(taskContent); - if (!nameMatch) errors.push('Task missing element'); + if (nameArr.length === 0) errors.push('Task missing element'); if (!hasAction) errors.push(`Task '${taskName}' missing `); if (!hasVerify) warnings.push(`Task '${taskName}' missing `); if (!hasDone) warnings.push(`Task '${taskName}' missing `);