From a56885495eeee3789dc8d3b23500e2f2681bd06e Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Tue, 25 Aug 2026 03:18:44 +0800 Subject: [PATCH] fix(browser): keep CDP binary payloads byte-identical through redaction (#94138) _redact_cdp_output applied redact_sensitive_text(force=True) to every string in CDP results, including the base64 screenshot/PDF payload of Page.captureScreenshot and Page.printToPDF. The Fernet pattern (gAAAA + base64 alphabet) matches arbitrary spans inside such payloads wherever gAAAA follows a + or /, collapsing them to first6...last4: decoded PNGs came out corrupt (valid header, CRC failures mid-IDAT, no IEND), the persisted full copies in tool_result_storage were redacted too, and vision_analyze then embedded corrupt images that the provider rejected with 400 invalid_image - killing resume sessions with a misleading provider error. Skip redaction for the two binary-payload methods: the payload is binary, not free text, so there is no secret to protect there. Every other method keeps full redaction. --- tests/tools/test_browser_cdp_tool.py | 53 ++++++++++++++++++++++++++++ tools/browser_cdp_tool.py | 33 +++++++++++++---- 2 files changed, 80 insertions(+), 6 deletions(-) diff --git a/tests/tools/test_browser_cdp_tool.py b/tests/tools/test_browser_cdp_tool.py index 521c624aa5..493abc2a81 100644 --- a/tests/tools/test_browser_cdp_tool.py +++ b/tests/tools/test_browser_cdp_tool.py @@ -201,6 +201,59 @@ def test_browser_level_redacts_secret_result(cdp_server): assert result["result"]["result"]["value"].startswith("sk-") +def test_screenshot_base64_passes_through_unredacted(cdp_server): + """The Fernet pattern matches arbitrary spans inside base64 payloads — + a screenshot whose base64 contains "gAAAA..." must stay byte-identical + instead of being collapsed to "first6...last4" (#94138).""" + # Real-world shape: the Fernet pattern fires when "gAAAA" follows a "+" + # or "/" inside the base64 stream (word-boundary requirement). + shot_b64 = "iVBORw0KGgoAAAANSUhEUg+" + "gAAAA" + "B" * 60 + "==" + cdp_server.on( + "Page.captureScreenshot", + lambda params, sid: {"data": shot_b64}, + ) + + result = json.loads(browser_cdp_tool.browser_cdp(method="Page.captureScreenshot")) + + assert result["success"] is True + assert result["result"]["data"] == shot_b64 + + +def test_print_to_pdf_base64_passes_through_unredacted(cdp_server): + pdf_b64 = "JVBERi0xLjcK/" + "gAAAA" + "C" * 60 + "=" + cdp_server.on( + "Page.printToPDF", + lambda params, sid: {"data": pdf_b64}, + ) + + result = json.loads(browser_cdp_tool.browser_cdp(method="Page.printToPDF")) + + assert result["success"] is True + assert result["result"]["data"] == pdf_b64 + + +def test_binary_payload_flag_keeps_secret_redaction_off_method_list(cdp_server): + """Fail-closed pin: only the two binary-payload methods skip redaction; + any other method's text output keeps full secret redaction.""" + fake_key = "sk-" + "CDPSECRETSTILLREDACTED1234567890" + cdp_server.on( + "Runtime.evaluate", + lambda params, sid: {"result": {"type": "string", "value": fake_key}}, + ) + cdp_server.on( + "Page.captureScreenshot", + lambda params, sid: {"data": "gAAAA" + "B" * 60}, + ) + + text_result = json.loads(browser_cdp_tool.browser_cdp(method="Runtime.evaluate")) + assert "CDPSECRETSTILLREDACTED" not in json.dumps(text_result) + + shot_result = json.loads( + browser_cdp_tool.browser_cdp(method="Page.captureScreenshot") + ) + assert shot_result["result"]["data"] == "gAAAA" + "B" * 60 + + # --------------------------------------------------------------------------- # Happy-path: target-attached call # --------------------------------------------------------------------------- diff --git a/tools/browser_cdp_tool.py b/tools/browser_cdp_tool.py index 8c23f8a6d7..fa961da0fe 100644 --- a/tools/browser_cdp_tool.py +++ b/tools/browser_cdp_tool.py @@ -43,18 +43,37 @@ _CDP_PRIVATE_PAGE_ALLOWED_METHODS = { } -def _redact_cdp_output(value: Any) -> Any: - """Redact browser-originated CDP result data before returning it.""" +_BINARY_PAYLOAD_CDP_METHODS = { + # Methods whose result is dominated by a base64-encoded binary payload. + # redact_sensitive_text's Fernet pattern ("gAAAA" + base64 alphabet) can + # match arbitrary spans inside such payloads — collapsing them to + # "first6...last4" and corrupting the decoded screenshot/PDF (#94138). + # The payload is binary, not free text the model reads, so redaction has + # no secret to protect there; every other method keeps full redaction. + "Page.captureScreenshot", + "Page.printToPDF", +} + + +def _redact_cdp_output(value: Any, *, binary_payload: bool = False) -> Any: + """Redact browser-originated CDP result data before returning it. + + When *binary_payload* is set (the calling method's result is a base64 + binary payload, see ``_BINARY_PAYLOAD_CDP_METHODS``), strings pass + through unchanged so the payload stays byte-identical (#94138). + """ from agent.redact import redact_sensitive_text if isinstance(value, str): + if binary_payload: + return value return redact_sensitive_text(value, force=True) if isinstance(value, list): - return [_redact_cdp_output(item) for item in value] + return [_redact_cdp_output(item, binary_payload=binary_payload) for item in value] if isinstance(value, tuple): - return tuple(_redact_cdp_output(item) for item in value) + return tuple(_redact_cdp_output(item, binary_payload=binary_payload) for item in value) if isinstance(value, dict): - return {key: _redact_cdp_output(item) for key, item in value.items()} + return {key: _redact_cdp_output(item, binary_payload=binary_payload) for key, item in value.items()} return value # ``websockets`` is a direct hermes-agent dependency because the browser CDP @@ -525,7 +544,9 @@ def browser_cdp( payload: Dict[str, Any] = { "success": True, "method": method, - "result": _redact_cdp_output(result), + "result": _redact_cdp_output( + result, binary_payload=method in _BINARY_PAYLOAD_CDP_METHODS + ), } if target_id: payload["target_id"] = target_id