mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-05 07:41:39 +00:00
fix(feishu): authorize interactive exec approval callbacks (#30739)
This commit is contained in:
parent
be27bfed01
commit
485292ac7d
2 changed files with 86 additions and 0 deletions
|
|
@ -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 "<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] 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)):
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue