From 4a9833d3e3dfcc36b5f7b76ac1a7e4a749db0976 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 15 Jul 2026 13:46:26 -0400 Subject: [PATCH] fix(#2278): use Edit() not Write() for Claude allow-permissions + migrate legacy (#2302) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GSD_CLAUDE_ALLOW_PERMISSIONS pre-populated Claude Code settings.json with Write(.planning/*) and Write(STATE.md). Claude Code has no standalone Write permission gate — file-editing tools are gated collectively via Edit(pattern) — so those rules never matched, fresh installs still hit first-run approval prompts for .planning/* and STATE.md, and Claude Code emitted a session-start warning about the unmatched rules. Swap the two entries to Edit(.planning/*) / Edit(STATE.md). Add a GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS list of the retired Write(...) forms, consulted by mergeClaudePermissions (actively remove stale entries when adding current ones, idempotent, user entries preserved) and by the uninstall cleanup filter (still removes the legacy form). Sample settings.json in docs/USER-GUIDE.md corrected to match. Co-authored-by: Claude Opus 4.8 (1M context) --- .changeset/steady-lemurs-run.md | 5 + bin/install.js | 33 +++++- docs/USER-GUIDE.md | 4 +- tests/install-regressions.test.cjs | 177 +++++++++++++++++++++++++++-- 4 files changed, 207 insertions(+), 12 deletions(-) create mode 100644 .changeset/steady-lemurs-run.md diff --git a/.changeset/steady-lemurs-run.md b/.changeset/steady-lemurs-run.md new file mode 100644 index 000000000..08367a2ae --- /dev/null +++ b/.changeset/steady-lemurs-run.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2302 +--- +**Claude Code installs now pre-approve `.planning/` and `STATE.md` writes** — the installer wrote `Write(.planning/*)`/`Write(STATE.md)` permission rules, but Claude Code has no standalone `Write` gate (file edits are gated via `Edit(pattern)`), so those rules never matched and every fresh install still hit first-run approval prompts (and a session-start warning). The installer now writes `Edit(...)` rules and migrates the stale `Write(...)` entries away on the next run. (#2278) diff --git a/bin/install.js b/bin/install.js index 7b6c7e385..cb5b267ec 100755 --- a/bin/install.js +++ b/bin/install.js @@ -168,15 +168,28 @@ const DEFAULT_RUNTIME = 'claude'; const GSD_CLAUDE_ALLOW_PERMISSIONS = Object.freeze([ 'Bash(npx gsd-core *)', 'Read(.planning/*)', - 'Write(.planning/*)', + 'Edit(.planning/*)', 'Read(STATE.md)', - 'Write(STATE.md)', + 'Edit(STATE.md)', ]); const GSD_CLAUDE_DENY_PERMISSIONS = Object.freeze([ 'Read(.env)', 'Read(.env.*)', 'Read(.secrets)', ]); +// #2278 — Stale allow-rule forms from before the fix. Claude Code has no +// standalone `Write` permission gate: file-editing tools (Write/Edit/ +// NotebookEdit) are gated collectively via `Edit(pattern)`. The original +// `Write(.planning/*)` / `Write(STATE.md)` entries were therefore silently +// unmatched (never granted anything) and Claude Code additionally surfaces a +// session-start warning about unmatched permission rules. This list lets +// mergeClaudePermissions and uninstall cleanup retire those stale entries on +// existing installs while the current GSD_CLAUDE_ALLOW_PERMISSIONS above +// carries the working `Edit(...)` forms. +const GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS = Object.freeze([ + 'Write(.planning/*)', + 'Write(STATE.md)', +]); /** * Merge GSD-owned permission entries into a Claude Code settings object. @@ -185,6 +198,12 @@ const GSD_CLAUDE_DENY_PERMISSIONS = Object.freeze([ * entries are appended only if not already present. No other permission sub-keys * (ask, disableBypassPermissionsMode, etc.) are touched. * + * Migration (#2278): before adding the current GSD_CLAUDE_ALLOW_PERMISSIONS, + * any stale GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS entry (e.g. the unmatched + * `Write(...)` forms from before the fix) is removed from permissions.allow, + * so existing installs end up with the working `Edit(...)` forms instead of + * both the dead legacy entry and its replacement sitting side by side. + * * Defensive: if settings is not a plain object, returns immediately without * throwing. If permissions.allow / permissions.deny exist but are not arrays * (malformed settings), they are replaced with valid arrays. @@ -205,6 +224,10 @@ function mergeClaudePermissions(settings) { settings.permissions.deny = []; } + settings.permissions.allow = settings.permissions.allow.filter( + (e) => !GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS.includes(e) + ); + for (const entry of GSD_CLAUDE_ALLOW_PERMISSIONS) { if (!settings.permissions.allow.includes(entry)) { settings.permissions.allow.push(entry); @@ -7591,8 +7614,11 @@ function uninstall(isGlobal, runtime = DEFAULT_RUNTIME) { let permissionsModified = false; if (Array.isArray(settings.permissions.allow)) { const before = settings.permissions.allow.length; + // #2278 — filter against the union of the current allow-rule forms + // AND the retired legacy forms, so uninstall still cleans up + // pre-fix installs that still carry the stale `Write(...)` entries. settings.permissions.allow = settings.permissions.allow.filter( - (e) => !GSD_CLAUDE_ALLOW_PERMISSIONS.includes(e) + (e) => !GSD_CLAUDE_ALLOW_PERMISSIONS.includes(e) && !GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS.includes(e) ); if (settings.permissions.allow.length !== before) { permissionsModified = true; @@ -12157,6 +12183,7 @@ module.exports = { // #768 — Claude Code permissions pre-population mergeClaudePermissions, GSD_CLAUDE_ALLOW_PERMISSIONS, + GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS, GSD_CLAUDE_DENY_PERMISSIONS, GSD_CODEX_MARKER, CODEX_AGENT_SANDBOX, diff --git a/docs/USER-GUIDE.md b/docs/USER-GUIDE.md index 5538b31d7..5da3aaa31 100644 --- a/docs/USER-GUIDE.md +++ b/docs/USER-GUIDE.md @@ -886,9 +886,9 @@ Since v1.3.1, the installer pre-populates `~/.claude/settings.json` (or "allow": [ "Bash(npx gsd-core *)", "Read(.planning/*)", - "Write(.planning/*)", + "Edit(.planning/*)", "Read(STATE.md)", - "Write(STATE.md)" + "Edit(STATE.md)" ], "deny": [ "Read(.env)", diff --git a/tests/install-regressions.test.cjs b/tests/install-regressions.test.cjs index 99d628cc7..40768a33c 100644 --- a/tests/install-regressions.test.cjs +++ b/tests/install-regressions.test.cjs @@ -37,7 +37,7 @@ try { else process.env.GSD_TEST_MODE = savedTestMode; } -const { install, mergeClaudePermissions, GSD_CLAUDE_ALLOW_PERMISSIONS, GSD_CLAUDE_DENY_PERMISSIONS, rewriteLegacyManagedNodeHookCommands, resolveNodeRunner } = installExports || {}; +const { install, mergeClaudePermissions, GSD_CLAUDE_ALLOW_PERMISSIONS, GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS, GSD_CLAUDE_DENY_PERMISSIONS, rewriteLegacyManagedNodeHookCommands, resolveNodeRunner } = installExports || {}; const { installRuntimeArtifacts, @@ -394,22 +394,26 @@ describe('mergeClaudePermissions (#768): fresh settings object', () => { 'permissions.allow must contain Bash(npx gsd-core *)'); }); - test('includes planning path entries in allow', () => { + test('includes planning path entries in allow (#2278: Edit, not Write)', () => { const settings = {}; mergeClaudePermissions(settings); assert.ok(settings.permissions.allow.includes('Read(.planning/*)'), 'permissions.allow must contain Read(.planning/*)'); - assert.ok(settings.permissions.allow.includes('Write(.planning/*)'), - 'permissions.allow must contain Write(.planning/*)'); + assert.ok(settings.permissions.allow.includes('Edit(.planning/*)'), + 'permissions.allow must contain Edit(.planning/*)'); + assert.ok(!settings.permissions.allow.includes('Write(.planning/*)'), + 'permissions.allow must NOT contain the unmatched Write(.planning/*) form (#2278)'); }); - test('includes STATE.md entries in allow', () => { + test('includes STATE.md entries in allow (#2278: Edit, not Write)', () => { const settings = {}; mergeClaudePermissions(settings); assert.ok(settings.permissions.allow.includes('Read(STATE.md)'), 'permissions.allow must contain Read(STATE.md)'); - assert.ok(settings.permissions.allow.includes('Write(STATE.md)'), - 'permissions.allow must contain Write(STATE.md)'); + assert.ok(settings.permissions.allow.includes('Edit(STATE.md)'), + 'permissions.allow must contain Edit(STATE.md)'); + assert.ok(!settings.permissions.allow.includes('Write(STATE.md)'), + 'permissions.allow must NOT contain the unmatched Write(STATE.md) form (#2278)'); }); test('includes .env denial entries in deny', () => { @@ -496,6 +500,115 @@ describe('mergeClaudePermissions (#768): non-destructive merge', () => { }); }); +// ─── #2278 — Claude Code has no standalone `Write` permission gate; the +// pre-populated allow-rules must use `Edit(pattern)`, and a merge against an +// existing install must retire the stale unmatched `Write(...)` forms. +describe('mergeClaudePermissions (#2278): legacy Write(...) → Edit(...) migration', () => { + test('GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS is exported and lists the stale Write(...) forms', () => { + assert.ok(Array.isArray(GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS), + 'GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS must be an array'); + assert.deepStrictEqual( + [...GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS].sort(), + ['Write(.planning/*)', 'Write(STATE.md)'].sort(), + 'GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS must contain exactly the retired Write(...) forms' + ); + }); + + test('fresh/empty settings: allow ends with Edit(...) forms, never Write(...)', () => { + const settings = {}; + mergeClaudePermissions(settings); + assert.ok(settings.permissions.allow.includes('Edit(.planning/*)'), + 'fresh merge must add Edit(.planning/*)'); + assert.ok(settings.permissions.allow.includes('Edit(STATE.md)'), + 'fresh merge must add Edit(STATE.md)'); + assert.ok(!settings.permissions.allow.includes('Write(.planning/*)'), + 'fresh merge must never add Write(.planning/*)'); + assert.ok(!settings.permissions.allow.includes('Write(STATE.md)'), + 'fresh merge must never add Write(STATE.md)'); + }); + + test('existing install with legacy Write(...) entries: migrated to Edit(...), user entries untouched', () => { + const settings = { + permissions: { + allow: ['Write(.planning/*)', 'Write(STATE.md)', 'Bash(git *)'], + deny: ['WebSearch'], + }, + }; + mergeClaudePermissions(settings); + + // Stale legacy forms must be gone. + assert.ok(!settings.permissions.allow.includes('Write(.planning/*)'), + 'legacy Write(.planning/*) must be removed by merge'); + assert.ok(!settings.permissions.allow.includes('Write(STATE.md)'), + 'legacy Write(STATE.md) must be removed by merge'); + + // Replaced by the working Edit(...) forms. + assert.ok(settings.permissions.allow.includes('Edit(.planning/*)'), + 'Edit(.planning/*) must be present after migration'); + assert.ok(settings.permissions.allow.includes('Edit(STATE.md)'), + 'Edit(STATE.md) must be present after migration'); + + // Unrelated user-added entries must survive untouched. + assert.ok(settings.permissions.allow.includes('Bash(git *)'), + 'unrelated user allow entry must survive migration'); + assert.ok(settings.permissions.deny.includes('WebSearch'), + 'unrelated user deny entry must survive migration'); + }); + + test('mixed state: settings.allow containing BOTH legacy and current forms simultaneously collapses to exactly one Edit(...) each', () => { + const settings = { + permissions: { + allow: ['Write(.planning/*)', 'Edit(.planning/*)', 'Write(STATE.md)', 'Edit(STATE.md)', 'Bash(git *)'], + deny: [], + }, + }; + mergeClaudePermissions(settings); + + // Legacy forms must be gone. + assert.ok(!settings.permissions.allow.includes('Write(.planning/*)'), + 'legacy Write(.planning/*) must be removed even when Edit(.planning/*) was already present'); + assert.ok(!settings.permissions.allow.includes('Write(STATE.md)'), + 'legacy Write(STATE.md) must be removed even when Edit(STATE.md) was already present'); + + // Current forms must appear exactly once (no duplicate from the pre-existing entry). + assert.strictEqual( + settings.permissions.allow.filter((e) => e === 'Edit(.planning/*)').length, + 1, + 'Edit(.planning/*) must appear exactly once, not duplicated' + ); + assert.strictEqual( + settings.permissions.allow.filter((e) => e === 'Edit(STATE.md)').length, + 1, + 'Edit(STATE.md) must appear exactly once, not duplicated' + ); + + // Unrelated user entry must survive. + assert.ok(settings.permissions.allow.includes('Bash(git *)'), + 'unrelated user allow entry must survive the mixed-state migration'); + }); + + test('idempotent: repeated merge produces no dupes and never re-adds legacy entries', () => { + const settings = { + permissions: { + allow: ['Write(.planning/*)', 'Write(STATE.md)'], + deny: [], + }, + }; + mergeClaudePermissions(settings); + mergeClaudePermissions(settings); + mergeClaudePermissions(settings); + + for (const entry of GSD_CLAUDE_ALLOW_PERMISSIONS) { + const count = settings.permissions.allow.filter((e) => e === entry).length; + assert.strictEqual(count, 1, `allow entry "${entry}" must appear exactly once after repeated merges`); + } + for (const legacy of GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS) { + assert.ok(!settings.permissions.allow.includes(legacy), + `legacy entry "${legacy}" must never reappear after repeated merges`); + } + }); +}); + describe('mergeClaudePermissions (#768): end-to-end install writes permissions to settings.json', () => { test('--claude --global install writes GSD allow/deny entries to settings.json', (t) => { const root = createTempDir('gsd-claude-perm-install-'); @@ -635,6 +748,56 @@ describe('mergeClaudePermissions (#768): end-to-end install writes permissions t assert.ok(deny.includes('WebSearch'), 'user WebSearch deny entry must survive uninstall'); }); + + test('#2278: uninstall removes GSD entries in both legacy Write(...) and current Edit(...) form', (t) => { + const root = createTempDir('gsd-claude-perm-uninstall-legacy-'); + t.after(() => cleanup(root)); + + const spawnOpts = { + encoding: 'utf8', + env: { ...process.env, HOME: root, USERPROFILE: root }, + }; + + // Install first (writes the current Edit(...) forms). + const r1 = spawnSync( + process.execPath, + [INSTALL_SCRIPT, '--claude', '--global', '--config-dir', root], + spawnOpts, + ); + assert.strictEqual(r1.status, 0, `install failed: ${r1.stderr}`); + + // Simulate a pre-fix install that still carries the stale Write(...) + // forms alongside the current Edit(...) forms and a user entry. + const settingsPath = path.join(root, 'settings.json'); + const settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); + settings.permissions.allow.push('Write(.planning/*)', 'Write(STATE.md)', 'Bash(git *)'); + fs.writeFileSync(settingsPath, JSON.stringify(settings, null, 2) + '\n'); + + // Uninstall + const r2 = spawnSync( + process.execPath, + [INSTALL_SCRIPT, '--claude', '--global', '--config-dir', root, '--uninstall'], + spawnOpts, + ); + assert.strictEqual(r2.status, 0, `uninstall failed: ${r2.stderr}`); + + const afterUninstall = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); + const allow = afterUninstall.permissions?.allow ?? []; + + // Both legacy and current GSD-owned forms must be removed. + assert.ok(!allow.includes('Write(.planning/*)'), + 'legacy Write(.planning/*) entry must be removed by uninstall'); + assert.ok(!allow.includes('Write(STATE.md)'), + 'legacy Write(STATE.md) entry must be removed by uninstall'); + assert.ok(!allow.includes('Edit(.planning/*)'), + 'current Edit(.planning/*) entry must be removed by uninstall'); + assert.ok(!allow.includes('Edit(STATE.md)'), + 'current Edit(STATE.md) entry must be removed by uninstall'); + + // User entry must survive. + assert.ok(allow.includes('Bash(git *)'), + 'user Bash(git *) allow entry must survive uninstall'); + }); }); // ─── #976 — args-form hook presence detection ─────────────────────────────────