Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions src/core/observability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,9 +263,17 @@ pub fn expected_error_kind(message: &str) -> Option<ExpectedErrorKind> {
// no longer reaches here — but other direct-mode ops the user invokes
// explicitly (execute / authorize) can still surface it, and it is the
// same user-config state with no Sentry-actionable signal.
//
// `"no api key supplied"` is Cohere's exact 401 body wording when the
// hosted `/v2/embed` endpoint is called with an empty bearer (the BYO-key
// embedding path with no key configured). The `CohereEmbedding::embed`
// guard now fast-fails before issuing that request, but this arm demotes
// any residual 401 of that shape — older clients, or a present-but-rejected
// key — so the flood (TAURI-RUST-52S) stays out of Sentry either way.
if lower.contains("api key not set")
|| lower.contains("missing api key")
|| lower.contains("no api key is configured")
|| lower.contains("no api key supplied")
{
return Some(ExpectedErrorKind::ApiKeyMissing);
}
Expand Down Expand Up @@ -2048,6 +2056,23 @@ mod tests {
);
}

/// Sentry TAURI-RUST-52S: Cohere's hosted `/v2/embed` returns a 401 with
/// body `"no api key supplied"` when called with an empty bearer (BYO-key
/// embedding path, no key configured). Verbatim wire shape from the embed
/// client must classify as `ApiKeyMissing` so the 8.7k-event flood stays
/// out of Sentry even from clients that predate the call-site guard.
#[test]
fn classifies_cohere_missing_api_key_401_as_api_key_missing() {
assert_eq!(
expected_error_kind(
"Cohere embed API error (401 Unauthorized): \
{\"id\":\"3176e92f-d18e-4019-bd43-314821504a61\",\
\"message\":\"no api key supplied\"}"
),
Some(ExpectedErrorKind::ApiKeyMissing)
);
}

/// Guard against over-suppression: a genuine BYO-key auth failure
/// (wrong/expired key the upstream rejected) is an actionable bug
/// shape and MUST still reach Sentry (stay `None`), not get demoted by
Expand Down
5 changes: 5 additions & 0 deletions src/openhuman/agent/harness/session/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -864,9 +864,14 @@ impl Agent {
)?;

