fix(providers): bound self-hosted provider discovery JSON reads by Alix-007 · Pull Request #95244 · openclaw/openclaw

Alix-007 · 2026-06-20T05:51:24Z

Summary

discoverLlamaCppRuntimeContextTokens and discoverOpenAICompatibleLocalModels in src/plugins/provider-self-hosted-setup.ts parsed their HTTP responses with an unbounded await response.json(). Self-hosted provider base URLs are user-supplied and untrusted — an endpoint reachable via SSRF (or simply a buggy/hostile local server) could stream an unbounded JSON body, driving the setup wizard into OOM. Both reads now go through the shared byte-bounded reader under a single cap before parsing.

This is the symmetric counterpart to #95108, which hardened the Anthropic Messages error-body read with the same @openclaw/media-core bound-stream reader. Here we apply the byte-cap to the two provider-discovery JSON reads.

Changes

Add a shared SELF_HOSTED_DISCOVERY_JSON_MAX_BYTES cap (4 MiB) and a readSelfHostedDiscoveryJson(response, label) helper that reads the body via readResponseWithLimit from @openclaw/media-core/read-response-with-limit (cancels the stream on overflow), then JSON.parses the decoded bytes.
Route both discoverLlamaCppRuntimeContextTokens (/props) and discoverOpenAICompatibleLocalModels (/models) through the bounded reader instead of await response.json(). Overflow throws are swallowed by the existing discovery error handling, so a capped endpoint degrades gracefully: /models returns [], /props skips the runtime context-token probe — no OOM.
Add two regression tests that feed an effectively-unbounded streamed body and assert the stream is cancelled exactly once and the read stays under the cap (instead of being drained/buffered).

Real behavior proof

Behavior addressed: Unbounded await response.json() on untrusted self-hosted discovery endpoints (/models and llama.cpp /props) could buffer an unbounded body into memory (OOM); the reads are now capped at 4 MiB and the stream is cancelled on overflow.

Real environment tested: Local clone on Node v22.22.0; a real loopback http server streaming 1 MiB JSON chunks "forever", driven through the real discoverOpenAICompatibleLocalModels (real fetchWithSsrFGuard + undici network stack) over a real socket. Vitest run via the repo's scripts/run-vitest.mjs.

Exact steps or command run after this patch:

Real-runtime proof (tsx, real socket): node --import tsx scripts/proof-bound-self-hosted-discovery.mts — starts an unbounded-JSON loopback server and calls the real discovery function against it.
Regression tests: node scripts/run-vitest.mjs src/plugins/provider-self-hosted-setup.test.ts --run
Lint: oxlint src/plugins/provider-self-hosted-setup.ts src/plugins/provider-self-hosted-setup.test.ts

Evidence after fix:

[proof] unbounded JSON server on http://127.0.0.1:32865/v1
[proof] byte cap under test = 4 MiB (4194304 bytes)
[plugins/self-hosted-provider-setup] Failed to discover proof-vllm models: Error: proof-vllm discovery response body too large: 4256527 bytes (limit: 4194304 bytes)

=== READBACK (after fix) ===
returned models           : []
server bytes flushed      : 5242880 (5.0 MiB)
server saw early abort    : true     <- reader cancelled the stream
elapsed                   : 259ms
RESULT: PASS — discovery read was bounded then the reader cancelled the stream. No OOM.

Vitest: Test Files 1 passed (1) / Tests 12 passed (12). oxlint: clean.

Observed result after fix: The unbounded body is capped at 4 MiB — the loopback server is hung up on (early abort) after ~5 MiB, the overflow error fires, and discovery returns [] instead of buffering. Negative control: reverting the source to await response.json() and re-running the two new tests fails with cancelCount expected 1, received 0 (the unbounded read never cancels and drained for ~200s instead of being capped), confirming the tests genuinely catch the bug.

What was not tested: A live production SSRF chain to a real malicious provider (the threat model is reproduced with a local unbounded-stream server). Full-project tsc could not run to completion on this constrained host (heap OOM in the type checker itself, unrelated to this change); the changed files type-check cleanly under a scoped check and the vitest transform pipeline.

AI-assisted: this change was prepared with AI assistance (analysis, implementation, and proof authoring); all code, tests, and the proof were reviewed and the proof executed locally by the contributor.

