* feat(#3415): ship local/no-unbounded-quantifier, burn down ReDoS class Phase 4 of epic #3212 (ADR-3212 §5/§7, the final phase). New rule flags an unbounded */+/{n,} quantifier over a broad character class ([\s\S], dotAll ., or a 1-2-unit negated class like [^\n]/[^)\n] — the exact #2128-fixed shape) applied to a regex whose match target is data-flow-traced to readFileSync content. eslint-rules/lib/readfilesync-trace.cjs extracts the data-flow tracer shared with no-crlf-fragile-split (Phase 2) rather than a second copy — no-crlf-fragile-split refactored onto it with zero behavior change, parity-tested. Real triage, not 798 mechanical edits: the ADR's census (2026-08-08) screened every unbounded quantifier in the tree unscoped. Correctly scoped to readFileSync-derived content (matching Phase 2's own G2/G3 scoping), the rule found 162 real hits across two detection waves — the second wave (93) surfaced only after a genuine off-by-one bug in this rule's own first draft was caught while writing its RuleTester tests and fixed (the bug silently missed every directly-quantified [\s\S]* with no gap before the quantifier — exactly the class this rule exists to catch). 3 hits landed in production src/ (commands.cts, milestone.cts, roadmap.cts) and were each empirically timed against adversarial input (matching #2128's own measured-not-assumed precedent) — all confirmed linear-time/benign, left unbounded with a measured-evidence comment rather than mechanically bounded. The remaining 159 are test-file fixture parsing (test-author-controlled, fixed-size content, not adversarial input) — each suppressed with a specific, non-generic reason. Zero functional behavior changed anywhere in this diff. tests/no-pending-3212-markers.test.cjs locks the epic's own closing invariant (ADR §7: "assert zero pending #3212 markers remain") — ground truth confirmed trivially true today (no phase left any such marker behind), now regression-locked going forward. Design: .gsd/phase/chore-3415-prohibition-with-teeth/40-design.md Test matrix: .gsd/phase/chore-3415-prohibition-with-teeth/50-test-matrix.md Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#3415): correct rule category mislabel, add CI test-scope entry An orthogonal Standards-axis review found eslint-rules/no-unbounded-quantifier.cjs mistakenly carried meta.docs.category: 'Portability', copied from a sibling rule without realizing what that implied: docs/contributing/cross-platform- portability-rules.md governs an ADR-1703 rule family under a hard "zero escape hatches" contract (tests/portability-rule-disable-ban.test.cjs's PROTECTED_RULES bans eslint-disable for those rules entirely). This rule is not part of that family — it's ADR-3212 (ReDoS/CWE-1333), a different epic — and its eslint-disable-next-line suppressions (159 of them, added earlier this same phase after empirical benign-verification) are an intentional, correct design, not a bypass. Corrected to category: 'Best Practices', matching the actual precedent (no-adhoc-regex-escape.cjs, Phase 1 of the same epic, which is also correctly outside PROTECTED_RULES), and the rule's own docstring now states this explicitly so a future reader doesn't have to re-derive it. Also registers a new scripts/ci-test-scope.cjs bucket so editing this rule or the shared eslint-rules/lib/readfilesync-trace.cjs helper re-runs their own test suites under targeted CI selection — was previously unregistered and invisible to that fast-path (this PR's own gsd-test checkpoint runs the full suite regardless, so this only affects future narrowly-scoped PRs). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#3415): bound no-unbounded-quantifier's own scanner (CWE-1333, ironic) Security review found the rule meant to catch algorithmic-complexity bugs had one of its own: hasUnboundedBroadQuantifier's negated-class inner scan walked from each `[^` occurrence to the next `]` (or EOF) with no bound, while the outer loop only ever advanced by one character — O(n²) total work on a pattern with many unclosed `[^` runs. Runs unconditionally inside checkPattern on any `new RegExp('literal string')` argument in any linted file, before the (cheap) readFileSync data-flow gate — so a single crafted string literal, no valid regex syntax required, could make `npm run lint` / CI hang. Empirically confirmed both the bug and the fix: pre-fix, n=4000/8000/ 16000/32000 chars took 30.8/115.6/463.8/1874.3ms (~4x work per 2x n, quadratic); extrapolated, the 300000-char repro from the finding would run ~165s. Post-fix (bail the inner scan once units exceeds the rule's own 1-2-unit scope, rather than continuing to hunt for a closing `]`), the same 300000-char input runs in 8.7ms via the real rule module, independently reconfirmed at 18ms via a fresh Linter.verify() call. New regression row in tests/no-unbounded-quantifier.rule.test.cjs asserts the RuleTester run on a 50000-char adversarial pattern completes and returns a defined result — no wall-clock assertion (CLAUDE.md Clock Seams / local/no-elapsed-assertion). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#3415): triage 3 new sites, re-raise ceiling after upstream batch next merged 12 more PRs during this PR's review. Two consequences: - tests/edit-phase.test.cjs (fix #3262, unrelated) added 3 new content.match(/<tag>([\s\S]*?)<\/tag>/) reads of this repo's own workflow .md content — the same Class A pattern as the ~159 sites already triaged elsewhere in this PR. Suppressed with the same established reason. - lint-allow-test-rule-refs' ratchet ceiling needed re-raising again (301 -> 303) for the same reason as the two prior bumps: organic growth from unrelated, already-reviewed PRs landing concurrently, not a defect in this branch's own diff. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
229 lines
11 KiB
JavaScript
229 lines
11 KiB
JavaScript
/**
|
|
* gsd-executor agent — MVP+TDD gate section contract
|
|
* Verifies the agent definition contains a section instructing the executor
|
|
* to halt and report when the runtime gate trips.
|
|
*/
|
|
const { test, describe } = require('node:test');
|
|
const assert = require('node:assert/strict');
|
|
const fs = require('fs');
|
|
const path = require('path');
|
|
|
|
const AGENT = path.join(__dirname, '..', 'agents', 'gsd-executor.md');
|
|
const REF = path.join(__dirname, '..', 'gsd-core', 'references', 'execute-mvp-tdd.md');
|
|
|
|
describe('gsd-executor — MVP+TDD gate section', () => {
|
|
const content = fs.readFileSync(AGENT, 'utf-8');
|
|
|
|
test('agent defines an MVP+TDD Gate section', () => {
|
|
assert.match(content, /MVP\+TDD\s*Gate|MVP[\s-]?TDD[\s-]?gate/i, 'must label the gate');
|
|
});
|
|
|
|
test('agent instructs halt-and-report when gate trips', () => {
|
|
assert.match(content, /halt|stop[^\n]*gate|gate[^\n]*halt/i, 'must instruct halt');
|
|
assert.match(content, /report|surface|emit/i, 'must instruct report');
|
|
});
|
|
|
|
test('agent references execute-mvp-tdd.md', () => {
|
|
assert.match(content, /execute-mvp-tdd\.md/, 'must reference the gate semantics file');
|
|
});
|
|
|
|
test('referenced file exists on disk', () => {
|
|
assert.ok(fs.existsSync(REF), `${REF} must exist`);
|
|
});
|
|
});
|
|
|
|
describe('gsd-executor — state.* calls use the named-only router form (#1863 regression)', () => {
|
|
// The runtime state-command router (gsd-core/bin/lib/state-command-router.cjs)
|
|
// parses record-metric / add-decision / add-blocker / record-session named-only
|
|
// via parseNamedArgs. Positional values are silently dropped, so state.cjs then
|
|
// throws its required-arg error and metrics/decisions/blockers/session continuity
|
|
// are never recorded. Each invocation in the executor agent must therefore pass
|
|
// the named flags the router expects (mirrors gsd-core/workflows/execute-plan.md).
|
|
const content = fs.readFileSync(AGENT, 'utf-8');
|
|
|
|
// Capture a `gsd_run query state.<cmd> ...` invocation, including backslash-continued lines.
|
|
function invocation(cmd) {
|
|
const re = new RegExp(String.raw`gsd_run query state\.${cmd}\b(?:[^\r\n]*\\\r?\n)*[^\r\n]*`);
|
|
const m = content.match(re);
|
|
assert.ok(m, `executor must invoke state.${cmd}`);
|
|
return m[0];
|
|
}
|
|
|
|
test('record-metric passes --phase/--plan/--duration/--tasks/--files', () => {
|
|
const call = invocation('record-metric');
|
|
for (const flag of ['--phase', '--plan', '--duration', '--tasks', '--files']) {
|
|
assert.ok(call.includes(flag), `record-metric must pass ${flag}, got:\n${call}`);
|
|
}
|
|
});
|
|
|
|
test('add-decision passes --summary (or --summary-file)', () => {
|
|
assert.match(invocation('add-decision'), /--summary(?:-file)?\b/);
|
|
});
|
|
|
|
test('add-blocker passes --text (or --text-file)', () => {
|
|
assert.match(invocation('add-blocker'), /--text(?:-file)?\b/);
|
|
});
|
|
|
|
test('record-session passes --stopped-at and --resume-file', () => {
|
|
const call = invocation('record-session');
|
|
assert.ok(call.includes('--stopped-at'), 'record-session must pass --stopped-at');
|
|
assert.ok(call.includes('--resume-file'), 'record-session must pass --resume-file');
|
|
});
|
|
|
|
test('no state.* call leads with a bare positional (quoted) value — the #1863 bug', () => {
|
|
// Buggy multi-line form: `state.<cmd> \` then a line whose first token is a quote.
|
|
const continued = /state\.(?:record-metric|add-decision|add-blocker|record-session)\b[^\r\n]*\\\r?\n\s*"/;
|
|
assert.ok(!continued.test(content),
|
|
'state.* calls must lead with --flags, not a positional quoted value on the next line');
|
|
// Buggy same-line form: `state.<cmd> "..."`
|
|
const inline = /state\.(?:record-metric|add-decision|add-blocker|record-session)\s+"/;
|
|
assert.ok(!inline.test(content),
|
|
'state.* calls must not pass a positional value immediately after the command');
|
|
});
|
|
|
|
test('sibling workflow record-session calls also use named flags (#1863 completeness)', () => {
|
|
// The same named-only router backs milestone-summary.md and forensics.md; both
|
|
// previously passed record-session positionally (`"" "stopped-at" "resume-file"`),
|
|
// silently dropping the values. Guard them alongside the executor.
|
|
for (const rel of ['gsd-core/workflows/milestone-summary.md', 'gsd-core/workflows/forensics.md']) {
|
|
const wf = fs.readFileSync(path.join(__dirname, '..', rel), 'utf-8');
|
|
// eslint-disable-next-line local/no-unbounded-quantifier -- parses maintainer-authored workflow markdown, bounded prose, not adversarial input
|
|
const m = wf.match(/gsd_run query state\.record-session\b(?:[^\r\n]*\\\r?\n)*[^\r\n]*/);
|
|
assert.ok(m, `${rel} must invoke state.record-session`);
|
|
assert.ok(m[0].includes('--stopped-at') && m[0].includes('--resume-file'),
|
|
`${rel} record-session must use --stopped-at/--resume-file, got:\n${m[0]}`);
|
|
assert.ok(!/state\.record-session\s+"/.test(wf),
|
|
`${rel} record-session must not lead with a positional value`);
|
|
}
|
|
});
|
|
});
|
|
|
|
|
|
// ────────────────────────────────────────────────────────────────────────
|
|
// Folded from tests/bug-3097-3099-executor-worktree-path-safety.test.cjs — consolidation epic #1969 (B7 #1976)
|
|
// ────────────────────────────────────────────────────────────────────────
|
|
{
|
|
const { describe: __foldDescribe } = require('node:test');
|
|
__foldDescribe("folded:bug-3097-3099-executor-worktree-path-safety (consolidation epic #1969 B7 #1976)", () => {
|
|
'use strict';
|
|
// allow-test-rule: source-text-is-the-product (see #3097)
|
|
// Reads markdown product files (gsd-executor.md, worktree-path-safety.md) to
|
|
// verify structural protocol.
|
|
|
|
// Regression guards for bug #3097 and #3099.
|
|
//
|
|
// #3097: gsd-executor's worktree HEAD guard used `if [ -f .git ]` to detect
|
|
// worktree mode. After a Bash `cd` out of the worktree into the main repo,
|
|
// `.git` is a DIRECTORY (not a file), so the test is false and the entire
|
|
// HEAD safety block is silently skipped. Commits then land on whatever branch
|
|
// the main repo has checked out — not the per-agent worktree branch.
|
|
//
|
|
// #3099: Executor agents construct absolute paths from `pwd` captured in the
|
|
// orchestrator context (main repo root). Edit/Write calls using these paths
|
|
// resolve to the main repo, not the worktree. git commit from the worktree
|
|
// sees a clean tree; the work is silently lost or leaks to main.
|
|
|
|
const { describe, test } = 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 executorSrc = fs.readFileSync(
|
|
path.join(ROOT, 'agents', 'gsd-executor.md'), 'utf8',
|
|
);
|
|
const executePhaseSrc = fs.readFileSync(
|
|
path.join(ROOT, 'gsd-core', 'workflows', 'execute-phase.md'), 'utf8',
|
|
);
|
|
|
|
describe('bug #3097: cwd-drift sentinel in gsd-executor.md', () => {
|
|
test('task_commit_protocol has cwd-drift assertion step (0a)', () => {
|
|
const protocolIdx = executorSrc.indexOf('<task_commit_protocol>');
|
|
const protocolEnd = executorSrc.indexOf('</task_commit_protocol>');
|
|
assert.ok(protocolIdx !== -1 && protocolEnd !== -1, 'task_commit_protocol block not found');
|
|
const protocol = executorSrc.slice(protocolIdx, protocolEnd);
|
|
assert.ok(
|
|
protocol.includes('cwd') || protocol.includes('drift') || protocol.includes('gsd-spawn-toplevel'),
|
|
'task_commit_protocol missing cwd-drift assertion step — #3097 fix not applied',
|
|
);
|
|
});
|
|
|
|
test('sentinel uses git rev-parse --git-dir to detect worktree', () => {
|
|
const protocolIdx = executorSrc.indexOf('<task_commit_protocol>');
|
|
const protocolEnd = executorSrc.indexOf('</task_commit_protocol>');
|
|
const protocol = executorSrc.slice(protocolIdx, protocolEnd);
|
|
assert.ok(
|
|
protocol.includes('rev-parse --git-dir') || protocol.includes('worktrees/'),
|
|
'cwd-drift detection does not use git rev-parse --git-dir or .git/worktrees/ pattern',
|
|
);
|
|
});
|
|
|
|
test('cwd-drift check precedes HEAD assertion', () => {
|
|
const protocolIdx = executorSrc.indexOf('<task_commit_protocol>');
|
|
const protocolEnd = executorSrc.indexOf('</task_commit_protocol>');
|
|
const protocol = executorSrc.slice(protocolIdx, protocolEnd);
|
|
const driftIdx = protocol.search(/cwd.drift|gsd-spawn-toplevel|drift.*assertion/i);
|
|
const headIdx = protocol.indexOf('Pre-commit HEAD safety assertion');
|
|
assert.ok(driftIdx !== -1, 'cwd-drift assertion not found');
|
|
assert.ok(headIdx !== -1, 'HEAD assertion not found');
|
|
assert.ok(driftIdx < headIdx, 'cwd-drift assertion must precede HEAD assertion (step 0a before step 0)');
|
|
});
|
|
});
|
|
|
|
describe('bug #3099: absolute-path safety guidance in gsd-executor.md', () => {
|
|
test('task_commit_protocol documents absolute-path safety', () => {
|
|
const protocolIdx = executorSrc.indexOf('<task_commit_protocol>');
|
|
const protocolEnd = executorSrc.indexOf('</task_commit_protocol>');
|
|
const protocol = executorSrc.slice(protocolIdx, protocolEnd);
|
|
assert.ok(
|
|
(protocol.includes('absolute') || protocol.includes('absolute-path')) &&
|
|
(protocol.includes('worktree') || protocol.includes('WT_ROOT')),
|
|
'task_commit_protocol missing absolute-path safety guidance — #3099 fix not applied',
|
|
);
|
|
});
|
|
|
|
test('execute-phase.md parallel_execution block references path safety', () => {
|
|
const parallelIdx = executePhaseSrc.indexOf('<parallel_execution>');
|
|
assert.ok(parallelIdx !== -1, 'parallel_execution block not found in execute-phase.md');
|
|
// Verify the worktree-path-safety.md reference is present in the execution_context
|
|
// (loaded via @ reference rather than inlined — the safe extract pattern)
|
|
assert.ok(
|
|
executePhaseSrc.includes('worktree-path-safety.md'),
|
|
'execute-phase.md does not reference worktree-path-safety.md in execution_context',
|
|
);
|
|
});
|
|
|
|
test('execute-phase prompt anchors subagent file paths to project_root before files_to_read (#280)', () => {
|
|
const filesIdx = executePhaseSrc.indexOf('<files_to_read>');
|
|
assert.ok(filesIdx !== -1, 'files_to_read block not found in execute-phase.md');
|
|
const dispatchSnippet = executePhaseSrc.slice(filesIdx, filesIdx + 1800);
|
|
assert.ok(
|
|
dispatchSnippet.includes('PROJECT_ROOT=$(git rev-parse --show-toplevel'),
|
|
'executor dispatch must compute PROJECT_ROOT in the prompt before file reads',
|
|
);
|
|
assert.ok(
|
|
dispatchSnippet.includes('${PROJECT_ROOT}/'),
|
|
'executor files_to_read paths must be anchored to ${PROJECT_ROOT}/',
|
|
);
|
|
});
|
|
|
|
test('worktree-path-safety.md reference file exists', () => {
|
|
assert.ok(
|
|
fs.existsSync(path.join(ROOT, 'gsd-core', 'references', 'worktree-path-safety.md')),
|
|
'gsd-core/references/worktree-path-safety.md does not exist',
|
|
);
|
|
});
|
|
|
|
test('worktree-path-safety.md contains cwd-drift and absolute-path guards', () => {
|
|
const safetySrc = fs.readFileSync(
|
|
path.join(ROOT, 'gsd-core', 'references', 'worktree-path-safety.md'), 'utf8',
|
|
);
|
|
assert.ok(safetySrc.includes('gsd-spawn-toplevel') || safetySrc.includes('cwd-drift'),
|
|
'worktree-path-safety.md missing cwd-drift sentinel content');
|
|
assert.ok(safetySrc.includes('WT_ROOT') || safetySrc.includes('absolute'),
|
|
'worktree-path-safety.md missing absolute-path guard content');
|
|
});
|
|
});
|
|
});
|
|
}
|