From 901d913497f47d92db3b997aa2dc757431db9ef7 Mon Sep 17 00:00:00 2001 From: Himanshu Soni Date: Thu, 26 Feb 2026 17:58:24 +0530 Subject: [PATCH 1/2] feat(core): implement native support for blocking hook execution via policy files --- .../core/src/hooks/hookEventHandler.test.ts | 4 + packages/core/src/hooks/hookEventHandler.ts | 96 ++++++++++++++- packages/core/src/hooks/hookSystem.test.ts | 43 ++++++- packages/core/src/hooks/runtimeHooks.test.ts | 17 ++- packages/core/src/policy/config.test.ts | 1 + packages/core/src/policy/config.ts | 4 + .../core/src/policy/policy-engine.test.ts | 110 +++++++++++++++++- packages/core/src/policy/policy-engine.ts | 48 ++++++++ packages/core/src/policy/toml-loader.test.ts | 45 ++++++- packages/core/src/policy/toml-loader.ts | 98 +++++++++++++++- packages/core/src/policy/types.ts | 45 +++++++ 11 files changed, 496 insertions(+), 15 deletions(-) diff --git a/packages/core/src/hooks/hookEventHandler.test.ts b/packages/core/src/hooks/hookEventHandler.test.ts index 9a07d396725..3c823a5e7be 100644 --- a/packages/core/src/hooks/hookEventHandler.test.ts +++ b/packages/core/src/hooks/hookEventHandler.test.ts @@ -22,6 +22,7 @@ import type { HookPlanner } from './hookPlanner.js'; import type { HookRunner } from './hookRunner.js'; import type { HookAggregator } from './hookAggregator.js'; + // Mock debugLogger const mockDebugLogger = vi.hoisted(() => ({ log: vi.fn(), @@ -73,6 +74,9 @@ describe('HookEventHandler', () => { .mockReturnValue('/test/project/.gemini/tmp/chats/session.json'), }), }), + getPolicyEngine: vi.fn().mockReturnValue({ + checkHook: vi.fn().mockResolvedValue({ decision: 'allow' }), + }), } as unknown as Config; mockHookPlanner = { diff --git a/packages/core/src/hooks/hookEventHandler.ts b/packages/core/src/hooks/hookEventHandler.ts index 00909094cee..56ee2ce7963 100644 --- a/packages/core/src/hooks/hookEventHandler.ts +++ b/packages/core/src/hooks/hookEventHandler.ts @@ -8,7 +8,7 @@ import type { Config } from '../config/config.js'; import type { HookPlanner, HookEventContext } from './hookPlanner.js'; import type { HookRunner } from './hookRunner.js'; import type { HookAggregator, AggregatedHookResult } from './hookAggregator.js'; -import { HookEventName, HookType } from './types.js'; +import { HookEventName, HookType, createHookOutput } from './types.js'; import type { HookConfig, HookInput, @@ -29,8 +29,10 @@ import type { PreCompressTrigger, HookExecutionResult, McpToolContext, + HookOutput, } from './types.js'; import { defaultHookTranslator } from './hookTranslator.js'; +import { PolicyDecision } from '../policy/types.js'; import type { GenerateContentParameters, GenerateContentResponse, @@ -298,12 +300,93 @@ export class HookEventHandler { }; } + // Filter hooks by policy + const policyEngine = this.config.getPolicyEngine(); + const allowedConfigs: HookConfig[] = []; + const policyErrors: Error[] = []; + const policyOutputs: HookOutput[] = []; + const policyResults: HookExecutionResult[] = []; + + for (let i = 0; i < plan.hookConfigs.length; i++) { + const config = plan.hookConfigs[i]; + const { decision, rule } = await policyEngine.checkHook( + config, + eventName, + ); + if (decision === PolicyDecision.DENY) { + const reason = + rule?.denyMessage || + `Blocked by policy rule: ${rule?.source || 'unknown'}`; + debugLogger.warn( + `[HookEventHandler] Hook execution denied by policy: ${config.command}`, + ); + const error = new Error(reason); + policyErrors.push(error); + + const output = createHookOutput(eventName, { + decision: 'deny', + reason, + }); + policyOutputs.push(output); + + policyResults.push({ + hookConfig: config, + eventName, + success: false, + error, + output, + duration: 0, + }); + + coreEvents.emitHookStart({ + hookName: this.getHookName(config), + eventName, + hookIndex: i + 1, + totalHooks: plan.hookConfigs.length, + }); + coreEvents.emitHookEnd({ + hookName: this.getHookName(config), + eventName, + success: false, + }); + } else { + allowedConfigs.push(config); + } + } + + plan.hookConfigs = allowedConfigs; + + if (plan.hookConfigs.length === 0) { + if (policyResults.length > 0) { + const aggregated = this.hookAggregator.aggregateResults( + policyResults, + eventName, + ); + this.processCommonHookOutputFields(aggregated); + this.logHookExecution( + eventName, + input, + policyResults, + aggregated, + requestContext, + ); + return aggregated; + } + + return { + success: true, + allOutputs: [], + errors: [], + totalDuration: 0, + }; + } + const onHookStart = (config: HookConfig, index: number) => { coreEvents.emitHookStart({ hookName: this.getHookName(config), eventName, - hookIndex: index + 1, - totalHooks: plan.hookConfigs.length, + hookIndex: index + 1 + policyResults.length, // Offset by denied hooks to preserve count + totalHooks: plan.hookConfigs.length + policyResults.length, }); }; @@ -332,9 +415,12 @@ export class HookEventHandler { onHookEnd, ); + // Combine policy results (denied hooks) with execution results + const combinedResults = [...policyResults, ...results]; + // Aggregate results const aggregated = this.hookAggregator.aggregateResults( - results, + combinedResults, eventName, ); @@ -345,7 +431,7 @@ export class HookEventHandler { this.logHookExecution( eventName, input, - results, + combinedResults, aggregated, requestContext, ); diff --git a/packages/core/src/hooks/hookSystem.test.ts b/packages/core/src/hooks/hookSystem.test.ts index 85f1a7407b6..13d9d0e2675 100644 --- a/packages/core/src/hooks/hookSystem.test.ts +++ b/packages/core/src/hooks/hookSystem.test.ts @@ -88,8 +88,21 @@ describe('HookSystem Integration', () => { }); // Provide getMessageBus mock for MessageBus integration tests - (config as unknown as { getMessageBus: () => unknown }).getMessageBus = - () => undefined; + // Provide getMessageBus mock for MessageBus integration tests + ( + config as unknown as { + getMessageBus: () => unknown; + getPolicyEngine: () => unknown; + } + ).getMessageBus = () => undefined; + ( + config as unknown as { + getMessageBus: () => unknown; + getPolicyEngine: () => unknown; + } + ).getPolicyEngine = () => ({ + checkHook: vi.fn().mockResolvedValue({ decision: 'allow' }), + }); hookSystem = new HookSystem(config); @@ -297,8 +310,19 @@ describe('HookSystem Integration', () => { }); ( - configWithDisabled as unknown as { getMessageBus: () => unknown } + configWithDisabled as unknown as { + getMessageBus: () => unknown; + getPolicyEngine: () => unknown; + } ).getMessageBus = () => undefined; + ( + configWithDisabled as unknown as { + getMessageBus: () => unknown; + getPolicyEngine: () => unknown; + } + ).getPolicyEngine = () => ({ + checkHook: vi.fn().mockResolvedValue({ decision: 'allow' }), + }); const systemWithDisabled = new HookSystem(configWithDisabled); await systemWithDisabled.initialize(); @@ -362,8 +386,19 @@ describe('HookSystem Integration', () => { }); ( - configForDisabling as unknown as { getMessageBus: () => unknown } + configForDisabling as unknown as { + getMessageBus: () => unknown; + getPolicyEngine: () => unknown; + } ).getMessageBus = () => undefined; + ( + configForDisabling as unknown as { + getMessageBus: () => unknown; + getPolicyEngine: () => unknown; + } + ).getPolicyEngine = () => ({ + checkHook: vi.fn().mockResolvedValue({ decision: 'allow' }), + }); const systemForDisabling = new HookSystem(configForDisabling); await systemForDisabling.initialize(); diff --git a/packages/core/src/hooks/runtimeHooks.test.ts b/packages/core/src/hooks/runtimeHooks.test.ts index 970eaca3973..9b1bb15deff 100644 --- a/packages/core/src/hooks/runtimeHooks.test.ts +++ b/packages/core/src/hooks/runtimeHooks.test.ts @@ -38,8 +38,21 @@ describe('Runtime Hooks', () => { }); // Stub getMessageBus - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (config as any).getMessageBus = () => undefined; + + ( + config as unknown as { + getMessageBus: () => unknown; + getPolicyEngine: () => unknown; + } + ).getMessageBus = () => undefined; + ( + config as unknown as { + getMessageBus: () => unknown; + getPolicyEngine: () => unknown; + } + ).getPolicyEngine = () => ({ + checkHook: vi.fn().mockResolvedValue({ decision: 'allow' }), + }); hookSystem = new HookSystem(config); }); diff --git a/packages/core/src/policy/config.test.ts b/packages/core/src/policy/config.test.ts index a9fae7a1fa0..9880ad5721f 100644 --- a/packages/core/src/policy/config.test.ts +++ b/packages/core/src/policy/config.test.ts @@ -58,6 +58,7 @@ describe('createPolicyEngineConfig', () => { const loadPoliciesSpy = vi.spyOn(tomlLoader, 'loadPoliciesFromToml'); loadPoliciesSpy.mockResolvedValue({ rules: [], + hookRules: [], checkers: [], errors: [], }); diff --git a/packages/core/src/policy/config.ts b/packages/core/src/policy/config.ts index 7de415cb37e..48bb287152f 100644 --- a/packages/core/src/policy/config.ts +++ b/packages/core/src/policy/config.ts @@ -13,6 +13,7 @@ import { type PolicyEngineConfig, PolicyDecision, type PolicyRule, + type HookRule, type ApprovalMode, type PolicySettings, } from './types.js'; @@ -197,6 +198,7 @@ export async function createPolicyEngineConfig( // Load policies from TOML files const { rules: tomlRules, + hookRules: tomlHookRules, checkers: tomlCheckers, errors, } = await loadPoliciesFromToml(securePolicyDirs, (p) => { @@ -231,6 +233,7 @@ export async function createPolicyEngineConfig( } const rules: PolicyRule[] = [...tomlRules]; + const hookRules: HookRule[] = [...tomlHookRules]; const checkers = [...tomlCheckers]; // Priority system for policy rules: @@ -376,6 +379,7 @@ export async function createPolicyEngineConfig( return { rules, + hookRules, checkers, defaultDecision: PolicyDecision.ASK_USER, approvalMode, diff --git a/packages/core/src/policy/policy-engine.test.ts b/packages/core/src/policy/policy-engine.test.ts index 0d110f8b2dc..d2875eb276f 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -14,6 +14,7 @@ import { InProcessCheckerType, ApprovalMode, PRIORITY_SUBAGENT_TOOL, + type HookRule, } from './types.js'; import type { FunctionCall } from '@google/genai'; import { SafetyCheckDecision } from '../safety/protocol.js'; @@ -2849,7 +2850,114 @@ describe('PolicyEngine', () => { const checkers = engine.getCheckers(); expect(checkers).toHaveLength(1); - expect(checkers[0].priority).toBe(2.5); + }); + }); + + describe('checkHook', () => { + it('should match hook by exact eventName and hookName', async () => { + const hookRules: HookRule[] = [ + { + eventName: 'BeforeTool', + hookName: 'test-hook', + decision: PolicyDecision.DENY, + }, + ]; + engine = new PolicyEngine({ hookRules }); + + const result = await engine.checkHook( + { name: 'test-hook' }, + 'BeforeTool', + ); + expect(result.decision).toBe(PolicyDecision.DENY); + }); + + it('should fallback to default decision if no rule matches', async () => { + const hookRules: HookRule[] = [ + { + eventName: 'BeforeTool', + hookName: 'test-hook', + decision: PolicyDecision.ALLOW, + }, + ]; + // Default decision is ASK_USER, which checkHook converts to DENY for headless execution + engine = new PolicyEngine({ hookRules }); + + const result = await engine.checkHook( + { name: 'other-hook' }, + 'BeforeTool', + ); + expect(result.decision).toBe(PolicyDecision.DENY); + }); + + it('should respect exact match over partial rules if priority is higher', async () => { + const hookRules: HookRule[] = [ + { + eventName: 'BeforeTool', + decision: PolicyDecision.DENY, + priority: 10, + }, + { + eventName: 'BeforeTool', + hookName: 'safe-hook', + decision: PolicyDecision.ALLOW, + priority: 20, + }, + ]; + engine = new PolicyEngine({ hookRules }); + + const safeResult = await engine.checkHook( + { name: 'safe-hook' }, + 'BeforeTool', + ); + expect(safeResult.decision).toBe(PolicyDecision.ALLOW); + + const otherResult = await engine.checkHook( + { name: 'unsafe-hook' }, + 'BeforeTool', + ); + expect(otherResult.decision).toBe(PolicyDecision.DENY); + }); + + it('should match commandPattern when hook has a command', async () => { + const hookRules: HookRule[] = [ + { + eventName: 'BeforeTool', + commandPattern: /npm run build/, + decision: PolicyDecision.DENY, + priority: 10, + }, + ]; + engine = new PolicyEngine({ hookRules }); + + const matchResult = await engine.checkHook( + { command: 'npm run build' }, + 'BeforeTool', + ); + expect(matchResult.decision).toBe(PolicyDecision.DENY); + + const noMatchResult = await engine.checkHook( + { command: 'ls' }, + 'BeforeTool', + ); + // No match -> Default Decision -> ASK_USER -> Headless -> DENY + expect(noMatchResult.decision).toBe(PolicyDecision.DENY); + }); + + it('should convert ASK_USER decision from a rule to DENY', async () => { + const hookRules: HookRule[] = [ + { + eventName: 'BeforeTool', + hookName: 'test-hook', + decision: PolicyDecision.ASK_USER, + }, + ]; + engine = new PolicyEngine({ hookRules }); + + const result = await engine.checkHook( + { name: 'test-hook' }, + 'BeforeTool', + ); + expect(result.decision).toBe(PolicyDecision.DENY); }); }); diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index 8f61d622c21..4944001c842 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -11,6 +11,7 @@ import { type PolicyRule, type SafetyCheckerRule, type HookCheckerRule, + type HookRule, ApprovalMode, type CheckResult, } from './types.js'; @@ -162,6 +163,7 @@ export class PolicyEngine { private rules: PolicyRule[]; private checkers: SafetyCheckerRule[]; private hookCheckers: HookCheckerRule[]; + private hookRules: HookRule[]; private readonly defaultDecision: PolicyDecision; private readonly nonInteractive: boolean; private readonly checkerRunner?: CheckerRunner; @@ -177,6 +179,9 @@ export class PolicyEngine { this.hookCheckers = (config.hookCheckers ?? []).sort( (a, b) => (b.priority ?? 0) - (a.priority ?? 0), ); + this.hookRules = (config.hookRules ?? []).sort( + (a, b) => (b.priority ?? 0) - (a.priority ?? 0), + ); this.defaultDecision = config.defaultDecision ?? PolicyDecision.ASK_USER; this.nonInteractive = config.nonInteractive ?? false; this.checkerRunner = checkerRunner; @@ -789,4 +794,47 @@ export class PolicyEngine { } return decision; } + + /** + * Check if a hook execution is allowed based on the configured hook policies. + */ + async checkHook( + hookConfig: { name?: string; command?: string }, + eventName: string, + ): Promise<{ decision: PolicyDecision; rule?: HookRule }> { + for (const rule of this.hookRules) { + if (rule.eventName && rule.eventName !== eventName) { + continue; + } + if (rule.hookName && rule.hookName !== hookConfig.name) { + continue; + } + if (rule.commandPattern && hookConfig.command) { + if (!rule.commandPattern.test(hookConfig.command)) { + continue; + } + } + + // If it matches all defined criteria + debugLogger.debug( + `[PolicyEngine.checkHook] MATCHED hook rule: eventName=${rule.eventName || 'any'}, hookName=${rule.hookName || 'any'}, commandPattern=${rule.commandPattern?.source || 'none'}, decision=${rule.decision}`, + ); + + let decision = rule.decision; + // Hook executions are completely headless and non-interactive. + if (decision === PolicyDecision.ASK_USER) { + decision = PolicyDecision.DENY; + } + + return { decision, rule }; + } + + // Default decision if no rule matches. + let decision = this.defaultDecision; + if (decision === PolicyDecision.ASK_USER || this.nonInteractive) { + decision = PolicyDecision.DENY; + } + + return { decision }; + } } diff --git a/packages/core/src/policy/toml-loader.test.ts b/packages/core/src/policy/toml-loader.test.ts index 1e4c008c5d1..224c44d2216 100644 --- a/packages/core/src/policy/toml-loader.test.ts +++ b/packages/core/src/policy/toml-loader.test.ts @@ -421,9 +421,52 @@ name = "allowed-path" `); expect(result.checkers).toHaveLength(1); - expect(result.checkers[0].priority).toBe(1.1); // tier 1 + 100/1000 expect(result.checkers[0].source).toBe('Default: test.toml'); }); + + it('should parse [[hook_rule]] configurations correctly', async () => { + const result = await runLoadPoliciesFromToml(` +[[hook_rule]] +hookName = "git-status-hook" +eventName = "BeforeTool" +commandPrefix = "git status" +decision = "allow" +priority = 50 +deny_message = "Git status is not allowed" +`); + + expect(result.hookRules).toHaveLength(1); + expect(result.hookRules[0].hookName).toBe('git-status-hook'); + expect(result.hookRules[0].eventName).toBe('BeforeTool'); + expect(result.hookRules[0].decision).toBe(PolicyDecision.ALLOW); + expect(result.hookRules[0].priority).toBe(1.05); // tier 1 + 50/1000 + expect(result.hookRules[0].denyMessage).toBe('Git status is not allowed'); + + // commandPrefix should be converted to regex + expect(result.hookRules[0].commandPattern?.test('git status')).toBe(true); + expect(result.hookRules[0].commandPattern?.test('git log')).toBe(false); + }); + + it('should parse hook rules with commandRegex', async () => { + const result = await runLoadPoliciesFromToml(` +[[hook_rule]] +eventName = "BeforeAgent" +commandRegex = "npm run .*" +decision = "deny" +priority = 100 +`); + + expect(result.hookRules).toHaveLength(1); + expect(result.hookRules[0].eventName).toBe('BeforeAgent'); + expect(result.hookRules[0].decision).toBe(PolicyDecision.DENY); + + expect(result.hookRules[0].commandPattern?.test('npm run build')).toBe( + true, + ); + expect(result.hookRules[0].commandPattern?.test('npm install')).toBe( + false, + ); + }); }); describe('Negative Tests', () => { diff --git a/packages/core/src/policy/toml-loader.ts b/packages/core/src/policy/toml-loader.ts index 6b164d59b8c..52de70dd2c2 100644 --- a/packages/core/src/policy/toml-loader.ts +++ b/packages/core/src/policy/toml-loader.ts @@ -11,8 +11,9 @@ import { type SafetyCheckerConfig, type SafetyCheckerRule, InProcessCheckerType, + type HookRule, } from './types.js'; -import { buildArgsPatterns, isSafeRegExp } from './utils.js'; +import { buildArgsPatterns, isSafeRegExp, escapeRegex } from './utils.js'; import fs from 'node:fs/promises'; import path from 'node:path'; import toml from '@iarna/toml'; @@ -51,6 +52,30 @@ const PolicyRuleSchema = z.object({ deny_message: z.string().optional(), }); +/** + * Schema for a single hook rule in the TOML file. + */ +const HookRuleSchema = z.object({ + hookName: z.string().optional(), + eventName: z.string().optional(), + commandPrefix: z.union([z.string(), z.array(z.string())]).optional(), + commandRegex: z.string().optional(), + argsPattern: z.string().optional(), + decision: z.nativeEnum(PolicyDecision), + priority: z + .number({ + required_error: 'priority is required', + invalid_type_error: 'priority must be a number', + }) + .int({ message: 'priority must be an integer' }) + .min(0, { message: 'priority must be >= 0' }) + .max(999, { + message: + 'priority must be <= 999 to prevent tier overflow. Priorities >= 1000 would jump to the next tier.', + }), + deny_message: z.string().optional(), +}); + /** * Schema for a single safety checker rule in the TOML file. */ @@ -84,6 +109,7 @@ const SafetyCheckerRuleSchema = z.object({ */ const PolicyFileSchema = z.object({ rule: z.array(PolicyRuleSchema).optional(), + hook_rule: z.array(HookRuleSchema).optional(), safety_checker: z.array(SafetyCheckerRuleSchema).optional(), }); @@ -121,6 +147,7 @@ export interface PolicyFileError { */ export interface PolicyLoadResult { rules: PolicyRule[]; + hookRules: HookRule[]; checkers: SafetyCheckerRule[]; errors: PolicyFileError[]; } @@ -268,6 +295,7 @@ export async function loadPoliciesFromToml( getPolicyTier: (path: string) => number, ): Promise { const rules: PolicyRule[] = []; + const hookRules: HookRule[] = []; const checkers: SafetyCheckerRule[] = []; const errors: PolicyFileError[] = []; @@ -436,6 +464,72 @@ export async function loadPoliciesFromToml( rules.push(...parsedRules); + // Transform hook rules + const parsedHookRules: HookRule[] = ( + validationResult.data.hook_rule ?? [] + ) + .map((rule) => { + const hookRule: HookRule = { + hookName: rule.hookName, + eventName: rule.eventName, + decision: rule.decision, + priority: transformPriority(rule.priority, tier), + source: `${tierName.charAt(0).toUpperCase() + tierName.slice(1)}: ${file}`, + denyMessage: rule.deny_message, + }; + + let commandPatternStr: string | undefined; + + if (rule.commandPrefix) { + // Convert prefix strings into an exact starting prefix matcher + const prefixes = Array.isArray(rule.commandPrefix) + ? rule.commandPrefix + : [rule.commandPrefix]; + const escapedPrefixes = prefixes.map((p) => escapeRegex(p)); + commandPatternStr = `^(${escapedPrefixes.join('|')})(\\s|$)`; + } else if (rule.commandRegex) { + commandPatternStr = rule.commandRegex; + } else if (rule.argsPattern) { + commandPatternStr = rule.argsPattern; + } + + if (commandPatternStr) { + try { + hookRule.commandPattern = new RegExp(commandPatternStr); + } catch (e) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + const error = e as Error; + errors.push({ + filePath, + fileName: file, + tier: tierName, + errorType: 'regex_compilation', + message: 'Invalid regex pattern in hook rule', + details: `Pattern: ${commandPatternStr}\nError: ${error.message}`, + }); + return null; + } + + if (!isSafeRegExp(commandPatternStr)) { + errors.push({ + filePath, + fileName: file, + tier: tierName, + errorType: 'regex_compilation', + message: + 'Unsafe regex pattern in hook rule (potential ReDoS)', + details: `Pattern: ${commandPatternStr}`, + }); + return null; + } + } + + return hookRule; + }) + .filter((rule): rule is HookRule => rule !== null); + + hookRules.push(...parsedHookRules); + // Transform checkers const parsedCheckers: SafetyCheckerRule[] = ( validationResult.data.safety_checker ?? [] @@ -530,5 +624,5 @@ export async function loadPoliciesFromToml( } } - return { rules, checkers, errors }; + return { rules, hookRules, checkers, errors }; } diff --git a/packages/core/src/policy/types.ts b/packages/core/src/policy/types.ts index 18c621c176f..01ce5b72448 100644 --- a/packages/core/src/policy/types.ts +++ b/packages/core/src/policy/types.ts @@ -98,6 +98,46 @@ export type SafetyCheckerConfig = | ExternalCheckerConfig | InProcessCheckerConfig; +export interface HookRule { + /** + * The name of the hook this rule applies to. + * If undefined, the rule applies to all hooks. + */ + hookName?: string; + + /** + * The name of the event this rule applies to. + * If undefined, the rule applies to all events. + */ + eventName?: string; + + /** + * Pattern to match against the hook command. + */ + commandPattern?: RegExp; + + /** + * The decision to make when this rule matches. + */ + decision: PolicyDecision; + + /** + * Priority of this rule. Higher numbers take precedence. + * Default is 0. + */ + priority?: number; + + /** + * Source of this rule. + */ + source?: string; + + /** + * Optional message to display when this rule results in a DENY decision. + */ + denyMessage?: string; +} + export interface PolicyRule { /** * A unique name for the policy rule, useful for identification and debugging. @@ -255,6 +295,11 @@ export interface PolicyEngineConfig { */ hookCheckers?: HookCheckerRule[]; + /** + * List of hook rules to apply to hook executions. + */ + hookRules?: HookRule[]; + /** * Default decision when no rules match. * Defaults to ASK_USER. From 86a18da1cf4afbd58e36e7feafc4fc5154a506f3 Mon Sep 17 00:00:00 2001 From: Himanshu Soni Date: Thu, 26 Feb 2026 18:33:33 +0530 Subject: [PATCH 2/2] fix(core): correct checkHook logic and consolidate HookRule pattern fields --- packages/core/src/policy/policy-engine.ts | 7 +++++-- packages/core/src/policy/toml-loader.test.ts | 4 ++-- packages/core/src/policy/toml-loader.ts | 9 +++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index 4944001c842..8ef6ed87127 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -809,8 +809,11 @@ export class PolicyEngine { if (rule.hookName && rule.hookName !== hookConfig.name) { continue; } - if (rule.commandPattern && hookConfig.command) { - if (!rule.commandPattern.test(hookConfig.command)) { + if (rule.commandPattern) { + if ( + !hookConfig.command || + !rule.commandPattern.test(hookConfig.command) + ) { continue; } } diff --git a/packages/core/src/policy/toml-loader.test.ts b/packages/core/src/policy/toml-loader.test.ts index 224c44d2216..0379b56d374 100644 --- a/packages/core/src/policy/toml-loader.test.ts +++ b/packages/core/src/policy/toml-loader.test.ts @@ -447,11 +447,11 @@ deny_message = "Git status is not allowed" expect(result.hookRules[0].commandPattern?.test('git log')).toBe(false); }); - it('should parse hook rules with commandRegex', async () => { + it('should parse hook rules with commandPattern', async () => { const result = await runLoadPoliciesFromToml(` [[hook_rule]] eventName = "BeforeAgent" -commandRegex = "npm run .*" +commandPattern = "npm run .*" decision = "deny" priority = 100 `); diff --git a/packages/core/src/policy/toml-loader.ts b/packages/core/src/policy/toml-loader.ts index 52de70dd2c2..2378413c9c0 100644 --- a/packages/core/src/policy/toml-loader.ts +++ b/packages/core/src/policy/toml-loader.ts @@ -59,8 +59,7 @@ const HookRuleSchema = z.object({ hookName: z.string().optional(), eventName: z.string().optional(), commandPrefix: z.union([z.string(), z.array(z.string())]).optional(), - commandRegex: z.string().optional(), - argsPattern: z.string().optional(), + commandPattern: z.string().optional(), decision: z.nativeEnum(PolicyDecision), priority: z .number({ @@ -487,10 +486,8 @@ export async function loadPoliciesFromToml( : [rule.commandPrefix]; const escapedPrefixes = prefixes.map((p) => escapeRegex(p)); commandPatternStr = `^(${escapedPrefixes.join('|')})(\\s|$)`; - } else if (rule.commandRegex) { - commandPatternStr = rule.commandRegex; - } else if (rule.argsPattern) { - commandPatternStr = rule.argsPattern; + } else if (rule.commandPattern) { + commandPatternStr = rule.commandPattern; } if (commandPatternStr) {