fix(auth): offer the API-key login on server-mode admin 403s

CHANGELOG.md

@@ -26,6 +26,7 @@ the frozen-backend fallback mirror it for their toolchains.
 - Every engine now has its own guide — 21 new pages under docs/engines plus an index covering all 16 TTS and 11 ASR engines, linked from both READMEs (#1556)
 
 ### Fixed
+- A remote browser hitting the admin-gated 403 ("loopback origin or admin API key required") now gets the API-key login form instead of endless console 403s, and the desktop build's admin 403 says plain "loopback origin required" so PIN-share guests are never offered a login form no key can satisfy (#1568)
 - The crash-isolated ASR sidecar and its download preflight now agree on which model to load — setting the shared faster-whisper model variable applies to both variants instead of the sidecar quietly using a different one (#1556)
 - "Ready" now requires the deep health probe (a working database-backed route), not just the identity probe — a backend whose install broke underneath can no longer be announced up while every real request fails (#1548)
 - Supervisor restarts after repeat crashes now back off (immediate, then 5s, then 15s) instead of respawning back-to-back, so a tight crash loop can't burn the whole restart budget in seconds (#1548)

backend/api/dependencies.py

@@ -157,6 +157,31 @@ def require_loopback(request: Request) -> None:
     raise HTTPException(status_code=403, detail="loopback origin required")
 
 
+def _admin_gate_403() -> None:
+    """Raise the admin-gate 403 with a detail that states what would ACTUALLY
+    satisfy the gate. The bundled UI routes any 403 whose detail mentions
+    "admin api key" to the API-key login form (frontend ``client.ts``; the
+    literal contract is locked by ``tests/test_auth_gate_detail_lockstep.py``),
+    so the wording must not name a key where presenting one cannot help.
+
+    The detail names the key only when the gate would accept one: server mode
+    WITH an API key configured. Every other rejection — desktop mode (the
+    credential checks in the callers only run under server mode) and a
+    server-mode deployment with only a share PIN or nothing configured — keeps
+    the plain loopback detail, because only loopback can use admin there.
+    Naming the key in those cases would trap a LAN-share guest in a login
+    form that can never succeed (#1213, #1525; PR #1569 review).
+    """
+    raise HTTPException(
+        status_code=403,
+        detail=(
+            "loopback origin or admin API key required"
+            if _server_mode() and remote_api_key()
+            else "loopback origin required"
+        ),
+    )
+
+
 def require_admin(request: Request) -> None:
     """Gate RCE/filesystem-capable admin routers.
 
@@ -180,7 +205,7 @@ def require_admin(request: Request) -> None:
             return
         if _request_presents_admin_credential(request):
             return
-    raise HTTPException(status_code=403, detail="loopback origin or admin API key required")
+    _admin_gate_403()
 
 
 def require_admin_action(request: Request) -> None:
@@ -198,7 +223,7 @@ def require_admin_action(request: Request) -> None:
         side_effectful_get=True,
     ):
         return
-    raise HTTPException(status_code=403, detail="loopback origin or admin API key required")
+    _admin_gate_403()
 
 
 def require_desktop(request: Request) -> None:

docs/api-auth.md

@@ -342,7 +342,7 @@ arbitrary path merely because it ends in `/ws/events` or `/ws/transcribe`.
 | Code | Meaning | What to do |
 |---|---|---|
 | **401** | Consumption auth failed — `{"detail": "PIN required"}` or `{"detail": "API key required"}`. | Supply the PIN / key (header, cookie, or query param above). A WebSocket surfaces this as close code **1008**. |
-| **403** | Authorization failed: loopback/native access was required, cookie Origin/CSRF validation failed, a server-mode mutation lacked an admin credential, or a native path capability was invalid/expired. | A PIN cannot grant admin or filesystem access. Re-authenticate the UI; scripts should use the API-key header; run native operations from the desktop app. |
+| **403** | Authorization failed: loopback/native access was required, cookie Origin/CSRF validation failed, a server-mode mutation lacked an admin credential, or a native path capability was invalid/expired. | A PIN cannot grant admin or filesystem access. Re-authenticate the UI; scripts should use the API-key header; run native operations from the desktop app. The admin gates are mode-distinct: server mode answers `{"detail": "loopback origin or admin API key required"}` (the bundled UI routes it to the API-key login form), the desktop build answers `{"detail": "loopback origin required"}` (a presented key cannot satisfy it — only loopback can). |
 | **429** | A failed administrator-session exchange exceeded its per-client limit, the GPU pool is saturated, or a model download is rate-limited. Ships with `Retry-After`; workload throttles also carry `X-VoiceStudio-Retryable: true`. | Back off for `Retry-After` seconds. For authentication, verify the master before retrying; a correct master is never locked out. |
 
 ---

frontend/src/api/client.test.ts

@@ -182,15 +182,16 @@ describe('apiFetch 401 routing', () => {
     dispatch.mockRestore();
   });
 
-  const stub401 = (detail: string) =>
+  const stubStatus = (status: number, statusText: string, detail: string) =>
     vi.fn(() =>
       Promise.resolve({
         ok: false,
-        status: 401,
-        statusText: 'Unauthorized',
+        status,
+        statusText,
         text: async () => JSON.stringify({ detail }),
       }),
     ) as any;
+  const stub401 = (detail: string) => stubStatus(401, 'Unauthorized', detail);
 
   const authEvent = () =>
     dispatch.mock.calls.map((c) => c[0]).find((e) => (e as Event).type === 'ov:auth-required');
@@ -227,6 +228,62 @@ describe('apiFetch 401 routing', () => {
     expect(authEvent()).toBeTruthy();
     expect((authEvent() as any).detail.mode).toBe('pin');
   });
+
+  const stub403 = (detail: string) => stubStatus(403, 'Forbidden', detail);
+
+  it('dispatches ov:auth-required {mode:"apikey"} on an admin-gate 403 (#1525)', async () => {
+    globalThis.fetch = stub403('loopback origin or admin API key required');
+    const { apiFetch } = await import('./client');
+    try {
+      await apiFetch('/system/info');
+    } catch {
+      /* ApiError expected */
+    }
+    expect(authEvent()).toBeTruthy();
+    expect((authEvent() as any).detail.mode).toBe('apikey');
+  });
+
+  it('does not dispatch ov:auth-required on other 403s (CSRF / desktop-only)', async () => {
+    globalThis.fetch = stub403('browser origin rejected');
+    const { apiFetch } = await import('./client');
+    try {
+      await apiFetch('/system/info');
+    } catch {
+      /* ApiError expected */
+    }
+    expect(authEvent()).toBeFalsy();
+  });
+
+  it('a stale 403 does not clear a session stored during its flight (PR #1569 race)', async () => {
+    // The request goes out with NO credential; while it is in flight the
+    // user completes the key exchange. A late 403 may only invalidate the
+    // credentials the failed request actually carried — wiping the fresh
+    // session would reload a successful login straight back into the gate.
+    globalThis.fetch = vi.fn(() => {
+      sessionStorage.setItem(
+        ADMIN_SESSION_STORAGE_KEY,
+        JSON.stringify({
+          token: `ovs_admin_session_${'N'.repeat(43)}`,
+          expiresAt: Date.now() / 1000 + 3600,
+          apiBase: API,
+        }),
+      );
+      return Promise.resolve({
+        ok: false,
+        status: 403,
+        statusText: 'Forbidden',
+        text: async () => JSON.stringify({ detail: 'loopback origin or admin API key required' }),
+      });
+    }) as any;
+    const { apiFetch } = await import('./client');
+    try {
+      await apiFetch('/system/info');
+    } catch {
+      /* ApiError expected */
+    }
+    expect(authEvent()).toBeTruthy();
+    expect(sessionStorage.getItem(ADMIN_SESSION_STORAGE_KEY)).not.toBeNull();
+  });
 });
 
 describe('apiFetch 404 from a non-VoiceStudio server (#1385)', () => {

frontend/src/api/client.ts

@@ -528,14 +528,32 @@ export async function apiFetch(path: string, opts: ApiFetchOptions = {}): Promis
       // "API key required" (BearerKeyMiddleware, OMNIVOICE_API_KEY) vs anything
       // else, i.e. "PIN required" (NetworkAccessMiddleware). Both are 401; the
       // detail is the only discriminator (only two 401 sites exist backend-side).
-      if (backendTarget && res.status === 401 && typeof window !== 'undefined') {
+      // The router-level admin gates answer 403 "loopback origin or admin API
+      // key required" (require_admin/require_admin_action) — same situation, the
+      // client just isn't admin-authenticated — so it routes to the API-key form
+      // too. Other 403s (CSRF "browser origin rejected", loopback-only routes)
+      // are NOT credential gaps; presenting a key won't help, so they stay plain
+      // errors.
+      const adminGate403 =
+        res.status === 403 &&
+        typeof detail === 'string' &&
+        detail.toLowerCase().includes('admin api key');
+      if (backendTarget && (res.status === 401 || adminGate403) && typeof window !== 'undefined') {
         // readError's declared `string` return isn't guaranteed at runtime —
         // `j.detail` can be a structured object/array on a future 401. Match only
         // real strings (avoids both a `.toLowerCase()` crash and `String()` itself
         // throwing on a malformed object); anything else falls back to PIN.
+        // (No adminGate403 arm here: "admin api key" ⊇ "api key", so the sniff
+        // below already yields 'apikey' for every admin-gate 403.)
         const mode =
           typeof detail === 'string' && detail.toLowerCase().includes('api key') ? 'apikey' : 'pin';
-        if (mode === 'apikey') clearAdminSession();
+        // A failed response may only invalidate the credentials it actually
+        // carried (`session` is captured at send time). Clearing blindly let
+        // a stale 403 that landed after a key exchange wipe the fresh
+        // session, reloading a successful login straight back into the gate.
+        if (mode === 'apikey' && session && getAdminSession(API)?.token === session.token) {
+          clearAdminSession();
+        }
         window.dispatchEvent(new CustomEvent('ov:auth-required', { detail: { mode } }));
       }
       // Structured details (e.g. the typed asr_model_missing 409) carry a

tests/backend/api/test_engines_route_shape.py

@@ -627,7 +627,7 @@ def test_engine_health_is_admin_gated(fresh_app):
     client = _client(fresh_app, host="10.0.0.5")
     r = client.get("/engines/omnivoice/health")
     assert r.status_code == 403
-    assert r.json()["detail"] == "loopback origin or admin API key required"
+    assert r.json()["detail"] == "loopback origin required"
 
 
 def test_server_mode_engine_mutations_require_api_key(fresh_app, monkeypatch):
@@ -802,7 +802,7 @@ def test_selftest_unknown_id_is_404(fresh_app):
 def test_selftest_is_admin_gated(fresh_app):
     r = _client(fresh_app, host="10.0.0.9").post("/engines/omnivoice/selftest")
     assert r.status_code == 403
-    assert r.json()["detail"] == "loopback origin or admin API key required"
+    assert r.json()["detail"] == "loopback origin required"
 
 
 def test_selftest_captures_synth_exception_without_500(fresh_app):

tests/test_auth_gate_detail_lockstep.py

@@ -0,0 +1,76 @@
+"""Cross-layer contract lock for the admin-gate 403 detail string.
+
+The backend's ``require_admin``/``require_admin_action`` answer 403 with a
+mode-distinct ``detail`` (``_admin_gate_403`` in backend/api/dependencies.py):
+"loopback origin or admin API key required" in server mode, plain
+"loopback origin required" on the desktop build. The SPA's ``apiFetch`` routes
+a 403 to the API-key login gate exactly when the detail contains the substring
+"admin api key" (frontend/src/api/client.ts) — i.e. when presenting the key
+could actually satisfy the gate. The per-mode behaviour is pinned by
+tests/test_loopback_server_mode.py; this file pins the LITERAL contract across
+layers: a backend reword keeps backend tests green while the frontend matcher
+silently stops firing, and a LAN user is back to raw 403 spam instead of the
+login form.
+"""
+
+from __future__ import annotations
+
+import re
+from pathlib import Path
+
+ROOT = Path(__file__).resolve().parents[1]
+DEPS = ROOT / "backend" / "api" / "dependencies.py"
+CLIENT = ROOT / "frontend" / "src" / "api" / "client.ts"
+
+
+def _frontend_sniff() -> str:
+    """The substring apiFetch matches on a 403 to admit it to the auth gate."""
+    text = CLIENT.read_text(encoding="utf-8")
+    # adminGate403 = ... detail.toLowerCase().includes('<sniff>')
+    m = re.search(r"adminGate403 =.*?includes\('([^']+)'\)", text, re.DOTALL)
+    assert m, "adminGate403 matcher not found in frontend/src/api/client.ts"
+    return m.group(1)
+
+
+def _key_named_details() -> set[str]:
+    """Every quoted string in dependencies.py that names the admin API key."""
+    return set(re.findall(r'"([^"]*admin API key[^"]*)"', DEPS.read_text(encoding="utf-8")))
+
+
+def test_key_named_details_match_frontend_sniff():
+    """Every backend literal naming the admin key must contain the SPA matcher."""
+    details = _key_named_details()
+    assert details, (
+        "no 'admin API key' detail literal left in dependencies.py — moved or "
+        "reworded? Update frontend/src/api/client.ts in the same change."
+    )
+    sniff = _frontend_sniff()
+    for detail in details:
+        # Case-insensitive substring, mirroring apiFetch's toLowerCase match.
+        assert sniff in detail.lower(), (
+            f"backend detail {detail!r} no longer contains the frontend matcher "
+            f"{sniff!r} — the SPA would stop routing it to the API-key gate. "
+            "Update frontend/src/api/client.ts in the same change."
+        )
+
+
+def test_frontend_sniff_rejects_details_a_key_cannot_fix():
+    """The sniff must not swallow 403s an API key cannot satisfy.
+
+    The desktop admin-gate arm (loopback-only regardless of credentials), the
+    legacy require_loopback desktop 403, the CSRF rejection, and the
+    desktop-only filesystem gate: routing any of these to the login form would
+    trap the user in a form that can never succeed.
+    """
+    sniff = _frontend_sniff()
+    unfixable = (
+        "loopback origin required",  # desktop admin arm + require_loopback
+        "browser origin rejected",  # BearerKeyMiddleware CSRF (main.py)
+        "desktop origin required",  # require_desktop — loopback-only forever
+        "native filesystem access requires loopback origin",  # require_native
+    )
+    for detail in unfixable:
+        assert sniff not in detail.lower(), (
+            f"frontend matcher {sniff!r} now also matches {detail!r}, which "
+            "an API key cannot satisfy — the login gate would loop."
+        )

tests/test_loopback_server_mode.py

@@ -253,6 +253,89 @@ def test_side_effectful_get_rejects_remote_api_key_outside_server_mode(monkeypat
     assert exc.value.status_code == 403
 
 
+# Mode-distinct admin-gate detail: the 403 message must state what would
+# ACTUALLY satisfy the gate. The bundled UI routes any 403 whose detail
+# mentions "admin api key" to the API-key login form (frontend client.ts;
+# the literal contract is locked by tests/test_auth_gate_detail_lockstep.py).
+# Server mode accepts the key, so naming it is right. Desktop mode rejects
+# every non-loopback client regardless of credentials — the checks above only
+# run under server mode — so it must keep the plain loopback detail: naming
+# the key there invites a login form that can never succeed (a desktop
+# LAN-share guest would lose the whole consumption UI to it, #1213).
+
+
+def test_require_admin_desktop_detail_is_plain_loopback(monkeypatch):
+    """Desktop build: no presented key can satisfy the gate."""
+    monkeypatch.delenv("OMNIVOICE_SERVER_MODE", raising=False)
+    monkeypatch.setenv("OMNIVOICE_API_KEY", "s3cret")  # a valid key can't help here
+
+    with pytest.raises(HTTPException) as exc:
+        require_admin(
+            _req_full("10.0.0.5", headers={"authorization": "Bearer s3cret"})
+        )
+
+    assert exc.value.status_code == 403
+    assert exc.value.detail == "loopback origin required"
+
+
+def test_require_admin_server_mode_detail_names_the_key(monkeypatch):
+    """Server mode with an API key configured: the 403 names the key."""
+    monkeypatch.setenv("OMNIVOICE_SERVER_MODE", "1")
+    monkeypatch.setenv("OMNIVOICE_API_KEY", "s3cret")
+
+    with pytest.raises(HTTPException) as exc:
+        require_admin(_req_full("172.17.0.1"))  # credential configured, none presented
+
+    assert exc.value.status_code == 403
+    assert exc.value.detail == "loopback origin or admin API key required"
+
+
+def test_require_admin_pin_only_server_mode_detail_is_plain_loopback(monkeypatch):
+    """Server mode with ONLY a share PIN (Greptile P1, PR #1569): the PIN
+    closes read-only bootstrap but no API key exists to present, so naming
+    the key would send the browser to a login form that can never succeed.
+    Only loopback can use admin here — the plain detail says so, and the
+    SPA leaves it a plain error instead of gating the whole UI."""
+    monkeypatch.setenv("OMNIVOICE_SERVER_MODE", "1")
+    monkeypatch.delenv("OMNIVOICE_API_KEY", raising=False)
+
+    with pytest.raises(HTTPException) as exc:
+        require_admin(_req_full("172.17.0.1", pin="424242"))  # PIN ≠ admin credential
+
+    assert exc.value.status_code == 403
+    assert exc.value.detail == "loopback origin required"
+
+
+def test_require_admin_action_desktop_detail_is_plain_loopback(monkeypatch):
+    """Desktop build, side-effectful GET: plain loopback detail."""
+    monkeypatch.delenv("OMNIVOICE_SERVER_MODE", raising=False)
+    monkeypatch.setenv("OMNIVOICE_API_KEY", "s3cret")
+
+    with pytest.raises(HTTPException) as exc:
+        require_admin_action(
+            _req_full(
+                "10.0.0.5",
+                method="GET",
+                headers={"authorization": "Bearer s3cret"},
+            )
+        )
+
+    assert exc.value.status_code == 403
+    assert exc.value.detail == "loopback origin required"
+
+
+def test_require_admin_action_server_mode_detail_names_the_key(monkeypatch):
+    """Server mode + key configured, side-effectful GET: names the key."""
+    monkeypatch.setenv("OMNIVOICE_SERVER_MODE", "1")
+    monkeypatch.setenv("OMNIVOICE_API_KEY", "s3cret")
+
+    with pytest.raises(HTTPException) as exc:
+        require_admin_action(_req_full("172.17.0.1", method="GET"))
+
+    assert exc.value.status_code == 403
+    assert exc.value.detail == "loopback origin or admin API key required"
+
+
 def test_side_effectful_get_rejects_pin_and_trusted_network(monkeypatch):
     monkeypatch.setenv("OMNIVOICE_SERVER_MODE", "1")
     monkeypatch.setenv("OMNIVOICE_TRUSTED_NETWORKS", "10.0.0.0/8")