fix(gateway): route Discord/Yuanbao's early pairing admission gate to the adapter's own multiplex profile - #74860
Conversation
|
Thanks for tracing this through the pre-gateway admission path. The premise is confirmed on current main: Discord's early check uses a bare store at Problems
Suggested changes
|
… the adapter's own multiplex profile DiscordAdapter._is_pairing_approved_user() (called from _is_allowed_user, the on_message admission gate) and YuanbaoAdapter._sender_may_designate_home()'s dm_policy=="pairing" branch both construct a bare PairingStore() — always the process-global/default-profile store. In a multiplex gateway, a secondary profile is served by its own adapter instance (gateway/run.py::_configure_profile_adapter wires message handlers, session store, etc. per profile), and its own DM-pairing whitelist lives in a separate, profile-scoped PairingStore(profile=name) (gateway/run.py already builds one per served profile into self.pairing_stores, and authz_mixin._pairing_store_for(source) already resolves it correctly downstream). These two adapter-level checks run before that downstream, correctly- scoped check — Discord's fires inside the on_message admission gate, which drops the message entirely (return False, False) on a False result; Yuanbao's gates whether an intake-only DM sender may even designate a home channel. Both consulted the wrong (global) whitelist, so a user approved only on the routed secondary profile's pairing store was rejected here before the message ever reached the profile-aware gateway layer. Fix: stamp _own_profile on every adapter instance alongside the existing set_authorization_check() wiring (gateway/run.py::_configure_profile_adapter, None for the active/default profile's adapters — same as _make_adapter_auth_check's own profile_name parameter), and have both checks construct PairingStore(profile=self._own_profile) when set, falling back to the global PairingStore() otherwise (preserving existing default-profile behavior exactly). Discord's component/button-click authorization (_component_check_auth) has the identical bare-PairingStore() pattern but is a module-level function with 6 call sites across several discord.ui.View subclasses that don't currently carry adapter/profile context — left out of scope here as a larger, separate refactor. Added a runner-level regression test proving GatewayRunner's real _configure_profile_adapter() wiring path (not a manually-assigned test double) actually stamps adapter._own_profile with the profile name — the existing direct adapter tests already covered the PairingStore selection logic but assigned _own_profile by hand, so they never exercised the production wiring itself.
170ac27 to
9b9a8a2
Compare
|
Rebased onto current `upstream/main` and added the requested runner-level regression test per your review. The existing direct-adapter tests (`_make_adapter(own_profile="coder")`) manually assigned `_own_profile`, so they verified the `PairingStore` selection logic but never exercised the real production wiring. Added `test_configure_profile_adapter_stamps_own_profile`: constructs a minimal `GatewayRunner` and calls its real `_configure_profile_adapter(adapter, "coder", Platform.DISCORD)`, then asserts `adapter._own_profile == "coder"` — proving the actual startup/reconnect wiring path (not a test double) does the stamping. Mutation-verified: temporarily removing the `adapter._own_profile = profile_name` line breaks it with `AttributeError`. Rebase hit a real, orthogonal conflict in `_configure_profile_adapter` — `main` added independent per-profile busy-text-mode resolution (`_busy_text_modes_by_profile`) to the same function body since this PR was opened. Merged both additions; kept both. Full `tests/gateway/test_discord_pairing_profile_scope.py` + `tests/test_yuanbao_pipeline.py` (77 tests) passes. Broader discord/yuanbao sweep (`tests/gateway/ -k "discord or yuanbao"`) has 4 pre-existing, unrelated failures in `test_discord_send.py`/`test_send_multiple_images.py` — confirmed identical with this PR's changes fully stashed against clean `upstream/main` (test-order pollution, not a regression from this change). Ruff clean. Overlap disclosure: open PR #76166 ("isolate multiplex access policy per profile," security-labeled, fixes #72348) is a much broader rewrite of Discord's multiplex authorization surface — adapter-local policy dataclass, session persistence, 6 interactive view producers, voice callbacks — that very likely supersedes this PR's narrower Discord half if it merges first. This PR's Yuanbao half is untouched by #76166. Also, open PR #69156 stamps a different attribute (`adapter.profile_name`, for Telegram's busy/approval checks) at the top of the same `_configure_profile_adapter` body this PR's `adapter._own_profile` stamp sits near the bottom of — textual proximity only, no semantic conflict. Squashed to a single commit on top of current `upstream/main`. |
What does this PR do?
DiscordAdapter._is_pairing_approved_user()(called from_is_allowed_user, theon_messageadmission gate) andYuanbaoAdapter._sender_may_designate_home()'sdm_policy == "pairing"branch both construct a barePairingStore()— always the process-global/default-profile store.In a multiplex gateway, a secondary profile is served by its own adapter instance (
gateway/run.py::_configure_profile_adapterwires the message handler, session store, etc. per profile), and its own DM-pairing whitelist lives in a separate, profile-scopedPairingStore(profile=name)—gateway/run.pyalready builds one per served profile intoself.pairing_stores, andauthz_mixin._pairing_store_for(source)already resolves it correctly for the downstream authorization check.The problem is these two adapter-level checks run before that downstream, correctly-scoped check ever sees the message:
on_messageadmission gate (_is_allowed_user), which drops the message entirely (return False, False) on aFalseresult — the comment right above this check even says it exists specifically "so normal guild/DM text messages do not get dropped at the adapter before the pairing-aware gateway layer can see them", but the store it consults isn't the pairing-aware layer's store._sender_may_designate_home).Net effect: a user approved only on a routed secondary profile's pairing store (
hermes -p coder pairing approve discord <id>) is silently rejected by these checks before the message ever reaches the profile-aware gateway layer — even though the downstream check would have admitted them.Related Issue
No existing issue — found while re-auditing the multiplex
PairingStore(profile=name)machinery (gateway/run.py:12502) for consumers still constructing the bare/global store, after confirmingauthz_mixin._pairing_store_foralready resolves per-profile correctly for the primary authorization path.Type of Change
Changes Made
gateway/platforms/base.py: addself._own_profile: Optional[str] = NonetoBasePlatformAdapter.__init__, documented as the multiplex profile the adapter instance serves.gateway/run.py::_configure_profile_adapter: stampadapter._own_profile = profile_namealongside the existingset_authorization_check(...)wiring (mirrors that call's ownprofile_nameparameter —Nonefor the active/default profile's adapters, the actual name for secondary-profile adapters).plugins/platforms/discord/adapter.py::_is_pairing_approved_user: constructPairingStore(profile=self._own_profile)when set, else the globalPairingStore()(unchanged default-profile behavior).gateway/platforms/yuanbao.py::_sender_may_designate_home: same fix for itsdm_policy == "pairing"branch.tests/gateway/test_discord_pairing_profile_scope.py(4 tests: default-profile behavior preserved, secondary-profile routes to its own store, the actual regression scenario, and an end-to-end_is_allowed_usercheck) + 2 new tests intests/test_yuanbao_pipeline.py::TestSenderMayDesignateHome.Out of scope: Discord's component/button-click authorization (
_component_check_auth) has the identical bare-PairingStore()pattern, but it's a module-level function with 6 call sites across severaldiscord.ui.Viewsubclasses that don't currently carry adapter/profile context — threading that through cleanly is a larger, separate refactor and is left for a follow-up.How to Test
Mutation-verified: stashed the four production-file changes (keeping the new tests) and confirmed the 4 profile-routing-specific tests fail against pre-fix code (
PairingStore()called with no args instead ofprofile="coder"), while the pre-existing default-profile tests still pass unchanged. Restored the fix and re-ran — all green. Also ran the broader neighboring suites:test_discord_connect.py,test_discord_component_auth.py,test_discord_roles_dm_scope.py,test_discord_missed_message_backfill.py,test_discord_slash_auth.py,test_multiplex_pairing_stores.py,test_pairing.py(gateway + hermes_cli),test_platform_base.py, fulltest_yuanbao_pipeline.py— 170+ tests, all pass. Ruff clean on all six changed files.Checklist
_is_pairing_approved_user; fix(gateway): fail-closed external-surface defaults + profile-aware multiplex authz #56273, merged, touches other Discord authz code but not this function) don't overlap