fix(security): require API key for remote admin writes

backend/api/dependencies.py

@@ -186,6 +186,32 @@ def require_loopback(request: Request) -> None:
     raise HTTPException(status_code=403, detail="loopback origin required")
 
 
+def require_admin(request: Request) -> None:
+    """Gate RCE/filesystem-capable admin routers.
+
+    Desktop callers keep the loopback-only contract. Docker cannot reliably
+    observe the host operator as loopback, so authenticated remote admin stays
+    available there, but every state-changing request must present the long API
+    key. An unconfigured server must never expose executable-path or filesystem
+    settings to every client that can reach its published port.
+
+    Read-only requests retain the bare-Docker bootstrap behaviour until an API
+    key is configured. Share PINs and trusted CIDRs are consumption credentials;
+    neither authorizes this gate.
+    """
+    host = request.client.host if request.client else None
+    if is_loopback(host):
+        return
+    if _server_mode():
+        method = str(getattr(request, "method", "GET")).upper()
+        read_only = method in {"GET", "HEAD", "OPTIONS"}
+        if read_only and not _admin_credential_configured(request):
+            return
+        if _request_presents_admin_credential(request):
+            return
+    raise HTTPException(status_code=403, detail="loopback origin or admin API key required")
+
+
 def require_local(request: Request) -> None:
     """Reject any request whose client.host is not loopback OR on a configured
     trusted network. The consumption-tier companion to :func:`require_loopback`:

backend/api/routers/settings.py

@@ -1,11 +1,10 @@
 """Settings API — HF token save/clear/state endpoints (Phase 1 AUTH-03 backend half).
 
 These endpoints are the backend half of the Wave 2 Settings → API Keys
-panel. Threat T-01-03 mitigation: every write endpoint is gated by the
-router-level `require_loopback` dep, so non-loopback origins get 403
-before the handler runs. Reads are loopback-gated too — the masked
-token preview is useful telemetry that we still don't want exposed on
-the LAN.
+panel. Threat T-01-03 mitigation: the router-level `require_admin` dependency
+keeps desktop callers loopback-only and requires the long API key for every
+remote server-mode mutation. Read-only bare-Docker discovery remains available
+until an API key is configured; once configured, reads require it too.
 
 The state endpoint duplicates `/system/hf-token/state` (which lives on
 `system.py` for legacy-router compatibility); both return the same shape.
@@ -20,14 +19,14 @@ from dataclasses import asdict
 from fastapi import APIRouter, Depends, HTTPException, Query
 from pydantic import BaseModel, Field
 
-from api.dependencies import require_loopback
+from api.dependencies import require_admin
 
 logger = logging.getLogger("omnivoice.api.settings")
 
 router = APIRouter(
     prefix="/api/settings",
     tags=["settings"],
-    dependencies=[Depends(require_loopback)],
+    dependencies=[Depends(require_admin)],
 )
 
 

backend/api/routers/system.py

@@ -11,7 +11,7 @@ from core.prefs import set_ as prefs_set, delete as prefs_delete
 from services import network_share
 from services import tailscale as _tailscale
 from api.schemas import SysinfoResponse, SystemInfoResponse, ModelStatusResponse
-from api.dependencies import require_loopback
+from api.dependencies import require_admin
 from fastapi.responses import FileResponse, StreamingResponse
 import torch
 import shutil
@@ -21,17 +21,16 @@ from core.version import APP_VERSION
 from services.model_manager import get_model_status, get_best_device, resolve_omnivoice_checkpoint
 from services.ffmpeg_utils import find_ffmpeg, run_ffmpeg
 
-# Router-level loopback gate. Every route mounted on `router` (GET + POST,
-# present and future) is gated by `require_loopback`, which 403s any request
-# whose `client.host` is not a loopback address. This closes the same trust
+# Router-level admin gate. Every route mounted on `router` (GET + POST,
+# present and future) is gated by `require_admin`: desktop requests must be
+# loopback; server-mode mutations require the long API key. This closes the trust
 # boundary that PR #81 only patched on `/system/set-env` and that the
 # 260518-ivy deferred-items file enumerated for follow-up: /model/unload/*,
 # /system/logs/clear, /system/logs/tauri/clear, /system/flush-memory,
 # /clean-audio (POSTs) plus the read-side info-disclosure routes
 # /system/info, /system/logs, /system/logs/tauri, /system/logs/stream.
