Skip to content

Commit 3ed8f76

Browse files
fix(providers): don't crash on a non-object HEADROOM_MODEL_LIMITS / models.json (#3089)
## Description `_load_custom_model_config` in both `headroom/providers/anthropic.py` and `headroom/providers/openai.py` loads the operator's custom model configuration from `HEADROOM_MODEL_LIMITS` (a JSON string or a file path) and `~/.headroom/models.json`, then reads it with `loaded.get(...)`: ```python loaded = json.loads(env_config) # or json.load(f) anthropic_config = loaded.get("anthropic", loaded) ``` The `try` guards only `except (json.JSONDecodeError, OSError)`. When the value is **valid JSON but not an object** (a JSON array, number, string, bool, or `null`), `json.loads` succeeds and returns a non-dict, so `loaded.get(...)` raises `AttributeError` — which is *not* one of the caught types. Instead of the intended warn-and-fall-back-to-defaults, a misconfigured `HEADROOM_MODEL_LIMITS` (e.g. `HEADROOM_MODEL_LIMITS='[1,2,3]'` or `'"gpt-4"'`) crashes provider initialization. The same gap exists in the `models.json` branch of both providers. ## Fix After each load, validate `isinstance(loaded, dict)` and raise `ValueError` with a clear message, and broaden the handler from `except (json.JSONDecodeError, OSError)` to `except (ValueError, OSError)`. `json.JSONDecodeError` is a subclass of `ValueError`, so this strictly supersets the previous handling: every previously-caught malformed value still warns and falls back, and a valid-JSON-but-non-object value now does too, instead of crashing. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature - [ ] Breaking change - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/providers/anthropic.py` and `headroom/providers/openai.py` (`_load_custom_model_config`): add an `isinstance(loaded, dict)` guard (raising `ValueError`) after the env-var load and after the `models.json` load, and change both `except` clauses to `(ValueError, OSError)`. - `tests/test_provider_model_fallback.py`: added parametrized `test_non_object_env_var_falls_back_to_defaults` (array / string / number / bool / null) for both providers, and `test_non_object_config_file_falls_back_to_defaults` for a non-object `models.json`. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check`) - [x] Type checking passes (`mypy`) - [x] New tests added ### Test Output ```text tests/test_provider_model_fallback.py 44 passed # uvx ruff@0.15.22 check -> All checks passed! # uvx mypy@1.20.2 headroom/providers/anthropic.py headroom/providers/openai.py -> Success: no issues found in 2 source files ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.12.11, project venv, pytest 9.1.1, ruff 0.15.22 and mypy 1.20.2 via uvx. - Exact command / steps: reverted both providers and ran the new regressions to capture the bug (`python -m pytest tests/test_provider_model_fallback.py::TestAnthropicConfigLoading::test_non_object_env_var_falls_back_to_defaults tests/test_provider_model_fallback.py::TestOpenAIConfigLoading::test_non_object_env_var_falls_back_to_defaults tests/test_provider_model_fallback.py::TestAnthropicConfigLoading::test_non_object_config_file_falls_back_to_defaults` -> 11 failed with `AttributeError` on `loaded.get` across the array/string/number/bool/null shapes); restored the fix; re-ran the full file (`python -m pytest tests/test_provider_model_fallback.py` -> 44 passed); then `uvx ruff@0.15.22 format`, `uvx ruff@0.15.22 check`, and `uvx mypy@1.20.2` on both providers. - Observed result: before the fix, `HEADROOM_MODEL_LIMITS='[1,2,3]'` (or `'"gpt-4"'`, `'42'`, `'true'`, `'null'`) raised `AttributeError` out of `_load_custom_model_config`; after the fix the same values log a warning and the loader returns the default `{"context_limits": {}, "pricing": {}[, "encodings": {}]}`, and a well-formed object config is unchanged. - Not tested: a live proxy boot with a corrupt `HEADROOM_MODEL_LIMITS` (the loader is exercised directly, which is the exact function provider init calls). ## Runtime Rollout Safety - Rollout-managed feature(s): none. This is defensive parsing in the provider model-config loader, not a rollout-channel-gated runtime feature. - Minimum rollout channel: N/A (no rollout-managed behavior). - Stable/default behavior changed: only for a previously-crashing input. A non-object `HEADROOM_MODEL_LIMITS` / `models.json` now warns and uses built-in defaults instead of raising. Well-formed object configs are parsed exactly as before. - Kill switch / disable path: N/A — remove or correct the malformed config value to load custom limits. - Unsafe override required: no. - Qualification impact: a corrupt or mistyped model-limits value degrades to built-in defaults with a warning rather than failing provider init. - Rollback path: revert this PR; the loader returns to catching only `json.JSONDecodeError`/`OSError`. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective - [x] New and existing unit tests pass locally with my changes - [x] I did **not** edit `CHANGELOG.md`: it is generated by release-please from my Conventional Commit PR title ## Additional Notes Both providers carry the same loader shape, so the guard and the widened `except` are applied identically to keep them in sync. The message names the offending source (`HEADROOM_MODEL_LIMITS` vs the resolved config-file path) so the warning is actionable.
1 parent 0ec73fa commit 3ed8f76

