fix(google-chat): scope multiplex profile config and fail ADC closed
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 <rayjun0412@gmail.com>
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user