fix(coderabbit): resolve all 12 findings on PR #3152
MAJOR (security/correctness): - commands/gsd/debug.md: add Write to allowed-tools (session file creation requires it — workflow explicitly says 'use Write tool, never heredoc') - workflows/debug.md: add SLUG sanitization guard to steps 1b+1c (status/ continue subcommands used raw user input in file paths — path traversal) - workflows/thread.md: sanitize $ARGUMENTS in RESUME mode before file path construction (was bypassing the sanitization guard in CLOSE/STATUS modes) MINOR (consistency/correctness): - docs/INVENTORY-MANIFEST.json: remove stale top-level 'workflows' array (duplicate of families.workflows introduced in earlier update) - commands/gsd/resume-work.md: normalize process to 'Execute end-to-end.' - commands/gsd/settings.md: normalize process to 'Execute end-to-end.' - commands/gsd/update.md: normalize otherwise branch to 'execute end-to-end.' - docs/adr/0002: add Status: Accepted + Date header (ADR convention) - workflows/extract-learnings.md: rename step extract_learnings → extract-learnings - tests/extract-learnings.test.cjs: tighten step-name assertion to exact name ARCHITECTURE: - scripts/command-contract-helpers.cjs: extract CANONICAL_TOOLS, parseFrontmatter, executionContextRefs as shared module — single source of truth consumed by both lint script and test suite (prevents silent lint/test disagreement) - scripts/lint-command-contract.cjs: require() helpers instead of duplicating - tests/command-contract.test.cjs: require() helpers; move readFileSync calls inside test() callbacks (registration-time throws surface as named failures)
This commit is contained in:
Tom Boucher
61
scripts/command-contract-helpers.cjs
Normal file
61
scripts/command-contract-helpers.cjs
Normal file
@@ -0,0 +1,61 @@
|
||||
'use strict';
|
||||
/**
|
||||
* command-contract-helpers.cjs (ADR-0002)
|
||||
*
|
||||
* Single source of truth for the commands/gsd/*.md contract constants and
|
||||
* parsers shared by scripts/lint-command-contract.cjs and
|
||||
* tests/command-contract.test.cjs.
|
||||
*
|
||||
* Keeping these in one place ensures the lint script and the test suite
|
||||
* always agree on what constitutes a valid tool, a valid @-ref, and a valid
|
||||
* frontmatter structure. A new canonical tool added here is automatically
|
||||
* enforced by both consumers.
|
||||
*/
|
||||
|
||||
const CANONICAL_TOOLS = new Set([
|
||||
'Read', 'Write', 'Edit', 'Bash', 'Glob', 'Grep',
|
||||
'Task', 'Agent', 'Skill', 'SlashCommand',
|
||||
'AskUserQuestion', 'WebFetch', 'WebSearch', 'TodoWrite',
|
||||
'mcp__context7__resolve-library-id',
|
||||
'mcp__context7__query-docs',
|
||||
'mcp__context7__*',
|
||||
]);
|
||||
|
||||
function parseFrontmatter(content) {
|
||||
const lines = content.split('\n');
|
||||
if (lines[0].trim() !== '---') return {};
|
||||
const end = lines.indexOf('---', 1);
|
||||
if (end === -1) return {};
|
||||
const fm = {};
|
||||
let key = null;
|
||||
for (const line of lines.slice(1, end)) {
|
||||
const kv = line.match(/^([a-zA-Z0-9_-]+):\s*(.*)/);
|
||||
if (kv) { key = kv[1]; fm[key] = kv[2].trim(); }
|
||||
else if (key && line.match(/^\s+-\s+/)) {
|
||||
const val = line.replace(/^\s+-\s+/, '').trim();
|
||||
fm[key] = fm[key] ? fm[key] + '\n' + val : val;
|
||||
}
|
||||
}
|
||||
return fm;
|
||||
}
|
||||
|
||||
function executionContextRefs(content) {
|
||||
const refs = [];
|
||||
const re = /<execution_context(?:_extended)?>([\s\S]*?)<\/execution_context(?:_extended)?>/g;
|
||||
let m;
|
||||
while ((m = re.exec(content)) !== null) {
|
||||
for (const rawLine of m[1].split('\n')) {
|
||||
const line = rawLine.trim();
|
||||
if (!line.startsWith('@')) continue;
|
||||
const token = line.split(/\s+/)[0];
|
||||
const trailingProse = line.length > token.length;
|
||||
const normalized = token
|
||||
.replace(/^@(?:~|\$HOME)\//, '')
|
||||
.replace(/^(?:\.claude\/)?(?:get-shit-done\/)?/, '');
|
||||
refs.push({ token, normalized, trailingProse });
|
||||
}
|
||||
}
|
||||
return refs;
|
||||
}
|
||||
|
||||
module.exports = { CANONICAL_TOOLS, parseFrontmatter, executionContextRefs };
|
||||
@@ -22,58 +22,11 @@ const ROOT = path.join(__dirname, '..');
|
||||
const COMMANDS_DIR = path.join(ROOT, 'commands', 'gsd');
|
||||
const GSD_ROOT = path.join(ROOT, 'get-shit-done');
|
||||
|
||||
// All tool names the Claude Code / GSD runtime recognises.
|
||||
// Wildcard entries (mcp__context7__*) match any mcp__context7__ prefixed name.
|
||||
const CANONICAL_TOOLS = new Set([
|
||||
'Read', 'Write', 'Edit', 'Bash', 'Glob', 'Grep',
|
||||
'Task', 'Agent', 'Skill', 'SlashCommand',
|
||||
'AskUserQuestion', 'WebFetch', 'WebSearch', 'TodoWrite',
|
||||
'mcp__context7__resolve-library-id',
|
||||
'mcp__context7__query-docs',
|
||||
'mcp__context7__*',
|
||||
]);
|
||||
|
||||
// ─── parsers ─────────────────────────────────────────────────────────────────
|
||||
|
||||
function parseFrontmatter(content) {
|
||||
const lines = content.split('\n');
|
||||
if (lines[0].trim() !== '---') return {};
|
||||
const end = lines.indexOf('---', 1);
|
||||
if (end === -1) return {};
|
||||
const fm = {};
|
||||
let key = null;
|
||||
for (const line of lines.slice(1, end)) {
|
||||
const kv = line.match(/^([a-zA-Z0-9_-]+):\s*(.*)/);
|
||||
if (kv) { key = kv[1]; fm[key] = kv[2].trim(); }
|
||||
else if (key && line.match(/^\s+-\s+/)) {
|
||||
const val = line.replace(/^\s+-\s+/, '').trim();
|
||||
fm[key] = fm[key] ? fm[key] + '\n' + val : val;
|
||||
}
|
||||
}
|
||||
return fm;
|
||||
}
|
||||
|
||||
function extractExecutionContextRefs(content) {
|
||||
const results = [];
|
||||
const blockRe = /<execution_context(?:_extended)?>([\s\S]*?)<\/execution_context(?:_extended)?>/g;
|
||||
let m;
|
||||
while ((m = blockRe.exec(content)) !== null) {
|
||||
const block = m[1];
|
||||
for (const rawLine of block.split('\n')) {
|
||||
const line = rawLine.trim();
|
||||
if (!line.startsWith('@')) continue;
|
||||
// Capture the @-reference token (stops at first space)
|
||||
const refToken = line.split(/\s+/)[0];
|
||||
const hasTrailingProse = line.length > refToken.length;
|
||||
// Normalise path: strip @~/.../get-shit-done/ or @$HOME/.../get-shit-done/ prefix
|
||||
const normalized = refToken
|
||||
.replace(/^@(?:~|\$HOME)\//, '')
|
||||
.replace(/^(?:\.claude\/)?(?:get-shit-done\/)?/, '');
|
||||
results.push({ ref: refToken, normalized, hasTrailingProse, rawLine });
|
||||
}
|
||||
}
|
||||
return results;
|
||||
}
|
||||
const {
|
||||
CANONICAL_TOOLS,
|
||||
parseFrontmatter,
|
||||
executionContextRefs: extractExecutionContextRefs,
|
||||
} = require('./command-contract-helpers.cjs');
|
||||
|
||||
// ─── check one file ───────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
@@ -30,11 +30,11 @@ const DRY_RUN = process.argv.includes('--dry-run');
|
||||
const ROOT = path.join(__dirname, '..');
|
||||
const COMMANDS_DIR = path.join(ROOT, 'commands', 'gsd');
|
||||
|
||||
const AT_PATH_RE = /@(?:~|\$HOME)\/.+?get-shit-done\/[^\s`\)]+/g;
|
||||
const AT_PATH_PATTERN = /@(?:~|\$HOME)\/.+?get-shit-done\/[^\s`\)]+/;
|
||||
const mkAtRe = () => new RegExp(AT_PATH_PATTERN.source, 'g');
|
||||
|
||||
function transformLine(line) {
|
||||
if (!AT_PATH_RE.test(line)) return line;
|
||||
AT_PATH_RE.lastIndex = 0;
|
||||
if (!AT_PATH_PATTERN.test(line)) return line;
|
||||
|
||||
const trimmed = line.trim();
|
||||
|
||||
@@ -76,8 +76,9 @@ function processFile(filePath) {
|
||||
if (/<(process|context)>/.test(t) && !t.includes('execution_context')) inProse = true;
|
||||
if (/<\/(process|context)>/.test(t) && !t.includes('execution_context')) inProse = false;
|
||||
|
||||
if (inProse && AT_PATH_RE.test(line)) {
|
||||
AT_PATH_RE.lastIndex = 0;
|
||||
if (inProse && AT_PATH_PATTERN.test(line)) {
|
||||
const re = mkAtRe();
|
||||
re.lastIndex = 0;
|
||||
out.push(transformLine(line));
|
||||
} else {
|
||||
out.push(line);
|
||||
|
||||
Reference in New Issue
Block a user