diff --git a/packages/core/src/permissions/permission-manager.test.ts b/packages/core/src/permissions/permission-manager.test.ts index 7196615ba2d..1a61fd93fdf 100644 --- a/packages/core/src/permissions/permission-manager.test.ts +++ b/packages/core/src/permissions/permission-manager.test.ts @@ -137,6 +137,11 @@ describe('parseRule', () => { expect(r.toolName).toBe('run_shell_command'); }); + it('parses Monitor alias', async () => { + const r = parseRule('Monitor'); + expect(r.toolName).toBe('monitor'); + }); + it('parses a shell tool with a specifier', async () => { const r = parseRule('Bash(git *)'); expect(r.toolName).toBe('run_shell_command'); @@ -144,6 +149,13 @@ describe('parseRule', () => { expect(r.specifierKind).toBe('command'); }); + it('parses Monitor with command specifier', async () => { + const r = parseRule('Monitor(tail -f *)'); + expect(r.toolName).toBe('monitor'); + expect(r.specifier).toBe('tail -f *'); + expect(r.specifierKind).toBe('command'); + }); + it('parses Read with path specifier', async () => { const r = parseRule('Read(./secrets/**)'); expect(r.toolName).toBe('read_file'); @@ -645,6 +657,28 @@ describe('matchesRule', () => { expect(matchesRule(rule, 'run_shell_command', 'echo hello')).toBe(false); }); + // Monitor command specifier + it('Monitor rule matches monitor invocations with command specifier', async () => { + const rule = parseRule('Monitor(tail -f *)'); + expect(matchesRule(rule, 'monitor')).toBe(false); // no command + expect(matchesRule(rule, 'monitor', 'tail -f /var/log/app.log')).toBe(true); + expect(matchesRule(rule, 'monitor', 'echo hello')).toBe(false); + }); + + it('Monitor rule does not match run_shell_command', async () => { + const rule = parseRule('Monitor(tail -f *)'); + expect( + matchesRule(rule, 'run_shell_command', 'tail -f /var/log/app.log'), + ).toBe(false); + }); + + it('Bash rule does not match monitor', async () => { + const rule = parseRule('Bash(tail -f *)'); + expect(matchesRule(rule, 'monitor', 'tail -f /var/log/app.log')).toBe( + false, + ); + }); + it('matchesRule checks individual simple commands (compound splitting is at PM level)', async () => { const rule = parseRule('Bash(git *)'); // matchesRule receives a simple command (already split by PM) @@ -933,6 +967,81 @@ describe('PermissionManager', () => { }); }); + describe('monitor command-level evaluation', () => { + it('Monitor(...) allow rule matches monitor invocations', async () => { + const pm2 = new PermissionManager( + makeConfig({ + permissionsAllow: ['Monitor(tail -f *)'], + }), + ); + pm2.initialize(); + expect( + await pm2.evaluate({ + toolName: 'monitor', + command: 'tail -f /var/log/app.log', + }), + ).toBe('allow'); + }); + + it('Monitor(...) deny rule blocks monitor invocations', async () => { + const pm2 = new PermissionManager( + makeConfig({ + permissionsDeny: ['Monitor(rm *)'], + }), + ); + pm2.initialize(); + expect( + await pm2.evaluate({ + toolName: 'monitor', + command: 'rm -rf /', + }), + ).toBe('deny'); + }); + + it('Monitor approval does NOT allow run_shell_command', async () => { + const pm2 = new PermissionManager( + makeConfig({ + permissionsAllow: ['Monitor(npm *)'], + }), + ); + pm2.initialize(); + // Same command via shell tool should NOT be allowed by Monitor rule + expect( + await pm2.evaluate({ + toolName: 'run_shell_command', + command: 'npm install', + }), + ).not.toBe('allow'); + }); + + it('Bash approval does NOT allow monitor', async () => { + const pm2 = new PermissionManager( + makeConfig({ + permissionsAllow: ['Bash(npm *)'], + }), + ); + pm2.initialize(); + // Same command via monitor tool should NOT be allowed by Bash rule + expect( + await pm2.evaluate({ + toolName: 'monitor', + command: 'npm install', + }), + ).not.toBe('allow'); + }); + + it('resolves default to allow for readonly monitor commands', async () => { + const pm2 = new PermissionManager(makeConfig({})); + pm2.initialize(); + expect( + await pm2.evaluate({ + toolName: 'monitor', + command: 'echo hello', + }), + ).toBe('allow'); + }); + }); + describe('compound command evaluation', () => { it('all sub-commands allowed → allow', async () => { pm = new PermissionManager( @@ -1691,6 +1800,19 @@ describe('buildPermissionRules', () => { const rules = buildPermissionRules({ toolName: 'run_shell_command' }); expect(rules).toEqual(['Bash']); }); + + it('generates Monitor rule with command specifier', async () => { + const rules = buildPermissionRules({ + toolName: 'monitor', + command: 'tail -f /var/log/app.log', + }); + expect(rules).toEqual(['Monitor(tail -f /var/log/app.log)']); + }); + + it('falls back to bare Monitor display name when no command', async () => { + const rules = buildPermissionRules({ toolName: 'monitor' }); + expect(rules).toEqual(['Monitor']); + }); }); describe('literal-specifier tools', () => { @@ -1741,6 +1863,10 @@ describe('buildHumanReadableRuleLabel', () => { expect(buildHumanReadableRuleLabel(['Bash'])).toBe('run commands'); }); + it('converts bare Monitor rule to "monitor commands"', () => { + expect(buildHumanReadableRuleLabel(['Monitor'])).toBe('monitor commands'); + }); + it('converts Read with absolute path specifier', () => { const label = buildHumanReadableRuleLabel(['Read(//Users/mochi/.qwen/**)']); expect(label).toBe('read files in /Users/mochi/.qwen/'); @@ -1761,6 +1887,11 @@ describe('buildHumanReadableRuleLabel', () => { expect(label).toBe("run 'git *' commands"); }); + it('converts Monitor with command specifier', () => { + const label = buildHumanReadableRuleLabel(['Monitor(tail -f *)']); + expect(label).toBe("monitor 'tail -f *' commands"); + }); + it('converts WebFetch with domain specifier', () => { const label = buildHumanReadableRuleLabel(['WebFetch(github.com)']); expect(label).toBe('fetch from github.com'); diff --git a/packages/core/src/permissions/permission-manager.ts b/packages/core/src/permissions/permission-manager.ts index f6e5c11ae62..1d6fdf2bf98 100644 --- a/packages/core/src/permissions/permission-manager.ts +++ b/packages/core/src/permissions/permission-manager.ts @@ -29,6 +29,13 @@ import type { const debugLogger = createDebugLogger('PERMISSIONS'); +/** + * Tools that spawn shell commands and share the same permission evaluation + * semantics: compound-command splitting, AST read-only analysis, and + * virtual file/network operation matching. + */ +const SHELL_LIKE_TOOLS = new Set(['run_shell_command', 'monitor']); + /** * Numeric priority for each PermissionDecision. * Higher number = more restrictive. Used to combine decisions by taking @@ -178,7 +185,7 @@ export class PermissionManager { // a concrete permission (deny/ask/allow) based on the command's readonly status. if ( decision === 'default' && - toolName === 'run_shell_command' && + SHELL_LIKE_TOOLS.has(toolName) && command !== undefined ) { return this.resolveDefaultPermission(command); @@ -256,7 +263,7 @@ export class PermissionManager { // must never downgrade an explicit 'allow' decision from a Bash rule. // Example: `git status` has no file ops; an allow rule for `Bash(git *)` // should return 'allow', not be downgraded to 'default'. - if (toolName === 'run_shell_command' && command !== undefined) { + if (SHELL_LIKE_TOOLS.has(toolName) && command !== undefined) { const cwd = pathCtx?.cwd ?? process.cwd(); const virtualDecision = this.evaluateShellVirtualOps( extractShellOperations(command, cwd), @@ -546,7 +553,7 @@ export class PermissionManager { hasRelevantRules(ctx: PermissionCheckContext): boolean { const { toolName, command, filePath, domain, specifier } = ctx; - if (ctx.toolName === 'run_shell_command' && command !== undefined) { + if (SHELL_LIKE_TOOLS.has(ctx.toolName) && command !== undefined) { const subCommands = splitCompoundCommand(command); if (subCommands.length > 1) { return subCommands.some((subCmd) => @@ -587,7 +594,7 @@ export class PermissionManager { // extracted from the command has a relevant rule. This ensures the PM is // consulted (and the confirmation dialog shown) when Read/Edit/etc. rules // would match equivalent shell commands. - if (ctx.toolName === 'run_shell_command' && ctx.command !== undefined) { + if (SHELL_LIKE_TOOLS.has(ctx.toolName) && ctx.command !== undefined) { const cwd = pathCtx?.cwd ?? process.cwd(); const ops = extractShellOperations(ctx.command, cwd); if ( @@ -622,7 +629,7 @@ export class PermissionManager { hasMatchingAskRule(ctx: PermissionCheckContext): boolean { const { toolName, command, filePath, domain, specifier } = ctx; - if (ctx.toolName === 'run_shell_command' && command !== undefined) { + if (SHELL_LIKE_TOOLS.has(ctx.toolName) && command !== undefined) { const subCommands = splitCompoundCommand(command); if (subCommands.length > 1) { return subCommands.some((subCmd) => @@ -654,7 +661,7 @@ export class PermissionManager { return true; } - if (ctx.toolName === 'run_shell_command' && ctx.command !== undefined) { + if (SHELL_LIKE_TOOLS.has(ctx.toolName) && ctx.command !== undefined) { const cwd = pathCtx?.cwd ?? process.cwd(); const ops = extractShellOperations(ctx.command, cwd); return ops.some((op) => { diff --git a/packages/core/src/permissions/rule-parser.ts b/packages/core/src/permissions/rule-parser.ts index b034069c57c..12951e6f477 100644 --- a/packages/core/src/permissions/rule-parser.ts +++ b/packages/core/src/permissions/rule-parser.ts @@ -118,6 +118,11 @@ export const TOOL_NAME_ALIASES: Readonly> = { Lsp: 'lsp', LspTool: 'lsp', + // Monitor tool + monitor: 'monitor', + Monitor: 'monitor', + MonitorTool: 'monitor', + // Legacy edit tool name replace: 'edit', }; @@ -125,7 +130,7 @@ export const TOOL_NAME_ALIASES: Readonly> = { /** * Shell tool canonical names. */ -const SHELL_TOOL_NAMES = new Set(['run_shell_command']); +const SHELL_TOOL_NAMES = new Set(['run_shell_command', 'monitor']); /** * File-reading tools — "Read" rules apply to all of these (best-effort). @@ -305,6 +310,8 @@ const CANONICAL_TO_RULE_DISPLAY: Readonly> = { write_file: 'Edit', // Shell run_shell_command: 'Bash', + // Monitor + monitor: 'Monitor', // Web web_fetch: 'WebFetch', // Agent / Skill @@ -419,6 +426,7 @@ const DISPLAY_NAME_TO_VERB: Readonly> = { Read: 'read files', Edit: 'edit files', Bash: 'run commands', + Monitor: 'monitor commands', WebFetch: 'fetch from', Agent: 'use agent', Skill: 'use skill', @@ -494,9 +502,13 @@ export function buildHumanReadableRuleLabel(rules: string[]): string { parts.push(`${verb} in ${cleanPath}`); break; } - case 'command': - parts.push(`run '${specifier}' commands`); + case 'command': { + const cmdVerb = DISPLAY_NAME_TO_VERB[displayName] ?? 'run'; + // Extract just the verb word (e.g. "run commands" → "run", "monitor commands" → "monitor") + const verbWord = cmdVerb.split(' ')[0]!; + parts.push(`${verbWord} '${specifier}' commands`); break; + } case 'domain': parts.push(`${verb} ${specifier}`); break; diff --git a/packages/core/src/tools/monitor.test.ts b/packages/core/src/tools/monitor.test.ts index 901e0d814e4..c6d1ffc1563 100644 --- a/packages/core/src/tools/monitor.test.ts +++ b/packages/core/src/tools/monitor.test.ts @@ -151,7 +151,7 @@ describe('MonitorTool', () => { }; expect(details.type).toBe('exec'); - expect(details.permissionRules).toEqual(['Bash(tail -f *)']); + expect(details.permissionRules).toEqual(['Monitor(tail -f *)']); }); it('does not consult Bash permission rules for monitor commands', async () => { @@ -183,8 +183,8 @@ describe('MonitorTool', () => { expect(pm.isCommandAllowed).not.toHaveBeenCalled(); // Both subcommands remain in confirmation scope expect(details.permissionRules).toEqual([ - 'Bash(git add *)', - 'Bash(git commit *)', + 'Monitor(git add *)', + 'Monitor(git commit *)', ]); }); @@ -201,7 +201,9 @@ describe('MonitorTool', () => { permissionRules?: string[]; }; - expect(details.permissionRules).toEqual(['Bash(python -c "print(1)")']); + expect(details.permissionRules).toEqual([ + 'Monitor(python -c "print(1)")', + ]); }); }); diff --git a/packages/core/src/tools/monitor.ts b/packages/core/src/tools/monitor.ts index 87a80d02130..1b4265dfab1 100644 --- a/packages/core/src/tools/monitor.ts +++ b/packages/core/src/tools/monitor.ts @@ -139,10 +139,12 @@ class MonitorToolInvocation extends BaseToolInvocation< const rules = await extractCommandRules(sub); allRules.push(...rules); } - permissionRules = [...new Set(allRules)].map((rule) => `Bash(${rule})`); + permissionRules = [...new Set(allRules)].map( + (rule) => `Monitor(${rule})`, + ); } catch (e) { debugLogger.warn('Failed to extract monitor command rules:', e); - permissionRules = [`Bash(${command})`]; + permissionRules = [`Monitor(${command})`]; } return {