From 4aa4d056f8cc30e0d338f5485556dc29f1c91ac8 Mon Sep 17 00:00:00 2001 From: sebastian-hsu Date: Wed, 26 Aug 2026 09:10:20 +0800 Subject: [PATCH 1/2] feat(googlechat): keyless ADC auth + send-once for the unified adapter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keyless ADC (MetadataTokenSource): mint a chat.bot-scoped token from the workload's own GCP identity — GCE metadata (SA email + base token) -> IAM Credentials generateAccessToken (self-impersonation). No SA key file. Config [googlechat].use_adc / GOOGLE_CHAT_USE_ADC; auth precedence SA key > ADC > static token; cache under the IAM-granted expireTime (fallback 3600s). Send-once for Google Chat: its write rate limit is 1/sec/space (create+patch+delete combined) so per-token streaming edits 429, and the unified adapter returns a synthetic message id that patch can't target (404). googlechat added to NON_STREAMING_PLATFORMS (renamed from NON_EDITABLE_PLATFORMS); resolve_streaming forces send-once on both the embedded dispatch (stream_prompt_blocks) and WebSocket gateway paths. Also: Dockerfile.claude OPENAB_BUILD_FEATURES arg, Helm googleChat.useAdc value, docs + config-first conformance entry + googlechat.toml schema record. Co-Authored-By: Claude Opus 4.8 --- charts/openab/templates/gateway.yaml | 6 +- charts/openab/values.yaml | 1 + config.toml.example | 1 + crates/openab-core/src/adapter.rs | 47 +- crates/openab-core/src/config.rs | 22 + crates/openab-core/src/gateway.rs | 38 +- .../openab-gateway/src/adapters/googlechat.rs | 451 +++++++++++++++++- crates/openab-gateway/src/lib.rs | 11 + .../tests/config_first_conformance.rs | 1 + docs/config-reference.md | 1 + docs/google-chat.md | 28 +- docs/platforms/schema/googlechat.toml | 14 +- docs/platforms/schema/lineworks.toml | 4 +- src/main.rs | 1 + 14 files changed, 588 insertions(+), 38 deletions(-) diff --git a/charts/openab/templates/gateway.yaml b/charts/openab/templates/gateway.yaml index 2a89dc79a..40454f165 100644 --- a/charts/openab/templates/gateway.yaml +++ b/charts/openab/templates/gateway.yaml @@ -157,10 +157,14 @@ spec: value: "false" {{- end }} {{- end }} - {{- $hasGoogleChat := or (($cfg.gateway).googleChat).saKeyJson (($cfg.gateway).googleChat).accessToken (($cfg.gateway).googleChat).audience }} + {{- $hasGoogleChat := or (($cfg.gateway).googleChat).saKeyJson (($cfg.gateway).googleChat).accessToken (($cfg.gateway).googleChat).audience (($cfg.gateway).googleChat).useAdc }} {{- if $hasGoogleChat }} - name: GOOGLE_CHAT_ENABLED value: "true" + {{- if (($cfg.gateway).googleChat).useAdc }} + - name: GOOGLE_CHAT_USE_ADC + value: "true" + {{- end }} {{- if (($cfg.gateway).googleChat).audience }} - name: GOOGLE_CHAT_AUDIENCE value: {{ ($cfg.gateway).googleChat.audience | quote }} diff --git a/charts/openab/values.yaml b/charts/openab/values.yaml index fd37b023c..2258f0e54 100644 --- a/charts/openab/values.yaml +++ b/charts/openab/values.yaml @@ -480,6 +480,7 @@ agents: audience: "" # JWT audience → GOOGLE_CHAT_AUDIENCE (set to your webhook URL to enable JWT verification) saKeyJson: "" # Service account key JSON string → GOOGLE_CHAT_SA_KEY_JSON (recommended, auto-refresh) accessToken: "" # Static OAuth2 access token → GOOGLE_CHAT_ACCESS_TOKEN (fallback, 1-hour TTL) + useAdc: false # Keyless ADC → GOOGLE_CHAT_USE_ADC. Mints chat.bot token from the pod's own GCP identity (GCE metadata + IAM Credentials generateAccessToken). No SA key file. Needs roles/iam.serviceAccountTokenCreator on the SA over itself + iamcredentials.googleapis.com enabled. Ignored when saKeyJson is set. webhookPath: "" # Gateway default: /webhook/googlechat → GOOGLE_CHAT_WEBHOOK_PATH # WeCom (企业微信) adapter config (gateway-side env vars) # See docs/wecom.md for full setup guide diff --git a/config.toml.example b/config.toml.example index 00add1a9b..8c0fddb6c 100644 --- a/config.toml.example +++ b/config.toml.example @@ -123,6 +123,7 @@ allowed_channels = ["1234567890"] # ↑ omitted + non-empty list → auto- # sa_key_json = "${GOOGLE_CHAT_SA_KEY_JSON}" # inline SA key; wins over sa_key_file # sa_key_file = "/etc/openab/sa.json" # env fallback: GOOGLE_CHAT_SA_KEY_FILE # access_token = "${GOOGLE_CHAT_ACCESS_TOKEN}" # static token alternative +# use_adc = true # keyless ADC: mint chat.bot token from the workload's own GCP identity (GCE metadata + IAM Credentials); no SA key. env fallback: GOOGLE_CHAT_USE_ADC. Ignored when sa_key_* is set. # audience = "projects//..." # enables webhook JWT verification (L1) # webhook_path = "/webhook/googlechat" # env fallback: GOOGLE_CHAT_WEBHOOK_PATH # allow_all_users = false # env fallback: GOOGLE_CHAT_ALLOW_ALL_USERS diff --git a/crates/openab-core/src/adapter.rs b/crates/openab-core/src/adapter.rs index fa7e95dba..e69d7cda2 100644 --- a/crates/openab-core/src/adapter.rs +++ b/crates/openab-core/src/adapter.rs @@ -35,6 +35,22 @@ fn reply_message_limit(platform: &str, adapter_limit: usize) -> usize { } } +/// Whether to use cosmetic streaming (placeholder + in-place edits) for +/// `platform`, given the adapter's own preference. Forces send-once for: +/// - `acp`: streams append-only `agent_message_chunk` deltas, not edits. +/// - platforms in `NON_STREAMING_PLATFORMS` — no message-edit API, or an edit +/// API that cannot be driven per-token (e.g. `googlechat`) — see +/// `NON_STREAMING_PLATFORMS` for the per-platform rationale and +/// `platform_supports_streaming`. +/// +/// This is the embedded/unified dispatch gate; the WebSocket +/// `run_gateway_adapter` path applies the same `platform_supports_streaming` check. +fn resolve_streaming(platform: &str, adapter_prefers_streaming: bool) -> bool { + platform != "acp" + && crate::gateway::platform_supports_streaming(platform) + && adapter_prefers_streaming +} + /// Parse `[[key:value]]` directives from the beginning of agent output. /// Returns parsed directives and the remaining content (directives stripped). pub fn parse_output_directives(content: &str) -> (OutputDirectives, String) { @@ -701,15 +717,14 @@ impl AdapterRouter { let adapter = adapter.clone(); let thread_channel = thread_channel.clone(); let message_limit = reply_message_limit(&thread_channel.platform, adapter.message_limit()); - // ACP must not inherit the unified adapter's Telegram streaming flag (wrong - // coupling): it streams append-only `agent_message_chunk` deltas built from the - // post+edit (`edit_message` snapshot) path, i.e. streaming=false. Decide it - // explicitly by platform rather than by whatever Telegram happens to be set to. - let streaming = if thread_channel.platform == "acp" { - false - } else { - adapter.use_streaming(other_bot_present) - }; + // Decide streaming explicitly by platform, not by whatever the unified + // adapter's Telegram flag happens to be. ACP streams append-only deltas + // (not cosmetic edits); Google Chat / LINE can't sustain in-place edits. + // See `resolve_streaming`. + let streaming = resolve_streaming( + &thread_channel.platform, + adapter.use_streaming(other_bot_present), + ); // Keep the full turn text (incl. inter-tool narration) when streaming // (it was already shown live) OR when `[reactions] narration_display` is // set. Otherwise a send-once turn delivers only the final answer block. @@ -1746,6 +1761,20 @@ mod tests { assert_eq!(crate::format::split_message(&long, reply_message_limit("acp", 4096)).len(), 1); } + #[test] + fn resolve_streaming_forces_send_once_for_acp_and_googlechat() { + // Editable platforms honor the adapter's own streaming preference. + assert!(resolve_streaming("discord", true)); + assert!(!resolve_streaming("discord", false)); + assert!(resolve_streaming("telegram", true)); + // ACP streams append-only deltas, not cosmetic edits → always send-once. + assert!(!resolve_streaming("acp", true)); + // Google Chat: synthetic id can't be patched (400 INVALID_ARGUMENT) → send-once regardless of pref. + assert!(!resolve_streaming("googlechat", true)); + // LINE: no edit API → send-once. + assert!(!resolve_streaming("line", true)); + } + #[test] fn select_delivery_text_send_once_keeps_only_final_block() { // Simulates: narration "n1" → tool (answer_start→2) → narration "n2" diff --git a/crates/openab-core/src/config.rs b/crates/openab-core/src/config.rs index a9bc26abd..d52f4dce0 100644 --- a/crates/openab-core/src/config.rs +++ b/crates/openab-core/src/config.rs @@ -1225,6 +1225,12 @@ pub struct GoogleChatConfig { /// checked when `allow_all_users` resolves to `false`. Env fallback: /// `GOOGLE_CHAT_ALLOWED_USERS` (comma-separated). pub allowed_users: Option>, + /// Use keyless ADC (GCE metadata server + IAM Credentials + /// `generateAccessToken` self-impersonation) to mint the `chat.bot` token, + /// instead of a SA key file or a static token. Env fallback: + /// `GOOGLE_CHAT_USE_ADC` (`true`/`1`; default false). Ignored when a SA key + /// is configured — the SA key takes precedence. + pub use_adc: Option, } /// Fully resolved Google Chat settings (config → env → default applied). @@ -1234,6 +1240,7 @@ pub struct ResolvedGoogleChat { pub sa_key_json: Option, pub sa_key_file: Option, pub access_token: Option, + pub use_adc: bool, pub audience: Option, pub webhook_path: String, pub allow_all_users: bool, @@ -1259,6 +1266,11 @@ impl GoogleChatConfig { sa_key_json: opt_str(&self.sa_key_json, "GOOGLE_CHAT_SA_KEY_JSON"), sa_key_file: opt_str(&self.sa_key_file, "GOOGLE_CHAT_SA_KEY_FILE"), access_token: opt_str(&self.access_token, "GOOGLE_CHAT_ACCESS_TOKEN"), + use_adc: self.use_adc.unwrap_or_else(|| { + std::env::var("GOOGLE_CHAT_USE_ADC") + .map(|v| v == "true" || v == "1") + .unwrap_or(false) + }), audience: opt_str(&self.audience, "GOOGLE_CHAT_AUDIENCE"), webhook_path: opt_str(&self.webhook_path, "GOOGLE_CHAT_WEBHOOK_PATH") .unwrap_or_else(|| "/webhook/googlechat".into()), @@ -2966,6 +2978,7 @@ allowed_users = ["U1234567890abcdef0123456789abcdef"] "GOOGLE_CHAT_SA_KEY_JSON", "GOOGLE_CHAT_SA_KEY_FILE", "GOOGLE_CHAT_ACCESS_TOKEN", + "GOOGLE_CHAT_USE_ADC", "GOOGLE_CHAT_AUDIENCE", "GOOGLE_CHAT_WEBHOOK_PATH", ] { @@ -2974,9 +2987,18 @@ allowed_users = ["U1234567890abcdef0123456789abcdef"] // --- defaults --- let r = GoogleChatConfig::default().resolve(); assert!(!r.enabled); + assert!(!r.use_adc); assert!(r.audience.is_none()); assert_eq!(r.webhook_path, "/webhook/googlechat"); + // --- use_adc: config value resolves without touching env --- + let r = GoogleChatConfig { + use_adc: Some(true), + ..Default::default() + } + .resolve(); + assert!(r.use_adc); + // --- config wins over env --- std::env::set_var("GOOGLE_CHAT_ENABLED", "true"); std::env::set_var("GOOGLE_CHAT_AUDIENCE", "env-aud"); diff --git a/crates/openab-core/src/gateway.rs b/crates/openab-core/src/gateway.rs index a3b74adbd..0a39f0519 100644 --- a/crates/openab-core/src/gateway.rs +++ b/crates/openab-core/src/gateway.rs @@ -57,12 +57,23 @@ fn platform_acks_writes(platform: &str) -> bool { /// for a *capability*. The right long-term model is a capability handshake at /// gateway-connect time ("can this adapter edit messages?"); until that exists, /// any new gateway platform that lacks a message-edit API MUST be added here. -const NON_EDITABLE_PLATFORMS: &[&str] = &["line", "lineworks"]; +/// Platforms where cosmetic (typewriter) streaming — a placeholder message +/// then rapid in-place edits — is not viable, so replies are forced send-once: +/// - `line` / `lineworks`: no message-edit API at all. +/// - `googlechat`: has an edit API, but the unified adapter's synthetic +/// `unified_` message id is not a valid resource name, so `patch` +/// rejects it with 400 INVALID_ARGUMENT before any edit applies; the +/// documented 1 write/sec-per-space quota (create + patch + delete +/// combined) further constrains high-frequency editing. +/// See . +const NON_STREAMING_PLATFORMS: &[&str] = &["line", "lineworks", "googlechat"]; /// Whether cosmetic streaming (placeholder + in-place edits) is possible on -/// `platform`. See `NON_EDITABLE_PLATFORMS`. -fn platform_supports_streaming(platform: &str) -> bool { - !NON_EDITABLE_PLATFORMS.contains(&platform) +/// `platform`. See `NON_STREAMING_PLATFORMS`. `pub(crate)` so the shared +/// dispatch path (`AdapterRouter::stream_prompt_blocks`) can force send-once on +/// these platforms too, not just the WebSocket `run_gateway_adapter` path. +pub(crate) fn platform_supports_streaming(platform: &str) -> bool { + !NON_STREAMING_PLATFORMS.contains(&platform) } /// Shared filter parameters for gateway event gating. @@ -1669,17 +1680,18 @@ mod tests { assert!(!platform_supports_streaming("line")); } + #[test] + fn googlechat_rate_limit_forces_send_once() { + // Google Chat has an edit API, but the unified adapter's synthetic + // message id is not a valid resource name, so patch returns 400 + // INVALID_ARGUMENT; the documented 1 write/sec-per-space quota further + // constrains high-frequency editing. Force send-once. + assert!(!platform_supports_streaming("googlechat")); + } + #[test] fn editable_platforms_still_allow_streaming() { - for platform in [ - "telegram", - "slack", - "discord", - "feishu", - "teams", - "googlechat", - "wecom", - ] { + for platform in ["telegram", "slack", "discord", "feishu", "teams", "wecom"] { assert!( platform_supports_streaming(platform), "{platform} should still support streaming", diff --git a/crates/openab-gateway/src/adapters/googlechat.rs b/crates/openab-gateway/src/adapters/googlechat.rs index 12d274ee4..e7150f7f4 100644 --- a/crates/openab-gateway/src/adapters/googlechat.rs +++ b/crates/openab-gateway/src/adapters/googlechat.rs @@ -263,6 +263,7 @@ impl GoogleChatJwtVerifier { pub struct GoogleChatAdapter { pub token_cache: Option, + pub metadata_source: Option, pub access_token: Option, pub jwt_verifier: Option, pub client: reqwest::Client, @@ -271,15 +272,19 @@ pub struct GoogleChatAdapter { impl GoogleChatAdapter { /// Build an adapter from resolved parts (#1379): SA key JSON (inline wins - /// over file path), optional static access token, optional JWT audience. + /// over file path), optional static access token, optional JWT audience, + /// and `use_adc` (keyless ADC via the GCE metadata server + IAM + /// Credentials). Auth precedence at send time: SA key > ADC > static token. /// Shared by env-derived construction and `apply_googlechat_config`. pub(crate) fn from_parts( sa_key_json: Option, sa_key_file: Option, access_token: Option, audience: Option, + use_adc: bool, ) -> Self { use tracing::{info, warn}; + let key_configured = sa_key_json.is_some() || sa_key_file.is_some(); let token_cache = sa_key_json .or_else(|| { sa_key_file.and_then(|path| { @@ -299,7 +304,24 @@ impl GoogleChatAdapter { info!("googlechat webhook JWT verification enabled (audience={aud})"); GoogleChatJwtVerifier::new(aud) }); - Self::new(token_cache, access_token, jwt_verifier) + // Precedence at send time (see `get_token`): SA key > ADC > static token. + if use_adc { + if key_configured && token_cache.is_none() { + // A key WAS configured but failed to load. Don't switch identity + // silently: name it. get_token still falls to ADC, but the operator + // is told the bot now presents the workload identity, not the key's. + warn!( + "Google Chat SA key was configured but could not be loaded; \ + falling back to the keyless ADC (workload) identity — this is NOT \ + the configured key identity. Fix the key, or unset use_adc." + ); + } else if token_cache.is_none() { + info!("googlechat keyless ADC enabled (chat.bot via IAM Credentials)"); + } + } + let mut adapter = Self::new(token_cache, access_token, jwt_verifier); + adapter.metadata_source = use_adc.then(MetadataTokenSource::new); + adapter } pub fn new( @@ -309,6 +331,7 @@ impl GoogleChatAdapter { ) -> Self { Self { token_cache, + metadata_source: None, access_token, jwt_verifier, client: reqwest::Client::new(), @@ -326,6 +349,24 @@ impl GoogleChatAdapter { } } } + if let Some(ref src) = self.metadata_source { + match src.get_token().await { + Ok(t) => return Some(t), + Err(e) => { + // F2: fall through to a configured static token instead of + // dropping the reply. ADC and the static token are the same + // workload's own credential, so this is not an identity switch. + if self.access_token.is_some() { + error!( + "googlechat ADC token mint failed ({e}); \ + falling back to the configured static access_token" + ); + } else { + error!("googlechat ADC token mint failed: {e}"); + } + } + } + } self.access_token.clone() } @@ -806,21 +847,40 @@ impl GoogleChatTokenCache { { let guard = self.token.read().await; if let Some((ref tok, ref ts, ttl)) = *guard { - if ts.elapsed().as_secs() < ttl.saturating_sub(TOKEN_REFRESH_MARGIN_SECS) { + if ts.elapsed().as_secs() < refresh_threshold(ttl) { return Ok(tok.clone()); } } } let mut guard = self.token.write().await; if let Some((ref tok, ref ts, ttl)) = *guard { - if ts.elapsed().as_secs() < ttl.saturating_sub(TOKEN_REFRESH_MARGIN_SECS) { + if ts.elapsed().as_secs() < refresh_threshold(ttl) { return Ok(tok.clone()); } } - let (new_token, expire) = self.refresh(client).await?; - *guard = Some((new_token.clone(), Instant::now(), expire)); - info!("googlechat access token refreshed (expires in {expire}s)"); - Ok(new_token) + match self.refresh(client).await { + Ok((new_token, expire)) => { + *guard = Some((new_token.clone(), Instant::now(), expire)); + info!("googlechat access token refreshed (expires in {expire}s)"); + Ok(new_token) + } + Err(e) => { + // F4: serve the still-valid cached token on a transient exchange + // failure instead of dropping the reply. + if let Some((ref tok, ref ts, ttl)) = *guard { + let elapsed = ts.elapsed().as_secs(); + if elapsed < ttl { + warn!( + "googlechat token refresh failed ({e}); serving cached token \ + still valid for {}s", + ttl - elapsed + ); + return Ok(tok.clone()); + } + } + Err(e) + } + } } async fn refresh(&self, client: &reqwest::Client) -> Result<(String, u64), String> { @@ -882,6 +942,222 @@ impl GoogleChatTokenCache { } } +// --- Keyless ADC token source (GCE metadata → IAM Credentials) --- + +/// Google Chat's `chat.bot` scope for the minted token. +const ADC_CHAT_BOT_SCOPE: &str = "https://www.googleapis.com/auth/chat.bot"; +/// Lifetime we request from IAM Credentials for the impersonated token, and +/// the fallback TTL we cache it under. IAM caps impersonated tokens at 3600s. +const ADC_TOKEN_LIFETIME_SECS: u64 = 3600; + +/// Cache TTL (seconds) to use for a minted token, derived from the IAM +/// response's `expireTime`. Falls back to the full lifetime when the field is +/// missing/unparseable, and clamps to `[0, ADC_TOKEN_LIFETIME_SECS]` so an +/// org-policy-shortened token isn't cached past its real expiry (0 forces a +/// fresh mint next call rather than serving a dead token). +fn ttl_from_expire_time(expire_time: &str, now: chrono::DateTime) -> u64 { + match chrono::DateTime::parse_from_rfc3339(expire_time) { + Ok(exp) => (exp.with_timezone(&chrono::Utc) - now) + .num_seconds() + .clamp(0, ADC_TOKEN_LIFETIME_SECS as i64) as u64, + Err(_) => ADC_TOKEN_LIFETIME_SECS, + } +} + +/// Age (seconds) at which a cached token must be refreshed: its ttl minus a +/// margin, where the margin is capped at half the ttl. A fixed 300 s margin +/// would make `elapsed < ttl - 300` always false for any `ttl <= 300`, forcing +/// a re-mint on every single send; capping keeps short-lived tokens cacheable. +/// For `ttl = 0` the threshold is 0, so an expired token is never served from +/// the cache. +fn refresh_threshold(ttl: u64) -> u64 { + ttl.saturating_sub(TOKEN_REFRESH_MARGIN_SECS.min(ttl / 2)) +} + +/// Mints a `chat.bot`-scoped access token **without** a service-account key +/// file, using the workload's own identity (keyless ADC). Flow, per refresh: +/// 1. read the default SA's email + a base token from the GCE metadata server +/// 2. call IAM Credentials `generateAccessToken` (self-impersonation) to +/// exchange the base token for a `chat.bot`-scoped token +/// +/// Requires `roles/iam.serviceAccountTokenCreator` on the SA over itself and +/// the `iamcredentials.googleapis.com` API enabled. The `*_base` fields are +/// overridable so tests can point them at a mock server. +pub struct MetadataTokenSource { + token: RwLock>, + // Private (F5): only `new` (prod, fixed trusted hosts) or the in-module + // `with_bases` (tests, mock server) may set these. Unexported ⇒ no in-process + // caller can retarget the metadata bearer to an arbitrary host. + metadata_base: String, + iam_credentials_base: String, + // No-redirect client (F5): a redirect from either endpoint must never carry + // the metadata bearer (`Authorization`) on to a third host. + client: reqwest::Client, +} + +impl Default for MetadataTokenSource { + fn default() -> Self { + Self::new() + } +} + +impl MetadataTokenSource { + /// Production constructor: the fixed, trusted GCP endpoints (IAM over HTTPS). + pub fn new() -> Self { + Self::with_bases( + "http://metadata.google.internal".into(), + "https://iamcredentials.googleapis.com".into(), + ) + } + + /// Construct with explicit endpoint bases. Prod always goes through `new` + /// with HTTPS IAM; tests point these at a mock server. + fn with_bases(metadata_base: String, iam_credentials_base: String) -> Self { + Self { + token: RwLock::new(None), + metadata_base, + iam_credentials_base, + client: reqwest::Client::builder() + .redirect(reqwest::redirect::Policy::none()) + .build() + .unwrap_or_default(), + } + } + + /// Cached-or-refresh, mirroring [`GoogleChatTokenCache::get_token`]: + /// double-checked locking around the RwLock so only one refresh runs. + pub async fn get_token(&self) -> Result { + { + let guard = self.token.read().await; + if let Some((ref tok, ref ts, ttl)) = *guard { + if ts.elapsed().as_secs() < refresh_threshold(ttl) { + return Ok(tok.clone()); + } + } + } + let mut guard = self.token.write().await; + if let Some((ref tok, ref ts, ttl)) = *guard { + if ts.elapsed().as_secs() < refresh_threshold(ttl) { + return Ok(tok.clone()); + } + } + match self.refresh().await { + Ok((token, ttl)) => { + if ttl == 0 { + // Freshly minted but already at/after its expireTime — almost + // always local clock skew (Google validates against its own + // clock, so the token may still work). Serve it once, but do + // not cache a dead token: the next call re-mints. + warn!( + "googlechat ADC minted a token with ttl=0 (clock skew?); \ + serving once without caching" + ); + return Ok(token); + } + *guard = Some((token.clone(), Instant::now(), ttl)); + info!("googlechat ADC token minted (chat.bot, ttl {ttl}s)"); + Ok(token) + } + Err(e) => { + // F4: during a transient metadata/IAM failure, serve the cached + // token while it is still valid rather than dropping the reply. + if let Some((ref tok, ref ts, ttl)) = *guard { + let elapsed = ts.elapsed().as_secs(); + if elapsed < ttl { + warn!( + "googlechat ADC refresh failed ({e}); serving cached token \ + still valid for {}s", + ttl - elapsed + ); + return Ok(tok.clone()); + } + } + Err(e) + } + } + } + + async fn refresh(&self) -> Result<(String, u64), String> { + // Use the source's own no-redirect client for every bearer-carrying call. + let client = &self.client; + // 1. Default SA email from the GCE metadata server. + let email = client + .get(format!( + "{}/computeMetadata/v1/instance/service-accounts/default/email", + self.metadata_base + )) + .header("Metadata-Flavor", "Google") + .send() + .await + .map_err(|e| format!("metadata email request failed: {e}"))? + .error_for_status() + .map_err(|e| format!("metadata email status: {e}"))? + .text() + .await + .map_err(|e| format!("metadata email read failed: {e}"))?; + let email = email.trim(); + if email.is_empty() { + return Err("metadata returned empty SA email".into()); + } + + // 2. Base access token for the default SA from the metadata server. + let base: serde_json::Value = client + .get(format!( + "{}/computeMetadata/v1/instance/service-accounts/default/token", + self.metadata_base + )) + .header("Metadata-Flavor", "Google") + .send() + .await + .map_err(|e| format!("metadata token request failed: {e}"))? + .error_for_status() + .map_err(|e| format!("metadata token status: {e}"))? + .json() + .await + .map_err(|e| format!("metadata token parse failed: {e}"))?; + let base_token = base + .get("access_token") + .and_then(|v| v.as_str()) + .ok_or("metadata token response missing access_token")?; + + // 3. Exchange the base token for a chat.bot-scoped token via IAM + // Credentials generateAccessToken (the SA impersonates itself). + let url = format!( + "{}/v1/projects/-/serviceAccounts/{email}:generateAccessToken", + self.iam_credentials_base + ); + let resp: serde_json::Value = client + .post(&url) + .bearer_auth(base_token) + .json(&serde_json::json!({ + "scope": [ADC_CHAT_BOT_SCOPE], + "lifetime": format!("{ADC_TOKEN_LIFETIME_SECS}s"), + })) + .send() + .await + .map_err(|e| format!("generateAccessToken request failed: {e}"))? + .error_for_status() + .map_err(|e| format!("generateAccessToken status: {e}"))? + .json() + .await + .map_err(|e| format!("generateAccessToken parse failed: {e}"))?; + let token = resp + .get("accessToken") + .and_then(|v| v.as_str()) + .ok_or("generateAccessToken response missing accessToken")? + .to_string(); + // Cache under the server-granted lifetime (respects an org policy that + // shortens impersonated tokens below the requested 3600s), falling back + // to the full lifetime when expireTime is absent/unparseable. + let ttl = resp + .get("expireTime") + .and_then(|v| v.as_str()) + .map(|e| ttl_from_expire_time(e, chrono::Utc::now())) + .unwrap_or(ADC_TOKEN_LIFETIME_SECS); + Ok((token, ttl)) + } +} + /// Convert markdown to Google Chat native formatting. /// /// Called by both `send_message` and `edit_message`. Assumes the caller passes @@ -1811,6 +2087,165 @@ mod tests { assert!(result.is_ok()); } + // --- Keyless ADC (MetadataTokenSource) tests --- + + #[test] + fn ttl_from_expire_time_derives_and_clamps() { + use chrono::{DateTime, Utc}; + let now: DateTime = "2026-08-25T00:00:00Z".parse().unwrap(); + // Normal: 30 min out → 1800s. + assert_eq!(ttl_from_expire_time("2026-08-25T00:30:00Z", now), 1800); + // Beyond the 3600s cap → clamped to 3600. + assert_eq!(ttl_from_expire_time("2026-08-25T05:00:00Z", now), 3600); + // Already expired → 0 (forces refresh next call, never caches a dead token). + assert_eq!(ttl_from_expire_time("2026-08-24T23:00:00Z", now), 0); + // Unparseable → safe fallback to the full lifetime. + assert_eq!(ttl_from_expire_time("not-a-timestamp", now), 3600); + } + + #[tokio::test] + async fn metadata_token_source_mints_chat_bot_token() { + use wiremock::matchers::{header, method, path, path_regex}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + let server = MockServer::start().await; + // GCE metadata: default SA email. + Mock::given(method("GET")) + .and(path( + "/computeMetadata/v1/instance/service-accounts/default/email", + )) + .and(header("Metadata-Flavor", "Google")) + .respond_with( + ResponseTemplate::new(200) + .set_body_string("openab-host@dev-seba.iam.gserviceaccount.com"), + ) + .mount(&server) + .await; + // GCE metadata: base access token. + Mock::given(method("GET")) + .and(path( + "/computeMetadata/v1/instance/service-accounts/default/token", + )) + .and(header("Metadata-Flavor", "Google")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "access_token": "base-tok", + "expires_in": 3600, + "token_type": "Bearer" + }))) + .mount(&server) + .await; + // IAM Credentials: generateAccessToken (self-impersonation) → chat.bot token. + Mock::given(method("POST")) + .and(path_regex( + r"/v1/projects/-/serviceAccounts/.*:generateAccessToken", + )) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "accessToken": "chat-bot-tok", + "expireTime": "2099-01-01T00:00:00Z" + }))) + .mount(&server) + .await; + + let src = MetadataTokenSource::with_bases(server.uri(), server.uri()); + + let token = src + .get_token() + .await + .expect("should mint a chat.bot token"); + assert_eq!(token, "chat-bot-tok"); + } + + #[test] + fn from_parts_use_adc_toggles_metadata_source() { + let with = GoogleChatAdapter::from_parts(None, None, None, None, true); + assert!(with.metadata_source.is_some(), "use_adc=true → ADC source"); + let without = GoogleChatAdapter::from_parts(None, None, None, None, false); + assert!( + without.metadata_source.is_none(), + "use_adc=false → no ADC source" + ); + } + + #[test] + fn from_parts_malformed_key_with_use_adc_installs_adc() { + // F1 regression: a configured-but-malformed SA key parses to + // token_cache=None; with use_adc=true the ADC source is still installed + // (from_parts logs a warning naming the identity switch). + let adapter = + GoogleChatAdapter::from_parts(Some("not valid json".into()), None, None, None, true); + assert!( + adapter.token_cache.is_none(), + "malformed SA key → no SA-key cache" + ); + assert!( + adapter.metadata_source.is_some(), + "use_adc=true → ADC source installed even when a key was configured but failed to load" + ); + } + + #[test] + fn from_parts_unreadable_key_file_with_use_adc_installs_adc() { + // F1 regression: an unreadable/absent key FILE also yields token_cache=None + // and must not suppress ADC. + let adapter = GoogleChatAdapter::from_parts( + None, + Some("/nonexistent/path/sa-key.json".into()), + None, + None, + true, + ); + assert!(adapter.token_cache.is_none(), "unreadable key file → no cache"); + assert!( + adapter.metadata_source.is_some(), + "use_adc=true → ADC source installed when the key file could not be read" + ); + } + + #[tokio::test] + async fn adc_takes_precedence_over_static_access_token() { + use wiremock::matchers::{method, path, path_regex}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path( + "/computeMetadata/v1/instance/service-accounts/default/email", + )) + .respond_with( + ResponseTemplate::new(200) + .set_body_string("openab-host@dev-seba.iam.gserviceaccount.com"), + ) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path( + "/computeMetadata/v1/instance/service-accounts/default/token", + )) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "access_token": "base-tok", "expires_in": 3600 + }))) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path_regex( + r"/v1/projects/-/serviceAccounts/.*:generateAccessToken", + )) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "accessToken": "chat-bot-tok" + }))) + .mount(&server) + .await; + + // Adapter has BOTH an ADC source and a static token; ADC must win. + let mut adapter = + GoogleChatAdapter::from_parts(None, None, Some("static-tok".into()), None, true); + // Repoint the ADC source at the mock server (bases are private now). + adapter.metadata_source = + Some(MetadataTokenSource::with_bases(server.uri(), server.uri())); + let token = adapter.get_token().await.expect("a token"); + assert_eq!(token, "chat-bot-tok", "ADC should win over static token"); + } + // --- Bot filtering logic test --- #[test] diff --git a/crates/openab-gateway/src/lib.rs b/crates/openab-gateway/src/lib.rs index d2b257c7d..7f5c7bf26 100644 --- a/crates/openab-gateway/src/lib.rs +++ b/crates/openab-gateway/src/lib.rs @@ -199,6 +199,9 @@ impl AppState { std::env::var("GOOGLE_CHAT_SA_KEY_FILE").ok(), std::env::var("GOOGLE_CHAT_ACCESS_TOKEN").ok(), std::env::var("GOOGLE_CHAT_AUDIENCE").ok(), + std::env::var("GOOGLE_CHAT_USE_ADC") + .map(|v| v == "true" || v == "1") + .unwrap_or(false), )) } else { None @@ -454,6 +457,7 @@ impl AppState { cfg.sa_key_file, cfg.access_token, cfg.audience, + cfg.use_adc, )) } else { None @@ -554,6 +558,7 @@ pub struct GatewayGoogleChatConfig { pub sa_key_file: Option, pub access_token: Option, pub audience: Option, + pub use_adc: bool, pub webhook_path: String, } @@ -752,6 +757,9 @@ pub async fn serve(config: ServeConfig) -> anyhow::Result<()> { std::env::var("GOOGLE_CHAT_SA_KEY_FILE").ok(), std::env::var("GOOGLE_CHAT_ACCESS_TOKEN").ok(), std::env::var("GOOGLE_CHAT_AUDIENCE").ok(), + std::env::var("GOOGLE_CHAT_USE_ADC") + .map(|v| v == "true" || v == "1") + .unwrap_or(false), )) } else { None @@ -1249,6 +1257,7 @@ mod l1_audit_tests { sa_key_file: None, access_token: Some("tok".into()), audience: None, + use_adc: false, webhook_path: "/hook/gc".into(), }); assert!(s.google_chat.is_some()); @@ -1262,6 +1271,7 @@ mod l1_audit_tests { sa_key_file: None, access_token: Some("tok".into()), audience: Some("aud".into()), + use_adc: false, webhook_path: "/hook/gc".into(), }); assert!(flagged(&s).is_empty()); @@ -1273,6 +1283,7 @@ mod l1_audit_tests { sa_key_file: None, access_token: None, audience: None, + use_adc: false, webhook_path: "/hook/gc".into(), }); assert!(s.google_chat.is_none()); diff --git a/crates/openab-gateway/tests/config_first_conformance.rs b/crates/openab-gateway/tests/config_first_conformance.rs index eeefc8d8c..4c8f19877 100644 --- a/crates/openab-gateway/tests/config_first_conformance.rs +++ b/crates/openab-gateway/tests/config_first_conformance.rs @@ -92,6 +92,7 @@ const COVERED: &[&str] = &[ "GOOGLE_CHAT_SA_KEY_JSON", "GOOGLE_CHAT_SA_KEY_FILE", "GOOGLE_CHAT_ACCESS_TOKEN", + "GOOGLE_CHAT_USE_ADC", "GOOGLE_CHAT_AUDIENCE", "GOOGLE_CHAT_WEBHOOK_PATH", "GOOGLE_CHAT_ALLOW_ALL_USERS", diff --git a/docs/config-reference.md b/docs/config-reference.md index af30e0940..13d863c82 100644 --- a/docs/config-reference.md +++ b/docs/config-reference.md @@ -225,6 +225,7 @@ Full first-class Google Chat section (config-first parity, #1379) — credential | `sa_key_json` | string | — | Inline service-account key JSON (wins over `sa_key_file`). Env: `GOOGLE_CHAT_SA_KEY_JSON`. | | `sa_key_file` | string | — | Path to a service-account key file. Env: `GOOGLE_CHAT_SA_KEY_FILE`. | | `access_token` | string | — | Static access token alternative. Env: `GOOGLE_CHAT_ACCESS_TOKEN`. | +| `use_adc` | bool | `false` | Keyless ADC — mint the `chat.bot` token from the workload's own GCP identity (GCE metadata + IAM Credentials `generateAccessToken` self-impersonation); no SA key file. Needs `roles/iam.serviceAccountTokenCreator` on the SA over itself + `iamcredentials.googleapis.com`. Ignored when a SA key is set. Env: `GOOGLE_CHAT_USE_ADC`. | | `audience` | string | — | JWT audience — enables webhook JWT verification (L1). Env: `GOOGLE_CHAT_AUDIENCE`. | | `webhook_path` | string | `/webhook/googlechat` | Env: `GOOGLE_CHAT_WEBHOOK_PATH`. | | `allow_all_users` | bool \| omit | `false` (deny-all) | Env: `GOOGLE_CHAT_ALLOW_ALL_USERS`. | diff --git a/docs/google-chat.md b/docs/google-chat.md index dcee33d0e..6cb2f1ee8 100644 --- a/docs/google-chat.md +++ b/docs/google-chat.md @@ -74,7 +74,7 @@ Google Chat uses a service account to authenticate outbound API calls (bot repli ## 3. Configure the Gateway -The gateway supports two authentication methods for sending replies: +The gateway supports three authentication methods for sending replies: ### Option A: Service Account Key (recommended — auto-refresh) @@ -112,6 +112,29 @@ docker run -d --name openab-gateway \ ghcr.io/openabdev/openab-gateway:latest ``` +### Option C: Keyless ADC (recommended on GCP — no key file) + +When the gateway runs on GCP (GKE / GCE / Cloud Run) **as** a service account, it can mint the `chat.bot` token from that identity — no service-account key file to mount, manage, or leak. The gateway reads a base token from the GCE metadata server and calls IAM Credentials `generateAccessToken` (the SA impersonates itself) for a `chat.bot`-scoped token, then caches and auto-refreshes it. + +Prerequisites: + +- The workload's service account **is** the Chat app's service account (the identity that sends must be the space member). +- Grant that SA `roles/iam.serviceAccountTokenCreator` **on itself**. +- Enable `iamcredentials.googleapis.com`. +- The **base token from the metadata server must carry `cloud-platform` (or `.../auth/iam`) scope** — `generateAccessToken` requires it on the caller. GKE Workload Identity and Cloud Run tokens are `cloud-platform`-scoped and satisfy this automatically. A **default-scope GCE VM does not**: it returns `403 PERMISSION_DENIED: "Request had insufficient authentication scopes."` even when the IAM binding above is correct — match on the word *scopes* (not *permission*) to tell it apart from a missing role. GCE access scopes are **immutable after creation**: create the VM with `--scopes=cloud-platform`, or run `gcloud compute instances set-scopes --scopes=cloud-platform` followed by a stop/start. + +> Why self-impersonation (not plain ADC): the `chat.bot` scope is a Workspace scope and is **not** a subset of `cloud-platform`, so no `?scopes=` parameter on the metadata token can produce it. `generateAccessToken` is the only keyless way to obtain a `chat.bot` token. + +```bash +# The pod/VM already runs as the target service account; no key is mounted. +export GOOGLE_CHAT_ENABLED=true +export GOOGLE_CHAT_USE_ADC=true +``` + +Precedence: if a SA key (`GOOGLE_CHAT_SA_KEY_JSON` / `GOOGLE_CHAT_SA_KEY_FILE`) is also set **and loads successfully**, the SA key wins and ADC is ignored. If a key is configured but **fails to load** (unreadable file / malformed JSON), the adapter falls back to the keyless ADC (workload) identity and logs a warning naming the switch — fix the key, or unset `use_adc` to make that failure explicit. + +> **Migrating an existing release from a SA key to ADC:** the chart renders the Google Chat Secret only when `saKeyJson` / `accessToken` is set, and that Secret carries `helm.sh/resource-policy: keep`. Switching to ADC-only stops Helm from managing it but **leaves the old key material in the cluster indefinitely**. Delete the orphaned Secret after the switch (`kubectl delete secret `), otherwise the "no key to mount or leak" benefit is undercut. + ### Local development ```bash @@ -199,7 +222,7 @@ Each field falls back to its `GOOGLE_CHAT_ALLOW_ALL_USERS` / `GOOGLE_CHAT_ALLOWE - Links: `[text](url)` → `` - Inline code, fenced code blocks: pass through unchanged - Tables and other unsupported syntax pass through as-is -- **Streaming (edit_message)** — when OAB streaming is enabled, the bot edits its initial reply in-place as tokens arrive (typewriter effect) +- **Send-once (no streaming)** — Google Chat is a request/response REST surface, so the adapter posts the full reply once with no in-place editing. It is in `NON_STREAMING_PLATFORMS`; see `docs/platforms/schema/googlechat.toml` for why (the unified adapter's synthetic message id is not a valid resource name → `patch` returns `400 INVALID_ARGUMENT`, and the API documents a 1 write/sec-per-space quota). - **Inbound attachments** — image, text file, and audio attachments are downloaded via Google Chat Media API and stored to `~/.openab/media/inbound/` (colocate filesystem store): - Images: resized to ≤1200px JPEG (q75); GIFs preserved. Max 10 MB. - Text files: only known text extensions (`.txt`, `.md`, `.json`, `.py`, `.rs`, etc.). Max 512 KB. @@ -221,6 +244,7 @@ Each field falls back to its `GOOGLE_CHAT_ALLOW_ALL_USERS` / `GOOGLE_CHAT_ALLOWE | `GOOGLE_CHAT_SA_KEY_JSON` | No | — | Service account key JSON string (enables auto-refresh) | | `GOOGLE_CHAT_SA_KEY_FILE` | No | — | Path to service account key JSON file (alternative to `SA_KEY_JSON`) | | `GOOGLE_CHAT_ACCESS_TOKEN` | No | — | Static OAuth2 access token (fallback, expires in 1 hour) | +| `GOOGLE_CHAT_USE_ADC` | No | `false` | Keyless ADC auth via the GCE metadata server + IAM Credentials `generateAccessToken` self-impersonation (GCP-hosted only) — see Option C. Ignored when `SA_KEY_JSON`/`SA_KEY_FILE` is set | | `GOOGLE_CHAT_WEBHOOK_PATH` | No | `/webhook/googlechat` | Webhook endpoint path | ## Security: Webhook Verification diff --git a/docs/platforms/schema/googlechat.toml b/docs/platforms/schema/googlechat.toml index 375e62663..797a5f5e7 100644 --- a/docs/platforms/schema/googlechat.toml +++ b/docs/platforms/schema/googlechat.toml @@ -131,9 +131,9 @@ pr = "" [[openab_features]] feature = "streaming" -status = "partial" -note = "No native streaming API. Gateway adapter leaves uses_native_streaming=false, so core streams via the post-then-edit_message loop; each edit sends the full accumulated text as a patch call." -source = ["crates/openab-gateway/src/adapters/googlechat.rs#edit_message", "crates/openab-core/src/adapter.rs#uses_native_streaming"] +status = "not_implemented" +note = "Send-once by design (no cosmetic streaming). The decisive reason is structural: the unified adapter returns a synthetic `unified_` message id that is not a valid resource name, so `spaces.messages.patch` rejects it with 400 INVALID_ARGUMENT ('Missing or malformed message resource name') before any content is applied — per-token post-then-edit cannot work at all. Separately, Google Chat documents a 1 write/sec-per-space quota (create+patch+delete combined; https://developers.google.com/workspace/chat/limits); treat that as a documented constraint on high-frequency editing rather than an observed hard failure, since enforcement is burst-tolerant in practice. googlechat is therefore in NON_STREAMING_PLATFORMS, and `resolve_streaming` forces send-once on BOTH the embedded dispatch and the WebSocket gateway paths — matching Google Chat's documented send-once default. (Previously 'partial': core attempted post-then-edit, which failed on every edit.)" +source = ["crates/openab-core/src/gateway.rs#NON_STREAMING_PLATFORMS", "crates/openab-core/src/adapter.rs#resolve_streaming", "crates/openab-core/src/adapter.rs#uses_native_streaming"] pr = "" [[openab_features]] @@ -245,6 +245,14 @@ kind = "openab_decision" source = "crates/openab-gateway/src/adapters/googlechat.rs#build_jwt" refs = [] +[[quirks]] +date = "2026-08-25" +title = "Keyless ADC outbound path (no SA key)" +note = "Outbound creds have a third option beside the SA-key JWT-bearer exchange and the static token: keyless ADC (use_adc / GOOGLE_CHAT_USE_ADC). MetadataTokenSource reads the default SA email + a base token from the GCE metadata server, then calls IAM Credentials generateAccessToken (the SA impersonates itself) for a chat.bot-scoped token, cached with the same 300 s refresh margin. Meant for workloads that already run as the Chat app's service account on GCP — no key file to mount or leak. Requires roles/iam.serviceAccountTokenCreator on the SA over itself + iamcredentials.googleapis.com. Auth precedence in get_token: SA key (token_cache) > ADC (metadata_source) > static access_token." +kind = "openab_decision" +source = "crates/openab-gateway/src/adapters/googlechat.rs#MetadataTokenSource" +refs = [] + [[quirks]] date = "2026-07-04" title = "Reactions are structurally impossible for the bot" diff --git a/docs/platforms/schema/lineworks.toml b/docs/platforms/schema/lineworks.toml index 066944446..9b6cd013e 100644 --- a/docs/platforms/schema/lineworks.toml +++ b/docs/platforms/schema/lineworks.toml @@ -132,8 +132,8 @@ pr = "" [[openab_features]] feature = "streaming" status = "n_a" -note = "No edit API to drive post+edit streaming. The platform is listed in NON_EDITABLE_PLATFORMS so the core forces streaming off and the cosmetic edit/delete commands are dropped by the dispatcher." -source = ["crates/openab-core/src/gateway.rs#NON_EDITABLE_PLATFORMS", "crates/openab-gateway/src/adapters/lineworks.rs#dispatch_lineworks_reply"] +note = "No edit API to drive post+edit streaming. The platform is listed in NON_STREAMING_PLATFORMS so the core forces streaming off and the cosmetic edit/delete commands are dropped by the dispatcher." +source = ["crates/openab-core/src/gateway.rs#NON_STREAMING_PLATFORMS", "crates/openab-gateway/src/adapters/lineworks.rs#dispatch_lineworks_reply"] pr = "" [[openab_features]] diff --git a/src/main.rs b/src/main.rs index a2ee786ac..03f86b748 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1284,6 +1284,7 @@ async fn main() -> anyhow::Result<()> { sa_key_json: r.sa_key_json, sa_key_file: r.sa_key_file, access_token: r.access_token, + use_adc: r.use_adc, audience: r.audience, webhook_path: r.webhook_path, }); From bf6ee1ede0d7aeb78b863a1259cbab4e5fe9ea9c Mon Sep 17 00:00:00 2001 From: "chaodu-obk[bot]" <307341165+chaodu-obk[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 19:59:32 +0000 Subject: [PATCH 2/2] fix(googlechat): address review findings F15-F28 - F15: bound every token-mint request (SA-key exchange, metadata, IAM Credentials) with a 10s TOKEN_REQUEST_TIMEOUT so a hung connection cannot stall senders behind the cache write lock or defeat the ADC -> static token degradation path - F16: reject empty/whitespace minted tokens at all three extraction sites (SA-key exchange, metadata base token, generateAccessToken) so a malformed response follows the degradation path instead of being cached as valid - F17: correct the shorthand precedence wording in config.toml.example, config.rs, config-reference.md, google-chat.md env table, and values.yaml to name the configured-but-unloadable-key -> ADC fallback - F19: refuse edit_message for non-resource-name (synthetic unified_) ids locally instead of sending a doomed patch (400 INVALID_ARGUMENT) - F20: cross-reference the two sibling streaming gates (resolve_streaming / platform_supports_streaming) in both docs - F21: document get_token precedence and its asymmetric failure behavior at the function - F22: replace from_parts' five positional args with a named GoogleChatParts struct; all call sites and tests name their fields - F23: install metadata_source only when no SA key loaded, so the code encodes the precedence it documents - F24: drop private review-numbering labels (F1/F2/F4/F5) from source comments and test comments - F25: fix the self-contradictory 'immutable after creation' GCE scope wording in docs/google-chat.md Option C - F26: identify the orphaned Secret (agentFullname convention + discovery commands) in the key-to-ADC migration note - F27: log the resolved service-account identity on successful mint - F28: classify generateAccessToken failures (insufficient_scope / missing_role / api_not_enabled) in the error string New regression tests: loaded-key-suppresses-ADC-source, blank-minted- token rejection (wiremock), synthetic-id edit_message no-op (wiremock, expect(0)), and error-classification table. --- charts/openab/values.yaml | 2 +- config.toml.example | 2 +- crates/openab-core/src/adapter.rs | 5 +- crates/openab-core/src/config.rs | 6 +- crates/openab-core/src/gateway.rs | 6 + .../openab-gateway/src/adapters/googlechat.rs | 333 ++++++++++++++++-- crates/openab-gateway/src/lib.rs | 44 ++- docs/config-reference.md | 2 +- docs/google-chat.md | 6 +- 9 files changed, 339 insertions(+), 67 deletions(-) diff --git a/charts/openab/values.yaml b/charts/openab/values.yaml index 2258f0e54..0d7cee8d2 100644 --- a/charts/openab/values.yaml +++ b/charts/openab/values.yaml @@ -480,7 +480,7 @@ agents: audience: "" # JWT audience → GOOGLE_CHAT_AUDIENCE (set to your webhook URL to enable JWT verification) saKeyJson: "" # Service account key JSON string → GOOGLE_CHAT_SA_KEY_JSON (recommended, auto-refresh) accessToken: "" # Static OAuth2 access token → GOOGLE_CHAT_ACCESS_TOKEN (fallback, 1-hour TTL) - useAdc: false # Keyless ADC → GOOGLE_CHAT_USE_ADC. Mints chat.bot token from the pod's own GCP identity (GCE metadata + IAM Credentials generateAccessToken). No SA key file. Needs roles/iam.serviceAccountTokenCreator on the SA over itself + iamcredentials.googleapis.com enabled. Ignored when saKeyJson is set. + useAdc: false # Keyless ADC → GOOGLE_CHAT_USE_ADC. Mints chat.bot token from the pod's own GCP identity (GCE metadata + IAM Credentials generateAccessToken). No SA key file. Needs roles/iam.serviceAccountTokenCreator on the SA over itself + iamcredentials.googleapis.com enabled. Ignored when saKeyJson is set and loads; a key that fails to load falls back to ADC with a warning (see docs/google-chat.md Option C). webhookPath: "" # Gateway default: /webhook/googlechat → GOOGLE_CHAT_WEBHOOK_PATH # WeCom (企业微信) adapter config (gateway-side env vars) # See docs/wecom.md for full setup guide diff --git a/config.toml.example b/config.toml.example index 8c0fddb6c..a1de58cbe 100644 --- a/config.toml.example +++ b/config.toml.example @@ -123,7 +123,7 @@ allowed_channels = ["1234567890"] # ↑ omitted + non-empty list → auto- # sa_key_json = "${GOOGLE_CHAT_SA_KEY_JSON}" # inline SA key; wins over sa_key_file # sa_key_file = "/etc/openab/sa.json" # env fallback: GOOGLE_CHAT_SA_KEY_FILE # access_token = "${GOOGLE_CHAT_ACCESS_TOKEN}" # static token alternative -# use_adc = true # keyless ADC: mint chat.bot token from the workload's own GCP identity (GCE metadata + IAM Credentials); no SA key. env fallback: GOOGLE_CHAT_USE_ADC. Ignored when sa_key_* is set. +# use_adc = true # keyless ADC: mint chat.bot token from the workload's own GCP identity (GCE metadata + IAM Credentials); no SA key. env fallback: GOOGLE_CHAT_USE_ADC. Ignored when a configured sa_key_* loads; a key that fails to load falls back to ADC with a warning (docs/google-chat.md Option C). # audience = "projects//..." # enables webhook JWT verification (L1) # webhook_path = "/webhook/googlechat" # env fallback: GOOGLE_CHAT_WEBHOOK_PATH # allow_all_users = false # env fallback: GOOGLE_CHAT_ALLOW_ALL_USERS diff --git a/crates/openab-core/src/adapter.rs b/crates/openab-core/src/adapter.rs index e69d7cda2..2bf3a7c6d 100644 --- a/crates/openab-core/src/adapter.rs +++ b/crates/openab-core/src/adapter.rs @@ -44,7 +44,10 @@ fn reply_message_limit(platform: &str, adapter_limit: usize) -> usize { /// `platform_supports_streaming`. /// /// This is the embedded/unified dispatch gate; the WebSocket -/// `run_gateway_adapter` path applies the same `platform_supports_streaming` check. +/// `run_gateway_adapter` path applies the same `platform_supports_streaming` +/// check but has no `acp` case (ACP is embedded-only). The two gates are +/// siblings: adding a platform to `NON_STREAMING_PLATFORMS` covers both +/// paths, while an embedded-only carve-out belongs here. fn resolve_streaming(platform: &str, adapter_prefers_streaming: bool) -> bool { platform != "acp" && crate::gateway::platform_supports_streaming(platform) diff --git a/crates/openab-core/src/config.rs b/crates/openab-core/src/config.rs index d52f4dce0..eedce548a 100644 --- a/crates/openab-core/src/config.rs +++ b/crates/openab-core/src/config.rs @@ -1228,8 +1228,10 @@ pub struct GoogleChatConfig { /// Use keyless ADC (GCE metadata server + IAM Credentials /// `generateAccessToken` self-impersonation) to mint the `chat.bot` token, /// instead of a SA key file or a static token. Env fallback: - /// `GOOGLE_CHAT_USE_ADC` (`true`/`1`; default false). Ignored when a SA key - /// is configured — the SA key takes precedence. + /// `GOOGLE_CHAT_USE_ADC` (`true`/`1`; default false). Ignored when a + /// configured SA key loads successfully — the SA key takes precedence; a + /// key that fails to load falls back to ADC with a warning naming the + /// identity switch (see docs/google-chat.md Option C). pub use_adc: Option, } diff --git a/crates/openab-core/src/gateway.rs b/crates/openab-core/src/gateway.rs index 0a39f0519..970100edf 100644 --- a/crates/openab-core/src/gateway.rs +++ b/crates/openab-core/src/gateway.rs @@ -72,6 +72,12 @@ const NON_STREAMING_PLATFORMS: &[&str] = &["line", "lineworks", "googlechat"]; /// `platform`. See `NON_STREAMING_PLATFORMS`. `pub(crate)` so the shared /// dispatch path (`AdapterRouter::stream_prompt_blocks`) can force send-once on /// these platforms too, not just the WebSocket `run_gateway_adapter` path. +/// +/// Sibling gate: the embedded/unified dispatch path wraps this in +/// `adapter::resolve_streaming`, which additionally forces send-once for +/// `acp` (embedded-only, streams append-only deltas). An embedded-only +/// non-streaming platform must be handled there — adding it to the shared +/// list above covers both paths, but a platform-specific carve-out does not. pub(crate) fn platform_supports_streaming(platform: &str) -> bool { !NON_STREAMING_PLATFORMS.contains(&platform) } diff --git a/crates/openab-gateway/src/adapters/googlechat.rs b/crates/openab-gateway/src/adapters/googlechat.rs index e7150f7f4..c3385911b 100644 --- a/crates/openab-gateway/src/adapters/googlechat.rs +++ b/crates/openab-gateway/src/adapters/googlechat.rs @@ -21,6 +21,12 @@ const AUDIO_MAX_DOWNLOAD: u64 = 25 * 1024 * 1024; // 25 MB /// Per-request timeout for Google Chat Media API downloads. Prevents a hung /// connection from blocking the spawned download task indefinitely. const MEDIA_REQUEST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); + +/// Bound every token-mint request (SA-key exchange, metadata, IAM Credentials) +/// so a hung connection cannot stall senders queued behind the token cache's +/// write lock (the refresh runs while holding it) or prevent the ADC → static +/// token degradation path from engaging. +const TOKEN_REQUEST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); /// Cap on text file attachments per message (matches Discord/Slack). const TEXT_FILE_COUNT_CAP: usize = 5; /// Cap on aggregate text file bytes per message (matches Discord/Slack 1 MB). @@ -270,20 +276,32 @@ pub struct GoogleChatAdapter { pub api_base: String, } +/// Named construction parts for [`GoogleChatAdapter::from_parts`], so call +/// sites name each field instead of counting five positional arguments. +#[derive(Default)] +pub(crate) struct GoogleChatParts { + pub sa_key_json: Option, + pub sa_key_file: Option, + pub access_token: Option, + pub audience: Option, + pub use_adc: bool, +} + impl GoogleChatAdapter { /// Build an adapter from resolved parts (#1379): SA key JSON (inline wins /// over file path), optional static access token, optional JWT audience, /// and `use_adc` (keyless ADC via the GCE metadata server + IAM /// Credentials). Auth precedence at send time: SA key > ADC > static token. /// Shared by env-derived construction and `apply_googlechat_config`. - pub(crate) fn from_parts( - sa_key_json: Option, - sa_key_file: Option, - access_token: Option, - audience: Option, - use_adc: bool, - ) -> Self { + pub(crate) fn from_parts(parts: GoogleChatParts) -> Self { use tracing::{info, warn}; + let GoogleChatParts { + sa_key_json, + sa_key_file, + access_token, + audience, + use_adc, + } = parts; let key_configured = sa_key_json.is_some() || sa_key_file.is_some(); let token_cache = sa_key_json .or_else(|| { @@ -320,7 +338,12 @@ impl GoogleChatAdapter { } } let mut adapter = Self::new(token_cache, access_token, jwt_verifier); - adapter.metadata_source = use_adc.then(MetadataTokenSource::new); + // Install ADC only when it can actually be consulted: a loaded SA key + // wins outright at send time (see `get_token`), so don't construct a + // dead-at-runtime source behind it. A configured key that FAILED to + // load still installs ADC — that is the named fallback warned above. + adapter.metadata_source = + (use_adc && adapter.token_cache.is_none()).then(MetadataTokenSource::new); adapter } @@ -339,6 +362,14 @@ impl GoogleChatAdapter { } } + /// Resolve the outbound bearer token. Precedence: SA key (`token_cache`) + /// > ADC (`metadata_source`) > static `access_token`. + /// + /// Failure behavior is asymmetric by design: an SA-key exchange error + /// hard-fails (`None` — the operator explicitly configured that identity, + /// so nothing is silently substituted), while an ADC mint error falls + /// through to the static token (both are the same workload's credential, + /// so degrading is not an identity switch). async fn get_token(&self) -> Option { if let Some(ref cache) = self.token_cache { match cache.get_token(&self.client).await { @@ -353,7 +384,7 @@ impl GoogleChatAdapter { match src.get_token().await { Ok(t) => return Some(t), Err(e) => { - // F2: fall through to a configured static token instead of + // Fall through to a configured static token instead of // dropping the reply. ADC and the static token are the same // workload's own credential, so this is not an identity switch. if self.access_token.is_some() { @@ -407,6 +438,20 @@ impl GoogleChatAdapter { match reply.command.as_deref() { Some("add_reaction") | Some("remove_reaction") | Some("create_topic") => return, Some("edit_message") => { + // Google Chat is send-once (see core's `NON_STREAMING_PLATFORMS`): + // the unified adapter's synthetic `unified_` id is not a + // valid `spaces/*/messages/*` resource name, and `patch` rejects + // it with 400 INVALID_ARGUMENT before any edit applies. Refuse + // non-resource-name ids here instead of sending a doomed request, + // so a future caller cannot silently reintroduce that failure. + if !reply.reply_to.starts_with("spaces/") { + tracing::warn!( + reply_to = %reply.reply_to, + "googlechat edit_message ignored: not a message resource name \ + (synthetic ids cannot be patched)" + ); + return; + } self.edit_message(&reply.reply_to, &reply.content.text).await; return; } @@ -865,7 +910,7 @@ impl GoogleChatTokenCache { Ok(new_token) } Err(e) => { - // F4: serve the still-valid cached token on a transient exchange + // Serve the still-valid cached token on a transient exchange // failure instead of dropping the reply. if let Some((ref tok, ref ts, ttl)) = *guard { let elapsed = ts.elapsed().as_secs(); @@ -887,6 +932,7 @@ impl GoogleChatTokenCache { let jwt = self.build_jwt().map_err(|e| format!("JWT build error: {e}"))?; let resp = client .post("https://oauth2.googleapis.com/token") + .timeout(TOKEN_REQUEST_TIMEOUT) .form(&[ ("grant_type", "urn:ietf:params:oauth:grant-type:jwt-bearer"), ("assertion", &jwt), @@ -903,12 +949,15 @@ impl GoogleChatTokenCache { let token = body .get("access_token") .and_then(|v| v.as_str()) + // Boundary validation: an empty token would be cached as "valid" + // and fail every send with 401 until the refresh threshold. + .filter(|s| !s.trim().is_empty()) .ok_or_else(|| { let err = body .get("error_description") .and_then(|v| v.as_str()) .unwrap_or("unknown error"); - format!("token exchange failed: {err}") + format!("token exchange returned missing/empty access_token: {err}") })? .to_string(); @@ -985,12 +1034,12 @@ fn refresh_threshold(ttl: u64) -> u64 { /// overridable so tests can point them at a mock server. pub struct MetadataTokenSource { token: RwLock>, - // Private (F5): only `new` (prod, fixed trusted hosts) or the in-module + // Private: only `new` (prod, fixed trusted hosts) or the in-module // `with_bases` (tests, mock server) may set these. Unexported ⇒ no in-process // caller can retarget the metadata bearer to an arbitrary host. metadata_base: String, iam_credentials_base: String, - // No-redirect client (F5): a redirect from either endpoint must never carry + // No-redirect client: a redirect from either endpoint must never carry // the metadata bearer (`Authorization`) on to a third host. client: reqwest::Client, } @@ -1042,24 +1091,31 @@ impl MetadataTokenSource { } } match self.refresh().await { - Ok((token, ttl)) => { + Ok(minted) => { + let MintedToken { token, ttl, sa_email } = minted; if ttl == 0 { // Freshly minted but already at/after its expireTime — almost // always local clock skew (Google validates against its own // clock, so the token may still work). Serve it once, but do // not cache a dead token: the next call re-mints. warn!( + service_account = %sa_email, "googlechat ADC minted a token with ttl=0 (clock skew?); \ serving once without caching" ); return Ok(token); } *guard = Some((token.clone(), Instant::now(), ttl)); - info!("googlechat ADC token minted (chat.bot, ttl {ttl}s)"); + // Name the resolved identity so on-call can confirm which SA + // self-impersonation actually used when diagnosing send errors. + info!( + service_account = %sa_email, + "googlechat ADC token minted (chat.bot, ttl {ttl}s)" + ); Ok(token) } Err(e) => { - // F4: during a transient metadata/IAM failure, serve the cached + // During a transient metadata/IAM failure, serve the cached // token while it is still valid rather than dropping the reply. if let Some((ref tok, ref ts, ttl)) = *guard { let elapsed = ts.elapsed().as_secs(); @@ -1077,7 +1133,7 @@ impl MetadataTokenSource { } } - async fn refresh(&self) -> Result<(String, u64), String> { + async fn refresh(&self) -> Result { // Use the source's own no-redirect client for every bearer-carrying call. let client = &self.client; // 1. Default SA email from the GCE metadata server. @@ -1087,6 +1143,7 @@ impl MetadataTokenSource { self.metadata_base )) .header("Metadata-Flavor", "Google") + .timeout(TOKEN_REQUEST_TIMEOUT) .send() .await .map_err(|e| format!("metadata email request failed: {e}"))? @@ -1107,6 +1164,7 @@ impl MetadataTokenSource { self.metadata_base )) .header("Metadata-Flavor", "Google") + .timeout(TOKEN_REQUEST_TIMEOUT) .send() .await .map_err(|e| format!("metadata token request failed: {e}"))? @@ -1118,7 +1176,8 @@ impl MetadataTokenSource { let base_token = base .get("access_token") .and_then(|v| v.as_str()) - .ok_or("metadata token response missing access_token")?; + .filter(|s| !s.trim().is_empty()) + .ok_or("metadata token response missing or empty access_token")?; // 3. Exchange the base token for a chat.bot-scoped token via IAM // Credentials generateAccessToken (the SA impersonates itself). @@ -1126,25 +1185,45 @@ impl MetadataTokenSource { "{}/v1/projects/-/serviceAccounts/{email}:generateAccessToken", self.iam_credentials_base ); - let resp: serde_json::Value = client + let resp = client .post(&url) .bearer_auth(base_token) .json(&serde_json::json!({ "scope": [ADC_CHAT_BOT_SCOPE], "lifetime": format!("{ADC_TOKEN_LIFETIME_SECS}s"), })) + .timeout(TOKEN_REQUEST_TIMEOUT) .send() .await - .map_err(|e| format!("generateAccessToken request failed: {e}"))? - .error_for_status() - .map_err(|e| format!("generateAccessToken status: {e}"))? + .map_err(|e| format!("generateAccessToken request failed: {e}"))?; + let status = resp.status(); + if !status.is_success() { + // Classify the common GCP causes so on-call can act on the log + // line directly instead of decoding GCP error prose (mirrors the + // operator guidance in docs/google-chat.md Option C). + let body: String = resp + .text() + .await + .unwrap_or_default() + .chars() + .take(400) + .collect(); + let reason = classify_generate_access_token_error(status.as_u16(), &body); + return Err(format!( + "generateAccessToken failed (status {status}, reason={reason}): {body}" + )); + } + let resp: serde_json::Value = resp .json() .await .map_err(|e| format!("generateAccessToken parse failed: {e}"))?; let token = resp .get("accessToken") .and_then(|v| v.as_str()) - .ok_or("generateAccessToken response missing accessToken")? + // Boundary validation: an empty token would be cached as "valid" + // for its full TTL and bypass the static-token degradation path. + .filter(|s| !s.trim().is_empty()) + .ok_or("generateAccessToken response missing or empty accessToken")? .to_string(); // Cache under the server-granted lifetime (respects an org policy that // shortens impersonated tokens below the requested 3600s), falling back @@ -1154,7 +1233,37 @@ impl MetadataTokenSource { .and_then(|v| v.as_str()) .map(|e| ttl_from_expire_time(e, chrono::Utc::now())) .unwrap_or(ADC_TOKEN_LIFETIME_SECS); - Ok((token, ttl)) + Ok(MintedToken { + token, + ttl, + sa_email: email.to_string(), + }) + } +} + +/// A successfully minted ADC token plus the identity that minted it, so the +/// caller can log which service account self-impersonation resolved to. +struct MintedToken { + token: String, + ttl: u64, + sa_email: String, +} + +/// Best-effort classification of common GCP `generateAccessToken` failures. +/// Ordering matters: the insufficient-scope 403 body says "scopes" (not +/// "permission"), which is the documented signal distinguishing it from a +/// missing `roles/iam.serviceAccountTokenCreator` binding. +fn classify_generate_access_token_error(status: u16, body: &str) -> &'static str { + let b = body.to_ascii_lowercase(); + if b.contains("api has not been used") || b.contains("service_disabled") || b.contains("is disabled") + { + "api_not_enabled" + } else if status == 403 && b.contains("scopes") { + "insufficient_scope" + } else if status == 403 { + "missing_role" + } else { + "unclassified" } } @@ -2157,9 +2266,12 @@ mod tests { #[test] fn from_parts_use_adc_toggles_metadata_source() { - let with = GoogleChatAdapter::from_parts(None, None, None, None, true); + let with = GoogleChatAdapter::from_parts(GoogleChatParts { + use_adc: true, + ..Default::default() + }); assert!(with.metadata_source.is_some(), "use_adc=true → ADC source"); - let without = GoogleChatAdapter::from_parts(None, None, None, None, false); + let without = GoogleChatAdapter::from_parts(GoogleChatParts::default()); assert!( without.metadata_source.is_none(), "use_adc=false → no ADC source" @@ -2168,11 +2280,14 @@ mod tests { #[test] fn from_parts_malformed_key_with_use_adc_installs_adc() { - // F1 regression: a configured-but-malformed SA key parses to + // Regression: a configured-but-malformed SA key parses to // token_cache=None; with use_adc=true the ADC source is still installed // (from_parts logs a warning naming the identity switch). - let adapter = - GoogleChatAdapter::from_parts(Some("not valid json".into()), None, None, None, true); + let adapter = GoogleChatAdapter::from_parts(GoogleChatParts { + sa_key_json: Some("not valid json".into()), + use_adc: true, + ..Default::default() + }); assert!( adapter.token_cache.is_none(), "malformed SA key → no SA-key cache" @@ -2185,15 +2300,13 @@ mod tests { #[test] fn from_parts_unreadable_key_file_with_use_adc_installs_adc() { - // F1 regression: an unreadable/absent key FILE also yields token_cache=None + // Regression: an unreadable/absent key FILE also yields token_cache=None // and must not suppress ADC. - let adapter = GoogleChatAdapter::from_parts( - None, - Some("/nonexistent/path/sa-key.json".into()), - None, - None, - true, - ); + let adapter = GoogleChatAdapter::from_parts(GoogleChatParts { + sa_key_file: Some("/nonexistent/path/sa-key.json".into()), + use_adc: true, + ..Default::default() + }); assert!(adapter.token_cache.is_none(), "unreadable key file → no cache"); assert!( adapter.metadata_source.is_some(), @@ -2201,6 +2314,58 @@ mod tests { ); } + #[test] + fn from_parts_loaded_key_suppresses_metadata_source() { + // A successfully loaded SA key wins outright at send time, so no ADC + // source is installed behind it (dead-at-runtime otherwise). + let key = serde_json::json!({ + "client_email": "sa@example.iam.gserviceaccount.com", + "private_key": "-----BEGIN PRIVATE KEY-----\nnot-a-real-key\n-----END PRIVATE KEY-----\n", + }) + .to_string(); + let adapter = GoogleChatAdapter::from_parts(GoogleChatParts { + sa_key_json: Some(key), + use_adc: true, + ..Default::default() + }); + assert!(adapter.token_cache.is_some(), "valid key JSON → SA-key cache"); + assert!( + adapter.metadata_source.is_none(), + "loaded SA key → ADC source not installed" + ); + } + + #[test] + fn classify_generate_access_token_error_covers_documented_cases() { + // Insufficient scope: the 403 body says "scopes" — the documented + // signal distinguishing it from a missing IAM role. + assert_eq!( + classify_generate_access_token_error( + 403, + r#"{"error":{"status":"PERMISSION_DENIED","message":"Request had insufficient authentication scopes."}}"# + ), + "insufficient_scope" + ); + // Missing serviceAccountTokenCreator: 403 without the scopes wording. + assert_eq!( + classify_generate_access_token_error( + 403, + r#"{"error":{"status":"IAM_PERMISSION_DENIED","message":"Permission 'iam.serviceAccounts.getAccessToken' denied on resource"}}"# + ), + "missing_role" + ); + // IAM Credentials API not enabled. + assert_eq!( + classify_generate_access_token_error( + 403, + r#"{"error":{"message":"IAM Service Account Credentials API has not been used in project 123 before or it is disabled."}}"# + ), + "api_not_enabled" + ); + // Anything else stays unclassified rather than guessing. + assert_eq!(classify_generate_access_token_error(500, "boom"), "unclassified"); + } + #[tokio::test] async fn adc_takes_precedence_over_static_access_token() { use wiremock::matchers::{method, path, path_regex}; @@ -2237,8 +2402,11 @@ mod tests { .await; // Adapter has BOTH an ADC source and a static token; ADC must win. - let mut adapter = - GoogleChatAdapter::from_parts(None, None, Some("static-tok".into()), None, true); + let mut adapter = GoogleChatAdapter::from_parts(GoogleChatParts { + access_token: Some("static-tok".into()), + use_adc: true, + ..Default::default() + }); // Repoint the ADC source at the mock server (bases are private now). adapter.metadata_source = Some(MetadataTokenSource::with_bases(server.uri(), server.uri())); @@ -2246,6 +2414,93 @@ mod tests { assert_eq!(token, "chat-bot-tok", "ADC should win over static token"); } + #[tokio::test] + async fn metadata_token_source_rejects_blank_minted_token() { + use wiremock::matchers::{method, path, path_regex}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path( + "/computeMetadata/v1/instance/service-accounts/default/email", + )) + .respond_with( + ResponseTemplate::new(200) + .set_body_string("openab-host@dev-seba.iam.gserviceaccount.com"), + ) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path( + "/computeMetadata/v1/instance/service-accounts/default/token", + )) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "access_token": "base-tok", "expires_in": 3600 + }))) + .mount(&server) + .await; + // A malformed IAM response with an empty accessToken must surface as a + // mint error (and thus follow the static-token degradation path), not + // be cached as a "valid" credential. + Mock::given(method("POST")) + .and(path_regex( + r"/v1/projects/-/serviceAccounts/.*:generateAccessToken", + )) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "accessToken": "" + }))) + .mount(&server) + .await; + + let src = MetadataTokenSource::with_bases(server.uri(), server.uri()); + let err = src.get_token().await.expect_err("blank token must be rejected"); + assert!( + err.contains("missing or empty accessToken"), + "unexpected error: {err}" + ); + } + + #[tokio::test] + async fn handle_reply_edit_message_ignores_synthetic_unified_id() { + use wiremock::matchers::method; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + // Expect ZERO requests: a synthetic `unified_` id is not a valid + // message resource name, so the edit must be refused locally instead + // of being sent to the API (which would 400 INVALID_ARGUMENT). + let mock_server = MockServer::start().await; + Mock::given(method("PATCH")) + .respond_with(ResponseTemplate::new(200)) + .expect(0) + .mount(&mock_server) + .await; + + let (event_tx, _event_rx) = tokio::sync::broadcast::channel::(16); + let mut adapter = GoogleChatAdapter::new(None, Some("fake-token".into()), None); + adapter.api_base = mock_server.uri(); + + let reply = GatewayReply { + schema: "openab.gateway.reply.v1".into(), + reply_to: "unified_a1b2c3d4e5f6".into(), + platform: "googlechat".into(), + channel: ReplyChannel { + id: "spaces/SP".into(), + thread_id: None, + }, + content: Content { + content_type: "text".into(), + attachments: Vec::new(), + text: "updated text".into(), + }, + command: Some("edit_message".into()), + request_id: None, + quote_message_id: None, + }; + + adapter.handle_reply(&reply, &event_tx).await; + // MockServer verifies the expect(0) on drop. + } + // --- Bot filtering logic test --- #[test] diff --git a/crates/openab-gateway/src/lib.rs b/crates/openab-gateway/src/lib.rs index 7f5c7bf26..92799eaa2 100644 --- a/crates/openab-gateway/src/lib.rs +++ b/crates/openab-gateway/src/lib.rs @@ -195,13 +195,15 @@ impl AppState { .unwrap_or(false); if enabled { Some(adapters::googlechat::GoogleChatAdapter::from_parts( - std::env::var("GOOGLE_CHAT_SA_KEY_JSON").ok(), - std::env::var("GOOGLE_CHAT_SA_KEY_FILE").ok(), - std::env::var("GOOGLE_CHAT_ACCESS_TOKEN").ok(), - std::env::var("GOOGLE_CHAT_AUDIENCE").ok(), - std::env::var("GOOGLE_CHAT_USE_ADC") - .map(|v| v == "true" || v == "1") - .unwrap_or(false), + adapters::googlechat::GoogleChatParts { + sa_key_json: std::env::var("GOOGLE_CHAT_SA_KEY_JSON").ok(), + sa_key_file: std::env::var("GOOGLE_CHAT_SA_KEY_FILE").ok(), + access_token: std::env::var("GOOGLE_CHAT_ACCESS_TOKEN").ok(), + audience: std::env::var("GOOGLE_CHAT_AUDIENCE").ok(), + use_adc: std::env::var("GOOGLE_CHAT_USE_ADC") + .map(|v| v == "true" || v == "1") + .unwrap_or(false), + }, )) } else { None @@ -453,11 +455,13 @@ impl AppState { self.googlechat_webhook_path = cfg.webhook_path; self.google_chat = if cfg.enabled { Some(adapters::googlechat::GoogleChatAdapter::from_parts( - cfg.sa_key_json, - cfg.sa_key_file, - cfg.access_token, - cfg.audience, - cfg.use_adc, + adapters::googlechat::GoogleChatParts { + sa_key_json: cfg.sa_key_json, + sa_key_file: cfg.sa_key_file, + access_token: cfg.access_token, + audience: cfg.audience, + use_adc: cfg.use_adc, + }, )) } else { None @@ -753,13 +757,15 @@ pub async fn serve(config: ServeConfig) -> anyhow::Result<()> { info!(path = %googlechat_webhook_path, "googlechat adapter enabled"); app = app.route(&googlechat_webhook_path, post(adapters::googlechat::webhook)); Some(adapters::googlechat::GoogleChatAdapter::from_parts( - std::env::var("GOOGLE_CHAT_SA_KEY_JSON").ok(), - std::env::var("GOOGLE_CHAT_SA_KEY_FILE").ok(), - std::env::var("GOOGLE_CHAT_ACCESS_TOKEN").ok(), - std::env::var("GOOGLE_CHAT_AUDIENCE").ok(), - std::env::var("GOOGLE_CHAT_USE_ADC") - .map(|v| v == "true" || v == "1") - .unwrap_or(false), + adapters::googlechat::GoogleChatParts { + sa_key_json: std::env::var("GOOGLE_CHAT_SA_KEY_JSON").ok(), + sa_key_file: std::env::var("GOOGLE_CHAT_SA_KEY_FILE").ok(), + access_token: std::env::var("GOOGLE_CHAT_ACCESS_TOKEN").ok(), + audience: std::env::var("GOOGLE_CHAT_AUDIENCE").ok(), + use_adc: std::env::var("GOOGLE_CHAT_USE_ADC") + .map(|v| v == "true" || v == "1") + .unwrap_or(false), + }, )) } else { None diff --git a/docs/config-reference.md b/docs/config-reference.md index 13d863c82..a8820d88d 100644 --- a/docs/config-reference.md +++ b/docs/config-reference.md @@ -225,7 +225,7 @@ Full first-class Google Chat section (config-first parity, #1379) — credential | `sa_key_json` | string | — | Inline service-account key JSON (wins over `sa_key_file`). Env: `GOOGLE_CHAT_SA_KEY_JSON`. | | `sa_key_file` | string | — | Path to a service-account key file. Env: `GOOGLE_CHAT_SA_KEY_FILE`. | | `access_token` | string | — | Static access token alternative. Env: `GOOGLE_CHAT_ACCESS_TOKEN`. | -| `use_adc` | bool | `false` | Keyless ADC — mint the `chat.bot` token from the workload's own GCP identity (GCE metadata + IAM Credentials `generateAccessToken` self-impersonation); no SA key file. Needs `roles/iam.serviceAccountTokenCreator` on the SA over itself + `iamcredentials.googleapis.com`. Ignored when a SA key is set. Env: `GOOGLE_CHAT_USE_ADC`. | +| `use_adc` | bool | `false` | Keyless ADC — mint the `chat.bot` token from the workload's own GCP identity (GCE metadata + IAM Credentials `generateAccessToken` self-impersonation); no SA key file. Needs `roles/iam.serviceAccountTokenCreator` on the SA over itself + `iamcredentials.googleapis.com`. Ignored when a SA key is set and loads successfully; a configured key that fails to load falls back to ADC with a warning (see `docs/google-chat.md` Option C). Env: `GOOGLE_CHAT_USE_ADC`. | | `audience` | string | — | JWT audience — enables webhook JWT verification (L1). Env: `GOOGLE_CHAT_AUDIENCE`. | | `webhook_path` | string | `/webhook/googlechat` | Env: `GOOGLE_CHAT_WEBHOOK_PATH`. | | `allow_all_users` | bool \| omit | `false` (deny-all) | Env: `GOOGLE_CHAT_ALLOW_ALL_USERS`. | diff --git a/docs/google-chat.md b/docs/google-chat.md index 6cb2f1ee8..1823562d7 100644 --- a/docs/google-chat.md +++ b/docs/google-chat.md @@ -121,7 +121,7 @@ Prerequisites: - The workload's service account **is** the Chat app's service account (the identity that sends must be the space member). - Grant that SA `roles/iam.serviceAccountTokenCreator` **on itself**. - Enable `iamcredentials.googleapis.com`. -- The **base token from the metadata server must carry `cloud-platform` (or `.../auth/iam`) scope** — `generateAccessToken` requires it on the caller. GKE Workload Identity and Cloud Run tokens are `cloud-platform`-scoped and satisfy this automatically. A **default-scope GCE VM does not**: it returns `403 PERMISSION_DENIED: "Request had insufficient authentication scopes."` even when the IAM binding above is correct — match on the word *scopes* (not *permission*) to tell it apart from a missing role. GCE access scopes are **immutable after creation**: create the VM with `--scopes=cloud-platform`, or run `gcloud compute instances set-scopes --scopes=cloud-platform` followed by a stop/start. +- The **base token from the metadata server must carry `cloud-platform` (or `.../auth/iam`) scope** — `generateAccessToken` requires it on the caller. GKE Workload Identity and Cloud Run tokens are `cloud-platform`-scoped and satisfy this automatically. A **default-scope GCE VM does not**: it returns `403 PERMISSION_DENIED: "Request had insufficient authentication scopes."` even when the IAM binding above is correct — match on the word *scopes* (not *permission*) to tell it apart from a missing role. GCE access scopes **cannot be changed while the VM is running**: create the VM with `--scopes=cloud-platform`, or run `gcloud compute instances set-scopes --scopes=cloud-platform` followed by a stop/start. > Why self-impersonation (not plain ADC): the `chat.bot` scope is a Workspace scope and is **not** a subset of `cloud-platform`, so no `?scopes=` parameter on the metadata token can produce it. `generateAccessToken` is the only keyless way to obtain a `chat.bot` token. @@ -133,7 +133,7 @@ export GOOGLE_CHAT_USE_ADC=true Precedence: if a SA key (`GOOGLE_CHAT_SA_KEY_JSON` / `GOOGLE_CHAT_SA_KEY_FILE`) is also set **and loads successfully**, the SA key wins and ADC is ignored. If a key is configured but **fails to load** (unreadable file / malformed JSON), the adapter falls back to the keyless ADC (workload) identity and logs a warning naming the switch — fix the key, or unset `use_adc` to make that failure explicit. -> **Migrating an existing release from a SA key to ADC:** the chart renders the Google Chat Secret only when `saKeyJson` / `accessToken` is set, and that Secret carries `helm.sh/resource-policy: keep`. Switching to ADC-only stops Helm from managing it but **leaves the old key material in the cluster indefinitely**. Delete the orphaned Secret after the switch (`kubectl delete secret `), otherwise the "no key to mount or leak" benefit is undercut. +> **Migrating an existing release from a SA key to ADC:** the chart renders the Google Chat Secret only when `saKeyJson` / `accessToken` is set, and that Secret carries `helm.sh/resource-policy: keep`. Switching to ADC-only stops Helm from managing it but **leaves the old key material in the cluster indefinitely**. Delete the orphaned Secret after the switch: it is the gateway Secret named by the chart's `openab.agentFullname` helper — `--gateway` by default (or the agent's `nameOverride`) — and it contains the `google-chat-sa-key-json` key. Find it with `kubectl get secrets -o name | grep gateway`, confirm with `kubectl get secret -o jsonpath='{.data}' | grep -o google-chat-sa-key-json`, then `kubectl delete secret `. Otherwise the "no key to mount or leak" benefit is undercut. ### Local development @@ -244,7 +244,7 @@ Each field falls back to its `GOOGLE_CHAT_ALLOW_ALL_USERS` / `GOOGLE_CHAT_ALLOWE | `GOOGLE_CHAT_SA_KEY_JSON` | No | — | Service account key JSON string (enables auto-refresh) | | `GOOGLE_CHAT_SA_KEY_FILE` | No | — | Path to service account key JSON file (alternative to `SA_KEY_JSON`) | | `GOOGLE_CHAT_ACCESS_TOKEN` | No | — | Static OAuth2 access token (fallback, expires in 1 hour) | -| `GOOGLE_CHAT_USE_ADC` | No | `false` | Keyless ADC auth via the GCE metadata server + IAM Credentials `generateAccessToken` self-impersonation (GCP-hosted only) — see Option C. Ignored when `SA_KEY_JSON`/`SA_KEY_FILE` is set | +| `GOOGLE_CHAT_USE_ADC` | No | `false` | Keyless ADC auth via the GCE metadata server + IAM Credentials `generateAccessToken` self-impersonation (GCP-hosted only) — see Option C. Ignored when `SA_KEY_JSON`/`SA_KEY_FILE` is set and loads; a configured key that fails to load falls back to ADC with a warning (Option C) | | `GOOGLE_CHAT_WEBHOOK_PATH` | No | `/webhook/googlechat` | Webhook endpoint path | ## Security: Webhook Verification