adam/open-swe
1
0
Fork 0
mirror of https://github.com/Sea-Haven-Industries/open-swe.git synced 2026-10-07 16:19:09 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix(security): SSRF-harden server-side image fetch + stop logging raw image URLs

Browse source 
INJ-01 (high): fetch_image_block used follow_redirects=True with no per-hop
revalidation and discarded the resolved-IP pin, so an attacker-authored Slack/
Linear image URL could 302-redirect the fetch to an internal host / cloud
metadata endpoint (blind SSRF), and DNS-rebinding could bypass the one-shot
is_url_safe check. Route image fetches through the same per-hop resolve+pin+
revalidate loop the http_request tool uses, lifted into url_safety as the shared
request_with_safe_redirects. Also strip the per-host Slack/Linear bearer token
on redirect so it can't be replayed to a redirect target.

SC-1 (low): linear.py logged full image URLs (which can carry signed tokens) at
DEBUG; multimodal logged them at INFO on every fetch. Log host-only.

Sink lived in multimodal.py (unchanged by the feature work) but PR #128 widened
its reach by no longer dropping images for text-only models. Fixing on the base
branch so #130/#129 inherit it on rebase. Adds fetch_image_block SSRF regression
tests (redirect-to-internal blocked; auth stripped on redirect).
This commit is contained in:
Adam Moussa 2026-07-08 18:18:07 -04:00
parent 0b1cc54173
commit 2028208237
No known key found for this signature in database
5 changed files with 196 additions and 100 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

83
agent/tools/http_request.py
View file

