feat(core): add agent tool authorization guard - #1390
Conversation
Add an Agent-level toolGuard hook that can deny local tool execution before the tool runs. Denied calls reuse the existing ToolDeniedError path so tool error/end hooks still receive audit context.\n\nAdds behavior and type coverage for the new guard API.\n\nRelated to VoltAgent#1177.
|
📝 WalkthroughWalkthroughThe agent now supports an optional typed ChangesTool guard authorization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ToolCaller
participant Agent
participant toolGuard
participant ToolHooks
participant Tool
ToolCaller->>Agent: Invoke tool
Agent->>toolGuard: Check tool arguments and context
toolGuard-->>Agent: Return allow or denial
alt Allowed
Agent->>ToolHooks: Run tool start hooks
Agent->>Tool: Execute tool
else Denied
Agent->>ToolHooks: Report ToolDeniedError
end
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/agent/agent.ts (1)
7294-7301: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftAuthorize provider tools before provider execution.
runInternalGenerateTextcompletes before Line 7294 evaluatestoolGuard. The provider tool call and result are already available at this point. A denied guard result cannot prevent provider-tool execution.This throw also bypasses
onToolErrorandonToolEndfor the target provider tool. The outer wrapper reports hooks forcallTool, not fortool.Authorize before dispatching the provider request. Route a target-provider denial through its error and end hooks. Add a test that confirms a denied provider tool does not execute.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/agent/agent.ts` around lines 7294 - 7301, Move the tool guard authorization in runInternalGenerateText to before the provider request is dispatched, so denied provider tools never execute. For a denied target provider tool, invoke that tool’s onToolError and onToolEnd hooks before propagating the denial, while preserving normal onToolStart and execution behavior for allowed tools. Add a test verifying the denied provider tool is not executed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/agent/agent.ts`:
- Around line 6673-6674: Remove the duplicate tool.hooks.onEnd invocation in
handleToolError for denied local calls, including the corresponding path near
the alternate call site, so each denied call triggers the end hook exactly once.
Add or update a test covering a denied local tool call and assert
tool.hooks.onEnd is invoked once.
In `@packages/core/src/agent/hooks/index.ts`:
- Around line 62-73: Update ToolGuardArgs, OnToolStartHookArgs, and
OnToolEndHookArgs so their tool property accepts the union of BaseTool and
ProviderTool, reflecting provider-defined tools at runtime. Update
AgentToolGuard and related hook usage to consume this type directly, then remove
all tool as any casts while preserving existing behavior.
---
Outside diff comments:
In `@packages/core/src/agent/agent.ts`:
- Around line 7294-7301: Move the tool guard authorization in
runInternalGenerateText to before the provider request is dispatched, so denied
provider tools never execute. For a denied target provider tool, invoke that
tool’s onToolError and onToolEnd hooks before propagating the denial, while
preserving normal onToolStart and execution behavior for allowed tools. Add a
test verifying the denied provider tool is not executed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 42826508-cdd6-4977-83db-8a094742deb6
📒 Files selected for processing (5)
packages/core/src/agent/agent.spec-d.tspackages/core/src/agent/agent.spec.tspackages/core/src/agent/agent.tspackages/core/src/agent/hooks/index.tspackages/core/src/agent/types.ts
| await this.assertToolGuardAllows(tool, args, oc, executionOptions); | ||
| await runToolStartHooks(); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Call the tool end hook once for denied local calls.
When toolGuard denies a local tool, execution enters handleToolError. That handler invokes tool.hooks.onEnd twice at Lines 6617-6631. A denied call can therefore create duplicate tool-level audit records or duplicate cleanup side effects.
Remove the duplicate invocation. Add a test that asserts tool.hooks.onEnd runs exactly once for a denied call.
Also applies to: 6734-6736
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/agent/agent.ts` around lines 6673 - 6674, Remove the
duplicate tool.hooks.onEnd invocation in handleToolError for denied local calls,
including the corresponding path near the alternate call site, so each denied
call triggers the end hook exactly once. Add or update a test covering a denied
local tool call and assert tool.hooks.onEnd is invoked once.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/src/agent/hooks/index.ts">
<violation number="1" location="packages/core/src/agent/hooks/index.ts:67">
P3: A type-valid guard result can state both authorization outcomes, but `assertToolGuardAllows` resolves that conflict as denial. Model object results as mutually exclusive variants so guard implementations cannot accidentally publish contradictory authorization decisions.</violation>
</file>
<file name="packages/core/src/agent/agent.ts">
<violation number="1" location="packages/core/src/agent/agent.ts:6673">
P2: Denying a tool call via toolGuard routes execution through handleToolError, which appears to invoke tool.hooks.onEnd twice. This can produce duplicate tool-level audit records or duplicate cleanup side effects for every denied call. Consider deduplicating the onEnd invocation in handleToolError and adding a test asserting onEnd fires exactly once for a denied call.</violation>
<violation number="2" location="packages/core/src/agent/agent.ts:7294">
P1: For provider tools this guard runs after the tool has already been executed, so it does not actually authorize the call.
In `executeProviderToolViaCallTool`, `runInternalGenerateText` (~line 7284) is invoked before this guard. It calls `generateText({ tools: { [tool.name]: tool }, toolChoice: { type: "tool", toolName: tool.name } })`, which makes the AI SDK invoke the provider tool's own `execute` and record its result. By the time `assertToolGuardAllows` is reached here, the provider tool has already run with all its side effects. A denial only prevents the result from being surfaced to the caller — it cannot stop the tool from executing. This is inconsistent with the local-tool paths where the guard runs before `tool.execute`, and it gives a false sense of authorization for provider tools routed via `callTool`.
Additionally, provider tools exposed directly to the model are passed through untouched in `ToolManager.prepareToolsForExecution` (`tools[tool.name] = tool;`), so they never go through `createToolExecutionFactory` and never hit `assertToolGuardAllows` at all on the direct-execution path. Providers routed only through `callTool` are the sole case that touches this guard, and that happens after execution. Consider moving the guard to before the provider tool's `callTool` execution begins (e.g., before `runInternalGenerateText`) and documenting/covering the direct pass-through case.</violation>
<violation number="3" location="packages/core/src/agent/agent.ts:7294">
P1: Denied routed provider calls skip that provider's `onToolError` and `onToolEnd` hooks, so audit logging records the enclosing `callTool` failure rather than the denied provider tool. Handle guard denials in this method with the target tool's error/end lifecycle before propagating or returning the denial.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| `Provider tool "${tool.name}" received arguments that do not match callTool input.`, | ||
| ); | ||
| } | ||
| await this.assertToolGuardAllows(tool, callInput, oc, executionOptions); |
There was a problem hiding this comment.
P1: For provider tools this guard runs after the tool has already been executed, so it does not actually authorize the call.
In executeProviderToolViaCallTool, runInternalGenerateText (~line 7284) is invoked before this guard. It calls generateText({ tools: { [tool.name]: tool }, toolChoice: { type: "tool", toolName: tool.name } }), which makes the AI SDK invoke the provider tool's own execute and record its result. By the time assertToolGuardAllows is reached here, the provider tool has already run with all its side effects. A denial only prevents the result from being surfaced to the caller — it cannot stop the tool from executing. This is inconsistent with the local-tool paths where the guard runs before tool.execute, and it gives a false sense of authorization for provider tools routed via callTool.
Additionally, provider tools exposed directly to the model are passed through untouched in ToolManager.prepareToolsForExecution (tools[tool.name] = tool;), so they never go through createToolExecutionFactory and never hit assertToolGuardAllows at all on the direct-execution path. Providers routed only through callTool are the sole case that touches this guard, and that happens after execution. Consider moving the guard to before the provider tool's callTool execution begins (e.g., before runInternalGenerateText) and documenting/covering the direct pass-through case.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/src/agent/agent.ts, line 7294:
<comment>For provider tools this guard runs after the tool has already been executed, so it does not actually authorize the call.
In `executeProviderToolViaCallTool`, `runInternalGenerateText` (~line 7284) is invoked before this guard. It calls `generateText({ tools: { [tool.name]: tool }, toolChoice: { type: "tool", toolName: tool.name } })`, which makes the AI SDK invoke the provider tool's own `execute` and record its result. By the time `assertToolGuardAllows` is reached here, the provider tool has already run with all its side effects. A denial only prevents the result from being surfaced to the caller — it cannot stop the tool from executing. This is inconsistent with the local-tool paths where the guard runs before `tool.execute`, and it gives a false sense of authorization for provider tools routed via `callTool`.
Additionally, provider tools exposed directly to the model are passed through untouched in `ToolManager.prepareToolsForExecution` (`tools[tool.name] = tool;`), so they never go through `createToolExecutionFactory` and never hit `assertToolGuardAllows` at all on the direct-execution path. Providers routed only through `callTool` are the sole case that touches this guard, and that happens after execution. Consider moving the guard to before the provider tool's `callTool` execution begins (e.g., before `runInternalGenerateText`) and documenting/covering the direct pass-through case.</comment>
<file context>
@@ -7242,6 +7291,7 @@ export class Agent {
`Provider tool "${tool.name}" received arguments that do not match callTool input.`,
);
}
+ await this.assertToolGuardAllows(tool, callInput, oc, executionOptions);
await hooks.onToolStart?.({
agent: this,
</file context>
|
Thanks for the review — I pushed Changes made:
Validation:
|
2 participants
Summary
toolGuardhook for per-tool authorization before local tool execution.false,{ allowed: false }, or{ denied: true, reason }.ToolDeniedErrorpath so denied tool calls still reachonToolError/onToolEndhooks for audit logging.Test Plan
vitest run packages/core/src/agent/agent.spec.ts packages/core/src/agent/hooks/index.spec.ts --config vitest.config.mts -t "Tool Execution|toolGuard|Hook Type Tests"pnpm --filter @voltagent/core typecheckpnpm --filter @voltagent/core buildbiome check packages/core/src/agent/agent.ts packages/core/src/agent/types.ts packages/core/src/agent/hooks/index.ts packages/core/src/agent/agent.spec.ts packages/core/src/agent/agent.spec-d.tsRelated to #1177
Summary by cubic
Adds an agent-level
toolGuardto authorize or deny tool calls before execution, with denials routed throughToolDeniedErrorsoonToolError/onToolEndinclude audit context. Addresses #1177.New Features
AgentOptions.toolGuard: sync/async guard returningbooleanor{ allowed?: boolean; denied?: boolean; reason?: string }.onToolStartfor local and provider tools; denied calls block withTOOL_FORBIDDEN(403).Bug Fixes
Written for commit bb52a3f. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
TOOL_FORBIDDENerror with relevant context while preserving lifecycle and error callbacks.