From 934cbefe30068391e5aaaa71eb54ddee332712f4 Mon Sep 17 00:00:00 2001 From: shanu Date: Wed, 12 Aug 2026 14:02:58 +0530 Subject: [PATCH 1/2] fix(embeddings): scope the memory client's default embedder to config credentials MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The namespace-document ingest tier builds its inline embedder through the keyless `default_embedding_provider()`, which hardcodes `(openhuman_dir=None, secrets_encrypt=true)`. That resolves the managed session from `default_state_dir()` with encryption forced on — a scope that diverges from where sign-in actually wrote the `app-session` token whenever the config disables secret encryption or roots the workspace/user elsewhere than the process default. "Test connection" builds through the config-aware `create_embedding_provider_with_config` (managed_credential_scope) and passes, while every ingest embed then fails with "No backend session" and the document persists vector-less — so a signed-in user's ingested memory is silently unsearchable. #5427 fixed the config-aware path but left this keyless one. Add `default_embedding_provider_with_config(config)` — routes managed construction through `managed_credential_scope`, the same (state_dir, encrypt) scope Test connection uses — and call it from the two config-holding callers: the memory-client embedding-host seam (`OpenHumanEmbeddingHost::default_embedding_provider`) and the agent-experience store. The keyless variant stays for genuinely config-less callers, doc-warned as a best-effort fallback. Adds an e2e binding test (axum mock captures the bearer) proving the config-scoped default embedder authenticates with the app-session token sign-in stored. --- src/openhuman/agent/experience/ops.rs | 5 +- src/openhuman/inference/embeddings/factory.rs | 127 ++++++++++++++++++ src/openhuman/inference/embeddings/mod.rs | 2 +- src/openhuman/memory/host_impls.rs | 10 +- 4 files changed, 141 insertions(+), 3 deletions(-) diff --git a/src/openhuman/agent/experience/ops.rs b/src/openhuman/agent/experience/ops.rs index 6f6b7c38a5..ff1d4e5169 100644 --- a/src/openhuman/agent/experience/ops.rs +++ b/src/openhuman/agent/experience/ops.rs @@ -103,7 +103,10 @@ async fn open_store_in_subdir( let memory = crate::openhuman::memory::store::UnifiedMemory::new_with_memory_dir( &config.workspace_dir, memory_subdir, - crate::openhuman::inference::embeddings::default_embedding_provider(), + // Config-scoped so the experience store's managed embedder reads the + // signed-in user's session, not the keyless `default_state_dir()` + // scope (#5501). + crate::openhuman::inference::embeddings::default_embedding_provider_with_config(config), config.memory.sqlite_open_timeout_secs, ) .map_err(|e| format!("open agent experience store '{memory_subdir}': {e:#}"))?; diff --git a/src/openhuman/inference/embeddings/factory.rs b/src/openhuman/inference/embeddings/factory.rs index d72795fa88..0020713f93 100644 --- a/src/openhuman/inference/embeddings/factory.rs +++ b/src/openhuman/inference/embeddings/factory.rs @@ -226,11 +226,50 @@ fn managed_credential_scope(config: &Config) -> (Option, bool) { (Some(state_dir_from_config(config)), config.secrets.encrypt) } +/// Returns the default embedding provider — cloud (OpenHuman backend, Voyage) — +/// scoped to `config`'s credential store. +/// +/// This is the [`default_embedding_provider`] every caller that holds a +/// `&Config` must use. It threads the caller's real credential-store location +/// ([`managed_credential_scope`]) into the cloud embedder's bearer resolver — the +/// same `(state_dir, encrypt)` pair sign-in wrote the `app-session` token to — so +/// a signed-in user's ingest/seal embeds read the session they actually have. +/// +/// The config-less [`default_embedding_provider`] hardcodes `(None, true)` and so +/// resolves `default_state_dir()` with encryption forced on; that only lands on +/// the right store for a default-root, encrypted, single-user install. Routing +/// the memory client's inline embedder through the keyless constructor is what +/// made a signed-in user's ingested documents persist vector-less — "Test +/// connection" passed (config-scoped) while the embed batch silently failed +/// (keyless scope) — #5501. +pub fn default_embedding_provider_with_config(config: &Config) -> Arc { + let (state_dir, encrypt_secrets) = managed_credential_scope(config); + // Never log `state_dir`: the user-scoped path embeds the OS username and/or + // `users/` (PII). Log only the non-identifying flag. + log::debug!( + "[embeddings::factory] building default managed embedder from config credential scope (encrypt={encrypt_secrets})" + ); + Arc::new(OpenHumanCloudEmbedding::new( + None, + state_dir, + encrypt_secrets, + DEFAULT_CLOUD_EMBEDDING_MODEL, + DEFAULT_CLOUD_EMBEDDING_DIMENSIONS, + )) +} + /// Returns the default embedding provider — cloud (OpenHuman backend, Voyage). /// /// The cloud embedder lazily resolves the session JWT and API URL on each /// call, so this can be constructed before login completes; the first /// `embed()` will fail with a clear message if the user is unauthenticated. +/// +/// **Keyless — prefer [`default_embedding_provider_with_config`].** This hardcodes +/// `(None, true)` for the credential scope, resolving `default_state_dir()` +/// (`~/.openhuman` root, or `users/` post-#5427) with encryption forced +/// on. That reads the wrong store whenever the caller's config disables secret +/// encryption or roots the workspace/user elsewhere than the process default +/// (#5356 / #5501). Only callers that genuinely hold no `&Config` should use it. pub fn default_embedding_provider() -> Arc { Arc::new(OpenHumanCloudEmbedding::new( None, @@ -472,4 +511,92 @@ mod tests { "managed provider must authenticate with the token from the config scope" ); } + + /// #5501 regression: the memory client's inline embedder is built through + /// [`default_embedding_provider_with_config`], which MUST authenticate with + /// the `app-session` token under the *config* credential scope — the same + /// scope "Test connection" uses. The pre-fix keyless + /// [`default_embedding_provider`] resolved `default_state_dir()` with + /// encryption forced on, so a signed-in user's ingested documents embedded + /// with "No backend session" and persisted vector-less while the connection + /// test still passed. A local mock stands in for the cloud backend (no + /// network) and captures the bearer it receives; a regression back to the + /// keyless constructor resolves a scope with no token and never reaches it. + #[tokio::test] + async fn default_provider_with_config_authenticates_with_config_scoped_token() { + use std::sync::{Arc, Mutex}; + + use axum::{extract::State, http::HeaderMap, routing::post, Json, Router}; + + #[derive(Clone, Default)] + struct Captured { + auth: Arc>>, + } + let captured = Captured::default(); + let app = Router::new() + .route( + "/openai/v1/embeddings", + post( + |State(cap): State, + headers: HeaderMap, + Json(_body): Json| async move { + *cap.auth.lock().unwrap() = headers + .get("authorization") + .and_then(|v| v.to_str().ok()) + .map(str::to_owned); + // The embedder validates returned dims against the + // requested default, so the mock must echo that width. + let embedding = vec![0.1_f32; DEFAULT_CLOUD_EMBEDDING_DIMENSIONS]; + Json(serde_json::json!({ + "object": "list", + "data": [{ "object": "embedding", "index": 0, "embedding": embedding }], + "model": DEFAULT_CLOUD_EMBEDDING_MODEL, + })) + }, + ), + ) + .with_state(captured.clone()); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let base = format!("http://127.0.0.1:{}", listener.local_addr().unwrap().port()); + tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + + // The app-session token lives ONLY under the config credential scope. + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + AuthService::from_config(&config) + .store_provider_token( + APP_SESSION_PROVIDER, + "default", + "sess-default-5501", + HashMap::new(), + true, + ) + .unwrap(); + + let provider = { + let _env_guard = crate::api::config::backend_env_test_lock(); + let prev = std::env::var("BACKEND_URL").ok(); + std::env::set_var("BACKEND_URL", &base); + let built = default_embedding_provider_with_config(&config); + match prev { + Some(v) => std::env::set_var("BACKEND_URL", v), + None => std::env::remove_var("BACKEND_URL"), + } + built + }; + + let vectors = provider + .embed(&["binding probe"]) + .await + .expect("config-scoped default embedder must resolve the app-session token and embed"); + assert_eq!( + vectors.first().map(|v| v.len()).unwrap_or(0), + DEFAULT_CLOUD_EMBEDDING_DIMENSIONS + ); + assert_eq!( + captured.auth.lock().unwrap().as_deref(), + Some("Bearer sess-default-5501"), + "the memory client's default embedder must authenticate with the config-scoped token" + ); + } } diff --git a/src/openhuman/inference/embeddings/mod.rs b/src/openhuman/inference/embeddings/mod.rs index e6571f2cf0..6bcab4e4c7 100644 --- a/src/openhuman/inference/embeddings/mod.rs +++ b/src/openhuman/inference/embeddings/mod.rs @@ -44,7 +44,7 @@ pub use cloud::{ pub use factory::{ create_embedding_provider, create_embedding_provider_with_config, create_embedding_provider_with_credentials, default_embedding_provider, - default_local_embedding_provider, + default_embedding_provider_with_config, default_local_embedding_provider, }; // `pub(crate)` helper — reused by the memory-tree OpenAI-compat adapter to gate // configs whose dimension the fixed-1024 tree can't store (#4056). Not part of diff --git a/src/openhuman/memory/host_impls.rs b/src/openhuman/memory/host_impls.rs index 1e888af81a..0bedbdd282 100644 --- a/src/openhuman/memory/host_impls.rs +++ b/src/openhuman/memory/host_impls.rs @@ -64,7 +64,15 @@ impl EmbeddingHost for OpenHumanEmbeddingHost { } fn default_embedding_provider(&self) -> Arc { - crate::openhuman::inference::embeddings::default_embedding_provider() + // Scope the managed embedder to THIS host's config credential store, not + // the keyless `default_state_dir()` hardcode. The memory client caches + // this provider for the process lifetime, so a keyless scope that misses + // the signed-in user's `app-session` token makes every ingested + // document persist vector-less while "Test connection" (config-scoped) + // still passes — #5501. + crate::openhuman::inference::embeddings::default_embedding_provider_with_config( + &self.config, + ) } fn create_embedding_provider_with_credentials( From 6f81e82932da7a0dbf73089d8b6028014bca39f6 Mon Sep 17 00:00:00 2001 From: shanu Date: Thu, 13 Aug 2026 15:41:05 +0530 Subject: [PATCH 2/2] test(embeddings): unbreak stale effective-provider test after tinymemory v1.0.1 Same pre-existing broken-main as folded into #5528: the inference coverage lane runs the whole openhuman::inference namespace, and tinymemory v1.0.1 changed the effective-embedder ladder (local Ollama now resolves from an explicit memory_tree.embedding_endpoint override or the unified workload_local_model setting, not the embeddings_provider string alone). The submodule bump landed on main without updating this test. Drive the deterministic explicit-endpoint rung so the test keeps its #5402 assertion. Not caused by this PR; fails identically on plain main. --- src/openhuman/inference/embeddings/rpc.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/openhuman/inference/embeddings/rpc.rs b/src/openhuman/inference/embeddings/rpc.rs index e3513ee10a..3171df58db 100644 --- a/src/openhuman/inference/embeddings/rpc.rs +++ b/src/openhuman/inference/embeddings/rpc.rs @@ -1104,8 +1104,18 @@ mod tests { config.memory.embedding_provider = "cloud".to_string(); // A managed session exists, so the ladder would resolve to cloud … std::fs::write(tmp.path().join("auth-profiles.json"), "{}").unwrap(); - // … except the unified workload setting routes embeddings to Ollama. + // … except a local Ollama route wins. As of tinymemory v1.0.1 the + // effective-embedder ladder no longer treats the `embeddings_provider` + // string alone as authoritative for local routing — local Ollama is + // resolved from an explicit `memory_tree.embedding_endpoint` override or + // the unified `workload_local_model` setting. Drive the explicit + // endpoint rung here: it resolves deterministically without an installed + // embedding host, and still exercises the point of the test — that + // `provider` (the picker) stays `cloud` while `effective_provider` + // reports the local route that bills nothing (#5402). config.embeddings_provider = Some("ollama:all-minilm:latest".into()); + config.memory_tree.embedding_endpoint = Some("http://localhost:11434".into()); + config.memory_tree.embedding_model = Some("all-minilm".into()); let out = get_settings(&config) .await