fix(discord): bound REST response body to prevent OOM flood by Alix-007 · Pull Request #95412 · openclaw/openclaw

Alix-007 · 2026-06-20T22:19:29Z

What Problem This Solves

This exists to stop Discord REST main responses from buffering attacker-sized bodies into memory before JSON parsing. RequestClient.executeRequest previously read the main response with unbounded await response.text() (extensions/discord/src/internal/rest.ts:252), so a controlled or hijacked REST endpoint, proxy, or gateway redirection path could stream an arbitrary response and turn the Discord extension into an OOM / DoS target.

Changes

Bound the Discord REST main-response read with readResponseWithLimit, using an 8 MiB cap plus an idle timeout derived from the request timeout.
Preserved the existing coerceResponseBody parsing path for normal JSON, empty responses, rate limits, and errors after the bounded read completes.
Added regression coverage for oversized stream cancellation, idle-timeout cancellation, and a normal-payload control.

Evidence

The earlier proof for this PR used an in-process stream fixture, so this proof was upgraded to a real terminal node:http check. The scratchpad proof script is local only and is not committed. It starts a real TCP loopback HTTP server, drives the exported RequestClient with its normal fetch path, streams a no-Content-Length body larger than 16 MiB, verifies bounded throw and socket abort, runs a raw-fetch negative control that consumes the full flood, and verifies a small JSON happy path. No in-process ReadableStream fixture is used.

Command: node --import tsx scratchpad/discord-http-proof.mts

[proof] real node:http server on http://127.0.0.1:43525, cap=8388608 bytes, would-stream=67108864 bytes
PASS  oversized Discord REST response throws bounded error :: threw=true msg="Discord REST response body exceeds 8388608 bytes (received 8454136)"
PASS  stream cancelled: server saw socket abort before sending full body :: aborted=true bytesSent=9437184 (<67108864) chunks=9
PASS  stream cancelled near 8 MiB cap :: bytesSent=9437184 (<33554432)
PASS  negative control: raw unbounded read buffers the full body past the 8 MiB cap :: buffered=67108864 bytes (> 8388608)
PASS  negative control consumed far more than bounded read :: unbounded buffered=67108864 vs bounded sent≈9437184
PASS  happy path: small JSON response still parsed :: id="channel" name="general" topic="real loopback proof"

[proof] 6 PASS, 0 FAIL
[proof] ALL PASS

Focused regression test:

Command: node scripts/run-vitest.mjs extensions/discord/src/internal/rest.test.ts

Test Files  1 passed (1)
Tests       34 passed (34)
Result      [test] passed 1 Vitest shard in 14.91s

Symmetry and Remaining Scope

#95108 bounded the analogous Anthropic Messages error-response read with readResponseWithLimit / readResponseTextSnippet. This PR is the symmetric fix for the Discord REST main-response path, reusing the same @openclaw/media-core helper through the response-limit-runtime plugin entry rather than introducing a new abstraction.

Sibling read check: extensions/discord/src/api.ts:194 still has an unbounded res.text() on a separate Discord API path and remains appropriate follow-up scope; this PR is scoped to RequestClient.executeRequest in extensions/discord/src/internal/rest.ts.

Label: security

AI-assisted: implemented and verified with AI assistance; the proof commands above were run for real against the actual code path.

clawsweeper · 2026-06-20T22:21:03Z

Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 6:44 PM ET / 22:44 UTC.

Summary
The PR routes Discord RequestClient.executeRequest response-body reads through the shared bounded response reader and adds overflow, idle-timeout, and normal-payload regression coverage.

PR surface: Source +19, Tests +72. Total +91 across 2 files.

Reproducibility: yes. Current main has an unbounded response.text() in RequestClient.executeRequest, and the PR body provides terminal proof against the real RequestClient path; I did not run tests in this read-only review.

Review metrics: 1 noteworthy metric.

Fail-Closed Response Ceiling: 1 added, 8 MiB. The new ceiling changes how existing Discord RequestClient users experience unexpectedly large successful responses.

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:

[P2] Have a maintainer accept the 8 MiB ceiling or request a tuned cap before merge.

Risk before merge

[P1] Successful Discord RequestClient responses over 8 MiB now throw where current main would attempt to buffer and parse them, so maintainers should accept or tune that ceiling before merge.
[P1] The PR is intentionally scoped to RequestClient; extensions/discord/src/api.ts still has an unbounded successful-response read if maintainers want a Discord-wide response-body policy.

Maintainer options:

Accept the 8 MiB RequestClient ceiling (recommended)
Land this focused hardening if maintainers agree that Discord RequestClient responses over 8 MiB should fail closed instead of being buffered.
Tune the cap before merge
Adjust the constant or make it endpoint-specific if maintainers expect any legitimate RequestClient route to return successful JSON above 8 MiB.
Pause for a broader Discord policy
Hold this PR if maintainers want requestDiscord and RequestClient covered together under one Discord response-body limit policy.

Next step before merge

[P1] The remaining action is maintainer acceptance or tuning of a compatibility-sensitive fail-closed response cap, not an automated code repair.

Security
Cleared: The diff reduces an unbounded Discord REST response-memory risk and adds no dependency, workflow, secret, lockfile, or package-execution surface.

Review details

Best possible solution:

Land the focused RequestClient hardening after maintainer acceptance or tuning of the 8 MiB ceiling, and track any broader Discord success-body policy separately.

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

Yes. Current main has an unbounded response.text() in RequestClient.executeRequest, and the PR body provides terminal proof against the real RequestClient path; I did not run tests in this read-only review.

Is this the best way to solve the issue?

Yes for the scoped RequestClient path. The PR uses the existing plugin SDK response-limit helper and preserves parsing/status behavior; the maintainer choice is the cap value and whether broader Discord API hardening is separate follow-up.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This is normal-priority Discord availability and security hardening with limited plugin-local blast radius.
merge-risk: 🚨 compatibility: The PR changes successful Discord RequestClient responses over 8 MiB from parsed values into thrown errors.
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 after-fix terminal output from a real loopback node:http server showing bounded overflow cancellation, a stalled-body timeout, an unbounded negative control, and a small JSON success path.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from a real loopback node:http server showing bounded overflow cancellation, a stalled-body timeout, an unbounded negative control, and a small JSON success path.

Evidence reviewed

PR surface:

Source +19, Tests +72. Total +91 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	20	1	+19
Tests	1	72	0	+72
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	92	1	+91

What I checked:

