From 7d4e13509bcec3836fcd553ecd9ee38487210b5c Mon Sep 17 00:00:00 2001 From: Hyu Date: Sun, 11 Oct 2026 00:17:19 +0800 Subject: [PATCH] fix(slack): transfer message attachments through file APIs --- src/langbot/libs/slack_api/api.py | 8 +- src/langbot/libs/slack_api/media.py | 125 +++++++++++ src/langbot/libs/slack_api/slackevent.py | 2 + .../pkg/platform/adapters/slack/adapter.py | 6 +- .../adapters/slack/message_converter.py | 22 +- src/langbot/pkg/platform/sources/slack.py | 31 +-- tests/unit_tests/platform/test_slack_media.py | 207 ++++++++++++++++++ 7 files changed, 365 insertions(+), 36 deletions(-) create mode 100644 src/langbot/libs/slack_api/media.py create mode 100644 tests/unit_tests/platform/test_slack_media.py diff --git a/src/langbot/libs/slack_api/api.py b/src/langbot/libs/slack_api/api.py index 3ffcf04c0..9774290be 100644 --- a/src/langbot/libs/slack_api/api.py +++ b/src/langbot/libs/slack_api/api.py @@ -167,8 +167,8 @@ class SlackClient: try: response = await self.client.chat_postMessage(channel=channel_id, text=text) if self.bot_user_id is None and response.get('ok'): - self.bot_user_id = response['message']['bot_id'] - return + self.bot_user_id = response.get('message', {}).get('bot_id') + return response.data if hasattr(response, 'data') else response except Exception as e: await self.logger.error(f'Error in send_message: {e}') raise e @@ -177,9 +177,9 @@ class SlackClient: try: response = await self.client.chat_postMessage(channel='@' + user_id, text=text) if self.bot_user_id is None and response.get('ok'): - self.bot_user_id = response['message']['bot_id'] + self.bot_user_id = response.get('message', {}).get('bot_id') - return + return response.data if hasattr(response, 'data') else response except Exception as e: await self.logger.error(f'Error in send_message: {traceback.format_exc()}') raise e diff --git a/src/langbot/libs/slack_api/media.py b/src/langbot/libs/slack_api/media.py new file mode 100644 index 000000000..f220af14f --- /dev/null +++ b/src/langbot/libs/slack_api/media.py @@ -0,0 +1,125 @@ +"""Transfer attachment bytes rather than degrading attachments to text.""" + +import asyncio +import mimetypes +from pathlib import Path +from urllib.parse import urlparse, unquote + +import aiohttp + +from langbot.pkg.utils import httpclient, image +from langbot_plugin.api.entities.builtin.platform import message as pm + +MAX_MEDIA_BYTES = 20 * 1024 * 1024 +MEDIA_TYPES = (pm.Image, pm.File, pm.Voice) + + +async def download_media(url: str, token: str = '') -> tuple[bytes, str]: + parsed = urlparse(url) + if parsed.scheme not in {'http', 'https'}: + raise ValueError('Slack media requires an HTTP(S) URL') + # Never disclose a bot token to arbitrary media hosts or redirects. + private = parsed.scheme == 'https' and parsed.hostname == 'files.slack.com' + headers = {'Authorization': f'Bearer {token}'} if private and token else {} + async with httpclient.get_session().get( + url, headers=headers, allow_redirects=not bool(headers), timeout=aiohttp.ClientTimeout(total=30) + ) as response: + if response.status != 200: + raise ValueError(f'Slack media download failed (HTTP {response.status})') + data = await httpclient.read_limited(response, max_bytes=MAX_MEDIA_BYTES) + return data, response.headers.get('Content-Type', 'application/octet-stream').split(';')[0] + + +async def attachment_bytes(component, token: str = '') -> tuple[bytes, str]: + name = getattr(component, 'name', '') + mime = '' + if component.base64: + value = component.base64 + if value.startswith('data:'): + header, value = value.split(',', 1) + mime = header[5:].split(';')[0] + data = await image.decode_base64_limited(value, max_bytes=MAX_MEDIA_BYTES) + elif component.path: + path = Path(component.path) + + def read(): + with path.open('rb') as stream: + return stream.read(MAX_MEDIA_BYTES + 1) + + data = await asyncio.to_thread(read) + name = name or path.name + elif component.url: + data, mime = await download_media(component.url, token) + name = name or Path(unquote(urlparse(component.url).path)).name + else: + raise ValueError('Slack attachment has no base64, path or URL') + if not data or len(data) > MAX_MEDIA_BYTES: + raise ValueError('Slack attachment is empty or exceeds the 20 MiB limit') + if not name: + if isinstance(component, pm.Image) and not mime: + mime = await asyncio.to_thread(_image_mime, data) + name = 'attachment' + (mimetypes.guess_extension(mime) or '.bin') + return data, name.replace('\\', '/').rsplit('/', 1)[-1] + + +def _image_mime(data: bytes) -> str: + import io + from PIL import Image + + with Image.open(io.BytesIO(data)) as picture: + return Image.MIME.get(picture.format, 'application/octet-stream') + + +async def send_chain( + bot, target_type: str, target_id: str, chain: pm.MessageChain, thread_ts: str | None = None +) -> dict: + from langbot.pkg.platform.adapters.slack.message_converter import SlackMessageConverter + + if target_type not in {'person', 'channel'}: + raise ValueError(f'Unsupported Slack target type: {target_type}') + responses = [] + pending = [] + channel = None + + async def flush(): + if not pending: + return + text = await SlackMessageConverter.yiri2target(pm.MessageChain(list(pending))) + pending.clear() + if text: + if thread_ts: + response = await bot.client.chat_postMessage(channel=target_id, text=text, thread_ts=thread_ts) + responses.append(response.data if hasattr(response, 'data') else response) + else: + method = bot.send_message_to_one if target_type == 'person' else bot.send_message_to_channel + responses.append(await method(text, target_id)) + + for component in chain: + if isinstance(component, MEDIA_TYPES): + data, filename = await attachment_bytes(component, bot.bot_token) + await flush() + if channel is None: + if target_type == 'person' and not target_id.startswith('D'): + opened = await bot.client.conversations_open(users=[target_id]) + channel = opened['channel']['id'] + else: + channel = target_id + response = await bot.client.files_upload_v2( + channel=channel, + file=data, + filename=filename, + title=filename, + **({'thread_ts': thread_ts} if thread_ts else {}), + ) + if not response.get('ok'): + raise RuntimeError(f'Slack file upload failed: {response.get("error", "unknown_error")}') + responses.append(response.data if hasattr(response, 'data') else response) + elif isinstance(component, pm.Forward): + await flush() + for node in component.node_list: + if node.message_chain: + responses.append(await send_chain(bot, target_type, target_id, node.message_chain, thread_ts)) + else: + pending.append(component) + await flush() + return {'target_type': target_type, 'target_id': target_id, 'responses': responses} diff --git a/src/langbot/libs/slack_api/slackevent.py b/src/langbot/libs/slack_api/slackevent.py index 77177816e..4f79a1d59 100644 --- a/src/langbot/libs/slack_api/slackevent.py +++ b/src/langbot/libs/slack_api/slackevent.py @@ -12,6 +12,8 @@ class SlackEvent(dict): @property def text(self) -> str: + if self.get('event', {}).get('text') is not None: + return self['event']['text'] if self.get('event', {}).get('channel_type') == 'im': blocks = self.get('event', {}).get('blocks', []) if not blocks: diff --git a/src/langbot/pkg/platform/adapters/slack/adapter.py b/src/langbot/pkg/platform/adapters/slack/adapter.py index e07e9bd0e..37d3ec35f 100644 --- a/src/langbot/pkg/platform/adapters/slack/adapter.py +++ b/src/langbot/pkg/platform/adapters/slack/adapter.py @@ -8,6 +8,7 @@ import pydantic from langbot.libs.slack_api.api import SlackClient from langbot.libs.slack_api.slackevent import SlackEvent +from langbot.libs.slack_api.media import send_chain from langbot.pkg.platform.adapters.slack.api_impl import SlackAPIMixin from langbot.pkg.platform.adapters.slack.errors import NotSupportedError from langbot.pkg.platform.adapters.slack.event_converter import SlackEventConverter @@ -96,8 +97,7 @@ class SlackAdapter(SlackAPIMixin, abstract_platform_adapter.AbstractPlatformAdap target_id: str, message: platform_message.MessageChain, ) -> platform_events.MessageResult: - content = await SlackMessageConverter.yiri2target(message) - raw = await self._send_text(str(target_type), str(target_id), content) + raw = await send_chain(self.bot, self._normalize_target_type(str(target_type)), str(target_id), message) return platform_events.MessageResult(raw=raw) async def reply_message( @@ -111,7 +111,7 @@ class SlackAdapter(SlackAPIMixin, abstract_platform_adapter.AbstractPlatformAdap raise ValueError('Slack reply_message requires a SlackEvent source object') target_type = 'channel' if source.type == 'channel' else 'person' target_id = source.channel_id if source.type == 'channel' else source.user_id - raw = await self._send_text(target_type, target_id, await SlackMessageConverter.yiri2target(message)) + raw = await send_chain(self.bot, target_type, target_id, message) return platform_events.MessageResult(message_id=source.message_id, raw=raw) async def call_platform_api(self, action: str, params: dict = {}) -> dict: diff --git a/src/langbot/pkg/platform/adapters/slack/message_converter.py b/src/langbot/pkg/platform/adapters/slack/message_converter.py index 07a246dae..20c6f82d4 100644 --- a/src/langbot/pkg/platform/adapters/slack/message_converter.py +++ b/src/langbot/pkg/platform/adapters/slack/message_converter.py @@ -4,6 +4,7 @@ import datetime from langbot.libs.slack_api.slackevent import SlackEvent from langbot.pkg.utils import image +from langbot.libs.slack_api.media import download_media import langbot_plugin.api.definition.abstract.platform.adapter as abstract_platform_adapter from langbot_plugin.api.entities.builtin.platform import message as platform_message @@ -55,15 +56,24 @@ class SlackMessageConverter(abstract_platform_adapter.AbstractMessageConverter): if event.type == 'channel': components.append(platform_message.At(target='SlackBot')) - if event.pic_url: - try: + for file in event.get('event', {}).get('files', []): + url = file.get('url_private_download') or file.get('url_private') + if not url: + raise ValueError('Slack attachment is missing its download URL') + data, mime = await download_media(url, bot_token) + mime = file.get('mimetype') or mime + encoded = await image.encode_base64(data) + if mime.startswith('image/') and mime != 'image/svg+xml': + components.append(platform_message.Image(base64=f'data:{mime};base64,{encoded}')) + else: components.append( - platform_message.Image( - url=event.pic_url, base64=await image.get_slack_image_to_base64(event.pic_url, bot_token) + platform_message.File( + id=file.get('id', ''), + name=file.get('name') or file.get('title') or 'attachment', + size=len(data), + base64=encoded, ) ) - except Exception: - components.append(platform_message.Image(url=event.pic_url)) if event.text: components.append(platform_message.Plain(text=event.text)) diff --git a/src/langbot/pkg/platform/sources/slack.py b/src/langbot/pkg/platform/sources/slack.py index 57a9c0f33..cbfd50b7d 100644 --- a/src/langbot/pkg/platform/sources/slack.py +++ b/src/langbot/pkg/platform/sources/slack.py @@ -6,6 +6,8 @@ import traceback import datetime from langbot.libs.slack_api.api import SlackClient +from langbot.libs.slack_api.media import send_chain +from langbot.pkg.platform.adapters.slack.message_converter import SlackMessageConverter as OmniMessageConverter import langbot_plugin.api.definition.abstract.platform.adapter as abstract_platform_adapter from langbot.libs.slack_api.slackevent import SlackEvent import langbot_plugin.api.entities.builtin.platform.events as platform_events @@ -61,12 +63,9 @@ class SlackEventConverter(abstract_platform_adapter.AbstractEventConverter): @staticmethod async def target2yiri(event: SlackEvent, bot: SlackClient): - yiri_chain = await SlackMessageConverter.target2yiri( - message=event.text, message_id=event.message_id, pic_url=event.pic_url, bot=bot - ) + yiri_chain = await OmniMessageConverter.target2yiri(event, bot.bot_token) if event.type == 'channel': - yiri_chain.insert(0, platform_message.At(target='SlackBot')) sender = platform_entities.GroupMember( id=event.user_id, @@ -127,27 +126,13 @@ class SlackAdapter(abstract_platform_adapter.AbstractMessagePlatformAdapter): ): slack_event = await SlackEventConverter.yiri2target(message_source) - content_list = await SlackMessageConverter.yiri2target(message) - - for content in content_list: - # Both text and image (URL) are sent as text messages - # Slack will auto-unfurl image URLs - message_content = content['content'] - if slack_event.type == 'channel': - await self.bot.send_message_to_channel(message_content, slack_event.channel_id) - if slack_event.type == 'im': - await self.bot.send_message_to_one(message_content, slack_event.user_id) + target_type = 'channel' if slack_event.type == 'channel' else 'person' + target_id = slack_event.channel_id if target_type == 'channel' else slack_event.user_id + return await send_chain(self.bot, target_type, target_id, message) async def send_message(self, target_type: str, target_id: str, message: platform_message.MessageChain): - content_list = await SlackMessageConverter.yiri2target(message) - for content in content_list: - # Both text and image (URL) are sent as text messages - # Slack will auto-unfurl image URLs - message_content = content['content'] - if target_type == 'person': - await self.bot.send_message_to_one(message_content, target_id) - if target_type == 'group': - await self.bot.send_message_to_channel(message_content, target_id) + target_type = 'channel' if target_type in {'group', 'channel'} else target_type + return await send_chain(self.bot, target_type, target_id, message) def register_listener( self, diff --git a/tests/unit_tests/platform/test_slack_media.py b/tests/unit_tests/platform/test_slack_media.py new file mode 100644 index 000000000..3238c3e7c --- /dev/null +++ b/tests/unit_tests/platform/test_slack_media.py @@ -0,0 +1,207 @@ +import base64 +from types import SimpleNamespace +from unittest.mock import AsyncMock, patch + +import pytest +from slack_sdk.web.async_slack_response import AsyncSlackResponse + +from langbot.libs.slack_api.api import SlackClient +from langbot.libs.slack_api.media import attachment_bytes, download_media, send_chain +from langbot.libs.slack_api.slackevent import SlackEvent +from langbot.pkg.platform.adapters.slack.message_converter import SlackMessageConverter +from langbot_plugin.api.entities.builtin.platform import message as pm + + +def sdk_response(data): + return AsyncSlackResponse( + client=None, + http_verb='POST', + api_url='https://slack.com/api/test', + req_args={}, + data=data, + headers={}, + status_code=200, + ) + + +@pytest.mark.asyncio +@pytest.mark.parametrize('target_type,target_id', [('person', 'U123'), ('channel', 'C123')]) +async def test_real_sdk_responses_for_text_and_attachment(target_type, target_id): + client = SlackClient('test-token', '', SimpleNamespace(error=AsyncMock()), unified_mode=True) + text_data = {'ok': True, 'channel': 'C123', 'message': {'ts': '1.2', 'bot_id': 'B123'}} + file_data = {'ok': True, 'files': [{'id': 'F123'}]} + client.client = SimpleNamespace( + chat_postMessage=AsyncMock(return_value=sdk_response(text_data)), + conversations_open=AsyncMock(return_value=sdk_response({'ok': True, 'channel': {'id': 'D123'}})), + files_upload_v2=AsyncMock(return_value=sdk_response(file_data)), + ) + result = await send_chain( + client, + target_type, + target_id, + pm.MessageChain( + [ + pm.Plain(text='picture'), + pm.File(name='image.png', base64='dGVzdA=='), + ] + ), + ) + assert result['responses'] == [text_data, file_data] + assert client.client.chat_postMessage.await_count == 1 + assert client.client.files_upload_v2.await_count == 1 + + +def bot(): + return SimpleNamespace( + bot_token='test-token', + client=SimpleNamespace( + conversations_open=AsyncMock(return_value={'ok': True, 'channel': {'id': 'D123'}}), + files_upload_v2=AsyncMock(return_value={'ok': True, 'files': [{'id': 'F123'}]}), + ), + send_message_to_one=AsyncMock(return_value={'ok': True}), + send_message_to_channel=AsyncMock(return_value={'ok': True}), + ) + + +@pytest.mark.asyncio +async def test_svg_base64_is_uploaded_to_dm_not_sent_as_filename(): + client = bot() + svg = b'' + await send_chain( + client, + 'person', + 'U123', + pm.MessageChain( + [ + pm.Plain(text='Here is the picture'), + pm.File(name='moon.svg', base64=base64.b64encode(svg).decode()), + ] + ), + ) + client.send_message_to_one.assert_awaited_once_with('Here is the picture', 'U123') + client.client.conversations_open.assert_awaited_once_with(users=['U123']) + client.client.files_upload_v2.assert_awaited_once_with( + channel='D123', file=svg, filename='moon.svg', title='moon.svg' + ) + + +@pytest.mark.asyncio +async def test_multiple_attachments_to_channel_preserve_bytes_and_text_order(tmp_path): + client = bot() + path = tmp_path / 'notes.txt' + path.write_bytes(b'hello') + await send_chain( + client, + 'channel', + 'C123', + pm.MessageChain( + [ + pm.Image(base64='data:image/png;base64,' + base64.b64encode(b'png bytes').decode()), + pm.File(path=str(path)), + pm.Plain(text='done'), + ] + ), + ) + calls = client.client.files_upload_v2.await_args_list + assert [c.kwargs['file'] for c in calls] == [b'png bytes', b'hello'] + assert [c.kwargs['filename'] for c in calls] == ['attachment.png', 'notes.txt'] + client.client.conversations_open.assert_not_awaited() + client.send_message_to_channel.assert_awaited_once_with('done', 'C123') + + +@pytest.mark.asyncio +async def test_upload_failure_is_not_reported_as_success(): + client = bot() + client.client.files_upload_v2.return_value = {'ok': False, 'error': 'missing_scope'} + with pytest.raises(RuntimeError, match='missing_scope'): + await send_chain( + client, + 'channel', + 'C123', + pm.MessageChain( + [ + pm.File(name='test.txt', base64='dGVzdA=='), + ] + ), + ) + + +@pytest.mark.asyncio +async def test_missing_attachment_data_fails_instead_of_faking_delivery(): + with pytest.raises(ValueError, match='no base64'): + await attachment_bytes(pm.File(name='missing.txt')) + + +@pytest.mark.asyncio +async def test_local_bytes_take_priority_over_private_url(): + data, name = await attachment_bytes( + pm.File(name='file.txt', url='https://files.slack.com/private', base64='dGVzdA==') + ) + assert (data, name) == (b'test', 'file.txt') + + +@pytest.mark.asyncio +async def test_received_multiple_files_keep_type_and_authenticated_bytes(): + event = SlackEvent( + { + 'event': { + 'channel_type': 'im', + 'text': 'attachments', + 'files': [ + {'id': 'F1', 'name': 'a.png', 'mimetype': 'image/png', 'url_private': 'https://files.slack.com/a'}, + { + 'id': 'F2', + 'name': 'b.pdf', + 'mimetype': 'application/pdf', + 'url_private': 'https://files.slack.com/b', + }, + ], + } + } + ) + with patch( + 'langbot.pkg.platform.adapters.slack.message_converter.download_media', + new=AsyncMock(return_value=(b'data', 'application/octet-stream')), + ) as download: + chain = await SlackMessageConverter.target2yiri(event, 'test-token') + assert isinstance(chain[1], pm.Image) + assert chain[1].base64 == 'data:image/png;base64,ZGF0YQ==' + assert isinstance(chain[2], pm.File) and chain[2].name == 'b.pdf' + assert chain[2].base64 == 'ZGF0YQ==' + assert chain[3].text == 'attachments' + assert download.await_count == 2 + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + 'url,authenticated', + [ + ('https://files.slack.com/file', True), + ('https://example.com/file', False), + ('https://files.slack.com.evil.test/file', False), + ], +) +async def test_download_credentials_are_scoped_to_slack(url, authenticated): + response = SimpleNamespace(status=200, headers={'Content-Type': 'image/png'}) + context = AsyncMock() + context.__aenter__.return_value = response + with ( + patch('langbot.libs.slack_api.media.httpclient.get_session') as session, + patch('langbot.libs.slack_api.media.httpclient.read_limited', new=AsyncMock(return_value=b'png')), + ): + session.return_value.get.return_value = context + assert await download_media(url, 'test-token') == (b'png', 'image/png') + kwargs = session.return_value.get.call_args.kwargs + assert bool(kwargs['headers']) == authenticated + if authenticated: + assert kwargs['allow_redirects'] is False + + +@pytest.mark.asyncio +async def test_download_http_failure_is_visible(): + context = AsyncMock() + context.__aenter__.return_value = SimpleNamespace(status=403) + with patch('langbot.libs.slack_api.media.httpclient.get_session') as session: + session.return_value.get.return_value = context + with pytest.raises(ValueError, match='HTTP 403'): + await download_media('https://files.slack.com/file', 'test-token')