fix(adapters): stop SSRF and arbitrary local file reads in media handling (#2753)

The platform adapters fetch URLs and open paths that come from inbound
messages, so both are attacker-influenced.

New guard module pkg/utils/outbound_url.py:

- assert_safe_outbound_url: checks the scheme, rejects literal private /
  loopback / link-local / reserved / multicast addresses, and resolves the
  hostname so a public-looking name pointing at an internal address is refused
  too. Optional allowed_domains confines a fetch to a known prefix.
- assert_safe_local_media_path: resolves a caller-supplied path and requires it
  to stay inside the media roots (data dir + temp dir), so symlinks and .. cannot
  escape. Directories are refused.

Wired in:

- DingTalk send_message: session_webhook is taken from the inbound event and was
  POSTed verbatim, so a forged event could redirect reply content (including LLM
  output) to any host. Now restricted to dingtalk.com.
- DingTalk send_attachment: component.url was fetched with no destination check;
  now goes through the SSRF guard, which also covers cloud metadata.
- DingTalk upload_image_media: caller-supplied file_path was opened directly; now
  confined to the media roots.
- Slack attachment_bytes: component.path was opened directly and uploaded to the
  channel (arbitrary file exfiltration); now confined to the media roots.
- Slack download_media: redirects were followed with no destination check. Every
  hop is now validated, the hop count is bounded, and a redirect off the Slack
  domain drops the bearer token.

Tests: 25 new cases for the guard (internal address forms, scheme rejection,
suffix impostor, traversal, symlink escape, directory). The Slack credential test
now stubs DNS because the guard resolves before fetching, and asserts
allow_redirects is False for every case since redirects are handled manually.

Co-authored-by: multica-agent <[email protected]>

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('.')

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:

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 "<none>"}')
+    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}')

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

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),))