← Back to PRs

#14222: core: add needsApproval to before_tool_call; move AgentShield to extension

by Eventedge open 2026-02-11 18:20 View on GitHub →
docs app: web-ui gateway cli docker agents size: XL
Maintainer feedback on PR #8727: AgentShield-specific interception cannot live in core. This change keeps core generic and moves AgentShield logic into an extension. ## What changed (core) - Extended `before_tool_call` hook result to support: - `needsApproval?: boolean` - `approvalReason?: string` - Updated core hook merge logic to preserve the new fields. - Updated tool wrapper (`wrapToolWithBeforeToolCallHook`) to: - short-circuit on `needsApproval` and return `jsonResult({ status: "approval-pending", ... })` - keep block behavior unchanged (throw; tool not executed) - Removed the AgentShield-specific wrapper + tests from core: - deleted `src/agents/pi-tools.agentshield.ts` - deleted `src/agents/pi-tools.agentshield.test.ts` - `pi-tools.ts` now only applies the generic before-tool-call wrapper. ## New extension Added `extensions/agentshield/`: - Registers a `before_tool_call` hook that calls an AgentShield trust server and maps decisions to: - allow (`undefined`) - block (`{ block: true, blockReason }`) - needs approval (`{ needsApproval: true, approvalReason }`) - Feature-gated by `AGENTSHIELD_APPROVALS_ENABLED=1` - Config: - `AGENTSHIELD_URL` - `AGENTSHIELD_MODE=all|selective` - `AGENTSHIELD_TOOL_FILTER=tool1,tool2,...` (when selective) - Docs: `extensions/agentshield/README.md` ## Tests - Core: `src/agents/pi-tools.before-tool-call.test.ts` covers allow/block/needsApproval precedence + default reasons. - Extension: `extensions/agentshield/index.test.ts` covers enable/mode/filter parsing + mapping helpers. ## Verification - `pnpm vitest run src/agents/pi-tools.before-tool-call.test.ts` - `pnpm vitest run extensions/agentshield/index.test.ts` Note: full unit suite currently has pre-existing failures unrelated to this change (e.g. config io / security fix tests), but the tests relevant to this PR pass and the diff is limited to the hook surface + new extension. <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> Extends the core `before_tool_call` hook with `needsApproval`/`approvalReason` fields and moves AgentShield-specific logic into `extensions/agentshield/`. Adds a full approval UX: gateway RPC handlers, encrypted retry store, fingerprint-based allowlist, CLI subcommands, and config-driven message forwarding — all feature-gated behind `AGENTSHIELD_APPROVALS_ENABLED=1`. - Core hook surface extended cleanly; `needsApproval` takes precedence over `block` when both are set - Extension registers a `before_tool_call` hook (priority 100) that calls an optional trust server and fails closed on errors - Gateway wires in three new RPC methods (`agentshield.approval.request/resolve/list`) behind the same env gate - Encrypted at-rest retry store uses AES-256-GCM for tool args; approval records store only SHA-256 fingerprints (no raw args) - **Issue**: `ensureDir` in `agentshield-retry-store.ts` creates directories with default permissions instead of `0o700` — inconsistent with the other stores that handle sensitive data - **Issue**: README documents env var as `AGENTSHIELD_TOOL_FILTER` but the code reads `AGENTSHIELD_TOOLS` - Tests cover core hook integration, extension env parsing, approval manager lifecycle, allowlist CRUD, retry store encryption, and gateway handler wiring <h3>Confidence Score: 3/5</h3> - Mostly safe to merge after fixing the two flagged issues: directory permissions in the retry store and the env var name mismatch in the README. - The core hook changes are clean and well-tested. The directory permissions bug in agentshield-retry-store.ts is a security inconsistency (stores encryption key and encrypted data in a world-readable directory). The env var name mismatch between docs and code will confuse users. Both are straightforward fixes. The rest of the PR — gateway wiring, approval manager, allowlist, CLI — is well-structured and properly feature-gated. - `src/infra/agentshield-retry-store.ts` (directory permissions), `extensions/agentshield/README.md` (env var name mismatch) <!-- greptile_other_comments_section --> <sub>(5/5) You can turn off certain types of comments like style [here](https://app.greptile.com/review/github)!</sub> **Context used:** - Context from `dashboard` - CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=fd949e91-5c3a-4ab5-90a1-cbe184fd6ce8)) <!-- /greptile_comment -->

Most Similar PRs