3 files changed

Lines changed: 53 additions & 4 deletions

File tree

headroom/providers/anthropic.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,11 @@ def _load_custom_model_config() -> dict[str, Any]:
276276
# Try to parse as JSON string
277277
loaded = json.loads(env_config)
278278

279+
if not isinstance(loaded, dict):
280+
raise ValueError(
281+
f"HEADROOM_MODEL_LIMITS must be a JSON object, got {type(loaded).__name__}"
282+
)
283+
279284
# Check for anthropic-specific config, fall back to root level
280285
anthropic_config = loaded.get("anthropic", loaded)
281286
if "context_limits" in anthropic_config:
@@ -284,7 +289,10 @@ def _load_custom_model_config() -> dict[str, Any]:
284289
config["pricing"].update(anthropic_config["pricing"])
285290

286291
logger.debug(f"Loaded custom model config from HEADROOM_MODEL_LIMITS: {loaded}")
287-
except (json.JSONDecodeError, OSError) as e:
292+
except (ValueError, OSError) as e:
293+
# ValueError covers json.JSONDecodeError (a subclass) and the
294+
# non-object guard above, so a malformed value warns and falls back
295+
# to defaults instead of crashing provider init.
288296
logger.warning(f"Failed to load HEADROOM_MODEL_LIMITS: {e}")
289297

290298
# Check config file. Prefer the canonical config-dir location, then fall
@@ -299,6 +307,9 @@ def _load_custom_model_config() -> dict[str, Any]:
299307
with open(config_file, encoding="utf-8") as f:
300308
loaded = json.load(f)
301309

310+
if not isinstance(loaded, dict):
311+
raise ValueError(f"{config_file} must contain a JSON object")
312+
302313
# Only load anthropic-specific config
303314
anthropic_config = loaded.get("anthropic", loaded)
304315
if "context_limits" in anthropic_config:
@@ -312,7 +323,7 @@ def _load_custom_model_config() -> dict[str, Any]:
312323
config["pricing"][model] = pricing
313324

314325
logger.debug(f"Loaded custom model config from {config_file}")
315-
except (json.JSONDecodeError, OSError) as e:
326+
except (ValueError, OSError) as e:
316327
logger.warning(f"Failed to load {config_file}: {e}")
317328

318329
return config

headroom/providers/openai.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,11 @@ def _load_custom_model_config() -> dict[str, Any]:
200200
# Try to parse as JSON string
201201
loaded = json.loads(env_config)
202202

203+
if not isinstance(loaded, dict):
204+
raise ValueError(
205+
f"HEADROOM_MODEL_LIMITS must be a JSON object, got {type(loaded).__name__}"
206+
)
207+
203208
openai_config = loaded.get("openai", loaded)
204209
if "context_limits" in openai_config:
205210
config["context_limits"].update(openai_config["context_limits"])
@@ -209,7 +214,10 @@ def _load_custom_model_config() -> dict[str, Any]:
209214
config["encodings"].update(openai_config["encodings"])
210215

