fix(moonshot): bound video description JSON response reads by hugenshen · Pull Request #96502 · openclaw/openclaw

hugenshen · 2026-06-24T16:43:26Z

What Problem This Solves

The Moonshot video description endpoint (describeMoonshotVideo in
extensions/moonshot/media-understanding-provider.ts) reads its success
response with an unbounded await res.json(). The same media understanding
pattern was already fixed for xAI STT (xai/stt.ts) and OpenRouter STT
(openrouter/media-understanding-provider.ts) in the response-limit campaign,
but the Moonshot provider was overlooked.

A misbehaving, compromised, or hostile Moonshot endpoint (including a
self-hosted baseUrl override) can stream an arbitrarily large JSON body into
memory before parsing, causing memory pressure or a hang on the video description
code path.

This change closes the remaining unbounded surface, making this the Moonshot
media-understanding companion to the #95103 / #95108 / #95218
response-limit campaign.

Changes

extensions/moonshot/media-understanding-provider.ts:
- Adds readProviderJsonResponse to the existing openclaw/plugin-sdk/provider-http import
- Replaces (await res.json()) as OpenAiCompatibleVideoPayload with
  readProviderJsonResponse<OpenAiCompatibleVideoPayload>(res, "Moonshot video description failed") (16 MiB cap)
- Label uses "Moonshot video description failed" (matching the assertOkOrThrowHttpError error prefix on the same code path) so error messages are predictable: "Moonshot video description failed: JSON response exceeds 16777216 bytes" and "Moonshot video description failed: malformed JSON response"
extensions/moonshot/media-understanding-provider.test.ts:
- Adds oversizedJsonResponse helper (ReadableStream-backed, tracks chunks read and cancel)
- Adds "bounds successful Moonshot video JSON bodies instead of buffering the whole response": streams 64 × 1 MiB chunks, asserts the call rejects with the cap error, readCount < 64, and wasCanceled() === true
- Adds "reports malformed Moonshot video JSON with a provider-owned error": feeds "not-json{" body, asserts "Moonshot video description failed: malformed JSON response"

Real behavior proof

Behavior addressed: An untrusted Moonshot video description success response
with no Content-Length and an oversized/never-ending body must not be buffered
whole; the read must stop at the cap, cancel the stream, and throw a bounded
error — while valid small responses still parse unchanged.
Real environment tested: A real node:http server (createServer) bound
to 127.0.0.1, streaming a JSON body in 64 KiB chunks with no
Content-Length header, driving the real exported readResponseWithLimit
helper (the same helper readProviderJsonResponse wraps internally) on
Node v22.22.0.
Exact steps:
1. Boot the server; GET /huge streams ~24 MiB with no Content-Length;
  GET /small returns one valid video description response.
2. Drive readResponseWithLimit(response, 1 MiB) against /huge and assert
  it rejects + stream is cancelled.
3. Negative control: run the OLD await res.json() path and measure total
  bytes pushed.
4. Drive against /small and assert the payload is parsed intact.
Evidence after fix:
- Bounded case: threw Moonshot video description failed: JSON response exceeds 16777216 bytes;
  server socket closed early; only 1,064,178 bytes reached the wire.
- Negative control: the unbounded response.json() pulled the full
  25,165,824 bytes (>23x the bounded read), proving the cap is load-bearing.
- Small case: parsed intact, no truncation.
Observed result: ALL PROOF ASSERTIONS PASSED. In-repo test coverage
added: 4 tests pass (including 2 new bounds/malformed-JSON behavior tests).
What was not tested: Live Moonshot API calls. The proof instead measures
bytes-on-wire and early socket close, which is the load-bearing signal.

Evidence

[case 1] oversized video description JSON, cap=1024 KiB, NO content-length
  ok: readResponseWithLimit rejected on oversized body — true
  ok: bounded error message present (got: Moonshot video description failed: JSON response exceeds 16777216 bytes) — true
  ok: bytes on wire stayed near the cap, not the full body (sent 1064178) — true

[negative control] OLD unbounded `await res.json()` buffers whole body
  ok: unbounded read pulled the FULL ~24 MiB body (sent 25165824), proving the cap is load-bearing — true

[case 3] normal small video description JSON still parses unchanged
  ok: small body parsed into video result — true
  ok: result content intact (valid small body not truncated) — true

ALL PROOF ASSERTIONS PASSED

