mirror of
https://github.com/AstrBotDevs/AstrBot.git
synced 2026-08-31 01:40:25 +08:00
fix: harden sandbox file transfers (#8840)
* fix: harden sandbox file transfers * fix: check CUA sandbox availability with shell probe * fix: address sandbox transfer review feedback * fix: preserve CUA health check cancellation * fix: tighten CUA health probe checks
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import base64
|
||||
import inspect
|
||||
import shlex
|
||||
@@ -15,6 +16,7 @@ from .cua_defaults import CUA_CONFIG_KEYS, CUA_DEFAULT_CONFIG
|
||||
from .shipyard_search_file_util import search_files_via_shell
|
||||
|
||||
_POSIX_OS_TYPES = {"linux", "darwin", "macos"}
|
||||
_CUA_SANDBOX_HEALTH_PROBE = "_astrbot_cua_ok_"
|
||||
|
||||
_CUA_BACKGROUND_LAUNCHER = """
|
||||
import subprocess, sys, time
|
||||
@@ -55,10 +57,18 @@ async def _write_base64_via_shell(
|
||||
encoded = base64.b64encode(data).decode("ascii")
|
||||
decoder = (
|
||||
"import base64,pathlib,sys; "
|
||||
"pathlib.Path(sys.argv[1]).write_bytes(base64.b64decode(sys.stdin.read()))"
|
||||
"path=pathlib.Path(sys.argv[1]); "
|
||||
"path.parent.mkdir(parents=True, exist_ok=True); "
|
||||
"path.write_bytes(base64.b64decode(sys.stdin.read()))"
|
||||
)
|
||||
chunk_size = 60_000
|
||||
encoded_lines = "\n".join(
|
||||
encoded[index : index + chunk_size]
|
||||
for index in range(0, len(encoded), chunk_size)
|
||||
)
|
||||
return await shell.exec(
|
||||
f"python3 -c {shlex.quote(decoder)} {shlex.quote(path)} <<'EOF'\n{encoded}\nEOF"
|
||||
f"python3 -c {shlex.quote(decoder)} {shlex.quote(path)} <<'EOF'\n"
|
||||
f"{encoded_lines}\nEOF"
|
||||
)
|
||||
|
||||
|
||||
@@ -882,4 +892,17 @@ class CuaBooter(ComputerBooter):
|
||||
Path(local_path).write_bytes(base64.b64decode(result.get("stdout", "")))
|
||||
|
||||
async def available(self) -> bool:
|
||||
return self._runtime is not None
|
||||
if self._runtime is None:
|
||||
return False
|
||||
try:
|
||||
result = await self._runtime.shell.exec(
|
||||
f"echo {_CUA_SANDBOX_HEALTH_PROBE}", timeout=10
|
||||
)
|
||||
except asyncio.CancelledError:
|
||||
raise
|
||||
except Exception as exc:
|
||||
logger.debug("[Computer] CUA sandbox health check failed: %s", exc)
|
||||
return False
|
||||
if result.get("exit_code") != 0:
|
||||
return False
|
||||
return _CUA_SANDBOX_HEALTH_PROBE in str(result.get("stdout", ""))
|
||||
|
||||
@@ -70,6 +70,12 @@ _SANDBOX_RUNTIME_TOOL_CONFIG = {
|
||||
_IMAGE_FILE_SUFFIXES = {".bmp", ".gif", ".jpeg", ".jpg", ".png", ".webp"}
|
||||
|
||||
|
||||
def _remote_basename(path: str) -> str:
|
||||
# Sandbox paths may come from POSIX or Windows runtimes; normalize separators
|
||||
# without interpreting the path against the host filesystem.
|
||||
return path.replace("\\", "/").rstrip("/").split("/")[-1]
|
||||
|
||||
|
||||
def _restricted_env_path_labels(umo: str, *, include_plugin_skills: bool) -> list[str]:
|
||||
"""Labels for the allowed directories in a local(not sandbox) and restricted(not admin) environment"""
|
||||
normalized_umo = normalize_umo_for_workspace(umo)
|
||||
@@ -772,7 +778,7 @@ class FileDownloadTool(FunctionTool):
|
||||
context.context.event.unified_msg_origin,
|
||||
)
|
||||
try:
|
||||
name = os.path.basename(remote_path)
|
||||
name = _remote_basename(remote_path) or os.path.basename(remote_path)
|
||||
|
||||
local_path = os.path.join(
|
||||
get_astrbot_temp_path(), f"sandbox_{uuid.uuid4().hex[:4]}_{name}"
|
||||
@@ -784,7 +790,7 @@ class FileDownloadTool(FunctionTool):
|
||||
|
||||
if also_send_to_user:
|
||||
try:
|
||||
name = os.path.basename(local_path)
|
||||
name = _remote_basename(remote_path) or os.path.basename(local_path)
|
||||
if Path(local_path).suffix.lower() in _IMAGE_FILE_SUFFIXES:
|
||||
message_component = Image.fromFileSystem(local_path)
|
||||
sent_as = "image"
|
||||
|
||||
@@ -15,6 +15,7 @@ from astrbot.core.astr_agent_context import AstrAgentContext
|
||||
from astrbot.core.computer.computer_client import get_booter
|
||||
from astrbot.core.message.message_event_result import MessageChain
|
||||
from astrbot.core.platform.message_session import MessageSession
|
||||
from astrbot.core.tools.computer_tools.fs import _remote_basename
|
||||
from astrbot.core.tools.computer_tools.util import (
|
||||
check_admin_permission,
|
||||
is_local_runtime,
|
||||
@@ -173,7 +174,7 @@ class SendMessageToUserTool(FunctionTool[AstrAgentContext]):
|
||||
quoted_path = shlex.quote(path)
|
||||
result = await sb.shell.exec(f"test -f {quoted_path} && echo '_&exists_'")
|
||||
if "_&exists_" in json.dumps(result):
|
||||
name = os.path.basename(path)
|
||||
name = _remote_basename(path) or os.path.basename(path)
|
||||
local_path = os.path.join(
|
||||
get_astrbot_temp_path(), f"sandbox_{uuid.uuid4().hex[:4]}_{name}"
|
||||
)
|
||||
@@ -259,7 +260,7 @@ class SendMessageToUserTool(FunctionTool[AstrAgentContext]):
|
||||
url = msg.get("url")
|
||||
name = (
|
||||
msg.get("text")
|
||||
or (os.path.basename(path) if path else "")
|
||||
or (_remote_basename(path) if path else "")
|
||||
or (os.path.basename(url) if url else "")
|
||||
or "file"
|
||||
)
|
||||
|
||||
@@ -5,6 +5,7 @@ import io
|
||||
import zipfile
|
||||
from types import SimpleNamespace
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
from mcp.types import CallToolResult, ImageContent
|
||||
@@ -41,6 +42,100 @@ def _make_context(
|
||||
return ContextWrapper(context=astr_ctx)
|
||||
|
||||
|
||||
def _make_sandbox_context(
|
||||
*,
|
||||
role: str = "admin",
|
||||
umo: str = "qq:friend:user-1",
|
||||
):
|
||||
config_holder = SimpleNamespace(
|
||||
get_config=lambda umo=None: {
|
||||
"provider_settings": {
|
||||
"computer_use_require_admin": True,
|
||||
"computer_use_runtime": "sandbox",
|
||||
}
|
||||
}
|
||||
)
|
||||
event = SimpleNamespace(
|
||||
role=role,
|
||||
unified_msg_origin=umo,
|
||||
send=AsyncMock(),
|
||||
)
|
||||
astr_ctx = SimpleNamespace(context=config_holder, event=event)
|
||||
return ContextWrapper(context=astr_ctx)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sandbox_file_download_handles_windows_remote_filename(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
tmp_path,
|
||||
):
|
||||
temp_root = tmp_path / "temp"
|
||||
temp_root.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
monkeypatch.setattr(
|
||||
fs_tools,
|
||||
"get_astrbot_temp_path",
|
||||
lambda: str(temp_root),
|
||||
)
|
||||
|
||||
async def _download_file(_remote_path, local_path):
|
||||
assert local_path.endswith("report.txt")
|
||||
assert "\\" not in local_path
|
||||
|
||||
booter = SimpleNamespace(download_file=AsyncMock(side_effect=_download_file))
|
||||
|
||||
async def _fake_get_booter(_ctx, _umo):
|
||||
return booter
|
||||
|
||||
monkeypatch.setattr(fs_tools, "get_booter", _fake_get_booter)
|
||||
|
||||
context = _make_sandbox_context()
|
||||
result = await fs_tools.FileDownloadTool().call(
|
||||
context,
|
||||
remote_path=r"C:\Users\AstrBot\report.txt",
|
||||
also_send_to_user=True,
|
||||
)
|
||||
|
||||
assert "report.txt" in result
|
||||
sent_chain = context.context.event.send.await_args.args[0]
|
||||
sent_file = sent_chain.chain[0]
|
||||
assert sent_file.name == "report.txt"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sandbox_file_download_strips_trailing_remote_slash(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
tmp_path,
|
||||
):
|
||||
temp_root = tmp_path / "temp"
|
||||
temp_root.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
monkeypatch.setattr(
|
||||
fs_tools,
|
||||
"get_astrbot_temp_path",
|
||||
lambda: str(temp_root),
|
||||
)
|
||||
|
||||
booter = SimpleNamespace(download_file=AsyncMock())
|
||||
|
||||
async def _fake_get_booter(_ctx, _umo):
|
||||
return booter
|
||||
|
||||
monkeypatch.setattr(fs_tools, "get_booter", _fake_get_booter)
|
||||
|
||||
context = _make_sandbox_context()
|
||||
result = await fs_tools.FileDownloadTool().call(
|
||||
context,
|
||||
remote_path="reports/export/",
|
||||
also_send_to_user=True,
|
||||
)
|
||||
|
||||
assert "export" in result
|
||||
sent_chain = context.context.event.send.await_args.args[0]
|
||||
sent_file = sent_chain.chain[0]
|
||||
assert sent_file.name == "export"
|
||||
|
||||
|
||||
def _setup_local_fs_tools(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
tmp_path,
|
||||
|
||||
@@ -8,6 +8,7 @@ import mcp
|
||||
import pytest
|
||||
|
||||
from astrbot.core.astr_agent_tool_exec import FunctionToolExecutor
|
||||
from astrbot.core.computer.booters.cua import CuaShellComponent
|
||||
from astrbot.core.config.default import CONFIG_METADATA_3
|
||||
from astrbot.core.provider.func_tool_manager import FunctionToolManager
|
||||
|
||||
@@ -640,6 +641,35 @@ async def test_cua_write_file_shell_fallback_uses_python_base64_decoder():
|
||||
assert "base64 -d" not in command
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cua_write_file_shell_fallback_creates_parent_directory():
|
||||
from astrbot.core.computer.booters.cua import CuaFileSystemComponent
|
||||
|
||||
sandbox = FakeSandbox()
|
||||
delattr(sandbox, "filesystem")
|
||||
|
||||
await CuaFileSystemComponent(sandbox).write_file("reports/summary.txt", "hello")
|
||||
|
||||
command = sandbox.shell.commands[0][0]
|
||||
assert "path.parent.mkdir(parents=True, exist_ok=True)" in command
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cua_write_file_shell_fallback_chunks_large_payloads():
|
||||
from astrbot.core.computer.booters.cua import CuaFileSystemComponent
|
||||
|
||||
sandbox = FakeSandbox()
|
||||
delattr(sandbox, "filesystem")
|
||||
|
||||
await CuaFileSystemComponent(sandbox).write_file("large.txt", "x" * 100_000)
|
||||
|
||||
command = sandbox.shell.commands[0][0]
|
||||
payload = command.split("<<'EOF'\n", 1)[1].rsplit("\nEOF", 1)[0]
|
||||
lines = payload.splitlines()
|
||||
assert len(lines) > 1
|
||||
assert max(len(line) for line in lines) <= 60_000
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cua_create_file_reports_mode_as_informational():
|
||||
from astrbot.core.computer.booters.cua import CuaFileSystemComponent
|
||||
@@ -902,7 +932,6 @@ async def test_cua_upload_file_fallback_rejects_non_posix_os_type(tmp_path):
|
||||
CuaFileSystemComponent,
|
||||
CuaGUIComponent,
|
||||
CuaPythonComponent,
|
||||
CuaShellComponent,
|
||||
_CuaRuntime,
|
||||
)
|
||||
|
||||
@@ -933,7 +962,6 @@ async def test_cua_upload_file_prefers_native_files_upload(tmp_path):
|
||||
CuaFileSystemComponent,
|
||||
CuaGUIComponent,
|
||||
CuaPythonComponent,
|
||||
CuaShellComponent,
|
||||
_CuaRuntime,
|
||||
)
|
||||
|
||||
@@ -1302,7 +1330,6 @@ async def test_cua_shutdown_clears_cached_components():
|
||||
CuaFileSystemComponent,
|
||||
CuaGUIComponent,
|
||||
CuaPythonComponent,
|
||||
CuaShellComponent,
|
||||
_CuaRuntime,
|
||||
)
|
||||
|
||||
@@ -1330,6 +1357,162 @@ async def test_cua_shutdown_clears_cached_components():
|
||||
assert booter._runtime is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cua_available_checks_shell_health():
|
||||
from astrbot.core.computer.booters.cua import (
|
||||
CuaBooter,
|
||||
CuaFileSystemComponent,
|
||||
CuaGUIComponent,
|
||||
CuaPythonComponent,
|
||||
CuaShellComponent,
|
||||
_CuaRuntime,
|
||||
)
|
||||
|
||||
class HealthShell(FakeShell):
|
||||
async def run(self, command: str, **kwargs):
|
||||
self.commands.append((command, kwargs))
|
||||
return {"stdout": "_astrbot_cua_ok_\n", "stderr": "", "exit_code": 0}
|
||||
|
||||
sandbox = FakeSandbox()
|
||||
sandbox.shell = HealthShell()
|
||||
booter = CuaBooter()
|
||||
booter._runtime = _CuaRuntime(
|
||||
sandbox_cm=object(),
|
||||
sandbox=sandbox,
|
||||
shell=CuaShellComponent(sandbox),
|
||||
python=CuaPythonComponent(sandbox),
|
||||
fs=CuaFileSystemComponent(sandbox),
|
||||
gui=CuaGUIComponent(sandbox),
|
||||
)
|
||||
|
||||
assert await booter.available() is True
|
||||
assert sandbox.shell.commands[-1][0] == "echo _astrbot_cua_ok_"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cua_available_rejects_unknown_health_exit_code():
|
||||
from astrbot.core.computer.booters.cua import (
|
||||
CuaBooter,
|
||||
CuaFileSystemComponent,
|
||||
CuaGUIComponent,
|
||||
CuaPythonComponent,
|
||||
_CuaRuntime,
|
||||
)
|
||||
|
||||
class UnknownExitCodeRuntimeShell:
|
||||
async def exec(self, command: str, **kwargs):
|
||||
return {"stdout": "_astrbot_cua_ok_\n", "stderr": "", "exit_code": None}
|
||||
|
||||
sandbox = FakeSandbox()
|
||||
booter = CuaBooter()
|
||||
booter._runtime = _CuaRuntime(
|
||||
sandbox_cm=object(),
|
||||
sandbox=sandbox,
|
||||
shell=UnknownExitCodeRuntimeShell(),
|
||||
python=CuaPythonComponent(sandbox),
|
||||
fs=CuaFileSystemComponent(sandbox),
|
||||
gui=CuaGUIComponent(sandbox),
|
||||
)
|
||||
|
||||
assert await booter.available() is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cua_available_returns_false_when_shell_health_fails():
|
||||
from astrbot.core.computer.booters.cua import (
|
||||
CuaBooter,
|
||||
CuaFileSystemComponent,
|
||||
CuaGUIComponent,
|
||||
CuaPythonComponent,
|
||||
CuaShellComponent,
|
||||
_CuaRuntime,
|
||||
)
|
||||
|
||||
class DisconnectedShell:
|
||||
async def run(self, command: str, **kwargs):
|
||||
raise RuntimeError("server disconnected")
|
||||
|
||||
sandbox = FakeSandbox()
|
||||
sandbox.shell = DisconnectedShell()
|
||||
booter = CuaBooter()
|
||||
booter._runtime = _CuaRuntime(
|
||||
sandbox_cm=object(),
|
||||
sandbox=sandbox,
|
||||
shell=CuaShellComponent(sandbox),
|
||||
python=CuaPythonComponent(sandbox),
|
||||
fs=CuaFileSystemComponent(sandbox),
|
||||
gui=CuaGUIComponent(sandbox),
|
||||
)
|
||||
|
||||
assert await booter.available() is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cua_available_propagates_cancellation():
|
||||
from astrbot.core.computer.booters.cua import (
|
||||
CuaBooter,
|
||||
CuaFileSystemComponent,
|
||||
CuaGUIComponent,
|
||||
CuaPythonComponent,
|
||||
CuaShellComponent,
|
||||
_CuaRuntime,
|
||||
)
|
||||
|
||||
class CancellingShell:
|
||||
async def run(self, command: str, **kwargs):
|
||||
raise asyncio.CancelledError
|
||||
|
||||
sandbox = FakeSandbox()
|
||||
sandbox.shell = CancellingShell()
|
||||
booter = CuaBooter()
|
||||
booter._runtime = _CuaRuntime(
|
||||
sandbox_cm=object(),
|
||||
sandbox=sandbox,
|
||||
shell=CuaShellComponent(sandbox),
|
||||
python=CuaPythonComponent(sandbox),
|
||||
fs=CuaFileSystemComponent(sandbox),
|
||||
gui=CuaGUIComponent(sandbox),
|
||||
)
|
||||
|
||||
with pytest.raises(asyncio.CancelledError):
|
||||
await booter.available()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cua_available_allows_shell_health_warning_on_stderr():
|
||||
from astrbot.core.computer.booters.cua import (
|
||||
CuaBooter,
|
||||
CuaFileSystemComponent,
|
||||
CuaGUIComponent,
|
||||
CuaPythonComponent,
|
||||
CuaShellComponent,
|
||||
_CuaRuntime,
|
||||
)
|
||||
|
||||
class WarningShell(FakeShell):
|
||||
async def run(self, command: str, **kwargs):
|
||||
self.commands.append((command, kwargs))
|
||||
return {
|
||||
"stdout": "_astrbot_cua_ok_\n",
|
||||
"stderr": "profile warning\n",
|
||||
"exit_code": 0,
|
||||
}
|
||||
|
||||
sandbox = FakeSandbox()
|
||||
sandbox.shell = WarningShell()
|
||||
booter = CuaBooter()
|
||||
booter._runtime = _CuaRuntime(
|
||||
sandbox_cm=object(),
|
||||
sandbox=sandbox,
|
||||
shell=CuaShellComponent(sandbox),
|
||||
python=CuaPythonComponent(sandbox),
|
||||
fs=CuaFileSystemComponent(sandbox),
|
||||
gui=CuaGUIComponent(sandbox),
|
||||
)
|
||||
|
||||
assert await booter.available() is True
|
||||
|
||||
|
||||
def test_cua_tools_are_registered_as_builtin_tools():
|
||||
from astrbot.core.tools.computer_tools.cua import (
|
||||
CuaKeyboardTypeTool,
|
||||
|
||||
@@ -235,3 +235,97 @@ async def test_non_admin_can_send_temp_file(tmp_path, monkeypatch):
|
||||
|
||||
assert "Message sent to session" in result
|
||||
ctx.context.context.send_message.assert_called_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_message_downloads_windows_sandbox_file_with_original_name(
|
||||
tmp_path, monkeypatch
|
||||
):
|
||||
"""Windows sandbox paths keep their basename when sent as files."""
|
||||
tool = SendMessageToUserTool()
|
||||
ctx = _make_context(runtime="sandbox")
|
||||
temp_root = tmp_path / "temp"
|
||||
temp_root.mkdir()
|
||||
monkeypatch.setattr(
|
||||
"astrbot.core.tools.message_tools.get_astrbot_temp_path",
|
||||
lambda: str(temp_root),
|
||||
)
|
||||
|
||||
async def _exec(_command):
|
||||
return {"content": "_&exists_"}
|
||||
|
||||
async def _download_file(_remote_path, local_path):
|
||||
assert local_path.endswith("report.txt")
|
||||
assert "\\" not in local_path
|
||||
with open(local_path, "w", encoding="utf-8") as file:
|
||||
file.write("report")
|
||||
|
||||
booter = SimpleNamespace(
|
||||
shell=SimpleNamespace(exec=AsyncMock(side_effect=_exec)),
|
||||
download_file=AsyncMock(side_effect=_download_file),
|
||||
)
|
||||
|
||||
async def mock_get_booter(*args, **kwargs):
|
||||
del args, kwargs
|
||||
return booter
|
||||
|
||||
monkeypatch.setattr(
|
||||
"astrbot.core.tools.message_tools.get_booter",
|
||||
mock_get_booter,
|
||||
)
|
||||
|
||||
result = await tool.call(
|
||||
ctx,
|
||||
messages=[{"type": "file", "path": r"C:\Users\AstrBot\report.txt"}],
|
||||
)
|
||||
|
||||
assert "Message sent to session" in result
|
||||
sent_chain = ctx.context.context.send_message.await_args.args[1]
|
||||
sent_file = sent_chain.chain[0]
|
||||
assert sent_file.name == "report.txt"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_message_downloads_trailing_slash_sandbox_file_with_basename(
|
||||
tmp_path, monkeypatch
|
||||
):
|
||||
tool = SendMessageToUserTool()
|
||||
ctx = _make_context(runtime="sandbox")
|
||||
temp_root = tmp_path / "temp"
|
||||
temp_root.mkdir()
|
||||
monkeypatch.setattr(
|
||||
"astrbot.core.tools.message_tools.get_astrbot_temp_path",
|
||||
lambda: str(temp_root),
|
||||
)
|
||||
|
||||
async def _exec(_command):
|
||||
return {"content": "_&exists_"}
|
||||
|
||||
async def _download_file(_remote_path, local_path):
|
||||
assert local_path.endswith("export")
|
||||
with open(local_path, "w", encoding="utf-8") as file:
|
||||
file.write("export")
|
||||
|
||||
booter = SimpleNamespace(
|
||||
shell=SimpleNamespace(exec=AsyncMock(side_effect=_exec)),
|
||||
download_file=AsyncMock(side_effect=_download_file),
|
||||
)
|
||||
|
||||
async def mock_get_booter(*args, **kwargs):
|
||||
del args, kwargs
|
||||
return booter
|
||||
|
||||
monkeypatch.setattr(
|
||||
"astrbot.core.tools.message_tools.get_booter",
|
||||
mock_get_booter,
|
||||
)
|
||||
|
||||
result = await tool.call(
|
||||
ctx,
|
||||
messages=[{"type": "file", "path": "reports/export/"}],
|
||||
)
|
||||
|
||||
assert "Message sent to session" in result
|
||||
sent_chain = ctx.context.context.send_message.await_args.args[1]
|
||||
sent_file = sent_chain.chain[0]
|
||||
assert sent_file.name == "export"
|
||||
|
||||
Reference in New Issue
Block a user