clawsweeper · 2026-06-20T05:52:48Z

Codex review: needs real behavior proof before merge. Reviewed June 24, 2026, 2:55 PM ET / 18:55 UTC.

Summary
Bounds the self-hosted provider /models and llama.cpp /props JSON reads with a 16 MiB readResponseWithLimit cap and adds stream-cancellation regression tests.

PR surface: Source +28, Tests +105. Total +133 across 2 files.

Reproducibility: yes. Current main and v2026.6.10 still call response.json() on the two untrusted success bodies, and the PR body describes a real loopback server streaming JSON chunks through the discovery path; I did not rerun that live proof in this read-only review.

Review metrics: 1 noteworthy metric.

Self-hosted discovery cap: 1 hard cap added: 16 MiB for /models and /props success JSON reads. This is the compatibility-sensitive behavior change maintainers should notice before merge because oversized discovery responses now fail soft instead of being parsed.

Stored data model
Persistent data-model change detected: serialized state: src/plugins/provider-self-hosted-setup.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: canonical
Canonical: #95244
Summary: This PR is the canonical open item for bounding self-hosted provider discovery JSON reads; the referenced Anthropic PR is adjacent hardening, not the same remaining work.

Members:

adjacent_distinct: fix(agents): bound Anthropic error streams #95108 - That merged PR bounded Anthropic error-body diagnostics, while this PR bounds separate self-hosted provider discovery success responses.

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

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

[P1] Add or link current-head loopback output showing the 16 MiB overflow cancellation and fail-soft discovery result.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: The PR includes terminal/live-output proof for the earlier 4 MiB cap and current tests after the cap bump, but not a current-head real loopback proof for the final 16 MiB behavior. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

[P1] A self-hosted endpoint that legitimately returns more than 16 MiB of /models or /props JSON will now fail discovery or skip the runtime-token probe instead of being parsed, so maintainers should explicitly accept that compatibility tradeoff.
[P1] The PR body's real loopback proof output still describes the original 4 MiB cap, while the current head uses 16 MiB; current-head proof should be refreshed or consciously accepted as equivalent before merge.

Maintainer options:

Refresh Final-Cap Proof (recommended)
Ask for a current-head loopback proof showing the 16 MiB cap cancels an unbounded discovery stream and preserves fail-soft /models and /props behavior.
Accept Existing Proof And Cap
A maintainer can accept the earlier loopback proof plus current 16 MiB regression tests as enough because the cap threshold changed but the bounded-reader path did not.
Retune The Discovery Budget
If maintainers expect legitimate self-hosted discovery JSON above 16 MiB, retune the cap to the intended provider JSON budget and keep the same bounded-reader shape.

Next step before merge

[P1] No automated code repair is indicated; merge needs maintainer handling for the current-head proof gap and the 16 MiB compatibility tradeoff.

Security
Cleared: The diff reduces untrusted response-buffering risk with an existing workspace helper and adds no dependencies, workflows, package metadata, secret handling, or new code-execution surface.

Review details

Best possible solution:

Merge the bounded-reader patch after current-head proof is refreshed or explicitly accepted for the 16 MiB cap; do not close it as superseded by the Anthropic-only hardening PR.

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

Yes. Current main and v2026.6.10 still call response.json() on the two untrusted success bodies, and the PR body describes a real loopback server streaming JSON chunks through the discovery path; I did not rerun that live proof in this read-only review.

Is this the best way to solve the issue?

Yes, with a proof/compatibility caveat. Applying the existing bounded reader at the exact /models and /props reads is the narrowest maintainable fix; the remaining question is accepting or refreshing proof for the final 16 MiB fail-soft budget.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This is a normal-priority provider hardening bug fix with limited surface area in self-hosted setup/discovery.
merge-risk: 🚨 compatibility: The new hard cap can change existing self-hosted setups whose discovery endpoints return more than 16 MiB of valid JSON.
rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR includes terminal/live-output proof for the earlier 4 MiB cap and current tests after the cap bump, but not a current-head real loopback proof for the final 16 MiB behavior. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +28, Tests +105. Total +133 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	30	2	+28
Tests	1	105	0	+105
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	135	2	+133

What I checked:

current main still has unbounded discovery reads: Current origin/main still parses both successful self-hosted discovery responses with response.json(): llama.cpp /props and OpenAI-compatible /models. (src/plugins/provider-self-hosted-setup.ts:136, b6bc3ed0dbd8)
latest release still has the same behavior: The latest release tag v2026.6.10 also uses unbounded response.json() on both affected success bodies, so this is not already shipped fixed behavior. (src/plugins/provider-self-hosted-setup.ts:136, aa69b12d0086)
PR head applies the shared bounded reader: The PR head adds a 16 MiB self-hosted discovery JSON cap and routes both /props and /models parsing through readSelfHostedDiscoveryJson. (src/plugins/provider-self-hosted-setup.ts:47, d84f514a1f17)
bounded reader dependency cancels on overflow: readResponseWithLimit reads stream chunks up to the byte cap, cancels the reader when the next chunk would exceed the cap, and throws the configured overflow error. (packages/media-core/src/read-response-with-limit.ts:96, b6bc3ed0dbd8)
PR tests cover the two affected success bodies: The added tests create a never-ending 1 MiB chunk stream and assert cancellation for both /models and llama.cpp /props while preserving fail-soft discovery behavior. (src/plugins/provider-self-hosted-setup.test.ts:477, d84f514a1f17)
related PR is adjacent, not superseding: The related merged PR bounded Anthropic Messages error-body diagnostics, but it did not touch self-hosted provider discovery success bodies. (d6cefe26f499)

Likely related people:

