Skip to content

fix(image): bound image-generation provider response reads#96893

Closed
Alix-007 wants to merge 1 commit into
openclaw:mainfrom
Alix-007:fix/bound-image-generation
Closed

fix(image): bound image-generation provider response reads#96893
Alix-007 wants to merge 1 commit into
openclaw:mainfrom
Alix-007:fix/bound-image-generation

Conversation

@Alix-007

@Alix-007 Alix-007 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

On the success path, three image-generation providers read the untrusted
provider response body with an unbounded await response.json(). A hostile or
malfunctioning provider can stream an arbitrarily large body (image responses
routinely carry base64 b64_json payloads), forcing the runtime to buffer the
whole thing into memory before parsing — an OOM / hang vector. The error path
already routes through a bounded reader; only the success path was unbounded.

The unbounded reads were:

  • src/image-generation/openai-compatible-image-provider.ts:272 — the shared
    OpenAI-compatible factory that backs the DeepInfra, LiteLLM, and xAI image
    providers.
  • extensions/openai/image-generation-provider.ts:1015 — OpenAI direct
    generate/edit JSON response.
  • extensions/microsoft-foundry/image-generation-provider.ts:371 — Microsoft
    Foundry MAI image generate/edit JSON response.

(The OpenAI Codex SSE path at line 493 already caps its read and is left
untouched.)

Changes

Replace each success-path await response.json() with the shared
readProviderJsonResponse helper (openclaw/plugin-sdk/provider-http), which
reads through readResponseWithLimit under the existing 16 MiB
(16777216-byte) cap and cancels the stream on overflow. No new abstraction is
introduced; the change is confined to the three provider modules and reuses the
same bounded reader already used by binary/error responses. Malformed-JSON and
empty-response handling is preserved.

Real behavior proof

  • Behavior addressed: unbounded buffering of an untrusted success-path JSON
    body on three image-generation providers.
  • Real environment tested: a loopback node:http TCP server streaming a

    16 MiB JSON body with no Content-Length, driving each real exported
    provider (buildLitellmImageGenerationProvider,
    buildOpenAIImageGenerationProvider,
    buildMicrosoftFoundryImageGenerationProvider) over a real fetch.

  • Exact steps: for each provider, point its configured base URL at the
    loopback server, invoke generateImage, and assert the call rejects with
    exceeds 16777216 bytes, the server socket is aborted, and the bytes the
    server managed to send are far below the full body. A negative control fetches
    the same body with raw response.json() to show it WOULD buffer the full
    payload. Happy-path checks send a small valid JSON image response and assert
    it still parses.
  • Observed result: all three bounded reads threw at the cap with the socket
    aborted after ~16 MiB (bytesSent=16842774, full=17825792); the negative
    control buffered the full 17825792 bytes; all three happy paths parsed a
    small image normally. Full terminal output below.
  • What was not tested: no live calls to the real DeepInfra / OpenAI / xAI /
    Azure Foundry endpoints (the providers are exercised against a local TCP
    server). The multipart edit branches share the same post-response bounded read
    as the generate branches proven here.

Evidence

PASS litellm (shared factory) image success JSON: threw "LiteLLM image generation: JSON response exceeds 16777216 bytes"; socket abort=true; bytesSent=16842774; full=17825792
PASS openai direct image success JSON: threw "OpenAI image generation: JSON response exceeds 16777216 bytes"; socket abort=true; bytesSent=16842774; full=17825792
PASS microsoft-foundry MAI image success JSON: threw "Microsoft Foundry MAI image generation: JSON response exceeds 16777216 bytes"; socket abort=true; bytesSent=16842774; full=17825792
PASS negative control: raw response.json() buffered 17825792 bytes; bytesSent=17825818; cap=16777216
PASS happy path: litellm (shared factory) small JSON parsed normally
PASS happy path: openai direct small JSON parsed normally
PASS happy path: microsoft-foundry MAI small JSON parsed normally
PASS image-generation node:http bounded success-path proof complete

Label: security

AI-assisted.

@clawsweeper

clawsweeper Bot commented Jun 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: the same image-generation bounded-read work is now covered by a broader open canonical PR with maintainer follow-up, proof-sufficient status, and the same affected files plus additional image providers.

Root-cause cluster
Relationship: superseded
Canonical: #96495
Summary: The broader proof-sufficient image-generation bounded-read PR is the safer canonical landing candidate for the same shared/OpenAI/Microsoft Foundry response-read problem.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Close this duplicate branch and concentrate review on #96495 as the canonical bounded image-generation response-read landing path.

