From bdb97b8573c4e9826339431d79068a88e41b2655 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 24 May 2026 04:27:17 -0700 Subject: [PATCH] fix(feishu): enforce auth and chat binding for approval buttons (#30744) --- gateway/platforms/feishu.py | 39 ++++++++++++++++-- tests/gateway/test_feishu_approval_buttons.py | 41 ++++++++++++++++--- 2 files changed, 72 insertions(+), 8 deletions(-) diff --git a/gateway/platforms/feishu.py b/gateway/platforms/feishu.py index 73199424350..832698722d8 100644 --- a/gateway/platforms/feishu.py +++ b/gateway/platforms/feishu.py @@ -2589,7 +2589,18 @@ class FeishuAdapter(BasePlatformAdapter): user_name = self._get_cached_sender_name(open_id) or open_id - if not self._submit_on_loop(loop, self._resolve_approval(approval_id, choice, user_name)): + chat_context = getattr(event, "context", None) + chat_id = str(getattr(chat_context, "open_chat_id", "") or "") + if not self._submit_on_loop( + loop, + self._resolve_approval( + approval_id=approval_id, + choice=choice, + user_name=user_name, + open_id=open_id, + chat_id=chat_id, + ), + ): return P2CardActionTriggerResponse() if P2CardActionTriggerResponse else None if P2CardActionTriggerResponse is None: @@ -2637,12 +2648,34 @@ class FeishuAdapter(BasePlatformAdapter): response.card = card return response - async def _resolve_approval(self, approval_id: Any, choice: str, user_name: str) -> None: + async def _resolve_approval( + self, + approval_id: Any, + choice: str, + user_name: str, + *, + open_id: str = "", + chat_id: str = "", + ) -> None: """Pop approval state and unblock the waiting agent thread.""" - state = self._approval_state.pop(approval_id, None) + state = self._approval_state.get(approval_id) if not state: logger.debug("[Feishu] Approval %s already resolved or unknown", approval_id) return + if not self._is_interactive_operator_authorized(open_id): + logger.warning("[Feishu] Unauthorized approval click by %s for approval %s", open_id or "", approval_id) + return + expected_chat_id = str(state.get("chat_id", "") or "") + if expected_chat_id and chat_id and expected_chat_id != chat_id: + logger.warning( + "[Feishu] Approval %s chat mismatch (expected=%s, got=%s)", + approval_id, expected_chat_id, chat_id, + ) + return + state = self._approval_state.pop(approval_id, None) + if not state: + logger.debug("[Feishu] Approval %s already resolved while validating callback", approval_id) + return try: from tools.approval import resolve_gateway_approval count = resolve_gateway_approval(state["session_key"], choice) diff --git a/tests/gateway/test_feishu_approval_buttons.py b/tests/gateway/test_feishu_approval_buttons.py index 76acf4a72a8..41335f1141a 100644 --- a/tests/gateway/test_feishu_approval_buttons.py +++ b/tests/gateway/test_feishu_approval_buttons.py @@ -320,7 +320,7 @@ class TestResolveApproval: } with patch("tools.approval.resolve_gateway_approval", return_value=1) as mock_resolve: - await adapter._resolve_approval(1, "once", "Norbert") + await adapter._resolve_approval(1, "once", "Norbert", open_id="ou_user1", chat_id="oc_12345") mock_resolve.assert_called_once_with("agent:main:feishu:group:oc_12345", "once") assert 1 not in adapter._approval_state @@ -335,7 +335,7 @@ class TestResolveApproval: } with patch("tools.approval.resolve_gateway_approval", return_value=1) as mock_resolve: - await adapter._resolve_approval(2, "deny", "Alice") + await adapter._resolve_approval(2, "deny", "Alice", open_id="ou_user1", chat_id="oc_12345") mock_resolve.assert_called_once_with("some-session", "deny") @@ -349,7 +349,7 @@ class TestResolveApproval: } with patch("tools.approval.resolve_gateway_approval", return_value=1) as mock_resolve: - await adapter._resolve_approval(3, "session", "Bob") + await adapter._resolve_approval(3, "session", "Bob", open_id="ou_user1", chat_id="oc_99") mock_resolve.assert_called_once_with("sess-3", "session") @@ -363,7 +363,7 @@ class TestResolveApproval: } with patch("tools.approval.resolve_gateway_approval", return_value=1) as mock_resolve: - await adapter._resolve_approval(4, "always", "Carol") + await adapter._resolve_approval(4, "always", "Carol", open_id="ou_user1", chat_id="oc_55") mock_resolve.assert_called_once_with("sess-4", "always") @@ -372,10 +372,41 @@ class TestResolveApproval: adapter = _make_adapter() with patch("tools.approval.resolve_gateway_approval") as mock_resolve: - await adapter._resolve_approval(99, "once", "Nobody") + await adapter._resolve_approval(99, "once", "Nobody", open_id="ou_user1", chat_id="oc_12345") mock_resolve.assert_not_called() + @pytest.mark.asyncio + async def test_unauthorized_click_does_not_resolve(self): + adapter = _make_adapter() + adapter._admins = {"ou_admin"} + adapter._approval_state[5] = { + "session_key": "sess-5", + "message_id": "msg_005", + "chat_id": "oc_12345", + } + + with patch("tools.approval.resolve_gateway_approval") as mock_resolve: + await adapter._resolve_approval(5, "once", "Mallory", open_id="ou_intruder", chat_id="oc_12345") + + mock_resolve.assert_not_called() + assert 5 in adapter._approval_state + + @pytest.mark.asyncio + async def test_chat_mismatch_does_not_resolve(self): + adapter = _make_adapter() + adapter._approval_state[6] = { + "session_key": "sess-6", + "message_id": "msg_006", + "chat_id": "oc_expected", + } + + with patch("tools.approval.resolve_gateway_approval") as mock_resolve: + await adapter._resolve_approval(6, "session", "Norbert", open_id="ou_user1", chat_id="oc_wrong") + + mock_resolve.assert_not_called() + assert 6 in adapter._approval_state + # =========================================================================== # _handle_card_action_event — non-approval card actions # ===========================================================================