From 738042478277a6cbe9b873fa4950bd1d86d925b2 Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Wed, 11 Mar 2026 16:58:58 -0700 Subject: [PATCH] fix(policy): ensure user policies are loaded when policyPaths is empty (#22090) --- integration-tests/user-policy.responses | 2 + integration-tests/user-policy.test.ts | 81 ++++++++++++++++++++ packages/cli/src/ui/hooks/useGeminiStream.ts | 4 +- packages/core/src/policy/config.test.ts | 50 ++++++++++++ packages/core/src/policy/config.ts | 4 +- 5 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 integration-tests/user-policy.responses create mode 100644 integration-tests/user-policy.test.ts diff --git a/integration-tests/user-policy.responses b/integration-tests/user-policy.responses new file mode 100644 index 0000000000..be840600ca --- /dev/null +++ b/integration-tests/user-policy.responses @@ -0,0 +1,2 @@ +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"run_shell_command","args":{"command":"ls -F"}}}]},"finishReason":"STOP","index":0}]},{"candidates":[{"content":{"parts":[{"text":"I ran ls -F"}]},"finishReason":"STOP","index":0}]}]} +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"I ran ls -F"}]},"finishReason":"STOP","index":0}]}]} diff --git a/integration-tests/user-policy.test.ts b/integration-tests/user-policy.test.ts new file mode 100644 index 0000000000..a07d6bcdea --- /dev/null +++ b/integration-tests/user-policy.test.ts @@ -0,0 +1,81 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { join } from 'node:path'; +import { TestRig, GEMINI_DIR } from './test-helper.js'; +import fs from 'node:fs'; + +describe('User Policy Regression Repro', () => { + let rig: TestRig; + + beforeEach(() => { + rig = new TestRig(); + }); + + afterEach(async () => { + if (rig) { + await rig.cleanup(); + } + }); + + it('should respect policies in ~/.gemini/policies/allowed-tools.toml', async () => { + rig.setup('user-policy-test', { + fakeResponsesPath: join(import.meta.dirname, 'user-policy.responses'), + }); + + // Create ~/.gemini/policies/allowed-tools.toml + const userPoliciesDir = join(rig.homeDir!, GEMINI_DIR, 'policies'); + fs.mkdirSync(userPoliciesDir, { recursive: true }); + fs.writeFileSync( + join(userPoliciesDir, 'allowed-tools.toml'), + ` +[[rule]] +toolName = "run_shell_command" +commandPrefix = "ls -F" +decision = "allow" +priority = 100 + `, + ); + + // Run gemini with a prompt that triggers ls -F + // approvalMode: 'default' in headless mode will DENY if it hits ASK_USER + const result = await rig.run({ + args: ['-p', 'Run ls -F', '--model', 'gemini-3.1-pro-preview'], + approvalMode: 'default', + }); + + expect(result).toContain('I ran ls -F'); + expect(result).not.toContain('Tool execution denied by policy'); + expect(result).not.toContain('Tool "run_shell_command" not found'); + + const toolLogs = rig.readToolLogs(); + const lsLog = toolLogs.find( + (l) => + l.toolRequest.name === 'run_shell_command' && + l.toolRequest.args.includes('ls -F'), + ); + expect(lsLog).toBeDefined(); + expect(lsLog?.toolRequest.success).toBe(true); + }); + + it('should FAIL if policy is not present (sanity check)', async () => { + rig.setup('user-policy-sanity-check', { + fakeResponsesPath: join(import.meta.dirname, 'user-policy.responses'), + }); + + // DO NOT create the policy file here + + // Run gemini with a prompt that triggers ls -F + const result = await rig.run({ + args: ['-p', 'Run ls -F', '--model', 'gemini-3.1-pro-preview'], + approvalMode: 'default', + }); + + // In non-interactive mode, it should be denied + expect(result).toContain('Tool "run_shell_command" not found'); + }); +}); diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index bc299e53e2..b481787bfd 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -1039,7 +1039,9 @@ export const useGeminiStream = ( return; } - const finishReasonMessages: Record = { + const finishReasonMessages: Partial< + Record + > = { [FinishReason.FINISH_REASON_UNSPECIFIED]: undefined, [FinishReason.STOP]: undefined, [FinishReason.MAX_TOKENS]: 'Response truncated due to token limits.', diff --git a/packages/core/src/policy/config.test.ts b/packages/core/src/policy/config.test.ts index 42a76e9fe5..0e2301c1c8 100644 --- a/packages/core/src/policy/config.test.ts +++ b/packages/core/src/policy/config.test.ts @@ -19,6 +19,7 @@ import { isDirectorySecure } from '../utils/security.js'; import { createPolicyEngineConfig, clearEmittedPolicyWarnings, + getPolicyDirectories, } from './config.js'; import { Storage } from '../config/storage.js'; import * as tomlLoader from './toml-loader.js'; @@ -746,3 +747,52 @@ modes = ["plan"] feedbackSpy.mockRestore(); }); }); + +describe('getPolicyDirectories', () => { + const USER_POLICIES_DIR = '/mock/user/policies'; + const SYSTEM_POLICIES_DIR = '/mock/system/policies'; + + beforeEach(() => { + vi.spyOn(Storage, 'getUserPoliciesDir').mockReturnValue(USER_POLICIES_DIR); + vi.spyOn(Storage, 'getSystemPoliciesDir').mockReturnValue( + SYSTEM_POLICIES_DIR, + ); + }); + + it('should include default user policies directory when policyPaths is undefined', () => { + const dirs = getPolicyDirectories(); + expect(dirs).toContain(USER_POLICIES_DIR); + }); + + it('should include default user policies directory when policyPaths is an empty array', () => { + // This is the specific case that regressed + const dirs = getPolicyDirectories(undefined, []); + expect(dirs).toContain(USER_POLICIES_DIR); + }); + + it('should replace default user policies directory when policyPaths has entries', () => { + const customPath = '/custom/policies'; + const dirs = getPolicyDirectories(undefined, [customPath]); + expect(dirs).toContain(customPath); + expect(dirs).not.toContain(USER_POLICIES_DIR); + }); + + it('should include all tiers in correct order', () => { + const defaultDir = '/default/policies'; + const workspaceDir = '/workspace/policies'; + const adminPath = '/admin/extra/policies'; + const userPath = '/user/custom/policies'; + + const dirs = getPolicyDirectories(defaultDir, [userPath], workspaceDir, [ + adminPath, + ]); + + // Order should be Admin -> User -> Workspace -> Default + // getPolicyDirectories returns them in that order (which is then reversed by the loader) + expect(dirs[0]).toBe(SYSTEM_POLICIES_DIR); + expect(dirs[1]).toBe(adminPath); + expect(dirs[2]).toBe(userPath); + expect(dirs[3]).toBe(workspaceDir); + expect(dirs[4]).toBe(defaultDir); + }); +}); diff --git a/packages/core/src/policy/config.ts b/packages/core/src/policy/config.ts index 435fb018d5..41f714cf96 100644 --- a/packages/core/src/policy/config.ts +++ b/packages/core/src/policy/config.ts @@ -124,7 +124,9 @@ export function getPolicyDirectories( ...(adminPolicyPaths ?? []), // User tier (second highest priority) - ...(policyPaths ?? [Storage.getUserPoliciesDir()]), + ...(policyPaths && policyPaths.length > 0 + ? policyPaths + : [Storage.getUserPoliciesDir()]), // Workspace Tier (third highest) workspacePoliciesDir,