diff --git a/agent_core/core/credentials/oauth_server.py b/agent_core/core/credentials/oauth_server.py index 8b5a60b3..271346ca 100644 --- a/agent_core/core/credentials/oauth_server.py +++ b/agent_core/core/credentials/oauth_server.py @@ -22,6 +22,7 @@ """ import asyncio +import hmac import html import ipaddress import logging @@ -126,7 +127,16 @@ def do_GET(self): # Validate OAuth state parameter to prevent CSRF expected_state = result_holder.get("expected_state") - if expected_state and returned_state != expected_state: + if expected_state is None: + # No state in auth URL -- log warning but allow (some providers don't support state) + logger.warning("[OAUTH] No state parameter in auth URL — CSRF protection disabled for this flow") + result_holder["code"] = params.get("code", [None])[0] + elif returned_state is None: + # State was expected but callback didn't include it -- reject + result_holder["error"] = "OAuth callback missing state parameter — possible CSRF attack" + result_holder["code"] = None + logger.warning("[OAUTH] Expected state but callback had none") + elif not hmac.compare_digest(str(returned_state), str(expected_state)): result_holder["error"] = "OAuth state mismatch — possible CSRF attack" result_holder["code"] = None logger.warning("[OAUTH] State mismatch: expected %s, got %s", expected_state, returned_state) diff --git a/app/config/settings.json b/app/config/settings.json deleted file mode 100644 index 669d7ebd..00000000 --- a/app/config/settings.json +++ /dev/null @@ -1,79 +0,0 @@ -{ - "version": "1.2.2", - "general": { - "agent_name": "CraftBot", - "os_language": "en" - }, - "proactive": { - "enabled": true - }, - "memory": { - "enabled": true - }, - "model": { - "llm_provider": "byteplus", - "vlm_provider": "byteplus", - "llm_model": "kimi-k2-250905", - "vlm_model": "seed-1-6-250915", - "slow_mode": true, - "slow_mode_tpm_limit": 25000 - }, - "api_keys": { - "openai": "", - "anthropic": "", - "google": "", - "byteplus": "", - "deepseek": "" - }, - "endpoints": { - "remote_model_url": "", - "byteplus_base_url": "https://ark.ap-southeast.bytepluses.com/api/v3", - "google_api_base": "", - "google_api_version": "", - "remote": "http://localhost:11434" - }, - "gui": { - "enabled": true, - "use_omniparser": false, - "omniparser_url": "http://127.0.0.1:7861" - }, - "cache": { - "prefix_ttl": 3600, - "session_ttl": 7200, - "min_tokens": 500 - }, - "oauth": { - "google": { - "client_id": "", - "client_secret": "" - }, - "linkedin": { - "client_id": "", - "client_secret": "" - }, - "slack": { - "client_id": "", - "client_secret": "" - }, - "notion": { - "client_id": "", - "client_secret": "" - }, - "outlook": { - "client_id": "" - } - }, - "web_search": { - "google_cse_id": "" - }, - "browser": { - "port": 7926, - "startup_ui": false - }, - "api_keys_configured": { - "openai": false, - "anthropic": false, - "google": true, - "byteplus": true - } -} \ No newline at end of file diff --git a/app/data/action/http_request.py b/app/data/action/http_request.py index 643299e9..d94042a4 100644 --- a/app/data/action/http_request.py +++ b/app/data/action/http_request.py @@ -165,40 +165,51 @@ def send_http_requests(input_data: dict) -> dict: allow_redirects = bool(input_data.get('allow_redirects', True)) verify_tls = bool(input_data.get('verify_tls', True)) allowed = {'GET','POST','PUT','PATCH','DELETE'} + + def _error(message, status_code=0, final_url='', elapsed_ms=0): + return {'status':'error','status_code':status_code,'response_headers':{},'body':'','final_url':final_url,'elapsed_ms':elapsed_ms,'message':message} + if method not in allowed: - return {'status':'error','status_code':0,'response_headers':{},'body':'','final_url':'','elapsed_ms':0,'message':'Unsupported method.'} + return _error('Unsupported method.') if not url or not (url.startswith('http://') or url.startswith('https://')): - return {'status':'error','status_code':0,'response_headers':{},'body':'','final_url':'','elapsed_ms':0,'message':'Invalid or missing URL.'} + return _error('Invalid or missing URL.') # SSRF protection: block requests to private/internal networks and cloud metadata - try: - from urllib.parse import urlparse as _urlparse - import ipaddress as _ipaddress - import socket as _socket - _parsed = _urlparse(url) + from urllib.parse import urlparse as _urlparse, urljoin as _urljoin + import ipaddress as _ipaddress + import socket as _socket + + _BLOCKED_HOSTS = frozenset({'169.254.169.254', 'metadata.google.internal', 'metadata.internal'}) + + def _is_url_ssrf_safe(check_url: str) -> 'str | None': + """Return an error message if the URL targets a blocked host, or None if safe.""" + _parsed = _urlparse(check_url) + # Block non-HTTP schemes (file://, gopher://, etc.) + if _parsed.scheme not in ('http', 'https'): + return f'Blocked: only http/https schemes are allowed (got {_parsed.scheme}).' _hostname = _parsed.hostname or '' - # Block cloud metadata endpoints - _BLOCKED_HOSTS = {'169.254.169.254', 'metadata.google.internal', 'metadata.internal'} if _hostname in _BLOCKED_HOSTS: - return {'status':'error','status_code':0,'response_headers':{},'body':'','final_url':'','elapsed_ms':0,'message':'Blocked: requests to cloud metadata endpoints are not allowed.'} - # Resolve hostname and check for private IPs + return 'Blocked: requests to cloud metadata endpoints are not allowed.' try: _resolved = _socket.getaddrinfo(_hostname, None) for _family, _type, _proto, _canonname, _sockaddr in _resolved: _ip = _ipaddress.ip_address(_sockaddr[0]) if _ip.is_private or _ip.is_loopback or _ip.is_link_local: - return {'status':'error','status_code':0,'response_headers':{},'body':'','final_url':'','elapsed_ms':0,'message':f'Blocked: requests to private/internal addresses ({_hostname}) are not allowed.'} - except (socket.gaierror, ValueError): - pass # Let the request library handle DNS resolution errors - except Exception: - pass # Best-effort SSRF check; don't block on parsing failures + return f'Blocked: requests to private/internal addresses ({_hostname}) are not allowed.' + except (_socket.gaierror, ValueError): + return f'Blocked: could not resolve hostname ({_hostname}).' + return None + + ssrf_error = _is_url_ssrf_safe(url) + if ssrf_error: + return _error(ssrf_error) if json_body is not None and data_body is not None: - return {'status':'error','status_code':0,'response_headers':{},'body':'','final_url':'','elapsed_ms':0,'message':'Provide either json or data, not both.'} + return _error('Provide either json or data, not both.') if not isinstance(headers, dict) or not isinstance(params, dict): - return {'status':'error','status_code':0,'response_headers':{},'body':'','final_url':'','elapsed_ms':0,'message':'headers and params must be objects.'} + return _error('headers and params must be objects.') headers = {str(k): str(v) for k, v in headers.items()} params = {str(k): str(v) for k, v in params.items()} - kwargs = {'headers': headers, 'params': params, 'timeout': timeout, 'allow_redirects': allow_redirects, 'verify': verify_tls} + kwargs = {'headers': headers, 'params': params, 'timeout': timeout, 'allow_redirects': False, 'verify': verify_tls} if json_body is not None: kwargs['json'] = json_body elif data_body is not None: @@ -206,6 +217,32 @@ def send_http_requests(input_data: dict) -> dict: try: t0 = time.time() resp = requests.request(method, url, **kwargs) + # Manually follow redirects with SSRF validation on each hop + _max_redirects = 10 + while allow_redirects and resp.is_redirect and _max_redirects > 0: + _max_redirects -= 1 + redirect_url = resp.headers.get('Location', '') + if not redirect_url: + break + # Resolve relative redirects + if not redirect_url.startswith(('http://', 'https://')): + redirect_url = _urljoin(resp.url, redirect_url) + redirect_error = _is_url_ssrf_safe(redirect_url) + if redirect_error: + return _error(f'Redirect blocked: {redirect_error}', status_code=resp.status_code, final_url=resp.url, elapsed_ms=int((time.time()-t0)*1000)) + # 307/308 preserve method; all others downgrade to GET per RFC 7231 + redirect_method = method if resp.status_code in (307, 308) else 'GET' + redirect_kwargs = {**kwargs, 'allow_redirects': False} + # Strip body on method downgrade to GET + if redirect_method == 'GET': + redirect_kwargs.pop('json', None) + redirect_kwargs.pop('data', None) + # Strip auth headers on cross-origin redirects + _orig_host = _urlparse(url).netloc + _redir_host = _urlparse(redirect_url).netloc + if _orig_host != _redir_host and 'Authorization' in redirect_kwargs.get('headers', {}): + redirect_kwargs['headers'] = {k: v for k, v in redirect_kwargs['headers'].items() if k != 'Authorization'} + resp = requests.request(redirect_method, redirect_url, **redirect_kwargs) elapsed_ms = int((time.time() - t0) * 1000) resp_headers = {k: v for k, v in resp.headers.items()} parsed_json = None diff --git a/app/external_comms/credentials.py b/app/external_comms/credentials.py index b1b23e78..87ea2f23 100644 --- a/app/external_comms/credentials.py +++ b/app/external_comms/credentials.py @@ -83,13 +83,11 @@ def save_credential(filename: str, credential) -> None: """ path = _get_credentials_dir() / filename try: - with open(path, "w", encoding="utf-8") as f: + # Create file with restricted permissions atomically (rw-------) + fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + os.fchmod(fd, 0o600) # Ensure permissions even if file pre-existed + with os.fdopen(fd, "w", encoding="utf-8") as f: json.dump(asdict(credential), f, indent=2, default=str) - # Restrict file permissions to owner read/write only (rw-------) - try: - os.chmod(path, stat.S_IRUSR | stat.S_IWUSR) - except OSError: - pass # Best-effort on platforms that don't support chmod logger.info(f"Saved credential: {filename}") except Exception as e: logger.error(f"Failed to save credential {filename}: {e}")