From f30fbbdca86c52bc3c670fcbb23b0c8e091af5b6 Mon Sep 17 00:00:00 2001 From: Mega Mind <146339422+M3gA-Mind@users.noreply.github.com> Date: Wed, 24 Jun 2026 22:53:24 +0530 Subject: [PATCH] fix(embeddings): fail fast when a keyed provider's API key is empty (TAURI-RUST-4TZ) (#4066) --- src/openhuman/embeddings/factory.rs | 6 ++- src/openhuman/embeddings/openai.rs | 50 ++++++++++++++++++++++++ src/openhuman/embeddings/openai_tests.rs | 35 +++++++++++++++++ src/openhuman/embeddings/voyage.rs | 3 +- 4 files changed, 91 insertions(+), 3 deletions(-) diff --git a/src/openhuman/embeddings/factory.rs b/src/openhuman/embeddings/factory.rs index 25bb69897..27cbf9940 100644 --- a/src/openhuman/embeddings/factory.rs +++ b/src/openhuman/embeddings/factory.rs @@ -50,7 +50,8 @@ pub fn create_embedding_provider( } "openai" => Ok(Box::new( OpenAiEmbedding::new("https://api.openai.com", "", model, dims) - .with_send_dimensions(model_supports_dimensions(model)), + .with_send_dimensions(model_supports_dimensions(model)) + .with_required_api_key(true), )), "cohere" => Ok(Box::new(CohereEmbedding::new("", model, dims))), name if name.starts_with("custom:") => { @@ -91,7 +92,8 @@ pub fn create_embedding_provider_with_credentials( } "openai" => Ok(Box::new( OpenAiEmbedding::new("https://api.openai.com", api_key, model, dims) - .with_send_dimensions(model_supports_dimensions(model)), + .with_send_dimensions(model_supports_dimensions(model)) + .with_required_api_key(true), )), "cohere" => Ok(Box::new(CohereEmbedding::new(api_key, model, dims))), "custom" => { diff --git a/src/openhuman/embeddings/openai.rs b/src/openhuman/embeddings/openai.rs index 5ceefb90e..95cfe5209 100644 --- a/src/openhuman/embeddings/openai.rs +++ b/src/openhuman/embeddings/openai.rs @@ -21,6 +21,13 @@ pub struct OpenAiEmbedding { /// LocalAI/Ollama — keep working unchanged. Set via /// [`Self::with_send_dimensions`] for the OpenAI / custom-OpenAI paths. send_dimensions: bool, + /// When true, this provider points at a hosted cloud endpoint that always + /// requires a bearer token (genuine OpenAI `api.openai.com`, Voyage), so an + /// empty `api_key` must fail fast instead of POSTing an unauthenticated + /// request. Off by default so the OpenAI-compatible provider keeps serving + /// keyless local/custom endpoints (LocalAI, Ollama-via-OpenAI). Set via + /// [`Self::with_required_api_key`]. See the guard in [`Self::embed`]. + requires_api_key: bool, } impl OpenAiEmbedding { @@ -32,6 +39,7 @@ impl OpenAiEmbedding { model: model.to_string(), dims, send_dimensions: false, + requires_api_key: false, } } @@ -45,6 +53,17 @@ impl OpenAiEmbedding { self } + /// Mark this provider as a keyed cloud endpoint that must have an API key. + /// When set, [`Self::embed`] fails fast (before any HTTP round-trip) if the + /// resolved `api_key` is empty, instead of silently omitting the + /// `Authorization` header. Use for genuine OpenAI (`api.openai.com`) and + /// Voyage; leave off for keyless local/custom OpenAI-compatible endpoints. + /// Returns `self` for builder chaining. + pub fn with_required_api_key(mut self, required: bool) -> Self { + self.requires_api_key = required; + self + } + /// Returns the configured base URL. pub fn base_url(&self) -> &str { &self.base_url @@ -143,6 +162,37 @@ impl EmbeddingProvider for OpenAiEmbedding { ); } + // Fast-fail when this is a keyed cloud provider (OpenAI / Voyage) but the + // resolved key is empty. The key collapses to "" when the stored BYO + // credential can't be read — the OS-keychain consent is `none`/declined + // (`cached_consent=none`) or the cred fails to decrypt — because + // `resolve_api_key` swallows every such failure into "". Without this + // guard the request goes out with NO `Authorization` header at all and + // OpenAI 401s "You didn't provide an API key" on every embed; the memory + // pipeline re-embeds per document and floods Sentry (TAURI-RUST-4TZ: + // 3.9k events). Bailing here skips the wasted request, and the "API key + // not set" wording is demoted by the `ApiKeyMissing` classifier in + // `core::observability` to a single low-cardinality breadcrumb. The + // remediation surfaces the keychain-consent / re-enter-key path so the + // stored key can actually be read. Scoped via `requires_api_key`: the + // OpenAI-compatible provider legitimately supports keyless local/custom + // endpoints (LocalAI, Ollama-via-OpenAI), which keep omitting the header + // rather than bailing — mirroring the Cohere guard (TAURI-RUST-52S). + if self.requires_api_key && self.api_key.trim().is_empty() { + let message = format!( + "Embedding API key not set (model={}) — re-enter your key or grant \ + keychain access in Settings → Memory", + self.model, + ); + crate::core::observability::report_error_or_expected( + message.as_str(), + "embeddings", + "openai_embed", + &[("model", self.model.as_str()), ("failure", "missing_key")], + ); + anyhow::bail!(message); + } + let url = self.embeddings_url(); tracing::debug!( diff --git a/src/openhuman/embeddings/openai_tests.rs b/src/openhuman/embeddings/openai_tests.rs index 0dce10181..ee26243e9 100644 --- a/src/openhuman/embeddings/openai_tests.rs +++ b/src/openhuman/embeddings/openai_tests.rs @@ -295,6 +295,41 @@ async fn embed_skips_auth_header_when_key_empty() { p.embed(&["test"]).await.unwrap(); } +/// A keyed cloud provider (`with_required_api_key(true)` — genuine OpenAI / +/// Voyage) with an empty key must bail BEFORE any HTTP request rather than +/// POSTing with no `Authorization` header and 401-ing on every embed +/// (TAURI-RUST-4TZ). The base URL points at an address nothing is listening on, +/// so asserting the error is the key-guard message — not a connection error — +/// proves no request was attempted. The "API key not set" wording is what the +/// `ApiKeyMissing` classifier keys on to demote the flood out of Sentry. +#[tokio::test] +async fn embed_required_key_empty_bails_without_request() { + for key in ["", " "] { + let p = OpenAiEmbedding::new("http://127.0.0.1:1", key, "text-embedding-3-small", 1) + .with_required_api_key(true); + let err = p.embed(&["hello"]).await.unwrap_err().to_string(); + assert!( + err.contains("API key not set"), + "expected key-guard message for key {key:?}, got: {err}" + ); + } +} + +/// The keyless local/custom path is unaffected: without +/// `with_required_api_key`, an empty key still omits the header and sends the +/// request (LocalAI / Ollama-via-OpenAI legitimately need no bearer). +#[tokio::test] +async fn embed_empty_key_without_requirement_still_sends() { + let app = Router::new().route( + "/v1/embeddings", + post(|| async { Json(serde_json::json!({ "data": [{ "embedding": [1.0] }] })) }), + ); + let url = start_mock(app).await; + let p = OpenAiEmbedding::new(&url, "", "m", 1); // no with_required_api_key + let result = p.embed(&["test"]).await.unwrap(); + assert_eq!(result.len(), 1); +} + // ── embed — error paths ───────────────────────────────── #[tokio::test] diff --git a/src/openhuman/embeddings/voyage.rs b/src/openhuman/embeddings/voyage.rs index 83228d5ab..44d84010a 100644 --- a/src/openhuman/embeddings/voyage.rs +++ b/src/openhuman/embeddings/voyage.rs @@ -28,7 +28,8 @@ impl VoyageEmbedding { let dims = if dims == 0 { VOYAGE_DEFAULT_DIMS } else { dims }; Self { - inner: OpenAiEmbedding::new(VOYAGE_API_BASE, api_key, model, dims), + inner: OpenAiEmbedding::new(VOYAGE_API_BASE, api_key, model, dims) + .with_required_api_key(true), } }