diff --git a/src/embed/medulla.rs b/src/embed/medulla.rs index 6b8965630..58cc2a695 100644 --- a/src/embed/medulla.rs +++ b/src/embed/medulla.rs @@ -162,7 +162,6 @@ impl Medulla<'_> { #[cfg(test)] mod tests { - use super::*; use crate::openhuman::medulla::all_medulla_registered_controllers; /// Every method this facade dispatches must name a registered controller. diff --git a/src/openhuman/medulla/ops.rs b/src/openhuman/medulla/ops.rs index 11108cde6..23954a180 100644 --- a/src/openhuman/medulla/ops.rs +++ b/src/openhuman/medulla/ops.rs @@ -234,11 +234,44 @@ fn call(result: Result, method: &'static str) -> Result, + } + + impl EnvGuard { + fn remove(key: &'static str) -> Self { + let prev = std::env::var(key).ok(); + // SAFETY: caller holds ENV_LOCK guard. + unsafe { std::env::remove_var(key) }; + Self { key, prev } + } + } + + impl Drop for EnvGuard { + fn drop(&mut self) { + match &self.prev { + // SAFETY: caller's ENV_LOCK guard is still alive during drop. + Some(v) => unsafe { std::env::set_var(self.key, v) }, + None => unsafe { std::env::remove_var(self.key) }, + } + } + } + + /// Serialize env mutation: tests modify process-globals. + static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + #[test] fn not_configured_encodes_an_expected_user_state_envelope() { let config = Config::default(); // No api_url and no env override, so resolution fails on base URL. - std::env::remove_var(resolve::MEDULLA_BASE_URL_ENV); + // Isolate environment to ensure BACKEND_URL and VITE_BACKEND_URL do not + // provide a fallback through effective_backend_api_url. + let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let _env_medulla = EnvGuard::remove(resolve::MEDULLA_BASE_URL_ENV); + let _env_backend = EnvGuard::remove("BACKEND_URL"); + let _env_vite = EnvGuard::remove("VITE_BACKEND_URL"); let err = resolved(&config).expect_err("must not resolve"); let decoded = StructuredRpcError::decode(&err).expect("structured envelope"); assert!( @@ -301,7 +334,12 @@ mod tests { #[tokio::test] async fn status_reports_unconfigured_rather_than_failing() { - std::env::remove_var(resolve::MEDULLA_BASE_URL_ENV); + // Isolate environment to ensure BACKEND_URL and VITE_BACKEND_URL do not + // provide a fallback through effective_backend_api_url. + let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let _env_medulla = EnvGuard::remove(resolve::MEDULLA_BASE_URL_ENV); + let _env_backend = EnvGuard::remove("BACKEND_URL"); + let _env_vite = EnvGuard::remove("VITE_BACKEND_URL"); let out = status(&Config::default()) .await .expect("status never errors"); diff --git a/src/openhuman/medulla/resolve.rs b/src/openhuman/medulla/resolve.rs index 21e3c82b2..6cd8bcbc8 100644 --- a/src/openhuman/medulla/resolve.rs +++ b/src/openhuman/medulla/resolve.rs @@ -11,8 +11,14 @@ //! //! `OPENHUMAN_MEDULLA_BASE_URL` remains as an override for pointing a //! development host at a different Medulla deployment than its OpenHuman one. -//! Unset — the normal case — everything resolves from `api_url`. +//! Unset — the normal case — everything resolves through +//! [`effective_backend_api_url`], the same chain every other hosted-backend +//! call uses: `api_url`, then the `BACKEND_URL` keys, then the prod or staging +//! default selected by `OPENHUMAN_APP_ENV`. Resolving from the raw `api_url` +//! instead would leave Medulla as the one backend surface with no default, +//! reporting itself unconfigured on an install where every other call works. +use crate::api::config::effective_backend_api_url; use crate::openhuman::config::Config; use crate::openhuman::credentials::session_support::get_session_token; @@ -55,15 +61,40 @@ impl NotConfigured { /// The configured base URL, if any. /// -/// Precedence: `OPENHUMAN_MEDULLA_BASE_URL`, then `config.api_url`. Empty or -/// whitespace-only values count as unset, so an exported-but-blank env var does -/// not shadow a working config value. +/// Precedence: `OPENHUMAN_MEDULLA_BASE_URL`, then whatever every other +/// hosted-backend call resolves to — [`effective_backend_api_url`], which +/// applies `config.api_url`, the `BACKEND_URL` env/compile-time keys, and +/// finally the environment-aware default (prod or staging by +/// `OPENHUMAN_APP_ENV`). +/// +/// Reading `config.api_url` directly, as this used to, made Medulla the one +/// hosted-backend surface with no default: an install that had never written an +/// explicit `api_url` — the normal case, since every other call falls through +/// to the default — reported "no Medulla backend configured" while auth, +/// billing and integrations all worked. Same deployment, so it resolves the +/// same way. It also inherits the local-AI guard for free: a user whose +/// `api_url` points at Ollama gets the hosted backend here rather than a +/// Medulla client aimed at a model runner. +/// +/// Empty or whitespace-only values count as unset, so an exported-but-blank env +/// var does not shadow a working config value. pub fn base_url(config: &Config) -> Option { let from_env = std::env::var(MEDULLA_BASE_URL_ENV) .ok() .filter(|v| !v.trim().is_empty()); + + let source_ident = if from_env.is_some() { + "env_override" + } else { + "effective_backend_api_url" + }; + log::debug!( + "[medulla] base_url source={} (URL redacted for security)", + source_ident + ); + from_env - .or_else(|| config.api_url.clone()) + .or_else(|| Some(effective_backend_api_url(&config.api_url))) .map(|v| v.trim().trim_end_matches('/').to_string()) .filter(|v| !v.is_empty()) } @@ -99,6 +130,38 @@ mod tests { /// Serialize env mutation: `base_url` reads a process-global. static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + /// RAII guard to snapshot and restore a process-global environment variable. + struct EnvGuard { + key: &'static str, + prev: Option, + } + + impl EnvGuard { + fn set(key: &'static str, val: &str) -> Self { + let prev = std::env::var(key).ok(); + // SAFETY: caller holds ENV_LOCK guard. + unsafe { std::env::set_var(key, val) }; + Self { key, prev } + } + + fn remove(key: &'static str) -> Self { + let prev = std::env::var(key).ok(); + // SAFETY: caller holds ENV_LOCK guard. + unsafe { std::env::remove_var(key) }; + Self { key, prev } + } + } + + impl Drop for EnvGuard { + fn drop(&mut self) { + match &self.prev { + // SAFETY: caller's ENV_LOCK guard is still alive during drop. + Some(v) => unsafe { std::env::set_var(self.key, v) }, + None => unsafe { std::env::remove_var(self.key) }, + } + } + } + fn config_with_api_url(url: Option<&str>) -> Config { let mut config = Config::default(); config.api_url = url.map(str::to_string); @@ -108,16 +171,15 @@ mod tests { #[test] fn env_override_wins_over_api_url() { let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); - std::env::set_var(MEDULLA_BASE_URL_ENV, "https://medulla.example"); + let _env = EnvGuard::set(MEDULLA_BASE_URL_ENV, "https://medulla.example"); let resolved = base_url(&config_with_api_url(Some("https://api.example"))); - std::env::remove_var(MEDULLA_BASE_URL_ENV); assert_eq!(resolved.as_deref(), Some("https://medulla.example")); } #[test] fn falls_back_to_api_url_when_env_unset() { let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); - std::env::remove_var(MEDULLA_BASE_URL_ENV); + let _env = EnvGuard::remove(MEDULLA_BASE_URL_ENV); let resolved = base_url(&config_with_api_url(Some("https://api.example/"))); assert_eq!(resolved.as_deref(), Some("https://api.example")); } @@ -127,25 +189,50 @@ mod tests { // An exported-but-empty var is a common shell accident; treating it as // "configured" would break an otherwise-working setup. let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); - std::env::set_var(MEDULLA_BASE_URL_ENV, " "); + let _env = EnvGuard::set(MEDULLA_BASE_URL_ENV, " "); let resolved = base_url(&config_with_api_url(Some("https://api.example"))); - std::env::remove_var(MEDULLA_BASE_URL_ENV); assert_eq!(resolved.as_deref(), Some("https://api.example")); } #[test] - fn none_when_neither_is_set() { + fn an_unconfigured_install_falls_back_to_the_hosted_backend() { + // Medulla and the OpenHuman backend are one deployment, so an install + // that never wrote an explicit `api_url` — the normal case — must reach + // the same host every other backend call defaults to, not report itself + // unconfigured while auth and billing work. let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); - std::env::remove_var(MEDULLA_BASE_URL_ENV); - assert_eq!(base_url(&config_with_api_url(None)), None); + let _env_medulla = EnvGuard::remove(MEDULLA_BASE_URL_ENV); + let _env_backend = EnvGuard::remove("BACKEND_URL"); + let _env_vite = EnvGuard::remove("VITE_BACKEND_URL"); + assert_eq!( + base_url(&config_with_api_url(None)), + Some(crate::api::config::effective_backend_api_url(&None)) + ); + } + + #[test] + fn a_local_model_runner_url_does_not_become_the_medulla_host() { + // `api_url` doubles as the inference endpoint. Pointing it at Ollama + // must not aim the Medulla client at a model runner that 404s every + // path it speaks — the same guard every other backend call gets. + let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let _env_medulla = EnvGuard::remove(MEDULLA_BASE_URL_ENV); + let _env_backend = EnvGuard::remove("BACKEND_URL"); + let _env_vite = EnvGuard::remove("VITE_BACKEND_URL"); + let resolved = base_url(&config_with_api_url(Some("http://localhost:11434"))); + // Assert that the local model runner URL does not become the Medulla host; + // instead, it falls back to the effective backend URL (same as all other backend calls). + assert_eq!( + resolved, + Some(crate::api::config::effective_backend_api_url(&None)) + ); } #[test] fn trailing_slashes_are_trimmed_so_paths_do_not_double_up() { let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); - std::env::set_var(MEDULLA_BASE_URL_ENV, "https://medulla.example///"); + let _env = EnvGuard::set(MEDULLA_BASE_URL_ENV, "https://medulla.example///"); let resolved = base_url(&config_with_api_url(None)); - std::env::remove_var(MEDULLA_BASE_URL_ENV); assert_eq!(resolved.as_deref(), Some("https://medulla.example")); } diff --git a/src/openhuman/people/tests.rs b/src/openhuman/people/tests.rs index 84e9659b5..30732016b 100644 --- a/src/openhuman/people/tests.rs +++ b/src/openhuman/people/tests.rs @@ -4,6 +4,7 @@ use std::sync::Arc; use chrono::Utc; +#[cfg(not(target_os = "macos"))] use crate::openhuman::people::address_book; use crate::openhuman::people::resolver::HandleResolver; use crate::openhuman::people::store::PeopleStore;