diff --git a/gateway/platforms/feishu.py b/gateway/platforms/feishu.py index a9b0447080d..73199424350 100644 --- a/gateway/platforms/feishu.py +++ b/gateway/platforms/feishu.py @@ -2563,10 +2563,30 @@ class FeishuAdapter(BasePlatformAdapter): if approval_id is None: logger.debug("[Feishu] Card action missing approval_id, ignoring") return P2CardActionTriggerResponse() if P2CardActionTriggerResponse else None + state = self._approval_state.get(approval_id) + if not state: + logger.debug("[Feishu] Approval %s already resolved or unknown", approval_id) + return P2CardActionTriggerResponse() if P2CardActionTriggerResponse else None choice = _APPROVAL_CHOICE_MAP.get(action_value.get("hermes_action"), "deny") operator = getattr(event, "operator", None) open_id = str(getattr(operator, "open_id", "") or "") + sender_id = SimpleNamespace(open_id=open_id, user_id=str(getattr(operator, "user_id", "") or "")) + if not self._allow_group_message(sender_id, state.get("chat_id", ""), is_bot=False): + logger.warning("[Feishu] Unauthorized approval click by %s", open_id or "") + return P2CardActionTriggerResponse() if P2CardActionTriggerResponse else None + + callback_chat_id = str(getattr(getattr(event, "context", None), "open_chat_id", "") or "") + expected_chat_id = str(state.get("chat_id", "") or "") + if callback_chat_id and expected_chat_id and callback_chat_id != expected_chat_id: + logger.warning( + "[Feishu] Approval callback chat mismatch for %s (expected=%s, got=%s)", + approval_id, + expected_chat_id, + callback_chat_id, + ) + return P2CardActionTriggerResponse() if P2CardActionTriggerResponse else None + 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)): diff --git a/tests/gateway/test_feishu_approval_buttons.py b/tests/gateway/test_feishu_approval_buttons.py index 8af56913c10..76acf4a72a8 100644 --- a/tests/gateway/test_feishu_approval_buttons.py +++ b/tests/gateway/test_feishu_approval_buttons.py @@ -448,6 +448,12 @@ class TestCardActionCallbackResponse: adapter = _make_adapter() adapter._loop = MagicMock() adapter._loop.is_closed = MagicMock(return_value=False) + adapter._allowed_group_users = {"ou_bob"} + adapter._approval_state[1] = { + "session_key": "sess-1", + "message_id": "msg-1", + "chat_id": "oc_12345", + } data = _make_card_action_data( {"hermes_action": "approve_once", "approval_id": 1}, open_id="ou_bob", @@ -469,6 +475,11 @@ class TestCardActionCallbackResponse: adapter = _make_adapter() adapter._loop = MagicMock() adapter._loop.is_closed = MagicMock(return_value=False) + adapter._approval_state[2] = { + "session_key": "sess-2", + "message_id": "msg-2", + "chat_id": "oc_12345", + } data = _make_card_action_data( {"hermes_action": "deny", "approval_id": 2}, ) @@ -510,6 +521,11 @@ class TestCardActionCallbackResponse: adapter = _make_adapter() adapter._loop = MagicMock() adapter._loop.is_closed = MagicMock(return_value=False) + adapter._approval_state[3] = { + "session_key": "sess-3", + "message_id": "msg-3", + "chat_id": "oc_12345", + } data = _make_card_action_data( {"hermes_action": "approve_session", "approval_id": 3}, open_id="ou_unknown", @@ -525,6 +541,11 @@ class TestCardActionCallbackResponse: adapter = _make_adapter() adapter._loop = MagicMock() adapter._loop.is_closed = MagicMock(return_value=False) + adapter._approval_state[4] = { + "session_key": "sess-4", + "message_id": "msg-4", + "chat_id": "oc_12345", + } data = _make_card_action_data( {"hermes_action": "approve_once", "approval_id": 4}, open_id="ou_expired", @@ -538,6 +559,51 @@ class TestCardActionCallbackResponse: assert "Old Name" not in card["elements"][0]["content"] assert "ou_expired" in card["elements"][0]["content"] + def test_rejects_approval_click_from_unauthorized_user(self, _patch_callback_card_types): + adapter = _make_adapter() + adapter._loop = MagicMock() + adapter._loop.is_closed = MagicMock(return_value=False) + adapter._allowed_group_users = {"ou_allowed"} + adapter._approval_state[5] = { + "session_key": "sess-5", + "message_id": "msg-5", + "chat_id": "oc_12345", + } + data = _make_card_action_data( + {"hermes_action": "approve_once", "approval_id": 5}, + open_id="ou_attacker", + ) + + with patch("asyncio.run_coroutine_threadsafe") as mock_submit: + response = adapter._on_card_action_trigger(data) + + assert response is not None + assert response.card is None + mock_submit.assert_not_called() + + def test_rejects_approval_click_when_callback_chat_mismatches(self, _patch_callback_card_types): + adapter = _make_adapter() + adapter._loop = MagicMock() + adapter._loop.is_closed = MagicMock(return_value=False) + adapter._allowed_group_users = {"ou_bob"} + adapter._approval_state[6] = { + "session_key": "sess-6", + "message_id": "msg-6", + "chat_id": "oc_expected", + } + data = _make_card_action_data( + {"hermes_action": "approve_once", "approval_id": 6}, + chat_id="oc_mismatch", + open_id="ou_bob", + ) + + with patch("asyncio.run_coroutine_threadsafe") as mock_submit: + response = adapter._on_card_action_trigger(data) + + assert response is not None + assert response.card is None + mock_submit.assert_not_called() + def test_returns_card_for_update_prompt_yes(self, _patch_callback_card_types): adapter = _make_adapter() adapter._loop = MagicMock()