mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-26 17:38:36 +00:00
fix(api): gate bare-model passthrough + route-alias model leak
Follow-ups on the salvaged #54426 routing contract: - Bare `model` without `provider` on the OpenAI-compatible endpoints (/v1/chat/completions, /v1/responses) is now opt-in via gateway.platforms.api_server.direct_model_requests (default off) — generic OpenAI clients hardcode model names ('gpt-4o', ...) and existing deployments rely on those falling back to the gateway default. Explicit `provider` requests and the Hermes-native session-chat + /v1/runs surfaces are always honored. Idea credit: PR #22825 by @mssteuer. - A model_routes alias with no `model` key can no longer leak the alias string as the executing model name (defensive; parse-time validation already drops such routes). - Fix mis-indented _run_agent call args in _handle_session_chat_stream. - Docs: document the opt-in flag.
This commit is contained in:
parent
d66a82000c
commit
9f384783e7
3 changed files with 203 additions and 14 deletions
|
|
@ -293,26 +293,41 @@ def _resolve_request_runtime_agent_kwargs(provider: str, target_model: Optional[
|
|||
}
|
||||
|
||||
|
||||
def _request_agent_overrides(body: Any, *, virtual_model: Optional[str] = None) -> Dict[str, Any]:
|
||||
def _request_agent_overrides(
|
||||
body: Any,
|
||||
*,
|
||||
virtual_model: Optional[str] = None,
|
||||
allow_bare_model: bool = True,
|
||||
) -> Dict[str, Any]:
|
||||
"""Extract per-request model/provider/options for _run_agent.
|
||||
|
||||
``/v1/models`` advertises a stable virtual model (usually ``hermes-agent``)
|
||||
for OpenAI-compatible clients. Treat that alias as "use the gateway
|
||||
default"; real model picker selections from the browser extension send the
|
||||
raw provider model id plus a provider slug and should override this turn.
|
||||
|
||||
``allow_bare_model`` controls whether a ``model`` value WITHOUT an
|
||||
accompanying ``provider`` is honored. Generic OpenAI clients routinely
|
||||
hardcode model names ("gpt-4o", ...), and existing deployments rely on
|
||||
those falling back to the gateway default on the OpenAI-compatible
|
||||
surfaces — so those handlers pass the opt-in
|
||||
``direct_model_requests`` config value here, while Hermes-native
|
||||
endpoints (session chat, /v1/runs) always allow it. A request that
|
||||
sends an explicit ``provider`` is unambiguously Hermes-aware and is
|
||||
always honored.
|
||||
"""
|
||||
if not isinstance(body, dict):
|
||||
return {}
|
||||
|
||||
overrides: Dict[str, Any] = {}
|
||||
model = _clean_request_string(body.get("model"))
|
||||
if model and model != virtual_model:
|
||||
overrides["requested_model"] = model
|
||||
|
||||
provider = _clean_request_string(body.get("provider"))
|
||||
if provider:
|
||||
overrides["requested_provider"] = provider
|
||||
|
||||
model = _clean_request_string(body.get("model"))
|
||||
if model and model != virtual_model and (provider or allow_bare_model):
|
||||
overrides["requested_model"] = model
|
||||
|
||||
model_options = body.get("model_options")
|
||||
if isinstance(model_options, dict):
|
||||
overrides["model_options"] = dict(model_options)
|
||||
|
|
@ -1188,6 +1203,18 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
self._model_routes: Dict[str, Dict[str, Any]] = self._parse_model_routes(
|
||||
extra.get("model_routes"),
|
||||
)
|
||||
# direct_model_requests: opt-in passthrough for a bare ``model`` value
|
||||
# (no ``provider``) on the OpenAI-compatible surfaces
|
||||
# (/v1/chat/completions, /v1/responses). Off by default: generic
|
||||
# OpenAI clients routinely hardcode model names ("gpt-4o", ...), and
|
||||
# existing deployments rely on those falling back to the gateway
|
||||
# default rather than switching the executing model. Requests that
|
||||
# send an explicit ``provider`` — and the Hermes-native session-chat
|
||||
# and /v1/runs endpoints — are always honored regardless of this flag.
|
||||
# (Idea credit: PR #22825 by @mssteuer.)
|
||||
self._direct_model_requests: bool = _coerce_request_bool(
|
||||
extra.get("direct_model_requests"), default=False
|
||||
)
|
||||
self._app: Optional["web.Application"] = None
|
||||
self._runner: Optional["web.AppRunner"] = None
|
||||
self._site: Optional["web.TCPSite"] = None
|
||||
|
|
@ -2144,7 +2171,14 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
session_key or "",
|
||||
)
|
||||
else:
|
||||
effective_model = route_model or request_model or model
|
||||
if route is not None:
|
||||
# The request's ``model`` field selected this route, so its
|
||||
# value is the route ALIAS — never usable as a model name.
|
||||
# A route with no ``model`` key keeps the global default
|
||||
# (pre-existing model_routes behavior).
|
||||
effective_model = route_model or model
|
||||
else:
|
||||
effective_model = request_model or model
|
||||
current_provider = _clean_request_string(runtime_kwargs.get("provider"))
|
||||
effective_provider = request_provider or route_provider or current_provider
|
||||
provider_runtime = None
|
||||
|
|
@ -2939,12 +2973,12 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
conversation_history=history,
|
||||
ephemeral_system_prompt=system_prompt,
|
||||
session_id=session_id,
|
||||
stream_delta_callback=_delta,
|
||||
tool_progress_callback=_tool_progress,
|
||||
gateway_session_key=gateway_session_key,
|
||||
route=route,
|
||||
**agent_overrides,
|
||||
)
|
||||
stream_delta_callback=_delta,
|
||||
tool_progress_callback=_tool_progress,
|
||||
gateway_session_key=gateway_session_key,
|
||||
route=route,
|
||||
**agent_overrides,
|
||||
)
|
||||
final_response = _resolve_media_to_data_urls(result.get("final_response", "") if isinstance(result, dict) else "")
|
||||
effective_session_id = result.get("session_id", session_id) if isinstance(result, dict) else session_id
|
||||
turn_messages = self._turn_transcript_messages(history, user_message, result) if isinstance(result, dict) else []
|
||||
|
|
@ -3144,7 +3178,11 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
# configured model_routes alias, this request's agent is created
|
||||
# with that route's model/provider instead of the global default.
|
||||
route = self._resolve_route(model_name)
|
||||
agent_overrides = _request_agent_overrides(body, virtual_model=self._model_name)
|
||||
agent_overrides = _request_agent_overrides(
|
||||
body,
|
||||
virtual_model=self._model_name,
|
||||
allow_bare_model=self._direct_model_requests,
|
||||
)
|
||||
selection_error = self._request_route_conflict_error(
|
||||
session_id=session_id,
|
||||
gateway_session_key=gateway_session_key,
|
||||
|
|
@ -4279,7 +4317,11 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
|
||||
stream = _coerce_request_bool(body.get("stream"), default=False)
|
||||
route = self._resolve_route(body.get("model"))
|
||||
agent_overrides = _request_agent_overrides(body, virtual_model=self._model_name)
|
||||
agent_overrides = _request_agent_overrides(
|
||||
body,
|
||||
virtual_model=self._model_name,
|
||||
allow_bare_model=self._direct_model_requests,
|
||||
)
|
||||
selection_error = self._request_route_conflict_error(
|
||||
session_id=session_id,
|
||||
gateway_session_key=gateway_session_key,
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@ from gateway.platforms.api_server import (
|
|||
_derive_chat_session_id,
|
||||
_hermes_version,
|
||||
_redact_api_error_text,
|
||||
_request_agent_overrides,
|
||||
check_api_server_requirements,
|
||||
cors_middleware,
|
||||
security_headers_middleware,
|
||||
|
|
@ -5336,3 +5337,132 @@ class TestKeyRejectionSetsNonRetryableFatalError:
|
|||
assert adapter.fatal_error_retryable is True
|
||||
finally:
|
||||
await adapter.disconnect()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Bare-model opt-in gate (direct_model_requests) for _request_agent_overrides
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestDirectModelRequestsGate:
|
||||
"""Bare ``model`` (no ``provider``) is opt-in on OpenAI-compatible
|
||||
endpoints so generic clients hardcoding "gpt-4o" keep falling back to
|
||||
the gateway default (idea credit: PR #22825 by @mssteuer)."""
|
||||
|
||||
def test_bare_model_dropped_when_disallowed(self):
|
||||
overrides = _request_agent_overrides(
|
||||
{"model": "openai/gpt-5"}, allow_bare_model=False
|
||||
)
|
||||
assert "requested_model" not in overrides
|
||||
|
||||
def test_bare_model_kept_when_allowed(self):
|
||||
overrides = _request_agent_overrides(
|
||||
{"model": "openai/gpt-5"}, allow_bare_model=True
|
||||
)
|
||||
assert overrides["requested_model"] == "openai/gpt-5"
|
||||
|
||||
def test_explicit_provider_always_honors_model(self):
|
||||
overrides = _request_agent_overrides(
|
||||
{"model": "MiniMax-M3", "provider": "minimax"}, allow_bare_model=False
|
||||
)
|
||||
assert overrides["requested_model"] == "MiniMax-M3"
|
||||
assert overrides["requested_provider"] == "minimax"
|
||||
|
||||
def test_model_options_survive_bare_model_drop(self):
|
||||
overrides = _request_agent_overrides(
|
||||
{"model": "openai/gpt-5", "model_options": {"fast": True}},
|
||||
allow_bare_model=False,
|
||||
)
|
||||
assert overrides == {"model_options": {"fast": True}}
|
||||
|
||||
def test_virtual_model_alias_still_ignored(self):
|
||||
overrides = _request_agent_overrides(
|
||||
{"model": "hermes-agent", "provider": "minimax"},
|
||||
virtual_model="hermes-agent",
|
||||
allow_bare_model=True,
|
||||
)
|
||||
assert "requested_model" not in overrides
|
||||
assert overrides["requested_provider"] == "minimax"
|
||||
|
||||
def test_adapter_flag_default_off(self):
|
||||
adapter = APIServerAdapter(PlatformConfig(enabled=True, extra={}))
|
||||
assert adapter._direct_model_requests is False
|
||||
|
||||
def test_adapter_flag_opt_in(self):
|
||||
adapter = APIServerAdapter(
|
||||
PlatformConfig(enabled=True, extra={"direct_model_requests": True})
|
||||
)
|
||||
assert adapter._direct_model_requests is True
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_chat_completions_bare_model_ignored_by_default(self):
|
||||
adapter = APIServerAdapter(PlatformConfig(enabled=True, extra={}))
|
||||
app = _create_app(adapter)
|
||||
async with TestClient(TestServer(app)) as cli:
|
||||
with patch.object(adapter, "_run_agent", new_callable=AsyncMock) as mock_run:
|
||||
mock_run.return_value = (
|
||||
{"final_response": "ok", "messages": [], "api_calls": 1},
|
||||
{"input_tokens": 1, "output_tokens": 1, "total_tokens": 2},
|
||||
)
|
||||
resp = await cli.post(
|
||||
"/v1/chat/completions",
|
||||
json={
|
||||
"model": "gpt-4o",
|
||||
"messages": [{"role": "user", "content": "hi"}],
|
||||
},
|
||||
)
|
||||
assert resp.status == 200
|
||||
assert mock_run.call_args.kwargs.get("requested_model") is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_chat_completions_bare_model_honored_when_enabled(self):
|
||||
adapter = APIServerAdapter(
|
||||
PlatformConfig(enabled=True, extra={"direct_model_requests": True})
|
||||
)
|
||||
app = _create_app(adapter)
|
||||
async with TestClient(TestServer(app)) as cli:
|
||||
with patch.object(adapter, "_run_agent", new_callable=AsyncMock) as mock_run:
|
||||
mock_run.return_value = (
|
||||
{"final_response": "ok", "messages": [], "api_calls": 1},
|
||||
{"input_tokens": 1, "output_tokens": 1, "total_tokens": 2},
|
||||
)
|
||||
resp = await cli.post(
|
||||
"/v1/chat/completions",
|
||||
json={
|
||||
"model": "openai/gpt-5",
|
||||
"messages": [{"role": "user", "content": "hi"}],
|
||||
},
|
||||
)
|
||||
assert resp.status == 200
|
||||
assert mock_run.call_args.kwargs.get("requested_model") == "openai/gpt-5"
|
||||
|
||||
|
||||
class TestRouteWithoutModelKeepsDefault:
|
||||
"""A model_routes alias whose route has no ``model`` key must keep the
|
||||
global default model — the alias string itself is never a model name."""
|
||||
|
||||
def test_alias_never_leaks_as_model(self, monkeypatch):
|
||||
captured = {}
|
||||
|
||||
class FakeAgent:
|
||||
def __init__(self, **kwargs):
|
||||
captured.update(kwargs)
|
||||
|
||||
_patch_create_agent_runtime(monkeypatch, captured, FakeAgent)
|
||||
adapter = _make_routing_adapter(
|
||||
{"alias": {"model": "", "api_key": "sk-route"}}
|
||||
)
|
||||
# _parse_model_routes drops routes without model; simulate a
|
||||
# credentials-only route surviving via direct dict (defensive path).
|
||||
adapter._model_routes = {"alias": {"api_key": "sk-route"}}
|
||||
monkeypatch.setattr(adapter, "_ensure_session_db", lambda: None)
|
||||
monkeypatch.setattr(adapter, "_session_model_override_for", lambda *_: None)
|
||||
|
||||
adapter._create_agent(
|
||||
session_id="s1",
|
||||
route=adapter._resolve_route("alias"),
|
||||
requested_model="alias",
|
||||
)
|
||||
|
||||
assert captured["model"] == "global/model"
|
||||
assert captured["api_key"] == "sk-route"
|
||||
|
|
|
|||
|
|
@ -251,6 +251,23 @@ If a request sends a `provider` that conflicts with a configured `model_routes`
|
|||
alias, Hermes rejects the request with `400` instead of silently remixing route
|
||||
credentials with another provider.
|
||||
|
||||
**Bare `model` values on the OpenAI-compatible endpoints are opt-in.** Generic
|
||||
OpenAI clients routinely hardcode model names (`gpt-4o`, ...), and existing
|
||||
deployments rely on those falling back to the gateway default. On
|
||||
`POST /v1/chat/completions` and `POST /v1/responses`, a `model` value sent
|
||||
WITHOUT a `provider` is therefore ignored unless you enable:
|
||||
|
||||
```yaml
|
||||
gateway:
|
||||
platforms:
|
||||
api_server:
|
||||
direct_model_requests: true
|
||||
```
|
||||
|
||||
Requests that include an explicit `provider` — and the Hermes-native
|
||||
`/v1/runs` and session-chat endpoints — always honor the requested model
|
||||
regardless of this flag.
|
||||
|
||||
Example:
|
||||
|
||||
```json
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue