Skip to content

fix(qqbot): authorize approval buttons for named-profile session keys - #98294

Open
LovePlayCode wants to merge 1 commit into
NousResearch:mainfrom
LovePlayCode:fix/qqbot-profile-approval-authz
Open

fix(qqbot): authorize approval buttons for named-profile session keys#98294
LovePlayCode wants to merge 1 commit into
NousResearch:mainfrom
LovePlayCode:fix/qqbot-profile-approval-authz

Conversation

@LovePlayCode

Copy link
Copy Markdown
Contributor

What does this PR do?

In a named profile (gateway multiplexing), every QQBot approval-button click was rejected as unauthorized and the pending approval timed out fail-closed (300s → command BLOCKED).

Root cause: _parse_gateway_session_key() (introduced with the owner-authorization check in #30737) required the literal main in the namespace slot of agent:<ns>:<platform>:<chat_type>:<chat_id>, but _session_key_namespace() (gateway/session.py) puts the multiplex profile name there for named profiles — a design that is explicitly documented as position-compatible for downstream parsers. So agent:coder:qqbot:dm:<openid> failed to parse, _is_authorized_interaction_for_session() returned False, and the click was dropped with Rejected unauthorized approval click.

This is not the dm/c2c chat_type family (#32528 & the ~25 PRs addressing it, including the broadest #40705) — those fix a different slot of the same key. This PR fixes the namespace slot; the two changes are orthogonal and can merge independently. Telegram / Slack / Teams already have profile-aware authorization fixes in flight (#65589 / #72657 / #93516); this is the QQBot counterpart.

The fix: drop the parts[1] != "main" assertion, keeping a non-empty check so malformed keys (agent::qqbot:...) are still rejected. The namespace slot takes no part in any authorization decision — the platform == "qqbot", operator == chat_id (c2c) and operator == session_user (group/guild) checks are what authorize, and they are unchanged and pinned by new negative tests.

Related Issue

Fixes #98292

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • gateway/platforms/qqbot/adapter.py_parse_gateway_session_key(): accept any non-empty namespace (multiplex profile) instead of requiring the literal main; docstring updated to document the namespace slot
  • tests/gateway/test_qqbot.py — new TestProfileNamespaceApprovalAuthz (6 tests): named-profile keys parse; default agent:main:... keys unchanged (no regression); malformed keys still rejected; end-to-end c2c/group approval clicks on named-profile keys resolve via resolve_gateway_approval; a wrong-operator click on a named-profile key is still rejected (the relaxation does not weaken the operator check)

How to Test

  1. uv run --extra dev pytest tests/gateway/test_qqbot.py -q69 passed (63 before + 6 new)
  2. Mutation check (fix reverted via git stash push gateway/platforms/qqbot/adapter.py):
    uv run --extra dev pytest tests/gateway/test_qqbot.py::TestProfileNamespaceApprovalAuthz -q3 failed (the positive named-profile cases fail, reproducing the production log line Rejected unauthorized approval click for session agent:coder:qqbot:group:g-1:owner); restored → 6 passed
  3. uv run --extra dev pytest tests/gateway/test_qqbot_credential_isolation.py tests/gateway/test_qqbot_scope_paths.py -q19 passed
  4. Minimal repro of the bug on main:
    from gateway.platforms.qqbot.adapter import QQAdapter
    QQAdapter._parse_gateway_session_key("agent:coder:qqbot:dm:u-1")  # → None (bug); parses after this PR

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass (targeted suites above; full-suite run pending CI)
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15 (arm64)

Documentation & Housekeeping

  • N/A — no config keys, no tool behavior, no architecture change (one parser assertion + tests)

Screenshots / Logs

Mutation-check run with the fix reverted reproduces the reported failure mode:

WARNING  gateway.platforms.qqbot.adapter:adapter.py:1180 [QQBot:a] Rejected unauthorized approval click for session agent:coder:qqbot:group:g-1:owner (operator=owner)

@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 labels Aug 30, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Overall: Minimal fix for named-profile approval authorization.

What it does

  • gateway/platforms/qqbot/adapter.py:_parse_gateway_session_key relaxes namespace check from parts[1] != "main" to not parts[1], accepting any non-empty profile name while keeping position of later slots. Docstring now references gateway.session._session_key_namespace.
  • Tests add coverage for named-profile c2c/group keys and verify wrong-operator still rejected.

Non-blocking notes

  • Validation is not parts[1] only — empty string rejected, but whitespace-only or unusual profile names would pass; acceptable since namespace is opaque and operator check is the real authz gate.
  • Consider asserting that profile namespace is not confused with platform slot in future — current slot-position test covers it.

No functional issues found.

Non-blocking — please use your judgment.

`_parse_gateway_session_key()` required the literal `main` in the
namespace slot of `agent:<ns>:<platform>:<chat_type>:<chat_id>`, but
`_session_key_namespace()` (gateway/session.py) puts the multiplex
profile name there for named profiles. Every approval button click in a
named-profile QQ session therefore failed authorization ("Rejected
unauthorized approval click") and the pending approval timed out
fail-closed.

The namespace slot takes no part in any authorization decision, so the
parser now accepts any non-empty namespace; the platform and operator
checks are unchanged and pinned by new negative tests.

Fixes NousResearch#98292
@LovePlayCode
LovePlayCode force-pushed the fix/qqbot-profile-approval-authz branch from 175b96f to 8e0f02e Compare August 31, 2026 07:57
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 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.

[Bug]: qqbot approval buttons rejected as unauthorized in named-profile sessions (session-key namespace vs "main")

3 participants