golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(#2128): bound the sibling bracket-prefix clause — complete the ReDoS fix

Browse Source 
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 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-07-10 09:44:49 -04:00
parent c1cd43a39f
commit b321bc04f4
11 changed files with 29 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
gsd-core/bin/lib/state-transition.cjs
View File

@@ -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

2
src/commands.cts
View File

@@ -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]);

2
src/phase.cts
View File

@@ -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<number>();
let m: RegExpExecArray | null;

6
src/roadmap-command-router.cts
View File

@@ -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');

6
src/roadmap-parser.cts
View File

@@ -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);

8
src/roadmap-upgrade.cts
View File

@@ -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<string, unknown> = {}
// 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) {

6
src/roadmap.cts
View File

@@ -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);

2
src/state-transition.cts
View File

@@ -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,

2
src/validate.cts
View File

@@ -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]);

4
src/verify.cts
View File

@@ -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];

15
tests/phase.test.cjs
View File

@@ -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 [