211216
logger.debug("Loaded custom OpenAI model config from HEADROOM_MODEL_LIMITS")
212-
except (json.JSONDecodeError, OSError) as e:
217+
except (ValueError, OSError) as e:
218+
# ValueError covers json.JSONDecodeError (a subclass) and the
219+
# non-object guard above, so a malformed value warns and falls back
220+
# to defaults instead of crashing provider init.
213221
logger.warning(f"Failed to load HEADROOM_MODEL_LIMITS: {e}")
214222

215223
# Check config file. Prefer the canonical config-dir location, then fall
@@ -224,6 +232,9 @@ def _load_custom_model_config() -> dict[str, Any]:
224232
with open(config_file, encoding="utf-8") as f:
225233
loaded = json.load(f)
226234

235+
if not isinstance(loaded, dict):
236+
raise ValueError(f"{config_file} must contain a JSON object")
237+
227238
openai_config = loaded.get("openai", {})
228239
if "context_limits" in openai_config:
229240
for model, limit in openai_config["context_limits"].items():
@@ -239,7 +250,7 @@ def _load_custom_model_config() -> dict[str, Any]:
239250
config["encodings"][model] = encoding
240251

241252
logger.debug(f"Loaded custom OpenAI model config from {config_file}")
242-
except (json.JSONDecodeError, OSError) as e:
253+
except (ValueError, OSError) as e:
243254
logger.warning(f"Failed to load {config_file}: {e}")
244255

245256
return config

tests/test_provider_model_fallback.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,25 @@ def test_env_var_overrides_config_file(self):
237237
# Env var should win
238238
assert loaded["context_limits"]["test-model"] == 100000
239239

240+
@pytest.mark.parametrize("raw", ["[1, 2, 3]", '"gpt-4"', "42", "true", "null"])
241+
def test_non_object_env_var_falls_back_to_defaults(self, raw):
242+
"""A valid-JSON-but-not-an-object env var must warn and use defaults,
243+
not crash provider init with AttributeError on ``loaded.get``."""
244+
with patch.dict(os.environ, {"HEADROOM_MODEL_LIMITS": raw}):
245+
loaded = anthropic_load_config()
246+
assert loaded == {"context_limits": {}, "pricing": {}}
247+
248+
def test_non_object_config_file_falls_back_to_defaults(self):
249+
"""A models.json whose top level is not an object must not crash."""
250+
with tempfile.TemporaryDirectory() as tmpdir:
251+
config_dir = Path(tmpdir) / ".headroom"
252+
config_dir.mkdir()
253+
(config_dir / "models.json").write_text("[1, 2, 3]")
254+
255+
with patch.object(Path, "home", return_value=Path(tmpdir)):
256+
loaded = anthropic_load_config()
257+
assert loaded == {"context_limits": {}, "pricing": {}}
258+
240259

241260
class TestOpenAIModelFallback:
242261
"""Tests for OpenAI provider model fallback."""
@@ -353,6 +372,14 @@ def test_load_pricing_from_config(self):
353372
loaded = openai_load_config()
354373
assert loaded["pricing"]["test-model"] == [5.0, 15.0]
355374

375+
@pytest.mark.parametrize("raw", ["[1, 2, 3]", '"gpt-4"', "42", "true", "null"])
376+
def test_non_object_env_var_falls_back_to_defaults(self, raw):
377+
"""A valid-JSON-but-not-an-object env var must warn and use defaults,
378+
not crash provider init with AttributeError on ``loaded.get``."""
379+
with patch.dict(os.environ, {"HEADROOM_MODEL_LIMITS": raw}):
380+
loaded = openai_load_config()
381+
assert loaded == {"context_limits": {}, "pricing": {}, "encodings": {}}
382+
356383

357384
class TestCrossProviderConsistency:
358385
"""Tests for consistency across providers."""

0 commit comments

Comments
 (0)