Current main still has the unbounded read: RequestClient.executeRequest on current main still calls await response.text() before parsing and status handling, so the central issue is not already implemented on main. (extensions/discord/src/internal/rest.ts:252, 95b97e5b0b5e)
PR uses the shared bounded reader: The PR adds readResponseBodyText, calls readResponseWithLimit with an 8 MiB cap and idle-timeout errors, then preserves the existing parse/status flow. (extensions/discord/src/internal/rest.ts:98, 172cffd38cd5)
Dependency contract supports the intended behavior: readResponseWithLimit returns a Buffer, throws the caller-provided overflow error when truncated, and cancels streams on overflow or idle timeout, so the PR's buffer.toString("utf8") path is valid. (packages/media-core/src/read-response-with-limit.ts:130, 95b97e5b0b5e)
Regression tests cover the changed path: The PR adds RequestClient tests for oversized body cancellation, stalled body cancellation, and a small JSON success payload under the cap. (extensions/discord/src/internal/rest.test.ts:695, 172cffd38cd5)
Sibling Discord API surface remains separate: requestDiscord already bounds non-OK error bodies but still reads successful responses with res.text(), matching the PR body's statement that Discord-wide success-body policy is follow-up scope. (extensions/discord/src/api.ts:194, 95b97e5b0b5e)
Plugin boundary is satisfied: The scoped extensions policy says bundled plugin production code should import through openclaw/plugin-sdk/*, and package.json exports ./plugin-sdk/response-limit-runtime as a public subpath. (package.json:980, 95b97e5b0b5e)

Likely related people:

xialonglee: Authored the recently merged Discord REST timeout and abort-propagation work that touched the same RequestClient/runtime area. (role: recent Discord REST contributor; confidence: high; commits: f3982d644290; files: extensions/discord/src/internal/rest.ts, extensions/discord/src/client.ts, extensions/discord/src/channel.ts)
steipete: Recent history shows repeated Discord REST/API hardening and ownership of the plugin SDK response-limit facade used by this PR. (role: Discord REST and plugin SDK adjacent contributor; confidence: high; commits: c66b21662d44, 3980eaa1c22e, 418056f7a0c8; files: extensions/discord/src/internal/rest.ts, extensions/discord/src/api.ts, src/plugin-sdk/response-limit-runtime.ts)
vincentkoc: Recently bounded Discord API error bodies and authored the merged Anthropic bounded-response precedent cited by this PR. (role: adjacent bounded-response contributor; confidence: medium; commits: 698efb23a648, d6cefe26f499; files: extensions/discord/src/api.ts, src/agents/anthropic-transport-stream.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.

Alix-007 · 2026-06-21T06:27:41Z

@clawsweeper re-review — Added the What Problem This Solves and Evidence headings the proof workflow expects (proof content unchanged, just restructured under the required sections). This should clear needs-pr-context.

clawsweeper · 2026-06-21T06:27:43Z

🦞🧹
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.

The Discord REST main response path read the body with an unbounded await response.text() before JSON-parsing it. A controlled or hijacked endpoint could stream an arbitrarily large body and exhaust memory (OOM). Wrap the read in the canonical readResponseWithLimit helper with an 8 MiB cap (well above any legitimate Discord JSON payload) plus an idle timeout tied to the request timeout, so the stream is cancelled at the cap or on stall instead of buffering unbounded. Normal payloads still parse fully. This mirrors PR openclaw#95108 which bounded the analogous Anthropic Messages error-response read with the same helper.

sallyom · 2026-06-28T14:19:49Z

Merge-ready for 172cffd.

Local and ClawSweeper review clean, CI green, best narrow fix, follows established pattern, and cap is acceptable because this Discord REST path reads bounded JSON/control responses rather than media bytes. The relevant Discord REST surfaces are documented as paginated/limited responses, such as channel messages, guild members, and reaction users; file/media content is metadata or separate payload data rather than inline REST response bodies. The 8 MiB ceiling is therefore comfortably above expected successful REST payloads while still preventing unbounded response buffering.

…#95412) The Discord REST main response path read the body with an unbounded await response.text() before JSON-parsing it. A controlled or hijacked endpoint could stream an arbitrarily large body and exhaust memory (OOM). Wrap the read in the canonical readResponseWithLimit helper with an 8 MiB cap (well above any legitimate Discord JSON payload) plus an idle timeout tied to the request timeout, so the stream is cancelled at the cap or on stall instead of buffering unbounded. Normal payloads still parse fully. This mirrors PR openclaw#95108 which bounded the analogous Anthropic Messages error-response read with the same helper.

…#95412) The Discord REST main response path read the body with an unbounded await response.text() before JSON-parsing it. A controlled or hijacked endpoint could stream an arbitrarily large body and exhaust memory (OOM). Wrap the read in the canonical readResponseWithLimit helper with an 8 MiB cap (well above any legitimate Discord JSON payload) plus an idle timeout tied to the request timeout, so the stream is cancelled at the cap or on stall instead of buffering unbounded. Normal payloads still parse fully. This mirrors PR openclaw#95108 which bounded the analogous Anthropic Messages error-response read with the same helper. (cherry picked from commit 2d2a50c)

…#95412) The Discord REST main response path read the body with an unbounded await response.text() before JSON-parsing it. A controlled or hijacked endpoint could stream an arbitrarily large body and exhaust memory (OOM). Wrap the read in the canonical readResponseWithLimit helper with an 8 MiB cap (well above any legitimate Discord JSON payload) plus an idle timeout tied to the request timeout, so the stream is cancelled at the cap or on stall instead of buffering unbounded. Normal payloads still parse fully. This mirrors PR openclaw#95108 which bounded the analogous Anthropic Messages error-response read with the same helper.

…#95412) The Discord REST main response path read the body with an unbounded await response.text() before JSON-parsing it. A controlled or hijacked endpoint could stream an arbitrarily large body and exhaust memory (OOM). Wrap the read in the canonical readResponseWithLimit helper with an 8 MiB cap (well above any legitimate Discord JSON payload) plus an idle timeout tied to the request timeout, so the stream is cancelled at the cap or on stall instead of buffering unbounded. Normal payloads still parse fully. This mirrors PR openclaw#95108 which bounded the analogous Anthropic Messages error-response read with the same helper. (cherry picked from commit 2d2a50c)

openclaw-barnacle Bot added channel: discord Channel integration: discord size: S triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 20, 2026

openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 21, 2026

Alix-007 force-pushed the fix/bound-discord-rest branch from 140a4f1 to 172cffd Compare June 23, 2026 13:13

sallyom self-assigned this Jun 28, 2026

sallyom merged commit 2d2a50c into openclaw:main Jun 28, 2026
93 checks passed

github-actions Bot mentioned this pull request Jun 28, 2026

📡 Upstream Digest — 2026-06-28 15:12 UTC curtismercier/openclaw-mods#1167

Open

This was referenced Jun 28, 2026

fix(terminal-core): strip controls from terminal links #96440

Merged

improve(qa-lab): bound Discord API response reads in mantis smoke #97548

Merged

clawsweeper Bot mentioned this pull request Jun 29, 2026

fix(discord): bound requestDiscord happy-path response reads to prevent OOM #97693

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(discord): bound REST response body to prevent OOM flood#95412

fix(discord): bound REST response body to prevent OOM flood#95412
sallyom merged 1 commit into
openclaw:mainfrom
Alix-007:fix/bound-discord-rest

Alix-007 commented Jun 20, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 20, 2026 •

edited

Loading

Uh oh!

Alix-007 commented Jun 21, 2026

Uh oh!

clawsweeper Bot commented Jun 21, 2026 •

edited

Loading

Uh oh!

sallyom commented Jun 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

Alix-007 commented Jun 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Changes

Evidence

Symmetry and Remaining Scope

Uh oh!

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

Uh oh!

Uh oh!

Alix-007 commented Jun 21, 2026

Uh oh!

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

Uh oh!

Uh oh!

sallyom commented Jun 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Alix-007 commented Jun 20, 2026 •

edited

Loading

clawsweeper Bot commented Jun 20, 2026 •

edited

Loading

clawsweeper Bot commented Jun 21, 2026 •

edited

Loading