fix(mattermost): reject oversized websocket events by sunlit-deng · Pull Request #99366 · openclaw/openclaw

sunlit-deng · 2026-07-03T04:45:35Z

Summary

Bounds inbound Mattermost monitor WebSocket messages at 16 MiB before application parsing, instead of inheriting the pinned ws client's 100 MiB default.

What Problem This Solves

A buggy or hostile self-hosted Mattermost server could make the monitor buffer a very large WebSocket message before JSON/schema validation had a chance to reject it. The production connection factory supplied no maxPayload, so ws 8.21.0 allowed up to 100 MiB per message.

Why This Change Was Made

The default Mattermost WebSocket factory is the narrow transport owner that can reject a complete fragmented or compressed message before it reaches capture and JSON parsing. A fixed 16 MiB ceiling:

removes 84% of the upstream default exposure;
matches the established Discord channel WebSocket ceiling;
stays well above Mattermost's large, double-encoded post-props/card envelopes;
leaves compatibility headroom for server-generated metadata and custom self-hosted/plugin events without adding another operator config surface.

The loopback regression uses the real production factory. It dispatches a normal post and a 1+ MiB valid post envelope, then sends a 16 MiB + 1 byte frame and verifies the ws receiver reports Max payload size exceeded.

User Impact

Normal Mattermost posts, cards, reactions, and integration events continue to work. An inbound WebSocket message above 16 MiB closes and reconnects the monitor instead of being buffered up to 100 MiB.

Evidence

Pinned dependency source: ws 8.21.0 defaults client maxPayload to 100 MiB and rejects excess length in the receiver before message emission.
Mattermost source: PostPropsMaxRunes is 800,000, and posted events serialize the post into the event data as a JSON string.
Sanitized AWS Crabbox cbx_78e494d6dac7, exact head 8839d638f956256cbdbb2ab0ccb620fa2b45c0f3:
- run_a5a491b2ad1b: all 11 focused Mattermost monitor WebSocket tests passed, including production-factory normal/large-valid/oversized behavior.
- run_dd52e6322300: pnpm check:changed passed, including project type checks, lint/format guards, and runtime import-cycle checks.
Fresh pre-commit autoreview: no accepted or actionable findings.

Co-authored-by: sunlit-deng [email protected]

clawsweeper · 2026-07-03T04:57:08Z

Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 2:42 AM ET / 06:42 UTC.

Summary
Adds a 4 MiB maxPayload to the production Mattermost monitor WebSocket client and a loopback test for normal, large-valid, and oversized frames.

PR surface: Source +6, Tests +75. Total +81 across 2 files.

Reproducibility: yes. A loopback WebSocket server can exercise createMattermostConnectOnce() with normal, large-valid, and 4 MiB + 1 frames, and current main/release omit maxPayload so they inherit the ws 100 MiB default.

Review metrics: 1 noteworthy metric.

Mattermost WebSocket default cap: 1 added: 4 MiB maxPayload. This changes the monitor's accepted inbound frame envelope from the ws client default, so maintainers should notice the compatibility boundary before merge.

Stored data model
Persistent data-model change detected: serialized state: extensions/mattermost/src/mattermost/monitor-websocket.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

none.

Risk before merge

[P1] Merging changes the Mattermost monitor from the ws 100 MiB default to a 4 MiB inbound event cap, so self-hosted or custom Mattermost servers that emit larger events would now close the monitor before delivery.
[P1] The proof supports normal and large-valid posted events under the selected cap, but maintainer acceptance is still needed for the exact 4 MiB compatibility envelope.

Maintainer options:

Accept the 4 MiB event envelope (recommended)
If maintainers agree Mattermost monitor events over 4 MiB are unsupported or hostile for this path, the current guard is a bounded hardening change with matching proof.
Raise the cap before merge
If supported self-hosted Mattermost plugins can legitimately exceed 4 MiB, raise MATTERMOST_WEBSOCKET_MAX_PAYLOAD_BYTES and extend the loopback test to the chosen ceiling.

Next step before merge

[P2] The remaining action is maintainer acceptance of the 4 MiB Mattermost event envelope, not an automated code repair.

Security
Cleared: No supply-chain, workflow, secret-handling, dependency-resolution, or permission regression was found; the diff is a bounded Mattermost runtime guard plus tests.

Review details

Best possible solution:

Land the fixed transport cap if maintainers accept the 4 MiB supported event envelope; otherwise choose a larger fixed cap and keep the same focused loopback test shape.

Do we have a high-confidence way to reproduce the issue?

Yes. A loopback WebSocket server can exercise createMattermostConnectOnce() with normal, large-valid, and 4 MiB + 1 frames, and current main/release omit maxPayload so they inherit the ws 100 MiB default.

Is this the best way to solve the issue?

Yes, with maintainer acceptance of the compatibility envelope. The default Mattermost WebSocket factory is the narrow owner boundary for this resource guard; parser-only validation would still buffer the frame first, and a config option would add unnecessary operator surface unless a higher supported limit is required.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against da67520b8d5f.

Label changes

Label justifications:

P2: This is a focused Mattermost channel hardening fix with limited blast radius and no evidence of an urgent live regression.
merge-risk: 🚨 compatibility: The new 4 MiB default can reject self-hosted or custom Mattermost WebSocket events that were previously accepted by the ws 100 MiB default.
merge-risk: 🚨 message-delivery: A frame over the new cap closes the monitor before OpenClaw can parse or dispatch any Mattermost message in that event.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal loopback output from the production default socket factory showing normal delivery, large-valid delivery, and oversized-frame rejection after the change.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal loopback output from the production default socket factory showing normal delivery, large-valid delivery, and oversized-frame rejection after the change.

Evidence reviewed

PR surface:

Source +6, Tests +75. Total +81 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	7	1	+6
Tests	1	75	0	+75
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	82	1	+81

What I checked:

PR implementation: The PR defines MATTERMOST_WEBSOCKET_MAX_PAYLOAD_BYTES = 4 * 1024 * 1024 and passes it as maxPayload to the default Mattermost ws client. (extensions/mattermost/src/mattermost/monitor-websocket.ts:49, a555c2866aaa)
Loopback regression coverage: The new test sends a normal post, a large valid post envelope below the cap, and a maxPayload + 1 frame that must report Max payload size exceeded. (extensions/mattermost/src/mattermost/monitor-websocket.test.ts:200, a555c2866aaa)
Current main behavior: Current main still creates the Mattermost WebSocket with only the optional debug proxy agent, so this branch introduces the first explicit monitor payload cap. (extensions/mattermost/src/mattermost/monitor-websocket.ts:114, da67520b8d5f)
Latest release behavior: The latest release tag v2026.6.11 has the same no-maxPayload Mattermost WebSocket factory, so this is an upgrade-visible behavior change. (extensions/mattermost/src/mattermost/monitor-websocket.ts:114, e085fa1a3ffd)
Runtime entry point: monitorMattermostProvider builds the monitor connection through createMattermostConnectOnce, making the default WebSocket factory the narrow runtime boundary for this guard. (extensions/mattermost/src/mattermost/monitor.ts:2237, da67520b8d5f)
Mattermost post parser contract: OpenClaw's Mattermost post schema accepts props, so a transport cap below valid props/cards would drop events before existing parsing can handle them. (extensions/mattermost/src/mattermost/client.ts:57, da67520b8d5f)

Likely related people:

steipete: Commit edbd86074f58 extracted the Mattermost WebSocket monitor module, and later commits repeatedly maintained this Mattermost plugin area. (role: introduced monitor extraction and recent area owner; confidence: high; commits: edbd86074f58, 827b0de0ce74, be3e10475fb0; files: extensions/mattermost/src/mattermost/monitor-websocket.ts, extensions/mattermost/src/mattermost/monitor.ts)
Qinsam: Commit 47839d3b expanded the Mattermost WebSocket monitor and its tests for stale connection detection after bot disable/enable cycles. (role: recent WebSocket monitor contributor; confidence: medium; commits: 47839d3b9a6d; files: extensions/mattermost/src/mattermost/monitor-websocket.ts, extensions/mattermost/src/mattermost/monitor-websocket.test.ts)
Alix-007: Commit 9241b970 recently bounded successful Mattermost REST JSON/text response reads, which is adjacent hardening for the same plugin's untrusted self-hosted server boundary. (role: adjacent Mattermost response-limit contributor; confidence: medium; commits: 9241b9701d9c; files: extensions/mattermost/src/mattermost/client.ts, extensions/mattermost/src/mattermost/client.test.ts)
pick-cat: Commit 54d2fa24 recently bounded null-body Mattermost error response reads in the same plugin area. (role: adjacent Mattermost response-limit contributor; confidence: medium; commits: 54d2fa240d8f; files: extensions/mattermost/src/mattermost/client.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

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

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

sunlit-deng · 2026-07-03T05:39:02Z

@clawsweeper re-review

clawsweeper · 2026-07-03T05:39:05Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Co-authored-by: sunlit-deng <[email protected]>

steipete · 2026-07-07T08:35:04Z

Merged via squash.

Prepared head SHA: 8839d638f956256cbdbb2ab0ccb620fa2b45c0f3
Landed commit: 24330a82b44ad33f7c2a1c1580b1b99d9489aa58

* fix(mattermost): bound monitor websocket payloads * fix(mattermost): preserve websocket payload headroom Co-authored-by: sunlit-deng <[email protected]> --------- Co-authored-by: Peter Steinberger <[email protected]>

openclaw-barnacle Bot added channel: mattermost Channel integration: mattermost size: S labels Jul 3, 2026

sunlit-deng force-pushed the sunlit/fix/mattermost-ws-max-payload branch from d3fc995 to a834f58 Compare July 3, 2026 05:38

fix(mattermost): bound monitor websocket payloads

a555c28

sunlit-deng force-pushed the sunlit/fix/mattermost-ws-max-payload branch from a834f58 to a555c28 Compare July 3, 2026 06:29

clawsweeper Bot mentioned this pull request Jul 7, 2026

fix(mattermost): bound gateway websocket payloads at 16 MiB #101404

Closed

steipete self-assigned this Jul 7, 2026

fix(mattermost): preserve websocket payload headroom

8839d63

Co-authored-by: sunlit-deng <[email protected]>

steipete merged commit 24330a8 into openclaw:main Jul 7, 2026
100 checks passed

github-actions Bot mentioned this pull request Jul 7, 2026

📡 Upstream Digest — 2026-07-07 10:48 UTC curtismercier/openclaw-mods#1229

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(mattermost): reject oversized websocket events#99366

fix(mattermost): reject oversized websocket events#99366
steipete merged 2 commits into
openclaw:mainfrom
sunlit-deng:sunlit/fix/mattermost-ws-max-payload

sunlit-deng commented Jul 3, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

Uh oh!

sunlit-deng commented Jul 3, 2026

Uh oh!

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

sunlit-deng commented Jul 3, 2026 • edited by steipete Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sunlit-deng commented Jul 3, 2026

Uh oh!

clawsweeper Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sunlit-deng commented Jul 3, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading