mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-28 18:19:28 +00:00
fix(platforms): block image upload redirects to private URLs
This commit is contained in:
dsad
Teknium
parent
4ee1d0de86
commit
0d6717e5aa
3 changed files with 373 additions and 63 deletions
|
|
@ -26,6 +26,7 @@ import time
|
|||
from collections import defaultdict
|
||||
from contextlib import suppress
|
||||
from typing import Callable, Dict, List, Optional, Any, Tuple
|
||||
from urllib.parse import urljoin
|
||||
|
||||
from agent.async_utils import (
|
||||
consume_detached_task_result as _consume_background_task_result,
|
||||
|
|
@ -67,6 +68,8 @@ _DISCORD_NONCONVERSATIONAL_METADATA_KEYS = frozenset({
|
|||
"non_conversational",
|
||||
"non_conversational_history",
|
||||
})
|
||||
_DISCORD_IMAGE_REDIRECT_STATUSES = {301, 302, 303, 307, 308}
|
||||
_DISCORD_IMAGE_MAX_REDIRECTS = 10
|
||||
# Upgrade-bridge fallback only. The primary mechanism is the persisted
|
||||
# non-conversational message-ID set populated from explicitly marked sends
|
||||
# (metadata["non_conversational"]). These regexes exist solely to recognize
|
||||
|
|
@ -135,6 +138,43 @@ from gateway.platforms.base import (
|
|||
from tools.url_safety import is_safe_url
|
||||
|
||||
|
||||
async def _read_url_image_with_redirect_guard(
|
||||
session: Any,
|
||||
url: str,
|
||||
*,
|
||||
timeout: Any,
|
||||
request_kwargs: Dict[str, Any],
|
||||
) -> Tuple[int, bytes, Dict[str, str]]:
|
||||
"""Read an image URL while re-checking every redirect target for SSRF."""
|
||||
current_url = url
|
||||
for _ in range(_DISCORD_IMAGE_MAX_REDIRECTS + 1):
|
||||
if not is_safe_url(current_url):
|
||||
raise ValueError("Blocked unsafe image URL redirect")
|
||||
|
||||
async with session.get(
|
||||
current_url,
|
||||
timeout=timeout,
|
||||
allow_redirects=False,
|
||||
**request_kwargs,
|
||||
) as resp:
|
||||
raw_headers = getattr(resp, "headers", {}) or {}
|
||||
headers = {str(key).lower(): value for key, value in dict(raw_headers).items()}
|
||||
status = int(getattr(resp, "status", 0))
|
||||
if status in _DISCORD_IMAGE_REDIRECT_STATUSES:
|
||||
location = headers.get("location")
|
||||
if not location:
|
||||
return status, b"", headers
|
||||
next_url = urljoin(current_url, str(location))
|
||||
if not is_safe_url(next_url):
|
||||
raise ValueError("Blocked redirect to private/internal address")
|
||||
current_url = next_url
|
||||
continue
|
||||
|
||||
return status, await resp.read(), headers
|
||||
|
||||
raise ValueError("Too many image URL redirects")
|
||||
|
||||
|
||||
def _truncate_discord_component_text(text: str, limit: int) -> str:
|
||||
"""Return text within Discord's UTF-16 component field budget."""
|
||||
return _prefix_within_utf16_limit(str(text or ""), max(0, limit))
|
||||
|
|
@ -3396,25 +3436,27 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||
_sess_kw, _req_kw = proxy_kwargs_for_aiohttp(_proxy)
|
||||
if aiohttp_session is None:
|
||||
aiohttp_session = _aiohttp.ClientSession(**_sess_kw)
|
||||
async with aiohttp_session.get(
|
||||
image_url, timeout=_aiohttp.ClientTimeout(total=30), **_req_kw,
|
||||
) as resp:
|
||||
if resp.status != 200:
|
||||
logger.warning(
|
||||
"[%s] Failed to download image (HTTP %d) in batch: %s",
|
||||
self.name, resp.status, image_url[:80],
|
||||
)
|
||||
continue
|
||||
data = await resp.read()
|
||||
ct = resp.headers.get("content-type", "image/png")
|
||||
ext = "png"
|
||||
if "jpeg" in ct or "jpg" in ct:
|
||||
ext = "jpg"
|
||||
elif "gif" in ct:
|
||||
ext = "gif"
|
||||
elif "webp" in ct:
|
||||
ext = "webp"
|
||||
files.append(_discord_mod.File(_io.BytesIO(data), filename=f"image_{len(files)}.{ext}"))
|
||||
status, data, headers = await _read_url_image_with_redirect_guard(
|
||||
aiohttp_session,
|
||||
image_url,
|
||||
timeout=_aiohttp.ClientTimeout(total=30),
|
||||
request_kwargs=_req_kw,
|
||||
)
|
||||
if status != 200:
|
||||
logger.warning(
|
||||
"[%s] Failed to download image (HTTP %d) in batch: %s",
|
||||
self.name, status, image_url[:80],
|
||||
)
|
||||
continue
|
||||
ct = headers.get("content-type", "image/png")
|
||||
ext = "png"
|
||||
if "jpeg" in ct or "jpg" in ct:
|
||||
ext = "jpg"
|
||||
elif "gif" in ct:
|
||||
ext = "gif"
|
||||
elif "webp" in ct:
|
||||
ext = "webp"
|
||||
files.append(_discord_mod.File(_io.BytesIO(data), filename=f"image_{len(files)}.{ext}"))
|
||||
except Exception as dl_err:
|
||||
logger.warning("[%s] Download failed for %s: %s", self.name, image_url[:80], dl_err)
|
||||
continue
|
||||
|
|
@ -4531,37 +4573,40 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||
_proxy = resolve_proxy_url(platform_env_var="DISCORD_PROXY")
|
||||
_sess_kw, _req_kw = proxy_kwargs_for_aiohttp(_proxy)
|
||||
async with aiohttp.ClientSession(**_sess_kw) as session:
|
||||
async with session.get(image_url, timeout=aiohttp.ClientTimeout(total=30), **_req_kw) as resp:
|
||||
if resp.status != 200:
|
||||
raise Exception(f"Failed to download image: HTTP {resp.status}")
|
||||
status, image_data, headers = await _read_url_image_with_redirect_guard(
|
||||
session,
|
||||
image_url,
|
||||
timeout=aiohttp.ClientTimeout(total=30),
|
||||
request_kwargs=_req_kw,
|
||||
)
|
||||
if status != 200:
|
||||
raise Exception(f"Failed to download image: HTTP {status}")
|
||||
|
||||
image_data = await resp.read()
|
||||
# Determine filename from URL or content type
|
||||
content_type = headers.get("content-type", "image/png")
|
||||
ext = "png"
|
||||
if "jpeg" in content_type or "jpg" in content_type:
|
||||
ext = "jpg"
|
||||
elif "gif" in content_type:
|
||||
ext = "gif"
|
||||
elif "webp" in content_type:
|
||||
ext = "webp"
|
||||
|
||||
# Determine filename from URL or content type
|
||||
content_type = resp.headers.get("content-type", "image/png")
|
||||
ext = "png"
|
||||
if "jpeg" in content_type or "jpg" in content_type:
|
||||
ext = "jpg"
|
||||
elif "gif" in content_type:
|
||||
ext = "gif"
|
||||
elif "webp" in content_type:
|
||||
ext = "webp"
|
||||
import io
|
||||
file = discord.File(io.BytesIO(image_data), filename=f"image.{ext}")
|
||||
|
||||
import io
|
||||
file = discord.File(io.BytesIO(image_data), filename=f"image.{ext}")
|
||||
|
||||
if self._is_forum_parent(channel):
|
||||
return await self._forum_post_file(
|
||||
channel,
|
||||
content=(caption or "").strip(),
|
||||
file=file,
|
||||
)
|
||||
|
||||
msg = await channel.send(
|
||||
content=caption if caption else None,
|
||||
if self._is_forum_parent(channel):
|
||||
return await self._forum_post_file(
|
||||
channel,
|
||||
content=(caption or "").strip(),
|
||||
file=file,
|
||||
)
|
||||
return SendResult(success=True, message_id=str(msg.id))
|
||||
|
||||
msg = await channel.send(
|
||||
content=caption if caption else None,
|
||||
file=file,
|
||||
)
|
||||
return SendResult(success=True, message_id=str(msg.id))
|
||||
|
||||
except ImportError:
|
||||
logger.warning(
|
||||
|
|
@ -4610,27 +4655,30 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||
_proxy = resolve_proxy_url(platform_env_var="DISCORD_PROXY")
|
||||
_sess_kw, _req_kw = proxy_kwargs_for_aiohttp(_proxy)
|
||||
async with aiohttp.ClientSession(**_sess_kw) as session:
|
||||
async with session.get(animation_url, timeout=aiohttp.ClientTimeout(total=30), **_req_kw) as resp:
|
||||
if resp.status != 200:
|
||||
raise Exception(f"Failed to download animation: HTTP {resp.status}")
|
||||
status, animation_data, _headers = await _read_url_image_with_redirect_guard(
|
||||
session,
|
||||
animation_url,
|
||||
timeout=aiohttp.ClientTimeout(total=30),
|
||||
request_kwargs=_req_kw,
|
||||
)
|
||||
if status != 200:
|
||||
raise Exception(f"Failed to download animation: HTTP {status}")
|
||||
|
||||
animation_data = await resp.read()
|
||||
import io
|
||||
file = discord.File(io.BytesIO(animation_data), filename="animation.gif")
|
||||
|
||||
import io
|
||||
file = discord.File(io.BytesIO(animation_data), filename="animation.gif")
|
||||
|
||||
if self._is_forum_parent(channel):
|
||||
return await self._forum_post_file(
|
||||
channel,
|
||||
content=(caption or "").strip(),
|
||||
file=file,
|
||||
)
|
||||
|
||||
msg = await channel.send(
|
||||
content=caption if caption else None,
|
||||
if self._is_forum_parent(channel):
|
||||
return await self._forum_post_file(
|
||||
channel,
|
||||
content=(caption or "").strip(),
|
||||
file=file,
|
||||
)
|
||||
return SendResult(success=True, message_id=str(msg.id))
|
||||
|
||||
msg = await channel.send(
|
||||
content=caption if caption else None,
|
||||
file=file,
|
||||
)
|
||||
return SendResult(success=True, message_id=str(msg.id))
|
||||
|
||||
except ImportError:
|
||||
logger.warning(
|
||||
|
|
|
|||
|
|
@ -52,6 +52,7 @@ from gateway.platforms.base import (
|
|||
is_host_excluded_by_no_proxy,
|
||||
resolve_proxy_url,
|
||||
safe_url_for_log,
|
||||
_ssrf_redirect_guard,
|
||||
cache_document_from_bytes,
|
||||
cache_video_from_bytes,
|
||||
)
|
||||
|
|
@ -2297,7 +2298,9 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
initial_comment_parts: List[str] = []
|
||||
try:
|
||||
async with _httpx.AsyncClient(
|
||||
timeout=30.0, follow_redirects=True
|
||||
timeout=30.0,
|
||||
follow_redirects=True,
|
||||
event_hooks={"response": [_ssrf_redirect_guard]},
|
||||
) as http_client:
|
||||
for image_url, alt_text in chunk:
|
||||
if alt_text:
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ Signal's native implementation is covered by test_signal.py.
|
|||
|
||||
import asyncio
|
||||
import sys
|
||||
import types
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
|
@ -264,6 +265,221 @@ class TestDiscordMultiImage:
|
|||
adapter._client = MagicMock()
|
||||
_run(adapter.send_multiple_images("67890", []))
|
||||
|
||||
def test_url_batch_blocks_private_redirect_before_send(self, adapter, monkeypatch):
|
||||
"""A public image URL must not redirect into private metadata and then upload."""
|
||||
import plugins.platforms.discord.adapter as discord_adapter
|
||||
|
||||
public_url = "https://cdn.example.test/image.png"
|
||||
private_url = "http://169.254.169.254/latest/meta-data/"
|
||||
safe_calls = []
|
||||
|
||||
def fake_is_safe_url(url):
|
||||
safe_calls.append(url)
|
||||
return not str(url).startswith("http://169.254.169.254")
|
||||
|
||||
class FakeResponse:
|
||||
status = 302
|
||||
headers = {"location": private_url}
|
||||
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, exc_type, exc, tb):
|
||||
return False
|
||||
|
||||
async def read(self):
|
||||
return b"metadata-secret"
|
||||
|
||||
class FakeSession:
|
||||
def get(self, url, **kwargs):
|
||||
assert kwargs.get("allow_redirects") is False
|
||||
return FakeResponse()
|
||||
|
||||
async def close(self):
|
||||
return None
|
||||
|
||||
fake_aiohttp = types.SimpleNamespace(
|
||||
ClientSession=lambda **kwargs: FakeSession(),
|
||||
ClientTimeout=lambda **kwargs: kwargs,
|
||||
)
|
||||
monkeypatch.setitem(sys.modules, "aiohttp", fake_aiohttp)
|
||||
monkeypatch.setattr(discord_adapter, "is_safe_url", fake_is_safe_url)
|
||||
|
||||
mock_channel = MagicMock()
|
||||
mock_channel.send = AsyncMock(return_value=MagicMock(id=1))
|
||||
adapter._client.get_channel = MagicMock(return_value=mock_channel)
|
||||
adapter._is_forum_parent = MagicMock(return_value=False)
|
||||
|
||||
_run(adapter.send_multiple_images("67890", [(public_url, "caption")]))
|
||||
|
||||
mock_channel.send.assert_not_awaited()
|
||||
assert private_url in safe_calls
|
||||
|
||||
def test_url_batch_follows_safe_redirect_location_header(self, adapter, monkeypatch):
|
||||
"""Redirect handling preserves aiohttp's case-insensitive Location behavior."""
|
||||
import plugins.platforms.discord.adapter as discord_adapter
|
||||
|
||||
public_url = "https://cdn.example.test/image.png"
|
||||
redirected_url = "https://assets.example.test/image.png"
|
||||
requested_urls = []
|
||||
|
||||
class RedirectResponse:
|
||||
status = 302
|
||||
headers = {"Location": redirected_url}
|
||||
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, exc_type, exc, tb):
|
||||
return False
|
||||
|
||||
async def read(self):
|
||||
raise AssertionError("redirect responses must not be read")
|
||||
|
||||
class ImageResponse:
|
||||
status = 200
|
||||
headers = {"Content-Type": "image/png"}
|
||||
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, exc_type, exc, tb):
|
||||
return False
|
||||
|
||||
async def read(self):
|
||||
return b"\x89PNG"
|
||||
|
||||
class FakeSession:
|
||||
def get(self, url, **kwargs):
|
||||
assert kwargs.get("allow_redirects") is False
|
||||
requested_urls.append(url)
|
||||
if url == public_url:
|
||||
return RedirectResponse()
|
||||
assert url == redirected_url
|
||||
return ImageResponse()
|
||||
|
||||
async def close(self):
|
||||
return None
|
||||
|
||||
fake_aiohttp = types.SimpleNamespace(
|
||||
ClientSession=lambda **kwargs: FakeSession(),
|
||||
ClientTimeout=lambda **kwargs: kwargs,
|
||||
)
|
||||
monkeypatch.setitem(sys.modules, "aiohttp", fake_aiohttp)
|
||||
monkeypatch.setattr(discord_adapter, "is_safe_url", lambda url: True)
|
||||
|
||||
mock_channel = MagicMock()
|
||||
mock_channel.send = AsyncMock(return_value=MagicMock(id=1))
|
||||
adapter._client.get_channel = MagicMock(return_value=mock_channel)
|
||||
adapter._is_forum_parent = MagicMock(return_value=False)
|
||||
|
||||
_run(adapter.send_multiple_images("67890", [(public_url, "caption")]))
|
||||
|
||||
assert requested_urls == [public_url, redirected_url]
|
||||
mock_channel.send.assert_awaited_once()
|
||||
|
||||
def test_send_image_blocks_private_redirect_before_send(self, adapter, monkeypatch):
|
||||
import plugins.platforms.discord.adapter as discord_adapter
|
||||
|
||||
public_url = "https://cdn.example.test/image.png"
|
||||
private_url = "http://169.254.169.254/latest/meta-data/"
|
||||
|
||||
class FakeResponse:
|
||||
status = 302
|
||||
headers = {"Location": private_url}
|
||||
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, exc_type, exc, tb):
|
||||
return False
|
||||
|
||||
async def read(self):
|
||||
return b"metadata-secret"
|
||||
|
||||
class FakeSession:
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, exc_type, exc, tb):
|
||||
return False
|
||||
|
||||
def get(self, url, **kwargs):
|
||||
assert kwargs.get("allow_redirects") is False
|
||||
return FakeResponse()
|
||||
|
||||
fake_aiohttp = types.SimpleNamespace(
|
||||
ClientSession=lambda **kwargs: FakeSession(),
|
||||
ClientTimeout=lambda **kwargs: kwargs,
|
||||
)
|
||||
monkeypatch.setitem(sys.modules, "aiohttp", fake_aiohttp)
|
||||
monkeypatch.setattr(
|
||||
discord_adapter,
|
||||
"is_safe_url",
|
||||
lambda url: not str(url).startswith("http://169.254.169.254"),
|
||||
)
|
||||
adapter._is_forum_parent = MagicMock(return_value=False)
|
||||
mock_channel = MagicMock()
|
||||
mock_channel.send = AsyncMock(return_value=MagicMock(id=1))
|
||||
adapter._client.get_channel = MagicMock(return_value=mock_channel)
|
||||
adapter._client.fetch_channel = AsyncMock(return_value=mock_channel)
|
||||
adapter.send = AsyncMock()
|
||||
|
||||
_run(adapter.send_image("67890", public_url, "caption"))
|
||||
|
||||
mock_channel.send.assert_not_awaited()
|
||||
|
||||
def test_send_animation_blocks_private_redirect_before_send(self, adapter, monkeypatch):
|
||||
import plugins.platforms.discord.adapter as discord_adapter
|
||||
|
||||
public_url = "https://cdn.example.test/animation.gif"
|
||||
private_url = "http://169.254.169.254/latest/meta-data/"
|
||||
|
||||
class FakeResponse:
|
||||
status = 302
|
||||
headers = {"Location": private_url}
|
||||
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, exc_type, exc, tb):
|
||||
return False
|
||||
|
||||
async def read(self):
|
||||
return b"metadata-secret"
|
||||
|
||||
class FakeSession:
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, exc_type, exc, tb):
|
||||
return False
|
||||
|
||||
def get(self, url, **kwargs):
|
||||
assert kwargs.get("allow_redirects") is False
|
||||
return FakeResponse()
|
||||
|
||||
fake_aiohttp = types.SimpleNamespace(
|
||||
ClientSession=lambda **kwargs: FakeSession(),
|
||||
ClientTimeout=lambda **kwargs: kwargs,
|
||||
)
|
||||
monkeypatch.setitem(sys.modules, "aiohttp", fake_aiohttp)
|
||||
monkeypatch.setattr(
|
||||
discord_adapter,
|
||||
"is_safe_url",
|
||||
lambda url: not str(url).startswith("http://169.254.169.254"),
|
||||
)
|
||||
adapter._is_forum_parent = MagicMock(return_value=False)
|
||||
mock_channel = MagicMock()
|
||||
mock_channel.send = AsyncMock(return_value=MagicMock(id=1))
|
||||
adapter._client.get_channel = MagicMock(return_value=mock_channel)
|
||||
adapter._client.fetch_channel = AsyncMock(return_value=mock_channel)
|
||||
adapter.send = AsyncMock()
|
||||
|
||||
_run(adapter.send_animation("67890", public_url, "caption"))
|
||||
|
||||
mock_channel.send.assert_not_awaited()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Slack
|
||||
|
|
@ -337,6 +553,49 @@ class TestSlackMultiImage:
|
|||
client = adapter._get_client("C12345")
|
||||
client.files_upload_v2.assert_not_called()
|
||||
|
||||
def test_url_batch_blocks_private_redirect_before_upload(self, adapter, monkeypatch):
|
||||
"""HTTP redirects are rechecked before Slack batch uploads remote bytes."""
|
||||
import httpx
|
||||
import tools.url_safety as url_safety
|
||||
|
||||
public_url = "https://cdn.example.test/image.png"
|
||||
private_url = "http://169.254.169.254/latest/meta-data/"
|
||||
safe_calls = []
|
||||
|
||||
def fake_is_safe_url(url):
|
||||
safe_calls.append(url)
|
||||
return not str(url).startswith("http://169.254.169.254")
|
||||
|
||||
class RedirectResponse:
|
||||
is_redirect = True
|
||||
url = public_url
|
||||
headers = {"location": private_url}
|
||||
next_request = None
|
||||
|
||||
class FakeAsyncClient:
|
||||
def __init__(self, **kwargs):
|
||||
self.event_hooks = kwargs.get("event_hooks", {})
|
||||
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, exc_type, exc, tb):
|
||||
return False
|
||||
|
||||
async def get(self, url):
|
||||
for hook in self.event_hooks.get("response", []):
|
||||
await hook(RedirectResponse())
|
||||
raise AssertionError("private redirect was not blocked before fetch")
|
||||
|
||||
monkeypatch.setattr(httpx, "AsyncClient", FakeAsyncClient)
|
||||
monkeypatch.setattr(url_safety, "is_safe_url", fake_is_safe_url)
|
||||
|
||||
_run(adapter.send_multiple_images("C12345", [(public_url, "caption")]))
|
||||
|
||||
client = adapter._get_client("C12345")
|
||||
client.files_upload_v2.assert_not_called()
|
||||
assert private_url in safe_calls
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Mattermost
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue