diff --git a/docs/ABSTRACTIONS.md b/docs/ABSTRACTIONS.md index 32444e1b..1214e16a 100644 --- a/docs/ABSTRACTIONS.md +++ b/docs/ABSTRACTIONS.md @@ -820,7 +820,7 @@ Tolaria delegates remote auth to the user's system git setup: - `CloneVaultModal` captures a remote URL and local destination - `clone_git_repo` and `create_getting_started_vault` both run system git clone work in blocking Tokio tasks so clone UIs stay responsive - On macOS, system-git commands prefer the user's login-shell `git` and `PATH`, and `git_add_remote` preflights HTTPS remotes through `git credential fill` so Keychain can prompt/grant access before the first fetch or push -- On Linux AppImage launches, every system-git command removes AppImage loader overrides such as `LD_LIBRARY_PATH`, `LD_PRELOAD`, and `GIT_EXEC_PATH` before spawning `git`, so helpers like `git-remote-https` bind against the host git/library stack instead of Tolaria's bundled WebKit/AppImage libraries +- On Linux AppImage launches, every system-git command and MCP Node subprocess removes AppImage loader overrides such as `LD_LIBRARY_PATH`, `LD_PRELOAD`, and `GIT_EXEC_PATH` before spawning, so helpers like `git-remote-https` and system `node` bind against the host library stack instead of Tolaria's bundled WebKit/AppImage libraries - On Linux AppImage Wayland launches, startup environment safeguards also set `GTK_IM_MODULE=fcitx` when common input-method variables already indicate fcitx and the user has not explicitly chosen a GTK IM module, allowing WebKitGTK editor input to reach fcitx5 on compositors such as niri without overriding deliberate `GTK_IM_MODULE` choices. - `git_add_remote` uses the same system git path and refuses remotes whose history is unrelated or ahead of the local vault - Existing `git_pull` / `git_push` commands keep surfacing raw git errors, and clone commands fail fast when git wants interactive terminal input diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index bd07441f..710291b3 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -539,7 +539,7 @@ Tolaria no longer implements provider-specific OAuth or remote-repository APIs. 1. User opens `CloneVaultModal` from onboarding or the vault menu 2. User pastes any git URL and chooses a local destination 3. The `clone_git_repo()` Tauri command runs `git clone` inside a blocking Tokio task so the Tauri window stays responsive during slow or failing clones -4. Linux AppImage builds strip AppImage loader variables from system-git subprocesses before spawning `git`, keeping `git-remote-https` on the host git/library stack +4. Linux AppImage builds strip AppImage loader variables from system-git and MCP Node subprocesses before spawning them, keeping `git-remote-https` and system `node` on the host library stack 5. `git_push()` / `git_pull()` continue to use the same system git path 6. On macOS, `git_add_remote()` asks Git's credential helper for HTTPS credentials before the first fetch so Keychain can grant access to the same saved credential item the shell uses 7. Clone commands disable interactive terminal / askpass prompts and surface the git failure back to the UI instead of freezing the app waiting for input diff --git a/docs/GETTING-STARTED.md b/docs/GETTING-STARTED.md index d051270f..c744ae27 100644 --- a/docs/GETTING-STARTED.md +++ b/docs/GETTING-STARTED.md @@ -69,7 +69,7 @@ pnpm playwright:regression # Full Playwright regression suite `create_getting_started_vault` clones the public starter repo and then removes every git remote from the new local copy. That means Getting Started vaults open local-only by default. Users connect a compatible remote later through the bottom-bar `No remote` chip or the command palette, both of which feed the same `AddRemoteModal` and `git_add_remote` backend flow. -Linux AppImage builds still use the user's system `git`. Before Tolaria spawns that `git` process, it removes AppImage loader overrides such as `LD_LIBRARY_PATH`, `LD_PRELOAD`, and `GIT_EXEC_PATH` so HTTPS clone helpers use the host git libraries instead of bundled AppImage libraries. +Linux AppImage builds still use the user's system `git` and `node`. Before Tolaria spawns those Git or MCP Node subprocesses, it removes AppImage loader overrides such as `LD_LIBRARY_PATH`, `LD_PRELOAD`, and `GIT_EXEC_PATH` so HTTPS clone helpers and MCP tooling use the host library stack instead of bundled AppImage libraries. ## Multiple Vaults At The Same Time diff --git a/src-tauri/src/mcp.rs b/src-tauri/src/mcp.rs index 355163d8..e9321dc5 100644 --- a/src-tauri/src/mcp.rs +++ b/src-tauri/src/mcp.rs @@ -3,6 +3,7 @@ use std::path::{Path, PathBuf}; use std::process::{Child, Command}; mod paths; +mod subprocess; const MCP_SERVER_NAME: &str = "tolaria"; const LEGACY_MCP_SERVER_NAME: &str = "laputa"; @@ -76,7 +77,7 @@ fn user_shell_candidates() -> Vec { } fn command_path_from_shell(shell: &Path, command: &str) -> Option { - crate::hidden_command(shell) + subprocess::command(shell) .arg("-lc") .arg(format!("command -v {command}")) .output() @@ -104,7 +105,7 @@ fn first_existing_path(stdout: &str) -> Option { } fn verify_node_version(node: &Path) -> Result<(), String> { - let output = crate::hidden_command(node) + let output = subprocess::command(node) .arg("--version") .output() .map_err(|e| format!("Failed to run {} --version: {e}", node.display()))?; @@ -143,9 +144,9 @@ fn node_major_version(version: &str) -> Option { fn node_lookup_command() -> Command { #[cfg(windows)] - let mut command = crate::hidden_command("where.exe"); + let mut command = subprocess::command("where.exe"); #[cfg(not(windows))] - let mut command = crate::hidden_command("which"); + let mut command = subprocess::command("which"); command.arg("node"); command @@ -397,7 +398,8 @@ pub fn spawn_ws_bridge_with_paths( let vault_path = vault_path.as_ref(); let active_vault_paths = active_vault_paths_json(vault_path, vault_paths); - let child = crate::hidden_command(node) + let mut command = subprocess::command(node); + let child = command .arg(&script) .env("VAULT_PATH", vault_path) .env("VAULT_PATHS", active_vault_paths) diff --git a/src-tauri/src/mcp/subprocess.rs b/src-tauri/src/mcp/subprocess.rs new file mode 100644 index 00000000..8fd171c4 --- /dev/null +++ b/src-tauri/src/mcp/subprocess.rs @@ -0,0 +1,78 @@ +use std::ffi::OsStr; +use std::process::Command; + +#[cfg(any(test, all(desktop, target_os = "linux")))] +const APPIMAGE_ENV_REMOVALS: [&str; 3] = ["LD_LIBRARY_PATH", "LD_PRELOAD", "GIT_EXEC_PATH"]; + +pub(super) fn command(program: impl AsRef) -> Command { + let mut command = crate::hidden_command(program); + sanitize_appimage_env(&mut command); + command +} + +#[cfg(all(desktop, target_os = "linux"))] +fn sanitize_appimage_env(command: &mut Command) { + sanitize_appimage_env_for_launch(command, appimage_env_present()); +} + +#[cfg(not(all(desktop, target_os = "linux")))] +fn sanitize_appimage_env(_command: &mut Command) {} + +#[cfg(any(test, all(desktop, target_os = "linux")))] +fn sanitize_appimage_env_for_launch(command: &mut Command, is_appimage: bool) { + if !is_appimage { + return; + } + + for key in APPIMAGE_ENV_REMOVALS { + command.env_remove(key); + } +} + +#[cfg(all(desktop, target_os = "linux"))] +fn appimage_env_present() -> bool { + ["APPIMAGE", "APPDIR"] + .into_iter() + .any(|key| std::env::var(key).is_ok_and(|value| !value.trim().is_empty())) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn command_envs(command: &Command) -> std::collections::HashMap> { + command + .get_envs() + .map(|(key, value)| { + ( + key.to_string_lossy().to_string(), + value.map(|entry| entry.to_string_lossy().to_string()), + ) + }) + .collect() + } + + #[test] + fn appimage_mcp_subprocesses_remove_loader_env() { + let mut command = crate::hidden_command("node"); + + sanitize_appimage_env_for_launch(&mut command, true); + + let envs = command_envs(&command); + for key in APPIMAGE_ENV_REMOVALS { + assert_eq!(envs.get(key), Some(&None)); + } + } + + #[test] + fn non_appimage_mcp_subprocesses_keep_parent_env_unmodified() { + let mut command = crate::hidden_command("node"); + + sanitize_appimage_env_for_launch(&mut command, false); + + let envs = command_envs(&command); + for key in APPIMAGE_ENV_REMOVALS { + assert!(!envs.contains_key(key)); + } + } +}