-# This router only ever serves the local Tauri shell and the dev frontend
-# at http://127.0.0.1:3901 — both are loopback origins.
-router = APIRouter(dependencies=[Depends(require_loopback)])
+# Native Tauri/dev callers remain loopback and need no credential.
+router = APIRouter(dependencies=[Depends(require_admin)])
 logger = logging.getLogger("omnivoice.api")
 
 # Cache device checks at module load — they don't change at runtime
@@ -843,7 +842,7 @@ async def set_env_var(body: dict):
     are set on ``os.environ`` for the running process.
 
     The loopback-origin gate that previously lived inline here is now applied
-    at the router level via `dependencies=[Depends(require_loopback)]` on
+    at the router level via `dependencies=[Depends(require_admin)]` on
     `router` — see the top of this file. Every route on this router is
     gated, including this one. The 403 body and behavior are unchanged.
     """

docs/api-auth.md

@@ -206,8 +206,9 @@ origin is unenforceable — NAT rewrites the source and even a
 requirement is dropped (issue #261, else the operator is 403'd out of their own
 `/system/*`). It is replaced by a **credential rule**, not removed:
 
-- **No credential configured** (no API key, no PIN) → admin is open. The bare
-  Docker flow; exposure rests entirely on your port mapping / firewall.
+- **No API key configured** → read-only admin discovery remains available for
+  the bare Docker bootstrap flow, but `POST`/`PUT`/`PATCH`/`DELETE` requests are
+  denied. Set `OMNIVOICE_API_KEY` before changing settings remotely.
 - **A credential is configured** → admin requires the **API key** (`Authorization:
   Bearer` / `?api_key` / `ov_key` cookie), or genuine loopback. The **6-digit
   share PIN does not gate admin** (it is brute-forceable), and trusted-network

tests/test_api.py

@@ -600,6 +600,30 @@ def test_set_env_allows_loopback():
             os.environ["HF_TOKEN"] = original
 
 
+def test_server_mode_remote_without_api_key_cannot_set_executable_path(monkeypatch, tmp_path):
+    """GHAS #506: bare Docker exposure must not become an executable setter."""
+    from fastapi.testclient import TestClient
+    from main import app
+
+    executable = tmp_path / "ffmpeg"
+    executable.write_bytes(b"not actually executable")
+    original = os.environ.get("FFMPEG_PATH")
+    monkeypatch.setenv("OMNIVOICE_SERVER_MODE", "1")
+    monkeypatch.delenv("OMNIVOICE_API_KEY", raising=False)
+    try:
+        response = TestClient(app, client=("172.17.0.1", 50000)).post(
+            "/system/set-env",
+            json={"key": "FFMPEG_PATH", "value": str(executable)},
+        )
+        assert response.status_code == 403
+        assert os.environ.get("FFMPEG_PATH") == original
+    finally:
+        if original is None:
+            os.environ.pop("FFMPEG_PATH", None)
+        else:
+            os.environ["FFMPEG_PATH"] = original
+
+
 def test_set_env_loopback_still_validates_allowlist():
     """Even on the loopback path, keys outside the allow-list must return 400 —
     the new guard must NOT bypass the existing allow-list enforcement."""
@@ -686,4 +710,3 @@ def test_static_audio_served_with_canonical_mime():
         )
     finally:
         tmp_wav.unlink(missing_ok=True)
-

tests/test_loopback_server_mode.py

@@ -10,7 +10,13 @@ from types import SimpleNamespace
 import pytest
 from fastapi import HTTPException
 
-from api.dependencies import is_loopback, is_local_host, require_local, require_loopback
+from api.dependencies import (
+    is_loopback,
+    is_local_host,
+    require_admin,
+    require_local,
+    require_loopback,
+)
 
 
 def _req(host):
@@ -134,7 +140,7 @@ def test_require_local_rejects_untrusted_non_loopback(monkeypatch):
     assert exc.value.status_code == 403
 
 
-def _req_full(host, *, headers=None, query=None, cookies=None, pin=None):
+def _req_full(host, *, headers=None, query=None, cookies=None, pin=None, method="GET"):
     """Richer stub carrying the channels the admin-credential check reads:
     headers, query params, cookies, and app.state.network_share.pin."""
     ns = SimpleNamespace(pin=pin) if pin is not None else None
@@ -145,6 +151,7 @@ def _req_full(host, *, headers=None, query=None, cookies=None, pin=None):
         query_params=query or {},
         cookies=cookies or {},
         app=app,
+        method=method,
     )
 
 
@@ -231,6 +238,43 @@ def test_server_mode_loopback_admin_never_needs_credential(monkeypatch):
     require_loopback(_req_full("127.0.0.1"))  # must not raise
 
 
+# GHAS #506/#440/#441: require_loopback permits an unconfigured bare Docker
+# server for compatibility. RCE/filesystem-capable routers use the stricter,
+# method-aware admin gate instead.
+
+
+@pytest.mark.parametrize("method", ["POST", "PUT", "PATCH", "DELETE"])
+def test_server_mode_admin_mutation_requires_api_key_when_unconfigured(monkeypatch, method):
+    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", method=method))
+    assert exc.value.status_code == 403
+
+
+def test_server_mode_admin_read_keeps_bare_docker_bootstrap(monkeypatch):
+    monkeypatch.setenv("OMNIVOICE_SERVER_MODE", "1")
+    monkeypatch.delenv("OMNIVOICE_API_KEY", raising=False)
+    require_admin(_req_full("172.17.0.1", method="GET"))
+
+
+def test_server_mode_admin_mutation_allows_api_key(monkeypatch):
+    monkeypatch.setenv("OMNIVOICE_SERVER_MODE", "1")
+    monkeypatch.setenv("OMNIVOICE_API_KEY", "s3cret")
+    require_admin(_req_full(
+        "172.17.0.1",
+        method="POST",
+        headers={"authorization": "Bearer s3cret"},
+    ))
+
+
+@pytest.mark.parametrize("method", ["GET", "POST", "PUT", "DELETE"])
+def test_loopback_admin_never_needs_api_key(monkeypatch, method):
+    monkeypatch.setenv("OMNIVOICE_SERVER_MODE", "1")
+    monkeypatch.delenv("OMNIVOICE_API_KEY", raising=False)
+    require_admin(_req_full("127.0.0.1", method=method))
+
+
 def test_is_local_host_unwraps_ipv4_mapped_ipv6(monkeypatch):
     # Dual-stack proxies (Caddy, Node.js) pass ::ffff:192.168.1.5 — should
     # match an IPv4 CIDR after unwrapping the mapped address.

tests/test_models_dir_setting.py

@@ -59,6 +59,23 @@ def test_rejects_path_with_null_byte(env):
     assert ei.value.status_code == 400
 
 
+def test_server_mode_remote_without_api_key_cannot_create_models_dir(env, monkeypatch, tmp_path):
+    """GHAS #440/#441: a published bare Docker port is not filesystem auth."""
+    from fastapi.testclient import TestClient
+
+    monkeypatch.setenv("OMNIVOICE_SERVER_MODE", "1")
+    monkeypatch.delenv("OMNIVOICE_API_KEY", raising=False)
+    target = tmp_path / "must-not-exist"
+    app = fastapi.FastAPI()
+    app.include_router(s.router)
+    response = TestClient(app, client=("172.17.0.1", 50000)).put(
+        "/api/settings/storage/models-dir",
+        json={"path": str(target)},
+    )
+    assert response.status_code == 403
+    assert not target.exists()
+
+
 def test_clear_reverts_to_default(env):
     user_env.set_user_env("OMNIVOICE_CACHE_DIR", "/old")
     res = s.set_models_dir(s._ModelsDirBody(path=""))