So I’m closing this here and keeping the remaining discussion on #96495.

Review details

Best possible solution:

Close this duplicate branch and concentrate review on #96495 as the canonical bounded image-generation response-read landing path.

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

Yes, at source level: current main still has raw success-path response.json() reads on the three provider-controlled image response bodies. I did not execute a live oversized-response repro in this read-only review.

Is this the best way to solve the issue?

No for this PR as the landing branch: the bounded-reader direction is sound, but #96495 already covers the same files with broader image-provider coverage, maintainer follow-up, and proof-sufficient status.

Security review:

Security review cleared: The diff reduces untrusted provider-response memory exposure and does not add dependency, workflow, lockfile, permission, script, credential, or publishing changes.

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • steipete: GitHub path history shows repeated work on the shared OpenAI-compatible image provider and OpenAI image-generation surface, including shared provider extraction and image SSRF propagation. (role: feature introducer and recent area contributor; confidence: high; commits: 719ec4f2927a, 0a09a8f02fcd, 0135a0a7803f; files: src/image-generation/openai-compatible-image-provider.ts, extensions/openai/image-generation-provider.ts)
  • vincentkoc: GitHub path history shows recent image response schema hardening, Microsoft Foundry MAI image provider work, and Codex image stream size hardening near this response-handling area. (role: recent image-provider hardening contributor; confidence: high; commits: eb7a082b777d, d0a84089a07d, 13be16d69987; files: src/image-generation/image-assets.ts, extensions/microsoft-foundry/image-generation-provider.ts, extensions/openai/image-generation-provider.ts)
  • Alix-007: This contributor authored the merged commit that introduced readProviderJsonResponse, the helper reused by this PR, so they are connected to the repair path beyond only opening this PR. (role: shared bounded-reader contributor; confidence: high; commits: 2592f8a51a4e; files: src/agents/provider-http-errors.ts, src/plugin-sdk/provider-http.ts)
  • joshavant: GitHub path history shows recent successful-provider-response bounding work in the same provider HTTP helper surface. (role: recent shared response-reader contributor; confidence: medium; commits: 0a14444924e3; files: src/agents/provider-http-errors.ts, src/plugin-sdk/provider-http.ts)

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

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 26, 2026
@wangmiao0668000666 wangmiao0668000666 mentioned this pull request Jun 26, 2026
Closed
@Alix-007
fix(image): bound image-generation provider response reads
6d26977 
Replace unbounded success-path `await response.json()` reads with the
shared `readProviderJsonResponse` helper (cancels the stream on overflow)
in three image-generation providers:

- src/image-generation/openai-compatible-image-provider.ts (shared factory
  backing DeepInfra/LiteLLM/xAI)
- extensions/openai/image-generation-provider.ts (OpenAI direct JSON)
- extensions/microsoft-foundry/image-generation-provider.ts (MAI image JSON)

A hostile or malfunctioning provider could otherwise stream an arbitrarily
large untrusted body (image responses carry base64 payloads) and force the
runtime to buffer the whole thing before parsing. The error path already
used the bounded reader; this brings the success path in line.

Pass an image-sized 64 MiB cap (`maxBytes`) instead of the generic 16 MiB
provider-JSON default. Inline `b64_json` image responses with multiple
results / high resolution can legitimately exceed 16 MiB, so the generic
default would have failed valid responses closed and regressed existing
users. 64 MiB matches the OpenAI Codex image read budget
(`MAX_CODEX_IMAGE_SSE_BYTES` / `MAX_CODEX_IMAGE_BASE64_CHARS`) in the same
module.

Update the affected provider tests: the closed provider-http mocks now pass
through the real `readProviderJsonResponse` (importActual) and serve real
streamed `Response` bodies, plus focused coverage for a valid large inline
payload that parses under the cap and an oversized body that is rejected at
the cap.
@Alix-007
Alix-007 force-pushed the fix/bound-image-generation branch from ca747a4 to 6d26977 Compare June 26, 2026 10:24
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 26, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 26, 2026
Merged
@sallyom

sallyom commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Thanks for the fix and the clear proof. This is now superseded by #96495, which has merged to main as 527f8f0c and covers these success-path image JSON reads with the canonical cap sizing.

#96495 also carries the broader sibling-provider coverage and SDK surface updates, so closing this PR as superseded.

@sallyom sallyom closed this Jun 26, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 27, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

extensions: openai merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Alix-007 @sallyom