diff --git a/hermes_cli/config.py b/hermes_cli/config.py index d04e8640f..ef5e3d2fc 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -13,6 +13,7 @@ This module provides: """ import copy +import logging import os import platform import re @@ -24,6 +25,7 @@ from dataclasses import dataclass from pathlib import Path from typing import Dict, Any, Optional, List, Tuple +logger = logging.getLogger(__name__) _IS_WINDOWS = platform.system() == "Windows" _ENV_VAR_NAME_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") @@ -1850,12 +1852,53 @@ def _normalize_custom_provider_entry( if not isinstance(entry, dict): return None + # Accept camelCase aliases commonly used in hand-written configs. + _CAMEL_ALIASES: Dict[str, str] = { + "apiKey": "api_key", + "baseUrl": "base_url", + "apiMode": "api_mode", + "keyEnv": "key_env", + "defaultModel": "default_model", + "contextLength": "context_length", + "rateLimitDelay": "rate_limit_delay", + } + _KNOWN_KEYS = { + "name", "api", "url", "base_url", "api_key", "key_env", + "api_mode", "transport", "model", "default_model", "models", + "context_length", "rate_limit_delay", + } + for camel, snake in _CAMEL_ALIASES.items(): + if camel in entry and snake not in entry: + logger.warning( + "providers.%s: camelCase key '%s' auto-mapped to '%s' " + "(use snake_case to avoid this warning)", + provider_key or "?", camel, snake, + ) + entry[snake] = entry[camel] + unknown = set(entry.keys()) - _KNOWN_KEYS - set(_CAMEL_ALIASES.keys()) + if unknown: + logger.warning( + "providers.%s: unknown config keys ignored: %s", + provider_key or "?", ", ".join(sorted(unknown)), + ) + + from urllib.parse import urlparse + base_url = "" - for url_key in ("api", "url", "base_url"): + for url_key in ("base_url", "url", "api"): raw_url = entry.get(url_key) if isinstance(raw_url, str) and raw_url.strip(): - base_url = raw_url.strip() - break + candidate = raw_url.strip() + parsed = urlparse(candidate) + if parsed.scheme and parsed.netloc: + base_url = candidate + break + else: + logger.warning( + "providers.%s: '%s' value '%s' is not a valid URL " + "(no scheme or host) — skipped", + provider_key or "?", url_key, candidate, + ) if not base_url: return None diff --git a/tests/hermes_cli/test_provider_config_validation.py b/tests/hermes_cli/test_provider_config_validation.py new file mode 100644 index 000000000..775e3284c --- /dev/null +++ b/tests/hermes_cli/test_provider_config_validation.py @@ -0,0 +1,137 @@ +"""Tests for providers config entry validation and normalization. + +Covers Issue #9332: camelCase keys silently ignored, non-URL strings +accepted as base_url, and unknown keys go unreported. +""" + +import logging +from unittest.mock import patch + +import pytest + +from hermes_cli.config import _normalize_custom_provider_entry + + +class TestNormalizeCustomProviderEntry: + """Tests for _normalize_custom_provider_entry validation.""" + + def test_valid_entry_snake_case(self): + """Standard snake_case entry should normalize correctly.""" + entry = { + "base_url": "https://api.example.com/v1", + "api_key": "sk-test-key", + } + result = _normalize_custom_provider_entry(entry, provider_key="myhost") + assert result is not None + assert result["name"] == "myhost" + assert result["base_url"] == "https://api.example.com/v1" + assert result["api_key"] == "sk-test-key" + + def test_camel_case_api_key_mapped(self): + """camelCase apiKey should be auto-mapped to api_key.""" + entry = { + "base_url": "https://api.example.com/v1", + "apiKey": "sk-test-key", + } + result = _normalize_custom_provider_entry(entry, provider_key="myhost") + assert result is not None + assert result["api_key"] == "sk-test-key" + + def test_camel_case_base_url_mapped(self): + """camelCase baseUrl should be auto-mapped to base_url.""" + entry = { + "baseUrl": "https://api.example.com/v1", + "api_key": "sk-test-key", + } + result = _normalize_custom_provider_entry(entry, provider_key="myhost") + assert result is not None + assert result["base_url"] == "https://api.example.com/v1" + + def test_non_url_api_field_rejected(self): + """Non-URL string in 'api' field should be skipped with a warning.""" + entry = { + "api": "openai-reverse-proxy", + "api_key": "sk-test-key", + } + result = _normalize_custom_provider_entry(entry, provider_key="nvidia") + # Should return None because no valid URL was found + assert result is None + + def test_valid_url_in_api_field_accepted(self): + """Valid URL in 'api' field should still be accepted.""" + entry = { + "api": "https://integrate.api.nvidia.com/v1", + "api_key": "sk-test-key", + } + result = _normalize_custom_provider_entry(entry, provider_key="nvidia") + assert result is not None + assert result["base_url"] == "https://integrate.api.nvidia.com/v1" + + def test_base_url_preferred_over_api(self): + """base_url should be checked before api field.""" + entry = { + "base_url": "https://correct.example.com/v1", + "api": "https://wrong.example.com/v1", + "api_key": "sk-test-key", + } + result = _normalize_custom_provider_entry(entry, provider_key="test") + assert result is not None + assert result["base_url"] == "https://correct.example.com/v1" + + def test_unknown_keys_logged(self, caplog): + """Unknown config keys should produce a warning.""" + entry = { + "base_url": "https://api.example.com/v1", + "api_key": "sk-test-key", + "unknownField": "value", + "anotherBad": 42, + } + with caplog.at_level(logging.WARNING): + result = _normalize_custom_provider_entry(entry, provider_key="test") + assert result is not None + assert any("unknown config keys" in r.message.lower() for r in caplog.records) + + def test_camel_case_warning_logged(self, caplog): + """camelCase alias mapping should produce a warning.""" + entry = { + "baseUrl": "https://api.example.com/v1", + "apiKey": "sk-test-key", + } + with caplog.at_level(logging.WARNING): + result = _normalize_custom_provider_entry(entry, provider_key="test") + assert result is not None + camel_warnings = [r for r in caplog.records if "camelcase" in r.message.lower() or "auto-mapped" in r.message.lower()] + assert len(camel_warnings) >= 1 + + def test_snake_case_takes_precedence_over_camel(self): + """If both snake_case and camelCase exist, snake_case wins.""" + entry = { + "api_key": "snake-key", + "apiKey": "camel-key", + "base_url": "https://api.example.com/v1", + } + result = _normalize_custom_provider_entry(entry, provider_key="test") + assert result is not None + assert result["api_key"] == "snake-key" + + def test_non_dict_returns_none(self): + """Non-dict entry should return None.""" + assert _normalize_custom_provider_entry("not-a-dict") is None + assert _normalize_custom_provider_entry(42) is None + assert _normalize_custom_provider_entry(None) is None + + def test_no_url_returns_none(self): + """Entry with no valid URL in any field should return None.""" + entry = { + "api_key": "sk-test-key", + } + result = _normalize_custom_provider_entry(entry, provider_key="test") + assert result is None + + def test_no_name_returns_none(self): + """Entry with no name and no provider_key should return None.""" + entry = { + "base_url": "https://api.example.com/v1", + } + result = _normalize_custom_provider_entry(entry, provider_key="") + assert result is None