From b1bc9bb6500ec112c1b457ad5f486e695725b030 Mon Sep 17 00:00:00 2001 From: Jony <619963502@qq.com> Date: Wed, 2 Sep 2026 03:35:12 -0700 Subject: [PATCH] fix(google-chat): scope multiplex profile config and fail ADC closed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Route every GOOGLE_CHAT_* / GOOGLE_APPLICATION_CREDENTIALS read through a module-local `_get_scoped_secret` (scope-authoritative under multiplex, os.environ fallback only for the unscoped default-profile constructor, so startup/reconnect never hits UnscopedSecretError — #70652 class). Snapshot Pub/Sub callback knobs on the instance while the scope is still installed, and seed them into `extra` from `_env_enablement`. When a scoped profile has no service-account setting, do NOT fall through to google.auth.default(): ADC reads the process env directly and would authenticate the profile as another profile's SA. Fail closed with an explicit error (adapter and standalone send). Also resolve the bot-id cache path at call time via get_hermes_home() so profiles don't share one identity cache. Fixes #73439. Salvaged from #73445 (Jony) with the ADC guard from #57674 (Ray, first submitter). Co-authored-by: Ray --- plugins/platforms/google_chat/adapter.py | 149 ++++++++++++++++++----- tests/gateway/test_google_chat.py | 55 +++++++++ 2 files changed, 172 insertions(+), 32 deletions(-) diff --git a/plugins/platforms/google_chat/adapter.py b/plugins/platforms/google_chat/adapter.py index e123bb2177..7be34ceee2 100644 --- a/plugins/platforms/google_chat/adapter.py +++ b/plugins/platforms/google_chat/adapter.py @@ -48,6 +48,45 @@ import time from pathlib import Path as _Path from typing import Any, Callable, Dict, List, Optional, Tuple +from agent.secret_scope import UnscopedSecretError as _UnscopedSecretError +from agent.secret_scope import get_secret as _scoped_get_secret +from agent.secret_scope import is_multiplex_active + + +def _get_scoped_secret(name: str, default: Optional[str] = None) -> Optional[str]: + """Scope-aware config/credential read with the default-profile fallback. + + Secondary profiles construct their adapters under a profile secret + scope -- the scope is authoritative and a scoped miss returns ``default`` + (no cross-profile borrow from ``os.environ``, which may hold another + profile's value). The DEFAULT profile's adapter constructs and connects + *unscoped* under multiplexing, where a bare ``get_secret`` would raise + ``UnscopedSecretError`` and crash startup/reconnect (#70652 class); there + ``os.environ`` is that profile's own value, so fall back to it. Same + pattern as ``whatsapp_common._get_wsecret`` and the WeCom/IRC/ntfy + plugin adapters. + """ + try: + val = _scoped_get_secret(name, default) + except _UnscopedSecretError: + val = os.getenv(name) + return val if val is not None else default + + +def _adc_would_borrow_foreign_credentials() -> bool: + """True when ADC would silently read another profile's SA from process env. + + ``google.auth.default()`` consults ``os.environ`` directly. Under + multiplexing a scoped profile only reaches the ADC branch after its own + scope had no service-account setting -- if the process env still carries + one (the default profile's), ADC would authenticate this profile as that + other identity. Fail closed instead. + """ + return is_multiplex_active() and bool( + os.environ.get("GOOGLE_CHAT_SERVICE_ACCOUNT_JSON") + or os.environ.get("GOOGLE_APPLICATION_CREDENTIALS") + ) + # Heavy google-cloud + googleapiclient imports are deferred to first # adapter use. Importing them eagerly here added ~110ms wall and ~33MB # RSS to *every* CLI invocation (the plugin loader imports this module at @@ -737,28 +776,48 @@ class GoogleChatAdapter(BasePlatformAdapter): # end-of-turn by on_processing_complete via patch-to-empty so # they don't sit in the chat forever as "Hermes is thinking…". self._orphan_typing_messages: Dict[str, List[str]] = {} - # FlowControl knobs (env-configurable). + # Snapshot profile-scoped settings while adapter construction still + # runs inside _profile_runtime_scope. Pub/Sub invokes callbacks from + # its own threads, where the ContextVar secret scope is intentionally + # unavailable; callbacks must use these instance values rather than + # consulting process-global environment state. + extra = self.config.extra try: - self._max_messages = int(os.getenv("GOOGLE_CHAT_MAX_MESSAGES", "1")) + self._max_messages = int( + extra.get("max_messages") + or _get_scoped_secret("GOOGLE_CHAT_MAX_MESSAGES", "1") + ) except (ValueError, TypeError): self._max_messages = 1 try: - self._max_bytes = int(os.getenv("GOOGLE_CHAT_MAX_BYTES", str(16 * 1024 * 1024))) + self._max_bytes = int( + extra.get("max_bytes") + or _get_scoped_secret("GOOGLE_CHAT_MAX_BYTES", str(16 * 1024 * 1024)) + ) except (ValueError, TypeError): self._max_bytes = 16 * 1024 * 1024 + self._bootstrap_spaces = str( + extra.get("bootstrap_spaces") + or _get_scoped_secret("GOOGLE_CHAT_BOOTSTRAP_SPACES", "") + or "" + ).strip() + self._debug_raw = bool( + extra.get("debug_raw") + or _get_scoped_secret("GOOGLE_CHAT_DEBUG_RAW") + ) self._http_events_url = ( - self.config.extra.get("http_events_url") - or os.getenv("GOOGLE_CHAT_HTTP_EVENTS_URL", "") + extra.get("http_events_url") + or _get_scoped_secret("GOOGLE_CHAT_HTTP_EVENTS_URL", "") or "" ).strip() self._http_events_audience = ( - self.config.extra.get("http_events_audience") - or os.getenv("GOOGLE_CHAT_HTTP_EVENTS_AUDIENCE", "") + extra.get("http_events_audience") + or _get_scoped_secret("GOOGLE_CHAT_HTTP_EVENTS_AUDIENCE", "") or self._http_events_url ).strip() self._http_events_service_account_email = ( - self.config.extra.get("http_events_service_account_email") - or os.getenv("GOOGLE_CHAT_HTTP_EVENTS_SERVICE_ACCOUNT_EMAIL", "") + extra.get("http_events_service_account_email") + or _get_scoped_secret("GOOGLE_CHAT_HTTP_EVENTS_SERVICE_ACCOUNT_EMAIL", "") or "" ).strip().lower() @@ -780,7 +839,7 @@ class GoogleChatAdapter(BasePlatformAdapter): """ sa_path = ( self.config.extra.get("service_account_json") - or os.getenv("GOOGLE_APPLICATION_CREDENTIALS") + or _get_scoped_secret("GOOGLE_APPLICATION_CREDENTIALS") ) if sa_path: # Inline JSON (rare, but supported). @@ -812,6 +871,13 @@ class GoogleChatAdapter(BasePlatformAdapter): # No explicit SA configured — try ADC. This is the Cloud Run / GCE # path; google-auth picks up the workload identity automatically. + if _adc_would_borrow_foreign_credentials(): + raise ValueError( + "Google Chat ADC skipped for this profile: service-account " + "credentials are set in the process environment but not in " + "this profile's secret scope. Set " + "GOOGLE_CHAT_SERVICE_ACCOUNT_JSON in this profile's .env." + ) try: import google.auth as google_auth except ImportError: @@ -917,8 +983,12 @@ class GoogleChatAdapter(BasePlatformAdapter): # ------------------------------------------------------------------ def _bot_id_cache_path(self) -> _Path: """Location where the resolved bot user_id is cached across restarts.""" - base = os.getenv("HERMES_HOME", str(_Path.home() / ".hermes")) - return _Path(base) / "google_chat_bot_id.json" + # Resolve at call time (connect() runs inside the profile scope) so + # multiplexed profiles do not share one bot-identity cache file; the + # thread-count store above already resolves the same way. + from hermes_constants import get_hermes_home as _get_hermes_home + + return _get_hermes_home() / "google_chat_bot_id.json" def _load_cached_bot_id(self) -> Optional[str]: path = self._bot_id_cache_path() @@ -953,7 +1023,7 @@ class GoogleChatAdapter(BasePlatformAdapter): if self.config.home_channel and self.config.home_channel.chat_id: candidate_spaces.append(self.config.home_channel.chat_id) # Env-configured allowed spaces (comma-separated). Optional. - extra_spaces = os.getenv("GOOGLE_CHAT_BOOTSTRAP_SPACES", "").strip() + extra_spaces = self._bootstrap_spaces if extra_spaces: candidate_spaces.extend( s.strip() for s in extra_spaces.split(",") if s.strip() @@ -1402,7 +1472,7 @@ class GoogleChatAdapter(BasePlatformAdapter): list(envelope.keys()), ce_type, ) - if os.getenv("GOOGLE_CHAT_DEBUG_RAW"): + if self._debug_raw: # Dangerous flag: contains message text and sender email. Route # through the global redaction filter and gate at DEBUG level so # default log configurations never surface it. Operators must @@ -3363,14 +3433,14 @@ def _check_for_registry() -> bool: if not check_google_chat_requirements(): return False project = ( - os.getenv("GOOGLE_CHAT_PROJECT_ID") - or os.getenv("GOOGLE_CLOUD_PROJECT") + _get_scoped_secret("GOOGLE_CHAT_PROJECT_ID") + or _get_scoped_secret("GOOGLE_CLOUD_PROJECT") ) subscription = ( - os.getenv("GOOGLE_CHAT_SUBSCRIPTION_NAME") - or os.getenv("GOOGLE_CHAT_SUBSCRIPTION") + _get_scoped_secret("GOOGLE_CHAT_SUBSCRIPTION_NAME") + or _get_scoped_secret("GOOGLE_CHAT_SUBSCRIPTION") ) - http_events_url = os.getenv("GOOGLE_CHAT_HTTP_EVENTS_URL") + http_events_url = _get_scoped_secret("GOOGLE_CHAT_HTTP_EVENTS_URL") return bool(http_events_url or (project and subscription)) @@ -3394,14 +3464,14 @@ def _env_enablement() -> Optional[Dict[str, Any]]: ``PlatformConfig`` rather than being merged into ``extra``. """ project = ( - os.getenv("GOOGLE_CHAT_PROJECT_ID") - or os.getenv("GOOGLE_CLOUD_PROJECT") + _get_scoped_secret("GOOGLE_CHAT_PROJECT_ID") + or _get_scoped_secret("GOOGLE_CLOUD_PROJECT") ) subscription = ( - os.getenv("GOOGLE_CHAT_SUBSCRIPTION_NAME") - or os.getenv("GOOGLE_CHAT_SUBSCRIPTION") + _get_scoped_secret("GOOGLE_CHAT_SUBSCRIPTION_NAME") + or _get_scoped_secret("GOOGLE_CHAT_SUBSCRIPTION") ) - http_events_url = os.getenv("GOOGLE_CHAT_HTTP_EVENTS_URL") + http_events_url = _get_scoped_secret("GOOGLE_CHAT_HTTP_EVENTS_URL") if not (http_events_url or (project and subscription)): return None seed: Dict[str, Any] = {} @@ -3411,23 +3481,32 @@ def _env_enablement() -> Optional[Dict[str, Any]]: seed["subscription_name"] = subscription if http_events_url: seed["http_events_url"] = http_events_url - http_events_audience = os.getenv("GOOGLE_CHAT_HTTP_EVENTS_AUDIENCE") + http_events_audience = _get_scoped_secret("GOOGLE_CHAT_HTTP_EVENTS_AUDIENCE") if http_events_audience: seed["http_events_audience"] = http_events_audience - http_events_sa_email = os.getenv("GOOGLE_CHAT_HTTP_EVENTS_SERVICE_ACCOUNT_EMAIL") + http_events_sa_email = _get_scoped_secret("GOOGLE_CHAT_HTTP_EVENTS_SERVICE_ACCOUNT_EMAIL") if http_events_sa_email: seed["http_events_service_account_email"] = http_events_sa_email + for env_name, extra_name in ( + ("GOOGLE_CHAT_MAX_MESSAGES", "max_messages"), + ("GOOGLE_CHAT_MAX_BYTES", "max_bytes"), + ("GOOGLE_CHAT_BOOTSTRAP_SPACES", "bootstrap_spaces"), + ("GOOGLE_CHAT_DEBUG_RAW", "debug_raw"), + ): + value = _get_scoped_secret(env_name) + if value: + seed[extra_name] = value sa_json = ( - os.getenv("GOOGLE_CHAT_SERVICE_ACCOUNT_JSON") - or os.getenv("GOOGLE_APPLICATION_CREDENTIALS") + _get_scoped_secret("GOOGLE_CHAT_SERVICE_ACCOUNT_JSON") + or _get_scoped_secret("GOOGLE_APPLICATION_CREDENTIALS") ) if sa_json: seed["service_account_json"] = sa_json - home = os.getenv("GOOGLE_CHAT_HOME_CHANNEL") + home = _get_scoped_secret("GOOGLE_CHAT_HOME_CHANNEL") if home: seed["home_channel"] = { "chat_id": home, - "name": os.getenv("GOOGLE_CHAT_HOME_CHANNEL_NAME", "Home"), + "name": _get_scoped_secret("GOOGLE_CHAT_HOME_CHANNEL_NAME", "Home"), } return seed @@ -3577,8 +3656,8 @@ async def _standalone_send( extra = getattr(pconfig, "extra", {}) or {} sa_value = ( extra.get("service_account_json") - or os.getenv("GOOGLE_CHAT_SERVICE_ACCOUNT_JSON") - or os.getenv("GOOGLE_APPLICATION_CREDENTIALS") + or _get_scoped_secret("GOOGLE_CHAT_SERVICE_ACCOUNT_JSON") + or _get_scoped_secret("GOOGLE_APPLICATION_CREDENTIALS") ) if service_account is None: @@ -3608,6 +3687,12 @@ async def _standalone_send( return {"error": f"Google Chat standalone send: SA JSON file is invalid: {exc}"} creds = service_account.Credentials.from_service_account_info(info, scopes=_CHAT_SCOPES) else: + if _adc_would_borrow_foreign_credentials(): + return {"error": ( + "Google Chat standalone send: ADC skipped for this profile: " + "service-account credentials are set in the process environment " + "but not in this profile's secret scope" + )} try: import google.auth as _google_auth except ImportError: diff --git a/tests/gateway/test_google_chat.py b/tests/gateway/test_google_chat.py index 19aa5163e6..d3a05ea00c 100644 --- a/tests/gateway/test_google_chat.py +++ b/tests/gateway/test_google_chat.py @@ -270,6 +270,61 @@ class TestEnvConfigLoading: cfg = load_gateway_config() assert _GC not in cfg.platforms + def test_multiplex_scoped_profile_never_borrows_process_env( + self, monkeypatch, tmp_path + ): + """Under multiplex a scoped profile sees ONLY its own Google Chat + settings, and the ADC branch fails closed instead of authenticating + as the default profile's service account (#73439).""" + from agent.secret_scope import ( + build_profile_secret_scope, + set_multiplex_active, + set_secret_scope, + ) + + self._clean_env(monkeypatch) + monkeypatch.setenv("GOOGLE_CHAT_PROJECT_ID", "default-proj") + monkeypatch.setenv("GOOGLE_CHAT_SUBSCRIPTION_NAME", "default-sub") + monkeypatch.setenv("GOOGLE_APPLICATION_CREDENTIALS", "/secrets/default.json") + monkeypatch.setenv("GOOGLE_CHAT_BOOTSTRAP_SPACES", "spaces/DEFAULT") + profile_home = tmp_path / "beta" + profile_home.mkdir() + (profile_home / ".env").write_text( + "GOOGLE_CHAT_PROJECT_ID=beta-proj\nGOOGLE_CHAT_SUBSCRIPTION_NAME=beta-sub\n" + ) + set_multiplex_active(True) + token = set_secret_scope(build_profile_secret_scope(profile_home)) + try: + seed = _gc_mod._env_enablement() or {} + beta = GoogleChatAdapter( + PlatformConfig(enabled=True, extra={"project_id": "beta-proj", "subscription_name": "beta-sub"}) + ) + with pytest.raises(ValueError, match="ADC skipped"): + beta._load_sa_credentials() + finally: + from agent.secret_scope import reset_secret_scope + + reset_secret_scope(token) + set_multiplex_active(False) + assert seed["project_id"] == "beta-proj" + assert "service_account_json" not in seed + assert beta._bootstrap_spaces == "" + + def test_multiplex_default_profile_constructs_unscoped(self, monkeypatch): + """The default profile's adapter is built OUTSIDE any scope while + multiplex is active (gateway startup/reconnect); it must keep reading + its own process env instead of raising UnscopedSecretError.""" + from agent.secret_scope import set_multiplex_active + + self._clean_env(monkeypatch) + monkeypatch.setenv("GOOGLE_CHAT_BOOTSTRAP_SPACES", "spaces/DEFAULT") + set_multiplex_active(True) + try: + default = GoogleChatAdapter(_base_config()) + finally: + set_multiplex_active(False) + assert default._bootstrap_spaces == "spaces/DEFAULT" + # =========================================================================== # Pure helpers