vincentkoc: Recent main history includes self-hosted probe body cancellation work in this file and the adjacent merged Anthropic stream-boundary PR using the same bounded-reader pattern. (role: recent area contributor; confidence: high; commits: eb7da0a2e558, d6cefe26f499; files: src/plugins/provider-self-hosted-setup.ts, src/plugins/provider-self-hosted-setup.test.ts, src/agents/anthropic-transport-stream.ts)
steipete: History shows provider seam extraction, self-hosted discovery guarding, and llama.cpp discovery changes on the same provider setup path. (role: provider seam and refactor contributor; confidence: high; commits: 4ca07559abe4, e862e0acb577, 29f36e007298; files: src/plugins/provider-self-hosted-setup.ts, src/plugin-sdk/provider-setup.ts, packages/media-core/src/read-response-with-limit.ts)
brokemac79: Merged commits added and refined llama.cpp runtime context discovery behavior around the /props read that this PR now bounds. (role: adjacent llama.cpp discovery contributor; confidence: medium; commits: 7c7d19ec8481, f4be39c4f452; files: src/plugins/provider-self-hosted-setup.ts, src/plugins/provider-self-hosted-setup.test.ts)
rendrag-git: Merged commits added self-hosted provider wildcard discovery behavior in the same model discovery surface that this PR hardens. (role: self-hosted discovery contributor; confidence: medium; commits: 2269ec727ff9, 3b361cf51c12; files: src/plugins/provider-self-hosted-setup.ts)
sallyom: This person pushed the PR-head tweak that raised the proposed cap from 4 MiB to 16 MiB and documented the compatibility rationale in the PR discussion. (role: current PR cap-tuning reviewer; confidence: medium; commits: d84f514a1f17; files: src/plugins/provider-self-hosted-setup.ts, src/plugins/provider-self-hosted-setup.test.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.

discoverLlamaCppRuntimeContextTokens and discoverOpenAICompatibleLocalModels parsed their HTTP responses via an unbounded await response.json(). Self-hosted provider base URLs are user-supplied and untrusted (an endpoint reachable via SSRF could stream an unbounded JSON body), so a hostile or buggy endpoint could drive the setup wizard into OOM. Route both reads through the shared byte-bounded reader (readResponseWithLimit from @openclaw/media-core) under a single 4 MiB cap before JSON.parse, mirroring the bound-stream hardening landed for Anthropic error bodies. Overflow cancels the stream and is swallowed by the existing discovery error handling, so a capped endpoint degrades gracefully (returns [] / skips the runtime context probe) instead of buffering the whole body.

Signed-off-by: sallyom <[email protected]>

sallyom · 2026-06-24T18:31:53Z

Merge-ready from my review.

I pushed one maintainer tweak in d84f514 to raise SELF_HOSTED_DISCOVERY_JSON_MAX_BYTES from 4 MiB to 16 MiB. The reason is compatibility: the other response-body cap fixes in this campaign use the established 16 MiB provider JSON budget, and this avoids making self-hosted setup/discovery stricter than the sibling provider JSON paths while still preventing unbounded buffering/OOM behavior.

This PR is not superseded by #95108. #95108 bounded Anthropic error streams; this PR still targets the separate self-hosted provider discovery reads in src/plugins/provider-self-hosted-setup.ts, and current main still needs this owner-path fix.

Local proof after the cap bump:

node scripts/run-vitest.mjs src/plugins/provider-self-hosted-setup.test.ts --run passed.
Local autoreview against origin/main was clean: no accepted/actionable findings.

Best-fix verdict: still the right narrow fix. The PR applies the existing capped response reader to the exact untrusted discovery reads, preserves the existing fail-soft behavior, and now uses the same 16 MiB budget as the established provider JSON cap pattern.

sallyom · 2026-06-24T18:48:02Z

@clawsweeper re-review

clawsweeper · 2026-06-24T18:48:05Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

sallyom · 2026-06-24T21:50:15Z

merge-ready, the real-socket proof with 4MiB cap remains valid for the bounded-reader/cancel behavior; the current-head focused tests cover the updated 16 MiB threshold.

…claw#95244) * fix(providers): bound self-hosted discovery JSON reads discoverLlamaCppRuntimeContextTokens and discoverOpenAICompatibleLocalModels parsed their HTTP responses via an unbounded await response.json(). Self-hosted provider base URLs are user-supplied and untrusted (an endpoint reachable via SSRF could stream an unbounded JSON body), so a hostile or buggy endpoint could drive the setup wizard into OOM. Route both reads through the shared byte-bounded reader (readResponseWithLimit from @openclaw/media-core) under a single 4 MiB cap before JSON.parse, mirroring the bound-stream hardening landed for Anthropic error bodies. Overflow cancels the stream and is swallowed by the existing discovery error handling, so a capped endpoint degrades gracefully (returns [] / skips the runtime context probe) instead of buffering the whole body. * tune self-hosted discovery cap Signed-off-by: sallyom <[email protected]> --------- Signed-off-by: sallyom <[email protected]> Co-authored-by: sallyom <[email protected]>

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

Alix-007 force-pushed the fix/bound-self-hosted-discovery-json branch from b660a25 to 374fa36 Compare June 23, 2026 13:13

tune self-hosted discovery cap

d84f514

Signed-off-by: sallyom <[email protected]>

sallyom merged commit dad5ce6 into openclaw:main Jun 24, 2026
114 of 120 checks passed

github-actions Bot mentioned this pull request Jun 24, 2026

📡 Upstream Digest — 2026-06-24 22:59 UTC curtismercier/openclaw-mods#1148

Open

wangmiao0668000666 mentioned this pull request Jun 25, 2026

fix(anthropic): bound streaming 200 success-body SSE reads via shared plugin-sdk guard #96632

Closed

clawsweeper Bot mentioned this pull request Jun 29, 2026

fix(plugins): bound self-hosted LLM provider probe response reads to prevent OOM #97779

Closed

clawsweeper Bot mentioned this pull request Jul 4, 2026

fix(plugins): wrap self-hosted discovery JSON parse in try/catch #99892

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(providers): bound self-hosted provider discovery JSON reads#95244

fix(providers): bound self-hosted provider discovery JSON reads#95244
sallyom merged 2 commits into
openclaw:mainfrom
Alix-007:fix/bound-self-hosted-discovery-json

Alix-007 commented Jun 20, 2026

Uh oh!

clawsweeper Bot commented Jun 20, 2026 •

edited

Loading

Uh oh!

sallyom commented Jun 24, 2026 •

edited

Loading

Uh oh!

sallyom commented Jun 24, 2026

Uh oh!

clawsweeper Bot commented Jun 24, 2026 •

edited

Loading

Uh oh!

sallyom commented Jun 24, 2026

Uh oh!

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!

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

Uh oh!

Uh oh!

sallyom commented Jun 24, 2026

Uh oh!

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

Uh oh!

Uh oh!

sallyom commented Jun 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jun 20, 2026 •

edited

Loading

sallyom commented Jun 24, 2026 •

edited

Loading

clawsweeper Bot commented Jun 24, 2026 •

edited

Loading