Skip to content

Commit 66ec415

Browse files
fix: restrict local file paths in message tools (#8660)
* fix: restrict local file paths in message tool * Update astrbot/core/tools/message_tools.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * fix: rf --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent 8f5178d commit 66ec415

2 files changed

Lines changed: 149 additions & 11 deletions

File tree

astrbot/core/tools/message_tools.py

Lines changed: 74 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import os
33
import shlex
44
import uuid
5+
from pathlib import Path
56

67
from pydantic import Field
78
from pydantic.dataclasses import dataclass
@@ -14,9 +15,55 @@
1415
from astrbot.core.computer.computer_client import get_booter
1516
from astrbot.core.message.message_event_result import MessageChain
1617
from astrbot.core.platform.message_session import MessageSession
17-
from astrbot.core.tools.computer_tools.util import check_admin_permission
18+
from astrbot.core.tools.computer_tools.util import (
19+
check_admin_permission,
20+
is_local_runtime,
21+
workspace_root,
22+
)
1823
from astrbot.core.tools.registry import builtin_tool
19-
from astrbot.core.utils.astrbot_path import get_astrbot_temp_path
24+
from astrbot.core.utils.astrbot_path import (
25+
get_astrbot_system_tmp_path,
26+
get_astrbot_temp_path,
27+
)
28+
29+
30+
def _file_send_allowed_roots(umo: str | None) -> tuple[Path, ...]:
31+
roots = []
32+
if umo:
33+
roots.append(workspace_root(umo))
34+
roots.extend(
35+
[
36+
Path(get_astrbot_temp_path()).resolve(strict=False),
37+
Path(get_astrbot_system_tmp_path()).resolve(strict=False),
38+
]
39+
)
40+
return tuple(roots)
41+
42+
43+
def _is_path_within(path: Path, roots: tuple[Path, ...]) -> bool:
44+
return any(path == root or path.is_relative_to(root) for root in roots)
45+
46+
47+
def _is_restricted_local_env(context: ContextWrapper[AstrAgentContext]) -> bool:
48+
if not is_local_runtime(context):
49+
return False
50+
cfg = context.context.context.get_config(
51+
umo=context.context.event.unified_msg_origin
52+
)
53+
provider_settings = cfg.get("provider_settings", {})
54+
require_admin = provider_settings.get("computer_use_require_admin", True)
55+
return require_admin and context.context.event.role != "admin"
56+
57+
58+
def _can_send_local_file(
59+
context: ContextWrapper[AstrAgentContext],
60+
local_path: Path,
61+
) -> bool:
62+
umo = context.context.event.unified_msg_origin
63+
allowed_roots = _file_send_allowed_roots(umo)
64+
if _is_path_within(local_path, allowed_roots):
65+
return True
66+
return is_local_runtime(context) and not _is_restricted_local_env(context)
2067

2168

2269
@builtin_tool
@@ -85,23 +132,38 @@ async def _resolve_path_from_sandbox(
85132
*,
86133
component_type: str = "file",
87134
) -> tuple[str, bool]:
88-
path = str(path)
89-
# if the path is relative, check if the file exists in user's local workspace
135+
path = str(path).strip()
136+
if not path:
137+
raise FileNotFoundError(f"{component_type} path is empty")
138+
139+
# Relative host paths are resolved only inside the user's workspace.
90140
if not os.path.isabs(path):
91141
unified_msg_origin = context.context.event.unified_msg_origin
92142
if unified_msg_origin:
93-
from astrbot.core.tools.computer_tools.util import workspace_root
94-
95143
try:
96144
ws_path = workspace_root(unified_msg_origin)
97-
ws_candidate = (ws_path / path).resolve()
145+
ws_candidate = (ws_path / path).resolve(strict=False)
98146
if ws_candidate.is_file() and ws_candidate.is_relative_to(ws_path):
99147
return str(ws_candidate), False
100148
except Exception:
101149
pass
102-
# check if the file exists in local environment (only allow absolute paths to prevent traversal)
103-
elif os.path.isfile(path):
104-
return path, False
150+
else:
151+
local_candidate = Path(path).expanduser().resolve(strict=False)
152+
if local_candidate.is_file():
153+
if _can_send_local_file(context, local_candidate):
154+
return str(local_candidate), False
155+
if is_local_runtime(context):
156+
allowed = ", ".join(
157+
str(root)
158+
for root in _file_send_allowed_roots(
159+
context.context.event.unified_msg_origin
160+
)
161+
)
162+
raise PermissionError(
163+
"Local file send is restricted for this user. "
164+
f"Allowed directories: {allowed}. "
165+
f"Blocked path: {local_candidate}."
166+
)
105167

106168
try:
107169
sb = await get_booter(
@@ -221,6 +283,8 @@ async def call(
221283
)
222284
except FileNotFoundError as exc:
223285
return f"error: {exc}"
286+
except PermissionError as exc:
287+
return f"error: {exc}"
224288
except Exception as exc:
225289
return f"error: failed to build messages[{idx}] component: {exc}"
226290

tests/unit/test_message_tools.py

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,15 @@ def _make_context(
1212
current_session="feishu:GroupMessage:oc_xxx",
1313
role="admin",
1414
require_admin=True,
15+
runtime="local",
1516
):
1617
"""Build a minimal ContextWrapper for SendMessageToUserTool."""
17-
cfg = {"provider_settings": {"computer_use_require_admin": require_admin}}
18+
cfg = {
19+
"provider_settings": {
20+
"computer_use_require_admin": require_admin,
21+
"computer_use_runtime": runtime,
22+
}
23+
}
1824
return SimpleNamespace(
1925
context=SimpleNamespace(
2026
event=SimpleNamespace(
@@ -161,3 +167,71 @@ async def mock_get_booter(*args, **kwargs):
161167

162168
assert "error: failed to build messages[1] component: sandbox unavailable" in result
163169
ctx.context.context.send_message.assert_not_called()
170+
171+
172+
@pytest.mark.asyncio
173+
async def test_non_admin_cannot_send_arbitrary_local_absolute_file(tmp_path):
174+
"""Non-admin users cannot send host files outside the allowed local roots."""
175+
tool = SendMessageToUserTool()
176+
ctx = _make_context(role="member", require_admin=True)
177+
secret_path = tmp_path / "secret.txt"
178+
secret_path.write_text("secret", encoding="utf-8")
179+
180+
result = await tool.call(
181+
ctx,
182+
messages=[{"type": "file", "path": str(secret_path)}],
183+
)
184+
185+
assert "error: Local file send is restricted for this user" in result
186+
assert str(secret_path) in result
187+
ctx.context.context.send_message.assert_not_called()
188+
189+
190+
@pytest.mark.asyncio
191+
async def test_non_admin_can_send_workspace_file(tmp_path, monkeypatch):
192+
"""Non-admin users can send files inside their per-session workspace."""
193+
tool = SendMessageToUserTool()
194+
ctx = _make_context(
195+
current_session="feishu:GroupMessage:oc_workspace",
196+
role="member",
197+
require_admin=True,
198+
)
199+
workspace_root = tmp_path / "workspaces"
200+
workspace_file = workspace_root / "feishu_GroupMessage_oc_workspace" / "result.txt"
201+
workspace_file.parent.mkdir(parents=True)
202+
workspace_file.write_text("result", encoding="utf-8")
203+
monkeypatch.setattr(
204+
"astrbot.core.tools.computer_tools.util.get_astrbot_workspaces_path",
205+
lambda: str(workspace_root),
206+
)
207+
208+
result = await tool.call(
209+
ctx,
210+
messages=[{"type": "file", "path": "result.txt"}],
211+
)
212+
213+
assert "Message sent to session" in result
214+
ctx.context.context.send_message.assert_called_once()
215+
216+
217+
@pytest.mark.asyncio
218+
async def test_non_admin_can_send_temp_file(tmp_path, monkeypatch):
219+
"""Non-admin users can send generated files under AstrBot temp."""
220+
tool = SendMessageToUserTool()
221+
ctx = _make_context(role="member", require_admin=True)
222+
temp_root = tmp_path / "temp"
223+
temp_root.mkdir()
224+
output_path = temp_root / "output.txt"
225+
output_path.write_text("output", encoding="utf-8")
226+
monkeypatch.setattr(
227+
"astrbot.core.tools.message_tools.get_astrbot_temp_path",
228+
lambda: str(temp_root),
229+
)
230+
231+
result = await tool.call(
232+
ctx,
233+
messages=[{"type": "file", "path": str(output_path)}],
234+
)
235+
236+
assert "Message sent to session" in result
237+
ctx.context.context.send_message.assert_called_once()

0 commit comments

Comments
 (0)