diff --git a/agent/redact.py b/agent/redact.py index 307e5dc3a..dc7c1957e 100644 --- a/agent/redact.py +++ b/agent/redact.py @@ -400,6 +400,30 @@ def _redact_url_userinfo(text: str) -> str: ) +def redact_cdp_url(value: object) -> str: + """Mask secrets in a CDP/browser endpoint URL before it is logged. + + 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 above). 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 for CDP URLs we opt INTO the two URL + redactors that the global pass leaves off. + + This is the single source of truth for CDP-URL log redaction. Every site + that emits a resolved CDP URL to a log or exception message -- the browser + tool's session/discovery logs and the supervisor's attach-timeout error -- + routes through here so the policy can never drift between call sites. + """ + text = redact_sensitive_text("" if value is None else str(value)) + if not text: + return text + text = _redact_url_query_params(text) + text = _redact_url_userinfo(text) + return text + + def _redact_http_request_target_query_params(text: str) -> str: """Redact sensitive query params in HTTP access-log request targets.""" def _sub(m: re.Match) -> str: diff --git a/tests/agent/test_redact.py b/tests/agent/test_redact.py index 4174cd58a..75fd3b6f7 100644 --- a/tests/agent/test_redact.py +++ b/tests/agent/test_redact.py @@ -4,7 +4,7 @@ import logging import pytest -from agent.redact import redact_sensitive_text, RedactingFormatter +from agent.redact import redact_cdp_url, redact_sensitive_text, RedactingFormatter @pytest.fixture(autouse=True) @@ -908,3 +908,45 @@ class TestFireworksToken: def test_prefix_visible_in_masked_output(self): result = redact_sensitive_text(self.KEY, force=True) assert result.startswith("fw_AA") + + +class TestRedactCdpUrl: + """redact_cdp_url() is the single chokepoint for CDP endpoint log redaction. + + Unlike the global pass (which deliberately lets web-URL query params and + userinfo through for OAuth/magic-link workflows), CDP endpoint credentials + are pure secrets and must always be masked. Both the browser tool's + session/discovery logs and the supervisor's attach-timeout error route + through this helper. + """ + + def test_masks_query_string_token(self): + url = "wss://cdp.example/devtools/browser/abc?token=super-secret-999" + out = redact_cdp_url(url) + assert "super-secret-999" not in out + assert "token=***" in out + + def test_masks_multiple_query_credentials(self): + url = "wss://provider.example/session?token=aaa-secret&apikey=bbb-secret" + out = redact_cdp_url(url) + assert "aaa-secret" not in out + assert "bbb-secret" not in out + + def test_masks_userinfo_password(self): + url = "wss://user:p4ssw0rd@cdp.example/devtools/browser/x" + out = redact_cdp_url(url) + assert "p4ssw0rd" not in out + assert "user:***@" in out + + def test_plain_url_passes_through(self): + url = "ws://localhost:9222/devtools/browser/abc123" + assert redact_cdp_url(url) == url + + def test_non_string_input_coerced(self): + # Exceptions and other objects are stringified, not crashed on. + exc = RuntimeError("connect failed: wss://h/x?token=leak-me") + out = redact_cdp_url(exc) + assert "leak-me" not in out + + def test_none_returns_empty(self): + assert redact_cdp_url(None) == "" diff --git a/tools/browser_supervisor.py b/tools/browser_supervisor.py index db523cae5..746e79cdb 100644 --- a/tools/browser_supervisor.py +++ b/tools/browser_supervisor.py @@ -342,12 +342,8 @@ class CDPSupervisor: if not self._ready_event.wait(timeout=timeout): self.stop() try: - from agent.redact import ( - _redact_url_query_params, - _redact_url_userinfo, - redact_sensitive_text, - ) - _safe_url = _redact_url_userinfo(_redact_url_query_params(redact_sensitive_text(self.cdp_url))) + from agent.redact import redact_cdp_url + _safe_url = redact_cdp_url(self.cdp_url) except Exception: _safe_url = "" raise TimeoutError( diff --git a/tools/browser_tool.py b/tools/browser_tool.py index 7d879e3c0..5ef1487aa 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -65,11 +65,7 @@ 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 agent.redact import redact_cdp_url 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 @@ -244,21 +240,13 @@ _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. + Thin wrapper over :func:`agent.redact.redact_cdp_url`, which is the single + source of truth for CDP-URL log redaction. Kept as a local name because + several browser-tool log sites reference it; the redaction policy itself + lives once in ``redact.py`` so the browser tool and the CDP supervisor + cannot drift apart. """ - text = redact_sensitive_text(value) - if not text: - return text - text = _redact_url_query_params(text) - text = _redact_url_userinfo(text) - return text + return redact_cdp_url(value) def _get_command_timeout() -> int: