mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-30 11:52:04 +00:00
fix(security): redact browser CDP endpoint logs
This commit is contained in:
parent
23c03ced75
commit
576424cc1c
2 changed files with 86 additions and 3 deletions
|
|
@ -46,6 +46,51 @@ class TestResolveCdpOverride:
|
|||
with patch("tools.browser_tool.requests.get", side_effect=RuntimeError("boom")):
|
||||
assert _resolve_cdp_override(HTTP_URL) == HTTP_URL
|
||||
|
||||
def test_redacts_secret_query_params_in_success_log(self):
|
||||
from tools.browser_tool import _resolve_cdp_override
|
||||
|
||||
raw = "https://cdp.example/json/version?access_token=super-secret-token-123456"
|
||||
resolved_ws = "wss://cdp.example/devtools/browser/abc?token=super-secret-token-123456"
|
||||
|
||||
response = Mock()
|
||||
response.raise_for_status.return_value = None
|
||||
response.json.return_value = {"webSocketDebuggerUrl": resolved_ws}
|
||||
|
||||
with patch("tools.browser_tool.requests.get", return_value=response), \
|
||||
patch("tools.browser_tool.logger.info") as mock_info:
|
||||
resolved = _resolve_cdp_override(raw)
|
||||
|
||||
assert resolved == resolved_ws
|
||||
mock_info.assert_called_once()
|
||||
_, logged_raw, logged_ws = mock_info.call_args.args
|
||||
assert "super-secret-token-123456" not in logged_raw
|
||||
assert "super-secret-token-123456" not in logged_ws
|
||||
assert "access_token=***" in logged_raw
|
||||
assert "token=***" in logged_ws
|
||||
|
||||
def test_redacts_secret_query_params_in_failure_log(self):
|
||||
from tools.browser_tool import _resolve_cdp_override
|
||||
|
||||
raw = "https://cdp.example?access_token=super-secret-token-123456"
|
||||
secret_error = RuntimeError(
|
||||
"upstream rejected https://cdp.example/json/version?access_token=super-secret-token-123456"
|
||||
)
|
||||
|
||||
with patch("tools.browser_tool.requests.get", side_effect=secret_error), \
|
||||
patch("tools.browser_tool.logger.warning") as mock_warning:
|
||||
resolved = _resolve_cdp_override(raw)
|
||||
|
||||
assert resolved == raw
|
||||
mock_warning.assert_called_once()
|
||||
_, logged_raw, logged_version_url, logged_error = mock_warning.call_args.args
|
||||
assert "super-secret-token-123456" not in logged_raw
|
||||
assert "super-secret-token-123456" not in logged_version_url
|
||||
assert "super-secret-token-123456" not in logged_error
|
||||
assert "access_token=***" in logged_raw
|
||||
assert "access_token=***" in logged_version_url
|
||||
assert "access_token=***" in logged_error
|
||||
assert logged_version_url.startswith("https://cdp.example")
|
||||
|
||||
def test_normalizes_provider_returned_http_cdp_url_when_creating_session(self, monkeypatch):
|
||||
import tools.browser_tool as browser_tool
|
||||
|
||||
|
|
@ -116,3 +161,4 @@ class TestGetCdpOverride:
|
|||
|
||||
assert resolved == WS_URL
|
||||
mock_get.assert_called_once_with(VERSION_URL, timeout=10)
|
||||
|
||||
|
|
|
|||
|
|
@ -65,6 +65,11 @@ import requests
|
|||
from typing import Dict, Any, Optional, List, Tuple, Union
|
||||
from pathlib import Path
|
||||
from agent.auxiliary_client import call_llm
|
||||
from agent.redact import (
|
||||
redact_sensitive_text,
|
||||
_redact_url_query_params,
|
||||
_redact_url_userinfo,
|
||||
)
|
||||
from hermes_constants import agent_browser_runnable, get_hermes_home
|
||||
from utils import env_int, is_truthy_value
|
||||
from hermes_cli.config import DEFAULT_CONFIG, cfg_get
|
||||
|
|
@ -236,6 +241,26 @@ _cached_command_timeout: Optional[int] = None
|
|||
_command_timeout_resolved = False
|
||||
|
||||
|
||||
def _sanitize_url_for_logs(value: object) -> str:
|
||||
"""Mask secrets in logged browser endpoint URLs and URL-like errors.
|
||||
|
||||
The global ``redact_sensitive_text`` deliberately passes web-URL query
|
||||
params and ``user:pass@`` userinfo through unmasked (OAuth callbacks,
|
||||
magic-link / pre-signed URLs the agent is meant to follow — see the
|
||||
web-URL note in ``agent/redact.py``). CDP discovery endpoints are NOT
|
||||
such a workflow: their query-string tokens and userinfo passwords are
|
||||
pure credentials that must never reach the logs. So at these log sites
|
||||
we opt INTO the URL redactors that the global pass leaves off, reusing
|
||||
the shared ``redact.py`` helpers rather than a second regex.
|
||||
"""
|
||||
text = redact_sensitive_text(value)
|
||||
if not text:
|
||||
return text
|
||||
text = _redact_url_query_params(text)
|
||||
text = _redact_url_userinfo(text)
|
||||
return text
|
||||
|
||||
|
||||
def _get_command_timeout() -> int:
|
||||
"""Return the configured browser command timeout from config.yaml.
|
||||
|
||||
|
|
@ -409,15 +434,27 @@ def _resolve_cdp_override(cdp_url: str) -> str:
|
|||
response.raise_for_status()
|
||||
payload = response.json()
|
||||
except Exception as exc:
|
||||
logger.warning("Failed to resolve CDP endpoint %s via %s: %s", raw, version_url, exc)
|
||||
logger.warning(
|
||||
"Failed to resolve CDP endpoint %s via %s: %s",
|
||||
_sanitize_url_for_logs(raw),
|
||||
_sanitize_url_for_logs(version_url),
|
||||
_sanitize_url_for_logs(exc),
|
||||
)
|
||||
return raw
|
||||
|
||||
ws_url = str(payload.get("webSocketDebuggerUrl") or "").strip()
|
||||
if ws_url:
|
||||
logger.info("Resolved CDP endpoint %s -> %s", raw, ws_url)
|
||||
logger.info(
|
||||
"Resolved CDP endpoint %s -> %s",
|
||||
_sanitize_url_for_logs(raw),
|
||||
_sanitize_url_for_logs(ws_url),
|
||||
)
|
||||
return ws_url
|
||||
|
||||
logger.warning("CDP discovery at %s did not return webSocketDebuggerUrl; using raw endpoint", version_url)
|
||||
logger.warning(
|
||||
"CDP discovery at %s did not return webSocketDebuggerUrl; using raw endpoint",
|
||||
_sanitize_url_for_logs(version_url),
|
||||
)
|
||||
return raw
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue