Feature/yuanbao group bot owner command - #29077
Conversation
|
The author is a developer from the Yuanbao team. This PR performs strict permission verification on custom slash commands, enhancing security. @alt-glitch @kovyrin @jbarket @valda |
… gating - Add QueryBotInfoReq/Rsp encode/decode in yuanbao_proto.py - Query and cache bot owner info via QueryBotInfoReq after WS auth-bind - Refactor OwnerCommandMiddleware: stricter owner validation, GROUP_ALLOWLIST - Allow owner commands to bypass group queue for concurrent dispatch - Reject non-owner slash commands and non-allowlisted commands in group chat - Require @bot mention for slash commands in group chat
aba6880 to
e34239d
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the Yuanbao security hardening work. I verified that the underlying gap still exists on current main: Yuanbao owner commands still depend on push.bot_owner_id at gateway/platforms/yuanbao.py:2041, and current main has no QueryBotInfo support.
Problems
gateway/platforms/yuanbao.py:1953in this PR duplicates the sameis_gateway_known_commandand group@botchecks already added atgateway/platforms/yuanbao.py:1944-1950.gateway/platforms/yuanbao.py:1930prefers cachedadapter.bot_infowhenever present; if the cachedowner_idis empty, it skips the existingpush.bot_owner_idfallback and valid owner commands can fail.gateway/platforms/yuanbao.py:2889calls_query_bot_info()only during initialopen(). I did not find a reconnect-path refresh, despite the PR body saying cached owner info refreshes after reconnect.gateway/platforms/yuanbao.py:4604says_bot_inforesets on disconnect and is re-fetched on every reconnect, but the PR head search only found initialization,_query_bot_info(), and the accessor.- No tests were added for the new owner-command branches or
QueryBotInfoRspdecoding.
Suggested changes
- Remove the duplicate gate block.
- Fall back to
push.bot_owner_idwhen cached bot info is missing an owner id. - Clear/refresh
_bot_infoon reconnect/disconnect, or adjust the lifecycle comments. - Add targeted tests for the new group command gate and protocol decoder.
This is an automated hermes-sweeper review.
| def _is_owner(adapter, push: dict, from_account: str) -> bool: | ||
| """Check whether from_account is the bot owner.""" | ||
| bot_info = adapter.bot_info if adapter else None | ||
| owner_id = ( |
There was a problem hiding this comment.
If adapter.bot_info exists but its owner_id is empty, this skips the existing push.bot_owner_id fallback and can reject the real owner. Consider falling back whenever the cached owner id is blank.
| await next_fn() | ||
| return | ||
|
|
||
| from hermes_cli.commands import is_gateway_known_command |
There was a problem hiding this comment.
This repeats the same is_gateway_known_command import/check and group @bot gate from the block just above; one copy should be removed.
|
|
||
| YuanbaoAdapter.set_active(adapter) | ||
|
|
||
| await self._query_bot_info() |
There was a problem hiding this comment.
This refreshes bot info on initial connect only. The reconnect path also needs the refresh if the cache is meant to survive connection churn safely.
|
|
||
| # Bot info cache: populated once after WS connect by _query_bot_info(). | ||
| # Contains {"bot_id": str, "owner_id": str} from QueryBotInfoRsp. | ||
| # Reset to None on disconnect; re-fetched on every reconnect. |
There was a problem hiding this comment.
The comment says _bot_info is reset on disconnect and re-fetched on every reconnect, but this patch does not implement that lifecycle. Either add the reset/refresh or narrow the comment.
Summary
Adds stricter owner-command handling for Yuanbao group chats.
What changed
QueryBotInfoReq/QueryBotInfoRspprotocol support ingateway/platforms/yuanbao_proto.pybot_infoaccessor onYuanbaoAdapterOwnerCommandMiddlewarecommand parsing and owner validation flowGROUP_ALLOWLISThandling for group-chat slash commands@mentioned/command)This improves safety and predictability for privileged commands such as
/approve,/deny,/stop, and/resetin Yuanbao group chats while preserving normal mention-based interaction behavior.Testing
Tested manually with Yuanbao WebSocket integration.
Verified:
@Botmention@Botno longer trigger slash-command responses unexpectedlyPlatforms Tested
Related
Closes #29065