feat(core): implement context-aware persistent policy approvals (#23257)

This commit is contained in:
Jerop Kipruto
2026-04-02 16:01:33 -04:00
committed by GitHub
parent 61b21e3d63
commit 64c928fce7
8 changed files with 297 additions and 31 deletions

View File

@@ -181,9 +181,16 @@ As described in the
rule that does not explicitly specify `modes` is considered "always active" and rule that does not explicitly specify `modes` is considered "always active" and
will apply to Plan Mode as well. will apply to Plan Mode as well.
If you want a rule to apply to other modes but _not_ to Plan Mode, you must To maintain the integrity of Plan Mode as a safe research environment,
explicitly specify the target modes. For example, to allow `npm test` in default persistent tool approvals are context-aware. Approvals granted in modes like
and Auto-Edit modes but not in Plan Mode: Default or Auto-Edit do not apply to Plan Mode, ensuring that tools trusted for
implementation don't automatically execute while you're researching. However,
approvals granted while in Plan Mode are treated as intentional choices for
global trust and apply to all modes.
If you want to manually restrict a rule to other modes but _not_ to Plan Mode,
you must explicitly specify the target modes. For example, to allow `npm test`
in default and Auto-Edit modes but not in Plan Mode:
```toml ```toml
[[rule]] [[rule]]

View File

@@ -171,6 +171,24 @@ modes specified, it is always active.
[Customizing Plan Mode Policies](../cli/plan-mode.md#customizing-policies). [Customizing Plan Mode Policies](../cli/plan-mode.md#customizing-policies).
- `yolo`: A mode where all tools are auto-approved (use with extreme caution). - `yolo`: A mode where all tools are auto-approved (use with extreme caution).
To maintain the integrity of Plan Mode as a safe research environment,
persistent tool approvals are context-aware. When you select **"Allow for all
future sessions"**, the policy engine explicitly includes the current mode and
all more permissive modes in the hierarchy (`plan` < `default` < `autoEdit` <
`yolo`).
- **Approvals in `plan` mode**: These represent an intentional choice to trust a
tool globally. The resulting rule explicitly includes all modes (`plan`,
`default`, `autoEdit`, and `yolo`).
- **Approvals in other modes**: These only apply to the current mode and those
more permissive. For example:
- An approval granted in **`default`** mode applies to `default`, `autoEdit`,
and `yolo`.
- An approval granted in **`autoEdit`** mode applies to `autoEdit` and `yolo`.
- An approval granted in **`yolo`** mode applies only to `yolo`. This ensures
that trust flows correctly to more permissive environments while maintaining
the safety of more restricted modes like `plan`.
## Rule matching ## Rule matching
When a tool call is made, the engine checks it against all active rules, When a tool call is made, the engine checks it against all active rules,
@@ -304,7 +322,8 @@ priority = 10
denyMessage = "Deletion is permanent" denyMessage = "Deletion is permanent"
# (Optional) An array of approval modes where this rule is active. # (Optional) An array of approval modes where this rule is active.
modes = ["autoEdit"] # If omitted or empty, the rule applies to all modes.
modes = ["default", "autoEdit", "yolo"]
# (Optional) A boolean to restrict the rule to interactive (true) or # (Optional) A boolean to restrict the rule to interactive (true) or
# non-interactive (false) environments. # non-interactive (false) environments.

View File

@@ -5,6 +5,7 @@
*/ */
import { type FunctionCall } from '@google/genai'; import { type FunctionCall } from '@google/genai';
import { type ApprovalMode } from '../policy/types.js';
import type { import type {
ToolConfirmationOutcome, ToolConfirmationOutcome,
ToolConfirmationPayload, ToolConfirmationPayload,
@@ -150,6 +151,7 @@ export interface UpdatePolicy {
commandPrefix?: string | string[]; commandPrefix?: string | string[];
mcpName?: string; mcpName?: string;
allowRedirection?: boolean; allowRedirection?: boolean;
modes?: ApprovalMode[];
} }
export interface ToolPolicyRejection { export interface ToolPolicyRejection {

View File

@@ -533,7 +533,6 @@ export async function createPolicyEngineConfig(
disableAlwaysAllow: settings.disableAlwaysAllow, disableAlwaysAllow: settings.disableAlwaysAllow,
}; };
} }
interface TomlRule { interface TomlRule {
toolName?: string; toolName?: string;
mcpName?: string; mcpName?: string;
@@ -542,10 +541,64 @@ interface TomlRule {
commandPrefix?: string | string[]; commandPrefix?: string | string[];
argsPattern?: string; argsPattern?: string;
allowRedirection?: boolean; allowRedirection?: boolean;
modes?: ApprovalMode[];
// Index signature to satisfy Record type if needed for toml.stringify // Index signature to satisfy Record type if needed for toml.stringify
[key: string]: unknown; [key: string]: unknown;
} }
/**
* Finds a rule in the rule array that matches the given criteria.
*/
function findMatchingRule(
rules: TomlRule[],
criteria: {
toolName: string;
mcpName?: string;
commandPrefix?: string | string[];
argsPattern?: string;
},
): TomlRule | undefined {
return rules.find(
(r) =>
r.toolName === criteria.toolName &&
r.mcpName === criteria.mcpName &&
JSON.stringify(r.commandPrefix) ===
JSON.stringify(criteria.commandPrefix) &&
r.argsPattern === criteria.argsPattern,
);
}
/**
* Creates a new TOML rule object from the given tool name and message.
*/
function createTomlRule(toolName: string, message: UpdatePolicy): TomlRule {
const rule: TomlRule = {
decision: 'allow',
priority: getAlwaysAllowPriorityFraction(),
toolName,
};
if (message.mcpName) {
rule.mcpName = message.mcpName;
}
if (message.commandPrefix) {
rule.commandPrefix = message.commandPrefix;
} else if (message.argsPattern) {
rule.argsPattern = message.argsPattern;
}
if (message.allowRedirection !== undefined) {
rule.allowRedirection = message.allowRedirection;
}
if (message.modes) {
rule.modes = message.modes;
}
return rule;
}
export function createPolicyUpdater( export function createPolicyUpdater(
policyEngine: PolicyEngine, policyEngine: PolicyEngine,
messageBus: MessageBus, messageBus: MessageBus,
@@ -585,6 +638,7 @@ export function createPolicyUpdater(
priority, priority,
argsPattern: new RegExp(pattern), argsPattern: new RegExp(pattern),
mcpName: message.mcpName, mcpName: message.mcpName,
modes: message.modes,
source: 'Dynamic (Confirmed)', source: 'Dynamic (Confirmed)',
allowRedirection: message.allowRedirection, allowRedirection: message.allowRedirection,
}); });
@@ -622,6 +676,7 @@ export function createPolicyUpdater(
priority, priority,
argsPattern, argsPattern,
mcpName: message.mcpName, mcpName: message.mcpName,
modes: message.modes,
source: 'Dynamic (Confirmed)', source: 'Dynamic (Confirmed)',
allowRedirection: message.allowRedirection, allowRedirection: message.allowRedirection,
}); });
@@ -662,39 +717,36 @@ export function createPolicyUpdater(
existingData.rule = []; existingData.rule = [];
} }
// Create new rule object // Normalize tool name for MCP
const newRule: TomlRule = { let normalizedToolName = toolName;
decision: 'allow',
priority: getAlwaysAllowPriorityFraction(),
};
if (message.mcpName) { if (message.mcpName) {
newRule.mcpName = message.mcpName;
const expectedPrefix = `${MCP_TOOL_PREFIX}${message.mcpName}_`; const expectedPrefix = `${MCP_TOOL_PREFIX}${message.mcpName}_`;
if (toolName.startsWith(expectedPrefix)) { if (toolName.startsWith(expectedPrefix)) {
newRule.toolName = toolName.slice(expectedPrefix.length); normalizedToolName = toolName.slice(expectedPrefix.length);
} else { }
newRule.toolName = toolName; }
// Look for an existing rule to update
const existingRule = findMatchingRule(existingData.rule, {
toolName: normalizedToolName,
mcpName: message.mcpName,
commandPrefix: message.commandPrefix,
argsPattern: message.argsPattern,
});
if (existingRule) {
if (message.allowRedirection !== undefined) {
existingRule.allowRedirection = message.allowRedirection;
}
if (message.modes) {
existingRule.modes = message.modes;
} }
} else { } else {
newRule.toolName = toolName; existingData.rule.push(
createTomlRule(normalizedToolName, message),
);
} }
if (message.commandPrefix) {
newRule.commandPrefix = message.commandPrefix;
} else if (message.argsPattern) {
// message.argsPattern was already validated above
newRule.argsPattern = message.argsPattern;
}
if (message.allowRedirection !== undefined) {
newRule.allowRedirection = message.allowRedirection;
}
// Add to rules
existingData.rule.push(newRule);
// Serialize back to TOML // Serialize back to TOML
// @iarna/toml stringify might not produce beautiful output but it handles escaping correctly // @iarna/toml stringify might not produce beautiful output but it handles escaping correctly
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion

