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.
2026-08-18 08:48:58 +05:30
parent 0ec73faa28
commit 3ed8f76019
3 changed files with 53 additions and 4 deletions
@@ -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
@@ -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
@@ -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."""