docs: propose unified LLM tool permission model#8512
Conversation
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider clarifying in the proposal how permission resolution should behave in multi-tenant or multi-space scenarios (e.g., different workspaces or bots sharing the same plugins) so that the
permission_keyand override source of truth are unambiguous. - It may help to explicitly describe how this model interacts with existing per-tool, in-handler authorization logic (e.g., recommended pattern when both handler-level checks and unified permissions are present, and which should be considered authoritative on deny/allow conflicts).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider clarifying in the proposal how permission resolution should behave in multi-tenant or multi-space scenarios (e.g., different workspaces or bots sharing the same plugins) so that the `permission_key` and override source of truth are unambiguous.
- It may help to explicitly describe how this model interacts with existing per-tool, in-handler authorization logic (e.g., recommended pattern when both handler-level checks and unified permissions are present, and which should be considered authoritative on deny/allow conflicts).Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request proposes a unified permission model for LLM tools in AstrBot, covering design details such as tool filtering, execution-level validation, and WebUI integration. The review feedback highlights several critical design improvements: moving permission configurations out of provider-specific settings to a global field, handling potential key collisions when handler_module_path is missing, avoiding race conditions by copying req.func_tool during filtering, and securing the system context identification mechanism to prevent unauthorized bypasses.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| 修改后持久化到配置中,例如: | ||
|
|
||
| ```json | ||
| { | ||
| "provider_settings": { | ||
| "undeclared_llm_tool_permission": "member", | ||
| "llm_tool_permission_overrides": { | ||
| "mcp:playwright:browser_navigate": "admin", | ||
| "plugin:plugins.example.main:write_file": "admin", | ||
| "builtin:future_task": "admin" | ||
| } | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
建议与改进:配置存储位置
将工具权限配置(如 undeclared_llm_tool_permission 和 llm_tool_permission_overrides)持久化在 provider_settings 中可能不够合适。
原因:
provider_settings 通常用于存储与特定 LLM 提供商(如 OpenAI、Claude、Ollama 等)相关的配置(例如 API 密钥、模型名称、温度等)。而工具权限是全局或通道(Channel/Origin)级别的属性,与具体使用哪个 LLM 提供商无关。如果用户切换了 LLM 提供商,不应该导致他们需要重新配置所有的工具权限。
建议:
建议将这些权限配置项放置在全局配置的独立根字段中(例如 tool_permission_settings),或者直接作为 provider_settings 的同级字段,以保证配置的内聚性和良好的用户体验。
| 建议 key 格式: | ||
|
|
||
| ```text | ||
| builtin:{tool_name} | ||
| plugin:{handler_module_path}:{tool_name} | ||
| mcp:{server_name}:{tool_name} | ||
| unknown:{tool_name} | ||
| ``` |
There was a problem hiding this comment.
建议与改进:处理 handler_module_path 为空的情况
在生成插件工具的 permission key 时,使用了 plugin:{handler_module_path}:{tool_name} 格式。
潜在问题:
在 FunctionTool 中,handler_module_path 是一个可选字段(str | None = None)。如果插件开发者是通过手动注册或动态构建的方式添加工具,该字段可能会为 None。这会导致生成的 key 变成 plugin:None:{tool_name},在存在多个此类工具时会产生冲突。
建议:
建议在设计中明确规定当 handler_module_path 为 None 时的兜底策略。例如:
- 优先尝试获取插件的注册名称/ID。
- 如果完全无法获取,则退化为
unknown:{tool_name},或者在注册时强制要求或自动填充一个唯一的模块标识符。
| 在 LLM 请求真正发出前,根据当前事件用户权限过滤 `req.func_tool`。 | ||
|
|
||
| 建议位置: | ||
|
|
||
| ```text | ||
| _plugin_tool_fix(event, req) | ||
| 之后 | ||
| AgentRunner.reset / provider request 之前 | ||
| ``` |
There was a problem hiding this comment.
建议与改进:避免过滤 req.func_tool 时的副作用与竞态条件
在请求阶段过滤 req.func_tool 时,需要特别注意并发安全和副作用。
潜在问题:
如果 req.func_tool 指向的是一个共享的全局 ToolSet 实例,直接在请求阶段对其进行 in-place 过滤(修改其内部的 tools 列表)会导致严重的竞态条件(Race Condition)。例如,一个普通用户的请求可能会意外地把管理员用户的可用工具也过滤掉,或者反之导致越权。
建议:
建议在文档中明确指出:过滤操作必须在 ToolSet 的深拷贝(Deep Copy)或请求局部副本上进行。确保每个请求的 req.func_tool 都是独立的实例,从而彻底避免并发修改带来的安全隐患。
|
|
||
| 执行层仍然可以兜底拒绝未授权调用。 | ||
|
|
||
| 注意:无 `event` 的系统任务 / cron / 后台任务不应简单视为普通用户,否则可能误伤内部系统上下文。建议有 `event` 时严格走 `event.is_admin()`,无 `event` 时单独识别系统上下文。 |
There was a problem hiding this comment.
建议与改进:明确“系统上下文”的安全识别机制
提案中提到:“无 event 时单独识别系统上下文”。
潜在问题:
如果“系统上下文”的识别机制不够严谨,可能会被恶意用户利用自然语言或 prompt injection 绕过权限检查(例如伪造无 event 的调用上下文)。
建议:
建议在设计中明确“系统上下文”的具体安全识别方案。例如:
- 引入一个不可伪造的、仅在系统内部任务(如 Cron、系统初始化)中传递的
SystemContextToken或TrustedContext标志。 - 明确规定哪些特定的事件类(如
CronMessageEvent)属于受信任的系统事件,并进行严格的类型校验,而不是简单地通过event is None来判断。
摘要
本 PR 新增一份关于 AstrBot LLM Tools 统一权限模型的方案文档。
目前插件 tools、MCP tools、Handoff tools 以及部分内置 tools 都可能进入 LLM tool-calling 工具集合。但第三方工具的权限控制主要依赖插件或工具开发者自行在 handler 内实现。
如果插件开发者忘记在工具内部检查管理员权限,普通用户可能通过自然语言诱导 LLM 调用高危工具。
提案内容
文档主要建议:
FunctionTool增加统一的permission字段;member/admin/disabled三种权限级别;为什么先提交文档
这个改动会涉及主 Agent 请求构建、工具执行、MCP 工具、插件注册和 WebUI 接口等多个核心模块。
为了避免在未确认方案前直接修改核心运行时代码,本 PR 先提交方案文档,方便维护者讨论统一权限模型和 WebUI 行为。
后续如果方案方向确认,可以再继续提交实现 PR。
Summary by Sourcery
Documentation: