Fix/discord bound probe getme json reads by hugenshen · Pull Request #97278 · openclaw/openclaw

hugenshen · 2026-06-27T16:34:18Z

What Problem This Solves

Discord health probes call GET /users/@me and still parse the success body with unbounded res.json(). On Node, response.json() buffers the entire body before parsing, so a hostile or misbehaving Discord REST endpoint can stream an oversized success payload during gateway health checks and push the process into memory pressure or OOM.

This closes the remaining unbounded Discord probe success read on current main. Open PR #96618 bounds other Discord REST success reads (PluralKit, webhook upload, voice upload) but explicitly leaves probeDiscord() on the old path.

No open upstream PR duplicates this surface.

Changes

Route the probe success JSON parse through readProviderJsonResponse(response, "discord.probe.getMe"), reusing the shared 16 MiB provider JSON reader instead of a plugin-local helper.
Preserve existing probe fail-soft behavior: HTTP errors, timeouts, and bounded overflow still surface as ok: false with a formatted error string.
Add an oversized success ReadableStream regression test with a response.json() sentinel.
Add scripts/proof-discord-probe-bound.mjs, which drives exported probeDiscord() against a local node:http server.

Real behavior proof

Behavior addressed: a successful /users/@me response with no Content-Length must not be buffered whole if it exceeds the 16 MiB provider JSON cap; the probe must fail closed instead of draining the full body.
Real environment tested: local node:http on 127.0.0.1, streaming ~24 MiB JSON in 64 KiB chunks with no Content-Length, driven through exported probeDiscord() with a fetcher redirected to the loopback server.
Exact proof shape:
1. Stream an oversized JSON body from /users/@me.
2. Call probeDiscord() and assert ok: false with the labeled cap error while the server stops sending near the cap.
3. Drive readProviderJsonResponse against the same stream to confirm the shared reader stops early (negative control vs unbounded drain).
4. Call probeDiscord() against a small valid body and confirm bot id/username parse intact.
What this proof does and does not prove: it demonstrates real bytes on wire, early stream cancellation, and unchanged small-body parsing through the exact production probe helper. It does not live-test the real Discord cloud API.

Evidence

pnpm test extensions/discord/src/probe.intents.test.ts

 Test Files  1 passed (1)
      Tests  9 passed (9)

Regression locked in:

bounds oversized getMe probe JSON responses and cancels the stream
→ probe returns ok=false with "discord.probe.getMe: JSON response exceeds 16777216 bytes"
→ cancelCount = 1
→ response.json() sentinel not invoked

Loopback HTTP proof (production probeDiscord path):

node --import tsx/esm scripts/proof-discord-probe-bound.mjs

[proof] probeDiscord getMe production path
  cap=16777216 bytes (16 MiB), would-stream≈25165824 bytes (24 MiB)

  ok: oversized getMe probe fails closed (ok=false)
  ok: bounded error present: "discord.probe.getMe: JSON response exceeds 16777216 bytes"
  ok: server wrote … bytes, stopped before full 24 MiB stream
  ok: shared reader rejects oversized body
  ok: small getMe parsed (id=bot-1, username=proof)

ALL PROOF ASSERTIONS PASSED

Label: security

AI-assisted.

clawsweeper · 2026-06-27T16:35:20Z

Codex review: needs maintainer review before merge. Reviewed June 28, 2026, 4:45 AM ET / 08:45 UTC.

Summary
The branch routes Discord probeDiscord() /users/@me success JSON parsing through readProviderJsonResponse and adds an oversized-stream regression test.

PR surface: Source +4, Tests +37. Total +41 across 2 files.

Reproducibility: yes. as source-reproducible: current main and v2026.6.10 still call native res.json() in the Discord probe success path, and the PR's added stream fixture would fail if that reader were still used. I did not execute tests because this review is read-only.

Review metrics: 1 noteworthy metric.

Capped Discord probe success reads: 1 changed. The behavioral change is limited to the single /users/@me success parse in probeDiscord(), keeping maintainer review focused on that health-check path.

Root-cause cluster
Relationship: canonical
Canonical: #97278
Summary: This PR is the canonical open fix for the Discord probe /users/@me success-read surface; related PRs cover the shared helper or adjacent Discord JSON reads but not this exact callsite.

Members:

partial_overlap: fix(agents): bound provider JSON response reads #95218 - The merged PR added the shared bounded JSON reader this PR reuses, but it did not migrate the Discord probe callsite.
partial_overlap: fix(discord,telegram,github-copilot,google-meet): bound JSON response reads to prevent OOM #96618 - The open PR covers adjacent Discord JSON reads while leaving the probe path out of its diff.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

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:

Update the PR body so the Changes and Evidence sections no longer say the proof script is included in the final diff.

Risk before merge

[P1] The PR body still says the branch adds scripts/proof-discord-probe-bound.mjs, but the current head deletes that script; this is an audit/proof-text mismatch rather than a runtime defect.
[P1] Two exact-head CI shards were still in progress when checked, so merge should wait for required checks to finish.

Maintainer options:

Decide the mitigation before merge
Land the focused bounded-reader migration after exact-head checks finish, preferably with the PR body updated to match the final two-file diff.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No ClawSweeper repair lane is needed; maintainers should review the focused fix after exact-head CI completes and decide whether to require the PR-body proof wording cleanup before merge.

Security
Cleared: The diff narrows an existing Discord HTTP response read through an existing SDK helper and adds only focused test coverage, with no dependency, workflow, permission, secret, lockfile, or external-download changes.

Review details

Best possible solution:

Land the focused bounded-reader migration after exact-head checks finish, preferably with the PR body updated to match the final two-file diff.

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

Yes, as source-reproducible: current main and v2026.6.10 still call native res.json() in the Discord probe success path, and the PR's added stream fixture would fail if that reader were still used. I did not execute tests because this review is read-only.

Is this the best way to solve the issue?

Yes. Reusing readProviderJsonResponse through the plugin SDK facade is the narrowest maintainable fix; a Discord-local helper would duplicate shared policy, and the related open PR does not cover this probe surface.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This is normal-priority Discord response-size hardening with limited plugin-local blast radius and no active outage evidence.
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 output from a local loopback node:http run driving exported probeDiscord() through oversized and small /users/@me responses after the fix; the final branch no longer keeps the proof script, but the proof remains behavior-relevant.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal output from a local loopback node:http run driving exported probeDiscord() through oversized and small /users/@me responses after the fix; the final branch no longer keeps the proof script, but the proof remains behavior-relevant.

Evidence reviewed

PR surface:

Source +4, Tests +37. Total +41 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	5	1	+4
Tests	1	37	0	+37
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	42	1	+41

What I checked:

Repository policy read: Root AGENTS.md was read fully and the scoped extensions policy was read; the relevant guidance is the bundled-plugin boundary and SDK-import rule. (AGENTS.md:29, 9c95abd49d45)
Current main still has the unbounded read: probeDiscord() on current main reads successful Discord /users/@me responses with native res.json(). (extensions/discord/src/probe.ts:158, 9c95abd49d45)
Latest release still has the same behavior: v2026.6.10 also uses native res.json() for the same Discord probe success path, so the fix has not shipped yet. (extensions/discord/src/probe.ts:158, aa69b12d0086)
PR head uses the bounded helper: The PR head replaces the native JSON reader with readProviderJsonResponse labeled discord.probe.getMe. (extensions/discord/src/probe.ts:159, 61db18d617bb)
Shared helper enforces the cap: readProviderJsonResponse delegates to readResponseWithLimit with the shared 16 MiB JSON cap and throws a labeled overflow error. (src/agents/provider-http-errors.ts:312, 9c95abd49d45)
Bounded reader cancels on overflow: readResponseWithLimit cancels the response body reader when a chunk would exceed the byte cap. (packages/media-core/src/read-response-with-limit.ts:96, 9c95abd49d45)

Likely related people:

