diff --git a/backend/app/auth.py b/backend/app/auth.py index 00e0f0b..be3090f 100644 --- a/backend/app/auth.py +++ b/backend/app/auth.py @@ -89,14 +89,20 @@ class ProviderConfig: using_demo: bool def __post_init__(self) -> None: - # Belt and braces around the shared demo key. resolve_provider_config() - # already pins the model, but this makes it a property of the config - # object itself: however it was built, and by whichever caller, the - # server-funded key can never be paired with an off-whitelist (i.e. - # possibly paid) model. Unreachable by design — a 500 here means a new - # code path tried to bypass the pinning, which is worth failing loudly - # rather than silently billing. - if DEMO_API_KEY and self.api_key == DEMO_API_KEY and self.model not in DEMO_MODELS: + # Belt and braces around server-funded turns: resolve_provider_config() + # already pins the model, and this makes it a property of the config + # object too, so a future caller can't construct an unpinned one. + # Unreachable by design — a raise here means a new code path bypassed + # the pinning, which is worth failing loudly rather than billing. + # + # The test is `using_demo`, NOT `api_key == DEMO_API_KEY`. Keying it on + # the key value looks stricter but is wrong: the demo key is a normal + # OpenRouter key, so a user can legitimately paste that same key into + # their own Settings as BYOK — and then every resolution raised, 500ing + # even GET /auth/me and taking the whole SPA down with it. `using_demo` + # is what actually means "the server is paying", and only the demo + # branch below sets it. + if self.using_demo and self.model not in DEMO_MODELS: raise ValueError( f"Refusing to use the shared demo key with non-whitelisted model {self.model!r}" ) diff --git a/backend/tests/test_chat.py b/backend/tests/test_chat.py index 1ee9886..755160a 100644 --- a/backend/tests/test_chat.py +++ b/backend/tests/test_chat.py @@ -180,20 +180,45 @@ def test_demo_key_endpoint_cannot_be_redirected(client, monkeypatch): assert FakeProvider.last_key == "demo-key" -def test_provider_config_refuses_demo_key_with_paid_model(monkeypatch): - """The structural backstop: even a hand-built config (a future code path - that forgets to go through resolve_provider_config) can't pair them.""" +def test_provider_config_refuses_server_funded_paid_model(monkeypatch): + """The structural backstop: a hand-built config (a future code path that + forgets to go through resolve_provider_config) can't run a server-funded + turn on an off-whitelist model.""" monkeypatch.setattr(auth, "DEMO_API_KEY", "demo-key") monkeypatch.setattr(auth, "DEMO_MODELS", ["free/allowed"]) with pytest.raises(ValueError): auth.ProviderConfig("http://demo", "demo-key", "expensive/paid-model", True) - # Mislabelling it as non-demo doesn't help: the key is what's checked. - with pytest.raises(ValueError): - auth.ProviderConfig("http://demo", "demo-key", "expensive/paid-model", False) + auth.ProviderConfig("http://demo", "demo-key", "free/allowed", True) # whitelisted: fine # The user's own key with any model stays fine. auth.ProviderConfig("http://any", "sk-mine", "expensive/paid-model", False) +def test_byok_user_may_reuse_the_demo_keys_value(client, monkeypatch): + """Regression: the demo key is just an OpenRouter key, so a user can paste + that same value into their own Settings. That's BYOK — they're paying — and + it must not trip the guard. It used to raise on every resolution, which + 500'd GET /auth/me and took the whole SPA down (no nav, no chat).""" + monkeypatch.setattr(auth, "demo_enabled", lambda: True) + monkeypatch.setattr(auth, "DEMO_API_KEY", "shared-key") + monkeypatch.setattr(auth, "DEMO_ENDPOINT_URL", "http://demo") + monkeypatch.setattr(auth, "DEMO_MODELS", ["free/allowed"]) + db = SessionLocal() + try: + settings = db.query(models.Settings).first() + settings.api_key = "shared-key" # same value, but supplied by the user + settings.model = "expensive/paid-model" # their spend, their choice + db.commit() + finally: + db.close() + + assert client.get("/api/auth/me").status_code == 200 + assert client.get("/api/chat/config").status_code == 200 + resp = _send(client) + assert resp.status_code == 200, resp.text + assert FakeProvider.last_model == "expensive/paid-model" + assert FakeProvider.last_key == "shared-key" + + def test_resolve_provider_config_is_the_single_choke_point(monkeypatch): """Turns, AI Chat and the connection test all resolve through this one function, so pinning it here pins every caller. No DB or HTTP needed."""