diff --git a/backend/app/channels/dingtalk.py b/backend/app/channels/dingtalk.py index 5a98811b6..93bfc7d99 100644 --- a/backend/app/channels/dingtalk.py +++ b/backend/app/channels/dingtalk.py @@ -631,7 +631,7 @@ class DingTalkChannel(Channel): # Directory prep and collision-safe publication are blocking # filesystem IO, so the whole sequence stays off the event loop. paths.ensure_thread_dirs(thread_id, user_id=effective_user_id) - uploads_dir = paths.sandbox_uploads_dir(thread_id, user_id=effective_user_id).resolve() + uploads_dir = paths.sandbox_uploads_dir(thread_id, user_id=effective_user_id) return publish_upload_bytes(uploads_dir, safe_filename, content) try: diff --git a/backend/app/channels/feishu.py b/backend/app/channels/feishu.py index 66c37e9a4..5220f661d 100644 --- a/backend/app/channels/feishu.py +++ b/backend/app/channels/feishu.py @@ -448,7 +448,7 @@ class FeishuChannel(Channel): def _persist(): paths.ensure_thread_dirs(thread_id, user_id=effective_user_id) - uploads_dir = paths.sandbox_uploads_dir(thread_id, user_id=effective_user_id).resolve() + uploads_dir = paths.sandbox_uploads_dir(thread_id, user_id=effective_user_id) return publish_upload_bytes(uploads_dir, filename, content) try: diff --git a/backend/app/channels/wechat.py b/backend/app/channels/wechat.py index 194f7e78e..0e6adb11a 100644 --- a/backend/app/channels/wechat.py +++ b/backend/app/channels/wechat.py @@ -27,6 +27,7 @@ from app.channels.base import Channel from app.channels.commands import is_known_channel_command from app.channels.connection_identity import attach_connection_identity from app.channels.message_bus import InboundMessage, InboundMessageType, MessageBus, OutboundMessage, ResolvedAttachment +from deerflow.uploads.manager import UnsafeUploadPathError, publish_upload_bytes logger = logging.getLogger(__name__) @@ -1150,10 +1151,8 @@ class WechatChannel(Channel): return None try: download_dir.mkdir(parents=True, exist_ok=True) - path = download_dir / filename - path.write_bytes(content) - return path - except OSError: + return publish_upload_bytes(download_dir, filename, content) + except (OSError, UnsafeUploadPathError): logger.exception("[WeChat] failed to persist inbound media file %s", filename) return None diff --git a/backend/tests/blocking_io/test_dingtalk_receive_file.py b/backend/tests/blocking_io/test_dingtalk_receive_file.py index 8e08ec5ab..02d25801c 100644 --- a/backend/tests/blocking_io/test_dingtalk_receive_file.py +++ b/backend/tests/blocking_io/test_dingtalk_receive_file.py @@ -55,3 +55,26 @@ async def test_receive_file_persist_does_not_block_event_loop(tmp_path, monkeypa assert "/uploads/a.pdf" in out.text assert out.files == [] + + +async def test_receive_file_rejects_symlinked_upload_directory(tmp_path, monkeypatch) -> None: + from app.channels.dingtalk import DingTalkChannel + from app.channels.message_bus import MessageBus + from deerflow.config.paths import Paths + + paths = await asyncio.to_thread(Paths, str(tmp_path)) + await asyncio.to_thread(paths.ensure_thread_dirs, "t1", user_id="default") + uploads = paths.sandbox_uploads_dir("t1", user_id="default") + outside = tmp_path / "outside" + await asyncio.to_thread(outside.mkdir) + await asyncio.to_thread(uploads.rmdir) + await asyncio.to_thread(uploads.symlink_to, outside, target_is_directory=True) + monkeypatch.setattr("app.channels.dingtalk.get_paths", lambda: paths) + + channel = DingTalkChannel(MessageBus(), config={}) + channel._download_by_code = AsyncMock(return_value=b"DATA") + + result = await channel._receive_single_file("dc", "file", "a.pdf", "t1", user_id="default") + + assert result == "" + assert not await asyncio.to_thread((outside / "a.pdf").exists) diff --git a/backend/tests/blocking_io/test_feishu_receive_file.py b/backend/tests/blocking_io/test_feishu_receive_file.py index 2231f79e0..cb6a27c1c 100644 --- a/backend/tests/blocking_io/test_feishu_receive_file.py +++ b/backend/tests/blocking_io/test_feishu_receive_file.py @@ -176,3 +176,28 @@ async def test_receive_file_renames_around_planted_symlink(tmp_path, monkeypatch assert not await asyncio.to_thread(outside.exists) assert await asyncio.to_thread((uploads / "report.pdf").is_symlink) assert await asyncio.to_thread((uploads / "report_1.pdf").read_bytes) == b"DATA" + + +async def test_receive_file_rejects_symlinked_upload_directory(tmp_path, monkeypatch) -> None: + from deerflow.config.paths import Paths + + paths = await asyncio.to_thread(Paths, str(tmp_path)) + await asyncio.to_thread(paths.ensure_thread_dirs, "thread-1", user_id="ou-user") + uploads = paths.sandbox_uploads_dir("thread-1", user_id="ou-user") + outside = tmp_path / "outside" + await asyncio.to_thread(outside.mkdir) + await asyncio.to_thread(uploads.rmdir) + await asyncio.to_thread(uploads.symlink_to, outside, target_is_directory=True) + monkeypatch.setattr("app.channels.feishu.get_paths", lambda: paths) + monkeypatch.setattr("app.channels.feishu.get_sandbox_provider", lambda: _MountedProvider()) + + result = await _channel_with_file(b"DATA", "report.pdf")._receive_single_file( + "message-1", + "file-key", + "file", + "thread-1", + user_id="ou-user", + ) + + assert result == "Failed to obtain the [file]" + assert not await asyncio.to_thread((outside / "report.pdf").exists) diff --git a/backend/tests/test_wechat_channel.py b/backend/tests/test_wechat_channel.py index 07c926509..089bc11d2 100644 --- a/backend/tests/test_wechat_channel.py +++ b/backend/tests/test_wechat_channel.py @@ -6,6 +6,7 @@ import asyncio import base64 import json import logging +from concurrent.futures import ThreadPoolExecutor from pathlib import Path from typing import Any from unittest import mock @@ -1017,6 +1018,46 @@ def test_handle_update_downloads_inbound_file(monkeypatch, tmp_path: Path): _run(go()) +def test_stage_downloaded_file_preserves_concurrent_same_name_payloads(tmp_path: Path): + from app.channels.wechat import WechatChannel + + channel = WechatChannel(bus=MessageBus(), config={"bot_token": "test-token", "state_dir": str(tmp_path)}) + payloads = [f"payload-{index}".encode() for index in range(8)] + + with ThreadPoolExecutor(max_workers=len(payloads)) as pool: + paths = list(pool.map(lambda payload: channel._stage_downloaded_file("report.pdf", payload), payloads)) + + assert all(path is not None for path in paths) + assert {path.name for path in paths if path is not None} == { + "report.pdf", + "report_1.pdf", + "report_2.pdf", + "report_3.pdf", + "report_4.pdf", + "report_5.pdf", + "report_6.pdf", + "report_7.pdf", + } + assert {path.read_bytes() for path in paths if path is not None} == set(payloads) + + +def test_stage_downloaded_file_renames_around_planted_symlink(tmp_path: Path): + from app.channels.wechat import WechatChannel + + channel = WechatChannel(bus=MessageBus(), config={"bot_token": "test-token", "state_dir": str(tmp_path)}) + download_dir = tmp_path / channel.DEFAULT_IMAGE_DOWNLOAD_DIRNAME + download_dir.mkdir() + victim = tmp_path / "victim.txt" + victim.write_bytes(b"victim") + (download_dir / "report.pdf").symlink_to(victim) + + stored = channel._stage_downloaded_file("report.pdf", b"attachment") + + assert stored == download_dir / "report_1.pdf" + assert stored.read_bytes() == b"attachment" + assert victim.read_bytes() == b"victim" + + def test_handle_update_downloads_inbound_file_with_media_aeskey_hex(monkeypatch, tmp_path: Path): from app.channels.wechat import WechatChannel