cleveragents/cleveragents-core
4
0
Fork 3
Code Issues 6.0k Pull Requests 115 Actions Packages Projects Releases Wiki Activity

fix/server-side-request-forgery-security-fix #33

Closed
aditya wants to merge 2 commits from fix/server-side-request-forgery-security-fix into tests/unit-tests
pull from: fix/server-side-request-forgery-security-fix
merge into: cleveragents:tests/unit-tests
Conversation 1 Commits 2 Files Changed 2
+110 -18
2 changed files with 110 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/cleveragents/agents/tool.py
+83 -17
View File
@@ -6,14 +6,17 @@ capabilities within the RxPy-based reactive architecture.
"""
import asyncio
import ipaddress
import json
import logging
import os
import re
import socket
import subprocess
from typing import Any
from typing import List
from typing import Optional, Union, Literal
from urllib.parse import urlparse
import aiohttp
@@ -22,6 +25,7 @@ from cleveragents.core.exceptions import AgentCreationError
from cleveragents.core.exceptions import ExecutionError
from cleveragents.templates.renderer import TemplateRenderer
logger = logging.getLogger(__name__)
@@ -290,7 +294,13 @@ class ToolAgent(Agent):
"float": float,
"__builtins__": {},
}
result = eval(expression, allowed_names) # pylint: disable=eval-used
# setting builtins to an empty dict prevents access to
# dangerous functions like __import__ and allows only
# the allowed functions to be used
result = eval( # pylint: disable=eval-used
expression, {"__builtins__": {}}, allowed_names
)
return str(result)
except Exception as e:
raise ExecutionError(f"Math evaluation failed: {e}") from e
@@ -309,10 +319,55 @@ class ToolAgent(Agent):
except json.JSONDecodeError as e:
raise ExecutionError(f"JSON parsing failed: {e}") from e
def _validate_url_safety(self, url: str) -> None:
"""Validate URL to prevent SSRF attacks."""
try:
parsed = urlparse(url)
except Exception as e:
raise ExecutionError(f"Invalid URL: {e}") from e
if not parsed.scheme or not parsed.hostname:
raise ExecutionError("Invalid URL format")
if parsed.scheme not in ['http', 'https']:
raise ExecutionError(f"Protocol '{parsed.scheme}' not allowed")
try:
ip_addresses = socket.getaddrinfo(
parsed.hostname, None, socket.AF_UNSPEC, socket.SOCK_STREAM
)
except socket.gaierror as e:
raise ExecutionError(f"Cannot resolve hostname '{parsed.hostname}': {e}") from e
blocked_ranges = [
ipaddress.ip_network('127.0.0.0/8'),
ipaddress.ip_network('10.0.0.0/8'),
ipaddress.ip_network('172.16.0.0/12'),
ipaddress.ip_network('192.168.0.0/16'),
ipaddress.ip_network('169.254.0.0/16'),
ipaddress.ip_network('::1/128'),
ipaddress.ip_network('fc00::/7'),
ipaddress.ip_network('fe80::/10'),
]
for addr_info in ip_addresses:
ip_str = addr_info[4][0]
try:
ip_obj = ipaddress.ip_address(ip_str)
except ValueError:
continue
for blocked_range in blocked_ranges:
if ip_obj in blocked_range:
raise ExecutionError(
f"Access to {parsed.hostname} ({ip_str}) blocked: "
f"resolves to restricted IP range {blocked_range}"
)
async def _http_request_tool(
self, args: dict[str, Any], context: Optional[dict[str, Any]] # pylint: disable=unused-argument
) -> str:
"""HTTP request tool."""
"""HTTP request tool with SSRF protection."""
url = args.get("url", "")
method = args.get("method", "GET").upper()
headers = args.get("headers", {})
@@ -321,20 +376,37 @@ class ToolAgent(Agent):
if not url:
raise ExecutionError("HTTP tool requires a URL")
if method not in ['GET', 'POST', 'PUT', 'DELETE', 'PATCH', 'HEAD']:
raise ExecutionError(f"HTTP method '{method}' not allowed")
self._validate_url_safety(url)
max_size = 10 * 1024 * 1024
try:
async with aiohttp.ClientSession() as session:
connector = aiohttp.TCPConnector(limit=10)
timeout = aiohttp.ClientTimeout(total=self.timeout, connect=5)
async with aiohttp.ClientSession(connector=connector, timeout=timeout) as session:
async with session.request(
method, url, headers=headers, json=data, timeout=self.timeout
method, url, headers=headers, json=data, allow_redirects=False
) as response:
content = await response.text()
if len(content) > max_size:
raise ExecutionError(
f"Response too large: {len(content)} bytes "
f"(max: {max_size})"
)
return f"Status: {response.status}\n\n{content}"
except aiohttp.ClientError as e:
raise ExecutionError(f"HTTP request failed: {e}") from e
except asyncio.TimeoutError as e:
raise ExecutionError("HTTP request timed out") from e
except Exception as e:
raise ExecutionError(f"HTTP request failed: {e}") from e
async def _file_read_tool(
self, args: dict[str, Any], context: Optional[dict[str, Any]]
) -> str:
"""File reading tool."""
"""File reading tool with comprehensive path validation."""
filepath = args.get("file", "")
if "args" in args and args["args"]:
filepath = args["args"][0]
@@ -342,15 +414,9 @@ class ToolAgent(Agent):
if not filepath:
raise ExecutionError("File read tool requires a file path")
if self.safe_mode:
# Always block directory traversal attempts
if ".." in filepath:
raise ExecutionError("Unsafe file path blocked in safe mode")
# Block absolute paths unless in unsafe mode
if filepath.startswith("/") and not (
context and context.get("_unsafe_mode", False)
):
raise ExecutionError("Unsafe file path blocked in safe mode")
# Use comprehensive validation instead of simple checks
unsafe_mode = bool(context and context.get("_unsafe_mode", False))
self._validate_file_path_safety(filepath, unsafe_mode)
try:
with open(filepath, "r", encoding="utf-8") as f:
@@ -363,7 +429,7 @@ class ToolAgent(Agent):
# Format: Special marker + metadata + full content
# The special marker helps identify this for clean display
return (
f"[FILE_READ_SUCCESS]📄 File: {filepath} | "
f"[FILE_READ_SUCCESS]:page_facing_up: File: {filepath} | "
f"Lines: {line_count} | Size: {char_count} chars\n"
f"[FILE_CONTENT_START]\n{content}\n[FILE_CONTENT_END]"
)
@@ -537,7 +603,7 @@ class ToolAgent(Agent):
# Validate inputs
self._validate_file_write_args(filepath, content)
# Check unsafe mode requirement
unsafe_mode = context and context.get("_unsafe_mode", False)
unsafe_mode = bool(context and context.get("_unsafe_mode", False))
if not unsafe_mode:
logger.error("File writing requires unsafe mode")
raise ExecutionError("File writing requires unsafe mode")
@@ -612,4 +678,4 @@ class ToolAgent(Agent):
"timeout": self.timeout,
}
)
return metadata
return metadata
tests/unit/agents/test_tool.py
+27 -1
View File
@@ -257,7 +257,13 @@ class TestToolAgent:
async def __aexit__(self, *args):
return None
with patch('aiohttp.ClientSession', return_value=AsyncSessionMock()):
# Mock socket.getaddrinfo to return a public IP
mock_getaddrinfo = Mock(return_value=[
(2, 1, 6, '', ('93.184.216.34', 0))
])
with patch('aiohttp.ClientSession', return_value=AsyncSessionMock()), \
patch('socket.getaddrinfo', mock_getaddrinfo):
result = await agent._http_request_tool({"url": "http://example.com"}, None)
assert "Status: 200" in result
@@ -274,6 +280,26 @@ class TestToolAgent:
assert "requires a URL" in str(exc_info.value)
@pytest.mark.asyncio
async def test_http_request_tool_blocks_ssrf(self, template_renderer):
"""Test HTTP request tool blocks SSRF attacks."""
config = {"type": "tool", "tools": ["http_request"]}
agent = ToolAgent("test_tool", config, template_renderer)
ssrf_urls = [
("http://localhost/admin", ("127.0.0.1", 0)),
("http://169.254.169.254/latest/meta-data/", ("169.254.169.254", 0)),
("http://192.168.1.1/config", ("192.168.1.1", 0)),
("http://10.0.0.1/internal", ("10.0.0.1", 0)),
]
for url, mock_ip in ssrf_urls:
mock_getaddrinfo = Mock(return_value=[(2, 1, 6, '', mock_ip)])
with patch('socket.getaddrinfo', mock_getaddrinfo):
with pytest.raises(ExecutionError) as exc_info:
await agent._http_request_tool({"url": url}, None)
assert "blocked" in str(exc_info.value).lower()
@pytest.mark.asyncio
async def test_file_read_tool(self, template_renderer):
"""Test file read tool."""