diff --git a/agent/display.py b/agent/display.py index 5a16a77d00de..91819aab5183 100644 --- a/agent/display.py +++ b/agent/display.py @@ -340,17 +340,88 @@ def _read_file_line_label(args: dict) -> str: return f"L{offset}-{offset + limit - 1}" -def redact_browser_typed_text_for_display(value: Any, typed_text: Any) -> Any: - """Apply secret redaction to browser_type text in display-facing payloads. +_DISPLAY_SECRET_PLACEHOLDER = "[redacted]" +_DISPLAY_TYPED_TEXT_PLACEHOLDER = "[typed text hidden]" + +_SENSITIVE_ARG_KEYS = frozenset({ + "accesstoken", + "apikey", + "authorization", + "authtoken", + "clientsecret", + "cookie", + "credentials", + "newpassword", + "passphrase", + "password", + "passwd", + "privatekey", + "pwd", + "refreshtoken", + "secret", + "sessiontoken", + "token", +}) + +_SENSITIVE_ARG_KEY_PARTS = ( + "apikey", + "authsecret", + "authtoken", + "clientsecret", + "password", + "passphrase", + "privatekey", + "refreshtoken", + "secrettoken", + "sessiontoken", +) + + +def _normalise_arg_key(key: Any) -> str: + return re.sub(r"[^a-z0-9]", "", str(key).lower()) + + +def _is_sensitive_arg_key(key: Any) -> bool: + normalised = _normalise_arg_key(key) + if not normalised: + return False + if normalised in _SENSITIVE_ARG_KEYS: + return True + return any(part in normalised for part in _SENSITIVE_ARG_KEY_PARTS) + + +def _redact_display_arg_value(key: Any, value: Any) -> Any: + """Recursively redact display-only copies of tool arguments.""" + if _is_sensitive_arg_key(key): + return _DISPLAY_SECRET_PLACEHOLDER + + if isinstance(value, dict): + concealed = str(value.get("type", "")).lower() == "concealed" + return { + item_key: ( + _DISPLAY_SECRET_PLACEHOLDER + if concealed and _normalise_arg_key(item_key) == "value" + else _redact_display_arg_value(item_key, item_value) + ) + for item_key, item_value in value.items() + } + if isinstance(value, list): + return [_redact_display_arg_value(key, item) for item in value] + if isinstance(value, tuple): + return tuple(_redact_display_arg_value(key, item) for item in value) + if isinstance(value, str): + return redact_sensitive_text(value, force=True) + return value + - Backends sometimes echo the attempted input in error strings or fallback - metadata. When the raw typed value contains a recognizable secret (API - key, token, JWT, etc.) the redacted form differs from the raw value, so we - replace every occurrence of the raw value with its redacted form before a - browser_type result reaches logs, callbacks, the model, or chat history. +def redact_browser_typed_text_for_display(value: Any, typed_text: Any) -> Any: + """Remove browser_type raw typed text from display-facing payloads. - Normal typed text (search queries, addresses, form fields) matches no - secret pattern, so it passes through unchanged and stays readable. + ``browser_type`` is commonly used for passwords, OTPs, account numbers, and + ordinary form entries that do not match API-key/token regexes. Therefore the + presentation layer must never echo the literal typed value. Replace any raw + occurrence with a stable placeholder before a browser_type result reaches + logs, callbacks, the model, or chat history. Redaction is forced here regardless of the global ``security.redact_secrets`` preference: a typed credential leaking into chat history is a security @@ -361,10 +432,7 @@ def redact_browser_typed_text_for_display(value: Any, typed_text: Any) -> Any: needle = str(typed_text) if needle == "": return value - redacted = redact_sensitive_text(needle, force=True) - if redacted == needle: - # Nothing secret-looking in the typed text; leave payload untouched. - return value + redacted = _DISPLAY_TYPED_TEXT_PLACEHOLDER if isinstance(value, str): return value.replace(needle, redacted) if isinstance(value, dict): @@ -382,18 +450,23 @@ def redact_browser_typed_text_for_display(value: Any, typed_text: Any) -> Any: def redact_tool_args_for_display(tool_name: str, args: dict | None) -> dict | None: """Return a copy of tool args safe for logs/progress UI. - For ``browser_type`` the ``text`` argument is run through the same - secret-pattern redactor used for logs. Recognizable credentials (API - keys, tokens) are masked before the value reaches tool progress - notifications; normal typed text is left intact for debuggability. + Display surfaces are visible to humans in gateway chats and can persist as + permanent messages. Never expose browser-typed text, and recursively mask + values carried under credential-like argument names or 1Password-style + ``type=concealed`` field payloads. This function only sanitizes the copy + used for progress/log callbacks; raw arguments still go to the tool. """ if not isinstance(args, dict): return args - if tool_name == "browser_type" and isinstance(args.get("text"), str): - safe_args = dict(args) - safe_args["text"] = redact_sensitive_text(args["text"], force=True) - return safe_args - return args + safe_args = { + key: _redact_display_arg_value(key, value) + for key, value in args.items() + } + if tool_name == "browser_type" and "text" in safe_args: + safe_args["text"] = _DISPLAY_TYPED_TEXT_PLACEHOLDER + if tool_name == "process" and safe_args.get("action") in {"write", "submit"} and "data" in safe_args: + safe_args["data"] = _DISPLAY_TYPED_TEXT_PLACEHOLDER + return safe_args def _delegate_task_goal_parts(tasks: Any, *, per_goal_len: int) -> tuple[int, list[str]]: diff --git a/tests/agent/test_display.py b/tests/agent/test_display.py index ab390fac249d..8a4fe62d0f2d 100644 --- a/tests/agent/test_display.py +++ b/tests/agent/test_display.py @@ -87,34 +87,55 @@ def test_read_file_preview_includes_requested_line_range(self): result = build_tool_preview("read_file", {"path": "./package.json", "offset": 1, "limit": 5}) assert result == "package.json L1-5" - def test_browser_type_preview_redacts_api_key(self): - secret = "sk-proj-ABCD1234567890EFGH" + def test_browser_type_preview_hides_typed_text(self): + secret = "not-a-real-password-123!" result = build_tool_preview("browser_type", {"ref": "@e3", "text": secret}) assert result is not None assert secret not in result - assert "sk-pro" in result and "..." in result + assert result == "[typed text hidden]" - def test_browser_type_preview_keeps_normal_text(self): + def test_browser_type_preview_hides_normal_text_too(self): text = "hello world search query" result = build_tool_preview("browser_type", {"ref": "@e3", "text": text}) assert result is not None - assert text in result + assert text not in result + assert result == "[typed text hidden]" - def test_browser_type_display_args_redact_api_key(self): - secret = "ghp_ABCDEFGHIJ1234567890" + def test_browser_type_display_args_hide_typed_text(self): + secret = "ghp_ABCDEF1234567890secret" safe_args = redact_tool_args_for_display( "browser_type", {"ref": "@e3", "text": secret} ) + assert isinstance(safe_args, dict) assert secret not in str(safe_args) assert safe_args["ref"] == "@e3" - assert safe_args["text"].startswith("ghp_AB") + assert safe_args["text"] == "[typed text hidden]" - def test_browser_type_display_args_keep_normal_text(self): + def test_browser_type_display_args_hide_normal_text(self): text = "my_normal_password_123" safe_args = redact_tool_args_for_display( "browser_type", {"ref": "@e3", "text": text} ) - assert safe_args == {"ref": "@e3", "text": text} + assert isinstance(safe_args, dict) + assert safe_args == {"ref": "@e3", "text": "[typed text hidden]"} + + def test_display_args_redact_sensitive_named_values_recursively(self): + safe_args = redact_tool_args_for_display( + "mcp__1password__item_edit", + { + "password": "plain-password", + "fields": [ + {"idOrTitle": "API key", "type": "concealed", "value": "secret-value"}, + {"idOrTitle": "note", "type": "text", "value": "visible-value"}, + ], + }, + ) + assert isinstance(safe_args, dict) + assert "plain-password" not in repr(safe_args) + assert "secret-value" not in repr(safe_args) + assert safe_args["password"] == "[redacted]" + assert safe_args["fields"][0]["value"] == "[redacted]" + assert safe_args["fields"][1]["value"] == "visible-value" def test_unknown_tool_with_fallback_key(self): """Unknown tool but with a recognized fallback key should still preview.""" @@ -272,7 +293,7 @@ def test_delegate_task_batch_message_includes_goals(self): ) assert "2x: Review PR A | Review PR B" in line - def test_browser_type_cute_message_redacts_api_key(self): + def test_browser_type_cute_message_hides_typed_text(self): secret = "sk-proj-ABCD1234567890EFGH" line = get_cute_tool_message( "browser_type", @@ -282,9 +303,10 @@ def test_browser_type_cute_message_redacts_api_key(self): ) assert secret not in line - assert "sk-pro" in line + assert "[typed text hidden]" in line + assert "sk-pro" not in line - def test_browser_type_cute_message_keeps_normal_text(self): + def test_browser_type_cute_message_hides_normal_text(self): text = "hello world" line = get_cute_tool_message( "browser_type", @@ -293,7 +315,8 @@ def test_browser_type_cute_message_keeps_normal_text(self): result='{"success": true, "typed": "hello world"}', ) - assert text in line + assert text not in line + assert "[typed text hidden]" in line class TestEditDiffPreview: diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index 171a7c11255d..9246e2e305c5 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -2886,8 +2886,8 @@ def test_sequential_tool_callbacks_fire_in_order(self, agent): assert starts == [("c1", "web_search", {"query": "hello"})] assert completes == [("c1", "web_search", {"query": "hello"}, '{"success": true}')] - def test_sequential_browser_type_callbacks_redact_api_key(self, agent): - secret = "sk-proj-ABCD1234567890EFGH" + def test_sequential_browser_type_callbacks_hide_typed_text(self, agent): + secret = "not-a-real-password-123!" tool_call = _mock_tool_call( name="browser_type", arguments=json.dumps({"ref": "@apikey", "text": secret}), @@ -2902,12 +2902,12 @@ def test_sequential_browser_type_callbacks_redact_api_key(self, agent): agent.tool_complete_callback = lambda tool_call_id, function_name, function_args, function_result: completes.append((tool_call_id, function_name, function_args, function_result)) agent.tool_progress_callback = lambda event, name, preview, args, **kw: progress.append((event, name, preview, args)) - with patch("run_agent.handle_function_call", return_value='{"success": true, "typed": "sk-pro...EFGH"}'): + with patch("run_agent.handle_function_call", return_value='{"success": true, "typed": "[typed text hidden]"}'): agent._execute_tool_calls_sequential(mock_msg, messages, "task-1") - assert starts[0][2]["text"].startswith("sk-pro") - assert completes[0][2]["text"].startswith("sk-pro") - assert progress[0][2].startswith("sk-pro") + assert starts[0][2]["text"] == "[typed text hidden]" + assert completes[0][2]["text"] == "[typed text hidden]" + assert progress[0][2] == "[typed text hidden]" assert secret not in repr(starts + completes + progress) def test_concurrent_tool_callbacks_fire_for_each_tool(self, agent): @@ -2931,8 +2931,8 @@ def test_concurrent_tool_callbacks_fire_for_each_tool(self, agent): assert {entry[0] for entry in completes} == {"c1", "c2"} assert {entry[3] for entry in completes} == {'{"id":1}', '{"id":2}'} - def test_concurrent_browser_type_callbacks_redact_api_key(self, agent): - secret = "sk-proj-ABCD1234567890EFGH" + def test_concurrent_browser_type_callbacks_hide_typed_text(self, agent): + secret = "not-a-real-password-123!" tc = _mock_tool_call( name="browser_type", arguments=json.dumps({"ref": "@apikey", "text": secret}), @@ -2947,12 +2947,12 @@ def test_concurrent_browser_type_callbacks_redact_api_key(self, agent): agent.tool_complete_callback = lambda tool_call_id, function_name, function_args, function_result: completes.append((tool_call_id, function_name, function_args, function_result)) agent.tool_progress_callback = lambda event, name, preview, args, **kw: progress.append((event, name, preview, args)) - with patch("run_agent.handle_function_call", return_value='{"success": true, "typed": "sk-pro...EFGH"}'): + with patch("run_agent.handle_function_call", return_value='{"success": true, "typed": "[typed text hidden]"}'): agent._execute_tool_calls_concurrent(mock_msg, messages, "task-1") - assert starts[0][2]["text"].startswith("sk-pro") - assert completes[0][2]["text"].startswith("sk-pro") - assert progress[0][2].startswith("sk-pro") + assert starts[0][2]["text"] == "[typed text hidden]" + assert completes[0][2]["text"] == "[typed text hidden]" + assert progress[0][2] == "[typed text hidden]" assert secret not in repr(starts + completes + progress) def test_invoke_tool_handles_agent_level_tools(self, agent): diff --git a/tests/tools/test_browser_type_redaction.py b/tests/tools/test_browser_type_redaction.py index 10a210f7b582..00d6b2416e18 100644 --- a/tests/tools/test_browser_type_redaction.py +++ b/tests/tools/test_browser_type_redaction.py @@ -1,9 +1,7 @@ """Regression tests for browser_type display redaction. -Typed text is passed through the same secret-pattern redactor used for logs: -recognizable credentials (API keys, tokens) are masked in display-facing -output, while normal typed text is left intact. The raw value is always sent -to the browser backend regardless. +Typed text is hidden from display-facing output regardless of whether it looks +like an API key/token. The raw value is still sent to the browser backend. """ import json @@ -12,7 +10,7 @@ from tools.browser_tool import browser_type -def test_browser_type_redacts_api_key_in_output(monkeypatch): +def test_browser_type_hides_api_key_in_output(monkeypatch): monkeypatch.delenv("CAMOFOX_URL", raising=False) monkeypatch.delenv("BROWSER_CDP_URL", raising=False) monkeypatch.setenv("HERMES_REDACT_SECRETS", "true") @@ -26,13 +24,13 @@ def test_browser_type_redacts_api_key_in_output(monkeypatch): assert result["success"] is True assert secret not in json.dumps(result) - assert result["typed"].startswith("sk-pro") + assert result["typed"] == "[typed text hidden]" # Raw secret still typed into the page. mock_run.assert_called_once() assert mock_run.call_args.args[2] == ["@apikey", secret] -def test_browser_type_keeps_normal_text_in_output(monkeypatch): +def test_browser_type_hides_normal_text_in_output(monkeypatch): monkeypatch.delenv("CAMOFOX_URL", raising=False) monkeypatch.delenv("BROWSER_CDP_URL", raising=False) monkeypatch.setenv("HERMES_REDACT_SECRETS", "true") @@ -45,12 +43,13 @@ def test_browser_type_keeps_normal_text_in_output(monkeypatch): result = json.loads(browser_type("@search", text, task_id="redaction-test")) assert result["success"] is True - assert result["typed"] == text + assert text not in json.dumps(result) + assert result["typed"] == "[typed text hidden]" mock_run.assert_called_once() assert mock_run.call_args.args[2] == ["@search", text] -def test_browser_type_failure_redacts_api_key_in_error(monkeypatch): +def test_browser_type_failure_hides_typed_text_in_error(monkeypatch): monkeypatch.delenv("CAMOFOX_URL", raising=False) monkeypatch.delenv("BROWSER_CDP_URL", raising=False) monkeypatch.setenv("HERMES_REDACT_SECRETS", "true") @@ -69,6 +68,6 @@ def test_browser_type_failure_redacts_api_key_in_error(monkeypatch): assert result["success"] is False assert secret not in raw_result - assert "sk-pro" in raw_result + assert "[typed text hidden]" in raw_result mock_run.assert_called_once() assert mock_run.call_args.args[2] == ["@apikey", secret] diff --git a/tools/browser_camofox.py b/tools/browser_camofox.py index 4151d3cdb153..da99a6769a02 100644 --- a/tools/browser_camofox.py +++ b/tools/browser_camofox.py @@ -699,8 +699,7 @@ def camofox_type(ref: str, text: str, task_id: Optional[str] = None) -> str: response = { "success": True, - # Match browser_tool.browser_type: run typed text through the - # secret-pattern redactor so API keys / tokens don't leak into + # Match browser_tool.browser_type: never echo raw typed text into # tool progress or chat history. The raw text is still typed into # the page; only the returned display value is redacted. "typed": display_text, diff --git a/tools/browser_tool.py b/tools/browser_tool.py index 82c248a3f715..464ab9371792 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -3095,10 +3095,8 @@ def browser_type(ref: str, text: str, task_id: Optional[str] = None) -> str: if result.get("success"): response = { "success": True, - # Run typed text through the secret-pattern redactor so API keys / - # tokens don't leak into tool progress or chat history. Normal - # text passes through unchanged. The raw value was already sent - # to the browser command above. + # Never echo raw typed text into tool progress or chat history. + # The raw value was already sent to the browser command above. "typed": display_text, "element": ref }