Skip to content

fix(qqbot): accept dm chat_type in approval authorization - #76114

Open
zhangyu921 wants to merge 1 commit into
NousResearch:mainfrom
zhangyu921:fix/qqbot-approval-dm
Open

zhangyu921 wants to merge 1 commit into
NousResearch:mainfrom
zhangyu921:fix/qqbot-approval-dm

Conversation

@zhangyu921

Copy link
Copy Markdown

Problem

QQ Bot DM (C2C) sessions build their gateway session key with chat_type="dm" (see build_session_key in gateway/session.py, which uses chat_type="dm" for DMs). But _is_authorized_interaction_for_session in the qqbot adapter only accepted c2c / group / guild.

Result: every approval-button click in a QQ private chat falls through to return False and is logged as:

Rejected unauthorized approval click for session agent:main:qqbot:dm:<openid> (operator=<same openid>)

Command approval via inline keyboard is completely unusable in QQ DMs. Group/guild approvals are unaffected.

Fix

Treat dm like c2c — both are 1:1 user chats where the operator's openid must equal the chat id. Group/guild logic unchanged.

Verification

  • dm + owner → authorized (was rejected before)
  • dm + attacker → still rejected (no permission loosened)
  • c2c + owner → authorized (regression pass)
  • group + other user → still rejected (regression pass)
  • Live-tested on QQ: with the fix, the approval button resolves the pending command; before it, every click was rejected.

QQ Bot DM (C2C) sessions build their gateway session key with
chat_type="dm" (build_session_key in gateway/session.py), but
_is_authorized_interaction_for_session in the qqbot adapter only
accepted "c2c"/"group"/"guild". DM approval button clicks therefore
fell through to return False and were rejected as unauthorized, making
command approval via inline keyboard unusable in QQ private chats.

Treat "dm" like "c2c" (both are 1:1 user chats) while leaving the
group/guild checks unchanged.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/qqbot QQ Bot adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages duplicate This issue or pull request already exists labels Aug 1, 2026
@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Duplicate of #31593: both patches change the same QQBot approval authorization allow-list to accept chat_type="dm" for DM session keys. #31593 is the earlier open canonical PR and includes focused regression coverage.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused QQBot authorization fix. Current main still has the reported mismatch: QQ C2C intake creates chat_type="dm" sources at gateway/platforms/qqbot/adapter.py:1293-1299, and build_session_key() serializes those as ...:dm:<chat_id> at gateway/session.py:1074-1086. The existing authorization branch accepts only c2c at gateway/platforms/qqbot/adapter.py:1105, then rejects other types at line 1115.

Problems

  • This PR has no regression test. Existing dispatch coverage tests a C2C owner path at tests/gateway/test_qqbot.py:853 and an unauthorized group path at line 884, but not the DM-shaped key this change enables.

Suggested changes

  • Add the focused DM approval-dispatch test already represented by the earlier canonical PR #31593: a matching operator for agent:main:qqbot:dm:u-42 should resolve the approval.

Automated hermes-sweeper review.

chat_type = parsed.get("chat_type", "")
chat_id = parsed.get("chat_id", "")
if chat_type == "c2c":
if chat_type in {"c2c", "dm"}:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a dispatch-level regression test for a matching C2C operator clicking approve:agent:main:qqbot:dm:<openid>:allow-once; the current tests cover the analogous c2c key but not this newly accepted dm form.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Aug 1, 2026
@alt-glitch alt-glitch mentioned this pull request Aug 1, 2026
23 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists platform/qqbot QQ Bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants