mirror of
https://github.com/rookiestar28/ComfyUI-OpenClaw.git
synced 2026-08-14 00:48:07 +00:00
fix: honor insecure LLM base URL override at request time
This commit is contained in:
@@ -390,6 +390,14 @@ def _format_llm_ssrf_error(exc: Exception) -> str:
|
||||
)
|
||||
|
||||
|
||||
def _llm_insecure_override_enabled() -> bool:
|
||||
return _env_flag(
|
||||
"OPENCLAW_ALLOW_INSECURE_BASE_URL",
|
||||
"MOLTBOT_ALLOW_INSECURE_BASE_URL",
|
||||
default=False,
|
||||
)
|
||||
|
||||
|
||||
def _extract_models_from_payload(payload: dict) -> list:
|
||||
"""
|
||||
Extract model IDs from common provider responses.
|
||||
@@ -592,6 +600,7 @@ async def llm_models_handler(request: web.Request) -> web.Response:
|
||||
allow_hosts=controls.get("allow_hosts"),
|
||||
allow_any_public_host=bool(controls.get("allow_any_public_host")),
|
||||
allow_loopback_hosts=controls.get("allow_loopback_hosts"),
|
||||
allow_insecure_base_url=_llm_insecure_override_enabled(),
|
||||
policy=STANDARD_OUTBOUND_POLICY,
|
||||
)
|
||||
except Exception as e:
|
||||
@@ -634,6 +643,7 @@ async def llm_models_handler(request: web.Request) -> web.Response:
|
||||
allow_hosts=controls.get("allow_hosts"),
|
||||
allow_any_public_host=bool(controls.get("allow_any_public_host")),
|
||||
allow_loopback_hosts=controls.get("allow_loopback_hosts"),
|
||||
allow_insecure_base_url=_llm_insecure_override_enabled(),
|
||||
)
|
||||
|
||||
models = _extract_models_from_payload(payload)
|
||||
|
||||
+11
-11
@@ -361,6 +361,13 @@ class LLMClient:
|
||||
# ad-hoc allowlists causes path drift and can reintroduce S65 regressions.
|
||||
return get_llm_egress_controls(provider, base_url or "")
|
||||
|
||||
def _allow_insecure_base_url(self) -> bool:
|
||||
return (
|
||||
os.environ.get("OPENCLAW_ALLOW_INSECURE_BASE_URL")
|
||||
or os.environ.get("MOLTBOT_ALLOW_INSECURE_BASE_URL")
|
||||
or ""
|
||||
).strip().lower() in ("1", "true", "yes", "y", "on")
|
||||
|
||||
def _validate_candidate_url(self, provider: str, base_url: Optional[str]) -> bool:
|
||||
"""
|
||||
Validate base_url against S16/S16.1 SSRF policy.
|
||||
@@ -393,21 +400,11 @@ class LLMClient:
|
||||
allow_hosts=controls.get("allow_hosts"),
|
||||
allow_any_public_host=bool(controls.get("allow_any_public_host")),
|
||||
allow_loopback_hosts=controls.get("allow_loopback_hosts"),
|
||||
allow_insecure_base_url=self._allow_insecure_base_url(),
|
||||
policy=STANDARD_OUTBOUND_POLICY,
|
||||
)
|
||||
return True
|
||||
except Exception as e:
|
||||
# Allow override via explicit risk-acceptance flag (keeps behavior consistent with runtime_config validation).
|
||||
if (
|
||||
os.environ.get("OPENCLAW_ALLOW_INSECURE_BASE_URL")
|
||||
or os.environ.get("MOLTBOT_ALLOW_INSECURE_BASE_URL")
|
||||
or ""
|
||||
).strip().lower() in ("1", "true", "yes", "y", "on"):
|
||||
logger.warning(
|
||||
f"Failover candidate {provider} with base_url={base_url} allowed by "
|
||||
f"OPENCLAW_ALLOW_INSECURE_BASE_URL despite SSRF policy: {e}"
|
||||
)
|
||||
return True
|
||||
logger.warning(
|
||||
f"Failover candidate {provider} with base_url={base_url} "
|
||||
f"blocked by SSRF policy: {e}"
|
||||
@@ -908,6 +905,7 @@ class LLMClient:
|
||||
allow_hosts=egress_controls.get("allow_hosts"),
|
||||
allow_any_public_host=bool(egress_controls.get("allow_any_public_host")),
|
||||
allow_loopback_hosts=egress_controls.get("allow_loopback_hosts"),
|
||||
allow_insecure_base_url=self._allow_insecure_base_url(),
|
||||
)
|
||||
|
||||
def _complete_openai_compat(
|
||||
@@ -954,6 +952,7 @@ class LLMClient:
|
||||
egress_controls.get("allow_any_public_host")
|
||||
),
|
||||
allow_loopback_hosts=egress_controls.get("allow_loopback_hosts"),
|
||||
allow_insecure_base_url=self._allow_insecure_base_url(),
|
||||
on_text_delta=on_text_delta,
|
||||
)
|
||||
except Exception as e:
|
||||
@@ -975,6 +974,7 @@ class LLMClient:
|
||||
allow_hosts=egress_controls.get("allow_hosts"),
|
||||
allow_any_public_host=bool(egress_controls.get("allow_any_public_host")),
|
||||
allow_loopback_hosts=egress_controls.get("allow_loopback_hosts"),
|
||||
allow_insecure_base_url=self._allow_insecure_base_url(),
|
||||
)
|
||||
|
||||
def get_provider_summary(self) -> Dict[str, Any]:
|
||||
|
||||
@@ -74,6 +74,7 @@ def make_request(
|
||||
allow_hosts: Optional[set[str]] = None,
|
||||
allow_any_public_host: bool = False,
|
||||
allow_loopback_hosts: Optional[set[str]] = None,
|
||||
allow_insecure_base_url: bool = False,
|
||||
) -> Dict[str, Any]:
|
||||
"""
|
||||
Make a request to Anthropic /v1/messages endpoint.
|
||||
@@ -106,6 +107,7 @@ def make_request(
|
||||
allow_hosts=allow_hosts,
|
||||
allow_any_public_host=allow_any_public_host,
|
||||
allow_loopback_hosts=allow_loopback_hosts,
|
||||
allow_insecure_base_url=allow_insecure_base_url,
|
||||
)
|
||||
|
||||
# Extract text from response
|
||||
|
||||
@@ -100,6 +100,7 @@ def make_request(
|
||||
allow_hosts: Optional[set[str]] = None,
|
||||
allow_any_public_host: bool = False,
|
||||
allow_loopback_hosts: Optional[set[str]] = None,
|
||||
allow_insecure_base_url: bool = False,
|
||||
) -> Dict[str, Any]:
|
||||
"""
|
||||
Make a request to an OpenAI-compatible /chat/completions endpoint.
|
||||
@@ -134,6 +135,7 @@ def make_request(
|
||||
allow_hosts=allow_hosts,
|
||||
allow_any_public_host=allow_any_public_host,
|
||||
allow_loopback_hosts=allow_loopback_hosts,
|
||||
allow_insecure_base_url=allow_insecure_base_url,
|
||||
)
|
||||
|
||||
# Extract text from response
|
||||
@@ -206,6 +208,7 @@ def make_request_stream(
|
||||
allow_hosts: Optional[set[str]] = None,
|
||||
allow_any_public_host: bool = False,
|
||||
allow_loopback_hosts: Optional[set[str]] = None,
|
||||
allow_insecure_base_url: bool = False,
|
||||
on_text_delta: Optional[Callable[[str], None]] = None,
|
||||
max_preview_chars: int = 16000,
|
||||
) -> Dict[str, Any]:
|
||||
@@ -258,6 +261,7 @@ def make_request_stream(
|
||||
allow_hosts=allow_hosts,
|
||||
allow_any_public_host=allow_any_public_host,
|
||||
allow_loopback_hosts=allow_loopback_hosts,
|
||||
allow_insecure_base_url=allow_insecure_base_url,
|
||||
):
|
||||
line = line.rstrip("\r\n")
|
||||
if not line or line.startswith(":"):
|
||||
|
||||
+18
-4
@@ -353,6 +353,7 @@ def validate_outbound_url(
|
||||
allow_hosts: Optional[Set[str]] = None,
|
||||
allow_any_public_host: bool = False,
|
||||
allow_loopback_hosts: Optional[Set[str]] = None,
|
||||
allow_insecure_base_url: bool = False,
|
||||
policy: Optional[OutboundPolicy] = None,
|
||||
) -> Tuple[str, str, int, list[str]]:
|
||||
"""
|
||||
@@ -364,6 +365,9 @@ def validate_outbound_url(
|
||||
allow_any_public_host: If True, allow any host that resolves to a public IP.
|
||||
allow_loopback_hosts: Optional host allowlist for controlled loopback-only
|
||||
exceptions. This does not allow general private networks.
|
||||
allow_insecure_base_url: Explicit risk-acceptance override for LLM-only
|
||||
paths. Keeps URL syntax checks and IP pinning, but skips strict
|
||||
allowlist/scheme/private-IP blocking.
|
||||
policy: S51 OutboundPolicy for scheme+port enforcement.
|
||||
|
||||
Returns:
|
||||
@@ -389,14 +393,20 @@ def validate_outbound_url(
|
||||
|
||||
port = parsed.port or (443 if parsed.scheme == "https" else 80)
|
||||
|
||||
# CRITICAL: this override is only for explicit LLM risk-acceptance paths.
|
||||
# Do not enable it for callbacks/bridge/control-plane surfaces.
|
||||
# S51: enforce scheme+port policy if provided
|
||||
if policy is not None:
|
||||
if policy is not None and not allow_insecure_base_url:
|
||||
deny_reason = policy.validate(parsed.scheme, port)
|
||||
if deny_reason:
|
||||
raise SSRFError(deny_reason)
|
||||
|
||||
# Deny-by-default logic
|
||||
if not allow_any_public_host and allow_hosts is None:
|
||||
if (
|
||||
not allow_insecure_base_url
|
||||
and not allow_any_public_host
|
||||
and allow_hosts is None
|
||||
):
|
||||
raise SSRFError(
|
||||
"Outbound requests denied by default. Provide allow_hosts or allow_any_public_host."
|
||||
)
|
||||
@@ -408,7 +418,7 @@ def validate_outbound_url(
|
||||
}
|
||||
|
||||
# Check allowlist if provided or enforced
|
||||
if not allow_any_public_host:
|
||||
if not allow_insecure_base_url and not allow_any_public_host:
|
||||
if allow_hosts is None:
|
||||
raise SSRFError("No allow_hosts allowed")
|
||||
|
||||
@@ -424,7 +434,7 @@ def validate_outbound_url(
|
||||
)
|
||||
for _, _, _, _, sockaddr in addr_infos:
|
||||
ip = sockaddr[0]
|
||||
if is_private_ip(ip):
|
||||
if is_private_ip(ip) and not allow_insecure_base_url:
|
||||
# CRITICAL:
|
||||
# Only allow loopback IPs when the target host is explicitly listed in
|
||||
# allow_loopback_hosts. Never relax this into blanket private-IP allow.
|
||||
@@ -603,6 +613,7 @@ def safe_request_json(
|
||||
allow_hosts: Optional[Set[str]] = None,
|
||||
allow_any_public_host: bool = False,
|
||||
allow_loopback_hosts: Optional[Set[str]] = None,
|
||||
allow_insecure_base_url: bool = False,
|
||||
headers: Optional[dict] = None,
|
||||
timeout_sec: int = 10,
|
||||
max_response_bytes: int = 1_000_000,
|
||||
@@ -631,6 +642,7 @@ def safe_request_json(
|
||||
allow_hosts=allow_hosts,
|
||||
allow_any_public_host=allow_any_public_host,
|
||||
allow_loopback_hosts=allow_loopback_hosts,
|
||||
allow_insecure_base_url=allow_insecure_base_url,
|
||||
policy=policy,
|
||||
)
|
||||
|
||||
@@ -718,6 +730,7 @@ def safe_request_text_stream(
|
||||
allow_hosts: Optional[Set[str]] = None,
|
||||
allow_any_public_host: bool = False,
|
||||
allow_loopback_hosts: Optional[Set[str]] = None,
|
||||
allow_insecure_base_url: bool = False,
|
||||
headers: Optional[dict] = None,
|
||||
timeout_sec: int = 10,
|
||||
max_line_bytes: int = 64 * 1024,
|
||||
@@ -744,6 +757,7 @@ def safe_request_text_stream(
|
||||
allow_hosts=allow_hosts,
|
||||
allow_any_public_host=allow_any_public_host,
|
||||
allow_loopback_hosts=allow_loopback_hosts,
|
||||
allow_insecure_base_url=allow_insecure_base_url,
|
||||
policy=policy,
|
||||
)
|
||||
|
||||
|
||||
@@ -305,6 +305,41 @@ class TestModelListAPI(unittest.IsolatedAsyncioTestCase):
|
||||
data = json.loads(resp.body)
|
||||
self.assertIn("only supported for OpenAI-compatible", data["error"])
|
||||
|
||||
@patch("api.config.get_effective_config")
|
||||
@patch("services.providers.keys.get_api_key_for_provider")
|
||||
@patch("api.config.check_rate_limit")
|
||||
@patch("api.config.require_admin_token")
|
||||
@patch("services.safe_io.safe_request_json")
|
||||
@patch("services.safe_io.validate_outbound_url")
|
||||
async def test_handler_insecure_override_reaches_request_time(
|
||||
self,
|
||||
mock_validate_url,
|
||||
mock_safe_request,
|
||||
mock_require_admin,
|
||||
mock_rate_limit,
|
||||
mock_get_key,
|
||||
mock_get_config,
|
||||
):
|
||||
mock_rate_limit.return_value = True
|
||||
mock_require_admin.return_value = (True, None)
|
||||
mock_get_config.return_value = (
|
||||
{"provider": "openai", "base_url": "http://192.168.2.27:8000/v1"},
|
||||
{},
|
||||
)
|
||||
mock_get_key.return_value = "sk-test"
|
||||
mock_safe_request.return_value = {"data": [{"id": "model-a"}]}
|
||||
|
||||
request = MagicMock()
|
||||
request.query = {}
|
||||
request.remote = "127.0.0.1"
|
||||
|
||||
with patch.dict("os.environ", {"OPENCLAW_ALLOW_INSECURE_BASE_URL": "1"}):
|
||||
resp = await llm_models_handler(request)
|
||||
|
||||
self.assertEqual(resp.status, 200)
|
||||
self.assertTrue(mock_validate_url.call_args.kwargs["allow_insecure_base_url"])
|
||||
self.assertTrue(mock_safe_request.call_args.kwargs["allow_insecure_base_url"])
|
||||
|
||||
@patch("api.config.get_effective_config")
|
||||
@patch("services.providers.keys.get_api_key_for_provider")
|
||||
@patch("api.config.check_rate_limit")
|
||||
|
||||
@@ -37,6 +37,7 @@ class TestS65EgressPolicyParity(unittest.TestCase):
|
||||
self.assertIn("allow_hosts", kwargs)
|
||||
self.assertIn("allow_any_public_host", kwargs)
|
||||
self.assertIn("allow_loopback_hosts", kwargs)
|
||||
self.assertIn("allow_insecure_base_url", kwargs)
|
||||
|
||||
def test_openai_compat_uses_safe_io(self):
|
||||
with patch("services.providers.openai_compat.safe_request_json") as mock_safe:
|
||||
@@ -58,6 +59,7 @@ class TestS65EgressPolicyParity(unittest.TestCase):
|
||||
self.assertIn("allow_hosts", kwargs)
|
||||
self.assertIn("allow_any_public_host", kwargs)
|
||||
self.assertIn("allow_loopback_hosts", kwargs)
|
||||
self.assertIn("allow_insecure_base_url", kwargs)
|
||||
|
||||
|
||||
class TestS65ModelListEgressConvergence(unittest.TestCase):
|
||||
|
||||
Reference in New Issue
Block a user