mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-12 21:03:05 -07:00
fix(core): fail closed in YOLO mode when shell parsing fails for restricted rules (#25935)
This commit is contained in:
@@ -20,7 +20,10 @@ import {
|
|||||||
import type { FunctionCall } from '@google/genai';
|
import type { FunctionCall } from '@google/genai';
|
||||||
import { SafetyCheckDecision } from '../safety/protocol.js';
|
import { SafetyCheckDecision } from '../safety/protocol.js';
|
||||||
import type { CheckerRunner } from '../safety/checker-runner.js';
|
import type { CheckerRunner } from '../safety/checker-runner.js';
|
||||||
import { initializeShellParsers } from '../utils/shell-utils.js';
|
import {
|
||||||
|
initializeShellParsers,
|
||||||
|
parseCommandDetails,
|
||||||
|
} from '../utils/shell-utils.js';
|
||||||
import { buildArgsPatterns } from './utils.js';
|
import { buildArgsPatterns } from './utils.js';
|
||||||
import {
|
import {
|
||||||
NoopSandboxManager,
|
NoopSandboxManager,
|
||||||
@@ -477,6 +480,77 @@ describe('PolicyEngine', () => {
|
|||||||
const { decision } = await engine.check({ name: 'test-tool' }, undefined);
|
const { decision } = await engine.check({ name: 'test-tool' }, undefined);
|
||||||
expect(decision).toBe(PolicyDecision.DENY);
|
expect(decision).toBe(PolicyDecision.DENY);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should fail closed in YOLO mode when shell parsing fails for restricted rule', async () => {
|
||||||
|
const originalMock = vi
|
||||||
|
.mocked(parseCommandDetails)
|
||||||
|
.getMockImplementation();
|
||||||
|
vi.mocked(parseCommandDetails).mockImplementationOnce(
|
||||||
|
(command: string) => {
|
||||||
|
if (command === 'echo bypass') {
|
||||||
|
return { details: [], hasError: true };
|
||||||
|
}
|
||||||
|
return originalMock!(command);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
const rules: PolicyRule[] = [
|
||||||
|
{
|
||||||
|
toolName: 'run_shell_command',
|
||||||
|
decision: PolicyDecision.ALLOW,
|
||||||
|
argsPattern: /"command":"echo/,
|
||||||
|
},
|
||||||
|
];
|
||||||
|
|
||||||
|
engine = new PolicyEngine({
|
||||||
|
rules,
|
||||||
|
approvalMode: ApprovalMode.YOLO,
|
||||||
|
});
|
||||||
|
|
||||||
|
const { decision } = await engine.check(
|
||||||
|
{ name: 'run_shell_command', args: { command: 'echo bypass' } },
|
||||||
|
undefined,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(decision).toBe(PolicyDecision.DENY);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should fail closed in YOLO mode when shell parsing has errors for restricted rule', async () => {
|
||||||
|
const originalMock = vi
|
||||||
|
.mocked(parseCommandDetails)
|
||||||
|
.getMockImplementation();
|
||||||
|
vi.mocked(parseCommandDetails).mockImplementationOnce(
|
||||||
|
(command: string) => {
|
||||||
|
if (command === 'echo bypass') {
|
||||||
|
return {
|
||||||
|
details: [{ name: 'echo', text: 'echo bypass', startIndex: 0 }],
|
||||||
|
hasError: true,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
return originalMock!(command);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
const rules: PolicyRule[] = [
|
||||||
|
{
|
||||||
|
toolName: 'run_shell_command',
|
||||||
|
decision: PolicyDecision.ALLOW,
|
||||||
|
argsPattern: /"command":"echo/,
|
||||||
|
},
|
||||||
|
];
|
||||||
|
|
||||||
|
engine = new PolicyEngine({
|
||||||
|
rules,
|
||||||
|
approvalMode: ApprovalMode.YOLO,
|
||||||
|
});
|
||||||
|
|
||||||
|
const { decision } = await engine.check(
|
||||||
|
{ name: 'run_shell_command', args: { command: 'echo bypass' } },
|
||||||
|
undefined,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(decision).toBe(PolicyDecision.DENY);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('addRule', () => {
|
describe('addRule', () => {
|
||||||
|
|||||||
@@ -364,14 +364,22 @@ export class PolicyEngine {
|
|||||||
const parsed = parseCommandDetails(command);
|
const parsed = parseCommandDetails(command);
|
||||||
const subCommands = parsed?.details ?? [];
|
const subCommands = parsed?.details ?? [];
|
||||||
|
|
||||||
if (subCommands.length === 0) {
|
// Handle parser failures or syntax errors
|
||||||
|
if (subCommands.length === 0 || parsed?.hasError) {
|
||||||
// If the matched rule says DENY, we should respect it immediately even if parsing fails.
|
// If the matched rule says DENY, we should respect it immediately even if parsing fails.
|
||||||
if (ruleDecision === PolicyDecision.DENY) {
|
if (ruleDecision === PolicyDecision.DENY) {
|
||||||
return { decision: PolicyDecision.DENY, rule };
|
return { decision: PolicyDecision.DENY, rule };
|
||||||
}
|
}
|
||||||
|
|
||||||
// In YOLO mode, we should proceed anyway even if we can't parse the command.
|
|
||||||
if (this.approvalMode === ApprovalMode.YOLO) {
|
if (this.approvalMode === ApprovalMode.YOLO) {
|
||||||
|
// Block execution if arguments cannot be validated
|
||||||
|
if (rule?.argsPattern) {
|
||||||
|
debugLogger.debug(
|
||||||
|
`[PolicyEngine.check] Parsing failed for restricted rule, forcing DENY: ${command}`,
|
||||||
|
);
|
||||||
|
return { decision: PolicyDecision.DENY, rule };
|
||||||
|
}
|
||||||
|
// Allow if no argument restrictions apply
|
||||||
return {
|
return {
|
||||||
decision: PolicyDecision.ALLOW,
|
decision: PolicyDecision.ALLOW,
|
||||||
rule,
|
rule,
|
||||||
|
|||||||
Reference in New Issue
Block a user