From 2d5dd6c03584988216c3f15e26644db9c152fef7 Mon Sep 17 00:00:00 2001 From: NIO Date: Thu, 9 Jul 2026 21:49:14 +0800 Subject: [PATCH] fix(googlechat): bound control and media requests (#102227) * fix(googlechat): add 30 s request timeout to withGoogleChatResponse * fix(googlechat): satisfy ResolvedGoogleChatAccount in api.test.ts * fix(googlechat): bound control and media requests Co-authored-by: NIO --------- Co-authored-by: NIO Co-authored-by: Peter Steinberger --- CHANGELOG.md | 1 + extensions/googlechat/src/api.ts | 56 ++++++++++-- extensions/googlechat/src/targets.test.ts | 104 ++++++++++++++++++++-- 3 files changed, 150 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74d9908736e..f07f466d677 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- **Google Chat request deadlines:** bound control calls to 30 seconds while giving media transfers size-aware total budgets and a separate 30-second stalled-body guard, preventing hung Chat API requests without breaking large attachment uploads. (#102227) Thanks @hugenshen. - **Browser actions on Node 24:** keep browser request cancellation bound to the client and response lifetime instead of Node 24.16+'s prematurely aborted body-stream signal, preventing valid POST actions from failing after JSON parsing. Thanks @obviyus and @vincentkoc. - **SecretRef model credentials:** keep resolved provider secrets behind process-local sentinels through auth storage, stream setup, SDK configuration, and managed local-provider probing, then inject plaintext only at the final network or provider-plugin boundary while retaining exact-value log redaction. (#102008, #102009) - **Lean local model shell access:** keep `exec` directly visible beside the default structured Tool Search controls so coding-tuned local models can use their shell fallback instead of searching for missing domain tools. (#87587) Thanks @vincentkoc. diff --git a/extensions/googlechat/src/api.ts b/extensions/googlechat/src/api.ts index 3ec7323fe6d..9a374d4a7c2 100644 --- a/extensions/googlechat/src/api.ts +++ b/extensions/googlechat/src/api.ts @@ -1,11 +1,10 @@ // Googlechat API module exposes the plugin public contract. import crypto from "node:crypto"; import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime"; -import { parseMediaContentLength } from "openclaw/plugin-sdk/media-runtime"; import { - readProviderJsonResponse, - readResponseTextLimited, -} from "openclaw/plugin-sdk/provider-http"; + parseMediaContentLength, + readResponseTextSnippet, +} from "openclaw/plugin-sdk/media-runtime"; import { readResponseWithLimit } from "openclaw/plugin-sdk/response-limit-runtime"; import { fetchWithSsrFGuard } from "openclaw/plugin-sdk/ssrf-runtime"; import type { ResolvedGoogleChatAccount } from "./accounts.js"; @@ -15,9 +14,46 @@ import type { GoogleChatCardV2, GoogleChatReaction, GoogleChatSpace } from "./ty const CHAT_API_BASE = "https://chat.googleapis.com/v1"; const CHAT_UPLOAD_BASE = "https://chat.googleapis.com/upload/v1"; +const GOOGLECHAT_API_TIMEOUT_MS = 30_000; +const GOOGLECHAT_MEDIA_TIMEOUT_GRACE_MS = 30_000; +const GOOGLECHAT_MEDIA_MIN_BYTES_PER_SECOND = 256 * 1024; +const GOOGLECHAT_MEDIA_MAX_TIMEOUT_MS = 15 * 60_000; +const GOOGLECHAT_RESPONSE_READ_IDLE_TIMEOUT_MS = 30_000; +const GOOGLECHAT_JSON_RESPONSE_MAX_BYTES = 16 * 1024 * 1024; +const GOOGLECHAT_ERROR_BODY_MAX_BYTES = 16 * 1024; + +function resolveGoogleChatMediaTimeoutMs(maxBytes?: number): number { + if (!maxBytes) { + return GOOGLECHAT_MEDIA_MAX_TIMEOUT_MS; + } + const transferMs = Math.ceil((maxBytes / GOOGLECHAT_MEDIA_MIN_BYTES_PER_SECOND) * 1000); + return Math.min(GOOGLECHAT_MEDIA_TIMEOUT_GRACE_MS + transferMs, GOOGLECHAT_MEDIA_MAX_TIMEOUT_MS); +} async function readGoogleChatJsonResponse(response: Response, label: string): Promise { - return readProviderJsonResponse(response, label); + const bytes = await readResponseWithLimit(response, GOOGLECHAT_JSON_RESPONSE_MAX_BYTES, { + chunkTimeoutMs: GOOGLECHAT_RESPONSE_READ_IDLE_TIMEOUT_MS, + onIdleTimeout: ({ chunkTimeoutMs }) => + new Error(`${label}: response body stalled after ${chunkTimeoutMs}ms`), + onOverflow: ({ maxBytes }) => new Error(`${label}: JSON response exceeds ${maxBytes} bytes`), + }); + try { + return JSON.parse(new TextDecoder().decode(bytes)) as T; + } catch (cause) { + throw new Error(`${label}: malformed JSON response`, { cause }); + } +} + +async function readGoogleChatErrorResponse(response: Response, label: string): Promise { + return ( + (await readResponseTextSnippet(response, { + maxBytes: GOOGLECHAT_ERROR_BODY_MAX_BYTES, + maxChars: GOOGLECHAT_ERROR_BODY_MAX_BYTES, + chunkTimeoutMs: GOOGLECHAT_RESPONSE_READ_IDLE_TIMEOUT_MS, + onIdleTimeout: ({ chunkTimeoutMs }) => + new Error(`${label} error response stalled after ${chunkTimeoutMs}ms`), + })) ?? "" + ); } const headersToObject = (headers?: HeadersInit): Record => @@ -33,6 +69,7 @@ async function withGoogleChatResponse(params: { init?: RequestInit; auditContext: string; errorPrefix?: string; + timeoutMs?: number; handleResponse: (response: Response) => Promise; }): Promise { const { @@ -41,6 +78,7 @@ async function withGoogleChatResponse(params: { init, auditContext, errorPrefix = "Google Chat API", + timeoutMs = GOOGLECHAT_API_TIMEOUT_MS, handleResponse, } = params; const token = await getGoogleChatAccessToken(account); @@ -54,10 +92,11 @@ async function withGoogleChatResponse(params: { }, }, auditContext, + timeoutMs, }); try { if (!response.ok) { - const text = await readResponseTextLimited(response).catch(() => ""); + const text = await readGoogleChatErrorResponse(response, errorPrefix); throw new Error(`${errorPrefix} ${response.status}: ${text || response.statusText}`); } return await handleResponse(response); @@ -112,6 +151,9 @@ async function fetchBuffer( url, init, auditContext: "googlechat.api.buffer", + // Media gets transfer time proportional to its accepted size, while a silent + // response body is still bounded independently below. + timeoutMs: resolveGoogleChatMediaTimeoutMs(options?.maxBytes), handleResponse: async (res) => { const maxBytes = options?.maxBytes; const lengthHeader = res.headers.get("content-length"); @@ -127,6 +169,7 @@ async function fetchBuffer( return { buffer, contentType }; } const buffer = await readResponseWithLimit(res, maxBytes, { + chunkTimeoutMs: GOOGLECHAT_RESPONSE_READ_IDLE_TIMEOUT_MS, onOverflow: () => new Error(`Google Chat media exceeds max bytes (${maxBytes})`), }); const contentType = res.headers.get("content-type") ?? undefined; @@ -255,6 +298,7 @@ export async function uploadGoogleChatAttachment(params: { }, auditContext: "googlechat.upload", errorPrefix: "Google Chat upload", + timeoutMs: resolveGoogleChatMediaTimeoutMs(body.length), handleResponse: async (response) => await readGoogleChatJsonResponse<{ attachmentDataRef?: { attachmentUploadToken?: string }; diff --git a/extensions/googlechat/src/targets.test.ts b/extensions/googlechat/src/targets.test.ts index 97e48307ba3..a079bdc5fd8 100644 --- a/extensions/googlechat/src/targets.test.ts +++ b/extensions/googlechat/src/targets.test.ts @@ -1,7 +1,12 @@ // Googlechat tests cover targets plugin behavior. import { afterAll, afterEach, describe, expect, it, vi } from "vitest"; import type { ResolvedGoogleChatAccount } from "./accounts.js"; -import { downloadGoogleChatMedia, sendGoogleChatMessage, updateGoogleChatMessage } from "./api.js"; +import { + downloadGoogleChatMedia, + sendGoogleChatMessage, + updateGoogleChatMessage, + uploadGoogleChatAttachment, +} from "./api.js"; import { clearGoogleChatApprovalCardBindingsForTest, registerGoogleChatManualApprovalFollowupSuppression, @@ -20,10 +25,12 @@ const mocks = vi.hoisted(() => ({ buildHostnameAllowlistPolicyFromSuffixAllowlist: vi.fn((hosts: string[]) => ({ hostnameAllowlist: hosts, })), - fetchWithSsrFGuard: vi.fn(async (params: { url: string; init?: RequestInit }) => ({ - response: await fetch(params.url, params.init), - release: async () => {}, - })), + fetchWithSsrFGuard: vi.fn( + async (params: { url: string; init?: RequestInit; timeoutMs?: number }) => ({ + response: await fetch(params.url, params.init), + release: async () => {}, + }), + ), googleAuthCtor: vi.fn(), gaxiosCtor: vi.fn(), getAccessToken: vi.fn().mockResolvedValue({ token: "access-token" }), @@ -115,6 +122,15 @@ function stubSuccessfulSend(name: string, threadName?: string) { return fetchMock; } +function createStalledResponse(status = 200): Response { + return new Response( + new ReadableStream({ + start() {}, + }), + { status }, + ); +} + async function expectDownloadToRejectForResponse( response: Response, expected: string | RegExp = /max bytes/i, @@ -133,6 +149,14 @@ function mockCallArg(mock: ReturnType, callIndex = 0, argIndex = 0 return call[argIndex]; } +function lastGuardedFetchOptions(): { timeoutMs?: number } { + const call = mocks.fetchWithSsrFGuard.mock.calls.at(-1); + if (!call) { + throw new Error("Expected guarded fetch call"); + } + return call[0] as { timeoutMs?: number }; +} + describe("normalizeGoogleChatTarget", () => { it("normalizes provider prefixes", () => { expect(normalizeGoogleChatTarget("googlechat:users/123")).toBe("users/123"); @@ -265,6 +289,7 @@ describe("downloadGoogleChatMedia", () => { authTesting.resetGoogleChatAuthForTests(); mocks.fetchWithSsrFGuard.mockClear(); vi.unstubAllGlobals(); + vi.useRealTimers(); }); it("rejects when content-length exceeds max bytes", async () => { @@ -279,6 +304,7 @@ describe("downloadGoogleChatMedia", () => { headers: { "content-length": "50", "content-type": "application/octet-stream" }, }); await expectDownloadToRejectForResponse(response); + expect(lastGuardedFetchOptions().timeoutMs).toBe(30_001); }); it("rejects malformed content-length before reading media", async () => { @@ -315,6 +341,73 @@ describe("downloadGoogleChatMedia", () => { }); await expectDownloadToRejectForResponse(response); }); + + it("cancels a media body that stops producing chunks", async () => { + vi.useFakeTimers(); + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(createStalledResponse())); + + const result = expect( + downloadGoogleChatMedia({ account, resourceName: "media/123", maxBytes: 10 }), + ).rejects.toThrow("Media download stalled: no data received for 30000ms"); + await vi.advanceTimersByTimeAsync(30_001); + await result; + }); + + it("cancels a stalled media error body", async () => { + vi.useFakeTimers(); + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(createStalledResponse(500))); + + const result = expect( + downloadGoogleChatMedia({ account, resourceName: "media/123", maxBytes: 10 }), + ).rejects.toThrow("Google Chat API error response stalled after 30000ms"); + await vi.advanceTimersByTimeAsync(30_001); + await result; + }); +}); + +describe("uploadGoogleChatAttachment", () => { + afterEach(() => { + authTesting.resetGoogleChatAuthForTests(); + mocks.fetchWithSsrFGuard.mockClear(); + vi.unstubAllGlobals(); + vi.useRealTimers(); + }); + + it("derives a bounded transfer deadline from the payload size", async () => { + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue( + new Response(JSON.stringify({ attachmentDataRef: { attachmentUploadToken: "token" } }), { + status: 200, + }), + ), + ); + + await uploadGoogleChatAttachment({ + account, + space: "spaces/AAA", + filename: "recording.wav", + buffer: Buffer.alloc(1024 * 1024), + }); + + expect(lastGuardedFetchOptions().timeoutMs).toBeGreaterThan(34_000); + }); + + it("cancels a stalled upload response body", async () => { + vi.useFakeTimers(); + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(createStalledResponse())); + + const result = expect( + uploadGoogleChatAttachment({ + account, + space: "spaces/AAA", + filename: "recording.wav", + buffer: Buffer.alloc(1024), + }), + ).rejects.toThrow("Google Chat upload failed: response body stalled after 30000ms"); + await vi.advanceTimersByTimeAsync(30_001); + await result; + }); }); describe("sendGoogleChatMessage", () => { @@ -350,6 +443,7 @@ describe("sendGoogleChatMessage", () => { messageName: "spaces/AAA/messages/123", threadName: "spaces/AAA/threads/xyz", }); + expect(lastGuardedFetchOptions().timeoutMs).toBe(30_000); }); it("does not set messageReplyOption for non-thread sends", async () => {