docs(hooks): document security implications of respond action

Add security boundary documentation explaining that the respond action
bypasses ApproveTool checks, allowing hooks to return results for any
tool without approval. Include recommendations for secure hook
implementation and code comments marking the security considerations.

Changes:
- Add "Security Boundaries" section to plugin-tool-injection docs
- Document bypass of approval checks and associated risks
- Provide security recommendations and example code
- Add inline security comments in hooks.go and loop.go
This commit is contained in:
harmoon 2026-04-05 13:54:52 +08:00
parent bf3172a628
commit e4b7bc60f2
4 changed files with 87 additions and 4 deletions

View file

@ -546,3 +546,42 @@ Through the hook system's `respond` action, external processes can:
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.
---
## 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.

View file

@ -546,3 +546,42 @@ func getWeatherData(city string) string {
3. **与内置工具共存**:不影响 PicoClaw 原有工具的正常运行
这为插件开发提供了灵活、优雅的解决方案。
---
## 安全边界说明
### 绕过审批检查
**重要**`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 只影响插件工具,不影响系统工具的审批流程。

View file

@ -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"

View file

@ -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