mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(slack): accept file_share events and download inbound files (#6855)
* fix(slack): accept file_share events and download inbound files Slack was dropping every Events API message with a subtype, including user uploads (subtype=file_share). Allowlist file_share, attach event files to the inbound message, and download private URLs in receive_file with the bot Bearer token (bytes via INBOUND_FILE_CONTENT_KEY). Fixes #6853 * fix(slack): tighten inbound file download guards Address review on #6855: allowlist https files.slack.com before Bearer fetch, cap aggregate in-memory bytes per file_share message, report Content-Encoding refusals as download failed, and share one _web_client_for_connection helper for inbound/outbound. * fix(slack): satisfy ruff format and AGENTS.md size gate Reformat test_slack_file_share.py and drop the AGENTS.md file_share note so channels/AGENTS.md stays under the 49152-byte hard limit (file was already near the ceiling without it).
This commit is contained in:
parent
2fe325f8ae
commit
585f87d3b3
@ -7,17 +7,39 @@ import html
|
|||||||
import logging
|
import logging
|
||||||
import re
|
import re
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
from urllib.parse import urlsplit
|
||||||
|
|
||||||
|
import httpx
|
||||||
from markdown_to_mrkdwn import SlackMarkdownConverter
|
from markdown_to_mrkdwn import SlackMarkdownConverter
|
||||||
|
|
||||||
from app.channels.allowed_users import parse_allowed_users, parse_string_or_int_user_id
|
from app.channels.allowed_users import parse_allowed_users, parse_string_or_int_user_id
|
||||||
from app.channels.base import Channel, ChannelStopTimeout
|
from app.channels.base import Channel, ChannelStopTimeout
|
||||||
from app.channels.commands import is_known_channel_command
|
from app.channels.commands import is_known_channel_command
|
||||||
from app.channels.connection_identity import attach_connection_identity
|
from app.channels.connection_identity import attach_connection_identity
|
||||||
from app.channels.message_bus import InboundMessageType, InboundReservation, MessageBus, OutboundMessage, ResolvedAttachment
|
from app.channels.message_bus import (
|
||||||
|
INBOUND_FILE_CONTENT_KEY,
|
||||||
|
InboundMessage,
|
||||||
|
InboundMessageType,
|
||||||
|
InboundReservation,
|
||||||
|
MessageBus,
|
||||||
|
OutboundMessage,
|
||||||
|
ResolvedAttachment,
|
||||||
|
)
|
||||||
|
from deerflow.uploads.manager import is_upload_staging_file, normalize_filename
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
SLACK_STOP_TIMEOUT_SECONDS = 5.0
|
SLACK_STOP_TIMEOUT_SECONDS = 5.0
|
||||||
|
# Match the manager's URL download ceiling. Slack workspace limits may be higher;
|
||||||
|
# this keeps a single upload from exhausting Gateway memory.
|
||||||
|
SLACK_MAX_INBOUND_FILE_BYTES = 50 * 1024 * 1024
|
||||||
|
# Cap total in-memory bytes for one file_share message (N files × per-file max).
|
||||||
|
SLACK_MAX_INBOUND_MESSAGE_BYTES = SLACK_MAX_INBOUND_FILE_BYTES
|
||||||
|
# Private download URLs are Slack-hosted; gate Bearer fetches to this suffix.
|
||||||
|
SLACK_ALLOWED_FILE_HOST_SUFFIXES = frozenset({"files.slack.com"})
|
||||||
|
# User-authored subtypes we still want to turn into chat turns. Everything else
|
||||||
|
# (edits, deletes, joins, bot_message, thread_broadcast, …) stays ignored so we
|
||||||
|
# do not double-fire or treat platform noise as a user message.
|
||||||
|
_SLACK_ACCEPTED_MESSAGE_SUBTYPES = frozenset({"file_share"})
|
||||||
|
|
||||||
_slack_md_converter = SlackMarkdownConverter()
|
_slack_md_converter = SlackMarkdownConverter()
|
||||||
|
|
||||||
@ -292,15 +314,19 @@ class SlackChannel(Channel):
|
|||||||
except Exception:
|
except Exception:
|
||||||
logger.warning("[Slack] failed to resolve bot user id; app mention text may include the bot mention", exc_info=True)
|
logger.warning("[Slack] failed to resolve bot user id; app mention text may include the bot mention", exc_info=True)
|
||||||
|
|
||||||
async def _get_web_client_for_message(self, msg: OutboundMessage):
|
async def _web_client_for_connection(self, connection_id: str | None):
|
||||||
if msg.connection_id and self._connection_repo is not None:
|
"""Resolve a WebClient for *connection_id*, falling back to the operator client.
|
||||||
credentials = await self._connection_repo.get_credentials(msg.connection_id)
|
|
||||||
|
WebClient keeps its own HTTP session and rate-limit state, so reuse one
|
||||||
|
per connection until its token changes. Shared by outbound sends and
|
||||||
|
inbound private-file downloads so the cache cannot drift.
|
||||||
|
"""
|
||||||
|
if connection_id and self._connection_repo is not None:
|
||||||
|
credentials = await self._connection_repo.get_credentials(connection_id)
|
||||||
access_token = credentials.get("access_token") if credentials else None
|
access_token = credentials.get("access_token") if credentials else None
|
||||||
if not access_token:
|
if not access_token:
|
||||||
return self._web_client
|
return self._web_client
|
||||||
# WebClient keeps its own HTTP session and rate-limit state, so
|
cached = self._connection_web_clients.get(connection_id)
|
||||||
# reuse one per connection until its token changes.
|
|
||||||
cached = self._connection_web_clients.get(msg.connection_id)
|
|
||||||
if cached is not None and cached[0] == access_token:
|
if cached is not None and cached[0] == access_token:
|
||||||
return cached[1]
|
return cached[1]
|
||||||
if self._web_client_factory is None:
|
if self._web_client_factory is None:
|
||||||
@ -308,10 +334,13 @@ class SlackChannel(Channel):
|
|||||||
|
|
||||||
self._web_client_factory = WebClient
|
self._web_client_factory = WebClient
|
||||||
web_client = self._web_client_factory(token=access_token)
|
web_client = self._web_client_factory(token=access_token)
|
||||||
self._connection_web_clients[msg.connection_id] = (access_token, web_client)
|
self._connection_web_clients[connection_id] = (access_token, web_client)
|
||||||
return web_client
|
return web_client
|
||||||
return self._web_client
|
return self._web_client
|
||||||
|
|
||||||
|
async def _get_web_client_for_message(self, msg: OutboundMessage):
|
||||||
|
return await self._web_client_for_connection(msg.connection_id)
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def _add_reaction_with_client(web_client, channel_id: str, timestamp: str, emoji: str) -> None:
|
def _add_reaction_with_client(web_client, channel_id: str, timestamp: str, emoji: str) -> None:
|
||||||
try:
|
try:
|
||||||
@ -377,8 +406,11 @@ class SlackChannel(Channel):
|
|||||||
logger.exception("Error processing Slack event")
|
logger.exception("Error processing Slack event")
|
||||||
|
|
||||||
def _handle_message_event(self, event: dict, *, team_id: str | None = None) -> None:
|
def _handle_message_event(self, event: dict, *, team_id: str | None = None) -> None:
|
||||||
# Ignore bot messages
|
# Ignore bot traffic and platform subtypes we do not handle as chat.
|
||||||
if event.get("bot_id") or event.get("subtype"):
|
# file_share is a user upload (optional caption + files[]) and must not
|
||||||
|
# be dropped just because Slack sets subtype.
|
||||||
|
subtype = event.get("subtype")
|
||||||
|
if event.get("bot_id") or (subtype and subtype not in _SLACK_ACCEPTED_MESSAGE_SUBTYPES):
|
||||||
return
|
return
|
||||||
|
|
||||||
user_id = event.get("user", "")
|
user_id = event.get("user", "")
|
||||||
@ -389,7 +421,8 @@ class SlackChannel(Channel):
|
|||||||
# Decode after mention stripping: a real mention is a raw <@...>, while
|
# Decode after mention stripping: a real mention is a raw <@...>, while
|
||||||
# a user-typed "<@...>" arrives as <@...> and must stay text.
|
# a user-typed "<@...>" arrives as <@...> and must stay text.
|
||||||
text = _unescape_slack_text(text)
|
text = _unescape_slack_text(text)
|
||||||
if not text:
|
files = self._extract_inbound_files(event)
|
||||||
|
if not text and not files:
|
||||||
return
|
return
|
||||||
|
|
||||||
connect_code = self._pending_connect_code(text)
|
connect_code = self._pending_connect_code(text)
|
||||||
@ -432,6 +465,7 @@ class SlackChannel(Channel):
|
|||||||
text=text,
|
text=text,
|
||||||
msg_type=msg_type,
|
msg_type=msg_type,
|
||||||
thread_ts=thread_ts,
|
thread_ts=thread_ts,
|
||||||
|
files=files,
|
||||||
metadata={
|
metadata={
|
||||||
# team_id is already resolved (payload team_id/team, else event team) by the caller.
|
# team_id is already resolved (payload team_id/team, else event team) by the caller.
|
||||||
"team_id": team_id,
|
"team_id": team_id,
|
||||||
@ -468,6 +502,226 @@ class SlackChannel(Channel):
|
|||||||
reservation.release()
|
reservation.release()
|
||||||
logger.info("[Slack] main loop stopped before reserved inbound could be scheduled")
|
logger.info("[Slack] main loop stopped before reserved inbound could be scheduled")
|
||||||
|
|
||||||
|
async def receive_file(self, msg: InboundMessage, thread_id: str, *, user_id: str | None = None) -> InboundMessage:
|
||||||
|
"""Download inbound Slack file_share attachments for the shared upload pipeline.
|
||||||
|
|
||||||
|
Slack private download URLs require the bot token in an Authorization
|
||||||
|
header, so this adapter never hands those URLs to the generic HTTP
|
||||||
|
reader. Bytes cross to ``ChannelManager`` through
|
||||||
|
``INBOUND_FILE_CONTENT_KEY`` (same shape as Telegram).
|
||||||
|
"""
|
||||||
|
del thread_id, user_id
|
||||||
|
if not msg.files:
|
||||||
|
return msg
|
||||||
|
|
||||||
|
web_client = await self._get_web_client_for_inbound(msg)
|
||||||
|
token = getattr(web_client, "token", None) if web_client is not None else None
|
||||||
|
if not isinstance(token, str) or not token:
|
||||||
|
notice = "[Slack attachment unavailable: bot token missing]"
|
||||||
|
msg.text = f"{msg.text}\n\n{notice}" if msg.text else notice
|
||||||
|
msg.files = []
|
||||||
|
return msg
|
||||||
|
|
||||||
|
materialized: list[dict[str, Any]] = []
|
||||||
|
unavailable: list[str] = []
|
||||||
|
materialized_bytes = 0
|
||||||
|
|
||||||
|
for file_info in msg.files:
|
||||||
|
if not isinstance(file_info, dict):
|
||||||
|
continue
|
||||||
|
|
||||||
|
filename = self._safe_inbound_filename(file_info.get("filename"), "attachment")
|
||||||
|
declared_size = self._file_size(file_info.get("size"))
|
||||||
|
if declared_size is not None and declared_size > SLACK_MAX_INBOUND_FILE_BYTES:
|
||||||
|
logger.warning("[Slack] inbound file exceeds download limit, skipping: %s", filename)
|
||||||
|
unavailable.append(f"{filename} (exceeds the download limit)")
|
||||||
|
continue
|
||||||
|
if declared_size is not None and materialized_bytes + declared_size > SLACK_MAX_INBOUND_MESSAGE_BYTES:
|
||||||
|
logger.warning("[Slack] inbound message exceeds aggregate download limit, skipping: %s", filename)
|
||||||
|
unavailable.append(f"{filename} (exceeds the message download limit)")
|
||||||
|
continue
|
||||||
|
|
||||||
|
url = file_info.get("url_private_download")
|
||||||
|
if not isinstance(url, str) or not url.strip():
|
||||||
|
logger.error("[Slack] cannot download inbound file without url_private_download: %s", filename)
|
||||||
|
unavailable.append(f"{filename} (download unavailable)")
|
||||||
|
continue
|
||||||
|
|
||||||
|
url = url.strip()
|
||||||
|
if not self._is_allowed_private_file_url(url):
|
||||||
|
logger.warning("[Slack] inbound file URL host is not allowlisted, skipping: %s", filename)
|
||||||
|
unavailable.append(f"{filename} (download unavailable)")
|
||||||
|
continue
|
||||||
|
|
||||||
|
remaining_budget = SLACK_MAX_INBOUND_MESSAGE_BYTES - materialized_bytes
|
||||||
|
per_file_cap = min(SLACK_MAX_INBOUND_FILE_BYTES, remaining_budget)
|
||||||
|
if per_file_cap <= 0:
|
||||||
|
logger.warning("[Slack] inbound message aggregate download budget exhausted, skipping: %s", filename)
|
||||||
|
unavailable.append(f"{filename} (exceeds the message download limit)")
|
||||||
|
continue
|
||||||
|
|
||||||
|
try:
|
||||||
|
content = await self._download_private_file(url, token, max_bytes=per_file_cap)
|
||||||
|
except Exception as exc:
|
||||||
|
logger.error(
|
||||||
|
"[Slack] failed to download inbound file %s: %s",
|
||||||
|
filename,
|
||||||
|
type(exc).__name__,
|
||||||
|
)
|
||||||
|
unavailable.append(f"{filename} (download failed)")
|
||||||
|
continue
|
||||||
|
|
||||||
|
if content is None:
|
||||||
|
# Stream aborted for size; encoding refusals raise above.
|
||||||
|
unavailable.append(f"{filename} (exceeds the download limit)")
|
||||||
|
continue
|
||||||
|
|
||||||
|
if len(content) > SLACK_MAX_INBOUND_FILE_BYTES:
|
||||||
|
logger.warning("[Slack] downloaded inbound file exceeds limit, skipping: %s", filename)
|
||||||
|
unavailable.append(f"{filename} (exceeds the download limit)")
|
||||||
|
continue
|
||||||
|
if materialized_bytes + len(content) > SLACK_MAX_INBOUND_MESSAGE_BYTES:
|
||||||
|
logger.warning("[Slack] downloaded inbound file exceeds aggregate message limit, skipping: %s", filename)
|
||||||
|
unavailable.append(f"{filename} (exceeds the message download limit)")
|
||||||
|
continue
|
||||||
|
|
||||||
|
materialized.append(
|
||||||
|
{
|
||||||
|
"type": "image" if file_info.get("type") == "image" else "file",
|
||||||
|
"filename": filename,
|
||||||
|
"mime_type": file_info.get("mime_type") if isinstance(file_info.get("mime_type"), str) else "application/octet-stream",
|
||||||
|
"size": len(content),
|
||||||
|
INBOUND_FILE_CONTENT_KEY: content,
|
||||||
|
}
|
||||||
|
)
|
||||||
|
materialized_bytes += len(content)
|
||||||
|
|
||||||
|
msg.files = materialized
|
||||||
|
if unavailable:
|
||||||
|
notice = f"[Slack attachment unavailable: {', '.join(unavailable)}]"
|
||||||
|
msg.text = f"{msg.text}\n\n{notice}" if msg.text else notice
|
||||||
|
return msg
|
||||||
|
|
||||||
|
async def _get_web_client_for_inbound(self, msg: InboundMessage):
|
||||||
|
"""Resolve the WebClient whose token can fetch private Slack files."""
|
||||||
|
return await self._web_client_for_connection(msg.connection_id)
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def _is_allowed_private_file_url(cls, url: str) -> bool:
|
||||||
|
"""Gate Bearer downloads to https://*.files.slack.com (and the bare host)."""
|
||||||
|
try:
|
||||||
|
parsed = urlsplit(url)
|
||||||
|
except ValueError:
|
||||||
|
return False
|
||||||
|
if parsed.scheme != "https":
|
||||||
|
return False
|
||||||
|
host = (parsed.hostname or "").lower()
|
||||||
|
if not host:
|
||||||
|
return False
|
||||||
|
return any(host == suffix or host.endswith(f".{suffix}") for suffix in SLACK_ALLOWED_FILE_HOST_SUFFIXES)
|
||||||
|
|
||||||
|
async def _download_private_file(self, url: str, token: str, *, max_bytes: int = SLACK_MAX_INBOUND_FILE_BYTES) -> bytes | None:
|
||||||
|
"""Stream a Slack private file URL with Bearer auth; never log the URL.
|
||||||
|
|
||||||
|
Returns ``None`` only when the streamed body exceeds *max_bytes*.
|
||||||
|
A non-identity ``Content-Encoding`` raises so callers can report
|
||||||
|
``(download failed)`` instead of a misleading size-limit notice.
|
||||||
|
"""
|
||||||
|
chunks: list[bytes] = []
|
||||||
|
total = 0
|
||||||
|
async with httpx.AsyncClient(timeout=httpx.Timeout(20.0)) as client:
|
||||||
|
async with client.stream(
|
||||||
|
"GET",
|
||||||
|
url,
|
||||||
|
headers={
|
||||||
|
"Authorization": f"Bearer {token}",
|
||||||
|
"Accept-Encoding": "identity",
|
||||||
|
},
|
||||||
|
) as response:
|
||||||
|
response.raise_for_status()
|
||||||
|
encoding = (response.headers.get("content-encoding") or "").strip().lower()
|
||||||
|
if encoding and encoding != "identity":
|
||||||
|
logger.warning(
|
||||||
|
"[Slack] inbound file response uses Content-Encoding %r, dropping before decode",
|
||||||
|
encoding,
|
||||||
|
)
|
||||||
|
raise RuntimeError(f"unsupported content-encoding: {encoding}")
|
||||||
|
async for chunk in response.aiter_raw():
|
||||||
|
total += len(chunk)
|
||||||
|
if total > max_bytes:
|
||||||
|
logger.warning(
|
||||||
|
"[Slack] inbound file exceeds %d bytes download limit, dropping",
|
||||||
|
max_bytes,
|
||||||
|
)
|
||||||
|
return None
|
||||||
|
chunks.append(chunk)
|
||||||
|
return b"".join(chunks)
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def _extract_inbound_files(cls, event: dict) -> list[dict[str, Any]]:
|
||||||
|
"""Build token-free file descriptors from a Slack message event."""
|
||||||
|
raw_files = event.get("files")
|
||||||
|
if not isinstance(raw_files, list):
|
||||||
|
return []
|
||||||
|
|
||||||
|
files: list[dict[str, Any]] = []
|
||||||
|
for index, raw in enumerate(raw_files):
|
||||||
|
if not isinstance(raw, dict):
|
||||||
|
continue
|
||||||
|
download_url = raw.get("url_private_download") or raw.get("url_private")
|
||||||
|
if not isinstance(download_url, str) or not download_url.strip():
|
||||||
|
continue
|
||||||
|
file_id = raw.get("id")
|
||||||
|
mime_type = raw.get("mimetype")
|
||||||
|
if not isinstance(mime_type, str) or not mime_type:
|
||||||
|
mime_type = "application/octet-stream"
|
||||||
|
fallback = f"slack-file-{file_id if isinstance(file_id, str) and file_id else index}.bin"
|
||||||
|
filename = cls._safe_inbound_filename(raw.get("name") or raw.get("title"), fallback)
|
||||||
|
is_image = mime_type.startswith("image/") or str(raw.get("filetype") or "").lower() in {
|
||||||
|
"png",
|
||||||
|
"jpg",
|
||||||
|
"jpeg",
|
||||||
|
"gif",
|
||||||
|
"webp",
|
||||||
|
"bmp",
|
||||||
|
}
|
||||||
|
files.append(
|
||||||
|
{
|
||||||
|
"type": "image" if is_image else "file",
|
||||||
|
"file_id": file_id if isinstance(file_id, str) else None,
|
||||||
|
"filename": filename,
|
||||||
|
"mime_type": mime_type,
|
||||||
|
"size": cls._file_size(raw.get("size")),
|
||||||
|
# Private URL only — Authorization is attached in receive_file.
|
||||||
|
"url_private_download": download_url.strip(),
|
||||||
|
}
|
||||||
|
)
|
||||||
|
return files
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _safe_inbound_filename(value: Any, fallback: str) -> str:
|
||||||
|
if not isinstance(value, str):
|
||||||
|
return fallback
|
||||||
|
try:
|
||||||
|
candidate = normalize_filename(value.strip())
|
||||||
|
except (UnicodeError, ValueError):
|
||||||
|
return fallback
|
||||||
|
if is_upload_staging_file(candidate) or any(ord(char) < 32 for char in candidate):
|
||||||
|
return fallback
|
||||||
|
return candidate
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _file_size(value: Any) -> int | None:
|
||||||
|
if isinstance(value, bool):
|
||||||
|
return None
|
||||||
|
if isinstance(value, int):
|
||||||
|
return value if value >= 0 else None
|
||||||
|
if isinstance(value, float) and value.is_integer() and value >= 0:
|
||||||
|
return int(value)
|
||||||
|
if isinstance(value, str) and value.strip().isdigit():
|
||||||
|
return int(value.strip())
|
||||||
|
return None
|
||||||
|
|
||||||
async def _publish_inbound_with_connection(
|
async def _publish_inbound_with_connection(
|
||||||
self,
|
self,
|
||||||
inbound,
|
inbound,
|
||||||
|
|||||||
471
backend/tests/test_slack_file_share.py
Normal file
471
backend/tests/test_slack_file_share.py
Normal file
@ -0,0 +1,471 @@
|
|||||||
|
"""Slack file_share / subtype handling for inbound uploads."""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import asyncio
|
||||||
|
from types import SimpleNamespace
|
||||||
|
from unittest.mock import AsyncMock, MagicMock, patch
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from app.channels.message_bus import INBOUND_FILE_CONTENT_KEY, InboundMessage, MessageBus
|
||||||
|
|
||||||
|
|
||||||
|
def _run(coro):
|
||||||
|
loop = asyncio.new_event_loop()
|
||||||
|
try:
|
||||||
|
return loop.run_until_complete(coro)
|
||||||
|
finally:
|
||||||
|
loop.close()
|
||||||
|
|
||||||
|
|
||||||
|
def _immediate_loop():
|
||||||
|
loop = MagicMock()
|
||||||
|
loop.is_running.return_value = True
|
||||||
|
loop.call_soon_threadsafe.side_effect = lambda callback, *args: callback(*args)
|
||||||
|
return loop
|
||||||
|
|
||||||
|
|
||||||
|
def _submit_coro(coro, loop, **_kwargs):
|
||||||
|
coro.close()
|
||||||
|
return True
|
||||||
|
|
||||||
|
|
||||||
|
def _file_share_event(**overrides):
|
||||||
|
event = {
|
||||||
|
"type": "message",
|
||||||
|
"subtype": "file_share",
|
||||||
|
"user": "U123456",
|
||||||
|
"text": "please review",
|
||||||
|
"channel": "C123",
|
||||||
|
"ts": "1710000000.000100",
|
||||||
|
"files": [
|
||||||
|
{
|
||||||
|
"id": "FABC",
|
||||||
|
"name": "report.pdf",
|
||||||
|
"title": "report.pdf",
|
||||||
|
"mimetype": "application/pdf",
|
||||||
|
"filetype": "pdf",
|
||||||
|
"size": 4,
|
||||||
|
"url_private": "https://files.slack.com/files-pri/T-F/report.pdf",
|
||||||
|
"url_private_download": "https://files.slack.com/files-pri/T-F/download/report.pdf",
|
||||||
|
}
|
||||||
|
],
|
||||||
|
}
|
||||||
|
event.update(overrides)
|
||||||
|
return event
|
||||||
|
|
||||||
|
|
||||||
|
class TestSlackFileShareSubtype:
|
||||||
|
def _channel(self):
|
||||||
|
from app.channels.slack import SlackChannel
|
||||||
|
|
||||||
|
bus = MessageBus()
|
||||||
|
bus.publish_inbound = AsyncMock()
|
||||||
|
channel = SlackChannel(bus=bus, config={"bot_user_id": "UBOT"})
|
||||||
|
channel._loop = _immediate_loop()
|
||||||
|
channel._add_reaction = MagicMock()
|
||||||
|
channel._send_running_reply = MagicMock()
|
||||||
|
return bus, channel
|
||||||
|
|
||||||
|
def test_file_share_with_caption_publishes_inbound_files(self):
|
||||||
|
bus, channel = self._channel()
|
||||||
|
with patch("app.channels.slack.asyncio.run_coroutine_threadsafe", side_effect=_submit_coro):
|
||||||
|
channel._handle_message_event(_file_share_event())
|
||||||
|
|
||||||
|
channel._add_reaction.assert_called_once()
|
||||||
|
inbound = bus.get_inbound_nowait()
|
||||||
|
assert inbound.text == "please review"
|
||||||
|
assert len(inbound.files) == 1
|
||||||
|
assert inbound.files[0]["filename"] == "report.pdf"
|
||||||
|
assert inbound.files[0]["type"] == "file"
|
||||||
|
assert inbound.files[0]["url_private_download"].endswith("/download/report.pdf")
|
||||||
|
|
||||||
|
def test_file_share_without_caption_still_publishes(self):
|
||||||
|
bus, channel = self._channel()
|
||||||
|
with patch("app.channels.slack.asyncio.run_coroutine_threadsafe", side_effect=_submit_coro):
|
||||||
|
channel._handle_message_event(_file_share_event(text=""))
|
||||||
|
|
||||||
|
inbound = bus.get_inbound_nowait()
|
||||||
|
assert inbound.text == ""
|
||||||
|
assert inbound.files[0]["filename"] == "report.pdf"
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"subtype",
|
||||||
|
["message_changed", "message_deleted", "channel_join", "bot_message", "thread_broadcast"],
|
||||||
|
)
|
||||||
|
def test_non_file_share_subtypes_are_still_dropped(self, subtype):
|
||||||
|
bus, channel = self._channel()
|
||||||
|
with patch("app.channels.slack.asyncio.run_coroutine_threadsafe", side_effect=_submit_coro):
|
||||||
|
channel._handle_message_event(
|
||||||
|
{
|
||||||
|
"type": "message",
|
||||||
|
"subtype": subtype,
|
||||||
|
"user": "U123456",
|
||||||
|
"text": "should not publish",
|
||||||
|
"channel": "C123",
|
||||||
|
"ts": "1710000000.000100",
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
channel._add_reaction.assert_not_called()
|
||||||
|
assert bus.inbound_queue.empty()
|
||||||
|
|
||||||
|
def test_bot_id_still_dropped_even_for_file_share(self):
|
||||||
|
bus, channel = self._channel()
|
||||||
|
with patch("app.channels.slack.asyncio.run_coroutine_threadsafe", side_effect=_submit_coro):
|
||||||
|
channel._handle_message_event(_file_share_event(bot_id="B123"))
|
||||||
|
|
||||||
|
channel._add_reaction.assert_not_called()
|
||||||
|
assert bus.inbound_queue.empty()
|
||||||
|
|
||||||
|
def test_plain_message_without_subtype_still_works(self):
|
||||||
|
bus, channel = self._channel()
|
||||||
|
with patch("app.channels.slack.asyncio.run_coroutine_threadsafe", side_effect=_submit_coro):
|
||||||
|
channel._handle_message_event(
|
||||||
|
{
|
||||||
|
"user": "U123456",
|
||||||
|
"text": "hello",
|
||||||
|
"channel": "C123",
|
||||||
|
"ts": "1710000000.000100",
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
inbound = bus.get_inbound_nowait()
|
||||||
|
assert inbound.text == "hello"
|
||||||
|
assert inbound.files == []
|
||||||
|
|
||||||
|
|
||||||
|
class TestSlackExtractInboundFiles:
|
||||||
|
def test_extracts_image_and_document_descriptors(self):
|
||||||
|
from app.channels.slack import SlackChannel
|
||||||
|
|
||||||
|
files = SlackChannel._extract_inbound_files(
|
||||||
|
{
|
||||||
|
"files": [
|
||||||
|
{
|
||||||
|
"id": "FIMG",
|
||||||
|
"name": "photo.png",
|
||||||
|
"mimetype": "image/png",
|
||||||
|
"filetype": "png",
|
||||||
|
"size": 8,
|
||||||
|
"url_private_download": "https://files.slack.com/files-pri/T-F/download/photo.png",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"id": "FDOC",
|
||||||
|
"name": "notes.txt",
|
||||||
|
"mimetype": "text/plain",
|
||||||
|
"size": 2,
|
||||||
|
"url_private_download": "https://files.slack.com/files-pri/T-F/download/notes.txt",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"id": "FBAD",
|
||||||
|
"name": "missing-url.bin",
|
||||||
|
"mimetype": "application/octet-stream",
|
||||||
|
},
|
||||||
|
]
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
assert [item["filename"] for item in files] == ["photo.png", "notes.txt"]
|
||||||
|
assert files[0]["type"] == "image"
|
||||||
|
assert files[1]["type"] == "file"
|
||||||
|
|
||||||
|
def test_rejects_upload_staging_filename(self):
|
||||||
|
from app.channels.slack import SlackChannel
|
||||||
|
|
||||||
|
files = SlackChannel._extract_inbound_files(
|
||||||
|
{
|
||||||
|
"files": [
|
||||||
|
{
|
||||||
|
"id": "F1",
|
||||||
|
"name": ".upload-hidden.part",
|
||||||
|
"mimetype": "application/pdf",
|
||||||
|
"url_private_download": "https://files.slack.com/files-pri/T-F/download/x",
|
||||||
|
}
|
||||||
|
]
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
assert files[0]["filename"] == "slack-file-F1.bin"
|
||||||
|
|
||||||
|
|
||||||
|
class TestSlackReceiveFile:
|
||||||
|
def test_downloads_bytes_with_bearer_token_and_strips_url(self, monkeypatch):
|
||||||
|
from app.channels import slack as slack_mod
|
||||||
|
from app.channels.slack import SlackChannel
|
||||||
|
|
||||||
|
seen: dict[str, str] = {}
|
||||||
|
|
||||||
|
class _FakeStreamResponse:
|
||||||
|
def __init__(self):
|
||||||
|
self.headers = {"content-encoding": "identity"}
|
||||||
|
|
||||||
|
def raise_for_status(self) -> None:
|
||||||
|
return None
|
||||||
|
|
||||||
|
async def aiter_raw(self):
|
||||||
|
yield b"da"
|
||||||
|
yield b"ta"
|
||||||
|
|
||||||
|
class _FakeStreamContext:
|
||||||
|
async def __aenter__(self):
|
||||||
|
return _FakeStreamResponse()
|
||||||
|
|
||||||
|
async def __aexit__(self, *exc_info):
|
||||||
|
return False
|
||||||
|
|
||||||
|
class _FakeAsyncClient:
|
||||||
|
def __init__(self, *args, **kwargs):
|
||||||
|
pass
|
||||||
|
|
||||||
|
async def __aenter__(self):
|
||||||
|
return self
|
||||||
|
|
||||||
|
async def __aexit__(self, *exc_info):
|
||||||
|
return False
|
||||||
|
|
||||||
|
def stream(self, method: str, url: str, **kwargs):
|
||||||
|
seen["method"] = method
|
||||||
|
seen["url"] = url
|
||||||
|
seen["authorization"] = kwargs.get("headers", {}).get("Authorization", "")
|
||||||
|
seen["accept_encoding"] = kwargs.get("headers", {}).get("Accept-Encoding", "")
|
||||||
|
return _FakeStreamContext()
|
||||||
|
|
||||||
|
monkeypatch.setattr(slack_mod.httpx, "AsyncClient", _FakeAsyncClient)
|
||||||
|
|
||||||
|
async def go():
|
||||||
|
channel = SlackChannel(MessageBus(), {"bot_token": "xoxb-test"})
|
||||||
|
channel._web_client = SimpleNamespace(token="xoxb-test")
|
||||||
|
content = await channel._download_private_file(
|
||||||
|
"https://files.slack.com/files-pri/T-F/download/report.pdf",
|
||||||
|
"xoxb-test",
|
||||||
|
)
|
||||||
|
assert content == b"data"
|
||||||
|
assert seen["method"] == "GET"
|
||||||
|
assert seen["authorization"] == "Bearer xoxb-test"
|
||||||
|
assert seen["accept_encoding"] == "identity"
|
||||||
|
|
||||||
|
channel2 = SlackChannel(MessageBus(), {"bot_token": "xoxb-test"})
|
||||||
|
channel2._web_client = SimpleNamespace(token="xoxb-test")
|
||||||
|
|
||||||
|
async def fake_download(url: str, token: str, *, max_bytes: int = 0) -> bytes | None:
|
||||||
|
assert token == "xoxb-test"
|
||||||
|
assert url.endswith("/download/report.pdf")
|
||||||
|
assert max_bytes > 0
|
||||||
|
return b"data"
|
||||||
|
|
||||||
|
channel2._download_private_file = fake_download # type: ignore[method-assign]
|
||||||
|
msg = InboundMessage(
|
||||||
|
channel_name="slack",
|
||||||
|
chat_id="C123",
|
||||||
|
user_id="U123",
|
||||||
|
text="caption",
|
||||||
|
files=[
|
||||||
|
{
|
||||||
|
"type": "file",
|
||||||
|
"file_id": "FABC",
|
||||||
|
"filename": "report.pdf",
|
||||||
|
"mime_type": "application/pdf",
|
||||||
|
"size": 4,
|
||||||
|
"url_private_download": "https://files.slack.com/files-pri/T-F/download/report.pdf",
|
||||||
|
}
|
||||||
|
],
|
||||||
|
)
|
||||||
|
result = await channel2.receive_file(msg, "thread-1")
|
||||||
|
assert result.text == "caption"
|
||||||
|
assert result.files == [
|
||||||
|
{
|
||||||
|
"type": "file",
|
||||||
|
"filename": "report.pdf",
|
||||||
|
"mime_type": "application/pdf",
|
||||||
|
"size": 4,
|
||||||
|
INBOUND_FILE_CONTENT_KEY: b"data",
|
||||||
|
}
|
||||||
|
]
|
||||||
|
|
||||||
|
_run(go())
|
||||||
|
|
||||||
|
def test_missing_token_clears_files_and_notices(self):
|
||||||
|
from app.channels.slack import SlackChannel
|
||||||
|
|
||||||
|
channel = SlackChannel(MessageBus(), {})
|
||||||
|
channel._web_client = None
|
||||||
|
msg = InboundMessage(
|
||||||
|
channel_name="slack",
|
||||||
|
chat_id="C123",
|
||||||
|
user_id="U123",
|
||||||
|
text="caption",
|
||||||
|
files=[
|
||||||
|
{
|
||||||
|
"type": "file",
|
||||||
|
"filename": "report.pdf",
|
||||||
|
"mime_type": "application/pdf",
|
||||||
|
"url_private_download": "https://files.slack.com/files-pri/T-F/download/report.pdf",
|
||||||
|
}
|
||||||
|
],
|
||||||
|
)
|
||||||
|
|
||||||
|
result = _run(channel.receive_file(msg, "thread-1"))
|
||||||
|
|
||||||
|
assert result.files == []
|
||||||
|
assert "bot token missing" in result.text
|
||||||
|
|
||||||
|
def test_rejects_non_slack_host_without_fetching(self):
|
||||||
|
from app.channels.slack import SlackChannel
|
||||||
|
|
||||||
|
channel = SlackChannel(MessageBus(), {"bot_token": "xoxb-test"})
|
||||||
|
channel._web_client = SimpleNamespace(token="xoxb-test")
|
||||||
|
called = {"download": False}
|
||||||
|
|
||||||
|
async def fake_download(url: str, token: str, *, max_bytes: int = 0) -> bytes | None:
|
||||||
|
called["download"] = True
|
||||||
|
return b"nope"
|
||||||
|
|
||||||
|
channel._download_private_file = fake_download # type: ignore[method-assign]
|
||||||
|
msg = InboundMessage(
|
||||||
|
channel_name="slack",
|
||||||
|
chat_id="C123",
|
||||||
|
user_id="U123",
|
||||||
|
text="caption",
|
||||||
|
files=[
|
||||||
|
{
|
||||||
|
"type": "file",
|
||||||
|
"filename": "evil.pdf",
|
||||||
|
"mime_type": "application/pdf",
|
||||||
|
"size": 4,
|
||||||
|
"url_private_download": "https://evil.example/report.pdf",
|
||||||
|
}
|
||||||
|
],
|
||||||
|
)
|
||||||
|
result = _run(channel.receive_file(msg, "thread-1"))
|
||||||
|
assert called["download"] is False
|
||||||
|
assert result.files == []
|
||||||
|
assert "download unavailable" in result.text
|
||||||
|
|
||||||
|
def test_content_encoding_refusal_is_download_failed(self, monkeypatch):
|
||||||
|
from app.channels import slack as slack_mod
|
||||||
|
from app.channels.slack import SlackChannel
|
||||||
|
|
||||||
|
class _FakeStreamResponse:
|
||||||
|
def __init__(self):
|
||||||
|
self.headers = {"content-encoding": "gzip"}
|
||||||
|
|
||||||
|
def raise_for_status(self) -> None:
|
||||||
|
return None
|
||||||
|
|
||||||
|
async def aiter_raw(self):
|
||||||
|
if False:
|
||||||
|
yield b""
|
||||||
|
|
||||||
|
class _FakeStreamContext:
|
||||||
|
async def __aenter__(self):
|
||||||
|
return _FakeStreamResponse()
|
||||||
|
|
||||||
|
async def __aexit__(self, *exc_info):
|
||||||
|
return False
|
||||||
|
|
||||||
|
class _FakeAsyncClient:
|
||||||
|
def __init__(self, *args, **kwargs):
|
||||||
|
pass
|
||||||
|
|
||||||
|
async def __aenter__(self):
|
||||||
|
return self
|
||||||
|
|
||||||
|
async def __aexit__(self, *exc_info):
|
||||||
|
return False
|
||||||
|
|
||||||
|
def stream(self, method: str, url: str, **kwargs):
|
||||||
|
return _FakeStreamContext()
|
||||||
|
|
||||||
|
monkeypatch.setattr(slack_mod.httpx, "AsyncClient", _FakeAsyncClient)
|
||||||
|
|
||||||
|
channel = SlackChannel(MessageBus(), {"bot_token": "xoxb-test"})
|
||||||
|
channel._web_client = SimpleNamespace(token="xoxb-test")
|
||||||
|
msg = InboundMessage(
|
||||||
|
channel_name="slack",
|
||||||
|
chat_id="C123",
|
||||||
|
user_id="U123",
|
||||||
|
text="caption",
|
||||||
|
files=[
|
||||||
|
{
|
||||||
|
"type": "file",
|
||||||
|
"filename": "report.pdf",
|
||||||
|
"mime_type": "application/pdf",
|
||||||
|
"size": 4,
|
||||||
|
"url_private_download": "https://files.slack.com/files-pri/T-F/download/report.pdf",
|
||||||
|
}
|
||||||
|
],
|
||||||
|
)
|
||||||
|
result = _run(channel.receive_file(msg, "thread-1"))
|
||||||
|
assert result.files == []
|
||||||
|
assert "download failed" in result.text
|
||||||
|
assert "exceeds the download limit" not in result.text
|
||||||
|
|
||||||
|
def test_aggregate_message_byte_cap(self):
|
||||||
|
from app.channels import slack as slack_mod
|
||||||
|
from app.channels.slack import SlackChannel
|
||||||
|
|
||||||
|
channel = SlackChannel(MessageBus(), {"bot_token": "xoxb-test"})
|
||||||
|
channel._web_client = SimpleNamespace(token="xoxb-test")
|
||||||
|
payloads = {
|
||||||
|
"https://files.slack.com/a.bin": b"a" * 10,
|
||||||
|
"https://files.slack.com/b.bin": b"b" * 10,
|
||||||
|
}
|
||||||
|
|
||||||
|
async def fake_download(url: str, token: str, *, max_bytes: int = 0) -> bytes | None:
|
||||||
|
body = payloads[url]
|
||||||
|
if len(body) > max_bytes:
|
||||||
|
return None
|
||||||
|
return body
|
||||||
|
|
||||||
|
channel._download_private_file = fake_download # type: ignore[method-assign]
|
||||||
|
# Shrink aggregate so two tiny files exceed it.
|
||||||
|
monkey_limit = 15
|
||||||
|
original = slack_mod.SLACK_MAX_INBOUND_MESSAGE_BYTES
|
||||||
|
slack_mod.SLACK_MAX_INBOUND_MESSAGE_BYTES = monkey_limit
|
||||||
|
try:
|
||||||
|
msg = InboundMessage(
|
||||||
|
channel_name="slack",
|
||||||
|
chat_id="C123",
|
||||||
|
user_id="U123",
|
||||||
|
text="caption",
|
||||||
|
files=[
|
||||||
|
{
|
||||||
|
"type": "file",
|
||||||
|
"filename": "a.bin",
|
||||||
|
"mime_type": "application/octet-stream",
|
||||||
|
"size": 10,
|
||||||
|
"url_private_download": "https://files.slack.com/a.bin",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"type": "file",
|
||||||
|
"filename": "b.bin",
|
||||||
|
"mime_type": "application/octet-stream",
|
||||||
|
"size": 10,
|
||||||
|
"url_private_download": "https://files.slack.com/b.bin",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
)
|
||||||
|
result = _run(channel.receive_file(msg, "thread-1"))
|
||||||
|
finally:
|
||||||
|
slack_mod.SLACK_MAX_INBOUND_MESSAGE_BYTES = original
|
||||||
|
|
||||||
|
assert len(result.files) == 1
|
||||||
|
assert result.files[0]["filename"] == "a.bin"
|
||||||
|
assert "message download limit" in result.text
|
||||||
|
|
||||||
|
|
||||||
|
class TestSlackPrivateFileUrlAllowlist:
|
||||||
|
def test_allows_files_slack_com_https(self):
|
||||||
|
from app.channels.slack import SlackChannel
|
||||||
|
|
||||||
|
assert SlackChannel._is_allowed_private_file_url("https://files.slack.com/files-pri/T-F/download/x")
|
||||||
|
assert SlackChannel._is_allowed_private_file_url("https://cdn.files.slack.com/files-pri/T-F/download/x")
|
||||||
|
|
||||||
|
def test_rejects_http_and_foreign_hosts(self):
|
||||||
|
from app.channels.slack import SlackChannel
|
||||||
|
|
||||||
|
assert not SlackChannel._is_allowed_private_file_url("http://files.slack.com/files-pri/T-F/download/x")
|
||||||
|
assert not SlackChannel._is_allowed_private_file_url("https://evil.example/x")
|
||||||
|
assert not SlackChannel._is_allowed_private_file_url("https://notfiles.slack.com.evil.io/x")
|
||||||
Loading…
x
Reference in New Issue
Block a user