View File

@@ -242,4 +242,57 @@ decision = "deny"
const content = memfs.readFileSync(policyFile, 'utf-8') as string; const content = memfs.readFileSync(policyFile, 'utf-8') as string;
expect(content).toContain('toolName = "test_tool"'); expect(content).toContain('toolName = "test_tool"');
}); });
it('should include modes if provided', async () => {
createPolicyUpdater(policyEngine, messageBus, mockStorage);
const policyFile = '/mock/user/.gemini/policies/auto-saved.toml';
vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile);
await messageBus.publish({
type: MessageBusType.UPDATE_POLICY,
toolName: 'test_tool',
persist: true,
modes: [ApprovalMode.DEFAULT, ApprovalMode.YOLO],
});
await vi.advanceTimersByTimeAsync(100);
const content = memfs.readFileSync(policyFile, 'utf-8') as string;
expect(content).toContain('modes = [ "default", "yolo" ]');
});
it('should update existing rule modes instead of appending redundant rule', async () => {
createPolicyUpdater(policyEngine, messageBus, mockStorage);
const policyFile = '/mock/user/.gemini/policies/auto-saved.toml';
vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile);
const existingContent = `
[[rule]]
decision = "allow"
priority = 950
toolName = "test_tool"
modes = [ "autoEdit", "yolo" ]
`;
const dir = path.dirname(policyFile);
memfs.mkdirSync(dir, { recursive: true });
memfs.writeFileSync(policyFile, existingContent);
// Now grant in DEFAULT mode, which should include [default, autoEdit, yolo]
await messageBus.publish({
type: MessageBusType.UPDATE_POLICY,
toolName: 'test_tool',
persist: true,
modes: [ApprovalMode.DEFAULT, ApprovalMode.AUTO_EDIT, ApprovalMode.YOLO],
});
await vi.advanceTimersByTimeAsync(100);
const content = memfs.readFileSync(policyFile, 'utf-8') as string;
// Should NOT have two [[rule]] entries for test_tool
const ruleCount = (content.match(/\[\[rule\]\]/g) || []).length;
expect(ruleCount).toBe(1);
expect(content).toContain('modes = [ "default", "autoEdit", "yolo" ]');
});
}); });

