feat(cli): deprecate --allowed-tools and excludeTools in favor of policy engine (#18508)

This commit is contained in:
Abhijit Balaji
2026-02-11 16:49:48 -08:00
committed by GitHub
parent c370d2397b
commit 0e85e021dc
9 changed files with 327 additions and 39 deletions

View File

@@ -383,7 +383,9 @@ export interface ConfigParameters {
question?: string;
coreTools?: string[];
/** @deprecated Use Policy Engine instead */
allowedTools?: string[];
/** @deprecated Use Policy Engine instead */
excludeTools?: string[];
toolDiscoveryCommand?: string;
toolCallCommand?: string;
@@ -516,7 +518,9 @@ export class Config {
private readonly question: string | undefined;
private readonly coreTools: string[] | undefined;
/** @deprecated Use Policy Engine instead */
private readonly allowedTools: string[] | undefined;
/** @deprecated Use Policy Engine instead */
private readonly excludeTools: string[] | undefined;
private readonly toolDiscoveryCommand: string | undefined;
private readonly toolCallCommand: string | undefined;
@@ -1487,11 +1491,12 @@ export class Config {
/**
* All the excluded tools from static configuration, loaded extensions, or
* other sources.
* other sources (like the Policy Engine).
*
* May change over time.
*/
getExcludeTools(): Set<string> | undefined {
// Right now this is present for backward compatibility with settings.json exclude
const excludeToolsSet = new Set([...(this.excludeTools ?? [])]);
for (const extension of this.getExtensionLoader().getExtensions()) {
if (!extension.isActive) {
@@ -1501,6 +1506,12 @@ export class Config {
excludeToolsSet.add(tool);
}
}
const policyExclusions = this.policyEngine.getExcludedTools();
for (const tool of policyExclusions) {
excludeToolsSet.add(tool);
}
return excludeToolsSet;
}

View File

@@ -2031,6 +2031,156 @@ describe('PolicyEngine', () => {
});
});
describe('getExcludedTools', () => {
interface TestCase {
name: string;
rules: PolicyRule[];
approvalMode?: ApprovalMode;
nonInteractive?: boolean;
expected: string[];
}
const testCases: TestCase[] = [
{
name: 'should return empty set when no rules provided',
rules: [],
expected: [],
},
{
name: 'should include tools with DENY decision',
rules: [
{ toolName: 'tool1', decision: PolicyDecision.DENY },
{ toolName: 'tool2', decision: PolicyDecision.ALLOW },
],
expected: ['tool1'],
},
{
name: 'should respect priority and ignore lower priority rules (DENY wins)',
rules: [
{ toolName: 'tool1', decision: PolicyDecision.DENY, priority: 100 },
{ toolName: 'tool1', decision: PolicyDecision.ALLOW, priority: 10 },
],
expected: ['tool1'],
},
{
name: 'should respect priority and ignore lower priority rules (ALLOW wins)',
rules: [
{ toolName: 'tool1', decision: PolicyDecision.ALLOW, priority: 100 },
{ toolName: 'tool1', decision: PolicyDecision.DENY, priority: 10 },
],
expected: [],
},
{
name: 'should NOT include ASK_USER tools even in non-interactive mode',
rules: [{ toolName: 'tool1', decision: PolicyDecision.ASK_USER }],
nonInteractive: true,
expected: [],
},
{
name: 'should ignore rules with argsPattern',
rules: [
{
toolName: 'tool1',
decision: PolicyDecision.DENY,
argsPattern: /something/,
},
],
expected: [],
},
{
name: 'should respect approval mode (PLAN mode)',
rules: [
{
toolName: 'tool1',
decision: PolicyDecision.DENY,
modes: [ApprovalMode.PLAN],
},
],
approvalMode: ApprovalMode.PLAN,
expected: ['tool1'],
},
{
name: 'should respect approval mode (DEFAULT mode)',
rules: [
{
toolName: 'tool1',
decision: PolicyDecision.DENY,
modes: [ApprovalMode.PLAN],
},
],
approvalMode: ApprovalMode.DEFAULT,
expected: [],
},
{
name: 'should respect wildcard ALLOW rules (e.g. YOLO mode)',
rules: [
{
decision: PolicyDecision.ALLOW,
priority: 999,
modes: [ApprovalMode.YOLO],
},
{
toolName: 'dangerous-tool',
decision: PolicyDecision.DENY,
priority: 10,
},
],
approvalMode: ApprovalMode.YOLO,
expected: [],
},
{
name: 'should respect server wildcard DENY',
rules: [{ toolName: 'server__*', decision: PolicyDecision.DENY }],
expected: ['server__*'],
},
{
name: 'should expand server wildcard for specific tools if already processed',
rules: [
{
toolName: 'server__*',
decision: PolicyDecision.DENY,
priority: 100,
},
{
toolName: 'server__tool1',
decision: PolicyDecision.DENY,
priority: 10,
},
],
expected: ['server__*', 'server__tool1'],
},
{
name: 'should NOT exclude tool if covered by a higher priority wildcard ALLOW',
rules: [
{
toolName: 'server__*',
decision: PolicyDecision.ALLOW,
priority: 100,
},
{
toolName: 'server__tool1',
decision: PolicyDecision.DENY,
priority: 10,
},
],
expected: [],
},
];
it.each(testCases)(
'$name',
({ rules, approvalMode, nonInteractive, expected }) => {
engine = new PolicyEngine({
rules,
approvalMode: approvalMode ?? ApprovalMode.DEFAULT,
nonInteractive: nonInteractive ?? false,
});
const excluded = engine.getExcludedTools();
expect(Array.from(excluded).sort()).toEqual(expected.sort());
},
);
});
describe('YOLO mode with ask_user tool', () => {
it('should return ASK_USER for ask_user tool even in YOLO mode', async () => {
const rules: PolicyRule[] = [

View File

@@ -26,6 +26,22 @@ import {
} from '../utils/shell-utils.js';
import { getToolAliases } from '../tools/tool-names.js';
function isWildcardPattern(name: string): boolean {
return name.endsWith('__*');
}
function getWildcardPrefix(pattern: string): string {
return pattern.slice(0, -3);
}
function matchesWildcard(pattern: string, toolName: string): boolean {
if (!isWildcardPattern(pattern)) {
return false;
}
const prefix = getWildcardPrefix(pattern);
return toolName.startsWith(prefix + '__');
}
function ruleMatches(
rule: PolicyRule | SafetyCheckerRule,
toolCall: FunctionCall,
@@ -43,8 +59,8 @@ function ruleMatches(
// Check tool name if specified
if (rule.toolName) {
// Support wildcard patterns: "serverName__*" matches "serverName__anyTool"
if (rule.toolName.endsWith('__*')) {
const prefix = rule.toolName.slice(0, -3); // Remove "__*"
if (isWildcardPattern(rule.toolName)) {
const prefix = getWildcardPrefix(rule.toolName);
if (serverName !== undefined) {
// Robust check: if serverName is provided, it MUST match the prefix exactly.
// This prevents "malicious-server" from spoofing "trusted-server" by naming itself "trusted-server__malicious".
@@ -53,7 +69,7 @@ function ruleMatches(
}
}
// Always verify the prefix, even if serverName matched
if (!toolCall.name || !toolCall.name.startsWith(prefix + '__')) {
if (!toolCall.name || !matchesWildcard(rule.toolName, toolCall.name)) {
return false;
}
} else if (toolCall.name !== rule.toolName) {
@@ -509,6 +525,90 @@ export class PolicyEngine {
return this.hookCheckers;
}
/**
* Get tools that are effectively denied by the current rules.
* This takes into account:
* 1. Global rules (no argsPattern)
* 2. Priority order (higher priority wins)
* 3. Non-interactive mode (ASK_USER becomes DENY)
*/
getExcludedTools(): Set<string> {
const excludedTools = new Set<string>();
const processedTools = new Set<string>();
let globalVerdict: PolicyDecision | undefined;
for (const rule of this.rules) {
// We only care about rules without args pattern for exclusion from the model
if (rule.argsPattern) {
continue;
}
// Check if rule applies to current approval mode
if (rule.modes && rule.modes.length > 0) {
if (!rule.modes.includes(this.approvalMode)) {
continue;
}
}
// Handle Global Rules
if (!rule.toolName) {
if (globalVerdict === undefined) {
globalVerdict = rule.decision;
if (globalVerdict !== PolicyDecision.DENY) {
// Global ALLOW/ASK found.
// Since rules are sorted by priority, this overrides any lower-priority rules.
// We can stop processing because nothing else will be excluded.
break;
}
// If Global DENY, we continue to find specific tools to add to excluded set
}
continue;
}
const toolName = rule.toolName;
// Check if already processed (exact match)
if (processedTools.has(toolName)) {
continue;
}
// Check if covered by a processed wildcard
let coveredByWildcard = false;
for (const processed of processedTools) {
if (
isWildcardPattern(processed) &&
matchesWildcard(processed, toolName)
) {
// It's covered by a higher-priority wildcard rule.
// If that wildcard rule resulted in exclusion, this tool should also be excluded.
if (excludedTools.has(processed)) {
excludedTools.add(toolName);
}
coveredByWildcard = true;
break;
}
}
if (coveredByWildcard) {
continue;
}
processedTools.add(toolName);
// Determine decision
let decision: PolicyDecision;
if (globalVerdict !== undefined) {
decision = globalVerdict;
} else {
decision = rule.decision;
}
if (decision === PolicyDecision.DENY) {
excludedTools.add(toolName);
}
}
return excludedTools;
}
private applyNonInteractiveMode(decision: PolicyDecision): PolicyDecision {
// In non-interactive mode, ASK_USER becomes DENY
if (this.nonInteractive && decision === PolicyDecision.ASK_USER) {