let local_embedding = config.workload_local_model("embeddings");
let embedding_api_key = crate::openhuman::embeddings::resolve_api_key(
config,
&config.memory.embedding_provider,
);
let memory: Arc<dyn Memory> = Arc::from(memory_store::create_memory_with_local_ai(
&config.memory,
local_embedding.as_deref(),
&embedding_api_key,
&config.embedding_routes,
Some(&config.storage.provider.config),
&config.workspace_dir,
Expand Down
3 changes: 3 additions & 0 deletions src/openhuman/channels/runtime/startup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,9 +329,12 @@ pub async fn start_channels(mut config: Config) -> Result<()> {
)?;
let temperature = config.default_temperature;
let local_embedding = config.workload_local_model("embeddings");
let embedding_api_key =
crate::openhuman::embeddings::resolve_api_key(&config, &config.memory.embedding_provider);
let mem: Arc<dyn Memory> = Arc::from(memory_store::create_memory_with_local_ai(
&config.memory,
local_embedding.as_deref(),
&embedding_api_key,
&[],
Some(&config.storage.provider.config),
&config.workspace_dir,
Expand Down
37 changes: 37 additions & 0 deletions src/openhuman/embeddings/cohere.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,25 @@ impl EmbeddingProvider for CohereEmbedding {
return Ok(Vec::new());
}

// Fast-fail when no API key is configured. Cohere's hosted `/v2/embed`
// always requires a bearer token; without this guard we POST
// `Authorization: Bearer ` (empty) and Cohere returns a 401 "no api key
// supplied" on every call. That 401 is not retryable, so each embed
// attempt bails and reports an error — the memory pipeline re-embeds per
// document and floods Sentry (TAURI-RUST-52S: 8.7k events from a single
// misconfigured user). Bailing here skips the wasted request entirely,
// and the "API key not set" wording is demoted to a breadcrumb by the
// `ApiKeyMissing` classifier in
// `core::observability::expected_error_kind` instead of surfacing as a
// Sentry event. Scoped to Cohere on purpose: the OpenAI-compatible
// provider legitimately supports keyless local/custom endpoints, so it
// omits the header rather than bailing.
if self.api_key.trim().is_empty() {
anyhow::bail!(
"Cohere API key not set. Configure via the web UI or set the appropriate env var."
);
}

let url = format!("{}/v2/embed", self.base_url);

tracing::debug!(
Expand Down Expand Up @@ -253,6 +272,24 @@ mod tests {
assert!(p.embed(&[]).await.unwrap().is_empty());
}

/// Missing / whitespace-only API key bails before any HTTP request with the
/// classifiable "API key not set" wording (TAURI-RUST-52S). `with_base_url`
/// points at an address nothing is listening on, so the assertion that the
/// error is the key-guard message — not a connection error — proves no
/// request was attempted.
#[tokio::test]
async fn embed_missing_api_key_bails_without_request() {
for key in ["", " "] {
let p = CohereEmbedding::new(key, "embed-english-v3.0", 1024)
.with_base_url("http://127.0.0.1:1");
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}"
);
}
}

// ── 429 backoff tests ──────────────────────────────────────

use axum::{http::StatusCode, routing::post, Router};
Expand Down
64 changes: 64 additions & 0 deletions src/openhuman/embeddings/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,3 +414,67 @@ pub(crate) fn resolve_api_key(config: &Config, provider_name: &str) -> String {
.flatten()
.unwrap_or_default()
}

#[cfg(test)]
mod tests {
use super::*;
use tempfile::TempDir;

/// The seam the memory factory depends on (TAURI-RUST-52S fix): the three
/// `create_memory_with_local_ai` call sites resolve the user's stored BYO
/// embedding credential via `resolve_api_key` and thread it into the
/// provider. If this lookup silently returns "" for a configured key —
/// wrong cred slug, encryption mismatch, profile-store regression — the
/// memory pipeline reverts to sending an empty bearer and Cohere 401s on
/// every embed. Lock the round-trip: store under `embeddings:<slug>`, read
/// it back; an unrelated provider must stay empty (no cross-bleed).
#[test]
fn resolve_api_key_returns_stored_embeddings_credential() {
let tmp = TempDir::new().unwrap();
let mut config = Config::default();
config.config_path = tmp.path().join("config.toml");

// Nothing stored yet → empty (the empty-key guard's "" input).
assert_eq!(resolve_api_key(&config, "cohere"), "");

// Store a Cohere embeddings key exactly as `set_api_key` does.
AuthService::from_config(&config)
.store_provider_token(
"embeddings:cohere",
"default",
"sk-cohere-test",
HashMap::new(),
true,
)
.unwrap();

// Resolve returns it; a provider with no stored key stays empty.
assert_eq!(resolve_api_key(&config, "cohere"), "sk-cohere-test");
assert_eq!(resolve_api_key(&config, "voyage"), "");
}

/// `custom:<url>` providers must look up under the `embeddings:custom`
/// slug (the inline URL is not part of the credential key), mirroring the
/// slug normalization in `embed`/`set_api_key`.
#[test]
fn resolve_api_key_normalizes_custom_prefix_to_custom_slug() {
let tmp = TempDir::new().unwrap();
let mut config = Config::default();
config.config_path = tmp.path().join("config.toml");

AuthService::from_config(&config)
.store_provider_token(
"embeddings:custom",
"default",
"sk-custom-test",
HashMap::new(),
true,
)
.unwrap();

assert_eq!(
resolve_api_key(&config, "custom:http://localhost:1234"),
"sk-custom-test"
);
}
}
39 changes: 35 additions & 4 deletions src/openhuman/memory_store/factories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,10 @@ pub fn create_memory(
config: &MemoryConfig,
workspace_dir: &Path,
) -> anyhow::Result<Box<dyn Memory>> {
create_memory_full(config, &[], None, None, workspace_dir)
// No `Config` in scope here (tests + migration), so no credential store to
// read — pass an empty key. Callers that select a keyed BYO provider must
// use `create_memory_with_local_ai`, which resolves the stored credential.
create_memory_full(config, &[], None, None, "", workspace_dir)
}

/// Create a memory instance honouring the unified per-workload embedding
Expand All @@ -309,9 +312,17 @@ pub fn create_memory(
/// `None`. Used by top-level entry points (agent harness, channels runtime)
/// that have the full `Config` in scope. The local-AI opt-in flips the
/// embedder to Ollama when `Some`.
///
/// `embedding_api_key` is the user's stored credential for the selected BYO
/// embedding provider, resolved by the caller via
/// [`crate::openhuman::embeddings::resolve_api_key`] (empty string when none is
/// configured). It is threaded into the keyed providers (cohere/openai/voyage/
/// custom) so they authenticate instead of sending an empty bearer; cloud /
/// managed / ollama / none ignore it.
pub fn create_memory_with_local_ai(
memory: &MemoryConfig,
local_embedding_model: Option<&str>,
embedding_api_key: &str,
embedding_routes: &[EmbeddingRouteConfig],
storage_provider: Option<&StorageProviderConfig>,
workspace_dir: &Path,
Expand All @@ -321,6 +332,7 @@ pub fn create_memory_with_local_ai(
embedding_routes,
storage_provider,
local_embedding_model,
embedding_api_key,
workspace_dir,
)
}
Expand Down Expand Up @@ -370,6 +382,7 @@ fn create_memory_full(
_embedding_routes: &[EmbeddingRouteConfig],
_storage_provider: Option<&StorageProviderConfig>,
local_embedding_model: Option<&str>,
embedding_api_key: &str,
workspace_dir: &Path,
) -> anyhow::Result<Box<dyn Memory>> {
// 1. Resolve the intended provider from config.
Expand Down Expand Up @@ -413,11 +426,29 @@ fn create_memory_full(
(local_ai_opt_in={local_ai_opt_in} gate_triggered={gate_triggered})",
);

// 3. Create the embedding provider.
// 3. Create the embedding provider, threading the user's stored BYO
// credential. The keyless `create_embedding_provider` left the API key
// empty for *every* provider, so a user who selected Cohere — even with
// a valid key configured — sent an empty `Bearer ` and got a guaranteed
// 401 "no api key supplied" on every embed (TAURI-RUST-52S), and the
// same gap silently broke BYO OpenAI / Voyage memory embeddings.
// cloud/managed/ollama/none ignore the key; the keyed providers now
// actually receive it. `embedding_api_key` is "" when no credential is
// stored, which the per-provider guards reject fast. A `custom:<url>`
// provider keeps its inline endpoint (the factory's `custom:` arm strips
// the prefix), so `custom_endpoint` stays `None` here. The key is never
// logged — the warning carries only provider/model/dims.
let embedder: Arc<dyn EmbeddingProvider> = Arc::from(
embeddings::create_embedding_provider(&provider, &model, dims).inspect_err(|err| {
embeddings::create_embedding_provider_with_credentials(
&provider,
&model,
dims,
embedding_api_key,
None,
)
.inspect_err(|err| {
log::warn!(
"[memory::factory] create_embedding_provider failed provider={provider} model={model} dims={dims}: {err}",
"[memory::factory] create_embedding_provider_with_credentials failed provider={provider} model={model} dims={dims}: {err}",
);
})?,
);
Expand Down
3 changes: 3 additions & 0 deletions src/openhuman/runtime_node/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,14 @@ fn build_runtime_tools(config: &Config) -> Result<Vec<Box<dyn Tool>>, String> {
.map_err(|e| e.to_string())?;
let runtime: Arc<dyn RuntimeAdapter> = Arc::new(NativeRuntime::new());
let local_embedding = config.workload_local_model("embeddings");
let embedding_api_key =
crate::openhuman::embeddings::resolve_api_key(config, &config.memory.embedding_provider);
trace!("[runtime_node::ops] build_runtime_tools: create_memory_with_local_ai");
let memory: Arc<dyn Memory> = Arc::from(
crate::openhuman::memory_store::create_memory_with_local_ai(
&config.memory,
local_embedding.as_deref(),
&embedding_api_key,
&config.embedding_routes,
Some(&config.storage.provider.config),
&config.workspace_dir,
Expand Down
Loading