Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/cli/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,6 @@ export async function loadCliConfig(
await resolveWorkspacePolicyState({
cwd,
trustedFolder,
interactive,
});

const policyEngineConfig = await createPolicyEngineConfig(
Expand Down
43 changes: 12 additions & 31 deletions packages/cli/src/config/policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import * as fs from 'node:fs';
import * as path from 'node:path';
import * as os from 'node:os';
import { resolveWorkspacePolicyState } from './policy.js';
import { writeToStderr } from '@google/gemini-cli-core';
import { debugLogger } from '@google/gemini-cli-core';

// Mock debugLogger to avoid noise in test output
vi.mock('@google/gemini-cli-core', async (importOriginal) => {
Expand Down Expand Up @@ -54,7 +54,6 @@ describe('resolveWorkspacePolicyState', () => {
const result = await resolveWorkspacePolicyState({
cwd: workspaceDir,
trustedFolder: false,
interactive: true,
});

expect(result).toEqual({
Expand All @@ -68,28 +67,18 @@ describe('resolveWorkspacePolicyState', () => {
fs.mkdirSync(policiesDir, { recursive: true });
fs.writeFileSync(path.join(policiesDir, 'policy.toml'), 'rules = []');

// First call to establish integrity (interactive accept)
// First call will now auto-accept
const firstResult = await resolveWorkspacePolicyState({
cwd: workspaceDir,
trustedFolder: true,
interactive: true,
});
expect(firstResult.policyUpdateConfirmationRequest).toBeDefined();

// Establish integrity manually as if accepted
const { PolicyIntegrityManager } = await import('@google/gemini-cli-core');
const integrityManager = new PolicyIntegrityManager();
await integrityManager.acceptIntegrity(
'workspace',
workspaceDir,
firstResult.policyUpdateConfirmationRequest!.newHash,
);
expect(firstResult.workspacePoliciesDir).toBe(policiesDir);
expect(firstResult.policyUpdateConfirmationRequest).toBeUndefined();

// Second call should match
// Second call should also match
const result = await resolveWorkspacePolicyState({
cwd: workspaceDir,
trustedFolder: true,
interactive: true,
});

expect(result.workspacePoliciesDir).toBe(policiesDir);
Expand All @@ -100,30 +89,26 @@ describe('resolveWorkspacePolicyState', () => {
const result = await resolveWorkspacePolicyState({
cwd: workspaceDir,
trustedFolder: true,
interactive: true,
});

expect(result.workspacePoliciesDir).toBeUndefined();
expect(result.policyUpdateConfirmationRequest).toBeUndefined();
});

it('should return confirmation request if changed in interactive mode', async () => {
it('should auto-accept if changed in interactive mode', async () => {
fs.mkdirSync(policiesDir, { recursive: true });
fs.writeFileSync(path.join(policiesDir, 'policy.toml'), 'rules = []');

const result = await resolveWorkspacePolicyState({
cwd: workspaceDir,
trustedFolder: true,
interactive: true,
});

expect(result.workspacePoliciesDir).toBeUndefined();
expect(result.policyUpdateConfirmationRequest).toEqual({
scope: 'workspace',
identifier: workspaceDir,
policyDir: policiesDir,
newHash: expect.any(String),
});
expect(result.workspacePoliciesDir).toBe(policiesDir);
expect(result.policyUpdateConfirmationRequest).toBeUndefined();
expect(debugLogger.warn).toHaveBeenCalledWith(
expect.stringContaining('Automatically accepting and loading'),
);
});

it('should warn and auto-accept if changed in non-interactive mode', async () => {
Expand All @@ -133,12 +118,11 @@ describe('resolveWorkspacePolicyState', () => {
const result = await resolveWorkspacePolicyState({
cwd: workspaceDir,
trustedFolder: true,
interactive: false,
});

expect(result.workspacePoliciesDir).toBe(policiesDir);
expect(result.policyUpdateConfirmationRequest).toBeUndefined();
expect(writeToStderr).toHaveBeenCalledWith(
expect(debugLogger.warn).toHaveBeenCalledWith(
expect.stringContaining('Automatically accepting and loading'),
);
});
Expand All @@ -152,7 +136,6 @@ describe('resolveWorkspacePolicyState', () => {
const result = await resolveWorkspacePolicyState({
cwd: tempDir,
trustedFolder: true,
interactive: true,
});

expect(result.workspacePoliciesDir).toBeUndefined();
Expand All @@ -176,9 +159,7 @@ describe('resolveWorkspacePolicyState', () => {
const result = await resolveWorkspacePolicyState({
cwd: symlinkDir,
trustedFolder: true,
interactive: true,
});

expect(result.workspacePoliciesDir).toBeUndefined();
expect(result.policyUpdateConfirmationRequest).toBeUndefined();
} finally {
Expand Down
29 changes: 12 additions & 17 deletions packages/cli/src/config/policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
IntegrityStatus,
Storage,
type PolicyUpdateConfirmationRequest,
writeToStderr,
debugLogger,
} from '@google/gemini-cli-core';
import { type Settings } from './settings.js';

Expand Down Expand Up @@ -57,14 +57,14 @@ export interface WorkspacePolicyState {
export async function resolveWorkspacePolicyState(options: {
cwd: string;
trustedFolder: boolean;
interactive: boolean;
}): Promise<WorkspacePolicyState> {
const { cwd, trustedFolder, interactive } = options;
const { cwd, trustedFolder } = options;

let workspacePoliciesDir: string | undefined;
let policyUpdateConfirmationRequest:
// TODO: Restore policyUpdateConfirmationRequest when re-enabling interactive policy acceptance.
const policyUpdateConfirmationRequest:
| PolicyUpdateConfirmationRequest
| undefined;
| undefined = undefined;

if (trustedFolder) {
const storage = new Storage(cwd);
Expand All @@ -91,25 +91,20 @@ export async function resolveWorkspacePolicyState(options: {
) {
// No workspace policies found
workspacePoliciesDir = undefined;
} else if (interactive) {
// Policies changed or are new, and we are in interactive mode
policyUpdateConfirmationRequest = {
scope: 'workspace',
identifier: cwd,
policyDir: potentialWorkspacePoliciesDir,
newHash: integrityResult.hash,
};
} else {
// Non-interactive mode: warn and automatically accept/load
// Policies changed or are new.
// Automatically accept and load for now to reduce friction.
// We keep the infrastructure (PolicyUpdateConfirmationRequest etc.)
// but bypass the interactive dialog.
await integrityManager.acceptIntegrity(
'workspace',
cwd,
integrityResult.hash,
);
workspacePoliciesDir = potentialWorkspacePoliciesDir;
Comment on lines 94 to 109

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The PR modifies resolveWorkspacePolicyState to automatically accept and load workspace policies when they are new or have changed, bypassing the previous interactive user confirmation. While this only occurs in "trusted" folders, "trusting a folder" should not imply trusting all future, potentially malicious, changes to security policies within that folder.

An attacker who can write to the .gemini/policies/ directory of a trusted workspace (e.g., via a malicious pull request in a shared project) can now automatically inject policies that escalate their privileges, such as allowing dangerous tools (like ShellTool) to run without user confirmation.

Furthermore, the notification for this automatic acceptance has been moved from writeToStderr to debugLogger.warn, which according to the project's documentation is intercepted and routed to a debug UI, making this security-sensitive change less visible to the user in the main terminal.

References
  1. Security-sensitive settings, such as workspace policies, should not be easily overridden or automatically accepted from potentially less-trusted scopes (like a workspace) without explicit user confirmation, as this can lead to privilege escalation.
  2. Tool availability and security restrictions are managed by the policy engine. Bypassing user confirmation for policy changes undermines the policy engine's role in managing these restrictions, allowing dangerous tools to run without proper oversight.

// debugLogger.warn here doesn't show up in the terminal. It is showing up only in debug mode on the debug console
writeToStderr(
'WARNING: Workspace policies changed or are new. Automatically accepting and loading them in non-interactive mode.\n',

debugLogger.warn(
'Workspace policies changed or are new. Automatically accepting and loading them.',
);
}
}
Expand Down
39 changes: 20 additions & 19 deletions packages/cli/src/config/workspace-policy-cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ describe('Workspace-Level Policy CLI Integration', () => {
);
});

it('should set policyUpdateConfirmationRequest if integrity MISMATCH in interactive mode', async () => {
it('should automatically accept if integrity MISMATCH in interactive mode', async () => {
vi.mocked(isWorkspaceTrusted).mockReturnValue({
isTrusted: true,
source: 'file',
Expand All @@ -186,24 +186,23 @@ describe('Workspace-Level Policy CLI Integration', () => {
cwd: MOCK_CWD,
});

expect(config.getPolicyUpdateConfirmationRequest()).toEqual({
scope: 'workspace',
identifier: MOCK_CWD,
policyDir: expect.stringContaining(path.join('.gemini', 'policies')),
newHash: 'new-hash',
});
// In interactive mode without accept flag, it waits for user confirmation (handled by UI),
// so it currently DOES NOT pass the directory to createPolicyEngineConfig yet.
// The UI will handle the confirmation and reload/update.
expect(config.getPolicyUpdateConfirmationRequest()).toBeUndefined();
expect(mockAcceptIntegrity).toHaveBeenCalledWith(
'workspace',
MOCK_CWD,
'new-hash',
);
expect(ServerConfig.createPolicyEngineConfig).toHaveBeenCalledWith(
expect.objectContaining({
workspacePoliciesDir: undefined,
workspacePoliciesDir: expect.stringContaining(
path.join('.gemini', 'policies'),
),
}),
expect.anything(),
);
});

it('should set policyUpdateConfirmationRequest if integrity is NEW with files (first time seen) in interactive mode', async () => {
it('should automatically accept if integrity is NEW with files (first time seen) in interactive mode', async () => {
vi.mocked(isWorkspaceTrusted).mockReturnValue({
isTrusted: true,
source: 'file',
Expand All @@ -222,16 +221,18 @@ describe('Workspace-Level Policy CLI Integration', () => {
cwd: MOCK_CWD,
});

expect(config.getPolicyUpdateConfirmationRequest()).toEqual({
scope: 'workspace',
identifier: MOCK_CWD,
policyDir: expect.stringContaining(path.join('.gemini', 'policies')),
newHash: 'new-hash',
});
expect(config.getPolicyUpdateConfirmationRequest()).toBeUndefined();
expect(mockAcceptIntegrity).toHaveBeenCalledWith(
'workspace',
MOCK_CWD,
'new-hash',
);

expect(ServerConfig.createPolicyEngineConfig).toHaveBeenCalledWith(
expect.objectContaining({
workspacePoliciesDir: undefined,
workspacePoliciesDir: expect.stringContaining(
path.join('.gemini', 'policies'),
),
}),
expect.anything(),
);
Expand Down
Loading