fix(gateway): route platform authorization reads through the profile 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
This commit is contained in:
aniruddhaadak80
@@ -250,6 +250,26 @@ def validate_signal_config(config: PlatformConfig) -> bool:
|
||||
# Signal Adapter
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _sig_secret(name: str, default: str = "") -> str:
|
||||
"""Resolve a per-profile ``SIGNAL_*`` setting honoring the active secret
|
||||
scope (#93522).
|
||||
|
||||
Under ``gateway.multiplex_profiles`` every secondary profile is
|
||||
constructed inside ``_profile_runtime_scope`` (``gateway/run.py``) and
|
||||
its ``.env`` lives in that scope — raw ``os.getenv`` misses it and
|
||||
leaks the default profile's values instead. The primary/active profile
|
||||
is constructed without a scope and legitimately owns ``os.environ``
|
||||
(same canonical shape as QQ's ``_resolve_qq_secret``).
|
||||
"""
|
||||
from agent.secret_scope import UnscopedSecretError, get_secret
|
||||
|
||||
try:
|
||||
val = get_secret(name, default)
|
||||
except UnscopedSecretError:
|
||||
val = os.getenv(name)
|
||||
return val if val is not None else default
|
||||
|
||||
|
||||
class SignalAdapter(BasePlatformAdapter):
|
||||
"""Signal messenger adapter using signal-cli HTTP daemon."""
|
||||
|
||||
@@ -268,7 +288,10 @@ class SignalAdapter(BasePlatformAdapter):
|
||||
self.ignore_stories = extra.get("ignore_stories", True)
|
||||
|
||||
# Parse allowlists — group policy is derived from presence of group allowlist
|
||||
group_allowed_str = os.getenv("SIGNAL_GROUP_ALLOWED_USERS", "")
|
||||
# Scoped reads (#93522): allowlists are per-profile authorization
|
||||
# config; raw os.getenv misses secondary profiles' .env values and
|
||||
# leaks the default profile's list into them.
|
||||
group_allowed_str = _sig_secret("SIGNAL_GROUP_ALLOWED_USERS", "")
|
||||
self.group_allow_from = set(_parse_comma_list(group_allowed_str))
|
||||
|
||||
# Mention filter — only respond in groups when the bot account is @mentioned.
|
||||
@@ -285,7 +308,7 @@ class SignalAdapter(BasePlatformAdapter):
|
||||
# every inbound DM from any contact gets a 👀 reaction).
|
||||
# "*" means all users allowed (open mode); empty means no restriction
|
||||
# recorded at adapter level (run.py still enforces auth separately).
|
||||
dm_allowed_str = os.getenv("SIGNAL_ALLOWED_USERS", "*")
|
||||
dm_allowed_str = _sig_secret("SIGNAL_ALLOWED_USERS", "*")
|
||||
self.dm_allow_from = set(_parse_comma_list(dm_allowed_str))
|
||||
|
||||
# HTTP client
|
||||
|
||||
@@ -1537,9 +1537,11 @@ class WeixinAdapter(BasePlatformAdapter):
|
||||
await self.handle_message(event)
|
||||
|
||||
def _open_dm_opted_in(self) -> bool:
|
||||
if os.getenv("GATEWAY_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}:
|
||||
# Scoped reads (#93522): the default profile's allow-all flag must
|
||||
# not leak into a multiplexed secondary profile's admission gate.
|
||||
if (_wx_secret("GATEWAY_ALLOW_ALL_USERS", "") or "").lower() in {"true", "1", "yes"}:
|
||||
return True
|
||||
return os.getenv("WEIXIN_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}
|
||||
return (_wx_secret("WEIXIN_ALLOW_ALL_USERS", "") or "").lower() in {"true", "1", "yes"}
|
||||
|
||||
def _is_dm_allowed(self, sender_id: str) -> bool:
|
||||
if self._dm_policy == "disabled":
|
||||
|
||||
@@ -97,7 +97,6 @@ from gateway.platforms.yuanbao_proto import (
|
||||
next_seq_no,
|
||||
)
|
||||
from gateway.session import build_session_key
|
||||
from gateway.authz_mixin import _platform_gate_env
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -1262,6 +1261,27 @@ class ChatRoutingMiddleware(InboundMiddleware):
|
||||
await next_fn()
|
||||
|
||||
|
||||
def _yb_secret(name: str, default: Optional[str] = None) -> Optional[str]:
|
||||
"""Resolve a per-profile ``YUANBAO_*`` / gateway setting honoring the
|
||||
active secret scope (#93522).
|
||||
|
||||
Under ``gateway.multiplex_profiles`` every secondary profile is
|
||||
constructed inside ``_profile_runtime_scope`` (``gateway/run.py``) and
|
||||
its ``.env`` lives in that scope — raw ``os.getenv`` misses it and
|
||||
leaks the default profile's values instead. The primary/active profile
|
||||
is constructed without a scope and legitimately owns ``os.environ``,
|
||||
so fall back to it there (same canonical shape as QQ's
|
||||
``_resolve_qq_secret``).
|
||||
"""
|
||||
from agent.secret_scope import UnscopedSecretError, get_secret
|
||||
|
||||
try:
|
||||
val = get_secret(name, default)
|
||||
except UnscopedSecretError:
|
||||
val = os.getenv(name)
|
||||
return val if val is not None else default
|
||||
|
||||
|
||||
class AccessPolicy:
|
||||
"""Platform-level DM / Group access control policy.
|
||||
|
||||
@@ -1283,9 +1303,9 @@ class AccessPolicy:
|
||||
self._group_allow_from = group_allow_from
|
||||
|
||||
def _open_dm_opted_in(self) -> bool:
|
||||
if _platform_gate_env("GATEWAY_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}:
|
||||
if (_yb_secret("GATEWAY_ALLOW_ALL_USERS", "") or "").lower() in {"true", "1", "yes"}:
|
||||
return True
|
||||
return _platform_gate_env("YUANBAO_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}
|
||||
return (_yb_secret("YUANBAO_ALLOW_ALL_USERS", "") or "").lower() in {"true", "1", "yes"}
|
||||
|
||||
def is_dm_allowed(self, sender_id: str) -> bool:
|
||||
"""Strict DM authorization — pairing does not imply access."""
|
||||
@@ -4943,27 +4963,29 @@ class YuanbaoAdapter(BasePlatformAdapter):
|
||||
|
||||
# Reply-to dedup: inbound_msg_id -> expire_ts
|
||||
# ------------------------------------------------------------------
|
||||
# Access control policy (DM / Group)
|
||||
# Access control policy (DM / Group) — scoped reads (#93522)
|
||||
# ------------------------------------------------------------------
|
||||
dm_policy: str = (
|
||||
_extra.get("dm_policy")
|
||||
or _platform_gate_env("YUANBAO_DM_POLICY", "pairing")
|
||||
or _yb_secret("YUANBAO_DM_POLICY")
|
||||
or "pairing"
|
||||
).strip().lower()
|
||||
|
||||
_dm_allow_from_raw: str = (
|
||||
_extra.get("dm_allow_from")
|
||||
or _platform_gate_env("YUANBAO_DM_ALLOW_FROM", "")
|
||||
or _yb_secret("YUANBAO_DM_ALLOW_FROM", "")
|
||||
)
|
||||
dm_allow_from: list[str] = [x.strip() for x in _dm_allow_from_raw.split(",") if x.strip()]
|
||||
|
||||
group_policy: str = (
|
||||
_extra.get("group_policy")
|
||||
or _platform_gate_env("YUANBAO_GROUP_POLICY", "pairing")
|
||||
or _yb_secret("YUANBAO_GROUP_POLICY")
|
||||
or "pairing"
|
||||
).strip().lower()
|
||||
|
||||
_group_allow_from_raw: str = (
|
||||
_extra.get("group_allow_from")
|
||||
or _platform_gate_env("YUANBAO_GROUP_ALLOW_FROM", "")
|
||||
or _yb_secret("YUANBAO_GROUP_ALLOW_FROM", "")
|
||||
)
|
||||
group_allow_from: list[str] = [x.strip() for x in _group_allow_from_raw.split(",") if x.strip()]
|
||||
|
||||
|
||||
@@ -893,9 +893,11 @@ class WeComAdapter(BasePlatformAdapter):
|
||||
return True
|
||||
|
||||
def _open_dm_opted_in(self) -> bool:
|
||||
if os.getenv("GATEWAY_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}:
|
||||
# Scoped reads (#93522): the default profile's allow-all flag must
|
||||
# not leak into a multiplexed secondary profile's admission gate.
|
||||
if (_get_scoped_secret("GATEWAY_ALLOW_ALL_USERS", "") or "").lower() in {"true", "1", "yes"}:
|
||||
return True
|
||||
return os.getenv("WECOM_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}
|
||||
return (_get_scoped_secret("WECOM_ALLOW_ALL_USERS", "") or "").lower() in {"true", "1", "yes"}
|
||||
|
||||
def _is_dm_allowed(self, sender_id: str) -> bool:
|
||||
if self._dm_policy == "disabled":
|
||||
|
||||
@@ -0,0 +1,212 @@
|
||||
"""Platform authorization reads must honor the per-profile secret scope (#93522).
|
||||
|
||||
Under ``gateway.multiplex_profiles`` every secondary profile is constructed
|
||||
inside ``_profile_runtime_scope`` and its ``.env`` lives in that scope —
|
||||
``gateway/run.py`` explicitly does NOT mutate ``os.environ`` with it. Raw
|
||||
``os.getenv`` authorization reads therefore (a) silently miss the secondary
|
||||
profile's own allowlists (fail-closed no-replies) and (b) leak the default
|
||||
profile's ``GATEWAY_ALLOW_ALL_USERS`` / allowlists into secondary profiles
|
||||
(fail-open admissions).
|
||||
|
||||
Covers the scoped env helpers of weixin / yuanbao / signal / wecom plus the
|
||||
``_open_dm_opted_in`` gates built on them. Canonical shape reference:
|
||||
QQ's ``_resolve_qq_secret``.
|
||||
"""
|
||||
|
||||
import contextlib
|
||||
import os
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.secret_scope import reset_secret_scope, set_secret_scope
|
||||
|
||||
|
||||
@contextlib.contextmanager
|
||||
def _scope(secrets):
|
||||
token = set_secret_scope(secrets)
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
reset_secret_scope(token)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def profile_scope():
|
||||
"""Install a secondary-profile secret scope; os.environ stays 'default'."""
|
||||
with _scope(
|
||||
{
|
||||
"WEIXIN_DM_POLICY": "allowlist",
|
||||
"WEIXIN_ALLOWED_USERS": "wx-alice",
|
||||
"WEIXIN_ALLOW_ALL_USERS": "",
|
||||
"YUANBAO_DM_POLICY": "open",
|
||||
"SIGNAL_ALLOWED_USERS": "sig-bob",
|
||||
"WECOM_DM_POLICY": "allowlist",
|
||||
"WECOM_ALLOWED_USERS": "wecom-carol",
|
||||
}
|
||||
):
|
||||
yield
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────────────
|
||||
# Scoped helpers: read the scope, never leak os.environ past it
|
||||
# ─────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"module_path,helper_name,var",
|
||||
[
|
||||
("gateway.platforms.weixin", "_wx_secret", "WEIXIN_ALLOWED_USERS"),
|
||||
("gateway.platforms.yuanbao", "_yb_secret", "YUANBAO_DM_POLICY"),
|
||||
("gateway.platforms.signal", "_sig_secret", "SIGNAL_ALLOWED_USERS"),
|
||||
("plugins.platforms.wecom.adapter", "_get_scoped_secret", "WECOM_ALLOWED_USERS"),
|
||||
],
|
||||
)
|
||||
def test_helper_reads_profile_scope_first(module_path, helper_name, var, monkeypatch):
|
||||
"""Scope value wins over anything in os.environ."""
|
||||
monkeypatch.setenv(var, "FROM-DEFAULT-ENV")
|
||||
module = __import__(module_path, fromlist=[helper_name])
|
||||
helper = getattr(module, helper_name)
|
||||
|
||||
with _scope({var: "from-profile-scope"}):
|
||||
assert helper(var, "") == "from-profile-scope"
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def multiplex_on(monkeypatch):
|
||||
"""Simulate a multiplexed deployment: scoped misses fail closed.
|
||||
|
||||
With multiplexing off, a scope miss deliberately falls through to
|
||||
os.environ (the cron-overlay contract) — the cross-profile leak only
|
||||
exists while the multiplexer is active, which is exactly the #93522
|
||||
scenario.
|
||||
"""
|
||||
monkeypatch.setattr("agent.secret_scope._MULTIPLEX_ACTIVE", True)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"module_path,helper_name",
|
||||
[
|
||||
("gateway.platforms.weixin", "_wx_secret"),
|
||||
("gateway.platforms.yuanbao", "_yb_secret"),
|
||||
("gateway.platforms.signal", "_sig_secret"),
|
||||
("plugins.platforms.wecom.adapter", "_get_scoped_secret"),
|
||||
],
|
||||
)
|
||||
def test_helper_does_not_leak_default_env_into_scoped_miss(
|
||||
module_path, helper_name, multiplex_on, monkeypatch
|
||||
):
|
||||
"""Scoped miss must return the default, NOT fall through to os.environ.
|
||||
|
||||
This is the fail-closed half of #93522: the default profile's
|
||||
``*_ALLOW_ALL_USERS=true`` in process env must not answer a secondary
|
||||
profile's gate.
|
||||
"""
|
||||
monkeypatch.setenv("WEIXIN_ALLOW_ALL_USERS", "true")
|
||||
monkeypatch.setenv("YUANBAO_ALLOW_ALL_USERS", "true")
|
||||
monkeypatch.setenv("SIGNAL_ALLOWED_USERS", "*")
|
||||
monkeypatch.setenv("WECOM_ALLOW_ALL_USERS", "true")
|
||||
|
||||
module = __import__(module_path, fromlist=[helper_name])
|
||||
helper = getattr(module, helper_name)
|
||||
|
||||
with _scope({}): # scope installed, keys absent
|
||||
assert helper("WEIXIN_ALLOW_ALL_USERS", "") == ""
|
||||
assert helper("YUANBAO_ALLOW_ALL_USERS", "") == ""
|
||||
assert helper("WECOM_ALLOW_ALL_USERS", "") == ""
|
||||
# signal's "*" default means open at adapter level — the scoped
|
||||
# read must still not see the unscoped value:
|
||||
assert helper("SIGNAL_ALLOWED_USERS", "restricted-default") == "restricted-default"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"module_path,helper_name,var",
|
||||
[
|
||||
("gateway.platforms.weixin", "_wx_secret", "WEIXIN_ALLOWED_USERS"),
|
||||
("gateway.platforms.yuanbao", "_yb_secret", "YUANBAO_DM_POLICY"),
|
||||
("gateway.platforms.signal", "_sig_secret", "SIGNAL_ALLOWED_USERS"),
|
||||
("plugins.platforms.wecom.adapter", "_get_scoped_secret", "WECOM_ALLOWED_USERS"),
|
||||
],
|
||||
)
|
||||
def test_helper_falls_back_to_environ_without_scope(module_path, helper_name, var, monkeypatch):
|
||||
"""Single-profile (no scope installed): os.environ remains authoritative."""
|
||||
monkeypatch.setenv(var, "legacy-env-value")
|
||||
module = __import__(module_path, fromlist=[helper_name])
|
||||
helper = getattr(module, helper_name)
|
||||
assert helper(var, "") == "legacy-env-value"
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────────────
|
||||
# Admission gates built on the helpers
|
||||
# ─────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_weixin_open_gate_ignores_default_env_under_scope(profile_scope, multiplex_on, monkeypatch):
|
||||
"""Default profile sets GATEWAY_ALLOW_ALL_USERS=true; secondary scope does
|
||||
NOT carry it — the gate must stay closed."""
|
||||
monkeypatch.setenv("GATEWAY_ALLOW_ALL_USERS", "true")
|
||||
|
||||
from gateway.platforms.weixin import WeixinAdapter
|
||||
|
||||
adapter = WeixinAdapter.__new__(WeixinAdapter)
|
||||
assert adapter._open_dm_opted_in() is False
|
||||
|
||||
|
||||
def test_weixin_open_gate_honors_scope_opt_in():
|
||||
from gateway.platforms.weixin import WeixinAdapter
|
||||
|
||||
with _scope({"WEIXIN_ALLOW_ALL_USERS": "yes"}):
|
||||
adapter = WeixinAdapter.__new__(WeixinAdapter)
|
||||
assert adapter._open_dm_opted_in() is True
|
||||
|
||||
|
||||
def test_yuanbao_access_policy_gate_ignores_default_env(profile_scope, multiplex_on, monkeypatch):
|
||||
monkeypatch.setenv("GATEWAY_ALLOW_ALL_USERS", "true")
|
||||
|
||||
from gateway.platforms.yuanbao import AccessPolicy
|
||||
|
||||
policy = AccessPolicy(
|
||||
dm_policy="open", dm_allow_from=[], group_policy="pairing", group_allow_from=[]
|
||||
)
|
||||
assert policy._open_dm_opted_in() is False
|
||||
|
||||
|
||||
def test_wecom_open_gate_ignores_default_env_under_scope(profile_scope, multiplex_on, monkeypatch):
|
||||
monkeypatch.setenv("GATEWAY_ALLOW_ALL_USERS", "true")
|
||||
|
||||
from plugins.platforms.wecom.adapter import WeComAdapter
|
||||
|
||||
adapter = WeComAdapter.__new__(WeComAdapter)
|
||||
assert adapter._open_dm_opted_in() is False
|
||||
|
||||
|
||||
def test_startup_guard_uses_scoped_gateway_flag(multiplex_on):
|
||||
"""_own_policy_open_startup_violation must consult the scope for
|
||||
GATEWAY_ALLOW_ALL_USERS, not raw os.environ (#93522)."""
|
||||
from gateway.platforms.base import Platform
|
||||
from gateway.run import _own_policy_open_startup_violation
|
||||
|
||||
class _PlatformConfig:
|
||||
enabled = True
|
||||
extra = {"dm_policy": "open"}
|
||||
|
||||
class _Config:
|
||||
platforms = {Platform.YUANBAO: _PlatformConfig()}
|
||||
|
||||
cfg = _Config()
|
||||
with _scope({"GATEWAY_ALLOW_ALL_USERS": "true"}):
|
||||
# Scope says opted-in even though os.environ doesn't -> no violation.
|
||||
assert _own_policy_open_startup_violation(cfg) is None
|
||||
|
||||
with _scope({}):
|
||||
# Scope lacks the opt-in while os.environ has it -> violation (the
|
||||
# default profile's flag must not satisfy this profile's check).
|
||||
monkey_env_backup = os.environ.get("GATEWAY_ALLOW_ALL_USERS")
|
||||
os.environ["GATEWAY_ALLOW_ALL_USERS"] = "true"
|
||||
try:
|
||||
reason = _own_policy_open_startup_violation(cfg)
|
||||
assert reason is not None and "yuanbao" in reason.lower()
|
||||
finally:
|
||||
if monkey_env_backup is None:
|
||||
os.environ.pop("GATEWAY_ALLOW_ALL_USERS", None)
|
||||
else:
|
||||
os.environ["GATEWAY_ALLOW_ALL_USERS"] = monkey_env_backup
|
||||
Reference in New Issue
Block a user