From f43870951f6b887c49dd196165f495d9dfcb5713 Mon Sep 17 00:00:00 2001 From: Jasper Date: Fri, 17 Jul 2026 22:49:32 +0200 Subject: [PATCH] fix(config): require absolute goose path roots (#10454) --- crates/goose-local-inference/src/paths.rs | 36 +++++++++++++++++++++-- crates/goose/src/config/paths.rs | 36 +++++++++++++++++++++-- crates/goose/src/plugins/discovery.rs | 6 ++-- ui/desktop/src/main.ts | 12 ++------ ui/desktop/src/utils/pathUtils.test.ts | 35 ++++++++++++++++++++++ ui/desktop/src/utils/pathUtils.ts | 32 ++++++++++++++++++++ 6 files changed, 140 insertions(+), 17 deletions(-) create mode 100644 ui/desktop/src/utils/pathUtils.test.ts diff --git a/crates/goose-local-inference/src/paths.rs b/crates/goose-local-inference/src/paths.rs index 4a79cbf30..6506cfb88 100644 --- a/crates/goose-local-inference/src/paths.rs +++ b/crates/goose-local-inference/src/paths.rs @@ -1,12 +1,12 @@ use etcetera::{choose_app_strategy, AppStrategy, AppStrategyArgs}; +use std::ffi::OsString; use std::path::PathBuf; pub struct Paths; impl Paths { fn get_dir(dir_type: DirType) -> PathBuf { - if let Ok(test_root) = std::env::var("GOOSE_PATH_ROOT") { - let base = PathBuf::from(test_root); + if let Some(base) = Self::path_root() { match dir_type { DirType::Config => base.join("config"), DirType::Data => base.join("data"), @@ -37,6 +37,14 @@ impl Paths { } } + fn path_root() -> Option { + Self::validated_path_root(std::env::var_os("GOOSE_PATH_ROOT")) + } + + fn validated_path_root(value: Option) -> Option { + value.map(PathBuf::from).filter(|path| path.is_absolute()) + } + pub fn config_dir() -> PathBuf { Self::get_dir(DirType::Config) } @@ -86,3 +94,27 @@ enum DirType { Agents, AgentsHome, } + +#[cfg(test)] +mod tests { + use super::Paths; + use std::ffi::OsString; + + #[test] + fn path_root_requires_an_absolute_path() { + assert_eq!(Paths::validated_path_root(None), None); + assert_eq!(Paths::validated_path_root(Some(OsString::new())), None); + assert_eq!( + Paths::validated_path_root(Some(OsString::from("relative/root"))), + None + ); + + let absolute = std::env::current_dir() + .unwrap() + .join("nonexistent-goose-root"); + assert_eq!( + Paths::validated_path_root(Some(absolute.clone().into_os_string())), + Some(absolute) + ); + } +} diff --git a/crates/goose/src/config/paths.rs b/crates/goose/src/config/paths.rs index 4a79cbf30..f7b0646c3 100644 --- a/crates/goose/src/config/paths.rs +++ b/crates/goose/src/config/paths.rs @@ -1,12 +1,12 @@ use etcetera::{choose_app_strategy, AppStrategy, AppStrategyArgs}; +use std::ffi::OsString; use std::path::PathBuf; pub struct Paths; impl Paths { fn get_dir(dir_type: DirType) -> PathBuf { - if let Ok(test_root) = std::env::var("GOOSE_PATH_ROOT") { - let base = PathBuf::from(test_root); + if let Some(base) = Self::path_root() { match dir_type { DirType::Config => base.join("config"), DirType::Data => base.join("data"), @@ -37,6 +37,14 @@ impl Paths { } } + pub(crate) fn path_root() -> Option { + Self::validated_path_root(std::env::var_os("GOOSE_PATH_ROOT")) + } + + fn validated_path_root(value: Option) -> Option { + value.map(PathBuf::from).filter(|path| path.is_absolute()) + } + pub fn config_dir() -> PathBuf { Self::get_dir(DirType::Config) } @@ -86,3 +94,27 @@ enum DirType { Agents, AgentsHome, } + +#[cfg(test)] +mod tests { + use super::Paths; + use std::ffi::OsString; + + #[test] + fn path_root_requires_an_absolute_path() { + assert_eq!(Paths::validated_path_root(None), None); + assert_eq!(Paths::validated_path_root(Some(OsString::new())), None); + assert_eq!( + Paths::validated_path_root(Some(OsString::from("relative/root"))), + None + ); + + let absolute = std::env::current_dir() + .unwrap() + .join("nonexistent-goose-root"); + assert_eq!( + Paths::validated_path_root(Some(absolute.clone().into_os_string())), + Some(absolute) + ); + } +} diff --git a/crates/goose/src/plugins/discovery.rs b/crates/goose/src/plugins/discovery.rs index cdd80a652..90b89e4bf 100644 --- a/crates/goose/src/plugins/discovery.rs +++ b/crates/goose/src/plugins/discovery.rs @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf}; use serde::{Deserialize, Serialize}; -use crate::config::Config; +use crate::config::{paths::Paths, Config}; use crate::plugins::plugin_install_dir; const PLUGINS_CONFIG_KEY: &str = "plugins"; @@ -192,9 +192,9 @@ fn load_all_settings(project_root: Option<&Path>) -> Vec<(SettingsScope, PluginS } fn user_settings_path() -> Option { - if let Ok(test_root) = std::env::var("GOOSE_PATH_ROOT") { + if let Some(path_root) = Paths::path_root() { return Some( - PathBuf::from(test_root) + path_root .join(".config") .join("goose") .join("settings.json"), diff --git a/ui/desktop/src/main.ts b/ui/desktop/src/main.ts index d15ee8013..b866a6d6d 100644 --- a/ui/desktop/src/main.ts +++ b/ui/desktop/src/main.ts @@ -30,7 +30,7 @@ import { checkBackendStatus } from './backendStatus'; import { startGooseServe } from './gooseServe'; import { GooseServeLeaseRegistry, type GooseServeLease } from './gooseServeLeaseRegistry'; import { acpWebSocketUrlFromHttpBase, normalizeAcpHttpBaseUrl } from './acp/url'; -import { expandTilde } from './utils/pathUtils'; +import { expandTilde, sanitizeGoosePathRoot } from './utils/pathUtils'; import log from './utils/logger'; import { ensureWinShims } from './utils/winShims'; import { addRecentDir, loadRecentDirs } from './utils/recentDirs'; @@ -861,14 +861,6 @@ const getBundledConfig = (): BundledConfig => { const { defaultProvider, defaultModel, predefinedModels, version } = getBundledConfig(); -const resolveGoosePathRoot = (): string | undefined => { - const pathRoot = process.env.GOOSE_PATH_ROOT?.trim(); - if (pathRoot) { - return expandTilde(pathRoot); - } - return undefined; -}; - const GENERATED_SECRET = crypto.randomBytes(32).toString('hex'); interface ExternalBackend { @@ -954,7 +946,7 @@ let appConfig = { GOOSE_DEFAULT_PROVIDER: defaultProvider, GOOSE_DEFAULT_MODEL: defaultModel, GOOSE_PREDEFINED_MODELS: predefinedModels, - GOOSE_PATH_ROOT: resolveGoosePathRoot(), + GOOSE_PATH_ROOT: sanitizeGoosePathRoot(process.env), GOOSE_WORKING_DIR: '', // Start with the env-var override; the OS region locale is filled in after app.ready // (see updateLocaleFromSystem below) since getSystemLocale() cannot be called earlier. diff --git a/ui/desktop/src/utils/pathUtils.test.ts b/ui/desktop/src/utils/pathUtils.test.ts new file mode 100644 index 000000000..367890b27 --- /dev/null +++ b/ui/desktop/src/utils/pathUtils.test.ts @@ -0,0 +1,35 @@ +import os from 'node:os'; +import path from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { isAbsoluteGoosePath, resolveGoosePathRoot, sanitizeGoosePathRoot } from './pathUtils'; + +describe('resolveGoosePathRoot', () => { + it('rejects empty and relative values', () => { + expect(resolveGoosePathRoot(undefined)).toBeUndefined(); + expect(resolveGoosePathRoot(' ')).toBeUndefined(); + expect(resolveGoosePathRoot('relative/root')).toBeUndefined(); + }); + + it('retains absolute paths without requiring them to exist', () => { + const absolute = path.resolve('nonexistent-goose-root'); + expect(resolveGoosePathRoot(` ${absolute} `)).toBe(absolute); + }); + + it('expands a home-relative root before validation', () => { + expect(resolveGoosePathRoot('~')).toBe(os.homedir()); + }); + + it('removes a rejected value from the child-process environment', () => { + const env = { GOOSE_PATH_ROOT: 'relative/root' }; + expect(sanitizeGoosePathRoot(env)).toBeUndefined(); + expect(env).not.toHaveProperty('GOOSE_PATH_ROOT'); + }); + + it('matches Rust absolute-path handling on Windows', () => { + expect(isAbsoluteGoosePath('C:\\goose\\root', 'win32')).toBe(true); + expect(isAbsoluteGoosePath('\\\\server\\share\\goose', 'win32')).toBe(true); + expect(isAbsoluteGoosePath('C:goose\\root', 'win32')).toBe(false); + expect(isAbsoluteGoosePath('\\goose\\root', 'win32')).toBe(false); + expect(isAbsoluteGoosePath('/goose/root', 'win32')).toBe(false); + }); +}); diff --git a/ui/desktop/src/utils/pathUtils.ts b/ui/desktop/src/utils/pathUtils.ts index 51af4abe4..7b3b686b8 100644 --- a/ui/desktop/src/utils/pathUtils.ts +++ b/ui/desktop/src/utils/pathUtils.ts @@ -23,3 +23,35 @@ export function expandTilde(filePath: string): string { } return filePath; } + +export function resolveGoosePathRoot(value: string | undefined): string | undefined { + const trimmed = value?.trim(); + if (!trimmed) { + return undefined; + } + + const expanded = expandTilde(trimmed); + return isAbsoluteGoosePath(expanded) ? expanded : undefined; +} + +export function isAbsoluteGoosePath( + filePath: string, + platform: 'win32' | 'posix' = process.platform === 'win32' ? 'win32' : 'posix' +): boolean { + if (platform !== 'win32') { + return path.posix.isAbsolute(filePath); + } + + const root = path.win32.parse(filePath).root; + return path.win32.isAbsolute(filePath) && root.length > 1; +} + +export function sanitizeGoosePathRoot(env: { GOOSE_PATH_ROOT?: string }): string | undefined { + const pathRoot = resolveGoosePathRoot(env.GOOSE_PATH_ROOT); + if (pathRoot) { + env.GOOSE_PATH_ROOT = pathRoot; + } else { + delete env.GOOSE_PATH_ROOT; + } + return pathRoot; +}