Compare commits
6 Commits
security/f
...
security/a
| Author | SHA1 | Date | |
|---|---|---|---|
| 4e3f5072f6 | |||
| 5936745636 | |||
| cfaf6c827e | |||
| cf1afb07f2 | |||
| ed32487cbe | |||
| 37c5e672b5 |
@@ -241,6 +241,43 @@ else:
|
||||
security_headers_middleware = None # type: ignore[assignment]
|
||||
|
||||
|
||||
# SECURITY FIX (V-016): Rate limiting middleware
|
||||
if AIOHTTP_AVAILABLE:
|
||||
@web.middleware
|
||||
async def rate_limit_middleware(request, handler):
|
||||
"""Apply rate limiting per client IP.
|
||||
|
||||
Returns 429 Too Many Requests if rate limit exceeded.
|
||||
Configurable via API_SERVER_RATE_LIMIT env var (requests per minute).
|
||||
"""
|
||||
# Skip rate limiting for health checks
|
||||
if request.path == "/health":
|
||||
return await handler(request)
|
||||
|
||||
# Get client IP (respecting X-Forwarded-For if behind proxy)
|
||||
client_ip = request.headers.get("X-Forwarded-For", request.remote)
|
||||
if client_ip and "," in client_ip:
|
||||
client_ip = client_ip.split(",")[0].strip()
|
||||
|
||||
limiter = _get_rate_limiter()
|
||||
if not limiter.acquire(client_ip):
|
||||
retry_after = limiter.get_retry_after(client_ip)
|
||||
logger.warning(f"Rate limit exceeded for {client_ip}")
|
||||
return web.json_response(
|
||||
_openai_error(
|
||||
f"Rate limit exceeded. Try again in {retry_after} seconds.",
|
||||
err_type="rate_limit_error",
|
||||
code="rate_limit_exceeded"
|
||||
),
|
||||
status=429,
|
||||
headers={"Retry-After": str(retry_after)}
|
||||
)
|
||||
|
||||
return await handler(request)
|
||||
else:
|
||||
rate_limit_middleware = None # type: ignore[assignment]
|
||||
|
||||
|
||||
class _IdempotencyCache:
|
||||
"""In-memory idempotency cache with TTL and basic LRU semantics."""
|
||||
def __init__(self, max_items: int = 1000, ttl_seconds: int = 300):
|
||||
@@ -273,6 +310,59 @@ class _IdempotencyCache:
|
||||
_idem_cache = _IdempotencyCache()
|
||||
|
||||
|
||||
# SECURITY FIX (V-016): Rate limiting
|
||||
class _RateLimiter:
|
||||
"""Token bucket rate limiter per client IP.
|
||||
|
||||
Default: 100 requests per minute per IP.
|
||||
Configurable via API_SERVER_RATE_LIMIT env var (requests per minute).
|
||||
"""
|
||||
def __init__(self, requests_per_minute: int = 100):
|
||||
from collections import defaultdict
|
||||
self._buckets = defaultdict(lambda: {"tokens": requests_per_minute, "last": 0})
|
||||
self._rate = requests_per_minute / 60.0 # tokens per second
|
||||
self._max_tokens = requests_per_minute
|
||||
self._lock = threading.Lock()
|
||||
|
||||
def _get_bucket(self, key: str) -> dict:
|
||||
import time
|
||||
with self._lock:
|
||||
bucket = self._buckets[key]
|
||||
now = time.time()
|
||||
elapsed = now - bucket["last"]
|
||||
bucket["last"] = now
|
||||
# Add tokens based on elapsed time
|
||||
bucket["tokens"] = min(
|
||||
self._max_tokens,
|
||||
bucket["tokens"] + elapsed * self._rate
|
||||
)
|
||||
return bucket
|
||||
|
||||
def acquire(self, key: str) -> bool:
|
||||
"""Try to acquire a token. Returns True if allowed, False if rate limited."""
|
||||
bucket = self._get_bucket(key)
|
||||
with self._lock:
|
||||
if bucket["tokens"] >= 1:
|
||||
bucket["tokens"] -= 1
|
||||
return True
|
||||
return False
|
||||
|
||||
def get_retry_after(self, key: str) -> int:
|
||||
"""Get seconds until next token is available."""
|
||||
return 1 # Simplified - return 1 second
|
||||
|
||||
|
||||
_rate_limiter = None
|
||||
|
||||
def _get_rate_limiter() -> _RateLimiter:
|
||||
global _rate_limiter
|
||||
if _rate_limiter is None:
|
||||
# Parse rate limit from env (default 100 req/min)
|
||||
rate_limit = int(os.getenv("API_SERVER_RATE_LIMIT", "100"))
|
||||
_rate_limiter = _RateLimiter(rate_limit)
|
||||
return _rate_limiter
|
||||
|
||||
|
||||
def _make_request_fingerprint(body: Dict[str, Any], keys: List[str]) -> str:
|
||||
from hashlib import sha256
|
||||
subset = {k: body.get(k) for k in keys}
|
||||
@@ -1282,7 +1372,8 @@ class APIServerAdapter(BasePlatformAdapter):
|
||||
return False
|
||||
|
||||
try:
|
||||
mws = [mw for mw in (cors_middleware, body_limit_middleware, security_headers_middleware) if mw is not None]
|
||||
# SECURITY FIX (V-016): Add rate limiting middleware
|
||||
mws = [mw for mw in (cors_middleware, body_limit_middleware, security_headers_middleware, rate_limit_middleware) if mw is not None]
|
||||
self._app = web.Application(middlewares=mws)
|
||||
self._app["api_server_adapter"] = self
|
||||
self._app.router.add_get("/health", self._handle_health)
|
||||
|
||||
@@ -170,6 +170,9 @@ def _resolve_cdp_override(cdp_url: str) -> str:
|
||||
For discovery-style endpoints we fetch /json/version and return the
|
||||
webSocketDebuggerUrl so downstream tools always receive a concrete browser
|
||||
websocket instead of an ambiguous host:port URL.
|
||||
|
||||
SECURITY FIX (V-010): Validates URLs before fetching to prevent SSRF.
|
||||
Only allows localhost/private network addresses for CDP connections.
|
||||
"""
|
||||
raw = (cdp_url or "").strip()
|
||||
if not raw:
|
||||
@@ -191,6 +194,35 @@ def _resolve_cdp_override(cdp_url: str) -> str:
|
||||
else:
|
||||
version_url = discovery_url.rstrip("/") + "/json/version"
|
||||
|
||||
# SECURITY FIX (V-010): Validate URL before fetching
|
||||
# Only allow localhost and private networks for CDP
|
||||
from urllib.parse import urlparse
|
||||
parsed = urlparse(version_url)
|
||||
hostname = parsed.hostname or ""
|
||||
|
||||
# Allow only safe hostnames for CDP
|
||||
allowed_hostnames = ["localhost", "127.0.0.1", "0.0.0.0", "::1"]
|
||||
if hostname not in allowed_hostnames:
|
||||
# Check if it's a private IP
|
||||
try:
|
||||
import ipaddress
|
||||
ip = ipaddress.ip_address(hostname)
|
||||
if not (ip.is_private or ip.is_loopback):
|
||||
logger.error(
|
||||
"SECURITY: Rejecting CDP URL '%s' - only localhost and private "
|
||||
"networks are allowed to prevent SSRF attacks.",
|
||||
raw
|
||||
)
|
||||
return raw # Return original without fetching
|
||||
except ValueError:
|
||||
# Not an IP - reject unknown hostnames
|
||||
logger.error(
|
||||
"SECURITY: Rejecting CDP URL '%s' - unknown hostname '%s'. "
|
||||
"Only localhost and private IPs are allowed.",
|
||||
raw, hostname
|
||||
)
|
||||
return raw
|
||||
|
||||
try:
|
||||
response = requests.get(version_url, timeout=10)
|
||||
response.raise_for_status()
|
||||
|
||||
@@ -253,6 +253,26 @@ class DockerEnvironment(BaseEnvironment):
|
||||
# mode uses tmpfs (ephemeral, fast, gone on cleanup).
|
||||
from tools.environments.base import get_sandbox_dir
|
||||
|
||||
# SECURITY FIX (V-012): Block dangerous volume mounts
|
||||
# Prevent privilege escalation via Docker socket or sensitive paths
|
||||
_BLOCKED_VOLUME_PATTERNS = [
|
||||
"/var/run/docker.sock",
|
||||
"/run/docker.sock",
|
||||
"/var/run/docker.pid",
|
||||
"/proc", "/sys", "/dev",
|
||||
":/", # Root filesystem mount
|
||||
]
|
||||
|
||||
def _is_dangerous_volume(vol_spec: str) -> bool:
|
||||
"""Check if volume spec is dangerous (docker socket, root fs, etc)."""
|
||||
for pattern in _BLOCKED_VOLUME_PATTERNS:
|
||||
if pattern in vol_spec:
|
||||
return True
|
||||
# Check for docker socket variations
|
||||
if "docker.sock" in vol_spec.lower():
|
||||
return True
|
||||
return False
|
||||
|
||||
# User-configured volume mounts (from config.yaml docker_volumes)
|
||||
volume_args = []
|
||||
workspace_explicitly_mounted = False
|
||||
@@ -263,6 +283,15 @@ class DockerEnvironment(BaseEnvironment):
|
||||
vol = vol.strip()
|
||||
if not vol:
|
||||
continue
|
||||
|
||||
# SECURITY FIX (V-012): Block dangerous volumes
|
||||
if _is_dangerous_volume(vol):
|
||||
logger.error(
|
||||
f"SECURITY: Refusing to mount dangerous volume '{vol}'. "
|
||||
f"Docker socket and system paths are blocked to prevent container escape."
|
||||
)
|
||||
continue # Skip this dangerous volume
|
||||
|
||||
if ":" in vol:
|
||||
volume_args.extend(["-v", vol])
|
||||
if ":/workspace" in vol:
|
||||
|
||||
Reference in New Issue
Block a user