vincentkoc: Recent commit e67f8ba459b4 changed the same probeDiscord() response-body lifecycle and adjacent probe tests. (role: recent Discord probe contributor; confidence: high; commits: e67f8ba459b4; files: extensions/discord/src/probe.ts, extensions/discord/src/probe.intents.test.ts)
steipete: Recent history includes Discord application-summary probe behavior and broad Discord/plugin SDK surface work, and the latest PR-head cleanup commit was by this account. (role: recent Discord and plugin SDK area contributor; confidence: high; commits: a81e3ee888fa, 23716de4468b, 61db18d617bb; files: extensions/discord/src/probe.ts, extensions/discord/src/probe.intents.test.ts, extensions/discord)
Alix-007: Merged commit 2592f8a51a4e added the bounded readProviderJsonResponse implementation this PR reuses. (role: shared bounded-reader contributor; confidence: high; commits: 2592f8a51a4e; files: src/agents/provider-http-errors.ts, src/agents/provider-http-errors.test.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.

steipete · 2026-06-28T08:57:05Z

Land-ready verification for 61db18d617bb713a4b1c787414d3a30b201d81bd:

Prepared change: retained the focused bounded Discord probe reader and regression, and removed the 144-line one-off proof script from the landed surface.
Focused proof: node scripts/run-vitest.mjs extensions/discord/src/probe.intents.test.ts — 9 tests passed.
Loopback live proof: node --import tsx/esm scripts/proof-discord-probe-bound.mjs passed before the proof-only file was removed; the production probe stopped near the 16 MiB cap, the unbounded negative control drained 24 MiB, and normal id/username parsing remained intact.
Fresh autoreview of the prepared shape: no findings, confidence 0.97.
Exact-head hosted CI: run 28316701620 passed.
Known gaps: no real Discord-cloud call (no test bot token) and no Crabbox/Testbox run ID (no configured provider/coordinator credentials).

The change stays behind the public plugin SDK, preserves fail-soft probe behavior, and has no remaining review findings.

steipete · 2026-06-28T08:57:25Z

Merged via squash.

Prepared head SHA: 61db18d617bb713a4b1c787414d3a30b201d81bd
Landed commit: 7ba9212665a728993550a65ef5d8fdd235d4a160

* fix(discord): bound probe getMe JSON response reads * test(discord): add oversized probe getMe JSON regression * test(discord): add loopback proof for bounded probe getMe reads * fix(scripts): satisfy oxlint in discord probe proof script * test(discord): keep probe proof in focused coverage --------- Co-authored-by: NIO <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>

openclaw-barnacle Bot added channel: discord Channel integration: discord scripts Repository scripts size: S labels Jun 27, 2026

steipete self-assigned this Jun 28, 2026

NIO and others added 5 commits June 28, 2026 01:34

fix(discord): bound probe getMe JSON response reads

60edabc

test(discord): add oversized probe getMe JSON regression

bc1d70d

test(discord): add loopback proof for bounded probe getMe reads

025f2df

fix(scripts): satisfy oxlint in discord probe proof script

a39475a

test(discord): keep probe proof in focused coverage

61db18d

steipete force-pushed the fix/discord-bound-probe-getme-json-reads branch from cb46de5 to 61db18d Compare June 28, 2026 08:34

openclaw-barnacle Bot added size: XS and removed scripts Repository scripts size: S labels Jun 28, 2026

steipete merged commit 7ba9212 into openclaw:main Jun 28, 2026
96 of 98 checks passed

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

📡 Upstream Digest — 2026-06-28 10:21 UTC curtismercier/openclaw-mods#1165

Open

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 probe getme json reads#97278

Fix/discord bound probe getme json reads#97278
steipete merged 5 commits into
openclaw:mainfrom
hugenshen:fix/discord-bound-probe-getme-json-reads

hugenshen commented Jun 27, 2026

Uh oh!

clawsweeper Bot commented Jun 27, 2026 •

edited

Loading

Uh oh!

steipete commented Jun 28, 2026

Uh oh!

Uh oh!

steipete commented Jun 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

hugenshen commented Jun 27, 2026

What Problem This Solves

Changes

Real behavior proof

Evidence

Uh oh!

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

Uh oh!

Uh oh!

steipete commented Jun 28, 2026

Uh oh!

Uh oh!

steipete commented Jun 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jun 27, 2026 •

edited

Loading