diff --git a/packages/core/src/permissions/permission-manager.test.ts b/packages/core/src/permissions/permission-manager.test.ts index 5b1048d5c..599c4c4d0 100644 --- a/packages/core/src/permissions/permission-manager.test.ts +++ b/packages/core/src/permissions/permission-manager.test.ts @@ -1254,6 +1254,57 @@ describe('PermissionManager', () => { expect(await pm.isToolEnabled('read_file')).toBe(true); expect(await pm.isToolEnabled('run_shell_command')).toBe(true); // not denied, just unreviewed }); + + // Non-core tools bypass coreTools allowlist + it('MCP tools bypass coreTools allowlist check', async () => { + pm = new PermissionManager(makeConfig({ coreTools: ['read_file'] })); + pm.initialize(); + // MCP tools should be enabled even if not in coreTools + expect( + await pm.isToolEnabled('mcp__markitdown__convert_to_markdown'), + ).toBe(true); + expect(await pm.isToolEnabled('mcp__puppeteer__navigate')).toBe(true); + }); + + it('Skill tool bypasses coreTools allowlist check', async () => { + pm = new PermissionManager(makeConfig({ coreTools: ['read_file'] })); + pm.initialize(); + expect(await pm.isToolEnabled('skill')).toBe(true); + }); + + it('Agent tool bypasses coreTools allowlist check', async () => { + pm = new PermissionManager(makeConfig({ coreTools: ['read_file'] })); + pm.initialize(); + expect(await pm.isToolEnabled('agent')).toBe(true); + }); + + it('exit_plan_mode tool bypasses coreTools allowlist check', async () => { + pm = new PermissionManager(makeConfig({ coreTools: ['read_file'] })); + pm.initialize(); + expect(await pm.isToolEnabled('exit_plan_mode')).toBe(true); + }); + + it('ask_user_question tool bypasses coreTools allowlist check', async () => { + pm = new PermissionManager(makeConfig({ coreTools: ['read_file'] })); + pm.initialize(); + expect(await pm.isToolEnabled('ask_user_question')).toBe(true); + }); + + it('Non-core tools still respect deny rules', async () => { + pm = new PermissionManager( + makeConfig({ + coreTools: ['read_file'], + permissionsDeny: ['mcp__markitdown'], + }), + ); + pm.initialize(); + // MCP tool should be disabled due to deny rule, even though it bypasses coreTools + expect( + await pm.isToolEnabled('mcp__markitdown__convert_to_markdown'), + ).toBe(false); + // Other MCP tools without deny rule should still be enabled + expect(await pm.isToolEnabled('mcp__puppeteer__navigate')).toBe(true); + }); }); describe('session rules', () => { diff --git a/packages/core/src/permissions/permission-manager.ts b/packages/core/src/permissions/permission-manager.ts index e17fac881..3f38347f9 100644 --- a/packages/core/src/permissions/permission-manager.ts +++ b/packages/core/src/permissions/permission-manager.ts @@ -396,6 +396,35 @@ export class PermissionManager { // Registry-level helper // --------------------------------------------------------------------------- + /** + * Core tools that are subject to the coreTools allowlist check. + * Tools not in this set (MCP, Skill, Agent, etc.) bypass the check. + */ + private static readonly CORE_TOOLS = new Set([ + 'read_file', + 'write_file', + 'edit', + 'glob', + 'grep_search', + 'run_shell_command', + 'list_directory', + 'web_fetch', + 'web_search', + 'todo_write', + 'save_memory', + 'lsp', + 'cron_create', + 'cron_list', + 'cron_delete', + ]); + + /** + * Check if a tool is a core tool subject to the coreTools allowlist check. + */ + private isCoreTool(toolName: string): boolean { + return PermissionManager.CORE_TOOLS.has(toolName); + } + /** * Determine whether a tool should be present in the tool registry. * @@ -403,13 +432,23 @@ export class PermissionManager { * (i.e. a whole-tool deny) matches. Specifier-based deny rules such as * `"Bash(rm -rf *)"` do NOT remove the tool from the registry – they only * deny specific invocations at runtime. + * + * Non-core tools (MCP, Skill, Agent, etc.) skip the coreTools allowlist + * check because they are dynamically discovered or essential for system + * operation. */ async isToolEnabled(toolName: string): Promise { const canonicalName = resolveToolName(toolName); - // If a coreTools allowlist is active, only explicitly listed tools are - // registered. This mirrors the legacy `tools.core` whitelist semantic: - // any tool NOT in the allowlist is excluded from the registry entirely. + // Non-core tools bypass coreTools allowlist check + if (!this.isCoreTool(canonicalName)) { + const decision = await this.evaluate({ toolName: canonicalName }); + return decision !== 'deny'; + } + + // Core tools: if a coreTools allowlist is active, only explicitly listed + // tools are registered. This mirrors the legacy `tools.core` whitelist + // semantic: any tool NOT in the allowlist is excluded from the registry. if (this.coreToolsAllowList !== null && this.coreToolsAllowList.size > 0) { if (!this.coreToolsAllowList.has(canonicalName)) { return false;