Merge pull request #71141 from NousResearch/bb/custom-endpoint-keys-and-models
fix: custom endpoint keys go to .env, and Save keeps the whole model list
This commit is contained in:
brooklyn!
GitHub
commit
8f8b66d8ac
|
|
@ -58,7 +58,7 @@ function formFromEndpoint(endpoint: CustomEndpoint): EndpointForm {
|
|||
}
|
||||
}
|
||||
|
||||
function toPayload(form: EndpointForm): CustomEndpointUpdate {
|
||||
function toPayload(form: EndpointForm, models?: string[]): CustomEndpointUpdate {
|
||||
const contextLength = Number.parseInt(form.contextLength, 10)
|
||||
|
||||
return {
|
||||
|
|
@ -69,7 +69,8 @@ function toPayload(form: EndpointForm): CustomEndpointUpdate {
|
|||
api_key: form.apiKey.trim() || undefined,
|
||||
context_length: Number.isFinite(contextLength) && contextLength > 0 ? contextLength : undefined,
|
||||
discover_models: form.discoverModels,
|
||||
make_default: form.makeDefault
|
||||
make_default: form.makeDefault,
|
||||
models: models?.length ? models : undefined
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -125,7 +126,7 @@ export function CustomEndpointsSettings({ onConfigSaved, onMainModelChanged }: C
|
|||
async function handleSave() {
|
||||
try {
|
||||
setSaving(true)
|
||||
const response = await saveCustomEndpoint(toPayload(form))
|
||||
const response = await saveCustomEndpoint(toPayload(form, discoveredModels))
|
||||
setEndpoints(response.endpoints)
|
||||
const saved = response.endpoints.find(endpoint => endpoint.id === response.id)
|
||||
|
||||
|
|
|
|||
|
|
@ -182,6 +182,7 @@ export interface CustomEndpointUpdate {
|
|||
id?: string
|
||||
make_default?: boolean
|
||||
model: string
|
||||
models?: string[]
|
||||
name: string
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -8209,6 +8209,25 @@ def save_env_value(key: str, value: str):
|
|||
invalidate_env_cache()
|
||||
|
||||
|
||||
def custom_endpoint_key_env(identity: str) -> str:
|
||||
"""Env var name holding a custom endpoint's API key.
|
||||
|
||||
``identity`` is whatever names the endpoint on the calling path — the
|
||||
Desktop panel's endpoint id, or ``host:port`` for the CLI setup flow.
|
||||
Two properties matter:
|
||||
|
||||
- It keys off the endpoint's own identity, not just its hostname, so two
|
||||
endpoints on one host (``127.0.0.1:8000`` and ``:8001``) get separate
|
||||
slots instead of the second save clobbering the first's credential.
|
||||
- The fixed ``HERMES_CUSTOM_`` prefix keeps the result a valid POSIX name
|
||||
even when the slug starts with a digit, which every IP-based local
|
||||
endpoint does (``127.0.0.1`` → ``127_0_0_1``). ``save_env_value``
|
||||
rejects digit-leading names outright.
|
||||
"""
|
||||
slug = re.sub(r"[^A-Z0-9]+", "_", str(identity or "").upper()).strip("_")
|
||||
return f"HERMES_CUSTOM_{slug}_API_KEY" if slug else "HERMES_CUSTOM_API_KEY"
|
||||
|
||||
|
||||
def remove_env_value(key: str) -> bool:
|
||||
"""Remove a key from ~/.hermes/.env and os.environ.
|
||||
|
||||
|
|
|
|||
|
|
@ -3975,13 +3975,17 @@ def _custom_provider_base_url_config_value(provider_info, resolved_base_url=""):
|
|||
|
||||
|
||||
def _save_custom_provider(
|
||||
base_url, api_key="", model="", context_length=None, name=None, api_mode=None
|
||||
base_url, api_key="", model="", context_length=None, name=None, api_mode=None,
|
||||
key_env=""
|
||||
):
|
||||
"""Save a custom endpoint to custom_providers in config.yaml.
|
||||
|
||||
Deduplicates by base_url — if the URL already exists, updates the
|
||||
model name, context_length, and api_mode but doesn't add a duplicate entry.
|
||||
Uses *name* when provided, otherwise auto-generates from the URL.
|
||||
|
||||
When *key_env* is set the caller has already written the key to ``.env``,
|
||||
so the entry references it instead of inlining the secret (#69449).
|
||||
"""
|
||||
from hermes_cli.config import load_config, save_config
|
||||
|
||||
|
|
@ -4013,6 +4017,10 @@ def _save_custom_provider(
|
|||
elif "api_mode" in entry:
|
||||
entry.pop("api_mode", None)
|
||||
changed = True
|
||||
if key_env and (entry.get("key_env") != key_env or entry.get("api_key")):
|
||||
entry["key_env"] = key_env
|
||||
entry.pop("api_key", None)
|
||||
changed = True
|
||||
if changed:
|
||||
cfg["custom_providers"] = providers
|
||||
save_config(cfg)
|
||||
|
|
@ -4023,7 +4031,9 @@ def _save_custom_provider(
|
|||
name = _auto_provider_name(base_url)
|
||||
|
||||
entry = {"name": name, "base_url": base_url}
|
||||
if api_key:
|
||||
if key_env:
|
||||
entry["key_env"] = key_env
|
||||
elif api_key:
|
||||
entry["api_key"] = api_key
|
||||
if model:
|
||||
entry["model"] = model
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ from __future__ import annotations
|
|||
import argparse
|
||||
import os
|
||||
import subprocess
|
||||
import urllib.parse
|
||||
|
||||
from hermes_cli.config import clear_model_endpoint_credentials
|
||||
|
||||
|
|
@ -833,7 +834,13 @@ def _model_flow_custom(config):
|
|||
"""
|
||||
from hermes_cli.main import _auto_provider_name, _prompt_custom_api_mode_selection, _save_custom_provider
|
||||
from hermes_cli.auth import _save_model_choice, deactivate_provider
|
||||
from hermes_cli.config import get_env_value, load_config, save_config
|
||||
from hermes_cli.config import (
|
||||
custom_endpoint_key_env,
|
||||
get_env_value,
|
||||
load_config,
|
||||
save_config,
|
||||
save_env_value,
|
||||
)
|
||||
from hermes_cli.secret_prompt import masked_secret_prompt
|
||||
|
||||
current_url = get_env_value("OPENAI_BASE_URL") or ""
|
||||
|
|
@ -988,6 +995,18 @@ def _model_flow_custom(config):
|
|||
print(f"Invalid context length: {context_length_str} — will auto-detect.")
|
||||
context_length = None
|
||||
|
||||
# The key goes to .env and config.yaml only references it (#69449). Keyed
|
||||
# on host:port so two servers on one machine keep separate credentials.
|
||||
custom_key_env = ""
|
||||
if effective_key:
|
||||
_parsed = urllib.parse.urlparse(effective_url)
|
||||
_identity = _parsed.hostname or ""
|
||||
if _parsed.port:
|
||||
_identity = f"{_identity}_{_parsed.port}"
|
||||
custom_key_env = custom_endpoint_key_env(_identity)
|
||||
save_env_value(custom_key_env, effective_key)
|
||||
print(f" API key saved to .env as {custom_key_env}")
|
||||
|
||||
if model_name:
|
||||
_save_model_choice(model_name)
|
||||
|
||||
|
|
@ -999,8 +1018,8 @@ def _model_flow_custom(config):
|
|||
cfg["model"] = model
|
||||
model["provider"] = "custom"
|
||||
model["base_url"] = effective_url
|
||||
if effective_key:
|
||||
model["api_key"] = effective_key
|
||||
if custom_key_env:
|
||||
model["api_key"] = f"${{{custom_key_env}}}"
|
||||
if api_mode:
|
||||
model["api_mode"] = api_mode
|
||||
else:
|
||||
|
|
@ -1025,8 +1044,8 @@ def _model_flow_custom(config):
|
|||
_caller_model = {"default": _caller_model} if _caller_model else {}
|
||||
_caller_model["provider"] = "custom"
|
||||
_caller_model["base_url"] = effective_url
|
||||
if effective_key:
|
||||
_caller_model["api_key"] = effective_key
|
||||
if custom_key_env:
|
||||
_caller_model["api_key"] = f"${{{custom_key_env}}}"
|
||||
if api_mode:
|
||||
_caller_model["api_mode"] = api_mode
|
||||
else:
|
||||
|
|
@ -1042,6 +1061,7 @@ def _model_flow_custom(config):
|
|||
context_length=context_length,
|
||||
name=display_name,
|
||||
api_mode=api_mode,
|
||||
key_env=custom_key_env,
|
||||
)
|
||||
_prune_replaced_custom_model_config_credentials(
|
||||
effective_url,
|
||||
|
|
|
|||
|
|
@ -72,6 +72,7 @@ from hermes_cli.config import (
|
|||
save_config,
|
||||
save_env_value,
|
||||
remove_env_value,
|
||||
custom_endpoint_key_env,
|
||||
check_config_version,
|
||||
detect_install_method,
|
||||
format_docker_update_message,
|
||||
|
|
@ -1271,6 +1272,7 @@ class CustomEndpointUpdate(BaseModel):
|
|||
context_length: Optional[int] = None
|
||||
discover_models: bool = True
|
||||
make_default: bool = False
|
||||
models: Optional[List[str]] = None
|
||||
|
||||
|
||||
class MessagingPlatformUpdate(BaseModel):
|
||||
|
|
@ -7593,6 +7595,38 @@ def _models_from_custom_endpoint_entry(entry: Dict[str, Any]) -> List[str]:
|
|||
return [model for model in models if model and not (model in seen or seen.add(model))]
|
||||
|
||||
|
||||
def _api_key_display(entry: Dict[str, Any]) -> Tuple[bool, Optional[str]]:
|
||||
"""Return ``(has_api_key, preview)`` for a provider or model config block.
|
||||
|
||||
Keys live in ``.env`` behind ``key_env``; only entries written before
|
||||
#69449 still carry a plaintext ``api_key``. Checking both keeps the panel
|
||||
honest either way — reading only ``api_key`` reported "no API key" for
|
||||
every endpoint whose key had been moved to ``.env``.
|
||||
"""
|
||||
plaintext = str(entry.get("api_key") or "").strip()
|
||||
if plaintext:
|
||||
return True, redact_key(plaintext)
|
||||
key_env = str(entry.get("key_env") or "").strip()
|
||||
if key_env:
|
||||
return True, f"${{{key_env}}}"
|
||||
return False, None
|
||||
|
||||
|
||||
def _config_api_key_is_env_ref(endpoint_id: str) -> bool:
|
||||
"""True when this endpoint's on-disk ``api_key`` is a ``${VAR}`` template.
|
||||
|
||||
``load_config()`` expands env refs, so a hand-written
|
||||
``api_key: ${MY_KEY}`` is indistinguishable from a literal secret by the
|
||||
time it reaches us. Such an entry is already keeping its secret out of
|
||||
config.yaml, so migrating it would only copy that secret into a second
|
||||
env var the user didn't ask for.
|
||||
"""
|
||||
providers = read_raw_config().get("providers")
|
||||
entry = providers.get(endpoint_id) if isinstance(providers, dict) else None
|
||||
raw_key = entry.get("api_key") if isinstance(entry, dict) else None
|
||||
return bool(isinstance(raw_key, str) and re.search(r"\$\{[^}]+\}", raw_key))
|
||||
|
||||
|
||||
def _custom_endpoint_response(cfg: Dict[str, Any]) -> Dict[str, Any]:
|
||||
model_cfg = cfg.get("model", {}) if isinstance(cfg.get("model"), dict) else {}
|
||||
current_provider = str(model_cfg.get("provider", "") or "")
|
||||
|
|
@ -7611,6 +7645,7 @@ def _custom_endpoint_response(cfg: Dict[str, Any]) -> Dict[str, Any]:
|
|||
endpoint_id = str(provider_id)
|
||||
models = _models_from_custom_endpoint_entry(raw_entry)
|
||||
endpoint_model = str(raw_entry.get("model") or raw_entry.get("default_model") or (models[0] if models else ""))
|
||||
has_api_key, api_key_preview = _api_key_display(raw_entry)
|
||||
endpoints.append({
|
||||
"id": endpoint_id,
|
||||
"name": str(raw_entry.get("name") or endpoint_id),
|
||||
|
|
@ -7619,13 +7654,14 @@ def _custom_endpoint_response(cfg: Dict[str, Any]) -> Dict[str, Any]:
|
|||
"models": models,
|
||||
"context_length": raw_entry.get("context_length"),
|
||||
"discover_models": bool(raw_entry.get("discover_models", True)),
|
||||
"has_api_key": bool(str(raw_entry.get("api_key", "") or "").strip()),
|
||||
"api_key_preview": redact_key(str(raw_entry.get("api_key", "") or "")) if raw_entry.get("api_key") else None,
|
||||
"has_api_key": has_api_key,
|
||||
"api_key_preview": api_key_preview,
|
||||
"is_current": endpoint_id == current_provider,
|
||||
"source": "providers",
|
||||
})
|
||||
|
||||
if current_provider.lower() == "custom" and current_base_url and not any(e["id"] == "custom" for e in endpoints):
|
||||
has_api_key, api_key_preview = _api_key_display(model_cfg)
|
||||
endpoints.insert(0, {
|
||||
"id": "custom",
|
||||
"name": "Custom",
|
||||
|
|
@ -7634,8 +7670,8 @@ def _custom_endpoint_response(cfg: Dict[str, Any]) -> Dict[str, Any]:
|
|||
"models": [current_model] if current_model else [],
|
||||
"context_length": model_cfg.get("context_length"),
|
||||
"discover_models": True,
|
||||
"has_api_key": bool(str(model_cfg.get("api_key", "") or "").strip()),
|
||||
"api_key_preview": redact_key(str(model_cfg.get("api_key", "") or "")) if model_cfg.get("api_key") else None,
|
||||
"has_api_key": has_api_key,
|
||||
"api_key_preview": api_key_preview,
|
||||
"is_current": True,
|
||||
"source": "direct-config",
|
||||
})
|
||||
|
|
@ -7668,7 +7704,7 @@ def _detach_main_model_from_provider(cfg: Dict[str, Any], provider_key: str) ->
|
|||
return
|
||||
if str(model_cfg.get("provider") or "").strip().lower() != provider_key:
|
||||
return
|
||||
for field in ("provider", "base_url", "api_key"):
|
||||
for field in ("provider", "base_url", "api_key", "key_env"):
|
||||
model_cfg.pop(field, None)
|
||||
cfg["model"] = model_cfg
|
||||
|
||||
|
|
@ -7710,19 +7746,47 @@ def _write_custom_endpoint(cfg: Dict[str, Any], body: CustomEndpointUpdate) -> T
|
|||
"model": model,
|
||||
"discover_models": bool(body.discover_models),
|
||||
})
|
||||
# Same for the model map: the panel names one default model, it does not
|
||||
# enumerate the provider's catalogue. Keep the other models (and their
|
||||
# context lengths) and just ensure this one is present.
|
||||
# Same for the model map: merge rather than replace, so existing models
|
||||
# keep their context lengths. ``body.models`` is the catalogue the panel's
|
||||
# Test button already discovered — without it only the one hand-typed
|
||||
# model survived Save, and every picker showed a single-entry list for a
|
||||
# provider serving dozens (#69988). A payload with no ``models`` (older
|
||||
# UI) still just ensures the named default is present.
|
||||
existing_models = entry.get("models")
|
||||
models_map: Dict[str, Any] = dict(existing_models) if isinstance(existing_models, dict) else {}
|
||||
current_model_entry = models_map.get(model)
|
||||
models_map[model] = dict(current_model_entry) if isinstance(current_model_entry, dict) else {}
|
||||
for candidate in (*(body.models or ()), model):
|
||||
model_id = str(candidate).strip()
|
||||
if not model_id:
|
||||
continue
|
||||
current = models_map.get(model_id)
|
||||
models_map[model_id] = dict(current) if isinstance(current, dict) else {}
|
||||
entry["models"] = models_map
|
||||
if body.context_length and body.context_length > 0:
|
||||
entry["context_length"] = int(body.context_length)
|
||||
entry["models"][model]["context_length"] = int(body.context_length)
|
||||
if body.api_key is not None and body.api_key.strip():
|
||||
entry["api_key"] = body.api_key.strip()
|
||||
|
||||
# API keys never belong in config.yaml (#69449). Write to .env and
|
||||
# reference it via ``key_env`` — the same indirection built-in providers
|
||||
# use and that runtime_provider.py already resolves at load time.
|
||||
env_var = custom_endpoint_key_env(endpoint_id)
|
||||
submitted_key = body.api_key.strip() if body.api_key is not None else None
|
||||
if submitted_key:
|
||||
save_env_value(env_var, submitted_key)
|
||||
entry["key_env"] = env_var
|
||||
entry.pop("api_key", None)
|
||||
elif submitted_key is not None:
|
||||
# Blank field means "clear the key", not "leave it alone".
|
||||
remove_env_value(env_var)
|
||||
entry.pop("key_env", None)
|
||||
entry.pop("api_key", None)
|
||||
elif str(entry.get("api_key") or "").strip() and not _config_api_key_is_env_ref(endpoint_id):
|
||||
# No new key submitted, but this entry still carries one an earlier
|
||||
# release wrote in plaintext. Migrate it on the next save so endpoints
|
||||
# configured before the fix get cleaned up too, without the user
|
||||
# having to re-enter the key.
|
||||
save_env_value(env_var, entry["api_key"].strip())
|
||||
entry["key_env"] = env_var
|
||||
entry.pop("api_key", None)
|
||||
|
||||
providers[endpoint_id] = entry
|
||||
cfg["providers"] = providers
|
||||
|
|
@ -7731,8 +7795,9 @@ def _write_custom_endpoint(cfg: Dict[str, Any], body: CustomEndpointUpdate) -> T
|
|||
cfg["model"] = _apply_main_model_assignment(
|
||||
cfg.get("model", {}), endpoint_id, model, base_url
|
||||
)
|
||||
if entry.get("api_key") and isinstance(cfg["model"], dict):
|
||||
cfg["model"]["api_key"] = entry["api_key"]
|
||||
if entry.get("key_env") and isinstance(cfg["model"], dict):
|
||||
cfg["model"]["key_env"] = entry["key_env"]
|
||||
cfg["model"].pop("api_key", None)
|
||||
|
||||
return endpoint_id, entry
|
||||
|
||||
|
|
@ -7783,7 +7848,10 @@ def activate_custom_endpoint(endpoint_id: str):
|
|||
raise HTTPException(status_code=400, detail="custom endpoint is incomplete")
|
||||
|
||||
model_cfg = _apply_main_model_assignment(cfg.get("model", {}), provider_key, model, base_url)
|
||||
if entry.get("api_key"):
|
||||
if entry.get("key_env"):
|
||||
model_cfg["key_env"] = entry["key_env"]
|
||||
model_cfg.pop("api_key", None)
|
||||
elif entry.get("api_key"):
|
||||
model_cfg["api_key"] = entry["api_key"]
|
||||
cfg["model"] = model_cfg
|
||||
save_config(cfg)
|
||||
|
|
@ -7807,6 +7875,7 @@ def delete_custom_endpoint(endpoint_id: str):
|
|||
providers.pop(provider_key, None)
|
||||
cfg["providers"] = providers
|
||||
_detach_main_model_from_provider(cfg, provider_key)
|
||||
remove_env_value(custom_endpoint_key_env(provider_key))
|
||||
save_config(cfg)
|
||||
response = _custom_endpoint_response(cfg)
|
||||
response["ok"] = True
|
||||
|
|
|
|||
|
|
@ -793,11 +793,16 @@ def test_model_flow_custom_persists_selected_api_mode(monkeypatch):
|
|||
"used_fallback": False,
|
||||
},
|
||||
)
|
||||
saved_env = {}
|
||||
monkeypatch.setattr("hermes_cli.config.load_config", lambda: saved_cfg)
|
||||
monkeypatch.setattr("hermes_cli.config.save_config", lambda cfg: saved_cfg.update(cfg))
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.config.save_env_value",
|
||||
lambda key, value: saved_env.__setitem__(key, value),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.main._save_custom_provider",
|
||||
lambda base_url, api_key="", model="", context_length=None, name=None, api_mode=None: captured_provider.update(
|
||||
lambda base_url, api_key="", model="", context_length=None, name=None, api_mode=None, key_env="": captured_provider.update(
|
||||
{
|
||||
"base_url": base_url,
|
||||
"api_key": api_key,
|
||||
|
|
@ -805,6 +810,7 @@ def test_model_flow_custom_persists_selected_api_mode(monkeypatch):
|
|||
"context_length": context_length,
|
||||
"name": name,
|
||||
"api_mode": api_mode,
|
||||
"key_env": key_env,
|
||||
}
|
||||
),
|
||||
)
|
||||
|
|
@ -825,10 +831,14 @@ def test_model_flow_custom_persists_selected_api_mode(monkeypatch):
|
|||
|
||||
assert saved_cfg["model"]["provider"] == "custom"
|
||||
assert saved_cfg["model"]["base_url"] == "https://codex.example.com/v1"
|
||||
assert saved_cfg["model"]["api_key"] == "test-key"
|
||||
assert saved_cfg["model"]["api_mode"] == "codex_responses"
|
||||
assert captured_provider["api_mode"] == "codex_responses"
|
||||
|
||||
# The key itself goes to .env; config.yaml only references it (#69449).
|
||||
key_env = captured_provider["key_env"]
|
||||
assert saved_cfg["model"]["api_key"] == f"${{{key_env}}}"
|
||||
assert saved_env[key_env] == "test-key"
|
||||
|
||||
|
||||
def test_cmd_model_forwards_nous_login_tls_options(monkeypatch):
|
||||
monkeypatch.setattr(hermes_main, "_require_tty", lambda *a: None)
|
||||
|
|
@ -923,3 +933,81 @@ def test_save_custom_provider_uses_provided_name(monkeypatch, tmp_path):
|
|||
entries = saved.get("custom_providers", [])
|
||||
assert len(entries) == 1
|
||||
assert entries[0]["name"] == "Ollama"
|
||||
|
||||
|
||||
def test_save_custom_provider_references_the_key_instead_of_inlining_it(monkeypatch, tmp_path):
|
||||
"""With key_env set the entry must not carry the secret (#69449)."""
|
||||
import yaml
|
||||
from hermes_cli.main import _save_custom_provider
|
||||
|
||||
cfg_path = tmp_path / "config.yaml"
|
||||
cfg_path.write_text(yaml.dump({}))
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.config.load_config", lambda: yaml.safe_load(cfg_path.read_text()) or {},
|
||||
)
|
||||
saved = {}
|
||||
monkeypatch.setattr("hermes_cli.config.save_config", lambda cfg: saved.update(cfg))
|
||||
|
||||
_save_custom_provider(
|
||||
"http://localhost:11434/v1",
|
||||
api_key="sk-secret",
|
||||
name="Ollama",
|
||||
key_env="HERMES_CUSTOM_LOCALHOST_11434_API_KEY",
|
||||
)
|
||||
|
||||
entry = saved["custom_providers"][0]
|
||||
assert entry["key_env"] == "HERMES_CUSTOM_LOCALHOST_11434_API_KEY"
|
||||
assert "api_key" not in entry
|
||||
assert "sk-secret" not in yaml.safe_dump(saved)
|
||||
|
||||
|
||||
def test_save_custom_provider_migrates_an_existing_plaintext_entry(monkeypatch, tmp_path):
|
||||
"""Re-saving a known URL swaps its inline key for the .env reference."""
|
||||
import yaml
|
||||
from hermes_cli.main import _save_custom_provider
|
||||
|
||||
existing = {
|
||||
"custom_providers": [
|
||||
{
|
||||
"name": "Ollama",
|
||||
"base_url": "http://localhost:11434/v1",
|
||||
"api_key": "sk-legacy",
|
||||
}
|
||||
]
|
||||
}
|
||||
monkeypatch.setattr("hermes_cli.config.load_config", lambda: existing)
|
||||
saved = {}
|
||||
monkeypatch.setattr("hermes_cli.config.save_config", lambda cfg: saved.update(cfg))
|
||||
|
||||
_save_custom_provider(
|
||||
"http://localhost:11434/v1",
|
||||
key_env="HERMES_CUSTOM_LOCALHOST_11434_API_KEY",
|
||||
)
|
||||
|
||||
entry = saved["custom_providers"][0]
|
||||
assert entry["key_env"] == "HERMES_CUSTOM_LOCALHOST_11434_API_KEY"
|
||||
assert "api_key" not in entry
|
||||
|
||||
|
||||
def test_custom_endpoint_key_env_is_a_valid_posix_name_for_ip_endpoints():
|
||||
"""Every IP-based local endpoint slugs to a digit-leading name.
|
||||
|
||||
``save_env_value`` rejects names that don't match
|
||||
``[A-Za-z_][A-Za-z0-9_]*``, so deriving ``127_0_0_1_8080_API_KEY`` would
|
||||
raise on exactly the local-proxy setups this is meant to protect. The
|
||||
fixed prefix makes the result valid by construction.
|
||||
"""
|
||||
import re
|
||||
|
||||
from hermes_cli.config import _ENV_VAR_NAME_RE, custom_endpoint_key_env
|
||||
|
||||
for identity in ("127.0.0.1_8080", "0.0.0.0", "10.0.0.7:11434", "", "-–-"):
|
||||
assert _ENV_VAR_NAME_RE.match(custom_endpoint_key_env(identity)), identity
|
||||
|
||||
|
||||
def test_custom_endpoint_key_env_separates_ports_on_one_host():
|
||||
"""Two servers on one machine must not collapse onto one .env slot."""
|
||||
from hermes_cli.config import custom_endpoint_key_env
|
||||
|
||||
assert custom_endpoint_key_env("127.0.0.1_8000") != custom_endpoint_key_env("127.0.0.1_8001")
|
||||
assert custom_endpoint_key_env("acme") == custom_endpoint_key_env("ACME")
|
||||
|
|
|
|||
|
|
@ -4664,15 +4664,15 @@ class TestWebServerEndpoints:
|
|||
assert models["acme/model-1"]["context_length"] == 200000
|
||||
|
||||
def test_deleting_the_active_custom_endpoint_clears_its_model_mirror(self):
|
||||
"""Deleting an endpoint must not leave its key running the agent.
|
||||
"""Deleting an endpoint must not leave its credential running the agent.
|
||||
|
||||
``activate`` copies the endpoint's base_url + api_key onto ``model``,
|
||||
and ``model.api_key`` outranks the environment at client construction
|
||||
(#62269). Without clearing that mirror the agent keeps authenticating
|
||||
to the deleted host with the deleted key, and the key the operator
|
||||
just removed through the dashboard stays in config.yaml.
|
||||
``activate`` mirrors the endpoint's base_url + credential reference
|
||||
onto ``model``, and that mirror outranks the environment at client
|
||||
construction (#62269). Without clearing it the agent keeps
|
||||
authenticating to the deleted host, and the credential the operator
|
||||
just removed through the dashboard survives the delete.
|
||||
"""
|
||||
from hermes_cli.config import load_config
|
||||
from hermes_cli.config import custom_endpoint_key_env, get_env_value, load_config
|
||||
|
||||
self.client.post(
|
||||
"/api/providers/custom-endpoints",
|
||||
|
|
@ -4688,8 +4688,10 @@ class TestWebServerEndpoints:
|
|||
"/api/providers/custom-endpoints/acme/activate", json={}
|
||||
).status_code == 200
|
||||
|
||||
env_var = custom_endpoint_key_env("acme")
|
||||
cfg = load_config()
|
||||
assert cfg["model"]["api_key"] == "sk-acme-secret"
|
||||
assert cfg["model"]["key_env"] == env_var
|
||||
assert get_env_value(env_var) == "sk-acme-secret"
|
||||
|
||||
assert self.client.request(
|
||||
"DELETE", "/api/providers/custom-endpoints/acme"
|
||||
|
|
@ -4699,12 +4701,14 @@ class TestWebServerEndpoints:
|
|||
assert "acme" not in (cfg.get("providers") or {})
|
||||
model_cfg = cfg.get("model") or {}
|
||||
assert not model_cfg.get("api_key"), "deleted endpoint's key still in config.yaml"
|
||||
assert not model_cfg.get("key_env"), "deleted endpoint's key ref still in config.yaml"
|
||||
assert not model_cfg.get("base_url"), "deleted endpoint's host still routed to"
|
||||
assert not model_cfg.get("provider")
|
||||
assert not get_env_value(env_var), "deleted endpoint's key still in .env"
|
||||
|
||||
def test_deleting_an_inactive_custom_endpoint_leaves_the_active_one_alone(self):
|
||||
"""Only the mirror of the DELETED provider is scrubbed."""
|
||||
from hermes_cli.config import load_config
|
||||
"""Only the DELETED provider's mirror and .env slot are scrubbed."""
|
||||
from hermes_cli.config import custom_endpoint_key_env, get_env_value, load_config
|
||||
|
||||
for name, key in (("acme", "sk-acme"), ("other", "sk-other")):
|
||||
self.client.post(
|
||||
|
|
@ -4723,8 +4727,283 @@ class TestWebServerEndpoints:
|
|||
|
||||
model_cfg = load_config().get("model") or {}
|
||||
assert model_cfg.get("provider") == "other"
|
||||
assert model_cfg.get("api_key") == "sk-other"
|
||||
assert model_cfg.get("key_env") == custom_endpoint_key_env("other")
|
||||
assert model_cfg.get("base_url") == "https://llm.other.corp/v1"
|
||||
assert get_env_value(custom_endpoint_key_env("other")) == "sk-other"
|
||||
|
||||
def test_custom_endpoint_save_persists_the_whole_discovered_catalogue(self):
|
||||
"""Test discovers N models; Save must keep all N (#69988).
|
||||
|
||||
Every downstream picker reads ``providers.<id>.models`` straight from
|
||||
config.yaml with no live probe, so persisting only the one hand-typed
|
||||
model left a provider serving dozens showing a single-entry list.
|
||||
"""
|
||||
from hermes_cli.config import load_config
|
||||
|
||||
discovered = ["glm-5.2", "qwen3-max", "llama-4-405b", "deepseek-v4"]
|
||||
resp = self.client.post(
|
||||
"/api/providers/custom-endpoints",
|
||||
json={
|
||||
"id": "proxy",
|
||||
"name": "Proxy",
|
||||
"base_url": "http://127.0.0.1:8000/v1",
|
||||
"model": "glm-5.2",
|
||||
"models": discovered,
|
||||
},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert sorted(load_config()["providers"]["proxy"]["models"]) == sorted(discovered)
|
||||
endpoint = next(e for e in resp.json()["endpoints"] if e["id"] == "proxy")
|
||||
assert sorted(endpoint["models"]) == sorted(discovered)
|
||||
|
||||
def test_custom_endpoint_save_with_catalogue_keeps_known_context_lengths(self):
|
||||
"""A discovered list merges onto the entry; it doesn't reset it."""
|
||||
from hermes_cli.config import load_config, save_config
|
||||
|
||||
cfg = load_config()
|
||||
cfg["providers"] = {
|
||||
"proxy": {
|
||||
"name": "Proxy",
|
||||
"base_url": "http://127.0.0.1:8000/v1",
|
||||
"model": "a",
|
||||
"models": {"a": {"context_length": 200000}},
|
||||
}
|
||||
}
|
||||
save_config(cfg)
|
||||
|
||||
self.client.post(
|
||||
"/api/providers/custom-endpoints",
|
||||
json={
|
||||
"id": "proxy",
|
||||
"name": "Proxy",
|
||||
"base_url": "http://127.0.0.1:8000/v1",
|
||||
"model": "a",
|
||||
"models": ["a", "b"],
|
||||
},
|
||||
)
|
||||
|
||||
models = load_config()["providers"]["proxy"]["models"]
|
||||
assert sorted(models) == ["a", "b"]
|
||||
assert models["a"]["context_length"] == 200000
|
||||
|
||||
def test_custom_endpoint_save_keeps_the_api_key_out_of_config(self):
|
||||
"""The key belongs in .env behind key_env, never in config.yaml (#69449)."""
|
||||
from hermes_cli.config import custom_endpoint_key_env, get_env_value, load_config
|
||||
|
||||
self.client.post(
|
||||
"/api/providers/custom-endpoints",
|
||||
json={
|
||||
"id": "proxy",
|
||||
"name": "Proxy",
|
||||
"base_url": "https://llm.example.com/v1",
|
||||
"model": "m",
|
||||
"api_key": "sk-super-secret",
|
||||
"make_default": True,
|
||||
},
|
||||
)
|
||||
|
||||
cfg = load_config()
|
||||
entry = cfg["providers"]["proxy"]
|
||||
env_var = custom_endpoint_key_env("proxy")
|
||||
assert entry["key_env"] == env_var
|
||||
assert "api_key" not in entry
|
||||
assert "api_key" not in cfg["model"]
|
||||
assert get_env_value(env_var) == "sk-super-secret"
|
||||
assert "sk-super-secret" not in yaml.safe_dump(cfg)
|
||||
|
||||
def test_custom_endpoint_edit_without_a_key_keeps_the_stored_one(self):
|
||||
"""The panel sends no api_key on an unrelated edit (the field is blank)."""
|
||||
from hermes_cli.config import custom_endpoint_key_env, get_env_value, load_config
|
||||
|
||||
common = {
|
||||
"id": "proxy",
|
||||
"name": "Proxy",
|
||||
"base_url": "https://llm.example.com/v1",
|
||||
}
|
||||
self.client.post(
|
||||
"/api/providers/custom-endpoints",
|
||||
json={**common, "model": "m1", "api_key": "sk-keep-me"},
|
||||
)
|
||||
self.client.post("/api/providers/custom-endpoints", json={**common, "model": "m2"})
|
||||
|
||||
entry = load_config()["providers"]["proxy"]
|
||||
assert entry["model"] == "m2"
|
||||
assert entry["key_env"] == custom_endpoint_key_env("proxy")
|
||||
assert get_env_value(custom_endpoint_key_env("proxy")) == "sk-keep-me"
|
||||
|
||||
def test_custom_endpoint_save_migrates_a_legacy_plaintext_key(self):
|
||||
"""Entries written before #69449 get cleaned up on their next save.
|
||||
|
||||
Requiring the user to re-type the key to get it out of config.yaml
|
||||
would leave the plaintext sitting there for anyone who never edits the
|
||||
endpoint again.
|
||||
"""
|
||||
from hermes_cli.config import custom_endpoint_key_env, get_env_value, load_config, save_config
|
||||
|
||||
cfg = load_config()
|
||||
cfg["providers"] = {
|
||||
"proxy": {
|
||||
"name": "Proxy",
|
||||
"base_url": "https://llm.example.com/v1",
|
||||
"model": "m",
|
||||
"api_key": "sk-legacy-plaintext",
|
||||
"models": {"m": {}},
|
||||
}
|
||||
}
|
||||
save_config(cfg)
|
||||
|
||||
self.client.post(
|
||||
"/api/providers/custom-endpoints",
|
||||
json={
|
||||
"id": "proxy",
|
||||
"name": "Proxy",
|
||||
"base_url": "https://llm.example.com/v1",
|
||||
"model": "m",
|
||||
},
|
||||
)
|
||||
|
||||
entry = load_config()["providers"]["proxy"]
|
||||
assert "api_key" not in entry
|
||||
assert entry["key_env"] == custom_endpoint_key_env("proxy")
|
||||
assert get_env_value(custom_endpoint_key_env("proxy")) == "sk-legacy-plaintext"
|
||||
|
||||
def test_custom_endpoint_save_leaves_a_hand_written_env_ref_alone(self, monkeypatch):
|
||||
"""``api_key: ${MY_KEY}`` is already safe — don't copy it elsewhere.
|
||||
|
||||
load_config() expands env refs, so such an entry looks like a literal
|
||||
secret by the time Save sees it. Migrating it would duplicate the
|
||||
user's secret into a second env var they never asked for.
|
||||
"""
|
||||
import yaml
|
||||
|
||||
from hermes_cli.config import custom_endpoint_key_env, get_config_path, get_env_value
|
||||
|
||||
monkeypatch.setenv("MY_PROXY_KEY", "sk-user-managed")
|
||||
get_config_path().write_text(
|
||||
yaml.safe_dump({
|
||||
"providers": {
|
||||
"proxy": {
|
||||
"name": "Proxy",
|
||||
"base_url": "https://llm.example.com/v1",
|
||||
"model": "m",
|
||||
"api_key": "${MY_PROXY_KEY}",
|
||||
}
|
||||
},
|
||||
}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
self.client.post(
|
||||
"/api/providers/custom-endpoints",
|
||||
json={
|
||||
"id": "proxy",
|
||||
"name": "Proxy",
|
||||
"base_url": "https://llm.example.com/v1",
|
||||
"model": "m",
|
||||
},
|
||||
)
|
||||
|
||||
raw = yaml.safe_load(get_config_path().read_text(encoding="utf-8"))
|
||||
assert raw["providers"]["proxy"]["api_key"] == "${MY_PROXY_KEY}"
|
||||
assert not get_env_value(custom_endpoint_key_env("proxy"))
|
||||
|
||||
def test_custom_endpoint_blank_api_key_clears_the_credential(self):
|
||||
"""An explicitly emptied field means "remove the key", not "keep it"."""
|
||||
from hermes_cli.config import custom_endpoint_key_env, get_env_value, load_config
|
||||
|
||||
common = {
|
||||
"id": "proxy",
|
||||
"name": "Proxy",
|
||||
"base_url": "https://llm.example.com/v1",
|
||||
"model": "m",
|
||||
}
|
||||
self.client.post(
|
||||
"/api/providers/custom-endpoints", json={**common, "api_key": "sk-drop-me"}
|
||||
)
|
||||
self.client.post("/api/providers/custom-endpoints", json={**common, "api_key": ""})
|
||||
|
||||
entry = load_config()["providers"]["proxy"]
|
||||
assert "api_key" not in entry
|
||||
assert "key_env" not in entry
|
||||
assert not get_env_value(custom_endpoint_key_env("proxy"))
|
||||
|
||||
def test_two_endpoints_on_one_host_keep_separate_credentials(self):
|
||||
"""Two local servers must not share an .env slot.
|
||||
|
||||
Deriving the env var from the hostname collapses ``127.0.0.1:8000``
|
||||
and ``:8001`` onto one name, so saving the second silently overwrites
|
||||
the first's key.
|
||||
"""
|
||||
from hermes_cli.config import custom_endpoint_key_env, get_env_value
|
||||
|
||||
for port, key in ((8000, "sk-first"), (8001, "sk-second")):
|
||||
self.client.post(
|
||||
"/api/providers/custom-endpoints",
|
||||
json={
|
||||
"id": f"local-{port}",
|
||||
"name": f"Local {port}",
|
||||
"base_url": f"http://127.0.0.1:{port}/v1",
|
||||
"model": "m",
|
||||
"api_key": key,
|
||||
},
|
||||
)
|
||||
|
||||
assert get_env_value(custom_endpoint_key_env("local-8000")) == "sk-first"
|
||||
assert get_env_value(custom_endpoint_key_env("local-8001")) == "sk-second"
|
||||
|
||||
def test_custom_endpoint_response_reports_a_key_held_in_env(self):
|
||||
"""has_api_key must follow key_env, not just a plaintext api_key.
|
||||
|
||||
Reading only ``api_key`` made the panel report "no API key" for every
|
||||
endpoint whose credential had been moved to .env.
|
||||
"""
|
||||
resp = self.client.post(
|
||||
"/api/providers/custom-endpoints",
|
||||
json={
|
||||
"id": "proxy",
|
||||
"name": "Proxy",
|
||||
"base_url": "https://llm.example.com/v1",
|
||||
"model": "m",
|
||||
"api_key": "sk-in-env",
|
||||
},
|
||||
)
|
||||
|
||||
endpoint = next(e for e in resp.json()["endpoints"] if e["id"] == "proxy")
|
||||
assert endpoint["has_api_key"] is True
|
||||
assert "sk-in-env" not in (endpoint["api_key_preview"] or "")
|
||||
|
||||
def test_activating_an_endpoint_carries_its_credential_either_way(self):
|
||||
"""Activate must work for both key_env and pre-#69449 plaintext entries."""
|
||||
from hermes_cli.config import load_config, save_config
|
||||
|
||||
cfg = load_config()
|
||||
cfg["providers"] = {
|
||||
"legacy": {
|
||||
"name": "Legacy",
|
||||
"base_url": "https://llm.legacy.com/v1",
|
||||
"model": "m",
|
||||
"api_key": "sk-legacy",
|
||||
"models": {"m": {}},
|
||||
},
|
||||
"modern": {
|
||||
"name": "Modern",
|
||||
"base_url": "https://llm.modern.com/v1",
|
||||
"model": "m",
|
||||
"key_env": "MODERN_API_KEY",
|
||||
"models": {"m": {}},
|
||||
},
|
||||
}
|
||||
save_config(cfg)
|
||||
|
||||
self.client.post("/api/providers/custom-endpoints/modern/activate", json={})
|
||||
model_cfg = load_config()["model"]
|
||||
assert model_cfg["key_env"] == "MODERN_API_KEY"
|
||||
assert "api_key" not in model_cfg
|
||||
|
||||
self.client.post("/api/providers/custom-endpoints/legacy/activate", json={})
|
||||
model_cfg = load_config()["model"]
|
||||
assert model_cfg["api_key"] == "sk-legacy"
|
||||
|
||||
def test_set_model_main_preserves_base_url_for_named_custom_provider(self):
|
||||
"""Selecting a named custom endpoint from the Desktop model picker
|
||||
|
|
|
|||
Loading…
Reference in New Issue