diff --git a/tests/tools/test_delegate.py b/tests/tools/test_delegate.py index f3a1a2632..a8f5511da 100644 --- a/tests/tools/test_delegate.py +++ b/tests/tools/test_delegate.py @@ -656,10 +656,12 @@ class TestDelegationCredentialResolution(unittest.TestCase): self.assertEqual(creds["provider"], "custom") def test_direct_endpoint_does_not_fall_back_to_openrouter_api_key_env(self): + """Remote endpoint without OPENAI_API_KEY should raise ValueError, + even if OPENROUTER_API_KEY is set (only OPENAI_API_KEY is checked).""" parent = _make_mock_parent(depth=0) cfg = { "model": "qwen2.5-coder", - "base_url": "http://localhost:1234/v1", + "base_url": "https://api.example.com/v1", # remote, not localhost } with patch.dict( os.environ, diff --git a/tests/tools/test_delegation_local_provider.py b/tests/tools/test_delegation_local_provider.py new file mode 100644 index 000000000..f3d4650d4 --- /dev/null +++ b/tests/tools/test_delegation_local_provider.py @@ -0,0 +1,229 @@ +#!/usr/bin/env python3 +""" +Tests for delegation with local/Ollama providers that don't require API keys. + +Ollama and other local model servers run on localhost and accept requests +without authentication. The delegation credential resolver should allow these +endpoints to work without requiring an API key. + +Run with: python -m pytest tests/tools/test_delegation_local_provider.py -v +""" + +import os +import unittest +from unittest.mock import MagicMock, patch + +from tools.delegate_tool import ( + _resolve_delegation_credentials, +) + + +def _make_mock_parent(depth=0): + """Create a mock parent agent with the fields delegate_task expects.""" + parent = MagicMock() + parent.base_url = "http://localhost:11434/v1" + parent.api_key = "ollama" + parent.provider = "custom" + parent.api_mode = "chat_completions" + parent.model = "glm-5.1:cloud" + parent.platform = "cli" + parent.providers_allowed = None + parent.providers_ignored = None + parent.providers_order = None + parent.provider_sort = None + parent._session_db = None + parent._delegate_depth = depth + parent._active_children = [] + parent._active_children_lock = __import__("threading").Lock() + parent._print_fn = None + parent.tool_progress_callback = None + parent.thinking_callback = None + parent._credential_pool = None + parent.reasoning_config = None + parent.max_tokens = None + parent.prefill_messages = None + parent.acp_command = None + parent.acp_args = [] + parent.valid_tool_names = ["terminal", "file", "web"] + parent.enabled_toolsets = None # None = all tools + return parent + + +class TestLocalProviderCredentials(unittest.TestCase): + """Tests for _resolve_delegation_credentials with local providers.""" + + # --- base_url path (localhost) --- + + def test_localhost_base_url_no_api_key_allowed(self): + """localhost base_url should work without an API key (Ollama, LM Studio, etc.).""" + parent = _make_mock_parent() + cfg = { + "model": "devstral-small-2:24b-cloud", + "provider": "custom", + "base_url": "http://localhost:11434/v1", + "api_key": "", + } + creds = _resolve_delegation_credentials(cfg, parent) + self.assertEqual(creds["base_url"], "http://localhost:11434/v1") + self.assertIsNotNone(creds["api_key"]) + # API key should be a harmless placeholder, not None + self.assertNotEqual(creds["api_key"], "") + + def test_127_base_url_no_api_key_allowed(self): + """127.0.0.1 base_url should work without an API key.""" + parent = _make_mock_parent() + cfg = { + "model": "devstral-small-2:24b-cloud", + "provider": "", + "base_url": "http://127.0.0.1:11434/v1", + "api_key": "", + } + creds = _resolve_delegation_credentials(cfg, parent) + self.assertEqual(creds["base_url"], "http://127.0.0.1:11434/v1") + self.assertIsNotNone(creds["api_key"]) + + def test_dotlocal_base_url_no_api_key_allowed(self): + """.local mDNS hostnames (e.g. studio.local) should work without an API key.""" + parent = _make_mock_parent() + cfg = { + "model": "devstral-small-2:24b-cloud", + "provider": "", + "base_url": "http://studio.local:11434/v1", + "api_key": "", + } + creds = _resolve_delegation_credentials(cfg, parent) + self.assertEqual(creds["base_url"], "http://studio.local:11434/v1") + self.assertIsNotNone(creds["api_key"]) + + def test_localhost_base_url_with_explicit_api_key_preserved(self): + """If user provides an API key for localhost, it should be preserved as-is.""" + parent = _make_mock_parent() + cfg = { + "model": "devstral-small-2:24b-cloud", + "provider": "custom", + "base_url": "http://localhost:11434/v1", + "api_key": "my-secret-key", + } + creds = _resolve_delegation_credentials(cfg, parent) + self.assertEqual(creds["api_key"], "my-secret-key") + + # --- base_url path (remote) should still require API key --- + + def test_remote_base_url_still_requires_api_key(self): + """Non-localhost base_url without API key should still raise ValueError.""" + parent = _make_mock_parent() + cfg = { + "model": "gpt-4o-mini", + "provider": "", + "base_url": "https://api.openai.com/v1", + "api_key": "", + } + with patch.dict(os.environ, {"OPENAI_API_KEY": ""}, clear=False): + with self.assertRaises(ValueError) as ctx: + _resolve_delegation_credentials(cfg, parent) + self.assertIn("API key", str(ctx.exception)) + + # --- provider path with custom/local --- + + @patch("hermes_cli.runtime_provider.resolve_runtime_provider") + def test_custom_provider_resolving_to_localhost_no_api_key(self, mock_resolve): + """When delegation.provider='custom' resolves to localhost, empty API key should be allowed.""" + mock_resolve.return_value = { + "provider": "custom", + "base_url": "http://localhost:11434/v1", + "api_key": "", + "api_mode": "chat_completions", + } + parent = _make_mock_parent() + cfg = {"model": "devstral-small-2:24b-cloud", "provider": "custom"} + creds = _resolve_delegation_credentials(cfg, parent) + self.assertEqual(creds["provider"], "custom") + self.assertEqual(creds["base_url"], "http://localhost:11434/v1") + # Should get a placeholder key, not raise ValueError + self.assertIsNotNone(creds["api_key"]) + self.assertNotEqual(creds["api_key"], "") + + @patch("hermes_cli.runtime_provider.resolve_runtime_provider") + def test_remote_provider_still_requires_api_key(self, mock_resolve): + """Provider resolving to a remote endpoint without API key should still raise.""" + mock_resolve.return_value = { + "provider": "openrouter", + "base_url": "https://openrouter.ai/api/v1", + "api_key": "", + "api_mode": "chat_completions", + } + parent = _make_mock_parent() + cfg = {"model": "some-model", "provider": "openrouter"} + with self.assertRaises(ValueError) as ctx: + _resolve_delegation_credentials(cfg, parent) + self.assertIn("no API key", str(ctx.exception)) + + # --- Integration: child agent gets local placeholder key --- + + @patch("tools.delegate_tool._load_config") + def test_local_delegation_uses_placeholder_key(self, mock_cfg): + """Delegation with localhost base_url should get 'ollama' placeholder API key.""" + mock_cfg.return_value = { + "model": "devstral-small-2:24b-cloud", + "provider": "custom", + "base_url": "http://localhost:11434/v1", + "api_key": "", + "max_iterations": 10, + "max_concurrent_children": 1, + } + parent = _make_mock_parent() + creds = _resolve_delegation_credentials(mock_cfg.return_value, parent) + self.assertEqual(creds["base_url"], "http://localhost:11434/v1") + self.assertEqual(creds["api_key"], "ollama") + + +class TestIsLocalBaseUrlHelper(unittest.TestCase): + """Tests for the _is_local_base_url helper function.""" + + def test_localhost_with_port(self): + from tools.delegate_tool import _is_local_base_url + self.assertTrue(_is_local_base_url("http://localhost:11434/v1")) + + def test_localhost_no_port(self): + from tools.delegate_tool import _is_local_base_url + self.assertTrue(_is_local_base_url("http://localhost/v1")) + + def test_127_ip(self): + from tools.delegate_tool import _is_local_base_url + self.assertTrue(_is_local_base_url("http://127.0.0.1:11434/v1")) + + def test_dotlocal(self): + from tools.delegate_tool import _is_local_base_url + self.assertTrue(_is_local_base_url("http://studio.local:11434/v1")) + + def test_remote_url(self): + from tools.delegate_tool import _is_local_base_url + self.assertFalse(_is_local_base_url("https://api.openai.com/v1")) + + def test_openrouter(self): + from tools.delegate_tool import _is_local_base_url + self.assertFalse(_is_local_base_url("https://openrouter.ai/api/v1")) + + def test_192_168_private(self): + from tools.delegate_tool import _is_local_base_url + self.assertTrue(_is_local_base_url("http://192.168.1.100:11434/v1")) + + def test_10_private(self): + from tools.delegate_tool import _is_local_base_url + self.assertTrue(_is_local_base_url("http://10.0.0.5:11434/v1")) + + def test_172_private(self): + from tools.delegate_tool import _is_local_base_url + self.assertTrue(_is_local_base_url("http://172.16.0.1:11434/v1")) + + def test_empty_string(self): + from tools.delegate_tool import _is_local_base_url + self.assertFalse(_is_local_base_url("")) + + def test_none(self): + from tools.delegate_tool import _is_local_base_url + self.assertFalse(_is_local_base_url(None)) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tools/delegate_tool.py b/tools/delegate_tool.py index 2bbf354cf..bdd99a90d 100644 --- a/tools/delegate_tool.py +++ b/tools/delegate_tool.py @@ -2098,6 +2098,37 @@ def _resolve_child_credential_pool(effective_provider: Optional[str], parent_age return None +def _is_local_base_url(base_url: Optional[str]) -> bool: + """Return True if base_url points to a local/private network address. + + Local providers (Ollama, LM Studio, llama.cpp server, etc.) typically + don't require authentication. This check covers: + - localhost / loopback (127.0.0.1, ::1) + - .local mDNS hostnames (e.g. studio.local) + - RFC 1918 private networks (10/8, 172.16/12, 192.168/16) + """ + if not base_url: + return False + hostname = base_url_hostname(base_url) + if not hostname: + return False + # localhost variants + if hostname in ("localhost", "127.0.0.1", "::1"): + return True + # mDNS .local hostnames + if hostname.endswith(".local"): + return True + # RFC 1918 private subnets + import ipaddress + + try: + ip = ipaddress.ip_address(hostname) + return ip.is_private or ip.is_loopback + except ValueError: + pass # not an IP address, that's fine + return False + + def _resolve_delegation_credentials(cfg: dict, parent_agent) -> dict: """Resolve credentials for subagent delegation. @@ -2111,6 +2142,10 @@ def _resolve_delegation_credentials(cfg: dict, parent_agent) -> dict: If neither base_url nor provider is configured, returns None values so the child inherits everything from the parent agent. + Local endpoints (localhost, 127.0.0.1, .local, RFC 1918 private nets) + don't require API keys — a placeholder "ollama" key is used when none + is provided, since these servers accept any or no authentication. + Raises ValueError with a user-friendly message on credential failure. """ configured_model = str(cfg.get("model") or "").strip() or None @@ -2120,6 +2155,10 @@ def _resolve_delegation_credentials(cfg: dict, parent_agent) -> dict: if configured_base_url: api_key = configured_api_key or os.getenv("OPENAI_API_KEY", "").strip() + # Local endpoints (Ollama, LM Studio, etc.) don't require auth. + # Use a dummy key so the OpenAI client doesn't reject the request. + if not api_key and _is_local_base_url(configured_base_url): + api_key = "ollama" if not api_key: raise ValueError( "Delegation base_url is configured but no API key was found. " @@ -2175,10 +2214,15 @@ def _resolve_delegation_credentials(cfg: dict, parent_agent) -> dict: api_key = runtime.get("api_key", "") if not api_key: - raise ValueError( - f"Delegation provider '{configured_provider}' resolved but has no API key. " - f"Set the appropriate environment variable or run 'hermes auth'." - ) + # Local providers don't require real API keys. + resolved_base = runtime.get("base_url", "") + if _is_local_base_url(resolved_base): + api_key = "ollama" + else: + raise ValueError( + f"Delegation provider '{configured_provider}' resolved but has no API key. " + f"Set the appropriate environment variable or run 'hermes auth'." + ) return { "model": configured_model,