mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-09 08:21:50 +00:00
fix(feishu): fail closed for update prompt card actions
This commit is contained in:
parent
410cb743bf
commit
3fa15b33dd
2 changed files with 125 additions and 5 deletions
|
|
@ -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 "<unknown>")
|
||||
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(
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue