From ca379c7f9fb539058fc8239b21fc2f0c0f27e465 Mon Sep 17 00:00:00 2001 From: YellowSnnowmann <167776381+YellowSnnowmann@users.noreply.github.com> Date: Wed, 20 May 2026 02:47:35 +0530 Subject: [PATCH] fix(config): guard env overrides during config load (#2201) --- src/openhuman/config/schema/load.rs | 16 +++++++++--- src/openhuman/config/schema/load_tests.rs | 30 +++++++++++++++++++---- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/src/openhuman/config/schema/load.rs b/src/openhuman/config/schema/load.rs index 3b7feb64f..7aba329a5 100644 --- a/src/openhuman/config/schema/load.rs +++ b/src/openhuman/config/schema/load.rs @@ -992,9 +992,19 @@ impl Config { /// with a [`HashMapEnv`] (see tests) without requiring the /// `TEST_ENV_LOCK` or tainting sibling tests. pub(crate) fn apply_env_overlay_with(&mut self, env: &E) { - if let Some(model) = env.get_any(&["OPENHUMAN_MODEL", "MODEL"]) { - if !model.is_empty() { - self.default_model = Some(model); + // Only the namespaced `OPENHUMAN_MODEL` is honoured. The bare `MODEL` + // env var used to be accepted as an alias but collides with vendor + // asset-tag env vars (e.g. Dell OptiPlex sets `MODEL=7080`), which + // silently clobbered the LLM model and 400'd every backend call + // (Sentry OPENHUMAN-TAURI-J8). + if let Some(model) = env.get("OPENHUMAN_MODEL") { + // Trim before checking so `OPENHUMAN_MODEL=" "` (a common + // shape from shells that pass through an unset-but-declared + // variable) doesn't clobber the configured default with a + // non-usable value. + let trimmed = model.trim(); + if !trimmed.is_empty() { + self.default_model = Some(trimmed.to_string()); } } diff --git a/src/openhuman/config/schema/load_tests.rs b/src/openhuman/config/schema/load_tests.rs index 7e9741387..6fac4b408 100644 --- a/src/openhuman/config/schema/load_tests.rs +++ b/src/openhuman/config/schema/load_tests.rs @@ -423,8 +423,9 @@ impl EnvLookup for HashMapEnv { } #[test] -fn env_overlay_model_prefers_openhuman_over_alias() { - // Both set → OPENHUMAN_MODEL wins. +fn env_overlay_model_only_honours_namespaced_var() { + // Both set → OPENHUMAN_MODEL wins; bare MODEL is ignored even when + // OPENHUMAN_MODEL is absent. let env = HashMapEnv::new() .with("OPENHUMAN_MODEL", "specific-v2") .with("MODEL", "alias-fallback"); @@ -432,11 +433,30 @@ fn env_overlay_model_prefers_openhuman_over_alias() { cfg.apply_env_overlay_with(&env); assert_eq!(cfg.default_model.as_deref(), Some("specific-v2")); - // Only alias set → alias wins. - let env = HashMapEnv::new().with("MODEL", "alias-only"); + // Only bare MODEL set → must NOT clobber default_model. Vendor + // asset-tag env vars (e.g. Dell OptiPlex `MODEL=7080`) would otherwise + // hijack the LLM model name and 400 every backend call + // (Sentry OPENHUMAN-TAURI-J8). + let env = HashMapEnv::new().with("MODEL", "7080"); let mut cfg = Config::default(); + let original = cfg.default_model.clone(); cfg.apply_env_overlay_with(&env); - assert_eq!(cfg.default_model.as_deref(), Some("alias-only")); + assert_eq!( + cfg.default_model, original, + "bare MODEL env var must not override default_model" + ); + + // Whitespace-only OPENHUMAN_MODEL must not clobber either. Some + // shells/CI runners pass an unset-but-declared env var through as + // `" "`, which `is_empty()` alone wouldn't reject. + let env = HashMapEnv::new().with("OPENHUMAN_MODEL", " "); + let mut cfg = Config::default(); + let original = cfg.default_model.clone(); + cfg.apply_env_overlay_with(&env); + assert_eq!( + cfg.default_model, original, + "whitespace-only OPENHUMAN_MODEL must not clobber default_model" + ); } #[test]