From 09be501eb7d6d9b785ab240281d4c3d189dd3e96 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 10 Jul 2026 08:49:36 -0400 Subject: [PATCH 01/10] =?UTF-8?q?feat(#2128):=20phase-id=20anti-divergence?= =?UTF-8?q?=20guard=20=E2=80=94=20canonical=20token=20source=20+=20drift?= =?UTF-8?q?=20scanner=20+=20guards?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 4 of epic #2121 (ADR-2121 Decision 7), closing the recurrence loop that produced #2111 / #2114 / #2104: no module outside src/phase-id.cts may re-implement phase-ID parsing without failing CI. - phase-id.cts: add PHASE_NUMBER_TOKEN_SOURCE — the canonical phase-number-token grammar (\d+[A-Z]?(?:\.\d+)*) for enumeration/scan call sites, the ANY-phase counterpart to phaseMarkdownRegexSource(n)'s known-number lookup. Extend-only (never touches normalizePhaseName; blast radius 79 fns / CRITICAL). - scripts/lint-phase-id-drift.cjs: pure findPhaseIdRegexDrift(text) + scanRepo(root), wired to `npm run check:phase-id-drift`. Flags a literal re-derivation of the canonical token (both /\d/ and new-RegExp `\\d` escaping, plus the [A-Za-z] and [.-] near-variants) anywhere in src/** outside phase-id.cts, unless sanctioned with `// phase-id-owner: `. Narrow by design: bare \d+, digits-only captures, \w ids, status-message text and pipe-tables are not flagged. - tests/phase-id-drift-guard.test.cjs: fail-first drift cases (AC1) + live scanRepo(ROOT) zero-drift (AC3) + identity guard — phase-id.cjs exports the complete locked surface and no consumer re-exports a divergent copy (AC2). Co-Authored-By: Claude Opus 4.8 --- package.json | 1 + scripts/lint-phase-id-drift.cjs | 133 ++++++++++++++++++++++++++ src/phase-id.cts | 13 +++ tests/phase-id-drift-guard.test.cjs | 143 ++++++++++++++++++++++++++++ 4 files changed, 290 insertions(+) create mode 100644 scripts/lint-phase-id-drift.cjs create mode 100644 tests/phase-id-drift-guard.test.cjs diff --git a/package.json b/package.json index 844d518ca..b05b647aa 100644 --- a/package.json +++ b/package.json @@ -78,6 +78,7 @@ "check:env": "node scripts/check-env.cjs", "check:alias-drift": "node scripts/check-alias-drift.cjs", "check:identity-drift": "node scripts/lint-package-identity-drift.cjs", + "check:phase-id-drift": "node scripts/lint-phase-id-drift.cjs", "check:integrity": "node scripts/check-npm-integrity.cjs", "build": "npm run generate:identity && npm run build:lib && npm run gen:plugin-skills && npm run gen:loop-host-contract && npm run gen:capability-registry && npm run build:hooks", "build:hooks": "node scripts/build-hooks.js", diff --git a/scripts/lint-phase-id-drift.cjs b/scripts/lint-phase-id-drift.cjs new file mode 100644 index 000000000..33cecc3cd --- /dev/null +++ b/scripts/lint-phase-id-drift.cjs @@ -0,0 +1,133 @@ +#!/usr/bin/env node +'use strict'; + +/** + * Anti-divergence drift guard for the phase-identifier parsing seam + * (epic #2121, Phase 4 / issue #2128, locked by ADR-2121 Decision 7). + * + * `src/phase-id.cts` is the SINGLE canonical owner of phase-ID parsing. Its + * `PHASE_NUMBER_TOKEN_SOURCE` (and `phaseMarkdownRegexSource` for a known number) + * is the one place the phase-number-token grammar `\d+[A-Z]?(?:\.\d+)*` is + * defined. Every other module that scans/enumerates phase headings must build + * its regex from that source rather than re-deriving the grammar as a literal — + * otherwise the trio drifts again (the #2111 / #2114 / #2104 recurrence loop this + * epic closes). + * + * This lint makes the invariant machine-enforced: it FAILS the moment a literal + * re-derivation of the canonical token grammar is introduced anywhere in + * `src/**` outside `phase-id.cts`, unless the site is deliberately sanctioned + * with a `// phase-id-owner: ` comment (on the same line or the line + * directly above). Sites that build their regex from `PHASE_NUMBER_TOKEN_SOURCE` + * carry no literal grammar and pass automatically. + * + * Detection is intentionally NARROW: only the contiguous canonical token + * (`\d+[A-Z]?(?:\.\d+)*`, its `[A-Za-z]` and `[.-]` near-variants, in both + * regex-literal `\d` and `new RegExp` template `\\d` escaping) is drift. Bare + * `\d+` probes, `[\w][\w.-]*` ids, digits-only captures, status-message text + * (`Phase\s+\d`), and pipe-table structures are NOT phase-token re-derivations + * and are not flagged. + */ + +const fs = require('node:fs'); +const path = require('node:path'); + +// The canonical phase-number token as it appears in SOURCE TEXT: +// \d+[A-Z]?(?:\.\d+)* in a regex literal -> one backslash before d/. +// \\d+[A-Z]?(?:\\.\\d+)* in a template string -> two backslashes +// Also tolerate the [A-Za-z] letter-class and the [.-] (dot-or-dash) separator +// near-variants that a few enumeration call sites use. +const TOKEN_DRIFT_RE = /\\{1,2}d\+\[A-Z(?:a-z)?\]\??\(\?:(?:\\{1,2}\.|\[\.-\])\\{1,2}d\+\)\*/; + +const OWNER_MARK = 'phase-id-owner:'; +const CANON_REF = 'PHASE_NUMBER_TOKEN_SOURCE'; + +/** + * Pure: find every literal re-derivation of the canonical phase-number token in + * `text` that is NOT sanctioned. A site is sanctioned when its line — or the + * line directly above it — contains `// phase-id-owner:`, or when the line + * references `PHASE_NUMBER_TOKEN_SOURCE` (i.e. it is built from the canonical + * source, not a literal). Returns [{ line, found }]. + */ +function findPhaseIdRegexDrift(text) { + const out = []; + const lines = text.split('\n'); + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + const m = TOKEN_DRIFT_RE.exec(line); + if (!m) continue; + if (line.includes(OWNER_MARK)) continue; + if (i > 0 && lines[i - 1].includes(OWNER_MARK)) continue; + if (line.includes(CANON_REF)) continue; + out.push({ line: i + 1, found: m[0] }); + } + return out; +} + +// Authored TypeScript source only (the generated bin/lib/*.cjs mirror it). +const SCAN_DIRS = ['src']; +const SCAN_EXT = new Set(['.cts', '.ts', '.mts']); +// The canonical owner defines the grammar; it is exempt by construction. +const EXEMPT = new Set([path.join('src', 'phase-id.cts')]); + +function walk(dir, acc) { + let entries; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return acc; + } + for (const entry of entries) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name === 'node_modules' || entry.name === 'dist' || entry.name === '.git') continue; + walk(full, acc); + } else if (entry.isFile() && SCAN_EXT.has(path.extname(entry.name))) { + acc.push(full); + } + } + return acc; +} + +/** + * Scan the authored source tree and return every unsanctioned phase-token + * re-derivation, each annotated with the repo-relative file path. + */ +function scanRepo(root) { + const violations = []; + for (const dir of SCAN_DIRS) { + for (const file of walk(path.join(root, dir), [])) { + const rel = path.relative(root, file); + if (EXEMPT.has(rel)) continue; + let text; + try { + text = fs.readFileSync(file, 'utf8'); + } catch { + continue; + } + for (const d of findPhaseIdRegexDrift(text)) { + violations.push({ file: rel, ...d }); + } + } + } + return violations; +} + +function main() { + const root = path.join(__dirname, '..'); + const violations = scanRepo(root); + if (violations.length === 0) { + process.stdout.write('ok phase-id-drift: no unsanctioned phase-token re-derivations outside phase-id.cts\n'); + return; + } + process.stderr.write('phase-id-drift: literal re-derivation(s) of the canonical phase-number token found.\n'); + process.stderr.write('Build the regex from phase-id.cjs `PHASE_NUMBER_TOKEN_SOURCE` (or phaseMarkdownRegexSource for a\n'); + process.stderr.write('known number), or sanction the site with a `// phase-id-owner: ` comment:\n'); + for (const d of violations) { + process.stderr.write(` ${d.file}:${d.line} ${d.found}\n`); + } + process.exitCode = 1; +} + +if (require.main === module) main(); + +module.exports = { findPhaseIdRegexDrift, scanRepo, TOKEN_DRIFT_RE }; diff --git a/src/phase-id.cts b/src/phase-id.cts index e08a4af43..7ce8f90f3 100644 --- a/src/phase-id.cts +++ b/src/phase-id.cts @@ -41,6 +41,18 @@ const OPTIONAL_PROJECT_CODE_PREFIX_SOURCE = '(?:[A-Z][A-Z0-9_]*-)?'; // source. Both forms must change together; see the #1729 regression test. const OPTIONAL_PHASE_TAG_SOURCE = '(?:\\s*\\([^)\\n]*\\))?'; +// #2128: the canonical phase-NUMBER-TOKEN grammar — a phase number with an +// optional single-letter variant suffix and optional dotted sub-phases +// (1, 01, 12A, 12.1, 3.2.1). This is the ENUMERATION/scan counterpart to +// phaseMarkdownRegexSource: use phaseMarkdownRegexSource(n) to build a source +// for ONE KNOWN number; reference this constant when a call site must match ANY +// phase and capture its token. Enumeration/parse sites inline this into a +// `new RegExp(...)` instead of re-deriving the grammar as a literal, so every +// phase-token producer shares one owner. The anti-divergence guard +// (scripts/lint-phase-id-drift.cjs) fails CI if a literal re-derivation is +// introduced outside this module without a `// phase-id-owner:` justification. +const PHASE_NUMBER_TOKEN_SOURCE = '\\d+[A-Z]?(?:\\.\\d+)*'; + function stripProjectCodePrefix(value: unknown, caseInsensitive = true): string { const input = String(value); const re = caseInsensitive ? PROJECT_CODE_PREFIX_STRIP_RE_I : PROJECT_CODE_PREFIX_STRIP_RE; @@ -350,6 +362,7 @@ export = { escapeRegex, OPTIONAL_PROJECT_CODE_PREFIX_SOURCE, OPTIONAL_PHASE_TAG_SOURCE, + PHASE_NUMBER_TOKEN_SOURCE, stripProjectCodePrefix, normalizePhaseName, getMilestoneFromPhaseId, diff --git a/tests/phase-id-drift-guard.test.cjs b/tests/phase-id-drift-guard.test.cjs new file mode 100644 index 000000000..3e20b291a --- /dev/null +++ b/tests/phase-id-drift-guard.test.cjs @@ -0,0 +1,143 @@ +'use strict'; +process.env.GSD_TEST_MODE = '1'; + +/** + * Anti-divergence guard for the phase-identifier parsing seam + * (epic #2121 Phase 4 / issue #2128, ADR-2121 Decision 7). + * + * `src/phase-id.cts` is the single canonical owner of phase-ID parsing. Two guards + * keep it that way: + * 1. DRIFT SCANNER (scripts/lint-phase-id-drift.cjs) — fails CI if any module + * outside phase-id.cts re-derives the canonical phase-number token as a + * literal without a `// phase-id-owner:` sanction. + * 2. IDENTITY guard — phase-id.cjs exports the complete locked surface, and no + * consumer re-exports a DIVERGENT copy of a canonical function (re-export, + * never re-implement). + * + * Behavioral throughout: assertions drive `findPhaseIdRegexDrift` / `scanRepo` + * and compare object identity — no `readFileSync().includes()` in a test body. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const ROOT = path.join(__dirname, '..'); +const { findPhaseIdRegexDrift, scanRepo } = require( + path.join(ROOT, 'scripts', 'lint-phase-id-drift.cjs'), +); +const phaseId = require(path.join(ROOT, 'gsd-core', 'bin', 'lib', 'phase-id.cjs')); + +// The locked canonical surface (ADR-2121 Decision 1/2; PHASE_NUMBER_TOKEN_SOURCE +// added in Phase 4). Every name is exported by phase-id.cjs; the identity guard +// forbids any other module from re-exporting a divergent copy of one. +const CANONICAL = [ + 'escapeRegex', 'OPTIONAL_PROJECT_CODE_PREFIX_SOURCE', 'OPTIONAL_PHASE_TAG_SOURCE', + 'PHASE_NUMBER_TOKEN_SOURCE', 'stripProjectCodePrefix', 'normalizePhaseName', + 'getMilestoneFromPhaseId', 'getPhaseDirFromPhaseId', 'phaseMarkdownRegexSource', + 'phaseMarkdownRegexSourceExact', 'comparePhaseNum', 'extractPhaseToken', + 'phaseTokenMatches', 'parsePhaseFromProse', 'stripConfiguredProjectCodePrefix', + 'isForeignPrefixedPhaseQuery', 'roadmapPhaseLookupSources', +]; + +describe('#2128 phase-id drift scanner: findPhaseIdRegexDrift (pure)', () => { + test('a regex built from PHASE_NUMBER_TOKEN_SOURCE is NOT drift', () => { + assert.deepEqual( + findPhaseIdRegexDrift('const re = new RegExp(`Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})`);'), + [], + ); + }); + + test('a literal re-derivation of the canonical token IS flagged (fail-first)', () => { + const v = findPhaseIdRegexDrift('const re = /Phase\\s+(\\d+[A-Z]?(?:\\.\\d+)*)/;'); + assert.equal(v.length, 1); + assert.equal(v[0].found, '\\d+[A-Z]?(?:\\.\\d+)*'); + }); + + test('a re-derivation inside a new RegExp template (\\\\d escaping) IS flagged', () => { + const v = findPhaseIdRegexDrift('new RegExp(`Phase\\\\s+(\\\\d+[A-Z]?(?:\\\\.\\\\d+)*)`)'); + assert.equal(v.length, 1); + }); + + test('the [A-Za-z] and [.-] near-variants ARE flagged', () => { + assert.equal(findPhaseIdRegexDrift('/(\\d+[A-Za-z]?(?:\\.\\d+)*)/').length, 1); + assert.equal(findPhaseIdRegexDrift('/(\\d+[A-Z]?(?:[.-]\\d+)*)/').length, 1); + }); + + test('a same-line // phase-id-owner: sanction suppresses the flag', () => { + assert.deepEqual( + findPhaseIdRegexDrift('const re = /(\\d+[A-Z]?(?:\\.\\d+)*)/; // phase-id-owner: sanctioned exception'), + [], + ); + }); + + test('a preceding-line // phase-id-owner: sanction suppresses the flag', () => { + assert.deepEqual( + findPhaseIdRegexDrift('// phase-id-owner: sanctioned exception\nconst re = /(\\d+[A-Z]?(?:\\.\\d+)*)/;'), + [], + ); + }); + + test('non-token phase regexes are NOT flagged (no false positives)', () => { + assert.deepEqual(findPhaseIdRegexDrift('/^Executing Phase\\s+\\d+/'), [], 'status-message bare \\d+'); + assert.deepEqual(findPhaseIdRegexDrift('/#{2,4}\\s*Phase\\s+(\\d+)[A-Z]?(?:\\.\\d+)*/'), [], 'digits-only capture is non-contiguous'); + assert.deepEqual(findPhaseIdRegexDrift('/Phase\\s+([\\w][\\w.-]*)/'), [], '\\w id grammar is not the canonical token'); + assert.deepEqual(findPhaseIdRegexDrift('/\\|\\s*Phase\\s*\\|\\s*Plans\\s*\\|/'), [], 'pipe-table structure'); + }); + + test('reports 1-based line numbers', () => { + const v = findPhaseIdRegexDrift('line1\nconst re = /(\\d+[A-Z]?(?:\\.\\d+)*)/;\nline3'); + assert.equal(v[0].line, 2); + }); +}); + +describe('#2128 phase-id drift scanner: the live repo is clean', () => { + test('scanRepo finds zero unsanctioned phase-token re-derivations', () => { + const violations = scanRepo(ROOT); + assert.deepEqual( + violations, + [], + 'unsanctioned phase-token re-derivation(s) — build from PHASE_NUMBER_TOKEN_SOURCE or add // phase-id-owner:\n' + + violations.map((d) => ` ${d.file}:${d.line} ${d.found}`).join('\n'), + ); + }); +}); + +describe('#2128 phase-id single-owner identity guard', () => { + test('phase-id.cjs exports the complete locked canonical surface', () => { + for (const name of CANONICAL) { + assert.ok(name in phaseId, `phase-id.cjs must export the canonical member '${name}'`); + } + }); + + test('no consumer module re-exports a DIVERGENT copy of a canonical phase-id function', () => { + // Forward guard: if any built lib module re-exports a name that phase-id.cjs + // owns, it MUST be the identical reference — a re-export, never a local + // re-implementation. All consumers pass today (none re-export); the guard + // fails the moment a divergent copy ships. + const libDir = path.join(ROOT, 'gsd-core', 'bin', 'lib'); + const consumers = fs.readdirSync(libDir).filter((f) => f.endsWith('.cjs') && f !== 'phase-id.cjs'); + let checked = 0; + for (const f of consumers) { + let mod; + try { + mod = require(path.join(libDir, f)); + } catch { + continue; // a module that cannot be required in isolation can't re-export anything + } + if (!mod || typeof mod !== 'object') continue; + checked++; + for (const name of CANONICAL) { + if (Object.prototype.hasOwnProperty.call(mod, name)) { + assert.strictEqual( + mod[name], + phaseId[name], + `${f} re-exports '${name}' but it is NOT the phase-id.cjs reference — re-export the canonical, do not re-implement`, + ); + } + } + } + assert.ok(checked > 0, 'expected to inspect at least one consumer module'); + }); +}); From dfad3a7510a2b73cecf08c6b83f5aa51a74b7453 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 10 Jul 2026 08:49:36 -0400 Subject: [PATCH 02/10] refactor(#2128): single-source 23 phase-token re-derivations; sanction 14 context-specific sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Route 23 literal re-derivations of the canonical phase-number token through phase-id.cjs `PHASE_NUMBER_TOKEN_SOURCE` (via new RegExp). Each conversion was proven BYTE-IDENTICAL (old.source === new.source && old.flags === new.flags), so the runtime regexes are unchanged — zero behavior change by construction. The remaining 14 phase-token sites are genuine but context-specific and stay literal with a `// phase-id-owner: ` sanction: dir-name parses whose dash-continuation semantics differ from extractPhaseToken, and the [A-Za-z] case-variant / [.-] dot-or-dash separator forms that are not source-byte-equal to the canonical token. Scanner (`npm run check:phase-id-drift`) is now green. Co-Authored-By: Claude Opus 4.8 --- src/audit.cts | 3 +++ src/init.cts | 20 ++++++++++++-------- src/milestone.cts | 4 ++-- src/phase.cts | 19 +++++++++++++------ src/roadmap-command-router.cts | 1 + src/roadmap-parser.cts | 1 + src/roadmap-upgrade.cts | 18 ++++++++++++------ src/roadmap.cts | 2 ++ src/state.cts | 5 +++-- src/uat.cts | 1 + src/validate.cts | 4 ++-- src/verify.cts | 12 ++++++++---- 12 files changed, 60 insertions(+), 30 deletions(-) diff --git a/src/audit.cts b/src/audit.cts index 41bf72a93..c7bf2f983 100644 --- a/src/audit.cts +++ b/src/audit.cts @@ -482,6 +482,7 @@ function scanUatGaps(planDir: string): UatGapItem[] { for (const dir of dirs) { const phaseDir = path.join(phasesDir, dir); + // phase-id-owner: cosmetic phase label derived from a dir name for JSON output; the single-segment capture is not equivalent to extractPhaseToken dash-continuation semantics, so not a behavior-preserving drop-in. const phaseMatch = dir.match(/^(\d+[A-Z]?(?:\.\d+)*)/i); const phaseNum = phaseMatch ? phaseMatch[1] : dir; @@ -552,6 +553,7 @@ function scanVerificationGaps(planDir: string): VerificationGapItem[] { for (const dir of dirs) { const phaseDir = path.join(phasesDir, dir); + // phase-id-owner: cosmetic phase label derived from a dir name for JSON output; the single-segment capture is not equivalent to extractPhaseToken dash-continuation semantics, so not a behavior-preserving drop-in. const phaseMatch = dir.match(/^(\d+[A-Z]?(?:\.\d+)*)/i); const phaseNum = phaseMatch ? phaseMatch[1] : dir; @@ -614,6 +616,7 @@ function scanContextQuestions(planDir: string): ContextQuestionItem[] { for (const dir of dirs) { const phaseDir = path.join(phasesDir, dir); + // phase-id-owner: cosmetic phase label derived from a dir name for JSON output; the single-segment capture is not equivalent to extractPhaseToken dash-continuation semantics, so not a behavior-preserving drop-in. const phaseMatch = dir.match(/^(\d+[A-Z]?(?:\.\d+)*)/i); const phaseNum = phaseMatch ? phaseMatch[1] : dir; diff --git a/src/init.cts b/src/init.cts index ee6e7ffa2..ab6bcafb1 100644 --- a/src/init.cts +++ b/src/init.cts @@ -73,7 +73,7 @@ const { extractCurrentMilestone, } = roadmapParser; const { pathExistsInternal, generateSlugInternal, toPosixPath } = coreUtils; -const { normalizePhaseName, phaseTokenMatches, stripProjectCodePrefix } = phaseId; +const { normalizePhaseName, phaseTokenMatches, stripProjectCodePrefix, PHASE_NUMBER_TOKEN_SOURCE } = phaseId; const { pruneOrphanedWorktrees } = worktreeSafety; const { @@ -1162,7 +1162,7 @@ function cmdInitMilestoneOp(cwd: string, raw: boolean): void { const roadmapRaw = fs.readFileSync(roadmapPath, 'utf-8'); const currentSection = extractCurrentMilestone(roadmapRaw, cwd); // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phasePattern = /#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)(?:\s*\([^)\n]*\))?\s*:/gi; + const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]*\\))?\\s*:`, 'gi'); let m: RegExpExecArray | null; while ((m = phasePattern.exec(currentSection)) !== null) { if (/^999(?:\.|$)/.test(m[1])) continue; @@ -1181,6 +1181,7 @@ function cmdInitMilestoneOp(cwd: string, raw: boolean): void { const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); for (const e of entries) { if (!e.isDirectory()) continue; + // phase-id-owner: dir-name phase-token parse; extractPhaseToken dash-separated sub-phase semantics differ, so a token-source swap would risk remapping phase<->directory matches. Kept local. const m = stripProjectCodePrefix(e.name).match(/^(\d+[A-Z]?(?:\.\d+)*)/); if (!m) continue; diskPhaseDirs.set(canonicalizePhase(m[1]), e.name); @@ -1319,14 +1320,14 @@ function cmdInitManager(cwd: string, raw: boolean): void { })(); const _checkboxStates = new Map(); - const _cbPattern = /-\s*\[(x| )\]\s*.*Phase\s+(\d+[A-Z]?(?:\.\d+)*)[:\s]/gi; + const _cbPattern = new RegExp(`-\\s*\\[(x| )\\]\\s*.*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})[:\\s]`, 'gi'); let _cbMatch: RegExpExecArray | null; while ((_cbMatch = _cbPattern.exec(content)) !== null) { _checkboxStates.set(_cbMatch[2], _cbMatch[1].toLowerCase() === 'x'); } // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phasePattern = /#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)(?:\s*\([^)\n]*\))?\s*:\s*([^\n]+)/gi; + const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]*\\))?\\s*:\\s*([^\\n]+)`, 'gi'); const phases: Record[] = []; let match: RegExpExecArray | null; @@ -1465,7 +1466,7 @@ function cmdInitManager(cwd: string, raw: boolean): void { ); const phaseMap = new Map(phases.map((p) => [normalizePhaseNumber(p['number'] as string), p])); - const _allCompletedPattern = /-\s*\[x\]\s*.*Phase\s+(\d+[A-Z]?(?:\.\d+)*)[:\s]/gi; + const _allCompletedPattern = new RegExp(`-\\s*\\[x\\]\\s*.*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})[:\\s]`, 'gi'); let _allMatch: RegExpExecArray | null; while ((_allMatch = _allCompletedPattern.exec(rawContent)) !== null) { const phaseNum = normalizePhaseNumber(_allMatch[1]); @@ -1499,7 +1500,7 @@ function cmdInitManager(cwd: string, raw: boolean): void { ) { phase['deps_satisfied'] = true; } else { - const depNums = (phase['depends_on'] as string).match(/\d+[A-Z]?(?:\.\d+)*/gi) || []; + const depNums = (phase['depends_on'] as string).match(new RegExp(`${PHASE_NUMBER_TOKEN_SOURCE}`, 'gi')) || []; phase['deps_satisfied'] = depNums.every((n) => completedNums.has(normalizePhaseNumber(n))); phase['dep_phases'] = depNums; } @@ -1689,13 +1690,13 @@ function cmdInitProgress(cwd: string, raw: boolean): void { cwd, ); // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const headingPattern = /#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)(?:\s*\([^)\n]*\))?\s*:\s*([^\n]+)/gi; + const headingPattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]*\\))?\\s*:\\s*([^\\n]+)`, 'gi'); let hm: RegExpExecArray | null; while ((hm = headingPattern.exec(roadmapContent)) !== null) { roadmapPhaseNums.add(hm[1]); roadmapPhaseNames.set(hm[1], hm[2].replace(/\(INSERTED\)/i, '').trim()); } - const cbPattern = /-\s*\[(x| )\]\s*.*Phase\s+(\d+[A-Z]?(?:\.\d+)*)[:\s]/gi; + const cbPattern = new RegExp(`-\\s*\\[(x| )\\]\\s*.*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})[:\\s]`, 'gi'); let cbm: RegExpExecArray | null; while ((cbm = cbPattern.exec(roadmapContent)) !== null) { roadmapCheckboxStates.set(cbm[2], cbm[1].toLowerCase() === 'x'); @@ -1714,13 +1715,16 @@ function cmdInitProgress(cwd: string, raw: boolean): void { .map((e) => e.name) .filter(isDirInMilestone) .sort((a, b) => { + // phase-id-owner: dir-name phase-token parse; extractPhaseToken dash-separated sub-phase semantics differ, so a token-source swap would risk remapping phase<->directory matches. Kept local. const pa = a.match(/^(\d+[A-Z]?(?:\.\d+)*)/i); + // phase-id-owner: dir-name phase-token parse; extractPhaseToken dash-separated sub-phase semantics differ, so a token-source swap would risk remapping phase<->directory matches. Kept local. const pb = b.match(/^(\d+[A-Z]?(?:\.\d+)*)/i); if (!pa || !pb) return a.localeCompare(b); return parseInt(pa[1], 10) - parseInt(pb[1], 10); }); for (const dir of dirs) { + // phase-id-owner: dir-name phase-token parse; extractPhaseToken dash-separated sub-phase semantics differ, so a token-source swap would risk remapping phase<->directory matches. Kept local. const dirMatch = dir.match(/^(\d+[A-Z]?(?:\.\d+)*)-?(.*)/i); const phaseNumber = dirMatch ? dirMatch[1] : dir; const phaseName = dirMatch && dirMatch[2] ? dirMatch[2] : null; diff --git a/src/milestone.cts b/src/milestone.cts index cafb7717a..3d4fcb5de 100644 --- a/src/milestone.cts +++ b/src/milestone.cts @@ -23,7 +23,7 @@ import ioMod = require('./io.cjs'); const { output, error } = ioMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { escapeRegex, normalizePhaseName, phaseTokenMatches } = phaseIdMod; +const { escapeRegex, normalizePhaseName, phaseTokenMatches, PHASE_NUMBER_TOKEN_SOURCE } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import roadmapParserMod = require('./roadmap-parser.cjs'); const { getMilestonePhaseFilter, extractCurrentMilestone, getMilestoneInfo } = roadmapParserMod; @@ -177,7 +177,7 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo const roadmapContent = fs.readFileSync(roadmapPath, 'utf-8'); const scopedContent = extractCurrentMilestone(roadmapContent, cwd); // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phasePattern = /#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)(?:\s*\([^)\n]*\))?\s*:\s*([^\n]+)/gi; + const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]*\\))?\\s*:\\s*([^\\n]+)`, 'gi'); const noDirectoryPhases: string[] = []; let pm: RegExpExecArray | null; const phaseDirEntries = ((): string[] => { diff --git a/src/phase.cts b/src/phase.cts index deb41fe4b..6de08a4d1 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -37,6 +37,7 @@ const { phaseTokenMatches, OPTIONAL_PROJECT_CODE_PREFIX_SOURCE, OPTIONAL_PHASE_TAG_SOURCE, + PHASE_NUMBER_TOKEN_SOURCE, } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports -- phase-locator.cjs is an export= CommonJS module import phaseLocatorMod = require('./phase-locator.cjs'); @@ -374,8 +375,9 @@ function cmdFindPhase(cwd: string, phase: string, raw: boolean): void { if (!match) continue; const dirMatch = - match.match(new RegExp(`^${OPTIONAL_PROJECT_CODE_PREFIX_SOURCE}(\\d+[A-Z]?(?:\\.\\d+)*)-?(.*)`, 'i')) || - match.match(/^(\d+[A-Z]?(?:\.\d+)*)-?(.*)/i); + match.match( + new RegExp(`^${OPTIONAL_PROJECT_CODE_PREFIX_SOURCE}(${PHASE_NUMBER_TOKEN_SOURCE})-?(.*)`, 'i') + ) || match.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})-?(.*)`, 'i')); const phaseNumber = dirMatch ? dirMatch[1] : normalized; const phaseName = dirMatch && dirMatch[2] ? dirMatch[2] : null; @@ -1672,7 +1674,7 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { .sort((a, b) => comparePhaseNum(a, b)); for (const dir of dirs) { - const dm = dir.match(/^(\d+[A-Z]?(?:\.\d+)*)-?(.*)/i); + const dm = dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})-?(.*)`, 'i')); if (dm) { if (/^999(?:\.|$)/.test(dm[1])) continue; if (comparePhaseNum(dm[1], phaseNum) > 0) { @@ -1705,7 +1707,10 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { // #1729: `(?:\s*\([^)\n]*\))?` after the number tolerates a pre-colon // ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE) so // `### Phase N (Cluster B): X` resolves. Captures are unchanged. - const phasePattern = /(?:#{2,4}|-\s*\[[ xX]\])\s*(?:\*\*|__)?\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)(?:\s*\([^)\n]*\))?\s*:\s*([^\n*]+)/gi; + const phasePattern = new RegExp( + `(?:#{2,4}|-\\s*\\[[ xX]\\])\\s*(?:\\*\\*|__)?\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]*\\))?\\s*:\\s*([^\\n*]+)`, + 'gi' + ); let pm: RegExpExecArray | null; while ((pm = phasePattern.exec(roadmapForPhases)) !== null) { if (comparePhaseNum(pm[1], phaseNum) > 0) { @@ -1741,8 +1746,10 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { if (isLastPhase && roadmapContent !== null) { try { const milestoneScope = extractCurrentMilestone(roadmapContent, cwd); - const cbPattern = - /-\s*\[(x| )\]\s*(?:\*\*|__)?\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)(?:\s*\([^)\n]*\))?\s*:\s*([^\n*]+)/gi; + const cbPattern = new RegExp( + `-\\s*\\[(x| )\\]\\s*(?:\\*\\*|__)?\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]*\\))?\\s*:\\s*([^\\n*]+)`, + 'gi' + ); let cbm: RegExpExecArray | null; let lowestOutstanding: { num: string; name: string } | null = null; while ((cbm = cbPattern.exec(milestoneScope)) !== null) { diff --git a/src/roadmap-command-router.cts b/src/roadmap-command-router.cts index 97f6a0090..2e23b7318 100644 --- a/src/roadmap-command-router.cts +++ b/src/roadmap-command-router.cts @@ -73,6 +73,7 @@ function checkW021(content: string): W021Warning[] { // #1729: `(?:\s*\([^)\n]*\))?` 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]*\))?\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]*\))?\s*:/i; let currentMilestoneMajor: number | null = null; diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index f2ab3f648..8647da8d1 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -459,6 +459,7 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p // the milestone as a bogus "46-6" id. const numericRe = roadmapUsesHyphenedIds ? /^0*(\d+(?:-\d{2,})*[A-Za-z]?(?:\.\d+)*)/ + // phase-id-owner: [A-Za-z] case-variant token (identical under /i); kept literal, not source-byte-equal to the canonical PHASE_NUMBER_TOKEN_SOURCE. : /^0*(\d+[A-Za-z]?(?:\.\d+)*)/; function isDirInMilestone(dirName: string): boolean { diff --git a/src/roadmap-upgrade.cts b/src/roadmap-upgrade.cts index 3487a0334..e7202a495 100644 --- a/src/roadmap-upgrade.cts +++ b/src/roadmap-upgrade.cts @@ -16,13 +16,16 @@ import planningWorkspace = require('./planning-workspace.cjs'); // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); const { planningDir } = planningWorkspace; -const { stripProjectCodePrefix } = phaseIdMod; +const { stripProjectCodePrefix, PHASE_NUMBER_TOKEN_SOURCE } = phaseIdMod; // ─── Regex helpers ──────────────────────────────────────────────────────────── // 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 = /^(#{2,4})\s*(?:\[[^\]]+\]\s*)?Phase\s+(\d+[A-Z]?(?:\.\d+)*)\s*:(.*)/i; +const LEGACY_PHASE_HEADING_RE = new RegExp( + `^(#{2,4})\\s*(?:\\[[^\\]]+\\]\\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; @@ -172,6 +175,7 @@ function extractPhaseNumFromDir(dirName: string): string | null { const stripped = stripProjectCodePrefix(dirName); // Matches: digits + optional letter + optional decimal suffix, followed by '-' or end. // e.g. "02.1-hotfix" → "02.1", "01-setup" → "01" + // phase-id-owner: strips a leading phase number from a dir name; extractPhaseToken returns the project-code-prefixed token, so it is not a behavior-preserving drop-in. const m = stripped.match(/^(\d+[A-Z]?(?:\.\d+)*)(?:-|$)/i); return m ? m[1] : null; } @@ -188,7 +192,7 @@ function buildNewDirName(oldDirName: string, newId: string, projectCode: string const stripped = stripProjectCodePrefix(oldDirName); // Extract slug: everything after "NN-" (the old phase num, including decimal like 02.1) - const slugMatch = stripped.match(/^\d+[A-Z]?(?:\.\d+)*-(.*)/i); + const slugMatch = stripped.match(new RegExp(`^${PHASE_NUMBER_TOKEN_SOURCE}-(.*)`, 'i')); const slug = slugMatch ? slugMatch[1] : stripped; // Build M-NN prefix (zero-pad both parts) @@ -341,7 +345,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( - /^(#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+)\d+[A-Z]?(?:\.\d+)*(\s*:)/i, + new RegExp(`^(#{2,4}\\s*(?:\\[[^\\]]+\\]\\s*)?Phase\\s+)${PHASE_NUMBER_TOKEN_SOURCE}(\\s*:)`, 'i'), `$1${mapping.newId}$2` ); if (newLine !== oldLine) { @@ -364,7 +368,9 @@ function computeMigrationPlan(cwd: string, options: Record = {} if (roadmapEdits.some(e => e.lineIndex === i)) continue; // Match checklist items: "- [ ] **Phase N:**" or "- [x] Phase N:" (also decimal) - const checklistMatch = line.match(/^(\s*-\s*\[[ x]\]\s*\*{0,2}Phase\s+)(\d+[A-Z]?(?:\.\d+)*)(\s*[:\s*])/i); + const checklistMatch = line.match( + new RegExp(`^(\\s*-\\s*\\[[ x]\\]\\s*\\*{0,2}Phase\\s+)(${PHASE_NUMBER_TOKEN_SOURCE})(\\s*[:\\s*])`, 'i') + ); if (checklistMatch) { const legacyNum = checklistMatch[2]; const cIntPart = parseInt(legacyNum, 10); @@ -393,7 +399,7 @@ function computeMigrationPlan(cwd: string, options: Record = {} if (newId) { const newLine = line.replace( - /^(\s*-\s*\[[ x]\]\s*\*{0,2}Phase\s+)\d+[A-Z]?(?:\.\d+)*(\s*[:\s*])/i, + new RegExp(`^(\\s*-\\s*\\[[ x]\\]\\s*\\*{0,2}Phase\\s+)${PHASE_NUMBER_TOKEN_SOURCE}(\\s*[:\\s*])`, 'i'), `$1${newId}$2` ); if (newLine !== line) { diff --git a/src/roadmap.cts b/src/roadmap.cts index 27f0e36c3..168dad3cb 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -298,6 +298,7 @@ function cmdRoadmapAnalyze(cwd: string, raw: boolean): void { // Extract all phase headings: ## Phase N: Name or ### Phase N: Name // #1729: `(?:\s*\([^)\n]*\))?` 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]*\))?\s*:\s*([^\n]+)/gi; const phases: Array<{ number: string; @@ -437,6 +438,7 @@ function cmdRoadmapAnalyze(cwd: string, raw: boolean): void { // The char class must allow `-` (not just `.`) so dash-separated milestone-prefixed // IDs (e.g. `1-01`) match the detail-heading scanner above; otherwise they truncate // at the dash (`1-01` -> `1`) and every such phase reports a phantom missing detail. + // 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 checklistPattern = /-\s*\[[ x]\]\s*\*\*Phase\s+(\d+[A-Z]?(?:[.-]\d+)*)/gi; const checklistPhases = new Set(); let checklistMatch: RegExpExecArray | null; diff --git a/src/state.cts b/src/state.cts index a4778f1c1..9bd235342 100644 --- a/src/state.cts +++ b/src/state.cts @@ -16,7 +16,7 @@ import configLoaderMod = require('./config-loader.cjs'); const { loadConfig } = configLoaderMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { escapeRegex, normalizePhaseName, extractPhaseToken, parsePhaseFromProse } = phaseIdMod; +const { escapeRegex, normalizePhaseName, extractPhaseToken, parsePhaseFromProse, PHASE_NUMBER_TOKEN_SOURCE } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import roadmapParserMod = require('./roadmap-parser.cjs'); const { getMilestoneInfo, getMilestonePhaseFilter, extractCurrentMilestone } = roadmapParserMod; @@ -1406,6 +1406,7 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined): Re // neither the denominator nor the numerator (mirrors the heading // exclusion below). Project-code-aware via phaseKeyFromDir. if (retiredPhaseNums.size > 0 && retiredPhaseNums.has(phaseKeyFromDir(dir))) continue; + // phase-id-owner: dir-name dedup grouping; diverges from extractPhaseToken/phaseKeyFromDir on project-code-prefixed and multi-segment milestone dirs. Kept local. const m = dir.match(/^0*(\d+[A-Za-z]?(?:\.\d+)*)/); const key = m ? m[1].toLowerCase() : dir; if (!seenPhaseNums.has(key)) { @@ -2394,7 +2395,7 @@ function cmdStateSync(cwd: string, options: StateSyncOptions | undefined, raw: b if (completed) diskCompletedPhases++; // Track the highest phase with incomplete plans (or any plans) - const phaseMatch = dir.match(/^(\d+[A-Z]?(?:\.\d+)*)/i); + const phaseMatch = dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); if (phaseMatch && plans > 0) { if (summaries < plans) { // Incomplete phase — this is likely the current one diff --git a/src/uat.cts b/src/uat.cts index 9846f45bb..c5c3900ab 100644 --- a/src/uat.cts +++ b/src/uat.cts @@ -82,6 +82,7 @@ function cmdAuditUat(cwd: string, raw: boolean): void { .sort(); for (const dir of dirs) { + // phase-id-owner: display phase field derived from a dir name (same family as the audit.cts sites); not equivalent to extractPhaseToken for dash-form dirs. const phaseMatch = dir.match(/^(\d+[A-Z]?(?:\.\d+)*)/i); const phaseNum = phaseMatch ? phaseMatch[1] : dir; const phaseDir = path.join(phasesDir, dir); diff --git a/src/validate.cts b/src/validate.cts index 5ed32d96c..05f96bd73 100644 --- a/src/validate.cts +++ b/src/validate.cts @@ -33,7 +33,7 @@ // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { OPTIONAL_PROJECT_CODE_PREFIX_SOURCE } = phaseIdMod; +const { OPTIONAL_PROJECT_CODE_PREFIX_SOURCE, PHASE_NUMBER_TOKEN_SOURCE } = phaseIdMod; // ── Issue #26: regex constants (W005, W006-archived) ──────────────────────── // Matches legacy numeric dirs (01-setup), milestone-prefixed dirs (02-01-setup), @@ -62,7 +62,7 @@ export function canonicalPlanStem(stem: string): string { // #2043: the plan component (after the phase number) must be zero-padded // (≥2 digits), so a digit-leading slug word (e.g. "46-6-rs-…") is not mistaken // for a "46-6" phase/plan pair. - const m = stem.match(/^(\d+[A-Z]?(?:\.\d+)*-\d{2,})/i); + const m = stem.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE}-\\d{2,})`, 'i')); return m ? m[1] : stem; } diff --git a/src/verify.cts b/src/verify.cts index f7dfa924f..488598df9 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -37,7 +37,7 @@ import configLoaderMod = require('./config-loader.cjs'); const { loadConfig, CONFIG_DEFAULTS } = configLoaderMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { normalizePhaseName, phaseTokenMatches, escapeRegex, getMilestoneFromPhaseId, OPTIONAL_PHASE_TAG_SOURCE } = phaseIdMod; +const { normalizePhaseName, phaseTokenMatches, escapeRegex, getMilestoneFromPhaseId, OPTIONAL_PHASE_TAG_SOURCE, PHASE_NUMBER_TOKEN_SOURCE } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseLocatorMod = require('./phase-locator.cjs'); const { findPhaseInternal } = phaseLocatorMod; @@ -1302,14 +1302,18 @@ function cmdValidateHealth( repairs.push('regenerateState'); } else { const stateContent = fs.readFileSync(statePath, 'utf-8'); - const phaseRefs = [...stateContent.matchAll(/[Pp]hase\s+(\d+[A-Z]?(?:\.\d+)*)/g)].map( + const phaseRefs = [ + ...stateContent.matchAll(new RegExp(`[Pp]hase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})`, 'g')), + ].map( (m) => m[1], ); const validPhases = collectDiskPhases(planBase); try { if (fs.existsSync(roadmapPath)) { const roadmapRaw = fs.readFileSync(roadmapPath, 'utf-8'); - const all = [...roadmapRaw.matchAll(/#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)/gi)]; + const all = [ + ...roadmapRaw.matchAll(new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})`, 'gi')), + ]; for (const m of all) validPhases.add(m[1]); } } catch { @@ -1809,7 +1813,7 @@ function cmdValidateHealth( const roadmapRaw = fs.readFileSync(roadmapPath, 'utf-8'); const scopedContent = extractCurrentMilestone(roadmapRaw, cwd); // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phasePattern = /#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)(?:\s*\([^)\n]*\))?\s*:\s*([^\n]+)/gi; + const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]*\\))?\\s*:\\s*([^\\n]+)`, 'gi'); const unstarted: string[] = []; let pm: RegExpExecArray | null; // Non-hoisted: load-order matters (circular dep guard) From e2eaa5b04637fe34b33ec062aedaec7eaae9be23 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 10 Jul 2026 09:14:15 -0400 Subject: [PATCH 03/10] =?UTF-8?q?fix(#2128):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20migrate=209=20mis-allowlisted=20sites,=20harden=20s?= =?UTF-8?q?canner=20+=20guards?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Correctness review of the Phase 4 guard found the allowlist over-broad and the scanner/guards evadable. Fixed all findings: - Migrate 9 sites that were wrongly sanctioned: their regex is the PURE canonical token (`\d+[A-Z]?(?:\.\d+)*`, no variant), byte-identical to already-migrated siblings. The old justification argued against swapping to the extractPhaseToken() FUNCTION (behavior-risky) — but the guard only wants the same regex built from the SOURCE string (byte-equal, zero risk). Coverage is now 32 migrated / 5 sanctioned, not the overstated 23 / 14 (audit.cts x3, uat.cts, init.cts x4, roadmap-upgrade.cts). Each conversion proven byte-equal (.source + .flags). - Harden the drift detector: also catch the `[0-9]`-in-place-of-`\d` variant; document the accepted limits (cross-line split, semantic restructuring — covered by the identity guard + review, not a text scan). - Sanction robustness: a `phase-id-owner:` marker now counts only inside a `//` comment (a bare substring in a string no longer suppresses a real flag), and the preceding-line window skips blank lines (an auto-formatter's blank line no longer reactivates the flag). - roadmap-parser.cts:462 comment: corrected — that regex carries no /i flag, so its [A-Za-z] class does real case work (matches state.cts:1409's rationale). - Identity guard: surface require failures instead of silently skipping, and floor coverage at >75% of consumer modules (inspects 156/157). Co-Authored-By: Claude Opus 4.8 --- scripts/lint-phase-id-drift.cjs | 34 ++++++++++++++++++++--------- src/audit.cts | 12 +++++----- src/init.cts | 12 ++++------ src/roadmap-parser.cts | 2 +- src/roadmap-upgrade.cts | 3 +-- src/uat.cts | 6 +++-- tests/phase-id-drift-guard.test.cjs | 34 +++++++++++++++++++++++------ 7 files changed, 67 insertions(+), 36 deletions(-) diff --git a/scripts/lint-phase-id-drift.cjs b/scripts/lint-phase-id-drift.cjs index 33cecc3cd..3516814e1 100644 --- a/scripts/lint-phase-id-drift.cjs +++ b/scripts/lint-phase-id-drift.cjs @@ -34,19 +34,31 @@ const path = require('node:path'); // The canonical phase-number token as it appears in SOURCE TEXT: // \d+[A-Z]?(?:\.\d+)* in a regex literal -> one backslash before d/. // \\d+[A-Z]?(?:\\.\\d+)* in a template string -> two backslashes -// Also tolerate the [A-Za-z] letter-class and the [.-] (dot-or-dash) separator -// near-variants that a few enumeration call sites use. -const TOKEN_DRIFT_RE = /\\{1,2}d\+\[A-Z(?:a-z)?\]\??\(\?:(?:\\{1,2}\.|\[\.-\])\\{1,2}d\+\)\*/; +// Tolerated near-variants so a trivial rewrite does not silently evade the guard: +// digit class \d \\d or [0-9] +// letter class [A-Z] or [A-Za-z] +// sub-phase sep \. \\. or [.-] (dot-or-dash) +// KNOWN, ACCEPTED limits of a per-line textual scan (covered instead by the +// identity guard + code review, not by this regex): a re-derivation split +// across lines via string concatenation, a capturing `(\.\d+)*` in place of the +// non-capturing group, or a semantically-equivalent restructuring. This guard +// targets the common case — an accidental copy of the exact grammar — not an +// adversary deliberately obfuscating a re-derivation. +const TOKEN_DRIFT_RE = /(?:\\{1,2}d|\[0-9\])\+\[A-Z(?:a-z)?\]\??\(\?:(?:\\{1,2}\.|\[\.-\])(?:\\{1,2}d|\[0-9\])\+\)\*/; -const OWNER_MARK = 'phase-id-owner:'; +// A `phase-id-owner:` sanction only counts inside a `//` line comment — a bare +// substring in a string literal or identifier must NOT suppress a real flag. +const OWNER_RE = /\/\/[^\n]*phase-id-owner:/; const CANON_REF = 'PHASE_NUMBER_TOKEN_SOURCE'; /** * Pure: find every literal re-derivation of the canonical phase-number token in - * `text` that is NOT sanctioned. A site is sanctioned when its line — or the - * line directly above it — contains `// phase-id-owner:`, or when the line - * references `PHASE_NUMBER_TOKEN_SOURCE` (i.e. it is built from the canonical - * source, not a literal). Returns [{ line, found }]. + * `text` that is NOT sanctioned. A site is sanctioned when its own line — or the + * nearest preceding NON-BLANK line (so an auto-formatter's blank line between a + * `// phase-id-owner:` comment and its regex does not reactivate the flag) — + * carries a `// phase-id-owner:` comment, or when the line references + * `PHASE_NUMBER_TOKEN_SOURCE` (built from the canonical source, not a literal). + * Returns [{ line, found }]. */ function findPhaseIdRegexDrift(text) { const out = []; @@ -55,9 +67,11 @@ function findPhaseIdRegexDrift(text) { const line = lines[i]; const m = TOKEN_DRIFT_RE.exec(line); if (!m) continue; - if (line.includes(OWNER_MARK)) continue; - if (i > 0 && lines[i - 1].includes(OWNER_MARK)) continue; + if (OWNER_RE.test(line)) continue; if (line.includes(CANON_REF)) continue; + let j = i - 1; + while (j >= 0 && lines[j].trim() === '') j--; // nearest preceding non-blank line + if (j >= 0 && OWNER_RE.test(lines[j])) continue; out.push({ line: i + 1, found: m[0] }); } return out; diff --git a/src/audit.cts b/src/audit.cts index c7bf2f983..f9a500b9a 100644 --- a/src/audit.cts +++ b/src/audit.cts @@ -20,6 +20,9 @@ const { planningDir } = planningWorkspace; // eslint-disable-next-line @typescript-eslint/no-require-imports import frontmatter = require('./frontmatter.cjs'); const { extractFrontmatter } = frontmatter; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import phaseIdMod = require('./phase-id.cjs'); +const { PHASE_NUMBER_TOKEN_SOURCE } = phaseIdMod; import { requireSafePath, sanitizeForDisplay } from './security.cjs'; // ─── Types ──────────────────────────────────────────────────────────────────── @@ -482,8 +485,7 @@ function scanUatGaps(planDir: string): UatGapItem[] { for (const dir of dirs) { const phaseDir = path.join(phasesDir, dir); - // phase-id-owner: cosmetic phase label derived from a dir name for JSON output; the single-segment capture is not equivalent to extractPhaseToken dash-continuation semantics, so not a behavior-preserving drop-in. - const phaseMatch = dir.match(/^(\d+[A-Z]?(?:\.\d+)*)/i); + const phaseMatch = dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); const phaseNum = phaseMatch ? phaseMatch[1] : dir; let files: string[]; @@ -553,8 +555,7 @@ function scanVerificationGaps(planDir: string): VerificationGapItem[] { for (const dir of dirs) { const phaseDir = path.join(phasesDir, dir); - // phase-id-owner: cosmetic phase label derived from a dir name for JSON output; the single-segment capture is not equivalent to extractPhaseToken dash-continuation semantics, so not a behavior-preserving drop-in. - const phaseMatch = dir.match(/^(\d+[A-Z]?(?:\.\d+)*)/i); + const phaseMatch = dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); const phaseNum = phaseMatch ? phaseMatch[1] : dir; let files: string[]; @@ -616,8 +617,7 @@ function scanContextQuestions(planDir: string): ContextQuestionItem[] { for (const dir of dirs) { const phaseDir = path.join(phasesDir, dir); - // phase-id-owner: cosmetic phase label derived from a dir name for JSON output; the single-segment capture is not equivalent to extractPhaseToken dash-continuation semantics, so not a behavior-preserving drop-in. - const phaseMatch = dir.match(/^(\d+[A-Z]?(?:\.\d+)*)/i); + const phaseMatch = dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); const phaseNum = phaseMatch ? phaseMatch[1] : dir; let files: string[]; diff --git a/src/init.cts b/src/init.cts index ab6bcafb1..13b2a890c 100644 --- a/src/init.cts +++ b/src/init.cts @@ -1181,8 +1181,7 @@ function cmdInitMilestoneOp(cwd: string, raw: boolean): void { const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); for (const e of entries) { if (!e.isDirectory()) continue; - // phase-id-owner: dir-name phase-token parse; extractPhaseToken dash-separated sub-phase semantics differ, so a token-source swap would risk remapping phase<->directory matches. Kept local. - const m = stripProjectCodePrefix(e.name).match(/^(\d+[A-Z]?(?:\.\d+)*)/); + const m = stripProjectCodePrefix(e.name).match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`)); if (!m) continue; diskPhaseDirs.set(canonicalizePhase(m[1]), e.name); } @@ -1715,17 +1714,14 @@ function cmdInitProgress(cwd: string, raw: boolean): void { .map((e) => e.name) .filter(isDirInMilestone) .sort((a, b) => { - // phase-id-owner: dir-name phase-token parse; extractPhaseToken dash-separated sub-phase semantics differ, so a token-source swap would risk remapping phase<->directory matches. Kept local. - const pa = a.match(/^(\d+[A-Z]?(?:\.\d+)*)/i); - // phase-id-owner: dir-name phase-token parse; extractPhaseToken dash-separated sub-phase semantics differ, so a token-source swap would risk remapping phase<->directory matches. Kept local. - const pb = b.match(/^(\d+[A-Z]?(?:\.\d+)*)/i); + const pa = a.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); + const pb = b.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); if (!pa || !pb) return a.localeCompare(b); return parseInt(pa[1], 10) - parseInt(pb[1], 10); }); for (const dir of dirs) { - // phase-id-owner: dir-name phase-token parse; extractPhaseToken dash-separated sub-phase semantics differ, so a token-source swap would risk remapping phase<->directory matches. Kept local. - const dirMatch = dir.match(/^(\d+[A-Z]?(?:\.\d+)*)-?(.*)/i); + const dirMatch = dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})-?(.*)`, 'i')); const phaseNumber = dirMatch ? dirMatch[1] : dir; const phaseName = dirMatch && dirMatch[2] ? dirMatch[2] : null; seenPhaseNums.add(phaseNumber.replace(/^0+/, '') || '0'); diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index 8647da8d1..091789c48 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -459,7 +459,7 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p // the milestone as a bogus "46-6" id. const numericRe = roadmapUsesHyphenedIds ? /^0*(\d+(?:-\d{2,})*[A-Za-z]?(?:\.\d+)*)/ - // phase-id-owner: [A-Za-z] case-variant token (identical under /i); kept literal, not source-byte-equal to the canonical PHASE_NUMBER_TOKEN_SOURCE. + // phase-id-owner: the [A-Za-z] letter class does real case handling here — this regex carries NO /i flag; kept literal, not source-byte-equal to the canonical PHASE_NUMBER_TOKEN_SOURCE. : /^0*(\d+[A-Za-z]?(?:\.\d+)*)/; function isDirInMilestone(dirName: string): boolean { diff --git a/src/roadmap-upgrade.cts b/src/roadmap-upgrade.cts index e7202a495..2c47995b9 100644 --- a/src/roadmap-upgrade.cts +++ b/src/roadmap-upgrade.cts @@ -175,8 +175,7 @@ function extractPhaseNumFromDir(dirName: string): string | null { const stripped = stripProjectCodePrefix(dirName); // Matches: digits + optional letter + optional decimal suffix, followed by '-' or end. // e.g. "02.1-hotfix" → "02.1", "01-setup" → "01" - // phase-id-owner: strips a leading phase number from a dir name; extractPhaseToken returns the project-code-prefixed token, so it is not a behavior-preserving drop-in. - const m = stripped.match(/^(\d+[A-Z]?(?:\.\d+)*)(?:-|$)/i); + const m = stripped.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})(?:-|$)`, 'i')); return m ? m[1] : null; } diff --git a/src/uat.cts b/src/uat.cts index c5c3900ab..e5cd5e964 100644 --- a/src/uat.cts +++ b/src/uat.cts @@ -29,6 +29,9 @@ const { planningDir } = planningWorkspace; // eslint-disable-next-line @typescript-eslint/no-require-imports import frontmatter = require('./frontmatter.cjs'); const { extractFrontmatter } = frontmatter; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import phaseIdMod = require('./phase-id.cjs'); +const { PHASE_NUMBER_TOKEN_SOURCE } = phaseIdMod; import { requireSafePath, sanitizeForDisplay } from './security.cjs'; // ─── Types ──────────────────────────────────────────────────────────────────── @@ -82,8 +85,7 @@ function cmdAuditUat(cwd: string, raw: boolean): void { .sort(); for (const dir of dirs) { - // phase-id-owner: display phase field derived from a dir name (same family as the audit.cts sites); not equivalent to extractPhaseToken for dash-form dirs. - const phaseMatch = dir.match(/^(\d+[A-Z]?(?:\.\d+)*)/i); + const phaseMatch = dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); const phaseNum = phaseMatch ? phaseMatch[1] : dir; const phaseDir = path.join(phasesDir, dir); const files = fs.readdirSync(phaseDir); diff --git a/tests/phase-id-drift-guard.test.cjs b/tests/phase-id-drift-guard.test.cjs index 3e20b291a..63858f23c 100644 --- a/tests/phase-id-drift-guard.test.cjs +++ b/tests/phase-id-drift-guard.test.cjs @@ -60,9 +60,10 @@ describe('#2128 phase-id drift scanner: findPhaseIdRegexDrift (pure)', () => { assert.equal(v.length, 1); }); - test('the [A-Za-z] and [.-] near-variants ARE flagged', () => { - assert.equal(findPhaseIdRegexDrift('/(\\d+[A-Za-z]?(?:\\.\\d+)*)/').length, 1); - assert.equal(findPhaseIdRegexDrift('/(\\d+[A-Z]?(?:[.-]\\d+)*)/').length, 1); + test('the [A-Za-z], [.-] and [0-9] near-variants ARE flagged (no trivial evasion)', () => { + assert.equal(findPhaseIdRegexDrift('/(\\d+[A-Za-z]?(?:\\.\\d+)*)/').length, 1, '[A-Za-z] letter class'); + assert.equal(findPhaseIdRegexDrift('/(\\d+[A-Z]?(?:[.-]\\d+)*)/').length, 1, '[.-] separator'); + assert.equal(findPhaseIdRegexDrift('/([0-9]+[A-Z]?(?:\\.[0-9]+)*)/').length, 1, '[0-9] in place of \\d'); }); test('a same-line // phase-id-owner: sanction suppresses the flag', () => { @@ -79,6 +80,18 @@ describe('#2128 phase-id drift scanner: findPhaseIdRegexDrift (pure)', () => { ); }); + test('a blank line between the // phase-id-owner: comment and the regex still suppresses', () => { + assert.deepEqual( + findPhaseIdRegexDrift('// phase-id-owner: sanctioned exception\n\nconst re = /(\\d+[A-Z]?(?:\\.\\d+)*)/;'), + [], + ); + }); + + test('a bare "phase-id-owner:" substring in a STRING (not a // comment) does NOT suppress', () => { + const v = findPhaseIdRegexDrift('const msg = "ping the phase-id-owner: for review"; const re = /(\\d+[A-Z]?(?:\\.\\d+)*)/;'); + assert.equal(v.length, 1); + }); + test('non-token phase regexes are NOT flagged (no false positives)', () => { assert.deepEqual(findPhaseIdRegexDrift('/^Executing Phase\\s+\\d+/'), [], 'status-message bare \\d+'); assert.deepEqual(findPhaseIdRegexDrift('/#{2,4}\\s*Phase\\s+(\\d+)[A-Z]?(?:\\.\\d+)*/'), [], 'digits-only capture is non-contiguous'); @@ -119,14 +132,18 @@ describe('#2128 phase-id single-owner identity guard', () => { const libDir = path.join(ROOT, 'gsd-core', 'bin', 'lib'); const consumers = fs.readdirSync(libDir).filter((f) => f.endsWith('.cjs') && f !== 'phase-id.cjs'); let checked = 0; + const requireFailures = []; for (const f of consumers) { let mod; try { mod = require(path.join(libDir, f)); - } catch { - continue; // a module that cannot be required in isolation can't re-export anything + } catch (e) { + // Surfaced, not silently skipped — a module that cannot be required + // would otherwise erode the guard's coverage without any signal. + requireFailures.push(`${f}: ${e.message}`); + continue; } - if (!mod || typeof mod !== 'object') continue; + if (!mod || typeof mod !== 'object') continue; // bare-function exports carry no named canonical member checked++; for (const name of CANONICAL) { if (Object.prototype.hasOwnProperty.call(mod, name)) { @@ -138,6 +155,9 @@ describe('#2128 phase-id single-owner identity guard', () => { } } } - assert.ok(checked > 0, 'expected to inspect at least one consumer module'); + assert.deepEqual(requireFailures, [], `consumer module(s) failed to require (guard coverage would silently degrade):\n ${requireFailures.join('\n ')}`); + // Coverage floor: the vast majority of the ~150 built lib modules export an + // object and must actually be inspected — not a token "at least one". + assert.ok(checked > consumers.length * 0.75, `expected to inspect most of the ${consumers.length} consumer modules, only inspected ${checked}`); }); }); From a1de52d71bea145085707c35874ac46c4eca140e Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 10 Jul 2026 09:24:49 -0400 Subject: [PATCH 04/10] fix(#2128): sanctions must be a dedicated // comment line (decoy-proof) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-review found the `// phase-id-owner:` suppression treated a `//` embedded in a string literal as a comment — help/doc text quoting the sanction syntax (the exact string the scanner's own main() prints) would silently suppress a real re-derivation. Require the marker to LEAD its own comment line (`^\s*//…`), so a `//` inside a string or trailing a code line never counts. All 5 real sanctions are already dedicated lines (scanRepo stays green); trailing same-line sanctions are no longer honored — put the comment on the line directly above. Co-Authored-By: Claude Opus 4.8 --- scripts/lint-phase-id-drift.cjs | 23 +++++++++++--------- tests/phase-id-drift-guard.test.cjs | 33 ++++++++++++++++++----------- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/scripts/lint-phase-id-drift.cjs b/scripts/lint-phase-id-drift.cjs index 3516814e1..ff04bfa72 100644 --- a/scripts/lint-phase-id-drift.cjs +++ b/scripts/lint-phase-id-drift.cjs @@ -46,18 +46,21 @@ const path = require('node:path'); // adversary deliberately obfuscating a re-derivation. const TOKEN_DRIFT_RE = /(?:\\{1,2}d|\[0-9\])\+\[A-Z(?:a-z)?\]\??\(\?:(?:\\{1,2}\.|\[\.-\])(?:\\{1,2}d|\[0-9\])\+\)\*/; -// A `phase-id-owner:` sanction only counts inside a `//` line comment — a bare -// substring in a string literal or identifier must NOT suppress a real flag. -const OWNER_RE = /\/\/[^\n]*phase-id-owner:/; +// A `phase-id-owner:` sanction must be a DEDICATED `//` comment line (the marker +// as the line's leading token). A `//` or the phrase embedded in a string +// literal or trailing a code line is NOT a comment and must never suppress a real +// flag — so sanctions live on their own line directly above the regex. +const OWNER_RE = /^\s*\/\/.*phase-id-owner:/; const CANON_REF = 'PHASE_NUMBER_TOKEN_SOURCE'; /** * Pure: find every literal re-derivation of the canonical phase-number token in - * `text` that is NOT sanctioned. A site is sanctioned when its own line — or the - * nearest preceding NON-BLANK line (so an auto-formatter's blank line between a - * `// phase-id-owner:` comment and its regex does not reactivate the flag) — - * carries a `// phase-id-owner:` comment, or when the line references - * `PHASE_NUMBER_TOKEN_SOURCE` (built from the canonical source, not a literal). + * `text` that is NOT sanctioned. A site is sanctioned when the nearest preceding + * NON-BLANK line is a dedicated `// phase-id-owner:` comment (blank lines between + * the comment and the regex are tolerated, so an auto-formatter cannot reactivate + * the flag), or when the regex line references `PHASE_NUMBER_TOKEN_SOURCE` (built + * from the canonical source, not a literal). A `//`/phrase inside a string or + * trailing a code line does NOT count — put the sanction on its own line above. * Returns [{ line, found }]. */ function findPhaseIdRegexDrift(text) { @@ -67,7 +70,6 @@ function findPhaseIdRegexDrift(text) { const line = lines[i]; const m = TOKEN_DRIFT_RE.exec(line); if (!m) continue; - if (OWNER_RE.test(line)) continue; if (line.includes(CANON_REF)) continue; let j = i - 1; while (j >= 0 && lines[j].trim() === '') j--; // nearest preceding non-blank line @@ -135,7 +137,8 @@ function main() { } process.stderr.write('phase-id-drift: literal re-derivation(s) of the canonical phase-number token found.\n'); process.stderr.write('Build the regex from phase-id.cjs `PHASE_NUMBER_TOKEN_SOURCE` (or phaseMarkdownRegexSource for a\n'); - process.stderr.write('known number), or sanction the site with a `// phase-id-owner: ` comment:\n'); + process.stderr.write('known number), or sanction the site with a dedicated `// phase-id-owner: `\n'); + process.stderr.write('comment on the line directly above the regex:\n'); for (const d of violations) { process.stderr.write(` ${d.file}:${d.line} ${d.found}\n`); } diff --git a/tests/phase-id-drift-guard.test.cjs b/tests/phase-id-drift-guard.test.cjs index 63858f23c..960483dac 100644 --- a/tests/phase-id-drift-guard.test.cjs +++ b/tests/phase-id-drift-guard.test.cjs @@ -66,29 +66,38 @@ describe('#2128 phase-id drift scanner: findPhaseIdRegexDrift (pure)', () => { assert.equal(findPhaseIdRegexDrift('/([0-9]+[A-Z]?(?:\\.[0-9]+)*)/').length, 1, '[0-9] in place of \\d'); }); - test('a same-line // phase-id-owner: sanction suppresses the flag', () => { + test('a dedicated preceding // phase-id-owner: comment line suppresses the flag', () => { assert.deepEqual( - findPhaseIdRegexDrift('const re = /(\\d+[A-Z]?(?:\\.\\d+)*)/; // phase-id-owner: sanctioned exception'), - [], - ); - }); - - test('a preceding-line // phase-id-owner: sanction suppresses the flag', () => { - assert.deepEqual( - findPhaseIdRegexDrift('// phase-id-owner: sanctioned exception\nconst re = /(\\d+[A-Z]?(?:\\.\\d+)*)/;'), + findPhaseIdRegexDrift(' // phase-id-owner: sanctioned exception\n const re = /(\\d+[A-Z]?(?:\\.\\d+)*)/;'), [], ); }); test('a blank line between the // phase-id-owner: comment and the regex still suppresses', () => { assert.deepEqual( - findPhaseIdRegexDrift('// phase-id-owner: sanctioned exception\n\nconst re = /(\\d+[A-Z]?(?:\\.\\d+)*)/;'), + findPhaseIdRegexDrift(' // phase-id-owner: sanctioned exception\n\n const re = /(\\d+[A-Z]?(?:\\.\\d+)*)/;'), [], ); }); - test('a bare "phase-id-owner:" substring in a STRING (not a // comment) does NOT suppress', () => { - const v = findPhaseIdRegexDrift('const msg = "ping the phase-id-owner: for review"; const re = /(\\d+[A-Z]?(?:\\.\\d+)*)/;'); + test('a trailing same-line // phase-id-owner: is NOT a sanction (must be a dedicated line above)', () => { + // The marker must lead its own comment line; a trailing comment on a code + // line is not honored, so the regex is still flagged. + const v = findPhaseIdRegexDrift('const re = /(\\d+[A-Z]?(?:\\.\\d+)*)/; // phase-id-owner: not honored here'); + assert.equal(v.length, 1); + }); + + test('a // phase-id-owner: embedded in a STRING literal does NOT suppress (decoy)', () => { + // A `//` inside a string is not a comment — help/doc text that quotes the + // sanction syntax must not silently suppress a real re-derivation. + const decoyLine = findPhaseIdRegexDrift('const help = "use // phase-id-owner: "; const re = /(\\d+[A-Z]?(?:\\.\\d+)*)/;'); + assert.equal(decoyLine.length, 1); + const decoyPrev = findPhaseIdRegexDrift('const help = "use // phase-id-owner: ";\nconst re = /(\\d+[A-Z]?(?:\\.\\d+)*)/;'); + assert.equal(decoyPrev.length, 1); + }); + + test('a bare "phase-id-owner:" substring with no // does NOT suppress', () => { + const v = findPhaseIdRegexDrift('const msg = "ping the phase-id-owner for review"; const re = /(\\d+[A-Z]?(?:\\.\\d+)*)/;'); assert.equal(v.length, 1); }); From c1cd43a39ff50d5a816db35f3a60dda358390aca Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 10 Jul 2026 09:31:17 -0400 Subject: [PATCH 05/10] =?UTF-8?q?fix(#2128):=20bound=20the=20phase-tag=20c?= =?UTF-8?q?lause=20to=20{0,200}=20=E2=80=94=20kill=20quadratic=20ReDoS?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The canonical OPTIONAL_PHASE_TAG_SOURCE tag clause `(?:\s*\([^)\n]*\))?` (and its inlined literal mirrors across 11 modules) had an UNBOUNDED body, making the optional-group + /g header scan quadratic on adversarial ROADMAP.md/STATE.md — a long run of `(` after a header ran ~18.8s at 1.7MB. Bound the body to {0,200} in the constant AND every mirror in lockstep (the #1729 "both forms change together" contract), so the scan is linear: the same 1.7MB input now resolves in ~9ms (measured), while real tags (a handful of chars) still match and a 201-char tag is rejected. Added a #2128 boundary regression to the #1729 suite. Pre-existing (byte-identical before/after the Phase 4 migrations); folded in at maintainer direction rather than deferred. Co-Authored-By: Claude Opus 4.8 --- src/commands.cts | 4 ++-- src/init.cts | 12 ++++++------ src/milestone.cts | 4 ++-- src/phase-id.cts | 4 ++-- src/phase.cts | 16 ++++++++-------- src/roadmap-command-router.cts | 6 +++--- src/roadmap-parser.cts | 12 ++++++------ src/roadmap.cts | 4 ++-- src/state.cts | 8 ++++---- src/validate.cts | 4 ++-- src/verify.cts | 8 ++++---- tests/phase.test.cjs | 15 +++++++++++++++ 12 files changed, 56 insertions(+), 41 deletions(-) diff --git a/src/commands.cts b/src/commands.cts index 426923c0d..1e52eb6fb 100644 --- a/src/commands.cts +++ b/src/commands.cts @@ -1516,8 +1516,8 @@ function cmdStats(cwd: string, format: string | undefined, raw: boolean): void { const roadmapContent = extractCurrentMilestone(roadmapRaw, cwd); // 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]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const headingPattern = /#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]*\))?\s*:\s*([^\n]+)/gi; + // #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; let match: RegExpExecArray | null; while ((match = headingPattern.exec(roadmapContent)) !== null) { const key = normalizePhaseName(match[1]); diff --git a/src/init.cts b/src/init.cts index 13b2a890c..4e41febdc 100644 --- a/src/init.cts +++ b/src/init.cts @@ -1161,8 +1161,8 @@ function cmdInitMilestoneOp(cwd: string, raw: boolean): void { const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md'); const roadmapRaw = fs.readFileSync(roadmapPath, 'utf-8'); const currentSection = extractCurrentMilestone(roadmapRaw, cwd); - // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]*\\))?\\s*:`, 'gi'); + // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). + const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]{0,200}\\))?\\s*:`, 'gi'); let m: RegExpExecArray | null; while ((m = phasePattern.exec(currentSection)) !== null) { if (/^999(?:\.|$)/.test(m[1])) continue; @@ -1325,8 +1325,8 @@ function cmdInitManager(cwd: string, raw: boolean): void { _checkboxStates.set(_cbMatch[2], _cbMatch[1].toLowerCase() === 'x'); } - // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]*\\))?\\s*:\\s*([^\\n]+)`, 'gi'); + // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). + const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]{0,200}\\))?\\s*:\\s*([^\\n]+)`, 'gi'); const phases: Record[] = []; let match: RegExpExecArray | null; @@ -1688,8 +1688,8 @@ function cmdInitProgress(cwd: string, raw: boolean): void { fs.readFileSync(path.join(planningDir(cwd), 'ROADMAP.md'), 'utf-8'), cwd, ); - // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const headingPattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]*\\))?\\s*:\\s*([^\\n]+)`, 'gi'); + // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). + const headingPattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]{0,200}\\))?\\s*:\\s*([^\\n]+)`, 'gi'); let hm: RegExpExecArray | null; while ((hm = headingPattern.exec(roadmapContent)) !== null) { roadmapPhaseNums.add(hm[1]); diff --git a/src/milestone.cts b/src/milestone.cts index 3d4fcb5de..f6f3386e6 100644 --- a/src/milestone.cts +++ b/src/milestone.cts @@ -176,8 +176,8 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo if (stateVersion && stateVersion === version) { const roadmapContent = fs.readFileSync(roadmapPath, 'utf-8'); const scopedContent = extractCurrentMilestone(roadmapContent, cwd); - // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]*\\))?\\s*:\\s*([^\\n]+)`, 'gi'); + // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). + const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]{0,200}\\))?\\s*:\\s*([^\\n]+)`, 'gi'); const noDirectoryPhases: string[] = []; let pm: RegExpExecArray | null; const phaseDirEntries = ((): string[] => { diff --git a/src/phase-id.cts b/src/phase-id.cts index 7ce8f90f3..ead7a3b27 100644 --- a/src/phase-id.cts +++ b/src/phase-id.cts @@ -37,9 +37,9 @@ const OPTIONAL_PROJECT_CODE_PREFIX_SOURCE = '(?:[A-Z][A-Z0-9_]*-)?'; // Enumeration/parse call sites that read phase headers from a regex *literal* // (rather than a `new RegExp` built from an interpolated phase number) cannot // reference this constant; they inline its literal-regex mirror instead — -// `(?:\s*\([^)\n]*\))?` — kept character-for-character equivalent to this +// `(?:\s*\([^)\n]{0,200}\))?` — kept character-for-character equivalent to this // source. Both forms must change together; see the #1729 regression test. -const OPTIONAL_PHASE_TAG_SOURCE = '(?:\\s*\\([^)\\n]*\\))?'; +const OPTIONAL_PHASE_TAG_SOURCE = '(?:\\s*\\([^)\\n]{0,200}\\))?'; // #2128: the canonical phase-NUMBER-TOKEN grammar — a phase number with an // optional single-letter variant suffix and optional dotted sub-phases diff --git a/src/phase.cts b/src/phase.cts index 6de08a4d1..6ae72d999 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -713,8 +713,8 @@ function cmdPhaseAdd(cwd: string, description: string, raw: boolean, customId?: // (section header, roadmap bullet, or on-disk directory) is counted: // 1) Section headers: ### Phase N: / ## Phase N: / #### Phase N: - // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const headerPattern = /#{2,4}\s*Phase\s+(\d+)[A-Z]?(?:\.\d+)*(?:\s*\([^)\n]*\))?:/gi; + // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). + const headerPattern = /#{2,4}\s*Phase\s+(\d+)[A-Z]?(?:\.\d+)*(?:\s*\([^)\n]{0,200}\))?:/gi; // 2) Roadmap bullet entries: - [ ] **Phase N: ...** (all checkbox variants) // The lookahead accepts colon, decimal-dot, whitespace, bold-close asterisk, // or end-of-line so titleless forms ("- [ ] **Phase 11**", "- [ ] Phase 11") @@ -811,8 +811,8 @@ function cmdPhaseAddBatch(cwd: string, descriptions: string[], raw: boolean): vo const content = extractCurrentMilestone(rawContent, cwd); let maxPhase = 0; if (config.phase_naming !== 'custom') { - // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phasePattern = /#{2,4}\s*Phase\s+(\d+)[A-Z]?(?:\.\d+)*(?:\s*\([^)\n]*\))?:/gi; + // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). + const phasePattern = /#{2,4}\s*Phase\s+(\d+)[A-Z]?(?:\.\d+)*(?:\s*\([^)\n]{0,200}\))?:/gi; let m: RegExpExecArray | null; while ((m = phasePattern.exec(content)) !== null) { const num = parseInt(m[1], 10); @@ -1195,7 +1195,7 @@ function updateRoadmapAfterPhaseRemoval( // #1729: fold an optional pre-colon ( ) tag into the suffix capture so it // is re-emitted verbatim — a tagged later phase still gets renumbered. content = content.replace( - /(#{2,4}\s*Phase\s+)(\d+(?:\.\d+)?)((?:\s*\([^)\n]*\))?\s*:)/gi, + /(#{2,4}\s*Phase\s+)(\d+(?:\.\d+)?)((?:\s*\([^)\n]{0,200}\))?\s*:)/gi, (_match, prefix: string, num: string, suffix: string) => `${prefix}${decrementRoadmapPhaseToken(num, removedInt)}${suffix}`, ); @@ -1704,11 +1704,11 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { // phase. Allow optional `**`/`__` emphasis after the marker and stop // the name capture at emphasis so bold names slug cleanly; the number // capture is unchanged. - // #1729: `(?:\s*\([^)\n]*\))?` after the number tolerates a pre-colon + // #1729: `(?:\s*\([^)\n]{0,200}\))?` after the number tolerates a pre-colon // ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE) so // `### Phase N (Cluster B): X` resolves. Captures are unchanged. const phasePattern = new RegExp( - `(?:#{2,4}|-\\s*\\[[ xX]\\])\\s*(?:\\*\\*|__)?\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]*\\))?\\s*:\\s*([^\\n*]+)`, + `(?:#{2,4}|-\\s*\\[[ xX]\\])\\s*(?:\\*\\*|__)?\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]{0,200}\\))?\\s*:\\s*([^\\n*]+)`, 'gi' ); let pm: RegExpExecArray | null; @@ -1747,7 +1747,7 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { try { const milestoneScope = extractCurrentMilestone(roadmapContent, cwd); const cbPattern = new RegExp( - `-\\s*\\[(x| )\\]\\s*(?:\\*\\*|__)?\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]*\\))?\\s*:\\s*([^\\n*]+)`, + `-\\s*\\[(x| )\\]\\s*(?:\\*\\*|__)?\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]{0,200}\\))?\\s*:\\s*([^\\n*]+)`, 'gi' ); let cbm: RegExpExecArray | null; diff --git a/src/roadmap-command-router.cts b/src/roadmap-command-router.cts index 2e23b7318..142c572e8 100644 --- a/src/roadmap-command-router.cts +++ b/src/roadmap-command-router.cts @@ -70,11 +70,11 @@ function checkW021(content: string): W021Warning[] { const MILESTONE_RE = /^#{1,3}\s+(?:\[[^\]]+\]\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]*\))?` 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]*\))?\s*:/i; + // #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; // 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]*\))?\s*:/i; + const UNPREFIXED_PHASE_RE = /^#{2,4}\s*(?:\[[^\]]+\]\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 091789c48..4093f7ede 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -98,8 +98,8 @@ function extractCurrentMilestone(content: string, cwd?: string): string { const preambleCutoff = firstMilestoneMatch ? firstMilestoneMatch.index! : detailsOpenIdx; const preamble = content.slice(0, preambleCutoff) .replace(/
[\s\S]*?<\/details>/gi, '') - // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - .replace(/^#{2,4}\s*Phase\s+[\w][\w.-]*(?:\s*\([^)\n]*\))?\s*:[^\n]*(?:\n(?!#{1,6}\s)[^\n]*)*\n?/gim, '') + // #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, ''); return preamble + content.slice(detailsOpenIdx, detailsEnd); } @@ -179,8 +179,8 @@ function extractCurrentMilestone(content: string, cwd?: string): string { const preamble = beforeMilestones .replace(/
[\s\S]*?<\/details>/gi, '') - // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - .replace(/^#{2,4}\s*Phase\s+[\w][\w.-]*(?:\s*\([^)\n]*\))?\s*:[^\n]*(?:\n(?!#{1,6}\s)[^\n]*)*\n?/gim, '') + // #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, ''); return detailsSection @@ -427,8 +427,8 @@ 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]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phaseHeadingPattern = /^(?:\[[^\]]+\]\s*)?Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]*\))?\s*:/i; + // #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; 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.cts b/src/roadmap.cts index 168dad3cb..4eb408c49 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -297,9 +297,9 @@ function cmdRoadmapAnalyze(cwd: string, raw: boolean): void { const phasesDir = planningPaths(cwd).phases; // Extract all phase headings: ## Phase N: Name or ### Phase N: Name - // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). + // #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]*\))?\s*:\s*([^\n]+)/gi; + const phasePattern = /#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+(\d+[A-Z]?(?:[.-]\d+)*)(?:\s*\([^)\n]{0,200}\))?\s*:\s*([^\n]+)/gi; const phases: Array<{ number: string; name: string; diff --git a/src/state.cts b/src/state.cts index 9bd235342..06b41be4f 100644 --- a/src/state.cts +++ b/src/state.cts @@ -1442,8 +1442,8 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined): Re // truth for total_phases (#549). let roadmapPhaseCount = 0; if (roadmapScope !== null) { - // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phaseHeadingPattern = /#{2,4}\s*Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]*\))?\s*:/gi; + // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). + const phaseHeadingPattern = /#{2,4}\s*Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]{0,200}\))?\s*:/gi; let m: RegExpExecArray | null; while ((m = phaseHeadingPattern.exec(roadmapScope)) !== null) { // Only count tokens that contain at least one digit — excludes @@ -2419,8 +2419,8 @@ function cmdStateSync(cwd: string, options: StateSyncOptions | undefined, raw: b try { let roadmapPhaseCount = 0; if (syncRoadmapScope !== null) { - // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phaseHeadingPattern = /#{2,4}\s*Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]*\))?\s*:/gi; + // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). + const phaseHeadingPattern = /#{2,4}\s*Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]{0,200}\))?\s*:/gi; let m: RegExpExecArray | null; while ((m = phaseHeadingPattern.exec(syncRoadmapScope)) !== null) { // Only count tokens that contain at least one digit — excludes diff --git a/src/validate.cts b/src/validate.cts index 05f96bd73..e0eea4835 100644 --- a/src/validate.cts +++ b/src/validate.cts @@ -113,8 +113,8 @@ export function buildRoadmapPhaseVariants(roadmapContent: string): RoadmapPhaseV const roadmapPhaseVariants = new Set(); // 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]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phasePattern = /#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]*\))?\s*:/gi; + // #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; 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 488598df9..58e2bb18e 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -1081,8 +1081,8 @@ function checkMilestonePrefixMismatches( } for (const section of sections) { const content = roadmapContent.slice(section.start, section.end); - // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phaseRx = /#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]*\))?\s*:/gi; + // #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; let pm: RegExpExecArray | null; while ((pm = phaseRx.exec(content)) !== null) { const phaseId = pm[1]; @@ -1812,8 +1812,8 @@ function cmdValidateHealth( if (isMarkedComplete) { const roadmapRaw = fs.readFileSync(roadmapPath, 'utf-8'); const scopedContent = extractCurrentMilestone(roadmapRaw, cwd); - // #1729: `(?:\s*\([^)\n]*\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]*\\))?\\s*:\\s*([^\\n]+)`, 'gi'); + // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). + const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]{0,200}\\))?\\s*:\\s*([^\\n]+)`, 'gi'); const unstarted: string[] = []; let pm: RegExpExecArray | null; // Non-hoisted: load-order matters (circular dep guard) diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index 82e1cdf45..d945c2d64 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -348,6 +348,21 @@ describe('#1729 regression: parenthetical tag before the colon in a phase header assert.ok(re.test('### Phase 26: X'), 'seam stays optional when no tag is present'); }); + test('#2128: the pre-colon tag is length-bounded so the tag clause cannot ReDoS', () => { + // The tag body `[^)\n]*` was unbounded, making the optional-group + /g scan + // quadratic on adversarial ROADMAP.md/STATE.md (a long run of `(` after a + // header). Bounding it to {0,200} keeps the match linear; a 200-char tag body + // 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'); + // 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. + assert.match(phaseId.OPTIONAL_PHASE_TAG_SOURCE, /\{0,\d+\}/, 'tag body must carry an explicit upper bound'); + }); + test('enumeration (roadmap analyze) lists a pre-colon-tagged phase, not just the resolver', () => { // The resolver (get-phase) and the capture-all enumeration regexes are // separate code paths. Fixing only the resolver left `roadmap analyze` From b321bc04f43417d15d1b39af278fe288779d1016 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 10 Jul 2026 09:44:49 -0400 Subject: [PATCH 06/10] =?UTF-8?q?fix(#2128):=20bound=20the=20sibling=20bra?= =?UTF-8?q?cket-prefix=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 [ From 2f6662d195f7c18f052ba197ecbf28538ee3e483 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 10 Jul 2026 10:11:43 -0400 Subject: [PATCH 07/10] fix(#2128): bound the remaining lazy-scan ReDoS vectors (files_modified, Plans-count, ) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A ReDoS-completeness audit surfaced a distinct class beyond the tag/bracket clause: unbounded `[\s\S]*?` / `[^\]]*` lazy-scans searching for a literal terminator that may never appear, driven quadratic by REPEATED structures in a large PLAN.md/ROADMAP.md. Folded all 7 in at maintainer direction: - files_modified `[^\]]*` -> `[^\]]{0,8000}` (commands.cts, verify.cts): 39.7s -> 0.9s. - Plans-count `[\s\S]*?` -> section-local `(?:(?!\n#{1,4}\s)[\s\S])*?` — stops at the next heading (semantically correct: Plans: belongs to the phase's own section) (roadmap.cts x3, phase.cts): 36s -> 4ms. - extraction `([\s\S]*?)` -> stop at the next same-tag opening `((?:(?!)[\s\S])*?)` (verify.cts x3, markdown-sectionizer.cts): ~6s -> 2ms. Every vector is now linear (comprehensively re-measured); real content matches (end-to-end `roadmap get-phase` still resolves Plans-counted phases). Pre-existing; byte-behavior preserved for realistic inputs. Co-Authored-By: Claude Opus 4.8 --- src/commands.cts | 2 +- src/markdown-sectionizer.cts | 2 +- src/phase.cts | 2 +- src/roadmap.cts | 6 +++--- src/verify.cts | 8 ++++---- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/commands.cts b/src/commands.cts index 93419df94..9ffd6522c 100644 --- a/src/commands.cts +++ b/src/commands.cts @@ -1334,7 +1334,7 @@ function cmdTodoMatchPhase(cwd: string, phase: string | undefined, raw: boolean) for (const pf of planFiles) { const planContent = platformReadSync(path.join(phaseDir, pf)); if (planContent === null) continue; - const fmFiles = planContent.match(/files_modified:\s*\[([^\]]*)\]/); + const fmFiles = planContent.match(/files_modified:\s*\[([^\]]{0,8000})\]/); if (fmFiles) { phasePlans.push(...fmFiles[1].split(',').map(s => s.trim().replace(/['"]/g, '')).filter(Boolean)); } diff --git a/src/markdown-sectionizer.cts b/src/markdown-sectionizer.cts index 0665ff39c..8a745d37a 100644 --- a/src/markdown-sectionizer.cts +++ b/src/markdown-sectionizer.cts @@ -537,7 +537,7 @@ export function extractTaggedBlocks(content: string, tagName: string): string[] // Escape the tag name for safe interpolation into a RegExp. const escapedTag = tagName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - const pattern = new RegExp(`<${escapedTag}>([\\s\\S]*?)`, 'g'); + const pattern = new RegExp(`<${escapedTag}>((?:(?!<${escapedTag}>)[\\s\\S])*?)`, 'g'); const results: string[] = []; let match: RegExpExecArray | null; diff --git a/src/phase.cts b/src/phase.cts index 24c68bf5d..78de4bfae 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -1519,7 +1519,7 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { } const planCountPattern = new RegExp( - `(#{2,4}\\s*Phase\\s+${phaseEscaped}[\\s\\S]*?\\*\\*Plans:\\*\\*\\s*)[^\\n]+`, + `(#{2,4}\\s*Phase\\s+${phaseEscaped}(?:(?!\\n#{1,4}\\s)[\\s\\S])*?\\*\\*Plans:\\*\\*\\s*)[^\\n]+`, 'i', ); roadmapContent = roadmapContent.replace( diff --git a/src/roadmap.cts b/src/roadmap.cts index 7f6febff9..427c44365 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -545,7 +545,7 @@ function cmdRoadmapUpdatePlanProgress(cwd: string, phaseNum: string | null | und // `**Plans:** N plans` — bold "Plans:" (colon inside bold) // `Plans: N plans` — plain text header const planCountPattern = new RegExp( - `(#{2,4}\\s*Phase\\s+${phasePattern}${OPTIONAL_PHASE_TAG_SOURCE}(?=[:\\s])[\\s\\S]*?(?:\\*\\*Plans\\*\\*:|\\*\\*Plans:\\*\\*|(?:^|\\n)Plans:)\\s*)[^\\n]+`, + `(#{2,4}\\s*Phase\\s+${phasePattern}${OPTIONAL_PHASE_TAG_SOURCE}(?=[:\\s])(?:(?!\\n#{1,4}\\s)[\\s\\S])*?(?:\\*\\*Plans\\*\\*:|\\*\\*Plans:\\*\\*|(?:^|\\n)Plans:)\\s*)[^\\n]+`, 'i' ); const planCountText = isComplete @@ -615,11 +615,11 @@ function cmdRoadmapUpdatePlanProgress(cwd: string, phaseNum: string | null | und // Pattern A: anchor to bare `Plans:` header (preferred). // Pattern B: fallback to bold summary when no bare header exists. const insertRowsPatternA = new RegExp( - `(#{2,4}\\s*Phase\\s+${phasePattern}${OPTIONAL_PHASE_TAG_SOURCE}(?=[:\\s])[\\s\\S]*?(?:^|\\n)(?:Plans:)[^\\n]*)`, + `(#{2,4}\\s*Phase\\s+${phasePattern}${OPTIONAL_PHASE_TAG_SOURCE}(?=[:\\s])(?:(?!\\n#{1,4}\\s)[\\s\\S])*?(?:^|\\n)(?:Plans:)[^\\n]*)`, 'i' ); const insertRowsPatternB = new RegExp( - `(#{2,4}\\s*Phase\\s+${phasePattern}${OPTIONAL_PHASE_TAG_SOURCE}(?=[:\\s])[\\s\\S]*?(?:\\*\\*Plans\\*\\*:|\\*\\*Plans:\\*\\*)[^\\n]*)`, + `(#{2,4}\\s*Phase\\s+${phasePattern}${OPTIONAL_PHASE_TAG_SOURCE}(?=[:\\s])(?:(?!\\n#{1,4}\\s)[\\s\\S])*?(?:\\*\\*Plans\\*\\*:|\\*\\*Plans:\\*\\*)[^\\n]*)`, 'i' ); diff --git a/src/verify.cts b/src/verify.cts index 27a71d92e..184344cf8 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -373,20 +373,20 @@ function scanFileWideNegativeGateConflict(content: string): { warnings: string[] const namem = tc.match(/([\s\S]*?)<\/name>/); const name = namem ? namem[1].trim() : 'unnamed'; // Extract entries. - const filesm = tc.match(/([\s\S]*?)<\/files>/); + const filesm = tc.match(/((?:(?!)[\s\S])*?)<\/files>/); const filesText = filesm ? filesm[1] : ''; 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}>([\\s\\S]*?)<\\/${tag}>`, 'g'); + const re = new RegExp(`<${tag}>((?:(?!<${tag}>)[\\s\\S])*?)<\\/${tag}>`, 'g'); let mm: RegExpExecArray | null; while ((mm = re.exec(tc)) !== null) gateFragments.push(mm[1]); } // Requirement text: /. const reqFragments: string[] = []; for (const tag of ['action', 'acceptance_criteria']) { - const re = new RegExp(`<${tag}>([\\s\\S]*?)<\\/${tag}>`, 'g'); + const re = new RegExp(`<${tag}>((?:(?!<${tag}>)[\\s\\S])*?)<\\/${tag}>`, 'g'); let mm: RegExpExecArray | null; while ((mm = re.exec(tc)) !== null) reqFragments.push(mm[1]); } @@ -2094,7 +2094,7 @@ function cmdVerifySchemaDrift( const planFiles = fs.readdirSync(phaseDir).filter((f) => f.endsWith('-PLAN.md')); for (const pf of planFiles) { const content = fs.readFileSync(path.join(phaseDir, pf), 'utf-8'); - const fmMatch = content.match(/files_modified:\s*\[([^\]]*)\]/); + const fmMatch = content.match(/files_modified:\s*\[([^\]]{0,8000})\]/); if (fmMatch) { const files = fmMatch[1].split(',').map((f) => f.trim()).filter(Boolean); allFiles.push(...files); From 01b691fae8db27d90a14c0dc5273c39a9b8c9a00 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 10 Jul 2026 10:42:18 -0400 Subject: [PATCH 08/10] 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 `); From 8e4ebb49e466db5e1ccbcbc9eef1a4e557073485 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 10 Jul 2026 11:12:45 -0400 Subject: [PATCH 09/10] fix(#2128): address shared-seam review regressions (#557, action-scan, comment-strip) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Final convergence review found the shared seam introduced 3 behavior regressions; fixed all + locked with tests: - #557 REGRESSION: stripTaggedBlocks's attribute-tolerance stripped `
` (the ACTIVE-milestone marker) that the old `
`-only regex preserved. The seam now takes `allowAttributes` (default false) — details/decisions strip is attr-INTOLERANT (preserves `
`); only `` opts in. Regression test added to roadmap-parser + markdown-sectionizer suites. - verify.cts actionZones (negative-grep-echo security scan): reverted to a bounded to-first-close scan `([\s\S]{0,20000}?)` so a grep-echo trick can't hide behind an unterminated inner (the seam's stop-at-next-open would drop it). ReDoS-safe via the cap. - check-command-router HTML-comment strip: `(?:-->|$)` fallback wiped to EOF (fail-closed spurious gate block) — replaced with stop-at-next-open so an unclosed `|$)/g, ' '); + // Stop-at-next-open body (ReDoS-safe, #2128); an UNCLOSED `|$)` fallback, which would + // wipe to EOF and fail-close the decision-coverage gate). + 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 c152f8acc..c9b0727c6 100644 --- a/src/markdown-sectionizer.cts +++ b/src/markdown-sectionizer.cts @@ -520,22 +520,25 @@ export function iterateBullets(sectionText: string): BulletItem[] { * fenced code blocks itself. If a `` block appears inside a fenced code * block and should be excluded, the caller should apply `stripFencedCode` first. * - * **Nested tags are NOT supported.** The underlying regex uses a non-greedy - * `[\s\S]*?` match, which means it closes at the FIRST `` encountered. - * Given `inner`, `extractTaggedBlocks(content, 'x')` returns - * `['inner']` — the inner `` is captured as literal text, and the second - * `` is left unmatched (or matched as a second block with empty inner text - * if another `` follows). Callers that need to handle nested tags must - * pre-process the input or use a proper XML/HTML parser. + * **Nested tags are NOT supported.** The body scan terminates at the NEXT + * opening of the same tag (the ReDoS-safe boundary, #2128). Given + * `inner`, `extractTaggedBlocks(content, 'x')` returns `['inner']` + * — the well-formed inner block; the unterminated outer `` is skipped. + * Callers that need true nesting must use a proper XML/HTML parser. + * + * `allowAttributes` (default `false`): when `true`, the opening tag may carry + * bounded attributes (``) — needed for `` blocks. + * Leave `false` for tags that must match exactly (e.g. ``), and never + * enable it for a tag where an attributed form is semantically distinct. * * Generalises `decisions.cts`'s bespoke `matchAll(/([\s\S]*?)<\/decisions>/g)` * so tier T1 can drop its own copy (tracked duplication until T1 lands). */ -export function extractTaggedBlocks(content: string, tagName: string): string[] { +export function extractTaggedBlocks(content: string, tagName: string, allowAttributes = false): string[] { if (typeof content !== 'string' || content.length === 0) return []; if (typeof tagName !== 'string' || tagName.length === 0) return []; - const pattern = taggedBlockPattern(tagName, 'g'); + const pattern = taggedBlockPattern(tagName, 'g', allowAttributes); const results: string[] = []; let match: RegExpExecArray | null; while ((match = pattern.exec(content)) !== null) { @@ -548,30 +551,37 @@ export function extractTaggedBlocks(content: string, tagName: string): string[] * 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. + * Safety: the body terminates at the NEXT opening of this tag (stop-at-next-open) + * instead of lazily rescanning the whole remaining document for a `` that + * may never appear — so a document full of unclosed `` openings scans + * LINEARLY, not quadratically (#2128). Group 1 is the block body. + * + * `allowAttributes`: when `true`, the opener accepts bounded attributes + * (``) and the body boundary is ``. + * When `false`, the opener is the EXACT `` and the boundary is exact ``, + * so an attributed `` is neither an opener nor a boundary — it is body + * content. That exact form is load-bearing for `
` stripping: `
` marks the ACTIVE milestone and must be preserved, not stripped (#557). */ -function taggedBlockPattern(tagName: string, flags: string): RegExp { +function taggedBlockPattern(tagName: string, flags: string, allowAttributes: boolean): RegExp { const esc = tagName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - return new RegExp(`<${esc}(?:\\s[^>]{0,1000})?>((?:(?!<${esc}[\\s>])[\\s\\S])*?)`, flags); + const open = allowAttributes ? `<${esc}(?:\\s[^>]{0,1000})?>` : `<${esc}>`; + const boundary = allowAttributes ? `<${esc}[\\s>]` : `<${esc}>`; + return new RegExp(`${open}((?:(?!${boundary})[\\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. + * hardened pattern, `.replace(…, '')` instead of body extraction. `allowAttributes` + * defaults to `false` so `
` (active milestone) is preserved (#557); + * 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 { +export function stripTaggedBlocks(content: string, tagName: string, allowAttributes = false, 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'), ''); + return content.replace(taggedBlockPattern(tagName, caseSensitive ? 'g' : 'gi', allowAttributes), ''); } // ─── replaceSection ─────────────────────────────────────────────────────────── diff --git a/src/verify.cts b/src/verify.cts index 8aa3a4778..4695fd5aa 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -206,7 +206,15 @@ 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 = extractTaggedBlocks(text, 'action'); + // Security scan: must see the FULL text up to the first — including a + // malformed inner — so a grep-echo-0 trick cannot hide behind a + // deliberately-unclosed tag. Use a bounded to-first-close scan (ReDoS-safe via + // the {0,20000} cap, #2128), NOT the stop-at-next-open extractTaggedBlocks seam + // (which would drop the span before an unterminated inner ). + const actionZones: string[] = []; + const actionRe = /([\s\S]{0,20000}?)<\/action>/g; + let acm: RegExpExecArray | null; + while ((acm = actionRe.exec(text)) !== null) actionZones.push(acm[1]); const scannableActionText = actionZones.map((zone) => zone.replace(cmdSpanRe, ' ')).join('\n'); // 3. Per shell SEGMENT (split lines on && / ||) extract count-grep literals and @@ -363,7 +371,7 @@ function scanFileWideNegativeGateConflict(content: string): { warnings: string[] reqText: string; // + text (requirement side) } const tasks: TaskInfo[] = []; - for (const tc of extractTaggedBlocks(text, 'task')) { + for (const tc of extractTaggedBlocks(text, 'task', true)) { // Extract task name. const namem = extractTaggedBlocks(tc, 'name'); const name = namem.length ? namem[0].trim() : 'unnamed'; @@ -565,7 +573,7 @@ function cmdVerifyPlanStructure(cwd: string, filePath: string, raw: boolean): vo } const tasks: Record[] = []; - for (const taskContent of extractTaggedBlocks(content, 'task')) { + for (const taskContent of extractTaggedBlocks(content, 'task', true)) { const nameArr = extractTaggedBlocks(taskContent, 'name'); const taskName = nameArr.length ? nameArr[0].trim() : 'unnamed'; const hasFiles = //.test(taskContent); diff --git a/tests/markdown-sectionizer.test.cjs b/tests/markdown-sectionizer.test.cjs index ac934f1f7..655b839b3 100644 --- a/tests/markdown-sectionizer.test.cjs +++ b/tests/markdown-sectionizer.test.cjs @@ -33,6 +33,7 @@ const { collectSection, iterateBullets, extractTaggedBlocks, + stripTaggedBlocks, replaceSection, } = require('../gsd-core/bin/lib/markdown-sectionizer.cjs'); @@ -1015,15 +1016,14 @@ describe('stripFencedCode and tokenizeHeadings: backtick info string with backti // ─── FIX 6: extractTaggedBlocks — nested tag behavior ───────────────────────── -describe('extractTaggedBlocks: nested same-name tag behavior (non-greedy limitation)', () => { - test('nested … closes at first (non-greedy; nested tags not supported)', () => { - // Non-greedy match: ([\s\S]*?) closes at the FIRST . - // So inner → first block captures "inner", second is unmatched. +describe('extractTaggedBlocks: nested same-name tag behavior (#2128 stop-at-next-open)', () => { + test('nested inner extracts the well-formed inner block', () => { + // #2128: the ReDoS-safe body scan terminates at the NEXT opening , so the + // unterminated outer is skipped and the inner block is extracted. const content = 'inner'; const result = extractTaggedBlocks(content, 'x'); - // The first match closes at the first , capturing "inner" - assert.equal(result.length, 1, 'non-greedy match produces exactly one result from nested input'); - assert.equal(result[0], 'inner', 'inner capture is the content up to the first closing tag'); + assert.equal(result.length, 1, 'exactly one result from nested input'); + assert.equal(result[0], 'inner', 'the well-formed inner block is extracted; the unterminated outer is skipped'); }); test('back-to-back blocks (not nested) are both extracted', () => { @@ -1033,6 +1033,24 @@ describe('extractTaggedBlocks: nested same-name tag behavior (non-greedy limitat assert.equal(result[0], 'first'); assert.equal(result[1], 'second'); }); + + test('#2128: a document full of unclosed openings stays linear and yields no match', () => { + const content = 'a\n'.repeat(50) + 'no closing tag'; + assert.deepEqual(extractTaggedBlocks(content, 'x'), [], 'no anywhere -> no blocks'); + }); + + test('#557 / #2128: attr-intolerant by default preserves
; opt-in matches ', () => { + // stripTaggedBlocks(details) must PRESERVE
(the active-milestone + // marker) and strip only bare
; extractTaggedBlocks(task, true) must + // match attributed tasks, and must NOT when allowAttributes is left false. + assert.equal( + stripTaggedBlocks('X
shipped
Y
active
Z', 'details'), + 'XY
active
Z', + '#557:
preserved; bare
stripped', + ); + assert.deepEqual(extractTaggedBlocks('body', 'task', true), ['body'], 'attributed task matched with allowAttributes=true'); + assert.deepEqual(extractTaggedBlocks('body', 'task'), [], 'attributed task NOT matched with allowAttributes=false'); + }); }); // Parity guard removed in T5 (ADR-1372): uat-predicate now imports stripFencedCode diff --git a/tests/roadmap-parser.test.cjs b/tests/roadmap-parser.test.cjs index f86b8fcc4..ef6789899 100644 --- a/tests/roadmap-parser.test.cjs +++ b/tests/roadmap-parser.test.cjs @@ -77,6 +77,19 @@ describe('roadmap-parser: stripShippedMilestones', () => { assert.ok(!result.includes('closed content'), 'content removed'); assert.ok(result.includes('after'), 'after content preserved'); }); + + test('#557: preserves an active
block while stripping shipped bare
', () => { + //
marks the ACTIVE milestone (roadmap.analyze must still see its + // phases); only closed/shipped bare
blocks are stripped. Regression for + // #557, which the #2128 shared-seam migration briefly reintroduced via the seam's + // attribute-tolerance — the details strip is now attr-INTOLERANT to keep #557 fixed. + const input = '
\nshipped phase\n
\n
\n- [ ] **Phase 9: Active**\n
\nafter'; + const result = stripShippedMilestones(input); + assert.ok(!result.includes('shipped phase'), 'shipped bare
stripped'); + assert.ok(result.includes('
'), 'active
tag preserved'); + assert.ok(result.includes('Phase 9: Active'), 'active-milestone phases preserved'); + assert.ok(result.includes('after'), 'trailing content preserved'); + }); }); // ─── extractCurrentMilestone ────────────────────────────────────────────────── From 27f73cb43f42b5f7bb99eb92ef073287e9c9fd59 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 10 Jul 2026 11:42:32 -0400 Subject: [PATCH 10/10] docs(changeset): backfill PR number (#2141) Co-Authored-By: Claude Opus 4.8 --- .changeset/phase-id-redos-hardening.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/phase-id-redos-hardening.md b/.changeset/phase-id-redos-hardening.md index 816a004a6..df9878105 100644 --- a/.changeset/phase-id-redos-hardening.md +++ b/.changeset/phase-id-redos-hardening.md @@ -1,5 +1,5 @@ --- type: Security -pr: 0 +pr: 2141 --- **Hardened phase/roadmap/plan markdown parsing against quadratic-time (ReDoS) CPU exhaustion** — a crafted `ROADMAP.md`, `STATE.md`, or `PLAN.md` with large runs of unclosed `(`, `[`, ``, `