fix(channels): reject unsigned webhook POSTs on encryption-configured channels (#401)

Closes #392 (uncontroversial part).

WeChat (`_handle_message`) and Feishu (`_handle_event`) gated their
signature/decryption checks behind a condition the REQUEST controls:

- WeChat: `if encrypt and self._crypto:` -- a POST with no `<Encrypt>`
  element took the false branch and reached `_safe_process_message`
  without any verification, even when `encoding_aes_key` + `token` were
  configured.
- Feishu: `if self.config.encrypt_key and "encrypt" in body:` -- a
  plaintext body skipped decryption entirely and was processed directly.

Since the webhook port is the channel's only inbound boundary, an
attacker could POST forged plaintext and reach the agent, spoofing
`sender_id` / `FromUserName` (and, with an empty allowlist, passing the
sender gate).

Fix: when encryption is configured, an inbound POST MUST carry the
encrypted field (`<Encrypt>` / `encrypt`) -- otherwise it is rejected
with 403 and never reaches the agent. Plaintext mode (no encryption
configured) is unchanged, so existing plaintext deployments are not
affected. The remaining fail-closed question (what to do when
credentials are entirely unset) is left for the maintainers to decide
as the policy part of the issue.

Regression tests (9 new):
- WeChat: plaintext rejected / missing Encrypt rejected / bad signature
  rejected / valid signature decrypts and processes / plaintext still
  accepted when no crypto.
- Feishu: plaintext rejected / non-dict body rejected / encrypted body
  decrypts and processes / plaintext still accepted when no encrypt_key.

93 tests in the two channel files pass; full suite 3045 passed, 13
skipped; ruff clean.

Co-authored-by: Xi Zhang <106144707+X-iZhang@users.noreply.github.com>

EvoScientist/channels/feishu/channel.py

@@ -828,8 +828,18 @@ class FeishuChannel(Channel, WebhookMixin, TokenMixin):
         except Exception:
             return web.Response(status=400)
 
-        # ── Decrypt if encrypt_key is configured ──
-        if self.config.encrypt_key and "encrypt" in body:
+        # When encryption is configured the inbound POST MUST carry an
+        # ``encrypt`` field. A plaintext body used to skip decryption and
+        # reach the agent directly, defeating the encryption setup (issue
+        # #392). Treat a missing ``encrypt`` field on an
+        # encryption-configured channel as an authentication failure.
+        if self.config.encrypt_key:
+            if not isinstance(body, dict) or "encrypt" not in body:
+                logger.warning(
+                    "Feishu event rejected: encrypt_key is configured but the "
+                    "body has no 'encrypt' field (possible signature bypass)"
+                )
+                return web.Response(status=403)
             try:
                 body = self._decrypt_event(body["encrypt"])
             except Exception:

EvoScientist/channels/wechat/channel.py

@@ -337,9 +337,21 @@ class WeChatChannel(Channel, WebhookMixin, TokenMixin):
         logger.info(f"WeChat callback POST received, body length={len(body)}")
         xml_data = parse_xml(body)
 
-        # If encrypted, decrypt first
+        # If encryption is configured, the inbound POST MUST carry an
+        # <Encrypt> element and a matching msg_signature. An unsigned body
+        # used to fall through to _safe_process_message and reach the agent
+        # regardless of credentials, which made the encryption setup
+        # ineffective (issue #392). Treat a missing <Encrypt> on an
+        # encryption-configured channel as an authentication failure.
         encrypt = xml_data.get("Encrypt", "")
-        if encrypt and self._crypto:
+        if self._crypto:
+            if not encrypt:
+                logger.warning(
+                    "WeChat POST rejected: encryption is configured but the "
+                    "body has no <Encrypt> element (possible signature bypass)"
+                )
+                return web.Response(status=403)
+
             signature = request.query.get("msg_signature", "")
             timestamp = request.query.get("timestamp", "")
             nonce = request.query.get("nonce", "")

tests/test_feishu_channel.py

@@ -2,6 +2,7 @@
 
 import json
 import sys
+from typing import ClassVar
 from unittest.mock import AsyncMock, MagicMock, patch
 
 import pytest
@@ -624,3 +625,94 @@ class TestFeishuWebSocketMode:
         assert channel._main_loop is None
         assert channel._ws_event_queue is None
         assert channel._access_token is None
+
+
+# ── Webhook signature bypass regression (issue #392) ──────────────
+
+
+class _FakeFeishuRequest:
+    """Minimal stand-in for aiohttp.web.Request for _handle_event tests."""
+
+    def __init__(self, json_body):
+        self._json = json_body
+
+    async def json(self):
+        return self._json
+
+
+class TestFeishuWebhookSignatureBypass:
+    """Regression tests for issue #392: when ``encrypt_key`` is configured,
+    a plaintext POST (no ``encrypt`` field) must NOT reach the agent."""
+
+    FORGED_V2_EVENT: ClassVar[dict] = {
+        "schema": "2.0",
+        "header": {"event_type": "im.message.receive_v1", "token": ""},
+        "event": {
+            "sender": {"sender_id": {"open_id": "attacker"}, "sender_type": "user"},
+            "message": {
+                "chat_id": "oc_chat",
+                "message_type": "text",
+                "message_id": "om_msg",
+                "content": json.dumps({"text": "forged"}),
+            },
+        },
+    }
+
+    def _make_channel_with_encrypt_key(self) -> FeishuChannel:
+        config = FeishuConfig(
+            app_id="id",
+            app_secret="secret",
+            encrypt_key="my-encrypt-key",
+        )
+        channel = FeishuChannel(config)
+        channel._running = True
+        channel._http_client = MagicMock()
+        channel._access_token = "fake-token"
+        channel._token_expires = 9999999999
+        channel._on_message = AsyncMock()  # type: ignore[assignment]
+        return channel
+
+    def _make_channel_without_encrypt_key(self) -> FeishuChannel:
+        config = FeishuConfig(app_id="id", app_secret="secret")
+        channel = FeishuChannel(config)
+        channel._running = True
+        channel._http_client = MagicMock()
+        channel._access_token = "fake-token"
+        channel._token_expires = 9999999999
+        channel._on_message = AsyncMock()  # type: ignore[assignment]
+        return channel
+
+    async def test_plaintext_rejected_when_encrypt_key_configured(self):
+        """Plaintext POST with no `encrypt` field → 403, agent not reached."""
+        channel = self._make_channel_with_encrypt_key()
+        resp = await channel._handle_event(_FakeFeishuRequest(self.FORGED_V2_EVENT))
+        assert resp.status == 403
+        channel._on_message.assert_not_called()
+
+    async def test_non_dict_body_rejected_when_encrypt_key_configured(self):
+        """Defensive: a non-dict JSON body (list/str/etc.) → 403."""
+        channel = self._make_channel_with_encrypt_key()
+        for junk in ([1, 2, 3], "string-body", 42):
+            resp = await channel._handle_event(_FakeFeishuRequest(junk))
+            assert resp.status == 403, f"body={junk!r} should be rejected"
+        channel._on_message.assert_not_called()
+
+    async def test_encrypted_body_decrypts_and_processes(self):
+        """A valid encrypted body → 200, agent reached (no behavior change)."""
+        channel = self._make_channel_with_encrypt_key()
+        decrypted_event = self.FORGED_V2_EVENT
+        with patch.object(
+            FeishuChannel, "_decrypt_event", return_value=decrypted_event
+        ):
+            resp = await channel._handle_event(
+                _FakeFeishuRequest({"encrypt": "encrypted-blob"})
+            )
+        assert resp.status == 200
+        channel._on_message.assert_called_once()
+
+    async def test_plaintext_accepted_when_encrypt_key_not_configured(self):
+        """No-regression: plaintext mode keeps working when no encrypt_key."""
+        channel = self._make_channel_without_encrypt_key()
+        resp = await channel._handle_event(_FakeFeishuRequest(self.FORGED_V2_EVENT))
+        assert resp.status == 200
+        channel._on_message.assert_called_once()

tests/test_wechat_channel.py

@@ -4,6 +4,7 @@ import asyncio
 import hashlib
 import time
 import xml.etree.ElementTree as ET
+from unittest.mock import AsyncMock, MagicMock
 
 import pytest
 
@@ -442,6 +443,126 @@ class TestMessageProcessing:
         assert channel._queue.empty()
 
 
+# ── Webhook signature bypass regression (issue #392) ──────────────
+
+
+class _FakeWeChatRequest:
+    """Minimal stand-in for aiohttp.web.Request for _handle_message tests."""
+
+    def __init__(self, text_body: str, query: dict | None = None):
+        self._text = text_body
+        self.query = query or {}
+
+    async def text(self) -> str:
+        return self._text
+
+
+class TestWebhookSignatureBypass:
+    """Regression tests for issue #392: when encryption is configured, an
+    unsigned POST must NOT reach the agent — verify before branching, not
+    inside the branch the request controls."""
+
+    PLAINTEXT_FORGED_XML = (
+        "<xml><MsgType><![CDATA[text]]></MsgType>"
+        "<Content><![CDATA[forged]]></Content>"
+        "<FromUserName><![CDATA[attacker]]></FromUserName></xml>"
+    )
+
+    def _make_channel_with_crypto(self) -> WeChatChannel:
+        """Channel whose `_crypto` is set, mimicking what start() does when
+        encoding_aes_key + token are configured. We set _crypto directly to
+        avoid the network roundtrip in start()."""
+        config = WeComConfig(
+            corp_id="corp",
+            agent_id="1",
+            secret="s",
+            token="t",
+            encoding_aes_key="a" * 43,
+        )
+        channel = WeChatChannel(config, backend="wecom")
+        channel._crypto = MagicMock()
+        channel._crypto.verify_signature.return_value = (
+            False  # default: signature won't match
+        )
+        channel._safe_process_message = AsyncMock()  # type: ignore[assignment]
+        return channel
+
+    def _make_channel_without_crypto(self) -> WeChatChannel:
+        config = WeComConfig(corp_id="corp", agent_id="1", secret="s")
+        channel = WeChatChannel(config, backend="wecom")
+        # _crypto stays None (plaintext mode)
+        channel._safe_process_message = AsyncMock()  # type: ignore[assignment]
+        return channel
+
+    async def test_plaintext_rejected_when_crypto_configured(self):
+        """An unsigned POST on an encryption-configured channel must 403."""
+        channel = self._make_channel_with_crypto()
+        resp = await channel._handle_message(
+            _FakeWeChatRequest(self.PLAINTEXT_FORGED_XML)
+        )
+        assert resp.status == 403
+        channel._safe_process_message.assert_not_called()
+
+    async def test_missing_encrypt_rejected_even_with_crypto_present(self):
+        """Even if the body has other XML, no <Encrypt> + crypto set → 403."""
+        channel = self._make_channel_with_crypto()
+        body = "<xml><MsgType><![CDATA[text]]></MsgType></xml>"
+        resp = await channel._handle_message(_FakeWeChatRequest(body))
+        assert resp.status == 403
+        channel._safe_process_message.assert_not_called()
+
+    async def test_invalid_signature_rejected(self):
+        """Encrypted body with wrong signature → 403 (no behavior change)."""
+        channel = self._make_channel_with_crypto()
+        body = "<xml><Encrypt><![CDATA[encrypted-blob]]></Encrypt></xml>"
+        # crypto.verify_signature returns False by default in _make_channel_with_crypto
+        resp = await channel._handle_message(
+            _FakeWeChatRequest(
+                body,
+                query={
+                    "msg_signature": "wrong",
+                    "timestamp": "1",
+                    "nonce": "n",
+                },
+            )
+        )
+        assert resp.status == 403
+        channel._safe_process_message.assert_not_called()
+
+    async def test_valid_signature_decrypts_and_processes(self):
+        """Encrypted body with valid signature → 200, agent reached."""
+        channel = self._make_channel_with_crypto()
+        channel._crypto.verify_signature.return_value = True
+        channel._crypto.decrypt.return_value = (
+            "<xml><MsgType><![CDATA[text]]></MsgType>"
+            "<Content><![CDATA[legit]]></Content>"
+            "<FromUserName><![CDATA[user1]]></FromUserName></xml>",
+            "user1",
+        )
+        body = "<xml><Encrypt><![CDATA[ok]]></Encrypt></xml>"
+        resp = await channel._handle_message(
+            _FakeWeChatRequest(
+                body,
+                query={
+                    "msg_signature": "right",
+                    "timestamp": "1",
+                    "nonce": "n",
+                },
+            )
+        )
+        assert resp.status == 200
+        channel._safe_process_message.assert_called_once()
+
+    async def test_plaintext_accepted_when_crypto_not_configured(self):
+        """No-regression: plaintext mode (no crypto) keeps working."""
+        channel = self._make_channel_without_crypto()
+        resp = await channel._handle_message(
+            _FakeWeChatRequest(self.PLAINTEXT_FORGED_XML)
+        )
+        assert resp.status == 200
+        channel._safe_process_message.assert_called_once()
+
+
 # ── Registration test ─────────────────────────────────────────────