diff --git a/docs/hooks/plugin-tool-injection.md b/docs/hooks/plugin-tool-injection.md index b95fab3e6..9e699867b 100644 --- a/docs/hooks/plugin-tool-injection.md +++ b/docs/hooks/plugin-tool-injection.md @@ -545,4 +545,43 @@ Through the hook system's `respond` action, external processes can: 2. **Provide tool implementation**: Return execution results directly, no need to register in ToolRegistry 3. **Coexist with built-in tools**: Does not affect normal operation of PicoClaw's original tools -This provides a flexible and elegant solution for plugin development. \ No newline at end of file +This provides a flexible and elegant solution for plugin development. + +--- + +## Security Boundaries + +### Bypassing Approval Checks + +**Important**: The `respond` action bypasses `ApproveTool` approval checks. + +This means: +- A `before_tool` hook can return `respond` for **any tool name**, including sensitive tools (like `bash`) +- The tool won't go through the approval process, directly returning the hook-provided result +- This is designed for plugin tools but introduces security risks + +### Security Recommendations + +1. **Review hook configuration**: Ensure only trusted hook processes are enabled +2. **Limit hook scope**: Add your own security checks in hook implementation +3. **Use `deny_tool` for rejection**: Use `deny_tool` action instead of `respond` with error for denying execution + +### Example: Hook-Internal Security Check + +```python +def handle_before_tool(params: dict) -> dict: + tool = params.get("tool", "") + args = params.get("arguments", {}) + + # Security check: only handle plugin tools + if tool in ["get_weather", "calculate"]: + return { + "action": "respond", + "result": execute_plugin_tool(tool, args), + } + + # Other tools continue normal flow (will go through approval) + return {"action": "continue"} +``` + +This ensures the hook only affects plugin tools, not system tool approval flow. \ No newline at end of file diff --git a/docs/hooks/plugin-tool-injection.zh.md b/docs/hooks/plugin-tool-injection.zh.md index 8eebda7e3..ccc7ff7f6 100644 --- a/docs/hooks/plugin-tool-injection.zh.md +++ b/docs/hooks/plugin-tool-injection.zh.md @@ -545,4 +545,43 @@ func getWeatherData(city string) string { 2. **提供工具实现**:直接返回执行结果,无需注册到 ToolRegistry 3. **与内置工具共存**:不影响 PicoClaw 原有工具的正常运行 -这为插件开发提供了灵活、优雅的解决方案。 \ No newline at end of file +这为插件开发提供了灵活、优雅的解决方案。 + +--- + +## 安全边界说明 + +### 绕过审批检查 + +**重要**:`respond` action 会绕过 `ApproveTool` 审批检查。 + +这意味着: +- `before_tool` hook 可以为**任何工具名称**返回 `respond`,包括敏感工具(如 `bash`) +- 工具不会经过审批流程,直接返回 hook 提供的结果 +- 这是为了支持插件工具而设计,但也带来了安全风险 + +### 安全建议 + +1. **审查 hook 配置**:确保只有可信的 hook 进程被启用 +2. **限制 hook 权限**:在 hook 实现中添加自己的安全检查 +3. **优先使用 `deny_tool`**:对于拒绝执行,使用 `deny_tool` action 而非 `respond` 返回错误 + +### 示例:hook 内置安全检查 + +```python +def handle_before_tool(params: dict) -> dict: + tool = params.get("tool", "") + args = params.get("arguments", {}) + + # 安全检查:只处理插件工具 + if tool in ["get_weather", "calculate"]: + return { + "action": "respond", + "result": execute_plugin_tool(tool, args), + } + + # 其他工具继续正常流程(会经过审批) + return {"action": "continue"} +``` + +这样可以确保 hook 只影响插件工具,不影响系统工具的审批流程。 \ No newline at end of file diff --git a/pkg/agent/hooks.go b/pkg/agent/hooks.go index c0dfa8399..526a5356b 100644 --- a/pkg/agent/hooks.go +++ b/pkg/agent/hooks.go @@ -25,7 +25,7 @@ type HookAction string const ( HookActionContinue HookAction = "continue" HookActionModify HookAction = "modify" - HookActionRespond HookAction = "respond" // Return result directly, skip tool execution + HookActionRespond HookAction = "respond" // Return result directly, skip tool execution. SECURITY: This bypasses ApproveTool checks, allowing hooks to return results for any tool (including sensitive ones like bash) without approval. Use with caution. HookActionDenyTool HookAction = "deny_tool" HookActionAbortTurn HookAction = "abort_turn" HookActionHardAbort HookAction = "hard_abort" diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index f11bc5399..d4c534c64 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -2349,7 +2349,12 @@ turnLoop: toolArgs = toolReq.Arguments } case HookActionRespond: - // Hook returns result directly, skip tool execution + // Hook returns result directly, skip tool execution. + // SECURITY: This bypasses ApproveTool, allowing hooks to respond + // for any tool name without approval. This is intentional for + // plugin tools but means a before_tool hook can override even + // sensitive tools like bash. Hook configuration should be + // carefully reviewed to prevent unauthorized tool execution. if toolReq != nil && toolReq.HookResult != nil { hookResult := toolReq.HookResult