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

fix(#2128): route <tag> scans through a hardened shared seam (root-cause ReDoS fix)

Browse Source 
A convergence audit showed the `<tag>[\s\S]*?</tag>` 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 `<details>` strips -> stripTaggedBlocks (behavior-identical —
  no <details> here carries attributes).
- verify: actionZones + both <task> loops + their nested <name>/<files>/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 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-07-10 10:42:18 -04:00
parent 2f6662d195
commit 01b691fae8
4 changed files with 50 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/check-command-router.cts
View File

@@ -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(/<!--[\s\S]*?-->/g, ' ');
const htmlStripped = text.replace(/<!--[\s\S]*?(?:-->|$)/g, ' ');
// Fenced-code stripping: delegate to the canonical CommonMark-correct seam.
// replaces the prior independent regex copy (```` ``` ``` ```` + `~~~ ~~~`).
return stripFencedCode(htmlStripped).text;

35
src/markdown-sectionizer.cts
View File

@@ -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])*?)</${escapedTag}>`, '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 `<tag>…</tag>` block regex shared by
* `extractTaggedBlocks` (extract bodies) and `stripTaggedBlocks` (remove blocks).
*
* Safety: the body uses a `(?:(?!<tag[\s>])[\s\S])*?` negative-lookahead scan
* that terminates at the NEXT opening `<tag>` instead of lazily rescanning the
* whole remaining document for a `</tag>` that may never appear — so a large
* document full of unclosed `<tag>` openings stays LINEAR, not quadratic
* (#2128). The opening tag tolerates optional attributes (`<tag foo="bar">`),
* 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])*?)</${esc}>`, flags);
}
/**
* Remove every `<tagName>…</tagName>` 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 `<details>` 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 ───────────────────────────────────────────────────────────
/**

10
src/roadmap-parser.cts
View File

@@ -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 <details> blocks.
*/
function stripShippedMilestones(content: string): string {
return content.replace(/<details>[\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+|✅|📋|🚧|🔄)|<details/im;
const firstMilestoneMatch = content.match(anyMilestoneOrDetails);
const preambleCutoff = firstMilestoneMatch ? firstMilestoneMatch.index! : detailsOpenIdx;
const preamble = content.slice(0, preambleCutoff)
.replace(/<details>[\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(/<details>[\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, '');

42
src/verify.cts
View File

@@ -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 = /<action>([\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; // <verify>+<automated>+<acceptance_criteria> text
reqText: string; // <action>+<acceptance_criteria> text (requirement side)
}
const taskRe = /<task[^>]*>([\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(/<name>([\s\S]*?)<\/name>/);
const name = namem ? namem[1].trim() : 'unnamed';
const namem = extractTaggedBlocks(tc, 'name');
const name = namem.length ? namem[0].trim() : 'unnamed';
// Extract <files> entries.
const filesm = tc.match(/<files>((?:(?!<files>)[\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: <verify>/<automated>/<acceptance_criteria>.
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: <action>/<acceptance_criteria>.
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. <automated>cmd</automated> nested inside <verify>)
// 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 = /<task[^>]*>([\s\S]*?)<\/task>/g;
const tasks: Record<string, unknown>[] = [];
let taskMatch: RegExpExecArray | null;
while ((taskMatch = taskPattern.exec(content)) !== null) {
const taskContent = taskMatch[1];
const nameMatch = taskContent.match(/<name>([\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 = /<files>/.test(taskContent);
const hasAction = /<action>/.test(taskContent);
const hasVerify = /<verify>/.test(taskContent);
const hasDone = /<done>/.test(taskContent);
if (!nameMatch) errors.push('Task missing <name> element');
if (nameArr.length === 0) errors.push('Task missing <name> element');
if (!hasAction) errors.push(`Task '${taskName}' missing <action>`);
if (!hasVerify) warnings.push(`Task '${taskName}' missing <verify>`);
if (!hasDone) warnings.push(`Task '${taskName}' missing <done>`);