From c1818b7e9ee2cf03d2ea7ddfa7a74d658ce98faa Mon Sep 17 00:00:00 2001 From: WAXLYY Date: Tue, 7 Apr 2026 01:02:56 +0300 Subject: [PATCH] fix(tools): redact query secrets in send_message errors --- .../test_send_message_missing_platforms.py | 23 +++++ tests/tools/test_send_message_tool.py | 27 ++++++ tools/send_message_tool.py | 93 ++++++++++++------- 3 files changed, 111 insertions(+), 32 deletions(-) diff --git a/tests/tools/test_send_message_missing_platforms.py b/tests/tools/test_send_message_missing_platforms.py index 8943109e0..881ae33d2 100644 --- a/tests/tools/test_send_message_missing_platforms.py +++ b/tests/tools/test_send_message_missing_platforms.py @@ -314,6 +314,29 @@ class TestSendDingtalk: assert "error" in result assert "DingTalk send failed" in result["error"] + def test_http_error_redacts_access_token_in_exception_text(self): + token = "supersecret-access-token-123456789" + resp = self._make_httpx_resp(status_code=401) + resp.raise_for_status = MagicMock( + side_effect=Exception( + f"POST https://oapi.dingtalk.com/robot/send?access_token={token} returned 401" + ) + ) + client_ctx, _ = self._make_httpx_client(resp) + + with patch("httpx.AsyncClient", return_value=client_ctx): + result = asyncio.run( + _send_dingtalk( + {"webhook_url": f"https://oapi.dingtalk.com/robot/send?access_token={token}"}, + "ch", + "hi", + ) + ) + + assert "error" in result + assert token not in result["error"] + assert "access_token=***" in result["error"] + def test_missing_config(self): with patch.dict(os.environ, {"DINGTALK_WEBHOOK_URL": ""}, clear=False): result = asyncio.run(_send_dingtalk({}, "ch", "hi")) diff --git a/tests/tools/test_send_message_tool.py b/tests/tools/test_send_message_tool.py index 7b4643af8..34cea278d 100644 --- a/tests/tools/test_send_message_tool.py +++ b/tests/tools/test_send_message_tool.py @@ -276,6 +276,33 @@ class TestSendMessageTool: thread_id=None, ) + def test_top_level_send_failure_redacts_query_token(self): + config, _telegram_cfg = _make_config() + leaked = "very-secret-query-token-123456" + + def _raise_and_close(coro): + coro.close() + raise RuntimeError( + f"transport error: https://api.example.com/send?access_token={leaked}" + ) + + with patch("gateway.config.load_gateway_config", return_value=config), \ + patch("tools.interrupt.is_interrupted", return_value=False), \ + patch("model_tools._run_async", side_effect=_raise_and_close): + result = json.loads( + send_message_tool( + { + "action": "send", + "target": "telegram:-1001", + "message": "hello", + } + ) + ) + + assert "error" in result + assert leaked not in result["error"] + assert "access_token=***" in result["error"] + class TestSendTelegramMediaDelivery: def test_sends_text_then_photo_for_media_tag(self, tmp_path, monkeypatch): diff --git a/tools/send_message_tool.py b/tools/send_message_tool.py index 32741f08a..eff0e7b55 100644 --- a/tools/send_message_tool.py +++ b/tools/send_message_tool.py @@ -12,6 +12,8 @@ import re import ssl import time +from agent.redact import redact_sensitive_text + logger = logging.getLogger(__name__) _TELEGRAM_TOPIC_TARGET_RE = re.compile(r"^\s*(-?\d+)(?::(\d+))?\s*$") @@ -20,6 +22,27 @@ _IMAGE_EXTS = {".jpg", ".jpeg", ".png", ".webp", ".gif"} _VIDEO_EXTS = {".mp4", ".mov", ".avi", ".mkv", ".3gp"} _AUDIO_EXTS = {".ogg", ".opus", ".mp3", ".wav", ".m4a"} _VOICE_EXTS = {".ogg", ".opus"} +_URL_SECRET_QUERY_RE = re.compile( + r"([?&](?:access_token|api[_-]?key|auth[_-]?token|token|signature|sig)=)([^&#\s]+)", + re.IGNORECASE, +) +_GENERIC_SECRET_ASSIGN_RE = re.compile( + r"\b(access_token|api[_-]?key|auth[_-]?token|signature|sig)\s*=\s*([^\s,;]+)", + re.IGNORECASE, +) + + +def _sanitize_error_text(text) -> str: + """Redact secrets from error text before surfacing it to users/models.""" + redacted = redact_sensitive_text(text) + redacted = _URL_SECRET_QUERY_RE.sub(lambda m: f"{m.group(1)}***", redacted) + redacted = _GENERIC_SECRET_ASSIGN_RE.sub(lambda m: f"{m.group(1)}=***", redacted) + return redacted + + +def _error(message: str) -> dict: + """Build a standardized error payload with redacted content.""" + return {"error": _sanitize_error_text(message)} SEND_MESSAGE_SCHEMA = { @@ -70,7 +93,7 @@ def _handle_list(): from gateway.channel_directory import format_directory_for_display return json.dumps({"targets": format_directory_for_display()}) except Exception as e: - return json.dumps({"error": f"Failed to load channel directory: {e}"}) + return json.dumps(_error(f"Failed to load channel directory: {e}")) def _handle_send(args): @@ -117,7 +140,7 @@ def _handle_send(args): from gateway.config import load_gateway_config, Platform config = load_gateway_config() except Exception as e: - return json.dumps({"error": f"Failed to load gateway config: {e}"}) + return json.dumps(_error(f"Failed to load gateway config: {e}")) platform_map = { "telegram": Platform.TELEGRAM, @@ -190,9 +213,11 @@ def _handle_send(args): except Exception: pass + if isinstance(result, dict) and "error" in result: + result["error"] = _sanitize_error_text(result["error"]) return json.dumps(result) except Exception as e: - return json.dumps({"error": f"Send failed: {e}"}) + return json.dumps(_error(f"Send failed: {e}")) def _parse_target_ref(platform_name: str, target_ref: str): @@ -434,7 +459,11 @@ async def _send_telegram(token, chat_id, message, media_files=None, thread_id=No except Exception as md_error: # Parse failed, fall back to plain text if "parse" in str(md_error).lower() or "markdown" in str(md_error).lower() or "html" in str(md_error).lower(): - logger.warning("Parse mode %s failed in _send_telegram, falling back to plain text: %s", send_parse_mode, md_error) + logger.warning( + "Parse mode %s failed in _send_telegram, falling back to plain text: %s", + send_parse_mode, + _sanitize_error_text(md_error), + ) if not _has_html: try: from gateway.platforms.telegram import _strip_mdv2 @@ -481,7 +510,7 @@ async def _send_telegram(token, chat_id, message, media_files=None, thread_id=No chat_id=int_chat_id, document=f, **thread_kwargs ) except Exception as e: - warning = f"Failed to send media {media_path}: {e}" + warning = _sanitize_error_text(f"Failed to send media {media_path}: {e}") logger.error(warning) warnings.append(warning) @@ -503,7 +532,7 @@ async def _send_telegram(token, chat_id, message, media_files=None, thread_id=No except ImportError: return {"error": "python-telegram-bot not installed. Run: pip install python-telegram-bot"} except Exception as e: - return {"error": f"Telegram send failed: {e}"} + return _error(f"Telegram send failed: {e}") async def _send_discord(token, chat_id, message): @@ -522,11 +551,11 @@ async def _send_discord(token, chat_id, message): async with session.post(url, headers=headers, json={"content": message}) as resp: if resp.status not in (200, 201): body = await resp.text() - return {"error": f"Discord API error ({resp.status}): {body}"} + return _error(f"Discord API error ({resp.status}): {body}") data = await resp.json() return {"success": True, "platform": "discord", "chat_id": chat_id, "message_id": data.get("id")} except Exception as e: - return {"error": f"Discord send failed: {e}"} + return _error(f"Discord send failed: {e}") async def _send_slack(token, chat_id, message): @@ -543,9 +572,9 @@ async def _send_slack(token, chat_id, message): data = await resp.json() if data.get("ok"): return {"success": True, "platform": "slack", "chat_id": chat_id, "message_id": data.get("ts")} - return {"error": f"Slack API error: {data.get('error', 'unknown')}"} + return _error(f"Slack API error: {data.get('error', 'unknown')}") except Exception as e: - return {"error": f"Slack send failed: {e}"} + return _error(f"Slack send failed: {e}") async def _send_whatsapp(extra, chat_id, message): @@ -571,9 +600,9 @@ async def _send_whatsapp(extra, chat_id, message): "message_id": data.get("messageId"), } body = await resp.text() - return {"error": f"WhatsApp bridge error ({resp.status}): {body}"} + return _error(f"WhatsApp bridge error ({resp.status}): {body}") except Exception as e: - return {"error": f"WhatsApp send failed: {e}"} + return _error(f"WhatsApp send failed: {e}") async def _send_signal(extra, chat_id, message): @@ -606,10 +635,10 @@ async def _send_signal(extra, chat_id, message): resp.raise_for_status() data = resp.json() if "error" in data: - return {"error": f"Signal RPC error: {data['error']}"} + return _error(f"Signal RPC error: {data['error']}") return {"success": True, "platform": "signal", "chat_id": chat_id} except Exception as e: - return {"error": f"Signal send failed: {e}"} + return _error(f"Signal send failed: {e}") async def _send_email(extra, chat_id, message): @@ -638,7 +667,7 @@ async def _send_email(extra, chat_id, message): server.quit() return {"success": True, "platform": "email", "chat_id": chat_id} except Exception as e: - return {"error": f"Email send failed: {e}"} + return _error(f"Email send failed: {e}") async def _send_sms(auth_token, chat_id, message): @@ -687,11 +716,11 @@ async def _send_sms(auth_token, chat_id, message): body = await resp.json() if resp.status >= 400: error_msg = body.get("message", str(body)) - return {"error": f"Twilio API error ({resp.status}): {error_msg}"} + return _error(f"Twilio API error ({resp.status}): {error_msg}") msg_sid = body.get("sid", "") return {"success": True, "platform": "sms", "chat_id": chat_id, "message_id": msg_sid} except Exception as e: - return {"error": f"SMS send failed: {e}"} + return _error(f"SMS send failed: {e}") async def _send_mattermost(token, extra, chat_id, message): @@ -711,11 +740,11 @@ async def _send_mattermost(token, extra, chat_id, message): async with session.post(url, headers=headers, json={"channel_id": chat_id, "message": message}) as resp: if resp.status not in (200, 201): body = await resp.text() - return {"error": f"Mattermost API error ({resp.status}): {body}"} + return _error(f"Mattermost API error ({resp.status}): {body}") data = await resp.json() return {"success": True, "platform": "mattermost", "chat_id": chat_id, "message_id": data.get("id")} except Exception as e: - return {"error": f"Mattermost send failed: {e}"} + return _error(f"Mattermost send failed: {e}") async def _send_matrix(token, extra, chat_id, message): @@ -753,11 +782,11 @@ async def _send_matrix(token, extra, chat_id, message): async with session.put(url, headers=headers, json=payload) as resp: if resp.status not in (200, 201): body = await resp.text() - return {"error": f"Matrix API error ({resp.status}): {body}"} + return _error(f"Matrix API error ({resp.status}): {body}") data = await resp.json() return {"success": True, "platform": "matrix", "chat_id": chat_id, "message_id": data.get("event_id")} except Exception as e: - return {"error": f"Matrix send failed: {e}"} + return _error(f"Matrix send failed: {e}") async def _send_homeassistant(token, extra, chat_id, message): @@ -777,10 +806,10 @@ async def _send_homeassistant(token, extra, chat_id, message): async with session.post(url, headers=headers, json={"message": message, "target": chat_id}) as resp: if resp.status not in (200, 201): body = await resp.text() - return {"error": f"Home Assistant API error ({resp.status}): {body}"} + return _error(f"Home Assistant API error ({resp.status}): {body}") return {"success": True, "platform": "homeassistant", "chat_id": chat_id} except Exception as e: - return {"error": f"Home Assistant send failed: {e}"} + return _error(f"Home Assistant send failed: {e}") async def _send_dingtalk(extra, chat_id, message): @@ -808,10 +837,10 @@ async def _send_dingtalk(extra, chat_id, message): resp.raise_for_status() data = resp.json() if data.get("errcode", 0) != 0: - return {"error": f"DingTalk API error: {data.get('errmsg', 'unknown')}"} + return _error(f"DingTalk API error: {data.get('errmsg', 'unknown')}") return {"success": True, "platform": "dingtalk", "chat_id": chat_id} except Exception as e: - return {"error": f"DingTalk send failed: {e}"} + return _error(f"DingTalk send failed: {e}") async def _send_wecom(extra, chat_id, message): @@ -829,16 +858,16 @@ async def _send_wecom(extra, chat_id, message): adapter = WeComAdapter(pconfig) connected = await adapter.connect() if not connected: - return {"error": f"WeCom: failed to connect — {adapter.fatal_error_message or 'unknown error'}"} + return _error(f"WeCom: failed to connect - {adapter.fatal_error_message or 'unknown error'}") try: result = await adapter.send(chat_id, message) if not result.success: - return {"error": f"WeCom send failed: {result.error}"} + return _error(f"WeCom send failed: {result.error}") return {"success": True, "platform": "wecom", "chat_id": chat_id, "message_id": result.message_id} finally: await adapter.disconnect() except Exception as e: - return {"error": f"WeCom send failed: {e}"} + return _error(f"WeCom send failed: {e}") async def _send_feishu(pconfig, chat_id, message, media_files=None, thread_id=None): @@ -864,11 +893,11 @@ async def _send_feishu(pconfig, chat_id, message, media_files=None, thread_id=No if message.strip(): last_result = await adapter.send(chat_id, message, metadata=metadata) if not last_result.success: - return {"error": f"Feishu send failed: {last_result.error}"} + return _error(f"Feishu send failed: {last_result.error}") for media_path, is_voice in media_files: if not os.path.exists(media_path): - return {"error": f"Media file not found: {media_path}"} + return _error(f"Media file not found: {media_path}") ext = os.path.splitext(media_path)[1].lower() if ext in _IMAGE_EXTS: @@ -883,7 +912,7 @@ async def _send_feishu(pconfig, chat_id, message, media_files=None, thread_id=No last_result = await adapter.send_document(chat_id, media_path, metadata=metadata) if not last_result.success: - return {"error": f"Feishu media send failed: {last_result.error}"} + return _error(f"Feishu media send failed: {last_result.error}") if last_result is None: return {"error": "No deliverable text or media remained after processing MEDIA tags"} @@ -895,7 +924,7 @@ async def _send_feishu(pconfig, chat_id, message, media_files=None, thread_id=No "message_id": last_result.message_id, } except Exception as e: - return {"error": f"Feishu send failed: {e}"} + return _error(f"Feishu send failed: {e}") def _check_send_message():