@ -1,13 +1,10 @@
from typing import Any
from urllib.parse import urljoin, urlparse, urlunparse
import httpx
from ..utils.url_safety import resolve_and_validate as _resolve_and_validate
_MAX_REDIRECTS = 5
_REDIRECT_CODES = {301, 302, 303, 307, 308}
from ..utils.url_safety import pinned_url as _pinned_url # noqa: F401 (kept for tests)
from ..utils.url_safety import request_with_safe_redirects
from ..utils.url_safety import resolve_and_validate as _resolve_and_validate # noqa: F401
def _blocked_response(url: str, reason: str) -> dict[str, Any]:
@ -20,79 +17,19 @@ def _blocked_response(url: str, reason: str) -> dict[str, Any]:
}
def _pinned_url(url: str, ip: str) -> str:
"""Rewrite ``url`` so the connection targets ``ip`` while keeping the path/query.
The original hostname is preserved separately for the ``Host`` header and TLS
SNI/cert verification (via httpx's ``sni_hostname`` request extension).
"""
parsed = urlparse(url)
host_literal = f"[{ip}]" if ":" in ip else ip
netloc = f"{host_literal}:{parsed.port}" if parsed.port else host_literal
return urlunparse(parsed._replace(netloc=netloc))
async def _request_with_safe_redirects(
client: httpx.AsyncClient,
method: str,
url: str,
**kwargs: Any,
) -> tuple[httpx.Response | None, dict[str, Any] | None]:
"""Issue a request while validating every redirect target before following it.
The hostname is resolved once per hop and the connection is pinned to the
validated IP, closing the DNS-rebinding race where a controlled resolver
returns a public IP at validation time and a private IP at connect time.
"""
current_method = method.upper()
current_url = url
request_kwargs = dict(kwargs)
# Pop caller headers/extensions ONCE so they're reused on every redirect hop
# (the per-hop Host + SNI are layered on top each time). Popping inside the
# loop dropped the caller's Authorization/Accept/etc. on the first redirect.
caller_headers = dict(request_kwargs.pop("headers", None) or {})
caller_extensions = dict(request_kwargs.pop("extensions", None) or {})
for redirect_count in range(_MAX_REDIRECTS + 1):
is_safe, reason, hostname, addr_infos = _resolve_and_validate(current_url)
if not is_safe or hostname is None or addr_infos is None:
return None, _blocked_response(current_url, reason)
pinned_ip = addr_infos[0][4][0]
parsed = urlparse(current_url)
headers = {**caller_headers, "Host": parsed.netloc}
extensions = {**caller_extensions, "sni_hostname": hostname}
response = await client.request(
current_method,
_pinned_url(current_url, pinned_ip),
follow_redirects=False,
headers=headers,
extensions=extensions,
**request_kwargs,
)
if response.status_code not in _REDIRECT_CODES:
return response, None
location = response.headers.get("Location")
if not location:
return response, None
if redirect_count == _MAX_REDIRECTS:
return None, _blocked_response(current_url, "Too many redirects")
current_url = urljoin(current_url, location)
if response.status_code == 303 or (
response.status_code in {301, 302} and current_method not in {"GET", "HEAD"}
):
current_method = "GET"
request_kwargs.pop("data", None)
request_kwargs.pop("content", None)
request_kwargs.pop("json", None)
return None, _blocked_response(current_url, "Too many redirects")
"""Thin wrapper over the shared SSRF-safe redirect loop that shapes a blocked
hop into the tool's error-response dict."""
response, blocked = await request_with_safe_redirects(client, method, url, **kwargs)
if blocked is not None:
blocked_url, reason = blocked
return None, _blocked_response(blocked_url, reason)
return response, None
async def http_request(

48
agent/utils/multimodal.py
View file

@ -13,7 +13,7 @@ from urllib.parse import urlparse
import httpx
from langchain_core.messages.content import create_image_block
from .url_safety import is_url_safe
from .url_safety import request_with_safe_redirects
logger = logging.getLogger(__name__)
@ -52,23 +52,23 @@ async def fetch_image_block(
image_url: str,
client: httpx.AsyncClient,
) -> dict[str, Any] | None:
"""Fetch image bytes and build an image content block."""
"""Fetch image bytes and build an image content block.
The fetch validates and pins every redirect hop (SSRF guard) and drops any
Authorization header once the URL redirects, so a per-host token is never
replayed to a redirect target the caller never chose to authenticate to.
URLs are logged host-only — a signed image URL can carry a bearer token.
"""
host = (urlparse(image_url).hostname or "").lower()
try:
safe, reason = is_url_safe(image_url)
if not safe:
logger.warning("Refusing to fetch image (SSRF guard) %s: %s", image_url, reason)
return None
logger.debug("Fetching image from %s", image_url)
headers = None
host = (urlparse(image_url).hostname or "").lower()
if host == "uploads.linear.app" or host.endswith(".uploads.linear.app"):
linear_api_key = os.environ.get("LINEAR_API_KEY", "")
if linear_api_key:
headers = {"Authorization": linear_api_key}
else:
logger.warning(
"LINEAR_API_KEY not set; cannot authenticate image fetch for %s",
image_url,
"LINEAR_API_KEY not set; cannot authenticate image fetch for %s", host
)
elif host == "files.slack.com" or host.endswith(".files.slack.com"):
slack_bot_token = os.environ.get("SLACK_BOT_TOKEN", "")
@ -76,41 +76,39 @@ async def fetch_image_block(
headers = {"Authorization": f"Bearer {slack_bot_token}"}
else:
logger.warning(
"SLACK_BOT_TOKEN not set; cannot authenticate image fetch for %s",
image_url,
"SLACK_BOT_TOKEN not set; cannot authenticate image fetch for %s", host
)
response = await client.get(image_url, headers=headers, follow_redirects=True)
response, blocked = await request_with_safe_redirects(
client, "GET", image_url, headers=headers, strip_auth_on_redirect=True
)
if blocked is not None:
_, reason = blocked
logger.warning("Refusing to fetch image (SSRF guard) from %s: %s", host, reason)
return None
response.raise_for_status()
content_type = response.headers.get("Content-Type", "").split(";")[0].strip()
if not content_type:
guessed, _ = mimetypes.guess_type(image_url)
if not guessed:
logger.warning(
"Could not determine content type for %s; skipping image",
image_url,
)
logger.warning("Could not determine content type from %s; skipping image", host)
return None
content_type = guessed
supported_types = {"image/jpeg", "image/png", "image/gif", "image/webp"}
if content_type not in supported_types:
logger.warning(
"Unsupported content type '%s' for %s; skipping image",
content_type,
image_url,
"Unsupported content type '%s' from %s; skipping image", content_type, host
)
return None
encoded = base64.b64encode(response.content).decode("ascii")
logger.info(
"Fetched image %s (%s, %d bytes)",
image_url,
content_type,
len(response.content),
"Fetched image from %s (%s, %d bytes)", host, content_type, len(response.content)
)
return create_image_block(base64=encoded, mime_type=content_type)
except Exception:
logger.exception("Failed to fetch image from %s", image_url)
logger.exception("Failed to fetch image from %s", host)
return None

97
agent/utils/url_safety.py
View file

@ -9,7 +9,13 @@ from __future__ import annotations
import ipaddress
import socket
from urllib.parse import urlparse
from typing import Any
from urllib.parse import urljoin, urlparse, urlunparse
import httpx
DEFAULT_MAX_REDIRECTS = 5
_REDIRECT_CODES = {301, 302, 303, 307, 308}
def resolve_and_validate(url: str) -> tuple[bool, str, str | None, list | None]:
@ -61,3 +67,92 @@ def is_url_safe(url: str) -> tuple[bool, str]:
"""Check if a URL is safe to request (not targeting private/internal networks)."""
is_safe, reason, _, _ = resolve_and_validate(url)
return is_safe, reason
def pinned_url(url: str, ip: str) -> str:
"""Rewrite ``url`` so the connection targets ``ip`` while keeping the path/query.
The original hostname is preserved separately for the ``Host`` header and TLS
SNI/cert verification (via httpx's ``sni_hostname`` request extension).
"""
parsed = urlparse(url)
host_literal = f"[{ip}]" if ":" in ip else ip
netloc = f"{host_literal}:{parsed.port}" if parsed.port else host_literal
return urlunparse(parsed._replace(netloc=netloc))
async def request_with_safe_redirects(
client: httpx.AsyncClient,
method: str,
url: str,
*,
max_redirects: int = DEFAULT_MAX_REDIRECTS,
strip_auth_on_redirect: bool = False,
**kwargs: Any,
) -> tuple[httpx.Response | None, tuple[str, str] | None]:
"""Issue a request, validating every redirect target before following it.
The hostname is resolved once per hop and the connection is pinned to the
validated IP, closing the DNS-rebinding race where a controlled resolver
returns a public IP at validation time and a private IP at connect time.
Returns ``(response, None)`` on success, or ``(None, (blocked_url, reason))``
when a hop fails validation or the redirect budget is exhausted. When
``strip_auth_on_redirect`` is set, the caller's ``Authorization`` header is
dropped once the request leaves the original URL, so a bearer token can't be
replayed to a redirect target the caller never chose to authenticate to.
"""
current_method = method.upper()
current_url = url
request_kwargs = dict(kwargs)
# Pop caller headers/extensions ONCE so they're reused on every redirect hop
# (the per-hop Host + SNI are layered on top each time). Popping inside the
# loop dropped the caller's Authorization/Accept/etc. on the first redirect.
caller_headers = dict(request_kwargs.pop("headers", None) or {})
caller_extensions = dict(request_kwargs.pop("extensions", None) or {})
for redirect_count in range(max_redirects + 1):
is_safe, reason, hostname, addr_infos = resolve_and_validate(current_url)
if not is_safe or hostname is None or addr_infos is None:
return None, (current_url, reason)
pinned_ip = addr_infos[0][4][0]
parsed = urlparse(current_url)
headers = {**caller_headers, "Host": parsed.netloc}
extensions = {**caller_extensions, "sni_hostname": hostname}
response = await client.request(
current_method,
pinned_url(current_url, pinned_ip),
follow_redirects=False,
headers=headers,
extensions=extensions,
**request_kwargs,
)
if response.status_code not in _REDIRECT_CODES:
return response, None
location = response.headers.get("Location")
if not location:
return response, None
if redirect_count == max_redirects:
return None, (current_url, "Too many redirects")
current_url = urljoin(current_url, location)
if strip_auth_on_redirect:
caller_headers = {
k: v for k, v in caller_headers.items() if k.lower() != "authorization"
}
if response.status_code == 303 or (
response.status_code in {301, 302} and current_method not in {"GET", "HEAD"}
):
current_method = "GET"
request_kwargs.pop("data", None)
request_kwargs.pop("content", None)
request_kwargs.pop("json", None)
return None, (current_url, "Too many redirects")

3
agent/webhooks/linear.py
View file

@ -5,6 +5,7 @@ object (``webapp.X``) so tests that monkeypatch them keep working.
"""
from typing import Any
from urllib.parse import urlparse
import httpx
from langchain_core.messages.content import create_text_block
@ -182,7 +183,7 @@ async def process_linear_issue( # noqa: PLR0912, PLR0915
resolved_model_id = fallback_model_id
image_model_override = (fallback_model_id, fallback_effort)
webapp.logger.info("Preparing %d image(s) for multimodal content", len(image_urls))
webapp.logger.debug("Image URLs: %s", image_urls)
webapp.logger.debug("Image hosts: %s", [urlparse(u).hostname for u in image_urls])
async with httpx.AsyncClient(timeout=webapp.DEFAULT_HTTP_TIMEOUT) as client:
for image_url in image_urls:

65
tests/test_http_security.py
View file

@ -20,6 +20,7 @@ fetch_url_tool = sys.modules["agent.tools.fetch_url"]
http_request_tool = sys.modules["agent.tools.http_request"]
# SSRF resolution now lives in the shared validator; patch DNS there.
url_safety = importlib.import_module("agent.utils.url_safety")
multimodal = importlib.import_module("agent.utils.multimodal")
_NO_JSON = object()
@ -310,3 +311,67 @@ async def test_http_request_returns_timeout_result(monkeypatch) -> None:
assert result["success"] is False
assert result["status_code"] == 0
assert "timed out after 7 seconds" in result["content"]
# --- fetch_image_block SSRF (INJ-01 regression) ------------------------------
def _image_dns(monkeypatch, ip_map: dict[str, str]) -> None:
monkeypatch.setattr(
url_safety.socket,
"getaddrinfo",
lambda host, port, *a, **k: [_addr_info(ip_map[host], port)],
)
async def test_fetch_image_block_blocks_redirect_to_internal_host(monkeypatch) -> None:
"""A public image URL that 302-redirects to the metadata endpoint must not be
followed: the redirect target is re-validated and the internal hop blocked."""
_image_dns(
monkeypatch,
{"public.example": "93.184.216.34", "internal.evil": "169.254.169.254"},
)
def responder(method, url, **kwargs):
return FakeResponse(
status_code=302,
url=url,
headers={"Location": "http://internal.evil/latest/meta-data/"},
)
client = FakeAsyncClient(responder)
result = await multimodal.fetch_image_block("http://public.example/x.png", client)
assert result is None
# Exactly one hop was issued (to the validated public host); the redirect to
# the internal host was blocked before any request left the process.
assert len(client.calls) == 1
assert all("169.254.169.254" not in call["url"] for call in client.calls)
async def test_fetch_image_block_strips_auth_header_on_redirect(monkeypatch) -> None:
"""The per-host Slack bearer token must not be replayed to a redirect target."""
monkeypatch.setenv("SLACK_BOT_TOKEN", "xoxb-secret")
_image_dns(
monkeypatch,
{"files.slack.com": "93.184.216.34", "cdn.public": "93.184.216.35"},
)
def responder(method, url, **kwargs):
if len(client.calls) == 1:
return FakeResponse(
status_code=302, url=url, headers={"Location": "https://cdn.public/x.png"}
)
resp = FakeResponse(status_code=200, url=url, headers={"Content-Type": "image/png"})
resp.content = b"\x89PNG\r\n"
return resp
client = FakeAsyncClient(responder)
result = await multimodal.fetch_image_block("https://files.slack.com/x.png", client)
assert result is not None
assert len(client.calls) == 2
first_auth = client.calls[0]["headers"].get("Authorization")
second_auth = client.calls[1]["headers"].get("Authorization")
assert first_auth == "Bearer xoxb-secret"
assert second_auth is None