fix(runway): bound video create/poll response reads#96907
Conversation
Rewrite the shared `readRunwayJsonResponse` helper (used by both the create-submit and task-poll success paths) to delegate to the shared byte-bounded `readProviderJsonResponse` (16 MiB cap) instead of an unbounded `await response.json()`, matching the already-merged byteplus/google/qwen video bounds. The download path already uses `readResponseWithLimit`; this closes the remaining unbounded success-path reads so a hostile or buggy endpoint cannot stream an unbounded body and force OpenClaw to buffer it before parsing. The existing isRecord() object guard and the `malformed JSON response` wrapping are preserved. 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:45 AM ET / 14:45 UTC. Summary PR surface: Source -2, Tests -4. Total -6 across 2 files. Reproducibility: yes. at source and proof level. Current main calls response.json() for Runway create and poll success bodies, and the PR body supplies loopback terminal proof contrasting bounded reads with the old unbounded path. 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 the shared-reader Runway fix after maintainer acceptance of the 16 MiB control-JSON cap; add committed oversized create/poll tests only if maintainers want provider-level regression coverage before merge. Do we have a high-confidence way to reproduce the issue? Yes, at source and proof level. Current main calls response.json() for Runway create and poll success bodies, and the PR body supplies loopback terminal proof contrasting bounded reads with the old unbounded path. Is this the best way to solve the issue? Yes, with maintainer compatibility sign-off. Reusing readProviderJsonResponse is the narrowest maintainable fix because it uses the established SDK helper and keeps Runway-specific object validation local; direct use of readProviderJsonObjectResponse would also be plausible but would not materially improve this patch. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 9241b9701d9c. Label changesLabel justifications:
Evidence reviewedPR surface: Source -2, Tests -4. 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
|
clawsweeper
Bot
added
proof: sufficient
|
Updated this branch against latest upstream Verified current head locally:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
3 participants
What Problem This Solves
The Runway video success paths read control JSON with an unbounded
await response.json()inside the sharedreadRunwayJsonResponsehelper, which is used by both the create-submit path and the task-poll path. 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 download path already uses
readResponseWithLimit, so this shared helper was the one remaining unbounded success-path read in the provider. This is a defense-in-depth fix and a sibling-consistency cleanup: in practice the Runway submit/poll responses are small control JSON ({ id },{ id, status, output }), but the read was not bounded the way the rest of this provider and the already-merged byteplus/google/qwen video bounds are. Fixing the shared helper covers both create and poll in one place.Changes
readRunwayJsonResponseto delegate to the sharedreadProviderJsonResponsehelper fromopenclaw/plugin-sdk/provider-http, reusing the existing 16 MiB provider JSON cap; no new abstraction. Its signature changes fromPick<Response, "json">toResponseso the bounded streaming reader can run.isRecord()top-level object guard and the${label}: malformed JSON responsewrapping, and keep both call sites (create-submit and task-poll), the status handling, the output-URL extraction, the download path, andrelease()cleanup unchanged.provider-httptest mock'sreadProviderJsonResponsereader real (viaimportActual) so the streaming size cap is exercised under test instead of stubbed, and update the focused Runway 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 with the provider's label. Small valid JSON must continue to parse.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. Runway'sgenerateVideoentry does not expose a request-level SSRF opt-in (unlike the OpenAI video provider), so it cannot fetch a loopback origin end-to-end. The proof therefore drives the real exportedreadProviderJsonResponse— the exact reader that the rewrittenreadRunwayJsonResponsenow calls — over a real loopback socket, replicating Runway's thinisRecord()wrapper and using Runway's actual create and poll labels, and contrasts it against the old unboundedresponse.json()the helper used before. Node v22.22.0.node --import tsx <scratchpad>/proof.mts— start the streaming server → fetch the streaming create route and run the real bounded reader with label"Runway video generation failed"and assert a bounded throw plus a server-side socket abort near the cap → fetch the streaming poll route and run the reader with label"Runway video status request failed"and assert the same → run a negative control showing the old unboundedresponse.json()(arrayBuffer()+JSON.parse) buffers the whole body past the cap → run the reader against small valid JSON and assert it still parses.Runway video generation failed: JSON response exceeds 16777216 bytesand the server aborted after 19,398,663 bytes; the poll read threwRunway video status request failed: JSON response exceeds 16777216 bytesand the server aborted after 20,054,023 bytes (both just past the cap, far below the ~64 MiB they would have streamed). The negative control buffered the full 67,108,871 bytes, proving the cap is load-bearing. The small happy path parsed and returned the valid JSON through the same reader.generateVideodoes not expose a loopback SSRF opt-in, the TCP proof drives the exact shared reader the helper now uses with Runway's real labels rather than the fullgenerateVideoentry; the provider-wiring (both call sites going through the rewritten helper) is covered by the focused in-repo Vitest regressions using the same reader and error text. The proof script is not committed.Evidence
Real
node:httpterminal proof (real loopback socket, real exported bounded reader, real Runway labels):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.