fix(browser): narrow credential-query denylist to unambiguous names
Follow-up on the salvaged #49830 hardening. The contributor's sensitive query-param set included bare English words (code, key, auth, session, sig) that double as ordinary page facets — ?code= on promo/challenge pages, ?key= as a search facet, ?session= on blogs — so web_extract and cloud browser_navigate would refuse a large slice of normal browsing. Narrow the set to unambiguously credential-named params (access_token, authorization, client_secret, password, token, x-amz-signature, ...). Prefix-based vendor-key redaction (is_safe_url) still catches recognizable key shapes; this set is the belt-and-suspenders for opaque secrets carried under an explicit credential-named parameter. Also fixes two intra-PR-staleness test breakages surfaced by salvaging onto current main: - web_extract_tool() no longer accepts use_llm_processing= (signature changed since the PR was authored) — dropped the invalid kwarg. - agent.redact now fully masks keyed 'token=<secret>' to 'token=***' instead of partial 'sk-...'; the console-redaction test now asserts the real invariant (secret body gone) rather than the exact mask format. Added a regression test that generic English-word query params are NOT blocked by the credential guard.
This commit is contained in:
parent
937e56be92
commit
cfbc7ed1f9
4 changed files with 39 additions and 10 deletions
|
|
@ -112,9 +112,13 @@ class TestBrowserConsole:
|
|||
result = json.loads(browser_console(task_id="test"))
|
||||
|
||||
serialized = json.dumps(result)
|
||||
# The secret body must be gone. The exact mask format
|
||||
# (partial ``sk-…7890`` vs full ``***`` for keyed ``token=`` values)
|
||||
# is owned by agent.redact and intentionally not pinned here.
|
||||
assert "BROWSERCONSOLESECRET" not in serialized
|
||||
assert "sk-" in result["console_messages"][0]["text"]
|
||||
assert "..." in result["console_messages"][0]["text"]
|
||||
redacted_text = result["console_messages"][0]["text"]
|
||||
assert fake_key not in redacted_text
|
||||
assert "***" in redacted_text or "..." in redacted_text
|
||||
|
||||
def test_redacts_secrets_from_eval_result(self):
|
||||
from tools.browser_tool import _browser_eval
|
||||
|
|
|
|||
|
|
@ -108,14 +108,36 @@ class TestWebExtractSecretExfil:
|
|||
from tools.web_tools import web_extract_tool
|
||||
|
||||
result = await web_extract_tool(
|
||||
urls=["https://example.com/callback?code=opaque-oauth-code"],
|
||||
use_llm_processing=False,
|
||||
urls=["https://example.com/callback?access_token=opaque-oauth-value"],
|
||||
)
|
||||
|
||||
parsed = json.loads(result)
|
||||
assert parsed["success"] is False
|
||||
assert "credential-like query parameter" in parsed["error"]
|
||||
assert "code" in parsed["error"]
|
||||
assert "access_token" in parsed["error"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_allows_ambiguous_english_word_query_param(self):
|
||||
"""Generic query names that double as normal page facets must NOT block.
|
||||
|
||||
``?code=`` (promo/challenge pages), ``?key=`` (search facets),
|
||||
``?session=`` etc. are ordinary browsing params. Only unambiguously
|
||||
credential-named params are blocked, so web_extract stays usable.
|
||||
"""
|
||||
from tools.web_tools import web_extract_tool
|
||||
|
||||
for url in (
|
||||
"https://leetcode.com/problems/two-sum/?code=twosum",
|
||||
"https://github.com/search?q=hermes&code=1",
|
||||
"https://example.com/blog?session=summer",
|
||||
):
|
||||
result = await web_extract_tool(urls=[url])
|
||||
parsed = json.loads(result)
|
||||
# Not blocked by the credential-query guard (may fail for other
|
||||
# reasons like a missing backend, but never with this specific
|
||||
# error string).
|
||||
if parsed.get("success") is False:
|
||||
assert "credential-like query parameter" not in parsed.get("error", ""), url
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_allows_normal_url(self):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue