Install-time injection of a disallowedTools deny-list into Claude copies of read-only verifier/auditor agents (mirrors the #443 effort injection); source agents stay runtime-neutral so Gemini/Qwen/Hermes are unaffected. Closes #767.
This commit is contained in:
Tom Boucher
5
.changeset/767-disallowed-tools-readonly-agents.md
Normal file
5
.changeset/767-disallowed-tools-readonly-agents.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Changed
|
||||
pr: 1081
|
||||
---
|
||||
**Read-only verifier/auditor agents now ship a Claude-Code `disallowedTools` deny-list** — the installer injects a framework-level write-tool deny-list into the Claude copies of the read-only verifier/auditor agents (gsd-verifier, gsd-plan-checker, gsd-integration-checker, gsd-doc-verifier, gsd-eval-auditor, gsd-ui-auditor, gsd-ui-checker) so write actions are blocked even if a tool grant is inherited. Injected for Claude only; other runtimes are unaffected. (#1081)
|
||||
@@ -1209,6 +1209,52 @@ function injectEffortFrontmatter(content, effortValue) {
|
||||
return `${before}effort: ${effortValue}${eol}${after}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* #767 — Inject `disallowedTools: <value>` into the YAML frontmatter of a Claude .md agent.
|
||||
* Mirrors injectEffortFrontmatter: idempotent (skips if disallowedTools: already present),
|
||||
* inserts immediately before the closing `---`. Claude-only — never call for other runtimes,
|
||||
* which break on unknown frontmatter keys.
|
||||
*/
|
||||
function injectDisallowedToolsFrontmatter(content, disallowedValue) {
|
||||
// Detect the dominant EOL from the first line (the opening `---`).
|
||||
// If the very first `---` is followed by \r\n, treat the whole file as CRLF.
|
||||
const eol = /^---\r\n/.test(content) ? '\r\n' : '\n';
|
||||
|
||||
// Build a frontmatter-matching regex that tolerates an optional \r before
|
||||
// each \n, so we handle both LF and CRLF files without needing to normalise
|
||||
// the whole content.
|
||||
const fmRe = /^---\r?\n([\s\S]*?)^---\r?$/m;
|
||||
const match = fmRe.exec(content);
|
||||
if (!match) return content; // no YAML frontmatter — leave unchanged
|
||||
|
||||
// Idempotency guard: don't insert a second disallowedTools: line.
|
||||
const fmBody = match[1]; // content between the two `---` lines
|
||||
if (/^disallowedTools:/m.test(fmBody)) return content;
|
||||
|
||||
// Locate the exact position of the closing `---` line so we can insert
|
||||
// before it using a simple string splice.
|
||||
const openLen = 3 + eol.length; // "---" + eol
|
||||
const closingStart = match.index + openLen + fmBody.length;
|
||||
|
||||
const before = content.slice(0, closingStart);
|
||||
const after = content.slice(closingStart);
|
||||
return `${before}disallowedTools: ${disallowedValue}${eol}${after}`;
|
||||
}
|
||||
|
||||
// #767 — Read-only verifier/auditor agents get a Claude-Code disallowedTools deny-list.
|
||||
// Group A (pure read-only) deny Write,Edit,MultiEdit. Group B report-writers Write one
|
||||
// output file so they deny only Edit,MultiEdit. gsd-nyquist-auditor is intentionally
|
||||
// excluded (it legitimately uses Write AND Edit to create/patch test files).
|
||||
const READONLY_AGENT_DISALLOWED_TOOLS = {
|
||||
'gsd-plan-checker': 'Write, Edit, MultiEdit',
|
||||
'gsd-integration-checker': 'Write, Edit, MultiEdit',
|
||||
'gsd-ui-checker': 'Write, Edit, MultiEdit',
|
||||
'gsd-verifier': 'Edit, MultiEdit',
|
||||
'gsd-doc-verifier': 'Edit, MultiEdit',
|
||||
'gsd-eval-auditor': 'Edit, MultiEdit',
|
||||
'gsd-ui-auditor': 'Edit, MultiEdit',
|
||||
};
|
||||
|
||||
/**
|
||||
* #2517 — Read a single GSD config file (defaults.json or per-project
|
||||
* config.json) into a plain object, returning null on missing/empty files
|
||||
@@ -10181,6 +10227,8 @@ function install(isGlobal, runtime = 'claude', options = {}) {
|
||||
const _universalEffort = resolveInstallTimeEffort(_effortCfg, _agentName);
|
||||
const _renderedEffort = _getGsdEffortCatalog().renderEffortForRuntime('claude', _universalEffort).value;
|
||||
content = injectEffortFrontmatter(content, _renderedEffort);
|
||||
const _disallowedTools = READONLY_AGENT_DISALLOWED_TOOLS[_agentName];
|
||||
if (_disallowedTools) content = injectDisallowedToolsFrontmatter(content, _disallowedTools);
|
||||
}
|
||||
// #3677 — normalize retired `/gsd:<cmd>` colon refs in the agent body
|
||||
// to the canonical hyphen form `/gsd-<cmd>` for hyphen-`name:`
|
||||
|
||||
@@ -230,6 +230,7 @@ GSD uses a multi-agent architecture where thin orchestrators (workflow files) sp
|
||||
| **Spawned by** | `/gsd-plan-phase` (verification loop, max 3 iterations) |
|
||||
| **Parallelism** | Single instance (iterative) |
|
||||
| **Tools** | Read, Bash, Glob, Grep |
|
||||
| **Disallowed Tools** | Write, Edit, MultiEdit |
|
||||
| **Model (balanced)** | Sonnet |
|
||||
| **Color** | Green |
|
||||
| **Produces** | PASS/FAIL verdict with specific feedback |
|
||||
@@ -255,6 +256,7 @@ GSD uses a multi-agent architecture where thin orchestrators (workflow files) sp
|
||||
| **Spawned by** | `/gsd-audit-milestone` |
|
||||
| **Parallelism** | Single instance |
|
||||
| **Tools** | Read, Bash, Grep, Glob |
|
||||
| **Disallowed Tools** | Write, Edit, MultiEdit |
|
||||
| **Model (balanced)** | Sonnet |
|
||||
| **Color** | Blue |
|
||||
| **Produces** | Integration verification report |
|
||||
@@ -270,6 +272,7 @@ GSD uses a multi-agent architecture where thin orchestrators (workflow files) sp
|
||||
| **Spawned by** | `/gsd-ui-phase` (validation loop, max 2 iterations) |
|
||||
| **Parallelism** | Single instance |
|
||||
| **Tools** | Read, Bash, Glob, Grep |
|
||||
| **Disallowed Tools** | Write, Edit, MultiEdit |
|
||||
| **Model (balanced)** | Sonnet |
|
||||
| **Color** | Cyan |
|
||||
| **Produces** | BLOCK/FLAG/PASS verdict |
|
||||
@@ -285,6 +288,7 @@ GSD uses a multi-agent architecture where thin orchestrators (workflow files) sp
|
||||
| **Spawned by** | `/gsd-execute-phase` (after all executors complete) |
|
||||
| **Parallelism** | Single instance |
|
||||
| **Tools** | Read, Write, Bash, Grep, Glob |
|
||||
| **Disallowed Tools** | Edit, MultiEdit |
|
||||
| **Model (balanced)** | Sonnet |
|
||||
| **Color** | Green |
|
||||
| **Produces** | `{phase}-VERIFICATION.md` |
|
||||
@@ -328,6 +332,7 @@ GSD uses a multi-agent architecture where thin orchestrators (workflow files) sp
|
||||
| **Spawned by** | `/gsd-ui-review` |
|
||||
| **Parallelism** | Single instance |
|
||||
| **Tools** | Read, Write, Bash, Grep, Glob |
|
||||
| **Disallowed Tools** | Edit, MultiEdit |
|
||||
| **Model (balanced)** | Sonnet |
|
||||
| **Color** | Pink |
|
||||
| **Produces** | `{phase}-UI-REVIEW.md` with scores |
|
||||
@@ -449,6 +454,7 @@ Communication style, decision patterns, debugging approach, UX preferences, vend
|
||||
| **Spawned by** | `/gsd-docs-update` (after doc-writer completes) |
|
||||
| **Parallelism** | Multiple instances (one per doc file) |
|
||||
| **Tools** | Read, Write, Bash, Grep, Glob |
|
||||
| **Disallowed Tools** | Edit, MultiEdit |
|
||||
| **Model (balanced)** | Sonnet |
|
||||
| **Color** | Orange |
|
||||
| **Produces** | Structured JSON verification results per doc |
|
||||
@@ -635,6 +641,7 @@ Twelve additional agents ship under `agents/gsd-*.md` and are used by specialty
|
||||
| **Spawned by** | `/gsd-eval-review` |
|
||||
| **Parallelism** | Single instance |
|
||||
| **Tools** | Read, Write, Bash, Grep, Glob |
|
||||
| **Disallowed Tools** | Edit, MultiEdit |
|
||||
| **Model (balanced)** | Sonnet |
|
||||
| **Color** | Red |
|
||||
| **Produces** | `EVAL-REVIEW.md` with dimension scores, findings, and remediation guidance |
|
||||
|
||||
@@ -834,3 +834,233 @@ describe('install — changeset CLI lands at scripts/changeset/cli.cjs (#935)',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Section N+1: #767 — disallowedTools injection for read-only agents ──────
|
||||
//
|
||||
// Verifies (installer-behavioral test — drives install() to a temp dir):
|
||||
// 1. Claude install: Group A agents have disallowedTools == {Write, Edit, MultiEdit} exactly.
|
||||
// 2. Claude install: Group B agents have disallowedTools == {Edit, MultiEdit} exactly.
|
||||
// 3. Negative: gsd-nyquist-auditor has NO disallowedTools key (legitimately writes+edits).
|
||||
// 4. Cross-runtime no-leak: Gemini-installed read-only agents do NOT contain disallowedTools.
|
||||
// 5. Source purity: source agents/*.md must not contain disallowedTools (inject-only).
|
||||
// 6. Parity: docs/AGENTS.md "Disallowed Tools" rows match READONLY_AGENT_DISALLOWED_TOOLS.
|
||||
// (DEFECT.GENERATIVE-FIX guard)
|
||||
|
||||
// #767 — must mirror READONLY_AGENT_DISALLOWED_TOOLS in bin/install.js.
|
||||
// If you change the map there, update this too (the parity test will catch drift).
|
||||
const READONLY_AGENT_DISALLOWED_TOOLS_767 = {
|
||||
'gsd-plan-checker': 'Write, Edit, MultiEdit',
|
||||
'gsd-integration-checker': 'Write, Edit, MultiEdit',
|
||||
'gsd-ui-checker': 'Write, Edit, MultiEdit',
|
||||
'gsd-verifier': 'Edit, MultiEdit',
|
||||
'gsd-doc-verifier': 'Edit, MultiEdit',
|
||||
'gsd-eval-auditor': 'Edit, MultiEdit',
|
||||
'gsd-ui-auditor': 'Edit, MultiEdit',
|
||||
};
|
||||
|
||||
const GROUP_A_767 = ['gsd-plan-checker', 'gsd-integration-checker', 'gsd-ui-checker'];
|
||||
const GROUP_B_767 = ['gsd-verifier', 'gsd-doc-verifier', 'gsd-eval-auditor', 'gsd-ui-auditor'];
|
||||
|
||||
const REPO_ROOT_767 = path.resolve(__dirname, '..');
|
||||
const SOURCE_AGENTS_DIR_767 = path.join(REPO_ROOT_767, 'agents');
|
||||
const AGENTS_DOC_PATH_767 = path.join(REPO_ROOT_767, 'docs', 'AGENTS.md');
|
||||
|
||||
function readFrontmatterText(mdPath) {
|
||||
const content = fs.readFileSync(mdPath, 'utf8');
|
||||
if (!content.startsWith('---')) return '';
|
||||
const end = content.indexOf('---', 3);
|
||||
if (end === -1) return '';
|
||||
return content.substring(3, end);
|
||||
}
|
||||
|
||||
function parseDisallowedToolsSet(fm) {
|
||||
const match = fm.match(/^disallowedTools:\s*(.+)$/m);
|
||||
if (!match) return null;
|
||||
return new Set(match[1].split(',').map((t) => t.trim()).filter(Boolean));
|
||||
}
|
||||
|
||||
/**
|
||||
* Run a global install for the given runtime, redirecting its home dir to
|
||||
* tmpHome. Stubs both HOME and USERPROFILE for Windows parity, and
|
||||
* suppresses the stale-SDK npm subprocess.
|
||||
*/
|
||||
function runGlobalInstall767(runtime, tmpHome) {
|
||||
const envVarMap = {
|
||||
claude: 'CLAUDE_CONFIG_DIR',
|
||||
gemini: 'GEMINI_CONFIG_DIR',
|
||||
qwen: 'QWEN_CONFIG_DIR',
|
||||
};
|
||||
const envVar = envVarMap[runtime];
|
||||
if (!envVar) throw new Error(`Unsupported runtime in #767 test: ${runtime}`);
|
||||
|
||||
const isolatedHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-767-home-'));
|
||||
|
||||
const prevEnvVar = process.env[envVar];
|
||||
const prevCwd = process.cwd();
|
||||
const prevHome = process.env.HOME;
|
||||
const prevUserProfile = process.env.USERPROFILE;
|
||||
const prevSkipStale = process.env.GSD_SKIP_STALE_SDK_CHECK;
|
||||
|
||||
process.env[envVar] = tmpHome;
|
||||
process.env.HOME = isolatedHome;
|
||||
process.env.USERPROFILE = isolatedHome;
|
||||
process.env.GSD_SKIP_STALE_SDK_CHECK = '1';
|
||||
process.chdir(REPO_ROOT_767);
|
||||
|
||||
try {
|
||||
install(true, runtime);
|
||||
} finally {
|
||||
process.chdir(prevCwd);
|
||||
if (prevEnvVar === undefined) delete process.env[envVar];
|
||||
else process.env[envVar] = prevEnvVar;
|
||||
if (prevHome === undefined) delete process.env.HOME;
|
||||
else process.env.HOME = prevHome;
|
||||
if (prevUserProfile === undefined) delete process.env.USERPROFILE;
|
||||
else process.env.USERPROFILE = prevUserProfile;
|
||||
if (prevSkipStale === undefined) delete process.env.GSD_SKIP_STALE_SDK_CHECK;
|
||||
else process.env.GSD_SKIP_STALE_SDK_CHECK = prevSkipStale;
|
||||
cleanup(isolatedHome);
|
||||
}
|
||||
|
||||
return tmpHome;
|
||||
}
|
||||
|
||||
describe('#767 Claude install: Group A agents have disallowedTools = {Write, Edit, MultiEdit}', () => {
|
||||
let tmpDir;
|
||||
let claudeHome;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempDir('gsd-767-claude-a-');
|
||||
claudeHome = path.join(tmpDir, 'claude-home');
|
||||
fs.mkdirSync(claudeHome, { recursive: true });
|
||||
runGlobalInstall767('claude', claudeHome);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
const EXPECTED_A_767 = new Set(['Write', 'Edit', 'MultiEdit']);
|
||||
|
||||
for (const agent of GROUP_A_767) {
|
||||
test(`${agent}: disallowedTools is exactly {Write, Edit, MultiEdit}`, () => {
|
||||
const fm = readFrontmatterText(path.join(claudeHome, 'agents', `${agent}.md`));
|
||||
const tools = parseDisallowedToolsSet(fm);
|
||||
assert.ok(tools !== null,
|
||||
`${agent} must have a disallowedTools key in Claude frontmatter\nFrontmatter:\n${fm}`);
|
||||
assert.deepEqual(tools, EXPECTED_A_767,
|
||||
`${agent} disallowedTools must be exactly {Write, Edit, MultiEdit}\nGot: ${[...tools].join(', ')}`);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe('#767 Claude install: Group B agents have disallowedTools = {Edit, MultiEdit}', () => {
|
||||
let tmpDir;
|
||||
let claudeHome;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempDir('gsd-767-claude-b-');
|
||||
claudeHome = path.join(tmpDir, 'claude-home');
|
||||
fs.mkdirSync(claudeHome, { recursive: true });
|
||||
runGlobalInstall767('claude', claudeHome);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
const EXPECTED_B_767 = new Set(['Edit', 'MultiEdit']);
|
||||
|
||||
for (const agent of GROUP_B_767) {
|
||||
test(`${agent}: disallowedTools is exactly {Edit, MultiEdit}`, () => {
|
||||
const fm = readFrontmatterText(path.join(claudeHome, 'agents', `${agent}.md`));
|
||||
const tools = parseDisallowedToolsSet(fm);
|
||||
assert.ok(tools !== null,
|
||||
`${agent} must have a disallowedTools key in Claude frontmatter\nFrontmatter:\n${fm}`);
|
||||
assert.deepEqual(tools, EXPECTED_B_767,
|
||||
`${agent} disallowedTools must be exactly {Edit, MultiEdit}\nGot: ${[...tools].join(', ')}`);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe('#767 Claude install: gsd-nyquist-auditor has no disallowedTools (legitimately writes+edits)', () => {
|
||||
let tmpDir;
|
||||
let claudeHome;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempDir('gsd-767-claude-nyquist-');
|
||||
claudeHome = path.join(tmpDir, 'claude-home');
|
||||
fs.mkdirSync(claudeHome, { recursive: true });
|
||||
runGlobalInstall767('claude', claudeHome);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('gsd-nyquist-auditor.md has NO disallowedTools key', () => {
|
||||
const fm = readFrontmatterText(path.join(claudeHome, 'agents', 'gsd-nyquist-auditor.md'));
|
||||
const tools = parseDisallowedToolsSet(fm);
|
||||
assert.equal(tools, null,
|
||||
`gsd-nyquist-auditor must NOT have disallowedTools in Claude frontmatter\nFrontmatter:\n${fm}`);
|
||||
});
|
||||
});
|
||||
|
||||
describe('#767 Gemini install: read-only agents do NOT contain disallowedTools (cross-runtime leak guard)', () => {
|
||||
let tmpDir;
|
||||
let geminiHome;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempDir('gsd-767-gemini-');
|
||||
geminiHome = path.join(tmpDir, 'gemini-home');
|
||||
fs.mkdirSync(geminiHome, { recursive: true });
|
||||
runGlobalInstall767('gemini', geminiHome);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
for (const agent of [...GROUP_A_767, ...GROUP_B_767]) {
|
||||
test(`Gemini ${agent}.md has no disallowedTools`, () => {
|
||||
const agentPath = path.join(geminiHome, 'agents', `${agent}.md`);
|
||||
const content = fs.readFileSync(agentPath, 'utf8');
|
||||
assert.ok(!content.includes('disallowedTools'),
|
||||
`${agent} (Gemini) must NOT contain disallowedTools\nContent excerpt:\n${content.slice(0, 400)}`);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe('#767 Source purity: source agents/*.md must not contain disallowedTools (inject-only)', () => {
|
||||
for (const agent of [...GROUP_A_767, ...GROUP_B_767]) {
|
||||
test(`source agents/${agent}.md has no disallowedTools`, () => {
|
||||
const content = fs.readFileSync(path.join(SOURCE_AGENTS_DIR_767, `${agent}.md`), 'utf8');
|
||||
assert.ok(!content.includes('disallowedTools'),
|
||||
`Source agents/${agent}.md must NOT contain disallowedTools (injection is install-time only, source must stay runtime-neutral)`);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe('#767 Parity: docs/AGENTS.md "Disallowed Tools" rows match READONLY_AGENT_DISALLOWED_TOOLS', () => {
|
||||
const agentsDoc = fs.readFileSync(AGENTS_DOC_PATH_767, 'utf8');
|
||||
|
||||
for (const [agent, expectedTools] of Object.entries(READONLY_AGENT_DISALLOWED_TOOLS_767)) {
|
||||
test(`docs/AGENTS.md has matching Disallowed Tools row for ${agent}`, () => {
|
||||
const agentHeaderIdx = agentsDoc.indexOf(`### ${agent}`);
|
||||
assert.ok(agentHeaderIdx !== -1,
|
||||
`docs/AGENTS.md must contain a ### ${agent} section`);
|
||||
|
||||
const nextSectionIdx = agentsDoc.indexOf('\n### ', agentHeaderIdx + 1);
|
||||
const sectionEnd = nextSectionIdx === -1 ? agentsDoc.length : nextSectionIdx;
|
||||
const section = agentsDoc.slice(agentHeaderIdx, sectionEnd);
|
||||
|
||||
const disallowedMatch = section.match(/\|\s*\*\*Disallowed Tools\*\*\s*\|\s*([^|]+)\|/);
|
||||
assert.ok(disallowedMatch,
|
||||
`docs/AGENTS.md section for ${agent} must have a "Disallowed Tools" table row`);
|
||||
|
||||
const docTools = disallowedMatch[1].trim();
|
||||
assert.equal(docTools, expectedTools,
|
||||
`docs/AGENTS.md "Disallowed Tools" for ${agent} must be "${expectedTools}" but got "${docTools}"`);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user