hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 30 Packages Projects Releases Wiki Activity

fix(feishu): gate approval/update-prompt card clicks on operator allowlist, not group policy

Browse Source 
The synchronous card-action handlers and the update-prompt resolver
authorized clicks with _allow_group_message(), which answers "may this
sender chat in this group?" — with group_policy=open it returns True
for everyone. The approval resolver already used the correct operator
gate (_is_interactive_operator_authorized), so the three code paths
disagreed: with an open group policy an out-of-allowlist click on an
update-prompt card was fully executed, and approval clicks returned a
resolved-looking card before being rejected asynchronously.

Authorize all three paths with _is_interactive_operator_authorized(),
which checks membership of admins ∪ allowed_group_users (wildcard and
the empty pairing-mode allowlist keep their existing allow semantics,
matching _admit's DM pairing default). A missing operator identity now
fails closed on the update-prompt resolver instead of skipping the
check.

Fixes #96045
This commit is contained in:
liuhao1024
2026-08-31 09:44:13 +08:00
committed by Teknium
parent c29485fa64
commit a89706e4cb
2 changed files with 90 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
plugins/platforms/feishu/adapter.py
View File

@@ -2815,8 +2815,7 @@ class FeishuAdapter(BasePlatformAdapter):
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):
if not self._is_interactive_operator_authorized(open_id):
logger.warning("[Feishu] Unauthorized approval click by %s", open_id or "<unknown>")
return P2CardActionTriggerResponse() if P2CardActionTriggerResponse else None
@@ -2875,8 +2874,7 @@ class FeishuAdapter(BasePlatformAdapter):
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):
if not self._is_interactive_operator_authorized(open_id):
logger.warning("[Feishu] Unauthorized update prompt click by %s", open_id or "<unknown>")
return P2CardActionTriggerResponse() if P2CardActionTriggerResponse else None
@@ -2982,11 +2980,9 @@ class FeishuAdapter(BasePlatformAdapter):
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
if not self._is_interactive_operator_authorized(open_id):
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(

86
tests/gateway/test_feishu_approval_buttons.py
View File

@@ -378,6 +378,30 @@ class TestCardActionCallbackResponse:
assert response.card is None
mock_submit.assert_not_called()
def test_rejects_approval_click_when_group_policy_open(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._group_policy = "open"
adapter._default_group_policy = "open"
adapter._approval_state[6] = {
"session_key": "sess-6",
"message_id": "msg-6",
"chat_id": "oc_12345",
}
data = _make_card_action_data(
{"hermes_action": "approve_once", "approval_id": 6},
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_update_prompt_unauthorized_operator_returns_no_card(self, _patch_callback_card_types):
adapter = _make_adapter()
@@ -401,6 +425,30 @@ class TestCardActionCallbackResponse:
assert response.card is None
mock_submit.assert_not_called()
def test_update_prompt_unauthorized_click_rejected_when_group_policy_open(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._group_policy = "open"
adapter._default_group_policy = "open"
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
mock_submit.assert_not_called()
def test_update_prompt_chat_mismatch_returns_no_card(self, _patch_callback_card_types):
adapter = _make_adapter()
@@ -441,9 +489,45 @@ class TestResolveUpdatePrompt:
"chat_id": "oc_12345",
}
await adapter._resolve_update_prompt(1, "y", "Alice")
await adapter._resolve_update_prompt(1, "y", "Alice", open_id="ou_user1", chat_id="oc_12345")
assert (tmp_path / ".hermes" / ".update_response").read_text() == "y"
assert 1 not in adapter._update_prompt_state
@pytest.mark.asyncio
async def test_unauthorized_operator_does_not_write_response(self, tmp_path, monkeypatch):
adapter = _make_adapter()
monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes"))
(tmp_path / ".hermes").mkdir()
adapter._allowed_group_users = {"ou_allowed"}
adapter._group_policy = "open"
adapter._default_group_policy = "open"
adapter._update_prompt_state[2] = {
"session_key": "sess-up-2",
"message_id": "msg_up_004",
"chat_id": "oc_12345",
}
await adapter._resolve_update_prompt(2, "y", "Mallory", open_id="ou_intruder", chat_id="oc_12345")
assert not (tmp_path / ".hermes" / ".update_response").exists()
assert 2 in adapter._update_prompt_state
@pytest.mark.asyncio
async def test_missing_operator_identity_does_not_write_response(self, tmp_path, monkeypatch):
adapter = _make_adapter()
monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes"))
(tmp_path / ".hermes").mkdir()
adapter._allowed_group_users = {"ou_allowed"}
adapter._update_prompt_state[3] = {
"session_key": "sess-up-3",
"message_id": "msg_up_005",
"chat_id": "oc_12345",
}
await adapter._resolve_update_prompt(3, "y", "Anonymous", open_id="", chat_id="oc_12345")
assert not (tmp_path / ".hermes" / ".update_response").exists()
assert 3 in adapter._update_prompt_state