Skip to content

fix(qqbot): authorize approval clicks on dm-spelled session keys - #98296

Closed
LanccBy2016 wants to merge 1 commit into
NousResearch:mainfrom
LanccBy2016:fix/qqbot-approval-dm-session-key
Closed

LanccBy2016 wants to merge 1 commit into
NousResearch:mainfrom
LanccBy2016:fix/qqbot-approval-dm-session-key

Conversation

@LanccBy2016

Copy link
Copy Markdown

Problem

On a 1:1 QQ C2C DM, every approval-button click is rejected as unauthorized, so the agent waits until its 5-minute approval timeout and multi-turn sessions appear frozen (response ready: time=1118.9s with repeated Rejected unauthorized approval click … (operator=…) warnings where operator is the session owner).

Root cause

Producer/consumer format drift on the session key:

  • Producer_on_c2c_message builds the gateway session source with build_source(chat_type="dm", …) (adapter.py:1327), so the key looks like agent:main:qqbot:dm:<user_openid>.
  • Consumer_is_authorized_interaction_for_session only matches the literal "c2c" (adapter.py:1134). "dm" falls through to return False.

The existing tests all hand-craft c2c-spelled keys (test_qqbot.py:922), so the drift is invisible to the suite.

Fix

Treat "dm" as a synonym of c2c in the authorization check. Both name the same private chat whose chat_id segment IS the owner's user_openid, so the operator == chat_id match still gates authorization — the relaxation adds no attack surface: a non-owner clicking the button still fails the check.

Tests

  • test_approval_click_on_dm_key_from_c2c_operator_resolves — regression reproducing the field bug (fails on current main, passes with the fix).
  • test_approval_click_on_dm_key_rejects_other_operator — asserts the cross-user rejection still holds on the dm spelling.
  • Full existing qqbot suites green: test_qqbot.py (65) + test_qqbot_credential_isolation.py / test_qqbot_scope_paths.py (19).

C2C DMs build their gateway session key with chat_type="dm" (build_source
in _on_c2c_message), but _is_authorized_interaction_for_session only
accepted the literal "c2c". Every approval-button click on a 1:1 QQ DM
was therefore rejected as unauthorized, the waiting agent stalled until
its 5-minute approval timeout, and multi-turn sessions appeared frozen.

Accept "dm" as a synonym of "c2c" — both name the same private chat whose
chat_id is the owner's user_openid, so the operator match still gates
authorization. Existing tests only ever exercised c2c-spelled keys, so
the producer/consumer format drift went unnoticed; add regression tests
for both the dm happy path and cross-user rejection.
@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 area/auth Authentication, OAuth, credential pools sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state duplicate This issue or pull request already exists labels Aug 30, 2026
@LanccBy2016

Copy link
Copy Markdown
Author

Heads-up on the cluster around this line: this is one of several open PRs proposing the same one-line relaxation for dm-spelled C2C keys — #48613, #67877, #76114, #81790, #82237 (and the broader routing-side family #40705 / issue #32528). Happy for the maintainer to pick whichever is most complete; this one ships both the positive regression (dm key + owner operator resolves) and the negative test (dm key + non-owner still rejected), verified against the live failure log (Rejected unauthorized approval click … operator == session owner, 5-min approval timeout, response time 1118s).

Also noting: this is orthogonal to #98294 (namespace slot main vs named-profile) — the two fixes address different segments of the same key and can merge independently.

@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Duplicate of #31593: both patches authorize dm-spelled QQ C2C session keys through the same owner-equality branch.

@LanccBy2016

Copy link
Copy Markdown
Author

Closing as directed by triage — deferring to the canonical #31593 for the dm-spelled key authorization. (Local fork branch will be deleted.)

@LanccBy2016
LanccBy2016 deleted the fix/qqbot-approval-dm-session-key branch August 30, 2026 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools 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:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants