diff --git a/docs/cli/settings.md b/docs/cli/settings.md index 89f1333c82..9b5318f42e 100644 --- a/docs/cli/settings.md +++ b/docs/cli/settings.md @@ -127,6 +127,7 @@ they appear in the UI. | ------------------------------------- | ----------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------- | | Tool Sandboxing | `security.toolSandboxing` | Experimental tool-level sandboxing (implementation in progress). | `false` | | Disable YOLO Mode | `security.disableYoloMode` | Disable YOLO mode, even if enabled by a flag. | `false` | +| Disable Always Allow | `security.disableAlwaysAllow` | Disable "Always allow" options in tool confirmation dialogs. | `false` | | Allow Permanent Tool Approval | `security.enablePermanentToolApproval` | Enable the "Allow for all future sessions" option in tool confirmation dialogs. | `false` | | Auto-add to Policy by Default | `security.autoAddToPolicyByDefault` | When enabled, the "Allow for all future sessions" option becomes the default choice for low-risk tools in trusted workspaces. | `false` | | Blocks extensions from Git | `security.blockGitExtensions` | Blocks installing and loading extensions from Git. | `false` | diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 6b67652745..50af23dce1 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -901,6 +901,12 @@ their corresponding top-level category object in your `settings.json` file. - **Default:** `false` - **Requires restart:** Yes +- **`security.disableAlwaysAllow`** (boolean): + - **Description:** Disable "Always allow" options in tool confirmation + dialogs. + - **Default:** `false` + - **Requires restart:** Yes + - **`security.enablePermanentToolApproval`** (boolean): - **Description:** Enable the "Allow for all future sessions" option in tool confirmation dialogs. @@ -1191,7 +1197,8 @@ their corresponding top-level category object in your `settings.json` file. #### `admin` - **`admin.secureModeEnabled`** (boolean): - - **Description:** If true, disallows yolo mode from being used. + - **Description:** If true, disallows YOLO mode and "Always allow" options + from being used. - **Default:** `false` - **`admin.extensions.enabled`** (boolean): diff --git a/packages/cli/src/acp/acpClient.test.ts b/packages/cli/src/acp/acpClient.test.ts index e2fc0f0d33..65b23247ef 100644 --- a/packages/cli/src/acp/acpClient.test.ts +++ b/packages/cli/src/acp/acpClient.test.ts @@ -176,6 +176,7 @@ describe('GeminiAgent', () => { getGemini31LaunchedSync: vi.fn().mockReturnValue(false), getHasAccessToPreviewModel: vi.fn().mockReturnValue(false), getCheckpointingEnabled: vi.fn().mockReturnValue(false), + getDisableAlwaysAllow: vi.fn().mockReturnValue(false), } as unknown as Mocked>>; mockSettings = { merged: { @@ -654,6 +655,7 @@ describe('Session', () => { getCheckpointingEnabled: vi.fn().mockReturnValue(false), getGitService: vi.fn().mockResolvedValue({} as GitService), waitForMcpInit: vi.fn(), + getDisableAlwaysAllow: vi.fn().mockReturnValue(false), } as unknown as Mocked; mockConnection = { sessionUpdate: vi.fn(), @@ -947,6 +949,61 @@ describe('Session', () => { ); }); + it('should exclude always allow options when disableAlwaysAllow is true', async () => { + mockConfig.getDisableAlwaysAllow = vi.fn().mockReturnValue(true); + const confirmationDetails = { + type: 'info', + onConfirm: vi.fn(), + }; + mockTool.build.mockReturnValue({ + getDescription: () => 'Test Tool', + toolLocations: () => [], + shouldConfirmExecute: vi.fn().mockResolvedValue(confirmationDetails), + execute: vi.fn().mockResolvedValue({ llmContent: 'Tool Result' }), + }); + + mockConnection.requestPermission.mockResolvedValue({ + outcome: { + outcome: 'selected', + optionId: ToolConfirmationOutcome.ProceedOnce, + }, + }); + + const stream1 = createMockStream([ + { + type: StreamEventType.CHUNK, + value: { + functionCalls: [{ name: 'test_tool', args: {} }], + }, + }, + ]); + const stream2 = createMockStream([ + { + type: StreamEventType.CHUNK, + value: { candidates: [] }, + }, + ]); + + mockChat.sendMessageStream + .mockResolvedValueOnce(stream1) + .mockResolvedValueOnce(stream2); + + await session.prompt({ + sessionId: 'session-1', + prompt: [{ type: 'text', text: 'Call tool' }], + }); + + expect(mockConnection.requestPermission).toHaveBeenCalledWith( + expect.objectContaining({ + options: expect.not.arrayContaining([ + expect.objectContaining({ + optionId: ToolConfirmationOutcome.ProceedAlways, + }), + ]), + }), + ); + }); + it('should use filePath for ACP diff content in permission request', async () => { const confirmationDetails = { type: 'edit', diff --git a/packages/cli/src/acp/acpClient.ts b/packages/cli/src/acp/acpClient.ts index c36e214d27..db2d04dab4 100644 --- a/packages/cli/src/acp/acpClient.ts +++ b/packages/cli/src/acp/acpClient.ts @@ -908,7 +908,7 @@ export class Session { const params: acp.RequestPermissionRequest = { sessionId: this.id, - options: toPermissionOptions(confirmationDetails), + options: toPermissionOptions(confirmationDetails, this.config), toolCall: { toolCallId: callId, status: 'pending', @@ -1457,60 +1457,76 @@ const basicPermissionOptions = [ function toPermissionOptions( confirmation: ToolCallConfirmationDetails, + config: Config, ): acp.PermissionOption[] { - switch (confirmation.type) { - case 'edit': - return [ - { + const disableAlwaysAllow = config.getDisableAlwaysAllow(); + const options: acp.PermissionOption[] = []; + + if (!disableAlwaysAllow) { + switch (confirmation.type) { + case 'edit': + options.push({ optionId: ToolConfirmationOutcome.ProceedAlways, name: 'Allow All Edits', kind: 'allow_always', - }, - ...basicPermissionOptions, - ]; - case 'exec': - return [ - { + }); + break; + case 'exec': + options.push({ optionId: ToolConfirmationOutcome.ProceedAlways, name: `Always Allow ${confirmation.rootCommand}`, kind: 'allow_always', - }, - ...basicPermissionOptions, - ]; - case 'mcp': - return [ - { - optionId: ToolConfirmationOutcome.ProceedAlwaysServer, - name: `Always Allow ${confirmation.serverName}`, - kind: 'allow_always', - }, - { - optionId: ToolConfirmationOutcome.ProceedAlwaysTool, - name: `Always Allow ${confirmation.toolName}`, - kind: 'allow_always', - }, - ...basicPermissionOptions, - ]; - case 'info': - return [ - { + }); + break; + case 'mcp': + options.push( + { + optionId: ToolConfirmationOutcome.ProceedAlwaysServer, + name: `Always Allow ${confirmation.serverName}`, + kind: 'allow_always', + }, + { + optionId: ToolConfirmationOutcome.ProceedAlwaysTool, + name: `Always Allow ${confirmation.toolName}`, + kind: 'allow_always', + }, + ); + break; + case 'info': + options.push({ optionId: ToolConfirmationOutcome.ProceedAlways, name: `Always Allow`, kind: 'allow_always', - }, - ...basicPermissionOptions, - ]; + }); + break; + case 'ask_user': + case 'exit_plan_mode': + // askuser and exit_plan_mode don't need "always allow" options + break; + default: + // No "always allow" options for other types + break; + } + } + + options.push(...basicPermissionOptions); + + // Exhaustive check + switch (confirmation.type) { + case 'edit': + case 'exec': + case 'mcp': + case 'info': case 'ask_user': - // askuser doesn't need "always allow" options since it's asking questions - return [...basicPermissionOptions]; case 'exit_plan_mode': - // exit_plan_mode doesn't need "always allow" options since it's a plan approval flow - return [...basicPermissionOptions]; + break; default: { const unreachable: never = confirmation; throw new Error(`Unexpected: ${unreachable}`); } } + + return options; } /** diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 769583ea62..cacbe814a5 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -785,6 +785,9 @@ export async function loadCliConfig( approvalMode, disableYoloMode: settings.security?.disableYoloMode || settings.admin?.secureModeEnabled, + disableAlwaysAllow: + settings.security?.disableAlwaysAllow || + settings.admin?.secureModeEnabled, showMemoryUsage: settings.ui?.showMemoryUsage || false, accessibility: { ...settings.ui?.accessibility, diff --git a/packages/cli/src/config/policy.ts b/packages/cli/src/config/policy.ts index 4bbd396fba..9837c2c355 100644 --- a/packages/cli/src/config/policy.ts +++ b/packages/cli/src/config/policy.ts @@ -63,6 +63,9 @@ export async function createPolicyEngineConfig( policyPaths: settings.policyPaths, adminPolicyPaths: settings.adminPolicyPaths, workspacePoliciesDir, + disableAlwaysAllow: + settings.security?.disableAlwaysAllow || + settings.admin?.secureModeEnabled, }; return createCorePolicyEngineConfig(policySettings, approvalMode); diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index af143afcc0..06129a4760 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -524,16 +524,19 @@ describe('Settings Loading and Merging', () => { const userSettingsContent = { security: { disableYoloMode: false, + disableAlwaysAllow: false, }, }; const workspaceSettingsContent = { security: { disableYoloMode: false, // This should be ignored + disableAlwaysAllow: false, // This should be ignored }, }; const systemSettingsContent = { security: { disableYoloMode: true, + disableAlwaysAllow: true, }, }; @@ -551,6 +554,7 @@ describe('Settings Loading and Merging', () => { const settings = loadSettings(MOCK_WORKSPACE_DIR); expect(settings.merged.security?.disableYoloMode).toBe(true); // System setting should be used + expect(settings.merged.security?.disableAlwaysAllow).toBe(true); // System setting should be used }); it.each([ diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 0f9be83236..bc56bde176 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -1541,6 +1541,16 @@ const SETTINGS_SCHEMA = { description: 'Disable YOLO mode, even if enabled by a flag.', showInDialog: true, }, + disableAlwaysAllow: { + type: 'boolean', + label: 'Disable Always Allow', + category: 'Security', + requiresRestart: true, + default: false, + description: + 'Disable "Always allow" options in tool confirmation dialogs.', + showInDialog: true, + }, enablePermanentToolApproval: { type: 'boolean', label: 'Allow Permanent Tool Approval', @@ -2267,7 +2277,8 @@ const SETTINGS_SCHEMA = { category: 'Admin', requiresRestart: false, default: false, - description: 'If true, disallows yolo mode from being used.', + description: + 'If true, disallows YOLO mode and "Always allow" options from being used.', showInDialog: false, mergeStrategy: MergeStrategy.REPLACE, }, diff --git a/packages/cli/src/test-utils/mockConfig.ts b/packages/cli/src/test-utils/mockConfig.ts index 1039d15c14..59d19b3412 100644 --- a/packages/cli/src/test-utils/mockConfig.ts +++ b/packages/cli/src/test-utils/mockConfig.ts @@ -122,6 +122,7 @@ export const createMockConfig = (overrides: Partial = {}): Config => getBannerTextNoCapacityIssues: vi.fn().mockResolvedValue(''), getBannerTextCapacityIssues: vi.fn().mockResolvedValue(''), isInteractiveShellEnabled: vi.fn().mockReturnValue(false), + getDisableAlwaysAllow: vi.fn().mockReturnValue(false), isSkillsSupportEnabled: vi.fn().mockReturnValue(false), reloadSkills: vi.fn().mockResolvedValue(undefined), reloadAgents: vi.fn().mockResolvedValue(undefined), diff --git a/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx b/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx index ab12ae496f..77d072b02e 100644 --- a/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx +++ b/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx @@ -42,6 +42,7 @@ describe('ToolConfirmationQueue', () => { const mockConfig = { isTrustedFolder: () => true, getIdeMode: () => false, + getDisableAlwaysAllow: () => false, getModel: () => 'gemini-pro', getDebugMode: () => false, getTargetDir: () => '/mock/target/dir', diff --git a/packages/cli/src/ui/components/messages/RedirectionConfirmation.test.tsx b/packages/cli/src/ui/components/messages/RedirectionConfirmation.test.tsx index 15763bdae7..df8522d99c 100644 --- a/packages/cli/src/ui/components/messages/RedirectionConfirmation.test.tsx +++ b/packages/cli/src/ui/components/messages/RedirectionConfirmation.test.tsx @@ -21,6 +21,7 @@ describe('ToolConfirmationMessage Redirection', () => { const mockConfig = { isTrustedFolder: () => true, getIdeMode: () => false, + getDisableAlwaysAllow: () => false, } as unknown as Config; it('should display redirection warning and tip for redirected commands', async () => { diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx index ec623f69a4..92c8b5743c 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx @@ -37,6 +37,7 @@ describe('ToolConfirmationMessage', () => { const mockConfig = { isTrustedFolder: () => true, getIdeMode: () => false, + getDisableAlwaysAllow: () => false, } as unknown as Config; it('should not display urls if prompt and url are the same', async () => { @@ -331,8 +332,8 @@ describe('ToolConfirmationMessage', () => { const mockConfig = { isTrustedFolder: () => true, getIdeMode: () => false, + getDisableAlwaysAllow: () => false, } as unknown as Config; - const { lastFrame, waitUntilReady, unmount } = renderWithProviders( { const mockConfig = { isTrustedFolder: () => false, getIdeMode: () => false, + getDisableAlwaysAllow: () => false, } as unknown as Config; const { lastFrame, waitUntilReady, unmount } = renderWithProviders( @@ -388,8 +390,8 @@ describe('ToolConfirmationMessage', () => { const mockConfig = { isTrustedFolder: () => true, getIdeMode: () => false, + getDisableAlwaysAllow: () => false, } as unknown as Config; - const { lastFrame, waitUntilReady, unmount } = renderWithProviders( { const mockConfig = { isTrustedFolder: () => true, getIdeMode: () => false, + getDisableAlwaysAllow: () => false, } as unknown as Config; - const { lastFrame, waitUntilReady, unmount } = renderWithProviders( { const mockConfig = { isTrustedFolder: () => true, getIdeMode: () => false, + getDisableAlwaysAllow: () => false, } as unknown as Config; - vi.mocked(useToolActions).mockReturnValue({ confirm: vi.fn(), cancel: vi.fn(), @@ -485,8 +487,8 @@ describe('ToolConfirmationMessage', () => { const mockConfig = { isTrustedFolder: () => true, getIdeMode: () => true, + getDisableAlwaysAllow: () => false, } as unknown as Config; - vi.mocked(useToolActions).mockReturnValue({ confirm: vi.fn(), cancel: vi.fn(), @@ -513,8 +515,8 @@ describe('ToolConfirmationMessage', () => { const mockConfig = { isTrustedFolder: () => true, getIdeMode: () => true, + getDisableAlwaysAllow: () => false, } as unknown as Config; - vi.mocked(useToolActions).mockReturnValue({ confirm: vi.fn(), cancel: vi.fn(), diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index 8bc329f3df..2e9e133a35 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -86,12 +86,14 @@ export const ToolConfirmationMessage: React.FC< const settings = useSettings(); const allowPermanentApproval = - settings.merged.security.enablePermanentToolApproval; + settings.merged.security.enablePermanentToolApproval && + !config.getDisableAlwaysAllow(); const handlesOwnUI = confirmationDetails.type === 'ask_user' || confirmationDetails.type === 'exit_plan_mode'; - const isTrustedFolder = config.isTrustedFolder(); + const isTrustedFolder = + config.isTrustedFolder() && !config.getDisableAlwaysAllow(); const handleConfirm = useCallback( (outcome: ToolConfirmationOutcome, payload?: ToolConfirmationPayload) => { diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 18dd627ea0..ea10e3994b 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -606,6 +606,7 @@ export interface ConfigParameters { recordResponses?: string; ptyInfo?: string; disableYoloMode?: boolean; + disableAlwaysAllow?: boolean; rawOutput?: boolean; acceptRawOutputRisk?: boolean; modelConfigServiceConfig?: ModelConfigServiceConfig; @@ -805,6 +806,7 @@ export class Config implements McpContext, AgentLoopContext { readonly fakeResponses?: string; readonly recordResponses?: string; private readonly disableYoloMode: boolean; + private readonly disableAlwaysAllow: boolean; private readonly rawOutput: boolean; private readonly acceptRawOutputRisk: boolean; private pendingIncludeDirectories: string[]; @@ -1045,11 +1047,13 @@ export class Config implements McpContext, AgentLoopContext { this.policyUpdateConfirmationRequest = params.policyUpdateConfirmationRequest; + this.disableAlwaysAllow = params.disableAlwaysAllow ?? false; this.policyEngine = new PolicyEngine( { ...params.policyEngineConfig, approvalMode: params.approvalMode ?? params.policyEngineConfig?.approvalMode, + disableAlwaysAllow: this.disableAlwaysAllow, }, checkerRunner, ); @@ -2203,6 +2207,10 @@ export class Config implements McpContext, AgentLoopContext { return this.disableYoloMode || !this.isTrustedFolder(); } + getDisableAlwaysAllow(): boolean { + return this.disableAlwaysAllow; + } + getRawOutput(): boolean { return this.rawOutput; } diff --git a/packages/core/src/policy/config.ts b/packages/core/src/policy/config.ts index 4c976bc160..392ab15c0c 100644 --- a/packages/core/src/policy/config.ts +++ b/packages/core/src/policy/config.ts @@ -16,6 +16,7 @@ import { type PolicyRule, type PolicySettings, type SafetyCheckerRule, + ALWAYS_ALLOW_PRIORITY_OFFSET, } from './types.js'; import type { PolicyEngine } from './policy-engine.js'; import { loadPoliciesFromToml, type PolicyFileError } from './toml-loader.js'; @@ -66,19 +67,6 @@ export const WORKSPACE_POLICY_TIER = 3; export const USER_POLICY_TIER = 4; export const ADMIN_POLICY_TIER = 5; -/** - * The fractional priority of "Always allow" rules (e.g., 950/1000). - * Higher fraction within a tier wins. - */ -export const ALWAYS_ALLOW_PRIORITY_FRACTION = 950; - -/** - * The fractional priority offset for "Always allow" rules (e.g., 0.95). - * This ensures consistency between in-memory rules and persisted rules. - */ -export const ALWAYS_ALLOW_PRIORITY_OFFSET = - ALWAYS_ALLOW_PRIORITY_FRACTION / 1000; - // Specific priority offsets and derived priorities for dynamic/settings rules. export const MCP_EXCLUDED_PRIORITY = USER_POLICY_TIER + 0.9; @@ -535,6 +523,7 @@ export async function createPolicyEngineConfig( checkers, defaultDecision: PolicyDecision.ASK_USER, approvalMode, + disableAlwaysAllow: settings.disableAlwaysAllow, }; } diff --git a/packages/core/src/policy/policy-engine.test.ts b/packages/core/src/policy/policy-engine.test.ts index a54da32376..376e465604 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -14,6 +14,7 @@ import { InProcessCheckerType, ApprovalMode, PRIORITY_SUBAGENT_TOOL, + ALWAYS_ALLOW_PRIORITY_FRACTION, } from './types.js'; import type { FunctionCall } from '@google/genai'; import { SafetyCheckDecision } from '../safety/protocol.js'; @@ -3229,4 +3230,116 @@ describe('PolicyEngine', () => { expect(hookCheckers[1].priority).toBe(5); }); }); + + describe('disableAlwaysAllow', () => { + it('should ignore "Always Allow" rules when disableAlwaysAllow is true', async () => { + const alwaysAllowRule: PolicyRule = { + toolName: 'test-tool', + decision: PolicyDecision.ALLOW, + priority: 3 + ALWAYS_ALLOW_PRIORITY_FRACTION / 1000, // 3.95 + source: 'Dynamic (Confirmed)', + }; + + const engine = new PolicyEngine({ + rules: [alwaysAllowRule], + disableAlwaysAllow: true, + defaultDecision: PolicyDecision.ASK_USER, + }); + + const result = await engine.check( + { name: 'test-tool', args: {} }, + undefined, + ); + expect(result.decision).toBe(PolicyDecision.ASK_USER); + }); + + it('should respect "Always Allow" rules when disableAlwaysAllow is false', async () => { + const alwaysAllowRule: PolicyRule = { + toolName: 'test-tool', + decision: PolicyDecision.ALLOW, + priority: 3 + ALWAYS_ALLOW_PRIORITY_FRACTION / 1000, // 3.95 + source: 'Dynamic (Confirmed)', + }; + + const engine = new PolicyEngine({ + rules: [alwaysAllowRule], + disableAlwaysAllow: false, + defaultDecision: PolicyDecision.ASK_USER, + }); + + const result = await engine.check( + { name: 'test-tool', args: {} }, + undefined, + ); + expect(result.decision).toBe(PolicyDecision.ALLOW); + }); + + it('should NOT ignore other rules when disableAlwaysAllow is true', async () => { + const normalRule: PolicyRule = { + toolName: 'test-tool', + decision: PolicyDecision.ALLOW, + priority: 1.5, // Not a .950 fraction + source: 'Normal Rule', + }; + + const engine = new PolicyEngine({ + rules: [normalRule], + disableAlwaysAllow: true, + defaultDecision: PolicyDecision.ASK_USER, + }); + + const result = await engine.check( + { name: 'test-tool', args: {} }, + undefined, + ); + expect(result.decision).toBe(PolicyDecision.ALLOW); + }); + }); + + describe('getExcludedTools with disableAlwaysAllow', () => { + it('should exclude tool if an Always Allow rule says ALLOW but disableAlwaysAllow is true (falling back to DENY)', async () => { + // To prove the ALWAYS_ALLOW rule is ignored, we set the default decision to DENY. + // If the rule was honored, the decision would be ALLOW (tool not excluded). + // Since it's ignored, it falls back to the default DENY (tool is excluded). + // In the real app, it usually falls back to ASK_USER, but ASK_USER also doesn't + // exclude the tool, so we use DENY here purely to make the test observable. + const alwaysAllowRule: PolicyRule = { + toolName: 'test-tool', + decision: PolicyDecision.ALLOW, + priority: 3 + ALWAYS_ALLOW_PRIORITY_FRACTION / 1000, + }; + + const engine = new PolicyEngine({ + rules: [alwaysAllowRule], + disableAlwaysAllow: true, + defaultDecision: PolicyDecision.DENY, + }); + + const excluded = engine.getExcludedTools( + undefined, + new Set(['test-tool']), + ); + expect(excluded.has('test-tool')).toBe(true); + }); + + it('should NOT exclude tool if ALWAYS_ALLOW is enabled and rule says ALLOW', async () => { + const alwaysAllowRule: PolicyRule = { + toolName: 'test-tool', + decision: PolicyDecision.ALLOW, + priority: 3 + ALWAYS_ALLOW_PRIORITY_FRACTION / 1000, + }; + + const engine = new PolicyEngine({ + rules: [alwaysAllowRule], + disableAlwaysAllow: false, + defaultDecision: PolicyDecision.DENY, + }); + + const excluded = engine.getExcludedTools( + undefined, + new Set(['test-tool']), + ); + expect(excluded.has('test-tool')).toBe(false); + }); + }); }); diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index b626666370..ec84eb23aa 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -13,6 +13,7 @@ import { type HookCheckerRule, ApprovalMode, type CheckResult, + ALWAYS_ALLOW_PRIORITY_FRACTION, } from './types.js'; import { stableStringify } from './stable-stringify.js'; import { debugLogger } from '../utils/debugLogger.js'; @@ -154,6 +155,7 @@ export class PolicyEngine { private hookCheckers: HookCheckerRule[]; private readonly defaultDecision: PolicyDecision; private readonly nonInteractive: boolean; + private readonly disableAlwaysAllow: boolean; private readonly checkerRunner?: CheckerRunner; private approvalMode: ApprovalMode; @@ -169,6 +171,7 @@ export class PolicyEngine { ); this.defaultDecision = config.defaultDecision ?? PolicyDecision.ASK_USER; this.nonInteractive = config.nonInteractive ?? false; + this.disableAlwaysAllow = config.disableAlwaysAllow ?? false; this.checkerRunner = checkerRunner; this.approvalMode = config.approvalMode ?? ApprovalMode.DEFAULT; } @@ -187,6 +190,13 @@ export class PolicyEngine { return this.approvalMode; } + private isAlwaysAllowRule(rule: PolicyRule): boolean { + return ( + rule.priority !== undefined && + Math.round((rule.priority % 1) * 1000) === ALWAYS_ALLOW_PRIORITY_FRACTION + ); + } + private shouldDowngradeForRedirection( command: string, allowRedirection?: boolean, @@ -422,6 +432,10 @@ export class PolicyEngine { } for (const rule of this.rules) { + if (this.disableAlwaysAllow && this.isAlwaysAllowRule(rule)) { + continue; + } + const match = toolCallsToTry.some((tc) => ruleMatches( rule, @@ -684,6 +698,10 @@ export class PolicyEngine { // Evaluate rules in priority order (they are already sorted in constructor) for (const rule of this.rules) { + if (this.disableAlwaysAllow && this.isAlwaysAllowRule(rule)) { + continue; + } + // Create a copy of the rule without argsPattern to see if it targets the tool // regardless of the runtime arguments it might receive. const ruleWithoutArgs: PolicyRule = { ...rule, argsPattern: undefined }; diff --git a/packages/core/src/policy/types.ts b/packages/core/src/policy/types.ts index 6fa45630d9..6e14e1fac9 100644 --- a/packages/core/src/policy/types.ts +++ b/packages/core/src/policy/types.ts @@ -285,6 +285,11 @@ export interface PolicyEngineConfig { */ nonInteractive?: boolean; + /** + * Whether to ignore "Always Allow" rules. + */ + disableAlwaysAllow?: boolean; + /** * Whether to allow hooks to execute. * When false, all hooks are denied. @@ -314,6 +319,7 @@ export interface PolicySettings { // Admin provided policies that will supplement the ADMIN level policies adminPolicyPaths?: string[]; workspacePoliciesDir?: string; + disableAlwaysAllow?: boolean; } export interface CheckResult { @@ -326,3 +332,16 @@ export interface CheckResult { * Effective priority matching Tier 1 (Default) read-only tools. */ export const PRIORITY_SUBAGENT_TOOL = 1.05; + +/** + * The fractional priority of "Always allow" rules (e.g., 950/1000). + * Higher fraction within a tier wins. + */ +export const ALWAYS_ALLOW_PRIORITY_FRACTION = 950; + +/** + * The fractional priority offset for "Always allow" rules (e.g., 0.95). + * This ensures consistency between in-memory rules and persisted rules. + */ +export const ALWAYS_ALLOW_PRIORITY_OFFSET = + ALWAYS_ALLOW_PRIORITY_FRACTION / 1000; diff --git a/packages/core/src/scheduler/policy.test.ts b/packages/core/src/scheduler/policy.test.ts index e802a4b220..32a92309e0 100644 --- a/packages/core/src/scheduler/policy.test.ts +++ b/packages/core/src/scheduler/policy.test.ts @@ -102,6 +102,32 @@ describe('policy.ts', () => { ); }); + it('should respect disableAlwaysAllow from config', async () => { + const mockPolicyEngine = { + check: vi.fn().mockResolvedValue({ decision: PolicyDecision.ALLOW }), + } as unknown as Mocked; + + const mockConfig = { + getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine), + getDisableAlwaysAllow: vi.fn().mockReturnValue(true), + } as unknown as Mocked; + + (mockConfig as unknown as { config: Config }).config = + mockConfig as Config; + + const toolCall = { + request: { name: 'test-tool', args: {} }, + tool: { name: 'test-tool' }, + } as ValidatingToolCall; + + // Note: checkPolicy calls config.getPolicyEngine().check() + // The PolicyEngine itself is already configured with disableAlwaysAllow + // when created in Config. Here we are just verifying that checkPolicy + // doesn't somehow bypass it. + await checkPolicy(toolCall, mockConfig); + expect(mockPolicyEngine.check).toHaveBeenCalled(); + }); + it('should throw if ASK_USER is returned in non-interactive mode', async () => { const mockPolicyEngine = { check: vi.fn().mockResolvedValue({ decision: PolicyDecision.ASK_USER }), diff --git a/schemas/settings.schema.json b/schemas/settings.schema.json index f61690e306..04df187a05 100644 --- a/schemas/settings.schema.json +++ b/schemas/settings.schema.json @@ -1495,6 +1495,13 @@ "default": false, "type": "boolean" }, + "disableAlwaysAllow": { + "title": "Disable Always Allow", + "description": "Disable \"Always allow\" options in tool confirmation dialogs.", + "markdownDescription": "Disable \"Always allow\" options in tool confirmation dialogs.\n\n- Category: `Security`\n- Requires restart: `yes`\n- Default: `false`", + "default": false, + "type": "boolean" + }, "enablePermanentToolApproval": { "title": "Allow Permanent Tool Approval", "description": "Enable the \"Allow for all future sessions\" option in tool confirmation dialogs.", @@ -2027,8 +2034,8 @@ "properties": { "secureModeEnabled": { "title": "Secure Mode Enabled", - "description": "If true, disallows yolo mode from being used.", - "markdownDescription": "If true, disallows yolo mode from being used.\n\n- Category: `Admin`\n- Requires restart: `no`\n- Default: `false`", + "description": "If true, disallows YOLO mode and \"Always allow\" options from being used.", + "markdownDescription": "If true, disallows YOLO mode and \"Always allow\" options from being used.\n\n- Category: `Admin`\n- Requires restart: `no`\n- Default: `false`", "default": false, "type": "boolean" },