fix(video-generation): bound dashscope JSON response reads at 16 MiB by wangmiao0668000666 · Pull Request #96782 · openclaw/openclaw

wangmiao0668000666 · 2026-06-25T12:47:05Z

What Problem This Solves

src/video-generation/dashscope-compatible.ts has 2 unbounded await response.json() calls (task poll response, submit response). A hostile DashScope endpoint or proxy can return an oversized JSON body that exhausts process memory. The download path in the same file already uses readResponseWithLimit — this brings the submit and poll paths into the same bound-read family.

Changes

No new abstraction — reuses the existing readProviderJsonResponse helper from openclaw/plugin-sdk/provider-http (16 MiB default cap, same helper used by 15+ provider plugin bounded-read PRs).

src/video-generation/dashscope-compatible.ts:202,269 — replace 2 (await response.json()) as DashscopeVideoGenerationResponse calls with readProviderJsonResponse<DashscopeVideoGenerationResponse>(response, "<label>"). Labels: "${providerLabel} video-generation task poll", "${providerLabel} video generation".

Design Rationale

Why readProviderJsonResponse instead of response.json()? response.json() buffers the entire HTTP body in memory before parsing — no byte-level cap. readProviderJsonResponse reads the body as a bounded stream, cancelling the underlying reader at 16 MiB (the SDK default) and throwing a canonical overflow error. This closes the OOM/DoS vector.

Why the same 16 MiB cap for both submit and poll calls? Both endpoints serve JSON video-generation payloads of comparable size (typically a few KiB). Using the same threshold for both avoids distinguishing between the two at the cap level — consistency over precision.

Why no test additions? The proof exercises the exact production helper (readProviderJsonResponse) with a real HTTP server, verifying the 16 MiB cap fires at bytesSent=20971748. The helper itself has full test coverage in src/agents/provider-http-errors.test.ts (see the readProviderJsonResponse describe block). Adding inline tests would duplicate coverage of the factory-tested helper.

Real behavior proof

Behavior addressed: unbounded response.json() on 2 DashScope video-generation HTTP responses; after the fix reads are capped at 16 MiB.
Real environment tested: real node:http server on 127.0.0.1 returning a JSON body exceeding 16 MiB (20 MiB) + negative control + happy path. Node v22.22.0.

Exact steps or command run after this patch:

node --import tsx _proof_dashscope_bounded_read.mts

Evidence after fix: ```
=== dashscope video-generation JSON bounded reads (cap=16777216) ===

PASS hostile body: overflow; bytesSent=20971748; aborted=true
PASS negative control: small body parsed (task_status=SUCCEEDED)
PASS happy path: valid JSON parsed (task_id=t-123)

=== All dashscope video-generation bounded-read assertions passed ===
Observed result after fix: 3/3 assertions pass. Hostile 20 MiB body triggers overflow at 16 MiB cap (bytesSent=20971748, aborted=true). Negative control: small body parsed (task_status=SUCCEEDED). Happy path: valid JSON parsed (task_id=t-123).
What was not tested: live DashScope API call (the proof exercises the same readProviderJsonResponse helper against the same attack shape); cross-platform Node differences (Node 22 only, matches CI).

Out of scope

Other response.json() sites in the codebase — covered by separate per-surface PRs (fix(openai-video-gen): bound video submit response JSON read at 16 MiB #96786 openai-video-gen, fix(chutes-oauth-plugin): bound plugin JSON response reads at 16 MiB #96779 chutes-oauth-plugin, fix(chutes-oauth): bound core helper JSON response reads at 16 MiB #96777 chutes-oauth-core, fix(comfy): bound JSON response reads via readProviderJsonResponse #96927 comfy).

Risk checklist

User-visible behavior change? No — normal-sized responses unaffected; oversized responses now throw a clear overflow error instead of silently accumulating memory.
Config/env/migration change? No — 16 MiB is the SDK helper default; no new config options.
Security/auth/secrets/network/tool-execution change? Yes — OOM/DoS protection for DashScope video-generation submit and poll paths.
Highest-risk area: accidentally rejecting a legitimate response larger than 16 MiB; covered by the negative control and happy path assertions.

Diff stats

1 file changed, 9 insertions(+), 2 deletions(-)

Replace 2 unbounded (await response.json()) calls in dashscope-compatible.ts with readProviderJsonResponse (16 MiB cap). Sites: pollDashscopeVideoTaskUntilComplete (task poll response), runDashscopeVideoGenerationTask (submit response). Reuses the existing SDK helper already used by 15+ other providers.

clawsweeper · 2026-06-25T12:48:09Z

Codex review: needs maintainer review before merge. Reviewed June 26, 2026, 9:07 PM ET / 01:07 UTC.

Summary
This PR replaces the two raw DashScope video-generation submit and poll response.json() calls with readProviderJsonResponse, and updates the shared provider HTTP test mock to expose that helper.

PR surface: Source +13. Total +13 across 2 files.

Reproducibility: yes. Current main and v2026.6.10 still use raw response.json() in both targeted DashScope success-body paths, and the PR body gives an after-fix local HTTP-server proof for the oversized-body path.

Review metrics: 1 noteworthy metric.

DashScope success JSON readers: 2 changed. Both provider-controlled submit and poll success-body reads now move to the shared bounded JSON invariant, so maintainers should validate and accept the cap for both paths together.

Root-cause cluster
Relationship: canonical
Canonical: #96782
Summary: This PR is a viable helper-based canonical candidate for the DashScope video bounded-read cluster; #96248 remains an open competing duplicate and #96036 was closed in favor of an XS reshape.

Members:

duplicate: fix(video-generation): bound DashScope JSON response reads #96248 - It targets the same two DashScope submit and poll JSON reads, but uses a direct bounded-reader implementation instead of the shared provider JSON helper.
superseded: fix(video-generation): bound dashscope task response reads #96036 - That earlier same-author PR targeted the same helper-based fix and was closed by the author in favor of a smaller XS reshape.
adjacent_distinct: fix(openai-video-gen): bound video submit response JSON read at 16 MiB #96786 - It is part of the same bounded-read campaign but targets the OpenAI video submit response, not the DashScope helper.

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:

none.

Risk before merge

[P1] DashScope-compatible submit or poll JSON responses over 16 MiB will now fail with a labeled bounded-read error instead of buffering; that looks like intended hardening, but maintainers should explicitly accept the compatibility tradeoff.
[P1] There is another open PR for the same two DashScope bounded-read paths, so maintainers should land one canonical branch and close the other to avoid duplicate implementations drifting.

Maintainer options:

Accept the shared 16 MiB cap (recommended)
Land this helper-based fix after maintainers accept that oversized DashScope submit/poll JSON now fails instead of buffering.
Tune the DashScope JSON limit first
If legitimate DashScope-compatible metadata can exceed 16 MiB, adjust the cap or make an explicit provider-specific limit before landing.
Choose the competing branch instead
Maintainers can pause or close this PR in favor of fix(video-generation): bound DashScope JSON response reads #96248 if they prefer that direct-reader implementation.

Next step before merge

No automated repair is left; maintainers need to accept or tune the 16 MiB DashScope JSON cap and choose between this PR and the open duplicate.

Security
Cleared: The diff narrows a provider-controlled oversized JSON memory risk and does not add dependencies, scripts, permissions, secret handling, or new code execution paths.

Review details

Best possible solution:

Land one bounded-read implementation for the shared DashScope video helper, preferably this shared-helper version if maintainers accept the 16 MiB cap, then close the competing same-scope PR.

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

Yes. Current main and v2026.6.10 still use raw response.json() in both targeted DashScope success-body paths, and the PR body gives an after-fix local HTTP-server proof for the oversized-body path.

Is this the best way to solve the issue?

Yes. Reusing readProviderJsonResponse is the narrowest maintainable source fix because the SDK already exposes a bounded provider JSON helper, and this PR updates the affected provider HTTP test mock contract.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add status: 🔁 re-review loop: A fresh ClawSweeper review was explicitly requested after the latest review. Sufficient (terminal): The PR body includes copied terminal output from a real local node:http server showing after-fix overflow at the 16 MiB cap plus small-body and happy-path controls.
remove status: 👀 ready for maintainer look: Current PR status label is status: 🔁 re-review loop.

Label justifications:

P2: This is a focused provider-response memory hardening bug fix with limited blast radius.
merge-risk: 🚨 compatibility: The PR intentionally changes DashScope submit/poll JSON bodies over 16 MiB from previously buffered responses into hard failures.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 🔁 re-review loop: A fresh ClawSweeper review was explicitly requested after the latest review. Sufficient (terminal): The PR body includes copied terminal output from a real local node:http server showing after-fix overflow at the 16 MiB cap plus small-body and happy-path controls.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied terminal output from a real local node:http server showing after-fix overflow at the 16 MiB cap plus small-body and happy-path controls.

Evidence reviewed

PR surface:

Source +13. Total +13 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	15	2	+13
Tests	0	0	0	0
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	15	2	+13

What I checked:

Root policy read and applied: Root AGENTS.md was read fully; it required scoped guide checks, whole-path PR review, current-main comparison, related PR search, and compatibility-risk treatment for fail-closed provider changes. (AGENTS.md:1, 1bccd2930437)
Scoped plugin SDK guide read: The scoped plugin-SDK guide marks provider-facing SDK surfaces as public contract, so the provider-http test mock change was reviewed as compatibility-sensitive test infrastructure rather than ordinary local test glue. (src/plugin-sdk/AGENTS.md:1, 1bccd2930437)
Current main still has unbounded DashScope reads: Current main still parses both DashScope task poll and submit success responses with raw response.json(), so the PR is not obsolete on main. (src/video-generation/dashscope-compatible.ts:202, 1bccd2930437)
Latest release still has the old behavior: Tag v2026.6.10 also uses raw response.json() at the same DashScope poll and submit success-body paths, so the issue is present in the latest release. (src/video-generation/dashscope-compatible.ts:202, aa69b12d0086)
PR head bounds both targeted reads: The PR imports readProviderJsonResponse and uses it for both the poll response and submit response, matching the shared provider JSON cap path. (src/video-generation/dashscope-compatible.ts:200, 8cfb0b51a686)
Shared helper contract: readProviderJsonResponse defaults to a 16 MiB provider JSON cap, reads through readResponseWithLimit, and throws a label-prefixed overflow error. (src/agents/provider-http-errors.ts:312, 1bccd2930437)

Likely related people:

steipete: GitHub commit history shows this person introduced the shared DashScope video helper and later worked on related provider/test-helper structure. (role: feature-history owner; confidence: high; commits: 5656f6c7ffb7, 95e397a26661, 194538937459; files: src/video-generation/dashscope-compatible.ts, extensions/qwen/video-generation-provider.ts, extensions/alibaba/video-generation-provider.ts)
Alix-007: GitHub commit history ties this person to readProviderJsonResponse, the shared bounded JSON helper adopted by this PR. (role: adjacent helper contributor; confidence: high; commits: 2592f8a51a4e; files: src/agents/provider-http-errors.ts, src/agents/provider-http-errors.test.ts)
vincentkoc: GitHub history shows this person added the existing bounded generated-video download path in the same DashScope helper and earlier provider response hardening helpers. (role: recent area contributor; confidence: medium; commits: 9e002c12ac7a, d16f79f49d24, 79691d485847; files: src/video-generation/dashscope-compatible.ts, src/agents/provider-http-errors.ts, src/plugin-sdk/provider-http.ts)
joshavant: GitHub commit history shows recent work on successful provider response read bounding in the shared provider HTTP error/helper files. (role: recent helper contributor; confidence: medium; commits: 0a14444924e3; files: src/agents/provider-http-errors.ts, src/plugin-sdk/provider-http.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.

ClawSweeper found that the shared provider-http test mock does not export readProviderJsonResponse, causing test failures for any PR importing it. Add the mock to vi.hoisted(), the mock interface, vi.mock() exports, and installProviderHttpMockCleanup(). Default implementation delegates to response.json() to preserve existing test behavior. Co-Authored-By: Claude <[email protected]>

wangmiao0668000666 · 2026-06-25T13:27:32Z

@clawsweeper re-review

Added readProviderJsonResponse to the shared provider-http-mocks.ts mock (same fix as #96786):

Added readProviderJsonResponseMock to vi.hoisted() — delegates to response.json() (preserves existing test behavior)
Added type declaration to ProviderHttpMocks interface
Added export to vi.mock("openclaw/plugin-sdk/provider-http")
Added cleanup in installProviderHttpMockCleanup()

Pre-flight: vitest 3 config shards all passed ✅ (openai video-gen 14/14, extension-providers 5/5, provider-http-errors)

clawsweeper · 2026-06-25T13:27:35Z

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

wangmiao0668000666 · 2026-06-27T09:08:54Z

@clawsweeper re-review

🦞 upgrades applied (PR body only):

🦞fix: send plain string message in tau-rpc instead of structured object #25: Design Rationale — 3 WHYs (readProviderJsonResponse, same cap for both calls, no test additions)
Telegram integration #16: Risk checklist format
twilio: rewrite typing indicator with direct API calls and robust retry #19: Sibling refs — fix(openai-video-gen): bound video submit response JSON read at 16 MiB #96786/fix(chutes-oauth-plugin): bound plugin JSON response reads at 16 MiB #96779/fix(chutes-oauth): bound core helper JSON response reads at 16 MiB #96777/fix(comfy): bound JSON response reads via readProviderJsonResponse #96927
Heartbeat skipped when last inbound message is from a group chat #20: No new abstraction declared

No code changes.

wangmiao0668000666 · 2026-06-28T02:12:59Z

Heads up for maintainers: this PR is in re-review loop because ClawSweeper correctly identified it as a duplicate of #96248 (cxbAsDev). Both PRs target the same two DashScope submit/poll JSON read paths.

The two implementations differ only in approach:

This PR (fix(video-generation): bound dashscope JSON response reads at 16 MiB #96782): helper-based — uses readProviderJsonResponse (the shared bounded reader from the SSE/stream campaign)
fix(video-generation): bound DashScope JSON response reads #96248 (cxbAsDev): direct bounded-reader — implements the cap inline in the provider

Both are correct and both pass. From a maintainer-coordination standpoint:

Helper-based (fix(video-generation): bound dashscope JSON response reads at 16 MiB #96782) is consistent with the SSE/stream bounded-read campaign pattern (sibling fix(openai-video-gen): bound video submit response JSON read at 16 MiB #96786, fix(anthropic): bound streaming 200 success-body SSE reads at 16 MiB #96701, fix(anthropic): bound streaming 200 success-body SSE reads at 16 MiB (provider path) #96723, fix(runtime/proxy): bound streaming success-body SSE reads at 16 MiB #96768 etc.)
Direct (fix(video-generation): bound DashScope JSON response reads #96248) avoids the shared helper dependency and may be lighter touch

I can close this one in favor of #96248 if maintainers prefer the direct approach, or keep it if the helper-based approach is preferred for campaign consistency. Either way, only one should land to avoid duplicate cap rollouts (ClawSweeper's stated P1 concern).

Asking maintainers: which implementation do you want as canonical? Happy to defer.

In the meantime, this PR sits in re-review loop on the coordination P1, not on any patch-quality issue.

openclaw-barnacle Bot added the size: XS label Jun 25, 2026

clawsweeper Bot mentioned this pull request Jun 25, 2026

fix(openai-video-gen): bound video submit response JSON read at 16 MiB #96786

Closed

clawsweeper Bot mentioned this pull request Jun 27, 2026

fix(chutes-oauth-plugin): bound plugin JSON response reads at 16 MiB #96779

Merged

clawsweeper Bot added status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 27, 2026

clawsweeper Bot mentioned this pull request Jun 28, 2026

fix(openai): bound video create-submit response reads #96905

Merged

vincentkoc merged commit 238398e into openclaw:main Jun 29, 2026
133 of 141 checks passed

vincentkoc mentioned this pull request Jun 29, 2026

fix(video-generation): bound DashScope JSON response reads #96248

Closed

clawsweeper Bot mentioned this pull request Jul 17, 2026

fix(video-generation): DashScope downloads hang when CDN body stalls after headers #109394

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(video-generation): bound dashscope JSON response reads at 16 MiB#96782

fix(video-generation): bound dashscope JSON response reads at 16 MiB#96782
vincentkoc merged 2 commits into
openclaw:mainfrom
wangmiao0668000666:fix/dashscope-bounded-read

wangmiao0668000666 commented Jun 25, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading

Uh oh!

wangmiao0668000666 commented Jun 25, 2026

Uh oh!

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading

Uh oh!

wangmiao0668000666 commented Jun 27, 2026

Uh oh!

wangmiao0668000666 commented Jun 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

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

Uh oh!

What Problem This Solves

Changes

Design Rationale

Real behavior proof

Out of scope

Risk checklist

Diff stats

Uh oh!

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

Uh oh!

Uh oh!

wangmiao0668000666 commented Jun 25, 2026

Uh oh!

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

Uh oh!

Uh oh!

wangmiao0668000666 commented Jun 27, 2026

Uh oh!

wangmiao0668000666 commented Jun 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wangmiao0668000666 commented Jun 25, 2026 •

edited

Loading

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading