mirror of
https://github.com/rookiestar28/ComfyUI-OpenClaw.git
synced 2026-08-14 08:52:45 +00:00
fix(security): harden S36/S37/R79 with safe_io response cleanup, expanded egress primitive guards, and roadmap/record alignment
This commit is contained in:
+203
-136
@@ -5,11 +5,13 @@ Implements S4: File/path/URL safety (deny-by-default).
|
||||
Any module that touches filesystem or outbound HTTP MUST use this layer.
|
||||
"""
|
||||
|
||||
import http.client
|
||||
import ipaddress
|
||||
import logging
|
||||
import os
|
||||
import socket
|
||||
import tempfile
|
||||
import urllib.request
|
||||
from typing import Any, Optional, Set, Tuple
|
||||
from urllib.parse import urlparse
|
||||
|
||||
@@ -243,17 +245,17 @@ def validate_outbound_url(
|
||||
*,
|
||||
allow_hosts: Optional[Set[str]] = None,
|
||||
allow_any_public_host: bool = False,
|
||||
) -> Tuple[str, str, int]:
|
||||
) -> Tuple[str, str, int, list[str]]:
|
||||
"""
|
||||
Validate a URL for safe outbound fetching.
|
||||
Validate a URL and resolve it for safe outbound fetching.
|
||||
|
||||
Args:
|
||||
url: URL to validate.
|
||||
allow_hosts: If provided, only these hosts are allowed.
|
||||
allow_any_public_host: If True, allow any host that resolves to a public IP (skips allowlist check).
|
||||
allow_any_public_host: If True, allow any host that resolves to a public IP.
|
||||
|
||||
Returns:
|
||||
Tuple of (scheme, host, port).
|
||||
Tuple of (scheme, host, port, resolved_ips).
|
||||
|
||||
Raises:
|
||||
SSRFError: If URL is invalid or blocked.
|
||||
@@ -287,7 +289,6 @@ def validate_outbound_url(
|
||||
# Check allowlist if provided or enforced
|
||||
if not allow_any_public_host:
|
||||
if allow_hosts is None:
|
||||
# Should be caught by check above, but for typing...
|
||||
raise SSRFError("No allow_hosts allowed")
|
||||
|
||||
normalized_allowlist = {_normalize_host(h) for h in allow_hosts}
|
||||
@@ -295,6 +296,7 @@ def validate_outbound_url(
|
||||
raise SSRFError(f"Host not in allowlist: {host}")
|
||||
|
||||
# DNS resolution + IP check
|
||||
resolved_ips = []
|
||||
try:
|
||||
addr_infos = socket.getaddrinfo(
|
||||
host, port, socket.AF_UNSPEC, socket.SOCK_STREAM
|
||||
@@ -303,10 +305,85 @@ def validate_outbound_url(
|
||||
ip = sockaddr[0]
|
||||
if is_private_ip(ip):
|
||||
raise SSRFError(f"Private/reserved IP blocked: {ip}")
|
||||
if ip not in resolved_ips:
|
||||
resolved_ips.append(ip)
|
||||
except socket.gaierror as e:
|
||||
raise SSRFError(f"DNS resolution failed: {e}")
|
||||
|
||||
return (parsed.scheme, host, port)
|
||||
if not resolved_ips:
|
||||
raise SSRFError(f"No IP resolved for {host}")
|
||||
|
||||
return (parsed.scheme, host, port, resolved_ips)
|
||||
|
||||
|
||||
def _build_pinned_opener(pinned_ips: list[str]) -> urllib.request.OpenerDirector:
|
||||
"""Build a safe opener pinned to specific IPs, trying them in order."""
|
||||
|
||||
class PinnedHTTPConnection(http.client.HTTPConnection):
|
||||
def connect(self):
|
||||
last_err = None
|
||||
for ip in pinned_ips:
|
||||
try:
|
||||
self.sock = socket.create_connection(
|
||||
(ip, self.port), self.timeout, self.source_address
|
||||
)
|
||||
return
|
||||
except OSError as e:
|
||||
last_err = e
|
||||
if last_err:
|
||||
raise last_err
|
||||
raise OSError("No resolved IPs to connect to")
|
||||
|
||||
class PinnedHTTPSConnection(http.client.HTTPSConnection):
|
||||
def connect(self):
|
||||
last_err = None
|
||||
for ip in pinned_ips:
|
||||
try:
|
||||
sock = socket.create_connection(
|
||||
(ip, self.port), self.timeout, self.source_address
|
||||
)
|
||||
self.sock = self._context.wrap_socket(
|
||||
sock, server_hostname=self.host
|
||||
)
|
||||
return
|
||||
except OSError as e:
|
||||
last_err = e
|
||||
if last_err:
|
||||
raise last_err
|
||||
raise OSError("No resolved IPs to connect to")
|
||||
|
||||
class PinnedHTTPHandler(urllib.request.HTTPHandler):
|
||||
def http_open(self, req):
|
||||
return self.do_open(PinnedHTTPConnection, req)
|
||||
|
||||
class PinnedHTTPSHandler(urllib.request.HTTPSHandler):
|
||||
def https_open(self, req):
|
||||
kwargs = {}
|
||||
if getattr(self, "_context", None) is not None:
|
||||
kwargs["context"] = self._context
|
||||
if getattr(self, "_check_hostname", None) is not None:
|
||||
kwargs["check_hostname"] = self._check_hostname
|
||||
|
||||
return self.do_open(PinnedHTTPSConnection, req, **kwargs)
|
||||
|
||||
class NoRedirectHandler(urllib.request.HTTPRedirectHandler):
|
||||
"""Stop redirects so we can handle them manually with re-validation/pinning."""
|
||||
|
||||
def http_error_302(self, req, fp, code, msg, headers):
|
||||
return fp
|
||||
|
||||
http_error_301 = http_error_303 = http_error_307 = http_error_308 = (
|
||||
http_error_302
|
||||
)
|
||||
|
||||
handlers = [
|
||||
PinnedHTTPHandler(),
|
||||
PinnedHTTPSHandler(),
|
||||
NoRedirectHandler(),
|
||||
urllib.request.ProxyHandler({}),
|
||||
]
|
||||
|
||||
return urllib.request.build_opener(*handlers)
|
||||
|
||||
|
||||
def safe_fetch(
|
||||
@@ -315,76 +392,66 @@ def safe_fetch(
|
||||
allow_hosts: Optional[Set[str]] = None,
|
||||
max_bytes: int = 10_000_000,
|
||||
timeout_sec: int = 10,
|
||||
max_redirects: int = 0, # Default: no redirects (safest)
|
||||
max_redirects: int = 0,
|
||||
) -> bytes:
|
||||
"""
|
||||
Safely fetch a URL with SSRF protections.
|
||||
|
||||
Args:
|
||||
url: URL to fetch.
|
||||
allow_hosts: Allowed hosts (required, deny-by-default).
|
||||
max_bytes: Maximum response size.
|
||||
timeout_sec: Request timeout.
|
||||
max_redirects: Maximum redirects to follow (0 = none).
|
||||
|
||||
Returns:
|
||||
Response body as bytes.
|
||||
|
||||
Raises:
|
||||
SSRFError: If URL or resolved IP is blocked.
|
||||
|
||||
Note:
|
||||
- System proxies are disabled to prevent SSRF bypass.
|
||||
- Each redirect hop is re-validated against allowlist.
|
||||
Safely fetch a URL with SSRF protections and IP pinning.
|
||||
"""
|
||||
import urllib.error
|
||||
import urllib.request
|
||||
import urllib.parse
|
||||
|
||||
# Validate initial URL
|
||||
validate_outbound_url(url, allow_hosts=allow_hosts)
|
||||
current_url = url
|
||||
redirects_followed = 0
|
||||
|
||||
# Build request
|
||||
request = urllib.request.Request(url)
|
||||
try:
|
||||
from ..config import PACK_VERSION
|
||||
except ImportError: # pragma: no cover
|
||||
from config import PACK_VERSION # type: ignore
|
||||
request.add_header("User-Agent", f"ComfyUI-OpenClaw/{PACK_VERSION}")
|
||||
while True:
|
||||
# Validate initial URL and resolve IPs
|
||||
scheme, host, port, pinned_ips = validate_outbound_url(
|
||||
current_url, allow_hosts=allow_hosts
|
||||
)
|
||||
|
||||
# Track redirect count for validation
|
||||
redirect_count = [0] # Use list for mutability in nested class
|
||||
|
||||
class SafeRedirectHandler(urllib.request.HTTPRedirectHandler):
|
||||
"""Redirect handler that re-validates each hop."""
|
||||
|
||||
def redirect_request(self, req, fp, code, msg, headers, newurl):
|
||||
if max_redirects == 0:
|
||||
raise SSRFError(f"Redirects disabled. Redirect to: {newurl}")
|
||||
|
||||
redirect_count[0] += 1
|
||||
if redirect_count[0] > max_redirects:
|
||||
raise SSRFError(f"Too many redirects (max {max_redirects})")
|
||||
|
||||
# Re-validate the redirect URL (SSRF check on each hop)
|
||||
# Build request
|
||||
request = urllib.request.Request(current_url)
|
||||
try:
|
||||
from ..config import PACK_VERSION
|
||||
except ImportError: # pragma: no cover
|
||||
try:
|
||||
validate_outbound_url(newurl, allow_hosts=allow_hosts)
|
||||
except SSRFError as e:
|
||||
raise SSRFError(f"Redirect blocked: {e}")
|
||||
from config import PACK_VERSION # type: ignore
|
||||
except ImportError:
|
||||
PACK_VERSION = "0.0.0"
|
||||
|
||||
return super().redirect_request(req, fp, code, msg, headers, newurl)
|
||||
request.add_header("User-Agent", f"ComfyUI-OpenClaw/{PACK_VERSION}")
|
||||
|
||||
# Build opener with:
|
||||
# 1. No proxies (prevent SSRF bypass via system proxy)
|
||||
# 2. Safe redirect handler
|
||||
opener = urllib.request.build_opener(
|
||||
urllib.request.ProxyHandler({}), SafeRedirectHandler() # Disable all proxies
|
||||
)
|
||||
# Build S37-hardened pinned opener
|
||||
opener = _build_pinned_opener(pinned_ips)
|
||||
|
||||
try:
|
||||
with opener.open(request, timeout=timeout_sec) as response:
|
||||
return response.read(max_bytes)
|
||||
except urllib.error.URLError as e:
|
||||
raise SSRFError(f"Fetch failed: {e}")
|
||||
try:
|
||||
with opener.open(request, timeout=timeout_sec) as response:
|
||||
code = response.getcode()
|
||||
|
||||
# Handle redirects manually (NoRedirectHandler returns a 3xx response object).
|
||||
if code in (301, 302, 303, 307, 308):
|
||||
if max_redirects > 0 and redirects_followed < max_redirects:
|
||||
redirects_followed += 1
|
||||
new_loc = response.headers.get("Location")
|
||||
if not new_loc:
|
||||
raise SSRFError(f"Redirect without Location header: {code}")
|
||||
|
||||
# Resolve relative URL
|
||||
current_url = urllib.parse.urljoin(current_url, new_loc)
|
||||
continue
|
||||
raise SSRFError(
|
||||
f"Steps limit exceeded or redirects disabled: {max_redirects}"
|
||||
)
|
||||
|
||||
return response.read(max_bytes)
|
||||
|
||||
except urllib.error.HTTPError as e:
|
||||
# Should mostly catch 4xx/5xx only
|
||||
raise SSRFError(f"Fetch failed: {e}")
|
||||
except urllib.error.URLError as e:
|
||||
if isinstance(e.reason, SSRFError):
|
||||
raise e.reason
|
||||
raise SSRFError(f"Fetch failed: {e}")
|
||||
|
||||
|
||||
def safe_request_json(
|
||||
@@ -400,83 +467,83 @@ def safe_request_json(
|
||||
) -> dict:
|
||||
"""
|
||||
Perform a safe HTTP request with JSON body (e.g., POST callback).
|
||||
|
||||
Args:
|
||||
method: HTTP method (GET, POST, etc.).
|
||||
url: Target URL.
|
||||
json_body: JSON-serializable body (will be encoded).
|
||||
allow_hosts: Allowed hosts (required).
|
||||
headers: Optional headers (only safe ones allowed).
|
||||
timeout_sec: Request timeout.
|
||||
max_response_bytes: Max response size.
|
||||
max_redirects: Max redirects to follow.
|
||||
|
||||
Returns:
|
||||
Parsed JSON response dict or empty dict on non-JSON response.
|
||||
|
||||
Raises:
|
||||
SSRFError: If URL or resolved IP is blocked.
|
||||
"""
|
||||
import json
|
||||
import urllib.error
|
||||
import urllib.request
|
||||
import urllib.parse
|
||||
|
||||
# Validate URL
|
||||
validate_outbound_url(url, allow_hosts=allow_hosts)
|
||||
current_url = url
|
||||
current_method = method
|
||||
current_body = json.dumps(json_body).encode("utf-8") if json_body else None
|
||||
redirects_followed = 0
|
||||
|
||||
# Prepare body
|
||||
body_bytes = json.dumps(json_body).encode("utf-8") if json_body else None
|
||||
while True:
|
||||
# Validate URL + Pin IPs
|
||||
scheme, host, port, pinned_ips = validate_outbound_url(
|
||||
current_url, allow_hosts=allow_hosts
|
||||
)
|
||||
|
||||
# Build request
|
||||
request = urllib.request.Request(url, data=body_bytes, method=method)
|
||||
try:
|
||||
from ..config import PACK_VERSION
|
||||
except ImportError: # pragma: no cover
|
||||
from config import PACK_VERSION # type: ignore
|
||||
request.add_header("User-Agent", f"ComfyUI-OpenClaw/{PACK_VERSION}")
|
||||
request.add_header("Content-Type", "application/json")
|
||||
|
||||
# Add safe headers (allowlist prefixes)
|
||||
ALLOWED_HEADER_PREFIXES = ("x-", "content-type")
|
||||
if headers:
|
||||
for key, value in headers.items():
|
||||
key_lower = key.lower()
|
||||
if any(key_lower.startswith(p) for p in ALLOWED_HEADER_PREFIXES):
|
||||
request.add_header(key, value)
|
||||
else:
|
||||
logger.debug(f"Skipping disallowed header: {key}")
|
||||
|
||||
# Track redirects
|
||||
redirect_count = [0]
|
||||
|
||||
class SafeRedirectHandler(urllib.request.HTTPRedirectHandler):
|
||||
def redirect_request(self, req, fp, code, msg, hdrs, newurl):
|
||||
if max_redirects == 0:
|
||||
raise SSRFError(f"Redirects disabled. Redirect to: {newurl}")
|
||||
redirect_count[0] += 1
|
||||
if redirect_count[0] > max_redirects:
|
||||
raise SSRFError(f"Too many redirects (max {max_redirects})")
|
||||
# Build request
|
||||
request = urllib.request.Request(
|
||||
current_url, data=current_body, method=current_method
|
||||
)
|
||||
try:
|
||||
from ..config import PACK_VERSION
|
||||
except ImportError: # pragma: no cover
|
||||
try:
|
||||
validate_outbound_url(newurl, allow_hosts=allow_hosts)
|
||||
except SSRFError as e:
|
||||
raise SSRFError(f"Redirect blocked: {e}")
|
||||
return super().redirect_request(req, fp, code, msg, hdrs, newurl)
|
||||
from config import PACK_VERSION # type: ignore
|
||||
except ImportError:
|
||||
PACK_VERSION = "0.0.0"
|
||||
|
||||
opener = urllib.request.build_opener(
|
||||
urllib.request.ProxyHandler({}),
|
||||
SafeRedirectHandler(),
|
||||
)
|
||||
request.add_header("User-Agent", f"ComfyUI-OpenClaw/{PACK_VERSION}")
|
||||
request.add_header("Content-Type", "application/json")
|
||||
|
||||
try:
|
||||
with opener.open(request, timeout=timeout_sec) as response:
|
||||
data = response.read(max_response_bytes)
|
||||
try:
|
||||
return json.loads(data.decode("utf-8"))
|
||||
except (json.JSONDecodeError, UnicodeDecodeError):
|
||||
return {"raw_response": data.decode("utf-8", errors="replace")[:1000]}
|
||||
except urllib.error.HTTPError as e:
|
||||
# HTTP errors are not SSRF; keep SSRFError only for validation/redirect blocks
|
||||
raise RuntimeError(f"HTTP error {e.code}: {e.reason}")
|
||||
except urllib.error.URLError as e:
|
||||
# Network errors are not SSRF; allow caller to retry
|
||||
raise RuntimeError(f"Request failed: {e}")
|
||||
# Add safe headers
|
||||
ALLOWED_HEADER_PREFIXES = ("x-", "content-type")
|
||||
if headers:
|
||||
for key, value in headers.items():
|
||||
key_lower = key.lower()
|
||||
if any(key_lower.startswith(p) for p in ALLOWED_HEADER_PREFIXES):
|
||||
request.add_header(key, value)
|
||||
else:
|
||||
logger.debug(f"Skipping disallowed header: {key}")
|
||||
|
||||
# Build Pinned Opener
|
||||
opener = _build_pinned_opener(pinned_ips)
|
||||
|
||||
try:
|
||||
with opener.open(request, timeout=timeout_sec) as response:
|
||||
code = response.getcode()
|
||||
|
||||
if code in (301, 302, 303, 307, 308):
|
||||
if max_redirects > 0 and redirects_followed < max_redirects:
|
||||
redirects_followed += 1
|
||||
new_loc = response.headers.get("Location")
|
||||
if not new_loc:
|
||||
raise RuntimeError(f"Redirect without Location: {code}")
|
||||
|
||||
current_url = urllib.parse.urljoin(current_url, new_loc)
|
||||
|
||||
# Handle Method/Body transformation rules
|
||||
if code in (301, 302, 303):
|
||||
current_method = "GET"
|
||||
current_body = None
|
||||
|
||||
continue
|
||||
raise RuntimeError(f"Too many redirects: {max_redirects}")
|
||||
|
||||
data = response.read(max_response_bytes)
|
||||
try:
|
||||
return json.loads(data.decode("utf-8"))
|
||||
except (json.JSONDecodeError, UnicodeDecodeError):
|
||||
return {
|
||||
"raw_response": data.decode("utf-8", errors="replace")[:1000]
|
||||
}
|
||||
|
||||
except urllib.error.HTTPError as e:
|
||||
raise RuntimeError(f"HTTP error {e.code}: {e.reason}")
|
||||
|
||||
except urllib.error.URLError as e:
|
||||
if isinstance(e.reason, SSRFError):
|
||||
raise e.reason
|
||||
raise RuntimeError(f"Request failed: {e}")
|
||||
|
||||
+18
-10
@@ -86,12 +86,13 @@ def is_auth_configured() -> bool:
|
||||
|
||||
|
||||
def should_require_replay_protection() -> bool:
|
||||
"""Check if replay protection is strictly required."""
|
||||
"""Check if replay protection is strictly required (S36 fail-closed default)."""
|
||||
# S36: Default to strict (1). Use "0" or "false" to opt-out (legacy compat).
|
||||
val = (
|
||||
_env_get(
|
||||
ENV_REQUIRE_REPLAY_PROTECTION, LEGACY_ENV_REQUIRE_REPLAY_PROTECTION, "0"
|
||||
ENV_REQUIRE_REPLAY_PROTECTION, LEGACY_ENV_REQUIRE_REPLAY_PROTECTION, "1"
|
||||
)
|
||||
or "0"
|
||||
or "1"
|
||||
).lower()
|
||||
return val in ("1", "true", "yes")
|
||||
|
||||
@@ -206,13 +207,20 @@ def verify_hmac(request: RequestLike, raw_body: bytes) -> Tuple[bool, str]:
|
||||
pass
|
||||
|
||||
if IdempotencyStore:
|
||||
store = IdempotencyStore()
|
||||
# Nonce key
|
||||
nonce_key = f"nonce:{nonce}"
|
||||
# TTL should allow for the drift window (buffer)
|
||||
is_dup, _ = store.check_and_record(nonce_key, ttl=600)
|
||||
if is_dup:
|
||||
return False, "nonce_used"
|
||||
try:
|
||||
store = IdempotencyStore()
|
||||
# Nonce key
|
||||
nonce_key = f"nonce:{nonce}"
|
||||
# TTL should allow for the drift window (buffer)
|
||||
is_dup, _ = store.check_and_record(nonce_key, ttl=600)
|
||||
if is_dup:
|
||||
return False, "nonce_used"
|
||||
except Exception as e:
|
||||
logger.error(f"Idempotency store check failed: {e}")
|
||||
if should_require_replay_protection():
|
||||
return False, "internal_error"
|
||||
# Else proceed (allow open in legacy/relaxed mode - risk acceptance)
|
||||
pass
|
||||
else:
|
||||
logger.warning("IdempotencyStore not available for nonce check")
|
||||
# Fail closed if configured to require protection, otherwise warn
|
||||
|
||||
@@ -40,12 +40,13 @@ class TestCallbackUrlPolicy(unittest.TestCase):
|
||||
mock_dns.return_value = [
|
||||
(2, 1, 6, "", ("93.184.216.34", 443)) # Public IP for example.com
|
||||
]
|
||||
scheme, host, port = validate_outbound_url(
|
||||
scheme, host, port, ips = validate_outbound_url(
|
||||
"https://example.com/hook", allow_hosts={"example.com"}
|
||||
)
|
||||
self.assertEqual(scheme, "https")
|
||||
self.assertEqual(host, "example.com")
|
||||
self.assertEqual(port, 443)
|
||||
self.assertEqual(ips, ["93.184.216.34"])
|
||||
mock_dns.assert_called_once()
|
||||
|
||||
def test_private_ip_blocked(self):
|
||||
|
||||
@@ -0,0 +1,325 @@
|
||||
"""
|
||||
S36/S37/R79 Webhook Replay & Egress Hardening Tests.
|
||||
|
||||
Covers:
|
||||
- S36: Webhook HMAC replay protections (strict default + escape hatch + dedupe failure).
|
||||
- S37: Safe IO connect-time anti-rebinding (Pinning verification).
|
||||
- R79: Egress static analysis contract (no direct network calls outside approved list).
|
||||
"""
|
||||
|
||||
import logging
|
||||
import os
|
||||
import socket
|
||||
import sys
|
||||
import unittest
|
||||
import urllib.error
|
||||
import urllib.request
|
||||
from unittest.mock import ANY, MagicMock, patch
|
||||
|
||||
import services.safe_io as safe_io
|
||||
import services.webhook_auth as webhook_auth
|
||||
|
||||
# Disable logging during tests to avoid noise
|
||||
logging.disable(logging.CRITICAL)
|
||||
|
||||
|
||||
class TestS36WebhookReplay(unittest.TestCase):
|
||||
|
||||
# Removed setUp to avoid clearing os.environ globally which causes issues
|
||||
|
||||
def test_s36_default_strict(self):
|
||||
"""S36: should_require_replay_protection defaults to True (Strict)."""
|
||||
# Ensure env var is not set effectively
|
||||
with patch.dict(os.environ, {}, clear=True):
|
||||
self.assertTrue(webhook_auth.should_require_replay_protection())
|
||||
|
||||
def test_s36_escape_hatch(self):
|
||||
"""S36: Explicit '0' or 'false' disables strict mode."""
|
||||
with patch.dict(
|
||||
os.environ, {"OPENCLAW_WEBHOOK_REQUIRE_REPLAY_PROTECTION": "0"}
|
||||
):
|
||||
self.assertFalse(webhook_auth.should_require_replay_protection())
|
||||
|
||||
with patch.dict(
|
||||
os.environ, {"OPENCLAW_WEBHOOK_REQUIRE_REPLAY_PROTECTION": "false"}
|
||||
):
|
||||
self.assertFalse(webhook_auth.should_require_replay_protection())
|
||||
|
||||
def test_verify_hmac_strict_missing_headers(self):
|
||||
"""S36: Strict mode rejects HMAC without timestamp/nonce."""
|
||||
secret = b"test_secret"
|
||||
with (
|
||||
patch("services.webhook_auth.get_hmac_secret", return_value=secret),
|
||||
patch(
|
||||
"services.webhook_auth.should_require_replay_protection",
|
||||
return_value=True,
|
||||
),
|
||||
):
|
||||
|
||||
# Req with Signature but no TS/Nonce
|
||||
req = MagicMock()
|
||||
sig = webhook_auth.hmac.new(secret, b"body", "sha256").hexdigest()
|
||||
req.headers = {"X-OpenClaw-Signature": "sha256=" + sig}
|
||||
# Also legacy sig to be safe?
|
||||
|
||||
valid, err = webhook_auth.verify_hmac(req, b"body")
|
||||
self.assertFalse(valid)
|
||||
self.assertIn("missing_timestamp", err)
|
||||
|
||||
def test_verify_hmac_legacy_compat(self):
|
||||
"""S36: Compat mode allows HMAC without timestamp/nonce."""
|
||||
secret = b"test_secret"
|
||||
with (
|
||||
patch("services.webhook_auth.get_hmac_secret", return_value=secret),
|
||||
patch(
|
||||
"services.webhook_auth.should_require_replay_protection",
|
||||
return_value=False,
|
||||
),
|
||||
):
|
||||
|
||||
req = MagicMock()
|
||||
sig = webhook_auth.hmac.new(secret, b"body", "sha256").hexdigest()
|
||||
req.headers = {"X-OpenClaw-Signature": "sha256=" + sig}
|
||||
|
||||
valid, err = webhook_auth.verify_hmac(req, b"body")
|
||||
self.assertTrue(valid, f"Should be valid in compat mode. Error: {err}")
|
||||
|
||||
def test_dedupe_unavailable_fail_closed(self):
|
||||
"""S36: Fail closed when dedupe backend is unavailable in strict mode."""
|
||||
secret = b"test_secret"
|
||||
|
||||
# Patch sys.modules to simulate ImportError for IdempotencyStore
|
||||
with (
|
||||
patch.dict("sys.modules", {"services.idempotency_store": None}),
|
||||
patch("services.webhook_auth.get_hmac_secret", return_value=secret),
|
||||
patch(
|
||||
"services.webhook_auth.should_require_replay_protection",
|
||||
return_value=True,
|
||||
),
|
||||
):
|
||||
|
||||
req = MagicMock()
|
||||
# Valid signature headers (includes TS/Nonce)
|
||||
# Timestamp must be current to pass drift check
|
||||
import time
|
||||
|
||||
now = int(time.time())
|
||||
ts = str(now)
|
||||
nonce = "bfs325"
|
||||
body = b"body"
|
||||
|
||||
# Re-compute sig! verify_hmac computes it from body arg.
|
||||
expected_sig = webhook_auth.hmac.new(secret, body, "sha256").hexdigest()
|
||||
|
||||
req.headers = {
|
||||
"X-OpenClaw-Signature": "sha256=" + expected_sig,
|
||||
"X-OpenClaw-Timestamp": ts,
|
||||
"X-OpenClaw-Nonce": nonce,
|
||||
}
|
||||
|
||||
valid, err = webhook_auth.verify_hmac(req, body)
|
||||
|
||||
# Should fail at dedupe step because IdempotencyStore import failed
|
||||
self.assertEqual(err, "internal_error")
|
||||
|
||||
def test_dedupe_runtime_error_fail_closed(self):
|
||||
"""S36: Fail closed when dedupe backend raises runtime exception (e.g. Redis down)."""
|
||||
secret = b"test_secret"
|
||||
|
||||
with (
|
||||
patch("services.webhook_auth.get_hmac_secret", return_value=secret),
|
||||
patch(
|
||||
"services.webhook_auth.should_require_replay_protection",
|
||||
return_value=True,
|
||||
),
|
||||
):
|
||||
# Mock store to raise Exception
|
||||
mock_store = MagicMock()
|
||||
mock_store.check_and_record.side_effect = RuntimeError("Connection refused")
|
||||
|
||||
with patch(
|
||||
"services.idempotency_store.IdempotencyStore", return_value=mock_store
|
||||
):
|
||||
req = MagicMock()
|
||||
import time
|
||||
|
||||
now = int(time.time())
|
||||
ts = str(now)
|
||||
nonce = "runtime_fail"
|
||||
body = b"body"
|
||||
sig = webhook_auth.hmac.new(secret, body, "sha256").hexdigest()
|
||||
|
||||
req.headers = {
|
||||
"X-OpenClaw-Signature": "sha256=" + sig,
|
||||
"X-OpenClaw-Timestamp": ts,
|
||||
"X-OpenClaw-Nonce": nonce,
|
||||
}
|
||||
|
||||
valid, err = webhook_auth.verify_hmac(req, body)
|
||||
|
||||
self.assertFalse(valid)
|
||||
self.assertEqual(err, "internal_error")
|
||||
|
||||
|
||||
class TestS37SafeIORebinding(unittest.TestCase):
|
||||
|
||||
def test_safe_fetch_pinning_discipline(self):
|
||||
"""S37: safe_fetch should connect to the RESOLVED IP, not the hostname (Anti-Rebinding)."""
|
||||
pinned_ip = "93.184.216.34"
|
||||
|
||||
# We need to ensure socket.create_connection is patched globally
|
||||
# And validate_outbound_url returns our pinned IP.
|
||||
|
||||
with (
|
||||
patch("services.safe_io.validate_outbound_url") as mock_validate,
|
||||
patch("socket.create_connection") as mock_create_conn,
|
||||
patch("ssl.SSLContext.wrap_socket") as mock_wrap,
|
||||
):
|
||||
|
||||
# 4-tuple return check!
|
||||
# 4-tuple return check!
|
||||
mock_validate.return_value = ("https", "example.com", 443, [pinned_ip])
|
||||
|
||||
mock_sock = MagicMock()
|
||||
mock_create_conn.return_value = mock_sock
|
||||
mock_wrap.return_value = mock_sock # Wrap returns the socket (wrapped)
|
||||
mock_sock.getpeername.return_value = (pinned_ip, 443)
|
||||
|
||||
# We expect network/protocol error because mock sock is empty
|
||||
try:
|
||||
safe_io.safe_fetch("https://example.com", allow_hosts={"example.com"})
|
||||
except Exception as e:
|
||||
pass
|
||||
|
||||
# VERIFY PINNING
|
||||
# The FIRST call to create_connection must be with the PINNED IP
|
||||
mock_create_conn.assert_called_with((pinned_ip, 443), ANY, ANY)
|
||||
|
||||
# VERIFY SNI
|
||||
# wrap_socket on a real Context instance calls the class method patch?
|
||||
# Arguments: (self, sock, server_side=False, do_handshake_on_connect=True, suppress_ragged_eofs=True, server_hostname=None, session=None)
|
||||
# We just check kwargs server_hostname
|
||||
|
||||
# Match call args to verify sock and hostname
|
||||
# mock_wrap.call_args[0][0] is self (Context)
|
||||
# mock_wrap.call_args[0][1] is sock ??
|
||||
# Or kwargs?
|
||||
# http.client calls context.wrap_socket(sock, server_hostname=...)
|
||||
|
||||
self.assertTrue(
|
||||
mock_wrap.called, "ssl.SSLContext.wrap_socket was not called"
|
||||
)
|
||||
args, kwargs = mock_wrap.call_args
|
||||
# args[0] is self (context object) if method bound? No, patch replaces unbound function on class.
|
||||
# So args[0] is context instance.
|
||||
# args[1] is sock.
|
||||
|
||||
self.assertEqual(kwargs.get("server_hostname"), "example.com")
|
||||
# Verify socket passed is our mock sock
|
||||
# Implementation note: Patching class method with Mock replaces it with unbound Mock.
|
||||
# When called on instance, it doesn't receive 'self' unless configured.
|
||||
# So args[0] is likely 'sock'.
|
||||
sock_arg = kwargs.get("sock")
|
||||
if sock_arg is None and len(args) > 0:
|
||||
sock_arg = args[0]
|
||||
self.assertIs(sock_arg, mock_sock)
|
||||
|
||||
|
||||
class TestR79EgressCompliance(unittest.TestCase):
|
||||
|
||||
def test_no_unsafe_primitives(self):
|
||||
"""R79: Static check for unsafe outbound calls outside safe_io (Allowlist Enforcement)."""
|
||||
|
||||
FORBIDDEN = [
|
||||
("requests.get(", "Direct requests.get"),
|
||||
("requests.post(", "Direct requests.post"),
|
||||
("requests.put(", "Direct requests.put"),
|
||||
("requests.delete(", "Direct requests.delete"),
|
||||
("requests.request(", "Direct requests.request"),
|
||||
("requests.Session(", "Direct requests.Session"),
|
||||
("urllib.request.urlopen(", "Direct urllib urlopen"),
|
||||
("urllib3.PoolManager(", "Direct urllib3.PoolManager"),
|
||||
("urllib3.request(", "Direct urllib3.request"),
|
||||
("httpx.Client(", "Direct httpx.Client"),
|
||||
("httpx.AsyncClient(", "Direct httpx.AsyncClient"),
|
||||
("httpx.request(", "Direct httpx.request"),
|
||||
("httpx.get(", "Direct httpx.get"),
|
||||
("httpx.post(", "Direct httpx.post"),
|
||||
("aiohttp.ClientSession", "Direct aiohttp ClientSession"),
|
||||
("aiohttp.request(", "Direct aiohttp.request"),
|
||||
]
|
||||
|
||||
# Explicit Allowed Files (Legacy or Approved Infrastructure)
|
||||
# Sourced from scan_r79.py output
|
||||
ALLOWED_FILES = {
|
||||
"services/safe_io.py",
|
||||
"services/llm_client.py",
|
||||
"services/webhook_auth.py",
|
||||
"connector/base.py",
|
||||
# Legacy/Approved Egress Paths
|
||||
"api/config.py",
|
||||
"services/queue_submit.py",
|
||||
# Connector Implementations
|
||||
"connector/llm_client.py",
|
||||
"connector/openclaw_client.py",
|
||||
"connector/platforms/discord_gateway.py",
|
||||
"connector/platforms/line_webhook.py",
|
||||
"connector/platforms/telegram_polling.py",
|
||||
"connector/platforms/wechat_webhook.py",
|
||||
"connector/platforms/whatsapp_webhook.py",
|
||||
# Providers
|
||||
"services/providers/anthropic.py",
|
||||
"services/providers/openai_compat.py",
|
||||
"services/providers/openai.py", # Allowed if present
|
||||
}
|
||||
|
||||
SKIP_DIRS = [
|
||||
"tests",
|
||||
"venv",
|
||||
".git",
|
||||
"__pycache__",
|
||||
"node_modules",
|
||||
"scripts",
|
||||
"REFERENCE",
|
||||
".agent",
|
||||
".planning",
|
||||
]
|
||||
|
||||
root_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))
|
||||
|
||||
violations = []
|
||||
|
||||
for dirpath, dirnames, filenames in os.walk(root_dir):
|
||||
dirnames[:] = [
|
||||
d for d in dirnames if d not in SKIP_DIRS and not d.startswith(".")
|
||||
]
|
||||
|
||||
for f in filenames:
|
||||
if not f.endswith(".py"):
|
||||
continue
|
||||
|
||||
full_path = os.path.join(dirpath, f)
|
||||
rel_path = os.path.relpath(full_path, root_dir).replace("\\", "/")
|
||||
|
||||
if rel_path in ALLOWED_FILES:
|
||||
continue
|
||||
|
||||
try:
|
||||
with open(full_path, "r", encoding="utf-8") as f_obj:
|
||||
content = f_obj.read()
|
||||
|
||||
for pattern, desc in FORBIDDEN:
|
||||
if pattern in content:
|
||||
violations.append(f"{rel_path}: Found {desc}")
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
if violations:
|
||||
self.fail(
|
||||
"R79 Egress Violations Found (Please fix or add to ALLOWED_FILES):\n"
|
||||
+ "\n".join(violations)
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -168,7 +168,7 @@ class TestURLSafety(unittest.TestCase):
|
||||
result = validate_outbound_url(
|
||||
"https://example.com", allow_hosts={"example.com"}
|
||||
)
|
||||
self.assertEqual(result, ("https", "example.com", 443))
|
||||
self.assertEqual(result, ("https", "example.com", 443, ["93.184.216.34"]))
|
||||
|
||||
@patch("socket.getaddrinfo")
|
||||
def test_reject_private_ip_from_dns(self, mock_dns):
|
||||
|
||||
@@ -101,7 +101,13 @@ class TestVerifyHmac(unittest.TestCase):
|
||||
body = b'{"test": "data"}'
|
||||
expected_sig = hmac.new(secret.encode(), body, hashlib.sha256).hexdigest()
|
||||
|
||||
with patch.dict(os.environ, {"MOLTBOT_WEBHOOK_HMAC_SECRET": secret}):
|
||||
with patch.dict(
|
||||
os.environ,
|
||||
{
|
||||
"MOLTBOT_WEBHOOK_HMAC_SECRET": secret,
|
||||
"MOLTBOT_WEBHOOK_REQUIRE_REPLAY_PROTECTION": "0", # Disable S36 strict default
|
||||
},
|
||||
):
|
||||
request = MockRequest(
|
||||
headers={"X-Moltbot-Signature": f"sha256={expected_sig}"}
|
||||
)
|
||||
@@ -159,6 +165,7 @@ class TestRequireAuth(unittest.TestCase):
|
||||
{
|
||||
"MOLTBOT_WEBHOOK_AUTH_MODE": "hmac",
|
||||
"MOLTBOT_WEBHOOK_HMAC_SECRET": secret,
|
||||
"MOLTBOT_WEBHOOK_REQUIRE_REPLAY_PROTECTION": "0", # Disable S36 strict default
|
||||
},
|
||||
):
|
||||
request = MockRequest(headers={"X-Moltbot-Signature": f"sha256={sig}"})
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
"""
|
||||
R79 Egress Compliance Scanner.
|
||||
Checks codebase for forbidden outbound network primitives.
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
|
||||
# Safe egress primitives
|
||||
ALLOWED_FILES = {
|
||||
"services/safe_io.py",
|
||||
"services/llm_client.py",
|
||||
"services/webhook_auth.py",
|
||||
"connector/base.py",
|
||||
# Legacy/Approved Egress Paths
|
||||
"api/config.py",
|
||||
"services/queue_submit.py",
|
||||
# Connector Implementations
|
||||
"connector/llm_client.py",
|
||||
"connector/openclaw_client.py",
|
||||
"connector/platforms/discord_gateway.py",
|
||||
"connector/platforms/line_webhook.py",
|
||||
"connector/platforms/telegram_polling.py",
|
||||
"connector/platforms/wechat_webhook.py",
|
||||
"connector/platforms/whatsapp_webhook.py",
|
||||
# Providers
|
||||
"services/providers/anthropic.py",
|
||||
"services/providers/openai_compat.py",
|
||||
"services/providers/openai.py",
|
||||
}
|
||||
|
||||
FORBIDDEN = [
|
||||
("requests.get(", "Direct requests.get"),
|
||||
("requests.post(", "Direct requests.post"),
|
||||
("requests.put(", "Direct requests.put"),
|
||||
("requests.delete(", "Direct requests.delete"),
|
||||
("requests.request(", "Direct requests.request"),
|
||||
("requests.Session(", "Direct requests.Session"),
|
||||
("urllib.request.urlopen(", "Direct urllib urlopen"),
|
||||
("urllib3.PoolManager(", "Direct urllib3.PoolManager"),
|
||||
("urllib3.request(", "Direct urllib3.request"),
|
||||
("httpx.Client(", "Direct httpx.Client"),
|
||||
("httpx.AsyncClient(", "Direct httpx.AsyncClient"),
|
||||
("httpx.request(", "Direct httpx.request"),
|
||||
("httpx.get(", "Direct httpx.get"),
|
||||
("httpx.post(", "Direct httpx.post"),
|
||||
("aiohttp.ClientSession", "Direct aiohttp ClientSession"),
|
||||
("aiohttp.request(", "Direct aiohttp.request"),
|
||||
]
|
||||
|
||||
SKIP_DIRS = [
|
||||
"tests",
|
||||
"venv",
|
||||
".git",
|
||||
"__pycache__",
|
||||
"node_modules",
|
||||
"scripts",
|
||||
"REFERENCE",
|
||||
".agent",
|
||||
".planning",
|
||||
]
|
||||
|
||||
|
||||
def scan():
|
||||
print("Starting R79 Egress Compliance Scan...")
|
||||
root_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))
|
||||
|
||||
violations = []
|
||||
|
||||
for dirpath, dirnames, filenames in os.walk(root_dir):
|
||||
dirnames[:] = [
|
||||
d for d in dirnames if d not in SKIP_DIRS and not d.startswith(".")
|
||||
]
|
||||
|
||||
for f in filenames:
|
||||
if not f.endswith(".py"):
|
||||
continue
|
||||
|
||||
full_path = os.path.join(dirpath, f)
|
||||
rel_path = os.path.relpath(full_path, root_dir).replace("\\", "/")
|
||||
|
||||
if rel_path in ALLOWED_FILES:
|
||||
continue
|
||||
|
||||
try:
|
||||
with open(full_path, "r", encoding="utf-8") as f_obj:
|
||||
content = f_obj.read()
|
||||
|
||||
for pattern, desc in FORBIDDEN:
|
||||
if pattern in content:
|
||||
violations.append(f"{rel_path}: Found {desc}")
|
||||
except Exception as e:
|
||||
print(f"Error reading {rel_path}: {e}")
|
||||
|
||||
if violations:
|
||||
print("\n[FAIL] R79 Violations Found:")
|
||||
for v in violations:
|
||||
print(f" - {v}")
|
||||
sys.exit(1)
|
||||
else:
|
||||
print("\n[PASS] No egress violations found.")
|
||||
sys.exit(0)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
scan()
|
||||
Reference in New Issue
Block a user