diff --git a/src/langbot/libs/dingtalk_api/api.py b/src/langbot/libs/dingtalk_api/api.py index 48032b72a..2eaff2447 100644 --- a/src/langbot/libs/dingtalk_api/api.py +++ b/src/langbot/libs/dingtalk_api/api.py @@ -18,12 +18,21 @@ from .thinking import card_text_fields, visible_think_markers import httpx import traceback from langbot.pkg.utils import httpclient +from langbot.pkg.utils.outbound_url import ( + UnsafeOutboundUrlError, + assert_safe_local_media_path, + assert_safe_outbound_url, +) _stdout_logger = logging.getLogger('langbot.dingtalk_api') DINGTALK_OPENAPI_BASE = 'https://api.dingtalk.com' +# Legitimate session webhooks come from DingTalk itself +# (https://oapi.dingtalk.com/robot/sendBySession?...). Anything else means the +# inbound event carried a forged destination. +DINGTALK_WEBHOOK_DOMAINS = ('dingtalk.com',) _MAX_MEDIA_BYTES = 10 * 1024 * 1024 _MAX_GATEWAY_MESSAGE_BYTES = 1024 * 1024 @@ -301,8 +310,14 @@ class DingTalkClient: else: payload = {'msgtype': 'text', 'text': {'content': content}} payload['at'] = {'atUserIds': [incoming_message.sender_staff_id]} + # The webhook URL arrives inside the inbound event, so verify it really + # points at DingTalk before posting the reply (and any LLM output) to it. + webhook_url = assert_safe_outbound_url( + incoming_message.session_webhook, + allowed_domains=DINGTALK_WEBHOOK_DOMAINS, + ) async with self._http_client_context() as client: - response = await client.post(incoming_message.session_webhook, json=payload, timeout=30.0) + response = await client.post(webhook_url, json=payload, timeout=30.0) return self._check_send_result(response) async def send_attachment(self, component, incoming_message): @@ -320,8 +335,12 @@ class DingTalkClient: elif getattr(component, 'path', None): body = await asyncio.to_thread(_read_local_media_limited, component.path) elif getattr(component, 'url', None): + # A message component can carry an arbitrary URL. Attachments are + # fetched from user-supplied hosts, so refuse anything that resolves + # to loopback, a private range, or link-local metadata. + media_url = assert_safe_outbound_url(component.url) async with self._http_client_context() as client: - async with client.stream('GET', component.url, timeout=30.0) as response: + async with client.stream('GET', media_url, timeout=30.0) as response: response.raise_for_status() body = await _read_httpx_media_limited(response) else: @@ -995,14 +1014,23 @@ class DingTalkClient: await self.logger.error(f'DingTalk upload_image_media: file not found {file_path}') return None + try: + # Caller-supplied path: confine it to the media roots so a plugin or + # external input cannot make the adapter upload arbitrary files. + safe_file_path = assert_safe_local_media_path(file_path) + except UnsafeOutboundUrlError as exc: + if self.logger: + await self.logger.error(f'DingTalk upload_image_media: rejected path: {exc}') + return None + token = await self.get_legacy_access_token() if not token: return None url = 'https://oapi.dingtalk.com/media/upload' try: - file_bytes = await asyncio.to_thread(_read_local_media_limited, file_path) - file_name = os.path.basename(file_path) + file_bytes = await asyncio.to_thread(_read_local_media_limited, safe_file_path) + file_name = os.path.basename(safe_file_path) # Best-effort content-type guess; DingTalk accepts the major image # mime types and otherwise infers from the bytes. ext = os.path.splitext(file_name)[1].lower().lstrip('.') diff --git a/src/langbot/libs/slack_api/media.py b/src/langbot/libs/slack_api/media.py index f220af14f..2d5f016ab 100644 --- a/src/langbot/libs/slack_api/media.py +++ b/src/langbot/libs/slack_api/media.py @@ -3,14 +3,16 @@ import asyncio import mimetypes from pathlib import Path -from urllib.parse import urlparse, unquote +from urllib.parse import unquote, urljoin, urlparse import aiohttp from langbot.pkg.utils import httpclient, image +from langbot.pkg.utils.outbound_url import assert_safe_local_media_path, assert_safe_outbound_url from langbot_plugin.api.entities.builtin.platform import message as pm MAX_MEDIA_BYTES = 20 * 1024 * 1024 +_MAX_REDIRECT_HOPS = 5 MEDIA_TYPES = (pm.Image, pm.File, pm.Voice) @@ -21,13 +23,29 @@ async def download_media(url: str, token: str = '') -> tuple[bytes, str]: # 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] + # Follow redirects manually so every hop is checked: a public URL that + # redirects to an internal address would otherwise bypass the guard. + current = assert_safe_outbound_url(url) + for _hop in range(_MAX_REDIRECT_HOPS): + async with httpclient.get_session().get( + current, + headers=headers, + allow_redirects=False, + timeout=aiohttp.ClientTimeout(total=30), + ) as response: + if response.status in (301, 302, 303, 307, 308): + location = response.headers.get('Location') + if not location: + raise ValueError('Slack media redirect had no Location') + current = assert_safe_outbound_url(urljoin(current, location)) + # A redirect leaves the Slack domain: do not carry the token. + headers = {} + continue + 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] + raise ValueError('Slack media download exceeded the redirect limit') async def attachment_bytes(component, token: str = '') -> tuple[bytes, str]: @@ -40,7 +58,11 @@ async def attachment_bytes(component, token: str = '') -> tuple[bytes, str]: mime = header[5:].split(';')[0] data = await image.decode_base64_limited(value, max_bytes=MAX_MEDIA_BYTES) elif component.path: - path = Path(component.path) + # A message component can carry an arbitrary path. Reading it blindly + # would let a crafted component exfiltrate any file the process can read + # to the chat platform, so confine it to the media roots. + safe_path = assert_safe_local_media_path(component.path) + path = Path(safe_path) def read(): with path.open('rb') as stream: diff --git a/src/langbot/pkg/utils/outbound_url.py b/src/langbot/pkg/utils/outbound_url.py new file mode 100644 index 000000000..f9d3e9a3e --- /dev/null +++ b/src/langbot/pkg/utils/outbound_url.py @@ -0,0 +1,166 @@ +"""Outbound URL safety checks for server-side fetches (SSRF guard). + +The platform adapters fetch URLs that come from inbound platform messages +(attachment URLs, session webhooks, media links). Those values are attacker +influenced, so the server must not be tricked into requesting internal +infrastructure: cloud metadata (169.254.169.254), the cluster network, or +loopback services. + +Call :func:`assert_safe_outbound_url` before every such request. It checks the +scheme, rejects literal private/loopback/link-local addresses, and resolves the +hostname so a public-looking name that points at an internal address is +rejected too. +""" + +from __future__ import annotations + +import ipaddress +import socket +import tempfile +import urllib.parse +from pathlib import Path + +_ALLOWED_SCHEMES = ('http', 'https') + + +class UnsafeOutboundUrlError(ValueError): + """Raised when an outbound URL would reach a non-public destination.""" + + +def _reject_reason(ip: ipaddress.IPv4Address | ipaddress.IPv6Address) -> str | None: + """Return why this address is not a safe outbound destination, if it is not.""" + mapped = getattr(ip, 'ipv4_mapped', None) + if mapped is not None: + ip = mapped + + if ip.is_private: + return 'private address' + if ip.is_loopback: + return 'loopback address' + if ip.is_link_local: + return 'link-local address' + if ip.is_multicast: + return 'multicast address' + if ip.is_reserved: + return 'reserved address' + if ip.is_unspecified: + return 'unspecified address' + return None + + +def _parse_and_check_scheme(url: str) -> urllib.parse.SplitResult: + parsed = urllib.parse.urlsplit(url) + if parsed.scheme.lower() not in _ALLOWED_SCHEMES: + raise UnsafeOutboundUrlError(f'unsupported URL scheme: {parsed.scheme or ""}') + if not parsed.hostname: + raise UnsafeOutboundUrlError('URL has no host') + return parsed + + +def _host_allowed(host: str, allowed_domains: tuple[str, ...] | None) -> bool: + if allowed_domains is None: + return True + host = host.lower().rstrip('.') + for domain in allowed_domains: + domain = domain.lower().lstrip('.') + if host == domain or host.endswith('.' + domain): + return True + return False + + +def assert_safe_outbound_url( + url: str, + *, + allowed_domains: tuple[str, ...] | None = None, +) -> str: + """Validate an outbound URL, returning it unchanged when it is safe. + + ``allowed_domains`` restricts the destination to those domain suffixes (used + where the legitimate destination is a fixed platform domain). When omitted, + any public destination is allowed but internal addresses are still refused. + """ + if not isinstance(url, str) or not url.strip(): + raise UnsafeOutboundUrlError('URL is empty') + + parsed = _parse_and_check_scheme(url) + host = parsed.hostname or '' + + if not _host_allowed(host, allowed_domains): + raise UnsafeOutboundUrlError(f'host is not an allowed destination: {host}') + + try: + literal = ipaddress.ip_address(host) + except ValueError: + literal = None + + if literal is not None: + reason = _reject_reason(literal) + if reason is not None: + raise UnsafeOutboundUrlError(f'refusing to fetch {host}: {reason}') + return url + + # Resolve so a public-looking name pointing at an internal address is caught. + try: + infos = socket.getaddrinfo(host, parsed.port or (443 if parsed.scheme == 'https' else 80)) + except socket.gaierror as exc: + raise UnsafeOutboundUrlError(f'cannot resolve host {host}: {exc}') from exc + + addresses = {info[4][0] for info in infos} + if not addresses: + raise UnsafeOutboundUrlError(f'host {host} did not resolve to an address') + + for address in addresses: + try: + ip = ipaddress.ip_address(address) + except ValueError: + continue + reason = _reject_reason(ip) + if reason is not None: + raise UnsafeOutboundUrlError(f'refusing to fetch {host} ({address}): {reason}') + + return url + + +def assert_safe_local_media_path(file_path: str, *, allowed_roots: tuple[str, ...] | None = None) -> str: + """Validate a local file path used as outbound media, returning it resolved. + + Message components can carry a ``path``/``file_path`` field. A platform + adapter that opens it blindly lets a crafted component read any file the + process can see (``/etc/passwd``, credentials, another tenant's data) and + upload it to the chat platform. + + The path must stay inside one of ``allowed_roots`` — by default the LangBot + data directory and the system temporary directory, which is where plugins and + the pipeline legitimately write generated media. Symlinks are resolved first, + so a link pointing outside the root is rejected too. + """ + from langbot.pkg.utils.paths import get_data_root + + if not isinstance(file_path, str) or not file_path.strip(): + raise UnsafeOutboundUrlError('file path is empty') + + roots = allowed_roots or (get_data_root(), tempfile.gettempdir()) + resolved_roots = [] + for root in roots: + try: + resolved_roots.append(Path(root).resolve()) + except OSError: # pragma: no cover - defensive + continue + + candidate = Path(file_path) + if not candidate.is_absolute(): + # Relative paths resolve against an unpredictable cwd; refuse them. + raise UnsafeOutboundUrlError(f'refusing to read a relative media path: {file_path}') + + try: + resolved = candidate.resolve(strict=True) + except (OSError, RuntimeError) as exc: + raise UnsafeOutboundUrlError(f'cannot resolve media path: {file_path}') from exc + + for root in resolved_roots: + if resolved == root or root in resolved.parents: + if not resolved.is_file(): + raise UnsafeOutboundUrlError(f'media path is not a regular file: {file_path}') + return str(resolved) + + raise UnsafeOutboundUrlError(f'refusing to read media path outside the allowed roots: {file_path}') diff --git a/tests/unit_tests/platform/test_slack_media.py b/tests/unit_tests/platform/test_slack_media.py index 3238c3e7c..edf58aaff 100644 --- a/tests/unit_tests/platform/test_slack_media.py +++ b/tests/unit_tests/platform/test_slack_media.py @@ -1,3 +1,4 @@ +import socket import base64 from types import SimpleNamespace from unittest.mock import AsyncMock, patch @@ -188,13 +189,18 @@ async def test_download_credentials_are_scoped_to_slack(url, authenticated): 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')), + # The SSRF guard resolves the host before fetching; the fake hosts in this + # test have no DNS record, so resolution is stubbed to a public address. + patch( + 'langbot.pkg.utils.outbound_url.socket.getaddrinfo', + return_value=[(socket.AF_INET, socket.SOCK_STREAM, 6, '', ('93.184.216.34', 443))], + ), ): 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 + assert kwargs['allow_redirects'] is False @pytest.mark.asyncio diff --git a/tests/unit_tests/utils/test_outbound_url_guard.py b/tests/unit_tests/utils/test_outbound_url_guard.py new file mode 100644 index 000000000..77bfabf9b --- /dev/null +++ b/tests/unit_tests/utils/test_outbound_url_guard.py @@ -0,0 +1,111 @@ +"""Outbound fetch guards: SSRF (url) and arbitrary local file reads (path).""" + +import socket + +import pytest + +from langbot.pkg.utils.outbound_url import ( + UnsafeOutboundUrlError, + assert_safe_local_media_path, + assert_safe_outbound_url, +) + + +@pytest.mark.parametrize( + 'url', + [ + 'http://127.0.0.1:5300/api', + 'http://localhost/x', + 'http://10.42.1.5:5300/', + 'http://192.168.1.1/', + 'http://172.16.0.10/', + 'http://169.254.169.254/latest/meta-data/', + 'http://[::1]/', + 'http://0.0.0.0/', + ], +) +def test_internal_addresses_are_refused(url): + with pytest.raises(UnsafeOutboundUrlError): + assert_safe_outbound_url(url) + + +@pytest.mark.parametrize('url', ['file:///etc/passwd', 'gopher://x/', 'ftp://example.com/', '', ' ']) +def test_non_http_schemes_and_empty_are_refused(url): + with pytest.raises(UnsafeOutboundUrlError): + assert_safe_outbound_url(url) + + +def test_public_url_is_allowed(): + assert assert_safe_outbound_url('https://example.com/image.png') == 'https://example.com/image.png' + + +def test_hostname_resolving_to_internal_is_refused(monkeypatch): + """A public-looking name pointing at a private address must be refused.""" + monkeypatch.setattr( + socket, + 'getaddrinfo', + lambda *a, **k: [(socket.AF_INET, socket.SOCK_STREAM, 6, '', ('10.0.0.7', 80))], + ) + with pytest.raises(UnsafeOutboundUrlError): + assert_safe_outbound_url('http://internal.example.com/x') + + +def test_allowed_domains_restricts_destination(monkeypatch): + monkeypatch.setattr( + socket, + 'getaddrinfo', + lambda *a, **k: [(socket.AF_INET, socket.SOCK_STREAM, 6, '', ('93.184.216.34', 443))], + ) + assert assert_safe_outbound_url('https://oapi.dingtalk.com/robot/sendBySession', allowed_domains=('dingtalk.com',)) + with pytest.raises(UnsafeOutboundUrlError): + assert_safe_outbound_url('https://evil.example.com/x', allowed_domains=('dingtalk.com',)) + + +def test_allowed_domain_does_not_match_suffix_impostor(monkeypatch): + """evil-dingtalk.com must not satisfy the dingtalk.com rule.""" + monkeypatch.setattr( + socket, + 'getaddrinfo', + lambda *a, **k: [(socket.AF_INET, socket.SOCK_STREAM, 6, '', ('93.184.216.34', 443))], + ) + with pytest.raises(UnsafeOutboundUrlError): + assert_safe_outbound_url('https://evil-dingtalk.com/x', allowed_domains=('dingtalk.com',)) + + +def test_local_media_path_inside_root_is_allowed(tmp_path): + media = tmp_path / 'image.png' + media.write_bytes(b'png') + assert assert_safe_local_media_path(str(media), allowed_roots=(str(tmp_path),)) == str(media.resolve()) + + +@pytest.mark.parametrize('path', ['/etc/passwd', '/etc/shadow', '//etc/passwd']) +def test_local_media_path_outside_root_is_refused(path): + with pytest.raises(UnsafeOutboundUrlError): + assert_safe_local_media_path(path, allowed_roots=('/tmp',)) + + +def test_relative_local_media_path_is_refused(): + with pytest.raises(UnsafeOutboundUrlError): + assert_safe_local_media_path('../../etc/passwd', allowed_roots=('/tmp',)) + + +def test_traversal_out_of_root_is_refused(tmp_path): + (tmp_path / 'sub').mkdir() + with pytest.raises(UnsafeOutboundUrlError): + assert_safe_local_media_path(str(tmp_path / 'sub' / '..' / '..' / 'etc' / 'passwd'), allowed_roots=(str(tmp_path),)) + + +def test_symlink_escaping_root_is_refused(tmp_path): + outside = tmp_path / 'secret.txt' + outside.write_text('secret') + root = tmp_path / 'root' + root.mkdir() + link = root / 'link.txt' + link.symlink_to(outside) + with pytest.raises(UnsafeOutboundUrlError): + assert_safe_local_media_path(str(link), allowed_roots=(str(root),)) + + +def test_directory_is_not_accepted_as_media(tmp_path): + with pytest.raises(UnsafeOutboundUrlError): + assert_safe_local_media_path(str(tmp_path), allowed_roots=(str(tmp_path),))