fix(plugin-sdk): bound live model catalog success body by Alix-007 · Pull Request #95246 · openclaw/openclaw

Alix-007 · 2026-06-20T05:58:05Z

Summary

fetchLiveProviderModelRows reads a live model catalog from a provider-controlled, runtime-fetched endpoint — the response body is untrusted just like an error body. The error path was already hardened (cancelUnreadResponseBody only runs on !response.ok), but the success path at the JSON read did await response.json() with no ceiling. A faulty or hostile provider can stream an effectively unbounded JSON document, so a single discovery call can exhaust process memory. This is the symmetric counterpart to the bounded-stream hardening landed for Anthropic error streams in #95108.

This patch reads the success body through the canonical byte-cap reader (readResponseWithLimit) before JSON.parse, cancelling the stream on overflow or idle stall.

Changes

src/plugin-sdk/provider-catalog-live-runtime.ts
- Import readResponseWithLimit from @openclaw/media-core/read-response-with-limit (same helper used by provider-http-errors, clawhub, media fetch).
- Add LIVE_MODEL_CATALOG_BODY_MAX_BYTES = 4 * 1024 * 1024 with a comment explaining the untrusted-runtime-body rationale.
- Add readLiveModelCatalogJson(response, timeoutMs) that reads the body under the byte cap (throwing a clear overflow error) and idle-stall timeout (reusing the call's existing timeoutMs), then JSON.parses the bounded buffer.
- Rewire the success branch from await response.json() to await readLiveModelCatalogJson(response, timeoutMs).
src/plugin-sdk/provider-catalog-live-runtime.test.ts
- Add a regression test that streams an unbounded success body and asserts the overflow error fires, the stream is cancelled, and the guard is released once.
- Add a regression test that asserts a stalled success stream is aborted with the idle-timeout error.

Cap rationale (why 4 MiB, not the 8 KiB used in #95108): 8 KiB is an error-snippet budget — correct for tiny provider error messages, but it would silently break legitimate catalogs (OpenRouter's live /v1/models is already >100 KB and grows; OpenAI's is tens of KB). The repo's existing model-catalog precedent caps at 1 MiB (QA_CHILD_STDOUT_MAX_BYTES); 4 MiB gives generous headroom for the largest known catalogs while still bounding memory, which is the actual vulnerability being closed. An overflow degrades gracefully — direct callers (buildLiveModelProviderConfig and the per-provider discovery wrappers) already try/catch and fall back to static catalogs.

Real behavior proof

Label: success-path catalog body bounding

Behavior addressed: the live model catalog success body was read via unbounded response.json(); the success stream was never cancelled, so a provider streaming an unbounded JSON document could exhaust process memory.
Real environment tested: Node 22 (v22.22.0), node --import tsx driving the real exported fetchLiveProviderModelRows read path against a Response wrapping a ReadableStream that emits a valid JSON array prefix then an effectively endless run of 64 KiB padding chunks; the stream is instrumented to count bytes actually pulled and whether its cancel() fires. Plus vitest on the file and oxlint/tsc.
Exact steps or command run after this patch:
1. tsx real-runtime harness driving fetchLiveProviderModelRows with the oversized stream and a release spy.
2. Negative control: temporarily revert the source line back to await response.json() and re-run the same harness.
3. node scripts/run-vitest.mjs src/plugin-sdk/provider-catalog-live-runtime.test.ts --run
4. npx oxlint <changed files> and tsc -p tsconfig.json --noEmit.

Evidence after fix (tsx readback):

error.message   : Live model catalog response exceeded 4194304 bytes (4194339 bytes received)
stream.cancelled: true
bytesPulled     : 4259875 bytes   (~4.06 MiB)
totalAvailable  : 13107200 bytes  (~12.5 MiB an unbounded reader would drain)
release calls   : 1
heap delta      : ~3.3 MiB

Negative control (source reverted to await response.json()):

stream.cancelled: false
bytesPulled     : 13041699 bytes  (drained the ENTIRE oversized stream — no bound)
heap delta      : ~15.5 MiB       (memory grows with the malicious payload)

vitest: Test Files 1 passed (1) / Tests 9 passed (9) (7 pre-existing + 2 new). oxlint exit 0; tsc reports no error TS for the changed file.

Observed result after fix: the success body is read under a 4 MiB ceiling, the upstream stream is cancelled on overflow, the guard is released exactly once, and the call surfaces a clear overflow error that callers already handle by falling back to static catalogs. Idle-stall is also bounded by the existing timeoutMs.
What was not tested: a real malicious upstream provider over the network (drove the real read path with an injected fetch guard instead); the SSRF guard / DNS lookup path (unchanged); other providers' catalog projection logic (unchanged — only the byte-read boundary moved).

AI-assisted: implemented and verified with AI assistance; the bounded-stream approach mirrors the symmetric error-body hardening in #95108 and reuses the canonical readResponseWithLimit helper rather than introducing a new abstraction.

clawsweeper · 2026-06-20T05:59:37Z

Codex review: needs maintainer review before merge. Reviewed June 21, 2026, 7:41 AM ET / 11:41 UTC.

Summary
The branch routes fetchLiveProviderModelRows success JSON through readResponseWithLimit with a 4 MiB cap and idle timeout, plus overflow and stall regression tests.

PR surface: Source +24, Tests +85. Total +109 across 2 files.

Reproducibility: yes. Current main calls response.json() on a provider-controlled live catalog success body, and the PR body includes a Node 22 runtime harness plus negative control showing the old path drains an oversized stream while the patched path cancels near the cap.

Review metrics: 1 noteworthy metric.

SDK live catalog body limit: 1 added: 4 MiB success-body cap. The PR creates a new public plugin SDK runtime limit that maintainers should notice before merge.

Stored data model
Persistent data-model change detected: serialized state: src/plugin-sdk/provider-catalog-live-runtime.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

[P1] A legitimate third-party live catalog above 4 MiB will now error or fall back instead of being fully buffered; this is the right hardening shape, but it is still a public SDK runtime compatibility decision.

Maintainer options:

Accept the 4 MiB live-catalog cap (recommended)
Merge as-is if maintainers accept that SDK live catalog success bodies above 4 MiB now fail or fall back instead of being fully buffered.
Adjust or document the cap first
Change the limit or add SDK documentation if maintainers want a different public live-catalog size contract before release.
Pause for catalog-size evidence
Hold the PR if maintainers want fresh live checks against known large provider catalogs before accepting the permanent limit.

Next step before merge

[P2] Maintainers need to accept the public SDK cap or request a different documented limit; there is no narrow automated repair indicated.

Security
Cleared: The diff hardens an existing provider-controlled response-body read and does not add dependencies, workflows, lockfile changes, package hooks, or new secret-handling surface.

Review details

Best possible solution:

Land the bounded success-body read after maintainers accept 4 MiB as the SDK live-catalog limit, or adjust/document the cap before merge if they want a different public contract.

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

Yes. Current main calls response.json() on a provider-controlled live catalog success body, and the PR body includes a Node 22 runtime harness plus negative control showing the old path drains an oversized stream while the patched path cancels near the cap.

Is this the best way to solve the issue?

Yes, with a maintainer-owned compatibility choice. Reusing readResponseWithLimit at the live-catalog read boundary is the narrow maintainable fix; the remaining decision is whether 4 MiB is the accepted public SDK limit.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This is focused provider-catalog memory hardening with limited blast radius and no evidence of an urgent live outage.
merge-risk: 🚨 compatibility: The PR changes public plugin SDK runtime behavior by rejecting or falling back on live catalog success bodies above 4 MiB.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix live terminal output from a Node 22 harness against the exported read path plus a negative control showing the old unbounded response.json() path.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live terminal output from a Node 22 harness against the exported read path plus a negative control showing the old unbounded response.json() path.

Evidence reviewed

PR surface:

Source +24, Tests +85. Total +109 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	26	2	+24
Tests	1	85	0	+85
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	111	2	+109

What I checked:

Current main unbounded success read: Current main still calls await response.json() on successful live provider catalog responses after the SSRF-guarded fetch, so provider-controlled success bodies are buffered without an explicit byte cap before row projection. (src/plugin-sdk/provider-catalog-live-runtime.ts:157, d1cbe29f3d52)
Patch bounds the success body: The PR adds LIVE_MODEL_CATALOG_BODY_MAX_BYTES = 4 * 1024 * 1024, reads the response through readResponseWithLimit, applies the existing timeout as chunkTimeoutMs, and parses JSON only after the capped read succeeds. (src/plugin-sdk/provider-catalog-live-runtime.ts:146, 211e80e3814f)
Bounded-reader contract: readResponseWithLimit reads through readResponsePrefix, cancels the stream on overflow or idle timeout, releases the reader lock, and throws caller-provided overflow/idle errors. (packages/media-core/src/read-response-with-limit.ts:129, d1cbe29f3d52)
Regression coverage in PR: The PR adds tests that assert an oversized live catalog success stream is cancelled after overflow and a stalled success stream aborts with the configured idle-timeout error while guarded fetch release still runs once. (src/plugin-sdk/provider-catalog-live-runtime.test.ts:160, 211e80e3814f)
Public SDK boundary: The scoped plugin SDK policy identifies this directory as the public contract between plugins and core, and the package exports ./plugin-sdk/provider-catalog-live-runtime, so changing the helper's runtime body limit is compatibility-sensitive. (src/plugin-sdk/AGENTS.md:1, d1cbe29f3d52)
Documented live catalog contract: The public provider-plugin docs describe provider-catalog-live-runtime as the shared guarded fetch lifecycle for /models-style discovery with TTL caching and static fallback behavior. Public docs: docs/plugins/sdk-provider-plugins.md. (docs/plugins/sdk-provider-plugins.md:208, d1cbe29f3d52)

Likely related people:

fuller-stack-dev: Authored the merged live provider model catalog SDK helper and related provider-routing work that introduced this success-path JSON read. (role: introduced behavior; confidence: high; commits: 57e0bdaabe0a, 60d1dbd77d43, c6905732fd66; files: src/plugin-sdk/provider-catalog-live-runtime.ts, src/plugin-sdk/provider-catalog-live-runtime.test.ts, docs/plugins/sdk-provider-plugins.md)
steipete: Merged the original live catalog helper PR and authored follow-up fixes around live catalog auth markers, empty-result retries, and SDK API baseline refreshes. (role: merger and follow-up contributor; confidence: high; commits: 57e0bdaabe0a, b784ec0370c4, 3a922600bd47; files: src/plugin-sdk/provider-catalog-live-runtime.ts, src/plugin-sdk/provider-catalog-live-runtime.test.ts, extensions/openai/openai-provider.ts)
vincentkoc: Recently maintained live catalog error-body cancellation and adjacent bounded provider response-stream hardening, making him relevant for response-limit review. (role: recent adjacent hardening contributor; confidence: medium; commits: 7f38b1a9107f, d6cefe26f499; files: src/plugin-sdk/provider-catalog-live-runtime.ts, src/agents/anthropic-transport-stream.ts, packages/media-core/src/read-response-with-limit.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

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 keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

steipete · 2026-06-22T16:30:02Z

Land-ready verification for 83676051f9d7df2f293994c8d78c159a5afdf3dd.

Maintainer improvements:

retained the 4 MiB bounded stream read and idle timeout;
preserved Response.json() compatibility for UTF-8 BOM-prefixed JSON with TextDecoder;
kept overflow/stall cancellation and guarded-fetch release semantics.

Proof:

node scripts/run-vitest.mjs run src/plugin-sdk/provider-catalog-live-runtime.test.ts — 1 file, 10 tests passed.
Official OpenRouter live catalog — 496,778 bytes / 339 models, below the 4 MiB cap.
oxfmt --check on touched files and git diff --check — passed.
Fresh Codex autoreview — no actionable findings.
Exact-head full CI run 27966792754 — success.

Known proof gaps: none.

github-actions · 2026-06-22T16:39:03Z

Dependency graph guard cleared

This PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh /allow-dependencies-change comment after the guard blocks that new head SHA.

Current SHA: 80805ad7a5836a5e4fdfd1c2b5be51901a715525

steipete · 2026-06-22T16:55:32Z

Closed via the maintainer replacement #95827, landed in 303e7781c1f5b80f860ab4e8bbb2b04e5b005340.

The original fork branch could not be safely brought forward after its large branch-sync operation failed, so I recreated the reviewed patch as one maintainer commit and preserved @Alix-007 with a Co-authored-by trailer. The replacement includes the bounded-read, timeout/cancellation, cleanup, BOM, unit, live API, autoreview, and exact-head CI proof.

Thank you, @Alix-007. For future PRs, keeping Allow edits by maintainers enabled helps us apply review fixups directly when GitHub's branch synchronization path permits it.

Canonical landed PR: #95827

Alix-007 · 2026-06-22T23:56:53Z

Thanks for the heads-up, @steipete! Confirmed — all my open PRs now have Allow edits by maintainers enabled, so you can apply review fixups directly from here on. Appreciate you landing this one cleanly and recreating it 🙏

openclaw-barnacle Bot added the size: S label Jun 20, 2026

Alix-007 mentioned this pull request Jun 20, 2026

fix(agents): bound OpenRouter model-scan catalog success body #95418

Merged

steipete self-assigned this Jun 22, 2026

steipete requested review from a team as code owners June 22, 2026 16:37

clawsweeper Bot mentioned this pull request Jun 22, 2026

fix(agents): bound OpenRouter model catalog response reads #95420

Merged

steipete closed this Jun 22, 2026

steipete force-pushed the fix/bound-provider-catalog-live-json branch from 8748987 to 80805ad Compare June 22, 2026 16:38

steipete mentioned this pull request Jun 22, 2026

fix(plugin-sdk): bound live model catalog success body #95827

Merged

This was referenced Jun 23, 2026

fix(video-generation): bound dashscope task response reads #96036

Closed

fix(image-generation): bound OpenAI-compatible image response reads #96136

Closed

fix(video-generation): bound OpenAI video submitted response reads #96144

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(plugin-sdk): bound live model catalog success body#95246

fix(plugin-sdk): bound live model catalog success body#95246
Alix-007 wants to merge 0 commit into
openclaw:mainfrom
Alix-007:fix/bound-provider-catalog-live-json

Alix-007 commented Jun 20, 2026

Uh oh!

clawsweeper Bot commented Jun 20, 2026 •

edited

Loading

Uh oh!

steipete commented Jun 22, 2026

Uh oh!

github-actions Bot commented Jun 22, 2026 •

edited

Loading

Uh oh!

steipete commented Jun 22, 2026

Uh oh!

Alix-007 commented Jun 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

Alix-007 commented Jun 20, 2026

Summary

Changes

Real behavior proof

Uh oh!

clawsweeper Bot commented Jun 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jun 22, 2026

Uh oh!

github-actions Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency graph guard cleared

Uh oh!

steipete commented Jun 22, 2026

Uh oh!

Alix-007 commented Jun 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jun 20, 2026 •

edited

Loading

github-actions Bot commented Jun 22, 2026 •

edited

Loading