From d577cb2fe9ea4c7e4766acd09b2dcf8f225e4f84 Mon Sep 17 00:00:00 2001 From: Alix-007 Date: Sun, 28 Jun 2026 22:30:17 +0800 Subject: [PATCH] fix(nextcloud-talk): bound external send/reaction response reads to prevent OOM (#96031) * fix(nextcloud-talk): bound external send/reaction response reads to prevent OOM Nextcloud Talk talks to self-hosted servers whose HTTP responses are not trusted to be small. The send and reaction paths buffered three external bodies without any byte cap: - success JSON via await response.json() - send error text via await response.text() - reaction error text via await response.text() A hostile or misbehaving Nextcloud endpoint could stream an unbounded body (no content-length) into memory, pressuring or hanging the plugin/provider path. Cap success JSON at 16 MiB via readResponseWithLimit and collapse error bodies to an 8 KiB readResponseTextSnippet, cancelling the stream on overflow. The 'message sent but receipt JSON unreadable -> unknown' fallback is preserved (an over-limit body now also routes through the existing catch). This is the symmetric counterpart to the #95103/#95108 response-limit campaign, reusing the shared @openclaw/media-core helpers (newly re-exported from plugin-sdk/response-limit-runtime for plugin consumers). * fix(nextcloud-talk): bound error bodies via public readResponseTextLimited (no new plugin-SDK surface) Re-exporting readResponseTextSnippet from plugin-sdk/response-limit-runtime pushed the public plugin-SDK export count past its surface budget, failing plugin-sdk-surface-report.test.ts. Drop that re-export and instead bound the Nextcloud Talk send/reaction error bodies through the already-public readResponseTextLimited (openclaw/plugin-sdk/provider-http), collapsing the bounded 8 KiB prefix to a short, log-safe snippet locally. Behavior is unchanged for callers; no new plugin-SDK surface is introduced. Success JSON still reads through readResponseWithLimit (16 MiB cap). The committed bounded-response-reads Vitest suite continues to prove the caps hold against 17 MiB streamed bodies with no content-length. * fix(nextcloud-talk): reuse shared readProviderJsonResponse for send success JSON The send success receipt parsed JSON by hand via readResponseWithLimit + a local NEXTCLOUD_TALK_JSON_MAX_BYTES cap + JSON.parse(TextDecoder.decode(...)), duplicating the shared provider-http helper that the sibling room-info.ts and bot-preflight.ts already use. extensions/AGENTS.md forbids re-implementing shared helpers locally. Swap the hand-rolled block for the one-stop readProviderJsonResponse<{ ocs?: ... }>(response, "Nextcloud Talk send"), which reads through the same bounded reader and throws on overflow/malformed JSON, so the outer try/catch still keeps the "unknown" receipt and behavior is equivalent. The error path keeps readResponseTextLimited (text, not JSON). --- .../src/send.cfg-threading.test.ts | 126 ++++++++++++++++++ extensions/nextcloud-talk/src/send.ts | 45 ++++++- 2 files changed, 166 insertions(+), 5 deletions(-) diff --git a/extensions/nextcloud-talk/src/send.cfg-threading.test.ts b/extensions/nextcloud-talk/src/send.cfg-threading.test.ts index 529806d2084..4d06506a2ff 100644 --- a/extensions/nextcloud-talk/src/send.cfg-threading.test.ts +++ b/extensions/nextcloud-talk/src/send.cfg-threading.test.ts @@ -358,3 +358,129 @@ describe("nextcloud-talk send cfg threading", () => { ).rejects.toThrow("Nextcloud Talk reaction failed: 403 forbidden"); }); }); + +describe("nextcloud-talk send bounded response reads", () => { + const fetchMock = vi.fn(); + const account = { + accountId: "default", + baseUrl: "https://nextcloud.example.com", + secret: "secret-value", + }; + + // Builds a streaming body with NO content-length so only the streaming byte + // cap can stop it. `chunks` chunks of `chunkBytes` each => total may exceed cap. + function streamingResponse(params: { + status: number; + chunkBytes: number; + chunks: number; + contentType: string; + fill?: number; + }): Response { + let remaining = params.chunks; + const stream = new ReadableStream({ + pull(controller) { + if (remaining <= 0) { + controller.close(); + return; + } + remaining -= 1; + controller.enqueue(new Uint8Array(params.chunkBytes).fill(params.fill ?? 0x7b)); + }, + }); + return new Response(stream, { + status: params.status, + headers: { "content-type": params.contentType }, + }); + } + + beforeEach(() => { + vi.stubGlobal("fetch", fetchMock); + hoisted.mockFetchGuard.mockImplementation(async (p: { url: string; init?: RequestInit }) => { + const response = await globalThis.fetch(p.url, p.init); + return { response, release: async () => {}, finalUrl: p.url }; + }); + hoisted.resolveNextcloudTalkAccount.mockReset(); + hoisted.resolveNextcloudTalkAccount.mockReturnValue(account); + hoisted.record.mockReset(); + hoisted.generateNextcloudTalkSignature.mockClear(); + }); + + afterEach(() => { + fetchMock.mockReset(); + hoisted.mockFetchGuard.mockReset(); + vi.unstubAllGlobals(); + }); + + it("keeps the unknown receipt when a success body exceeds the JSON byte cap", async () => { + // 17 MiB streamed as 200-OK JSON with no content-length: over the 16 MiB cap. + fetchMock.mockResolvedValueOnce( + streamingResponse({ + status: 200, + chunkBytes: 1024 * 1024, + chunks: 17, + contentType: "application/json", + }), + ); + + const result = await sendMessageNextcloudTalk("room:abc", "hello", { + cfg: { source: "provided" }, + }); + + // Over-limit success body must not throw and must fall back to the unknown receipt. + expect(result.messageId).toBe("unknown"); + expect(result.timestamp).toBeUndefined(); + }); + + it("bounds an oversized error body into a short send-failure snippet", async () => { + fetchMock.mockResolvedValueOnce( + streamingResponse({ + status: 400, + chunkBytes: 1024 * 1024, + chunks: 17, + contentType: "text/plain", + }), + ); + + await expect( + sendMessageNextcloudTalk("room:abc", "hello", { cfg: { source: "provided" } }), + ).rejects.toThrow(/Nextcloud Talk: bad request/); + }); + + it("bounds an oversized reaction error body into a short snippet", async () => { + fetchMock.mockResolvedValueOnce( + streamingResponse({ + status: 500, + chunkBytes: 1024 * 1024, + chunks: 17, + contentType: "text/plain", + }), + ); + + let caught: unknown; + try { + await sendReactionNextcloudTalk("room:abc", "m-1", "👍", { cfg: { source: "provided" } }); + } catch (error) { + caught = error; + } + + expect(caught).toBeInstanceOf(Error); + // The collapsed snippet caps the message far below the streamed 17 MiB body. + expect((caught as Error).message.length).toBeLessThan(4_000); + }); + + it("still parses a normal small success body", async () => { + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ ocs: { data: { id: 99, timestamp: 1_700_000_000 } } }), { + status: 200, + headers: { "content-type": "application/json" }, + }), + ); + + const result = await sendMessageNextcloudTalk("room:abc", "hello", { + cfg: { source: "provided" }, + }); + + expect(result.messageId).toBe("99"); + expect(result.timestamp).toBe(1_700_000_000); + }); +}); diff --git a/extensions/nextcloud-talk/src/send.ts b/extensions/nextcloud-talk/src/send.ts index 8defeb3e2a1..6da4629cb76 100644 --- a/extensions/nextcloud-talk/src/send.ts +++ b/extensions/nextcloud-talk/src/send.ts @@ -1,5 +1,9 @@ // Nextcloud Talk plugin module implements send behavior. import { createMessageReceiptFromOutboundResults } from "openclaw/plugin-sdk/channel-outbound"; +import { + readProviderJsonResponse, + readResponseTextLimited, +} from "openclaw/plugin-sdk/provider-http"; import { stripNextcloudTalkTargetPrefix } from "./normalize.js"; import { convertMarkdownTables, @@ -13,6 +17,36 @@ import { } from "./send.runtime.js"; import type { CoreConfig, NextcloudTalkSendResult } from "./types.js"; +// Nextcloud Talk runs against self-hosted servers whose responses are not +// trusted to be small. Cap error bodies so a hostile or misbehaving endpoint +// cannot stream an unbounded body into memory. (Success JSON is bounded by the +// shared readProviderJsonResponse helper.) +const NEXTCLOUD_TALK_ERROR_SNIPPET_MAX_BYTES = 8 * 1024; +const NEXTCLOUD_TALK_ERROR_SNIPPET_MAX_CHARS = 200; + +/** Collapses whitespace and caps an error-body prefix to a short, log-safe snippet. */ +function collapseErrorSnippet(text: string): string { + const collapsed = text.replace(/\s+/g, " ").trim(); + if (collapsed.length > NEXTCLOUD_TALK_ERROR_SNIPPET_MAX_CHARS) { + return `${collapsed.slice(0, NEXTCLOUD_TALK_ERROR_SNIPPET_MAX_CHARS)}…`; + } + return collapsed; +} + +/** Reads a bounded, collapsed error-body snippet without buffering hostile responses. */ +async function readNextcloudTalkErrorSnippet(response: Response): Promise { + try { + // readResponseTextLimited caps the read at the byte budget and cancels the + // upstream stream once full, so a hostile endpoint cannot stream an + // unbounded body into memory. Collapse the bounded prefix locally to keep a + // short, log-safe error snippet (no new plugin SDK surface required). + const text = await readResponseTextLimited(response, NEXTCLOUD_TALK_ERROR_SNIPPET_MAX_BYTES); + return collapseErrorSnippet(text); + } catch { + return ""; + } +} + type NextcloudTalkSendOpts = { cfg: CoreConfig; baseUrl?: string; @@ -161,7 +195,7 @@ export async function sendMessageNextcloudTalk( try { if (!response.ok) { - const errorBody = await response.text().catch(() => ""); + const errorBody = await readNextcloudTalkErrorSnippet(response); const status = response.status; let errorMsg = `Nextcloud Talk send failed (${status})`; @@ -184,14 +218,14 @@ export async function sendMessageNextcloudTalk( let messageId = "unknown"; let timestamp: number | undefined; try { - const data = (await response.json()) as { + const data = await readProviderJsonResponse<{ ocs?: { data?: { id?: number | string; timestamp?: number; }; }; - }; + }>(response, "Nextcloud Talk send"); if (data.ocs?.data?.id != null) { messageId = String(data.ocs.data.id); } @@ -199,7 +233,8 @@ export async function sendMessageNextcloudTalk( timestamp = data.ocs.data.timestamp; } } catch { - // Response parsing failed, but message was sent. + // Response parsing failed (including an over-limit body), but the message + // was already accepted by the server, so keep the "unknown" receipt. } if (opts.verbose) { @@ -259,7 +294,7 @@ export async function sendReactionNextcloudTalk( try { if (!response.ok) { - const errorBody = await response.text().catch(() => ""); + const errorBody = await readNextcloudTalkErrorSnippet(response); throw new Error(`Nextcloud Talk reaction failed: ${response.status} ${errorBody}`.trim()); }