diff --git a/.changeset/767-disallowed-tools-readonly-agents.md b/.changeset/767-disallowed-tools-readonly-agents.md new file mode 100644 index 000000000..8951f695a --- /dev/null +++ b/.changeset/767-disallowed-tools-readonly-agents.md @@ -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) diff --git a/bin/install.js b/bin/install.js index cfa1e10fb..a95e144a5 100755 --- a/bin/install.js +++ b/bin/install.js @@ -1209,6 +1209,52 @@ function injectEffortFrontmatter(content, effortValue) { return `${before}effort: ${effortValue}${eol}${after}`; } +/** + * #767 — Inject `disallowedTools: ` 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:` colon refs in the agent body // to the canonical hyphen form `/gsd-` for hyphen-`name:` diff --git a/docs/AGENTS.md b/docs/AGENTS.md index 6f9cdb484..d82c2ec73 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -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 | diff --git a/tests/install.test.cjs b/tests/install.test.cjs index b4c97d09d..9b4659688 100644 --- a/tests/install.test.cjs +++ b/tests/install.test.cjs @@ -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}"`); + }); + } +});