mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-13 05:12:55 -07:00
fix(policy): ensure user policies are loaded when policyPaths is empty (#22090)
This commit is contained in:
@@ -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}]}]}
|
||||||
@@ -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');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1039,7 +1039,9 @@ export const useGeminiStream = (
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const finishReasonMessages: Record<FinishReason, string | undefined> = {
|
const finishReasonMessages: Partial<
|
||||||
|
Record<FinishReason, string | undefined>
|
||||||
|
> = {
|
||||||
[FinishReason.FINISH_REASON_UNSPECIFIED]: undefined,
|
[FinishReason.FINISH_REASON_UNSPECIFIED]: undefined,
|
||||||
[FinishReason.STOP]: undefined,
|
[FinishReason.STOP]: undefined,
|
||||||
[FinishReason.MAX_TOKENS]: 'Response truncated due to token limits.',
|
[FinishReason.MAX_TOKENS]: 'Response truncated due to token limits.',
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ import { isDirectorySecure } from '../utils/security.js';
|
|||||||
import {
|
import {
|
||||||
createPolicyEngineConfig,
|
createPolicyEngineConfig,
|
||||||
clearEmittedPolicyWarnings,
|
clearEmittedPolicyWarnings,
|
||||||
|
getPolicyDirectories,
|
||||||
} from './config.js';
|
} from './config.js';
|
||||||
import { Storage } from '../config/storage.js';
|
import { Storage } from '../config/storage.js';
|
||||||
import * as tomlLoader from './toml-loader.js';
|
import * as tomlLoader from './toml-loader.js';
|
||||||
@@ -746,3 +747,52 @@ modes = ["plan"]
|
|||||||
feedbackSpy.mockRestore();
|
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);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -124,7 +124,9 @@ export function getPolicyDirectories(
|
|||||||
...(adminPolicyPaths ?? []),
|
...(adminPolicyPaths ?? []),
|
||||||
|
|
||||||
// User tier (second highest priority)
|
// User tier (second highest priority)
|
||||||
...(policyPaths ?? [Storage.getUserPoliciesDir()]),
|
...(policyPaths && policyPaths.length > 0
|
||||||
|
? policyPaths
|
||||||
|
: [Storage.getUserPoliciesDir()]),
|
||||||
|
|
||||||
// Workspace Tier (third highest)
|
// Workspace Tier (third highest)
|
||||||
workspacePoliciesDir,
|
workspacePoliciesDir,
|
||||||
|
|||||||
Reference in New Issue
Block a user