From 5911b8c0dc4302cb6d60f88b6beecc195fe89751 Mon Sep 17 00:00:00 2001 From: Ocean Bennett <204957658+undergroundrap@users.noreply.github.com> Date: Fri, 5 Jun 2026 15:12:14 -0400 Subject: [PATCH] fix(models): allow same endpoint URL with different keys (#2758) * fix(models): allow same endpoint URL with different keys * fix(models): show endpoint key fingerprints --- routes/model_routes.py | 41 ++++++++++++++++--- static/js/admin.js | 5 ++- tests/test_model_routes.py | 81 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 7 deletions(-) diff --git a/routes/model_routes.py b/routes/model_routes.py index 6220305d3..14d1b94e6 100644 --- a/routes/model_routes.py +++ b/routes/model_routes.py @@ -5,6 +5,7 @@ import re import uuid import json import socket +import hashlib import time as _time import logging import httpx @@ -850,6 +851,14 @@ def _visible_models(cached_models, hidden_models, pinned_models=None): return [m for m in merged if m not in hidden] +def _api_key_fingerprint(api_key: Optional[str]) -> str: + """Stable, non-secret label for distinguishing same-URL credentials.""" + key = (api_key or "").strip() + if not key: + return "" + return hashlib.sha256(key.encode("utf-8")).hexdigest()[:8] + + def setup_model_routes(model_discovery): router = APIRouter(prefix="/api") @@ -1397,6 +1406,7 @@ def setup_model_routes(model_discovery): "name": r.name, "base_url": r.base_url, "has_key": bool(r.api_key), + "api_key_fingerprint": _api_key_fingerprint(r.api_key), "is_enabled": r.is_enabled, "models": visible, "pinned_models": pinned, @@ -1463,21 +1473,34 @@ def setup_model_routes(model_discovery): ) explicit_timeout = _explicit_model_list_timeout(base_url, requested_kind, refresh_timeout) - # Dedupe: if an endpoint with the same base_url already exists and - # is reachable by the caller (shared or owned by them), return it - # instead of creating a duplicate row. Fixes "Scan for Servers" - # re-adding manually-added endpoints under their host:port name. + # Dedupe: if an endpoint with the same base_url and compatible + # credentials already exists and is reachable by the caller (shared or + # owned by them), return it instead of creating a duplicate row. Keep + # same-url/different-key rows distinct so users can group the same + # provider URL under multiple credentials. from src.auth_helpers import get_current_user as _gcu_dedup _caller = _gcu_dedup(request) or None + _incoming_api_key = api_key.strip() _db_dedup = SessionLocal() try: - existing = ( + _same_url_rows = ( _db_dedup.query(ModelEndpoint) .filter(ModelEndpoint.base_url == base_url) .filter((ModelEndpoint.owner.is_(None)) | (ModelEndpoint.owner == _caller)) .order_by(ModelEndpoint.owner.desc()) # prefer owned over shared - .first() + .all() ) + existing = None + _empty_key_existing = None + for _candidate in _same_url_rows: + _candidate_key = (getattr(_candidate, "api_key", None) or "").strip() + if _candidate_key == _incoming_api_key: + existing = _candidate + break + if _incoming_api_key and not _candidate_key and _empty_key_existing is None: + _empty_key_existing = _candidate + if existing is None and _incoming_api_key and _empty_key_existing is not None: + existing = _empty_key_existing if existing: changed = False # Persist any incoming pinned IDs onto the existing row. An @@ -1526,6 +1549,8 @@ def setup_model_routes(model_discovery): "id": existing.id, "name": existing.name, "base_url": existing.base_url, + "has_key": bool(existing.api_key), + "api_key_fingerprint": _api_key_fingerprint(existing.api_key), "models": _visible_models( existing_models, getattr(existing, "hidden_models", None), @@ -1599,6 +1624,8 @@ def setup_model_routes(model_discovery): "id": ep_id, "name": name.strip(), "base_url": base_url, + "has_key": bool(api_key.strip()), + "api_key_fingerprint": _api_key_fingerprint(api_key), "models": _merge_model_ids(model_ids, _pinned), "pinned_models": _pinned, "online": bool(model_ids) or bool(_pinned) or bool(ping.get("reachable")), @@ -1948,6 +1975,8 @@ def setup_model_routes(model_discovery): "name": ep.name, "model_type": ep.model_type, "base_url": ep.base_url, + "has_key": bool(ep.api_key), + "api_key_fingerprint": _api_key_fingerprint(ep.api_key), "pinned_models": _normalize_model_ids(getattr(ep, "pinned_models", None)), "endpoint_kind": getattr(ep, "endpoint_kind", None) or "auto", "model_refresh_mode": getattr(ep, "model_refresh_mode", None) or "auto", diff --git a/static/js/admin.js b/static/js/admin.js index 5211bf62d..5d3d4a356 100644 --- a/static/js/admin.js +++ b/static/js/admin.js @@ -417,6 +417,9 @@ async function loadEndpoints() { const justAddedClass = (_recentlyAddedEpId && String(ep.id) === _recentlyAddedEpId) ? ' adm-ep-just-added' : ''; const category = ep.category || (_isLocalEndpoint(ep.base_url) ? 'local' : 'api'); const kindLabel = ep.endpoint_kind && ep.endpoint_kind !== 'auto' ? ep.endpoint_kind.toUpperCase() : ''; + const keyLabel = ep.has_key + ? (ep.api_key_fingerprint ? ` (key ${esc(ep.api_key_fingerprint)})` : ' (key set)') + : ''; return `
@@ -434,7 +437,7 @@ async function loadEndpoints() { ${hasModels ? '' : ''}
-
${esc(ep.base_url)}${category === 'local' ? `` : ''}${ep.has_key ? ' (key set)' : ''}
+
${esc(ep.base_url)}${category === 'local' ? `` : ''}${keyLabel}
${hasModels ? `` : ''} `; }); diff --git a/tests/test_model_routes.py b/tests/test_model_routes.py index 4d68546c6..f3475c30a 100644 --- a/tests/test_model_routes.py +++ b/tests/test_model_routes.py @@ -37,6 +37,7 @@ from routes.model_routes import ( _curate_models, _visible_models, _normalize_model_ids, + _api_key_fingerprint, _is_chat_model, _classify_endpoint, _effective_endpoint_kind, @@ -754,6 +755,16 @@ def test_visible_models_handles_malformed_strings(): assert _visible_models("only-cached", None, None) == ["only-cached"] +def test_api_key_fingerprint_is_stable_and_non_secret(): + fp_one = _api_key_fingerprint("key-one") + + assert _api_key_fingerprint("") == "" + assert fp_one == _api_key_fingerprint(" key-one ") + assert fp_one != _api_key_fingerprint("key-two") + assert len(fp_one) == 8 + assert "key-one" not in fp_one + + def _create_form_kwargs(**overrides): """Defaults for every Form() param create_model_endpoint reads directly. @@ -791,6 +802,29 @@ def _patch_create_deps(monkeypatch, db): monkeypatch.setattr(auth_helpers, "get_current_user", lambda req: None) +def test_list_model_endpoints_returns_key_fingerprint(monkeypatch): + endpoint_with_key = _make_endpoint( + api_key="key-one", + cached_models=json.dumps(["m1"]), + ) + endpoint_without_key = _make_endpoint( + id="ep2", + api_key=None, + cached_models=json.dumps(["m2"]), + ) + db = _PinnedFakeDb([endpoint_with_key, endpoint_without_key]) + monkeypatch.setattr(model_routes, "SessionLocal", lambda: db) + monkeypatch.setattr(model_routes, "require_admin", lambda request: None) + endpoint = _get_route("/api/model-endpoints", "GET") + + result = endpoint(_PinnedFakeRequest()) + + assert result[0]["has_key"] is True + assert result[0]["api_key_fingerprint"] == _api_key_fingerprint("key-one") + assert result[1]["has_key"] is False + assert result[1]["api_key_fingerprint"] == "" + + def test_post_creates_endpoint_with_pinned_models(monkeypatch): db = _PinnedFakeDb([]) # no existing row → fresh create path _patch_create_deps(monkeypatch, db) @@ -856,6 +890,53 @@ def test_post_dedupe_existing_does_not_clobber_pinned_when_omitted(monkeypatch): assert json.loads(existing.pinned_models) == ["keep-me"] assert result["pinned_models"] == ["keep-me"] assert db.committed == 0 # nothing to persist + + +def test_post_same_base_url_different_api_key_creates_distinct_endpoint(monkeypatch): + existing = _make_endpoint( + base_url="https://api.example.test/v1", + api_key="key-one", + ) + db = _PinnedFakeDb([existing]) + _patch_create_deps(monkeypatch, db) + create = _get_route("/api/model-endpoints", "POST") + + result = create( + _PinnedFakeRequest(), + base_url="https://api.example.test/v1", + **_create_form_kwargs(api_key="key-two"), + ) + + assert result.get("existing") is not True + assert result["has_key"] is True + assert result["api_key_fingerprint"] == _api_key_fingerprint("key-two") + assert len(db.added) == 1 + assert db.added[0].base_url == "https://api.example.test/v1" + assert db.added[0].api_key == "key-two" + + +def test_post_same_base_url_same_api_key_still_dedupes(monkeypatch): + existing = _make_endpoint( + base_url="https://api.example.test/v1", + api_key="key-one", + ) + db = _PinnedFakeDb([existing]) + _patch_create_deps(monkeypatch, db) + create = _get_route("/api/model-endpoints", "POST") + + result = create( + _PinnedFakeRequest(), + base_url="https://api.example.test/v1", + **_create_form_kwargs(api_key="key-one"), + ) + + assert result["existing"] is True + assert result["id"] == existing.id + assert result["has_key"] is True + assert result["api_key_fingerprint"] == _api_key_fingerprint("key-one") + assert db.added == [] + + class _RouteQuery: def __init__(self, rows): self.rows = list(rows)