From 3ed8f7601935cb08eebfd34007e97675903180a5 Mon Sep 17 00:00:00 2001 From: Abhay Singh Date: Tue, 18 Aug 2026 08:48:58 +0530 Subject: [PATCH] fix(providers): don't crash on a non-object HEADROOM_MODEL_LIMITS / models.json (#3089) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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. --- headroom/providers/anthropic.py | 15 +++++++++++++-- headroom/providers/openai.py | 15 +++++++++++++-- tests/test_provider_model_fallback.py | 27 +++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/headroom/providers/anthropic.py b/headroom/providers/anthropic.py index 8d4b8313e..fb7278ab4 100644 --- a/headroom/providers/anthropic.py +++ b/headroom/providers/anthropic.py @@ -276,6 +276,11 @@ def _load_custom_model_config() -> dict[str, Any]: # Try to parse as JSON string loaded = json.loads(env_config) + if not isinstance(loaded, dict): + raise ValueError( + f"HEADROOM_MODEL_LIMITS must be a JSON object, got {type(loaded).__name__}" + ) + # Check for anthropic-specific config, fall back to root level anthropic_config = loaded.get("anthropic", loaded) if "context_limits" in anthropic_config: @@ -284,7 +289,10 @@ def _load_custom_model_config() -> dict[str, Any]: config["pricing"].update(anthropic_config["pricing"]) logger.debug(f"Loaded custom model config from HEADROOM_MODEL_LIMITS: {loaded}") - except (json.JSONDecodeError, OSError) as e: + except (ValueError, OSError) as e: + # ValueError covers json.JSONDecodeError (a subclass) and the + # non-object guard above, so a malformed value warns and falls back + # to defaults instead of crashing provider init. logger.warning(f"Failed to load HEADROOM_MODEL_LIMITS: {e}") # Check config file. Prefer the canonical config-dir location, then fall @@ -299,6 +307,9 @@ def _load_custom_model_config() -> dict[str, Any]: with open(config_file, encoding="utf-8") as f: loaded = json.load(f) + if not isinstance(loaded, dict): + raise ValueError(f"{config_file} must contain a JSON object") + # Only load anthropic-specific config anthropic_config = loaded.get("anthropic", loaded) if "context_limits" in anthropic_config: @@ -312,7 +323,7 @@ def _load_custom_model_config() -> dict[str, Any]: config["pricing"][model] = pricing logger.debug(f"Loaded custom model config from {config_file}") - except (json.JSONDecodeError, OSError) as e: + except (ValueError, OSError) as e: logger.warning(f"Failed to load {config_file}: {e}") return config diff --git a/headroom/providers/openai.py b/headroom/providers/openai.py index bb61e9300..b0e2b9afc 100644 --- a/headroom/providers/openai.py +++ b/headroom/providers/openai.py @@ -200,6 +200,11 @@ def _load_custom_model_config() -> dict[str, Any]: # Try to parse as JSON string loaded = json.loads(env_config) + if not isinstance(loaded, dict): + raise ValueError( + f"HEADROOM_MODEL_LIMITS must be a JSON object, got {type(loaded).__name__}" + ) + openai_config = loaded.get("openai", loaded) if "context_limits" in openai_config: config["context_limits"].update(openai_config["context_limits"]) @@ -209,7 +214,10 @@ def _load_custom_model_config() -> dict[str, Any]: config["encodings"].update(openai_config["encodings"]) logger.debug("Loaded custom OpenAI model config from HEADROOM_MODEL_LIMITS") - except (json.JSONDecodeError, OSError) as e: + except (ValueError, OSError) as e: + # ValueError covers json.JSONDecodeError (a subclass) and the + # non-object guard above, so a malformed value warns and falls back + # to defaults instead of crashing provider init. logger.warning(f"Failed to load HEADROOM_MODEL_LIMITS: {e}") # Check config file. Prefer the canonical config-dir location, then fall @@ -224,6 +232,9 @@ def _load_custom_model_config() -> dict[str, Any]: with open(config_file, encoding="utf-8") as f: loaded = json.load(f) + if not isinstance(loaded, dict): + raise ValueError(f"{config_file} must contain a JSON object") + openai_config = loaded.get("openai", {}) if "context_limits" in openai_config: for model, limit in openai_config["context_limits"].items(): @@ -239,7 +250,7 @@ def _load_custom_model_config() -> dict[str, Any]: config["encodings"][model] = encoding logger.debug(f"Loaded custom OpenAI model config from {config_file}") - except (json.JSONDecodeError, OSError) as e: + except (ValueError, OSError) as e: logger.warning(f"Failed to load {config_file}: {e}") return config diff --git a/tests/test_provider_model_fallback.py b/tests/test_provider_model_fallback.py index b19b4e49b..9a83ec0ac 100644 --- a/tests/test_provider_model_fallback.py +++ b/tests/test_provider_model_fallback.py @@ -237,6 +237,25 @@ class TestAnthropicConfigLoading: # Env var should win assert loaded["context_limits"]["test-model"] == 100000 + @pytest.mark.parametrize("raw", ["[1, 2, 3]", '"gpt-4"', "42", "true", "null"]) + def test_non_object_env_var_falls_back_to_defaults(self, raw): + """A valid-JSON-but-not-an-object env var must warn and use defaults, + not crash provider init with AttributeError on ``loaded.get``.""" + with patch.dict(os.environ, {"HEADROOM_MODEL_LIMITS": raw}): + loaded = anthropic_load_config() + assert loaded == {"context_limits": {}, "pricing": {}} + + def test_non_object_config_file_falls_back_to_defaults(self): + """A models.json whose top level is not an object must not crash.""" + with tempfile.TemporaryDirectory() as tmpdir: + config_dir = Path(tmpdir) / ".headroom" + config_dir.mkdir() + (config_dir / "models.json").write_text("[1, 2, 3]") + + with patch.object(Path, "home", return_value=Path(tmpdir)): + loaded = anthropic_load_config() + assert loaded == {"context_limits": {}, "pricing": {}} + class TestOpenAIModelFallback: """Tests for OpenAI provider model fallback.""" @@ -353,6 +372,14 @@ class TestOpenAIConfigLoading: loaded = openai_load_config() assert loaded["pricing"]["test-model"] == [5.0, 15.0] + @pytest.mark.parametrize("raw", ["[1, 2, 3]", '"gpt-4"', "42", "true", "null"]) + def test_non_object_env_var_falls_back_to_defaults(self, raw): + """A valid-JSON-but-not-an-object env var must warn and use defaults, + not crash provider init with AttributeError on ``loaded.get``.""" + with patch.dict(os.environ, {"HEADROOM_MODEL_LIMITS": raw}): + loaded = openai_load_config() + assert loaded == {"context_limits": {}, "pricing": {}, "encodings": {}} + class TestCrossProviderConsistency: """Tests for consistency across providers."""