* feat(#768): pre-populate settings.json permissions.allow/deny for Claude Code Adds mergeClaudePermissions() to bin/install.js which non-destructively appends GSD's known-safe tool-call patterns to permissions.allow and defense-in-depth credential-file patterns to permissions.deny during Claude Code installs. Merge is idempotent (no duplicates on reinstall) and additive (existing user entries preserved). Uninstall removes only the exact GSD-owned entries. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore: update changeset pr number to 819 --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
@@ -37,7 +37,7 @@ try {
|
||||
else process.env.GSD_TEST_MODE = savedTestMode;
|
||||
}
|
||||
|
||||
const { installRuntimeArtifacts, uninstallRuntimeArtifacts } = installExports || {};
|
||||
const { installRuntimeArtifacts, uninstallRuntimeArtifacts, mergeClaudePermissions, GSD_CLAUDE_ALLOW_PERMISSIONS, GSD_CLAUDE_DENY_PERMISSIONS } = installExports || {};
|
||||
|
||||
const INSTALL_SCRIPT = path.join(__dirname, '..', 'bin', 'install.js');
|
||||
const REAL_COMMANDS_DIR = path.join(__dirname, '..', 'commands', 'gsd');
|
||||
@@ -304,3 +304,298 @@ describe('U3 (#2973): uninstallRuntimeArtifacts hermes migrates dev-preferences
|
||||
assert.strictEqual(fs.readFileSync(skillFile, 'utf8'), '# my hermes prefs\n');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── #768 — mergeClaudePermissions: pre-populate permissions.allow/deny ──────
|
||||
|
||||
describe('mergeClaudePermissions (#768): exports and permission constants', () => {
|
||||
test('mergeClaudePermissions is exported', () => {
|
||||
assert.strictEqual(typeof mergeClaudePermissions, 'function',
|
||||
'mergeClaudePermissions must be exported from bin/install.js');
|
||||
});
|
||||
|
||||
test('GSD_CLAUDE_ALLOW_PERMISSIONS is a non-empty array of strings', () => {
|
||||
assert.ok(Array.isArray(GSD_CLAUDE_ALLOW_PERMISSIONS),
|
||||
'GSD_CLAUDE_ALLOW_PERMISSIONS must be an array');
|
||||
assert.ok(GSD_CLAUDE_ALLOW_PERMISSIONS.length > 0,
|
||||
'GSD_CLAUDE_ALLOW_PERMISSIONS must not be empty');
|
||||
for (const entry of GSD_CLAUDE_ALLOW_PERMISSIONS) {
|
||||
assert.strictEqual(typeof entry, 'string', `allow entry must be a string, got: ${JSON.stringify(entry)}`);
|
||||
}
|
||||
});
|
||||
|
||||
test('GSD_CLAUDE_DENY_PERMISSIONS is a non-empty array of strings', () => {
|
||||
assert.ok(Array.isArray(GSD_CLAUDE_DENY_PERMISSIONS),
|
||||
'GSD_CLAUDE_DENY_PERMISSIONS must be an array');
|
||||
assert.ok(GSD_CLAUDE_DENY_PERMISSIONS.length > 0,
|
||||
'GSD_CLAUDE_DENY_PERMISSIONS must not be empty');
|
||||
for (const entry of GSD_CLAUDE_DENY_PERMISSIONS) {
|
||||
assert.strictEqual(typeof entry, 'string', `deny entry must be a string, got: ${JSON.stringify(entry)}`);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('mergeClaudePermissions (#768): fresh settings object', () => {
|
||||
test('populates permissions.allow and permissions.deny on empty settings', () => {
|
||||
const settings = {};
|
||||
mergeClaudePermissions(settings);
|
||||
assert.ok(Array.isArray(settings.permissions?.allow), 'permissions.allow must be an array');
|
||||
assert.ok(Array.isArray(settings.permissions?.deny), 'permissions.deny must be an array');
|
||||
for (const entry of GSD_CLAUDE_ALLOW_PERMISSIONS) {
|
||||
assert.ok(settings.permissions.allow.includes(entry),
|
||||
`permissions.allow must contain "${entry}"`);
|
||||
}
|
||||
for (const entry of GSD_CLAUDE_DENY_PERMISSIONS) {
|
||||
assert.ok(settings.permissions.deny.includes(entry),
|
||||
`permissions.deny must contain "${entry}"`);
|
||||
}
|
||||
});
|
||||
|
||||
test('includes Bash(npx gsd-core *) in allow', () => {
|
||||
const settings = {};
|
||||
mergeClaudePermissions(settings);
|
||||
assert.ok(settings.permissions.allow.includes('Bash(npx gsd-core *)'),
|
||||
'permissions.allow must contain Bash(npx gsd-core *)');
|
||||
});
|
||||
|
||||
test('includes planning path entries in allow', () => {
|
||||
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/*)');
|
||||
});
|
||||
|
||||
test('includes STATE.md entries in allow', () => {
|
||||
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)');
|
||||
});
|
||||
|
||||
test('includes .env denial entries in deny', () => {
|
||||
const settings = {};
|
||||
mergeClaudePermissions(settings);
|
||||
assert.ok(settings.permissions.deny.includes('Read(.env)'),
|
||||
'permissions.deny must contain Read(.env)');
|
||||
assert.ok(settings.permissions.deny.includes('Read(.env.*)'),
|
||||
'permissions.deny must contain Read(.env.*)');
|
||||
assert.ok(settings.permissions.deny.includes('Read(.secrets)'),
|
||||
'permissions.deny must contain Read(.secrets)');
|
||||
});
|
||||
});
|
||||
|
||||
describe('mergeClaudePermissions (#768): non-destructive merge', () => {
|
||||
test('appends to existing allow/deny arrays without overwriting user entries', () => {
|
||||
const settings = {
|
||||
permissions: {
|
||||
allow: ['Bash(git *)'],
|
||||
deny: ['WebSearch'],
|
||||
},
|
||||
};
|
||||
mergeClaudePermissions(settings);
|
||||
// User entries must be preserved
|
||||
assert.ok(settings.permissions.allow.includes('Bash(git *)'),
|
||||
'existing allow entries must be preserved');
|
||||
assert.ok(settings.permissions.deny.includes('WebSearch'),
|
||||
'existing deny entries must be preserved');
|
||||
// GSD entries must be added
|
||||
assert.ok(settings.permissions.allow.includes('Bash(npx gsd-core *)'),
|
||||
'GSD allow entry must be added');
|
||||
assert.ok(settings.permissions.deny.includes('Read(.env)'),
|
||||
'GSD deny entry must be added');
|
||||
});
|
||||
|
||||
test('does not duplicate entries on repeated calls (idempotent)', () => {
|
||||
const 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 two merges`);
|
||||
}
|
||||
for (const entry of GSD_CLAUDE_DENY_PERMISSIONS) {
|
||||
const count = settings.permissions.deny.filter((e) => e === entry).length;
|
||||
assert.strictEqual(count, 1, `deny entry "${entry}" must appear exactly once after two merges`);
|
||||
}
|
||||
});
|
||||
|
||||
test('preserves other permission sub-keys (ask, disableBypassPermissionsMode)', () => {
|
||||
const settings = {
|
||||
permissions: {
|
||||
ask: ['Bash'],
|
||||
disableBypassPermissionsMode: 'disable',
|
||||
allow: [],
|
||||
deny: [],
|
||||
},
|
||||
};
|
||||
mergeClaudePermissions(settings);
|
||||
assert.deepStrictEqual(settings.permissions.ask, ['Bash'],
|
||||
'permissions.ask must be preserved');
|
||||
assert.strictEqual(settings.permissions.disableBypassPermissionsMode, 'disable',
|
||||
'permissions.disableBypassPermissionsMode must be preserved');
|
||||
});
|
||||
|
||||
test('handles permissions with non-array allow/deny gracefully (replaces with array)', () => {
|
||||
// If allow/deny exist but are not arrays (malformed settings), must not crash
|
||||
// and must result in valid arrays.
|
||||
const settings = { permissions: { allow: null, deny: null } };
|
||||
mergeClaudePermissions(settings);
|
||||
assert.ok(Array.isArray(settings.permissions.allow));
|
||||
assert.ok(Array.isArray(settings.permissions.deny));
|
||||
assert.ok(settings.permissions.allow.includes('Bash(npx gsd-core *)'));
|
||||
});
|
||||
|
||||
test('handles settings that are not plain objects (returns unchanged)', () => {
|
||||
// Guard: if settings is not a plain object, do nothing
|
||||
const badInputs = [null, undefined, [], 'string', 42];
|
||||
for (const bad of badInputs) {
|
||||
// Must not throw
|
||||
assert.doesNotThrow(() => mergeClaudePermissions(bad),
|
||||
`mergeClaudePermissions must not throw on: ${JSON.stringify(bad)}`);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
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-');
|
||||
t.after(() => cleanup(root));
|
||||
|
||||
const result = spawnSync(
|
||||
process.execPath,
|
||||
[INSTALL_SCRIPT, '--claude', '--global', '--config-dir', root],
|
||||
{ encoding: 'utf8', env: { ...process.env, HOME: root, USERPROFILE: root } },
|
||||
);
|
||||
|
||||
assert.strictEqual(result.status, 0,
|
||||
`installer exited ${result.status}\n${result.stdout}\n${result.stderr}`);
|
||||
|
||||
const settingsPath = path.join(root, 'settings.json');
|
||||
assert.ok(fs.existsSync(settingsPath), 'settings.json must exist after claude install');
|
||||
|
||||
const settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8'));
|
||||
assert.ok(Array.isArray(settings.permissions?.allow),
|
||||
'settings.json must have permissions.allow array');
|
||||
assert.ok(Array.isArray(settings.permissions?.deny),
|
||||
'settings.json must have permissions.deny array');
|
||||
|
||||
assert.ok(settings.permissions.allow.includes('Bash(npx gsd-core *)'),
|
||||
'settings.json permissions.allow must include Bash(npx gsd-core *)');
|
||||
assert.ok(settings.permissions.allow.includes('Read(.planning/*)'),
|
||||
'settings.json permissions.allow must include Read(.planning/*)');
|
||||
assert.ok(settings.permissions.deny.includes('Read(.env)'),
|
||||
'settings.json permissions.deny must include Read(.env)');
|
||||
});
|
||||
|
||||
test('non-claude runtime (gemini) does NOT write GSD allow/deny permissions to settings.json', (t) => {
|
||||
const root = createTempDir('gsd-gemini-perm-install-');
|
||||
t.after(() => cleanup(root));
|
||||
|
||||
const result = spawnSync(
|
||||
process.execPath,
|
||||
[INSTALL_SCRIPT, '--gemini', '--global', '--config-dir', root],
|
||||
{ encoding: 'utf8', env: { ...process.env, HOME: root, USERPROFILE: root } },
|
||||
);
|
||||
|
||||
assert.strictEqual(result.status, 0,
|
||||
`installer exited ${result.status}\n${result.stdout}\n${result.stderr}`);
|
||||
|
||||
const settingsPath = path.join(root, 'settings.json');
|
||||
// If settings.json doesn't exist, permissions are definitely not written — pass.
|
||||
if (fs.existsSync(settingsPath)) {
|
||||
const settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8'));
|
||||
const allow = settings.permissions?.allow ?? [];
|
||||
assert.ok(!allow.includes('Bash(npx gsd-core *)'),
|
||||
'Gemini settings.json must NOT include Bash(npx gsd-core *) in permissions.allow');
|
||||
}
|
||||
});
|
||||
|
||||
test('--claude --global reinstall is idempotent (no duplicate permission entries)', (t) => {
|
||||
const root = createTempDir('gsd-claude-perm-idempotent-');
|
||||
t.after(() => cleanup(root));
|
||||
|
||||
const spawnOpts = {
|
||||
encoding: 'utf8',
|
||||
env: { ...process.env, HOME: root, USERPROFILE: root },
|
||||
};
|
||||
const args = [INSTALL_SCRIPT, '--claude', '--global', '--config-dir', root];
|
||||
|
||||
// First install
|
||||
const r1 = spawnSync(process.execPath, args, spawnOpts);
|
||||
assert.strictEqual(r1.status, 0, `first install failed: ${r1.stderr}`);
|
||||
|
||||
// Second install (reinstall)
|
||||
const r2 = spawnSync(process.execPath, args, spawnOpts);
|
||||
assert.strictEqual(r2.status, 0, `reinstall failed: ${r2.stderr}`);
|
||||
|
||||
const settings = JSON.parse(fs.readFileSync(path.join(root, 'settings.json'), 'utf8'));
|
||||
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 two installs`);
|
||||
}
|
||||
for (const entry of GSD_CLAUDE_DENY_PERMISSIONS) {
|
||||
const count = (settings.permissions?.deny ?? []).filter((e) => e === entry).length;
|
||||
assert.strictEqual(count, 1,
|
||||
`deny entry "${entry}" must appear exactly once after two installs`);
|
||||
}
|
||||
});
|
||||
|
||||
test('--claude --global uninstall removes GSD permission entries from settings.json', (t) => {
|
||||
const root = createTempDir('gsd-claude-perm-uninstall-');
|
||||
t.after(() => cleanup(root));
|
||||
|
||||
const spawnOpts = {
|
||||
encoding: 'utf8',
|
||||
env: { ...process.env, HOME: root, USERPROFILE: root },
|
||||
};
|
||||
|
||||
// Install first
|
||||
const r1 = spawnSync(
|
||||
process.execPath,
|
||||
[INSTALL_SCRIPT, '--claude', '--global', '--config-dir', root],
|
||||
spawnOpts,
|
||||
);
|
||||
assert.strictEqual(r1.status, 0, `install failed: ${r1.stderr}`);
|
||||
|
||||
// Verify permissions were written
|
||||
const settingsPath = path.join(root, 'settings.json');
|
||||
const afterInstall = JSON.parse(fs.readFileSync(settingsPath, 'utf8'));
|
||||
assert.ok((afterInstall.permissions?.allow ?? []).includes('Bash(npx gsd-core *)'),
|
||||
'permissions.allow must contain GSD entry after install');
|
||||
|
||||
// Now add a user permission to make sure we don't nuke it
|
||||
afterInstall.permissions.allow.push('Bash(git *)');
|
||||
afterInstall.permissions.deny.push('WebSearch');
|
||||
fs.writeFileSync(settingsPath, JSON.stringify(afterInstall, 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 ?? [];
|
||||
const deny = afterUninstall.permissions?.deny ?? [];
|
||||
|
||||
// GSD entries must be removed
|
||||
assert.ok(!allow.includes('Bash(npx gsd-core *)'),
|
||||
'GSD Bash allow entry must be removed by uninstall');
|
||||
assert.ok(!allow.includes('Read(.planning/*)'),
|
||||
'GSD Read(.planning/*) allow entry must be removed by uninstall');
|
||||
assert.ok(!deny.includes('Read(.env)'),
|
||||
'GSD Read(.env) deny entry must be removed by uninstall');
|
||||
|
||||
// User entries must survive
|
||||
assert.ok(allow.includes('Bash(git *)'),
|
||||
'user Bash(git *) allow entry must survive uninstall');
|
||||
assert.ok(deny.includes('WebSearch'),
|
||||
'user WebSearch deny entry must survive uninstall');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user