policy: extract legacy policy from core tool scheduler to policy engine (#15902)

packages/cli/src/services/prompt-processors/shellProcessor.test.ts

@@ -9,7 +9,11 @@ import { ConfirmationRequiredError, ShellProcessor } from './shellProcessor.js';
 import { createMockCommandContext } from '../../test-utils/mockCommandContext.js';
 import type { CommandContext } from '../../ui/commands/types.js';
 import type { Config } from '@google/gemini-cli-core';
-import { ApprovalMode, getShellConfiguration } from '@google/gemini-cli-core';
+import {
+  ApprovalMode,
+  getShellConfiguration,
+  PolicyDecision,
+} from '@google/gemini-cli-core';
 import { quote } from 'shell-quote';
 import { createPartFromText } from '@google/genai';
 import type { PromptPipelineContent } from './types.js';
@@ -60,15 +64,23 @@ const SUCCESS_RESULT = {
 describe('ShellProcessor', () => {
   let context: CommandContext;
   let mockConfig: Partial<Config>;
+  let mockPolicyEngineCheck: Mock;
 
   beforeEach(() => {
     vi.clearAllMocks();
 
+    mockPolicyEngineCheck = vi.fn().mockResolvedValue({
+      decision: PolicyDecision.ALLOW,
+    });
+
     mockConfig = {
       getTargetDir: vi.fn().mockReturnValue('/test/dir'),
       getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
       getEnableInteractiveShell: vi.fn().mockReturnValue(false),
       getShellExecutionConfig: vi.fn().mockReturnValue({}),
+      getPolicyEngine: vi.fn().mockReturnValue({
+        check: mockPolicyEngineCheck,
+      }),
     };
 
     context = createMockCommandContext({
@@ -124,9 +136,8 @@ describe('ShellProcessor', () => {
     const prompt: PromptPipelineContent = createPromptPipelineContent(
       'The current status is: !{git status}',
     );
-    mockCheckCommandPermissions.mockReturnValue({
-      allAllowed: true,
-      disallowedCommands: [],
+    mockPolicyEngineCheck.mockResolvedValue({
+      decision: PolicyDecision.ALLOW,
     });
     mockShellExecute.mockReturnValue({
       result: Promise.resolve({ ...SUCCESS_RESULT, output: 'On branch main' }),
@@ -134,10 +145,12 @@ describe('ShellProcessor', () => {
 
     const result = await processor.process(prompt, context);
 
-    expect(mockCheckCommandPermissions).toHaveBeenCalledWith(
-      'git status',
-      expect.any(Object),
-      context.session.sessionShellAllowlist,
+    expect(mockPolicyEngineCheck).toHaveBeenCalledWith(
+      {
+        name: 'run_shell_command',
+        args: { command: 'git status' },
+      },
+      undefined,
     );
     expect(mockShellExecute).toHaveBeenCalledWith(
       'git status',
@@ -155,9 +168,8 @@ describe('ShellProcessor', () => {
     const prompt: PromptPipelineContent = createPromptPipelineContent(
       '!{git status} in !{pwd}',
     );
-    mockCheckCommandPermissions.mockReturnValue({
-      allAllowed: true,
-      disallowedCommands: [],
+    mockPolicyEngineCheck.mockResolvedValue({
+      decision: PolicyDecision.ALLOW,
     });
 
     mockShellExecute
@@ -173,7 +185,7 @@ describe('ShellProcessor', () => {
 
     const result = await processor.process(prompt, context);
 
-    expect(mockCheckCommandPermissions).toHaveBeenCalledTimes(2);
+    expect(mockPolicyEngineCheck).toHaveBeenCalledTimes(2);
     expect(mockShellExecute).toHaveBeenCalledTimes(2);
     expect(result).toEqual([{ text: 'On branch main in /usr/home' }]);
   });
@@ -183,9 +195,8 @@ describe('ShellProcessor', () => {
     const prompt: PromptPipelineContent = createPromptPipelineContent(
       'Do something dangerous: !{rm -rf /}',
     );
-    mockCheckCommandPermissions.mockReturnValue({
-      allAllowed: false,
-      disallowedCommands: ['rm -rf /'],
+    mockPolicyEngineCheck.mockResolvedValue({
+      decision: PolicyDecision.ASK_USER,
     });
 
     await expect(processor.process(prompt, context)).rejects.toThrow(
@@ -198,11 +209,11 @@ describe('ShellProcessor', () => {
     const prompt: PromptPipelineContent = createPromptPipelineContent(
       'Do something dangerous: !{rm -rf /}',
     );
-    mockCheckCommandPermissions.mockReturnValue({
-      allAllowed: false,
-      disallowedCommands: ['rm -rf /'],
+    // In YOLO mode, PolicyEngine returns ALLOW
+    mockPolicyEngineCheck.mockResolvedValue({
+      decision: PolicyDecision.ALLOW,
     });
-    // Override the approval mode for this test
+    // Override the approval mode for this test (though PolicyEngine mock handles the decision)
     (mockConfig.getApprovalMode as Mock).mockReturnValue(ApprovalMode.YOLO);
     mockShellExecute.mockReturnValue({
       result: Promise.resolve({ ...SUCCESS_RESULT, output: 'deleted' }),
@@ -227,17 +238,14 @@ describe('ShellProcessor', () => {
     const prompt: PromptPipelineContent = createPromptPipelineContent(
       'Do something forbidden: !{reboot}',
     );
-    mockCheckCommandPermissions.mockReturnValue({
-      allAllowed: false,
-      disallowedCommands: ['reboot'],
-      isHardDenial: true, // This is the key difference
-      blockReason: 'System commands are blocked',
+    mockPolicyEngineCheck.mockResolvedValue({
+      decision: PolicyDecision.DENY,
     });
     // Set approval mode to YOLO
     (mockConfig.getApprovalMode as Mock).mockReturnValue(ApprovalMode.YOLO);
 
     await expect(processor.process(prompt, context)).rejects.toThrow(
-      /Blocked command: "reboot". Reason: System commands are blocked/,
+      /Blocked command: "reboot". Reason: Blocked by policy/,
     );
 
     // Ensure it never tried to execute
@@ -249,9 +257,8 @@ describe('ShellProcessor', () => {
     const prompt: PromptPipelineContent = createPromptPipelineContent(
       'Do something dangerous: !{rm -rf /}',
     );
-    mockCheckCommandPermissions.mockReturnValue({
-      allAllowed: false,
-      disallowedCommands: ['rm -rf /'],
+    mockPolicyEngineCheck.mockResolvedValue({
+      decision: PolicyDecision.ASK_USER,
     });
 
     try {
@@ -273,14 +280,12 @@ describe('ShellProcessor', () => {
     const prompt: PromptPipelineContent = createPromptPipelineContent(
       '!{cmd1} and !{cmd2}',
     );
-    mockCheckCommandPermissions.mockImplementation((cmd) => {
-      if (cmd === 'cmd1') {
-        return { allAllowed: false, disallowedCommands: ['cmd1'] };
+    mockPolicyEngineCheck.mockImplementation(async (toolCall) => {
+      const cmd = toolCall.args.command;
+      if (cmd === 'cmd1' || cmd === 'cmd2') {
+        return { decision: PolicyDecision.ASK_USER };
       }
-      if (cmd === 'cmd2') {
-        return { allAllowed: false, disallowedCommands: ['cmd2'] };
-      }
-      return { allAllowed: true, disallowedCommands: [] };
+      return { decision: PolicyDecision.ALLOW };
     });
 
     try {
@@ -301,11 +306,12 @@ describe('ShellProcessor', () => {
       'First: !{echo "hello"}, Second: !{rm -rf /}',
     );
 
-    mockCheckCommandPermissions.mockImplementation((cmd) => {
+    mockPolicyEngineCheck.mockImplementation(async (toolCall) => {
+      const cmd = toolCall.args.command;
       if (cmd.includes('rm')) {
-        return { allAllowed: false, disallowedCommands: [cmd] };
+        return { decision: PolicyDecision.ASK_USER };
       }
-      return { allAllowed: true, disallowedCommands: [] };
+      return { decision: PolicyDecision.ALLOW };
     });
 
     await expect(processor.process(prompt, context)).rejects.toThrow(
@@ -322,10 +328,13 @@ describe('ShellProcessor', () => {
       'Allowed: !{ls -l}, Disallowed: !{rm -rf /}',
     );
 
-    mockCheckCommandPermissions.mockImplementation((cmd) => ({
-      allAllowed: !cmd.includes('rm'),
-      disallowedCommands: cmd.includes('rm') ? [cmd] : [],
-    }));
+    mockPolicyEngineCheck.mockImplementation(async (toolCall) => {
+      const cmd = toolCall.args.command;
+      if (cmd.includes('rm')) {
+        return { decision: PolicyDecision.ASK_USER };
+      }
+      return { decision: PolicyDecision.ALLOW };
+    });
 
     try {
       await processor.process(prompt, context);
@@ -344,13 +353,12 @@ describe('ShellProcessor', () => {
       'Run !{cmd1} and !{cmd2}',
     );
 
-    // Add commands to the session allowlist
+    // Add commands to the session allowlist (conceptually, in this test we just mock the engine allowing them)
     context.session.sessionShellAllowlist = new Set(['cmd1', 'cmd2']);
 
     // checkCommandPermissions should now pass for these
-    mockCheckCommandPermissions.mockReturnValue({
-      allAllowed: true,
-      disallowedCommands: [],
+    mockPolicyEngineCheck.mockResolvedValue({
+      decision: PolicyDecision.ALLOW,
     });
 
     mockShellExecute
@@ -363,20 +371,58 @@ describe('ShellProcessor', () => {
 
     const result = await processor.process(prompt, context);
 
-    expect(mockCheckCommandPermissions).toHaveBeenCalledWith(
-      'cmd1',
-      expect.any(Object),
-      context.session.sessionShellAllowlist,
-    );
-    expect(mockCheckCommandPermissions).toHaveBeenCalledWith(
-      'cmd2',
-      expect.any(Object),
-      context.session.sessionShellAllowlist,
-    );
+    expect(mockPolicyEngineCheck).not.toHaveBeenCalled();
     expect(mockShellExecute).toHaveBeenCalledTimes(2);
     expect(result).toEqual([{ text: 'Run output1 and output2' }]);
   });
 
+  it('should support the full confirmation flow (Ask -> Approve -> Retry)', async () => {
+    // 1. Initial State: Command NOT allowed
+    const processor = new ShellProcessor('test-command');
+    const prompt: PromptPipelineContent =
+      createPromptPipelineContent('!{echo "once"}');
+
+    // Policy Engine says ASK_USER
+    mockPolicyEngineCheck.mockResolvedValue({
+      decision: PolicyDecision.ASK_USER,
+    });
+
+    // 2. First Attempt: processing should fail with ConfirmationRequiredError
+    try {
+      await processor.process(prompt, context);
+      expect.fail('Should have thrown ConfirmationRequiredError');
+    } catch (e) {
+      expect(e).toBeInstanceOf(ConfirmationRequiredError);
+      expect(mockPolicyEngineCheck).toHaveBeenCalledTimes(1);
+    }
+
+    // 3. User Approves: Add to session allowlist (simulating UI action)
+    context.session.sessionShellAllowlist.add('echo "once"');
+
+    // 4. Retry: calling process() again with the same context
+    // Reset mocks to ensure we track new calls cleanly
+    mockPolicyEngineCheck.mockClear();
+
+    // Mock successful execution
+    mockShellExecute.mockReturnValue({
+      result: Promise.resolve({ ...SUCCESS_RESULT, output: 'once' }),
+    });
+
+    const result = await processor.process(prompt, context);
+
+    // 5. Verify Success AND Policy Engine Bypass
+    expect(mockPolicyEngineCheck).not.toHaveBeenCalled();
+    expect(mockShellExecute).toHaveBeenCalledWith(
+      'echo "once"',
+      expect.any(String),
+      expect.any(Function),
+      expect.any(Object),
+      false,
+      expect.any(Object),
+    );
+    expect(result).toEqual([{ text: 'once' }]);
+  });
+
   it('should trim whitespace from the command inside the injection before interpolation', async () => {
     const processor = new ShellProcessor('test-command');
     const prompt: PromptPipelineContent = createPromptPipelineContent(
@@ -389,9 +435,8 @@ describe('ShellProcessor', () => {
 
     const expectedCommand = `ls ${expectedEscapedArgs} -l`;
 
-    mockCheckCommandPermissions.mockReturnValue({
-      allAllowed: true,
-      disallowedCommands: [],
+    mockPolicyEngineCheck.mockResolvedValue({
+      decision: PolicyDecision.ALLOW,
     });
     mockShellExecute.mockReturnValue({
       result: Promise.resolve({ ...SUCCESS_RESULT, output: 'total 0' }),
@@ -399,10 +444,9 @@ describe('ShellProcessor', () => {
 
     await processor.process(prompt, context);
 
-    expect(mockCheckCommandPermissions).toHaveBeenCalledWith(
-      expectedCommand,
-      expect.any(Object),
-      context.session.sessionShellAllowlist,
+    expect(mockPolicyEngineCheck).toHaveBeenCalledWith(
+      { name: 'run_shell_command', args: { command: expectedCommand } },
+      undefined,
     );
     expect(mockShellExecute).toHaveBeenCalledWith(
       expectedCommand,
@@ -421,7 +465,7 @@ describe('ShellProcessor', () => {
 
     const result = await processor.process(prompt, context);
 
-    expect(mockCheckCommandPermissions).not.toHaveBeenCalled();
+    expect(mockPolicyEngineCheck).not.toHaveBeenCalled();
     expect(mockShellExecute).not.toHaveBeenCalled();
 
     // It replaces !{} with an empty string.
@@ -615,20 +659,20 @@ describe('ShellProcessor', () => {
 
       const expectedEscapedArgs = getExpectedEscapedArgForPlatform(rawArgs);
       const expectedResolvedCommand = `rm ${expectedEscapedArgs}`;
-      mockCheckCommandPermissions.mockReturnValue({
-        allAllowed: false,
-        disallowedCommands: [expectedResolvedCommand],
-        isHardDenial: false,
+      mockPolicyEngineCheck.mockResolvedValue({
+        decision: PolicyDecision.ASK_USER,
       });
 
       await expect(processor.process(prompt, context)).rejects.toThrow(
         ConfirmationRequiredError,
       );
 
-      expect(mockCheckCommandPermissions).toHaveBeenCalledWith(
-        expectedResolvedCommand,
-        expect.any(Object),
-        context.session.sessionShellAllowlist,
+      expect(mockPolicyEngineCheck).toHaveBeenCalledWith(
+        {
+          name: 'run_shell_command',
+          args: { command: expectedResolvedCommand },
+        },
+        undefined,
       );
     });
 
@@ -638,15 +682,12 @@ describe('ShellProcessor', () => {
         createPromptPipelineContent('!{rm {{args}}}');
       const expectedEscapedArgs = getExpectedEscapedArgForPlatform(rawArgs);
       const expectedResolvedCommand = `rm ${expectedEscapedArgs}`;
-      mockCheckCommandPermissions.mockReturnValue({
-        allAllowed: false,
-        disallowedCommands: [expectedResolvedCommand],
-        isHardDenial: true,
-        blockReason: 'It is forbidden.',
+      mockPolicyEngineCheck.mockResolvedValue({
+        decision: PolicyDecision.DENY,
       });
 
       await expect(processor.process(prompt, context)).rejects.toThrow(
-        `Blocked command: "${expectedResolvedCommand}". Reason: It is forbidden.`,
+        `Blocked command: "${expectedResolvedCommand}". Reason: Blocked by policy.`,
       );
     });
   });

packages/cli/src/services/prompt-processors/shellProcessor.ts

@@ -5,12 +5,11 @@
  */
 
 import {
-  ApprovalMode,
-  checkCommandPermissions,
   escapeShellArg,
   getShellConfiguration,
   ShellExecutionService,
   flatMapTextParts,
+  PolicyDecision,
 } from '@google/gemini-cli-core';
 
 import type { CommandContext } from '../../ui/commands/types.js';
@@ -81,7 +80,6 @@ export class ShellProcessor implements IPromptProcessor {
         `Security configuration not loaded. Cannot verify shell command permissions for '${this.commandName}'. Aborting.`,
       );
     }
-    const { sessionShellAllowlist } = context.session;
 
     const injections = extractInjections(
       prompt,
@@ -121,21 +119,25 @@ export class ShellProcessor implements IPromptProcessor {
 
       if (!command) continue;
 
+      if (context.session.sessionShellAllowlist?.has(command)) {
+        continue;
+      }
+
       // Security check on the final, escaped command string.
-      const { allAllowed, disallowedCommands, blockReason, isHardDenial } =
-        checkCommandPermissions(command, config, sessionShellAllowlist);
+      const { decision } = await config.getPolicyEngine().check(
+        {
+          name: 'run_shell_command',
+          args: { command },
+        },
+        undefined,
+      );
 
-      if (!allAllowed) {
-        if (isHardDenial) {
-          throw new Error(
-            `${this.commandName} cannot be run. Blocked command: "${command}". Reason: ${blockReason || 'Blocked by configuration.'}`,
-          );
-        }
-
-        // If not a hard denial, respect YOLO mode and auto-approve.
-        if (config.getApprovalMode() !== ApprovalMode.YOLO) {
-          disallowedCommands.forEach((uc) => commandsToConfirm.add(uc));
-        }
+      if (decision === PolicyDecision.DENY) {
+        throw new Error(
+          `${this.commandName} cannot be run. Blocked command: "${command}". Reason: Blocked by policy.`,
+        );
+      } else if (decision === PolicyDecision.ASK_USER) {
+        commandsToConfirm.add(command);
       }
     }
 

packages/cli/src/ui/hooks/useToolScheduler.test.ts

@@ -33,6 +33,7 @@ import {
   ApprovalMode,
   HookSystem,
   PREVIEW_GEMINI_MODEL,
+  PolicyDecision,
 } from '@google/gemini-cli-core';
 import { MockTool } from '@google/gemini-cli-core/src/test-utils/mock-tool.js';
 import { createMockMessageBus } from '@google/gemini-cli-core/src/test-utils/mock-message-bus.js';
@@ -80,13 +81,21 @@ const mockConfig = {
   getGeminiClient: () => null, // No client needed for these tests
   getShellExecutionConfig: () => ({ terminalWidth: 80, terminalHeight: 24 }),
   getMessageBus: () => null,
-  getPolicyEngine: () => null,
   isInteractive: () => false,
   getExperiments: () => {},
   getEnableHooks: () => false,
 } as unknown as Config;
 mockConfig.getMessageBus = vi.fn().mockReturnValue(createMockMessageBus());
 mockConfig.getHookSystem = vi.fn().mockReturnValue(new HookSystem(mockConfig));
+mockConfig.getPolicyEngine = vi.fn().mockReturnValue({
+  check: async () => {
+    const mode = mockConfig.getApprovalMode();
+    if (mode === ApprovalMode.YOLO) {
+      return { decision: PolicyDecision.ALLOW };
+    }
+    return { decision: PolicyDecision.ASK_USER };
+  },
+});
 
 function createMockConfigOverride(overrides: Partial<Config> = {}): Config {
   return { ...mockConfig, ...overrides } as Config;