fix(config): validate providers config entries — reject non-URL base, accept camelCase aliases (#9332)

Cherry-picked from PR #9359 by @luyao618.

- Accept camelCase aliases (apiKey, baseUrl, apiMode, keyEnv, defaultModel,
  contextLength, rateLimitDelay) with auto-mapping to snake_case + warning
- Validate URL field values with urlparse (scheme + netloc check) — reject
  non-URL strings like 'openai-reverse-proxy' that were silently accepted
- Warn on unknown keys in provider config entries
- Re-order URL field priority: base_url > url > api (was api > url > base_url)
- 12 new tests covering all scenarios

Closes #9332
This commit is contained in:
luyao618 2026-04-20 04:48:41 -07:00 committed by Teknium
parent bc2559c44d
commit 2cdae233e2
2 changed files with 183 additions and 3 deletions

View file

@ -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

View file

@ -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