fix(teams): route approval-card authorization through the shared gateway check

[shanthans-es]

What does this PR do?

Fixes Teams approval-card buttons (Allow Once / Allow Session / Always Allow / Deny) rejecting every clicker with "⛔ Not authorized" — a much narrower bug than it first looked like, and unrelated to invoke-handling itself (_on_card_action and the underlying adaptiveCard/action wiring are fully implemented and working).

The actual cause: _on_card_action() authorizes the click by comparing the clicker's raw AAD object ID / Teams conversation ID against TEAMS_ALLOWED_USERS. Every setup doc (and the interactive hermes setup teams prompt) has operators put an email/UPN in that variable. An email never equals the GUID Teams hands back on an invoke activity, so the comparison rejects every clicker unconditionally — including an operator whose plain DMs the bot already accepts fine via pairing-store approval, since regular messages go through a different, correct code path.

Slack's and Telegram's equivalent button handlers avoid this exact trap by delegating to GatewayRunner._is_user_authorized() (which checks the env allowlist UNION the pairing store, and normalizes identity correctly per platform). Teams' card-action handler was the one adapter with its own bespoke, untested identity comparison instead of using that shared path.

Related

Not a duplicate of #75800 (open) — that PR fixes a session-binding/IDOR issue in the same handler; this fixes authorization identity matching. Both touch _on_card_action() in plugins/platforms/teams/adapter.py and may want to be sequenced/rebased against each other, but they're independent bugs.

No existing issue filed for this one — found via source-level investigation of a "Teams approval buttons never work" report, not a live GitHub search hit. Happy to file one first if maintainers prefer that workflow.

Changes Made

• plugins/platforms/teams/adapter.py: replace _on_card_action's inline env-var/GUID comparison with a new _is_card_action_authorized() that builds a SessionSource via the adapter's existing build_source() and calls the shared GatewayRunner._is_user_authorized(), falling back to the old raw-ID comparison only when the runner isn't reachable (same default-deny posture as before in that fallback).
• tests/gateway/test_teams.py: adds TestTeamsCardActionAuthorization (4 tests) — no prior test coverage existed for _on_card_action at all.

How to Test

Sabotage check: reverted adapter.py against the new tests — test_allowed_via_shared_runner_despite_mismatched_env_identity fails on the original code (AssertionError: '⛔ Not authorized.' != '⛔ Not authorized.'), confirming it exercises the real bug. With the fix, all pass.

cd tests && python -m pytest gateway/test_teams.py -q
# 27 passed

Also ran tests/gateway/test_slack_approval_buttons.py and tests/gateway/test_config_driven_access_policy.py (the shared-authz surfaces this now shares code with) — no regressions, 100 passed total. ruff check clean on both changed files.

🤖 Generated with Claude Code

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

Overall: the right fix — routing card-action auth through the shared runner check makes approval buttons follow the same allowlist ∪ pairing-store semantics as plain messages, and the new _is_card_action_authorized mirrors the message path's user_id preference order (aad_object_id first at plugins/platforms/teams/adapter.py:1076-1078, matching the message path's :941), so pairing approvals recorded from DMs will actually match. The runner-first, env-fallback-default-deny ordering and tests are sound. Three points:

1. plugins/platforms/teams/adapter.py:1089 — chat_type="dm" is hardcoded even for group channel clicks. The message path derives it from conversation.conversation_type (personal → dm, groupChat → group, channel → channel, :927-936), and the invoke activity's conversation carries the same field. If GatewayRunner._is_user_authorized applies any chat-type-scoped policy (group policy, owner-only-in-groups), a button click in a Teams channel or group chat is now evaluated as a DM — a clicker whom the group policy would reject as a message sender can still approve dangerous commands by clicking the card in that channel. Derive chat_type from conversation.conversation_type exactly as the message path does, defaulting to dm only when absent.

2. plugins/platforms/teams/adapter.py:1107-1108 — the env fallback reads raw os.environ, breaking the multiplex fail-closed rule. The repo's profile-safety contract is explicit: authorization config (*_ALLOWED_USERS, *_ALLOW_ALL_USERS) must be read via the scope-aware helper — under gateway.multiplex_profiles, os.environ holds the default profile's values, so a secondary-profile Teams bot would accept clicks against the wrong profile's allowlist (or an ALLOW_ALL it never set). This adapter already ships the canonical helper (_get_scoped_secret, :100-117); use it for both fallback reads. (The pattern existed before this PR, but the repo rule is to fix it when touching the site.)

3. plugins/platforms/teams/adapter.py:1082 — __self__ introspection is a brittle runner back-channel. If the handler wiring ever gains a functools.partial/decorator wrapper, getattr(handler, "__self__", None) silently yields None and every click falls back to env-only auth — a confusing default-deny nobody will connect to a wiring refactor. Prefer an explicit stash (e.g. the runner assignment sets adapter._gateway_runner) with this as back-compat, or at minimum a logger.warning (not debug) when the runner is unexpectedly missing.

Minor: test_denied_when_shared_runner_rejects asserts the runner's verdict overrides even a literal allowlist match — good — but add the inverse group case from finding 1 (a groupChat invocation builds chat_type="group") once the fix lands.

[shanthans-es]

Thanks for the review — pushed 2c18647 addressing points 1 and 2:

1. Fixed. Factored the message path's conversation_type → chat_type mapping into a shared _chat_type_for_conversation() helper and reused it in _is_card_action_authorized() instead of the hardcoded "dm". Added a regression test asserting a groupChat click authorizes with chat_type == "group" — confirmed it fails against the pre-fix hardcoded value.

2. Fixed. Both fallback reads (TEAMS_ALLOWED_USERS, TEAMS_ALLOW_ALL_USERS) now go through the adapter's existing _get_scoped_secret() instead of raw os.getenv. Added a regression test that stubs the scoped reader and confirms it's consulted.

Also bumped the "falling back to env-only auth" logging from debug to warning per your note, including a new warning for the previously-silent no-runner-reachable case.

3. Left as-is, with reasoning: the __self__ introspection is the same pattern Slack (plugins/platforms/slack/adapter.py::_is_interactive_user_authorized) and Telegram already use to reach the gateway runner from an adapter — there's no existing _gateway_runner-style stash anywhere in the codebase to mirror, so introducing one here would make Teams inconsistent with its siblings rather than more consistent, and would mean touching the shared set_message_handler wiring in gateway/platforms/base.py for a hypothetical future refactor risk rather than a live bug. Happy to revisit if you'd rather standardize this across all three adapters in a follow-up.

29/29 relevant tests pass (tests/gateway/test_teams.py), plus test_slack_approval_buttons.py + test_config_driven_access_policy.py for regressions on the now-shared code path (102 passed total). ruff check clean.

[shanthans-es]

Bumping this — hit the exact bug again today (2026-09-20) in production: a Teams approval card for another agent's tirith security-scan flag came through, and none of Allow Once/Allow Session/Deny registered a click, consistent with the auth-check bug this PR fixes. Would appreciate a maintainer look when you get a chance.