fix(browser): bind CDP binary exemptions to exact method result paths
Second review round: honoring base64Encoded recursively let any nested
dict spoof {"base64Encoded": true, "data": "<secret>"} past the redactor
(Runtime.evaluate returns arbitrary by-value JSON), and two carriers were
missed entirely — Network.streamResourceContent returns unflagged binary
bufferedData, and Network.getRequestPostData's postData was not covered.
Replace the ambient field sets with per-method exact result-path specs:
_CDP_ALWAYS_BINARY_PATHS for declared-binary paths (screenshots, PDFs,
streamResourceContent, beginFrame screenshotData, the nested
CacheStorage.requestCachedResponse.response.body) and
_CDP_FLAGGED_BINARY_PATHS for paths whose carrier object's base64Encoded
sibling gates the exemption (Network/Fetch.getResponseBody body, IO.read
data, getRequestPostData postData). Path suffixes propagate only into the
matching subtree, so base64Encoded is type information solely on trusted
carrier objects — never ambient trust in nested JSON.
This commit is contained in:
liuhao1024
Teknium
@@ -338,6 +338,106 @@ def test_fetch_get_response_body_base64_discriminator_passes_through(cdp_server)
|
||||
assert result["result"]["body"] == body_b64
|
||||
|
||||
|
||||
def test_runtime_evaluate_spoofed_base64_flag_still_redacts(cdp_server):
|
||||
"""base64Encoded is trusted ONLY on the protocol-defined carrier paths.
|
||||
A Runtime.evaluate by-value object carrying
|
||||
{"base64Encoded": true, "data": "<secret>"} is untrusted nested JSON —
|
||||
the secret must still be redacted (second review on #94142)."""
|
||||
fake_key = "sk-" + "CDPSPOOFEDFLAG1234567890"
|
||||
cdp_server.on(
|
||||
"Runtime.evaluate",
|
||||
lambda params, sid: {
|
||||
"result": {
|
||||
"type": "object",
|
||||
"value": {"base64Encoded": True, "data": fake_key},
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
result = json.loads(browser_cdp_tool.browser_cdp(method="Runtime.evaluate"))
|
||||
|
||||
assert result["success"] is True
|
||||
assert "CDPSPOOFEDFLAG" not in json.dumps(result)
|
||||
|
||||
|
||||
def test_stream_resource_content_unflagged_buffered_data_passes_through(cdp_server):
|
||||
"""Network.streamResourceContent returns bare binary bufferedData with no
|
||||
base64Encoded sibling — declared-binary path, must stay byte-identical."""
|
||||
chunk_b64 = "Q2FjaGU/" + "gAAAA" + "E" * 60 + "=="
|
||||
cdp_server.on(
|
||||
"Network.streamResourceContent",
|
||||
lambda params, sid: {"bufferedData": chunk_b64},
|
||||
)
|
||||
|
||||
result = json.loads(
|
||||
browser_cdp_tool.browser_cdp(method="Network.streamResourceContent")
|
||||
)
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["result"]["bufferedData"] == chunk_b64
|
||||
|
||||
|
||||
def test_get_request_post_data_flagged_passes_through(cdp_server):
|
||||
"""Network.getRequestPostData's postData honors its base64Encoded
|
||||
discriminator on the trusted result path."""
|
||||
post_b64 = "cG9zdA==" + "gAAAA" + "F" * 60 + "="
|
||||
cdp_server.on(
|
||||
"Network.getRequestPostData",
|
||||
lambda params, sid: {"postData": post_b64, "base64Encoded": True},
|
||||
)
|
||||
|
||||
result = json.loads(
|
||||
browser_cdp_tool.browser_cdp(method="Network.getRequestPostData")
|
||||
)
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["result"]["postData"] == post_b64
|
||||
|
||||
|
||||
def test_get_request_post_data_unflagged_still_redacts(cdp_server):
|
||||
fake_key = "sk-" + "CDPPOSTDATASECRET1234567890"
|
||||
cdp_server.on(
|
||||
"Network.getRequestPostData",
|
||||
lambda params, sid: {
|
||||
"postData": f"leak {fake_key} here",
|
||||
"base64Encoded": False,
|
||||
},
|
||||
)
|
||||
|
||||
result = json.loads(
|
||||
browser_cdp_tool.browser_cdp(method="Network.getRequestPostData")
|
||||
)
|
||||
|
||||
assert result["success"] is True
|
||||
assert "CDPPOSTDATASECRET" not in json.dumps(result)
|
||||
|
||||
|
||||
def test_nested_unflagged_binary_path_passes_through(cdp_server):
|
||||
"""CacheStorage.requestCachedResponse.response.body is a nested binary
|
||||
carrier — the path must exempt the nested field while unrelated nested
|
||||
text keeps redaction."""
|
||||
fake_key = "sk-" + "CDPNESTEDSECRET1234567890"
|
||||
body_b64 = "SUNBRQ/" + "gAAAA" + "G" * 60 + "=="
|
||||
cdp_server.on(
|
||||
"CacheStorage.requestCachedResponse",
|
||||
lambda params, sid: {
|
||||
"response": {
|
||||
"url": "https://example.test/x",
|
||||
"body": body_b64,
|
||||
"note": fake_key,
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
result = json.loads(
|
||||
browser_cdp_tool.browser_cdp(method="CacheStorage.requestCachedResponse")
|
||||
)
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["result"]["response"]["body"] == body_b64
|
||||
assert "CDPNESTEDSECRET" not in json.dumps(result)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Happy-path: target-attached call
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -43,38 +43,52 @@ _CDP_PRIVATE_PAGE_ALLOWED_METHODS = {
|
||||
}
|
||||
|
||||
|
||||
_CDP_BINARY_RESULT_FIELDS: Dict[str, frozenset] = {
|
||||
# Protocol carriers whose result.<field> is an opaque base64 payload with
|
||||
# no discriminator flag of their own. 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 bytes (#94138). The payload is binary, not free text the model
|
||||
# reads, so redaction has no secret to protect there. The generic
|
||||
# body/read methods (Network.getResponseBody, Fetch.getResponseBody,
|
||||
# IO.read, Network.streamResourceContent) are NOT here: they carry a
|
||||
# base64Encoded sibling and go through the discriminator below.
|
||||
"Page.captureScreenshot": frozenset({"data"}),
|
||||
"Page.printToPDF": frozenset({"data"}),
|
||||
_CDP_ALWAYS_BINARY_PATHS: Dict[str, tuple] = {
|
||||
# method → result paths that are ALWAYS opaque base64 payloads (the
|
||||
# protocol declares them binary with no flag of their own).
|
||||
# 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 bytes (#94138). The payload
|
||||
# is binary, not free text the model reads, so redaction has no secret to
|
||||
# protect there.
|
||||
"Page.captureScreenshot": (("data",),),
|
||||
"Page.printToPDF": (("data",),),
|
||||
"Network.streamResourceContent": (("bufferedData",),),
|
||||
"HeadlessExperimental.beginFrame": (("screenshotData",),),
|
||||
"CacheStorage.requestCachedResponse": (("response", "body"),),
|
||||
}
|
||||
|
||||
# Generic body/read methods signal "this string is opaque base64 bytes" via a
|
||||
# sibling boolean. Honor the protocol discriminator anywhere it appears: the
|
||||
# flag is type information, so it is safe to trust even for methods not
|
||||
# listed above (#94138 review on #94142).
|
||||
_BASE64_DISCRIMINATED_FIELDS = frozenset({"body", "data", "bufferedData"})
|
||||
_CDP_FLAGGED_BINARY_PATHS: Dict[str, tuple] = {
|
||||
# method → result paths that are opaque base64 ONLY when the dict that
|
||||
# carries the final field has a ``base64Encoded`` sibling that is exactly
|
||||
# ``True``. The discriminator is type information only at these
|
||||
# protocol-defined paths; ``base64Encoded: false`` or absent means text,
|
||||
# which is redacted.
|
||||
"Network.getResponseBody": (("body",),),
|
||||
"Fetch.getResponseBody": (("body",),),
|
||||
"IO.read": (("data",),),
|
||||
"Network.getRequestPostData": (("postData",),),
|
||||
}
|
||||
|
||||
|
||||
def _redact_cdp_output(value: Any, *, exempt_fields: frozenset = frozenset()) -> Any:
|
||||
def _redact_cdp_output(
|
||||
value: Any,
|
||||
*,
|
||||
always_paths: tuple = (),
|
||||
flagged_paths: tuple = (),
|
||||
) -> Any:
|
||||
"""Redact browser-originated CDP result data before returning it.
|
||||
|
||||
Policy: semantic text is redacted; opaque bytes stay byte-identical
|
||||
(#94138). *exempt_fields* names the calling method's top-level binary
|
||||
payload fields (``_CDP_BINARY_RESULT_FIELDS``), so only those exact
|
||||
``method.result.<field>`` slots skip redaction — sibling fields keep
|
||||
full redaction. Any dict whose ``base64Encoded`` sibling is exactly
|
||||
``True`` exempts its ``body``/``data``/``bufferedData`` string the same
|
||||
way; ``base64Encoded: false`` or absent means the value is text and is
|
||||
redacted.
|
||||
(#94138). Exemptions come ONLY from the calling method's spec
|
||||
(``_CDP_ALWAYS_BINARY_PATHS`` / ``_CDP_FLAGGED_BINARY_PATHS``) as exact
|
||||
result paths — every other string in every result keeps full
|
||||
``redact_sensitive_text(force=True)``. Path suffixes are propagated only
|
||||
into the matching subtree, so ``base64Encoded`` is honored solely as a
|
||||
sibling on the trusted carrier object, never as ambient trust in
|
||||
arbitrary nested JSON (a ``Runtime.evaluate`` by-value object could
|
||||
otherwise spoof ``{"base64Encoded": true, "data": "<secret>"}`` past the
|
||||
redactor — second review on #94142).
|
||||
"""
|
||||
from agent.redact import redact_sensitive_text
|
||||
|
||||
@@ -88,16 +102,26 @@ def _redact_cdp_output(value: Any, *, exempt_fields: frozenset = frozenset()) ->
|
||||
base64_flagged = value.get("base64Encoded") is True
|
||||
redacted: Dict[str, Any] = {}
|
||||
for key, item in value.items():
|
||||
if (
|
||||
isinstance(item, str)
|
||||
and (
|
||||
key in exempt_fields
|
||||
or (base64_flagged and key in _BASE64_DISCRIMINATED_FIELDS)
|
||||
)
|
||||
terminal_always = any(
|
||||
len(p) == 1 and p[0] == key for p in always_paths
|
||||
)
|
||||
terminal_flagged = any(
|
||||
len(p) == 1 and p[0] == key for p in flagged_paths
|
||||
)
|
||||
if isinstance(item, str) and (
|
||||
terminal_always or (terminal_flagged and base64_flagged)
|
||||
):
|
||||
redacted[key] = item
|
||||
else:
|
||||
redacted[key] = _redact_cdp_output(item)
|
||||
redacted[key] = _redact_cdp_output(
|
||||
item,
|
||||
always_paths=tuple(
|
||||
p[1:] for p in always_paths if len(p) > 1 and p[0] == key
|
||||
),
|
||||
flagged_paths=tuple(
|
||||
p[1:] for p in flagged_paths if len(p) > 1 and p[0] == key
|
||||
),
|
||||
)
|
||||
return redacted
|
||||
return value
|
||||
|
||||
@@ -571,7 +595,8 @@ def browser_cdp(
|
||||
"method": method,
|
||||
"result": _redact_cdp_output(
|
||||
result,
|
||||
exempt_fields=_CDP_BINARY_RESULT_FIELDS.get(method, frozenset())
|
||||
always_paths=_CDP_ALWAYS_BINARY_PATHS.get(method, ()),
|
||||
flagged_paths=_CDP_FLAGGED_BINARY_PATHS.get(method, ()),
|
||||
),
|
||||
}
|
||||
if target_id:
|
||||
|
||||
Reference in New Issue
Block a user