fix(fal): bound JSON response reads to prevent OOM#96321
Conversation
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Keep open: the fal hardening target is valid, but this PR is not merge-ready because it reimplements the existing shared provider JSON reader, adds new fail-closed 1 MiB success-response caps without fal payload-size proof, and still lacks contributor real behavior proof. Canonical path: Close this PR as superseded by #96495. So I’m closing this here and keeping the remaining discussion on #96495. Review detailsBest possible solution: Close this PR as superseded by #96495. Do we have a high-confidence way to reproduce the issue? Yes. Current main calls Is this the best way to solve the issue? No, not as written. Bounding the reads is the right direction, but the best OpenClaw shape is to route through the existing provider JSON helper instead of creating a second inline JSON-reader policy in fal. Security review: Security review cleared: The diff hardens untrusted provider response reads and adds no dependencies, workflow permissions, secret handling, or downloaded-code execution surface. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 643410c1f3c0. |
clawsweeper
Bot
added
rating: 🧂 unranked krab
Replace unbounded await response.json() with readResponseWithLimit (1 MiB cap) in video/music/image generation providers. A malicious or faulty fal API endpoint returning an oversized JSON response body would previously buffer the entire body in memory before parsing. readResponseWithLimit cancels the stream once the cap is reached, preventing unbounded buffering. - video-generation-provider: bound fetchFalJson (central JSON fetcher) - music-generation-provider: bound generateMusic response read - image-generation-provider: bound generateImage response read Each path uses 1 MiB cap — fal JSON API responses (queue status, model results) are small metadata payloads. Co-Authored-By: Claude <[email protected]>
14d7432 to
6c6273b
Compare
|
Closing to bring open PR count within fixer limit; remaining bound-JSON PRs cover the same pattern. |
1 participant
What Problem This Solves
Replace unbounded
await response.json()withreadResponseWithLimit(1 MiB cap)in fal video/music/image generation providers — the symmetric counterpart to the
bound-stream work already merged for OpenRouter (#95420, #95418), Google (#95417),
provider-http (#95218), and other modules.
A malicious or faulty fal API endpoint returning an oversized JSON response body
would previously buffer the entire body in memory before parsing.
readResponseWithLimitcancels the stream once the cap is reached, preventingunbounded buffering. These modules already use
readResponseWithLimitforvideo/image downloads, but the JSON API response paths were still unbounded.
Evidence
video-generation-provider.test.ts(18 tests),music-generation-provider.test.ts(6 tests),image-generation-provider.test.ts(25 tests)oxlintexits 0 on changed files;git diff --checkclean;oxfmtproduces no deltaawait response.json()would allow an oversized streamed 200-OK body tobuffer without bound
JSON.parsepattern establishedby fix(agents): bound OpenRouter model catalog response reads #95420, fix(agents): bound Google prompt cache response reads #95417, fix(agents): bound OpenRouter model-scan catalog success body #95418, fix(agents): bound provider JSON response reads #95218 — 1 MiB cap is safe for fal metadata payloads
Summary
Bound 3 unbounded JSON response reads in fal video/music/image generation
providers with 1 MiB cap.
Changes
extensions/fal/video-generation-provider.ts— boundfetchFalJson(centralJSON fetcher covering queue submit, status polling, and result retrieval)
extensions/fal/music-generation-provider.ts— boundgenerateMusicJSONresponse read
extensions/fal/image-generation-provider.ts— boundgenerateImageJSONresponse read
extensions/fal/video-generation-provider.test.ts— updatedreleasedJsonhelper and non-JSON mock to include
arrayBuffer()forreadResponseWithLimitextensions/fal/music-generation-provider.test.ts— addedmakeJsonResponsehelper providing
arrayBuffer()on mock Response objects