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
11 changes: 7 additions & 4 deletions src-tauri/src/commands/message_search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
_ => {}
}
}
}
Expand Down
149 changes: 133 additions & 16 deletions src-tauri/src/services/acp/goose_serve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,20 @@ 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. 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::<crate::services::e2e_mode::E2eMode>()
.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(),
);
}
// 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.
Expand Down Expand Up @@ -395,18 +409,46 @@ fn storage_environment_matches_parent(
command: &Command,
parent_value: impl Fn(&str) -> Option<std::ffi::OsString>,
) -> 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<std::ffi::OsString>,
) -> 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<std::ffi::OsString>,
) -> bool {
keys.iter().all(|key| {
let parent = parent_value(key);
let child = command
.as_std()
Expand Down Expand Up @@ -1252,6 +1294,27 @@ fn apply_runtime_goose_provider_env(command: &mut Command, runtime_config: &Runt
}
}

/// 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<Option<String>, String>,
) {
match config_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!(
"ignoring goose config {DATABRICKS_HOST_ENV}; keeping existing value: {error}"
),
}
}

pub(crate) fn reserve_free_port() -> Result<u16, String> {
let listener = std::net::TcpListener::bind((LOCALHOST, 0))
.map_err(|error| format!("Failed to reserve Goose serve port: {error}"))?;
Expand All @@ -1264,10 +1327,11 @@ pub(crate) fn reserve_free_port() -> Result<u16, String> {
#[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,
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;
Expand Down Expand Up @@ -1528,6 +1592,59 @@ 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_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())),
);

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]
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);

assert_eq!(
env_value(&command, DATABRICKS_HOST_ENV),
Some(OsString::from("https://runtime.databricks.com"))
);
}
}

#[test]
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!(config_environment_matches_parent(
&Command::new("goose"),
|_| None
));
}

// Stock berd defaults declare no fastModelId (the release-time distribution
// injector supplies it), so a stock build exports no GOOSE_FAST_MODEL.
#[test]
Expand Down
107 changes: 107 additions & 0 deletions src-tauri/src/services/goose_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -68,6 +69,55 @@ fn data_dir_for_path_root(root: Option<OsString>) -> Result<PathBuf, String> {
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<Option<String>, String> {
databricks_host_from_config_path(&config_path()?)
}

fn databricks_host_from_config_path(path: &Path) -> Result<Option<String>, 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::<yaml_serde::Value>(&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<OsString>) -> Option<PathBuf> {
value.map(PathBuf::from).filter(|path| path.is_absolute())
}
Expand Down Expand Up @@ -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://proxy.example.com/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:[email protected]\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);
Expand Down
Loading