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.
This commit is contained in:
@@ -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
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user