diff --git a/tests/test_admin_model_registry_refresh.py b/tests/test_admin_model_registry_refresh.py index d0065e06..5a38e1d5 100644 --- a/tests/test_admin_model_registry_refresh.py +++ b/tests/test_admin_model_registry_refresh.py @@ -352,6 +352,90 @@ def test_update_endpoint_skips_refresh_on_empty_body( assert calls == [] # gate held: empty body did not trigger a refresh +def test_create_rejects_invalid_api_surface(storage: SQLiteBackend) -> None: + """POST with a bogus server_compat.api_surface returns 400 rather than + persisting a value that would make get_provider() raise on every later + ChatSession init for the alias.""" + _seed_model_def(storage, definition_id="m1", alias="local", model="m") + registry = _make_registry(alias="local", model="m") + client = _make_client(storage, registry) + + resp = client.post( + "/v1/api/admin/model-definitions", + json={ + "alias": "bad", + "model": "x", + "provider": "openai-compatible", + "base_url": "http://localhost:9000/v1", + "api_key": "sk-x", + "capabilities": {"server_compat": {"api_surface": "BOGUS"}}, + }, + ) + assert resp.status_code == 400, resp.text + assert "api_surface" in resp.json()["error"] + # And the alias is not persisted + assert not registry.has_alias("bad") + + +def test_create_rejects_non_canonical_api_surface(storage: SQLiteBackend) -> None: + """Strict validation: ' Responses ' / 'CHAT' don't round-trip through the + admin `` whose options are the canonical + ``"chat"`` / ``"responses"`` strings, so anything else fails to round-trip + through edit/save. The provider factory raises ``ValueError`` at request + time for an unknown surface; validating here turns that into a 400 at + write time so an admin can't poison a model alias via direct API calls. + """ + if not isinstance(caps, dict): + return None + sc = caps.get("server_compat") + if not isinstance(sc, dict): + return None + raw = sc.get("api_surface") + if raw is None or raw == "": + return None + if not isinstance(raw, str) or raw not in _API_SURFACE_CHOICES: + return f"Invalid server_compat.api_surface: {raw!r}" + return None + + # Keep in sync with turnstone.core.providers._google.GOOGLE_DEFAULT_BASE_URL _PROVIDER_DEFAULT_URLS: dict[str, str] = { "openai": "https://api.openai.com/v1", @@ -8391,6 +8418,9 @@ async def admin_create_model_definition(request: Request) -> JSONResponse: ctx_raw = body.get("context_window", 32768) context_window = max(0, int(ctx_raw)) if isinstance(ctx_raw, (int, float)) else 0 caps = body.get("capabilities", {}) + err_msg = _validate_api_surface(caps) + if err_msg: + return JSONResponse({"error": err_msg}, status_code=400) capabilities = json.dumps(caps) if isinstance(caps, dict) else "{}" enabled = bool(body.get("enabled", True)) @@ -8545,6 +8575,9 @@ async def admin_update_model_definition(request: Request) -> JSONResponse: updates["context_window"] = max(0, int(ctx_raw)) if isinstance(ctx_raw, (int, float)) else 0 if "capabilities" in body: caps = body["capabilities"] + err_msg = _validate_api_surface(caps) + if err_msg: + return JSONResponse({"error": err_msg}, status_code=400) updates["capabilities"] = json.dumps(caps) if isinstance(caps, dict) else "{}" if "enabled" in body: updates["enabled"] = bool(body["enabled"]) diff --git a/turnstone/console/static/admin.js b/turnstone/console/static/admin.js index 64bb8ead..9b02c136 100644 --- a/turnstone/console/static/admin.js +++ b/turnstone/console/static/admin.js @@ -5001,6 +5001,7 @@ function showCreateModelModal() { document.getElementById("model-max-tokens").value = ""; document.getElementById("model-reasoning-effort").value = ""; document.getElementById("model-server-type").value = ""; + document.getElementById("model-api-surface").value = ""; document.getElementById("model-thinking-mode").value = ""; document.getElementById("model-thinking-param").value = ""; document.getElementById("model-thinking-param-row").style.display = "none"; @@ -5077,8 +5078,9 @@ function showEditModelModal(definitionId) { document.getElementById("model-thinking-param").value = ""; } _toggleThinkingParam(); - // Server compat: server_type and extra_body workarounds + // Server compat: server_type, api_surface, and extra_body workarounds document.getElementById("model-server-type").value = sc.server_type || ""; + document.getElementById("model-api-surface").value = sc.api_surface || ""; var eb = sc.extra_body || {}; var ebText = JSON.stringify(eb, null, 2); document.getElementById("model-extra-body").value = @@ -5160,26 +5162,34 @@ function submitCreateModel() { if (savedParam) caps.thinking_param = savedParam; } - // Build server_compat from structured fields + // Build server_compat from structured fields. Only meaningful for + // openai-compatible aliases — for other providers the section is hidden + // but the form values can linger after a provider switch, so gate the + // whole block on the active provider to keep persisted state honest. var serverCompat = {}; - var serverType = document.getElementById("model-server-type").value; - if (serverType) serverCompat.server_type = serverType; + var providerVal = document.getElementById("model-provider").value; var ebEl = document.getElementById("model-extra-body"); - var ebText = ebEl.value.trim(); ebEl.removeAttribute("aria-invalid"); ebEl.style.borderColor = ""; - if (ebText) { - try { - var ebParsed = JSON.parse(ebText); - if (!_isPlainObject(ebParsed)) { - throw new Error("not an object"); + if (providerVal === "openai-compatible") { + var serverType = document.getElementById("model-server-type").value; + if (serverType) serverCompat.server_type = serverType; + var apiSurface = document.getElementById("model-api-surface").value; + if (apiSurface) serverCompat.api_surface = apiSurface; + var ebText = ebEl.value.trim(); + if (ebText) { + try { + var ebParsed = JSON.parse(ebText); + if (!_isPlainObject(ebParsed)) { + throw new Error("not an object"); + } + serverCompat.extra_body = ebParsed; + } catch (e) { + ebEl.setAttribute("aria-invalid", "true"); + ebEl.style.borderColor = "var(--red)"; + _showModelError("Extra body params must be a JSON object"); + return; } - serverCompat.extra_body = ebParsed; - } catch (e) { - ebEl.setAttribute("aria-invalid", "true"); - ebEl.style.borderColor = "var(--red)"; - _showModelError("Extra body params must be a JSON object"); - return; } } if (Object.keys(serverCompat).length > 0) { @@ -5410,6 +5420,13 @@ function detectModel() { stOpts2.indexOf(ssc.server_type) !== -1 ) stEl2.value = ssc.server_type; + // Restrict to the known set so a hostile detect response can't + // smuggle a non-listed value into the form. + var _SURFACE_SUGGESTABLE = { chat: 1, responses: 1 }; + if (ssc.api_surface && _SURFACE_SUGGESTABLE[ssc.api_surface]) { + var asEl = document.getElementById("model-api-surface"); + if (!asEl.value) asEl.value = ssc.api_surface; + } if (ssc.extra_body) { var ebEl2 = document.getElementById("model-extra-body"); if (!ebEl2.value.trim()) { diff --git a/turnstone/console/static/index.html b/turnstone/console/static/index.html index dd29d0ee..74090fea 100644 --- a/turnstone/console/static/index.html +++ b/turnstone/console/static/index.html @@ -3920,6 +3920,17 @@ + +