fix(pixverse): bound video-generation success JSON response reads at 16 MiB#96885
Closed
wangmiao0668000666 wants to merge 2 commits into
Closed
fix(pixverse): bound video-generation success JSON response reads at 16 MiB#96885wangmiao0668000666 wants to merge 2 commits into
wangmiao0668000666 wants to merge 2 commits into
Conversation
…16 MiB Replace unbounded response.json() in readPixVerseJson with readProviderJsonResponse from openclaw/plugin-sdk/provider-http. This prevents OOM from hostile or misconfigured PixVerse endpoints. Also adds readProviderJsonResponseMock to the shared provider-http test-mocks so extension tests can mock it alongside other provider-http exports. Co-Authored-By: Claude <[email protected]>
Contributor
|
Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 10:33 PM ET / 02:33 UTC. Summary PR surface: Source +3. Total +3 across 2 files. Reproducibility: yes. Current main directly calls Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Merge the shared bounded-reader change after maintainers accept the 16 MiB fail-closed cap for PixVerse success JSON and current-head checks complete. Do we have a high-confidence way to reproduce the issue? Yes. Current main directly calls Is this the best way to solve the issue? Yes. Reusing AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 6830aa39eaa1. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +3. Total +3 across 2 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
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
Bot
added
proof: sufficient
Contributor
Author
|
@clawsweeper re-review |
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
rating: 🐚 platinum hermit
wangmiao0668000666
added a commit
to wangmiao0668000666/openclaw
that referenced
this pull request
Jun 26, 2026
Remove the inner try/catch that wrapped readProviderJsonResponse errors as 'malformed JSON response'. readProviderJsonResponse already handles both overflow and malformed JSON internally. This matches the sibling pixverse fix (openclaw#96885) which removed the same catch wrapper. Co-Authored-By: Claude <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Problem This Solves
extensions/pixverse/video-generation-provider.ts:189callsawait response.json()with no byte-level cap. A hostile, compromised, or misconfigured PixVerse endpoint can return a success status with noContent-Lengthand stream an arbitrarily large JSON payload, causing OOM.The fix routes through
readProviderJsonResponse(openclaw/plugin-sdk/provider-http, 16 MiB default cap), which reads the body as a bounded stream and cancels on overflow. The existingreadPixVerseSuccesslayer is preserved for PixVerse-specific envelope validation.Changes
extensions/pixverse/video-generation-provider.ts:186-190— replacepayload = await response.json()inreadPixVerseJsonwithreadProviderJsonResponse(response, label). Removes the manual try/catch (malformed JSON handling is built into the SDK helper). Type changed fromPick<Response, "json">toResponse.extensions/pixverse/video-generation-provider.test.ts— addreadProviderJsonResponseMockto destructured mock imports (shared mock handles the delegation).src/plugin-sdk/test-helpers/provider-http-mocks.ts— addreadProviderJsonResponseMockto shared mock interface and thevi.mock("openclaw/plugin-sdk/provider-http")block, so extension tests can mock it alongside other provider-http exports.Real behavior proof
Drives
readProviderJsonResponseover a realnode:httpserver (chunked transfer, no Content-Length). Node v22.22.0.response.json()on PixVerse video generation success responses; now capped at 16 MiB (SDK default) with cancellation on overflow.node:httpserver (127.0.0.1, chunked transfer, no Content-Length). Node v22.22.0.node --import tsx _proof_pixverse.mts(working tree only, not committed)readProviderJsonResponsehelper used in production).Out of scope
response.json()call sites inextensions/pixverse/— covered in separate bounded-read sibling PRs.Risk
response.json()toreadProviderJsonResponse. Error behavior preserves the same upstream catch path. Byte cap is 16 MiB — invisible to normal PixVerse JSON responses (typically < 1 MiB).Diff stats
AI-assisted: implemented and proof-tested with AI assistance; reviewed by a human before submission.