fix(openai): bound video create-submit response reads#96905
Conversation
Replace the unbounded `await response.json()` on the OpenAI video create-submit success path with the shared byte-bounded `readProviderJsonResponse` (16 MiB cap), matching the already-merged byteplus/google/qwen video bounds. The poll path already uses `pollProviderOperationJson` and the download path already uses `readResponseWithLimit`; this closes the remaining unbounded success-path read so a hostile or buggy endpoint cannot stream an unbounded body and force OpenClaw to buffer it before parsing. Keep the shared provider-http test mock's JSON reader real so the streaming size cap is exercised under test.
|
Codex review: needs maintainer review before merge. Reviewed June 28, 2026, 10:46 AM ET / 14:46 UTC. Summary PR surface: Source +4, Tests -10. Total -6 across 2 files. Reproducibility: yes. at source/proof level: current main reaches raw response.json() on the successful OpenAI video submit response, and the PR body includes loopback terminal proof showing bounded versus unbounded behavior. I did not rerun that proof in this read-only review. Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. 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: Land this branch as the canonical OpenAI video submit bounded-read fix once maintainers accept the shared 16 MiB cap and exact-head required checks finish. Do we have a high-confidence way to reproduce the issue? Yes at source/proof level: current main reaches raw response.json() on the successful OpenAI video submit response, and the PR body includes loopback terminal proof showing bounded versus unbounded behavior. I did not rerun that proof in this read-only review. Is this the best way to solve the issue? Yes, with cap acceptance: reusing the existing shared readProviderJsonResponse helper is the narrow owner-boundary fix, and sibling poll/download paths already use bounded helpers. A local custom cap would duplicate provider HTTP behavior and leave tests easier to drift. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 9241b9701d9c. Label changesLabel justifications:
Evidence reviewedPR surface: Source +4, Tests -10. Total -6 across 2 files. View PR surface stats
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
|
|
Addressed the author-side lint finding from the ClawSweeper review: removed the unused |
|
Merged latest upstream Verified current head locally:
The remaining non-code decision is still the canonical-branch choice versus #96786; this branch is up to date from the author side. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Updated this branch against latest upstream Verified current head locally:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
What Problem This Solves
The OpenAI video create-submit success path read the control JSON with an unbounded
await response.json(), so a hostile, misconfigured, or SSRF-reachable endpoint could stream a no-Content-Lengthbody indefinitely and force OpenClaw to buffer the whole payload before parsing, causing OOM pressure or a hang on the video generation path.The poll path already goes through the bounded
pollProviderOperationJsonhelper and the download path already usesreadResponseWithLimit, so the create-submit JSON read atextensions/openai/video-generation-provider.tswas the one remaining unbounded success-path read. This is a defense-in-depth fix and a sibling-consistency cleanup: in practice the OpenAI submit response is a small control JSON ({ id, model, status, ... }), but the read was not bounded the way the rest of this provider and the already-merged byteplus/google/qwen video bounds are.Changes
(await response.json()) as OpenAIVideoResponsewith the sharedreadProviderJsonResponsehelper fromopenclaw/plugin-sdk/provider-http, reusing the existing 16 MiB provider JSON cap; no new abstraction.assertOkOrThrowHttpErrorbehavior, thesubmitted.id/ status extraction, the poll and download paths, and therelease()cleanup.provider-httptest mock'sreadProviderJsonResponsereader real (viaimportActual) so the streaming size cap is exercised under test instead of stubbed, and update the focused OpenAI video tests to stream their submit/poll JSON through realResponsebodies.Real behavior proof
Content-Lengthbody must stop at the 16 MiB cap, cancel the stream, and throw a bounded...exceeds 16777216 byteserror. Small valid JSON must continue to parse and the full poll + download path must keep working.node:httpserver (createServer) bound to127.0.0.1, streaming JSON in 64 KiB chunks with noContent-Lengthheader and a would-stream size of about 64 MiB. The script drove the real exportedbuildOpenAIVideoGenerationProvider().generateVideo()over a real loopback socket through the realpostJsonRequest, guarded fetch path, andreadProviderJsonResponse, using the provider's request-levelallowPrivateNetworkopt-in for the loopback target. Node v22.22.0.node --import tsx <scratchpad>/proof.mts— start the streaming server → call the real provider create-submit against the streaming route and assert a bounded throw plus a server-side socket abort near the cap → run a negative control using the old unboundedfetch().arrayBuffer()against the same body → call the real provider against a small valid submit JSON and drive the full poll + download path.OpenAI video generation failed: JSON response exceeds 16777216 bytesand the server aborted after 17,825,799 bytes (just past the cap, far below the ~64 MiB it would have streamed). The negative control buffered the full 67,108,871 bytes, proving the cap is load-bearing. The small happy path parsed the submit JSON and returned the generated video through the real poll + download path.Evidence
Real
node:httpterminal proof (real loopback socket, real exported OpenAI provider, real bounded reader):Focused Vitest:
Type checks:
Lint and format:
Note: the repo autoreview step could not run because its review LLM gateway returned
503 Service Unavailableduring this session; the checks above were run locally instead.Label: security
AI-assisted.