mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix: allow local providers (Ollama, LM Studio) without API keys in delegation
Local model servers running on localhost, 127.0.0.1, .local mDNS hostnames, or RFC 1918 private networks don't require authentication. Previously, _resolve_delegation_credentials() hard-required an API key for any base_url, making it impossible to use Ollama or similar local servers for subagent delegation without setting a dummy key. Changes: - Add _is_local_base_url() helper detecting localhost, loopback, .local, and RFC 1918 private network addresses - base_url path: skip API key requirement for local endpoints, use 'ollama' placeholder key (accepted by local servers) - provider path: same logic — if resolve_runtime_provider returns an empty API key but the resolved base_url is local, use 'ollama' placeholder - Remote endpoints still require a real API key (no security regression) - Update existing test to use remote URL (was testing localhost which is now allowed) - Add 19 new tests covering local provider credential resolution
This commit is contained in:
parent
94f1758742
commit
0e4bc9474d
3 changed files with 280 additions and 5 deletions
|
|
@ -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,
|
||||
|
|
|
|||
229
tests/tools/test_delegation_local_provider.py
Normal file
229
tests/tools/test_delegation_local_provider.py
Normal file
|
|
@ -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()
|
||||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue