← Back to PRs

#11778: fix(plugins): enforce monotonic hook deny merges

by coygeek open 2026-02-08 08:58 View on GitHub →
stale
## Fix Summary Hook merge logic uses nullish-coalescing assignment that allows later (lower-priority) hooks to overwrite earlier security decisions. A `block: true` or `cancel: true` from a high-priority guard hook can be neutralized by a lower-priority hook returning `false`. ## Issue Linkage Fixes #11777 ## Security Snapshot - CVSS v3.1: 7.7 (High) - CVSS v4.0: 9.2 (Critical) ## Implementation Details ### Files Changed - `src/plugins/hooks.security-merge.test.ts` (+83/-0) - `src/plugins/hooks.ts` (+3/-3) ### Technical Analysis Hook merge logic uses nullish-coalescing assignment that allows later (lower-priority) hooks to overwrite earlier security decisions. A `block: true` or `cancel: true` from a high-priority guard hook can be neutralized by a lower-priority hook returning `false`. ## Validation Evidence - Command: `pnpm exec vitest run src/plugins/hooks.security-merge.test.ts src/agents/pi-tools.before-tool-call.test.ts` - Status: passed ## Risk and Compatibility non-breaking; no known regression impact ## AI-Assisted Disclosure GPT-5.3-Codex <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR fixes hook result merging for security-critical decisions so that `block` (before_tool_call) and `cancel` (message_sending) become monotonic: once any higher-priority hook returns `true`, a lower-priority hook returning `false` can no longer neutralize it. It also adds a focused regression test validating these semantics for both tool blocking and message cancellation. The change is localized to `createHookRunner`’s merge functions in `src/plugins/hooks.ts`, which is where hook handlers are executed sequentially by priority and their results are combined. <h3>Confidence Score: 4/5</h3> - This PR is close to safe to merge, but there is a logic regression in how `blockReason` is merged that should be fixed first. - The monotonic `block`/`cancel` behavior is a clear security improvement and the new tests cover the bypass scenario. However, the `blockReason` merge order was reversed in `before_tool_call`, which can produce incorrect/less-informative reasons for a final block decision and is likely unintended given the previous semantics. - src/plugins/hooks.ts <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs