fix(providers): bound model-catalog response reads (#54735) - #54765
Closed
iceTTTT wants to merge 1 commit into
Closed
fix(providers): bound model-catalog response reads (#54735)#54765iceTTTT wants to merge 1 commit into
iceTTTT wants to merge 1 commit into
Conversation
ProviderProfile.fetch_models() parsed the model-catalog response with
json.loads(resp.read().decode()) and no size cap. A malicious, compromised,
or misconfigured model endpoint could return an arbitrarily large body and
make Hermes allocate it while opening the model picker, probing providers, or
refreshing live model lists.
Add a shared _read_json_capped() helper with a two-layer guard:
1. reject up front if the declared Content-Length exceeds the cap;
2. read at most cap+1 bytes and reject if the body actually ran past the cap
(covers a missing or lying Content-Length on a streamed response).
On oversize it raises ValueError, which the existing
`except Exception: return None` path turns into a fall-back to the static
model list — the exact fail-closed behavior the issue asks for.
Applied to both unbounded sites: the generic ProviderProfile.fetch_models()
(which the openrouter plugin reuses via super()) and the sibling
AnthropicProfile.fetch_models() override, so the whole bug class is closed.
Cap is 16 MiB (_MAX_MODELS_RESPONSE_BYTES) — generous for real catalogs
(tens-to-hundreds of KB) while bounding malicious payloads.
Tests assert the behavior contract (oversize Content-Length -> None,
oversize streamed body -> None, normal small body -> list) against a real
local HTTPServer, plus helper-level checks that the read is bounded
(read(cap+1), never an unbounded read()).
Closes NousResearch#54735
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author
|
Closing in favor of #42930, which already bounds the read in providers/base.py and is approved. The anthropic sibling read can be folded into that PR or a focused follow-up. Apologies for the duplicate. |
This was referenced Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ProviderProfile.fetch_models()fetched the provider model catalog and parsed it withjson.loads(resp.read().decode())— no cap on the response body. A malicious, compromised, or misconfigured model endpoint could return an arbitrarily large body and make Hermes allocate it while opening the model picker, probing providers, or refreshing live model lists.The call already treats fetch failures as non-fatal and falls back to the static model list, so oversized responses should fail closed the same way. This PR makes them do exactly that.
Change
Add a shared
_read_json_capped()helper inproviders/base.pywith a two-layer guard:Content-Lengthexceeds the cap.cap + 1bytes and reject if the body actually ran past the cap (covers a missing or lyingContent-Lengthon a streamed response).On oversize it raises
ValueError, which the existingexcept Exception: return Nonepath turns into the same fall-back to the static model list — the exact fail-closed behavior the issue asks for.Applied to both unbounded sites so the whole bug class is closed:
providers/base.py— genericProviderProfile.fetch_models()(the openrouter plugin reuses this viasuper().fetch_models(...)).plugins/model-providers/anthropic/__init__.py— the siblingAnthropicProfile.fetch_models()override, which had the identical unboundedresp.read()shape. The issue's Scope note flagged provider-specific fetches for a separate sweep; this one is structurally identical to the base path, so it's folded in here.Cap is 16 MiB (
_MAX_MODELS_RESPONSE_BYTES) — generous for real catalogs (tens-to-hundreds of KB) while bounding a malicious payload.Tests
New
tests/providers/test_fetch_models_bounded.pyasserts the behavior contract (not a model-name snapshot) against a real localHTTPServer:Content-Length→NoneContent-Length) →Noneread(cap + 1), never an unboundedread())Verified with
scripts/run_tests.sh(new file + regression ontest_fetch_models_base_url,test_provider_profiles,test_plugin_discovery, anthropic picker/persistence) andruff.Relation to #42930
#42930 (approved) bounds the read in
providers/base.pyonly. This PR additionally covers the siblingAnthropicProfile.fetch_models()unbounded read, closing the bug class rather than the single reported site.Closes #54735