diff --git a/CHANGELOG.md b/CHANGELOG.md index 054455a4f..e96cf82c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2772,6 +2772,13 @@ This release closes that milestone with **765 merged pull requests**. ### Security +- **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 + `DeerFlowClient.delete_upload`) remove the upload it pointed to, plus that + file's companion `.md`, while reporting the requested name as deleted. + Symlinks now return 404, matching the upload listing; links that leave the + uploads directory are still rejected with 400. ([#5547]) - **frontend:** Tool steps no longer turn non-web URLs into links. The `web_fetch` URL and `web_search` / `image_search` result links in the chain-of-thought panel skipped the scheme allowlist that markdown links use, @@ -4292,3 +4299,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#5505]: https://github.com/bytedance/deer-flow/pull/5505 [#5524]: https://github.com/bytedance/deer-flow/pull/5524 [#5526]: https://github.com/bytedance/deer-flow/pull/5526 +[#5547]: https://github.com/bytedance/deer-flow/pull/5547 diff --git a/CHANGELOG_zh.md b/CHANGELOG_zh.md index 89ad96a20..f0e348891 100644 --- a/CHANGELOG_zh.md +++ b/CHANGELOG_zh.md @@ -2085,6 +2085,11 @@ ### 安全 +- **上传:** 删除上传文件时不再跟随符号链接删除另一个文件。沙箱可写的 uploads 目录中若被 + 放置符号链接,`DELETE /api/threads/{id}/uploads/{filename}`(以及 + `DeerFlowClient.delete_upload`)此前会删除链接指向的上传文件及其配套 `.md`,却仍报告 + 删除的是请求的文件名。现在符号链接返回 404,与上传列表一致;指向 uploads 目录之外的 + 链接仍以 400 拒绝。([#5547]) - **前端:** 工具步骤不再把非 Web URL 渲染为链接。思维链面板中的 `web_fetch` URL 与 `web_search` / `image_search` 结果链接此前绕过了 Markdown 链接使用的协议白名单, 被提示注入的工具调用可在聊天中放入 `file:` 或系统协议处理程序链接(`ms-msdt:`、 @@ -3499,3 +3504,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写,核心包含子 [#5505]: https://github.com/bytedance/deer-flow/pull/5505 [#5524]: https://github.com/bytedance/deer-flow/pull/5524 [#5526]: https://github.com/bytedance/deer-flow/pull/5526 +[#5547]: https://github.com/bytedance/deer-flow/pull/5547 diff --git a/backend/app/gateway/AGENTS.md b/backend/app/gateway/AGENTS.md index f49511fbd..baf23aa55 100644 --- a/backend/app/gateway/AGENTS.md +++ b/backend/app/gateway/AGENTS.md @@ -89,7 +89,7 @@ owner-scoped assistant version selection remains enabled. | **Subagents** (`/api/subagents`) | Admin managed-worker CRUD and listing. | | **Integrations** (`/api/integrations`) | `GET /lark/status` - inspect managed Lark/Feishu CLI integration state, including `sandbox_runtime_mode` / `sandbox_runtime_ready` (whether `lark-cli` will actually be present in the sandbox at chat time); `POST /lark/install` - admin-only install of the official `lark-*` managed skill pack; `POST /lark/config/start` and `/lark/config/complete` - internal first-time Lark connection setup; `POST /lark/config/credentials` - atomically switch the caller's per-user Lark app after validating the new `app_id`/`app_secret` through the official CLI's live tenant-token probe, revoke/remove the previous OAuth tokens, and restore the prior credential tree if the switch fails; `POST /lark/auth/start` and `/lark/auth/complete` - browser device-flow user authorization without terminal access, with optional `domains` / exact `scope` for incremental permission grants. Config and auth flows carry a server-issued, per-user generation persisted under the credential lock; a rejected direct switch leaves the current generation unchanged, stale completions return 409, and browser re-registration uses the same token-clearing/revocation transaction as direct credential switches. | | **Memory** (`/api/memory`) | `GET /` - memory data; `POST /reload` - force reload; `GET /config` - config; `GET /status` - config + data | -| **Uploads** (`/api/threads/{id}/uploads`) | `POST /` - upload files (auto-converts PDF/PPT/Excel/Word); non-mounted sandbox sync uses a non-releasing request lease; `GET /list` - list; `DELETE /{filename}` - delete | +| **Uploads** (`/api/threads/{id}/uploads`) | `POST /` - upload files (auto-converts PDF/PPT/Excel/Word); non-mounted sandbox sync uses a non-releasing request lease; `GET /list` - list; `DELETE /{filename}` - delete a regular file; a symlink is never followed and 404s like `GET /list` | | **Threads** (`/api/threads/{id}`) | `DELETE /` - remove DeerFlow-managed local thread data after LangGraph thread deletion; `POST /branches` - branch a completed assistant turn with a replay checkpoint; inherited titles take next-free displayed sibling suffixes, including explicit/renamed ones, while explicit titles stay unchanged. Durable `branch` admission rejects races. Workspace files are not checkpointed, so the branch only best-effort copies the current workspace when branching from the **latest** turn (`workspace_clone_mode="current_thread_best_effort"`); branching from an older/historical turn skips the copy (`workspace_clone_mode="skipped_historical_turn"`) so the branch never inherits files that only exist in a later timeline. Thread-scoped runtime channels (`sandbox`, `thread_data`) are not copied onto the branch: the parent's `sandbox_id` binds path mappings and the release lifecycle to the parent's workspace, so the branch lazily acquires its own sandbox instead. Branch creation also seeds the new thread's run-event feed from the branch checkpoint's visible messages (`history_seed_mode` in the response): the thread feed reads run_events, not checkpoints, so without the seed the inherited history disappears from the UI after the branch's first run (#4380). Seeded rows are grouped into one synthetic run per inherited turn (`branch-seed-{thread_id}-{n}`, a new turn opening at every persisted human message, including an allowlisted hidden `ask_clarification` reply) because `run_id` is a turn identity to the feed's consumers, not a provenance tag: regenerating an inherited answer supersedes that row's whole `run_id` in `GET /messages/page`, so one shared id for the entire seed deleted the complete inherited history on a branch's first regenerate (#4458); `GET /goal`, `PUT /goal`, `DELETE /goal` - read, set, and clear the active thread goal; `POST /compact` - summarize older active context, deriving memory policy and bucket from the state-producing checkpoint rather than request `agent_name`, and block while a run is in flight; unexpected failures return a generic 500 detail | | **Artifacts** (`/api/threads/{id}/artifacts`) | `GET /{path}` - stream regular text and binary artifacts with `FileResponse`, including byte-`Range` 206/416 behavior used by bounded text previews and media seeking; active content (`text/html`, `text/xml`, `application/xml`, `text/xsl`, any `+xml` type such as XHTML/SVG; `.skill` members too) is always forced as a download attachment to reduce XSS risk; `?download=true` still forces download for other file types. `PUT /{path}` atomically replaces an existing UTF-8 text file under `/mnt/user-data/outputs` when its expected SHA-256 still matches; active runs conflict, and non-mounted sandbox providers receive the same update under a request lease. The outputs-only rule is `path_utils.resolve_outputs_confined_path`, shared with IM-channel attachment delivery: it collapses `..` before the prefix check and re-checks the resolved host path against the resolved outputs root, since `resolve_thread_virtual_path` only confines to `user-data/`; a percent-encoded `..` or a symlink planted in `outputs/` must not reach a sibling `uploads/` file. Atomic replacement applies the existing POSIX permission handling when descriptor-based APIs are available and otherwise keeps the platform-native temporary-file permissions (Windows). | | **Suggestions** (`/api/suggestions`) | `GET /config` - returns global suggestions config boolean; `POST /threads/{id}/suggestions` - generate follow-up questions; rich list/block model content is normalized and inline reasoning (`...`, including unclosed/truncated blocks from reasoning models like MiniMax-M3) is stripped before JSON parsing | diff --git a/backend/docs/FILE_UPLOAD.md b/backend/docs/FILE_UPLOAD.md index dd79ca2f8..f709554cf 100644 --- a/backend/docs/FILE_UPLOAD.md +++ b/backend/docs/FILE_UPLOAD.md @@ -235,6 +235,7 @@ backend/.deer-flow/threads/ - 最大文件大小:100MB(可在 nginx.conf 中配置 `client_max_body_size`) - 文件名安全性:系统会自动验证文件路径,防止目录遍历攻击 +- 删除只作用于普通文件:上传目录中的符号链接不会被跟随,删除请求按文件不存在(404)处理 - 线程隔离:每个线程的上传文件相互隔离,无法跨线程访问 - 自动文档转换默认关闭;如需启用,需在 `config.yaml` 中显式设置 `uploads.auto_convert_documents: true` diff --git a/backend/packages/harness/deerflow/uploads/manager.py b/backend/packages/harness/deerflow/uploads/manager.py index 22a631cfb..db7d1f193 100644 --- a/backend/packages/harness/deerflow/uploads/manager.py +++ b/backend/packages/harness/deerflow/uploads/manager.py @@ -324,6 +324,11 @@ def delete_file_safe(base_dir: Path, filename: str, *, convertible_extensions: s If *convertible_extensions* is provided and the file's extension matches, the companion ``.md`` file is also removed (if it exists). + Only regular files are deleted. Upload directories may be mounted into + local sandboxes, so a sandbox process can plant a symlink under an upload + name; following it would delete the upload it aliases instead. Such + entries are reported as not found, matching ``list_files_in_dir``. + Args: base_dir: Directory containing the file. filename: Name of file to delete. @@ -337,10 +342,10 @@ def delete_file_safe(base_dir: Path, filename: str, *, convertible_extensions: s FileNotFoundError: If the file does not exist. PathTraversalError: If path traversal is detected. """ - file_path = (base_dir / filename).resolve() + file_path = base_dir / filename validate_path_traversal(file_path, base_dir) - if not file_path.is_file(): + if file_path.is_symlink() or not file_path.is_file(): raise FileNotFoundError(f"File not found: {filename}") file_path.unlink() diff --git a/backend/tests/test_client.py b/backend/tests/test_client.py index 38b9fb162..12ba05544 100644 --- a/backend/tests/test_client.py +++ b/backend/tests/test_client.py @@ -4341,6 +4341,30 @@ class TestUploadDeleteSymlink: # The outside file must NOT have been deleted. assert outside.exists() + def test_delete_upload_symlink_to_sibling_upload(self, client): + """A symlink aliasing another upload is not followed to delete that upload.""" + with tempfile.TemporaryDirectory() as tmp: + uploads_dir = Path(tmp) / "uploads" + uploads_dir.mkdir() + + victim = uploads_dir / "victim.txt" + victim.write_text("keep me") + + link = uploads_dir / "alias.txt" + try: + link.symlink_to(victim.name) + except OSError as exc: + if getattr(exc, "winerror", None) == 1314: + pytest.skip("symlink creation requires Developer Mode or elevated privileges on Windows") + raise + + with patch("deerflow.client.get_uploads_dir", return_value=uploads_dir): + with pytest.raises(FileNotFoundError): + client.delete_upload("thread-1", "alias.txt") + + assert victim.read_text() == "keep me" + assert link.is_symlink() + 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: diff --git a/backend/tests/test_uploads_manager.py b/backend/tests/test_uploads_manager.py index 0b5ec0eda..9bbc004eb 100644 --- a/backend/tests/test_uploads_manager.py +++ b/backend/tests/test_uploads_manager.py @@ -286,3 +286,24 @@ class TestDeleteFileSafe: def test_delete_traversal_raises(self, tmp_path): with pytest.raises(PathTraversalError, match="traversal"): delete_file_safe(tmp_path, "../outside.txt") + + def test_delete_symlink_to_sibling_upload_keeps_target(self, tmp_path): + """A symlink planted in the uploads dir must not delete the upload it aliases.""" + victim = tmp_path / "victim.pdf" + victim.write_bytes(b"pdf-bytes") + companion = tmp_path / "victim.md" + companion.write_text("converted", encoding="utf-8") + alias = tmp_path / "alias.pdf" + try: + alias.symlink_to(victim.name) + except OSError as exc: + if getattr(exc, "winerror", None) == 1314: + pytest.skip("symlink creation requires Developer Mode or elevated privileges on Windows") + raise + + with pytest.raises(FileNotFoundError): + delete_file_safe(tmp_path, "alias.pdf", convertible_extensions={".pdf"}) + + assert victim.read_bytes() == b"pdf-bytes" + assert companion.exists() + assert alias.is_symlink() diff --git a/backend/tests/test_uploads_router.py b/backend/tests/test_uploads_router.py index 37bcd0ec5..46252f818 100644 --- a/backend/tests/test_uploads_router.py +++ b/backend/tests/test_uploads_router.py @@ -949,6 +949,24 @@ def test_delete_uploaded_file_removes_generated_markdown_companion(tmp_path): assert not (thread_uploads_dir / "report.md").exists() +def test_delete_uploaded_file_rejects_symlink_to_sibling_upload(tmp_path): + thread_uploads_dir = tmp_path / "uploads" + thread_uploads_dir.mkdir(parents=True) + victim = thread_uploads_dir / "victim.pdf" + victim.write_bytes(b"pdf-bytes") + (thread_uploads_dir / "victim.md").write_text("converted", encoding="utf-8") + _symlink_to_or_skip(thread_uploads_dir / "alias.pdf", victim) + + with patch.object(uploads, "get_uploads_dir", return_value=thread_uploads_dir): + with pytest.raises(HTTPException) as exc_info: + asyncio.run(call_unwrapped(uploads.delete_uploaded_file, "thread-local", "alias.pdf", request=MagicMock())) + + assert exc_info.value.status_code == 404 + assert victim.read_bytes() == b"pdf-bytes" + assert (thread_uploads_dir / "victim.md").exists() + assert (thread_uploads_dir / "alias.pdf").is_symlink() + + def test_auto_convert_documents_enabled_defaults_to_false_on_config_errors(): class BrokenConfig: def __getattribute__(self, name):