diff --git a/tests/tools/test_browser_cdp_override.py b/tests/tools/test_browser_cdp_override.py index 73f0f574f7f..25efccc348d 100644 --- a/tests/tools/test_browser_cdp_override.py +++ b/tests/tools/test_browser_cdp_override.py @@ -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) + diff --git a/tools/browser_tool.py b/tools/browser_tool.py index 0c255bf6c81..ec587cc1697 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -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