fix(security): normalize remote API keys
This commit is contained in:
+11
-10
@@ -100,6 +100,13 @@ def _server_mode() -> bool:
|
||||
return os.environ.get("OMNIVOICE_SERVER_MODE", "").strip().lower() in _TRUTHY
|
||||
|
||||
|
||||
def remote_api_key() -> str | None:
|
||||
"""The normalized remote-backend bearer key, or None when remote mode is
|
||||
off. Surrounding whitespace is configuration noise, never a valid secret.
|
||||
Read at call time so tests can monkeypatch the environment."""
|
||||
return os.environ.get("OMNIVOICE_API_KEY", "").strip() or None
|
||||
|
||||
|
||||
def _configured_pin(request) -> str | None:
|
||||
"""The active share PIN (``app.state.network_share.pin``) or None. Read via
|
||||
getattr so a bare Request stub (or a request that hit before lifespan set
|
||||
@@ -117,7 +124,7 @@ def _admin_credential_configured(request) -> bool:
|
||||
opted out of bare-server discovery. Remote admin then remains closed until
|
||||
they configure and present the long API key.
|
||||
"""
|
||||
if os.environ.get("OMNIVOICE_API_KEY", "").strip():
|
||||
if remote_api_key():
|
||||
return True
|
||||
return bool(_configured_pin(request))
|
||||
|
||||
@@ -136,7 +143,7 @@ def _request_presents_admin_credential(request) -> bool:
|
||||
admin. Net: remote admin in server mode requires the API key; a PIN-only
|
||||
deployment keeps admin loopback-only. getattr-defensive so a minimal Request
|
||||
stub never raises."""
|
||||
api_key = os.environ.get("OMNIVOICE_API_KEY") or ""
|
||||
api_key = remote_api_key() or ""
|
||||
if not api_key:
|
||||
return False
|
||||
headers = getattr(request, "headers", None) or {}
|
||||
@@ -146,7 +153,7 @@ def _request_presents_admin_credential(request) -> bool:
|
||||
auth = headers.get("authorization", "")
|
||||
supplied = auth[7:].strip() if auth.lower().startswith("bearer ") else ""
|
||||
if not supplied:
|
||||
supplied = query.get("api_key") or cookies.get("ov_key") or ""
|
||||
supplied = (query.get("api_key") or cookies.get("ov_key") or "").strip()
|
||||
return bool(supplied and secrets.compare_digest(supplied, api_key))
|
||||
|
||||
|
||||
@@ -285,12 +292,6 @@ def require_native_access(request: Request) -> None:
|
||||
raise HTTPException(status_code=403, detail="native filesystem access requires loopback origin")
|
||||
|
||||
|
||||
def remote_api_key() -> str | None:
|
||||
"""The remote-backend bearer key (Wave 2.3), or None when remote mode is
|
||||
off. Read at call time so tests can monkeypatch the env."""
|
||||
return os.environ.get("OMNIVOICE_API_KEY") or None
|
||||
|
||||
|
||||
def ws_remote_authorized(websocket) -> bool:
|
||||
"""Whether a WebSocket handshake presents the remote API key.
|
||||
|
||||
@@ -309,5 +310,5 @@ def ws_remote_authorized(websocket) -> bool:
|
||||
websocket.query_params.get("api_key")
|
||||
or websocket.cookies.get("ov_key")
|
||||
or ""
|
||||
)
|
||||
).strip()
|
||||
return secrets.compare_digest(supplied, key)
|
||||
|
||||
+8
-3
@@ -375,7 +375,10 @@ from services.model_manager import (
|
||||
)
|
||||
from services import network_share
|
||||
|
||||
from api.dependencies import is_local_host # loopback + OMNIVOICE_TRUSTED_NETWORKS
|
||||
from api.dependencies import ( # loopback + OMNIVOICE_TRUSTED_NETWORKS
|
||||
is_local_host,
|
||||
remote_api_key,
|
||||
)
|
||||
|
||||
from api.routers import (
|
||||
system,
|
||||
@@ -1197,7 +1200,7 @@ class BearerKeyMiddleware:
|
||||
async def __call__(self, scope, receive, send):
|
||||
if scope["type"] not in ("http", "websocket"):
|
||||
return await self.app(scope, receive, send)
|
||||
key = os.environ.get("OMNIVOICE_API_KEY") or ""
|
||||
key = remote_api_key() or ""
|
||||
if not key:
|
||||
return await self.app(scope, receive, send)
|
||||
client = scope["client"][0] if scope.get("client") else None
|
||||
@@ -1215,7 +1218,9 @@ class BearerKeyMiddleware:
|
||||
auth = conn.headers.get("authorization", "")
|
||||
supplied = auth[7:].strip() if auth.lower().startswith("bearer ") else ""
|
||||
if not supplied:
|
||||
supplied = conn.query_params.get("api_key") or conn.cookies.get("ov_key") or ""
|
||||
supplied = (
|
||||
conn.query_params.get("api_key") or conn.cookies.get("ov_key") or ""
|
||||
).strip()
|
||||
|
||||
if not secrets.compare_digest(supplied, key):
|
||||
if scope["type"] == "websocket":
|
||||
|
||||
@@ -30,6 +30,12 @@ def test_inert_without_env(monkeypatch):
|
||||
assert c.get("/health").status_code == 200
|
||||
|
||||
|
||||
def test_whitespace_only_env_is_not_an_api_key(monkeypatch):
|
||||
monkeypatch.setenv("OMNIVOICE_API_KEY", " ")
|
||||
c = _client()
|
||||
assert c.get("/v1/audio/voices").status_code != 401
|
||||
|
||||
|
||||
def test_loopback_bypasses_key(key_env):
|
||||
c = _client(("127.0.0.1", 1))
|
||||
assert c.get("/system/info").status_code == 200
|
||||
|
||||
@@ -407,6 +407,26 @@ def test_server_mode_admin_mutation_allows_api_key(monkeypatch):
|
||||
))
|
||||
|
||||
|
||||
def test_whitespace_only_api_key_cannot_authorize_admin_mutation(monkeypatch):
|
||||
monkeypatch.setenv("OMNIVOICE_SERVER_MODE", "1")
|
||||
monkeypatch.setenv("OMNIVOICE_API_KEY", " ")
|
||||
|
||||
for credential in (
|
||||
{"query": {"api_key": " "}},
|
||||
{"cookies": {"ov_key": " "}},
|
||||
):
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
require_admin(
|
||||
_req_full("172.17.0.1", method="POST", **credential)
|
||||
)
|
||||
assert exc.value.status_code == 403
|
||||
|
||||
monkeypatch.setenv("OMNIVOICE_API_KEY", " s3cret ")
|
||||
require_admin(
|
||||
_req_full("172.17.0.1", method="POST", query={"api_key": " s3cret "})
|
||||
)
|
||||
|
||||
|
||||
def test_server_mode_desktop_capability_rejects_remote_api_key(monkeypatch):
|
||||
monkeypatch.setenv("OMNIVOICE_SERVER_MODE", "1")
|
||||
monkeypatch.setenv("OMNIVOICE_API_KEY", "s3cret")
|
||||
|
||||
Reference in New Issue
Block a user