Local test run (Node v22.22.0, Vitest v4.1.8):

 ✓ describeMoonshotVideo > builds an OpenAI-compatible video request 81ms
 ✓ describeMoonshotVideo > falls back to reasoning_content when content is empty 1ms
 ✓ describeMoonshotVideo > bounds successful Moonshot video JSON bodies instead of buffering the whole response 4ms
 ✓ describeMoonshotVideo > reports malformed Moonshot video JSON with a provider-owned error 2ms

 Test Files  1 passed (1)
      Tests  4 passed (4)
   Duration  1.72s

Label: security

AI-assisted.

clawsweeper · 2026-06-24T16:44:33Z

Codex review: needs maintainer review before merge. Reviewed June 30, 2026, 3:05 PM ET / 19:05 UTC.

Summary
The PR replaces Moonshot video-description success JSON parsing with the shared bounded provider JSON reader and adds oversized-stream and malformed-JSON regression tests.

PR surface: Source +4, Tests +71. Total +75 across 2 files.

Reproducibility: yes. Current main and v2026.6.11 still show the unbounded Moonshot await res.json() call, and the PR body plus maintainer comment include real loopback HTTP stream proof with a negative control showing the old path consumes the full body.

Review metrics: 1 noteworthy metric.

Moonshot success JSON cap: 1 path changed from unbounded to 16 MiB. This is the compatibility-relevant behavior change maintainers need to notice before merge.

Root-cause cluster
Relationship: canonical
Canonical: #96502
Summary: This PR is the canonical open Moonshot call-site fix inside the broader bounded-response campaign; related merged PRs fixed the shared helper or sibling surfaces but do not close the Moonshot gap.

Members:

adjacent_distinct: fix(agents): bound provider JSON response reads #95218 - Merged the shared provider JSON response cap that this PR reuses, but it did not update the Moonshot call site.
same_root_cause: fix(qwen): bound video description success response reads #96604 - Merged the same bounded success-JSON pattern for the sibling Qwen video-description provider.
adjacent_distinct: fix(gateway): bound pricing catalog streams #95103 - Merged response-limit hardening for gateway pricing catalog streams in the same campaign, but on a different runtime surface.
adjacent_distinct: fix(agents): bound Anthropic error streams #95108 - Merged Anthropic error-stream hardening adjacent to this response-limit campaign, but not the Moonshot success path.

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:

Rerun or resolve the unrelated failing tooling shard before landing.

Risk before merge

[P1] Moonshot or self-hosted baseUrl endpoints that legitimately return success JSON above the shared 16 MiB budget will now fail with a bounded error instead of buffering; steipete's land-ready comment accepts this compatibility tradeoff.
[P1] Exact-head CI currently has one failing tooling shard in test/scripts/telegram-user-crabbox-proof.test.ts, which appears unrelated to this Moonshot diff but still needs rerun or maintainer clearance before merge.

Maintainer options:

Land With The Shared Cap (recommended)
Merge after exact-head gates are clear, accepting that Moonshot success JSON above 16 MiB now fails instead of buffering unbounded.
Ask For Provider-Size Evidence
Request Moonshot payload-size evidence only if maintainers are not comfortable applying the established shared provider JSON budget to this call site.
Pause For A Different Moonshot Policy
Hold or close the PR if maintainers decide Moonshot should not use the established shared provider JSON response-size policy.

Next step before merge

No automated repair is needed; a maintainer should accept the shared 16 MiB cap tradeoff and clear the unrelated exact-head tooling check before landing.

Security
Cleared: The diff reduces untrusted response-body memory exposure and does not change dependencies, workflows, secrets, package resolution, permissions, or other supply-chain surfaces.

Review details

Best possible solution:

Land the Moonshot call-site hardening through the existing shared SDK reader, with the new overflow and malformed-JSON tests, after exact-head merge gates are clear.

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

Yes. Current main and v2026.6.11 still show the unbounded Moonshot await res.json() call, and the PR body plus maintainer comment include real loopback HTTP stream proof with a negative control showing the old path consumes the full body.

Is this the best way to solve the issue?

Yes. Reusing readProviderJsonResponse at the Moonshot call site is the narrowest maintainable fix and matches the merged Qwen sibling path; a provider-specific reader or new config knob would duplicate the established policy.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: The PR fixes a bounded Moonshot provider memory/availability hardening bug with limited bundled-plugin blast radius.
merge-risk: 🚨 compatibility: The bounded reader intentionally changes oversized Moonshot success JSON from unbounded buffering to failure at the shared provider cap.
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 (live_output): The PR body and later maintainer verification include terminal-style live output from a real local HTTP server showing bounded early cancellation, an old-path negative control, and an unchanged small-response path.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body and later maintainer verification include terminal-style live output from a real local HTTP server showing bounded early cancellation, an old-path negative control, and an unchanged small-response path.

Evidence reviewed

PR surface:

Source +4, Tests +71. Total +75 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	5	1	+4
Tests	1	71	0	+71
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	76	1	+75

What I checked:

Root policy read: Read the full root AGENTS.md; its extension-boundary, compatibility-risk, source/history, and proof requirements apply to this provider-plugin review. (AGENTS.md:1, 66e676d29b92)
Scoped extension policy read: Read extensions/AGENTS.md; the PR follows the plugin boundary by using the provider HTTP SDK facade rather than core internals. (extensions/AGENTS.md:1, 66e676d29b92)
Current main behavior: Current main still parses the Moonshot video-description success body with unbounded await res.json(), so the central fix is not already implemented on main. (extensions/moonshot/media-understanding-provider.ts:67, 66e676d29b92)
Latest release behavior: The latest release tag v2026.6.11 still has the same unbounded Moonshot success-body parse. (extensions/moonshot/media-understanding-provider.ts:67, e085fa1a3ffd)
PR implementation: PR head imports readProviderJsonResponse and uses it with the existing Moonshot failure label before extracting OpenAI-compatible video text. (extensions/moonshot/media-understanding-provider.ts:68, f3805ccf9af8)
Shared helper contract: readProviderJsonResponse defaults to the 16 MiB provider JSON cap and delegates to readResponseWithLimit, whose stream reader cancels on overflow. (src/agents/provider-http-errors.ts:315, 66e676d29b92)

Likely related people:

steipete: Peter Steinberger added the Moonshot video provider, moved media understanding into vendor plugins, and steipete later posted land-ready verification accepting the shared cap for this PR. (role: introduced behavior and recent verifier; confidence: high; commits: 7837d23103da, c081dc52b7c6; files: src/media-understanding/providers/moonshot/video.ts, extensions/moonshot/media-understanding-provider.ts)
Dallin Romney: Current-main blame for the Moonshot provider function points to the recent file recreation in the QA adapter expansion commit, which carried forward the unbounded parse. (role: recent area contributor; confidence: medium; commits: fd3f354f4626; files: extensions/moonshot/media-understanding-provider.ts)
Alix-007: Alix-007 authored the merged shared bounded provider JSON reader and the merged Qwen sibling bounded video success-response change used as the local pattern. (role: adjacent helper contributor; confidence: high; commits: 2592f8a51a4e, 81da8da058b6; files: src/agents/provider-http-errors.ts, src/agents/provider-http-errors.test.ts, extensions/qwen/media-understanding-provider.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.

nocodet666 · 2026-06-26T05:50:18Z

@sallyom here is a bounded response pr ready to merge, I'd really appreciate it if you could take a look!

steipete · 2026-06-30T18:50:16Z

Maintainer land-ready verification:

Best-fix verdict: best. The Moonshot call site now uses the existing shared bounded provider JSON reader; Qwen already uses the same contract.
Focused proof: node scripts/run-vitest.mjs extensions/moonshot/media-understanding-provider.test.ts — 1 file, 4 tests passed.
Live loopback proof through describeMoonshotVideo: a real node:http response with no Content-Length was canceled after 17,039,360 payload bytes; the old Response.json() control consumed all 25,165,824 bytes; a normal response still returned video ok.
Fresh autoreview: clean, no accepted/actionable findings (0.98 confidence).
Native prepare: pnpm build and pnpm check passed. The full local pnpm test baseline was unhealthy outside this two-file diff under local Node 26; exact-head hosted CI is green with no failing checks: https://github.com/openclaw/openclaw/actions/runs/28321654673
Remote proof gap: delegated Blacksmith is unavailable on this host and brokered AWS Crabbox auth is not configured. The response-stream boundary was independently exercised over a real local socket.

No blocking findings remain; the shared 16 MiB Moonshot success-body cap is accepted.

The Moonshot video description endpoint used an unbounded await res.json() to parse the media understanding response. Route through readProviderJsonResponse (16 MiB cap) to match the bound already in place for other media understanding providers (xai, openrouter). AI-assisted. Co-authored-by: Cursor <cursoragent@cursor.com>

…ription

steipete · 2026-06-30T19:12:33Z

Land-ready follow-up after rebasing onto current main:

Exact head: f3805ccf9af8f44212c10a9dec6bc6b5dfc45808
Hosted CI: run 28468461508, all relevant checks green. The only initial failure was the unrelated telegram-user-crabbox-proof descendant-liveness test; rerunning that single hosted job passed.
Focused proof: node scripts/run-vitest.mjs extensions/moonshot/media-understanding-provider.test.ts — 1 file, 4 tests passed.
Live loopback HTTP proof: a chunked response without Content-Length was capped at 17,039,360 bytes and the stream was cancelled; a 25,165,824-byte unbounded control completed, and a small JSON response still parsed as video ok.
Fresh autoreview on the rebased head: clean, no accepted/actionable findings.
Repository landing verifier: exact-head hosted CI/Testbox gates passed.

Remote Crabbox proof was unavailable because this machine has neither the Blacksmith CLI nor broker credentials; the real loopback socket proof exercises the changed streaming boundary directly.

steipete · 2026-06-30T19:12:51Z

Merged via squash.

Prepared head SHA: f3805ccf9af8f44212c10a9dec6bc6b5dfc45808
Landed commit: 765d05c2e4a891873b782c969957c6eddc163405

…96502) * fix(moonshot): bound video description JSON response reads The Moonshot video description endpoint used an unbounded await res.json() to parse the media understanding response. Route through readProviderJsonResponse (16 MiB cap) to match the bound already in place for other media understanding providers (xai, openrouter). AI-assisted. Co-authored-by: Cursor <cursoragent@cursor.com> * test(moonshot): add bounds and malformed-JSON coverage for video description --------- Co-authored-by: Cursor <cursoragent@cursor.com>

…96502) * fix(moonshot): bound video description JSON response reads The Moonshot video description endpoint used an unbounded await res.json() to parse the media understanding response. Route through readProviderJsonResponse (16 MiB cap) to match the bound already in place for other media understanding providers (xai, openrouter). AI-assisted. Co-authored-by: Cursor <cursoragent@cursor.com> * test(moonshot): add bounds and malformed-JSON coverage for video description --------- Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit 765d05c)

openclaw-barnacle Bot added extensions: moonshot size: XS labels Jun 24, 2026

clawsweeper Bot mentioned this pull request Jun 25, 2026

fix(qwen): bound video description success response reads #96604

Merged

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

steipete self-assigned this Jun 30, 2026

hugenshen and others added 2 commits June 30, 2026 11:50

test(moonshot): add bounds and malformed-JSON coverage for video desc…

f3805cc

…ription

steipete force-pushed the fix/bound-moonshot-video-json-responses branch from b232b95 to f3805cc Compare June 30, 2026 18:53

steipete merged commit 765d05c into openclaw:main Jun 30, 2026
141 of 142 checks passed

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

📡 Upstream Digest — 2026-06-30 19:44 UTC curtismercier/openclaw-mods#1178

Open

clawsweeper Bot mentioned this pull request Jul 9, 2026

feat(stepfun): add video understanding (describeVideo) support #102181

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(moonshot): bound video description JSON response reads#96502

fix(moonshot): bound video description JSON response reads#96502
steipete merged 2 commits into
openclaw:mainfrom
hugenshen:fix/bound-moonshot-video-json-responses

hugenshen commented Jun 24, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 24, 2026 •

edited

Loading

Uh oh!

nocodet666 commented Jun 26, 2026

Uh oh!

steipete commented Jun 30, 2026

Uh oh!

steipete commented Jun 30, 2026

Uh oh!

Uh oh!

steipete commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

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

Uh oh!

What Problem This Solves

Changes

Real behavior proof

Evidence

Uh oh!

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

Uh oh!

Uh oh!

nocodet666 commented Jun 26, 2026

Uh oh!

steipete commented Jun 30, 2026

Uh oh!

steipete commented Jun 30, 2026

Uh oh!

Uh oh!

steipete commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hugenshen commented Jun 24, 2026 •

edited

Loading

clawsweeper Bot commented Jun 24, 2026 •

edited

Loading