diff --git a/backend/api/dependencies.py b/backend/api/dependencies.py index d23b2093..8348749e 100644 --- a/backend/api/dependencies.py +++ b/backend/api/dependencies.py @@ -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) diff --git a/backend/main.py b/backend/main.py index 64a14c4f..a48e65cf 100644 --- a/backend/main.py +++ b/backend/main.py @@ -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": diff --git a/tests/test_bearer_middleware.py b/tests/test_bearer_middleware.py index 699df0dd..784a70dd 100644 --- a/tests/test_bearer_middleware.py +++ b/tests/test_bearer_middleware.py @@ -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 diff --git a/tests/test_loopback_server_mode.py b/tests/test_loopback_server_mode.py index 6da90568..e38ab86f 100644 --- a/tests/test_loopback_server_mode.py +++ b/tests/test_loopback_server_mode.py @@ -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")