feat: generic remote CDP browser type(SKY-10648) (#6373)

skyvern/webeye/cdp_retry.py

@@ -41,20 +41,28 @@ async def connect_over_cdp_with_retry(
     playwright: Playwright,
     browser_address: str,
     headers: dict[str, str] | None = None,
+    log_browser_address: str | None = None,
 ) -> Browser:
     browser_address = strip_browser_address_discriminator(browser_address)
+    browser_address_for_logs = log_browser_address or browser_address
     for attempt in range(1, _CDP_RETRY_ATTEMPTS + 1):
         try:
             browser = await playwright.chromium.connect_over_cdp(browser_address, headers=headers)
             if attempt > 1:
                 LOG.info(
                     "CDP connection recovered after retry",
-                    browser_address=browser_address,
+                    browser_address=browser_address_for_logs,
                     successful_attempt=attempt,
                 )
             return browser
         except Exception as e:
             if not is_cdp_connection_error(e) or attempt == _CDP_RETRY_ATTEMPTS:
+                # When the caller passed log_browser_address as a safe label, the raw
+                # browser_address may carry session tokens in path/query — Playwright's
+                # exception text would otherwise expose them. Re-raise a RuntimeError
+                # with only the safe label + error class name.
+                if log_browser_address is not None:
+                    raise RuntimeError(f"CDP connection to {log_browser_address} failed ({type(e).__name__})") from None
                 raise
             backoff = (
                 _CDP_RETRY_BACKOFF_SECONDS[attempt - 1]
@@ -63,7 +71,7 @@ async def connect_over_cdp_with_retry(
             )
             LOG.warning(
                 "CDP connection failed, retrying",
-                browser_address=browser_address,
+                browser_address=browser_address_for_logs,
                 attempt=attempt,
                 max_attempts=_CDP_RETRY_ATTEMPTS,
                 backoff_seconds=backoff,

tests/unit/test_cdp_connection_retry.py

@@ -37,6 +37,33 @@ class TestRetryBehavior:
         assert pw.chromium.connect_over_cdp.call_count == 2
         mock_sleep.assert_called_once_with(1)
 
+    @pytest.mark.asyncio
+    async def test_retry_logs_use_redacted_browser_address_when_provided(self):
+        secret_address = "wss://cdp.vendor.test/devtools/browser/SECRET?token=ABC"
+        pw = _make_playwright(
+            [
+                PWError("BrowserType.connect_over_cdp: connect ECONNREFUSED cdp.vendor.test"),
+                "browser",
+            ]
+        )
+        with (
+            patch("skyvern.webeye.cdp_retry._sleep", new_callable=AsyncMock),
+            patch("skyvern.webeye.cdp_retry.LOG") as mock_log,
+        ):
+            result = await connect_over_cdp_with_retry(
+                pw,
+                secret_address,
+                log_browser_address="remote-cdp-vendor:cdp.vendor.test",
+            )
+        assert result == "browser"
+        assert pw.chromium.connect_over_cdp.call_args.args[0] == secret_address
+        warning_values = [str(v) for call in mock_log.warning.call_args_list for v in call.kwargs.values()]
+        info_values = [str(v) for call in mock_log.info.call_args_list for v in call.kwargs.values()]
+        logged = " ".join(warning_values + info_values)
+        assert "remote-cdp-vendor:cdp.vendor.test" in logged
+        assert "SECRET" not in logged
+        assert "token=ABC" not in logged
+
     @pytest.mark.asyncio
     async def test_succeeds_after_two_transient_failures(self):
         pw = _make_playwright(
@@ -68,6 +95,50 @@ class TestRetryBehavior:
             await connect_over_cdp_with_retry(pw, "http://10.0.0.1:9224")
         assert pw.chromium.connect_over_cdp.call_count == 1
 
+    @pytest.mark.asyncio
+    async def test_final_attempt_raised_exception_does_not_leak_url_when_label_provided(self):
+        """When log_browser_address is set, the final-attempt exception is a plain
+        RuntimeError whose message contains only the safe label and the original
+        error class name — never the raw browser_address."""
+        secret_address = "wss://cdp.vendor.test/devtools/browser/SECRET?token=ABC"
+        underlying_msg = (
+            "BrowserType.connect_over_cdp: connect ECONNREFUSED wss://cdp.vendor.test/devtools/browser/SECRET?token=ABC"
+        )
+        error = PWError(underlying_msg)
+        pw = _make_playwright([error, error, error])
+
+        with patch("skyvern.webeye.cdp_retry._sleep", new_callable=AsyncMock):
+            with pytest.raises(RuntimeError) as excinfo:
+                await connect_over_cdp_with_retry(
+                    pw,
+                    secret_address,
+                    log_browser_address="remote-cdp-vendor:cdp.vendor.test",
+                )
+
+        message = str(excinfo.value)
+        assert "SECRET" not in message
+        assert "token=ABC" not in message
+        assert "/devtools/browser/" not in message
+        assert "remote-cdp-vendor:cdp.vendor.test" in message
+        # Original error class name preserved for log debuggability.
+        assert "PWError" in message or "Error" in message
+        # Chain suppressed so __cause__ cannot leak the URL either.
+        assert excinfo.value.__cause__ is None
+
+    @pytest.mark.asyncio
+    async def test_final_attempt_unchanged_when_no_label(self):
+        """When log_browser_address is not provided, behavior is unchanged — the original
+        Playwright exception (with its full URL) bubbles up as today."""
+        underlying_msg = "BrowserType.connect_over_cdp: connect ECONNREFUSED 10.0.0.1:9224"
+        error = PWError(underlying_msg)
+        pw = _make_playwright([error, error, error])
+
+        with patch("skyvern.webeye.cdp_retry._sleep", new_callable=AsyncMock):
+            with pytest.raises(PWError) as excinfo:
+                await connect_over_cdp_with_retry(pw, "http://10.0.0.1:9224")
+
+        assert str(excinfo.value) == underlying_msg
+
     @pytest.mark.asyncio
     async def test_backoff_is_bounded(self):
         error = PWError("connect ECONNRESET")