mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-28 14:04:41 -07:00
feat(hooks): Hook Event Handling (#9097)
This commit is contained in:
@@ -1250,4 +1250,291 @@ 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');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -10,11 +10,15 @@ import {
|
||||
type PolicyEngineConfig,
|
||||
type PolicyRule,
|
||||
type SafetyCheckerRule,
|
||||
type HookCheckerRule,
|
||||
type HookExecutionContext,
|
||||
getHookSource,
|
||||
} from './types.js';
|
||||
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';
|
||||
|
||||
function ruleMatches(
|
||||
rule: PolicyRule | SafetyCheckerRule,
|
||||
@@ -61,12 +65,34 @@ 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[];
|
||||
private hookCheckers: HookCheckerRule[];
|
||||
private readonly defaultDecision: PolicyDecision;
|
||||
private readonly nonInteractive: boolean;
|
||||
private readonly checkerRunner?: CheckerRunner;
|
||||
private readonly allowHooks: boolean;
|
||||
|
||||
constructor(config: PolicyEngineConfig = {}, checkerRunner?: CheckerRunner) {
|
||||
this.rules = (config.rules ?? []).sort(
|
||||
@@ -75,9 +101,13 @@ export class PolicyEngine {
|
||||
this.checkers = (config.checkers ?? []).sort(
|
||||
(a, b) => (b.priority ?? 0) - (a.priority ?? 0),
|
||||
);
|
||||
this.hookCheckers = (config.hookCheckers ?? []).sort(
|
||||
(a, b) => (b.priority ?? 0) - (a.priority ?? 0),
|
||||
);
|
||||
this.defaultDecision = config.defaultDecision ?? PolicyDecision.ASK_USER;
|
||||
this.nonInteractive = config.nonInteractive ?? false;
|
||||
this.checkerRunner = checkerRunner;
|
||||
this.allowHooks = config.allowHooks ?? true;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -206,6 +236,99 @@ export class PolicyEngine {
|
||||
return this.checkers;
|
||||
}
|
||||
|
||||
/**
|
||||
* Add a new hook checker to the policy engine.
|
||||
*/
|
||||
addHookChecker(checker: HookCheckerRule): void {
|
||||
this.hookCheckers.push(checker);
|
||||
this.hookCheckers.sort((a, b) => (b.priority ?? 0) - (a.priority ?? 0));
|
||||
}
|
||||
|
||||
/**
|
||||
* Get all current hook checkers.
|
||||
*/
|
||||
getHookCheckers(): readonly HookCheckerRule[] {
|
||||
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) {
|
||||
|
||||
@@ -12,6 +12,36 @@ export enum PolicyDecision {
|
||||
ASK_USER = 'ask_user',
|
||||
}
|
||||
|
||||
/**
|
||||
* Valid sources for hook execution
|
||||
*/
|
||||
export type HookSource = 'project' | 'user' | 'system' | 'extension';
|
||||
|
||||
/**
|
||||
* Array of valid hook source values for runtime validation
|
||||
*/
|
||||
const VALID_HOOK_SOURCES: HookSource[] = [
|
||||
'project',
|
||||
'user',
|
||||
'system',
|
||||
'extension',
|
||||
];
|
||||
|
||||
/**
|
||||
* Safely extract and validate hook source from input
|
||||
* Returns 'project' as default if the value is invalid or missing
|
||||
*/
|
||||
export function getHookSource(input: Record<string, unknown>): HookSource {
|
||||
const source = input['hook_source'];
|
||||
if (
|
||||
typeof source === 'string' &&
|
||||
VALID_HOOK_SOURCES.includes(source as HookSource)
|
||||
) {
|
||||
return source as HookSource;
|
||||
}
|
||||
return 'project';
|
||||
}
|
||||
|
||||
export enum ApprovalMode {
|
||||
DEFAULT = 'default',
|
||||
AUTO_EDIT = 'autoEdit',
|
||||
@@ -115,6 +145,42 @@ export interface SafetyCheckerRule {
|
||||
checker: SafetyCheckerConfig;
|
||||
}
|
||||
|
||||
export interface HookExecutionContext {
|
||||
eventName: string;
|
||||
hookSource?: HookSource;
|
||||
trustedFolder?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Rule for applying safety checkers to hook executions.
|
||||
* Similar to SafetyCheckerRule but with hook-specific matching criteria.
|
||||
*/
|
||||
export interface HookCheckerRule {
|
||||
/**
|
||||
* The name of the hook event this rule applies to.
|
||||
* If undefined, the rule applies to all hook events.
|
||||
*/
|
||||
eventName?: string;
|
||||
|
||||
/**
|
||||
* The source of hooks this rule applies to.
|
||||
* If undefined, the rule applies to all hook sources.
|
||||
*/
|
||||
hookSource?: HookSource;
|
||||
|
||||
/**
|
||||
* Priority of this checker. Higher numbers run first.
|
||||
* Default is 0.
|
||||
*/
|
||||
priority?: number;
|
||||
|
||||
/**
|
||||
* Specifies an external or built-in safety checker to execute for
|
||||
* additional validation of a hook execution.
|
||||
*/
|
||||
checker: SafetyCheckerConfig;
|
||||
}
|
||||
|
||||
export interface PolicyEngineConfig {
|
||||
/**
|
||||
* List of policy rules to apply.
|
||||
@@ -122,10 +188,15 @@ export interface PolicyEngineConfig {
|
||||
rules?: PolicyRule[];
|
||||
|
||||
/**
|
||||
* List of safety checkers to apply.
|
||||
* List of safety checkers to apply to tool calls.
|
||||
*/
|
||||
checkers?: SafetyCheckerRule[];
|
||||
|
||||
/**
|
||||
* List of safety checkers to apply to hook executions.
|
||||
*/
|
||||
hookCheckers?: HookCheckerRule[];
|
||||
|
||||
/**
|
||||
* Default decision when no rules match.
|
||||
* Defaults to ASK_USER.
|
||||
@@ -137,6 +208,13 @@ export interface PolicyEngineConfig {
|
||||
* When true, ASK_USER decisions become DENY.
|
||||
*/
|
||||
nonInteractive?: boolean;
|
||||
|
||||
/**
|
||||
* Whether to allow hooks to execute.
|
||||
* When false, all hooks are denied.
|
||||
* Defaults to true.
|
||||
*/
|
||||
allowHooks?: boolean;
|
||||
}
|
||||
|
||||
export interface PolicySettings {
|
||||
|
||||
Reference in New Issue
Block a user