fix(client): stop embedded uploads from writing through symlinks (#5578)

* fix(client): stop embedded uploads from writing through symlinks

DeerFlowClient.upload_files copied each file with shutil.copy2 and let
convert_file_to_markdown write the companion straight into the uploads
directory. Local and AIO sandboxes can write to that directory, so a
symlink planted at an upload name or at the companion's name was
followed: the upload's bytes and the converted Markdown landed in
whatever host file the link pointed to, and the call reported success.
The Gateway refuses symlinked destinations and the IM channels write
through write_upload_file_no_symlink; the embedded client never adopted
either.

Uploads now go through copy_upload_file_no_symlink, a new helper next
to write_upload_file_no_symlink. It keeps copy2's content, permission
bits and timestamps, so files stay readable to Docker sandboxes, but
applies them to the descriptor opened with O_NOFOLLOW and opens the
source first so a missing source cannot truncate an existing upload.
As in the Gateway, a file with an unsafe destination is skipped and
listed in skipped_files, success turns false, and the message says how
many were skipped. The companion is converted inside a private temporary
directory and then written with write_upload_file_no_symlink; one whose
name is unsafe is left out like a failed conversion, and the original
upload is kept.

* docs(changelog): note embedded upload symlink fix (#5578)

* fix(client): keep copy2's same-file guard and companion permissions

Review follow-up. Two regressions in the previous commit.

copy_upload_file_no_symlink opened the destination before comparing it
with the source, and that open truncates. Passing a file that already
sits in the thread's uploads directory therefore copied an emptied file
over itself: the upload reported success with size 0 and the original
bytes were gone, where copy2 raised SameFileError and left the file
alone. The destination is now compared with the source through
os.path.samestat before anything is opened, so identity — including a
hardlink or another spelling of the same path — raises SameFileError as
before.

The Markdown companion was published with write_upload_file_no_symlink,
which creates a new file as 0600 and ignores the converted file's mode.
Under umask 022 the companion became 0600 while its own document stayed
0644, so a bind-mounted sandbox running as another uid could read the
upload but not the Markdown the response advertises. It now goes
through the same copy helper as the upload, which preserves the
converter's permission bits.

CHANGELOG.md

@@ -2781,6 +2781,13 @@ This release closes that milestone with **765 merged pull requests**.
 
 ### Security
 
+- **client:** `DeerFlowClient.upload_files` no longer writes through a
+  symlink. A symlink planted in the sandbox-writable uploads directory, at an
+  upload's name or its Markdown companion's name, made the embedded client
+  overwrite the host file it pointed to while reporting success. The file is
+  now skipped and listed in `skipped_files` with `success: false`, matching
+  the Gateway; an unsafe companion is left out and the upload kept. Copies
+  keep the source's permission bits and timestamps. ([#5578])
 - **uploads:** Deleting an upload no longer follows a symlink to delete a
   different file. A symlink planted in the sandbox-writable uploads directory
   made `DELETE /api/threads/{id}/uploads/{filename}` (and
@@ -4310,3 +4317,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag.
 [#5526]: https://github.com/bytedance/deer-flow/pull/5526
 [#5534]: https://github.com/bytedance/deer-flow/pull/5534
 [#5547]: https://github.com/bytedance/deer-flow/pull/5547
+[#5578]: https://github.com/bytedance/deer-flow/pull/5578

CHANGELOG_zh.md

@@ -2091,6 +2091,10 @@
 
 ### 安全
 
+- **客户端：** `DeerFlowClient.upload_files` 不再写穿符号链接。沙箱可写的 uploads 目录中，
+  若在上传文件名或其 Markdown 配套文件名处放置符号链接，嵌入式客户端此前会覆盖链接指向的宿主
+  文件并报告成功。现在该文件会被跳过并列入 `skipped_files`，`success` 为 `false`，与 Gateway
+  一致；不安全的配套文件会被省略，原上传保留。复制时保留源文件的权限位与时间戳。([#5578])
 - **上传：** 删除上传文件时不再跟随符号链接删除另一个文件。沙箱可写的 uploads 目录中若被
   放置符号链接，`DELETE /api/threads/{id}/uploads/{filename}`（以及
   `DeerFlowClient.delete_upload`）此前会删除链接指向的上传文件及其配套 `.md`，却仍报告
@@ -3512,3 +3516,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写，核心包含子
 [#5526]: https://github.com/bytedance/deer-flow/pull/5526
 [#5534]: https://github.com/bytedance/deer-flow/pull/5534
 [#5547]: https://github.com/bytedance/deer-flow/pull/5547
+[#5578]: https://github.com/bytedance/deer-flow/pull/5578

backend/docs/FILE_UPLOAD.md

@@ -236,6 +236,7 @@ backend/.deer-flow/threads/
 - 最大文件大小：100MB（可在 nginx.conf 中配置 `client_max_body_size`）
 - 文件名安全性：系统会自动验证文件路径，防止目录遍历攻击
 - 删除只作用于普通文件：上传目录中的符号链接不会被跟随，删除请求按文件不存在（404）处理
+- 上传（HTTP 与嵌入式 `DeerFlowClient`）不会写穿符号链接：目标名已是符号链接的文件会被跳过并列入 `skipped_files`，转换生成的 Markdown 也不会写入同名符号链接
 - 线程隔离：每个线程的上传文件相互隔离，无法跨线程访问
 - 自动文档转换默认关闭；如需启用，需在 `config.yaml` 中显式设置 `uploads.auto_convert_documents: true`
 

backend/packages/harness/deerflow/client.py

@@ -21,7 +21,7 @@ import copy
 import logging
 import mimetypes
 import os
-import shutil
+import tempfile
 import uuid
 from collections.abc import Generator, Iterator, Mapping, Sequence
 from dataclasses import dataclass, field
@@ -69,7 +69,9 @@ from deerflow.tools.builtins.tool_search import assemble_deferred_tools, build_m
 from deerflow.trace_context import DEERFLOW_TRACE_METADATA_KEY, bind_trace_id, ensure_trace_id, generate_trace_id, get_current_trace_id, reset_trace_id
 from deerflow.tracing import build_tracing_callbacks, inject_langfuse_metadata
 from deerflow.uploads.manager import (
+    UnsafeUploadPathError,
     claim_unique_filename,
+    copy_upload_file_no_symlink,
     delete_file_safe,
     enrich_file_listing,
     ensure_uploads_dir,
@@ -1603,13 +1605,19 @@ class DeerFlowClient:
 
         For PDF, PPT, Excel, and Word files, they are also converted to Markdown.
 
+        The uploads directory is writable from inside the sandbox, so neither
+        the upload nor its Markdown companion is ever written through an
+        existing symlink. As in the Gateway, a file whose destination name is
+        a symlink or other non-regular file is skipped and listed in
+        ``skipped_files``; a companion with such a name is left out.
+
         Args:
             thread_id: Target thread ID.
             files: List of local file paths to upload.
 
         Returns:
-            Dict with success, files, message — matching the Gateway API
-            ``UploadResponse`` schema.
+            Dict with success, files, message, skipped_files — matching the
+            Gateway API ``UploadResponse`` schema.
 
         Raises:
             FileNotFoundError: If any file does not exist.
@@ -1635,6 +1643,7 @@ class DeerFlowClient:
 
         uploads_dir = ensure_uploads_dir(thread_id)
         uploaded_files: list[dict] = []
+        skipped_files: list[str] = []
 
         conversion_pool = None
         if has_convertible_file:
@@ -1654,8 +1663,12 @@ class DeerFlowClient:
 
         try:
             for src_path, dest_name in resolved_files:
-                dest = uploads_dir / dest_name
-                shutil.copy2(src_path, dest)
+                try:
+                    dest = copy_upload_file_no_symlink(uploads_dir, dest_name, src_path)
+                except UnsafeUploadPathError:
+                    logger.warning("Skipping upload with unsafe destination: %s", dest_name)
+                    skipped_files.append(dest_name)
+                    continue
 
                 info: dict[str, Any] = {
                     "filename": dest_name,
@@ -1673,12 +1686,24 @@ class DeerFlowClient:
                     # cannot silently overwrite each other.
                     provisional_md_name = Path(dest_name).with_suffix(".md").name
                     unique_md_name = claim_unique_filename(provisional_md_name, seen_names)
-                    md_output = dest.with_name(unique_md_name)
                     try:
-                        if conversion_pool is not None:
-                            md_path = conversion_pool.submit(_convert_in_thread, dest, md_output).result()
-                        else:
-                            md_path = asyncio.run(convert_file_to_markdown(dest, output_path=md_output))
+                        # Convert outside the sandbox-writable uploads dir, then
+                        # publish without following a symlink at the companion name.
+                        with tempfile.TemporaryDirectory() as md_dir:
+                            md_output = Path(md_dir) / unique_md_name
+                            if conversion_pool is not None:
+                                converted = conversion_pool.submit(_convert_in_thread, dest, md_output).result()
+                            else:
+                                converted = asyncio.run(convert_file_to_markdown(dest, output_path=md_output))
+                            md_path = None
+                            if converted is not None:
+                                # copy, not write_bytes: the companion keeps the
+                                # converter's permissions, so a sandbox running as
+                                # another uid can still read it.
+                                md_path = copy_upload_file_no_symlink(uploads_dir, unique_md_name, converted)
+                    except UnsafeUploadPathError:
+                        logger.warning("Skipping markdown companion with unsafe destination: %s", unique_md_name)
+                        md_path = None
                     except Exception:
                         logger.warning(
                             "Failed to convert %s to markdown",
@@ -1693,9 +1718,9 @@ class DeerFlowClient:
                         info["markdown_virtual_path"] = upload_virtual_path(md_path.name)
                         info["markdown_artifact_url"] = upload_artifact_url(thread_id, md_path.name)
                     else:
-                        # Conversion failed and wrote nothing, so release the
-                        # claim; holding it would rename a later same-stem
-                        # upload against a name nothing occupies.
+                        # No companion was written, so release the claim;
+                        # holding it would rename a later same-stem upload
+                        # against a name this request never filled.
                         seen_names.discard(unique_md_name)
 
                 uploaded_files.append(info)
@@ -1703,10 +1728,15 @@ class DeerFlowClient:
             if conversion_pool is not None:
                 conversion_pool.shutdown(wait=True)
 
+        message = f"Successfully uploaded {len(uploaded_files)} file(s)"
+        if skipped_files:
+            message += f"; skipped {len(skipped_files)} unsafe file(s)"
+
         return {
-            "success": True,
+            "success": not skipped_files,
             "files": uploaded_files,
-            "message": f"Successfully uploaded {len(uploaded_files)} file(s)",
+            "message": message,
+            "skipped_files": skipped_files,
         }
 
     def list_uploads(self, thread_id: str) -> dict:

backend/packages/harness/deerflow/uploads/manager.py

@@ -7,6 +7,7 @@ Both Gateway and Client delegate to these functions.
 import errno
 import logging
 import os
+import shutil
 import stat
 from pathlib import Path
 from urllib.parse import quote
@@ -284,6 +285,54 @@ def write_upload_file_no_symlink(base_dir: Path, filename: str, data: bytes) ->
     return dest
 
 
+def _reject_same_file(base_dir: Path, filename: str, src: Path, src_stat: os.stat_result) -> None:
+    """Raise :class:`shutil.SameFileError` when *filename* already is *src*.
+
+    Compares identity with ``os.path.samestat`` — what ``copy2`` itself uses —
+    rather than the path text, so a hardlink or a differently spelled path to
+    the same file is caught too.
+    ``lstat`` keeps a planted symlink from being resolved here; the open
+    itself rejects that destination.
+    """
+    dest = base_dir / normalize_filename(filename)
+    try:
+        dest_stat = os.lstat(dest)
+    except (FileNotFoundError, NotADirectoryError):
+        return
+    if os.path.samestat(src_stat, dest_stat):
+        raise shutil.SameFileError(f"{src!r} and {dest!r} are the same file")
+
+
+def copy_upload_file_no_symlink(base_dir: Path, filename: str, src: Path) -> Path:
+    """Copy *src* into an upload destination without following a destination symlink.
+
+    Matches ``shutil.copy2`` for content, permission bits and timestamps, but
+    opens the destination through :func:`open_upload_file_no_symlink` and
+    applies the metadata to that descriptor, never to the name. The source is
+    opened first, so a missing source leaves an existing destination intact.
+    Where descriptor-based ``chmod``/``utime`` are unavailable (Windows), the
+    destination keeps its default mode and the copy time.
+
+    Copying a file onto itself raises :class:`shutil.SameFileError` as
+    ``copy2`` does, and does so before the destination is opened: opening it
+    truncates, which would otherwise leave the caller copying an emptied file
+    over itself. Re-uploading a file that already sits in the uploads
+    directory takes exactly that path.
+    """
+    with open(src, "rb") as src_fh:
+        src_stat = os.fstat(src_fh.fileno())
+        _reject_same_file(base_dir, filename, src, src_stat)
+        dest, fh = open_upload_file_no_symlink(base_dir, filename)
+        with fh:
+            shutil.copyfileobj(src_fh, fh)
+            fh.flush()
+            if os.chmod in os.supports_fd:
+                os.chmod(fh.fileno(), stat.S_IMODE(src_stat.st_mode))
+            if os.utime in os.supports_fd:
+                os.utime(fh.fileno(), ns=(src_stat.st_atime_ns, src_stat.st_mtime_ns))
+    return dest
+
+
 def list_files_in_dir(directory: Path) -> dict:
     """List files (not directories) in *directory*.
 

backend/tests/test_client.py

@@ -3,6 +3,9 @@
 import asyncio
 import concurrent.futures
 import json
+import os
+import shutil
+import stat
 import tempfile
 import zipfile
 from enum import Enum
@@ -2709,6 +2712,51 @@ class TestUploads:
             assert (uploads_dir / "a.md").read_text(encoding="utf-8") == "FROM:a.pdf"
             assert not (uploads_dir / "a_1.md").exists()
 
+    def test_upload_files_rejects_reuploading_a_file_already_in_the_thread(self, client):
+        """Uploading an existing upload onto itself must not destroy its bytes."""
+        with tempfile.TemporaryDirectory() as tmp:
+            uploads_dir = Path(tmp) / "uploads"
+            uploads_dir.mkdir()
+            existing = uploads_dir / "existing.txt"
+            existing.write_text("IMPORTANT BYTES")
+
+            with patch("deerflow.client.get_uploads_dir", return_value=uploads_dir), patch("deerflow.client.ensure_uploads_dir", return_value=uploads_dir):
+                with pytest.raises(shutil.SameFileError):
+                    client.upload_files("thread-1", [existing])
+
+            assert existing.read_text() == "IMPORTANT BYTES"
+
+    def test_upload_files_markdown_companion_keeps_converted_permissions(self, client):
+        """The companion stays as readable as the converter wrote it (sandbox reads it)."""
+        if os.chmod not in os.supports_fd:
+            pytest.skip("descriptor-based chmod is unavailable on this platform")
+
+        with tempfile.TemporaryDirectory() as tmp:
+            tmp_path = Path(tmp)
+            uploads_dir = tmp_path / "uploads"
+            uploads_dir.mkdir()
+            pdf = tmp_path / "report.pdf"
+            pdf.write_bytes(b"PDF")
+
+            async def fake_convert(path: Path, output_path: Path | None = None) -> Path:
+                md_path = output_path if output_path is not None else path.with_suffix(".md")
+                md_path.write_text("converted", encoding="utf-8")
+                os.chmod(md_path, 0o644)
+                return md_path
+
+            with (
+                patch("deerflow.client.get_uploads_dir", return_value=uploads_dir),
+                patch("deerflow.client.ensure_uploads_dir", return_value=uploads_dir),
+                patch("deerflow.utils.file_conversion.CONVERTIBLE_EXTENSIONS", {".pdf"}),
+                patch("deerflow.utils.file_conversion.convert_file_to_markdown", side_effect=fake_convert),
+            ):
+                result = client.upload_files("thread-1", [pdf])
+
+            assert result["files"][0]["markdown_file"] == "report.md"
+            companion = uploads_dir / "report.md"
+            assert companion.read_text(encoding="utf-8") == "converted"
+            assert stat.S_IMODE(companion.stat().st_mode) == 0o644
+
     def test_list_uploads(self, client):
         with tempfile.TemporaryDirectory() as tmp:
             uploads_dir = Path(tmp)
@@ -4365,6 +4413,80 @@ class TestUploadDeleteSymlink:
             assert victim.read_text() == "keep me"
             assert link.is_symlink()
 
+    def test_upload_files_skips_symlinked_destination(self, client):
+        """A symlink planted at an upload name is skipped, not written through."""
+        with tempfile.TemporaryDirectory() as tmp:
+            tmp_path = Path(tmp)
+            uploads_dir = tmp_path / "uploads"
+            uploads_dir.mkdir()
+
+            outside = tmp_path / "outside.txt"
+            outside.write_text("original")
+            link = uploads_dir / "note.txt"
+            try:
+                link.symlink_to(outside)
+            except OSError as exc:
+                if getattr(exc, "winerror", None) == 1314:
+                    pytest.skip("symlink creation requires Developer Mode or elevated privileges on Windows")
+                raise
+
+            src_dir = tmp_path / "src"
+            src_dir.mkdir()
+            (src_dir / "note.txt").write_text("uploaded")
+            (src_dir / "other.txt").write_text("other")
+
+            with patch("deerflow.client.get_uploads_dir", return_value=uploads_dir), patch("deerflow.client.ensure_uploads_dir", return_value=uploads_dir):
+                result = client.upload_files("thread-1", [src_dir / "note.txt", src_dir / "other.txt"])
+
+            parsed = UploadResponse(**result)
+            assert parsed.success is False
+            assert parsed.skipped_files == ["note.txt"]
+            assert [f.filename for f in parsed.files] == ["other.txt"]
+            assert parsed.message == "Successfully uploaded 1 file(s); skipped 1 unsafe file(s)"
+            assert outside.read_text() == "original"
+            assert link.is_symlink()
+            assert (uploads_dir / "other.txt").read_text() == "other"
+
+    def test_upload_files_does_not_write_markdown_companion_through_symlink(self, client):
+        """A symlink planted at the companion name does not receive converted text."""
+        with tempfile.TemporaryDirectory() as tmp:
+            tmp_path = Path(tmp)
+            uploads_dir = tmp_path / "uploads"
+            uploads_dir.mkdir()
+
+            outside = tmp_path / "outside.md"
+            outside.write_text("original")
+            link = uploads_dir / "report.md"
+            try:
+                link.symlink_to(outside)
+            except OSError as exc:
+                if getattr(exc, "winerror", None) == 1314:
+                    pytest.skip("symlink creation requires Developer Mode or elevated privileges on Windows")
+                raise
+
+            pdf = tmp_path / "report.pdf"
+            pdf.write_bytes(b"PDF")
+
+            async def fake_convert(path: Path, output_path: Path | None = None) -> Path:
+                md_path = output_path if output_path is not None else path.with_suffix(".md")
+                md_path.write_text(f"FROM:{path.name}", encoding="utf-8")
+                return md_path
+
+            with (
+                patch("deerflow.client.get_uploads_dir", return_value=uploads_dir),
+                patch("deerflow.client.ensure_uploads_dir", return_value=uploads_dir),
+                patch("deerflow.utils.file_conversion.CONVERTIBLE_EXTENSIONS", {".pdf"}),
+                patch("deerflow.utils.file_conversion.convert_file_to_markdown", side_effect=fake_convert),
+            ):
+                result = client.upload_files("thread-1", [pdf])
+
+            assert result["success"] is True
+            assert [f["filename"] for f in result["files"]] == ["report.pdf"]
+            assert "markdown_file" not in result["files"][0]
+            assert outside.read_text() == "original"
+            assert link.is_symlink()
+            assert (uploads_dir / "report.pdf").read_bytes() == b"PDF"
+
     def test_upload_filename_with_spaces_and_unicode(self, client):
         """Files with spaces and unicode characters in names upload correctly."""
         with tempfile.TemporaryDirectory() as tmp:

backend/tests/test_uploads_manager.py

@@ -2,6 +2,8 @@
 
 import errno
 import os
+import shutil
+import stat
 from unittest.mock import patch
 
 import pytest
@@ -11,6 +13,7 @@ from deerflow.uploads.manager import (
     UnsafeUploadPathError,
     claim_unique_filename,
     cleanup_stale_upload_staging_files,
+    copy_upload_file_no_symlink,
     delete_file_safe,
     list_files_in_dir,
     normalize_filename,
@@ -191,6 +194,100 @@ class TestWriteUploadFileNoSymlink:
         assert not (tmp_path / "pipe.txt").exists()
 
 
+# ---------------------------------------------------------------------------
+# copy_upload_file_no_symlink
+# ---------------------------------------------------------------------------
+
+
+class TestCopyUploadFileNoSymlink:
+    def test_copies_content_mode_and_timestamps(self, tmp_path):
+        uploads = tmp_path / "uploads"
+        uploads.mkdir()
+        src = tmp_path / "notes.txt"
+        src.write_bytes(b"hello")
+        os.chmod(src, 0o640)
+        os.utime(src, ns=(1_700_000_000_000_000_000, 1_700_000_000_000_000_000))
+
+        dest = copy_upload_file_no_symlink(uploads, "notes.txt", src)
+
+        assert dest == uploads / "notes.txt"
+        assert dest.read_bytes() == b"hello"
+        if os.chmod in os.supports_fd:
+            assert stat.S_IMODE(os.stat(dest).st_mode) == 0o640
+        if os.utime in os.supports_fd:
+            assert os.stat(dest).st_mtime_ns == 1_700_000_000_000_000_000
+
+    def test_overwrites_existing_regular_file(self, tmp_path):
+        uploads = tmp_path / "uploads"
+        uploads.mkdir()
+        (uploads / "notes.txt").write_bytes(b"old contents")
+        src = tmp_path / "notes.txt"
+        src.write_bytes(b"new contents")
+
+        dest = copy_upload_file_no_symlink(uploads, "notes.txt", src)
+
+        assert dest.read_bytes() == b"new contents"
+
+    def test_rejects_symlink_destination(self, tmp_path):
+        uploads = tmp_path / "uploads"
+        uploads.mkdir()
+        outside = tmp_path / "outside.txt"
+        outside.write_bytes(b"original")
+        link = uploads / "notes.txt"
+        try:
+            link.symlink_to(outside)
+        except OSError as exc:
+            if getattr(exc, "winerror", None) == 1314:
+                pytest.skip("symlink creation requires Developer Mode or elevated privileges on Windows")
+            raise
+        src = tmp_path / "notes.txt"
+        src.write_bytes(b"attacker-chosen target")
+
+        with pytest.raises(UnsafeUploadPathError):
+            copy_upload_file_no_symlink(uploads, "notes.txt", src)
+
+        assert outside.read_bytes() == b"original"
+        assert link.is_symlink()
+
+    def test_rejects_copying_a_file_onto_itself(self, tmp_path):
+        uploads = tmp_path / "uploads"
+        uploads.mkdir()
+        src = uploads / "notes.txt"
+        src.write_bytes(b"IMPORTANT")
+
+        with pytest.raises(shutil.SameFileError):
+            copy_upload_file_no_symlink(uploads, "notes.txt", src)
+
+        assert src.read_bytes() == b"IMPORTANT"
+
+    def test_rejects_a_hardlink_to_the_destination(self, tmp_path):
+        """Identity, not path text: another name for the same inode is the same file."""
+        uploads = tmp_path / "uploads"
+        uploads.mkdir()
+        dest = uploads / "notes.txt"
+        dest.write_bytes(b"IMPORTANT")
+        src = uploads / "same-inode.txt"
+        try:
+            os.link(dest, src)
+        except (OSError, NotImplementedError) as exc:
+            pytest.skip(f"hardlinks unavailable on this platform: {exc}")
+
+        with pytest.raises(shutil.SameFileError):
+            copy_upload_file_no_symlink(uploads, "notes.txt", src)
+
+        assert dest.read_bytes() == b"IMPORTANT"
+
+    def test_missing_source_leaves_existing_destination_untouched(self, tmp_path):
+        uploads = tmp_path / "uploads"
+        uploads.mkdir()
+        (uploads / "notes.txt").write_bytes(b"keep me")
+
+        with pytest.raises(FileNotFoundError):
+            copy_upload_file_no_symlink(uploads, "notes.txt", tmp_path / "missing.txt")
+
+        assert (uploads / "notes.txt").read_bytes() == b"keep me"
+
+
 # ---------------------------------------------------------------------------
 # list_files_in_dir
 # ---------------------------------------------------------------------------