From 3fa15b33dd910699f18a0529f448f97eb8f02042 Mon Sep 17 00:00:00 2001 From: Dusk1e Date: Thu, 28 May 2026 15:07:46 +0300 Subject: [PATCH] fix(feishu): fail closed for update prompt card actions --- gateway/platforms/feishu.py | 58 +++++++++++++-- tests/gateway/test_feishu_approval_buttons.py | 72 +++++++++++++++++++ 2 files changed, 125 insertions(+), 5 deletions(-) diff --git a/gateway/platforms/feishu.py b/gateway/platforms/feishu.py index 70fb719a43a..b361ebc8cfc 100644 --- a/gateway/platforms/feishu.py +++ b/gateway/platforms/feishu.py @@ -2644,7 +2644,8 @@ class FeishuAdapter(BasePlatformAdapter): if prompt_id is None: logger.debug("[Feishu] Card action missing update_prompt_id, ignoring") return P2CardActionTriggerResponse() if P2CardActionTriggerResponse else None - if prompt_id not in self._update_prompt_state: + state = self._update_prompt_state.get(prompt_id) + if not state: logger.debug("[Feishu] Update prompt %s already resolved or unknown", prompt_id) return P2CardActionTriggerResponse() if P2CardActionTriggerResponse else None @@ -2655,12 +2656,33 @@ class FeishuAdapter(BasePlatformAdapter): operator = getattr(event, "operator", None) open_id = str(getattr(operator, "open_id", "") or "") - if not self._is_interactive_operator_authorized(open_id): + 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 update prompt 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] Update prompt callback chat mismatch for %s (expected=%s, got=%s)", + prompt_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_update_prompt(prompt_id, answer, user_name)): + if not self._submit_on_loop( + loop, + self._resolve_update_prompt( + prompt_id, + answer, + user_name, + open_id=open_id, + chat_id=callback_chat_id, + ), + ): return P2CardActionTriggerResponse() if P2CardActionTriggerResponse else None if P2CardActionTriggerResponse is None: @@ -2711,12 +2733,38 @@ class FeishuAdapter(BasePlatformAdapter): except Exception as exc: logger.error("Failed to resolve gateway approval from Feishu button: %s", exc) - async def _resolve_update_prompt(self, prompt_id: Any, answer: str, user_name: str) -> None: + async def _resolve_update_prompt( + self, + prompt_id: Any, + answer: str, + user_name: str, + *, + open_id: str = "", + chat_id: str = "", + ) -> None: """Persist an update prompt answer for the detached update process.""" - state = self._update_prompt_state.pop(prompt_id, None) + state = self._update_prompt_state.get(prompt_id) if not state: logger.debug("[Feishu] Update prompt %s already resolved or unknown", prompt_id) return + if open_id: + sender_id = SimpleNamespace(open_id=open_id, user_id="") + if not self._allow_group_message(sender_id, state.get("chat_id", ""), is_bot=False): + logger.warning("[Feishu] Unauthorized update prompt click by %s for prompt %s", open_id, prompt_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] Update prompt %s chat mismatch (expected=%s, got=%s)", + prompt_id, + expected_chat_id, + chat_id, + ) + return + state = self._update_prompt_state.pop(prompt_id, None) + if not state: + logger.debug("[Feishu] Update prompt %s already resolved while validating callback", prompt_id) + return try: self._write_update_prompt_response(answer) logger.info( diff --git a/tests/gateway/test_feishu_approval_buttons.py b/tests/gateway/test_feishu_approval_buttons.py index e739d47b087..999ac648d23 100644 --- a/tests/gateway/test_feishu_approval_buttons.py +++ b/tests/gateway/test_feishu_approval_buttons.py @@ -642,6 +642,7 @@ class TestCardActionCallbackResponse: adapter = _make_adapter() adapter._loop = MagicMock() adapter._loop.is_closed = MagicMock(return_value=False) + adapter._allowed_group_users = {"ou_bob"} adapter._update_prompt_state[1] = { "session_key": "sess-up-1", "message_id": "msg_up_003", @@ -667,6 +668,7 @@ class TestCardActionCallbackResponse: adapter = _make_adapter() adapter._loop = MagicMock() adapter._loop.is_closed = MagicMock(return_value=False) + adapter._allowed_group_users = {"ou_user1"} adapter._update_prompt_state[2] = { "session_key": "sess-up-2", "message_id": "msg_up_004", @@ -717,6 +719,7 @@ class TestCardActionCallbackResponse: adapter = _make_adapter() adapter._loop = MagicMock() adapter._loop.is_closed = MagicMock(return_value=False) + adapter._allowed_group_users = {"ou_user1"} adapter._update_prompt_state[1] = { "session_key": "sess-up-1", "message_id": "msg_up_005", @@ -754,6 +757,52 @@ class TestCardActionCallbackResponse: assert response.card is None mock_submit.assert_not_called() + def test_update_prompt_empty_allowlists_fail_closed(self, _patch_callback_card_types): + adapter = _make_adapter() + adapter._loop = MagicMock() + adapter._loop.is_closed = MagicMock(return_value=False) + adapter._update_prompt_state[7] = { + "session_key": "sess-up-7", + "message_id": "msg_up_007", + "chat_id": "oc_12345", + } + data = _make_card_action_data( + {"hermes_update_prompt_action": "y", "update_prompt_id": 7}, + open_id="ou_intruder", + ) + + 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 + assert 7 in adapter._update_prompt_state + mock_submit.assert_not_called() + + def test_update_prompt_chat_mismatch_returns_no_card(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._update_prompt_state[8] = { + "session_key": "sess-up-8", + "message_id": "msg_up_008", + "chat_id": "oc_expected", + } + data = _make_card_action_data( + {"hermes_update_prompt_action": "y", "update_prompt_id": 8}, + 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 + assert 8 in adapter._update_prompt_state + mock_submit.assert_not_called() + class TestResolveUpdatePrompt: """Test update prompt resolution persists the response file.""" @@ -800,3 +849,26 @@ class TestResolveUpdatePrompt: await adapter._resolve_update_prompt(99, "n", "Nobody") assert not (tmp_path / ".hermes" / ".update_response").exists() + + @pytest.mark.asyncio + async def test_chat_mismatch_does_not_write_response_file(self, tmp_path, monkeypatch): + adapter = _make_adapter() + adapter._allowed_group_users = {"ou_bob"} + monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes")) + (tmp_path / ".hermes").mkdir() + adapter._update_prompt_state[10] = { + "session_key": "sess-up-10", + "message_id": "msg_up_010", + "chat_id": "oc_expected", + } + + await adapter._resolve_update_prompt( + 10, + "y", + "Bob", + open_id="ou_bob", + chat_id="oc_wrong", + ) + + assert not (tmp_path / ".hermes" / ".update_response").exists() + assert 10 in adapter._update_prompt_state