* 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>
62b73fd2ea