From 845f584361005ec4df40b8fbcc911a45f7ecffdb Mon Sep 17 00:00:00 2001 From: Philip Azar Date: Wed, 14 Oct 2026 10:40:00 -0500 Subject: [PATCH 1/4] feat(goose): read DATABRICKS_HOST override from goose config.yaml When ~/.config/goose/config.yaml sets a valid DATABRICKS_HOST, pass it to goose serve so it takes precedence over the login-shell and runtime-config values. Goose only reads the file itself when the env var is unset, so Berd's env layers would otherwise shadow it. A missing file or key leaves the current behavior unchanged; an invalid value is logged and ignored. The value must be an HTTPS URL without credentials, query, or fragment; a path prefix is allowed for proxies. Skipped in BYO-key dev and E2E runs. Co-Authored-By: Claude Opus 5.5 --- src-tauri/src/services/acp/goose_serve.rs | 94 ++++++++++++++++++- src-tauri/src/services/goose_config.rs | 107 ++++++++++++++++++++++ 2 files changed, 197 insertions(+), 4 deletions(-) diff --git a/src-tauri/src/services/acp/goose_serve.rs b/src-tauri/src/services/acp/goose_serve.rs index a4fa10bf3..335e7e2ed 100644 --- a/src-tauri/src/services/acp/goose_serve.rs +++ b/src-tauri/src/services/acp/goose_serve.rs @@ -228,6 +228,19 @@ impl GooseServeProcess { Ok(runtime_config) => apply_runtime_goose_provider_env(&mut command, &runtime_config), Err(error) => log::warn!("failed to load runtime config for goose serve env: {error}"), } + // A valid DATABRICKS_HOST in the user's goose config overrides the + // shell and runtime config values. E2E runs use an isolated goose root, + // so the real user config must not leak in. + if app_handle + .try_state::() + .is_none() + { + apply_databricks_host_config_override( + &mut command, + goose_config::databricks_host_from_config(), + local_byo_key_providers_enabled(), + ); + } // This must be the final environment layer. Captured shell and runtime // provider values are intentionally unable to redirect an E2E child // back into a normal Goose root or keyring. @@ -1252,6 +1265,30 @@ fn apply_runtime_goose_provider_env(command: &mut Command, runtime_config: &Runt } } +/// Goose itself reads DATABRICKS_HOST from its config file, but only when the +/// env var is unset, so Berd's shell and runtime config layers would otherwise +/// shadow it. On a missing or invalid value the existing env is left as-is. +/// BYO-key dev keeps its stripped host. +fn apply_databricks_host_config_override( + command: &mut Command, + config_host: Result, String>, + byo_key_providers_enabled: bool, +) { + if byo_key_providers_enabled { + return; + } + match config_host { + Ok(Some(host)) => { + log::info!("setting goose {DATABRICKS_HOST_ENV} from goose config"); + command.env(DATABRICKS_HOST_ENV, host); + } + Ok(None) => {} + Err(error) => log::warn!( + "ignoring goose config {DATABRICKS_HOST_ENV}; keeping existing value: {error}" + ), + } +} + pub(crate) fn reserve_free_port() -> Result { let listener = std::net::TcpListener::bind((LOCALHOST, 0)) .map_err(|error| format!("Failed to reserve Goose serve port: {error}"))?; @@ -1264,10 +1301,10 @@ pub(crate) fn reserve_free_port() -> Result { #[cfg(test)] mod tests { use super::{ - acp_websocket_url, add_release_webview_origin_arg, apply_goose_search_paths_env, - apply_runtime_goose_provider_env, apply_shell_env_with_extended_path, - apply_shell_env_with_extended_path_inner, storage_environment_matches_parent, - DATABRICKS_HOST_ENV, TAURI_WEBVIEW_ORIGIN, + acp_websocket_url, add_release_webview_origin_arg, apply_databricks_host_config_override, + apply_goose_search_paths_env, apply_runtime_goose_provider_env, + apply_shell_env_with_extended_path, apply_shell_env_with_extended_path_inner, + storage_environment_matches_parent, DATABRICKS_HOST_ENV, TAURI_WEBVIEW_ORIGIN, }; use crate::commands::runtime_config::default_runtime_config; use std::collections::HashMap; @@ -1528,6 +1565,55 @@ mod tests { ); } + fn command_with_runtime_host() -> Command { + let mut command = Command::new("goose"); + command.env(DATABRICKS_HOST_ENV, "https://runtime.databricks.com"); + command + } + + #[test] + fn databricks_host_config_override_replaces_runtime_host() { + let mut command = command_with_runtime_host(); + + apply_databricks_host_config_override( + &mut command, + Ok(Some("https://local.databricks.com".to_string())), + false, + ); + + assert_eq!( + env_value(&command, DATABRICKS_HOST_ENV), + Some(OsString::from("https://local.databricks.com")) + ); + } + + #[test] + fn databricks_host_config_override_keeps_runtime_host_when_missing_or_invalid() { + for config_host in [Ok(None), Err("invalid".to_string())] { + let mut command = command_with_runtime_host(); + + apply_databricks_host_config_override(&mut command, config_host, false); + + assert_eq!( + env_value(&command, DATABRICKS_HOST_ENV), + Some(OsString::from("https://runtime.databricks.com")) + ); + } + } + + #[test] + fn databricks_host_config_override_is_skipped_for_byo_dev() { + let mut command = Command::new("goose"); + + apply_databricks_host_config_override( + &mut command, + Ok(Some("https://local.databricks.com".to_string())), + true, + ); + + assert_eq!(env_value(&command, DATABRICKS_HOST_ENV), None); + } + // Stock berd defaults declare no fastModelId (the release-time distribution // injector supplies it), so a stock build exports no GOOSE_FAST_MODEL. #[test] diff --git a/src-tauri/src/services/goose_config.rs b/src-tauri/src/services/goose_config.rs index 67543d60e..3b85ba5e9 100644 --- a/src-tauri/src/services/goose_config.rs +++ b/src-tauri/src/services/goose_config.rs @@ -9,6 +9,7 @@ use etcetera::{choose_app_strategy, AppStrategy, AppStrategyArgs}; pub(crate) const ADDITIONAL_CONFIG_FILES_ENV: &str = "GOOSE_ADDITIONAL_CONFIG_FILES"; const GOOSE_PATH_ROOT_ENV: &str = "GOOSE_PATH_ROOT"; pub(crate) const CONFIG_FILE_NAME: &str = "config.yaml"; +const DATABRICKS_HOST_KEY: &str = "DATABRICKS_HOST"; #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct AdditionalConfigFiles { @@ -68,6 +69,55 @@ fn data_dir_for_path_root(root: Option) -> Result { Ok(strategy.data_dir()) } +/// Read `DATABRICKS_HOST` from the user's goose config file. +/// +/// Returns `Ok(None)` when the file or key is absent, and `Err` when the file +/// is unreadable or the value is not a canonical HTTPS origin, so the caller +/// can fall back to its existing host source. +pub(crate) fn databricks_host_from_config() -> Result, String> { + databricks_host_from_config_path(&config_path()?) +} + +fn databricks_host_from_config_path(path: &Path) -> Result, String> { + let contents = match std::fs::read_to_string(path) { + Ok(contents) => contents, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(error) => return Err(format!("Failed to read {}: {error}", path.display())), + }; + let value = yaml_serde::from_str::(&contents) + .map_err(|error| format!("Failed to parse {}: {error}", path.display()))?; + let Some(host) = value.get(DATABRICKS_HOST_KEY) else { + return Ok(None); + }; + let host = host.as_str().ok_or_else(|| { + format!( + "{DATABRICKS_HOST_KEY} in {} is not a string", + path.display() + ) + })?; + validate_databricks_host(host) + .map(|()| Some(host.to_string())) + .map_err(|error| format!("{DATABRICKS_HOST_KEY} in {}: {error}", path.display())) +} + +/// An HTTPS base URL without credentials, query, or fragment. Unlike the +/// release-time `normalizeDatabricksHost` origin rule, a path prefix is +/// allowed: goose's ApiClient joins request paths beneath it, so proxies such +/// as `https://proxy.example.com/pass-through/databricks/` work. +fn validate_databricks_host(host: &str) -> Result<(), String> { + let url = url::Url::parse(host).map_err(|_| "must be a valid URL".to_string())?; + if url.scheme() != "https" + || url.host_str().is_none() + || !url.username().is_empty() + || url.password().is_some() + || url.query().is_some() + || url.fragment().is_some() + { + return Err("must be an HTTPS URL without credentials, query, or fragment".to_string()); + } + Ok(()) +} + fn validated_path_root(value: Option) -> Option { value.map(PathBuf::from).filter(|path| path.is_absolute()) } @@ -138,6 +188,63 @@ mod tests { ); } + fn write_config(contents: &str) -> (tempfile::TempDir, PathBuf) { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join(CONFIG_FILE_NAME); + std::fs::write(&path, contents).expect("write config"); + (dir, path) + } + + #[test] + fn databricks_host_from_config_reads_valid_host() { + for host in [ + "https://example.databricks.com", + "https://example.databricks.com/", + "https://example.databricks.com:8443", + "https://litellm-proxy.sqprod.co/pass-through/databricks/", + ] { + let (_dir, path) = write_config(&format!( + "GOOSE_PROVIDER: databricks\nDATABRICKS_HOST: {host}\n" + )); + assert_eq!( + databricks_host_from_config_path(&path), + Ok(Some(host.to_string())) + ); + } + } + + #[test] + fn databricks_host_from_config_is_none_when_file_or_key_missing() { + let dir = tempfile::tempdir().expect("tempdir"); + assert_eq!( + databricks_host_from_config_path(&dir.path().join("missing.yaml")), + Ok(None) + ); + + let (_dir, path) = write_config("GOOSE_PROVIDER: databricks\n"); + assert_eq!(databricks_host_from_config_path(&path), Ok(None)); + } + + #[test] + fn databricks_host_from_config_rejects_invalid_values() { + for contents in [ + "DATABRICKS_HOST: 42\n", + "DATABRICKS_HOST: not a url\n", + "DATABRICKS_HOST: http://example.databricks.com\n", + "DATABRICKS_HOST: https://user:pw@example.databricks.com\n", + "DATABRICKS_HOST: https://example.databricks.com?q=1\n", + "DATABRICKS_HOST: https://example.databricks.com/#frag\n", + "DATABRICKS_HOST: file:///etc/hosts\n", + "DATABRICKS_HOST: [unterminated\n", + ] { + let (_dir, path) = write_config(contents); + assert!( + databricks_host_from_config_path(&path).is_err(), + "expected error for {contents:?}" + ); + } + } + #[test] fn path_root_requires_an_absolute_path() { assert_eq!(validated_path_root(None), None); From 47890bbce2009cd922d358d2b247b62a12cde292 Mon Sep 17 00:00:00 2001 From: Philip Azar Date: Wed, 14 Oct 2026 10:50:02 -0500 Subject: [PATCH 2/4] fix(goose): defer to goose config DATABRICKS_HOST instead of pinning it Pinning the file's host into the env froze it at spawn, so later edits in settings were shadowed while the file-backed token changed, pairing a new token with the old host. Remove the env var instead so goose keeps re-reading its own config file; the user config.yaml is its last merged layer. Also skip the override when the child would resolve a different goose config file than Berd (shell-only GOOSE_PATH_ROOT, XDG_CONFIG_HOME, etc.). Co-Authored-By: Claude Opus 5.5 --- src-tauri/src/services/acp/goose_serve.rs | 117 ++++++++++++++-------- 1 file changed, 74 insertions(+), 43 deletions(-) diff --git a/src-tauri/src/services/acp/goose_serve.rs b/src-tauri/src/services/acp/goose_serve.rs index 335e7e2ed..51a08005a 100644 --- a/src-tauri/src/services/acp/goose_serve.rs +++ b/src-tauri/src/services/acp/goose_serve.rs @@ -229,16 +229,17 @@ impl GooseServeProcess { Err(error) => log::warn!("failed to load runtime config for goose serve env: {error}"), } // A valid DATABRICKS_HOST in the user's goose config overrides the - // shell and runtime config values. E2E runs use an isolated goose root, - // so the real user config must not leak in. + // shell and runtime config values. Only check when the child resolves + // the same config file as this process. E2E runs use an isolated goose + // root, so the real user config must not leak in. if app_handle .try_state::() .is_none() + && config_environment_matches_parent(&command, |name| std::env::var_os(name)) { apply_databricks_host_config_override( &mut command, goose_config::databricks_host_from_config(), - local_byo_key_providers_enabled(), ); } // This must be the final environment layer. Captured shell and runtime @@ -408,18 +409,46 @@ fn storage_environment_matches_parent( command: &Command, parent_value: impl Fn(&str) -> Option, ) -> bool { - [ - "GOOSE_PATH_ROOT", - "HOME", - "XDG_DATA_HOME", - "APPDATA", - "LOCALAPPDATA", - "USERPROFILE", - "HOMEDRIVE", - "HOMEPATH", - ] - .iter() - .all(|key| { + environment_matches_parent( + command, + &[ + "GOOSE_PATH_ROOT", + "HOME", + "XDG_DATA_HOME", + "APPDATA", + "LOCALAPPDATA", + "USERPROFILE", + "HOMEDRIVE", + "HOMEPATH", + ], + parent_value, + ) +} + +/// Whether the child resolves the same goose config file as this process. +fn config_environment_matches_parent( + command: &Command, + parent_value: impl Fn(&str) -> Option, +) -> bool { + environment_matches_parent( + command, + &[ + "GOOSE_PATH_ROOT", + "HOME", + "XDG_CONFIG_HOME", + "APPDATA", + "USERPROFILE", + ], + parent_value, + ) +} + +fn environment_matches_parent( + command: &Command, + keys: &[&str], + parent_value: impl Fn(&str) -> Option, +) -> bool { + keys.iter().all(|key| { let parent = parent_value(key); let child = command .as_std() @@ -1265,22 +1294,19 @@ fn apply_runtime_goose_provider_env(command: &mut Command, runtime_config: &Runt } } -/// Goose itself reads DATABRICKS_HOST from its config file, but only when the -/// env var is unset, so Berd's shell and runtime config layers would otherwise -/// shadow it. On a missing or invalid value the existing env is left as-is. -/// BYO-key dev keeps its stripped host. +/// Goose reads DATABRICKS_HOST from its config file only when the env var is +/// unset, so Berd's shell and runtime config layers would otherwise shadow it. +/// Removing the env var (rather than pinning the file's value) lets goose keep +/// re-reading the file, so settings edits apply live and stay paired with the +/// file-backed token. On a missing or invalid value the existing env is kept. fn apply_databricks_host_config_override( command: &mut Command, config_host: Result, String>, - byo_key_providers_enabled: bool, ) { - if byo_key_providers_enabled { - return; - } match config_host { - Ok(Some(host)) => { - log::info!("setting goose {DATABRICKS_HOST_ENV} from goose config"); - command.env(DATABRICKS_HOST_ENV, host); + Ok(Some(_)) => { + log::info!("using goose config {DATABRICKS_HOST_ENV} instead of env value"); + command.env_remove(DATABRICKS_HOST_ENV); } Ok(None) => {} Err(error) => log::warn!( @@ -1304,7 +1330,8 @@ mod tests { acp_websocket_url, add_release_webview_origin_arg, apply_databricks_host_config_override, apply_goose_search_paths_env, apply_runtime_goose_provider_env, apply_shell_env_with_extended_path, apply_shell_env_with_extended_path_inner, - storage_environment_matches_parent, DATABRICKS_HOST_ENV, TAURI_WEBVIEW_ORIGIN, + config_environment_matches_parent, storage_environment_matches_parent, DATABRICKS_HOST_ENV, + TAURI_WEBVIEW_ORIGIN, }; use crate::commands::runtime_config::default_runtime_config; use std::collections::HashMap; @@ -1572,19 +1599,19 @@ mod tests { } #[test] - fn databricks_host_config_override_replaces_runtime_host() { + fn databricks_host_config_override_defers_to_goose_config_file() { let mut command = command_with_runtime_host(); apply_databricks_host_config_override( &mut command, Ok(Some("https://local.databricks.com".to_string())), - false, ); - assert_eq!( - env_value(&command, DATABRICKS_HOST_ENV), - Some(OsString::from("https://local.databricks.com")) - ); + assert_eq!(env_value(&command, DATABRICKS_HOST_ENV), None); + assert!(command + .as_std() + .get_envs() + .any(|(key, value)| key == DATABRICKS_HOST_ENV && value.is_none())); } #[test] @@ -1592,7 +1619,7 @@ mod tests { for config_host in [Ok(None), Err("invalid".to_string())] { let mut command = command_with_runtime_host(); - apply_databricks_host_config_override(&mut command, config_host, false); + apply_databricks_host_config_override(&mut command, config_host); assert_eq!( env_value(&command, DATABRICKS_HOST_ENV), @@ -1602,16 +1629,20 @@ mod tests { } #[test] - fn databricks_host_config_override_is_skipped_for_byo_dev() { - let mut command = Command::new("goose"); - - apply_databricks_host_config_override( - &mut command, - Ok(Some("https://local.databricks.com".to_string())), - true, - ); + fn config_environment_guard_rejects_child_only_config_roots() { + for key in ["GOOSE_PATH_ROOT", "XDG_CONFIG_HOME"] { + let mut command = Command::new("goose"); + command.env(key, "/synthetic/child-root"); + assert!(!config_environment_matches_parent(&command, |_| None)); + assert!(config_environment_matches_parent(&command, |name| { + (name == key).then(|| OsString::from("/synthetic/child-root")) + })); + } - assert_eq!(env_value(&command, DATABRICKS_HOST_ENV), None); + assert!(config_environment_matches_parent( + &Command::new("goose"), + |_| None + )); } // Stock berd defaults declare no fastModelId (the release-time distribution From f07d75f8a6c64a025208790bf7c1155ef6de0204 Mon Sep 17 00:00:00 2001 From: Philip Azar Date: Wed, 14 Oct 2026 12:11:20 -0500 Subject: [PATCH 3/4] test(goose): use a neutral proxy host in DATABRICKS_HOST test Co-Authored-By: Claude Opus 5.5 (1M context) --- src-tauri/src/services/goose_config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src-tauri/src/services/goose_config.rs b/src-tauri/src/services/goose_config.rs index 3b85ba5e9..7bfe30999 100644 --- a/src-tauri/src/services/goose_config.rs +++ b/src-tauri/src/services/goose_config.rs @@ -201,7 +201,7 @@ mod tests { "https://example.databricks.com", "https://example.databricks.com/", "https://example.databricks.com:8443", - "https://litellm-proxy.sqprod.co/pass-through/databricks/", + "https://proxy.example.com/pass-through/databricks/", ] { let (_dir, path) = write_config(&format!( "GOOSE_PROVIDER: databricks\nDATABRICKS_HOST: {host}\n" From e0ff402dbf20312ede36b02c732355ade0cd25f7 Mon Sep 17 00:00:00 2001 From: Philip Azar Date: Wed, 14 Oct 2026 12:11:47 -0500 Subject: [PATCH 4/4] fix(search): tolerate WAL sidecars vanishing while securing cache permissions A closing SQLite connection can delete derived.sqlite-wal/-shm between the exists() check and set_permissions(), failing SearchIndex::open with ENOENT. This made reopening_after_offline_delete_cannot_preserve_prior_generation_rows flaky (~1 in 30 locally). Treat NotFound as success instead. Co-Authored-By: Claude Opus 5.5 (1M context) --- src-tauri/src/commands/message_search.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src-tauri/src/commands/message_search.rs b/src-tauri/src/commands/message_search.rs index b42c50b29..87cc15500 100644 --- a/src-tauri/src/commands/message_search.rs +++ b/src-tauri/src/commands/message_search.rs @@ -582,10 +582,13 @@ fn secure_cache_permissions(destination: &Path) -> Result<(), String> { for suffix in ["", "-wal", "-shm"] { let mut path = destination.as_os_str().to_os_string(); path.push(suffix); - let path = PathBuf::from(path); - if path.exists() { - std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600)) - .map_err(error_string)?; + // A closing connection may delete its -wal/-shm files at any time, + // so a missing sidecar is not an error. + match std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600)) { + Err(error) if error.kind() != std::io::ErrorKind::NotFound => { + return Err(error_string(error)); + } + _ => {} } } }