From 44115a67ceaead94f8d4eef79f40c35c9c432d52 Mon Sep 17 00:00:00 2001 From: Jack Amadeo Date: Tue, 23 Sep 2025 20:38:07 -0400 Subject: [PATCH] Don't load user's shell env on app startup (#4681) Co-authored-by: Michael Neale --- Cargo.lock | 1 + crates/goose-mcp/Cargo.toml | 1 + crates/goose-mcp/src/developer/shell.rs | 33 +++++++-------- crates/goose/src/config/base.rs | 10 +++-- crates/goose/src/config/mod.rs | 2 +- ui/desktop/src/main.ts | 56 ++++++++++--------------- ui/desktop/src/utils/loadEnv.ts | 38 ----------------- 7 files changed, 48 insertions(+), 93 deletions(-) delete mode 100644 ui/desktop/src/utils/loadEnv.ts diff --git a/Cargo.lock b/Cargo.lock index 22173c0445..b1de675077 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2816,6 +2816,7 @@ dependencies = [ "docx-rs", "etcetera", "glob", + "goose", "http-body-util", "hyper 1.6.0", "ignore", diff --git a/crates/goose-mcp/Cargo.toml b/crates/goose-mcp/Cargo.toml index 386e8cacf6..a728ad2be8 100644 --- a/crates/goose-mcp/Cargo.toml +++ b/crates/goose-mcp/Cargo.toml @@ -11,6 +11,7 @@ description.workspace = true workspace = true [dependencies] +goose = { path = "../goose" } mcp-core = { path = "../mcp-core" } mcp-server = { path = "../mcp-server" } rmcp = { version = "0.6.0", features = ["server", "client", "transport-io", "macros"] } diff --git a/crates/goose-mcp/src/developer/shell.rs b/crates/goose-mcp/src/developer/shell.rs index 699c9699aa..3350091bd9 100644 --- a/crates/goose-mcp/src/developer/shell.rs +++ b/crates/goose-mcp/src/developer/shell.rs @@ -1,4 +1,5 @@ -use std::{env, process::Stdio}; +use goose::config::get_config_dir; +use std::{env, ffi::OsString, process::Stdio}; #[cfg(unix)] #[allow(unused_imports)] // False positive: trait is used for process_group method @@ -8,30 +9,23 @@ use std::os::unix::process::CommandExt; pub struct ShellConfig { pub executable: String, pub args: Vec, + #[allow(dead_code)] + pub envs: Vec<(OsString, OsString)>, } impl Default for ShellConfig { fn default() -> Self { - if cfg!(windows) { - // Detect the default shell on Windows - #[cfg(windows)] - { - Self::detect_windows_shell() - } - #[cfg(not(windows))] - { - // This branch should never be taken on non-Windows - // but we need it for compilation - Self { - executable: "cmd".to_string(), - args: vec!["/c".to_string()], - } - } - } else { - // Use bash on Unix/macOS (keep existing behavior) + #[cfg(windows)] + { + Self::detect_windows_shell() + } + #[cfg(not(windows))] + { + let bash_env = get_config_dir().join(".bash_env").into_os_string(); Self { executable: "bash".to_string(), args: vec!["-c".to_string()], + envs: vec![(OsString::from("BASH_ENV"), bash_env)], } } } @@ -50,6 +44,7 @@ impl ShellConfig { "-NonInteractive".to_string(), "-Command".to_string(), ], + envs: vec![], } } else if let Ok(ps_path) = which::which("powershell") { // Windows PowerShell 5.1 @@ -60,12 +55,14 @@ impl ShellConfig { "-NonInteractive".to_string(), "-Command".to_string(), ], + envs: vec![], } } else { // Fall back to cmd.exe Self { executable: "cmd".to_string(), args: vec!["/c".to_string()], + envs: vec![], } } } diff --git a/crates/goose/src/config/base.rs b/crates/goose/src/config/base.rs index 92a6b07fde..52e7532e9c 100644 --- a/crates/goose/src/config/base.rs +++ b/crates/goose/src/config/base.rs @@ -116,14 +116,18 @@ enum SecretStorage { // Global instance static GLOBAL_CONFIG: OnceCell = OnceCell::new(); +pub fn get_config_dir() -> PathBuf { + choose_app_strategy(APP_STRATEGY.clone()) + .expect("goose requires a home dir") + .config_dir() +} + impl Default for Config { fn default() -> Self { // choose_app_strategy().config_dir() // - macOS/Linux: ~/.config/goose/ // - Windows: ~\AppData\Roaming\Block\goose\config\ - let config_dir = choose_app_strategy(APP_STRATEGY.clone()) - .expect("goose requires a home dir") - .config_dir(); + let config_dir = get_config_dir(); std::fs::create_dir_all(&config_dir).expect("Failed to create config directory"); diff --git a/crates/goose/src/config/mod.rs b/crates/goose/src/config/mod.rs index f060299a6d..7c009b879d 100644 --- a/crates/goose/src/config/mod.rs +++ b/crates/goose/src/config/mod.rs @@ -7,7 +7,7 @@ pub mod signup_openrouter; pub mod signup_tetrate; pub use crate::agents::ExtensionConfig; -pub use base::{Config, ConfigError, APP_STRATEGY}; +pub use base::{get_config_dir, Config, ConfigError, APP_STRATEGY}; pub use custom_providers::CustomProviderConfig; pub use experiments::ExperimentManager; pub use extensions::{ExtensionConfigManager, ExtensionEntry}; diff --git a/ui/desktop/src/main.ts b/ui/desktop/src/main.ts index d321cfcfbf..dacbb22b62 100644 --- a/ui/desktop/src/main.ts +++ b/ui/desktop/src/main.ts @@ -25,7 +25,6 @@ import { spawn } from 'child_process'; import 'dotenv/config'; import { startGoosed } from './goosed'; import { expandTilde, getBinaryPath } from './utils/pathUtils'; -import { loadShellEnv } from './utils/loadEnv'; import log from './utils/logger'; import { ensureWinShims } from './utils/winShims'; import { addRecentDir, loadRecentDirs } from './utils/recentDirs'; @@ -448,46 +447,37 @@ const parseArgs = () => { return { dirPath }; }; -const getGooseProvider = () => { - loadShellEnv(app.isPackaged); +interface BundledConfig { + defaultProvider?: string; + defaultModel?: string; + predefinedModels?: string; + baseUrlShare?: string; + version?: string; +} + +const getBundledConfig = (): BundledConfig => { //{env-macro-start}// //needed when goose is bundled for a specific provider //{env-macro-end}// - return [ - process.env.GOOSE_DEFAULT_PROVIDER, - process.env.GOOSE_DEFAULT_MODEL, - process.env.GOOSE_PREDEFINED_MODELS, - ]; + return { + defaultProvider: process.env.GOOSE_DEFAULT_PROVIDER, + defaultModel: process.env.GOOSE_DEFAULT_MODEL, + predefinedModels: process.env.GOOSE_PREDEFINED_MODELS, + baseUrlShare: process.env.GOOSE_BASE_URL_SHARE, + version: process.env.GOOSE_VERSION, + }; }; -const getSharingUrl = () => { - // checks app env for sharing url - loadShellEnv(app.isPackaged); // will try to take it from the zshrc file - // if GOOSE_BASE_URL_SHARE is found, we will set process.env.GOOSE_BASE_URL_SHARE, otherwise we return what it is set - // to in the env at bundle time - return process.env.GOOSE_BASE_URL_SHARE; -}; - -const getVersion = () => { - // checks app env for sharing url - loadShellEnv(app.isPackaged); // will try to take it from the zshrc file - // to in the env at bundle time - return process.env.GOOSE_VERSION; -}; - -const [provider, model, predefinedModels] = getGooseProvider(); - -const sharingUrl = getSharingUrl(); - -const gooseVersion = getVersion(); +const { defaultProvider, defaultModel, predefinedModels, baseUrlShare, version } = + getBundledConfig(); const SERVER_SECRET = process.env.GOOSE_EXTERNAL_BACKEND ? 'test' : crypto.randomBytes(32).toString('hex'); let appConfig = { - GOOSE_DEFAULT_PROVIDER: provider, - GOOSE_DEFAULT_MODEL: model, + GOOSE_DEFAULT_PROVIDER: defaultProvider, + GOOSE_DEFAULT_MODEL: defaultModel, GOOSE_PREDEFINED_MODELS: predefinedModels, GOOSE_API_HOST: 'http://127.0.0.1', GOOSE_PORT: 0, @@ -603,8 +593,8 @@ const createChat = async ( GOOSE_PORT: port, GOOSE_WORKING_DIR: working_dir, REQUEST_DIR: dir, - GOOSE_BASE_URL_SHARE: sharingUrl, - GOOSE_VERSION: gooseVersion, + GOOSE_BASE_URL_SHARE: baseUrlShare, + GOOSE_VERSION: version, recipe: recipe, }), ], @@ -1927,7 +1917,7 @@ async function appMain() { if (aboutGooseMenuItem.submenu) { aboutGooseMenuItem.submenu.append( new MenuItem({ - label: `Version ${gooseVersion || app.getVersion()}`, + label: `Version ${version || app.getVersion()}`, enabled: false, }) ); diff --git a/ui/desktop/src/utils/loadEnv.ts b/ui/desktop/src/utils/loadEnv.ts deleted file mode 100644 index 514bc4b72a..0000000000 --- a/ui/desktop/src/utils/loadEnv.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { execSync } from 'child_process'; -import log from './logger'; - -export function loadShellEnv(isProduction: boolean = false): void { - // Only proceed if running on macOS and in production mode - if (process.platform !== 'darwin' || !isProduction) { - log.info( - `Skipping zsh environment loading: ${ - process.platform !== 'darwin' ? 'Not running on macOS' : 'Not in production mode' - }` - ); - return; - } - - try { - log.info('LOADING ENV'); - - const shell = process.env.SHELL || '/bin/bash'; // Detect user's shell - - const envStr = execSync(`${shell} -l -i -c 'env'`, { - encoding: 'utf-8', - }); - - // Parse and set environment variables - envStr.split('\n').forEach((line) => { - const matches = line.match(/^([^=]+)=(.*)$/); - if (matches) { - const [, key, value] = matches; - log.info(`Setting ${key}`); - process.env[key] = value; - } - }); - - log.info('Successfully loaded zsh environment variables'); - } catch (error) { - log.error('Failed to load zsh environment variables:', error); - } -}