fix(slack): validate file downloads against SSRF

_download_slack_file fetched url_private_download with follow_redirects=True and no SSRF validation, unlike the shared network guard used across other channels (napcat/dingtalk/qq) and the maintainer's recent image-download hardening. A file URL that pointed at — or redirected to — an internal address let the bot server issue requests to loopback/RFC1918/cloud-metadata targets, and cross-host redirects could carry the request off Slack. Validate the URL through validate_url_target before requesting, stop following redirects, and reject a redirect response. Authorized Slack file downloads return 200 directly, so normal downloads (which still send the bot token) are unaffected; the HTML-login guard already handled the redirect-to- login case.
2026-08-31 16:21:50 +03:00 · 2026-08-22 01:34:34 +08:00
parent 7604dc47f9
commit a4acd83966
2 changed files with 138 additions and 1 deletions
@@ -21,6 +21,11 @@ from nanobot.channels.base import BaseChannel
 from nanobot.config.paths import get_media_dir
 from nanobot.config.schema import Base
 from nanobot.pairing import is_approved
+from nanobot.security.network import (
+    PinnedDNSAsyncTransport,
+    httpx_env_proxy_mounts,
+    validate_url_target,
+)
 from nanobot.utils.helpers import safe_filename, split_message


@@ -89,6 +94,13 @@ SLACK_SOCKET_CONNECT_TIMEOUT_S = 45.0
 _HTML_DOWNLOAD_PREFIXES = (b"<!doctype html", b"<html")


+async def _validate_slack_download_request(request: httpx.Request) -> None:
+    """Validate every Slack file request, including redirects, before transport."""
+    ok, error = validate_url_target(str(request.url))
+    if not ok:
+        raise httpx.RequestError(f"unsafe Slack file URL: {error}", request=request)
+
+
 class SlackChannel(BaseChannel):
    """Slack channel using Socket Mode."""

@@ -562,7 +574,13 @@ class SlackChannel(BaseChannel):
        filename = safe_filename(f"{file_id}_{name}")
        path = Path(get_media_dir("slack")) / filename
        try:
-            async with httpx.AsyncClient(timeout=SLACK_DOWNLOAD_TIMEOUT, follow_redirects=True) as client:
+            async with httpx.AsyncClient(
+                timeout=SLACK_DOWNLOAD_TIMEOUT,
+                follow_redirects=True,
+                transport=PinnedDNSAsyncTransport(),
+                mounts=httpx_env_proxy_mounts(),
+                event_hooks={"request": [_validate_slack_download_request]},
+            ) as client:
                response = await client.get(
                    url,
                    headers={"Authorization": f"Bearer {self.config.bot_token}"},
@@ -1,5 +1,7 @@
 from __future__ import annotations

+from collections.abc import Callable
+from pathlib import Path
 from types import SimpleNamespace
 from unittest.mock import AsyncMock

@@ -837,3 +839,120 @@ def test_to_mrkdwn_still_converts_unfenced_markdown_tables() -> None:

    assert "| a | b |" not in out
    assert "a" in out and "1" in out and "b" in out and "2" in out
+
+
+# ── file download SSRF ─────────────────────────────────────────────
+
+
+def _patch_download_transport(
+    monkeypatch: pytest.MonkeyPatch,
+    handler: Callable[[httpx.Request], httpx.Response],
+) -> None:
+    monkeypatch.setattr(
+        "nanobot.channels.slack.runtime.PinnedDNSAsyncTransport",
+        lambda: httpx.MockTransport(handler),
+    )
+    monkeypatch.setattr("nanobot.channels.slack.runtime.httpx_env_proxy_mounts", lambda: {})
+
+
+def _patch_download_validation(
+    monkeypatch: pytest.MonkeyPatch,
+    validated: list[str],
+) -> None:
+    def validate(url: str) -> tuple[bool, str]:
+        validated.append(url)
+        if "169.254.169.254" in url:
+            return False, "blocked metadata address"
+        return True, ""
+
+    monkeypatch.setattr("nanobot.channels.slack.runtime.validate_url_target", validate)
+
+
+@pytest.mark.asyncio
+async def test_download_blocks_ssrf_target(monkeypatch: pytest.MonkeyPatch) -> None:
+    """An internal file URL is rejected before the transport sees it."""
+    requests: list[httpx.Request] = []
+    validated: list[str] = []
+
+    def handler(request: httpx.Request) -> httpx.Response:
+        requests.append(request)
+        return httpx.Response(200, content=b"should not be fetched")
+
+    _patch_download_transport(monkeypatch, handler)
+    _patch_download_validation(monkeypatch, validated)
+    channel = SlackChannel(SlackConfig(enabled=True, bot_token="xoxb-test"), MessageBus())
+    url = "http://169.254.169.254/latest/meta-data/"
+
+    path, _marker = await channel._download_slack_file(
+        {"id": "F1", "name": "x.bin", "url_private_download": url}
+    )
+
+    assert path is None
+    assert requests == []
+    assert validated == [url]
+
+
+@pytest.mark.asyncio
+async def test_download_blocks_unsafe_redirect(monkeypatch: pytest.MonkeyPatch) -> None:
+    """Redirect targets are validated before the redirected request is sent."""
+    requests: list[httpx.Request] = []
+    validated: list[str] = []
+
+    def handler(request: httpx.Request) -> httpx.Response:
+        requests.append(request)
+        return httpx.Response(
+            302,
+            headers={"location": "http://169.254.169.254/latest/meta-data/"},
+        )
+
+    _patch_download_transport(monkeypatch, handler)
+    _patch_download_validation(monkeypatch, validated)
+    channel = SlackChannel(SlackConfig(enabled=True, bot_token="xoxb-test"), MessageBus())
+    url = "https://files.slack.com/files-pri/x"
+
+    path, _marker = await channel._download_slack_file(
+        {"id": "F1", "name": "x.bin", "url_private_download": url}
+    )
+
+    assert path is None
+    assert len(requests) == 1
+    assert validated == [url, "http://169.254.169.254/latest/meta-data/"]
+
+
+@pytest.mark.asyncio
+async def test_download_follows_safe_redirect(
+    monkeypatch: pytest.MonkeyPatch,
+    tmp_path: Path,
+) -> None:
+    """Public redirects still download the file without forwarding cross-host auth."""
+    requests: list[httpx.Request] = []
+    validated: list[str] = []
+
+    def handler(request: httpx.Request) -> httpx.Response:
+        requests.append(request)
+        if request.url.host == "files.slack.com":
+            return httpx.Response(302, headers={"location": "https://cdn.example/file.bin"})
+        return httpx.Response(
+            200,
+            content=b"filedata",
+            headers={"content-type": "application/octet-stream"},
+        )
+
+    _patch_download_transport(monkeypatch, handler)
+    _patch_download_validation(monkeypatch, validated)
+    monkeypatch.setattr(
+        "nanobot.channels.slack.runtime.get_media_dir", lambda _channel=None: str(tmp_path)
+    )
+    channel = SlackChannel(SlackConfig(enabled=True, bot_token="xoxb-test"), MessageBus())
+    url = "https://files.slack.com/files-pri/x"
+
+    path, marker = await channel._download_slack_file(
+        {"id": "F1", "name": "x.bin", "url_private_download": url}
+    )
+
+    assert path is not None
+    assert Path(path).read_bytes() == b"filedata"
+    assert marker == "[file: x.bin]"
+    assert validated == [url, "https://cdn.example/file.bin"]
+    assert requests[0].headers["Authorization"] == "Bearer xoxb-test"
+    assert "Authorization" not in requests[1].headers