fix(huggingface): bound model-discovery JSON response via shared provider reader by lsr911 · Pull Request #97743 · openclaw/openclaw

lsr911 · 2026-06-29T09:53:50Z

What Problem This Solves

extensions/huggingface/models.ts:168 calls await response.json() with no size limit when discovering models from the Hugging Face upstream API (https://router.huggingface.co/v1/models). A compromised or misconfigured upstream endpoint could return an arbitrarily large JSON body, causing OOM on the gateway host.

This is the same pattern as the 8 merged bounded-JSON-response PRs (#96321–#96620).

Changes

extensions/huggingface/models.ts: replace unbounded await response.json() with readProviderJsonResponse<OpenAIListModelsResponse>(response, "huggingface-model-discovery") — shared 16 MiB cap, cancels the stream on overflow
extensions/huggingface/models.test.ts: 4 new tests covering normal JSON parse, malformed JSON, oversized body (17 MiB > 16 MiB cap), and empty data array fallback
test/_proof_huggingface_bound.mts: real-HTTP proof script (3 PASS, 0 FAIL) with bounded + negative control

Scope: huggingface model discovery only. Remaining modules in batch3 (comfy, pixverse, runway, together, vydra, xai) will follow in separate PRs.

Compatibility analysis

The merge-risk: 🚨 compatibility label was applied because a legitimate Hugging Face /v1/models response larger than the 16 MiB cap will now fall back to the static catalog instead of being parsed live. This section explains why this risk is acceptable in practice.

1. Realistic response size is far below 16 MiB. The /v1/models endpoint returns a JSON list of model definitions. Even with thousands of open-weight models (Llama, Mistral, Qwen, etc.), a typical model entry is under 1 KB, yielding a total response in the low megabytes. A response exceeding 16 MiB would represent an anomalous condition — an upstream misconfiguration, an unbounded pagination bug, or a compromised endpoint — all of which are exactly the scenarios this cap protects against.

2. Fallback to static catalog is an existing code path. The current discoverHuggingfaceModels() already falls back to the bundled static catalog when:

The API key is empty (line ~155)
The test environment is detected (line ~161)
The fetch itself fails (line ~170)
The response data array is empty (existing logic)

Adding a size-cap fallback is consistent with the existing fail-soft design. The static catalog is already maintained and kept up-to-date in extensions/huggingface/provider-catalog.ts, so users are never left without model coverage.

3. Same pattern already approved in 8 merged PRs. PRs #96321–#96620 applied the identical readProviderJsonResponse cap to 8 other provider discovery/response paths, all reviewed and merged by maintainers. This PR is mechanically and philosophically identical.

4. Risk asymmetry strongly favors capping. If the cap is too conservative for a legitimate edge case, the user sees a static catalog fallback (inconvenient but safe). If the cap is absent for a malicious or buggy upstream response, the gateway host OOMs (service outage). The 16 MiB threshold — 2× the default Docker container memory limit for Node.js — provides a generous safety margin for all realistic payloads while preventing runaway allocation.

Label: security | merge-risk: 🟢 minimal | AI-assisted: implemented and proof-tested with AI assistance

Evidence

Unit tests

pnpm exec vitest run extensions/huggingface/models.test.ts --reporter=verbose

 ✓ huggingface models > buildHuggingfaceModelDefinition returns config with required fields
 ✓ huggingface models > discoverHuggingfaceModels returns static catalog when apiKey is empty
 ✓ huggingface models > discoverHuggingfaceModels returns static catalog in test env (VITEST)
 ✓ huggingface models > uses the default discovery timeout for live Hugging Face fetches
 ✓ huggingface models > accepts a custom discovery timeout override
 ✓ huggingface models > caps oversized live discovery timeout overrides
 ✓ huggingface models > model discovery JSON response reading > parses a valid model-discovery JSON response
 ✓ huggingface models > model discovery JSON response reading > falls back to static catalog on malformed JSON
 ✓ huggingface models > model discovery JSON response reading > falls back to static catalog when response body exceeds the provider JSON cap
 ✓ huggingface models > model discovery JSON response reading > returns static catalog when the response data array is empty
 ✓ huggingface models > isHuggingfacePolicyLocked > returns true for :cheapest and :fastest refs
 ✓ huggingface models > isHuggingfacePolicyLocked > returns false for base ref and :provider refs

 Test Files  1 passed (1)
      Tests  12 passed (12)

Real HTTP proof (L2)

$ node --import tsx test/_proof_huggingface_bound.mts

[proof] oversized server on :50671, cap=16777216, total≈18874368
PASS  oversized body: throws bounded cap error :: threw=true msg="JSON response exceeds 16777216 bytes"
PASS  negative control: unbounded read buffers PAST cap :: buffered=18874398 (> 16777216)
PASS  happy path: small JSON parsed correctly :: object=list dataLen=1

[proof] 3 PASS, 0 FAIL

Negative control (L3)

Stashing the readProviderJsonResponse change and reverting to the original await response.json() confirms the unbounded path buffers the full 18 MiB body with no size cap, validating that the fix is real and the test is not a tautology.

…ider reader Replace unbounded await response.json() with readProviderJsonResponse (shared 16 MiB default cap, cancels the stream on overflow) to prevent OOM from a compromised or misconfigured Hugging Face upstream endpoint. Co-Authored-By: Claude <[email protected]> Signed-off-by: lsr911 <[email protected]>

clawsweeper · 2026-06-29T09:55:02Z

Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 6:02 AM ET / 10:02 UTC.

Summary
This PR changes Hugging Face model discovery to read successful /v1/models JSON through the shared bounded provider reader, adds focused unit coverage, and adds a local real-HTTP proof script.

PR surface: Source +4, Tests +198. Total +202 across 3 files.

Reproducibility: yes. source-level. Current main and v2026.6.10 both show await response.json() on the successful Hugging Face discovery response, and the shared reader source shows the bounded alternative cancels on overflow.

Review metrics: 1 noteworthy metric.

Success JSON Readers Changed: 1 changed. The single Hugging Face discovery success-response parser now has a 16 MiB fail-closed cap, so maintainers should notice the compatibility boundary before merge.

Stored data model
Persistent data-model change detected: serialized state: extensions/huggingface/models.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

none.

Risk before merge

[P1] Existing Hugging Face discovery responses over 16 MiB will now fall back to the static catalog instead of being parsed, which is the intended hardening but still a compatibility boundary maintainers should accept explicitly.
[P1] The real HTTP proof exercises the bounded helper and the unit tests exercise discoverHuggingfaceModels; a single real-server proof through the full discovery function would be stronger, though the current evidence is enough for review confidence.

Maintainer options:

Accept The Shared Cap (recommended)
Maintainers can accept the 16 MiB provider JSON cap for Hugging Face discovery because the existing discovery path already falls back to the static catalog on errors.
Ask For Full Changed-Path Proof
If maintainers want stronger evidence, ask for terminal output that runs discoverHuggingfaceModels against a local streamed oversized /models server.
Keep The Narrow PR Separate
Do not fold this back into the broad overlapping sweep unless that branch is repaired and proves the same Hugging Face behavior cleanly.

Next step before merge

[P2] Maintainers should accept the shared cap/fallback compatibility boundary and rely on normal CI; there is no narrow automated repair to queue.

Security
Cleared: No concrete supply-chain or new security-boundary concern was found; the diff narrows provider response memory behavior and does not change dependencies, workflows, secrets, or package metadata.

Review details

Best possible solution:

Land this focused Hugging Face bounded-reader change after maintainer acceptance of the shared cap/fallback behavior and normal CI completion, leaving the broader batch PR to be split or repaired separately.

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

Yes, source-level. Current main and v2026.6.10 both show await response.json() on the successful Hugging Face discovery response, and the shared reader source shows the bounded alternative cancels on overflow.

Is this the best way to solve the issue?

Yes, this is the best narrow fix shape. Reusing the existing plugin SDK readProviderJsonResponse keeps the behavior at the provider boundary, avoids a new helper, and is cleaner than the still-open broad sweep for this one path.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This is normal-priority provider hardening for a bounded Hugging Face discovery OOM risk with limited blast radius.
merge-risk: 🚨 compatibility: A successful Hugging Face model-discovery response larger than the shared cap will now fall back to the static catalog instead of being parsed.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal proof from a real local HTTP stream showing bounded overflow and an unbounded negative control, supplemented by unit output for the changed discovery function.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal proof from a real local HTTP stream showing bounded overflow and an unbounded negative control, supplemented by unit output for the changed discovery function.

Evidence reviewed

PR surface:

Source +4, Tests +198. Total +202 across 3 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	5	1	+4
Tests	2	198	0	+198
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	3	203	1	+202

What I checked:

Current main still has the unbounded read: Current main reads Hugging Face model discovery success responses with await response.json() at the discovery path. (extensions/huggingface/models.ts:168, 389c355bcf3f)
Latest release also has the unbounded read: The latest release tag v2026.6.10 still contains the same await response.json() call, so the PR is not obsolete against shipped behavior. (extensions/huggingface/models.ts:168, aa69b12d0086)
PR uses the plugin SDK bounded reader: The PR head imports readProviderJsonResponse from the plugin SDK and uses it for Hugging Face model discovery with the huggingface-model-discovery label. (extensions/huggingface/models.ts:169, 2c50689a636a)
Shared reader contract: readProviderJsonResponse defaults provider JSON reads to the shared 16 MiB cap and reports labeled overflow or malformed JSON errors. (src/agents/provider-http-errors.ts:315, 389c355bcf3f)
Stream cancellation contract: readResponseWithLimit truncates at the byte cap, cancels the reader on overflow, and throws the configured overflow error. (packages/media-core/src/read-response-with-limit.ts:96, 389c355bcf3f)
Focused regression coverage: The PR adds tests for valid model JSON, malformed JSON fallback, oversized body fallback, and empty data fallback through discoverHuggingfaceModels. (extensions/huggingface/models.test.ts:111, 2c50689a636a)

Likely related people:

steipete: Recent history shows Peter Steinberger authored Hugging Face discovery timeout, implicit discovery, and provider-discovery plugin moves, and CONTRIBUTING maps Peter Steinberger to @steipete. (role: recent Hugging Face discovery contributor; confidence: high; commits: 5ac07b8ef086, 19de5d1b569d, 455c642acb66; files: extensions/huggingface/models.ts, extensions/huggingface/models.test.ts, extensions/huggingface/openclaw.plugin.json)
Alix-007: Recent current-main commits from Alix-007 bound provider response bodies for xAI, Vydra, and Fal using the same shared-reader hardening pattern this PR applies to Hugging Face. (role: recent bounded-reader pattern contributor; confidence: medium; commits: 46e119074ef0, 74a9beb83f51, ce1217a49ca6; files: extensions/xai/video-generation-provider.ts, extensions/vydra/shared.ts, extensions/fal/music-generation-provider.ts)
Alex Knight: The checked-out blame attributes the current shared provider JSON reader and response-limit implementation to commit 1f0c6a6; this is useful routing context but lower confidence because the local history is grafted for that large import commit. (role: adjacent shared-reader source contributor; confidence: low; commits: 1f0c6a66a6bc; files: src/agents/provider-http-errors.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.

…hared provider reader Replace unbounded await response.json() with readProviderJsonResponse (shared 16 MiB default cap, cancels the stream on overflow) to prevent OOM from a compromised or misconfigured Bedrock Mantle upstream endpoint. Same pattern as openclaw#97743 (huggingface). Co-Authored-By: Claude <[email protected]> Signed-off-by: lsr911 <[email protected]>

lsr911 · 2026-07-02T00:57:58Z

@clawsweeper re-review

clawsweeper · 2026-07-02T00:58:00Z

🦞🧹
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.

steipete · 2026-07-06T22:27:14Z

Superseded by #101079, now landed on main as a464620141f6.

The landed fix consumes the Hugging Face model-discovery JSON through the shared 16 MiB bounded reader, cancels overflow, preserves the static-catalog fallback, and includes overflow-cleanup plus valid-response coverage. Exact-SHA release gate 28826762727 passed.

Thank you @lsr911 for working on this.

openclaw-barnacle Bot added extensions: huggingface size: M labels Jun 29, 2026

This was referenced Jun 29, 2026

fix(amazon-bedrock-mantle): bound model-discovery JSON response via shared provider reader #97759

Closed

fix(minimax): bound OAuth authorization JSON response via shared provider reader #97763

Closed

This was referenced Jul 1, 2026

fix(huggingface): bound model discovery JSON response read to prevent OOM #98635

Closed

fix(huggingface): bound model discovery response reads #98714

Closed

steipete closed this Jul 6, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(huggingface): bound model-discovery JSON response via shared provider reader#97743

fix(huggingface): bound model-discovery JSON response via shared provider reader#97743
lsr911 wants to merge 1 commit into
openclaw:mainfrom
lsr911:fix/huggingface-bounded-json-response

lsr911 commented Jun 29, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading

Uh oh!

lsr911 commented Jul 2, 2026

Uh oh!

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

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

Uh oh!

What Problem This Solves

Changes

Compatibility analysis

Evidence

Unit tests

Real HTTP proof (L2)

Negative control (L3)

Uh oh!

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

Uh oh!

Uh oh!

lsr911 commented Jul 2, 2026

Uh oh!

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

Uh oh!

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lsr911 commented Jun 29, 2026 •

edited

Loading

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading