fix: resolve user login PATH for MCP extension subprocesses (#7896)
This commit is contained in:
@@ -1,3 +1,5 @@
|
||||
#[cfg(not(windows))]
|
||||
use crate::subprocess::merged_path;
|
||||
use crate::subprocess::SubprocessExt;
|
||||
#[cfg(target_os = "macos")]
|
||||
use base64::Engine;
|
||||
@@ -863,21 +865,24 @@ impl ComputerControllerServer {
|
||||
)
|
||||
})?
|
||||
}
|
||||
_ => Command::new(shell)
|
||||
.arg(shell_arg)
|
||||
.arg(&command)
|
||||
.env("GOOSE_TERMINAL", "1")
|
||||
.env("AGENT", "goose")
|
||||
.set_no_window()
|
||||
.output()
|
||||
.await
|
||||
.map_err(|e| {
|
||||
_ => {
|
||||
let mut cmd = Command::new(shell);
|
||||
cmd.arg(shell_arg)
|
||||
.arg(&command)
|
||||
.env("GOOSE_TERMINAL", "1")
|
||||
.env("AGENT", "goose");
|
||||
#[cfg(not(windows))]
|
||||
if let Some(path) = merged_path() {
|
||||
cmd.env("PATH", path);
|
||||
}
|
||||
cmd.set_no_window().output().await.map_err(|e| {
|
||||
ErrorData::new(
|
||||
ErrorCode::INTERNAL_ERROR,
|
||||
format!("Failed to run script: {}", e),
|
||||
None,
|
||||
)
|
||||
})?,
|
||||
})?
|
||||
}
|
||||
};
|
||||
|
||||
let output_str = String::from_utf8_lossy(&output.stdout).into_owned();
|
||||
@@ -1065,16 +1070,18 @@ impl ComputerControllerServer {
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
fn run_peekaboo_cmd(&self, args: &[&str]) -> Result<String, ErrorData> {
|
||||
let output = std::process::Command::new("peekaboo")
|
||||
.args(args)
|
||||
.output()
|
||||
.map_err(|e| {
|
||||
ErrorData::new(
|
||||
ErrorCode::INTERNAL_ERROR,
|
||||
format!("Failed to run peekaboo: {}", e),
|
||||
None,
|
||||
)
|
||||
})?;
|
||||
let mut cmd = std::process::Command::new("peekaboo");
|
||||
cmd.args(args);
|
||||
if let Some(path) = merged_path() {
|
||||
cmd.env("PATH", path);
|
||||
}
|
||||
let output = cmd.output().map_err(|e| {
|
||||
ErrorData::new(
|
||||
ErrorCode::INTERNAL_ERROR,
|
||||
format!("Failed to run peekaboo: {}", e),
|
||||
None,
|
||||
)
|
||||
})?;
|
||||
let stdout = String::from_utf8_lossy(&output.stdout).into_owned();
|
||||
let stderr = String::from_utf8_lossy(&output.stderr).into_owned();
|
||||
if !output.status.success() {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use std::sync::OnceLock;
|
||||
use tokio::process::Command;
|
||||
|
||||
#[cfg(windows)]
|
||||
@@ -27,3 +28,86 @@ impl SubprocessExt for std::process::Command {
|
||||
self
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve the user's full PATH by running a login shell.
|
||||
///
|
||||
/// When goosed is launched from a desktop app (e.g. Electron), it may inherit
|
||||
/// a minimal PATH like `/usr/bin:/bin`. This function spawns a login shell to
|
||||
/// source the user's profile and recover the full PATH.
|
||||
///
|
||||
/// Ported from `crates/goose/src/agents/platform_extensions/developer/shell.rs`
|
||||
/// where it was introduced in #5774 for the developer extension. This makes the
|
||||
/// same fix available to all MCP extensions in goose-mcp.
|
||||
#[cfg(not(windows))]
|
||||
fn resolve_login_shell_path() -> Option<String> {
|
||||
use std::path::PathBuf;
|
||||
use std::process::Stdio;
|
||||
|
||||
// Prefer the user's configured shell so we source the right profile files.
|
||||
// Fall back to /bin/bash (common default) then sh as last resort.
|
||||
let shell = std::env::var("SHELL")
|
||||
.ok()
|
||||
.filter(|s| PathBuf::from(s).is_file())
|
||||
.unwrap_or_else(|| {
|
||||
if PathBuf::from("/bin/bash").is_file() {
|
||||
"/bin/bash".to_string()
|
||||
} else {
|
||||
"sh".to_string()
|
||||
}
|
||||
});
|
||||
|
||||
std::process::Command::new(&shell)
|
||||
.args(["-l", "-i", "-c", "echo $PATH"])
|
||||
.stdin(Stdio::null())
|
||||
.stderr(Stdio::null())
|
||||
.output()
|
||||
.ok()
|
||||
.and_then(|output| {
|
||||
if output.status.success() {
|
||||
// Take the last non-empty line — interactive shells may emit
|
||||
// extra output from profile scripts before our echo.
|
||||
String::from_utf8_lossy(&output.stdout)
|
||||
.lines()
|
||||
.rev()
|
||||
.find(|line| !line.trim().is_empty())
|
||||
.map(|line| line.trim().to_string())
|
||||
.filter(|path| !path.is_empty())
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
/// Returns the user's full login shell PATH, resolved once and cached.
|
||||
///
|
||||
/// Call this before spawning subprocesses to ensure they inherit the user's
|
||||
/// full PATH rather than the restricted one from the desktop app launcher.
|
||||
#[cfg(not(windows))]
|
||||
pub fn user_login_path() -> Option<&'static str> {
|
||||
static CACHED: OnceLock<Option<String>> = OnceLock::new();
|
||||
CACHED.get_or_init(resolve_login_shell_path).as_deref()
|
||||
}
|
||||
|
||||
/// Merge the login shell PATH with the current process PATH.
|
||||
///
|
||||
/// Prepends login shell entries so user tools are found first, while
|
||||
/// preserving any runtime PATH additions (e.g. from direnv, nix, or
|
||||
/// auto-install helpers like ensure_peekaboo).
|
||||
#[cfg(not(windows))]
|
||||
pub fn merged_path() -> Option<String> {
|
||||
let login = user_login_path()?;
|
||||
let current = std::env::var("PATH").unwrap_or_default();
|
||||
if current.is_empty() {
|
||||
return Some(login.to_string());
|
||||
}
|
||||
// Deduplicate: login shell entries first, then any current entries not already present.
|
||||
let login_entries: Vec<&str> = login.split(':').collect();
|
||||
let mut seen: std::collections::HashSet<&str> = login_entries.iter().copied().collect();
|
||||
let mut merged = login_entries;
|
||||
for entry in current.split(':') {
|
||||
if seen.insert(entry) {
|
||||
merged.push(entry);
|
||||
}
|
||||
}
|
||||
Some(merged.join(":"))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user