Files
6krrt/tests/test_admin_bodies_are_strict.py
adlee-was-taken 6b9826c27e fix(admin): unknown request keys are errors, and base providers are read-only
Sweep findings #2 and #7 (plans/admin-portal-functional-sweep.md).

#2. Every admin request body now inherits `_AdminBody` with
`extra="forbid"`. This is the rule StrictModel already enforces for the
config models, for the reason CLAUDE.md gives: with Pydantic's default a
typo "loads cleanly, does nothing, and still looks configured".

On this surface the default was destructive, not merely untidy.
`_CloudFallbackBody` takes a nested `{"cloud_fallback": {...}}`, so a caller
sending the flat shape the field names suggest had every key discarded,
leaving the field None -- which is the documented signal to REMOVE the
block. Trying to save a cloud classifier deleted it and returned 200 "A
restart is required for this change to take effect". The quiet version:
`POST /api/profiles/` with `{"name":"x","min_teir":3}` returned 200 and
created a profile with no filters at all.

Note the asymmetry this removes. The inner dict was already strict, because
RouterConfig validates it on the way to disk -- `{"timeout_secondz": 20}`
correctly 422'd. Only the outer wrapper was loose, so validation got
stricter the deeper you went.

Strictness immediately found a real client/server mismatch: the profiles UI
sent `name` on the update path, which `_ProfileUpdateBody` has no field for
(the name is in the URL; the endpoint cannot rename). It was being silently
dropped. Both save paths now strip it. Verified in the browser -- inline
edit and duplicate-then-create both still save, with allowed_model_ids
preserved.

#7. `POST /api/providers/{name}` had no provenance check at all, making it
the one way to write to something the portal itself labels "base config /
read-only". The result could not be undone from the portal: the write forks
the provider into config.local.yaml, and DELETE then refuses if it is the
default_provider, so hand-editing the one file in this deployment that is
not in git was the only way back. It now refuses a base-only definition
exactly as delete does.

One POST serves both create and update, so this also means an overlay entry
cannot shadow a base provider -- matching admin_profile_create, which
refuses that with a 409.

Which exposed a smaller thing worth fixing: both profile 403s advised
"add an overlay profile with the same name to override it", and
admin_profile_create rejects precisely that. The message now says what the
API actually supports.

The new provider test also demonstrates the fixture shape that fixes sweep
finding #12 -- pointing `base_dir` at tmp_path isolates a test from the
machine's real config.local.yaml.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRQXz5SYZYVWscxS1QqF6U
2026-09-08 18:30:06 -04:00

138 lines
5.4 KiB
Python

"""An unknown key in an admin request body is an error, not a silent no-op.
CLAUDE.md already records why the config models forbid extras: with
Pydantic's default a typo "loads cleanly, does nothing, and still looks
configured". The request surface had the same hole, and on one endpoint it
was destructive rather than merely quiet.
"""
from __future__ import annotations
import sqlite3
from pathlib import Path
import pytest
from starlette.testclient import TestClient
import admin
import dispatcher
ROOT = Path(__file__).resolve().parent.parent
SCHEMA_SQL = (ROOT / "config" / "schema.sql").read_text()
@pytest.fixture
def client(tmp_path, monkeypatch):
db_path = tmp_path / "strict.db"
conn = sqlite3.connect(db_path)
conn.executescript(SCHEMA_SQL)
admin.ensure_admin_tables(conn)
conn.execute(
"""
INSERT INTO models (
model_id, provider, base_model_id, tier, context_window,
effective_context_window, max_output_tokens,
cost_per_1m_prompt, cost_per_1m_completion,
supports_vision, supports_json_mode,
latency_class, reasoning_mode, context_variant,
access_level, availability, last_updated
) VALUES ('m1', 'neuralwatt', 'm1', 2, 100000, 90000, 4096, 1.0, 2.0,
0, 1, 'standard', 'default', 'full', 'public', 'active',
'2026-08-22T00:00:00+00:00')
"""
)
conn.commit()
conn.close()
monkeypatch.setattr(dispatcher.cfg.database, "path", str(db_path))
return TestClient(dispatcher.app)
# --- the destructive case that motivated this ------------------------------
def test_the_flat_cloud_fallback_shape_is_refused_not_treated_as_a_delete():
"""The bug: `_CloudFallbackBody` nests under `cloud_fallback`, so a flat
body had every key discarded, leaving the field None -- which is the
documented "remove the block" signal. A caller trying to SAVE a cloud
classifier deleted it and was told the save succeeded.
"""
with pytest.raises(Exception) as exc:
admin._CloudFallbackBody(
base_url="https://api.example.com/v1",
model="some-model",
api_key_env="SOME_KEY",
)
assert "extra" in str(exc.value).lower()
def test_the_correct_nested_cloud_fallback_shape_still_validates():
body = admin._CloudFallbackBody(
cloud_fallback={"base_url": "https://api.example.com/v1",
"model": "some-model"}
)
assert body.cloud_fallback["model"] == "some-model"
# --- the quiet cases -------------------------------------------------------
def test_a_typo_in_a_profile_field_is_refused(client):
"""`min_teir` used to return 200 and create a profile with no filters."""
resp = client.post("/admin/api/profiles/",
json={"name": "typoprof", "min_teir": 3})
assert resp.status_code == 422, resp.text
assert "extra" in resp.text.lower()
names = [p["name"] for p in client.get("/admin/api/profiles").json()]
assert "typoprof" not in names
@pytest.mark.parametrize(
"model_cls, kwargs",
[
(admin._AvailabilityBody, {"availability": "active", "resaon": "x"}),
(admin._ValueBody, {"value": True, "vaule": 1}),
(admin._ClassifierConfigBody, {"mode": "local_llm", "modee": "x"}),
(admin._ProfileCreateBody, {"name": "p", "provdier": "x"}),
(admin._ProfileUpdateBody, {"min_tier": 1, "max_teir": 3}),
(admin._ProviderUpdateBody, {"base_url": "https://x/v1",
"api_key_env": "K",
"has_energy_telemetry": False,
"enabled": True,
"requires_allowlist": True}),
(admin._AllowlistAddBody, {"model_id": "m", "notes": "x"}),
],
)
def test_every_admin_body_forbids_unknown_keys(model_cls, kwargs):
"""One base class covers the whole surface; this asserts nobody escaped.
The ``_ProviderUpdateBody`` case is a real near-miss rather than a
contrived one: the field is ``require_allowlist``, and ``requires_``
reads so naturally that a caller would never notice it had been dropped.
"""
with pytest.raises(Exception) as exc:
model_cls(**kwargs)
assert "extra" in str(exc.value).lower(), model_cls.__name__
def test_known_keys_are_unaffected(client):
"""Strictness must not cost the happy path.
Deliberately exercised through a DB-backed endpoint rather than profile
CRUD: profile writes land in ``config/config.local.yaml``, which this
fixture does not isolate, so asserting on them here would couple the test
to whatever the machine's overlay already contains. That coupling is
finding #12 in plans/admin-portal-functional-sweep.md.
"""
resp = client.post("/admin/api/models/m1/neuralwatt/availability",
json={"availability": "deprecated", "reason": "test"})
assert resp.status_code == 200, resp.text
assert resp.json()["effective_availability"] == "deprecated"
def test_known_keys_are_unaffected_on_the_models_that_write_files():
"""The same, for the file-writing bodies, without touching a file."""
assert admin._ProfileCreateBody(name="p", min_tier=2).min_tier == 2
assert admin._ProviderUpdateBody(
base_url="https://x/v1", api_key_env="K",
has_energy_telemetry=False, enabled=True,
).enabled is True
assert admin._AllowlistAddBody(model_id="m", note="n").note == "n"