fix(qqbot): resolve credentials under the active profile secret scope
The QQ adapter read QQ_APP_ID, QQ_CLIENT_SECRET, the QQ_STT_* backend config and the QQ_ALLOW_ALL_USERS policy flag through raw os.getenv, bypassing the active profile secret scope. In multiplex mode a secondary profile whose secret lives in its own .env (installed as an isolated scope, not into os.environ) would silently fall back to the default/primary profile's value — the same cross-profile collision fixed for the WeChat/weixin adapter in #59662. Route these reads through a scope-aware resolver that reads the profile scope when one is installed (secondary profiles and per-turn inbound) and falls back to os.environ otherwise. The fallback is deliberate: the primary/active profile is constructed without a scope and owns os.environ, so a bare get_secret would raise UnscopedSecretError and break its startup. Mirrors gateway.config._getenv. Adds regression tests including active-profile-no-scope construction (the fail-closed case), plus scope-wins-over-environ, two-profile isolation, single-profile fallback, explicit-config precedence and STT key scoping.
This commit is contained in:
@@ -147,6 +147,31 @@ def _coerce_list(value: Any) -> List[str]:
|
||||
return _coerce_list_impl(value)
|
||||
|
||||
|
||||
def _resolve_qq_secret(name: str, default: str = "") -> str:
|
||||
"""Resolve a per-profile ``QQ_*`` setting honoring the active secret scope.
|
||||
|
||||
When a profile secret scope is installed — every secondary multiplex
|
||||
profile is constructed and handled inside ``_profile_runtime_scope``
|
||||
(``gateway/run.py``), as is each per-turn inbound message — read from it so
|
||||
profiles never see each other's ``os.environ`` values. This is the
|
||||
cross-profile credential collision fixed for the WeChat adapter in #59662.
|
||||
|
||||
The primary/active profile is constructed without a scope and legitimately
|
||||
owns ``os.environ``, so fall back to it there instead of failing closed: a
|
||||
bare ``get_secret`` would raise ``UnscopedSecretError`` on the active
|
||||
profile's ``__init__`` and break its startup. Same pattern as the Slack
|
||||
``SLACK_APP_TOKEN`` read (#59739) and
|
||||
``gateway.platforms.whatsapp_common._get_wsecret``.
|
||||
"""
|
||||
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
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# QQAdapter
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -202,9 +227,11 @@ class QQAdapter(BasePlatformAdapter):
|
||||
super().__init__(config, Platform.QQBOT)
|
||||
|
||||
extra = config.extra or {}
|
||||
self._app_id = str(extra.get("app_id") or os.getenv("QQ_APP_ID", "")).strip()
|
||||
self._app_id = str(
|
||||
extra.get("app_id") or _resolve_qq_secret("QQ_APP_ID", "")
|
||||
).strip()
|
||||
self._client_secret = str(
|
||||
extra.get("client_secret") or os.getenv("QQ_CLIENT_SECRET", "")
|
||||
extra.get("client_secret") or _resolve_qq_secret("QQ_CLIENT_SECRET", "")
|
||||
).strip()
|
||||
self._markdown_support = bool(extra.get("markdown_support", True))
|
||||
|
||||
@@ -2202,13 +2229,13 @@ class QQAdapter(BasePlatformAdapter):
|
||||
}
|
||||
|
||||
# 2. QQ-specific env vars (set by `hermes setup gateway` / `hermes gateway`)
|
||||
qq_stt_key = os.getenv("QQ_STT_API_KEY", "")
|
||||
qq_stt_key = _resolve_qq_secret("QQ_STT_API_KEY", "")
|
||||
if qq_stt_key:
|
||||
base_url = os.getenv(
|
||||
base_url = _resolve_qq_secret(
|
||||
"QQ_STT_BASE_URL",
|
||||
"https://open.bigmodel.cn/api/coding/paas/v4",
|
||||
)
|
||||
model = os.getenv("QQ_STT_MODEL", "glm-asr")
|
||||
model = _resolve_qq_secret("QQ_STT_MODEL", "glm-asr")
|
||||
return {
|
||||
"base_url": base_url.rstrip("/"),
|
||||
"api_key": qq_stt_key,
|
||||
@@ -3170,7 +3197,7 @@ class QQAdapter(BasePlatformAdapter):
|
||||
def _open_dm_opted_in(self) -> bool:
|
||||
if os.getenv("GATEWAY_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}:
|
||||
return True
|
||||
return os.getenv("QQ_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}
|
||||
return _resolve_qq_secret("QQ_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}
|
||||
|
||||
def _is_dm_allowed(self, user_id: str) -> bool:
|
||||
if self._dm_policy == "disabled":
|
||||
|
||||
102
tests/gateway/test_qqbot_credential_isolation.py
Normal file
102
tests/gateway/test_qqbot_credential_isolation.py
Normal file
@@ -0,0 +1,102 @@
|
||||
"""Credential isolation for the QQ (qqbot) gateway adapter.
|
||||
|
||||
Covers the multiplex credential-collision class (same class as the
|
||||
WeChat/weixin adapter tracked in #59662): the QQ adapter resolves its ``QQ_*``
|
||||
settings through the active profile secret scope rather than raw ``os.getenv``,
|
||||
so a secondary profile whose secret lives in its own ``.env`` (installed as an
|
||||
isolated scope, not into ``os.environ``) does not fall back to the
|
||||
default/primary profile's value.
|
||||
|
||||
Also guards the primary/active profile: it is constructed without a scope and
|
||||
legitimately owns ``os.environ``, so the resolver must fall back to
|
||||
``os.environ`` there (not fail closed) even when multiplexing is active —
|
||||
otherwise the active profile's adapter would raise ``UnscopedSecretError`` on
|
||||
construction and fail to start.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
from agent import secret_scope as ss
|
||||
from gateway.config import PlatformConfig
|
||||
from gateway.platforms.qqbot.adapter import QQAdapter
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset_multiplex():
|
||||
ss.set_multiplex_active(False)
|
||||
yield
|
||||
ss.set_multiplex_active(False)
|
||||
|
||||
|
||||
def _make_adapter(extra=None):
|
||||
return QQAdapter(PlatformConfig(enabled=True, extra=extra or {}))
|
||||
|
||||
|
||||
class TestQQCredentialScope:
|
||||
def test_credentials_read_scope_not_environ(self, monkeypatch):
|
||||
# os.environ holds another profile's values; the scoped values must win.
|
||||
monkeypatch.setenv("QQ_APP_ID", "global-app")
|
||||
monkeypatch.setenv("QQ_CLIENT_SECRET", "global-secret")
|
||||
ss.set_multiplex_active(True)
|
||||
tok = ss.set_secret_scope(
|
||||
{"QQ_APP_ID": "profileA-app", "QQ_CLIENT_SECRET": "profileA-secret"}
|
||||
)
|
||||
try:
|
||||
adapter = _make_adapter()
|
||||
finally:
|
||||
ss.reset_secret_scope(tok)
|
||||
assert adapter._app_id == "profileA-app"
|
||||
assert adapter._client_secret == "profileA-secret"
|
||||
|
||||
def test_two_profiles_isolated(self):
|
||||
ss.set_multiplex_active(True)
|
||||
tok_a = ss.set_secret_scope({"QQ_CLIENT_SECRET": "secret-A"})
|
||||
try:
|
||||
a = _make_adapter()
|
||||
finally:
|
||||
ss.reset_secret_scope(tok_a)
|
||||
tok_b = ss.set_secret_scope({"QQ_CLIENT_SECRET": "secret-B"})
|
||||
try:
|
||||
b = _make_adapter()
|
||||
finally:
|
||||
ss.reset_secret_scope(tok_b)
|
||||
assert a._client_secret == "secret-A"
|
||||
assert b._client_secret == "secret-B"
|
||||
|
||||
def test_single_profile_still_reads_environ(self, monkeypatch):
|
||||
# No scope + multiplex inactive (default single-profile deployment):
|
||||
# legacy os.environ behavior is preserved — no regression.
|
||||
monkeypatch.setenv("QQ_CLIENT_SECRET", "legacy-secret")
|
||||
adapter = _make_adapter()
|
||||
assert adapter._client_secret == "legacy-secret"
|
||||
|
||||
def test_active_profile_no_scope_reads_environ_without_raising(self, monkeypatch):
|
||||
# The primary/active profile is built with NO scope while multiplexing
|
||||
# is active. A bare get_secret() would fail closed (UnscopedSecretError)
|
||||
# and break its startup; the resolver must fall back to os.environ.
|
||||
monkeypatch.setenv("QQ_APP_ID", "primary-app")
|
||||
monkeypatch.setenv("QQ_CLIENT_SECRET", "primary-secret")
|
||||
ss.set_multiplex_active(True)
|
||||
assert ss.current_secret_scope() is None # no scope installed
|
||||
adapter = _make_adapter() # must not raise
|
||||
assert adapter._app_id == "primary-app"
|
||||
assert adapter._client_secret == "primary-secret"
|
||||
|
||||
def test_explicit_config_extra_takes_precedence(self, monkeypatch):
|
||||
# An explicit value in config.extra still wins over env/scope.
|
||||
monkeypatch.setenv("QQ_CLIENT_SECRET", "env-secret")
|
||||
adapter = _make_adapter(extra={"client_secret": "explicit"})
|
||||
assert adapter._client_secret == "explicit"
|
||||
|
||||
|
||||
class TestQQSttConfigScope:
|
||||
def test_stt_api_key_reads_scope(self, monkeypatch):
|
||||
monkeypatch.setenv("QQ_STT_API_KEY", "global-stt-key")
|
||||
ss.set_multiplex_active(True)
|
||||
tok = ss.set_secret_scope({"QQ_STT_API_KEY": "profileA-stt-key"})
|
||||
try:
|
||||
adapter = _make_adapter()
|
||||
stt = adapter._resolve_stt_config()
|
||||
finally:
|
||||
ss.reset_secret_scope(tok)
|
||||
assert stt is not None
|
||||
assert stt["api_key"] == "profileA-stt-key"
|
||||
Reference in New Issue
Block a user