From e59b877477fa95075d91268aa0095012add17989 Mon Sep 17 00:00:00 2001 From: Hung Le Date: Tue, 18 Aug 2026 00:50:08 +0530 Subject: [PATCH 01/15] feat(messaging): support Google Chat on Hermes over keyless Pub/Sub REST pull Google Chat was the only messaging channel restricted to OpenClaw. Enable it for Hermes without placing the service-account key inside the sandbox. The bundled Hermes adapter receives Chat events over a gRPC Pub/Sub StreamingPull and signs its bot token in-process from that key. Neither survives in a sandbox. The OpenShell L7 protocol set has no gRPC variant, so the transport cannot be inspected, and raw relay would disable the very inspection the credential swap depends on. The adapter also offers no seam for a pre-minted token and hardcodes an httplib2 transport that cannot proxy HTTPS here. Rebind the bundled adapter to the transports a sandbox allows: pull the same subscription over the Pub/Sub REST API, and send replies through the L7 proxy carrying the gateway-minted placeholder. The rebind ships as a sibling plugin module that the plugin loads only when the channel is configured, so a Hermes sandbox without Google Chat never wraps the platform registry. Add the Hermes policy preset and provider profile for the channel, flip the managed-image platform lists, and cover the new render and gate paths in the existing messaging tests. Correct the DM allowlist prompt while it is in reach. It told every operator to enter users/NNN ids and stated that an email entry is ignored, which holds for OpenClaw but is inverted for Hermes, so a Hermes operator following it saved an allowlist that could never match. Filling the allowlist also switches the DM policy from pairing to allowlist, so a wrong-form entry drops the sender with no reply, no pairing code, and no log line at the default level. The prompt now names the form each agent expects, states that consequence, and points an operator who does not know their id at the pairing reply, which prints it on OpenClaw. --- agents/hermes/config/managed-policy.ts | 1 + agents/hermes/plugin/__init__.py | 66 +++ .../plugin/googlechat_sandbox_adapter.py | 478 ++++++++++++++++++ .../sandbox/policy-channel-agent-gate.test.ts | 12 +- src/lib/actions/sandbox/policy-channel.ts | 2 + .../applier/build/messaging-build-applier.mts | 13 +- .../messaging/applier/setup-applier.test.ts | 9 + .../messaging/channels/googlechat/manifest.ts | 148 +++++- .../channels/googlechat/policy/hermes.yaml | 48 ++ .../googlechat/provider-profile/hermes.yaml | 77 +++ .../googlechat/runtime-contract.test.ts | 8 +- .../googlechat/template-resolver.test.ts | 36 ++ .../channels/googlechat/template-resolver.ts | 18 +- src/lib/messaging/channels/manifests.test.ts | 3 +- src/lib/messaging/channels/metadata.test.ts | 24 + src/lib/messaging/utils.test.ts | 1 + .../onboard/messaging-bridge-provider.test.ts | 1 + src/lib/onboard/messaging-bridge-provider.ts | 19 +- src/lib/onboard/messaging-prep.ts | 1 + test/managed-image-capability-union.test.ts | 5 +- 20 files changed, 951 insertions(+), 19 deletions(-) create mode 100644 agents/hermes/plugin/googlechat_sandbox_adapter.py create mode 100644 src/lib/messaging/channels/googlechat/policy/hermes.yaml create mode 100644 src/lib/messaging/channels/googlechat/provider-profile/hermes.yaml diff --git a/agents/hermes/config/managed-policy.ts b/agents/hermes/config/managed-policy.ts index d95a6db43e2..0e0e5da39bd 100644 --- a/agents/hermes/config/managed-policy.ts +++ b/agents/hermes/config/managed-policy.ts @@ -46,6 +46,7 @@ export const MANAGED_IMAGE_HERMES_SUPPORTED_PLATFORMS = [ "slack", "whatsapp", "teams", + "google_chat", ] as const; // Hermes v0.19.0 also packages platform plugins and built-in adapters that are diff --git a/agents/hermes/plugin/__init__.py b/agents/hermes/plugin/__init__.py index a187ac48b06..41ef29533b7 100644 --- a/agents/hermes/plugin/__init__.py +++ b/agents/hermes/plugin/__init__.py @@ -7,6 +7,10 @@ and quiet runtime grounding when Hermes runs inside an OpenShell sandbox managed by NemoClaw. +Layout: channel-specific runtime overrides live in sibling modules loaded by +register() only when that channel is configured, so a sandbox without the +channel carries none of its behavior. Today that is googlechat_sandbox_adapter.py. + Skill hot-reload: Hermes caches its skill slash-command registry in a module-global dict on first scan. New skills dropped on disk are invisible until the cache is cleared. This plugin provides a nemoclaw_reload_skills @@ -22,9 +26,11 @@ """ import atexit +import importlib.util import inspect import ipaddress import json +import logging import os import re import subprocess @@ -96,6 +102,7 @@ "qqbot", "yuanbao", "webhook", + "google_chat", ) _RAW_MESSAGING_TOOL_RE = re.compile( r"^\s*send_message\s*:\s*(?P.+?)\s*$", @@ -1418,10 +1425,69 @@ def _handle_reload_skills(tool_input=None, context=None, **_kwargs): return "\n".join(lines) +# ───────────────────────────────────────────────────────────────────────────── +# Google Chat: keyless adapter override (NemoClaw) +# +# The override itself lives in the sibling ``googlechat_sandbox_adapter`` +# module — the same per-concern split Hermes' own plugins use +# (``plugins/platforms/discord`` ships adapter.py, recovery.py, and +# voice_mixer.py beside __init__.py). It is loaded only when the sandbox is +# configured for the Google Chat channel, so a Hermes sandbox without that +# channel never wraps the platform registry. +# ───────────────────────────────────────────────────────────────────────────── +_GOOGLE_CHAT_SUBSCRIPTION_ENV = "GOOGLE_CHAT_SUBSCRIPTION_NAME" +_GOOGLE_CHAT_MODULE = "googlechat_sandbox_adapter.py" + + +def _load_googlechat_adapter(): + """Load the sibling Google Chat override module by file path. + + Hermes loads this plugin as a directory module under a synthetic name + (``hermes_cli/plugins.py`` ``_load_directory_module``), so there is no + package context for a relative import; load the sibling the same way. + """ + path = os.path.join(os.path.dirname(__file__), _GOOGLE_CHAT_MODULE) + spec = importlib.util.spec_from_file_location( + "nemoclaw_hermes_googlechat_sandbox_adapter", + path, + ) + if spec is None or spec.loader is None: + return None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def _install_googlechat_sandbox_adapter(ctx): + """Install the keyless Google Chat override when that channel is configured. + + Gating on the rendered subscription keeps every other Hermes sandbox free of + the platform-registry wrap. Returns whether the override module was loaded + and invoked; the module reports its own registry-wrap failures. + """ + if not _get_env_value(_GOOGLE_CHAT_SUBSCRIPTION_ENV): + return False + try: + module = _load_googlechat_adapter() + except Exception: + # Load failure must not abort plugin registration, but it has to be + # visible: without the override the bundled gRPC adapter hangs under the + # REST-only egress policy and the channel goes quiet with no other clue. + logging.getLogger("gateway.platforms.google_chat").exception( + "[GoogleChat][NemoClaw] loading %s failed", _GOOGLE_CHAT_MODULE, + ) + return False + if module is None: + return False + module.install(ctx) + return True + + def register(ctx): """Register NemoClaw tools and hooks with Hermes.""" _install_nous_tool_broker_patch() _install_messaging_response_patch() + _install_googlechat_sandbox_adapter(ctx) # Register status tool ctx.register_tool( diff --git a/agents/hermes/plugin/googlechat_sandbox_adapter.py b/agents/hermes/plugin/googlechat_sandbox_adapter.py new file mode 100644 index 00000000000..79808f51ad8 --- /dev/null +++ b/agents/hermes/plugin/googlechat_sandbox_adapter.py @@ -0,0 +1,478 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Google Chat adapter for Hermes running inside a NemoClaw sandbox. + +Rebinds the bundled Hermes Google Chat adapter to the transports and +credentials a sandbox allows: events arrive over the Pub/Sub REST API instead +of gRPC, and replies leave through the OpenShell L7 proxy carrying a +gateway-minted placeholder instead of a locally signed token. + +Loaded by the NemoClaw plugin's ``register()`` only when the sandbox is +configured for the Google Chat channel, so a Hermes sandbox without that +channel never wraps the platform registry. Companion NemoClaw config lives in +``src/lib/messaging/channels/googlechat/`` (manifest, policy preset, provider +profile); the OpenClaw equivalent of this file is that channel's +``runtime/googlechat-outbound-auth.ts`` boot preload. + +Why the bundled transports are replaced +--------------------------------------- +Standalone Hermes receives Chat events over a gRPC Pub/Sub StreamingPull and +signs its bot token in-process from a service-account key. Neither survives +inside a NemoClaw sandbox: + +* **Inbound.** All sandbox egress traverses the OpenShell L7 proxy, whose + protocol set is ``rest, websocket, graphql, sql, json-rpc, mcp`` — there is no + gRPC variant, so ``protocol: grpc`` fails policy validation. Raw relay + (``tls: skip``) would carry the bytes but disables the inspection that the + credential swap depends on, and the policy engine rejects pairing an + inspecting middleware with a skipped endpoint. So this module PULLS the same + subscription over the Pub/Sub REST unary API (``:pull`` / ``:acknowledge``), + which the proxy can read. +* **Outbound.** The service-account key must stay out of the sandbox: the + gateway mints the token and the L7 proxy swaps the + ``openshell:resolve:env:GOOGLE_CHAT_ACCESS_TOKEN`` placeholder on the way out. + The bundled adapter has no seam for a pre-minted token and hardcodes + ``httplib2.Http()``, which cannot proxy HTTPS here, so this module supplies + placeholder credentials plus an aiohttp transport. + +This is not a stopgap awaiting gRPC support: L7 inspection and gRPC are in +tension by design, so keyless Google Chat would not get simpler if OpenShell +added a gRPC protocol variant. Hermes does also support an inbound HTTP-events +webhook, which would remove the inbound half of this override but not the +outbound half, at the cost of a public inbound URL; NemoClaw keeps pull. + +Override mechanism +------------------ +This is a registered-adapter wrap via the sanctioned ``platform_registry`` +seam, NOT a runtime monkeypatch of the bundled module. ``register`` is wrapped +so the bundled ``google_chat`` entry is kept as-is except for its +``adapter_factory`` (bound to the keyless methods per instance), +``check_fn`` and ``required_env``. Everything else the bundled entry carries — +``env_enablement_fn``, ``validate_config``, ``is_connected``, +``allowed_users_env``, ``standalone_sender_fn``, ``platform_hint`` — is +preserved, and binding onto the instance keeps the override immune to the +synthetic module identity of the lazily loaded bundled plugin. + +B300 live-validation points (managed image, not reproducible on a standalone +Hermes host): + +* the registry wrap supersedes the bundled deferred loader in the managed-image + plugin-load order (same runtime HERMES_HOME scope); +* ``from plugins.platforms.google_chat.adapter import GoogleChatAdapter`` + resolves under /opt/hermes in the managed image; +* the base ``connect()``'s ``SubscriberClient(credentials=placeholder)`` + constructs without egress (it is never ``.subscribe()``'d); +* ``AuthorizedHttp(placeholder_creds)`` on the reply path emits the placeholder + bearer for the L7 swap. +""" + +import asyncio +import importlib.util +import logging +import types + +_GC_REST_PLACEHOLDER_TOKEN = "openshell:resolve:env:GOOGLE_CHAT_ACCESS_TOKEN" +_GC_PULL_TIMEOUT = 95.0 # aiohttp cap > the Pub/Sub server long-poll hold (~90s) +_GC_LOG = logging.getLogger("gateway.platforms.google_chat") + + +class _RestPubsubMessage: + """Adapt a Pub/Sub REST receivedMessage to the four members the inherited + _on_pubsub_message touches: .data (bytes), .attributes (dict), .ack(), .nack().""" + + def __init__(self, pmsg, ack_sink, ack_id): + import base64 + + self._ack_sink = ack_sink + self._ack_id = ack_id + raw = pmsg.get("data") or "" + self.data = base64.b64decode(raw) if raw else b"" + self.attributes = pmsg.get("attributes") or {} + + def ack(self): + if self._ack_id: + self._ack_sink.append(self._ack_id) + + def nack(self): + # Omit the ackId from the batch -> Pub/Sub redelivers, matching the + # streaming client's message.nack() semantics. + pass + + +async def _gc_rest_pull_supervisor(self): + """Drop-in replacement for GoogleChatAdapter._run_supervisor that pulls via the + Pub/Sub REST unary API instead of gRPC StreamingPull, then feeds each message + into the UNCHANGED self._on_pubsub_message (run in a worker thread to preserve + the 'callback off the event loop' contract). Keyless: the bearer is the + placeholder the L7 proxy rewrites, so nothing is signed in-process and this path + needs no google.auth.""" + import aiohttp + + sub = self._subscription_path + pull_url = f"https://pubsub.googleapis.com/v1/{sub}:pull" + ack_url = f"https://pubsub.googleapis.com/v1/{sub}:acknowledge" + headers = { + "Authorization": f"Bearer {_GC_REST_PLACEHOLDER_TOKEN}", + "Content-Type": "application/json", + } + _GC_LOG.info( + "[GoogleChat][NemoClaw] keyless Pub/Sub REST :pull transport active (sub=%s)", + sub, + ) + async with aiohttp.ClientSession(trust_env=True) as session: + while not self._shutting_down: + try: + async with session.post( + pull_url, + json={"maxMessages": self._max_messages or 1}, + headers=headers, + timeout=aiohttp.ClientTimeout(total=_GC_PULL_TIMEOUT), + ) as resp: + if resp.status != 200: + body = await resp.text() + _GC_LOG.warning( + "[GoogleChat][NemoClaw] :pull HTTP %s: %s", + resp.status, + body[:200], + ) + await asyncio.sleep(3) + continue + payload = await resp.json() + except asyncio.CancelledError: + raise + except Exception as exc: # noqa: BLE001 - transient pull errors are retried + _GC_LOG.warning("[GoogleChat][NemoClaw] :pull error: %s", exc) + await asyncio.sleep(3) + continue + + received = payload.get("receivedMessages") or [] + if not received: + await asyncio.sleep(0.2) # guard against fast-empty long-poll returns + continue + + acks: list = [] + for received_message in received: + shim = _RestPubsubMessage( + received_message.get("message") or {}, + acks, + received_message.get("ackId"), + ) + try: + await asyncio.to_thread(self._on_pubsub_message, shim) + except Exception: # noqa: BLE001 - one bad message must not kill the loop + _GC_LOG.exception("[GoogleChat][NemoClaw] message handler raised") + + if acks: + try: + async with session.post( + ack_url, + json={"ackIds": acks}, + headers=headers, + timeout=aiohttp.ClientTimeout(total=30), + ) as ack_resp: + if ack_resp.status != 200: + _GC_LOG.warning( + "[GoogleChat][NemoClaw] :acknowledge HTTP %s", + ack_resp.status, + ) + except Exception as exc: # noqa: BLE001 - a failed ack just redelivers + _GC_LOG.warning("[GoogleChat][NemoClaw] :acknowledge error: %s", exc) + + +def _gc_placeholder_credentials(): + """A google.auth Credentials whose token is the OpenShell placeholder the L7 + proxy rewrites to the gateway-minted bearer. refresh() is a no-op so nothing is + signed in-process. Feeds both the idle SubscriberClient built by the inherited + connect() and the reply path's AuthorizedSession, so both carry the placeholder.""" + from google.auth import credentials as ga_credentials + + class _PlaceholderCredentials(ga_credentials.Credentials): + def __init__(self): + super().__init__() + self.token = _GC_REST_PLACEHOLDER_TOKEN + + def refresh(self, request): # signed gateway-side; nothing to do here + self.token = _GC_REST_PLACEHOLDER_TOKEN + + return _PlaceholderCredentials() + + +async def _gc_rest_pull_connect(self, *, is_reconnect: bool = False) -> bool: + """Keyless REST-pull ``connect()``: the bundled ``connect()`` MINUS the gRPC + Pub/Sub ``SubscriberClient`` + ``get_subscription()`` sanity check, which hangs + for 30 s under the REST-only OpenShell egress policy (gRPC is blocked), and + MINUS the legacy single-user OAuth probe (``__init__`` already defaults those to + ``None``; attachments degrade to text). The REST ``:pull`` supervisor surfaces a + bad subscription at first pull instead of a gRPC precheck. Everything else + mirrors the bundled ``connect()``: lazy google-module load, config validate, + placeholder creds, Chat REST client, thread-count store, bot-id resolution, then + start the REST-pull supervisor.""" + import asyncio as _asyncio + + import plugins.platforms.google_chat.adapter as _gc + + if not _gc._load_google_modules(): + self._set_fatal_error( + code="missing_deps", + message="google-cloud-pubsub / google-api-python-client not installed", + retryable=False, + ) + return False + + self._loop = _asyncio.get_running_loop() + try: + project_id, subscription_path = self._validate_config() + credentials = self._load_sa_credentials() + except (ValueError, FileNotFoundError) as exc: + msg = _gc._redact_sensitive(str(exc)) + _gc.logger.error("[GoogleChat] Config validation failed: %s", msg) + self._set_fatal_error(code="config_invalid", message=msg, retryable=False) + return False + + self._project_id = project_id + self._subscription_path = subscription_path + self._credentials = credentials + + try: + self._chat_api = await _asyncio.to_thread( + lambda: _gc.build_service( + "chat", "v1", credentials=credentials, cache_discovery=False + ) + ) + except Exception as exc: # noqa: BLE001 + msg = _gc._redact_sensitive(str(exc)) + _gc.logger.error("[GoogleChat] Failed to build Chat API client: %s", msg) + self._set_fatal_error(code="chat_api_init", message=msg, retryable=False) + return False + + try: + await _asyncio.to_thread(self._thread_count_store.load) + except Exception: # noqa: BLE001 + _gc.logger.warning( + "[GoogleChat] thread-count store load failed (treating all threads as fresh)", + exc_info=True, + ) + + # SKIP bundled gRPC block (pubsub_v1.SubscriberClient + get_subscription()): + # gRPC is denied by the REST-only egress policy and hangs; the keyless REST + # :pull loop never touches self._subscriber (stays None; disconnect() guards it). + + self._bot_user_id = self._load_cached_bot_id() + if not self._bot_user_id: + self._bot_user_id = await self._resolve_bot_user_id() + if self._bot_user_id: + self._save_cached_bot_id(self._bot_user_id) + else: + _gc.logger.info( + "[GoogleChat] bot_user_id not yet resolved; will resolve on first " + "addedToSpace or member lookup" + ) + + if subscription_path is not None: + self._supervisor_task = _asyncio.create_task(self._run_supervisor()) + else: + self._supervisor_task = None + + self._mark_connected() + _gc.logger.info( + "[GoogleChat][NemoClaw] Connected keyless (REST :pull, no gRPC subscriber); " + "project=%s, subscription=%s, bot_user_id=%s", + project_id or "", + "" if subscription_path else "", + self._bot_user_id or "", + ) + return True + + +def _gc_gateway_proxy_url(): + """The OpenShell egress proxy URL, read from the ``hermes.real gateway`` + process's IMMUTABLE ``/proc//environ``. os.environ is unreliable (the gateway + clears the proxy vars during an agent turn), and ``/proc/self`` is NOT the gateway + in the reply's execution context — the Chat reply runs under ``asyncio.to_thread`` + whose ``/proc/self/environ`` does not carry the launcher-set proxy. So locate the + gateway process by cmdline and read its startup env, which keeps the proxy for the + life of the process. Returns ``""`` when no proxy is found (direct egress).""" + import glob + + for cmd_path in glob.glob("/proc/[0-9]*/cmdline"): + try: + cmdline = open(cmd_path, "rb").read() + except OSError: + continue + if b"hermes.real" not in cmdline or b"gateway" not in cmdline or b"dashboard" in cmdline: + continue + try: + with open(cmd_path.rsplit("/", 1)[0] + "/environ", "rb") as handle: + env = {} + for pair in handle.read().split(b"\0"): + key, sep, value = pair.partition(b"=") + if sep: + env[key.decode("utf-8", "replace")] = value.decode("utf-8", "replace") + except OSError: + continue + url = ( + env.get("https_proxy") + or env.get("HTTPS_PROXY") + or env.get("http_proxy") + or env.get("HTTP_PROXY") + or "" + ) + if url: + return url + return "" + + +class _GcAiohttpTransport: + """An ``httplib2.Http``-compatible transport (``.request()`` only) that routes + Chat REST calls through the OpenShell L7 proxy via aiohttp — the ONLY transport + that works from the gateway process in this sandbox. + + The bundled Chat client is google-api-python-client over httplib2, whose HTTPS + proxy support is unusable here: without PySocks ``ProxyInfo.isgood()`` is falsy, so + httplib2 silently connects DIRECT — which the proxy-only sandbox netns cannot + resolve (name-resolution failure); WITH PySocks its ``PROXY_TYPE_HTTP`` rejects the + CONNECT tunnel. aiohttp CONNECT-tunnels through the proxy exactly as the inbound + ``:pull`` does, and the gateway process is the one the proxy authorizes and injects + the minted bot token for (ad-hoc processes are refused at CONNECT). The proxy is + read from the gateway's ``/proc//environ`` (os.environ is unreliable), and TLS + trusts the system CA bundle, which carries the proxy's MITM root. googleapiclient + only calls ``.request(uri, method, body, headers)`` and reads ``resp.status`` + + content, which this provides via an ``httplib2.Response``.""" + + def request(self, uri, method="GET", body=None, headers=None, **kwargs): + import asyncio + + import aiohttp + + import plugins.platforms.google_chat.adapter as _gc + + proxy = _gc_gateway_proxy_url() or None + data = body.encode("utf-8") if isinstance(body, str) else body + + async def _run(): + # Mirror the inbound :pull exactly: ClientSession(trust_env=True) with no + # ssl override. That is the transport the L7 proxy authorizes for this + # gateway process, and its default TLS trust already accepts the proxy's + # MITM chain (pinning a hand-built CA context rejected it). The proxy is + # still passed explicitly because os.environ may be cleared by reply time. + async with aiohttp.ClientSession(trust_env=True) as session: + async with session.request( + method, + uri, + data=data, + headers=dict(headers or {}), + proxy=proxy, + timeout=aiohttp.ClientTimeout(total=60), + ) as resp: + content = await resp.read() + info = {"status": resp.status} + for key, value in resp.headers.items(): + info[key.lower()] = value + return _gc.httplib2.Response(info), content + + # The Chat calls run under ``asyncio.to_thread`` (a worker thread with no + # running event loop), so a fresh loop here via asyncio.run is safe. + return asyncio.run(_run()) + + +def _gc_rest_reply_http(self): + """``_new_authed_http()`` override for the keyless Chat reply path. + + Returns ``AuthorizedHttp(placeholder_creds, http=_GcAiohttpTransport())``. The + bundled ``httplib2.Http`` cannot proxy HTTPS in this sandbox (see + ``_GcAiohttpTransport``), so every Chat call that flows through + ``_new_authed_http`` — reply create, message patch, typing card, bot-id lookup — + is routed via aiohttp instead. Credentials stay the placeholder creds bound in + connect(); ``AuthorizedHttp`` adds the placeholder bearer header that the L7 proxy + swaps for the minted bot token, exactly as on the ``:pull`` path.""" + import plugins.platforms.google_chat.adapter as _gc + + return _gc.AuthorizedHttp(self._credentials, http=_GcAiohttpTransport()) + + +def _gc_bind_overrides(adapter): + """Bind the keyless REST-pull behavior onto a bundled GoogleChatAdapter + instance: a connect() that skips the gRPC subscriber precheck, the REST `:pull` + supervisor (inbound), placeholder credentials, and a reply transport routed + through the L7 proxy with the system CA (outbound Chat REST). Binding onto the + instance with types.MethodType — rather than subclassing + re-registering — keeps + every other bundled method AND the bundled registry metadata (env_enablement_fn + seeds project_id/subscription into PlatformConfig.extra, validate_config, …) + intact, and is immune to the synthetic module identity of the lazily loaded + bundled plugin.""" + adapter.connect = types.MethodType(_gc_rest_pull_connect, adapter) + adapter._run_supervisor = types.MethodType(_gc_rest_pull_supervisor, adapter) + adapter._load_sa_credentials = types.MethodType( + lambda self: _gc_placeholder_credentials(), adapter + ) + adapter._new_authed_http = types.MethodType(_gc_rest_reply_http, adapter) + return adapter + + +def _gc_spec_available(module_name: str) -> bool: + try: + return importlib.util.find_spec(module_name) is not None + except (ImportError, ValueError): + return False + + +def _nemoclaw_gc_check() -> bool: + """Passive dependency probe (side-effect free — no heavy import): the keyless + REST-pull path needs aiohttp; the inherited connect()/reply path needs the + google-auth + pubsub SDKs shipped in the managed image.""" + return all( + _gc_spec_available(module_name) + for module_name in ("aiohttp", "google.auth", "google.cloud.pubsub_v1") + ) + + +def install(ctx) -> None: + """Make the bundled Google Chat adapter run keyless REST-pull WITHOUT dropping + the bundled registry entry's config-seeding metadata. + + A fresh ``register_platform`` (last-writer-wins) loses the bundled entry's + ``env_enablement_fn`` — the hook that seeds ``PlatformConfig.extra.project_id`` + / ``subscription_name`` from ``GOOGLE_CHAT_*`` env — so connect() fails with + "GOOGLE_CHAT_PROJECT_ID is not set". Instead wrap ``platform_registry.register`` + so when the bundled ``google_chat`` entry registers (its deferred loader runs at + gateway start) it is KEPT as-is except: (a) ``adapter_factory`` is wrapped to + bind the REST-pull supervisor + placeholder credentials onto each instance, and + (b) ``check_fn`` / ``required_env`` are swapped to the keyless contract (no SA + JSON in the sandbox). Every other field — env_enablement_fn, validate_config, + is_connected, allowed_users_env, standalone_sender_fn, platform_hint, … — is + preserved. ``ctx`` is unused; the registry is the single override point.""" + del ctx + try: + from gateway.platform_registry import platform_registry as _preg + except Exception: # noqa: BLE001 - keep plugin load resilient + _GC_LOG.exception("[GoogleChat][NemoClaw] platform_registry import failed") + return + if getattr(_preg, "_nemoclaw_gc_wrapped", False): + return + _orig_register = _preg.register + + def _wrapped_register(entry, *args, **kwargs): + try: + if getattr(entry, "name", None) == "google_chat": + _orig_factory = entry.adapter_factory + + def _factory(cfg, _f=_orig_factory): + try: + return _gc_bind_overrides(_f(cfg)) + except Exception: # noqa: BLE001 - fall back to the stock adapter + _GC_LOG.exception("[GoogleChat][NemoClaw] instance override failed") + return _f(cfg) + + entry.adapter_factory = _factory + entry.check_fn = _nemoclaw_gc_check + entry.required_env = ["GOOGLE_CHAT_SUBSCRIPTION_NAME"] + _GC_LOG.info( + "[GoogleChat][NemoClaw] wrapped bundled google_chat adapter " + "(keyless REST-pull; bundled metadata preserved)" + ) + except Exception: # noqa: BLE001 - never let the wrap abort a registration + _GC_LOG.exception("[GoogleChat][NemoClaw] register wrap failed") + return _orig_register(entry, *args, **kwargs) + + _preg.register = _wrapped_register + _preg._nemoclaw_gc_wrapped = True diff --git a/src/lib/actions/sandbox/policy-channel-agent-gate.test.ts b/src/lib/actions/sandbox/policy-channel-agent-gate.test.ts index 3b9f0a9e17a..984e78e45f1 100644 --- a/src/lib/actions/sandbox/policy-channel-agent-gate.test.ts +++ b/src/lib/actions/sandbox/policy-channel-agent-gate.test.ts @@ -156,8 +156,10 @@ describe("channel lifecycle agent gate", () => { ["start", ["googlechat"], () => startSandboxChannel("da-test", { channel: "googlechat" })], ["stop", [], () => stopSandboxChannel("da-test", { channel: "googlechat" })], ])("rejects a stale channel during %s before reading channel state or mutating the sandbox", async (_verb, disabledChannels, run) => { - getSandboxMock.mockReturnValue({ name: "da-test", agent: "hermes" }); - vi.spyOn(defs, "loadAgent").mockReturnValue(agentFixture("hermes")); + // googlechat now supports openclaw + hermes, so exercise the unsupported-pair + // lifecycle gate with a non-messaging custom agent (supported by no channel). + getSandboxMock.mockReturnValue({ name: "da-test", agent: "custom-agent" }); + vi.spyOn(defs, "loadAgent").mockReturnValue(agentFixture("custom-agent")); const configuredChannelsMock = vi .spyOn(registry, "getConfiguredMessagingChannelsFromEntry") .mockReturnValue(["googlechat"]); @@ -176,9 +178,9 @@ describe("channel lifecycle agent gate", () => { const errorText = (errSpy.mock.calls as unknown[][]) .map((call) => call.map(String).join(" ")) .join("\n"); - expect(errorText).toMatch(/Channel 'googlechat' does not support agent 'hermes'/); - expect(errorText).toMatch(/Channel-supported agents: openclaw/); - expect(errorText).toMatch(/Channels supported by agent 'hermes':/); + expect(errorText).toMatch(/Channel 'googlechat' does not support agent 'custom-agent'/); + expect(errorText).toMatch(/Channel-supported agents: openclaw, hermes/); + expect(errorText).toMatch(/Channels supported by agent 'custom-agent': \(none\)/); expect(configuredChannelsMock).not.toHaveBeenCalled(); expect(disabledChannelsMock).not.toHaveBeenCalled(); diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index 8194e1b42c4..67c021dd587 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -855,8 +855,10 @@ async function applyChannelAddToGatewayAndRegistry( // nothing for them. Their provider must be created HERE (same seam onboarding // uses): the pasted secret is env-only and gone once this process exits, so a // deferred rebuild cannot configure it. + const bridgeAgent = registry.getSandbox(sandboxName)?.agent === "hermes" ? "hermes" : "openclaw"; const bridgeDefs = collectMessagingBridgeTokenDefs({ sandboxName, + agent: bridgeAgent, enabledChannels: [channelName], disabledChannelNames: new Set(), getCredential, diff --git a/src/lib/messaging/applier/build/messaging-build-applier.mts b/src/lib/messaging/applier/build/messaging-build-applier.mts index 08639e9674b..422f0d15e2b 100755 --- a/src/lib/messaging/applier/build/messaging-build-applier.mts +++ b/src/lib/messaging/applier/build/messaging-build-applier.mts @@ -1882,7 +1882,18 @@ function installHermesUvPackages(selectedPackages: readonly string[], env: Env): "--", ...selectedPackages, ], - env, + // uv (rustls/webpki) ignores the corporate-only SSL_CERT_FILE the build step + // exports, so a PyPI fetch behind a corporate MITM proxy fails with + // `invalid peer certificate: UnknownIssuer`. Point uv at the full system + // bundle (public roots + corporate CA, merged into /etc/ssl/certs by + // `update-ca-certificates`, Dockerfile #6210) and enable UV_SYSTEM_CERTS so + // uv trusts it. (uv renamed UV_NATIVE_TLS → UV_SYSTEM_CERTS.) Harmless + // off-proxy — the same bundle still carries the public roots. + { + ...env, + UV_SYSTEM_CERTS: "1", + SSL_CERT_FILE: "/etc/ssl/certs/ca-certificates.crt", + }, ); } diff --git a/src/lib/messaging/applier/setup-applier.test.ts b/src/lib/messaging/applier/setup-applier.test.ts index 1484422577f..24a896ac960 100644 --- a/src/lib/messaging/applier/setup-applier.test.ts +++ b/src/lib/messaging/applier/setup-applier.test.ts @@ -57,6 +57,14 @@ const ALL_CHANNEL_ENV = { MSTEAMS_TENANT_ID: "teams-tenant-id", TEAMS_ALLOWED_USERS: "00000000-0000-0000-0000-000000000001", MSTEAMS_PORT: "3978", + // The service-account validator only requires non-empty client_email and + // private_key, so keep the fixture free of PEM markers: a real-looking key + // block trips the detect-private-key pre-commit hook. + GOOGLECHAT_SERVICE_ACCOUNT: + '{"client_email":"bot@demo.iam.gserviceaccount.com","private_key":"test-key-material","project_id":"demo"}', + GOOGLE_CHAT_PROJECT_ID: "demo", + GOOGLE_CHAT_SUBSCRIPTION_NAME: "projects/demo/subscriptions/hermes-chat-events-sub", + GOOGLECHAT_ALLOWED_USERS: "user@example.com", } as const; const ALL_CHANNELS = createBuiltInChannelManifestRegistry() @@ -621,6 +629,7 @@ describe("MessagingSetupApplier", () => { "slack", "whatsapp", "teams", + "googlechat_hermes", ]); expect(renderResult.appliedTargets).toEqual([ "/sandbox/.hermes/.env", diff --git a/src/lib/messaging/channels/googlechat/manifest.ts b/src/lib/messaging/channels/googlechat/manifest.ts index 6e353995336..e91f241efdb 100644 --- a/src/lib/messaging/channels/googlechat/manifest.ts +++ b/src/lib/messaging/channels/googlechat/manifest.ts @@ -31,7 +31,7 @@ export const googlechatManifest = { "┃ GOOGLECHAT_APP_PRINCIPAL= nemoclaw channels add googlechat", "┃ nemoclaw rebuild --yes", ], - supportedAgents: ["openclaw"], + supportedAgents: ["openclaw", "hermes"], auth: { mode: "token-paste", }, @@ -106,11 +106,48 @@ export const googlechatManifest = { envKey: "GOOGLECHAT_ALLOWED_USERS", statePath: "allowedIds.googlechat", prompt: { - label: "Google Chat DM allowlist (comma-separated user IDs)", - help: "Optional: restrict who can DM the bot. Enter Google Chat user IDs (users/NNN) — NOT emails: the bot matches IDs only by default, so an email entry is ignored. Leave blank to require pairing (recommended).", + label: "Google Chat DM allowlist (comma-separated)", + help: [ + "Optional: restrict who can DM the bot.", + " OpenClaw: users/NNN (emails ignored)", + " Hermes: email (users/NNN ignored)", + " Blank: pairing mode (recommended) — OpenClaw's pairing reply shows your users/NNN", + " Filling this switches DM policy to allowlist — a wrong-form entry is dropped silently, with no pairing code.", + ].join("\n"), emptyValueMessage: "bot will require manual pairing", }, }, + // ── Hermes-only Pub/Sub pull config ── + // OpenClaw receives Chat events on an inbound webhook and never reads these. + // Hermes' adapter also supports an inbound HTTP-events webhook, but NemoClaw + // runs it in Pub/Sub pull mode instead: pulling needs no public inbound URL. + // It PULLS events from a Pub/Sub subscription bound to the Chat topic, so it + // needs the project + subscription. Prompted only under the hermes-gated + // hook and rendered only into ~/.hermes/.env. + { + id: "projectId", + kind: "config", + required: false, + envKey: "GOOGLE_CHAT_PROJECT_ID", + statePath: "googlechatConfig.projectId", + prompt: { + label: "Google Chat GCP project ID (Hermes Pub/Sub pull)", + help: "The Google Cloud project that owns the Pub/Sub subscription Hermes pulls Chat events from. OpenClaw ignores this.", + emptyValueMessage: "required for the Hermes Google Chat channel", + }, + }, + { + id: "subscriptionName", + kind: "config", + required: false, + envKey: "GOOGLE_CHAT_SUBSCRIPTION_NAME", + statePath: "googlechatConfig.subscriptionName", + prompt: { + label: "Google Chat Pub/Sub subscription (projects/

/subscriptions/)", + help: "The pull subscription bound to the Chat events topic. Hermes pulls from it over the Pub/Sub REST API; the gateway-minted token is scoped to both chat.bot and pubsub.", + emptyValueMessage: "required for the Hermes Google Chat channel", + }, + }, ], // Outbound auth is gateway-minted: the OpenShell `google-service-account-jwt` // refresh provider mints the Google Chat bot token from the pasted service @@ -129,7 +166,18 @@ export const googlechatManifest = { // clears only per-channel sandbox tokens and is intentionally a no-op for a bridge // channel, so an empty `credentials` does not leave the service account behind. credentials: [], - policyPresets: [{ name: "googlechat", policyKeys: ["googlechat"] }], + policyPresets: [ + { + name: "googlechat", + policyKeys: ["googlechat"], + // Hermes reaches Google Chat over Pub/Sub REST pull + Chat REST reply — a + // different egress shape from OpenClaw's inbound webhook — so it resolves a + // distinct concrete policy key backed by policy/hermes.yaml. + agentPolicyKeys: { + hermes: ["googlechat_hermes"], + }, + }, + ], render: [ { id: "googlechat-openclaw-channel", @@ -201,6 +249,36 @@ export const googlechatManifest = { }, }, }, + // ── Hermes render ── + // No SA JSON and no access-token env are delivered: the keyless Google Chat + // bridge provider (provider-profile/hermes.yaml) mints a chat.bot+pubsub + // token gateway-side, and the NemoClaw Google Chat adapter override emits the + // `openshell:resolve:env:GOOGLE_CHAT_ACCESS_TOKEN` placeholder on both the + // Pub/Sub :pull and the Chat reply, which the L7 proxy swaps for the minted + // bearer. Only non-secret pull config + the allowlist reach the sandbox here. + { + id: "googlechat-hermes-env", + kind: "env-lines", + agent: "hermes", + target: "~/.hermes/.env", + lines: [ + "GOOGLE_CHAT_PROJECT_ID={{googlechatConfig.projectId}}", + "GOOGLE_CHAT_SUBSCRIPTION_NAME={{googlechatConfig.subscriptionName}}", + "GOOGLE_CHAT_ALLOWED_USERS={{allowedIds.googlechat.csv}}", + ], + }, + { + id: "googlechat-hermes-platform", + kind: "json-fragment", + agent: "hermes", + target: "~/.hermes/config.yaml", + fragment: { + path: "platforms.google_chat", + value: { + enabled: true, + }, + }, + }, ], runtime: { openclaw: { @@ -275,12 +353,45 @@ export const googlechatManifest = { }, required: true, }, + // Hermes keyless REST-pull adapter: the inherited connect() constructs a + // (never-subscribed) Pub/Sub SubscriberClient and the reply path uses + // google-auth, so the managed image must ship these. The base image has + // aiohttp (used by the REST :pull loop) but not the google-* SDKs. + // The adapter override itself is Hermes-side runtime code and lives in + // agents/hermes/plugin/googlechat_sandbox_adapter.py, loaded by that + // plugin only when GOOGLE_CHAT_SUBSCRIPTION_NAME is rendered below. + { + id: "hermesGooglePubsubPackage", + agent: "hermes", + manager: "hermes-uv-pip", + spec: "google-cloud-pubsub==2.39.0", + required: true, + }, + { + id: "hermesGoogleApiClientPackage", + agent: "hermes", + manager: "hermes-uv-pip", + spec: "google-api-python-client==2.194.0", + required: true, + }, + { + id: "hermesGoogleAuthPackage", + agent: "hermes", + manager: "hermes-uv-pip", + spec: "google-auth==2.55.1", + required: true, + }, ], hooks: [ { + // OpenClaw-only: this gates the inbound webhook audience. NemoClaw runs + // Hermes in Pub/Sub pull mode, which serves no inbound webhook, so this + // skip-channel gate must not run for Hermes (it would otherwise skip the + // channel and drop its policy preset). id: "googlechat-tunnel-audience-gate", phase: "enroll", handler: "googlechat.tunnelAudienceGate", + agents: ["openclaw"], inputs: ["audienceType", "audience"], outputs: [ { @@ -307,13 +418,40 @@ export const googlechatManifest = { id: "googlechat-config-prompt", phase: "enroll", handler: "common.configPrompt", + outputs: [ + { + id: "allowFrom", + kind: "config", + }, + ], + }, + { + // OpenClaw-only: appPrincipal is the add-on's OAuth principal for inbound + // webhook verification — meaningless to Hermes pull mode. + id: "googlechat-openclaw-config-prompt", + phase: "enroll", + handler: "common.configPrompt", + agents: ["openclaw"], outputs: [ { id: "appPrincipal", kind: "config", }, + ], + }, + { + // Hermes-only: collect the Pub/Sub project + subscription for pull mode. + id: "googlechat-hermes-config-prompt", + phase: "enroll", + handler: "common.configPrompt", + agents: ["hermes"], + outputs: [ + { + id: "projectId", + kind: "config", + }, { - id: "allowFrom", + id: "subscriptionName", kind: "config", }, ], diff --git a/src/lib/messaging/channels/googlechat/policy/hermes.yaml b/src/lib/messaging/channels/googlechat/policy/hermes.yaml new file mode 100644 index 00000000000..d00a93effa9 --- /dev/null +++ b/src/lib/messaging/channels/googlechat/policy/hermes.yaml @@ -0,0 +1,48 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +preset: + name: googlechat + description: "Hermes Google Chat: Pub/Sub REST pull for inbound events + Chat REST reply; the bot token is minted gateway-side (chat.bot + pubsub), never in the sandbox" + +network_policies: + googlechat_hermes: + name: googlechat_hermes + endpoints: + # Inbound: pull Chat events from the Pub/Sub subscription over the REST + # unary API (:pull, :acknowledge, :modifyAckDeadline). Not gRPC + # StreamingPull — REST keeps the egress L7-inspectable so the + # gateway-minted token is injected by the proxy (keyless). The + # subscription lives under /v1/projects/{project}/subscriptions/{sub}. + - host: pubsub.googleapis.com + port: 443 + protocol: rest + enforcement: enforce + rules: + # Pub/Sub REST pull uses Google custom-method verbs with a colon suffix: + # `/v1/projects/{p}/subscriptions/{s}:pull` / `:acknowledge` / + # `:modifyAckDeadline`. Match the whole /v1 tree so the `:verb` suffix + # is covered (a `/v1/projects/**` glob can miss the trailing `:pull`). + - allow: { method: POST, path: "/v1/**" } + # Outbound reply: send/update/delete messages in Chat spaces. Same rule + # shape as the OpenClaw preset — writes scoped to the Chat `spaces` tree, + # reads across the Chat REST v1 tree. + - host: chat.googleapis.com + port: 443 + protocol: rest + enforcement: enforce + rules: + - allow: { method: GET, path: "/v1/**" } + - allow: { method: POST, path: "/v1/spaces/**" } + - allow: { method: PATCH, path: "/v1/spaces/**" } + - allow: { method: DELETE, path: "/v1/spaces/**" } + # OpenShell matches the egressing binary by its resolved realpath and + # compares exactly (no glob). Hermes' venv python is a symlink chain + # `/opt/hermes/.venv/bin/python` -> `/usr/bin/python3` -> `/usr/bin/python3.13`, + # so the concrete interpreter `/usr/bin/python3.13` must be listed or the + # egress is denied. Mirrors the provider-profile binary list. Revisit the + # minor version when the Hermes base image bumps Python. + binaries: + - { path: /opt/hermes/.venv/bin/python } + - { path: /usr/bin/python3 } + - { path: /usr/bin/python3.13 } diff --git a/src/lib/messaging/channels/googlechat/provider-profile/hermes.yaml b/src/lib/messaging/channels/googlechat/provider-profile/hermes.yaml new file mode 100644 index 00000000000..15a17e84dbc --- /dev/null +++ b/src/lib/messaging/channels/googlechat/provider-profile/hermes.yaml @@ -0,0 +1,77 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Google Chat outbound + Pub/Sub-pull bridge provider profile (Hermes, keyless). +# +# Hermes reaches Google Chat over the Pub/Sub REST pull API (inbound events) and +# the Chat REST API (outbound replies). The OpenShell gateway MINTS one bot access +# token from the service-account key (ProviderCredentialRefreshStrategy +# google_service_account_jwt) scoped to BOTH chat.bot and pubsub, and injects it as +# `Authorization: Bearer ` on pubsub.googleapis.com AND chat.googleapis.com. +# The service-account private key stays gateway-side; the sandbox only ever sees the +# `openshell:resolve:env:GOOGLE_CHAT_ACCESS_TOKEN` placeholder, which the L7 proxy +# rewrites to the minted token. +# +# Paired with the NemoClaw Google Chat adapter override (agents/hermes/plugin), which +# returns that placeholder for both the :pull and the Chat reply instead of signing a +# JWT in-process, so no service-account key ever enters the sandbox. +# +# The refresh material VALUES are supplied at onboard/rebuild time by +# src/lib/onboard/messaging-bridge-provider.ts. Non-secret values use `--material`; +# the private key uses `--secret-material-env` so it never appears in argv. The block +# below declares their shape so the gateway knows which keys are secret. +id: google-chat-hermes-bridge +display_name: Google Chat Bridge (Hermes) +description: Gateway-minted Google Chat bot token (chat.bot + pubsub) for Hermes Pub/Sub pull + Chat reply +category: agent +credentials: + - name: access_token + description: Google Chat bot OAuth access token (gateway-minted via service-account JWT; chat.bot + pubsub) + env_vars: + - GOOGLE_CHAT_ACCESS_TOKEN + required: true + auth_style: bearer + header_name: Authorization + query_param: '' + refresh: + strategy: google-service-account-jwt + scopes: + - https://www.googleapis.com/auth/chat.bot + - https://www.googleapis.com/auth/pubsub + material: + - name: client_email + description: Service-account client email (JWT issuer) + required: true + - name: private_key + description: Service-account RSA private key (PEM); signs the JWT assertion + required: true + secret: true + - name: scope + description: OAuth scope(s) to mint the token for +endpoints: + - host: pubsub.googleapis.com + port: 443 + protocol: rest + access: read-write + enforcement: enforce + - host: chat.googleapis.com + port: 443 + protocol: rest + access: read-write + enforcement: enforce +# OpenShell resolves the egressing process to its REAL interpreter via +# /proc/pid/exe, not the launched path. Hermes' venv python is a symlink chain +# `/opt/hermes/.venv/bin/python` -> `/usr/bin/python3` -> `/usr/bin/python3.13`, +# so the credential-reach gate sees `/usr/bin/python3.13`. The gate matches +# binaries EXACTLY: a `/usr/bin/python3*` glob does NOT cover `python3.13` +# (OpenShell's prover re-drafts it as a pending `credential_reach_expansion` +# and 403s the CONNECT), and the venv symlink is resolved away before the +# match. So the concrete interpreter path must be listed. `/usr/bin/python3` is +# the symlink OpenShell resolves FROM; `/usr/bin/python3.13` is the realpath it +# resolves TO and matches on. Revisit the minor version when the Hermes base +# image bumps Python. +binaries: + - /opt/hermes/.venv/bin/python + - /usr/bin/python3 + - /usr/bin/python3.13 +inference_capable: false diff --git a/src/lib/messaging/channels/googlechat/runtime-contract.test.ts b/src/lib/messaging/channels/googlechat/runtime-contract.test.ts index 11ed6b20963..c284cf22236 100644 --- a/src/lib/messaging/channels/googlechat/runtime-contract.test.ts +++ b/src/lib/messaging/channels/googlechat/runtime-contract.test.ts @@ -17,8 +17,12 @@ const googlechatPolicy = readFileSync(new URL("./policy/openclaw.yaml", import.m // wiring a refactor could otherwise weaken silently in the unit lane. function renderFragmentValue(path: string): Record { - const entry = googlechatManifest.render.find((fragment) => fragment.fragment.path === path); - return (entry?.fragment.value ?? {}) as Record; + const entry = googlechatManifest.render.find( + (render) => "fragment" in render && render.fragment.path === path, + ); + return entry && "fragment" in entry + ? ((entry.fragment.value ?? {}) as Record) + : {}; } describe("googlechat runtime security contract", () => { diff --git a/src/lib/messaging/channels/googlechat/template-resolver.test.ts b/src/lib/messaging/channels/googlechat/template-resolver.test.ts index 85cb3d53307..2e0e454235e 100644 --- a/src/lib/messaging/channels/googlechat/template-resolver.test.ts +++ b/src/lib/messaging/channels/googlechat/template-resolver.test.ts @@ -74,6 +74,42 @@ describe("Google Chat template resolver", () => { ).toBeUndefined(); }); + it("resolves the Hermes pull config + CSV allowlist; drops each when unset", () => { + const set: SandboxMessagingInputReference[] = [ + configInput("projectId", "googlechatConfig.projectId", "nemoclaw-project-500901"), + configInput( + "subscriptionName", + "googlechatConfig.subscriptionName", + "projects/nemoclaw-project-500901/subscriptions/hermes-chat-events-sub", + ), + configInput("allowFrom", "allowedIds.googlechat", "users/111, users/222"), + ]; + expect( + resolveGooglechatTemplateReference("googlechatConfig.projectId", { inputs: set })?.value, + ).toBe("nemoclaw-project-500901"); + expect( + resolveGooglechatTemplateReference("googlechatConfig.subscriptionName", { inputs: set }) + ?.value, + ).toBe("projects/nemoclaw-project-500901/subscriptions/hermes-chat-events-sub"); + expect( + resolveGooglechatTemplateReference("allowedIds.googlechat.csv", { inputs: set })?.value, + ).toBe("users/111,users/222"); + + // Unset → undefined so the render engine drops the env line (no literal + // "{{...}}" leaks into ~/.hermes/.env). + const empty: SandboxMessagingInputReference[] = []; + expect( + resolveGooglechatTemplateReference("googlechatConfig.projectId", { inputs: empty })?.value, + ).toBeUndefined(); + expect( + resolveGooglechatTemplateReference("googlechatConfig.subscriptionName", { inputs: empty }) + ?.value, + ).toBeUndefined(); + expect( + resolveGooglechatTemplateReference("allowedIds.googlechat.csv", { inputs: empty })?.value, + ).toBeUndefined(); + }); + it("returns undefined for references it does not own", () => { expect(resolveGooglechatTemplateReference("teamsConfig.appId", { inputs: [] })).toBeUndefined(); }); diff --git a/src/lib/messaging/channels/googlechat/template-resolver.ts b/src/lib/messaging/channels/googlechat/template-resolver.ts index 5d39ae69e13..b07ac0d9184 100644 --- a/src/lib/messaging/channels/googlechat/template-resolver.ts +++ b/src/lib/messaging/channels/googlechat/template-resolver.ts @@ -43,6 +43,17 @@ export const resolveGooglechatTemplateReference: BuiltInRenderTemplateResolver = nonEmptyString(stateValue(context, "googlechatConfig.appPrincipal")) ?? APP_PRINCIPAL_DISCOVERY_SENTINEL, ); + // Hermes Pub/Sub pull config, rendered into ~/.hermes/.env. Undefined when + // unset so the render engine drops the line entirely rather than emitting a + // literal "{{...}}" placeholder for the adapter to read as a subscription. + case "googlechatConfig.projectId": + return resolvedRenderTemplateReference( + nonEmptyString(stateValue(context, "googlechatConfig.projectId")), + ); + case "googlechatConfig.subscriptionName": + return resolvedRenderTemplateReference( + nonEmptyString(stateValue(context, "googlechatConfig.subscriptionName")), + ); default: break; } @@ -51,7 +62,7 @@ export const resolveGooglechatTemplateReference: BuiltInRenderTemplateResolver = // `allowFrom` key; `dmPolicy` resolving to undefined drops `dm.policy`. When // both drop, the empty `dm` object is removed by the render engine and // OpenClaw falls back to its default (pairing) DM policy. - const allowReference = reference.match(/^allowedIds[.]googlechat[.](values|dmPolicy)$/); + const allowReference = reference.match(/^allowedIds[.]googlechat[.](values|dmPolicy|csv)$/); if (!allowReference?.[1]) return undefined; const ids = allowedIds(context, "googlechat"); switch (allowReference[1]) { @@ -59,6 +70,11 @@ export const resolveGooglechatTemplateReference: BuiltInRenderTemplateResolver = return resolvedRenderTemplateReference(nonEmptyArray(ids)); case "dmPolicy": return resolvedRenderTemplateReference(ids.length > 0 ? "allowlist" : undefined); + case "csv": + // Hermes reads GOOGLE_CHAT_ALLOWED_USERS as a comma-separated env value. + // Undefined when empty so the line drops and the adapter's own default-deny + // (unknown senders rejected) governs, not an empty allowlist string. + return resolvedRenderTemplateReference(ids.length > 0 ? ids.join(",") : undefined); default: return undefined; } diff --git a/src/lib/messaging/channels/manifests.test.ts b/src/lib/messaging/channels/manifests.test.ts index 966c61de0ec..4fdf8945316 100644 --- a/src/lib/messaging/channels/manifests.test.ts +++ b/src/lib/messaging/channels/manifests.test.ts @@ -17,7 +17,6 @@ describe("built-in channel manifests", () => { it("registers every known channel for supported gateway agents", () => { const registry = createBuiltInChannelManifestRegistry(); const channelNames = knownChannelNames(); - const hermesChannelNames = channelNames.filter((channelName) => channelName !== "googlechat"); expect(BUILT_IN_CHANNEL_MANIFESTS.map((manifest) => manifest.id)).toEqual(channelNames); expect(registry.list().map((manifest) => manifest.id)).toEqual(channelNames); @@ -25,7 +24,7 @@ describe("built-in channel manifests", () => { channelNames, ); expect(registry.listAvailable({ agent: "hermes" }).map((manifest) => manifest.id)).toEqual( - hermesChannelNames, + channelNames, ); }); diff --git a/src/lib/messaging/channels/metadata.test.ts b/src/lib/messaging/channels/metadata.test.ts index 728b4592ec6..372309d8489 100644 --- a/src/lib/messaging/channels/metadata.test.ts +++ b/src/lib/messaging/channels/metadata.test.ts @@ -42,6 +42,7 @@ describe("built-in messaging channel metadata", () => { "slack", "whatsapp", "teams", + "googlechat", ]); }); @@ -95,6 +96,8 @@ describe("built-in messaging channel metadata", () => { "GOOGLECHAT_AUDIENCE", "GOOGLECHAT_APP_PRINCIPAL", "GOOGLECHAT_ALLOWED_USERS", + "GOOGLE_CHAT_PROJECT_ID", + "GOOGLE_CHAT_SUBSCRIPTION_NAME", ]); expect(getMessagingConfigEnvAliases()).toEqual({ DISCORD_SERVER_ID: ["DISCORD_SERVER_IDS"], @@ -185,6 +188,27 @@ describe("built-in messaging channel metadata", () => { manager: "hermes-uv-pip", spec: "aiohttp==3.14.3", }, + { + channelId: "googlechat", + packageId: "hermesGooglePubsubPackage", + agents: ["hermes"], + manager: "hermes-uv-pip", + spec: "google-cloud-pubsub==2.39.0", + }, + { + channelId: "googlechat", + packageId: "hermesGoogleApiClientPackage", + agents: ["hermes"], + manager: "hermes-uv-pip", + spec: "google-api-python-client==2.194.0", + }, + { + channelId: "googlechat", + packageId: "hermesGoogleAuthPackage", + agents: ["hermes"], + manager: "hermes-uv-pip", + spec: "google-auth==2.55.1", + }, ]); }); diff --git a/src/lib/messaging/utils.test.ts b/src/lib/messaging/utils.test.ts index f423466bfb7..17df8e59481 100644 --- a/src/lib/messaging/utils.test.ts +++ b/src/lib/messaging/utils.test.ts @@ -42,6 +42,7 @@ describe("listSupportedMessagingChannelIdsForAgent", () => { "slack", "whatsapp", "teams", + "googlechat", ]); }); }); diff --git a/src/lib/onboard/messaging-bridge-provider.test.ts b/src/lib/onboard/messaging-bridge-provider.test.ts index 1eba60e4ea1..d2c6dc148c6 100644 --- a/src/lib/onboard/messaging-bridge-provider.test.ts +++ b/src/lib/onboard/messaging-bridge-provider.test.ts @@ -48,6 +48,7 @@ function collectInput( ) { return { sandboxName: "sbx", + agent: GC_PROFILE.agent, getCredential: () => null, enabledChannels: ["googlechat"], disabledChannelNames: new Set(), diff --git a/src/lib/onboard/messaging-bridge-provider.ts b/src/lib/onboard/messaging-bridge-provider.ts index 2f43465aba2..0205702e9b6 100644 --- a/src/lib/onboard/messaging-bridge-provider.ts +++ b/src/lib/onboard/messaging-bridge-provider.ts @@ -87,6 +87,13 @@ export interface MessagingBridgeSecretResolveDeps { export interface CollectMessagingBridgeTokenDefsInput extends MessagingBridgeSecretResolveDeps { readonly sandboxName: string; + /** + * Sandbox agent. Bridge profiles are per-agent (`provider-profile/.yaml`), + * and a channel can ship both openclaw and hermes profiles (Google Chat does), so + * only the profile matching this sandbox's agent must produce a bridge — otherwise + * an openclaw sandbox would also mint the hermes bridge under the same name. + */ + readonly agent: MessagingAgentId; readonly enabledChannels: readonly string[] | null; readonly disabledChannelNames: ReadonlySet; /** Injected for tests; defaults to convention discovery. */ @@ -258,6 +265,7 @@ export function collectMessagingBridgeTokenDefs( const profiles = input.profiles ?? listMessagingBridgeProfiles(); const defs: { name: string; envKey: string; token: string; providerType: string }[] = []; for (const profile of profiles) { + if (profile.agent !== input.agent) continue; if (input.disabledChannelNames.has(profile.channelId)) continue; if (input.enabledChannels != null && !input.enabledChannels.includes(profile.channelId)) continue; @@ -399,8 +407,15 @@ function buildRefreshMaterial( { key: "client_email", value: clientEmail }, { key: "private_key", value: privateKey }, ]; - // Scope comes from the profile's declared refresh scopes (single source of truth). - if (profile.scopes[0]) material.push({ key: "scope", value: profile.scopes[0] }); + // Scope comes from the profile's declared refresh scopes (single source of + // truth). Join all of them space-separated so ONE minted token carries every + // scope — a Google service-account JWT mints a multi-scope token from a + // space-separated `scope` claim. Hermes Google Chat needs chat.bot AND pubsub + // in a single credential (reply + Pub/Sub REST pull); taking only scopes[0] + // dropped pubsub and made `:pull` fail with 403 "insufficient scopes". + if (profile.scopes.length > 0) { + material.push({ key: "scope", value: profile.scopes.join(" ") }); + } // This strategy always emits private_key as material, so force it into the // secret set (delivered via --secret-material-env, never argv) regardless of // what the profile declares. A profile whose secretMaterialKeys omitted it diff --git a/src/lib/onboard/messaging-prep.ts b/src/lib/onboard/messaging-prep.ts index d0dd50eae56..71d3af005c6 100644 --- a/src/lib/onboard/messaging-prep.ts +++ b/src/lib/onboard/messaging-prep.ts @@ -141,6 +141,7 @@ export function prepareCreateSandboxMessaging( messagingTokenDefs.push( ...collectMessagingBridgeTokenDefs({ sandboxName: input.sandboxName, + agent: input.agentName?.trim().toLowerCase() === "hermes" ? "hermes" : "openclaw", getCredential: input.getCredential, env: input.env, normalizeCredentialValue: input.normalizeCredentialValue, diff --git a/test/managed-image-capability-union.test.ts b/test/managed-image-capability-union.test.ts index 583262c7d05..1e3a7c8bf0c 100644 --- a/test/managed-image-capability-union.test.ts +++ b/test/managed-image-capability-union.test.ts @@ -50,6 +50,9 @@ describe("managed-image capability union", () => { expect(collectManagedImageHermesUvPackages()).toEqual([ "microsoft-teams-apps==2.0.13.4", "aiohttp==3.14.3", + "google-cloud-pubsub==2.39.0", + "google-api-python-client==2.194.0", + "google-auth==2.55.1", ]); }); @@ -149,7 +152,7 @@ describe("managed-image capability union", () => { NEMOCLAW_MANAGED_IMAGE_CAPABILITY_UNION: "1", }); expect(fs.readFileSync(trace, "utf8").trim()).toBe( - "pip install --python /opt/hermes/.venv/bin/python --no-cache -- microsoft-teams-apps==2.0.13.4 aiohttp==3.14.3", + "pip install --python /opt/hermes/.venv/bin/python --no-cache -- microsoft-teams-apps==2.0.13.4 aiohttp==3.14.3 google-cloud-pubsub==2.39.0 google-api-python-client==2.194.0 google-auth==2.55.1", ); } finally { fs.rmSync(temporaryRoot, { force: true, recursive: true }); From a546a775f572daa5acd63b0045f09a4708e13382 Mon Sep 17 00:00:00 2001 From: Hung Le Date: Tue, 18 Aug 2026 07:44:34 +0530 Subject: [PATCH 02/15] fix(messaging): allow only pull and acknowledge on the Hermes Pub/Sub route The Google Chat preset for Hermes allowed POST to every Pub/Sub v1 path, and the comment justified that width with a claim about glob matching that does not hold. The L7 matcher is glob.match(pattern, ["/"], path), so `/` is the only delimiter and `*` already spans the `:verb` suffix inside a segment. The adapter issues exactly two Pub/Sub requests, `:pull` and `:acknowledge`, so restrict the route to those. The gateway injects a bearer carrying the pubsub scope here, and the previous rule also permitted publish and subscription administration from inside the sandbox. Presets cannot template the configured subscription, so the rules match the subscription path shape. Drop the stale `:modifyAckDeadline` mention; the adapter never issues it. --- .../channels/googlechat/policy/hermes.yaml | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/lib/messaging/channels/googlechat/policy/hermes.yaml b/src/lib/messaging/channels/googlechat/policy/hermes.yaml index d00a93effa9..9708d09aa67 100644 --- a/src/lib/messaging/channels/googlechat/policy/hermes.yaml +++ b/src/lib/messaging/channels/googlechat/policy/hermes.yaml @@ -10,7 +10,7 @@ network_policies: name: googlechat_hermes endpoints: # Inbound: pull Chat events from the Pub/Sub subscription over the REST - # unary API (:pull, :acknowledge, :modifyAckDeadline). Not gRPC + # unary API (:pull, :acknowledge). Not gRPC # StreamingPull — REST keeps the egress L7-inspectable so the # gateway-minted token is injected by the proxy (keyless). The # subscription lives under /v1/projects/{project}/subscriptions/{sub}. @@ -19,11 +19,14 @@ network_policies: protocol: rest enforcement: enforce rules: - # Pub/Sub REST pull uses Google custom-method verbs with a colon suffix: - # `/v1/projects/{p}/subscriptions/{s}:pull` / `:acknowledge` / - # `:modifyAckDeadline`. Match the whole /v1 tree so the `:verb` suffix - # is covered (a `/v1/projects/**` glob can miss the trailing `:pull`). - - allow: { method: POST, path: "/v1/**" } + # Pub/Sub REST pull uses Google custom-method verbs with a colon + # suffix: `/v1/projects/{p}/subscriptions/{s}:pull`. The L7 matcher is + # `glob.match(pattern, ["/"], path)`, so `/` is the only delimiter and + # `*` spans the `:verb` suffix within a segment. Allow just the two + # operations the adapter issues; a broader `/v1/**` would also permit + # publish and subscription administration on the injected bearer. + - allow: { method: POST, path: "/v1/projects/*/subscriptions/*:pull" } + - allow: { method: POST, path: "/v1/projects/*/subscriptions/*:acknowledge" } # Outbound reply: send/update/delete messages in Chat spaces. Same rule # shape as the OpenClaw preset — writes scoped to the Chat `spaces` tree, # reads across the Chat REST v1 tree. From e947a2a80731f466d83cdc940f649191db42428e Mon Sep 17 00:00:00 2001 From: Hung Le Date: Tue, 18 Aug 2026 08:15:30 +0530 Subject: [PATCH 03/15] fix(messaging): close the proc handle in the Google Chat proxy lookup The gateway proxy lookup opened every `/proc//cmdline` it scanned without closing it. The Chat reply transport calls that lookup on every outbound request, so each reply leaked one descriptor per process on the host. Read the file under a context manager instead. Also drop the redundant `asyncio` import inside the reply transport; the module already imports asyncio at top level. --- agents/hermes/plugin/googlechat_sandbox_adapter.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/agents/hermes/plugin/googlechat_sandbox_adapter.py b/agents/hermes/plugin/googlechat_sandbox_adapter.py index 79808f51ad8..0afb0cae130 100644 --- a/agents/hermes/plugin/googlechat_sandbox_adapter.py +++ b/agents/hermes/plugin/googlechat_sandbox_adapter.py @@ -296,7 +296,8 @@ def _gc_gateway_proxy_url(): for cmd_path in glob.glob("/proc/[0-9]*/cmdline"): try: - cmdline = open(cmd_path, "rb").read() + with open(cmd_path, "rb") as cmdline_handle: + cmdline = cmdline_handle.read() except OSError: continue if b"hermes.real" not in cmdline or b"gateway" not in cmdline or b"dashboard" in cmdline: @@ -340,8 +341,6 @@ class _GcAiohttpTransport: content, which this provides via an ``httplib2.Response``.""" def request(self, uri, method="GET", body=None, headers=None, **kwargs): - import asyncio - import aiohttp import plugins.platforms.google_chat.adapter as _gc From 1b97989b6753c33d5443904b9f6479e671736cae Mon Sep 17 00:00:00 2001 From: Hung Le Date: Tue, 18 Aug 2026 14:57:54 +0530 Subject: [PATCH 04/15] refactor(messaging): attach the Google Chat override through the Hermes registry seams The Hermes Google Chat delta was a repository-local fork: it copied the bundled `connect()`, rebound four private methods onto each adapter instance, replaced `platform_registry.register` for the whole process, and lived in the shared Hermes plugin. Rework it to use the seams Hermes already publishes. * `platform_registry.get("google_chat")` resolves the bundled entry and forces its deferred loader, so the override no longer depends on registration order. * `PlatformEntry` is a dataclass, so `dataclasses.replace` preserves every field and changes only `adapter_factory`, `check_fn` and `required_env`. `register()` documents last-writer-wins for exactly this case. * The delta is now a subclass. Reporting no subscription from `_validate_config` makes the bundled `connect()` skip its gRPC precheck, which is fatal under a REST-only egress policy, and skip its own supervisor, so the copied `connect()` is gone and the REST pull starts from the subclass instead. * The module moves to `channels/googlechat/runtime/hermes-adapter.py`, the channel-owned location `src/lib/messaging/AGENTS.md` specifies. The Hermes image copies it beside the plugin, which still loads it only when the channel is configured. Three Hermes internals remain bound, because `PlatformEntry` carries no credential or transport field and `adapter_factory` is its only injection point. Pin them at image build instead: `image-build-probes.py googlechat-override-seams` fails the build when one of those definitions moves, rather than letting the channel fall back to the stock gRPC and service-account adapter unnoticed. This follows the existing pinning practice for Hermes internals in the same Dockerfile. Behavior is unchanged: the same events arrive over the same Pub/Sub REST pull and replies leave over the same proxied transport. Also adds the two Hermes Google Chat config keys to the non-secret allowlist test, which the channel needed and no run had exercised, and shortens the comments this channel's files had accumulated. --- agents/hermes/Dockerfile | 11 +- agents/hermes/image-build-probes.py | 30 ++ agents/hermes/plugin/__init__.py | 38 +- .../plugin/googlechat_sandbox_adapter.py | 477 ------------------ src/lib/messaging-channel-config.test.ts | 2 + .../applier/build/messaging-build-applier.mts | 11 +- .../messaging/channels/googlechat/manifest.ts | 29 +- .../channels/googlechat/policy/hermes.yaml | 30 +- .../googlechat/provider-profile/hermes.yaml | 39 +- .../googlechat/runtime/hermes-adapter.py | 355 +++++++++++++ src/lib/onboard/messaging-bridge-provider.ts | 15 +- test/hermes-image-build-probes.test.ts | 1 + 12 files changed, 452 insertions(+), 586 deletions(-) delete mode 100644 agents/hermes/plugin/googlechat_sandbox_adapter.py create mode 100644 src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py diff --git a/agents/hermes/Dockerfile b/agents/hermes/Dockerfile index b82d987ff24..1519f9ccd3d 100644 --- a/agents/hermes/Dockerfile +++ b/agents/hermes/Dockerfile @@ -78,6 +78,10 @@ COPY agents/hermes/security-dependencies.patch /scripts/hermes-security-dependen FROM scratch AS hermes-agent-payload COPY agents/hermes/plugin/ /opt/nemoclaw-hermes-plugin/ +# Channel-owned Google Chat runtime asset, loaded by the plugin beside its own +# __init__.py. +COPY src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py \ + /opt/nemoclaw-hermes-plugin/googlechat_adapter.py COPY agents/hermes/generate-config.ts /opt/nemoclaw-hermes-config/generate-config.ts COPY agents/hermes/config/ /opt/nemoclaw-hermes-config/config/ COPY agents/hermes/image-build-probes.py /opt/nemoclaw-hermes-config/image-build-probes.py @@ -378,7 +382,7 @@ RUN find /opt/nemoclaw-hermes-config -type d -exec chmod 755 {} + \ /scripts/patch-bundled-npm-tar.mts \ && chmod -R a+rX /src/lib/messaging -ARG NEMOCLAW_HERMES_IMAGE_BUILD_PROBES_SHA256=89f530da6a8296c8bab449a2a324d8efedca1a90b04a5e0f57cf9203ed2011cd +ARG NEMOCLAW_HERMES_IMAGE_BUILD_PROBES_SHA256=521756ea979d58df44d52b7fc1c2f062a11db397835b925f15bc0265e33e48ea # hadolint ignore=DL4006 RUN printf '%s %s\n' \ "$NEMOCLAW_HERMES_IMAGE_BUILD_PROBES_SHA256" /opt/nemoclaw-hermes-config/image-build-probes.py \ @@ -636,6 +640,11 @@ RUN /usr/bin/python3 -I /usr/local/lib/nemoclaw/patch-hermes-discord-recovery-pe && /usr/bin/python3 -I /opt/nemoclaw-hermes-config/image-build-probes.py \ discord-recovery-source +# Pin the Hermes definitions the Google Chat override binds, so an upgrade fails +# this build instead of silently falling back to the stock adapter. +RUN /usr/bin/python3 -I /opt/nemoclaw-hermes-config/image-build-probes.py \ + googlechat-override-seams + # Hermes v0.19.0's bundled Langfuse plugin rejects OpenShell resolver # placeholders before the SDK can construct its Basic-auth headers. Accept only # the exact public/secret placeholder bound to the matching standard Langfuse diff --git a/agents/hermes/image-build-probes.py b/agents/hermes/image-build-probes.py index 80c8329bb2b..cb27ac76917 100644 --- a/agents/hermes/image-build-probes.py +++ b/agents/hermes/image-build-probes.py @@ -389,6 +389,35 @@ def reopen_probe(conn): assert store.call(reopen_probe) == ("gateway-reopened",) +def verify_googlechat_override_seams() -> None: + """Fail the build when a Google Chat definition the channel override binds moves. + + The override subclasses the bundled adapter because ``PlatformEntry`` carries + no credential or transport field. Those names are Hermes internals, so pin + them: an upgrade that renames one stops the build instead of letting the + channel fall back to the stock adapter unnoticed. + """ + path = "/opt/hermes/plugins/platforms/google_chat/adapter.py" + source = Path(path).read_text(encoding="utf-8") + expected = { + "def _validate_config(self) -> Tuple[str, Optional[str]]:": 1, + "def _load_sa_credentials(self) -> Any:": 1, + "def _new_authed_http(self) -> Any:": 1, + "async def connect(self, *, is_reconnect: bool = False) -> bool:": 1, + # connect() gates its gRPC subscriber precheck and its own supervisor on + # this test; the override reports no subscription so both are skipped. + "if subscription_path is not None:": 2, + } + for needle, count in expected.items(): + actual = source.count(needle) + assert actual == count, ( + f"{path}: expected {count} occurrence(s) of {needle!r}, found {actual}. " + "The Google Chat channel override binds this definition; re-review " + "src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py before " + "upgrading Hermes." + ) + + COMMANDS: dict[str, Callable[[], None]] = { "cron-backup": verify_cron_backup, "cron-create": verify_cron_create, @@ -399,6 +428,7 @@ def reopen_probe(conn): "discord-recovery-source": verify_discord_recovery_source, "discord-reopen": verify_discord_reopen, "gateway-process-identity": verify_gateway_process_identity, + "googlechat-override-seams": verify_googlechat_override_seams, "gateway-runtime-metadata": verify_gateway_runtime_metadata, "langfuse-credentials": verify_langfuse_credentials, "neutral-platform-inertness": verify_neutral_platform_inertness, diff --git a/agents/hermes/plugin/__init__.py b/agents/hermes/plugin/__init__.py index 41ef29533b7..75b77053365 100644 --- a/agents/hermes/plugin/__init__.py +++ b/agents/hermes/plugin/__init__.py @@ -9,7 +9,7 @@ Layout: channel-specific runtime overrides live in sibling modules loaded by register() only when that channel is configured, so a sandbox without the -channel carries none of its behavior. Today that is googlechat_sandbox_adapter.py. +channel carries none of its behavior. Today that is googlechat_adapter.py. Skill hot-reload: Hermes caches its skill slash-command registry in a module-global dict on first scan. New skills dropped on disk are invisible @@ -1425,30 +1425,23 @@ def _handle_reload_skills(tool_input=None, context=None, **_kwargs): return "\n".join(lines) -# ───────────────────────────────────────────────────────────────────────────── -# Google Chat: keyless adapter override (NemoClaw) -# -# The override itself lives in the sibling ``googlechat_sandbox_adapter`` -# module — the same per-concern split Hermes' own plugins use -# (``plugins/platforms/discord`` ships adapter.py, recovery.py, and -# voice_mixer.py beside __init__.py). It is loaded only when the sandbox is -# configured for the Google Chat channel, so a Hermes sandbox without that -# channel never wraps the platform registry. -# ───────────────────────────────────────────────────────────────────────────── +# Google Chat: the channel owns the override. Source lives in +# src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py; the Hermes +# image copies it in beside this file. Loaded only when the channel is +# configured, so other sandboxes never replace the bundled platform entry. _GOOGLE_CHAT_SUBSCRIPTION_ENV = "GOOGLE_CHAT_SUBSCRIPTION_NAME" -_GOOGLE_CHAT_MODULE = "googlechat_sandbox_adapter.py" +_GOOGLE_CHAT_MODULE = "googlechat_adapter.py" def _load_googlechat_adapter(): - """Load the sibling Google Chat override module by file path. + """Load the sibling override module by path. - Hermes loads this plugin as a directory module under a synthetic name - (``hermes_cli/plugins.py`` ``_load_directory_module``), so there is no - package context for a relative import; load the sibling the same way. + Hermes loads this plugin as a directory module under a synthetic name, so + there is no package context for a relative import. """ path = os.path.join(os.path.dirname(__file__), _GOOGLE_CHAT_MODULE) spec = importlib.util.spec_from_file_location( - "nemoclaw_hermes_googlechat_sandbox_adapter", + "nemoclaw_hermes_googlechat_adapter", path, ) if spec is None or spec.loader is None: @@ -1458,12 +1451,11 @@ def _load_googlechat_adapter(): return module -def _install_googlechat_sandbox_adapter(ctx): - """Install the keyless Google Chat override when that channel is configured. +def _install_googlechat_adapter(ctx): + """Install the Google Chat override when that channel is configured. - Gating on the rendered subscription keeps every other Hermes sandbox free of - the platform-registry wrap. Returns whether the override module was loaded - and invoked; the module reports its own registry-wrap failures. + Returns whether the module was loaded and invoked; it reports its own + registration failures. """ if not _get_env_value(_GOOGLE_CHAT_SUBSCRIPTION_ENV): return False @@ -1487,7 +1479,7 @@ def register(ctx): """Register NemoClaw tools and hooks with Hermes.""" _install_nous_tool_broker_patch() _install_messaging_response_patch() - _install_googlechat_sandbox_adapter(ctx) + _install_googlechat_adapter(ctx) # Register status tool ctx.register_tool( diff --git a/agents/hermes/plugin/googlechat_sandbox_adapter.py b/agents/hermes/plugin/googlechat_sandbox_adapter.py deleted file mode 100644 index 0afb0cae130..00000000000 --- a/agents/hermes/plugin/googlechat_sandbox_adapter.py +++ /dev/null @@ -1,477 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 -"""Google Chat adapter for Hermes running inside a NemoClaw sandbox. - -Rebinds the bundled Hermes Google Chat adapter to the transports and -credentials a sandbox allows: events arrive over the Pub/Sub REST API instead -of gRPC, and replies leave through the OpenShell L7 proxy carrying a -gateway-minted placeholder instead of a locally signed token. - -Loaded by the NemoClaw plugin's ``register()`` only when the sandbox is -configured for the Google Chat channel, so a Hermes sandbox without that -channel never wraps the platform registry. Companion NemoClaw config lives in -``src/lib/messaging/channels/googlechat/`` (manifest, policy preset, provider -profile); the OpenClaw equivalent of this file is that channel's -``runtime/googlechat-outbound-auth.ts`` boot preload. - -Why the bundled transports are replaced ---------------------------------------- -Standalone Hermes receives Chat events over a gRPC Pub/Sub StreamingPull and -signs its bot token in-process from a service-account key. Neither survives -inside a NemoClaw sandbox: - -* **Inbound.** All sandbox egress traverses the OpenShell L7 proxy, whose - protocol set is ``rest, websocket, graphql, sql, json-rpc, mcp`` — there is no - gRPC variant, so ``protocol: grpc`` fails policy validation. Raw relay - (``tls: skip``) would carry the bytes but disables the inspection that the - credential swap depends on, and the policy engine rejects pairing an - inspecting middleware with a skipped endpoint. So this module PULLS the same - subscription over the Pub/Sub REST unary API (``:pull`` / ``:acknowledge``), - which the proxy can read. -* **Outbound.** The service-account key must stay out of the sandbox: the - gateway mints the token and the L7 proxy swaps the - ``openshell:resolve:env:GOOGLE_CHAT_ACCESS_TOKEN`` placeholder on the way out. - The bundled adapter has no seam for a pre-minted token and hardcodes - ``httplib2.Http()``, which cannot proxy HTTPS here, so this module supplies - placeholder credentials plus an aiohttp transport. - -This is not a stopgap awaiting gRPC support: L7 inspection and gRPC are in -tension by design, so keyless Google Chat would not get simpler if OpenShell -added a gRPC protocol variant. Hermes does also support an inbound HTTP-events -webhook, which would remove the inbound half of this override but not the -outbound half, at the cost of a public inbound URL; NemoClaw keeps pull. - -Override mechanism ------------------- -This is a registered-adapter wrap via the sanctioned ``platform_registry`` -seam, NOT a runtime monkeypatch of the bundled module. ``register`` is wrapped -so the bundled ``google_chat`` entry is kept as-is except for its -``adapter_factory`` (bound to the keyless methods per instance), -``check_fn`` and ``required_env``. Everything else the bundled entry carries — -``env_enablement_fn``, ``validate_config``, ``is_connected``, -``allowed_users_env``, ``standalone_sender_fn``, ``platform_hint`` — is -preserved, and binding onto the instance keeps the override immune to the -synthetic module identity of the lazily loaded bundled plugin. - -B300 live-validation points (managed image, not reproducible on a standalone -Hermes host): - -* the registry wrap supersedes the bundled deferred loader in the managed-image - plugin-load order (same runtime HERMES_HOME scope); -* ``from plugins.platforms.google_chat.adapter import GoogleChatAdapter`` - resolves under /opt/hermes in the managed image; -* the base ``connect()``'s ``SubscriberClient(credentials=placeholder)`` - constructs without egress (it is never ``.subscribe()``'d); -* ``AuthorizedHttp(placeholder_creds)`` on the reply path emits the placeholder - bearer for the L7 swap. -""" - -import asyncio -import importlib.util -import logging -import types - -_GC_REST_PLACEHOLDER_TOKEN = "openshell:resolve:env:GOOGLE_CHAT_ACCESS_TOKEN" -_GC_PULL_TIMEOUT = 95.0 # aiohttp cap > the Pub/Sub server long-poll hold (~90s) -_GC_LOG = logging.getLogger("gateway.platforms.google_chat") - - -class _RestPubsubMessage: - """Adapt a Pub/Sub REST receivedMessage to the four members the inherited - _on_pubsub_message touches: .data (bytes), .attributes (dict), .ack(), .nack().""" - - def __init__(self, pmsg, ack_sink, ack_id): - import base64 - - self._ack_sink = ack_sink - self._ack_id = ack_id - raw = pmsg.get("data") or "" - self.data = base64.b64decode(raw) if raw else b"" - self.attributes = pmsg.get("attributes") or {} - - def ack(self): - if self._ack_id: - self._ack_sink.append(self._ack_id) - - def nack(self): - # Omit the ackId from the batch -> Pub/Sub redelivers, matching the - # streaming client's message.nack() semantics. - pass - - -async def _gc_rest_pull_supervisor(self): - """Drop-in replacement for GoogleChatAdapter._run_supervisor that pulls via the - Pub/Sub REST unary API instead of gRPC StreamingPull, then feeds each message - into the UNCHANGED self._on_pubsub_message (run in a worker thread to preserve - the 'callback off the event loop' contract). Keyless: the bearer is the - placeholder the L7 proxy rewrites, so nothing is signed in-process and this path - needs no google.auth.""" - import aiohttp - - sub = self._subscription_path - pull_url = f"https://pubsub.googleapis.com/v1/{sub}:pull" - ack_url = f"https://pubsub.googleapis.com/v1/{sub}:acknowledge" - headers = { - "Authorization": f"Bearer {_GC_REST_PLACEHOLDER_TOKEN}", - "Content-Type": "application/json", - } - _GC_LOG.info( - "[GoogleChat][NemoClaw] keyless Pub/Sub REST :pull transport active (sub=%s)", - sub, - ) - async with aiohttp.ClientSession(trust_env=True) as session: - while not self._shutting_down: - try: - async with session.post( - pull_url, - json={"maxMessages": self._max_messages or 1}, - headers=headers, - timeout=aiohttp.ClientTimeout(total=_GC_PULL_TIMEOUT), - ) as resp: - if resp.status != 200: - body = await resp.text() - _GC_LOG.warning( - "[GoogleChat][NemoClaw] :pull HTTP %s: %s", - resp.status, - body[:200], - ) - await asyncio.sleep(3) - continue - payload = await resp.json() - except asyncio.CancelledError: - raise - except Exception as exc: # noqa: BLE001 - transient pull errors are retried - _GC_LOG.warning("[GoogleChat][NemoClaw] :pull error: %s", exc) - await asyncio.sleep(3) - continue - - received = payload.get("receivedMessages") or [] - if not received: - await asyncio.sleep(0.2) # guard against fast-empty long-poll returns - continue - - acks: list = [] - for received_message in received: - shim = _RestPubsubMessage( - received_message.get("message") or {}, - acks, - received_message.get("ackId"), - ) - try: - await asyncio.to_thread(self._on_pubsub_message, shim) - except Exception: # noqa: BLE001 - one bad message must not kill the loop - _GC_LOG.exception("[GoogleChat][NemoClaw] message handler raised") - - if acks: - try: - async with session.post( - ack_url, - json={"ackIds": acks}, - headers=headers, - timeout=aiohttp.ClientTimeout(total=30), - ) as ack_resp: - if ack_resp.status != 200: - _GC_LOG.warning( - "[GoogleChat][NemoClaw] :acknowledge HTTP %s", - ack_resp.status, - ) - except Exception as exc: # noqa: BLE001 - a failed ack just redelivers - _GC_LOG.warning("[GoogleChat][NemoClaw] :acknowledge error: %s", exc) - - -def _gc_placeholder_credentials(): - """A google.auth Credentials whose token is the OpenShell placeholder the L7 - proxy rewrites to the gateway-minted bearer. refresh() is a no-op so nothing is - signed in-process. Feeds both the idle SubscriberClient built by the inherited - connect() and the reply path's AuthorizedSession, so both carry the placeholder.""" - from google.auth import credentials as ga_credentials - - class _PlaceholderCredentials(ga_credentials.Credentials): - def __init__(self): - super().__init__() - self.token = _GC_REST_PLACEHOLDER_TOKEN - - def refresh(self, request): # signed gateway-side; nothing to do here - self.token = _GC_REST_PLACEHOLDER_TOKEN - - return _PlaceholderCredentials() - - -async def _gc_rest_pull_connect(self, *, is_reconnect: bool = False) -> bool: - """Keyless REST-pull ``connect()``: the bundled ``connect()`` MINUS the gRPC - Pub/Sub ``SubscriberClient`` + ``get_subscription()`` sanity check, which hangs - for 30 s under the REST-only OpenShell egress policy (gRPC is blocked), and - MINUS the legacy single-user OAuth probe (``__init__`` already defaults those to - ``None``; attachments degrade to text). The REST ``:pull`` supervisor surfaces a - bad subscription at first pull instead of a gRPC precheck. Everything else - mirrors the bundled ``connect()``: lazy google-module load, config validate, - placeholder creds, Chat REST client, thread-count store, bot-id resolution, then - start the REST-pull supervisor.""" - import asyncio as _asyncio - - import plugins.platforms.google_chat.adapter as _gc - - if not _gc._load_google_modules(): - self._set_fatal_error( - code="missing_deps", - message="google-cloud-pubsub / google-api-python-client not installed", - retryable=False, - ) - return False - - self._loop = _asyncio.get_running_loop() - try: - project_id, subscription_path = self._validate_config() - credentials = self._load_sa_credentials() - except (ValueError, FileNotFoundError) as exc: - msg = _gc._redact_sensitive(str(exc)) - _gc.logger.error("[GoogleChat] Config validation failed: %s", msg) - self._set_fatal_error(code="config_invalid", message=msg, retryable=False) - return False - - self._project_id = project_id - self._subscription_path = subscription_path - self._credentials = credentials - - try: - self._chat_api = await _asyncio.to_thread( - lambda: _gc.build_service( - "chat", "v1", credentials=credentials, cache_discovery=False - ) - ) - except Exception as exc: # noqa: BLE001 - msg = _gc._redact_sensitive(str(exc)) - _gc.logger.error("[GoogleChat] Failed to build Chat API client: %s", msg) - self._set_fatal_error(code="chat_api_init", message=msg, retryable=False) - return False - - try: - await _asyncio.to_thread(self._thread_count_store.load) - except Exception: # noqa: BLE001 - _gc.logger.warning( - "[GoogleChat] thread-count store load failed (treating all threads as fresh)", - exc_info=True, - ) - - # SKIP bundled gRPC block (pubsub_v1.SubscriberClient + get_subscription()): - # gRPC is denied by the REST-only egress policy and hangs; the keyless REST - # :pull loop never touches self._subscriber (stays None; disconnect() guards it). - - self._bot_user_id = self._load_cached_bot_id() - if not self._bot_user_id: - self._bot_user_id = await self._resolve_bot_user_id() - if self._bot_user_id: - self._save_cached_bot_id(self._bot_user_id) - else: - _gc.logger.info( - "[GoogleChat] bot_user_id not yet resolved; will resolve on first " - "addedToSpace or member lookup" - ) - - if subscription_path is not None: - self._supervisor_task = _asyncio.create_task(self._run_supervisor()) - else: - self._supervisor_task = None - - self._mark_connected() - _gc.logger.info( - "[GoogleChat][NemoClaw] Connected keyless (REST :pull, no gRPC subscriber); " - "project=%s, subscription=%s, bot_user_id=%s", - project_id or "", - "" if subscription_path else "", - self._bot_user_id or "", - ) - return True - - -def _gc_gateway_proxy_url(): - """The OpenShell egress proxy URL, read from the ``hermes.real gateway`` - process's IMMUTABLE ``/proc//environ``. os.environ is unreliable (the gateway - clears the proxy vars during an agent turn), and ``/proc/self`` is NOT the gateway - in the reply's execution context — the Chat reply runs under ``asyncio.to_thread`` - whose ``/proc/self/environ`` does not carry the launcher-set proxy. So locate the - gateway process by cmdline and read its startup env, which keeps the proxy for the - life of the process. Returns ``""`` when no proxy is found (direct egress).""" - import glob - - for cmd_path in glob.glob("/proc/[0-9]*/cmdline"): - try: - with open(cmd_path, "rb") as cmdline_handle: - cmdline = cmdline_handle.read() - except OSError: - continue - if b"hermes.real" not in cmdline or b"gateway" not in cmdline or b"dashboard" in cmdline: - continue - try: - with open(cmd_path.rsplit("/", 1)[0] + "/environ", "rb") as handle: - env = {} - for pair in handle.read().split(b"\0"): - key, sep, value = pair.partition(b"=") - if sep: - env[key.decode("utf-8", "replace")] = value.decode("utf-8", "replace") - except OSError: - continue - url = ( - env.get("https_proxy") - or env.get("HTTPS_PROXY") - or env.get("http_proxy") - or env.get("HTTP_PROXY") - or "" - ) - if url: - return url - return "" - - -class _GcAiohttpTransport: - """An ``httplib2.Http``-compatible transport (``.request()`` only) that routes - Chat REST calls through the OpenShell L7 proxy via aiohttp — the ONLY transport - that works from the gateway process in this sandbox. - - The bundled Chat client is google-api-python-client over httplib2, whose HTTPS - proxy support is unusable here: without PySocks ``ProxyInfo.isgood()`` is falsy, so - httplib2 silently connects DIRECT — which the proxy-only sandbox netns cannot - resolve (name-resolution failure); WITH PySocks its ``PROXY_TYPE_HTTP`` rejects the - CONNECT tunnel. aiohttp CONNECT-tunnels through the proxy exactly as the inbound - ``:pull`` does, and the gateway process is the one the proxy authorizes and injects - the minted bot token for (ad-hoc processes are refused at CONNECT). The proxy is - read from the gateway's ``/proc//environ`` (os.environ is unreliable), and TLS - trusts the system CA bundle, which carries the proxy's MITM root. googleapiclient - only calls ``.request(uri, method, body, headers)`` and reads ``resp.status`` + - content, which this provides via an ``httplib2.Response``.""" - - def request(self, uri, method="GET", body=None, headers=None, **kwargs): - import aiohttp - - import plugins.platforms.google_chat.adapter as _gc - - proxy = _gc_gateway_proxy_url() or None - data = body.encode("utf-8") if isinstance(body, str) else body - - async def _run(): - # Mirror the inbound :pull exactly: ClientSession(trust_env=True) with no - # ssl override. That is the transport the L7 proxy authorizes for this - # gateway process, and its default TLS trust already accepts the proxy's - # MITM chain (pinning a hand-built CA context rejected it). The proxy is - # still passed explicitly because os.environ may be cleared by reply time. - async with aiohttp.ClientSession(trust_env=True) as session: - async with session.request( - method, - uri, - data=data, - headers=dict(headers or {}), - proxy=proxy, - timeout=aiohttp.ClientTimeout(total=60), - ) as resp: - content = await resp.read() - info = {"status": resp.status} - for key, value in resp.headers.items(): - info[key.lower()] = value - return _gc.httplib2.Response(info), content - - # The Chat calls run under ``asyncio.to_thread`` (a worker thread with no - # running event loop), so a fresh loop here via asyncio.run is safe. - return asyncio.run(_run()) - - -def _gc_rest_reply_http(self): - """``_new_authed_http()`` override for the keyless Chat reply path. - - Returns ``AuthorizedHttp(placeholder_creds, http=_GcAiohttpTransport())``. The - bundled ``httplib2.Http`` cannot proxy HTTPS in this sandbox (see - ``_GcAiohttpTransport``), so every Chat call that flows through - ``_new_authed_http`` — reply create, message patch, typing card, bot-id lookup — - is routed via aiohttp instead. Credentials stay the placeholder creds bound in - connect(); ``AuthorizedHttp`` adds the placeholder bearer header that the L7 proxy - swaps for the minted bot token, exactly as on the ``:pull`` path.""" - import plugins.platforms.google_chat.adapter as _gc - - return _gc.AuthorizedHttp(self._credentials, http=_GcAiohttpTransport()) - - -def _gc_bind_overrides(adapter): - """Bind the keyless REST-pull behavior onto a bundled GoogleChatAdapter - instance: a connect() that skips the gRPC subscriber precheck, the REST `:pull` - supervisor (inbound), placeholder credentials, and a reply transport routed - through the L7 proxy with the system CA (outbound Chat REST). Binding onto the - instance with types.MethodType — rather than subclassing + re-registering — keeps - every other bundled method AND the bundled registry metadata (env_enablement_fn - seeds project_id/subscription into PlatformConfig.extra, validate_config, …) - intact, and is immune to the synthetic module identity of the lazily loaded - bundled plugin.""" - adapter.connect = types.MethodType(_gc_rest_pull_connect, adapter) - adapter._run_supervisor = types.MethodType(_gc_rest_pull_supervisor, adapter) - adapter._load_sa_credentials = types.MethodType( - lambda self: _gc_placeholder_credentials(), adapter - ) - adapter._new_authed_http = types.MethodType(_gc_rest_reply_http, adapter) - return adapter - - -def _gc_spec_available(module_name: str) -> bool: - try: - return importlib.util.find_spec(module_name) is not None - except (ImportError, ValueError): - return False - - -def _nemoclaw_gc_check() -> bool: - """Passive dependency probe (side-effect free — no heavy import): the keyless - REST-pull path needs aiohttp; the inherited connect()/reply path needs the - google-auth + pubsub SDKs shipped in the managed image.""" - return all( - _gc_spec_available(module_name) - for module_name in ("aiohttp", "google.auth", "google.cloud.pubsub_v1") - ) - - -def install(ctx) -> None: - """Make the bundled Google Chat adapter run keyless REST-pull WITHOUT dropping - the bundled registry entry's config-seeding metadata. - - A fresh ``register_platform`` (last-writer-wins) loses the bundled entry's - ``env_enablement_fn`` — the hook that seeds ``PlatformConfig.extra.project_id`` - / ``subscription_name`` from ``GOOGLE_CHAT_*`` env — so connect() fails with - "GOOGLE_CHAT_PROJECT_ID is not set". Instead wrap ``platform_registry.register`` - so when the bundled ``google_chat`` entry registers (its deferred loader runs at - gateway start) it is KEPT as-is except: (a) ``adapter_factory`` is wrapped to - bind the REST-pull supervisor + placeholder credentials onto each instance, and - (b) ``check_fn`` / ``required_env`` are swapped to the keyless contract (no SA - JSON in the sandbox). Every other field — env_enablement_fn, validate_config, - is_connected, allowed_users_env, standalone_sender_fn, platform_hint, … — is - preserved. ``ctx`` is unused; the registry is the single override point.""" - del ctx - try: - from gateway.platform_registry import platform_registry as _preg - except Exception: # noqa: BLE001 - keep plugin load resilient - _GC_LOG.exception("[GoogleChat][NemoClaw] platform_registry import failed") - return - if getattr(_preg, "_nemoclaw_gc_wrapped", False): - return - _orig_register = _preg.register - - def _wrapped_register(entry, *args, **kwargs): - try: - if getattr(entry, "name", None) == "google_chat": - _orig_factory = entry.adapter_factory - - def _factory(cfg, _f=_orig_factory): - try: - return _gc_bind_overrides(_f(cfg)) - except Exception: # noqa: BLE001 - fall back to the stock adapter - _GC_LOG.exception("[GoogleChat][NemoClaw] instance override failed") - return _f(cfg) - - entry.adapter_factory = _factory - entry.check_fn = _nemoclaw_gc_check - entry.required_env = ["GOOGLE_CHAT_SUBSCRIPTION_NAME"] - _GC_LOG.info( - "[GoogleChat][NemoClaw] wrapped bundled google_chat adapter " - "(keyless REST-pull; bundled metadata preserved)" - ) - except Exception: # noqa: BLE001 - never let the wrap abort a registration - _GC_LOG.exception("[GoogleChat][NemoClaw] register wrap failed") - return _orig_register(entry, *args, **kwargs) - - _preg.register = _wrapped_register - _preg._nemoclaw_gc_wrapped = True diff --git a/src/lib/messaging-channel-config.test.ts b/src/lib/messaging-channel-config.test.ts index 6ada1ace4f1..6e71cb34a08 100644 --- a/src/lib/messaging-channel-config.test.ts +++ b/src/lib/messaging-channel-config.test.ts @@ -36,6 +36,8 @@ describe("messaging channel config", () => { "GOOGLECHAT_AUDIENCE", "GOOGLECHAT_APP_PRINCIPAL", "GOOGLECHAT_ALLOWED_USERS", + "GOOGLE_CHAT_PROJECT_ID", + "GOOGLE_CHAT_SUBSCRIPTION_NAME", ]); }); diff --git a/src/lib/messaging/applier/build/messaging-build-applier.mts b/src/lib/messaging/applier/build/messaging-build-applier.mts index 422f0d15e2b..4445109d510 100755 --- a/src/lib/messaging/applier/build/messaging-build-applier.mts +++ b/src/lib/messaging/applier/build/messaging-build-applier.mts @@ -1882,13 +1882,10 @@ function installHermesUvPackages(selectedPackages: readonly string[], env: Env): "--", ...selectedPackages, ], - // uv (rustls/webpki) ignores the corporate-only SSL_CERT_FILE the build step - // exports, so a PyPI fetch behind a corporate MITM proxy fails with - // `invalid peer certificate: UnknownIssuer`. Point uv at the full system - // bundle (public roots + corporate CA, merged into /etc/ssl/certs by - // `update-ca-certificates`, Dockerfile #6210) and enable UV_SYSTEM_CERTS so - // uv trusts it. (uv renamed UV_NATIVE_TLS → UV_SYSTEM_CERTS.) Harmless - // off-proxy — the same bundle still carries the public roots. + // uv (rustls) ignores the corporate-only SSL_CERT_FILE, so a PyPI fetch + // behind a MITM proxy fails with `UnknownIssuer`. Point it at the merged + // system bundle instead; harmless off-proxy, and UV_SYSTEM_CERTS is the + // current name for UV_NATIVE_TLS. { ...env, UV_SYSTEM_CERTS: "1", diff --git a/src/lib/messaging/channels/googlechat/manifest.ts b/src/lib/messaging/channels/googlechat/manifest.ts index e91f241efdb..fd439aa72e2 100644 --- a/src/lib/messaging/channels/googlechat/manifest.ts +++ b/src/lib/messaging/channels/googlechat/manifest.ts @@ -118,12 +118,10 @@ export const googlechatManifest = { }, }, // ── Hermes-only Pub/Sub pull config ── - // OpenClaw receives Chat events on an inbound webhook and never reads these. - // Hermes' adapter also supports an inbound HTTP-events webhook, but NemoClaw - // runs it in Pub/Sub pull mode instead: pulling needs no public inbound URL. - // It PULLS events from a Pub/Sub subscription bound to the Chat topic, so it - // needs the project + subscription. Prompted only under the hermes-gated - // hook and rendered only into ~/.hermes/.env. + // OpenClaw uses an inbound webhook and ignores these. Hermes supports a + // webhook too, but NemoClaw pulls instead — no public inbound URL needed — + // so it needs the project and subscription. Hermes-gated prompt, rendered + // only into ~/.hermes/.env. { id: "projectId", kind: "config", @@ -250,12 +248,9 @@ export const googlechatManifest = { }, }, // ── Hermes render ── - // No SA JSON and no access-token env are delivered: the keyless Google Chat - // bridge provider (provider-profile/hermes.yaml) mints a chat.bot+pubsub - // token gateway-side, and the NemoClaw Google Chat adapter override emits the - // `openshell:resolve:env:GOOGLE_CHAT_ACCESS_TOKEN` placeholder on both the - // Pub/Sub :pull and the Chat reply, which the L7 proxy swaps for the minted - // bearer. Only non-secret pull config + the allowlist reach the sandbox here. + // Neither the SA JSON nor a token reaches the sandbox: the bridge provider + // mints one gateway-side and the runtime asset sends only the placeholder, + // which the L7 proxy swaps. Non-secret pull config and the allowlist only. { id: "googlechat-hermes-env", kind: "env-lines", @@ -353,13 +348,9 @@ export const googlechatManifest = { }, required: true, }, - // Hermes keyless REST-pull adapter: the inherited connect() constructs a - // (never-subscribed) Pub/Sub SubscriberClient and the reply path uses - // google-auth, so the managed image must ship these. The base image has - // aiohttp (used by the REST :pull loop) but not the google-* SDKs. - // The adapter override itself is Hermes-side runtime code and lives in - // agents/hermes/plugin/googlechat_sandbox_adapter.py, loaded by that - // plugin only when GOOGLE_CHAT_SUBSCRIPTION_NAME is rendered below. + // The base image ships aiohttp but not the google-* SDKs, which the + // inherited connect() and reply path both need. The Hermes-side delta lives + // in runtime/hermes-adapter.py. { id: "hermesGooglePubsubPackage", agent: "hermes", diff --git a/src/lib/messaging/channels/googlechat/policy/hermes.yaml b/src/lib/messaging/channels/googlechat/policy/hermes.yaml index 9708d09aa67..85d5886abb9 100644 --- a/src/lib/messaging/channels/googlechat/policy/hermes.yaml +++ b/src/lib/messaging/channels/googlechat/policy/hermes.yaml @@ -9,27 +9,20 @@ network_policies: googlechat_hermes: name: googlechat_hermes endpoints: - # Inbound: pull Chat events from the Pub/Sub subscription over the REST - # unary API (:pull, :acknowledge). Not gRPC - # StreamingPull — REST keeps the egress L7-inspectable so the - # gateway-minted token is injected by the proxy (keyless). The - # subscription lives under /v1/projects/{project}/subscriptions/{sub}. + # Inbound: REST :pull / :acknowledge, not gRPC StreamingPull — REST keeps + # the egress inspectable so the proxy can inject the minted token. - host: pubsub.googleapis.com port: 443 protocol: rest enforcement: enforce rules: - # Pub/Sub REST pull uses Google custom-method verbs with a colon - # suffix: `/v1/projects/{p}/subscriptions/{s}:pull`. The L7 matcher is - # `glob.match(pattern, ["/"], path)`, so `/` is the only delimiter and - # `*` spans the `:verb` suffix within a segment. Allow just the two - # operations the adapter issues; a broader `/v1/**` would also permit - # publish and subscription administration on the injected bearer. + # The L7 matcher is `glob.match(pattern, ["/"], path)`, so `*` spans the + # `:verb` suffix. Allow only the two operations the adapter issues: a + # broader `/v1/**` would also permit publish and subscription admin. - allow: { method: POST, path: "/v1/projects/*/subscriptions/*:pull" } - allow: { method: POST, path: "/v1/projects/*/subscriptions/*:acknowledge" } - # Outbound reply: send/update/delete messages in Chat spaces. Same rule - # shape as the OpenClaw preset — writes scoped to the Chat `spaces` tree, - # reads across the Chat REST v1 tree. + # Outbound reply: same rule shape as the OpenClaw preset — writes scoped to + # the Chat `spaces` tree, reads across the v1 tree. - host: chat.googleapis.com port: 443 protocol: rest @@ -39,12 +32,9 @@ network_policies: - allow: { method: POST, path: "/v1/spaces/**" } - allow: { method: PATCH, path: "/v1/spaces/**" } - allow: { method: DELETE, path: "/v1/spaces/**" } - # OpenShell matches the egressing binary by its resolved realpath and - # compares exactly (no glob). Hermes' venv python is a symlink chain - # `/opt/hermes/.venv/bin/python` -> `/usr/bin/python3` -> `/usr/bin/python3.13`, - # so the concrete interpreter `/usr/bin/python3.13` must be listed or the - # egress is denied. Mirrors the provider-profile binary list. Revisit the - # minor version when the Hermes base image bumps Python. + # OpenShell matches the resolved realpath EXACTLY, and Hermes' venv python + # resolves to /usr/bin/python3.13, so the concrete path must be listed. + # Revisit when the base image bumps Python. binaries: - { path: /opt/hermes/.venv/bin/python } - { path: /usr/bin/python3 } diff --git a/src/lib/messaging/channels/googlechat/provider-profile/hermes.yaml b/src/lib/messaging/channels/googlechat/provider-profile/hermes.yaml index 15a17e84dbc..4b04d0a0ea2 100644 --- a/src/lib/messaging/channels/googlechat/provider-profile/hermes.yaml +++ b/src/lib/messaging/channels/googlechat/provider-profile/hermes.yaml @@ -1,25 +1,12 @@ # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 # -# Google Chat outbound + Pub/Sub-pull bridge provider profile (Hermes, keyless). +# Google Chat bridge for Hermes: keyless reply + Pub/Sub pull. # -# Hermes reaches Google Chat over the Pub/Sub REST pull API (inbound events) and -# the Chat REST API (outbound replies). The OpenShell gateway MINTS one bot access -# token from the service-account key (ProviderCredentialRefreshStrategy -# google_service_account_jwt) scoped to BOTH chat.bot and pubsub, and injects it as -# `Authorization: Bearer ` on pubsub.googleapis.com AND chat.googleapis.com. -# The service-account private key stays gateway-side; the sandbox only ever sees the -# `openshell:resolve:env:GOOGLE_CHAT_ACCESS_TOKEN` placeholder, which the L7 proxy -# rewrites to the minted token. -# -# Paired with the NemoClaw Google Chat adapter override (agents/hermes/plugin), which -# returns that placeholder for both the :pull and the Chat reply instead of signing a -# JWT in-process, so no service-account key ever enters the sandbox. -# -# The refresh material VALUES are supplied at onboard/rebuild time by -# src/lib/onboard/messaging-bridge-provider.ts. Non-secret values use `--material`; -# the private key uses `--secret-material-env` so it never appears in argv. The block -# below declares their shape so the gateway knows which keys are secret. +# * The gateway mints one chat.bot + pubsub token and injects it on both hosts. +# * The private key stays gateway-side; the sandbox holds only the placeholder. +# * messaging-bridge-provider.ts supplies the material at onboard, the key via +# `--secret-material-env` so it never reaches argv. id: google-chat-hermes-bridge display_name: Google Chat Bridge (Hermes) description: Gateway-minted Google Chat bot token (chat.bot + pubsub) for Hermes Pub/Sub pull + Chat reply @@ -59,17 +46,11 @@ endpoints: protocol: rest access: read-write enforcement: enforce -# OpenShell resolves the egressing process to its REAL interpreter via -# /proc/pid/exe, not the launched path. Hermes' venv python is a symlink chain -# `/opt/hermes/.venv/bin/python` -> `/usr/bin/python3` -> `/usr/bin/python3.13`, -# so the credential-reach gate sees `/usr/bin/python3.13`. The gate matches -# binaries EXACTLY: a `/usr/bin/python3*` glob does NOT cover `python3.13` -# (OpenShell's prover re-drafts it as a pending `credential_reach_expansion` -# and 403s the CONNECT), and the venv symlink is resolved away before the -# match. So the concrete interpreter path must be listed. `/usr/bin/python3` is -# the symlink OpenShell resolves FROM; `/usr/bin/python3.13` is the realpath it -# resolves TO and matches on. Revisit the minor version when the Hermes base -# image bumps Python. +# OpenShell resolves the process to its real interpreter via /proc/pid/exe and +# matches binaries EXACTLY — a `/usr/bin/python3*` glob does not cover +# `python3.13`, and an unmatched binary is 403'd at CONNECT. Hermes' venv python +# resolves to /usr/bin/python3.13, so list the concrete path. Revisit when the +# base image bumps Python. binaries: - /opt/hermes/.venv/bin/python - /usr/bin/python3 diff --git a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py new file mode 100644 index 00000000000..ff5efe53b6b --- /dev/null +++ b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py @@ -0,0 +1,355 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Google Chat connect-time shim for Hermes inside a NemoClaw sandbox. + +Channel-owned runtime asset (`src/lib/messaging/AGENTS.md`). OpenClaw's +equivalents are the sibling `googlechat-*.ts` preloads. + +What a sandbox forces: + +* Inbound — the OpenShell L7 protocol set has no gRPC, so the bundled + StreamingPull cannot be inspected and raw relay would defeat the credential + swap. Pull the same subscription over the Pub/Sub REST API instead. +* Outbound — the service-account key stays gateway-side, so the sandbox sends + only the `openshell:resolve:env:GOOGLE_CHAT_ACCESS_TOKEN` placeholder. The + bundled `httplib2` client cannot proxy HTTPS here; aiohttp replaces it. + +How it attaches, through seams Hermes publishes rather than patches: + +* `platform_registry.get()` resolves the bundled entry and forces the deferred + loader, so registration order does not matter. +* `PlatformEntry` is a dataclass, so `dataclasses.replace` keeps every field and + changes only `adapter_factory`, `check_fn` and `required_env`. +* `register()` documents last-writer-wins for exactly this case. + +The delta is a subclass; everything else runs the bundled implementation. + +Upstream coupling: `_validate_config`, `_load_sa_credentials` and +`_new_authed_http` are Hermes internals, because `PlatformEntry` carries no +credential or transport field. `image-build-probes.py googlechat-override-seams` +pins them, so drift fails the image build instead of silently falling back to +the stock gRPC and service-account adapter. +""" + +import asyncio +import dataclasses +import importlib.util +import logging + +_GC_REST_PLACEHOLDER_TOKEN = "openshell:resolve:env:GOOGLE_CHAT_ACCESS_TOKEN" +_GC_PULL_TIMEOUT = 95.0 # aiohttp cap > the Pub/Sub server long-poll hold (~90s) +_GC_LOG = logging.getLogger("gateway.platforms.google_chat") + + +class _RestPubsubMessage: + """Present a REST ``receivedMessage`` as the four members + ``_on_pubsub_message`` touches: ``.data``, ``.attributes``, ``.ack()``, ``.nack()``.""" + + def __init__(self, pmsg, ack_sink, ack_id): + import base64 + + self._ack_sink = ack_sink + self._ack_id = ack_id + raw = pmsg.get("data") or "" + self.data = base64.b64decode(raw) if raw else b"" + self.attributes = pmsg.get("attributes") or {} + + def ack(self): + if self._ack_id: + self._ack_sink.append(self._ack_id) + + def nack(self): + # Omit the ackId from the batch -> Pub/Sub redelivers, matching the + # streaming client's message.nack() semantics. + pass + + +def _gc_placeholder_credentials(): + """Credentials whose token is the placeholder the L7 proxy swaps. + + ``refresh()`` is a no-op: nothing is signed inside the sandbox. + """ + from google.auth import credentials as ga_credentials + + class _PlaceholderCredentials(ga_credentials.Credentials): + def __init__(self): + super().__init__() + self.token = _GC_REST_PLACEHOLDER_TOKEN + + def refresh(self, request): # signed gateway-side; nothing to do here + self.token = _GC_REST_PLACEHOLDER_TOKEN + + return _PlaceholderCredentials() + + +def _gc_gateway_proxy_url(): + """Return the egress proxy URL, or ``""`` for direct egress. + + Read from the gateway process's immutable ``/proc//environ``, not + ``os.environ``: + + * the gateway clears the proxy vars during an agent turn; + * ``/proc/self`` is not the gateway — replies run in an ``asyncio.to_thread`` + worker whose environ never carried the launcher-set proxy. + """ + import glob + + for cmd_path in glob.glob("/proc/[0-9]*/cmdline"): + try: + with open(cmd_path, "rb") as cmdline_handle: + cmdline = cmdline_handle.read() + except OSError: + continue + if b"hermes.real" not in cmdline or b"gateway" not in cmdline or b"dashboard" in cmdline: + continue + try: + with open(cmd_path.rsplit("/", 1)[0] + "/environ", "rb") as handle: + env = {} + for pair in handle.read().split(b"\0"): + key, sep, value = pair.partition(b"=") + if sep: + env[key.decode("utf-8", "replace")] = value.decode("utf-8", "replace") + except OSError: + continue + url = ( + env.get("https_proxy") + or env.get("HTTPS_PROXY") + or env.get("http_proxy") + or env.get("HTTP_PROXY") + or "" + ) + if url: + return url + return "" + + +class _GcAiohttpTransport: + """``httplib2.Http``-shaped transport (``.request()`` only) built on aiohttp. + + googleapiclient calls only ``.request(uri, method, body, headers)`` and reads + ``resp.status`` plus content, so an ``httplib2.Response`` satisfies it. + + Why httplib2 cannot be used here: + + * without PySocks, ``ProxyInfo.isgood()`` is falsy and it connects direct, + which the proxy-only sandbox netns cannot resolve; + * with PySocks, ``PROXY_TYPE_HTTP`` refuses the CONNECT tunnel. + + aiohttp CONNECT-tunnels exactly as the inbound pull does, from the one process + the proxy authorizes and injects the minted token for. + """ + + def request(self, uri, method="GET", body=None, headers=None, **kwargs): + import aiohttp + + import plugins.platforms.google_chat.adapter as _gc + + proxy = _gc_gateway_proxy_url() or None + data = body.encode("utf-8") if isinstance(body, str) else body + + async def _run(): + # Mirror the inbound :pull exactly: ClientSession(trust_env=True) with no + # ssl override. That is the transport the L7 proxy authorizes for this + # gateway process, and its default TLS trust already accepts the proxy's + # MITM chain (pinning a hand-built CA context rejected it). The proxy is + # still passed explicitly because os.environ may be cleared by reply time. + async with aiohttp.ClientSession(trust_env=True) as session: + async with session.request( + method, + uri, + data=data, + headers=dict(headers or {}), + proxy=proxy, + timeout=aiohttp.ClientTimeout(total=60), + ) as resp: + content = await resp.read() + info = {"status": resp.status} + for key, value in resp.headers.items(): + info[key.lower()] = value + return _gc.httplib2.Response(info), content + + # The Chat calls run under ``asyncio.to_thread`` (a worker thread with no + # running event loop), so a fresh loop here via asyncio.run is safe. + return asyncio.run(_run()) + + +def _gc_spec_available(module_name: str) -> bool: + try: + return importlib.util.find_spec(module_name) is not None + except (ImportError, ValueError): + return False + + +def _nemoclaw_gc_check() -> bool: + """Passive probe: aiohttp for the pull, google SDKs for the inherited paths.""" + return all( + _gc_spec_available(module_name) + for module_name in ("aiohttp", "google.auth", "google.cloud.pubsub_v1") + ) + + +def _sandbox_adapter_class(): + """Build the subclass lazily: the bundled module only imports inside the sandbox.""" + import plugins.platforms.google_chat.adapter as _gc + + class SandboxGoogleChatAdapter(_gc.GoogleChatAdapter): + """Bundled adapter with the sandbox transport and credential delta.""" + + _sandbox_subscription = None + + def _validate_config(self): + """Report no subscription so the bundled ``connect()`` skips two steps: + + * its gRPC subscriber precheck, fatal under a REST-only egress policy; + * its own supervisor, replaced here by ``_rest_pull``. + + Validation still runs upstream; the real subscription is kept for the pull. + """ + project_id, subscription_path = super()._validate_config() + self._sandbox_subscription = subscription_path + return project_id, None + + def _load_sa_credentials(self): + """Return the placeholder the L7 proxy swaps; nothing is signed here.""" + return _gc_placeholder_credentials() + + def _new_authed_http(self): + """Route every Chat REST call through aiohttp instead of httplib2. + + Covers reply create, message patch, typing card and bot-id lookup. + Credentials stay the placeholder the L7 proxy swaps. + """ + import plugins.platforms.google_chat.adapter as _gc + + return _gc.AuthorizedHttp(self._credentials, http=_GcAiohttpTransport()) + + async def connect(self, *, is_reconnect: bool = False) -> bool: + """Run the bundled connect(), then start the REST pull it skipped.""" + connected = await super().connect(is_reconnect=is_reconnect) + if connected and self._sandbox_subscription: + self._supervisor_task = asyncio.create_task(self._rest_pull()) + _GC_LOG.info( + "[GoogleChat][NemoClaw] keyless REST pull active (no gRPC subscriber)" + ) + return connected + + async def _rest_pull(self): + """Pull the subscription over the Pub/Sub REST unary API. + + Started by ``connect()`` in place of the bundled gRPC supervisor. + Messages reach the unchanged ``_on_pubsub_message`` in a worker thread, + keeping its off-the-event-loop contract. + """ + import aiohttp + + sub = self._sandbox_subscription + pull_url = f"https://pubsub.googleapis.com/v1/{sub}:pull" + ack_url = f"https://pubsub.googleapis.com/v1/{sub}:acknowledge" + headers = { + "Authorization": f"Bearer {_GC_REST_PLACEHOLDER_TOKEN}", + "Content-Type": "application/json", + } + _GC_LOG.info( + "[GoogleChat][NemoClaw] keyless Pub/Sub REST :pull transport active (sub=%s)", + sub, + ) + async with aiohttp.ClientSession(trust_env=True) as session: + while not self._shutting_down: + try: + async with session.post( + pull_url, + json={"maxMessages": self._max_messages or 1}, + headers=headers, + timeout=aiohttp.ClientTimeout(total=_GC_PULL_TIMEOUT), + ) as resp: + if resp.status != 200: + body = await resp.text() + _GC_LOG.warning( + "[GoogleChat][NemoClaw] :pull HTTP %s: %s", + resp.status, + body[:200], + ) + await asyncio.sleep(3) + continue + payload = await resp.json() + except asyncio.CancelledError: + raise + except Exception as exc: # noqa: BLE001 - transient pull errors are retried + _GC_LOG.warning("[GoogleChat][NemoClaw] :pull error: %s", exc) + await asyncio.sleep(3) + continue + + received = payload.get("receivedMessages") or [] + if not received: + await asyncio.sleep(0.2) # guard against fast-empty long-poll returns + continue + + acks: list = [] + for received_message in received: + shim = _RestPubsubMessage( + received_message.get("message") or {}, + acks, + received_message.get("ackId"), + ) + try: + await asyncio.to_thread(self._on_pubsub_message, shim) + except Exception: # noqa: BLE001 - one bad message must not kill the loop + _GC_LOG.exception("[GoogleChat][NemoClaw] message handler raised") + + if acks: + try: + async with session.post( + ack_url, + json={"ackIds": acks}, + headers=headers, + timeout=aiohttp.ClientTimeout(total=30), + ) as ack_resp: + if ack_resp.status != 200: + _GC_LOG.warning( + "[GoogleChat][NemoClaw] :acknowledge HTTP %s", + ack_resp.status, + ) + except Exception as exc: # noqa: BLE001 - a failed ack just redelivers + _GC_LOG.warning("[GoogleChat][NemoClaw] :acknowledge error: %s", exc) + + + return SandboxGoogleChatAdapter + + +def install(ctx) -> None: + """Replace the registered google_chat entry with the sandbox delta. + + ``ctx`` is unused: the platform registry is the single attachment point. + """ + del ctx + try: + from gateway.platform_registry import platform_registry + except Exception: # noqa: BLE001 - keep plugin load resilient + _GC_LOG.exception("[GoogleChat][NemoClaw] platform_registry import failed") + return + + entry = platform_registry.get("google_chat") + if entry is None: + _GC_LOG.error( + "[GoogleChat][NemoClaw] no google_chat platform entry; leaving Hermes untouched" + ) + return + + try: + adapter_class = _sandbox_adapter_class() + except Exception: # noqa: BLE001 - a failed import must not break other platforms + _GC_LOG.exception("[GoogleChat][NemoClaw] building the sandbox adapter failed") + return + + platform_registry.register( + dataclasses.replace( + entry, + adapter_factory=adapter_class, + check_fn=_nemoclaw_gc_check, + required_env=["GOOGLE_CHAT_SUBSCRIPTION_NAME"], + ) + ) + _GC_LOG.info( + "[GoogleChat][NemoClaw] google_chat adapter replaced with the sandbox delta " + "(bundled entry metadata preserved)" + ) diff --git a/src/lib/onboard/messaging-bridge-provider.ts b/src/lib/onboard/messaging-bridge-provider.ts index 0205702e9b6..79c7c889c30 100644 --- a/src/lib/onboard/messaging-bridge-provider.ts +++ b/src/lib/onboard/messaging-bridge-provider.ts @@ -88,10 +88,8 @@ export interface MessagingBridgeSecretResolveDeps { export interface CollectMessagingBridgeTokenDefsInput extends MessagingBridgeSecretResolveDeps { readonly sandboxName: string; /** - * Sandbox agent. Bridge profiles are per-agent (`provider-profile/.yaml`), - * and a channel can ship both openclaw and hermes profiles (Google Chat does), so - * only the profile matching this sandbox's agent must produce a bridge — otherwise - * an openclaw sandbox would also mint the hermes bridge under the same name. + * Sandbox agent. Bridge profiles are per-agent, and a channel may ship both + * (Google Chat does), so only the matching profile may produce a bridge. */ readonly agent: MessagingAgentId; readonly enabledChannels: readonly string[] | null; @@ -407,12 +405,9 @@ function buildRefreshMaterial( { key: "client_email", value: clientEmail }, { key: "private_key", value: privateKey }, ]; - // Scope comes from the profile's declared refresh scopes (single source of - // truth). Join all of them space-separated so ONE minted token carries every - // scope — a Google service-account JWT mints a multi-scope token from a - // space-separated `scope` claim. Hermes Google Chat needs chat.bot AND pubsub - // in a single credential (reply + Pub/Sub REST pull); taking only scopes[0] - // dropped pubsub and made `:pull` fail with 403 "insufficient scopes". + // Join every declared scope space-separated so ONE minted token carries all + // of them. Hermes Google Chat needs chat.bot AND pubsub in a single + // credential; taking only scopes[0] made `:pull` fail with 403. if (profile.scopes.length > 0) { material.push({ key: "scope", value: profile.scopes.join(" ") }); } diff --git a/test/hermes-image-build-probes.test.ts b/test/hermes-image-build-probes.test.ts index 22d52590b08..4d88925a461 100644 --- a/test/hermes-image-build-probes.test.ts +++ b/test/hermes-image-build-probes.test.ts @@ -23,6 +23,7 @@ const commands = [ "discord-reopen", "gateway-process-identity", "gateway-runtime-metadata", + "googlechat-override-seams", "langfuse-credentials", "neutral-platform-inertness", "profile-policy", From 8428a6871b412a4cb82a29a62b902cc9092ce570 Mon Sep 17 00:00:00 2001 From: Hung Le Date: Tue, 18 Aug 2026 16:17:07 +0530 Subject: [PATCH 05/15] fix(hermes): vendor the Google Chat wheels the managed union installs offline The capability-union layer installs every channel's Hermes packages with `--network=none` from a read-only wheel set, so the three Google Chat specs this branch added to the manifest had nothing to resolve against. uv failed with "google-cloud-pubsub was not found in the cache", which took the whole atomic install down with it, and the build surfaced that four steps later as a missing microsoft-teams-apps. Google Chat is the only channel whose Python dependencies Hermes does not package. The base image syncs `anthropic messaging web pty mcp`, and the `messaging` extra already carries telegram, discord, and slack, while WhatsApp runs through a Node bridge. Hermes declares no google_chat extra at all, and `_load_google_modules()` imports pubsub, googleapiclient, and grpc all-or-nothing even though this integration pulls Pub/Sub over REST. Vendor only the 18 packages the base venv lacks; uv satisfies the remaining 10 from the installed distributions, the same way the existing Teams wheels rely on fastapi and cryptography already being present. Only grpcio needs a per-architecture wheel. --- agents/hermes/Dockerfile | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/agents/hermes/Dockerfile b/agents/hermes/Dockerfile index 1519f9ccd3d..5593edf3194 100644 --- a/agents/hermes/Dockerfile +++ b/agents/hermes/Dockerfile @@ -141,7 +141,7 @@ COPY agents/hermes/validate-cli-adapter.py /usr/local/lib/nemoclaw/validate-herm COPY agents/hermes/hermes-cli-adapter-v1.json /usr/local/share/nemoclaw/hermes-cli-adapter-v1.json -# Fetch the exact managed Teams capability wheels outside RUN instructions so +# Fetch the exact managed capability wheels outside RUN instructions so # both the standard build and the protected --network none rebuild install the # reviewed union from immutable inputs rather than a reusable networked layer. FROM scratch AS hermes-managed-teams-common-wheels @@ -152,13 +152,38 @@ ADD --chmod=0444 --checksum=sha256:e2b0257d9b8782830df61eb6aa993a1ddc0349daddd84 ADD --chmod=0444 --checksum=sha256:c61057695b9f1a97de9b6f54f0c66206903f56c22427b0bca31e0fc34da49311 https://files.pythonhosted.org/packages/8d/91/01e6aeddd78639c74489c24785f2cec0f842ba16f3b594db202386bd721d/microsoft_teams_common-2.0.15-py3-none-any.whl /microsoft_teams_common-2.0.15-py3-none-any.whl ADD --chmod=0444 --checksum=sha256:dd17e95a7c71bce75e8108113438ba7c4a086b3bcad4f57a8c09b7af3d753c2d https://files.pythonhosted.org/packages/94/b0/d807279f4b55d16d1f120d5ac4344c6e39b56732e2a224d40bded7fd67ad/msal-1.37.0-py3-none-any.whl /msal-1.37.0-py3-none-any.whl +# Google Chat needs the google SDKs, which Hermes does not package: its +# pyproject declares no google_chat extra, and `_load_google_modules()` +# imports pubsub, googleapiclient, and grpc all-or-nothing even in REST-pull +# mode. Vendor only what the base venv lacks; uv satisfies the rest from the +# installed distributions. +ADD --chmod=0444 --checksum=sha256:cdf9c67e7ca2402d86ccbfde5f2503fc83e3cc3f58cc78456ae96cad24a6d2de https://files.pythonhosted.org/packages/bc/c1/a8a92ae1bc4b1a8f804c776d7d3f0c771b78a62c3ad4df1be41b3fd8c767/google_api_core-2.34.0-py3-none-any.whl /google_api_core-2.34.0-py3-none-any.whl +ADD --chmod=0444 --checksum=sha256:61eaaac3b8fc8fdf11c08af87abc3d1342d1b37319cc1b57405f86ef7697e717 https://files.pythonhosted.org/packages/b0/34/5a624e49f179aa5b0cb87b2ce8093960299030ff40423bfbde09360eb908/google_api_python_client-2.194.0-py3-none-any.whl /google_api_python_client-2.194.0-py3-none-any.whl +ADD --chmod=0444 --checksum=sha256:eada68dfd52b3b81191827601e2a0c3fa12540c818534b630ddc5355769c3995 https://files.pythonhosted.org/packages/e8/1d/f6d3ca1ad0725f2e08a1c6915640748a52de2e66596160a4d53b010cccf0/google_auth-2.55.1-py3-none-any.whl /google_auth-2.55.1-py3-none-any.whl +ADD --chmod=0444 --checksum=sha256:67088a8387dc3f66016a690634f1cb9b7a48a31406f8998e088efc64203e7a12 https://files.pythonhosted.org/packages/68/8a/96410d0c3e02b584d1e7f5d4000b2e1710855650b226cd0601f2b154940a/google_auth_httplib2-0.4.1-py3-none-any.whl /google_auth_httplib2-0.4.1-py3-none-any.whl +ADD --chmod=0444 --checksum=sha256:7210d691a46d7a66559696899ebe6eb731e63de29b624964b3be4dd2d12d3e19 https://files.pythonhosted.org/packages/93/20/dd0b27d4ad4577c062e77ff968ca3e2d404186cd78c8a2a53a0ef5fe5389/google_cloud_pubsub-2.39.0-py3-none-any.whl /google_cloud_pubsub-2.39.0-py3-none-any.whl +ADD --chmod=0444 --checksum=sha256:28a1934bcd33b9c9da66ac301a0a4227e3367f095a17d0375cb98f0a09d93b79 https://files.pythonhosted.org/packages/9a/51/186c02b8549b69ccda44429cf6ff5081e4b61a602ddfe6a8020d1be31d1b/googleapis_common_protos-1.75.1-py3-none-any.whl /googleapis_common_protos-1.75.1-py3-none-any.whl +ADD --chmod=0444 --checksum=sha256:0f5e680b20aa0a9441e68c769da04d94d70fca4e43751a82d8abb8aa6a7181ca https://files.pythonhosted.org/packages/84/ab/be3ad0d46cffe35fd1e7cc3f9947edd6cb3c552229de3be2742f15f7ea47/grpc_google_iam_v1-0.14.5-py3-none-any.whl /grpc_google_iam_v1-0.14.5-py3-none-any.whl +ADD --chmod=0444 --checksum=sha256:f6a838a7c5fb84ae98833ec0ef81ed438c26e11e54b2ddb8e92ad328c861de69 https://files.pythonhosted.org/packages/d6/00/73204406228cf989bea6b0fd9fe4702fab49a8a152a0c6f90856dadb6ac7/grpcio_status-1.83.0-py3-none-any.whl /grpcio_status-1.83.0-py3-none-any.whl +ADD --chmod=0444 --checksum=sha256:dc6705cacdf3fb0a2aba7629fa33c90fd93e30035db0c157325826be177e4816 https://files.pythonhosted.org/packages/33/a0/550eec327e5f5c7b732531c489f5307efec41f047b0d703bd4ca1e5ad2db/httplib2-0.32.0-py3-none-any.whl /httplib2-0.32.0-py3-none-any.whl +ADD --chmod=0444 --checksum=sha256:df081c4c6bcfdb1211e3e86140376792643128a25f8d72d1d27675936e7e96ad https://files.pythonhosted.org/packages/e7/23/ff077e61886ee020a17ce9c8b6fa11c601c8d8345b09ea24f605445df62a/opentelemetry_sdk-1.44.0-py3-none-any.whl /opentelemetry_sdk-1.44.0-py3-none-any.whl +ADD --chmod=0444 --checksum=sha256:1cacde7b0ad306f84c5ef08c3dbe1bbaf20165bba6f8bff43b670e555a086bcb https://files.pythonhosted.org/packages/a6/0e/49df70d9b81fb5cbae4bbf2a49d865b09bcbcbc4eb53f5851b1027738d78/opentelemetry_semantic_conventions-0.65b0-py3-none-any.whl /opentelemetry_semantic_conventions-0.65b0-py3-none-any.whl +ADD --chmod=0444 --checksum=sha256:dc76880b8ee951cca002098574376cf71e055f9f16d9ba6570fb8a06f726d281 https://files.pythonhosted.org/packages/61/3a/cfee3c50294f55a2f0f9575052dec2c2a48891ad4b1c2a133b05a87026cd/proto_plus-1.28.3-py3-none-any.whl /proto_plus-1.28.3-py3-none-any.whl +ADD --chmod=0444 --checksum=sha256:4bc97768d8fe4ad6743c8a19403e314511ed9f6d13205b687e52421c023ac1b9 https://files.pythonhosted.org/packages/19/c7/5f7c636ec43e0c545e28d1f1db71990108306f7bdcb89f069ba97e428e7f/protobuf-7.35.1-py3-none-any.whl /protobuf-7.35.1-py3-none-any.whl +ADD --chmod=0444 --checksum=sha256:deda9277cfd454080ec40b207fb6df82206a3a2688735233cdcd8d3d565f088b https://files.pythonhosted.org/packages/9a/3b/6163796d69c3977d1e4287bea4a6979161cbbdd170ebb430511e8e1999ce/pyasn1-0.6.4-py3-none-any.whl /pyasn1-0.6.4-py3-none-any.whl +ADD --chmod=0444 --checksum=sha256:29253a9207ce32b64c3ac6600edc75368f98473906e8fd1043bd6b5b1de2c14a https://files.pythonhosted.org/packages/47/8d/d529b5d697919ba8c11ad626e835d4039be708a35b0d22de83a269a6682c/pyasn1_modules-0.4.2-py3-none-any.whl /pyasn1_modules-0.4.2-py3-none-any.whl +ADD --chmod=0444 --checksum=sha256:850ba148bd908d7e2411587e247a1e4f0327839c40e2e5e6d05a007ecc69911d https://files.pythonhosted.org/packages/10/bd/c038d7cc38edc1aa5bf91ab8068b63d4308c66c4c8bb3cbba7dfbc049f9c/pyparsing-3.3.2-py3-none-any.whl /pyparsing-3.3.2-py3-none-any.whl +ADD --chmod=0444 --checksum=sha256:962201ba1c4edcab02e60f9a0d3821e82dfc5d2d6662a21abd533879bdb8a686 https://files.pythonhosted.org/packages/a9/99/3ae339466c9183ea5b8ae87b34c0b897eda475d2aec2307cae60e5cd4f29/uritemplate-4.2.0-py3-none-any.whl /uritemplate-4.2.0-py3-none-any.whl + FROM hermes-managed-teams-common-wheels AS hermes-managed-teams-amd64-wheels ADD --chmod=0444 --checksum=sha256:10e481880b307a6a438c1cc7b0a1fa8754247239ef5a2e8fe82bd8a1e76e7682 https://files.pythonhosted.org/packages/68/20/fc1812f20ec75af2d4b5e391e93f15621af3102da46142e26177eb506b32/dependency_injector-4.49.1-cp310-abi3-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl /dependency_injector-4.49.1-cp310-abi3-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl +ADD --chmod=0444 --checksum=sha256:7bd82671b39065ba18cd536e9cd45b27ff649053f81ddd2c6a966d595067080f https://files.pythonhosted.org/packages/e8/c0/539fe0832f2dd6500a28f5263071623fb34e8d4867aec632ccf81bd21156/grpcio-1.83.0-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.whl /grpcio-1.83.0-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.whl FROM hermes-managed-teams-common-wheels AS hermes-managed-teams-arm64-wheels ADD --chmod=0444 --checksum=sha256:e05da5bc73a3e026f962a223672002934c0f415064b6e2c3db0b255e46c7b521 https://files.pythonhosted.org/packages/32/ab/b1e1826aacc37d07ba0101230de7b5cbbb5ac6364b78ca3957f0a90d6a51/dependency_injector-4.49.1-cp310-abi3-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl /dependency_injector-4.49.1-cp310-abi3-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl +ADD --chmod=0444 --checksum=sha256:10b3fa0475eb572c9a81a6fe37fa16a9c500c0c91cfc148cac15692b7e3c2867 https://files.pythonhosted.org/packages/b0/e8/d0389e09cd6b4c4d3089b92967ae4e3ffd64795bd349bf2f85cd6656d3da/grpcio-1.83.0-cp313-cp313-manylinux2014_aarch64.manylinux_2_17_aarch64.whl /grpcio-1.83.0-cp313-cp313-manylinux2014_aarch64.manylinux_2_17_aarch64.whl FROM scratch AS hermes-managed-teams-0-wheels From f7c76c8eaef9a9ae8f5863afd1b2a18e1597f7a4 Mon Sep 17 00:00:00 2001 From: Hung Le Date: Tue, 18 Aug 2026 17:28:05 +0530 Subject: [PATCH 06/15] fix(hermes): vendor opentelemetry-api for the managed union install The wheel set landed one package short: `google-cloud-pubsub` requires `opentelemetry-api>=1.27.0`, and the base venv does not carry it, so the offline union install still had nothing to resolve against. The first pass derived the missing set from a local Hermes checkout rather than the version the image pins. Recomputed against the exact `HERMES_VERSION=v2026.7.20` tarball, whose checksum matches `HERMES_TARBALL_SHA256`, with `agents/hermes/security-dependencies.patch` applied: 19 of the 28 packages are missing, not 18. --- agents/hermes/Dockerfile | 1 + 1 file changed, 1 insertion(+) diff --git a/agents/hermes/Dockerfile b/agents/hermes/Dockerfile index 5593edf3194..4fd553cff07 100644 --- a/agents/hermes/Dockerfile +++ b/agents/hermes/Dockerfile @@ -166,6 +166,7 @@ ADD --chmod=0444 --checksum=sha256:28a1934bcd33b9c9da66ac301a0a4227e3367f095a17d ADD --chmod=0444 --checksum=sha256:0f5e680b20aa0a9441e68c769da04d94d70fca4e43751a82d8abb8aa6a7181ca https://files.pythonhosted.org/packages/84/ab/be3ad0d46cffe35fd1e7cc3f9947edd6cb3c552229de3be2742f15f7ea47/grpc_google_iam_v1-0.14.5-py3-none-any.whl /grpc_google_iam_v1-0.14.5-py3-none-any.whl ADD --chmod=0444 --checksum=sha256:f6a838a7c5fb84ae98833ec0ef81ed438c26e11e54b2ddb8e92ad328c861de69 https://files.pythonhosted.org/packages/d6/00/73204406228cf989bea6b0fd9fe4702fab49a8a152a0c6f90856dadb6ac7/grpcio_status-1.83.0-py3-none-any.whl /grpcio_status-1.83.0-py3-none-any.whl ADD --chmod=0444 --checksum=sha256:dc6705cacdf3fb0a2aba7629fa33c90fd93e30035db0c157325826be177e4816 https://files.pythonhosted.org/packages/33/a0/550eec327e5f5c7b732531c489f5307efec41f047b0d703bd4ca1e5ad2db/httplib2-0.32.0-py3-none-any.whl /httplib2-0.32.0-py3-none-any.whl +ADD --chmod=0444 --checksum=sha256:94b98c893a91b88657eaac1e3ba89618cdb85be6918196705354f34728b2cdef https://files.pythonhosted.org/packages/ca/6f/a04e900f465ff3221ccc395522503e2d10e79fa21f2723c8e177aae1e0d1/opentelemetry_api-1.44.0-py3-none-any.whl /opentelemetry_api-1.44.0-py3-none-any.whl ADD --chmod=0444 --checksum=sha256:df081c4c6bcfdb1211e3e86140376792643128a25f8d72d1d27675936e7e96ad https://files.pythonhosted.org/packages/e7/23/ff077e61886ee020a17ce9c8b6fa11c601c8d8345b09ea24f605445df62a/opentelemetry_sdk-1.44.0-py3-none-any.whl /opentelemetry_sdk-1.44.0-py3-none-any.whl ADD --chmod=0444 --checksum=sha256:1cacde7b0ad306f84c5ef08c3dbe1bbaf20165bba6f8bff43b670e555a086bcb https://files.pythonhosted.org/packages/a6/0e/49df70d9b81fb5cbae4bbf2a49d865b09bcbcbc4eb53f5851b1027738d78/opentelemetry_semantic_conventions-0.65b0-py3-none-any.whl /opentelemetry_semantic_conventions-0.65b0-py3-none-any.whl ADD --chmod=0444 --checksum=sha256:dc76880b8ee951cca002098574376cf71e055f9f16d9ba6570fb8a06f726d281 https://files.pythonhosted.org/packages/61/3a/cfee3c50294f55a2f0f9575052dec2c2a48891ad4b1c2a133b05a87026cd/proto_plus-1.28.3-py3-none-any.whl /proto_plus-1.28.3-py3-none-any.whl From 2d7867df99a964313120a643d3566d5b80792ca8 Mon Sep 17 00:00:00 2001 From: Hung Le Date: Tue, 18 Aug 2026 17:03:47 +0530 Subject: [PATCH 07/15] test(messaging): cover the Hermes Google Chat REST pull boundary Nothing checked in exercised the adapter itself. The image-build probe pins upstream method names in the bundled source, and the runtime contract test covers the OpenClaw side, so a regression could have sent an unapproved request, replaced the credential placeholder, or dropped a message whose acknowledgement failed without any of it failing a check. Drive the real `_rest_pull` under `python3` against two doubles supplied on PYTHONPATH, one for aiohttp and one for the bundled Hermes adapter, and assert what crosses the wire: every request carries the placeholder bearer and nothing else, the only two URLs reached are `:pull` and `:acknowledge` on the configured subscription, a nacked message produces no acknowledge, and both acknowledgement failure modes leave the message to be redelivered instead of ending the pull loop. Each assertion was checked against a mutation of the adapter that it is meant to catch: a changed placeholder, `:acknowledge` swapped for `:modifyAckDeadline`, a rejected acknowledge breaking the loop, the transport error escaping its handler, and `nack()` acknowledging anyway. --- .../googlechat/runtime/hermes-adapter.test.ts | 264 ++++++++++++++++++ 1 file changed, 264 insertions(+) create mode 100644 src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts diff --git a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts new file mode 100644 index 00000000000..71597251760 --- /dev/null +++ b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts @@ -0,0 +1,264 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; + +const ADAPTER = path.join(path.dirname(fileURLToPath(import.meta.url)), "hermes-adapter.py"); +const PLACEHOLDER = "openshell:resolve:env:GOOGLE_CHAT_ACCESS_TOKEN"; +const SUBSCRIPTION = "projects/nemoclaw-test/subscriptions/hermes-chat"; + +// Stand-ins for the two imports the override reaches for at runtime. The bundled +// Hermes adapter and aiohttp both live in the sandbox image, so the checked-in +// test supplies the smallest surface `_rest_pull` touches. +const HERMES_STUB = ` +class GoogleChatAdapter: + """Only what the subclass definition needs; _rest_pull calls none of it.""" + + +class AuthorizedHttp: + def __init__(self, credentials, http=None): + self.credentials = credentials + self.http = http +`; + +const AIOHTTP_STUB = ` +"""Recording aiohttp double. Every request lands in REQUESTS; responses are scripted.""" + +REQUESTS = [] +SCRIPT = [] + + +class ClientTimeout: + def __init__(self, total=None): + self.total = total + + +class _Response: + def __init__(self, status, payload=None, text=""): + self.status = status + self._payload = payload if payload is not None else {} + self._text = text + + async def json(self): + return self._payload + + async def text(self): + return self._text + + async def __aenter__(self): + return self + + async def __aexit__(self, *exc_info): + return False + + +class ClientSession: + def __init__(self, *args, **kwargs): + self.kwargs = kwargs + + async def __aenter__(self): + return self + + async def __aexit__(self, *exc_info): + return False + + def post(self, url, json=None, headers=None, timeout=None): + REQUESTS.append( + { + "url": url, + "method": "POST", + "authorization": (headers or {}).get("Authorization"), + "body": json, + } + ) + if not SCRIPT: + raise AssertionError("aiohttp double ran out of scripted responses: " + url) + status, payload, on_send = SCRIPT.pop(0) + if on_send is not None: + on_send() + if status == "transport-error": + raise OSError("proxy refused the acknowledge") + return _Response(status, payload) +`; + +// Drives the real _rest_pull against the doubles above and prints what crossed the +// wire. Scenario names match the test titles below. +const DRIVER = ` +import asyncio +import base64 +import importlib.util +import json +import sys + +import aiohttp + +ADAPTER_PATH, SCENARIO = sys.argv[1], sys.argv[2] +SUBSCRIPTION = ${JSON.stringify(SUBSCRIPTION)} + +spec = importlib.util.spec_from_file_location("nemoclaw_googlechat_adapter", ADAPTER_PATH) +module = importlib.util.module_from_spec(spec) +spec.loader.exec_module(module) + +adapter = object.__new__(module._sandbox_adapter_class()) +adapter._sandbox_subscription = SUBSCRIPTION +adapter._shutting_down = False +adapter._max_messages = 1 + +handled = [] + + +def _received(ack_id, text): + return { + "receivedMessages": [ + { + "ackId": ack_id, + "message": {"data": base64.b64encode(text.encode()).decode(), "attributes": {}}, + } + ] + } + + +def _stop(): + adapter._shutting_down = True + + +def _handler(message): + handled.append(message.data.decode()) + if SCENARIO == "nack": + message.nack() + else: + message.ack() + + +adapter._on_pubsub_message = _handler + +if SCENARIO == "acknowledged": + aiohttp.SCRIPT.extend( + [ + (200, _received("ack-1", "hello"), None), + (200, {}, _stop), + ] + ) +elif SCENARIO == "acknowledge-fails": + # The ack is rejected, so Pub/Sub redelivers the same ackId on the next pull. + aiohttp.SCRIPT.extend( + [ + (200, _received("ack-1", "hello"), None), + (500, {}, None), + (200, _received("ack-1", "hello"), None), + (200, {}, _stop), + ] + ) +elif SCENARIO == "acknowledge-raises": + # The acknowledge never reaches Pub/Sub, so the same ackId comes back. + aiohttp.SCRIPT.extend( + [ + (200, _received("ack-1", "hello"), None), + ("transport-error", None, None), + (200, _received("ack-1", "hello"), None), + (200, {}, _stop), + ] + ) +elif SCENARIO == "nack": + aiohttp.SCRIPT.extend([(200, _received("ack-1", "hello"), _stop)]) +else: + raise SystemExit("unknown scenario " + SCENARIO) + +asyncio.run(adapter._rest_pull()) + +print(json.dumps({"requests": aiohttp.REQUESTS, "handled": handled})) +`; + +interface RecordedRequest { + readonly url: string; + readonly method: string; + readonly authorization: string | null; + readonly body: Record | null; +} + +const workspace = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-googlechat-pull-")); + +function runScenario(scenario: string): { requests: RecordedRequest[]; handled: string[] } { + const result = spawnSync("python3", [path.join(workspace, "driver.py"), ADAPTER, scenario], { + encoding: "utf8", + env: { ...process.env, PYTHONPATH: workspace, PYTHONDONTWRITEBYTECODE: "1" }, + timeout: 30_000, + }); + expect(result.status, `${result.stdout}${result.stderr}`).toBe(0); + return JSON.parse(result.stdout.trim().split("\n").at(-1) ?? ""); +} + +describe("Hermes Google Chat keyless REST pull", () => { + beforeAll(() => { + const bundled = path.join(workspace, "plugins", "platforms", "google_chat"); + fs.mkdirSync(bundled, { recursive: true }); + fs.writeFileSync(path.join(workspace, "plugins", "__init__.py"), ""); + fs.writeFileSync(path.join(workspace, "plugins", "platforms", "__init__.py"), ""); + fs.writeFileSync(path.join(bundled, "__init__.py"), ""); + fs.writeFileSync(path.join(bundled, "adapter.py"), HERMES_STUB); + fs.writeFileSync(path.join(workspace, "aiohttp.py"), AIOHTTP_STUB); + fs.writeFileSync(path.join(workspace, "driver.py"), DRIVER); + }); + + afterAll(() => { + fs.rmSync(workspace, { recursive: true, force: true }); + }); + + it("sends the credential placeholder and nothing else on every request", () => { + const { requests } = runScenario("acknowledged"); + + expect(requests.length).toBeGreaterThan(0); + expect(new Set(requests.map((request) => request.authorization))).toEqual( + new Set([`Bearer ${PLACEHOLDER}`]), + ); + }); + + it("reaches no Pub/Sub operation beyond pull and acknowledge", () => { + const { requests, handled } = runScenario("acknowledged"); + + expect(handled).toEqual(["hello"]); + expect(new Set(requests.map((request) => `${request.method} ${request.url}`))).toEqual( + new Set([ + `POST https://pubsub.googleapis.com/v1/${SUBSCRIPTION}:pull`, + `POST https://pubsub.googleapis.com/v1/${SUBSCRIPTION}:acknowledge`, + ]), + ); + const acknowledge = requests.filter((request) => request.url.endsWith(":acknowledge")); + expect(acknowledge.map((request) => request.body)).toEqual([{ ackIds: ["ack-1"] }]); + }); + + it("keeps a message eligible for redelivery when acknowledgement fails", () => { + const { requests, handled } = runScenario("acknowledge-fails"); + + // The rejected acknowledgement neither raises nor ends the pull, so the + // redelivered copy is handled again instead of being lost. + expect(handled).toEqual(["hello", "hello"]); + expect(requests.filter((request) => request.url.endsWith(":pull"))).toHaveLength(2); + expect( + requests + .filter((request) => request.url.endsWith(":acknowledge")) + .map((request) => request.body), + ).toEqual([{ ackIds: ["ack-1"] }, { ackIds: ["ack-1"] }]); + }); + + it("keeps pulling when the acknowledge transport itself fails", () => { + const { requests, handled } = runScenario("acknowledge-raises"); + + // A rejected connection must not escape _rest_pull; letting it end the loop + // would stop inbound delivery for the whole session, not just this message. + expect(handled).toEqual(["hello", "hello"]); + expect(requests.filter((request) => request.url.endsWith(":pull"))).toHaveLength(2); + }); + + it("acknowledges nothing for a message the handler nacks", () => { + const { requests, handled } = runScenario("nack"); + + expect(handled).toEqual(["hello"]); + expect(requests.filter((request) => request.url.endsWith(":acknowledge"))).toEqual([]); + }); +}); From 7f83e8a7459cbe170213eee76acbc2c0045285f7 Mon Sep 17 00:00:00 2001 From: Hung Le Date: Tue, 18 Aug 2026 18:54:02 +0530 Subject: [PATCH 08/15] refactor(messaging): tighten the Google Chat override prose and test scaffolding The review this branch answers is a LOC and simplicity review, and the added surface still carried more explanation than it needed. Compress the module, class, and method docstrings to the facts a reader cannot derive from the code itself, and keep only the three that record a decision: why the L7 proxy set rules out gRPC, why the proxy URL comes from the gateway's /proc environ, and why httplib2 cannot carry the outbound call. Fold the sibling-module loader into the installer it serves, since the split bought only a second docstring. In the adapter test, replace the scripted if/elif chain with a scenario table, drop the unused branches of the aiohttp double, and merge the two cases that ran the same scenario twice. No behavior changes. Each of the five adapter mutations the test is meant to catch was replayed against the compressed test: a changed placeholder, `:acknowledge` swapped for `:modifyAckDeadline`, a rejected acknowledge breaking the loop, the transport error escaping its handler, and `nack()` acknowledging anyway. --- agents/hermes/plugin/__init__.py | 37 ++---- .../messaging/channels/googlechat/manifest.ts | 29 ++--- .../googlechat/provider-profile/hermes.yaml | 18 ++- .../googlechat/runtime/hermes-adapter.py | 117 ++++++------------ .../googlechat/runtime/hermes-adapter.test.ts | 103 +++++---------- 5 files changed, 101 insertions(+), 203 deletions(-) diff --git a/agents/hermes/plugin/__init__.py b/agents/hermes/plugin/__init__.py index 75b77053365..f475bcf1c4d 100644 --- a/agents/hermes/plugin/__init__.py +++ b/agents/hermes/plugin/__init__.py @@ -1433,45 +1433,28 @@ def _handle_reload_skills(tool_input=None, context=None, **_kwargs): _GOOGLE_CHAT_MODULE = "googlechat_adapter.py" -def _load_googlechat_adapter(): - """Load the sibling override module by path. - - Hermes loads this plugin as a directory module under a synthetic name, so - there is no package context for a relative import. - """ - path = os.path.join(os.path.dirname(__file__), _GOOGLE_CHAT_MODULE) - spec = importlib.util.spec_from_file_location( - "nemoclaw_hermes_googlechat_adapter", - path, - ) - if spec is None or spec.loader is None: - return None - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - return module - - def _install_googlechat_adapter(ctx): """Install the Google Chat override when that channel is configured. - Returns whether the module was loaded and invoked; it reports its own - registration failures. + The module is loaded by path: Hermes imports this plugin as a directory + module under a synthetic name, so a relative import has no package context. + Load failure must not abort plugin registration, but it has to be visible — + without the override the bundled gRPC adapter hangs under the REST-only + egress policy and the channel goes quiet with no other clue. """ if not _get_env_value(_GOOGLE_CHAT_SUBSCRIPTION_ENV): return False + path = os.path.join(os.path.dirname(__file__), _GOOGLE_CHAT_MODULE) try: - module = _load_googlechat_adapter() + spec = importlib.util.spec_from_file_location("nemoclaw_hermes_googlechat", path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + module.install(ctx) except Exception: - # Load failure must not abort plugin registration, but it has to be - # visible: without the override the bundled gRPC adapter hangs under the - # REST-only egress policy and the channel goes quiet with no other clue. logging.getLogger("gateway.platforms.google_chat").exception( "[GoogleChat][NemoClaw] loading %s failed", _GOOGLE_CHAT_MODULE, ) return False - if module is None: - return False - module.install(ctx) return True diff --git a/src/lib/messaging/channels/googlechat/manifest.ts b/src/lib/messaging/channels/googlechat/manifest.ts index fd439aa72e2..cd7980ec258 100644 --- a/src/lib/messaging/channels/googlechat/manifest.ts +++ b/src/lib/messaging/channels/googlechat/manifest.ts @@ -118,10 +118,9 @@ export const googlechatManifest = { }, }, // ── Hermes-only Pub/Sub pull config ── - // OpenClaw uses an inbound webhook and ignores these. Hermes supports a - // webhook too, but NemoClaw pulls instead — no public inbound URL needed — - // so it needs the project and subscription. Hermes-gated prompt, rendered - // only into ~/.hermes/.env. + // Hermes supports a webhook too, but NemoClaw pulls instead, so it needs the + // project and subscription. OpenClaw ignores both. Rendered only into + // ~/.hermes/.env. { id: "projectId", kind: "config", @@ -168,9 +167,8 @@ export const googlechatManifest = { { name: "googlechat", policyKeys: ["googlechat"], - // Hermes reaches Google Chat over Pub/Sub REST pull + Chat REST reply — a - // different egress shape from OpenClaw's inbound webhook — so it resolves a - // distinct concrete policy key backed by policy/hermes.yaml. + // Pub/Sub REST pull + Chat REST reply is a different egress shape from + // OpenClaw's inbound webhook, so it resolves its own policy key. agentPolicyKeys: { hermes: ["googlechat_hermes"], }, @@ -248,9 +246,8 @@ export const googlechatManifest = { }, }, // ── Hermes render ── - // Neither the SA JSON nor a token reaches the sandbox: the bridge provider - // mints one gateway-side and the runtime asset sends only the placeholder, - // which the L7 proxy swaps. Non-secret pull config and the allowlist only. + // Non-secret pull config and the allowlist only: no SA JSON or token reaches + // the sandbox, which sends the placeholder the L7 proxy swaps. { id: "googlechat-hermes-env", kind: "env-lines", @@ -348,9 +345,8 @@ export const googlechatManifest = { }, required: true, }, - // The base image ships aiohttp but not the google-* SDKs, which the - // inherited connect() and reply path both need. The Hermes-side delta lives - // in runtime/hermes-adapter.py. + // The base image ships aiohttp but not the google-* SDKs, which the inherited + // connect() and reply path both need. { id: "hermesGooglePubsubPackage", agent: "hermes", @@ -375,10 +371,9 @@ export const googlechatManifest = { ], hooks: [ { - // OpenClaw-only: this gates the inbound webhook audience. NemoClaw runs - // Hermes in Pub/Sub pull mode, which serves no inbound webhook, so this - // skip-channel gate must not run for Hermes (it would otherwise skip the - // channel and drop its policy preset). + // OpenClaw-only: gates the inbound webhook audience. Hermes pull mode + // serves no webhook, and running this gate would skip the channel and drop + // its policy preset. id: "googlechat-tunnel-audience-gate", phase: "enroll", handler: "googlechat.tunnelAudienceGate", diff --git a/src/lib/messaging/channels/googlechat/provider-profile/hermes.yaml b/src/lib/messaging/channels/googlechat/provider-profile/hermes.yaml index 4b04d0a0ea2..9f6af94e8de 100644 --- a/src/lib/messaging/channels/googlechat/provider-profile/hermes.yaml +++ b/src/lib/messaging/channels/googlechat/provider-profile/hermes.yaml @@ -1,12 +1,10 @@ # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 # -# Google Chat bridge for Hermes: keyless reply + Pub/Sub pull. -# -# * The gateway mints one chat.bot + pubsub token and injects it on both hosts. -# * The private key stays gateway-side; the sandbox holds only the placeholder. -# * messaging-bridge-provider.ts supplies the material at onboard, the key via -# `--secret-material-env` so it never reaches argv. +# Google Chat bridge for Hermes: keyless reply + Pub/Sub pull. The gateway mints +# one chat.bot + pubsub token and injects it on both hosts; the private key stays +# gateway-side, delivered by messaging-bridge-provider.ts through +# `--secret-material-env` so it never reaches argv or the sandbox. id: google-chat-hermes-bridge display_name: Google Chat Bridge (Hermes) description: Gateway-minted Google Chat bot token (chat.bot + pubsub) for Hermes Pub/Sub pull + Chat reply @@ -46,11 +44,9 @@ endpoints: protocol: rest access: read-write enforcement: enforce -# OpenShell resolves the process to its real interpreter via /proc/pid/exe and -# matches binaries EXACTLY — a `/usr/bin/python3*` glob does not cover -# `python3.13`, and an unmatched binary is 403'd at CONNECT. Hermes' venv python -# resolves to /usr/bin/python3.13, so list the concrete path. Revisit when the -# base image bumps Python. +# OpenShell resolves the process via /proc/pid/exe and matches binaries EXACTLY: +# a `/usr/bin/python3*` glob misses `python3.13` and is 403'd at CONNECT, so list +# the concrete path Hermes' venv resolves to. Revisit on a base-image Python bump. binaries: - /opt/hermes/.venv/bin/python - /usr/bin/python3 diff --git a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py index ff5efe53b6b..36d9512e039 100644 --- a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py +++ b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py @@ -2,33 +2,22 @@ # SPDX-License-Identifier: Apache-2.0 """Google Chat connect-time shim for Hermes inside a NemoClaw sandbox. -Channel-owned runtime asset (`src/lib/messaging/AGENTS.md`). OpenClaw's -equivalents are the sibling `googlechat-*.ts` preloads. - -What a sandbox forces: - -* Inbound — the OpenShell L7 protocol set has no gRPC, so the bundled - StreamingPull cannot be inspected and raw relay would defeat the credential - swap. Pull the same subscription over the Pub/Sub REST API instead. -* Outbound — the service-account key stays gateway-side, so the sandbox sends - only the `openshell:resolve:env:GOOGLE_CHAT_ACCESS_TOKEN` placeholder. The - bundled `httplib2` client cannot proxy HTTPS here; aiohttp replaces it. - -How it attaches, through seams Hermes publishes rather than patches: - -* `platform_registry.get()` resolves the bundled entry and forces the deferred - loader, so registration order does not matter. -* `PlatformEntry` is a dataclass, so `dataclasses.replace` keeps every field and - changes only `adapter_factory`, `check_fn` and `required_env`. -* `register()` documents last-writer-wins for exactly this case. - -The delta is a subclass; everything else runs the bundled implementation. - -Upstream coupling: `_validate_config`, `_load_sa_credentials` and -`_new_authed_http` are Hermes internals, because `PlatformEntry` carries no -credential or transport field. `image-build-probes.py googlechat-override-seams` -pins them, so drift fails the image build instead of silently falling back to -the stock gRPC and service-account adapter. +Channel-owned runtime asset (`src/lib/messaging/AGENTS.md`), beside OpenClaw's +`googlechat-*.ts` preloads. The sandbox forces two deltas on the bundled +adapter, and everything else runs unchanged through a subclass: + +* Inbound — the OpenShell L7 protocol set has no gRPC, so StreamingPull cannot + be inspected; pull the same subscription over the Pub/Sub REST API. +* Outbound — the service-account key stays gateway-side, so requests carry the + `openshell:resolve:env:GOOGLE_CHAT_ACCESS_TOKEN` placeholder over aiohttp. + +It attaches through published seams: `platform_registry.get()` forces the +deferred loader, `dataclasses.replace` keeps every `PlatformEntry` field, and +`register()` documents last-writer-wins. `_validate_config`, +`_load_sa_credentials` and `_new_authed_http` are Hermes internals only because +`PlatformEntry` carries no credential or transport field; +`image-build-probes.py googlechat-override-seams` pins them so drift fails the +image build instead of silently restoring the stock gRPC adapter. """ import asyncio @@ -42,8 +31,7 @@ class _RestPubsubMessage: - """Present a REST ``receivedMessage`` as the four members - ``_on_pubsub_message`` touches: ``.data``, ``.attributes``, ``.ack()``, ``.nack()``.""" + """REST ``receivedMessage`` shaped as what ``_on_pubsub_message`` touches.""" def __init__(self, pmsg, ack_sink, ack_id): import base64 @@ -59,16 +47,12 @@ def ack(self): self._ack_sink.append(self._ack_id) def nack(self): - # Omit the ackId from the batch -> Pub/Sub redelivers, matching the - # streaming client's message.nack() semantics. + # Omit the ackId -> Pub/Sub redelivers, matching message.nack(). pass def _gc_placeholder_credentials(): - """Credentials whose token is the placeholder the L7 proxy swaps. - - ``refresh()`` is a no-op: nothing is signed inside the sandbox. - """ + """Credentials carrying the placeholder the L7 proxy swaps; nothing is signed here.""" from google.auth import credentials as ga_credentials class _PlaceholderCredentials(ga_credentials.Credentials): @@ -85,12 +69,8 @@ def refresh(self, request): # signed gateway-side; nothing to do here def _gc_gateway_proxy_url(): """Return the egress proxy URL, or ``""`` for direct egress. - Read from the gateway process's immutable ``/proc//environ``, not - ``os.environ``: - - * the gateway clears the proxy vars during an agent turn; - * ``/proc/self`` is not the gateway — replies run in an ``asyncio.to_thread`` - worker whose environ never carried the launcher-set proxy. + Read from the gateway's ``/proc//environ``: it clears the proxy vars + during a turn, and replies run in a worker thread that never carried them. """ import glob @@ -126,17 +106,10 @@ def _gc_gateway_proxy_url(): class _GcAiohttpTransport: """``httplib2.Http``-shaped transport (``.request()`` only) built on aiohttp. - googleapiclient calls only ``.request(uri, method, body, headers)`` and reads - ``resp.status`` plus content, so an ``httplib2.Response`` satisfies it. - - Why httplib2 cannot be used here: - - * without PySocks, ``ProxyInfo.isgood()`` is falsy and it connects direct, - which the proxy-only sandbox netns cannot resolve; - * with PySocks, ``PROXY_TYPE_HTTP`` refuses the CONNECT tunnel. - - aiohttp CONNECT-tunnels exactly as the inbound pull does, from the one process - the proxy authorizes and injects the minted token for. + googleapiclient only calls ``.request()`` and reads ``resp.status`` plus + content, so an ``httplib2.Response`` satisfies it. httplib2 itself cannot be + used: without PySocks it connects direct, which the proxy-only netns cannot + resolve, and with PySocks ``PROXY_TYPE_HTTP`` refuses the CONNECT tunnel. """ def request(self, uri, method="GET", body=None, headers=None, **kwargs): @@ -148,11 +121,10 @@ def request(self, uri, method="GET", body=None, headers=None, **kwargs): data = body.encode("utf-8") if isinstance(body, str) else body async def _run(): - # Mirror the inbound :pull exactly: ClientSession(trust_env=True) with no - # ssl override. That is the transport the L7 proxy authorizes for this - # gateway process, and its default TLS trust already accepts the proxy's - # MITM chain (pinning a hand-built CA context rejected it). The proxy is - # still passed explicitly because os.environ may be cleared by reply time. + # Mirror the inbound :pull: ClientSession(trust_env=True) with no ssl + # override, whose default trust already accepts the proxy's MITM chain + # (a hand-built CA context was rejected). The proxy is passed explicitly + # because os.environ may be cleared by reply time. async with aiohttp.ClientSession(trust_env=True) as session: async with session.request( method, @@ -168,8 +140,7 @@ async def _run(): info[key.lower()] = value return _gc.httplib2.Response(info), content - # The Chat calls run under ``asyncio.to_thread`` (a worker thread with no - # running event loop), so a fresh loop here via asyncio.run is safe. + # Chat calls run under ``asyncio.to_thread``, so a fresh loop is safe here. return asyncio.run(_run()) @@ -198,12 +169,9 @@ class SandboxGoogleChatAdapter(_gc.GoogleChatAdapter): _sandbox_subscription = None def _validate_config(self): - """Report no subscription so the bundled ``connect()`` skips two steps: - - * its gRPC subscriber precheck, fatal under a REST-only egress policy; - * its own supervisor, replaced here by ``_rest_pull``. - - Validation still runs upstream; the real subscription is kept for the pull. + """Report no subscription so the bundled ``connect()`` skips its gRPC + precheck and supervisor; upstream validation still runs, and the real + subscription is kept for ``_rest_pull``. """ project_id, subscription_path = super()._validate_config() self._sandbox_subscription = subscription_path @@ -214,10 +182,8 @@ def _load_sa_credentials(self): return _gc_placeholder_credentials() def _new_authed_http(self): - """Route every Chat REST call through aiohttp instead of httplib2. - - Covers reply create, message patch, typing card and bot-id lookup. - Credentials stay the placeholder the L7 proxy swaps. + """Route every Chat REST call (reply, patch, typing card, bot-id lookup) + through aiohttp instead of httplib2, still on the placeholder token. """ import plugins.platforms.google_chat.adapter as _gc @@ -234,11 +200,9 @@ async def connect(self, *, is_reconnect: bool = False) -> bool: return connected async def _rest_pull(self): - """Pull the subscription over the Pub/Sub REST unary API. - - Started by ``connect()`` in place of the bundled gRPC supervisor. - Messages reach the unchanged ``_on_pubsub_message`` in a worker thread, - keeping its off-the-event-loop contract. + """Pull the subscription over the Pub/Sub REST unary API, in place of the + bundled gRPC supervisor. Messages reach the unchanged + ``_on_pubsub_message`` in a worker thread, keeping its contract. """ import aiohttp @@ -317,10 +281,7 @@ async def _rest_pull(self): def install(ctx) -> None: - """Replace the registered google_chat entry with the sandbox delta. - - ``ctx`` is unused: the platform registry is the single attachment point. - """ + """Replace the registered google_chat entry with the sandbox delta.""" del ctx try: from gateway.platform_registry import platform_registry diff --git a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts index 71597251760..afcf762ec5a 100644 --- a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts +++ b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts @@ -39,16 +39,14 @@ class ClientTimeout: class _Response: - def __init__(self, status, payload=None, text=""): - self.status = status - self._payload = payload if payload is not None else {} - self._text = text + def __init__(self, status, payload): + self.status, self._payload = status, payload or {} async def json(self): return self._payload async def text(self): - return self._text + return "" async def __aenter__(self): return self @@ -69,20 +67,12 @@ class ClientSession: def post(self, url, json=None, headers=None, timeout=None): REQUESTS.append( - { - "url": url, - "method": "POST", - "authorization": (headers or {}).get("Authorization"), - "body": json, - } + {"url": url, "method": "POST", "authorization": (headers or {}).get("Authorization"), "body": json} ) - if not SCRIPT: - raise AssertionError("aiohttp double ran out of scripted responses: " + url) + assert SCRIPT, "aiohttp double ran out of scripted responses: " + url status, payload, on_send = SCRIPT.pop(0) - if on_send is not None: - on_send() - if status == "transport-error": - raise OSError("proxy refused the acknowledge") + on_send and on_send() + assert status != "transport-error", "proxy refused the acknowledge" return _Response(status, payload) `; @@ -113,14 +103,8 @@ handled = [] def _received(ack_id, text): - return { - "receivedMessages": [ - { - "ackId": ack_id, - "message": {"data": base64.b64encode(text.encode()).decode(), "attributes": {}}, - } - ] - } + message = {"data": base64.b64encode(text.encode()).decode(), "attributes": {}} + return {"receivedMessages": [{"ackId": ack_id, "message": message}]} def _stop(): @@ -129,45 +113,29 @@ def _stop(): def _handler(message): handled.append(message.data.decode()) - if SCENARIO == "nack": - message.nack() - else: - message.ack() + message.nack() if SCENARIO == "nack" else message.ack() adapter._on_pubsub_message = _handler - -if SCENARIO == "acknowledged": - aiohttp.SCRIPT.extend( - [ - (200, _received("ack-1", "hello"), None), - (200, {}, _stop), - ] - ) -elif SCENARIO == "acknowledge-fails": - # The ack is rejected, so Pub/Sub redelivers the same ackId on the next pull. - aiohttp.SCRIPT.extend( - [ - (200, _received("ack-1", "hello"), None), - (500, {}, None), - (200, _received("ack-1", "hello"), None), - (200, {}, _stop), - ] - ) -elif SCENARIO == "acknowledge-raises": - # The acknowledge never reaches Pub/Sub, so the same ackId comes back. - aiohttp.SCRIPT.extend( - [ - (200, _received("ack-1", "hello"), None), +delivery, redelivery = _received("ack-1", "hello"), _received("ack-1", "hello") + +# Each entry is (pull or acknowledge response status, payload, side effect). +# A rejected acknowledge means Pub/Sub redelivers the same ackId on the next pull. +aiohttp.SCRIPT.extend( + { + "acknowledged": [(200, delivery, None), (200, {}, _stop)], + "acknowledge-fails": [ + (200, delivery, None), (500, {}, None), (200, redelivery, None), (200, {}, _stop), + ], + "acknowledge-raises": [ + (200, delivery, None), ("transport-error", None, None), - (200, _received("ack-1", "hello"), None), + (200, redelivery, None), (200, {}, _stop), - ] - ) -elif SCENARIO == "nack": - aiohttp.SCRIPT.extend([(200, _received("ack-1", "hello"), _stop)]) -else: - raise SystemExit("unknown scenario " + SCENARIO) + ], + "nack": [(200, delivery, _stop)], + }[SCENARIO] +) asyncio.run(adapter._rest_pull()) @@ -209,27 +177,22 @@ describe("Hermes Google Chat keyless REST pull", () => { fs.rmSync(workspace, { recursive: true, force: true }); }); - it("sends the credential placeholder and nothing else on every request", () => { - const { requests } = runScenario("acknowledged"); + it("sends only the credential placeholder, and only to pull and acknowledge", () => { + const { requests, handled } = runScenario("acknowledged"); - expect(requests.length).toBeGreaterThan(0); + expect(handled).toEqual(["hello"]); expect(new Set(requests.map((request) => request.authorization))).toEqual( new Set([`Bearer ${PLACEHOLDER}`]), ); - }); - - it("reaches no Pub/Sub operation beyond pull and acknowledge", () => { - const { requests, handled } = runScenario("acknowledged"); - - expect(handled).toEqual(["hello"]); expect(new Set(requests.map((request) => `${request.method} ${request.url}`))).toEqual( new Set([ `POST https://pubsub.googleapis.com/v1/${SUBSCRIPTION}:pull`, `POST https://pubsub.googleapis.com/v1/${SUBSCRIPTION}:acknowledge`, ]), ); - const acknowledge = requests.filter((request) => request.url.endsWith(":acknowledge")); - expect(acknowledge.map((request) => request.body)).toEqual([{ ackIds: ["ack-1"] }]); + expect( + requests.filter((request) => request.url.endsWith(":acknowledge")).map((r) => r.body), + ).toEqual([{ ackIds: ["ack-1"] }]); }); it("keeps a message eligible for redelivery when acknowledgement fails", () => { From 8c5e6c4c25b1d3ce02b97ec8f4fe2bc58edd3072 Mon Sep 17 00:00:00 2001 From: Hung Le Date: Tue, 18 Aug 2026 19:02:46 +0530 Subject: [PATCH 09/15] test(messaging): cover the Google Chat bridge scopes and Hermes egress policy Two behaviors this branch introduced had no regression guard. Both were found by live failures, and both would fail the same way again with every check green. The bridge minted its token from `scopes[0]` alone, which left Hermes with chat.bot but not pubsub and returned 403 on every `:pull`; the profile list was also filtered by channel only, so a channel shipping a profile per agent could configure the wrong one. Assert that one minted token carries every declared scope, and that only the profile matching the sandbox agent produces a bridge. The Hermes policy preset was narrowed from `/v1/**` to the two Pub/Sub operations the adapter issues, but nothing pinned it: widening it back would also permit publish and subscription administration. Pin the Pub/Sub and Chat rules, the reachable host set, and the absence of Pub/Sub egress for OpenClaw, which runs on an inbound webhook instead. Each assertion was replayed against the mutation it exists to catch: the agent filter removed, the scope list truncated to its first entry, the Pub/Sub rules widened to `/v1/**`, a third host added, and Chat writes opened beyond the spaces tree. --- .../channels/googlechat/policy.test.ts | 57 +++++++++++++++++++ .../onboard/messaging-bridge-provider.test.ts | 50 ++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 src/lib/messaging/channels/googlechat/policy.test.ts diff --git a/src/lib/messaging/channels/googlechat/policy.test.ts b/src/lib/messaging/channels/googlechat/policy.test.ts new file mode 100644 index 00000000000..9afdec1459d --- /dev/null +++ b/src/lib/messaging/channels/googlechat/policy.test.ts @@ -0,0 +1,57 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; +import YAML from "yaml"; + +import { loadMessagingChannelPolicyPreset } from "../policy"; + +type PolicyRule = { readonly allow?: { readonly method?: string; readonly path?: string } }; +type PolicyEndpoint = { readonly host?: string; readonly rules?: readonly PolicyRule[] }; + +const PUBSUB_PULL = "/v1/projects/*/subscriptions/*:pull"; +const PUBSUB_ACKNOWLEDGE = "/v1/projects/*/subscriptions/*:acknowledge"; + +function endpointsFor(agent: string): readonly PolicyEndpoint[] { + const preset = loadMessagingChannelPolicyPreset("googlechat", { agent }); + expect(preset, `no googlechat policy preset for ${agent}`).toBeTruthy(); + const parsed = YAML.parse(preset ?? "") as { + network_policies?: Record; + }; + return Object.values(parsed.network_policies ?? {}).flatMap((policy) => policy.endpoints ?? []); +} + +function rulesForHost(agent: string, host: string): readonly PolicyRule[] { + return endpointsFor(agent).find((endpoint) => endpoint.host === host)?.rules ?? []; +} + +describe("Google Chat Hermes egress policy", () => { + // The adapter test proves the override only issues :pull and :acknowledge. This + // pins the other half: the preset must not hand the sandbox anything wider, + // because a `/v1/**` allow would also cover publish and subscription admin. + it("allows exactly the two Pub/Sub operations the REST pull issues", () => { + expect(rulesForHost("hermes", "pubsub.googleapis.com")).toEqual([ + { allow: { method: "POST", path: PUBSUB_PULL } }, + { allow: { method: "POST", path: PUBSUB_ACKNOWLEDGE } }, + ]); + }); + + it("keeps Chat writes inside the spaces tree", () => { + expect(rulesForHost("hermes", "chat.googleapis.com")).toEqual([ + { allow: { method: "GET", path: "/v1/**" } }, + { allow: { method: "POST", path: "/v1/spaces/**" } }, + { allow: { method: "PATCH", path: "/v1/spaces/**" } }, + { allow: { method: "DELETE", path: "/v1/spaces/**" } }, + ]); + }); + + it("reaches no host beyond Pub/Sub and Chat", () => { + expect(new Set(endpointsFor("hermes").map((endpoint) => endpoint.host))).toEqual( + new Set(["pubsub.googleapis.com", "chat.googleapis.com"]), + ); + }); + + it("grants OpenClaw no Pub/Sub egress, since it runs on an inbound webhook", () => { + expect(rulesForHost("openclaw", "pubsub.googleapis.com")).toEqual([]); + }); +}); diff --git a/src/lib/onboard/messaging-bridge-provider.test.ts b/src/lib/onboard/messaging-bridge-provider.test.ts index ca3fb4df006..b60fc1caf1c 100644 --- a/src/lib/onboard/messaging-bridge-provider.test.ts +++ b/src/lib/onboard/messaging-bridge-provider.test.ts @@ -37,6 +37,21 @@ const GC_PROFILE: MessagingBridgeProfile = { sourceSecretEnv: "GOOGLECHAT_SERVICE_ACCOUNT", }; +// Google Chat is the one channel shipping a profile per agent, so a sandbox must +// pick exactly one. Hermes also needs pubsub on top of chat.bot: one token, both +// scopes, because `:pull` 403s without it. +const GC_HERMES_PROFILE: MessagingBridgeProfile = { + ...GC_PROFILE, + agent: "hermes", + profilePath: "/repo/src/lib/messaging/channels/googlechat/provider-profile/hermes.yaml", + profileId: "google-chat-bridge-hermes", +}; + +const GC_PUBSUB_SCOPES = [ + "https://www.googleapis.com/auth/chat.bot", + "https://www.googleapis.com/auth/pubsub", +]; + const BRIDGE_DEF = { name: "sbx-googlechat-bridge", providerType: GC_PROFILE.profileId, @@ -94,6 +109,20 @@ describe("collectMessagingBridgeTokenDefs", () => { ); }); + it("emits only the profile whose agent matches the sandbox", () => { + // Both profiles carry the same channelId, so filtering on the channel alone + // would configure the OpenClaw bridge on a Hermes sandbox and the reverse. + const defs = collectMessagingBridgeTokenDefs( + collectInput({ + agent: "hermes", + getCredential: () => SA_JSON, + profiles: [GC_PROFILE, GC_HERMES_PROFILE], + }), + ); + + expect(defs.map((def) => def.providerType)).toEqual([GC_HERMES_PROFILE.profileId]); + }); + it("emits the bridge token def from an env-only secret (resolution parity)", () => { const defs = collectMessagingBridgeTokenDefs( collectInput({ @@ -195,6 +224,27 @@ describe("configureMessagingBridgeRefreshes", () => { expect(process.env[secretEnvName]).toBe(parentSecret); }); + it("mints one token carrying every scope the profile declares", () => { + // Hermes reads Pub/Sub and writes Chat with the same minted token, so sending + // only the first scope leaves `:pull` rejected with 403 at runtime. + const runOpenshell = vi.fn((_args: string[], _opts: { env?: NodeJS.ProcessEnv }) => ({ + status: 0, + })); + + const result = configureMessagingBridgeRefreshes([BRIDGE_DEF], { + runOpenshell, + redact, + getCredential: () => SA_JSON, + log: noLog, + profiles: [{ ...GC_PROFILE, scopes: GC_PUBSUB_SCOPES }], + }); + + expect(result).toEqual({ ok: true }); + const args = runOpenshell.mock.calls[0][0]; + expect(args).toContain(`scope=${GC_PUBSUB_SCOPES.join(" ")}`); + expect(args).not.toContain(`scope=${GC_PUBSUB_SCOPES[0]}`); + }); + it("forces private_key off argv even when the profile omits it from secretMaterialKeys", () => { // A misconfigured / edited / reused profile that marks other material secret // but not private_key must still never leak the raw key into argv. From 9cd2fcc598dd972a975bd9ec2dc5df5bf48f9531 Mon Sep 17 00:00:00 2001 From: Hung Le Date: Wed, 19 Aug 2026 01:23:59 +0530 Subject: [PATCH 10/15] fix(messaging): replace the Google Chat pull loop on a repeat connect The REST pull loop exits only on shutdown, so a second `connect()` on the same adapter without an intervening `disconnect()` would leave two consumers pulling one subscription and answering every message twice. Hermes v2026.7.20 builds a fresh adapter for each reconnect, so this holds the invariant rather than fixing a reachable path. Cancelling through the bundled `_supervisor_task` does not work here. On the no-subscription branch this override takes, `connect()` sets `self._supervisor_task = None`, so the handle no longer points at a running pull. Track the task on a subclass-owned `_sandbox_pull_task`, cancel and await that before starting a replacement, then bind both handles so the bundled `disconnect()` can still cancel the pull. The regression test mirrors that upstream assignment in its adapter double, drives the sequence on observed progress instead of fixed sleeps, and asserts the first task was cancelled and replaced, that the replacement is reachable through the bundled handle, and that one delivery is handled once. Two bridge call sites also mapped any unrecognized agent to OpenClaw, which would hand a sandbox no channel manifest supports the OpenClaw Google Chat bridge and its credential. Resolve the agent through `tryGetMessagingAgentId` instead, keeping the documented default that an unset agent is OpenClaw while a named unsupported one configures no bridge at all. Also suppress Ruff S105 on the resolver placeholder, matching the five BLE001 suppressions already in the file. --- src/lib/actions/sandbox/policy-channel.ts | 32 +++++--- .../googlechat/runtime/hermes-adapter.py | 33 +++++++- .../googlechat/runtime/hermes-adapter.test.ts | 78 +++++++++++++++++-- src/lib/onboard/messaging-prep.test.ts | 22 ++++++ src/lib/onboard/messaging-prep.ts | 35 ++++++--- 5 files changed, 170 insertions(+), 30 deletions(-) diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index f7e68c488bf..b374381596d 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -856,17 +856,27 @@ async function applyChannelAddToGatewayAndRegistry( // nothing for them. Their provider must be created HERE (same seam onboarding // uses): the pasted secret is env-only and gone once this process exits, so a // deferred rebuild cannot configure it. - const bridgeAgent = registry.getSandbox(sandboxName)?.agent === "hermes" ? "hermes" : "openclaw"; - const bridgeDefs = collectMessagingBridgeTokenDefs({ - sandboxName, - agent: bridgeAgent, - enabledChannels: [channelName], - disabledChannelNames: new Set(), - getCredential, - env: process.env, - // Env-map values (string | undefined) fit the store helper's input union. - normalizeCredentialValue: (value) => normalizeCredentialValue(value as string | undefined), - }); + // Mirror toMessagingAgentId: an unrecorded agent is OpenClaw, but a recorded + // agent no manifest supports configures no bridge rather than borrowing + // OpenClaw's credential. + const recordedAgent = registry.getSandbox(sandboxName)?.agent?.trim().toLowerCase(); + const bridgeAgent = recordedAgent + ? tryGetMessagingAgentId({ name: recordedAgent }, messagingManifestRegistry.list()) + : "openclaw"; + const bridgeDefs = + bridgeAgent === null + ? [] + : collectMessagingBridgeTokenDefs({ + sandboxName, + agent: bridgeAgent, + enabledChannels: [channelName], + disabledChannelNames: new Set(), + getCredential, + env: process.env, + // Env-map values (string | undefined) fit the store helper's input union. + normalizeCredentialValue: (value) => + normalizeCredentialValue(value as string | undefined), + }); if ( bridgeDefs.length === 0 && bridgeProviderNamesForChannel(sandboxName, channelName).length > 0 diff --git a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py index 36d9512e039..18fdc4faff6 100644 --- a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py +++ b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py @@ -25,7 +25,8 @@ import importlib.util import logging -_GC_REST_PLACEHOLDER_TOKEN = "openshell:resolve:env:GOOGLE_CHAT_ACCESS_TOKEN" +# Not a credential: the L7 proxy swaps this resolver placeholder at egress. +_GC_REST_PLACEHOLDER_TOKEN = "openshell:resolve:env:GOOGLE_CHAT_ACCESS_TOKEN" # noqa: S105 _GC_PULL_TIMEOUT = 95.0 # aiohttp cap > the Pub/Sub server long-poll hold (~90s) _GC_LOG = logging.getLogger("gateway.platforms.google_chat") @@ -167,6 +168,7 @@ class SandboxGoogleChatAdapter(_gc.GoogleChatAdapter): """Bundled adapter with the sandbox transport and credential delta.""" _sandbox_subscription = None + _sandbox_pull_task = None def _validate_config(self): """Report no subscription so the bundled ``connect()`` skips its gRPC @@ -193,12 +195,39 @@ async def connect(self, *, is_reconnect: bool = False) -> bool: """Run the bundled connect(), then start the REST pull it skipped.""" connected = await super().connect(is_reconnect=is_reconnect) if connected and self._sandbox_subscription: - self._supervisor_task = asyncio.create_task(self._rest_pull()) + await self._stop_rest_pull() + task = asyncio.create_task(self._rest_pull()) + # Own handle survives the next connect(); the bundled one keeps + # the bundled disconnect() able to cancel the pull. + self._sandbox_pull_task = task + self._supervisor_task = task _GC_LOG.info( "[GoogleChat][NemoClaw] keyless REST pull active (no gRPC subscriber)" ) return connected + async def _stop_rest_pull(self): + """Drop a running pull before starting another. + + The loop below exits only on shutdown, so a second ``connect()`` on + this adapter without an intervening ``disconnect()`` would leave two + consumers pulling one subscription and every message handled twice. + Hermes v2026.7.20 builds a fresh adapter for each reconnect, so this + holds the invariant rather than fixing a reachable path. + + Read the subclass handle: the bundled ``connect()`` sets + ``_supervisor_task`` to ``None`` on the no-subscription branch this + override takes, which would hide a task still running. + """ + task = self._sandbox_pull_task + if task is None or task.done(): + return + task.cancel() + try: + await asyncio.wait_for(task, timeout=5.0) + except (asyncio.CancelledError, asyncio.TimeoutError): + pass + async def _rest_pull(self): """Pull the subscription over the Pub/Sub REST unary API, in place of the bundled gRPC supervisor. Messages reach the unchanged diff --git a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts index afcf762ec5a..e93ffc2745e 100644 --- a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts +++ b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts @@ -17,7 +17,14 @@ const SUBSCRIPTION = "projects/nemoclaw-test/subscriptions/hermes-chat"; // test supplies the smallest surface `_rest_pull` touches. const HERMES_STUB = ` class GoogleChatAdapter: - """Only what the subclass definition needs; _rest_pull calls none of it.""" + """Only what the subclass needs: _rest_pull calls none of it, and connect() + stands in for the bundled implementation the override delegates to.""" + + async def connect(self, *, is_reconnect=False): + # Mirrors Hermes v2026.7.20: the no-subscription branch this override + # takes clears the bundled supervisor handle. + self._supervisor_task = None + return True class AuthorizedHttp: @@ -98,6 +105,7 @@ adapter = object.__new__(module._sandbox_adapter_class()) adapter._sandbox_subscription = SUBSCRIPTION adapter._shutting_down = False adapter._max_messages = 1 +adapter._supervisor_task = None # the bundled __init__ sets this; object.__new__ skips it handled = [] @@ -134,14 +142,55 @@ aiohttp.SCRIPT.extend( (200, {}, _stop), ], "nack": [(200, delivery, _stop)], + # One delivery, then empty long-polls: a second live loop would keep + # consuming these, so the surviving one must be the only consumer. + # Empty long-poll first, so the loop is mid-sleep at reconnect; the + # delivery and its acknowledge belong to whatever survives. + "reconnect": [(200, {}, None), (200, delivery, None)] + [(200, {}, None)] * 8, }[SCENARIO] ) -asyncio.run(adapter._rest_pull()) - -print(json.dumps({"requests": aiohttp.REQUESTS, "handled": handled})) +async def _until(predicate, limit=500): + for _ in range(limit): + if predicate(): + return + await asyncio.sleep(0) + raise AssertionError("condition never held") + + +async def _main(): + if SCENARIO != "reconnect": + await adapter._rest_pull() + return {} + await adapter.connect() + first = adapter._sandbox_pull_task + await _until(lambda: len(aiohttp.REQUESTS) >= 1) + pulls_before = len(aiohttp.REQUESTS) + await adapter.connect(is_reconnect=True) + second = adapter._sandbox_pull_task + await _until(lambda: handled) + adapter._shutting_down = True + await asyncio.wait_for(second, timeout=5) + return { + "firstCancelled": first.cancelled(), + "replaced": first is not second, + "boundToBundledHandle": adapter._supervisor_task is second, + "pullsBeforeReconnect": pulls_before, + } + + +report = asyncio.run(_main()) +report.update({"requests": aiohttp.REQUESTS, "handled": handled}) +print(json.dumps(report)) `; +interface ReconnectReport { + readonly firstCancelled?: boolean; + readonly replaced?: boolean; + readonly boundToBundledHandle?: boolean; + readonly pullsBeforeReconnect?: number; +} + interface RecordedRequest { readonly url: string; readonly method: string; @@ -151,7 +200,9 @@ interface RecordedRequest { const workspace = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-googlechat-pull-")); -function runScenario(scenario: string): { requests: RecordedRequest[]; handled: string[] } { +function runScenario( + scenario: string, +): ReconnectReport & { requests: RecordedRequest[]; handled: string[] } { const result = spawnSync("python3", [path.join(workspace, "driver.py"), ADAPTER, scenario], { encoding: "utf8", env: { ...process.env, PYTHONPATH: workspace, PYTHONDONTWRITEBYTECODE: "1" }, @@ -218,6 +269,23 @@ describe("Hermes Google Chat keyless REST pull", () => { expect(requests.filter((request) => request.url.endsWith(":pull"))).toHaveLength(2); }); + it("replaces the pull loop on reconnect instead of running two", () => { + // The pull loop exits only on shutdown, so a second connect() without an + // intervening disconnect() would leave two consumers on one subscription and + // every message handled twice. The pinned gateway rebuilds the adapter per + // reconnect, so this pins the invariant rather than a reachable path. + const report = runScenario("reconnect"); + + expect(report.firstCancelled).toBe(true); + expect(report.replaced).toBe(true); + // The bundled connect() clears its own handle on this branch, so the + // replacement has to be rebound for the bundled disconnect() to cancel it. + expect(report.boundToBundledHandle).toBe(true); + // The first loop really was pulling before the reconnect cancelled it. + expect(report.pullsBeforeReconnect).toBeGreaterThan(0); + expect(report.handled).toEqual(["hello"]); + }); + it("acknowledges nothing for a message the handler nacks", () => { const { requests, handled } = runScenario("nack"); diff --git a/src/lib/onboard/messaging-prep.test.ts b/src/lib/onboard/messaging-prep.test.ts index 2a5d52f4747..3b6a3365175 100644 --- a/src/lib/onboard/messaging-prep.test.ts +++ b/src/lib/onboard/messaging-prep.test.ts @@ -114,6 +114,28 @@ describe("prepareCreateSandboxMessaging", () => { expect(result.reusableMessagingProviders).not.toContain("demo-googlechat-bridge"); }); + it("configures no bridge for an agent no channel manifest supports", () => { + // Defaulting an unknown agent to OpenClaw would hand a sandbox with no + // messaging support the OpenClaw Google Chat bridge and its credential. + const result = prepareCreateSandboxMessaging( + createInput({ + agentName: "deepagents", + enabledChannels: ["googlechat"], + env: { + GOOGLECHAT_SERVICE_ACCOUNT: JSON.stringify({ + client_email: "bot@p.iam.gserviceaccount.com", + private_key: "fake-test-private-key-material", + }), + }, + providerExistsInGateway: () => true, + }), + ); + + expect(result.messagingTokenDefs.map((def) => def.name)).not.toContain( + "demo-googlechat-bridge", + ); + }); + it("does not reuse a bridge provider that is absent from the gateway", () => { const result = prepareCreateSandboxMessaging( createInput({ diff --git a/src/lib/onboard/messaging-prep.ts b/src/lib/onboard/messaging-prep.ts index 71d3af005c6..d54c59dbe59 100644 --- a/src/lib/onboard/messaging-prep.ts +++ b/src/lib/onboard/messaging-prep.ts @@ -3,7 +3,8 @@ import type { WebSearchConfig } from "../inference/web-search"; import * as webSearch from "../inference/web-search"; -import { listMessagingCredentialMetadata } from "../messaging/channels"; +import { BUILT_IN_CHANNEL_MANIFESTS, listMessagingCredentialMetadata } from "../messaging/channels"; +import { tryGetMessagingAgentId } from "../messaging/utils"; import { type ChannelDef, getChannelTokenKeys } from "../sandbox/channels"; import * as braveProviderProfile from "./brave-provider-profile"; import { @@ -138,17 +139,27 @@ export function prepareCreateSandboxMessaging( // gateway-side) and the L7 proxy injects it. The credential value is a sentinel // (minted by refresh, configured post-create in onboard's // upsertMessagingProviders wrapper). Today only Google Chat uses this. - messagingTokenDefs.push( - ...collectMessagingBridgeTokenDefs({ - sandboxName: input.sandboxName, - agent: input.agentName?.trim().toLowerCase() === "hermes" ? "hermes" : "openclaw", - getCredential: input.getCredential, - env: input.env, - normalizeCredentialValue: input.normalizeCredentialValue, - enabledChannels: input.enabledChannels, - disabledChannelNames, - }), - ); + // Resolve the agent instead of defaulting it: an agent no manifest supports + // must configure no bridge, not the OpenClaw one. + // Mirror toMessagingAgentId: an unset agent is OpenClaw, but a named agent no + // manifest supports configures no bridge rather than borrowing OpenClaw's. + const agentName = input.agentName?.trim().toLowerCase(); + const bridgeAgent = agentName + ? tryGetMessagingAgentId({ name: agentName }, BUILT_IN_CHANNEL_MANIFESTS) + : "openclaw"; + if (bridgeAgent !== null) { + messagingTokenDefs.push( + ...collectMessagingBridgeTokenDefs({ + sandboxName: input.sandboxName, + agent: bridgeAgent, + getCredential: input.getCredential, + env: input.env, + normalizeCredentialValue: input.normalizeCredentialValue, + enabledChannels: input.enabledChannels, + disabledChannelNames, + }), + ); + } const extraPlaceholderKeys = input.registerExtraPlaceholderProviders( input.sandboxName, From 44eab71c61ef882661bb2b875a578316468d5b64 Mon Sep 17 00:00:00 2001 From: Hung Le Date: Wed, 19 Aug 2026 02:25:49 +0530 Subject: [PATCH 11/15] refactor(messaging): drop the unreachable reconnect path and centralize bridge agent selection The previous commit added a second pull-task handle and a same-instance reconnect path. Its own documentation recorded why that cannot happen: in the pinned Hermes v2026.7.20 both reconnect paths call `_create_adapter` before `connect(is_reconnect=True)`, so `connect()` never runs twice on one adapter. That scaffolding protected no caller while widening the private Hermes surface the channel-owned refactor had just narrowed. Remove it, leaving two lines that record the assumption, and `connect()` returns to awaiting the bundled implementation and starting the pull. Both bridge callers had also grown the same agent branch: normalize the name, consult a manifest registry, default an unset agent to OpenClaw, reject an unsupported one. `messagingBridgeProfilesForAgent` now owns that selection, and both callers pass the recorded agent unnormalized. `collectMessagingBridgeTokenDefs` filters through it, and the reuse pass in onboarding selects provider names from the same filtered profiles, so an agent no profile declares neither mints nor reuses a bridge. Removing the empty `except` that CodeQL flagged falls out of the same deletion. --- src/lib/actions/sandbox/policy-channel.ts | 33 +++----- .../googlechat/runtime/hermes-adapter.py | 33 +------- .../googlechat/runtime/hermes-adapter.test.ts | 78 ++----------------- src/lib/onboard/messaging-bridge-provider.ts | 23 ++++-- src/lib/onboard/messaging-prep.test.ts | 4 + src/lib/onboard/messaging-prep.ts | 43 +++++----- 6 files changed, 64 insertions(+), 150 deletions(-) diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index b374381596d..18ed10bc65f 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -856,27 +856,18 @@ async function applyChannelAddToGatewayAndRegistry( // nothing for them. Their provider must be created HERE (same seam onboarding // uses): the pasted secret is env-only and gone once this process exits, so a // deferred rebuild cannot configure it. - // Mirror toMessagingAgentId: an unrecorded agent is OpenClaw, but a recorded - // agent no manifest supports configures no bridge rather than borrowing - // OpenClaw's credential. - const recordedAgent = registry.getSandbox(sandboxName)?.agent?.trim().toLowerCase(); - const bridgeAgent = recordedAgent - ? tryGetMessagingAgentId({ name: recordedAgent }, messagingManifestRegistry.list()) - : "openclaw"; - const bridgeDefs = - bridgeAgent === null - ? [] - : collectMessagingBridgeTokenDefs({ - sandboxName, - agent: bridgeAgent, - enabledChannels: [channelName], - disabledChannelNames: new Set(), - getCredential, - env: process.env, - // Env-map values (string | undefined) fit the store helper's input union. - normalizeCredentialValue: (value) => - normalizeCredentialValue(value as string | undefined), - }); + const bridgeDefs = collectMessagingBridgeTokenDefs({ + sandboxName, + // Unnormalized: the bridge profile filter owns the unset default and rejects + // an agent no profile declares. + agent: registry.getSandbox(sandboxName)?.agent, + enabledChannels: [channelName], + disabledChannelNames: new Set(), + getCredential, + env: process.env, + // Env-map values (string | undefined) fit the store helper's input union. + normalizeCredentialValue: (value) => normalizeCredentialValue(value as string | undefined), + }); if ( bridgeDefs.length === 0 && bridgeProviderNamesForChannel(sandboxName, channelName).length > 0 diff --git a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py index 18fdc4faff6..fa3cbdc572d 100644 --- a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py +++ b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py @@ -168,7 +168,6 @@ class SandboxGoogleChatAdapter(_gc.GoogleChatAdapter): """Bundled adapter with the sandbox transport and credential delta.""" _sandbox_subscription = None - _sandbox_pull_task = None def _validate_config(self): """Report no subscription so the bundled ``connect()`` skips its gRPC @@ -194,40 +193,16 @@ def _new_authed_http(self): async def connect(self, *, is_reconnect: bool = False) -> bool: """Run the bundled connect(), then start the REST pull it skipped.""" connected = await super().connect(is_reconnect=is_reconnect) + # Hermes v2026.7.20 builds a fresh adapter for every reconnect, so this + # never runs twice on one instance; a release that reuses the instance + # would need the running pull replaced here. if connected and self._sandbox_subscription: - await self._stop_rest_pull() - task = asyncio.create_task(self._rest_pull()) - # Own handle survives the next connect(); the bundled one keeps - # the bundled disconnect() able to cancel the pull. - self._sandbox_pull_task = task - self._supervisor_task = task + self._supervisor_task = asyncio.create_task(self._rest_pull()) _GC_LOG.info( "[GoogleChat][NemoClaw] keyless REST pull active (no gRPC subscriber)" ) return connected - async def _stop_rest_pull(self): - """Drop a running pull before starting another. - - The loop below exits only on shutdown, so a second ``connect()`` on - this adapter without an intervening ``disconnect()`` would leave two - consumers pulling one subscription and every message handled twice. - Hermes v2026.7.20 builds a fresh adapter for each reconnect, so this - holds the invariant rather than fixing a reachable path. - - Read the subclass handle: the bundled ``connect()`` sets - ``_supervisor_task`` to ``None`` on the no-subscription branch this - override takes, which would hide a task still running. - """ - task = self._sandbox_pull_task - if task is None or task.done(): - return - task.cancel() - try: - await asyncio.wait_for(task, timeout=5.0) - except (asyncio.CancelledError, asyncio.TimeoutError): - pass - async def _rest_pull(self): """Pull the subscription over the Pub/Sub REST unary API, in place of the bundled gRPC supervisor. Messages reach the unchanged diff --git a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts index e93ffc2745e..afcf762ec5a 100644 --- a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts +++ b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts @@ -17,14 +17,7 @@ const SUBSCRIPTION = "projects/nemoclaw-test/subscriptions/hermes-chat"; // test supplies the smallest surface `_rest_pull` touches. const HERMES_STUB = ` class GoogleChatAdapter: - """Only what the subclass needs: _rest_pull calls none of it, and connect() - stands in for the bundled implementation the override delegates to.""" - - async def connect(self, *, is_reconnect=False): - # Mirrors Hermes v2026.7.20: the no-subscription branch this override - # takes clears the bundled supervisor handle. - self._supervisor_task = None - return True + """Only what the subclass definition needs; _rest_pull calls none of it.""" class AuthorizedHttp: @@ -105,7 +98,6 @@ adapter = object.__new__(module._sandbox_adapter_class()) adapter._sandbox_subscription = SUBSCRIPTION adapter._shutting_down = False adapter._max_messages = 1 -adapter._supervisor_task = None # the bundled __init__ sets this; object.__new__ skips it handled = [] @@ -142,54 +134,13 @@ aiohttp.SCRIPT.extend( (200, {}, _stop), ], "nack": [(200, delivery, _stop)], - # One delivery, then empty long-polls: a second live loop would keep - # consuming these, so the surviving one must be the only consumer. - # Empty long-poll first, so the loop is mid-sleep at reconnect; the - # delivery and its acknowledge belong to whatever survives. - "reconnect": [(200, {}, None), (200, delivery, None)] + [(200, {}, None)] * 8, }[SCENARIO] ) -async def _until(predicate, limit=500): - for _ in range(limit): - if predicate(): - return - await asyncio.sleep(0) - raise AssertionError("condition never held") - - -async def _main(): - if SCENARIO != "reconnect": - await adapter._rest_pull() - return {} - await adapter.connect() - first = adapter._sandbox_pull_task - await _until(lambda: len(aiohttp.REQUESTS) >= 1) - pulls_before = len(aiohttp.REQUESTS) - await adapter.connect(is_reconnect=True) - second = adapter._sandbox_pull_task - await _until(lambda: handled) - adapter._shutting_down = True - await asyncio.wait_for(second, timeout=5) - return { - "firstCancelled": first.cancelled(), - "replaced": first is not second, - "boundToBundledHandle": adapter._supervisor_task is second, - "pullsBeforeReconnect": pulls_before, - } - - -report = asyncio.run(_main()) -report.update({"requests": aiohttp.REQUESTS, "handled": handled}) -print(json.dumps(report)) -`; +asyncio.run(adapter._rest_pull()) -interface ReconnectReport { - readonly firstCancelled?: boolean; - readonly replaced?: boolean; - readonly boundToBundledHandle?: boolean; - readonly pullsBeforeReconnect?: number; -} +print(json.dumps({"requests": aiohttp.REQUESTS, "handled": handled})) +`; interface RecordedRequest { readonly url: string; @@ -200,9 +151,7 @@ interface RecordedRequest { const workspace = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-googlechat-pull-")); -function runScenario( - scenario: string, -): ReconnectReport & { requests: RecordedRequest[]; handled: string[] } { +function runScenario(scenario: string): { requests: RecordedRequest[]; handled: string[] } { const result = spawnSync("python3", [path.join(workspace, "driver.py"), ADAPTER, scenario], { encoding: "utf8", env: { ...process.env, PYTHONPATH: workspace, PYTHONDONTWRITEBYTECODE: "1" }, @@ -269,23 +218,6 @@ describe("Hermes Google Chat keyless REST pull", () => { expect(requests.filter((request) => request.url.endsWith(":pull"))).toHaveLength(2); }); - it("replaces the pull loop on reconnect instead of running two", () => { - // The pull loop exits only on shutdown, so a second connect() without an - // intervening disconnect() would leave two consumers on one subscription and - // every message handled twice. The pinned gateway rebuilds the adapter per - // reconnect, so this pins the invariant rather than a reachable path. - const report = runScenario("reconnect"); - - expect(report.firstCancelled).toBe(true); - expect(report.replaced).toBe(true); - // The bundled connect() clears its own handle on this branch, so the - // replacement has to be rebound for the bundled disconnect() to cancel it. - expect(report.boundToBundledHandle).toBe(true); - // The first loop really was pulling before the reconnect cancelled it. - expect(report.pullsBeforeReconnect).toBeGreaterThan(0); - expect(report.handled).toEqual(["hello"]); - }); - it("acknowledges nothing for a message the handler nacks", () => { const { requests, handled } = runScenario("nack"); diff --git a/src/lib/onboard/messaging-bridge-provider.ts b/src/lib/onboard/messaging-bridge-provider.ts index 79c7c889c30..4dedead32e7 100644 --- a/src/lib/onboard/messaging-bridge-provider.ts +++ b/src/lib/onboard/messaging-bridge-provider.ts @@ -88,10 +88,11 @@ export interface MessagingBridgeSecretResolveDeps { export interface CollectMessagingBridgeTokenDefsInput extends MessagingBridgeSecretResolveDeps { readonly sandboxName: string; /** - * Sandbox agent. Bridge profiles are per-agent, and a channel may ship both - * (Google Chat does), so only the matching profile may produce a bridge. + * Recorded sandbox agent, unnormalized. Bridge profiles are per-agent and a + * channel may ship both (Google Chat does), so the profile filter selects the + * matching one and rejects an agent no profile declares. */ - readonly agent: MessagingAgentId; + readonly agent: string | null | undefined; readonly enabledChannels: readonly string[] | null; readonly disabledChannelNames: ReadonlySet; /** Injected for tests; defaults to convention discovery. */ @@ -260,10 +261,9 @@ function bridgeProviderNameFor(sandboxName: string, channelId: string): string { export function collectMessagingBridgeTokenDefs( input: CollectMessagingBridgeTokenDefsInput, ): { name: string; envKey: string; token: string; providerType: string }[] { - const profiles = input.profiles ?? listMessagingBridgeProfiles(); + const profiles = messagingBridgeProfilesForAgent(input.agent, input.profiles); const defs: { name: string; envKey: string; token: string; providerType: string }[] = []; for (const profile of profiles) { - if (profile.agent !== input.agent) continue; if (input.disabledChannelNames.has(profile.channelId)) continue; if (input.enabledChannels != null && !input.enabledChannels.includes(profile.channelId)) continue; @@ -279,6 +279,19 @@ export function collectMessagingBridgeTokenDefs( return defs; } +/** + * Single authority for which bridge profiles an agent may use. An unset agent is + * OpenClaw, matching `toMessagingAgentId`; a recorded agent no profile declares + * selects nothing, so it mints and reuses no bridge. + */ +export function messagingBridgeProfilesForAgent( + agent: string | null | undefined, + profiles: readonly MessagingBridgeProfile[] = listMessagingBridgeProfiles(), +): MessagingBridgeProfile[] { + const name = agent?.trim().toLowerCase() || "openclaw"; + return profiles.filter((profile) => profile.agent === name); +} + /** * Gateway-minted bridge provider name(s) for a channel — the providers * `channels remove` must tear down. A bridge-backed channel has no diff --git a/src/lib/onboard/messaging-prep.test.ts b/src/lib/onboard/messaging-prep.test.ts index 3b6a3365175..4314dbdeb1a 100644 --- a/src/lib/onboard/messaging-prep.test.ts +++ b/src/lib/onboard/messaging-prep.test.ts @@ -134,6 +134,10 @@ describe("prepareCreateSandboxMessaging", () => { expect(result.messagingTokenDefs.map((def) => def.name)).not.toContain( "demo-googlechat-bridge", ); + // The provider name derives from the channel alone, so reuse has to be gated + // as well; otherwise the sandbox adopts whichever bridge already exists. + expect(result.reusableMessagingProviders).not.toContain("demo-googlechat-bridge"); + expect(result.reusableMessagingChannels).not.toContain("googlechat"); }); it("does not reuse a bridge provider that is absent from the gateway", () => { diff --git a/src/lib/onboard/messaging-prep.ts b/src/lib/onboard/messaging-prep.ts index d54c59dbe59..9461f903b47 100644 --- a/src/lib/onboard/messaging-prep.ts +++ b/src/lib/onboard/messaging-prep.ts @@ -3,13 +3,13 @@ import type { WebSearchConfig } from "../inference/web-search"; import * as webSearch from "../inference/web-search"; -import { BUILT_IN_CHANNEL_MANIFESTS, listMessagingCredentialMetadata } from "../messaging/channels"; -import { tryGetMessagingAgentId } from "../messaging/utils"; +import { listMessagingCredentialMetadata } from "../messaging/channels"; import { type ChannelDef, getChannelTokenKeys } from "../sandbox/channels"; import * as braveProviderProfile from "./brave-provider-profile"; import { bridgeProviderNamesForChannel, collectMessagingBridgeTokenDefs, + messagingBridgeProfilesForAgent, } from "./messaging-bridge-provider"; export type NamedMessagingChannel = { name: string } & ChannelDef; @@ -141,25 +141,18 @@ export function prepareCreateSandboxMessaging( // upsertMessagingProviders wrapper). Today only Google Chat uses this. // Resolve the agent instead of defaulting it: an agent no manifest supports // must configure no bridge, not the OpenClaw one. - // Mirror toMessagingAgentId: an unset agent is OpenClaw, but a named agent no - // manifest supports configures no bridge rather than borrowing OpenClaw's. - const agentName = input.agentName?.trim().toLowerCase(); - const bridgeAgent = agentName - ? tryGetMessagingAgentId({ name: agentName }, BUILT_IN_CHANNEL_MANIFESTS) - : "openclaw"; - if (bridgeAgent !== null) { - messagingTokenDefs.push( - ...collectMessagingBridgeTokenDefs({ - sandboxName: input.sandboxName, - agent: bridgeAgent, - getCredential: input.getCredential, - env: input.env, - normalizeCredentialValue: input.normalizeCredentialValue, - enabledChannels: input.enabledChannels, - disabledChannelNames, - }), - ); - } + const bridgeProfiles = messagingBridgeProfilesForAgent(input.agentName); + messagingTokenDefs.push( + ...collectMessagingBridgeTokenDefs({ + sandboxName: input.sandboxName, + agent: input.agentName, + getCredential: input.getCredential, + env: input.env, + normalizeCredentialValue: input.normalizeCredentialValue, + enabledChannels: input.enabledChannels, + disabledChannelNames, + }), + ); const extraPlaceholderKeys = input.registerExtraPlaceholderProviders( input.sandboxName, @@ -190,10 +183,16 @@ export function prepareCreateSandboxMessaging( // Bridge channels have no token def at all when their env-only secret is // gone (fresh process), so the envKey loop above misses them. The gateway // still holds the refresh material — reuse the provider by name instead. + // Same profile selection as the mint above: the provider name derives from the + // channel alone, so an agent with no profile must reuse nothing either. if (input.enabledChannels != null) { for (const channel of input.enabledChannels) { if (disabledChannelNames.has(channel)) continue; - for (const name of bridgeProviderNamesForChannel(input.sandboxName, channel)) { + for (const name of bridgeProviderNamesForChannel( + input.sandboxName, + channel, + bridgeProfiles, + )) { if (messagingTokenDefs.some((def) => def.name === name && def.token)) continue; if (reusableMessagingProviders.includes(name)) continue; if (!input.providerExistsInGateway(name)) continue; From 62d6e5dd9f18c04e9747b2bf7700e095539a3478 Mon Sep 17 00:00:00 2001 From: Hung Le Date: Wed, 19 Aug 2026 11:36:15 +0530 Subject: [PATCH 12/15] fix(onboard): match the bridge binding before reusing a gateway provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bridge provider is named `--bridge`, which carries no agent, and onboard offers to delete and recreate a sandbox name under a different agent (`getSandboxAgentDrift`, with messaging preparation running earlier in the same function). Recreate cleanup detaches the provider but keeps it for reuse, so an OpenClaw Google Chat provider could be reattached to a Hermes sandbox whenever the source secret was no longer resolvable. That mints the previous agent's token: for Hermes an OpenClaw profile whose scopes omit pubsub, which fails `:pull` with 403. Reuse now walks the selected bridge profiles and checks the gateway binding with `providerMatchesGatewayCredential` instead of accepting any provider with the right name. The two Google Chat profiles carry distinct ids, and the id becomes the gateway provider type, so a stale binding no longer matches. Refusing the stale provider exposed an older gap: a selected bridge channel with no usable provider and no source secret simply vanished from the create intent, and onboard can act on that intent by deleting and recreating the sandbox. Report those channels and fail the preflight, naming the secret to supply, the same way the `channels add` path already does. Destroy also skipped these providers entirely: its suffix set is derived from manifest credentials, and a bridge-profile channel declares none. Derive the bridge suffixes from the profiles as well, deduplicated because a channel may ship one profile per agent. Cleanup stays best-effort — `provider delete` runs with `ignoreError` — so the reuse check above is the guarantee, not cleanup. A public-boundary test covers the ordering the guard depends on. It drives the real `createSandbox` in a child process with an existing OpenClaw sandbox, a requested Hermes agent, Google Chat selected, no source secret, and a gateway still holding the OpenClaw binding. It asserts the child exits with the guard's own status, that neither completion marker is reached, that the binding is read exactly once, and that nothing mutates: no sandbox delete, no provider attach or detach, no provider create, update or delete, no registry write. Removing the preflight exit, emptying the reported channels, reverting reuse to a name-only check, or leaving the binding unread each fail it. Both cases in that file now use `test/helpers/onboard-child-process-harness.ts` rather than hand-rolling the workspace, environment, spawn, result decoding and cleanup, since adding the second case is what made that duplication exist. --- src/lib/onboard/messaging-prep.test.ts | 89 ++++++- src/lib/onboard/messaging-prep.ts | 40 +++- .../sandbox-messaging-preflight.test.ts | 79 ++++--- .../onboard/sandbox-messaging-preflight.ts | 14 ++ src/lib/onboard/sandbox-provider-cleanup.ts | 5 + test/onboard-pre-destructive-intent.test.ts | 221 ++++++++++++++---- test/sandbox-provider-cleanup.test.ts | 1 + 7 files changed, 351 insertions(+), 98 deletions(-) diff --git a/src/lib/onboard/messaging-prep.test.ts b/src/lib/onboard/messaging-prep.test.ts index 4314dbdeb1a..06e84b882bf 100644 --- a/src/lib/onboard/messaging-prep.test.ts +++ b/src/lib/onboard/messaging-prep.test.ts @@ -78,12 +78,18 @@ describe("prepareCreateSandboxMessaging", () => { // Deferred rebuild in a fresh process: the pasted secret is env-only and // gone, so no bridge token def exists — but the gateway still durably // holds the refresh material, so the provider only needs re-attaching. - const providerExistsInGateway = vi.fn((name: string) => name === "demo-googlechat-bridge"); + // The gateway holds the OpenClaw binding, which is the agent being onboarded. + const providerMatchesGatewayCredential = vi.fn( + (name: string, type: string, credentialKey: string) => + name === "demo-googlechat-bridge" && + type === "google-chat-bridge" && + credentialKey === "GOOGLE_CHAT_ACCESS_TOKEN", + ); const result = prepareCreateSandboxMessaging( createInput({ enabledChannels: ["googlechat"], - providerExistsInGateway, + providerMatchesGatewayCredential, }), ); @@ -92,10 +98,67 @@ describe("prepareCreateSandboxMessaging", () => { ); expect(result.reusableMessagingProviders).toContain("demo-googlechat-bridge"); expect(result.reusableMessagingChannels).toContain("googlechat"); - expect(providerExistsInGateway).toHaveBeenCalledWith("demo-googlechat-bridge"); + expect(providerMatchesGatewayCredential).toHaveBeenCalledWith( + "demo-googlechat-bridge", + "google-chat-bridge", + "GOOGLE_CHAT_ACCESS_TOKEN", + ); + }); + + it("refuses and reports a bridge the gateway holds for a different agent", () => { + // onboard recreates a sandbox name under a new agent (`Delete and recreate + // '' as ?`), and the provider name carries no agent. Reusing the + // stale binding would mint the previous agent's token — for Hermes that means + // an OpenClaw profile whose scopes omit pubsub, so `:pull` fails with 403. + // Refusing it also has to surface, or the channel silently leaves the intent. + const result = prepareCreateSandboxMessaging( + createInput({ + agentName: "hermes", + enabledChannels: ["googlechat"], + providerMatchesGatewayCredential: (_name: string, type: string) => + type === "google-chat-bridge", + }), + ); + + expect(result.reusableMessagingProviders).not.toContain("demo-googlechat-bridge"); + expect(result.reusableMessagingChannels).not.toContain("googlechat"); + expect(result.missingBridgeChannels).toEqual(["googlechat"]); + }); + + it("reuses a bridge the gateway holds for the agent being onboarded", () => { + const result = prepareCreateSandboxMessaging( + createInput({ + agentName: "hermes", + enabledChannels: ["googlechat"], + providerMatchesGatewayCredential: (_name: string, type: string) => + type === "google-chat-hermes-bridge", + }), + ); + + expect(result.reusableMessagingProviders).toContain("demo-googlechat-bridge"); + expect(result.reusableMessagingChannels).toContain("googlechat"); + expect(result.missingBridgeChannels).toEqual([]); + }); + + it("does not reuse a Hermes bridge when onboarding OpenClaw", () => { + const gatewayHoldsHermesBinding = vi.fn( + (_name: string, type: string) => type === "google-chat-hermes-bridge", + ); + + const result = prepareCreateSandboxMessaging( + createInput({ + enabledChannels: ["googlechat"], + providerMatchesGatewayCredential: gatewayHoldsHermesBinding, + }), + ); + + expect(result.reusableMessagingProviders).not.toContain("demo-googlechat-bridge"); + expect(result.reusableMessagingChannels).not.toContain("googlechat"); }); it("routes the bridge through upsert instead of reuse when the secret is resolvable", () => { + const providerMatchesGatewayCredential = vi.fn(() => true); + const result = prepareCreateSandboxMessaging( createInput({ enabledChannels: ["googlechat"], @@ -105,13 +168,17 @@ describe("prepareCreateSandboxMessaging", () => { private_key: "fake-test-private-key-material", }), }, - providerExistsInGateway: () => true, + providerMatchesGatewayCredential, }), ); const def = result.messagingTokenDefs.find((d) => d.name === "demo-googlechat-bridge"); expect(def?.token).toBeTruthy(); expect(result.reusableMessagingProviders).not.toContain("demo-googlechat-bridge"); + expect(result.missingBridgeChannels).toEqual([]); + // The token definition already owns the provider, so reuse never asks the + // gateway — a matcher that would have said yes is never consulted. + expect(providerMatchesGatewayCredential).not.toHaveBeenCalled(); }); it("configures no bridge for an agent no channel manifest supports", () => { @@ -127,7 +194,6 @@ describe("prepareCreateSandboxMessaging", () => { private_key: "fake-test-private-key-material", }), }, - providerExistsInGateway: () => true, }), ); @@ -140,28 +206,35 @@ describe("prepareCreateSandboxMessaging", () => { expect(result.reusableMessagingChannels).not.toContain("googlechat"); }); - it("does not reuse a bridge provider that is absent from the gateway", () => { + it("does not reuse a bridge provider without an exact gateway binding", () => { const result = prepareCreateSandboxMessaging( createInput({ enabledChannels: ["googlechat"], - providerExistsInGateway: () => false, + providerMatchesGatewayCredential: () => false, }), ); expect(result.reusableMessagingProviders).toEqual([]); expect(result.reusableMessagingChannels).toEqual([]); + expect(result.missingBridgeChannels).toEqual(["googlechat"]); }); it("does not reuse the bridge provider of a disabled channel", () => { + // The matcher would accept this provider, so only the disabled guard can + // keep it out — and a disabled channel is not a missing one either. + const providerMatchesGatewayCredential = vi.fn(() => true); + const result = prepareCreateSandboxMessaging( createInput({ enabledChannels: ["googlechat"], disabledChannels: ["googlechat"], - providerExistsInGateway: () => true, + providerMatchesGatewayCredential, }), ); expect(result.reusableMessagingProviders).toEqual([]); + expect(result.missingBridgeChannels).toEqual([]); + expect(providerMatchesGatewayCredential).not.toHaveBeenCalled(); }); it("reports missing Brave API keys before registering extra placeholder providers", () => { diff --git a/src/lib/onboard/messaging-prep.ts b/src/lib/onboard/messaging-prep.ts index 9461f903b47..776f3370647 100644 --- a/src/lib/onboard/messaging-prep.ts +++ b/src/lib/onboard/messaging-prep.ts @@ -52,6 +52,8 @@ export interface CreateSandboxMessagingPrepResult { hasMessagingTokens: boolean; reusableMessagingProviders: string[]; reusableMessagingChannels: string[]; + /** Selected bridge channels with no usable provider and no source secret. */ + missingBridgeChannels: string[]; missingWebSearchCredentialEnv: string | null; } @@ -120,6 +122,7 @@ export function prepareCreateSandboxMessaging( hasMessagingTokens: messagingTokenDefs.some(({ token }) => !!token), reusableMessagingProviders: [], reusableMessagingChannels: [], + missingBridgeChannels: [], missingWebSearchCredentialEnv, }; } @@ -183,19 +186,19 @@ export function prepareCreateSandboxMessaging( // Bridge channels have no token def at all when their env-only secret is // gone (fresh process), so the envKey loop above misses them. The gateway // still holds the refresh material — reuse the provider by name instead. - // Same profile selection as the mint above: the provider name derives from the - // channel alone, so an agent with no profile must reuse nothing either. + // The name carries the channel but not the agent, and onboard can recreate a + // sandbox name under a different agent, so match the gateway binding against + // the selected profile rather than accepting any provider with that name. if (input.enabledChannels != null) { - for (const channel of input.enabledChannels) { + for (const profile of bridgeProfiles) { + const channel = profile.channelId; + if (!input.enabledChannels.includes(channel)) continue; if (disabledChannelNames.has(channel)) continue; - for (const name of bridgeProviderNamesForChannel( - input.sandboxName, - channel, - bridgeProfiles, - )) { + for (const name of bridgeProviderNamesForChannel(input.sandboxName, channel, [profile])) { if (messagingTokenDefs.some((def) => def.name === name && def.token)) continue; if (reusableMessagingProviders.includes(name)) continue; - if (!input.providerExistsInGateway(name)) continue; + if (!input.providerMatchesGatewayCredential(name, profile.profileId, profile.credentialKey)) + continue; reusableMessagingProviders.push(name); if (!reusableMessagingChannels.includes(channel)) { reusableMessagingChannels.push(channel); @@ -204,8 +207,27 @@ export function prepareCreateSandboxMessaging( } } + // A selected bridge channel that ends with neither a token def nor a matching + // gateway provider would otherwise vanish from the create intent, and onboard + // can act on that intent by deleting and recreating the sandbox. Report it so + // the caller can stop and ask for the source secret again. + const selectedChannels = input.enabledChannels; + const missingBridgeChannels = + selectedChannels == null + ? [] + : [...new Set(bridgeProfiles.map((profile) => profile.channelId))].filter( + (channel) => + selectedChannels.includes(channel) && + !disabledChannelNames.has(channel) && + !reusableMessagingChannels.includes(channel) && + !bridgeProviderNamesForChannel(input.sandboxName, channel, bridgeProfiles).some( + (name) => messagingTokenDefs.some((def) => def.name === name && def.token), + ), + ); + return { disabledChannelNames, + missingBridgeChannels, messagingTokenDefs, extraPlaceholderKeys, hasMessagingTokens, diff --git a/src/lib/onboard/sandbox-messaging-preflight.test.ts b/src/lib/onboard/sandbox-messaging-preflight.test.ts index c69ff7c829f..b2ab4a1c196 100644 --- a/src/lib/onboard/sandbox-messaging-preflight.test.ts +++ b/src/lib/onboard/sandbox-messaging-preflight.test.ts @@ -22,6 +22,7 @@ function createResult(overrides = {}) { hasMessagingTokens: false, reusableMessagingProviders: [], reusableMessagingChannels: [], + missingBridgeChannels: [], missingWebSearchCredentialEnv: null, ...overrides, }; @@ -201,38 +202,39 @@ describe("prepareSandboxMessagingPreflight", () => { it.each([ { mode: "interactive", nonInteractive: false }, { mode: "non-interactive", nonInteractive: true }, - ])("aborts an enabled channel with an unavailable credential hash before $mode onboarding (#7808)", async ({ - nonInteractive, - }) => { - const planWithoutHash = createPlan("demo", "discord", undefined); - const incompletePlan = { - ...planWithoutHash, - credentialBindings: planWithoutHash.credentialBindings.map((binding) => { - const { credentialHash: _credentialHash, ...bindingWithoutHash } = binding; - return { ...bindingWithoutHash, credentialAvailable: false }; - }), - }; - const listSandboxes = vi.fn(() => ({ - sandboxes: [ - { name: "other", messaging: { plan: createPlan("other", "discord", "other-hash") } }, - ], - })); - const deps = createDeps({ - readMessagingPlanFromEnv: vi.fn(() => incompletePlan), - isNonInteractive: vi.fn(() => nonInteractive), - registry: { listSandboxes }, - }); + ])( + "aborts an enabled channel with an unavailable credential hash before $mode onboarding (#7808)", + async ({ nonInteractive }) => { + const planWithoutHash = createPlan("demo", "discord", undefined); + const incompletePlan = { + ...planWithoutHash, + credentialBindings: planWithoutHash.credentialBindings.map((binding) => { + const { credentialHash: _credentialHash, ...bindingWithoutHash } = binding; + return { ...bindingWithoutHash, credentialAvailable: false }; + }), + }; + const listSandboxes = vi.fn(() => ({ + sandboxes: [ + { name: "other", messaging: { plan: createPlan("other", "discord", "other-hash") } }, + ], + })); + const deps = createDeps({ + readMessagingPlanFromEnv: vi.fn(() => incompletePlan), + isNonInteractive: vi.fn(() => nonInteractive), + registry: { listSandboxes }, + }); - await expect(prepareSandboxMessagingPreflight(baseInput, deps)).rejects.toMatchObject({ - code: 1, - }); - expect(deps.error).toHaveBeenCalledWith( - expect.stringContaining("credential hashes are unavailable for discord"), - ); - expect(listSandboxes).not.toHaveBeenCalled(); - expect(deps.promptYesNoOrDefault).not.toHaveBeenCalled(); - expect(deps.prepareCreateSandboxMessaging).not.toHaveBeenCalled(); - }); + await expect(prepareSandboxMessagingPreflight(baseInput, deps)).rejects.toMatchObject({ + code: 1, + }); + expect(deps.error).toHaveBeenCalledWith( + expect.stringContaining("credential hashes are unavailable for discord"), + ); + expect(listSandboxes).not.toHaveBeenCalled(); + expect(deps.promptYesNoOrDefault).not.toHaveBeenCalled(); + expect(deps.prepareCreateSandboxMessaging).not.toHaveBeenCalled(); + }, + ); it("aborts a second Slack Socket Mode sandbox on the same gateway (#7808)", async () => { let gatewayName = "startup-gateway"; @@ -283,6 +285,21 @@ describe("prepareSandboxMessagingPreflight", () => { ); }); + it("fails before recreate/delete when a selected bridge channel has no usable provider", async () => { + const deps = createDeps({ + prepareCreateSandboxMessaging: vi.fn(() => + createResult({ missingBridgeChannels: ["googlechat"] }), + ), + }); + + await expect( + prepareSandboxMessagingPreflight({ ...baseInput, enabledChannels: ["googlechat"] }, deps), + ).rejects.toMatchObject({ code: 1 }); + expect(deps.error).toHaveBeenCalledWith( + " googlechat mints its outbound token gateway-side and needs GOOGLECHAT_SERVICE_ACCOUNT to configure it.", + ); + }); + it("names the selected Tavily credential when recreate preflight fails", async () => { const deps = createDeps({ prepareCreateSandboxMessaging: vi.fn(() => diff --git a/src/lib/onboard/sandbox-messaging-preflight.ts b/src/lib/onboard/sandbox-messaging-preflight.ts index f37ce284c87..8d525f1fe90 100644 --- a/src/lib/onboard/sandbox-messaging-preflight.ts +++ b/src/lib/onboard/sandbox-messaging-preflight.ts @@ -3,6 +3,7 @@ import type { WebSearchConfig } from "../inference/web-search"; import type { SandboxMessagingPlan } from "../messaging/manifest/types"; +import { bridgeSecretEnvsForChannel } from "./messaging-bridge-provider"; import { enforceMessagingChannelConflicts as defaultEnforceMessagingChannelConflicts, type MessagingConflictGuardDeps, @@ -87,6 +88,19 @@ export async function prepareSandboxMessagingPreflight( providerMatchesGatewayCredential: deps.providerMatchesGatewayCredential, }); + // Fail before the caller can act on this intent: onboard may delete and + // recreate the sandbox, and a selected channel that resolved to nothing would + // be dropped from the replacement without a word. + result.missingBridgeChannels.forEach((channel) => { + deps.error( + ` ${channel} mints its outbound token gateway-side and needs ${bridgeSecretEnvsForChannel(channel).join(", ")} to configure it.`, + ); + deps.error(" Paste the secret at the enrollment prompt or export the env var, then re-run."); + }); + if (result.missingBridgeChannels.length > 0) { + deps.exitProcess(1); + } + if (result.missingWebSearchCredentialEnv) { const envKey = result.missingWebSearchCredentialEnv; deps.error(` Web search is enabled, but ${envKey} is not available in this process.`); diff --git a/src/lib/onboard/sandbox-provider-cleanup.ts b/src/lib/onboard/sandbox-provider-cleanup.ts index f278cb7eccc..2b9d1a26257 100644 --- a/src/lib/onboard/sandbox-provider-cleanup.ts +++ b/src/lib/onboard/sandbox-provider-cleanup.ts @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { listMessagingProviderSuffixes } from "../messaging/channels"; +import { listMessagingBridgeProfiles } from "./messaging-bridge-provider"; import { NAME_MAX_LENGTH, NAME_VALID_PATTERN } from "../name-validation"; export { @@ -67,6 +68,10 @@ export type SandboxRecreateCleanupDeps = DetachSandboxProvidersDeps & { export const SANDBOX_PROVIDER_SUFFIXES = [ ...listMessagingProviderSuffixes().map((suffix) => suffix.replace(/^-/, "")), + // Bridge-profile channels mint their provider outside the manifest credentials + // (nothing is delivered into the sandbox), so the credential-derived suffixes + // above miss them and destroy would leave the provider still minting a token. + ...new Set(listMessagingBridgeProfiles().map((profile) => `${profile.channelId}-bridge`)), "brave-search", "tavily-search", ] as readonly string[]; diff --git a/test/onboard-pre-destructive-intent.test.ts b/test/onboard-pre-destructive-intent.test.ts index 351c1bd757b..01c80666754 100644 --- a/test/onboard-pre-destructive-intent.test.ts +++ b/test/onboard-pre-destructive-intent.test.ts @@ -2,29 +2,34 @@ // SPDX-License-Identifier: Apache-2.0 import assert from "node:assert/strict"; -import { spawnSync } from "node:child_process"; import fs from "node:fs"; -import os from "node:os"; import path from "node:path"; import { describe, it } from "vitest"; +import { + createOnboardProcessWorkspace, + minimalSpawnEnv, + runOnboardProcess, + testRepoRoot, + trailingJsonPayload, +} from "./helpers/onboard-child-process-harness"; import { onboardScriptMocksPath } from "./helpers/onboard-split-context"; +const onboardPath = JSON.stringify(path.join(testRepoRoot, "src", "lib", "onboard.ts")); +const runnerPath = JSON.stringify(path.join(testRepoRoot, "src", "lib", "runner.ts")); +const registryPath = JSON.stringify(path.join(testRepoRoot, "src", "lib", "state", "registry.ts")); +const defsPath = JSON.stringify(path.join(testRepoRoot, "src", "lib", "agent", "defs.ts")); + describe("onboard sandbox create intent boundary", () => { - it("rejects stale credential capabilities before real create mutations (#6226)", { - timeout: 60_000, - }, () => { - const repoRoot = path.join(import.meta.dirname, ".."); - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-intent-boundary-")); - try { - const scriptPath = path.join(tmpDir, "stale-binding.js"); - const onboardPath = JSON.stringify(path.join(repoRoot, "src", "lib", "onboard.ts")); - const runnerPath = JSON.stringify(path.join(repoRoot, "src", "lib", "runner.ts")); - const registryPath = JSON.stringify( - path.join(repoRoot, "src", "lib", "state", "registry.ts"), - ); - - const script = String.raw` + it( + "rejects stale credential capabilities before real create mutations (#6226)", + { + timeout: 60_000, + }, + () => { + const workspace = createOnboardProcessWorkspace("nemoclaw-intent-boundary-"); + try { + const script = String.raw` const runner = require(${runnerPath}); const registry = require(${registryPath}); const childProcess = require("node:child_process"); @@ -106,39 +111,155 @@ const resolved = { } })(); `; - fs.writeFileSync(scriptPath, script); - - const childEnv = Object.fromEntries( - Object.entries(process.env).filter(([name]) => !/^(?:DISCORD|TELEGRAM)_/.test(name)), - ); - const result = spawnSync( - process.execPath, - ["--require", JSON.parse(onboardScriptMocksPath), scriptPath], - { - cwd: repoRoot, - encoding: "utf-8", - timeout: 55_000, - env: { - ...childEnv, - HOME: tmpDir, - NEMOCLAW_NON_INTERACTIVE: "1", - NEMOCLAW_RECREATE_SANDBOX: "1", - NEMOCLAW_RECREATE_WITHOUT_BACKUP: "1", + const scriptPath = workspace.path("stale-binding.js"); + fs.writeFileSync(scriptPath, script); + + const result = runOnboardProcess( + ["--require", JSON.parse(onboardScriptMocksPath), scriptPath], + { + env: minimalSpawnEnv(workspace.homeDir, { + NEMOCLAW_NON_INTERACTIVE: "1", + NEMOCLAW_RECREATE_SANDBOX: "1", + NEMOCLAW_RECREATE_WITHOUT_BACKUP: "1", + }), + timeoutMs: 55_000, }, - }, - ); - - assert.equal(result.status, 0, result.stderr); - const payloadLine = result.stdout - .trim() - .split("\n") - .find((line) => line.startsWith("{") && line.endsWith("}")); - assert.ok(payloadLine, `expected JSON payload in stdout:\n${result.stdout}`); - const payload = JSON.parse(payloadLine); - assert.match(payload.error, /missing credential binding|credential binding set changed/); - assert.deepEqual(payload.mutations, []); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } - }); + ); + + assert.equal(result.status, 0, result.stderr); + const payload = trailingJsonPayload(result.stdout) as { + error: string; + mutations: string[]; + }; + assert.match(payload.error, /missing credential binding|credential binding set changed/); + assert.deepEqual(payload.mutations, []); + } finally { + workspace.remove(); + } + }, + ); + + it( + "refuses a selected bridge channel with no usable provider before deleting the sandbox", + { + timeout: 60_000, + }, + () => { + // Onboard offers to delete and recreate a sandbox name under another agent. + // The bridge provider name carries no agent, so the gateway still holds the + // OpenClaw binding; without the source secret there is nothing to mint from + // and nothing safe to reuse. That has to stop the run before the delete. + const workspace = createOnboardProcessWorkspace("nemoclaw-bridge-boundary-"); + try { + const script = String.raw` +const runner = require(${runnerPath}); +const registry = require(${registryPath}); +const defs = require(${defsPath}); +const childProcess = require("node:child_process"); +const record = (text) => { console.log("CMD " + text); return text; }; + +// The gateway still holds the OpenClaw bridge binding for this sandbox name. +const staleBinding = [ + "Name: my-assistant-googlechat-bridge", + "Type: google-chat-bridge", + "Credential keys: GOOGLE_CHAT_ACCESS_TOKEN", + "Config keys: ", + "", +].join(String.fromCharCode(10)); + +runner.run = (command) => { + const text = record(Array.isArray(command) ? command.join(" ") : String(command)); + if (text.includes("provider get") && text.includes("googlechat-bridge")) { + return { status: 0, stdout: staleBinding }; + } + return { status: 0 }; +}; +runner.runCapture = (command) => { + const text = record(Array.isArray(command) ? command.join(" ") : String(command)); + return text.includes("provider get") && text.includes("googlechat-bridge") ? staleBinding : ""; +}; +registry.getSandbox = (name) => ({ name, agent: "openclaw" }); +registry.removeSandbox = (name) => { record("registry remove " + name); }; +registry.updateSandbox = (name) => { record("registry update " + name); }; +registry.registerSandbox = (entry) => { record("registry register " + entry.name); }; +childProcess.spawn = () => { throw new Error("unexpected sandbox create"); }; + +const { createSandbox } = require(${onboardPath}); + +(async () => { + try { + await createSandbox( + null, + "gpt-5.4", + "nvidia-prod", + null, + "my-assistant", + null, + ["googlechat"], + null, + defs.loadAgent("hermes"), + null, + null, + null, + [], + null, + { recreate: true, toolDisclosure: "progressive", observabilityEnabled: false, extraProviders: [] }, + ); + console.log("CREATE-RETURNED"); + } catch (error) { + console.log("CREATE-THREW " + String(error.message || error)); + } +})(); +`; + const scriptPath = workspace.path("stale-bridge.js"); + fs.writeFileSync(scriptPath, script); + // The run reaches the gateway before it reaches the failure under test, and + // the binary resolver exits the process when OpenShell is absent. + const openshellStub = workspace.writeExecutable("openshell", "#!/bin/sh\nexit 0\n"); + + const result = runOnboardProcess( + ["--require", JSON.parse(onboardScriptMocksPath), scriptPath], + { + env: minimalSpawnEnv(workspace.homeDir, { + NEMOCLAW_NON_INTERACTIVE: "1", + NEMOCLAW_RECREATE_SANDBOX: "1", + NEMOCLAW_RECREATE_WITHOUT_BACKUP: "1", + NEMOCLAW_OPENSHELL_BIN: openshellStub, + }), + timeoutMs: 55_000, + }, + ); + + const output = result.output; + assert.equal(result.error, undefined, output); + assert.equal(result.signal, null, output); + // The guard exits the process, so neither marker can be reached and the + // status is the guard's own. A hang or an unrelated throw fails here. + assert.equal(result.status, 1, output); + assert.doesNotMatch(output, /CREATE-RETURNED|CREATE-THREW/, output); + assert.match(output, /GOOGLECHAT_SERVICE_ACCOUNT/, output); + + // Commands are read from the log the child emits as each one is issued, + // since a payload printed at the end would never arrive. The binding read + // has to have happened, and nothing may mutate before the refusal. + const issued = output + .split(String.fromCharCode(10)) + .filter((line) => line.startsWith("CMD ")); + assert.equal( + issued.filter((line) => /provider get .*my-assistant-googlechat-bridge/.test(line)) + .length, + 1, + output, + ); + const mutating = issued.filter((line) => + /sandbox delete|sandbox provider (?:attach|detach)|provider (?:create|update|delete)|registry (?:remove|update|register)/.test( + line, + ), + ); + assert.deepEqual(mutating, [], output); + } finally { + workspace.remove(); + } + }, + ); }); diff --git a/test/sandbox-provider-cleanup.test.ts b/test/sandbox-provider-cleanup.test.ts index 8be3385bced..21548996ab0 100644 --- a/test/sandbox-provider-cleanup.test.ts +++ b/test/sandbox-provider-cleanup.test.ts @@ -39,6 +39,7 @@ describe("SANDBOX_PROVIDER_SUFFIXES", () => { "slack-bridge", "slack-app", "teams-bridge", + "googlechat-bridge", "brave-search", "tavily-search", ].sort(), From f0495c4307ad9c9ff6868b938ea48ccc454190aa Mon Sep 17 00:00:00 2001 From: Hung Le Date: Wed, 19 Aug 2026 14:20:50 +0530 Subject: [PATCH 13/15] fix(messaging): retire an unreadable Google Chat delivery Shaping a REST receivedMessage decodes its base64 payload, and that construction sat outside the per-message guard. A delivery whose data field cannot be decoded raised out of the loop, and nothing restarts the pull: connect() starts the task once, no callback observes it, and the platform keeps reporting itself connected because the bundled is_connected hook reads configuration only. Inbound would go quiet for the rest of the session. Guarding the construction alone would leave the poisoned delivery eligible for repeated redelivery until the subscription's own retention or dead-letter policy retired it, since redelivery cannot repair the same bytes and GOOGLE_CHAT_MAX_MESSAGES defaults to one per pull. The guard now acknowledges it, which is how the bundled handler retires an envelope it cannot parse. The handler branch is unchanged and synthesizes no acknowledgement, because the handler owns that policy and its failure can be transient. Neighbouring reads shared the defect and are covered now: the ack id, which raises on a receivedMessages entry that is not a mapping, and the envelope read, materialised inside the pull guard so that a 200 body which is not an object and a receivedMessages which is not iterable both become failed pulls that retry. The scripted aiohttp double signalled a refused connection through assert, which python -O strips. An inherited PYTHONOPTIMIZE turned that refusal into an ordinary non-200 response, so the acknowledge-transport scenario still observed two pulls and two handled messages with the production guard removed. Both checks now raise explicitly, and the refusal raises ConnectionError, which reaches the pull loop through the same clause a real proxy refusal would. One scenario carries all four malformed shapes, and reverting any single guard fails it. The transport scenario pins the double: it fails under PYTHONOPTIMIZE=1 when the acknowledge guard stops catching transport errors. --- .../googlechat/runtime/hermes-adapter.py | 31 ++++++++++++--- .../googlechat/runtime/hermes-adapter.test.ts | 38 ++++++++++++++++++- 2 files changed, 61 insertions(+), 8 deletions(-) diff --git a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py index fa3cbdc572d..d478dcf49eb 100644 --- a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py +++ b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py @@ -240,6 +240,10 @@ async def _rest_pull(self): await asyncio.sleep(3) continue payload = await resp.json() + # Read and materialise the envelope here, so a body that is + # not an object and a receivedMessages that is not iterable + # are both failed pulls rather than escaping exceptions. + received = list(payload.get("receivedMessages") or []) except asyncio.CancelledError: raise except Exception as exc: # noqa: BLE001 - transient pull errors are retried @@ -247,21 +251,36 @@ async def _rest_pull(self): await asyncio.sleep(3) continue - received = payload.get("receivedMessages") or [] if not received: await asyncio.sleep(0.2) # guard against fast-empty long-poll returns continue acks: list = [] for received_message in received: - shim = _RestPubsubMessage( - received_message.get("message") or {}, - acks, - received_message.get("ackId"), - ) + ack_id = None + try: + ack_id = received_message.get("ackId") + shim = _RestPubsubMessage( + received_message.get("message") or {}, acks, ack_id + ) + except Exception: # noqa: BLE001 - a corrupt delivery ends nothing + # Redelivery cannot repair the same undecodable bytes, so + # retire it the way the bundled handler retires an + # unparseable envelope. Otherwise it may be redelivered + # repeatedly, and GOOGLE_CHAT_MAX_MESSAGES defaults to 1, + # so each redelivery costs a whole pull response. + _GC_LOG.exception( + "[GoogleChat][NemoClaw] retiring an unreadable delivery" + ) + if ack_id: + acks.append(ack_id) + continue try: await asyncio.to_thread(self._on_pubsub_message, shim) except Exception: # noqa: BLE001 - one bad message must not kill the loop + # Nothing is added to acks here: the handler acknowledges + # for itself, and unlike an unreadable payload its failure + # can be transient, so redelivery stays the default. _GC_LOG.exception("[GoogleChat][NemoClaw] message handler raised") if acks: diff --git a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts index afcf762ec5a..d279f821499 100644 --- a/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts +++ b/src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts @@ -69,10 +69,15 @@ class ClientSession: REQUESTS.append( {"url": url, "method": "POST", "authorization": (headers or {}).get("Authorization"), "body": json} ) - assert SCRIPT, "aiohttp double ran out of scripted responses: " + url + # Raise rather than assert: python -O strips assert statements, and an + # inherited PYTHONOPTIMIZE would silently turn the refusal below into an + # ordinary non-200 response, passing the test without its claim. + if not SCRIPT: + raise AssertionError("aiohttp double ran out of scripted responses: " + url) status, payload, on_send = SCRIPT.pop(0) on_send and on_send() - assert status != "transport-error", "proxy refused the acknowledge" + if status == "transport-error": + raise ConnectionError("proxy refused the acknowledge") return _Response(status, payload) `; @@ -119,6 +124,14 @@ def _handler(message): adapter._on_pubsub_message = _handler delivery, redelivery = _received("ack-1", "hello"), _received("ack-1", "hello") +# Two corrupted deliveries: an entry that is not a mapping at all, and one whose +# base64 cannot be decoded. Both raise while the message is shaped, and only the +# second carries an ack id to retire it with. The scenario also opens with a pull +# body whose receivedMessages is not iterable, which fails both on the read and on +# the loop over it; an empty container would be coerced to {} by the double and +# prove nothing. +malformed = {"receivedMessages": [None, {"ackId": "ack-bad", "message": {"data": "abcde"}}]} + # Each entry is (pull or acknowledge response status, payload, side effect). # A rejected acknowledge means Pub/Sub redelivers the same ackId on the next pull. aiohttp.SCRIPT.extend( @@ -133,6 +146,10 @@ aiohttp.SCRIPT.extend( (200, redelivery, None), (200, {}, _stop), ], + "malformed-payload": [ + (200, {"receivedMessages": 1}, None), (200, malformed, None), (200, {}, None), + (200, delivery, None), (200, {}, _stop), + ], "nack": [(200, delivery, _stop)], }[SCENARIO] ) @@ -218,6 +235,23 @@ describe("Hermes Google Chat keyless REST pull", () => { expect(requests.filter((request) => request.url.endsWith(":pull"))).toHaveLength(2); }); + it("retires an undecodable delivery and survives a malformed pull response", () => { + const { requests, handled } = runScenario("malformed-payload"); + + // Reading the response envelope and shaping each message both raise here: + // receivedMessages is not iterable, one entry is not a mapping, and one payload + // is not decodable. Any of them ending the pull would silence inbound for the + // rest of the session, and leaving the entry that has an ack id unacknowledged + // would permit repeated poison-message redelivery. + expect(handled).toEqual(["hello"]); + expect(requests.filter((request) => request.url.endsWith(":pull"))).toHaveLength(3); + expect( + requests + .filter((request) => request.url.endsWith(":acknowledge")) + .map((request) => request.body), + ).toEqual([{ ackIds: ["ack-bad"] }, { ackIds: ["ack-1"] }]); + }); + it("acknowledges nothing for a message the handler nacks", () => { const { requests, handled } = runScenario("nack"); From 3668e10e91931fa172c431189261c90b8e0e73c0 Mon Sep 17 00:00:00 2001 From: Hung Le Date: Wed, 19 Aug 2026 15:12:20 +0530 Subject: [PATCH 14/15] fix(hermes): admit the Google Chat asset into the portable context The Hermes portable build context walks every directory the Dockerfile copies and rejects a file that its reviewed manifest does not name, so the five files this branch adds under the Google Chat channel failed installer-integration on the first one it reached. Its Dockerfile parser also rejects a COPY that continues onto a second line, which the new plugin-asset COPY did. Join that instruction, name its source in the local COPY allowlist, and add the five files to the reviewed manifest. The parser and the reviewed manifest are the same two lists the gate compares, so both had to move together. installer-integration clones the pushed revision rather than the working tree, so this was verified by running the parser against the working tree and by the unit suite under src/lib/onboard/experimental, which builds its fixture from the manifest. --- agents/hermes/Dockerfile | 3 +-- .../experimental/hermes-portable-build-context-files.ts | 5 +++++ .../onboard/experimental/hermes-portable-build-context.ts | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/agents/hermes/Dockerfile b/agents/hermes/Dockerfile index 4fd553cff07..e496c41bdd0 100644 --- a/agents/hermes/Dockerfile +++ b/agents/hermes/Dockerfile @@ -80,8 +80,7 @@ FROM scratch AS hermes-agent-payload COPY agents/hermes/plugin/ /opt/nemoclaw-hermes-plugin/ # Channel-owned Google Chat runtime asset, loaded by the plugin beside its own # __init__.py. -COPY src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py \ - /opt/nemoclaw-hermes-plugin/googlechat_adapter.py +COPY src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py /opt/nemoclaw-hermes-plugin/googlechat_adapter.py COPY agents/hermes/generate-config.ts /opt/nemoclaw-hermes-config/generate-config.ts COPY agents/hermes/config/ /opt/nemoclaw-hermes-config/config/ COPY agents/hermes/image-build-probes.py /opt/nemoclaw-hermes-config/image-build-probes.py diff --git a/src/lib/onboard/experimental/hermes-portable-build-context-files.ts b/src/lib/onboard/experimental/hermes-portable-build-context-files.ts index 54cfeb53e48..a6b9773fbad 100644 --- a/src/lib/onboard/experimental/hermes-portable-build-context-files.ts +++ b/src/lib/onboard/experimental/hermes-portable-build-context-files.ts @@ -204,7 +204,10 @@ export const HERMES_PORTABLE_BUILD_CONTEXT_FILES = [ { path: "src/lib/messaging/channels/googlechat/hooks/tunnel-runtime.test.ts", mode: "100644" }, { path: "src/lib/messaging/channels/googlechat/hooks/tunnel-runtime.ts", mode: "100644" }, { path: "src/lib/messaging/channels/googlechat/manifest.ts", mode: "100644" }, + { path: "src/lib/messaging/channels/googlechat/policy.test.ts", mode: "100644" }, + { path: "src/lib/messaging/channels/googlechat/policy/hermes.yaml", mode: "100644" }, { path: "src/lib/messaging/channels/googlechat/policy/openclaw.yaml", mode: "100644" }, + { path: "src/lib/messaging/channels/googlechat/provider-profile/hermes.yaml", mode: "100644" }, { path: "src/lib/messaging/channels/googlechat/provider-profile/openclaw.yaml", mode: "100644" }, { path: "src/lib/messaging/channels/googlechat/rendered-config-parser.ts", mode: "100644" }, { path: "src/lib/messaging/channels/googlechat/runtime-contract.test.ts", mode: "100644" }, @@ -224,6 +227,8 @@ export const HERMES_PORTABLE_BUILD_CONTEXT_FILES = [ path: "src/lib/messaging/channels/googlechat/runtime/googlechat-trusted-proxy-fetch.ts", mode: "100644", }, + { path: "src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py", mode: "100644" }, + { path: "src/lib/messaging/channels/googlechat/runtime/hermes-adapter.test.ts", mode: "100644" }, { path: "src/lib/messaging/channels/googlechat/template-resolver.test.ts", mode: "100644" }, { path: "src/lib/messaging/channels/googlechat/template-resolver.ts", mode: "100644" }, { path: "src/lib/messaging/channels/googlechat/tunnel/lifecycle.test.ts", mode: "100644" }, diff --git a/src/lib/onboard/experimental/hermes-portable-build-context.ts b/src/lib/onboard/experimental/hermes-portable-build-context.ts index 98a70c3a618..895e9f15d6e 100644 --- a/src/lib/onboard/experimental/hermes-portable-build-context.ts +++ b/src/lib/onboard/experimental/hermes-portable-build-context.ts @@ -83,6 +83,7 @@ const LOCAL_COPY_SOURCES = [ "src/lib/actions/sandbox/openshell-child-visible-credentials.v0.0.101.json", "src/lib/hermes-managed-route.ts", "src/lib/messaging/", + "src/lib/messaging/channels/googlechat/runtime/hermes-adapter.py", "src/lib/tool-disclosure.ts", "tools/mcp-tool-discovery-runtime/reviewed-runtime-bundle/managed-startup-image-runtime.bundle", "tools/mcp-tool-discovery-runtime/reviewed-runtime-bundle/mcp-tool-discovery/BUNDLED_PACKAGES.json", From 7441b4349839c9850e5108dfcc0eb989f396b4d6 Mon Sep 17 00:00:00 2001 From: Hung Le Date: Wed, 19 Aug 2026 18:57:20 +0530 Subject: [PATCH 15/15] docs(messaging): scope Google Chat guidance to the agent that needs it The appPrincipal walkthrough lived in the channel's enrollmentNotes, which `common.tokenPaste` prints for every agent right after the service-account key is saved. A Hermes operator therefore read a block that only applies to OpenClaw, ending in a `GOOGLECHAT_APP_PRINCIPAL= ... channels add` and rebuild instruction that does nothing on a Pub/Sub pull deployment. The prompt itself was already correct: appPrincipal is collected by googlechat-openclaw-config-prompt, which is gated to OpenClaw, so Hermes was never asked for the value. Only the explanatory text leaked. Move it to prompt.help on the appPrincipal input, the same field allowFrom already uses for its multi-line guidance, so it renders where it is actionable and inherits the hook's agent gate. The Pub/Sub subscription prompt gains the binding operators miss most often, as four aligned lines rather than prose: which account needs roles/pubsub.publisher on the topic, where its address is shown, and what the failure looks like. A Chat app built with Interactive features publishes as the gcp-sa-gsuiteaddons service agent rather than chat-api-push@system, so granting Publisher only to the latter leaves the channel connected while no event reaches the subscription and Chat reports the bot as not responding. That was the last blocker in live setup, and nothing in the flow said so. This is prose and its placement, but it does change what onboarding prints: Hermes loses the appPrincipal block at the service-account step and gains two lines at the subscription step. Verified on a live Hermes onboard and by resolving the manifest: no enrollmentNotes remain, the guidance is 13 help lines on appPrincipal, and the Hermes-visible prompts are serviceAccount, allowFrom, projectId and subscriptionName. No test was added, since asserting manifest wording would be a source-shape test and that budget is zero. --- .../messaging/channels/googlechat/manifest.ts | 41 +++++++++++-------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/lib/messaging/channels/googlechat/manifest.ts b/src/lib/messaging/channels/googlechat/manifest.ts index cd7980ec258..a1f84c1a568 100644 --- a/src/lib/messaging/channels/googlechat/manifest.ts +++ b/src/lib/messaging/channels/googlechat/manifest.ts @@ -14,23 +14,6 @@ export const googlechatManifest = { id: "googlechat", displayName: "Google Chat", description: "Google Chat (Chat API) bot messaging (experimental)", - enrollmentNotes: [ - "┃ GOOGLE CHAT — appPrincipal", - "┃", - "┃ Workspace account → leave blank, done.", - "┃ Personal Gmail → needs the add-on's ~21-digit ID (not an email), stable across rebuilds.", - "┃", - "┃ If you already know it, paste it at the prompt and you're done.", - "┃ If not, leave it blank — the first DM reveals it once the sandbox is live:", - "┃", - "┃ 1. Watch the gateway log:", - '┃ nemoclaw logs --follow | grep "unexpected add-on principal"', - "┃ 2. DM the bot once — it won't reply yet, that's expected. The log prints:", - "┃ unexpected add-on principal: ", - "┃ 3. Save that and rebuild:", - "┃ GOOGLECHAT_APP_PRINCIPAL= nemoclaw channels add googlechat", - "┃ nemoclaw rebuild --yes", - ], supportedAgents: ["openclaw", "hermes"], auth: { mode: "token-paste", @@ -96,6 +79,21 @@ export const googlechatManifest = { "appPrincipal is the add-on's numeric OAuth client ID (uniqueId, ~21 digits), not an email.", prompt: { label: "Google Chat appPrincipal", + help: [ + " Workspace account → leave blank, done.", + " Personal Gmail → needs the add-on's ~21-digit ID (not an email), stable across rebuilds.", + "", + " If you already know it, paste it at the prompt and you're done.", + " If not, leave it blank — the first DM reveals it once the sandbox is live:", + "", + " 1. Watch the gateway log:", + ' nemoclaw logs --follow | grep "unexpected add-on principal"', + " 2. DM the bot once — it won't reply yet, that's expected. The log prints:", + " unexpected add-on principal: ", + " 3. Save that and rebuild:", + " GOOGLECHAT_APP_PRINCIPAL= nemoclaw channels add googlechat", + " nemoclaw rebuild --yes", + ].join("\n"), emptyValueMessage: "Workspace accounts do not need it; personal accounts must set it later", }, }, @@ -141,7 +139,14 @@ export const googlechatManifest = { statePath: "googlechatConfig.subscriptionName", prompt: { label: "Google Chat Pub/Sub subscription (projects/

/subscriptions/)", - help: "The pull subscription bound to the Chat events topic. Hermes pulls from it over the Pub/Sub REST API; the gateway-minted token is scoped to both chat.bot and pubsub.", + help: [ + "The pull subscription bound to the Chat events topic. Hermes pulls from it over the Pub/Sub REST API; the gateway-minted token is scoped to both chat.bot and pubsub.", + " Its topic must grant roles/pubsub.publisher to the app's push account:", + " Interactive features service-@gcp-sa-gsuiteaddons.iam.gserviceaccount.com", + " Classic bot chat-api-push@system.gserviceaccount.com", + " Shown at Chat API → Configuration → Connection settings", + " Missing it channel connects, no event arrives, Chat says the bot is not responding", + ].join("\n"), emptyValueMessage: "required for the Hermes Google Chat channel", }, },