Skip to content

fix(qqbot): authorize dm chat_type in interaction approval - #32752

Open
ky256 wants to merge 1 commit into
NousResearch:mainfrom
ky256:fix/qqbot-dm-chat-type-authorization
Open

fix(qqbot): authorize dm chat_type in interaction approval#32752
ky256 wants to merge 1 commit into
NousResearch:mainfrom
ky256:fix/qqbot-dm-chat-type-authorization

Conversation

@ky256

@ky256 ky256 commented May 26, 2026

Copy link
Copy Markdown

Problem

Approval / update button clicks from QQ private chats are unconditionally rejected with:

WARNING [QQBot:<app_id>] Rejected unauthorized approval click for session
   agent:main:qqbot:dm:<openid> (operator=<openid>)

…even though the operator's openid matches the session owner. As a result, any tool requiring user approval (sensitive commands, MCP confirmations, etc.) blocks the agent indefinitely on the QQBot platform.

Root cause

QQBot DM session keys are constructed as agent:main:qqbot:dm:<openid>, so _parse_gateway_session_key returns chat_type == "dm". But _is_authorized_interaction_for_session only accepts the literal c2c:

if chat_type == "c2c":
    return bool(chat_id) and operator == chat_id

DMs fall through to the final return False and the click is rejected.

Fix

Accept both c2c (legacy literal) and dm (what the SessionDB actually emits for QQ private chats).

- if chat_type == "c2c":
+ if chat_type in ("c2c", "dm"):
      return bool(chat_id) and operator == chat_id

Tests

Adds TestIsAuthorizedInteractionForSession with 11 cases:

  • c2c / dm match grants
  • c2c / dm mismatch denies
  • group match grants
  • group chat/user mismatch denies
  • unknown chat_type denies
  • empty operator denies
  • wrong platform denies
  • malformed session_key denies

All 170 tests in tests/gateway/test_qqbot.py pass after the fix; the new test_dm_match_grants reliably fails against main (verified by reverting the one-line change locally), so it is real regression coverage rather than tautology.

Reproduction

Reproduced live in a private QQBot deployment: prior to the fix, every approval button click in a QQ DM produced the unauthorized warning and the tool call hung. After the fix, the same click logged:

INFO [QQBot:<app_id>] Button resolved 1 approval(s) for session
   agent:main:qqbot:dm:<openid> (choice=always, operator=<openid>)

…and the agent continued normally.

Made with Cursor

QQBot DM session keys are constructed with chat_type="dm" (see
SessionDB session_key), but _is_authorized_interaction_for_session
matched only the literal "c2c".

Every approval / update button click in QQ private chat was therefore
rejected with "Rejected unauthorized approval click", and any tool
requiring user approval would block forever.

Accept both literals and add regression tests covering c2c, dm,
group/guild, malformed keys, and platform mismatches.

Co-authored-by: Cursor <cursoragent@cursor.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists platform/qqbot QQ Bot adapter comp/gateway Gateway runner, session dispatch, delivery duplicate This issue or pull request already exists labels May 26, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #31593 which applies the same one-line fix (accepting 'dm' chat_type alongside 'c2c' in _is_authorized_interaction_for_session). Both fix #32528.

@Bartok9

Bartok9 commented May 29, 2026

Copy link
Copy Markdown
Contributor

Cross-reference: this PR also resolves #34432 (QQ Bot approval button clicks rejected for C2C/private messages).

Verified against current origin/main (86a389fee): the "dm" branch is still absent from _is_authorized_interaction_for_session (adapter.py:1082-1094) — only c2c, group, and guild are handled, so DM clicks fall through to return False.

One clarification on #34432's stated root cause: that issue claims "dm" occupies the chat_id slot (parts[4]) with the openid in parts[5]. That's not what _handle_c2c_message actually builds — it calls build_source(chat_id=user_openid, user_id=user_openid, chat_type="dm") (adapter.py:1271-1277), producing agent:main:qqbot:dm:<openid> (5 parts). So chat_id == operator == <openid> and chat_type == "dm". The real defect is exactly what this PR fixes: chat_type == "c2c" never matches "dm", so operator == chat_id is never evaluated for private chats.

This PR's chat_type in ("c2c", "dm") change is the correct fix and the test coverage (c2c/dm match + mismatch) is on point. LGTM.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused QQBot regression fix. Current main still constructs C2C/private sources with chat_type="dm" at gateway/platforms/qqbot/adapter.py:1293-1297, while _is_authorized_interaction_for_session accepts only "c2c" at gateway/platforms/qqbot/adapter.py:1101-1113. Approval dispatch uses that guard at gateway/platforms/qqbot/adapter.py:1148, so a matching DM owner is currently denied.

The proposed ("c2c", "dm") condition preserves the existing owner comparison, and the added matching/mismatching DM tests cover the regression directly. This is an automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data label Jul 13, 2026
@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 13, 2026
@alt-glitch alt-glitch removed the sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data label Jul 13, 2026
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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants