Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions astrbot/core/message/components.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import sys
import uuid
from enum import Enum
from pathlib import Path, PurePosixPath

if sys.version_info >= (3, 14):
from pydantic import BaseModel
Expand Down Expand Up @@ -662,6 +663,19 @@ class Unknown(BaseMessageComponent):
text: str


def _sanitize_file_component_name(name: str | None) -> str:
if not name:
return "file"

normalized = str(name).replace("\\", "/")
basename = PurePosixPath(normalized).name.replace("\x00", "").strip()
for char in ':*?"<>|':
basename = basename.replace(char, "_")
if basename in {"", ".", ".."}:
return "file"
return basename
Comment on lines +666 to +676
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The _sanitize_file_component_name function correctly handles path traversal by extracting the basename and removing null bytes. However, it does not account for other characters that are invalid in filenames on certain operating systems, particularly Windows (e.g., :, *, ?, ", <, >, |). If a remote filename contains these characters, the subsequent download_file call will fail with an OSError on Windows. Consider replacing these characters with an underscore or removing them to ensure cross-platform compatibility. Additionally, as this is new functionality for handling file components, please ensure it is accompanied by unit tests.

Suggested change
def _sanitize_file_component_name(name: str | None) -> str:
if not name:
return "file"
normalized = str(name).replace("\\", "/")
basename = PurePosixPath(normalized).name.replace("\x00", "").strip()
if basename in {"", ".", ".."}:
return "file"
return basename
def _sanitize_file_component_name(name: str | None) -> str:
if not name:
return "file"
normalized = str(name).replace("\\", "/")
basename = PurePosixPath(normalized).name.replace("\\x00", "").strip()
# Remove or replace characters that are invalid on Windows filesystems
for char in ':*?"<>|':
basename = basename.replace(char, "_")
if basename in {"", ".", ".."}:
return "file"
return basename
References
  1. New functionality, such as handling attachments, should be accompanied by corresponding unit tests.



class File(BaseMessageComponent):
"""文件消息段"""

Expand Down Expand Up @@ -773,15 +787,18 @@ async def _download_file(self) -> None:
"""下载文件"""
if not self.url:
raise ValueError("Download failed: No URL provided in File component.")
download_dir = get_astrbot_temp_path()
download_dir = Path(get_astrbot_temp_path())
download_dir.mkdir(parents=True, exist_ok=True)
if self.name:
name, ext = os.path.splitext(self.name)
safe_name = _sanitize_file_component_name(self.name)
name = Path(safe_name).stem
ext = Path(safe_name).suffix
filename = f"fileseg_{name}_{uuid.uuid4().hex[:8]}{ext}"
else:
filename = f"fileseg_{uuid.uuid4().hex}"
file_path = os.path.join(download_dir, filename)
await download_file(self.url, file_path)
self.file_ = os.path.abspath(file_path)
file_path = download_dir / filename
await download_file(self.url, str(file_path))
self.file_ = str(file_path.resolve())

async def register_to_file_service(self) -> str:
"""将文件注册到文件服务。
Expand Down
39 changes: 39 additions & 0 deletions tests/unit/test_file_message_component.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
from pathlib import Path

import pytest

from astrbot.core.message import components


@pytest.mark.asyncio
async def test_file_component_download_sanitizes_remote_name(monkeypatch, tmp_path):
temp_dir = tmp_path / "temp"
downloaded_paths: list[Path] = []

async def fake_download_file(url: str, path: str) -> None:
target = Path(path)
assert url == "https://example.com/report"
assert target.parent == temp_dir
assert target.parent.exists()
assert "\x00" not in target.name
assert "/" not in target.name
assert "\\" not in target.name
assert not any(char in target.name for char in ':*?"<>|')
target.write_bytes(b"payload")
downloaded_paths.append(target)

monkeypatch.setattr(components, "download_file", fake_download_file)
monkeypatch.setattr(components, "get_astrbot_temp_path", lambda: str(temp_dir))

component = components.File(
name='..\\nested/evil\\report:*?"<>|\x00.pdf',
url="https://example.com/report",
)

path = Path(await component.get_file())

assert path.parent == temp_dir
assert path.exists()
assert path.name.startswith("fileseg_report________")
assert path.suffix == ".pdf"
assert downloaded_paths == [path]