mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
security(browser): enforce cloud-metadata floor on all backends; CDP is non-local
browser_navigate's always-blocked cloud-metadata floor (169.254.169.254,
metadata.google.internal, ECS/Azure/GCP IMDS) was gated on
`not _is_local_backend()`, contradicting both the adjacent comment and the
is_always_blocked_url docstring ("denied regardless of backend"). A default
local headless Chromium on a cloud VM — or an off-host CDP browser — could
navigate to IMDS and read instance credentials into the model context. Make the
floor unconditional on the initial-nav and post-redirect paths.
Also: _is_local_backend() ignored a CDP override while _is_local_mode() honors
it, so an off-host CDP browser was treated as "local" and skipped the broader
private/internal SSRF check too. Treat a CDP override as non-local.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
9f03095044
commit
0a75616514
3 changed files with 74 additions and 10 deletions
|
|
@ -162,6 +162,30 @@ class TestGetCdpOverride:
|
|||
assert resolved == WS_URL
|
||||
mock_get.assert_called_once_with(VERSION_URL, timeout=10)
|
||||
|
||||
def test_camofox_yields_to_config_cdp_override(self, monkeypatch):
|
||||
"""CAMOFOX_URL + a persistent browser.cdp_url config override must NOT
|
||||
report camofox mode: the CDP browser takes precedence so navigation is
|
||||
not routed through Camofox, and the CDP backend stays non-local for SSRF
|
||||
checks. Regression for the env-only suppression gap (config CDP was
|
||||
ignored, so CAMOFOX_URL + config CDP still dispatched to Camofox)."""
|
||||
import tools.browser_camofox as bc
|
||||
|
||||
monkeypatch.setenv("CAMOFOX_URL", "http://localhost:9377")
|
||||
monkeypatch.delenv("BROWSER_CDP_URL", raising=False)
|
||||
|
||||
# No CDP anywhere -> camofox mode is on.
|
||||
with patch("hermes_cli.config.read_raw_config", return_value={}):
|
||||
assert bc.is_camofox_mode() is True
|
||||
|
||||
# A config-only CDP override suppresses camofox.
|
||||
with patch("hermes_cli.config.read_raw_config",
|
||||
return_value={"browser": {"cdp_url": HTTP_URL}}):
|
||||
assert bc.is_camofox_mode() is False
|
||||
|
||||
# The env override still suppresses camofox.
|
||||
monkeypatch.setenv("BROWSER_CDP_URL", HTTP_URL)
|
||||
with patch("hermes_cli.config.read_raw_config", return_value={}):
|
||||
assert bc.is_camofox_mode() is False
|
||||
|
||||
class TestCreateCdpSession:
|
||||
"""_create_cdp_session() must sanitize the CDP URL before logging.
|
||||
|
|
|
|||
|
|
@ -93,16 +93,40 @@ def get_camofox_url() -> str:
|
|||
return os.getenv("CAMOFOX_URL", "").rstrip("/")
|
||||
|
||||
|
||||
def _config_cdp_url() -> str:
|
||||
"""Persistent ``browser.cdp_url`` from config.yaml, or empty string.
|
||||
|
||||
Read here (instead of importing ``browser_tool._get_cdp_override`` to avoid
|
||||
a circular import) so Camofox can yield to a config-based CDP override the
|
||||
same way it already yields to the ``BROWSER_CDP_URL`` env override.
|
||||
"""
|
||||
try:
|
||||
from hermes_cli.config import read_raw_config
|
||||
|
||||
browser_cfg = read_raw_config().get("browser", {})
|
||||
if isinstance(browser_cfg, dict):
|
||||
return str(browser_cfg.get("cdp_url", "") or "").strip()
|
||||
except Exception:
|
||||
pass
|
||||
return ""
|
||||
|
||||
|
||||
def is_camofox_mode() -> bool:
|
||||
"""True when Camofox backend is configured and no CDP override is active.
|
||||
|
||||
When the user has explicitly connected to a live Chromium-family browser via
|
||||
``/browser connect`` (which sets ``BROWSER_CDP_URL``), the CDP connection
|
||||
takes priority over Camofox so the browser tools operate on the real
|
||||
browser instead of being silently routed to the Camofox backend.
|
||||
A CDP override takes priority over Camofox so the browser tools operate on
|
||||
the real CDP browser (and a CDP backend is treated as non-local for SSRF
|
||||
checks) instead of being silently routed to Camofox. The override may come
|
||||
from the ``BROWSER_CDP_URL`` env var (set by ``/browser connect``) OR a
|
||||
persistent ``browser.cdp_url`` in config.yaml — both are honored, matching
|
||||
``browser_tool._get_cdp_override()``'s precedence. (Previously only the env
|
||||
var suppressed Camofox, so ``CAMOFOX_URL`` + a config CDP override still
|
||||
routed navigation through Camofox.)
|
||||
"""
|
||||
if os.getenv("BROWSER_CDP_URL", "").strip():
|
||||
return False
|
||||
if _config_cdp_url():
|
||||
return False
|
||||
return bool(get_camofox_url())
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -800,6 +800,20 @@ def _is_local_backend() -> bool:
|
|||
that the terminal cannot. In this case, SSRF protection should be
|
||||
enabled even though the browser is technically "local".
|
||||
"""
|
||||
# A CDP override points the browser at a separate Chrome process whose
|
||||
# network position is not guaranteed to match the terminal (it may live
|
||||
# off-host). Don't treat it as a trusted local backend — otherwise a
|
||||
# model-driven navigate could reach internal/metadata services reachable
|
||||
# from the CDP host but not the terminal. This MUST be checked before the
|
||||
# camofox short-circuit below so a Camofox backend combined with a CDP
|
||||
# override still fails the local check instead of returning local and
|
||||
# skipping the private/internal SSRF gate. The override is honored from
|
||||
# either the BROWSER_CDP_URL env var or a persistent `browser.cdp_url`
|
||||
# config (both via _get_cdp_override(), and both now suppress camofox in
|
||||
# browser_camofox.py). _is_local_mode() already treats any CDP override as
|
||||
# non-local; keep the two helpers in agreement.
|
||||
if _get_cdp_override():
|
||||
return False
|
||||
if _is_camofox_mode():
|
||||
return True
|
||||
if _get_cloud_provider() is not None:
|
||||
|
|
@ -2740,7 +2754,10 @@ def browser_navigate(url: str, task_id: Optional[str] = None) -> str:
|
|||
# 169.254.169.254 / metadata.google.internal / ECS task metadata
|
||||
# via a browser, and routing those to a local Chromium sidecar
|
||||
# on an EC2/GCP/Azure host exfiltrates IAM credentials (#16234).
|
||||
if not _is_local_backend() and _is_always_blocked_url(url):
|
||||
# The floor is UNCONDITIONAL — it must fire for every backend,
|
||||
# including the pure-local headless Chromium and off-host CDP cases
|
||||
# (a local Chromium on a cloud VM still reaches the host IMDS).
|
||||
if _is_always_blocked_url(url):
|
||||
return json.dumps({
|
||||
"success": False,
|
||||
"error": "Blocked: URL targets a cloud metadata endpoint",
|
||||
|
|
@ -2808,12 +2825,11 @@ def browser_navigate(url: str, task_id: Optional[str] = None) -> str:
|
|||
# Skipped for local backends (same rationale as the pre-nav check),
|
||||
# and for the hybrid local sidecar (we're already on a local browser
|
||||
# hitting a private URL by design).
|
||||
# Always-blocked floor (cloud metadata / IMDS) is enforced even
|
||||
# when auto_local_this_nav is true — see pre-nav check for
|
||||
# rationale (#16234).
|
||||
# Always-blocked floor (cloud metadata / IMDS) is enforced for every
|
||||
# backend and even when auto_local_this_nav is true — see pre-nav
|
||||
# check for rationale (#16234).
|
||||
if (
|
||||
not _is_local_backend()
|
||||
and final_url
|
||||
final_url
|
||||
and final_url != url
|
||||
and _is_always_blocked_url(final_url)
|
||||
):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue