mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-04 22:51:08 +00:00
fix(permissions): allow non-core tools to bypass coreTools allowlist
The coreTools configuration was incorrectly restricting all tools including MCP, Skill, Agent, and other dynamically discovered tools. These tools should not be subject to the coreTools whitelist as they are either: - Dynamically discovered from user configuration (MCP tools) - Essential for system operation (skill, agent, exit_plan_mode, ask_user_question) This fix introduces a CORE_TOOLS set that explicitly lists built-in tools subject to coreTools allowlist. Tools not in this set bypass the check. Fixes #2782 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This commit is contained in:
parent
de66ee198e
commit
ce3dfab18c
2 changed files with 93 additions and 3 deletions
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -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<boolean> {
|
||||
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;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue