From 1a9d1f566d06f251330bb48c6f2fdaf9cb0edaed Mon Sep 17 00:00:00 2001 From: Yufeng He <40085740+he-yufeng@users.noreply.github.com> Date: Thu, 18 Jun 2026 23:15:25 +0800 Subject: [PATCH 1/4] fix(core): enforce persona tool boundaries (#8786) * fix(core): enforce persona tool boundaries * refactor: filter persona tools in one pass --------- Co-authored-by: Soulter <905617992@qq.com> --- astrbot/core/astr_agent_tool_exec.py | 9 ++++- astrbot/core/astr_main_agent.py | 24 +++++++++--- tests/unit/test_astr_agent_tool_exec.py | 33 ++++++++++++++++ tests/unit/test_astr_main_agent.py | 51 +++++++++++++++++++++++++ 4 files changed, 110 insertions(+), 7 deletions(-) diff --git a/astrbot/core/astr_agent_tool_exec.py b/astrbot/core/astr_agent_tool_exec.py index de5caad55..8c3ed661f 100644 --- a/astrbot/core/astr_agent_tool_exec.py +++ b/astrbot/core/astr_agent_tool_exec.py @@ -266,8 +266,13 @@ class FunctionToolExecutor(BaseFunctionToolExecutor[AstrAgentContext]): # "all tools", including runtime computer-use tools. if tools is None: toolset = ToolSet() - for registered_tool in llm_tools.func_list: - if isinstance(registered_tool, HandoffTool): + handoff_names = { + tool.name + for tool in tool_mgr.func_list + if isinstance(tool, HandoffTool) + } + for registered_tool in tool_mgr.get_full_tool_set(): + if registered_tool.name in handoff_names: continue if registered_tool.active: toolset.add_tool(registered_tool) diff --git a/astrbot/core/astr_main_agent.py b/astrbot/core/astr_main_agent.py index 4de3d77bf..3420a18b8 100644 --- a/astrbot/core/astr_main_agent.py +++ b/astrbot/core/astr_main_agent.py @@ -456,10 +456,10 @@ async def _ensure_persona_and_skills( cfg: dict, plugin_context: Context, event: AstrMessageEvent, -) -> None: +) -> set[str] | None: """Ensure persona and skills are applied to the request's system prompt or user prompt.""" if not req.conversation: - return + return None ( persona_id, @@ -514,11 +514,13 @@ async def _ensure_persona_and_skills( # inject toolset in the persona if (persona and persona.get("tools") is None) or not persona: + persona_allowed_tools = None persona_toolset = tmgr.get_full_tool_set() for tool in list(persona_toolset): if not tool.active: persona_toolset.remove_tool(tool.name) else: + persona_allowed_tools = {str(tool_name) for tool_name in persona["tools"]} persona_toolset = ToolSet() if persona["tools"]: for tool_name in persona["tools"]: @@ -599,6 +601,7 @@ async def _ensure_persona_and_skills( ) except Exception: pass + return persona_allowed_tools async def _request_img_caption( @@ -931,12 +934,13 @@ async def _decorate_llm_request( plugin_context: Context, config: MainAgentBuildConfig, provider: Provider | None = None, -) -> None: +) -> set[str] | None: cfg = config.provider_settings or plugin_context.get_config( umo=event.unified_msg_origin ).get("provider_settings", {}) _apply_prompt_prefix(req, cfg) + persona_allowed_tools = None main_provider_supports_image = provider is not None and _provider_supports_modality( provider, "image" @@ -945,7 +949,9 @@ async def _decorate_llm_request( quote_images_already_captioned = False if req.conversation: - await _ensure_persona_and_skills(req, cfg, plugin_context, event) + persona_allowed_tools = await _ensure_persona_and_skills( + req, cfg, plugin_context, event + ) if img_cap_prov_id and req.image_urls and not main_provider_supports_image: await _ensure_img_caption( @@ -974,6 +980,7 @@ async def _decorate_llm_request( tz = plugin_context.get_config().get("timezone") _append_system_reminders(event, req, cfg, tz) _apply_workspace_extra_prompt(event, req) + return persona_allowed_tools def _plugin_tool_fix(event: AstrMessageEvent, req: ProviderRequest) -> None: @@ -1502,7 +1509,9 @@ async def build_main_agent( else: return None - await _decorate_llm_request(event, req, plugin_context, config, provider=provider) + persona_allowed_tools = await _decorate_llm_request( + event, req, plugin_context, config, provider=provider + ) await _apply_kb(event, req, plugin_context, config) @@ -1538,6 +1547,11 @@ async def build_main_agent( ) ) + if persona_allowed_tools is not None and req.func_tool: + req.func_tool.tools = [ + tool for tool in req.func_tool.tools if tool.name in persona_allowed_tools + ] + fallback_providers = _get_fallback_chat_providers( provider, plugin_context, config.provider_settings ) diff --git a/tests/unit/test_astr_agent_tool_exec.py b/tests/unit/test_astr_agent_tool_exec.py index 5fab9fe0a..61fb4048c 100644 --- a/tests/unit/test_astr_agent_tool_exec.py +++ b/tests/unit/test_astr_agent_tool_exec.py @@ -3,9 +3,16 @@ from types import SimpleNamespace import mcp import pytest +from astrbot.core.agent.agent import Agent +from astrbot.core.agent.handoff import HandoffTool from astrbot.core.agent.run_context import ContextWrapper +from astrbot.core.agent.tool import FunctionTool from astrbot.core.astr_agent_tool_exec import FunctionToolExecutor from astrbot.core.message.components import Image +from astrbot.core.provider.func_tool_manager import ( + FunctionToolManager, + _PermissionGuardedTool, +) class _DummyEvent: @@ -29,6 +36,32 @@ def _build_run_context(message_components: list[object] | None = None): return ContextWrapper(context=ctx) +def test_build_handoff_toolset_keeps_permission_guards_for_default_tools(): + mgr = FunctionToolManager() + plugin_tool = FunctionTool( + name="admin_only_mcp", + description="admin tool", + parameters={"type": "object", "properties": {}}, + ) + handoff = HandoffTool(Agent(name="child")) + mgr.func_list = [plugin_tool, handoff] + + event = _DummyEvent() + context = SimpleNamespace( + get_config=lambda **_kwargs: { + "provider_settings": {"computer_use_runtime": "none"} + }, + get_llm_tool_manager=lambda: mgr, + ) + run_context = ContextWrapper(context=SimpleNamespace(event=event, context=context)) + + toolset = FunctionToolExecutor._build_handoff_toolset(run_context, tools=None) + + assert toolset is not None + assert isinstance(toolset.get_tool("admin_only_mcp"), _PermissionGuardedTool) + assert toolset.get_tool("transfer_to_child") is None + + @pytest.mark.asyncio async def test_collect_handoff_image_urls_normalizes_filters_and_appends_event_image( monkeypatch: pytest.MonkeyPatch, diff --git a/tests/unit/test_astr_main_agent.py b/tests/unit/test_astr_main_agent.py index 6265cb077..e49cc4c31 100644 --- a/tests/unit/test_astr_main_agent.py +++ b/tests/unit/test_astr_main_agent.py @@ -817,6 +817,57 @@ class TestEnsurePersonaAndSkills: assert req.func_tool is not None + @pytest.mark.asyncio + async def test_persona_empty_tools_filters_late_builtin_tools( + self, mock_event, mock_context, mock_provider + ): + module = ama + persona = {"name": "locked", "prompt": "No tools.", "tools": []} + mock_context.persona_manager.resolve_selected_persona = AsyncMock( + return_value=("locked", persona, None, False) + ) + mock_context.get_config.return_value = { + "provider_settings": { + "web_search": True, + "websearch_provider": "baidu_ai_search", + } + } + config = module.MainAgentBuildConfig( + tool_call_timeout=60, + provider_settings={ + "web_search": True, + "websearch_provider": "baidu_ai_search", + }, + computer_use_runtime="none", + ) + req = ProviderRequest(prompt="hello") + req.conversation = MagicMock(persona_id="locked", history="[]") + + with ( + patch("astrbot.core.astr_main_agent.AgentRunner") as mock_runner_cls, + patch("astrbot.core.astr_main_agent.AstrAgentContext"), + ): + mock_runner = MagicMock() + mock_runner.reset = AsyncMock() + mock_runner_cls.return_value = mock_runner + + result = await module.build_main_agent( + event=mock_event, + plugin_context=mock_context, + config=config, + provider=mock_provider, + req=req, + apply_reset=False, + ) + assert result is not None + try: + assert result.provider_request.func_tool is None or ( + result.provider_request.func_tool.empty() + ) + finally: + if result.reset_coro: + result.reset_coro.close() + @pytest.mark.asyncio async def test_subagent_dedupe_uses_default_persona_tools( self, mock_event, mock_context From 49b86320cbf706a5d558ea16ef4edd9b4bc374cf Mon Sep 17 00:00:00 2001 From: Weilong Liao <37870767+Soulter@users.noreply.github.com> Date: Thu, 18 Jun 2026 23:17:50 +0800 Subject: [PATCH 2/4] docs: clarify OpenAPI chat username identity (#8880) --- astrbot/dashboard/schemas.py | 10 +++++++++- docs/en/dev/openapi.md | 2 ++ docs/public/openapi.json | 3 ++- docs/zh/dev/openapi.md | 2 ++ openspec/openapi-v1.yaml | 1 + 5 files changed, 16 insertions(+), 2 deletions(-) diff --git a/astrbot/dashboard/schemas.py b/astrbot/dashboard/schemas.py index f29fcda7e..e6d939fc8 100644 --- a/astrbot/dashboard/schemas.py +++ b/astrbot/dashboard/schemas.py @@ -183,7 +183,15 @@ class OpenApiChatRequest(OpenModel): message: Any = None session_id: str | None = None conversation_id: str | None = None - username: str | None = None + username: str | None = Field( + default=None, + description=( + "Caller-declared WebChat sender/session owner. This value is used " + "as the message sender identity and may participate in " + "sender-ID-based permission checks; trusted integrations should " + "validate or map it before accepting end-user input." + ), + ) config_id: str | None = None config_name: str | None = None platform_id: str | None = None diff --git a/docs/en/dev/openapi.md b/docs/en/dev/openapi.md index 0bb15d852..51ece2cf8 100644 --- a/docs/en/dev/openapi.md +++ b/docs/en/dev/openapi.md @@ -130,6 +130,8 @@ Notes: `POST /api/v1/chat` additionally requires `username`, with optional `session_id` (a UUID is auto-generated if omitted). +`username` is a caller-declared WebChat identity. It is used as the message sender and session owner in the message pipeline, including sender-ID-based command permission checks. Treat API keys with the `chat` scope as trusted backend credentials. If you expose chat access to end users, proxy requests through your own service and map each external user to an allowed `username`; do not let clients submit administrator IDs or other reserved sender IDs directly. + ```json { "username": "alice", diff --git a/docs/public/openapi.json b/docs/public/openapi.json index 4f7f4247c..88c700817 100644 --- a/docs/public/openapi.json +++ b/docs/public/openapi.json @@ -5944,7 +5944,8 @@ ], "properties": { "username": { - "type": "string" + "type": "string", + "description": "Caller-declared WebChat sender/session owner. This value is used as the message sender identity and may participate in sender-ID-based command permission checks. Treat chat-scoped API keys as trusted backend credentials and map or validate usernames before accepting end-user input." }, "session_id": { "type": "string" diff --git a/docs/zh/dev/openapi.md b/docs/zh/dev/openapi.md index 6c782f05c..ea278640f 100644 --- a/docs/zh/dev/openapi.md +++ b/docs/zh/dev/openapi.md @@ -131,6 +131,8 @@ X-API-Key: abk_xxx `POST /api/v1/chat` 额外需要 `username`，可选 `session_id`（不传会自动创建 UUID）。 +`username` 是调用方声明的 WebChat 用户标识，会作为本次消息的 sender 和会话 owner 进入消息管道，并参与基于 sender ID 的指令权限判断。因此，带有 `chat` scope 的 API Key 应仅发放给可信后端服务。如果需要面向终端用户开放，请在自己的服务端将外部用户映射到受控的 `username`，不要允许客户端直接传入管理员 ID 或其他保留 sender ID。 + ```json { "username": "alice", diff --git a/openspec/openapi-v1.yaml b/openspec/openapi-v1.yaml index bd33a7a83..80cbeef3b 100644 --- a/openspec/openapi-v1.yaml +++ b/openspec/openapi-v1.yaml @@ -5266,6 +5266,7 @@ components: properties: username: type: string + description: Caller-declared WebChat sender/session owner. This value is used as the message sender identity and may participate in sender-ID-based command permission checks. Treat chat-scoped API keys as trusted backend credentials and map or validate usernames before accepting end-user input. session_id: type: string conversation_id: From 309e05d3ccc8753c592ef340edc52049da3abc12 Mon Sep 17 00:00:00 2001 From: Weilong Liao <37870767+Soulter@users.noreply.github.com> Date: Thu, 18 Jun 2026 23:35:33 +0800 Subject: [PATCH 3/4] fix: enforce future task owner checks (#8881) --- astrbot/core/tools/cron_tools.py | 28 +++++++-- tests/unit/test_cron_tools.py | 98 ++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 5 deletions(-) diff --git a/astrbot/core/tools/cron_tools.py b/astrbot/core/tools/cron_tools.py index bbfa5729d..ee6ff27e2 100644 --- a/astrbot/core/tools/cron_tools.py +++ b/astrbot/core/tools/cron_tools.py @@ -23,6 +23,23 @@ def _extract_job_session(job: Any) -> str | None: return str(session) if session is not None else None +def _extract_job_sender(job: Any) -> str | None: + payload = getattr(job, "payload", None) + if not isinstance(payload, dict): + return None + sender_id = payload.get("sender_id") + return str(sender_id) if sender_id is not None else None + + +def _job_belongs_to_current_sender( + job: Any, current_umo: str, current_sender_id: str +) -> bool: + return ( + _extract_job_session(job) == current_umo + and _extract_job_sender(job) == current_sender_id + ) + + def _parse_run_at(run_at: Any) -> datetime | None: if run_at in (None, ""): return None @@ -133,6 +150,7 @@ class FutureTaskTool(FunctionTool[AstrAgentContext]): return f"Scheduled future task {job.job_id} ({job.name}) {suffix}." current_umo = context.context.event.unified_msg_origin + current_sender_id = str(context.context.event.get_sender_id()) if action == "edit": job_id = kwargs.get("job_id") if not job_id: @@ -146,8 +164,8 @@ class FutureTaskTool(FunctionTool[AstrAgentContext]): job = await cron_mgr.db.get_cron_job(str(job_id)) if not job: return f"error: cron job {job_id} not found." - if _extract_job_session(job) != current_umo: - return "error: you can only edit future tasks in the current umo." + if not _job_belongs_to_current_sender(job, current_umo, current_sender_id): + return "error: you can only edit your own future tasks." payload = dict(job.payload) if isinstance(job.payload, dict) else {} @@ -214,8 +232,8 @@ class FutureTaskTool(FunctionTool[AstrAgentContext]): job = await cron_mgr.db.get_cron_job(str(job_id)) if not job: return f"error: cron job {job_id} not found." - if _extract_job_session(job) != current_umo: - return "error: you can only delete future tasks in the current umo." + if not _job_belongs_to_current_sender(job, current_umo, current_sender_id): + return "error: you can only delete your own future tasks." await cron_mgr.delete_job(str(job_id)) return f"Deleted cron job {job_id}." @@ -223,7 +241,7 @@ class FutureTaskTool(FunctionTool[AstrAgentContext]): jobs = [ job for job in await cron_mgr.list_jobs() - if _extract_job_session(job) == current_umo + if _job_belongs_to_current_sender(job, current_umo, current_sender_id) ] if not jobs: return "No cron jobs found." diff --git a/tests/unit/test_cron_tools.py b/tests/unit/test_cron_tools.py index a0e520d46..2cc254fbe 100644 --- a/tests/unit/test_cron_tools.py +++ b/tests/unit/test_cron_tools.py @@ -8,6 +8,36 @@ import pytest from astrbot.core.tools.cron_tools import FutureTaskTool +def _context(cron_mgr, *, umo: str = "test:group:shared", sender_id: str = "user-1"): + return SimpleNamespace( + context=SimpleNamespace( + context=SimpleNamespace(cron_manager=cron_mgr), + event=SimpleNamespace( + unified_msg_origin=umo, + get_sender_id=lambda: sender_id, + ), + ) + ) + + +def _job(job_id: str, *, umo: str = "test:group:shared", sender_id: str = "user-1"): + return SimpleNamespace( + job_id=job_id, + name=f"name-{job_id}", + job_type="active_agent", + run_once=False, + cron_expression="0 8 * * *", + enabled=True, + next_run_time=None, + payload={ + "session": umo, + "sender_id": sender_id, + "note": f"note-{job_id}", + "origin": "tool", + }, + ) + + def test_future_task_schema_has_action_and_create_cron_guidance(): """The merged tool should expose action routing and unambiguous cron guidance.""" tool = FutureTaskTool() @@ -124,3 +154,71 @@ async def test_future_task_edit_updates_existing_job(): }, ) assert result == "Updated future task job-1 (new name)." + + +@pytest.mark.asyncio +async def test_future_task_edit_rejects_same_umo_different_sender(): + """Same-session users should not edit another sender's task.""" + tool = FutureTaskTool() + existing_job = _job("job-1", sender_id="admin-user") + cron_mgr = SimpleNamespace( + db=SimpleNamespace(get_cron_job=AsyncMock(return_value=existing_job)), + update_job=AsyncMock(), + ) + + result = await tool.call( + _context(cron_mgr, sender_id="attacker-user"), + action="edit", + job_id="job-1", + note="attacker note", + ) + + assert result == "error: you can only edit your own future tasks." + cron_mgr.update_job.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_future_task_delete_rejects_same_umo_different_sender(): + """Same-session users should not delete another sender's task.""" + tool = FutureTaskTool() + existing_job = _job("job-1", sender_id="admin-user") + cron_mgr = SimpleNamespace( + db=SimpleNamespace(get_cron_job=AsyncMock(return_value=existing_job)), + delete_job=AsyncMock(), + ) + + result = await tool.call( + _context(cron_mgr, sender_id="attacker-user"), + action="delete", + job_id="job-1", + ) + + assert result == "error: you can only delete your own future tasks." + cron_mgr.delete_job.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_future_task_list_filters_by_umo_and_sender(): + """List mode should show only tasks owned by the current sender.""" + tool = FutureTaskTool() + own_job = _job("own-job", sender_id="user-1") + same_umo_other_sender = _job("other-sender-job", sender_id="user-2") + different_umo_same_sender = _job( + "other-umo-job", + umo="test:group:other", + sender_id="user-1", + ) + cron_mgr = SimpleNamespace( + list_jobs=AsyncMock( + return_value=[own_job, same_umo_other_sender, different_umo_same_sender] + ) + ) + + result = await tool.call( + _context(cron_mgr, sender_id="user-1"), + action="list", + ) + + assert "own-job" in result + assert "other-sender-job" not in result + assert "other-umo-job" not in result From 59734c22b6edbeeea98c5713fdce3d0b34788c0d Mon Sep 17 00:00:00 2001 From: Weilong Liao <37870767+Soulter@users.noreply.github.com> Date: Fri, 19 Jun 2026 00:28:36 +0800 Subject: [PATCH 4/4] fix(core): reject hardlinked files in restricted local fs tools Reject multi-linked regular files in restricted local filesystem tools so workspace hardlink aliases cannot read or overwrite files outside allowed directories. Fixes #8868. --- astrbot/core/tools/computer_tools/fs.py | 24 ++++++++++++++++ tests/test_computer_fs_tools.py | 38 +++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/astrbot/core/tools/computer_tools/fs.py b/astrbot/core/tools/computer_tools/fs.py index 390b4095d..c3236c8a1 100644 --- a/astrbot/core/tools/computer_tools/fs.py +++ b/astrbot/core/tools/computer_tools/fs.py @@ -34,6 +34,7 @@ Local path resolution rule: """ import os +import stat import uuid from dataclasses import dataclass, field from pathlib import Path @@ -182,6 +183,25 @@ def _is_path_within_allowed_roots( ) +def _reject_multi_link_file(path: str) -> None: + try: + path_stat = os.stat(path) + except FileNotFoundError: + return + except OSError as exc: + raise PermissionError( + "Access denied: unable to inspect restricted path link count. " + f"Blocked path: {path}." + ) from exc + + if stat.S_ISREG(path_stat.st_mode) and path_stat.st_nlink > 1: + raise PermissionError( + "Access denied: file has multiple hard links and may alias content " + "outside allowed directories. " + f"Link count: {path_stat.st_nlink}. Blocked path: {path}." + ) + + def _normalize_rw_path( path: str, *, @@ -208,6 +228,8 @@ def _normalize_rw_path( f"{access} access is restricted for this user. " f"Allowed directories: {allowed}. Blocked path: {normalized_path}." ) + if restricted: + _reject_multi_link_file(normalized_path) return normalized_path @@ -602,6 +624,8 @@ class GrepTool(FunctionTool): "Read access is restricted for this user. " f"Allowed directories: {allowed}. Blocked paths: {blocked}." ) + for path in normalized: + _reject_multi_link_file(path) return normalized diff --git a/tests/test_computer_fs_tools.py b/tests/test_computer_fs_tools.py index 1277122ef..11571665a 100644 --- a/tests/test_computer_fs_tools.py +++ b/tests/test_computer_fs_tools.py @@ -2,6 +2,7 @@ from __future__ import annotations import base64 import io +import os import zipfile from types import SimpleNamespace from typing import Any @@ -194,6 +195,13 @@ def _make_large_text() -> str: return "".join(f"line-{index:05d}-{'x' * 48}\n" for index in range(6000)) +def _make_hardlink_or_skip(source, link) -> None: + try: + os.link(source, link) + except (AttributeError, OSError) as exc: + pytest.skip(f"hard links are unavailable on this filesystem: {exc}") + + def _make_epub_bytes(*, chapter_count: int = 1) -> bytes: manifest_items = [ '' @@ -363,6 +371,36 @@ async def test_restricted_local_member_cannot_write_plugin_provided_skill( assert plugin_skill.read_text(encoding="utf-8") == "# Demo Skill\n" +@pytest.mark.asyncio +async def test_restricted_local_member_rejects_workspace_hardlink_alias( + monkeypatch: pytest.MonkeyPatch, + tmp_path, +): + workspace = _setup_local_fs_tools(monkeypatch, tmp_path) + outside_dir = tmp_path / "outside" + outside_dir.mkdir() + outside_file = outside_dir / "secret.txt" + outside_file.write_text("outside-secret\n", encoding="utf-8") + hardlink_path = workspace / "linked.txt" + _make_hardlink_or_skip(outside_file, hardlink_path) + + read_result = await fs_tools.FileReadTool().call( + _make_context(role="member"), + path="linked.txt", + ) + write_result = await fs_tools.FileWriteTool().call( + _make_context(role="member"), + path="linked.txt", + content="changed\n", + ) + + assert "multiple hard links" in read_result + assert "may alias content outside allowed directories" in read_result + assert "multiple hard links" in write_result + assert "may alias content outside allowed directories" in write_result + assert outside_file.read_text(encoding="utf-8") == "outside-secret\n" + + def test_detect_text_encoding_allows_utf8_probe_cut_mid_character(): sample = '{"results": ["中文内容"]}'.encode()[:-1]