diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e617bd55..7d60d919b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -941,6 +941,13 @@ This release closes that milestone with **765 merged pull requests**. ### Fixed +- **uploads:** Deleting an uploaded document no longer deletes the converted + Markdown beside it. Conversion names a companion after the document's stem + and falls back to a `_N` suffix when that name is taken, so the `.md` next to + a document can belong to another document sharing the stem, or to the user: + uploading `a.docx` and `a.pdf` produced `a.md` and `a_1.md`, and deleting + `a.pdf` destroyed `a.docx`'s companion. Companions now survive their + document, stay listed, and can be deleted on their own. ([#5673]) - **subagents:** Recognize zero-byte regular deliverables in remote sandbox acceptance checks. Readable empty files now satisfy `exists` and `file_written` and deterministically fail `non-empty`, instead of remaining @@ -4331,3 +4338,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#5547]: https://github.com/bytedance/deer-flow/pull/5547 [#5578]: https://github.com/bytedance/deer-flow/pull/5578 [#5611]: https://github.com/bytedance/deer-flow/pull/5611 +[#5673]: https://github.com/bytedance/deer-flow/pull/5673 diff --git a/CHANGELOG_zh.md b/CHANGELOG_zh.md index 4b2e4341a..d3adc69c9 100644 --- a/CHANGELOG_zh.md +++ b/CHANGELOG_zh.md @@ -727,6 +727,10 @@ ### 修复 +- **上传：** 删除已上传的文档时，不再连带删除其旁边转换生成的 Markdown。转换以文档主干名 + 命名配套文件，名称被占用时回退为 `_N` 后缀，因此文档旁的 `.md` 可能属于主干名相同的另一个 + 文档，或属于用户自己：上传 `a.docx` 与 `a.pdf` 会生成 `a.md` 与 `a_1.md`，删除 `a.pdf` + 却会销毁 `a.docx` 的配套文件。现在配套文件会保留、继续出现在列表中，可单独删除。([#5673]) - **nginx：** 把 600 秒读取超时扩展到其余两个会等待 Gateway 的 location，它们在线程路由的修复 之后仍沿用 nginx 默认的 60 秒。`/api/` 兜底 location 之后：无状态的 `POST /api/runs/wait` 阻塞在同一套运行完成等待上，并在客户端断开时取消该运行，因此等待超过 60 秒的 API 调用方会 @@ -3522,3 +3526,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写，核心包含子 [#5547]: https://github.com/bytedance/deer-flow/pull/5547 [#5578]: https://github.com/bytedance/deer-flow/pull/5578 [#5611]: https://github.com/bytedance/deer-flow/pull/5611 +[#5673]: https://github.com/bytedance/deer-flow/pull/5673 diff --git a/backend/app/gateway/AGENTS.md b/backend/app/gateway/AGENTS.md index 4d50d5582..e184ff3dc 100644 --- a/backend/app/gateway/AGENTS.md +++ b/backend/app/gateway/AGENTS.md @@ -102,7 +102,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 a regular file; a symlink 404s like `GET /list` | +| **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 404s like `GET /list`; a converted `.md` is kept (#5672) | | **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/app/gateway/routers/uploads.py b/backend/app/gateway/routers/uploads.py index f6290dd4d..20211ca12 100644 --- a/backend/app/gateway/routers/uploads.py +++ b/backend/app/gateway/routers/uploads.py @@ -50,6 +50,7 @@ router = APIRouter(prefix="/api/threads/{thread_id}/uploads", tags=["uploads"]) # project-shelf attach route. They are re-exported deliberately — do not # prune them as "unused". __all__ = [ + "CONVERTIBLE_EXTENSIONS", "UnsafeUploadPathError", "claim_unique_filename", "convert_file_to_markdown", @@ -329,7 +330,7 @@ def _list_uploaded_files_for_thread(thread_id: str, user_id: str) -> dict: def _delete_uploaded_file_for_thread(thread_id: str, filename: str, user_id: str) -> dict: uploads_dir = get_uploads_dir(thread_id, user_id=user_id) - return delete_file_safe(uploads_dir, filename, convertible_extensions=CONVERTIBLE_EXTENSIONS) + return delete_file_safe(uploads_dir, filename) async def _stream_upload_file(file: UploadFile) -> AsyncIterator[bytes]: diff --git a/backend/docs/FILE_UPLOAD.md b/backend/docs/FILE_UPLOAD.md index d506cd6ec..2e878c0d0 100644 --- a/backend/docs/FILE_UPLOAD.md +++ b/backend/docs/FILE_UPLOAD.md @@ -236,6 +236,7 @@ backend/.deer-flow/threads/ - 最大文件大小：100MB（可在 nginx.conf 中配置 `client_max_body_size`） - 文件名安全性：系统会自动验证文件路径，防止目录遍历攻击 - 删除只作用于普通文件：上传目录中的符号链接不会被跟随，删除请求按文件不存在（404）处理 +- 删除文档不会一并删除其转换生成的 Markdown：该 `.md` 的归属无法从文件名确定（同主干名的另一个文档或用户自己上传的文件都可能占用该名称），因此不再依据推测删除。它仍会出现在上传列表中，可单独删除（见 issue #5672） - 上传（HTTP 与嵌入式 `DeerFlowClient`）不会写穿符号链接：目标名已是符号链接的文件会被跳过并列入 `skipped_files`，转换生成的 Markdown 也不会写入同名符号链接 - 转换读取的是本次上传写入的字节，而非落盘后的文件名：HTTP 上传在 uploads 之外的私有副本上转换，嵌入式客户端转换调用方提供的源文件，因此沙箱替换该文件名无法让宿主文件内容被转换进 uploads - 线程隔离：每个线程的上传文件相互隔离，无法跨线程访问 diff --git a/backend/packages/harness/deerflow/client.py b/backend/packages/harness/deerflow/client.py index 0da26a315..f4af2c220 100644 --- a/backend/packages/harness/deerflow/client.py +++ b/backend/packages/harness/deerflow/client.py @@ -1778,10 +1778,9 @@ class DeerFlowClient: PermissionError: If path traversal is detected. """ validate_thread_id(thread_id) - from deerflow.utils.file_conversion import CONVERTIBLE_EXTENSIONS uploads_dir = get_uploads_dir(thread_id) - return delete_file_safe(uploads_dir, filename, convertible_extensions=CONVERTIBLE_EXTENSIONS) + return delete_file_safe(uploads_dir, filename) # ------------------------------------------------------------------ # Public API — artifacts diff --git a/backend/packages/harness/deerflow/uploads/manager.py b/backend/packages/harness/deerflow/uploads/manager.py index b8d3bd7c9..ac8fad171 100644 --- a/backend/packages/harness/deerflow/uploads/manager.py +++ b/backend/packages/harness/deerflow/uploads/manager.py @@ -367,11 +367,15 @@ def list_files_in_dir(directory: Path) -> dict: return {"files": files, "count": len(files)} -def delete_file_safe(base_dir: Path, filename: str, *, convertible_extensions: set[str] | None = None) -> dict: +def delete_file_safe(base_dir: Path, filename: str) -> dict: """Delete a file inside *base_dir* after path-traversal validation. - If *convertible_extensions* is provided and the file's extension matches, - the companion ``.md`` file is also removed (if it exists). + Only the requested file is removed. A converted document's Markdown + companion is left in place: conversion names it after the document's stem + and falls back to a ``_N`` suffix when that name is taken, so the ``.md`` + beside a document may belong to another document sharing that stem, or to + the user. Removing it on that guess destroyed the wrong file. It stays + listed and can be deleted on its own (issue #5672). Only regular files are deleted. Upload directories may be mounted into local sandboxes, so a sandbox process can plant a symlink under an upload @@ -381,8 +385,6 @@ def delete_file_safe(base_dir: Path, filename: str, *, convertible_extensions: s Args: base_dir: Directory containing the file. filename: Name of file to delete. - convertible_extensions: Lowercase extensions (e.g. ``{".pdf", ".docx"}``) - whose companion markdown should be cleaned up. Returns: Dict with success and message. @@ -399,10 +401,6 @@ def delete_file_safe(base_dir: Path, filename: str, *, convertible_extensions: s file_path.unlink() - # Clean up companion markdown generated during upload conversion. - if convertible_extensions and file_path.suffix.lower() in convertible_extensions: - file_path.with_suffix(".md").unlink(missing_ok=True) - return {"success": True, "message": f"Deleted {filename}"} diff --git a/backend/tests/test_client.py b/backend/tests/test_client.py index 3982bc15a..62a409f1f 100644 --- a/backend/tests/test_client.py +++ b/backend/tests/test_client.py @@ -2959,6 +2959,21 @@ class TestUploads: assert "delete-me.txt" in result["message"] assert not (uploads_dir / "delete-me.txt").exists() + def test_delete_upload_keeps_the_converted_markdown(self, client): + """A .md sharing the document's stem may belong to another document.""" + with tempfile.TemporaryDirectory() as tmp: + uploads_dir = Path(tmp) + (uploads_dir / "report.docx").write_bytes(b"docx-bytes") + (uploads_dir / "report.md").write_text("converted from the docx", encoding="utf-8") + (uploads_dir / "report.pdf").write_bytes(b"pdf-bytes") + + with patch("deerflow.client.get_uploads_dir", return_value=uploads_dir): + result = client.delete_upload("thread-1", "report.pdf") + + assert result["success"] is True + assert not (uploads_dir / "report.pdf").exists() + assert (uploads_dir / "report.md").read_text(encoding="utf-8") == "converted from the docx" + def test_delete_upload_not_found(self, client): with tempfile.TemporaryDirectory() as tmp: with patch("deerflow.client.get_uploads_dir", return_value=Path(tmp)): diff --git a/backend/tests/test_uploads_manager.py b/backend/tests/test_uploads_manager.py index 2f38405f6..d92500eb5 100644 --- a/backend/tests/test_uploads_manager.py +++ b/backend/tests/test_uploads_manager.py @@ -384,6 +384,21 @@ class TestDeleteFileSafe: with pytest.raises(PathTraversalError, match="traversal"): delete_file_safe(tmp_path, "../outside.txt") + def test_delete_keeps_the_converted_markdown(self, tmp_path): + """Companion ownership cannot be proven from the name, so nothing is guessed at.""" + (tmp_path / "a.docx").write_bytes(b"DOCX") + (tmp_path / "a.md").write_text("converted from the docx", encoding="utf-8") + (tmp_path / "a.pdf").write_bytes(b"PDF") + (tmp_path / "a_1.md").write_text("converted from the pdf", encoding="utf-8") + + result = delete_file_safe(tmp_path, "a.pdf") + + assert result["success"] is True + assert not (tmp_path / "a.pdf").exists() + # a.md belongs to a.docx; deleting a.pdf used to remove it. + assert (tmp_path / "a.md").read_text(encoding="utf-8") == "converted from the docx" + assert (tmp_path / "a_1.md").read_text(encoding="utf-8") == "converted from the pdf" + 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" @@ -399,7 +414,7 @@ class TestDeleteFileSafe: raise with pytest.raises(FileNotFoundError): - delete_file_safe(tmp_path, "alias.pdf", convertible_extensions={".pdf"}) + delete_file_safe(tmp_path, "alias.pdf") assert victim.read_bytes() == b"pdf-bytes" assert companion.exists() diff --git a/backend/tests/test_uploads_router.py b/backend/tests/test_uploads_router.py index 28b0431c3..0bc1fbf48 100644 --- a/backend/tests/test_uploads_router.py +++ b/backend/tests/test_uploads_router.py @@ -1246,18 +1246,23 @@ def test_upload_files_closes_conversion_descriptor_when_cancelled_while_copy_is_ assert duplicated[0] in closed, "the descriptor handed to the queued copy was never closed" -def test_delete_uploaded_file_removes_generated_markdown_companion(tmp_path): +def test_delete_uploaded_file_keeps_the_converted_markdown(tmp_path): + """The .md next to a document may belong to another document, or to the user.""" thread_uploads_dir = tmp_path / "uploads" thread_uploads_dir.mkdir(parents=True) + (thread_uploads_dir / "report.docx").write_bytes(b"docx-bytes") + (thread_uploads_dir / "report.md").write_text("converted from the docx", encoding="utf-8") (thread_uploads_dir / "report.pdf").write_bytes(b"pdf-bytes") - (thread_uploads_dir / "report.md").write_text("converted", encoding="utf-8") + (thread_uploads_dir / "report_1.md").write_text("converted from the pdf", encoding="utf-8") with patch.object(uploads, "get_uploads_dir", return_value=thread_uploads_dir): result = asyncio.run(call_unwrapped(uploads.delete_uploaded_file, "thread-aio", "report.pdf", request=MagicMock())) assert result == {"success": True, "message": "Deleted report.pdf"} assert not (thread_uploads_dir / "report.pdf").exists() - assert not (thread_uploads_dir / "report.md").exists() + # report.md belongs to report.docx; deleting report.pdf used to remove it. + assert (thread_uploads_dir / "report.md").read_text(encoding="utf-8") == "converted from the docx" + assert (thread_uploads_dir / "report_1.md").read_text(encoding="utf-8") == "converted from the pdf" def test_delete_uploaded_file_rejects_symlink_to_sibling_upload(tmp_path):