fix(gateway): secondary-profile allowlists work and allow-all no longer leaks across profiles (#93522, salvage #93545 + #93605) - #93639
Conversation
…nbao/wecom) WEIXIN_DM_POLICY/ALLOWED_USERS/GROUP_ALLOWED_USERS, YUANBAO's equivalents, WECOM_DM_POLICY/ALLOWED_USERS/GROUP_POLICY, and the startup guard's GATEWAY_ALLOW_ALL_USERS check still read raw os.getenv at adapter construction time. Under gateway.multiplex_profiles that reads the process env instead of the per-profile secret scope, so a secondary profile either silently drops every DM (its own env-only allowlist is invisible) or inherits the default profile's allow-all/allowlist config. Route these reads through the existing scoped helpers (_wx_secret, _get_scoped_secret, gateway.authz_mixin._platform_gate_env, and gateway.config._getenv) already used for the adjacent credential reads in the same adapters. Fixes #93522.
…secret scope Under gateway.multiplex_profiles, secondary profiles are constructed inside _profile_runtime_scope and their .env lives in the profile's secret scope - gateway/run.py explicitly does NOT mutate os.environ with it. Four adapters still read their AUTHORIZATION config via raw os.getenv, so every secondary profile either (a) silently missed its own env-only allowlists/policies (fail-closed: all DMs dropped at intake) or (b) inherited the default profile's GATEWAY_ALLOW_ALL_USERS=true / allowlists from the shared process env (fail-open admissions): - weixin.py: WEIXIN_DM_POLICY / WEIXIN_ALLOWED_USERS / WEIXIN_GROUP_ALLOWED_USERS / WEIXIN_ALLOW_ALL_USERS + GATEWAY_ALLOW_ALL_USERS in _open_dm_opted_in - yuanbao.py: YUANBAO_DM_POLICY / DM_ALLOW_FROM / GROUP_POLICY / GROUP_ALLOW_FROM / ALLOW_ALL_USERS (new _yb_secret helper; AccessPolicy hard-gates intake) - signal.py: SIGNAL_GROUP_ALLOWED_USERS / SIGNAL_ALLOWED_USERS (new _sig_secret helper; empty scoped group list previously meant "drop all groups" silently) - wecom/adapter.py: WECOM_DM_POLICY / WECOM_ALLOWED_USERS / WECOM_GROUP_POLICY / WECOM_ALLOW_ALL_USERS + GATEWAY_ALLOW_ALL_USERS - while credentials one line above already used _get_scoped_secret - gateway/run.py::_own_policy_open_startup_violation: the open-policy startup guard validated GATEWAY_ALLOW_ALL_USERS via raw os.getenv even though its sibling dm/group reads already used the scoped _getenv All reads now go through the canonical fail-closed scoped shape QQ's _resolve_qq_secret already used (scope hit wins; unscoped single-profile callers keep legacy os.environ behavior). Regression suite drives the real scope contextvar across all four helpers plus the admission gates and the startup guard, asserting both directions: profile values are visible under multiplex, default-profile values never leak. Fixes #93522
andrexibiza
left a comment
There was a problem hiding this comment.
Reviewed exact head 7cb62237ee2656c3ca2bb91fd2b8ea2c881006df against current main a0ca7c19204e514f9590ce3b812e029b315ab9e9 (merge base 7e67f64fcee9340f40a1c6f912fc650aa4984510). Exact-head CI 32702203904, Docker 32702203281, and Nix 32702203232 are all green. The current-main delta since the merge base is in cron/dashboard/console surfaces, not these gateway authz files, so I do not see a composition collision there.
The Weixin/Yuanbao/WeCom/startup-guard changes close the publication gap left on #93545, and the Signal helper does remove the original cross-profile os.environ borrowing. There is still one security blocker on the Signal side, though:
Blocker — a scoped SIGNAL_ALLOWED_USERS miss still becomes wildcard authority before the runner auth gate. In gateway/platforms/signal.py, the constructor now does _sig_secret("SIGNAL_ALLOWED_USERS", "*"). With multiplexing active and a secondary profile scope installed but no SIGNAL_ALLOWED_USERS entry, the scoped read returns the supplied default, so self.dm_allow_from becomes {"*"}. The existing reaction gate treats "*" as authorized, and that reaction path intentionally runs before run.py authorization. Result: profile B no longer inherits profile A's allowlist, but B can still emit the pre-auth 👀 reaction to an arbitrary DM even though B never opted into open access. That is the other side of the same authority boundary: not borrowing another profile is insufficient if absence itself is promoted to wildcard authority.
The regression suite currently cannot catch this. test_helper_does_not_leak_default_env_into_scoped_miss supplies a synthetic "restricted-default" to _sig_secret, while no new test constructs the real SignalAdapter with the production "*" default and drives the pre-auth reaction gate.
Required repair: distinguish unscoped single-profile fallback from installed scoped absence for SIGNAL_ALLOWED_USERS. A scoped miss should produce an empty adapter allowlist; "*" should only mean open when the profile explicitly supplied it (scope/YAML). Add a regression with default-profile SIGNAL_ALLOWED_USERS=* plus a secondary installed scope that omits the key and prove an untrusted DM gets no pre-auth reaction; then prove an explicit * in the secondary profile still opts in. #88559 already contains the useful prior-art shape here: scoped miss → empty, explicit * → open, plus a reaction-path guard. I would salvage that narrow Signal semantic rather than merge the stale broader PR.
One adjacent profile-isolation seam is still visible on this head: SIGNAL_REQUIRE_MENTION remains a raw process-env read; #88559 also scoped that value. I would either compose that tiny residual while this file is open or leave it explicitly owned as follow-up rather than imply all Signal profile-local behavior is now scoped. I am not treating that alone as the merge blocker above; the wildcard/pre-auth reaction path is the concrete security issue.
Topology/provenance otherwise looks right. #93545 is the earlier partial implementation by @chelsealong and its exact salvaged commit 168e2e6e... remains authored by her; #93605 is the completing implementation from issue reporter @aniruddhaadak80 and this exact head remains authored by him. #93639 should become the single delivery owner after the Signal fix; #93545 and #93605 are competing/superseded inputs and should not merge in parallel. #88559 is older, broader, currently non-mergeable prior art for the remaining Signal semantics—not a second merge lane.
Summary
Secondary-profile allowlists on weixin/yuanbao/signal/wecom group chats work again, and a default profile's
GATEWAY_ALLOW_ALL_USERS=truecan no longer leak admission into other profiles. Undergateway.multiplex_profiles, these four adapters read DM/group policies and allowlists via rawos.getenvat construction time — bypassing the per-profile secret scope (which deliberately never mutatesos.environ), so secondary-profile authz config was invisible (silent drops) and default-profile allow-all was global (fail-open). Fixes #93522.Changes
gateway/platforms/weixin.py,yuanbao.py,signal.py,plugins/platforms/wecom/adapter.py: every authorization read (DM/group policy, allowlists, allow-all opt-ins, open-DM opt-in gates) now routes through the profile-scoped secret helpers, matching the existing qqbot pattern__init__-snapshotted lists includedtests/gateway/test_multiplex_profile_authz.py+tests/gateway/test_platform_authz_scope.py: secondary profile env-only allowlist admits its sender; default-profile allow-all does not leakValidation
GATEWAY_ALLOW_ALL_USERSSalvaged from #93545 (@chelsealong, earliest) with follow-up from #93605 (@aniruddhaadak80 — signal coverage, allow-all gates, cross-adapter suite); both cherry-picked with authorship preserved.
Infographic