Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 32 additions & 2 deletions gateway/platforms/slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -3463,14 +3463,30 @@ async def _download_slack_file(
) -> str:
"""Download a Slack file using the bot token for auth, with retry."""
import httpx
from gateway.platforms.base import _ssrf_redirect_guard, safe_url_for_log
from tools.url_safety import is_safe_url

# SSRF guard: the download attaches the bot token, so a URL that
# resolves to (or 3xx-redirects into) a private/internal address would
# both leak the token and let the server reach internal services
# (CWE-918). The outbound send_image() path is already guarded; this
# is the inbound sibling that was missing the same protection.
if not is_safe_url(url):
raise ValueError(
f"Blocked unsafe Slack file URL (SSRF protection): {safe_url_for_log(url)}"
)

bot_token = (
self._team_clients[team_id].token
if team_id and team_id in self._team_clients
else self.config.token
)

async with httpx.AsyncClient(timeout=30.0, follow_redirects=True) as client:
async with httpx.AsyncClient(
timeout=30.0,
follow_redirects=True,
event_hooks={"response": [_ssrf_redirect_guard]},
) as client:
for attempt in range(3):
try:
response = await client.get(
Expand Down Expand Up @@ -3519,14 +3535,28 @@ async def _download_slack_file(
async def _download_slack_file_bytes(self, url: str, team_id: str = "") -> bytes:
"""Download a Slack file and return raw bytes, with retry."""
import httpx
from gateway.platforms.base import _ssrf_redirect_guard, safe_url_for_log
from tools.url_safety import is_safe_url

# SSRF guard (CWE-918): see _download_slack_file. This sibling path
# also attaches the bot token and must validate the destination plus
# every redirect hop.
if not is_safe_url(url):
raise ValueError(
f"Blocked unsafe Slack file URL (SSRF protection): {safe_url_for_log(url)}"
)

bot_token = (
self._team_clients[team_id].token
if team_id and team_id in self._team_clients
else self.config.token
)

async with httpx.AsyncClient(timeout=30.0, follow_redirects=True) as client:
async with httpx.AsyncClient(
timeout=30.0,
follow_redirects=True,
event_hooks={"response": [_ssrf_redirect_guard]},
) as client:
for attempt in range(3):
try:
response = await client.get(
Expand Down
104 changes: 104 additions & 0 deletions tests/gateway/test_slack_download_ssrf.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
"""SSRF regression tests for inbound Slack file downloads.

``_download_slack_file`` / ``_download_slack_file_bytes`` attach the bot
token and follow redirects, so they must validate the destination (CWE-918)
exactly like the already-guarded outbound ``send_image`` path: a pre-flight
``is_safe_url`` check plus a per-redirect guard.
"""
import asyncio
from types import SimpleNamespace

import pytest

from gateway.platforms.base import _ssrf_redirect_guard

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main moved SlackAdapter from gateway/platforms/slack.py to plugins/platforms/slack/adapter.py in 5600105478ffde29d7566b45421b100eaa29c4ef; this import must be retargeted along with the production change.

from gateway.platforms.slack import SlackAdapter


def _fake_adapter():
self = SlackAdapter.__new__(SlackAdapter)
self.config = SimpleNamespace(token="xoxb-test-token")
self._team_clients = {}
return self


class _NetworkTouched(RuntimeError):
pass


class _RecordingClient:
"""Captures AsyncClient kwargs; refuses to perform real I/O."""

last_kwargs = None

def __init__(self, **kwargs):
type(self).last_kwargs = kwargs

async def __aenter__(self):
return self

async def __aexit__(self, *exc):
return False

async def get(self, *args, **kwargs):
raise _NetworkTouched("network access attempted")


@pytest.mark.parametrize(
"method_name",
["_download_slack_file", "_download_slack_file_bytes"],
)
def test_unsafe_url_blocked_before_network(monkeypatch, method_name):
import tools.url_safety as url_safety

calls = {"checked": []}

def fake_is_safe_url(url, *a, **k):
calls["checked"].append(url)
return False

monkeypatch.setattr(url_safety, "is_safe_url", fake_is_safe_url)

# If the guard is bypassed, the fake client raises _NetworkTouched; a
# correct implementation raises ValueError *before* touching httpx.
monkeypatch.setattr("httpx.AsyncClient", _RecordingClient)

self = _fake_adapter()
method = getattr(self, method_name)
args = ("http://169.254.169.254/latest/meta-data/", ".jpg") \
if method_name == "_download_slack_file" \
else ("http://169.254.169.254/latest/meta-data/",)

with pytest.raises(ValueError):
asyncio.run(method(*args))

assert calls["checked"], "download must call is_safe_url before fetching"


@pytest.mark.parametrize(
"method_name",
["_download_slack_file", "_download_slack_file_bytes"],
)
def test_redirect_guard_is_wired(monkeypatch, method_name):
import tools.url_safety as url_safety

monkeypatch.setattr(url_safety, "is_safe_url", lambda *a, **k: True)
monkeypatch.setattr("httpx.AsyncClient", _RecordingClient)

self = _fake_adapter()
method = getattr(self, method_name)
args = ("https://files.slack.com/x.jpg", ".jpg") \
if method_name == "_download_slack_file" \
else ("https://files.slack.com/x.jpg",)

# The fake client raises when .get() is called; we only care that the
# client was constructed with the redirect guard hook.
with pytest.raises(_NetworkTouched):
asyncio.run(method(*args))

kwargs = _RecordingClient.last_kwargs
assert kwargs is not None
hooks = kwargs.get("event_hooks", {})
assert _ssrf_redirect_guard in hooks.get("response", []), (
"AsyncClient must register _ssrf_redirect_guard to block "
"redirect-based SSRF"
)
Loading