#11778: fix(plugins): enforce monotonic hook deny merges
stale
Cluster:
Plugin Hook Enhancements
## 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
#17930: fix: evaluate tool_result_persist hooks lazily to avoid race condition
by TheArkifaneVashtorr · 2026-02-16
79.2%
#6405: feat(security): Add HTTP API security hooks for plugin scanning
by masterfung · 2026-02-01
78.5%
#20541: fix(hooks): clear internal hooks before plugins register
by ramarnat · 2026-02-19
76.1%
#18004: feat: Add before_message_dispatch hook for blocking inbound messages
by Andy-Haigh · 2026-02-16
75.9%
#14746: fix(hooks): use globalThis for handler registry to survive bundler ...
by openperf · 2026-02-12
75.6%
#11432: fix(security): add --ignore-scripts to npm install in hook and plug...
by coygeek · 2026-02-07
75.5%
#13471: fix: security audit distinguishes internal hooks from external webh...
by jarvisz8 · 2026-02-10
75.4%
#11817: fix(build): compile bundled hook handlers into dist
by AnonO6 · 2026-02-08
75.1%
#11153: refactor(hooks): replace console.warn/error with subsystem logger
by hclsys · 2026-02-07
75.0%
#10539: feat(hooks): add blocking capability to message_received hookFeat/m...
by khalidovicGPT · 2026-02-06
75.0%