mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-22 02:54:31 -07:00
Clean up dead code (#17443)
This commit is contained in:
Tommaso Sciortino
committed by
GitHub
GitHub
parent
84e882770b
commit
80e1fa198f
@@ -1821,291 +1821,4 @@ describe('PolicyEngine', () => {
|
||||
expect(result.decision).toBe(PolicyDecision.DENY);
|
||||
});
|
||||
});
|
||||
|
||||
describe('checkHook', () => {
|
||||
it('should allow hooks by default', async () => {
|
||||
engine = new PolicyEngine({}, mockCheckerRunner);
|
||||
const decision = await engine.checkHook({
|
||||
eventName: 'BeforeTool',
|
||||
hookSource: 'user',
|
||||
});
|
||||
expect(decision).toBe(PolicyDecision.ALLOW);
|
||||
});
|
||||
|
||||
it('should deny all hooks when allowHooks is false', async () => {
|
||||
engine = new PolicyEngine({ allowHooks: false }, mockCheckerRunner);
|
||||
const decision = await engine.checkHook({
|
||||
eventName: 'BeforeTool',
|
||||
hookSource: 'user',
|
||||
});
|
||||
expect(decision).toBe(PolicyDecision.DENY);
|
||||
});
|
||||
|
||||
it('should deny project hooks in untrusted folders', async () => {
|
||||
engine = new PolicyEngine({}, mockCheckerRunner);
|
||||
const decision = await engine.checkHook({
|
||||
eventName: 'BeforeTool',
|
||||
hookSource: 'project',
|
||||
trustedFolder: false,
|
||||
});
|
||||
expect(decision).toBe(PolicyDecision.DENY);
|
||||
});
|
||||
|
||||
it('should allow project hooks in trusted folders', async () => {
|
||||
engine = new PolicyEngine({}, mockCheckerRunner);
|
||||
const decision = await engine.checkHook({
|
||||
eventName: 'BeforeTool',
|
||||
hookSource: 'project',
|
||||
trustedFolder: true,
|
||||
});
|
||||
expect(decision).toBe(PolicyDecision.ALLOW);
|
||||
});
|
||||
|
||||
it('should allow user hooks in untrusted folders', async () => {
|
||||
engine = new PolicyEngine({}, mockCheckerRunner);
|
||||
const decision = await engine.checkHook({
|
||||
eventName: 'BeforeTool',
|
||||
hookSource: 'user',
|
||||
trustedFolder: false,
|
||||
});
|
||||
expect(decision).toBe(PolicyDecision.ALLOW);
|
||||
});
|
||||
|
||||
it('should run hook checkers and deny on DENY decision', async () => {
|
||||
const hookCheckers = [
|
||||
{
|
||||
eventName: 'BeforeTool',
|
||||
checker: { type: 'external' as const, name: 'test-hook-checker' },
|
||||
},
|
||||
];
|
||||
engine = new PolicyEngine({ hookCheckers }, mockCheckerRunner);
|
||||
|
||||
vi.mocked(mockCheckerRunner.runChecker).mockResolvedValue({
|
||||
decision: SafetyCheckDecision.DENY,
|
||||
reason: 'Hook checker denied',
|
||||
});
|
||||
|
||||
const decision = await engine.checkHook({
|
||||
eventName: 'BeforeTool',
|
||||
hookSource: 'user',
|
||||
});
|
||||
|
||||
expect(decision).toBe(PolicyDecision.DENY);
|
||||
expect(mockCheckerRunner.runChecker).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ name: 'hook:BeforeTool' }),
|
||||
expect.objectContaining({ name: 'test-hook-checker' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('should run hook checkers and allow on ALLOW decision', async () => {
|
||||
const hookCheckers = [
|
||||
{
|
||||
eventName: 'BeforeTool',
|
||||
checker: { type: 'external' as const, name: 'test-hook-checker' },
|
||||
},
|
||||
];
|
||||
engine = new PolicyEngine({ hookCheckers }, mockCheckerRunner);
|
||||
|
||||
vi.mocked(mockCheckerRunner.runChecker).mockResolvedValue({
|
||||
decision: SafetyCheckDecision.ALLOW,
|
||||
});
|
||||
|
||||
const decision = await engine.checkHook({
|
||||
eventName: 'BeforeTool',
|
||||
hookSource: 'user',
|
||||
});
|
||||
|
||||
expect(decision).toBe(PolicyDecision.ALLOW);
|
||||
});
|
||||
|
||||
it('should return ASK_USER when checker requests it', async () => {
|
||||
const hookCheckers = [
|
||||
{
|
||||
checker: { type: 'external' as const, name: 'test-hook-checker' },
|
||||
},
|
||||
];
|
||||
engine = new PolicyEngine({ hookCheckers }, mockCheckerRunner);
|
||||
|
||||
vi.mocked(mockCheckerRunner.runChecker).mockResolvedValue({
|
||||
decision: SafetyCheckDecision.ASK_USER,
|
||||
reason: 'Needs confirmation',
|
||||
});
|
||||
|
||||
const decision = await engine.checkHook({
|
||||
eventName: 'BeforeTool',
|
||||
hookSource: 'user',
|
||||
});
|
||||
|
||||
expect(decision).toBe(PolicyDecision.ASK_USER);
|
||||
});
|
||||
|
||||
it('should return DENY for ASK_USER in non-interactive mode', async () => {
|
||||
const hookCheckers = [
|
||||
{
|
||||
checker: { type: 'external' as const, name: 'test-hook-checker' },
|
||||
},
|
||||
];
|
||||
engine = new PolicyEngine(
|
||||
{ hookCheckers, nonInteractive: true },
|
||||
mockCheckerRunner,
|
||||
);
|
||||
|
||||
vi.mocked(mockCheckerRunner.runChecker).mockResolvedValue({
|
||||
decision: SafetyCheckDecision.ASK_USER,
|
||||
reason: 'Needs confirmation',
|
||||
});
|
||||
|
||||
const decision = await engine.checkHook({
|
||||
eventName: 'BeforeTool',
|
||||
hookSource: 'user',
|
||||
});
|
||||
|
||||
expect(decision).toBe(PolicyDecision.DENY);
|
||||
});
|
||||
|
||||
it('should match hook checkers by eventName', async () => {
|
||||
const hookCheckers = [
|
||||
{
|
||||
eventName: 'AfterTool',
|
||||
checker: { type: 'external' as const, name: 'after-tool-checker' },
|
||||
},
|
||||
{
|
||||
eventName: 'BeforeTool',
|
||||
checker: { type: 'external' as const, name: 'before-tool-checker' },
|
||||
},
|
||||
];
|
||||
engine = new PolicyEngine({ hookCheckers }, mockCheckerRunner);
|
||||
|
||||
vi.mocked(mockCheckerRunner.runChecker).mockResolvedValue({
|
||||
decision: SafetyCheckDecision.ALLOW,
|
||||
});
|
||||
|
||||
await engine.checkHook({
|
||||
eventName: 'BeforeTool',
|
||||
hookSource: 'user',
|
||||
});
|
||||
|
||||
expect(mockCheckerRunner.runChecker).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.objectContaining({ name: 'before-tool-checker' }),
|
||||
);
|
||||
expect(mockCheckerRunner.runChecker).not.toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.objectContaining({ name: 'after-tool-checker' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('should match hook checkers by hookSource', async () => {
|
||||
const hookCheckers = [
|
||||
{
|
||||
hookSource: 'project' as const,
|
||||
checker: { type: 'external' as const, name: 'project-checker' },
|
||||
},
|
||||
{
|
||||
hookSource: 'user' as const,
|
||||
checker: { type: 'external' as const, name: 'user-checker' },
|
||||
},
|
||||
];
|
||||
engine = new PolicyEngine({ hookCheckers }, mockCheckerRunner);
|
||||
|
||||
vi.mocked(mockCheckerRunner.runChecker).mockResolvedValue({
|
||||
decision: SafetyCheckDecision.ALLOW,
|
||||
});
|
||||
|
||||
await engine.checkHook({
|
||||
eventName: 'BeforeTool',
|
||||
hookSource: 'user',
|
||||
});
|
||||
|
||||
expect(mockCheckerRunner.runChecker).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.objectContaining({ name: 'user-checker' }),
|
||||
);
|
||||
expect(mockCheckerRunner.runChecker).not.toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.objectContaining({ name: 'project-checker' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('should deny when hook checker throws an error', async () => {
|
||||
const hookCheckers = [
|
||||
{
|
||||
checker: { type: 'external' as const, name: 'failing-checker' },
|
||||
},
|
||||
];
|
||||
engine = new PolicyEngine({ hookCheckers }, mockCheckerRunner);
|
||||
|
||||
vi.mocked(mockCheckerRunner.runChecker).mockRejectedValue(
|
||||
new Error('Checker failed'),
|
||||
);
|
||||
|
||||
const decision = await engine.checkHook({
|
||||
eventName: 'BeforeTool',
|
||||
hookSource: 'user',
|
||||
});
|
||||
|
||||
expect(decision).toBe(PolicyDecision.DENY);
|
||||
});
|
||||
|
||||
it('should run hook checkers in priority order', async () => {
|
||||
const hookCheckers = [
|
||||
{
|
||||
priority: 5,
|
||||
checker: { type: 'external' as const, name: 'low-priority' },
|
||||
},
|
||||
{
|
||||
priority: 20,
|
||||
checker: { type: 'external' as const, name: 'high-priority' },
|
||||
},
|
||||
{
|
||||
priority: 10,
|
||||
checker: { type: 'external' as const, name: 'medium-priority' },
|
||||
},
|
||||
];
|
||||
engine = new PolicyEngine({ hookCheckers }, mockCheckerRunner);
|
||||
|
||||
vi.mocked(mockCheckerRunner.runChecker).mockImplementation(
|
||||
async (_call, config) => {
|
||||
if (config.name === 'high-priority') {
|
||||
return { decision: SafetyCheckDecision.DENY, reason: 'denied' };
|
||||
}
|
||||
return { decision: SafetyCheckDecision.ALLOW };
|
||||
},
|
||||
);
|
||||
|
||||
await engine.checkHook({
|
||||
eventName: 'BeforeTool',
|
||||
hookSource: 'user',
|
||||
});
|
||||
|
||||
// Should only call the high-priority checker (first in sorted order)
|
||||
expect(mockCheckerRunner.runChecker).toHaveBeenCalledTimes(1);
|
||||
expect(mockCheckerRunner.runChecker).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.objectContaining({ name: 'high-priority' }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('addHookChecker', () => {
|
||||
it('should add a new hook checker and maintain priority order', () => {
|
||||
engine = new PolicyEngine({}, mockCheckerRunner);
|
||||
|
||||
engine.addHookChecker({
|
||||
priority: 5,
|
||||
checker: { type: 'external', name: 'checker1' },
|
||||
});
|
||||
engine.addHookChecker({
|
||||
priority: 10,
|
||||
checker: { type: 'external', name: 'checker2' },
|
||||
});
|
||||
|
||||
const checkers = engine.getHookCheckers();
|
||||
expect(checkers).toHaveLength(2);
|
||||
expect(checkers[0].priority).toBe(10);
|
||||
expect(checkers[0].checker.name).toBe('checker2');
|
||||
expect(checkers[1].priority).toBe(5);
|
||||
expect(checkers[1].checker.name).toBe('checker1');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -11,8 +11,6 @@ import {
|
||||
type PolicyRule,
|
||||
type SafetyCheckerRule,
|
||||
type HookCheckerRule,
|
||||
type HookExecutionContext,
|
||||
getHookSource,
|
||||
ApprovalMode,
|
||||
type CheckResult,
|
||||
} from './types.js';
|
||||
@@ -20,7 +18,6 @@ import { stableStringify } from './stable-stringify.js';
|
||||
import { debugLogger } from '../utils/debugLogger.js';
|
||||
import type { CheckerRunner } from '../safety/checker-runner.js';
|
||||
import { SafetyCheckDecision } from '../safety/protocol.js';
|
||||
import type { HookExecutionRequest } from '../confirmation-bus/types.js';
|
||||
import {
|
||||
SHELL_TOOL_NAMES,
|
||||
initializeShellParsers,
|
||||
@@ -81,26 +78,6 @@ function ruleMatches(
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a hook checker rule matches a hook execution context.
|
||||
*/
|
||||
function hookCheckerMatches(
|
||||
rule: HookCheckerRule,
|
||||
context: HookExecutionContext,
|
||||
): boolean {
|
||||
// Check event name if specified
|
||||
if (rule.eventName && rule.eventName !== context.eventName) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check hook source if specified
|
||||
if (rule.hookSource && rule.hookSource !== context.hookSource) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
export class PolicyEngine {
|
||||
private rules: PolicyRule[];
|
||||
private checkers: SafetyCheckerRule[];
|
||||
@@ -108,7 +85,6 @@ export class PolicyEngine {
|
||||
private readonly defaultDecision: PolicyDecision;
|
||||
private readonly nonInteractive: boolean;
|
||||
private readonly checkerRunner?: CheckerRunner;
|
||||
private readonly allowHooks: boolean;
|
||||
private approvalMode: ApprovalMode;
|
||||
|
||||
constructor(config: PolicyEngineConfig = {}, checkerRunner?: CheckerRunner) {
|
||||
@@ -124,7 +100,6 @@ export class PolicyEngine {
|
||||
this.defaultDecision = config.defaultDecision ?? PolicyDecision.ASK_USER;
|
||||
this.nonInteractive = config.nonInteractive ?? false;
|
||||
this.checkerRunner = checkerRunner;
|
||||
this.allowHooks = config.allowHooks ?? true;
|
||||
this.approvalMode = config.approvalMode ?? ApprovalMode.DEFAULT;
|
||||
}
|
||||
|
||||
@@ -495,84 +470,6 @@ export class PolicyEngine {
|
||||
return this.hookCheckers;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a hook execution is allowed based on the configured policies.
|
||||
* Runs hook-specific safety checkers if configured.
|
||||
*/
|
||||
async checkHook(
|
||||
request: HookExecutionRequest | HookExecutionContext,
|
||||
): Promise<PolicyDecision> {
|
||||
// If hooks are globally disabled, deny all hook executions
|
||||
if (!this.allowHooks) {
|
||||
return PolicyDecision.DENY;
|
||||
}
|
||||
|
||||
const context: HookExecutionContext =
|
||||
'input' in request
|
||||
? {
|
||||
eventName: request.eventName,
|
||||
hookSource: getHookSource(request.input),
|
||||
trustedFolder:
|
||||
typeof request.input['trusted_folder'] === 'boolean'
|
||||
? request.input['trusted_folder']
|
||||
: undefined,
|
||||
}
|
||||
: request;
|
||||
|
||||
// In untrusted folders, deny project-level hooks
|
||||
if (context.trustedFolder === false && context.hookSource === 'project') {
|
||||
return PolicyDecision.DENY;
|
||||
}
|
||||
|
||||
// Run hook-specific safety checkers if configured
|
||||
if (this.checkerRunner && this.hookCheckers.length > 0) {
|
||||
for (const checkerRule of this.hookCheckers) {
|
||||
if (hookCheckerMatches(checkerRule, context)) {
|
||||
debugLogger.debug(
|
||||
`[PolicyEngine.checkHook] Running hook checker: ${checkerRule.checker.name} for event: ${context.eventName}`,
|
||||
);
|
||||
try {
|
||||
// Create a synthetic function call for the checker runner
|
||||
// This allows reusing the existing checker infrastructure
|
||||
const syntheticCall = {
|
||||
name: `hook:${context.eventName}`,
|
||||
args: {
|
||||
hookSource: context.hookSource,
|
||||
trustedFolder: context.trustedFolder,
|
||||
},
|
||||
};
|
||||
|
||||
const result = await this.checkerRunner.runChecker(
|
||||
syntheticCall,
|
||||
checkerRule.checker,
|
||||
);
|
||||
|
||||
if (result.decision === SafetyCheckDecision.DENY) {
|
||||
debugLogger.debug(
|
||||
`[PolicyEngine.checkHook] Hook checker denied: ${result.reason}`,
|
||||
);
|
||||
return PolicyDecision.DENY;
|
||||
} else if (result.decision === SafetyCheckDecision.ASK_USER) {
|
||||
debugLogger.debug(
|
||||
`[PolicyEngine.checkHook] Hook checker requested ASK_USER: ${result.reason}`,
|
||||
);
|
||||
// For hooks, ASK_USER is treated as DENY in non-interactive mode
|
||||
return this.applyNonInteractiveMode(PolicyDecision.ASK_USER);
|
||||
}
|
||||
} catch (error) {
|
||||
debugLogger.debug(
|
||||
`[PolicyEngine.checkHook] Hook checker failed: ${error}`,
|
||||
);
|
||||
return PolicyDecision.DENY;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Default: Allow hooks
|
||||
return PolicyDecision.ALLOW;
|
||||
}
|
||||
|
||||
private applyNonInteractiveMode(decision: PolicyDecision): PolicyDecision {
|
||||
// In non-interactive mode, ASK_USER becomes DENY
|
||||
if (this.nonInteractive && decision === PolicyDecision.ASK_USER) {
|
||||
|
||||
Reference in New Issue
Block a user