From b321bc04f43417d15d1b39af278fe288779d1016 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 10 Jul 2026 09:44:49 -0400 Subject: [PATCH] =?UTF-8?q?fix(#2128):=20bound=20the=20sibling=20bracket-p?= =?UTF-8?q?refix=20clause=20=E2=80=94=20complete=20the=20ReDoS=20fix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review caught that the prior commit bounded only the paren tag clause and left the SIBLING bracket-prefix `(?:\[[^\]]+\]\s*)?` (same host regexes, before Phase) UNBOUNDED — the identical quadratic reachable via a `[...]` run (measured ~16s at 1.7MB). Bound `[^\]]+`/`[^\]]*` -> {1,200}/{0,200} across all 19 phase/milestone heading prefixes. Comprehensive re-measurement now shows EVERY vector linear (bracket/paren/id/name/milestone all ~2-44ms at 2.45MB; bracket scaling 2k->2ms, 4k->5ms, 8k->10ms). Also: update the #1729 literal-mirror parity test off its stale unbounded constant, and add limit-1 (199) boundary coverage. Co-Authored-By: Claude Opus 4.8 --- gsd-core/bin/lib/state-transition.cjs | 2 +- src/commands.cts | 2 +- src/phase.cts | 2 +- src/roadmap-command-router.cts | 6 +++--- src/roadmap-parser.cts | 6 +++--- src/roadmap-upgrade.cts | 8 ++++---- src/roadmap.cts | 6 +++--- src/state-transition.cts | 2 +- src/validate.cts | 2 +- src/verify.cts | 4 ++-- tests/phase.test.cjs | 15 +++++++++------ 11 files changed, 29 insertions(+), 26 deletions(-) diff --git a/gsd-core/bin/lib/state-transition.cjs b/gsd-core/bin/lib/state-transition.cjs index e56900497..12937e4ad 100644 --- a/gsd-core/bin/lib/state-transition.cjs +++ b/gsd-core/bin/lib/state-transition.cjs @@ -1442,7 +1442,7 @@ function reconcileByPhaseTable(content, deps, timestamp, log) { * source to substitute. This is honest — better than silently leaving `[X]` * which looks like a value. */ -const TEMPLATE_PLACEHOLDER_VALUE = /^\s*\[[^\]]+\]\s*$|^\s*-\s*$/; +const TEMPLATE_PLACEHOLDER_VALUE = /^\s*\[[^\]]{1,200}\]\s*$|^\s*-\s*$/; function stripTemplatePlaceholders(content, timestamp, log) { // Scan body `**Field:** value` lines; when value matches the placeholder // shape, replace with `(pending)`. We deliberately do NOT touch fields that diff --git a/src/commands.cts b/src/commands.cts index 1e52eb6fb..93419df94 100644 --- a/src/commands.cts +++ b/src/commands.cts @@ -1517,7 +1517,7 @@ function cmdStats(cwd: string, format: string | undefined, raw: boolean): void { // Matches both plain numeric (Phase 1:) and milestone-prefixed (Phase 2-01:) headings. // Also tolerates optional [bracket-token] scope prefix on phase headings. // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const headingPattern = /#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]{0,200}\))?\s*:\s*([^\n]+)/gi; + const headingPattern = /#{2,4}\s*(?:\[[^\]]{1,200}\]\s*)?Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]{0,200}\))?\s*:\s*([^\n]+)/gi; let match: RegExpExecArray | null; while ((match = headingPattern.exec(roadmapContent)) !== null) { const key = normalizePhaseName(match[1]); diff --git a/src/phase.cts b/src/phase.cts index 6ae72d999..24c68bf5d 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -719,7 +719,7 @@ function cmdPhaseAdd(cwd: string, description: string, raw: boolean, customId?: // The lookahead accepts colon, decimal-dot, whitespace, bold-close asterisk, // or end-of-line so titleless forms ("- [ ] **Phase 11**", "- [ ] Phase 11") // are counted and cannot collide with a freshly-added phase. (#1229) - const bulletPattern = /^[ \t]*-[ \t]*\[[^\]]*\][ \t]*\*{0,2}Phase[ \t]+(\d+)(?=[:.\s*]|$)/gim; + const bulletPattern = /^[ \t]*-[ \t]*\[[^\]]{0,200}\][ \t]*\*{0,2}Phase[ \t]+(\d+)(?=[:.\s*]|$)/gim; const usedPhaseNums = new Set(); let m: RegExpExecArray | null; diff --git a/src/roadmap-command-router.cts b/src/roadmap-command-router.cts index 142c572e8..7ef26d5a1 100644 --- a/src/roadmap-command-router.cts +++ b/src/roadmap-command-router.cts @@ -67,14 +67,14 @@ function checkW021(content: string): W021Warning[] { // Milestone section heading: ## [GSD] v2.0 — Label OR ## v2.0: Label OR ## Roadmap v2.0 // OR ## ✅ v2.0 OR ## 🚧 v2.0 (emoji-prefixed variants used by roadmap templates) // Capture the major integer. - const MILESTONE_RE = /^#{1,3}\s+(?:\[[^\]]+\]\s+|Roadmap\s+|[✅🚧]\s*)?v(\d+)\.\d+(?:\s|:|\s*—)/iu; + const MILESTONE_RE = /^#{1,3}\s+(?:\[[^\]]{1,200}\]\s+|Roadmap\s+|[✅🚧]\s*)?v(\d+)\.\d+(?:\s|:|\s*—)/iu; // Migrated phase heading: ### Phase M-NN: Name (M-NN or unpadded M-N form) // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const PHASE_RE = /^#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+(\d+)-(\d+)(?:-\d+)*(?:\s*\([^)\n]{0,200}\))?\s*:/i; + const PHASE_RE = /^#{2,4}\s*(?:\[[^\]]{1,200}\]\s*)?Phase\s+(\d+)-(\d+)(?:-\d+)*(?:\s*\([^)\n]{0,200}\))?\s*:/i; // Unprefixed legacy phase heading: ### Phase N: Name (no hyphen sub-index) // phase-id-owner: UNPREFIXED_PHASE_RE token uses the [A-Za-z] case-variant (identical to the canonical [A-Z] token under /i); kept literal, not source-byte-equal to PHASE_NUMBER_TOKEN_SOURCE. - const UNPREFIXED_PHASE_RE = /^#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+(\d+[A-Za-z]?(?:\.\d+)*)(?:\s*\([^)\n]{0,200}\))?\s*:/i; + const UNPREFIXED_PHASE_RE = /^#{2,4}\s*(?:\[[^\]]{1,200}\]\s*)?Phase\s+(\d+[A-Za-z]?(?:\.\d+)*)(?:\s*\([^)\n]{0,200}\))?\s*:/i; let currentMilestoneMajor: number | null = null; const lines = content.split('\n'); diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index 4093f7ede..4cadfdd75 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -215,7 +215,7 @@ interface RoadmapPhaseResult { function findRoadmapPhaseInContent(content: string, phaseNum: unknown, phaseSource?: string): RoadmapPhaseResult | null { // #1729: OPTIONAL_PHASE_TAG_SOURCE after the number tolerates a pre-colon ( ) tag. const headingPattern = new RegExp( - `^(?:\\[[^\\]]+\\]\\s*)?Phase\\s+${phaseSource ?? phaseMarkdownRegexSource(phaseNum)}${OPTIONAL_PHASE_TAG_SOURCE}:\\s*(.+)$`, + `^(?:\\[[^\\]]{1,200}\\]\\s*)?Phase\\s+${phaseSource ?? phaseMarkdownRegexSource(phaseNum)}${OPTIONAL_PHASE_TAG_SOURCE}:\\s*(.+)$`, 'i' ); const headings = tokenizeHeadings(content); @@ -370,7 +370,7 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p let roadmap = extractCurrentMilestone(roadmapContent, cwd); const hasVersionedMilestonesGlobal = /^#{1,3}\s+.*v\d+\.\d+/mi.test(roadmapContent); - const hasPhaseHeadings = /#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+[\w]/i.test(roadmapContent); + const hasPhaseHeadings = /#{2,4}\s*(?:\[[^\]]{1,200}\]\s*)?Phase\s+[\w]/i.test(roadmapContent); if (!hasVersionedMilestonesGlobal && hasPhaseHeadings && phaseIdConvention === 'milestone-prefixed') { console.warn( '[gsd] Deprecated: free-form ROADMAP.md detected (no versioned milestone headings). ' + @@ -428,7 +428,7 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p // Use tokenizeHeadings (fence-aware) instead of stripFencedLines + regex. // T4 seam migration: phase headings inside fences are excluded automatically. // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phaseHeadingPattern = /^(?:\[[^\]]+\]\s*)?Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]{0,200}\))?\s*:/i; + const phaseHeadingPattern = /^(?:\[[^\]]{1,200}\]\s*)?Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]{0,200}\))?\s*:/i; for (const h of tokenizeHeadings(roadmap)) { if (h.level < 2 || h.level > 4) continue; const pm = phaseHeadingPattern.exec(h.text); diff --git a/src/roadmap-upgrade.cts b/src/roadmap-upgrade.cts index 2c47995b9..72985fbb1 100644 --- a/src/roadmap-upgrade.cts +++ b/src/roadmap-upgrade.cts @@ -23,16 +23,16 @@ const { stripProjectCodePrefix, PHASE_NUMBER_TOKEN_SOURCE } = phaseIdMod; // Matches legacy phase headings: ### Phase N: Name (also decimal: Phase 2.1:) // Captures: (hashes)(spaces)(phase-number)(rest-of-line) const LEGACY_PHASE_HEADING_RE = new RegExp( - `^(#{2,4})\\s*(?:\\[[^\\]]+\\]\\s*)?Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})\\s*:(.*)`, + `^(#{2,4})\\s*(?:\\[[^\\]]{1,200}\\]\\s*)?Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})\\s*:(.*)`, 'i' ); // Matches already-migrated phase headings: ### Phase M-NN: Name -const MIGRATED_PHASE_HEADING_RE = /^#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+\d+-\d{2}\s*:/i; +const MIGRATED_PHASE_HEADING_RE = /^#{2,4}\s*(?:\[[^\]]{1,200}\]\s*)?Phase\s+\d+-\d{2}\s*:/i; // Matches milestone section headings: ## v1.0, ## Roadmap v2.0, ## ✅ v1.0, ## [GSD] v1.0, etc. // The optional bracket-token prefix (e.g., [GSD]) must be tested before the emoji group. -const MILESTONE_HEADING_RE = /^##\s+(?:\[[^\]]+\]\s+|Roadmap\s+|[✅🚧]\s*)?v(\d+)\.(\d+)(?:\s|:)/iu; +const MILESTONE_HEADING_RE = /^##\s+(?:\[[^\]]{1,200}\]\s+|Roadmap\s+|[✅🚧]\s*)?v(\d+)\.(\d+)(?:\s|:)/iu; // ─── Types ──────────────────────────────────────────────────────────────────── @@ -344,7 +344,7 @@ function computeMigrationPlan(cwd: string, options: Record = {} // Rewrite heading line: "### Phase N: Name" → "### Phase M-NN: Name" const oldLine = lines[entry.lineIndex]; const newLine = oldLine.replace( - new RegExp(`^(#{2,4}\\s*(?:\\[[^\\]]+\\]\\s*)?Phase\\s+)${PHASE_NUMBER_TOKEN_SOURCE}(\\s*:)`, 'i'), + new RegExp(`^(#{2,4}\\s*(?:\\[[^\\]]{1,200}\\]\\s*)?Phase\\s+)${PHASE_NUMBER_TOKEN_SOURCE}(\\s*:)`, 'i'), `$1${mapping.newId}$2` ); if (newLine !== oldLine) { diff --git a/src/roadmap.cts b/src/roadmap.cts index 4eb408c49..7f6febff9 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -126,7 +126,7 @@ function countPhasePlansAndSummaries(phaseDir: string): PhasePlansAndSummaries { function searchPhaseInContent(content: string, escapedPhase: string, phaseNum: string): PhaseSearchResult | null { // #1729: OPTIONAL_PHASE_TAG_SOURCE after the number tolerates a pre-colon ( ) tag. const headingPattern = new RegExp( - `^(?:\\[[^\\]]+\\]\\s*)?Phase\\s+${escapedPhase}${OPTIONAL_PHASE_TAG_SOURCE}:\\s*(.+)$`, + `^(?:\\[[^\\]]{1,200}\\]\\s*)?Phase\\s+${escapedPhase}${OPTIONAL_PHASE_TAG_SOURCE}:\\s*(.+)$`, 'i' ); const headings = tokenizeHeadings(content); @@ -299,7 +299,7 @@ function cmdRoadmapAnalyze(cwd: string, raw: boolean): void { // Extract all phase headings: ## Phase N: Name or ### Phase N: Name // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). // phase-id-owner: uses the [.-] (dot-or-dash) separator variant, not the canonical dot-only token; a swap to PHASE_NUMBER_TOKEN_SOURCE would drop hyphenated phase-id matches. - const phasePattern = /#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+(\d+[A-Z]?(?:[.-]\d+)*)(?:\s*\([^)\n]{0,200}\))?\s*:\s*([^\n]+)/gi; + const phasePattern = /#{2,4}\s*(?:\[[^\]]{1,200}\]\s*)?Phase\s+(\d+[A-Z]?(?:[.-]\d+)*)(?:\s*\([^)\n]{0,200}\))?\s*:\s*([^\n]+)/gi; const phases: Array<{ number: string; name: string; @@ -344,7 +344,7 @@ function cmdRoadmapAnalyze(cwd: string, raw: boolean): void { const restOfContent = content.slice(sectionStart); // #3691: `\d` → `\d[\d.]*` so decimal phase headings (e.g. `### Phase 02.3:`) are // recognised as section boundaries. - const nextHeader = restOfContent.match(/\n#{2,4}\s+(?:\[[^\]]+\]\s*)?Phase\s+\d[\d.-]*/i); + const nextHeader = restOfContent.match(/\n#{2,4}\s+(?:\[[^\]]{1,200}\]\s*)?Phase\s+\d[\d.-]*/i); const sectionEnd = nextHeader ? sectionStart + nextHeader.index! : content.length; const section = content.slice(sectionStart, sectionEnd); diff --git a/src/state-transition.cts b/src/state-transition.cts index d796aac10..ca93e82aa 100644 --- a/src/state-transition.cts +++ b/src/state-transition.cts @@ -1818,7 +1818,7 @@ function reconcileByPhaseTable( * source to substitute. This is honest — better than silently leaving `[X]` * which looks like a value. */ -const TEMPLATE_PLACEHOLDER_VALUE = /^\s*\[[^\]]+\]\s*$|^\s*-\s*$/; +const TEMPLATE_PLACEHOLDER_VALUE = /^\s*\[[^\]]{1,200}\]\s*$|^\s*-\s*$/; function stripTemplatePlaceholders( content: string, diff --git a/src/validate.cts b/src/validate.cts index e0eea4835..edffe68bf 100644 --- a/src/validate.cts +++ b/src/validate.cts @@ -114,7 +114,7 @@ export function buildRoadmapPhaseVariants(roadmapContent: string): RoadmapPhaseV // Matches both legacy numeric (Phase 1:), decimal (Phase 2.1:), milestone-prefixed (Phase 2-01:), // and bracket-prefixed (### [GSD] Phase 2-01:) headings. // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phasePattern = /#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]{0,200}\))?\s*:/gi; + const phasePattern = /#{2,4}\s*(?:\[[^\]]{1,200}\]\s*)?Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]{0,200}\))?\s*:/gi; let m: RegExpExecArray | null; while ((m = phasePattern.exec(roadmapContent)) !== null) { roadmapPhases.add(m[1]); diff --git a/src/verify.cts b/src/verify.cts index 58e2bb18e..27a71d92e 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -1073,7 +1073,7 @@ function checkMilestonePrefixMismatches( ): MilestoneMismatch[] { const mismatches: MilestoneMismatch[] = []; const sections: { version: string; start: number; end: number }[] = []; - const sectionRx = /^#{1,3}\s+(?:\[[^\]]+\]\s*)?.*v(\d+\.\d+)/gim; + const sectionRx = /^#{1,3}\s+(?:\[[^\]]{1,200}\]\s*)?.*v(\d+\.\d+)/gim; let m: RegExpExecArray | null; while ((m = sectionRx.exec(roadmapContent)) !== null) { if (sections.length > 0) sections[sections.length - 1].end = m.index; @@ -1082,7 +1082,7 @@ function checkMilestonePrefixMismatches( for (const section of sections) { const content = roadmapContent.slice(section.start, section.end); // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phaseRx = /#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]{0,200}\))?\s*:/gi; + const phaseRx = /#{2,4}\s*(?:\[[^\]]{1,200}\]\s*)?Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]{0,200}\))?\s*:/gi; let pm: RegExpExecArray | null; while ((pm = phaseRx.exec(content)) !== null) { const phaseId = pm[1]; diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index d945c2d64..67908c960 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -355,8 +355,10 @@ describe('#1729 regression: parenthetical tag before the colon in a phase header // still matches (real tags are a handful of chars), 201 does not. const phaseId = require('../gsd-core/bin/lib/phase-id.cjs'); const re = new RegExp(`Phase\\s+0*26${phaseId.OPTIONAL_PHASE_TAG_SOURCE}\\s*:`); - assert.ok(re.test(`### Phase 26 (${'x'.repeat(200)}): T`), 'a 200-char tag body is within the bound'); - assert.ok(!re.test(`### Phase 26 (${'x'.repeat(201)}): T`), 'a 201-char tag body exceeds the bound'); + // Boundary coverage (CLAUDE.md): limit-1, limit, limit+1. + assert.ok(re.test(`### Phase 26 (${'x'.repeat(199)}): T`), 'a 199-char tag body (limit-1) is within the bound'); + assert.ok(re.test(`### Phase 26 (${'x'.repeat(200)}): T`), 'a 200-char tag body (limit) is within the bound'); + assert.ok(!re.test(`### Phase 26 (${'x'.repeat(201)}): T`), 'a 201-char tag body (limit+1) exceeds the bound'); // Linearity guard: the adversarial input that was ~18.8s unbounded resolves // near-instantly now. Assert bounded work, not wall-clock (no clock seam): // the bounded source contains an explicit upper repetition limit. @@ -425,11 +427,12 @@ describe('#1729 regression: parenthetical tag before the colon in a phase header test('the literal enumeration mirror stays equivalent to the exported seam (drift guard)', () => { // Resolver sites compose OPTIONAL_PHASE_TAG_SOURCE; literal enumeration sites - // inline `(?:\s*\([^)\n]*\))?`. If one is edited without the other the two - // header families silently diverge. Assert behavioral equivalence over a - // representative header corpus so the split cannot drift undetected. + // inline `(?:\s*\([^)\n]{0,200}\))?`. If one is edited without the other the + // two header families silently diverge (the body is bounded to {0,200} in + // both since #2128 — a ReDoS fix that MUST stay in lockstep). Assert + // behavioral equivalence over a representative header corpus. const phaseId = require('../gsd-core/bin/lib/phase-id.cjs'); - const LITERAL_MIRROR = '(?:\\s*\\([^)\\n]*\\))?'; + const LITERAL_MIRROR = '(?:\\s*\\([^)\\n]{0,200}\\))?'; const seam = new RegExp(`^Phase\\s+26${phaseId.OPTIONAL_PHASE_TAG_SOURCE}\\s*:`); const mirror = new RegExp(`^Phase\\s+26${LITERAL_MIRROR}\\s*:`); for (const sample of [