fix(mattermost): bound successful REST JSON/text response reads#96033
Conversation
The Mattermost REST client already bounds error bodies (readResponseTextLimited) and streams guarded responses without buffering, but the success path still called `await res.json()` / `await res.text()`, reading the whole body into memory before parsing. A self-hosted or compromised Mattermost server can return an arbitrarily large (or never-terminating, content-length-less) JSON/text body and force the plugin to buffer it unbounded. Read successful JSON through the shared readProviderJsonResponse (16 MiB cap, cancels the stream and throws a bounded error on overflow, same as the provider HTTP path) and cap non-JSON success bodies with readResponseTextLimited. uploadMattermostFile's file-info JSON is bounded the same way. Symmetric follow-up to the openclaw#95103 / openclaw#95108 response-limit campaign. AI-assisted.
|
@clawsweeper review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Codex review: needs maintainer review before merge. Reviewed June 26, 2026, 2:22 AM ET / 06:22 UTC. Summary PR surface: Source +23, Tests +93. Total +116 across 4 files. Reproducibility: yes. source-level. Current main and v2026.6.10 use unbounded res.json()/res.text() in Mattermost success paths, and the PR body provides terminal proof with a no-Content-Length streaming server; I did not run tests in this read-only review. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the bounded readers and regression tests if maintainers accept the Mattermost response-size caps; otherwise tune the caps before merge while preserving bounded cancellation. Do we have a high-confidence way to reproduce the issue? Yes, source-level. Current main and v2026.6.10 use unbounded res.json()/res.text() in Mattermost success paths, and the PR body provides terminal proof with a no-Content-Length streaming server; I did not run tests in this read-only review. Is this the best way to solve the issue? Yes, pending maintainer acceptance of the caps. The patch uses shared bounded readers in the client, upload, and probe paths and now rejects oversized text instead of truncating. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4fc504d321b6. Label changesLabel justifications:
Evidence reviewedPR surface: Source +23, Tests +93. Total +116 across 4 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
Addressed the ClawSweeper finding by bounding the sibling Mattermost probe success JSON path too. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Addressed the ClawSweeper rank-up move for Mattermost text fallback: oversized non-JSON 2xx responses now reject with a bounded error instead of returning a truncated prefix, and Verified current head:
Validation note: @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Merge-ready for 3161c88. Local and ClawSweeper review clean, current CI green, best narrow fix, and follows the established bounded provider-response-read pattern. The caps are acceptable because:
|
…claw#96033) * fix(mattermost): bound successful REST JSON/text response reads The Mattermost REST client already bounds error bodies (readResponseTextLimited) and streams guarded responses without buffering, but the success path still called `await res.json()` / `await res.text()`, reading the whole body into memory before parsing. A self-hosted or compromised Mattermost server can return an arbitrarily large (or never-terminating, content-length-less) JSON/text body and force the plugin to buffer it unbounded. Read successful JSON through the shared readProviderJsonResponse (16 MiB cap, cancels the stream and throws a bounded error on overflow, same as the provider HTTP path) and cap non-JSON success bodies with readResponseTextLimited. uploadMattermostFile's file-info JSON is bounded the same way. Symmetric follow-up to the openclaw#95103 / openclaw#95108 response-limit campaign. AI-assisted. * fix(mattermost): bound probe success JSON reads * fix(mattermost): reject oversized success text bodies
…claw#96033) * fix(mattermost): bound successful REST JSON/text response reads The Mattermost REST client already bounds error bodies (readResponseTextLimited) and streams guarded responses without buffering, but the success path still called `await res.json()` / `await res.text()`, reading the whole body into memory before parsing. A self-hosted or compromised Mattermost server can return an arbitrarily large (or never-terminating, content-length-less) JSON/text body and force the plugin to buffer it unbounded. Read successful JSON through the shared readProviderJsonResponse (16 MiB cap, cancels the stream and throws a bounded error on overflow, same as the provider HTTP path) and cap non-JSON success bodies with readResponseTextLimited. uploadMattermostFile's file-info JSON is bounded the same way. Symmetric follow-up to the openclaw#95103 / openclaw#95108 response-limit campaign. AI-assisted. * fix(mattermost): bound probe success JSON reads * fix(mattermost): reject oversized success text bodies
…claw#96033) * fix(mattermost): bound successful REST JSON/text response reads The Mattermost REST client already bounds error bodies (readResponseTextLimited) and streams guarded responses without buffering, but the success path still called `await res.json()` / `await res.text()`, reading the whole body into memory before parsing. A self-hosted or compromised Mattermost server can return an arbitrarily large (or never-terminating, content-length-less) JSON/text body and force the plugin to buffer it unbounded. Read successful JSON through the shared readProviderJsonResponse (16 MiB cap, cancels the stream and throws a bounded error on overflow, same as the provider HTTP path) and cap non-JSON success bodies with readResponseTextLimited. uploadMattermostFile's file-info JSON is bounded the same way. Symmetric follow-up to the openclaw#95103 / openclaw#95108 response-limit campaign. AI-assisted. * fix(mattermost): bound probe success JSON reads * fix(mattermost): reject oversized success text bodies
…claw#96033) * fix(mattermost): bound successful REST JSON/text response reads The Mattermost REST client already bounds error bodies (readResponseTextLimited) and streams guarded responses without buffering, but the success path still called `await res.json()` / `await res.text()`, reading the whole body into memory before parsing. A self-hosted or compromised Mattermost server can return an arbitrarily large (or never-terminating, content-length-less) JSON/text body and force the plugin to buffer it unbounded. Read successful JSON through the shared readProviderJsonResponse (16 MiB cap, cancels the stream and throws a bounded error on overflow, same as the provider HTTP path) and cap non-JSON success bodies with readResponseTextLimited. uploadMattermostFile's file-info JSON is bounded the same way. Symmetric follow-up to the openclaw#95103 / openclaw#95108 response-limit campaign. AI-assisted. * fix(mattermost): bound probe success JSON reads * fix(mattermost): reject oversized success text bodies (cherry picked from commit 9241b97)
…claw#96033) * fix(mattermost): bound successful REST JSON/text response reads The Mattermost REST client already bounds error bodies (readResponseTextLimited) and streams guarded responses without buffering, but the success path still called `await res.json()` / `await res.text()`, reading the whole body into memory before parsing. A self-hosted or compromised Mattermost server can return an arbitrarily large (or never-terminating, content-length-less) JSON/text body and force the plugin to buffer it unbounded. Read successful JSON through the shared readProviderJsonResponse (16 MiB cap, cancels the stream and throws a bounded error on overflow, same as the provider HTTP path) and cap non-JSON success bodies with readResponseTextLimited. uploadMattermostFile's file-info JSON is bounded the same way. Symmetric follow-up to the openclaw#95103 / openclaw#95108 response-limit campaign. AI-assisted. * fix(mattermost): bound probe success JSON reads * fix(mattermost): reject oversized success text bodies
What Problem This Solves
The Mattermost REST success path buffered untrusted server JSON/text responses without a size cap.
extensions/mattermost/src/mattermost/client.tstalks to Mattermost servers the plugin does not control: self-hosted instances, LAN deployments, or compromised/malfunctioning hosts. The client already hardened two of its three read paths:readResponseTextLimited(8 KiB), andresponseWithReleaseinstead ofarrayBuffer(),but the successful response path was still unbounded.
request()calledawait res.json()/await res.text(), anduploadMattermostFile()calledawait res.json()on the file-info result. Each of those buffers the entire body into memory before parsing.A Mattermost server can return a response with no
Content-Lengthand an arbitrarily large — or never-terminating — JSON/text body (every REST call goes throughrequest(): users, channels, posts, file uploads). Because nothing capped the success path, the plugin would buffer the whole stream, applying unbounded memory pressure / hang to the plugin and the surrounding agent runtime. This is not generic cleanup: it closes the one remaining unbounded ingest point on an untrusted-server boundary.Changes
request()JSON success bodies with sharedreadProviderJsonResponse(16 MiB cap; cancels the stream and throws a bounded… exceeds 16777216 byteserror on overflow, then wraps malformed JSON), matching the provider HTTP path used elsewhere in the repo.request()non-JSON success bodies at 64 KiB and reject on overflow instead of returning a silently truncated prefix (non-JSON success bodies are a rare fallback; the API is JSON-first).uploadMattermostFile()file-info JSON the same way throughreadProviderJsonResponse.readProviderJsonResponse,readResponseTextLimitedfromopenclaw/plugin-sdk/provider-http) with no new abstraction and no second buffering layer added to the guarded fetch adapter or the error path.Real behavior proof
Content-Lengthand a body larger than the cap is no longer buffered unbounded; the stream is cancelled at the cap and a bounded error is thrown. Normal small responses still parse intact.node:httpserver bound to127.0.0.1(noContent-Length), driving the real exportedcreateMattermostClient(...).request(...)anduploadMattermostFile(...)through the production guarded fetch path (fetchWithSsrFGuard+allowPrivateNetwork), undernode --import tsx.{-chunks forever withcontent-type: application/jsonand noContent-Length.client.request("/flood-json")is awaited; the server counts chunks actually written and tracks socket close.uploadMattermostFile(...)is pointed at the same flood endpoint.await res.text()and once via the new boundedrequest().JSON response exceeds 16777216 bytes, the server stops after writing ~17-19 MiB (reader cancelled just past the 16 MiB cap), and the socket is released. The negative control proves the cap is load-bearing: the oldawait res.text()drained and buffered the entire 41 MiB body (42,991,618 bytes) — i.e. it would have OOM'd on a truly unbounded body — while the new path cancelled at ~19 MiB.readMattermostError!res.bodyin-memory fallback branch (already bounded / not an unbounded network read).Evidence
node --import ./node_modules/tsx/dist/loader.mjs proof.mts→SUMMARY pass=7 fail=0(output above; harness not committed).node scripts/run-vitest.mjs extensions/mattermost/src/mattermost/client.test.ts --run→ 23 passed (includes the new "bounds and cancels oversized guarded Mattermost success JSON bodies" regression test).oxlint extensions/mattermost/src/mattermost/client.ts→ clean.tsgo:extensions(tsconfig.extensions.json, the CI extensions typecheck) → nomattermost/client.tserrors.check-extension-plugin-sdk-boundary.mjs --mode=plugin-sdk-internal→ no violations (imports are publicplugin-sdkentrypoints).This is the symmetric follow-up to the #95103 / #95108 response-limit campaign, applying the same bounded-stream discipline to the last unbounded read point in the Mattermost REST client's success path.
Refreshed current-head focused validation after fixing text overflow semantics:
Known validation note:
node scripts/run-tsgo.mjs -p extensions/mattermost/tsconfig.json --incremental falseruns in this worktree now, but fails on existing Mattermost SDK/API drift outside the touchedclient.ts/client.test.tssurface (for exampleextensions/mattermost/src/channel.tsand monitor/reaction imports from plugin-sdk).Label: security
AI-assisted.