From 2bb22643ada840f4369e80ec0f1a3db6a5aed4b6 Mon Sep 17 00:00:00 2001 From: Daoyuan Li <94409450+DaoyuanLi2816@users.noreply.github.com> Date: Tue, 21 Jul 2026 03:37:10 -0700 Subject: [PATCH] fix(feishu): check response.success() on send_file's reply/create calls (#4335) willem-bd's review on PR #4234 (Feishu card response.success() checks) flagged send_file (around lines 313-318) as having the same unchecked- response pattern: after _upload_image/_upload_file (which already raise on a business-level failure), the resulting file/image message.reply or message.create call's response was never checked, so a failed send logged "file sent" and returned True exactly like a real success. Adds the same response.success() check used by _reply_card/_create_card/ _update_card, raising (caught by the existing try/except, which already logs and returns False) so the caller can no longer mistake a silent business-level failure for a delivered file/image. --- backend/app/channels/feishu.py | 6 +- backend/tests/test_channels.py | 144 +++++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 2 deletions(-) diff --git a/backend/app/channels/feishu.py b/backend/app/channels/feishu.py index 1d1906388..82c35c6b3 100644 --- a/backend/app/channels/feishu.py +++ b/backend/app/channels/feishu.py @@ -312,10 +312,12 @@ class FeishuChannel(Channel): if msg.thread_ts: request = self._ReplyMessageRequest.builder().message_id(msg.thread_ts).request_body(self._ReplyMessageRequestBody.builder().msg_type(msg_type).content(content).build()).build() - await asyncio.to_thread(self._api_client.im.v1.message.reply, request) + response = await asyncio.to_thread(self._api_client.im.v1.message.reply, request) else: request = self._CreateMessageRequest.builder().receive_id_type("chat_id").request_body(self._CreateMessageRequestBody.builder().receive_id(msg.chat_id).msg_type(msg_type).content(content).build()).build() - await asyncio.to_thread(self._api_client.im.v1.message.create, request) + response = await asyncio.to_thread(self._api_client.im.v1.message.create, request) + if not response.success(): + raise RuntimeError(f"Feishu file send failed: code={response.code}, msg={response.msg}, log_id={response.get_log_id()}") logger.info("[Feishu] file sent: %s (type=%s)", attachment.filename, msg_type) return True diff --git a/backend/tests/test_channels.py b/backend/tests/test_channels.py index 65abbcca8..e0b6ce749 100644 --- a/backend/tests/test_channels.py +++ b/backend/tests/test_channels.py @@ -5311,6 +5311,150 @@ class TestFeishuChannel: _run(go()) +class TestFeishuSendFileSuccessChecks: + """``send_file`` uploads via ``_upload_image``/``_upload_file`` (which already + raise on a ``response.success() is False`` business failure), then sends the + resulting file/image message with a raw ``message.reply``/``message.create`` + call whose response was never checked. lark-oapi signals that same kind of + business-level failure (invalid receiver, permission error, etc.) by + returning ``success()=False`` without raising, so a failed file/image send + logged "file sent" and returned ``True`` exactly like a real success. + """ + + def test_send_file_returns_false_on_reply_business_failure(self, tmp_path): + from lark_oapi.api.im.v1 import ReplyMessageRequest, ReplyMessageRequestBody + + from app.channels.feishu import FeishuChannel + + async def go(): + bus = MessageBus() + channel = FeishuChannel(bus, config={}) + channel._api_client = MagicMock() + channel._ReplyMessageRequest = ReplyMessageRequest + channel._ReplyMessageRequestBody = ReplyMessageRequestBody + channel._upload_image = AsyncMock(return_value="img-key-1") + + failure_response = MagicMock() + failure_response.success.return_value = False + failure_response.code = 99991400 + failure_response.msg = "param invalid" + failure_response.get_log_id.return_value = "log-send-file-1" + channel._api_client.im.v1.message.reply = MagicMock(return_value=failure_response) + + path = tmp_path / "image.png" + path.write_bytes(b"png") + attachment = ResolvedAttachment( + virtual_path="/mnt/user-data/outputs/image.png", + actual_path=path, + filename="image.png", + mime_type="image/png", + size=path.stat().st_size, + is_image=True, + ) + msg = OutboundMessage( + channel_name="feishu", + chat_id="chat-1", + thread_id="thread-1", + text="", + is_final=True, + thread_ts="om-source-msg", + ) + + result = await channel.send_file(msg, attachment) + + assert result is False + + _run(go()) + + def test_send_file_returns_false_on_create_business_failure(self, tmp_path): + from lark_oapi.api.im.v1 import CreateMessageRequest, CreateMessageRequestBody + + from app.channels.feishu import FeishuChannel + + async def go(): + bus = MessageBus() + channel = FeishuChannel(bus, config={}) + channel._api_client = MagicMock() + channel._CreateMessageRequest = CreateMessageRequest + channel._CreateMessageRequestBody = CreateMessageRequestBody + channel._upload_file = AsyncMock(return_value="file-key-1") + + failure_response = MagicMock() + failure_response.success.return_value = False + failure_response.code = 99991400 + failure_response.msg = "param invalid" + failure_response.get_log_id.return_value = "log-send-file-2" + channel._api_client.im.v1.message.create = MagicMock(return_value=failure_response) + + path = tmp_path / "report.pdf" + path.write_bytes(b"pdf") + attachment = ResolvedAttachment( + virtual_path="/mnt/user-data/outputs/report.pdf", + actual_path=path, + filename="report.pdf", + mime_type="application/pdf", + size=path.stat().st_size, + is_image=False, + ) + msg = OutboundMessage( + channel_name="feishu", + chat_id="chat-1", + thread_id="thread-1", + text="", + is_final=True, + thread_ts=None, + ) + + result = await channel.send_file(msg, attachment) + + assert result is False + + _run(go()) + + def test_send_file_returns_true_on_reply_business_success(self, tmp_path): + """Control case: a genuinely successful response still returns True.""" + from lark_oapi.api.im.v1 import ReplyMessageRequest, ReplyMessageRequestBody + + from app.channels.feishu import FeishuChannel + + async def go(): + bus = MessageBus() + channel = FeishuChannel(bus, config={}) + channel._api_client = MagicMock() + channel._ReplyMessageRequest = ReplyMessageRequest + channel._ReplyMessageRequestBody = ReplyMessageRequestBody + channel._upload_image = AsyncMock(return_value="img-key-1") + + success_response = MagicMock() + success_response.success.return_value = True + channel._api_client.im.v1.message.reply = MagicMock(return_value=success_response) + + path = tmp_path / "image.png" + path.write_bytes(b"png") + attachment = ResolvedAttachment( + virtual_path="/mnt/user-data/outputs/image.png", + actual_path=path, + filename="image.png", + mime_type="image/png", + size=path.stat().st_size, + is_image=True, + ) + msg = OutboundMessage( + channel_name="feishu", + chat_id="chat-1", + thread_id="thread-1", + text="", + is_final=True, + thread_ts="om-source-msg", + ) + + result = await channel.send_file(msg, attachment) + + assert result is True + + _run(go()) + + class TestWeComChannel: def test_publish_ws_inbound_starts_stream_and_publishes_message(self, monkeypatch): from app.channels.wecom import WeComChannel