diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 89dc7d935b47..8c98c8816b8c 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -1696,6 +1696,115 @@ def clear_runtime_main() -> None: _RUNTIME_MAIN_MODEL = "" +# Mapping of base_url patterns to (provider_name, env_var_name) +_BASE_URL_TO_PROVIDER = { + "dashscope.aliyuncs.com": ("alibaba", "DASHSCOPE_API_KEY"), + "coding.dashscope.aliyuncs.com": ("alibaba", "DASHSCOPE_API_KEY"), + "api.moonshot.cn": ("moonshot", "MOONSHOT_API_KEY"), + "api.deepseek.com": ("deepseek", "DEEPSEEK_API_KEY"), + "api.minimax.chat": ("minimax", "MINIMAX_API_KEY"), + "api.zhipuai.cn": ("zhipu", "ZHIPU_API_KEY"), + "api.siliconflow.cn": ("siliconflow", "SILICONFLOW_API_KEY"), + "api-inference.modelscope.cn": ("modelscope", "MODELSCOPE_API_KEY"), +} + + +def _read_env_var_from_file(env_var: str) -> Optional[str]: + """Read an env var value directly from ~/.hermes/.env file. + + Bypasses os.getenv to avoid stale/masked values in gateway processes. + Returns the raw value from the file, or None if not found. + """ + try: + from hermes_cli.config import get_hermes_home + env_path = get_hermes_home() / ".env" + if not env_path.exists(): + return None + with open(env_path, encoding="utf-8") as f: + for line in f: + line = line.strip() + if line.startswith(f"{env_var}="): + value = line.split("=", 1)[1].strip().strip('"\'') + # Skip masked placeholder values + if value == "***" or value.startswith("***"): + continue + return value + except Exception: + pass + return None + + +def _get_api_key_for_base_url(base_url: str) -> Optional[str]: + """Find API key for a base_url by matching to known providers. + + Checks credential pool first (get_credential), then env vars. + Also reads directly from .env file to bypass stale/masked gateway env. + Returns None if no matching provider found or no key available. + """ + if not base_url: + return None + base_lower = base_url.lower() + + # Find matching provider by checking if any pattern is in the base_url + for pattern, (provider_name, env_var) in _BASE_URL_TO_PROVIDER.items(): + if pattern in base_lower: + # Try credential pool first + try: + pool_present, entry = _select_pool_entry(provider_name) + if pool_present and entry: + key = _pool_runtime_api_key(entry) + # Skip masked values from pool + if key and key != "***" and not key.startswith("***"): + logger.debug( + "Auxiliary client: found API key for %s in credential pool", + provider_name, + ) + return key + except Exception as exc: + logger.debug( + "Auxiliary client: credential pool lookup failed for %s: %s", + provider_name, exc, + ) + + # Try environment variable (via dotenv reload) + try: + from hermes_cli.env_loader import load_hermes_dotenv + load_hermes_dotenv() + except Exception: + pass + key = os.getenv(env_var, "").strip() + # Skip masked values from env + if key and key != "***" and not key.startswith("***"): + logger.debug( + "Auxiliary client: found API key for %s in env var %s", + provider_name, env_var, + ) + return key + + # Fallback: read directly from .env file (bypasses stale gateway env) + key = _read_env_var_from_file(env_var) + if key: + logger.debug( + "Auxiliary client: found API key for %s by reading .env file directly", + provider_name, + ) + return key + + # No key found for this provider + logger.debug( + "Auxiliary client: no API key found for provider %s (pattern %s)", + provider_name, pattern, + ) + return None + + # No matching provider found + logger.debug( + "Auxiliary client: base_url %s not matched to any known provider", + base_url, + ) + return None + + def _resolve_custom_runtime() -> Tuple[Optional[str], Optional[str], Optional[str]]: """Resolve the active custom/main endpoint the same way the main CLI does. @@ -3270,9 +3379,15 @@ def _wrap_if_needed(client_obj, final_model_str: str, base_url_str: str = "", # ── Custom endpoint (OPENAI_BASE_URL + OPENAI_API_KEY) ─────────── if provider == "custom": if explicit_base_url: - custom_base = _to_openai_base_url(explicit_base_url).strip() +custom_base = _to_openai_base_url(explicit_base_url).strip() + # Try to find API key from multiple sources: + # 1. Explicit API key passed in + # 2. Credential pool (match base_url to known providers) + # 3. OPENAI_API_KEY env var + # 4. "no-key-required" fallback for local servers custom_key = ( (explicit_api_key or "").strip() + or _get_api_key_for_base_url(custom_base) or os.getenv("OPENAI_API_KEY", "").strip() or "no-key-required" # local servers don't need auth ) diff --git a/tests/agent/test_auxiliary_base_url_api_key.py b/tests/agent/test_auxiliary_base_url_api_key.py new file mode 100644 index 000000000000..eb9d06cde1a8 --- /dev/null +++ b/tests/agent/test_auxiliary_base_url_api_key.py @@ -0,0 +1,294 @@ +"""Tests for _get_api_key_for_base_url and custom endpoint API key resolution. + +This tests the fix for GitHub issue where auxiliary tasks with custom base_url +but no explicit api_key would fail with 401 errors because the resolution chain +did not look up credentials from known providers based on the base_url pattern. + +The fix adds: +1. _BASE_URL_TO_PROVIDER mapping for known Chinese/international providers +2. _get_api_key_for_base_url() to match base_url to provider credentials +3. Integration in resolve_provider_client() custom branch +""" + +import os +import logging +from unittest.mock import patch, MagicMock +import pytest + +from agent.auxiliary_client import ( + _get_api_key_for_base_url, + _BASE_URL_TO_PROVIDER, + _read_env_var_from_file, + resolve_provider_client, +) + + +class TestBaseUrlToProviderMapping: + """Tests for _BASE_URL_TO_PROVIDER static mapping.""" + + def test_mapping_contains_dashscope(self): + """Alibaba DashScope endpoints are mapped.""" + assert "dashscope.aliyuncs.com" in _BASE_URL_TO_PROVIDER + assert "coding.dashscope.aliyuncs.com" in _BASE_URL_TO_PROVIDER + assert _BASE_URL_TO_PROVIDER["dashscope.aliyuncs.com"] == ("alibaba", "DASHSCOPE_API_KEY") + assert _BASE_URL_TO_PROVIDER["coding.dashscope.aliyuncs.com"] == ("alibaba", "DASHSCOPE_API_KEY") + + def test_mapping_contains_deepseek(self): + """DeepSeek endpoint is mapped.""" + assert "api.deepseek.com" in _BASE_URL_TO_PROVIDER + assert _BASE_URL_TO_PROVIDER["api.deepseek.com"] == ("deepseek", "DEEPSEEK_API_KEY") + + def test_mapping_contains_moonshot(self): + """Moonshot (Kimi) endpoint is mapped.""" + assert "api.moonshot.cn" in _BASE_URL_TO_PROVIDER + assert _BASE_URL_TO_PROVIDER["api.moonshot.cn"] == ("moonshot", "MOONSHOT_API_KEY") + + def test_mapping_contains_minimax(self): + """MiniMax endpoint is mapped.""" + assert "api.minimax.chat" in _BASE_URL_TO_PROVIDER + assert _BASE_URL_TO_PROVIDER["api.minimax.chat"] == ("minimax", "MINIMAX_API_KEY") + + def test_mapping_contains_zhipu(self): + """Zhipu (GLM) endpoint is mapped.""" + assert "api.zhipuai.cn" in _BASE_URL_TO_PROVIDER + assert _BASE_URL_TO_PROVIDER["api.zhipuai.cn"] == ("zhipu", "ZHIPU_API_KEY") + + def test_mapping_contains_siliconflow(self): + """SiliconFlow endpoint is mapped.""" + assert "api.siliconflow.cn" in _BASE_URL_TO_PROVIDER + assert _BASE_URL_TO_PROVIDER["api.siliconflow.cn"] == ("siliconflow", "SILICONFLOW_API_KEY") + + def test_mapping_contains_modelscope(self): + """ModelScope endpoint is mapped.""" + assert "api-inference.modelscope.cn" in _BASE_URL_TO_PROVIDER + assert _BASE_URL_TO_PROVIDER["api-inference.modelscope.cn"] == ("modelscope", "MODELSCOPE_API_KEY") + + +class TestReadEnvVarFromFile: + """Tests for _read_env_var_from_file helper.""" + + def test_reads_valid_key_from_env_file(self, tmp_path, monkeypatch): + """Can read a valid API key from .env file.""" + env_file = tmp_path / ".env" + env_file.write_text("DASHSCOPE_API_KEY=sk-test-12345\nOTHER_KEY=value\n") + monkeypatch.setattr("hermes_cli.config.get_hermes_home", lambda: tmp_path) + result = _read_env_var_from_file("DASHSCOPE_API_KEY") + assert result == "sk-test-12345" + + def test_skips_masked_placeholder_values(self, tmp_path, monkeypatch): + """Skips '***' masked placeholder values.""" + env_file = tmp_path / ".env" + env_file.write_text("DASHSCOPE_API_KEY=***\n") + monkeypatch.setattr("hermes_cli.config.get_hermes_home", lambda: tmp_path) + result = _read_env_var_from_file("DASHSCOPE_API_KEY") + assert result is None + + def test_skips_masked_values_with_prefix(self, tmp_path, monkeypatch): + """Skips values starting with '***'.""" + env_file = tmp_path / ".env" + env_file.write_text("DASHSCOPE_API_KEY=***masked\n") + monkeypatch.setattr("hermes_cli.config.get_hermes_home", lambda: tmp_path) + result = _read_env_var_from_file("DASHSCOPE_API_KEY") + assert result is None + + def test_returns_none_for_missing_var(self, tmp_path, monkeypatch): + """Returns None when var is not in .env.""" + env_file = tmp_path / ".env" + env_file.write_text("OTHER_KEY=value\n") + monkeypatch.setattr("hermes_cli.config.get_hermes_home", lambda: tmp_path) + result = _read_env_var_from_file("DASHSCOPE_API_KEY") + assert result is None + + def test_returns_none_when_env_file_missing(self, tmp_path, monkeypatch): + """Returns None when .env file does not exist.""" + monkeypatch.setattr("hermes_cli.config.get_hermes_home", lambda: tmp_path) + result = _read_env_var_from_file("DASHSCOPE_API_KEY") + assert result is None + + def test_handles_quoted_values(self, tmp_path, monkeypatch): + """Strips quotes from values.""" + env_file = tmp_path / ".env" + env_file.write_text('DASHSCOPE_API_KEY="sk-test-quoted"\n') + monkeypatch.setattr("hermes_cli.config.get_hermes_home", lambda: tmp_path) + result = _read_env_var_from_file("DASHSCOPE_API_KEY") + assert result == "sk-test-quoted" + + +class TestGetApiKeyForBaseUrl: + """Tests for _get_api_key_for_base_url resolution chain.""" + + def test_returns_none_for_empty_url(self): + """Empty base_url returns None.""" + result = _get_api_key_for_base_url("") + assert result is None + result = _get_api_key_for_base_url(None) + assert result is None + + def test_returns_none_for_unknown_provider(self, monkeypatch): + """Unknown base_url pattern returns None.""" + monkeypatch.setenv("OPENAI_API_KEY", "sk-openai-key") + result = _get_api_key_for_base_url("https://unknown.example.com/v1") + assert result is None + + def test_finds_key_from_env_var(self, monkeypatch): + """Finds key from matching env var.""" + monkeypatch.setenv("DASHSCOPE_API_KEY", "sk-dashscope-test") + result = _get_api_key_for_base_url("https://dashscope.aliyuncs.com/v1") + assert result == "sk-dashscope-test" + + def test_finds_key_from_coding_dashscope_endpoint(self, monkeypatch): + """Matches coding.dashscope.aliyuncs.com pattern.""" + monkeypatch.setenv("DASHSCOPE_API_KEY", "sk-codeplan-test") + result = _get_api_key_for_base_url("https://coding.dashscope.aliyuncs.com/v1") + assert result == "sk-codeplan-test" + + def test_matches_subdomain_pattern(self, monkeypatch): + """Matches pattern embedded in larger URL.""" + monkeypatch.setenv("DEEPSEEK_API_KEY", "sk-deepseek-test") + # Pattern 'api.deepseek.com' should match this URL + result = _get_api_key_for_base_url("https://api.deepseek.com/chat/completions") + assert result == "sk-deepseek-test" + + def test_skips_masked_env_var_value(self, monkeypatch): + """Skips '***' values in env vars.""" + monkeypatch.setenv("DASHSCOPE_API_KEY", "***") + result = _get_api_key_for_base_url("https://dashscope.aliyuncs.com/v1") + assert result is None + + def test_fallback_to_env_file_reading(self, tmp_path, monkeypatch): + """Falls back to reading .env file when env var is masked.""" + # Set env to masked value + monkeypatch.setenv("DASHSCOPE_API_KEY", "***") + # Write real value to .env file + env_file = tmp_path / ".env" + env_file.write_text("DASHSCOPE_API_KEY=sk-real-from-file\n") + monkeypatch.setattr("hermes_cli.config.get_hermes_home", lambda: tmp_path) + result = _get_api_key_for_base_url("https://dashscope.aliyuncs.com/v1") + assert result == "sk-real-from-file" + + def test_case_insensitive_matching(self, monkeypatch): + """Pattern matching is case-insensitive.""" + monkeypatch.setenv("DASHSCOPE_API_KEY", "sk-test-case") + # Uppercase URL should still match + result = _get_api_key_for_base_url("HTTPS://DASHSCOPE.ALIYUNCS.COM/V1") + assert result == "sk-test-case" + + @patch("agent.auxiliary_client._select_pool_entry") + def test_prefers_credential_pool_over_env(self, mock_pool, monkeypatch): + """Credential pool takes priority over env var.""" + # Create a mock entry with runtime_api_key attribute + mock_entry = MagicMock() + mock_entry.runtime_api_key = "sk-from-pool" + mock_pool.return_value = (True, mock_entry) + monkeypatch.setenv("DASHSCOPE_API_KEY", "sk-from-env") + result = _get_api_key_for_base_url("https://dashscope.aliyuncs.com/v1") + assert result == "sk-from-pool" + + @patch("agent.auxiliary_client._select_pool_entry") + def test_falls_back_to_env_when_pool_empty(self, mock_pool, monkeypatch): + """Falls back to env var when pool has no entry.""" + mock_pool.return_value = (False, None) + monkeypatch.setenv("DASHSCOPE_API_KEY", "sk-from-env") + result = _get_api_key_for_base_url("https://dashscope.aliyuncs.com/v1") + assert result == "sk-from-env" + + @patch("agent.auxiliary_client._select_pool_entry") + def test_skips_masked_pool_value(self, mock_pool, monkeypatch): + """Skips masked values from credential pool.""" + # Create a mock entry with masked runtime_api_key + mock_entry = MagicMock() + mock_entry.runtime_api_key = "***" + mock_pool.return_value = (True, mock_entry) + monkeypatch.setenv("DASHSCOPE_API_KEY", "sk-from-env") + result = _get_api_key_for_base_url("https://dashscope.aliyuncs.com/v1") + assert result == "sk-from-env" + + +class TestResolveProviderClientCustomBranch: + """Tests for resolve_provider_client custom branch with base_url.""" + + def test_resolves_key_from_base_url_pattern(self, monkeypatch): + """Custom endpoint with known base_url gets API key from provider env.""" + monkeypatch.setenv("DASHSCOPE_API_KEY", "sk-dashscope-key") + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + + client, model = resolve_provider_client( + provider="custom", + model="kimi-k2.5", + explicit_base_url="https://coding.dashscope.aliyuncs.com/v1", + explicit_api_key=None, # Should be resolved from base_url + ) + + assert client is not None + assert client.api_key == "sk-dashscope-key" + + def test_falls_back_to_openai_api_key(self, monkeypatch): + """Unknown base_url falls back to OPENAI_API_KEY.""" + monkeypatch.setenv("OPENAI_API_KEY", "sk-openai-key") + monkeypatch.delenv("DASHSCOPE_API_KEY", raising=False) + + client, model = resolve_provider_client( + provider="custom", + model="local-model", + explicit_base_url="https://local-server.example.com/v1", + explicit_api_key=None, + ) + + assert client is not None + assert client.api_key == "sk-openai-key" + + def test_uses_no_key_required_for_local_servers(self, monkeypatch): + """Uses 'no-key-required' when no keys available.""" + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + monkeypatch.delenv("DASHSCOPE_API_KEY", raising=False) + + client, model = resolve_provider_client( + provider="custom", + model="local-model", + explicit_base_url="http://localhost:8080/v1", + explicit_api_key=None, + ) + + assert client is not None + assert client.api_key == "no-key-required" + + def test_explicit_api_key_takes_priority(self, monkeypatch): + """Explicit api_key takes priority over base_url resolution.""" + monkeypatch.setenv("DASHSCOPE_API_KEY", "sk-dashscope-env") + + client, model = resolve_provider_client( + provider="custom", + model="kimi-k2.5", + explicit_base_url="https://coding.dashscope.aliyuncs.com/v1", + explicit_api_key="sk-explicit-key", # Should win + ) + + assert client is not None + assert client.api_key == "sk-explicit-key" + + +class TestIntegrationWithAuxiliaryConfig: + """Integration tests simulating auxiliary.vision config scenario.""" + + def test_vision_config_with_base_url_no_api_key(self, monkeypatch): + """Simulates auxiliary.vision with base_url but api_key: null.""" + # This is the bug scenario: config.yaml has: + # auxiliary: + # vision: + # provider: alibaba + # base_url: https://coding.dashscope.aliyuncs.com/v1 + # api_key: null + + monkeypatch.setenv("DASHSCOPE_API_KEY", "sk-vision-key") + + # _resolve_task_provider_model would return provider="custom", base_url=... + client, model = resolve_provider_client( + provider="custom", + model="kimi-k2.5", + explicit_base_url="https://coding.dashscope.aliyuncs.com/v1", + explicit_api_key=None, + ) + + assert client is not None + assert client.api_key == "sk-vision-key" + assert model == "kimi-k2.5" \ No newline at end of file