mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-27 08:28:00 +00:00
62b73fd2ea
* feat(dingtalk): support inbound file and image attachments DingTalk previously dropped picture and file (document) messages because `_on_chatbot_message` ignored any message with empty text, so users could not send files to the agent. This adds inbound attachment support, mirroring `FeishuChannel`: - `_extract_files` parses `picture`/`richText` image downloadCodes and `file` (document) descriptors. `dingtalk_stream.ChatbotMessage.from_dict` does not parse `file` messages, so `_DingTalkMessageHandler.process` stashes the raw callback payload on the message (`_df_raw_data`) for the document descriptor. - `receive_file` downloads each attachment by `downloadCode` via the robot `messageFiles/download` OpenAPI, persists it into the thread uploads bucket, syncs it into a non-local sandbox, and prepends the sandbox virtual path to the message text so the agent can read the file by path. - Filenames go through the shared `uploads.normalize_filename` helper, which strips directory components and rejects traversal patterns. Outbound `send_file` already existed; this completes DingTalk file parity with Feishu on the inbound side. Adds 21 tests covering extraction, download-by-code, persistence/sandbox sync, filename sanitization, and the handler raw-data stash. * fix(dingtalk): address inbound-file review feedback Follow-up to the review on #4423: - Make the fallback filename safe by construction. `download_code` is attacker-controllable webhook data and was embedded into `fallback_name` unsanitized; it only avoided escaping the uploads directory because the resulting write failed with OSError. It is now restricted to `[A-Za-z0-9_-]` before use. Covered by a test that reproduces the old behaviour (`uploads/dingtalk_../../evil.png`) and by a test that actually exercises the previously untested `except ValueError` branch (`".."`, whose basename — unlike `../../etc/passwd` — does raise). - Log the swallowed `get_image_list()` failure instead of silently returning no images, so an SDK parse failure is distinguishable from a richText message that genuinely has no inline images. - Surface failed downloads to the agent as a short `[failed to load ...]` marker rather than silently omitting the attachment, so a user whose file did not load does not simply appear to be ignored. Keeps the cleaner text shape while restoring the signal Feishu provides. Tests: 119 passed (was 115). * fix(dingtalk): claim unique upload names and refuse symlinked destinations Round 2 review follow-up on #4423. Both findings reproduce as failing tests against the previous head. - Inbound attachments no longer overwrite each other. Generated names repeat across messages (every picture message yields "image.png", richText yields "image_0.png"), so a later attachment silently replaced an earlier one whose virtual path had already been prepended to the message text — the agent could read bytes that were not the ones its prompt referenced. The destination name is now claimed with the shared `claim_unique_filename` against the live directory contents, which also covers a real filename sent twice (`quote.xlsx`), a case Feishu's inline naming does not handle either. The claim and the write happen under one lock so two attachments cannot resolve to the same free name. - Writes go through the shared `write_upload_file_no_symlink` instead of `Path.write_bytes`. Uploads dirs may be mounted into local sandboxes, so a sandbox process could leave a symlink at a future upload name and redirect a gateway-privileged write outside the bucket; the regression test shows the old code creating the out-of-bucket target. Tests: 123 passed (was 119). * fix(dingtalk): harden the inbound download path (self-audit) Proactive hardening pass over the new inbound path; each fix reproduces as a failing test against the previous head. - Contain token failures. `_get_access_token()` sat outside the try in `_download_by_code`, and the manager awaits `receive_file` without one — a DingTalk auth hiccup during a file message aborted the whole chat turn with no reply. Token acquisition moves inside the try, and `receive_file` gains per-attachment isolation so no unforeseen error can escape past the marker. - Cap inbound size. The download buffered arbitrary bytes in memory (`response.content`) with no limit, while outbound uploads already enforce one. The body is now streamed and dropped once it exceeds `_MAX_INBOUND_FILE_SIZE_BYTES` (50 MB), surfacing as a failed-load marker. - Sanitize the failure marker. It embedded the raw webhook `fileName`; a newline could forge a standalone `/mnt/user-data/uploads/...` line inside msg.text and an over-long name bloated it. Markers now collapse whitespace and cap at 80 chars. - Keep blocking IO off the event loop. `ensure_thread_dirs`, the uploads-dir resolve, sync `SandboxProvider.acquire`, and `sandbox.update_file` all ran on the loop; directory prep now lives inside the same `asyncio.to_thread` as the claim+write, and sandbox sync uses `acquire_async` + an offloaded `update_file`. Locked by a strict Blockbuster anchor (tests/blocking_io/test_dingtalk_receive_file.py), verified to fail with `BlockingError: Blocking call to os.mkdir` when the offload is reverted. Tests: 127 + 1 blocking-io anchor (was 123); tests/blocking_io/ suite 55 passed. * fix(dingtalk): surface missing-sandbox sync as a failed load Round 3 follow-up on #4423: - When a non-local sandbox acquire succeeds but the provider cannot resolve the instance, _receive_single_file returned the virtual path anyway — a path the agent's sandbox cannot read. Mirror Feishu: log and return "", so the [failed to load ...] marker fires instead. Red-first test: test_missing_sandbox_after_acquire_yields_marker. - Drop the dead GetResponse / FakeClient.get scaffolding left in test_oversized_download_is_dropped from its red-first iteration. Tests: 128 + 1 blocking-io anchor (was 127 + 1). * fix(dingtalk): treat non-local sandbox sync failure as a failed load Round 4 follow-up on #4423. The sync except-branch logged and still returned the virtual path when acquire or update_file raised on a non-local sandbox — the same handing-the-agent-an-unreadable-path failure mode the sandbox-is-None branch was just fixed for, and exactly the leg the suite did not exercise. Feishu's except-branch returns its failure marker; DingTalk now does the equivalent (return "" so the failed-load marker fires). Red-first test: test_update_file_failure_yields_marker. Tests: 129 + 1 blocking-io anchor (was 128 + 1). --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>