View File

@@ -52,6 +52,18 @@ export enum ApprovalMode {
PLAN = 'plan', PLAN = 'plan',
} }
/**
* The order of permissiveness for approval modes.
* Tools allowed in a less permissive mode should also be allowed
* in more permissive modes.
*/
export const MODES_BY_PERMISSIVENESS = [
ApprovalMode.PLAN,
ApprovalMode.DEFAULT,
ApprovalMode.AUTO_EDIT,
ApprovalMode.YOLO,
];
/** /**
* Configuration for the built-in allowed-path checker. * Configuration for the built-in allowed-path checker.
*/ */

View File

@@ -49,6 +49,7 @@ describe('policy.ts', () => {
} as unknown as Mocked<PolicyEngine>; } as unknown as Mocked<PolicyEngine>;
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine), getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
@@ -76,6 +77,7 @@ describe('policy.ts', () => {
} as unknown as Mocked<PolicyEngine>; } as unknown as Mocked<PolicyEngine>;
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine), getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
@@ -106,6 +108,7 @@ describe('policy.ts', () => {
} as unknown as Mocked<PolicyEngine>; } as unknown as Mocked<PolicyEngine>;
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine), getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine),
getDisableAlwaysAllow: vi.fn().mockReturnValue(true), getDisableAlwaysAllow: vi.fn().mockReturnValue(true),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
@@ -132,6 +135,7 @@ describe('policy.ts', () => {
} as unknown as Mocked<PolicyEngine>; } as unknown as Mocked<PolicyEngine>;
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine), getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine),
isInteractive: vi.fn().mockReturnValue(false), isInteractive: vi.fn().mockReturnValue(false),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
@@ -155,6 +159,7 @@ describe('policy.ts', () => {
} as unknown as Mocked<PolicyEngine>; } as unknown as Mocked<PolicyEngine>;
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine), getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
@@ -176,6 +181,7 @@ describe('policy.ts', () => {
} as unknown as Mocked<PolicyEngine>; } as unknown as Mocked<PolicyEngine>;
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine), getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine),
isInteractive: vi.fn().mockReturnValue(true), isInteractive: vi.fn().mockReturnValue(true),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
@@ -198,6 +204,7 @@ describe('policy.ts', () => {
} as unknown as Mocked<PolicyEngine>; } as unknown as Mocked<PolicyEngine>;
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine), getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine),
isInteractive: vi.fn().mockReturnValue(true), isInteractive: vi.fn().mockReturnValue(true),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
@@ -217,6 +224,7 @@ describe('policy.ts', () => {
} as unknown as Mocked<PolicyEngine>; } as unknown as Mocked<PolicyEngine>;
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine), getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
@@ -233,6 +241,7 @@ describe('policy.ts', () => {
describe('updatePolicy', () => { describe('updatePolicy', () => {
it('should set AUTO_EDIT mode for auto-edit transition tools', async () => { it('should set AUTO_EDIT mode for auto-edit transition tools', async () => {
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
setApprovalMode: vi.fn(), setApprovalMode: vi.fn(),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
@@ -262,6 +271,7 @@ describe('policy.ts', () => {
it('should handle standard policy updates (persist=false)', async () => { it('should handle standard policy updates (persist=false)', async () => {
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
setApprovalMode: vi.fn(), setApprovalMode: vi.fn(),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
@@ -293,6 +303,7 @@ describe('policy.ts', () => {
it('should handle standard policy updates with persistence', async () => { it('should handle standard policy updates with persistence', async () => {
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
isTrustedFolder: vi.fn().mockReturnValue(false), isTrustedFolder: vi.fn().mockReturnValue(false),
getWorkspacePoliciesDir: vi.fn().mockReturnValue(undefined), getWorkspacePoliciesDir: vi.fn().mockReturnValue(undefined),
setApprovalMode: vi.fn(), setApprovalMode: vi.fn(),
@@ -326,6 +337,7 @@ describe('policy.ts', () => {
it('should handle shell command prefixes', async () => { it('should handle shell command prefixes', async () => {
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
setApprovalMode: vi.fn(), setApprovalMode: vi.fn(),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
@@ -365,6 +377,7 @@ describe('policy.ts', () => {
it('should handle MCP policy updates (server scope)', async () => { it('should handle MCP policy updates (server scope)', async () => {
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
setApprovalMode: vi.fn(), setApprovalMode: vi.fn(),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
@@ -405,6 +418,7 @@ describe('policy.ts', () => {
it('should NOT publish update for ProceedOnce', async () => { it('should NOT publish update for ProceedOnce', async () => {
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
setApprovalMode: vi.fn(), setApprovalMode: vi.fn(),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
@@ -431,6 +445,7 @@ describe('policy.ts', () => {
it('should NOT publish update for Cancel', async () => { it('should NOT publish update for Cancel', async () => {
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
setApprovalMode: vi.fn(), setApprovalMode: vi.fn(),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
@@ -456,6 +471,7 @@ describe('policy.ts', () => {
it('should NOT publish update for ModifyWithEditor', async () => { it('should NOT publish update for ModifyWithEditor', async () => {
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
setApprovalMode: vi.fn(), setApprovalMode: vi.fn(),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
@@ -481,6 +497,7 @@ describe('policy.ts', () => {
it('should handle MCP ProceedAlwaysTool (specific tool name)', async () => { it('should handle MCP ProceedAlwaysTool (specific tool name)', async () => {
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
setApprovalMode: vi.fn(), setApprovalMode: vi.fn(),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
@@ -521,6 +538,7 @@ describe('policy.ts', () => {
it('should handle MCP ProceedAlways (persist: false)', async () => { it('should handle MCP ProceedAlways (persist: false)', async () => {
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
setApprovalMode: vi.fn(), setApprovalMode: vi.fn(),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
@@ -561,6 +579,7 @@ describe('policy.ts', () => {
it('should handle MCP ProceedAlwaysAndSave (persist: true)', async () => { it('should handle MCP ProceedAlwaysAndSave (persist: true)', async () => {
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
isTrustedFolder: vi.fn().mockReturnValue(false), isTrustedFolder: vi.fn().mockReturnValue(false),
getWorkspacePoliciesDir: vi.fn().mockReturnValue(undefined), getWorkspacePoliciesDir: vi.fn().mockReturnValue(undefined),
setApprovalMode: vi.fn(), setApprovalMode: vi.fn(),
@@ -603,6 +622,7 @@ describe('policy.ts', () => {
it('should determine persistScope: workspace in trusted folders', async () => { it('should determine persistScope: workspace in trusted folders', async () => {
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
isTrustedFolder: vi.fn().mockReturnValue(true), isTrustedFolder: vi.fn().mockReturnValue(true),
getWorkspacePoliciesDir: vi getWorkspacePoliciesDir: vi
.fn() .fn()
@@ -633,6 +653,7 @@ describe('policy.ts', () => {
it('should determine persistScope: user in untrusted folders', async () => { it('should determine persistScope: user in untrusted folders', async () => {
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
isTrustedFolder: vi.fn().mockReturnValue(false), isTrustedFolder: vi.fn().mockReturnValue(false),
getWorkspacePoliciesDir: vi getWorkspacePoliciesDir: vi
.fn() .fn()
@@ -663,6 +684,7 @@ describe('policy.ts', () => {
it('should narrow edit tools with argsPattern', async () => { it('should narrow edit tools with argsPattern', async () => {
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
isTrustedFolder: vi.fn().mockReturnValue(false), isTrustedFolder: vi.fn().mockReturnValue(false),
getWorkspacePoliciesDir: vi.fn().mockReturnValue(undefined), getWorkspacePoliciesDir: vi.fn().mockReturnValue(undefined),
getTargetDir: vi.fn().mockReturnValue('/mock/dir'), getTargetDir: vi.fn().mockReturnValue('/mock/dir'),
@@ -703,6 +725,7 @@ describe('policy.ts', () => {
it('should work when context is created via Object.create (prototype chain)', async () => { it('should work when context is created via Object.create (prototype chain)', async () => {
const mockConfig = { const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
setApprovalMode: vi.fn(), setApprovalMode: vi.fn(),
} as unknown as Mocked<Config>; } as unknown as Mocked<Config>;
const mockMessageBus = { const mockMessageBus = {
@@ -868,4 +891,78 @@ describe('Plan Mode Denial Consistency', () => {
expect(resultMessage).toBe('Tool execution denied by policy.'); expect(resultMessage).toBe('Tool execution denied by policy.');
expect(resultErrorType).toBe(ToolErrorType.POLICY_VIOLATION); expect(resultErrorType).toBe(ToolErrorType.POLICY_VIOLATION);
}); });
describe('updatePolicy - context-aware modes', () => {
const testCases = [
{
currentMode: ApprovalMode.DEFAULT,
expectedModes: [
ApprovalMode.DEFAULT,
ApprovalMode.AUTO_EDIT,
ApprovalMode.YOLO,
],
description:
'include current and more permissive modes in DEFAULT mode',
},
{
currentMode: ApprovalMode.AUTO_EDIT,
expectedModes: [ApprovalMode.AUTO_EDIT, ApprovalMode.YOLO],
description:
'include current and more permissive modes in AUTO_EDIT mode',
},
{
currentMode: ApprovalMode.YOLO,
expectedModes: [ApprovalMode.YOLO],
description: 'include current and more permissive modes in YOLO mode',
},
{
currentMode: ApprovalMode.PLAN,
expectedModes: [
ApprovalMode.PLAN,
ApprovalMode.DEFAULT,
ApprovalMode.AUTO_EDIT,
ApprovalMode.YOLO,
],
description: 'include all modes explicitly when granted in PLAN mode',
},
];
testCases.forEach(({ currentMode, expectedModes, description }) => {
it(`should ${description}`, async () => {
const mockConfig = {
getApprovalMode: vi.fn().mockReturnValue(currentMode),
isTrustedFolder: vi.fn().mockReturnValue(false),
getWorkspacePoliciesDir: vi.fn().mockReturnValue(undefined),
} as unknown as Mocked<Config>;
const mockMessageBus = {
publish: vi.fn(),
} as unknown as Mocked<MessageBus>;
const context = {
config: mockConfig,
messageBus: mockMessageBus,
} as unknown as AgentLoopContext;
const tool = { name: 'test-tool' } as AnyDeclarativeTool;
await updatePolicy(
tool,
ToolConfirmationOutcome.ProceedAlwaysAndSave,
undefined,
context,
mockMessageBus,
);
expect(mockMessageBus.publish).toHaveBeenCalledWith(
expect.objectContaining({
type: MessageBusType.UPDATE_POLICY,
toolName: 'test-tool',
persist: true,
modes: expectedModes,
}),
);
});
});
});
}); });

View File

@@ -7,6 +7,7 @@
import { ToolErrorType } from '../tools/tool-error.js'; import { ToolErrorType } from '../tools/tool-error.js';
import { import {
ApprovalMode, ApprovalMode,
MODES_BY_PERMISSIVENESS,
PolicyDecision, PolicyDecision,
type CheckResult, type CheckResult,
type PolicyRule, type PolicyRule,
@@ -126,6 +127,23 @@ export async function updatePolicy(
// Determine persist scope if we are persisting. // Determine persist scope if we are persisting.
let persistScope: 'workspace' | 'user' | undefined; let persistScope: 'workspace' | 'user' | undefined;
let modes: ApprovalMode[] | undefined;
const currentMode = context.config.getApprovalMode();
// If this is an 'Always Allow' selection, we restrict it to the current mode
// and more permissive modes.
if (
outcome === ToolConfirmationOutcome.ProceedAlways ||
outcome === ToolConfirmationOutcome.ProceedAlwaysTool ||
outcome === ToolConfirmationOutcome.ProceedAlwaysServer ||
outcome === ToolConfirmationOutcome.ProceedAlwaysAndSave
) {
const modeIndex = MODES_BY_PERMISSIVENESS.indexOf(currentMode);
if (modeIndex !== -1) {
modes = MODES_BY_PERMISSIVENESS.slice(modeIndex);
}
}
if (outcome === ToolConfirmationOutcome.ProceedAlwaysAndSave) { if (outcome === ToolConfirmationOutcome.ProceedAlwaysAndSave) {
// If folder is trusted and workspace policies are enabled, we prefer workspace scope. // If folder is trusted and workspace policies are enabled, we prefer workspace scope.
if ( if (
@@ -147,6 +165,7 @@ export async function updatePolicy(
confirmationDetails, confirmationDetails,
messageBus, messageBus,
persistScope, persistScope,
modes,
); );
return; return;
} }
@@ -160,6 +179,7 @@ export async function updatePolicy(
persistScope, persistScope,
toolInvocation, toolInvocation,
context.config, context.config,
modes,
); );
} }
@@ -192,6 +212,7 @@ async function handleStandardPolicyUpdate(
persistScope?: 'workspace' | 'user', persistScope?: 'workspace' | 'user',
toolInvocation?: AnyToolInvocation, toolInvocation?: AnyToolInvocation,
config?: Config, config?: Config,
modes?: ApprovalMode[],
): Promise<void> { ): Promise<void> {
if ( if (
outcome === ToolConfirmationOutcome.ProceedAlways || outcome === ToolConfirmationOutcome.ProceedAlways ||
@@ -214,6 +235,7 @@ async function handleStandardPolicyUpdate(
toolName: tool.name, toolName: tool.name,
persist: outcome === ToolConfirmationOutcome.ProceedAlwaysAndSave, persist: outcome === ToolConfirmationOutcome.ProceedAlwaysAndSave,
persistScope, persistScope,
modes,
...options, ...options,
}); });
} }
@@ -232,6 +254,7 @@ async function handleMcpPolicyUpdate(
>, >,
messageBus: MessageBus, messageBus: MessageBus,
persistScope?: 'workspace' | 'user', persistScope?: 'workspace' | 'user',
modes?: ApprovalMode[],
): Promise<void> { ): Promise<void> {
const isMcpAlways = const isMcpAlways =
outcome === ToolConfirmationOutcome.ProceedAlways || outcome === ToolConfirmationOutcome.ProceedAlways ||
@@ -257,5 +280,6 @@ async function handleMcpPolicyUpdate(
mcpName: confirmationDetails.serverName, mcpName: confirmationDetails.serverName,
persist, persist,
persistScope, persistScope,
modes,
}); });
} }