* test(#4568): pin the N-segment phase-grammar defect across all 6 shell/markdown sites Manually traced against the current tree: the validating regex at code-review.md rejects a 3-segment id (23.1.2), and execute-plan.md's extraction truncates a 23.1.2-01-PLAN.md filename down to 1.2-01. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#4568): widen the phase-number regex to accept N-segment ids at all 6 shell/markdown sites Widens `?` to `*` on the dotted-segment group at all 6 sites (byte-identical behavior for 1- and 2-segment ids, character class unchanged): code-review.md, code-review-fix.md, gsd-code-fixer.md, gsd-code-fixer.compact.md (validating sites, plus their comment/error-message text), execute-plan.md's plan-filename extraction, and plan-phase.md's --research-phase flag capture. Also disambiguates the nsegment-phase-grammar test's plan-phase.md anchor, which was matching an unrelated earlier `--research-phase` occurrence (line 77's generic-value capture) instead of the targeted site (line 131). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore(#4634): extend lint-phase-id-drift to ban the single-segment phase regex in workflows/ and agents/ Adds findSingleSegmentPhaseRegexDrift, banning the bounded `[0-9]+(\.[0-9]+)?` shape (and its \d/doubled-backslash near-variants) on any phase-carrying line across gsd-core/workflows/**/*.md, gsd-core/references/**/*.md, and the newly-scanned agents/**/*.md, sanctioned the same way as the existing shell-arith rule. Wired into scanAll; confirmed zero violations against the real tree post-#4568 fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(#4568): add Fixed changeset Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore: regenerate conformance-tier manifests for the new test file The emitted-attribution gate also flags 4 files growing: code-review-fix.md (+21 bytes), code-review.md (+21 bytes), gsd-code-fixer.compact.md (+9 bytes), gsd-code-fixer.md (+6 bytes). The growth is the fix itself: each site's validation regex widened from a bounded single-optional-dotted-segment shape to the unbounded form, and the accompanying comment/error-message text grew by a few characters to mention the new 3-segment example. Emitted-Drift-Ack-Growth: code-review-fix.md — widens the phase-number validation regex from a bounded single-dotted-segment shape to accept N-segment ids, and adds a 3-segment example to the comment/error text (#4568) Emitted-Drift-Ack-Growth: code-review.md — widens the phase-number validation regex from a bounded single-dotted-segment shape to accept N-segment ids, and adds a 3-segment example to the comment/error text (#4568) Emitted-Drift-Ack-Growth: gsd-code-fixer.compact.md — widens the padded_phase validation regex from a bounded single-dotted-segment shape to accept N-segment ids, and adds a 3-segment example to the error text (#4568) Emitted-Drift-Ack-Growth: gsd-code-fixer.md — widens the padded_phase validation regex from a bounded single-dotted-segment shape to accept N-segment ids, and adds a 3-segment example to the comment/error text (#4568) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore(#4568): backfill changeset pr number to 4646 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
5
.changeset/graceful-cats-hop.md
Normal file
5
.changeset/graceful-cats-hop.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 4646
|
||||
---
|
||||
**A three-segment (or deeper) phase id no longer breaks phase-number validation or extraction** — code-review, code-review-fix, the gsd-code-fixer agent (both variants), execute-plan's plan-filename parsing, and plan-phase's --research-phase flag all re-derived a two-segment-max regex; a nested phase like 23.1.2 was rejected outright or silently truncated to the wrong id. All six sites now accept an arbitrary number of dotted segments, matching the canonical grammar. (#4568)
|
||||
@@ -136,10 +136,10 @@ branch=$(git branch --show-current)
|
||||
test -n "$branch" || { echo "Detached HEAD is not supported for review-fix (#2686)"; exit 1; }
|
||||
|
||||
# padded_phase is interpolated into a worktree PATH and a git BRANCH NAME —
|
||||
# validate at this sink too (defense in depth): digits + optional single
|
||||
# dotted numeric suffix only (e.g. '02' or '36.14'); reject '../', spaces, shell metachars.
|
||||
if ! [[ "$padded_phase" =~ ^[0-9]+(\.[0-9]+)?$ ]]; then
|
||||
echo "Invalid padded_phase for review-fix: '$padded_phase' (expected e.g. '02' or '36.14')"; exit 1
|
||||
# validate at this sink too (defense in depth): digits + one or more dotted
|
||||
# numeric segments only (e.g. '02' or '36.14'); reject '../', spaces, shell metachars.
|
||||
if ! [[ "$padded_phase" =~ ^[0-9]+(\.[0-9]+)*$ ]]; then
|
||||
echo "Invalid padded_phase for review-fix: '$padded_phase' (expected e.g. '02', '36.14', or '23.1.2')"; exit 1
|
||||
fi
|
||||
|
||||
# Recovery-sentinel: ${phase_dir}/.review-fix-recovery-pending.json existing means
|
||||
|
||||
@@ -254,13 +254,13 @@ test -n "$branch" || { echo "Detached HEAD is not supported for review-fix (#268
|
||||
|
||||
# #2647 defense-in-depth: padded_phase is interpolated into a worktree PATH
|
||||
# and a git BRANCH NAME below. The orchestrator (code-review-fix.md) already
|
||||
# validates it as ^[0-9]+(\.[0-9]+)?$, but this agent prompt is a literal bash
|
||||
# validates it as ^[0-9]+(\.[0-9]+)*$, but this agent prompt is a literal bash
|
||||
# contract any caller can spawn — validate at the SINK too, so a future caller
|
||||
# that forgets cannot turn ${padded_phase} into a path-traversal or branch-name
|
||||
# injection. Reject anything that is not digits + an optional single dotted
|
||||
# numeric suffix (e.g. '02' or '36.14'); reject '../', spaces, shell metachars.
|
||||
if ! [[ "$padded_phase" =~ ^[0-9]+(\.[0-9]+)?$ ]]; then
|
||||
echo "Invalid padded_phase for review-fix: '$padded_phase' (expected e.g. '02' or '36.14')"; exit 1
|
||||
# injection. Reject anything that is not digits + one or more dotted numeric
|
||||
# segments (e.g. '02' or '36.14'); reject '../', spaces, shell metachars.
|
||||
if ! [[ "$padded_phase" =~ ^[0-9]+(\.[0-9]+)*$ ]]; then
|
||||
echo "Invalid padded_phase for review-fix: '$padded_phase' (expected e.g. '02', '36.14', or '23.1.2')"; exit 1
|
||||
fi
|
||||
|
||||
# Recovery-sentinel handling (#2839):
|
||||
|
||||
@@ -35,9 +35,9 @@ Parse from init JSON: `phase_found`, `phase_dir`, `phase_number`, `phase_name`,
|
||||
|
||||
**Input sanitization (defense-in-depth):**
|
||||
```bash
|
||||
# Validate PADDED_PHASE contains only digits and optional dot (e.g., "02", "03.1")
|
||||
if ! [[ "$PADDED_PHASE" =~ ^[0-9]+(\.[0-9]+)?$ ]]; then
|
||||
echo "Error: Invalid phase number format: '${PADDED_PHASE}'. Expected digits (e.g., 02, 03.1)."
|
||||
# Validate PADDED_PHASE contains only digits and dotted segments (e.g., "02", "03.1", "23.1.2")
|
||||
if ! [[ "$PADDED_PHASE" =~ ^[0-9]+(\.[0-9]+)*$ ]]; then
|
||||
echo "Error: Invalid phase number format: '${PADDED_PHASE}'. Expected digits (e.g., 02, 03.1, 23.1.2)."
|
||||
# Exit workflow
|
||||
fi
|
||||
```
|
||||
|
||||
@@ -59,9 +59,9 @@ Parse from init JSON: `phase_found`, `phase_dir`, `phase_number`, `phase_name`,
|
||||
|
||||
**Input sanitization (defense-in-depth):**
|
||||
```bash
|
||||
# Validate PADDED_PHASE contains only digits and optional dot (e.g., "02", "03.1")
|
||||
if ! [[ "$PADDED_PHASE" =~ ^[0-9]+(\.[0-9]+)?$ ]]; then
|
||||
echo "Error: Invalid phase number format: '${PADDED_PHASE}'. Expected digits (e.g., 02, 03.1)."
|
||||
# Validate PADDED_PHASE contains only digits and dotted segments (e.g., "02", "03.1", "23.1.2")
|
||||
if ! [[ "$PADDED_PHASE" =~ ^[0-9]+(\.[0-9]+)*$ ]]; then
|
||||
echo "Error: Invalid phase number format: '${PADDED_PHASE}'. Expected digits (e.g., 02, 03.1, 23.1.2)."
|
||||
# Exit workflow
|
||||
fi
|
||||
```
|
||||
|
||||
@@ -67,7 +67,7 @@ Find first PLAN without matching SUMMARY. Decimal phases supported (`01.1-hotfix
|
||||
**Exclude `external_job_waiting` plans from selection.** When choosing the first PLAN that lacks a matching SUMMARY, skip any plan whose `plan_id` matches an async-job manifest in `.planning/async-jobs/` (any status) — that plan is `external_job_waiting` or awaiting reconciliation, never work to (re-)dispatch (re-dispatching would duplicate the external job). Reconcile via the manifest / safe_resume_gate instead.
|
||||
|
||||
```bash
|
||||
PHASE=$(echo "$PLAN_PATH" | grep -oE '[0-9]+(\.[0-9]+)?-[0-9]+')
|
||||
PHASE=$(echo "$PLAN_PATH" | grep -oE '[0-9]+(\.[0-9]+)*-[0-9]+')
|
||||
# config settings can be fetched via gsd_run query config-get if needed
|
||||
```
|
||||
|
||||
|
||||
@@ -128,7 +128,7 @@ In research-only mode, two modifiers control behavior when `RESEARCH.md` already
|
||||
```bash
|
||||
RESEARCH_ONLY=false
|
||||
VIEW_ONLY=false
|
||||
if [[ "$ARGUMENTS" =~ --research-phase[[:space:]]+([0-9]+(\.[0-9]+)?) ]]; then
|
||||
if [[ "$ARGUMENTS" =~ --research-phase[[:space:]]+([0-9]+(\.[0-9]+)*) ]]; then
|
||||
RESEARCH_ONLY=true
|
||||
PHASE="${BASH_REMATCH[1]}"
|
||||
fi
|
||||
|
||||
@@ -165,6 +165,7 @@ module.exports = {
|
||||
"tests/no-unguarded-nonportable-exec.rule.test.cjs",
|
||||
"tests/npm-audit-baseline.test.cjs",
|
||||
"tests/npm-integrity-gate.test.cjs",
|
||||
"tests/nsegment-phase-grammar.test.cjs",
|
||||
"tests/onboard-command.test.cjs",
|
||||
"tests/opencode-command-dir-plural.test.cjs",
|
||||
"tests/opencode-plugin-adapter.test.cjs",
|
||||
|
||||
@@ -294,6 +294,12 @@ function findShellPhaseArithDrift(text) {
|
||||
// #4634: the markdown scan roots — shell embedded in workflow/reference docs.
|
||||
const MD_SCAN_DIRS = [path.join('gsd-core', 'workflows'), path.join('gsd-core', 'references')];
|
||||
|
||||
// #4568 (epic #4634): the single-segment phase regex ban scans a THIRD root,
|
||||
// `agents/**/*.md`, that the #4619 shell-arithmetic extension above never
|
||||
// touched — the gsd-code-fixer agent prompts re-derive the phase-number
|
||||
// grammar too. Reuses the same `walkMd` walker as the shell-arith scan.
|
||||
const SINGLE_SEGMENT_SCAN_DIRS = [...MD_SCAN_DIRS, 'agents'];
|
||||
|
||||
/**
|
||||
* Scan `gsd-core/workflows/**\/*.md` and `gsd-core/references/**\/*.md` for
|
||||
* unsanctioned `$((10#...))` shell arithmetic. Returns [{ file, line, found }]
|
||||
@@ -318,6 +324,71 @@ function scanMarkdownShellArith(root) {
|
||||
return violations;
|
||||
}
|
||||
|
||||
// #4568 (epic #4634): ban the single-optional-dotted-segment phase regex
|
||||
// shape `[0-9]+(\.[0-9]+)?` (and its `\d`/doubled-backslash near-variants)
|
||||
// outright — this is exactly the grammar that hard-rejects or silently
|
||||
// truncates a 3-or-more-segment phase id like `23.1.2`. The canonical
|
||||
// grammar (`src/phase-id.cts`) uses the unbounded `(?:\.\d+)*` form; shell
|
||||
// snippets embedded in markdown can't import that module, so textual parity
|
||||
// (`*` in place of `?`) is the fix, and this rule is the ratchet against a
|
||||
// future site re-deriving the bounded form. Deliberately narrow to the
|
||||
// bounded ONE-optional-segment shape — the fixed `*`-form is not flagged.
|
||||
const SINGLE_SEGMENT_PHASE_DRIFT_RE =
|
||||
/(?:\\{1,2}d|\[0-9\])\+\(\\{1,2}\.(?:\\{1,2}d|\[0-9\])\+\)\?/;
|
||||
|
||||
// A single-segment shape like `[0-9]+(\.[0-9]+)?` is not inherently
|
||||
// phase-specific (e.g. it could describe a version number), so the rule
|
||||
// only fires on a line whose text plausibly carries a phase-number
|
||||
// variable — a case-insensitive `phase` substring anywhere on the line,
|
||||
// mirroring the phase-carrying filter `findShellPhaseArithDrift` already
|
||||
// applies to its own variable-name capture.
|
||||
const PHASE_CARRYING_LINE_RE = /phase/i;
|
||||
|
||||
/**
|
||||
* Pure: find every unsanctioned single-optional-dotted-segment phase regex
|
||||
* in `text`, restricted to lines that plausibly carry a phase-number
|
||||
* variable. Sanctioned by an HTML comment `<!-- phase-id-owner: ... -->` on
|
||||
* the nearest preceding non-blank line (same convention as the shell-arith
|
||||
* rule). Returns [{ line, found }].
|
||||
*/
|
||||
function findSingleSegmentPhaseRegexDrift(text) {
|
||||
const out = [];
|
||||
const lines = text.split('\n');
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
const line = lines[i];
|
||||
const m = SINGLE_SEGMENT_PHASE_DRIFT_RE.exec(line);
|
||||
if (!m) continue;
|
||||
if (!PHASE_CARRYING_LINE_RE.test(line)) continue;
|
||||
if (isSanctionedByPrecedingComment(lines, i, MD_OWNER_RE)) continue;
|
||||
out.push({ line: i + 1, found: m[0] });
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* Scan `gsd-core/workflows/**\/*.md`, `gsd-core/references/**\/*.md`, and
|
||||
* `agents/**\/*.md` for unsanctioned single-optional-dotted-segment phase
|
||||
* regexes. Returns [{ file, line, found }] with repo-relative paths.
|
||||
*/
|
||||
function scanMarkdownSingleSegmentPhaseRegex(root) {
|
||||
const violations = [];
|
||||
for (const dir of SINGLE_SEGMENT_SCAN_DIRS) {
|
||||
for (const file of walkMd(path.join(root, dir), [])) {
|
||||
const rel = path.relative(root, file);
|
||||
let text;
|
||||
try {
|
||||
text = fs.readFileSync(file, 'utf8');
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
for (const d of findSingleSegmentPhaseRegexDrift(text)) {
|
||||
violations.push({ file: rel, kind: 'single-segment-phase-regex', ...d });
|
||||
}
|
||||
}
|
||||
}
|
||||
return violations;
|
||||
}
|
||||
|
||||
// Authored TypeScript source only (the generated bin/lib/*.cjs mirror it).
|
||||
const SCAN_DIRS = ['src'];
|
||||
const SCAN_EXT = new Set(['.cts', '.ts', '.mts']);
|
||||
@@ -466,7 +537,11 @@ function scanRepo(root) {
|
||||
* pinned-clean `scanRepo` test spuriously fail.
|
||||
*/
|
||||
function scanAll(root) {
|
||||
return [...scanRepo(root), ...scanMarkdownShellArith(root)];
|
||||
return [
|
||||
...scanRepo(root),
|
||||
...scanMarkdownShellArith(root),
|
||||
...scanMarkdownSingleSegmentPhaseRegex(root),
|
||||
];
|
||||
}
|
||||
|
||||
function main() {
|
||||
@@ -488,6 +563,10 @@ function main() {
|
||||
process.stderr.write('`$((10#...))` base-10-forced shell arithmetic is banned outright in\n');
|
||||
process.stderr.write('gsd-core/workflows/**/*.md and gsd-core/references/**/*.md — sanction with\n');
|
||||
process.stderr.write('`<!-- phase-id-owner: <reason> -->` on the line directly above.\n');
|
||||
process.stderr.write('The single-optional-dotted-segment phase regex `[0-9]+(\\.[0-9]+)?` (or its \\d\n');
|
||||
process.stderr.write('near-variant) is banned outright in gsd-core/workflows/**/*.md,\n');
|
||||
process.stderr.write('gsd-core/references/**/*.md, and agents/**/*.md — widen it to `*` (unbounded\n');
|
||||
process.stderr.write('segments) or sanction with `<!-- phase-id-owner: <reason> -->`.\n');
|
||||
process.stderr.write('A `.replace(\'{slug}\', ... || \'phase\')` fallback is banned outright (#4126) —\n');
|
||||
process.stderr.write('use `renderPhaseBranchName(` or sanction with\n');
|
||||
process.stderr.write('`// phase-id-owner: <reason>` on the line directly above:\n');
|
||||
@@ -505,7 +584,9 @@ module.exports = {
|
||||
findNameValidityDrift,
|
||||
findBranchSlugFallbackDrift,
|
||||
findShellPhaseArithDrift,
|
||||
findSingleSegmentPhaseRegexDrift,
|
||||
scanMarkdownShellArith,
|
||||
scanMarkdownSingleSegmentPhaseRegex,
|
||||
scanRepo,
|
||||
scanAll,
|
||||
countSelectorBaselines,
|
||||
@@ -515,4 +596,5 @@ module.exports = {
|
||||
NAME_VALIDITY_DRIFT_RE,
|
||||
BRANCH_SLUG_FALLBACK_DRIFT_RE,
|
||||
SHELL_PHASE_ARITH_DRIFT_RE,
|
||||
SINGLE_SEGMENT_PHASE_DRIFT_RE,
|
||||
};
|
||||
|
||||
@@ -2,13 +2,18 @@
|
||||
|
||||
const test = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const path = require('node:path');
|
||||
|
||||
const {
|
||||
findNameValidityDrift,
|
||||
findBranchSlugFallbackDrift,
|
||||
findShellPhaseArithDrift,
|
||||
findSingleSegmentPhaseRegexDrift,
|
||||
scanMarkdownSingleSegmentPhaseRegex,
|
||||
} = require('../scripts/lint-phase-id-drift.cjs');
|
||||
|
||||
const ROOT = path.join(__dirname, '..');
|
||||
|
||||
test('findNameValidityDrift flags a regex-literal re-derivation of the name-validity class', () => {
|
||||
const text = [
|
||||
'function isNameable(s) {',
|
||||
@@ -111,3 +116,46 @@ test('findShellPhaseArithDrift skips a full-line comment merely mentioning the p
|
||||
const text = '# Note: $((10#$PHASE_NUMBER)) is a hard shell syntax error on a decimal id.';
|
||||
assert.deepEqual(findShellPhaseArithDrift(text), []);
|
||||
});
|
||||
|
||||
// #4568 (epic #4634): the single-optional-dotted-segment phase regex ban.
|
||||
test('findSingleSegmentPhaseRegexDrift flags the bounded [0-9]+(\\.[0-9]+)? shape on a phase-carrying line', () => {
|
||||
const text = 'if ! [[ "$PADDED_PHASE" =~ ^[0-9]+(\\.[0-9]+)?$ ]]; then';
|
||||
const found = findSingleSegmentPhaseRegexDrift(text);
|
||||
assert.equal(found.length, 1);
|
||||
assert.equal(found[0].line, 1);
|
||||
});
|
||||
|
||||
test('findSingleSegmentPhaseRegexDrift flags the \\d near-variant on a phase-carrying line', () => {
|
||||
const text = 'if ! [[ "$padded_phase" =~ ^\\d+(\\.\\d+)?$ ]]; then';
|
||||
const found = findSingleSegmentPhaseRegexDrift(text);
|
||||
assert.equal(found.length, 1);
|
||||
});
|
||||
|
||||
test('findSingleSegmentPhaseRegexDrift flags the doubled-backslash template-string form', () => {
|
||||
const text = "const re = new RegExp('^\\\\d+(\\\\.\\\\d+)?$'); // phase check";
|
||||
const found = findSingleSegmentPhaseRegexDrift(text);
|
||||
assert.equal(found.length, 1);
|
||||
});
|
||||
|
||||
test('findSingleSegmentPhaseRegexDrift is SILENT on the fixed unbounded (*) form', () => {
|
||||
const text = 'if ! [[ "$PADDED_PHASE" =~ ^[0-9]+(\\.[0-9]+)*$ ]]; then';
|
||||
assert.deepEqual(findSingleSegmentPhaseRegexDrift(text), []);
|
||||
});
|
||||
|
||||
test('findSingleSegmentPhaseRegexDrift does NOT flag a non-phase-carrying line (e.g. a version number)', () => {
|
||||
const text = 'if ! [[ "$VERSION" =~ ^[0-9]+(\\.[0-9]+)?$ ]]; then';
|
||||
assert.deepEqual(findSingleSegmentPhaseRegexDrift(text), []);
|
||||
});
|
||||
|
||||
test('findSingleSegmentPhaseRegexDrift does NOT flag a site sanctioned with an HTML comment', () => {
|
||||
const text = [
|
||||
'<!-- phase-id-owner: deliberate, tracked in #4634 -->',
|
||||
'if ! [[ "$PADDED_PHASE" =~ ^[0-9]+(\\.[0-9]+)?$ ]]; then',
|
||||
].join('\n');
|
||||
assert.deepEqual(findSingleSegmentPhaseRegexDrift(text), []);
|
||||
});
|
||||
|
||||
test('scanMarkdownSingleSegmentPhaseRegex against the real repo tree reports zero violations (#4568 fixed)', () => {
|
||||
const violations = scanMarkdownSingleSegmentPhaseRegex(ROOT);
|
||||
assert.deepEqual(violations, []);
|
||||
});
|
||||
|
||||
179
tests/nsegment-phase-grammar.test.cjs
Normal file
179
tests/nsegment-phase-grammar.test.cjs
Normal file
@@ -0,0 +1,179 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* #4568 (epic #4634) — six shell snippets embedded in workflow/agent markdown
|
||||
* validate or extract phase numbers with the regex shape `[0-9]+(\.[0-9]+)?`
|
||||
* (or its `\d` near-variant) — an optional SINGLE dotted segment. Any
|
||||
* three-or-more-segment phase id (e.g. `23.1.2`, produced by a nested `phase
|
||||
* insert`) is either hard-rejected or silently truncated to the wrong value.
|
||||
* The canonical grammar in src/phase-id.cts already uses the unbounded form
|
||||
* (`\d+(?:\.\d+)*`) — shell cannot import that module, so the fix is textual
|
||||
* parity: widen `?` to `*` at each site.
|
||||
*
|
||||
* These tests are BEHAVIORAL: for each site, the actual regex/extraction
|
||||
* line is read live off disk (via a narrow, anchored string search) and
|
||||
* executed in a real bash subprocess — never hand-retyped — so the test
|
||||
* breaks loudly if a future edit changes a site's shape instead of silently
|
||||
* drifting from the real file.
|
||||
*/
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const { execFileSync } = require('node:child_process');
|
||||
|
||||
const TIMEOUT = 5000;
|
||||
|
||||
const CODE_REVIEW = path.join(__dirname, '..', 'gsd-core', 'workflows', 'code-review.md');
|
||||
const CODE_REVIEW_FIX = path.join(__dirname, '..', 'gsd-core', 'workflows', 'code-review-fix.md');
|
||||
const CODE_FIXER = path.join(__dirname, '..', 'agents', 'gsd-code-fixer.md');
|
||||
const CODE_FIXER_COMPACT = path.join(__dirname, '..', 'agents', 'gsd-code-fixer.compact.md');
|
||||
const EXECUTE_PLAN = path.join(__dirname, '..', 'gsd-core', 'workflows', 'execute-plan.md');
|
||||
const PLAN_PHASE = path.join(__dirname, '..', 'gsd-core', 'workflows', 'plan-phase.md');
|
||||
|
||||
/**
|
||||
* Pure: find the line containing `anchor` and pull the regex substring
|
||||
* between `=~ ` and ` ]]` on it. Throws loudly if either the anchor or the
|
||||
* pattern shape is not found, so a future rewrite of the site's surrounding
|
||||
* code breaks this test instead of silently testing stale text.
|
||||
*/
|
||||
function extractAnchoredRegex(fileText, anchor) {
|
||||
const lines = fileText.split('\n');
|
||||
const line = lines.find((l) => l.includes(anchor));
|
||||
assert.ok(line, `anchor not found: ${anchor}`);
|
||||
const m = line.match(/=~\s+(\S+)\s+\]\]/);
|
||||
assert.ok(m, `no "=~ <pattern> ]]" shape found on anchor line: ${line}`);
|
||||
return m[1];
|
||||
}
|
||||
|
||||
/**
|
||||
* Pure: find the line containing `anchor` and pull the single-quoted
|
||||
* `grep -oE '...'` pattern off it.
|
||||
*/
|
||||
function extractGrepPattern(fileText, anchor) {
|
||||
const lines = fileText.split('\n');
|
||||
const line = lines.find((l) => l.includes(anchor));
|
||||
assert.ok(line, `anchor not found: ${anchor}`);
|
||||
const m = line.match(/grep -oE '([^']+)'/);
|
||||
assert.ok(m, `no grep -oE '...' shape found on anchor line: ${line}`);
|
||||
return m[1];
|
||||
}
|
||||
|
||||
/** Run a validating-site regex (bash `[[ =~ ]]`) against `value`, returning true/false. */
|
||||
function matchesValidatingRegex(pattern, value) {
|
||||
const script = `if [[ "$TEST_INPUT" =~ ${pattern} ]]; then echo MATCH; else echo NOMATCH; fi`;
|
||||
const out = execFileSync('bash', [], {
|
||||
input: script,
|
||||
encoding: 'utf8',
|
||||
timeout: TIMEOUT,
|
||||
env: { ...process.env, TEST_INPUT: value },
|
||||
}).trim();
|
||||
return out === 'MATCH';
|
||||
}
|
||||
|
||||
describe('#4568 — validating sites accept N-segment phase ids and still reject injection', () => {
|
||||
const sites = [
|
||||
{ name: 'code-review.md', file: CODE_REVIEW, anchor: 'if ! [[ "$PADDED_PHASE" =~ ' },
|
||||
{ name: 'code-review-fix.md', file: CODE_REVIEW_FIX, anchor: 'if ! [[ "$PADDED_PHASE" =~ ' },
|
||||
{ name: 'gsd-code-fixer.md', file: CODE_FIXER, anchor: 'if ! [[ "$padded_phase" =~ ' },
|
||||
{ name: 'gsd-code-fixer.compact.md', file: CODE_FIXER_COMPACT, anchor: 'if ! [[ "$padded_phase" =~ ' },
|
||||
];
|
||||
|
||||
for (const site of sites) {
|
||||
describe(site.name, () => {
|
||||
const text = fs.readFileSync(site.file, 'utf8');
|
||||
const pattern = extractAnchoredRegex(text, site.anchor);
|
||||
|
||||
test('regression control: 1-segment id (6) matches', () => {
|
||||
assert.equal(matchesValidatingRegex(pattern, '6'), true);
|
||||
});
|
||||
|
||||
test('regression control: 2-segment id (36.14) matches', () => {
|
||||
assert.equal(matchesValidatingRegex(pattern, '36.14'), true);
|
||||
});
|
||||
|
||||
test('N-segment id (23.1.2) matches (fails before the fix)', () => {
|
||||
assert.equal(matchesValidatingRegex(pattern, '23.1.2'), true);
|
||||
});
|
||||
|
||||
test('path-traversal injection (../1) is rejected', () => {
|
||||
assert.equal(matchesValidatingRegex(pattern, '../1'), false);
|
||||
});
|
||||
|
||||
test('shell-metacharacter injection (1; rm -rf /) is rejected', () => {
|
||||
assert.equal(matchesValidatingRegex(pattern, '1; rm -rf /'), false);
|
||||
});
|
||||
|
||||
test('empty string is rejected', () => {
|
||||
assert.equal(matchesValidatingRegex(pattern, ''), false);
|
||||
});
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe('#4568 — execute-plan.md extracts the full N-segment phase from a plan filename', () => {
|
||||
const text = fs.readFileSync(EXECUTE_PLAN, 'utf8');
|
||||
const pattern = extractGrepPattern(text, 'grep -oE');
|
||||
|
||||
function extractPhase(planPath) {
|
||||
const script = `echo "$PLAN_PATH" | grep -oE '${pattern}'`;
|
||||
let out;
|
||||
try {
|
||||
out = execFileSync('bash', [], {
|
||||
input: script,
|
||||
encoding: 'utf8',
|
||||
timeout: TIMEOUT,
|
||||
env: { ...process.env, PLAN_PATH: planPath },
|
||||
}).trim();
|
||||
} catch {
|
||||
out = '';
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
test('regression control: 1-segment plan filename extracts correctly', () => {
|
||||
assert.equal(extractPhase('/x/06-01-PLAN.md'), '06-01');
|
||||
});
|
||||
|
||||
test('regression control: 2-segment plan filename extracts correctly', () => {
|
||||
assert.equal(extractPhase('/x/36.14-01-PLAN.md'), '36.14-01');
|
||||
});
|
||||
|
||||
test('N-segment plan filename extracts the FULL phase, not a truncated one (fails before the fix)', () => {
|
||||
assert.equal(extractPhase('/x/23.1.2-01-PLAN.md'), '23.1.2-01');
|
||||
});
|
||||
});
|
||||
|
||||
describe('#4568 — plan-phase.md captures the full N-segment --research-phase value', () => {
|
||||
const text = fs.readFileSync(PLAN_PHASE, 'utf8');
|
||||
const pattern = extractAnchoredRegex(text, '=~ --research-phase[[:space:]]+(');
|
||||
|
||||
function captureResearchPhase(args) {
|
||||
const script = [
|
||||
'if [[ "$ARGUMENTS" =~ ' + pattern + ' ]]; then',
|
||||
' echo "${BASH_REMATCH[1]}"',
|
||||
'else',
|
||||
' echo NOMATCH',
|
||||
'fi',
|
||||
].join('\n');
|
||||
return execFileSync('bash', [], {
|
||||
input: script,
|
||||
encoding: 'utf8',
|
||||
timeout: TIMEOUT,
|
||||
env: { ...process.env, ARGUMENTS: args },
|
||||
}).trim();
|
||||
}
|
||||
|
||||
test('regression control: 1-segment --research-phase captures correctly', () => {
|
||||
assert.equal(captureResearchPhase('--research-phase 6'), '6');
|
||||
});
|
||||
|
||||
test('regression control: 2-segment --research-phase captures correctly', () => {
|
||||
assert.equal(captureResearchPhase('--research-phase 36.14'), '36.14');
|
||||
});
|
||||
|
||||
test('N-segment --research-phase captures the FULL value, not a truncated one (fails before the fix)', () => {
|
||||
assert.equal(captureResearchPhase('--research-phase 23.1.2'), '23.1.2');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user